On 2025/07/24 4:31, Paolo Abeni wrote:
> The virtio specifications allows for up to 128 bits for the
> device features. Soon we are going to use some of the 'extended'
> bits features for the virtio net driver.
>
> Add support to allow extended features negotiation on a per
> devices basis. Devices willing to negotiated extended features
> need to implemented a new pair of features getter/setter, the
> core will conditionally use them instead of the basic one.
>
> Note that 'bad_features' don't need to be extended, as they are
> bound to the 64 bits limit.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v3 -> v4:
> - use new virtio_features macro names
>
> v2 -> v3:
> - _array -> _ex
>
> v1 -> v2:
> - uint128_t -> uint64_t[]
> ---
> hw/virtio/virtio-bus.c | 11 ++++++++---
> hw/virtio/virtio.c | 18 +++++++++++++-----
> include/hw/virtio/virtio.h | 4 ++++
> 3 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 11adfbf3ab..cef944e015 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -62,9 +62,14 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
> }
>
> /* Get the features of the plugged device. */
> - assert(vdc->get_features != NULL);
> - vdev->host_features = vdc->get_features(vdev, vdev->host_features,
> - &local_err);
> + if (vdc->get_features_ex) {
> + vdc->get_features_ex(vdev, vdev->host_features_ex, &local_err);
> + } else {
> + assert(vdc->get_features != NULL);
> + virtio_features_from_u64(vdev->host_features_ex,
> + vdc->get_features(vdev, vdev->host_features,
> + &local_err));
> + }
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index c6896e000c..a7c695f633 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3093,7 +3093,9 @@ static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val)
> bad = virtio_features_andnot(tmp, val, vdev->host_features_ex);
> virtio_features_and(tmp, val, vdev->host_features_ex);
>
> - if (k->set_features) {
> + if (k->set_features_ex) {
> + k->set_features_ex(vdev, val);
> + } else if (k->set_features) {
> bad = bad || virtio_features_use_ex(tmp);
> k->set_features(vdev, tmp[0]);
> }
> @@ -3136,9 +3138,8 @@ virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev,
> }
> }
>
> -int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> +int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
> {
> - uint64_t features[VIRTIO_FEATURES_NU64S];
> int ret;
> /*
> * The driver must not attempt to set features after feature negotiation
> @@ -3148,13 +3149,12 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> return -EINVAL;
> }
>
> - if (val & (1ull << VIRTIO_F_BAD_FEATURE)) {
> + if (features[0] & (1ull << VIRTIO_F_BAD_FEATURE)) {
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s: guest driver for %s has enabled UNUSED(30) feature bit!\n",
> __func__, vdev->name);
> }
>
> - virtio_features_from_u64(features, val);
> ret = virtio_set_features_nocheck(vdev, features);
> if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
> /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
> @@ -3174,6 +3174,14 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> return ret;
> }
>
> +int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> +{
> + uint64_t features[VIRTIO_FEATURES_NU64S];
> +
> + virtio_features_from_u64(features, val);
> + return virtio_set_features_ex(vdev, features);
> +}
> +
Nitpick: this patch will be smaller by a few lines if you move
virtio_set_features() before virtio_set_features_ex(). It will also
align the order of these functions with the header file.
> void virtio_reset(void *opaque)
> {
> VirtIODevice *vdev = opaque;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 39e4059a66..2aeb021fb3 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -178,6 +178,9 @@ struct VirtioDeviceClass {
> /* This is what a VirtioDevice must implement */
> DeviceRealize realize;
> DeviceUnrealize unrealize;
> + void (*get_features_ex)(VirtIODevice *vdev, uint64_t *requested_features,
> + Error **errp);
> + void (*set_features_ex)(VirtIODevice *vdev, const uint64_t *val);
> uint64_t (*get_features)(VirtIODevice *vdev,
> uint64_t requested_features,
> Error **errp);
> @@ -373,6 +376,7 @@ void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);
> void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index);
> void virtio_update_irq(VirtIODevice *vdev);
> int virtio_set_features(VirtIODevice *vdev, uint64_t val);
> +int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *val);
>
> /* Base devices. */
> typedef struct VirtIOBlkConf VirtIOBlkConf;