Re: [PATCH RFC v3 07/13] vhost: add support for negotiating extended features

[Jason Wang <jasowang@redhat.com>]

On Fri, Jul 18, 2025 at 4:53 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Similar to virtio infra, vhost core maintains the features status
> in the full extended format and allows the devices to implement
> extended version of the getter/setter.
>
> Note that 'protocol_features' are not extended: they are only
> used by vhost-user, and the latter device is not going to implement
> extended features soon.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v2 -> v3:
>   - fix compile warning
>   - _array -> _ex
>
> v1 -> v2:
>   - uint128_t -> uint64_t[]
>   - add _ex() variant of features manipulation helpers
> ---
>  hw/virtio/vhost.c                 | 73 +++++++++++++++++++++++++++----
>  include/hw/virtio/vhost-backend.h |  6 +++
>  include/hw/virtio/vhost.h         | 33 ++++++++++++--
>  3 files changed, 100 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index c30ea1156e..85ae1e4d4c 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
>  static int vhost_dev_set_features(struct vhost_dev *dev,
>                                    bool enable_log)
>  {
> -    uint64_t features = dev->acked_features;
> +    uint64_t features[VIRTIO_FEATURES_DWORDS];
>      int r;
> +
> +    virtio_features_copy(features, dev->acked_features_ex);
>      if (enable_log) {
> -        features |= 0x1ULL << VHOST_F_LOG_ALL;
> +        virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
>      }
>      if (!vhost_dev_has_iommu(dev)) {
> -        features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
> +        virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
>      }
>      if (dev->vhost_ops->vhost_force_iommu) {
>          if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
> -            features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
> +            virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
>         }
>      }
> -    r = dev->vhost_ops->vhost_set_features(dev, features);
> +
> +    if (virtio_features_use_extended(features) &&
> +        !dev->vhost_ops->vhost_set_features_ex) {
> +        VHOST_OPS_DEBUG(r, "extended features without device support");
> +        r = -EINVAL;
> +        goto out;
> +    }
> +
> +    if (dev->vhost_ops->vhost_set_features_ex) {
> +        r = dev->vhost_ops->vhost_set_features_ex(dev, features);
> +    } else {
> +        r = dev->vhost_ops->vhost_set_features(dev, features[0]);
> +    }
>      if (r < 0) {
>          VHOST_OPS_DEBUG(r, "vhost_set_features failed");
>          goto out;
> @@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
>      }
>  }
>
> +static int vhost_dev_get_features(struct vhost_dev *hdev,
> +                                  uint64_t *features)
> +{
> +    uint64_t features64;
> +    int r;
> +
> +    if (hdev->vhost_ops->vhost_get_features_ex) {
> +        return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
> +    }
> +
> +    r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
> +    virtio_features_from_u64(features, features64);
> +    return r;
> +}

Nit: let's have a vhost_dev_set_features() as well?

Thanks