Virtualization - Jan 2022 - [PATCH 11/31] vhost: Add vhost_svq_valid_device_features to shadow vq

? 2022/1/22 ??4:27, Eugenio P?rez ??:
> This allows SVQ to negotiate features with the device. For the device,
> SVQ is a driver. While this function needs to bypass all non-transport
> features, it needs to disable the features that SVQ does not support
> when forwarding buffers. This includes packed vq layout, indirect
> descriptors or event idx.
>
> Signed-off-by: Eugenio P?rez <eperezma at redhat.com>
> ---
>   hw/virtio/vhost-shadow-virtqueue.h |  2 ++
>   hw/virtio/vhost-shadow-virtqueue.c | 44 ++++++++++++++++++++++++++++++
>   hw/virtio/vhost-vdpa.c             | 21 ++++++++++++++
>   3 files changed, 67 insertions(+)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
> index c9ffa11fce..d963867a04 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -15,6 +15,8 @@
>   
>   typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
>   
> +bool vhost_svq_valid_device_features(uint64_t *features);
> +
>   void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int
svq_kick_fd);
>   void vhost_svq_set_guest_call_notifier(VhostShadowVirtqueue *svq, int
call_fd);
>   const EventNotifier *vhost_svq_get_dev_kick_notifier(
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
> index 9619c8082c..51442b3dbf 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -45,6 +45,50 @@ const EventNotifier *vhost_svq_get_dev_kick_notifier(
>       return &svq->hdev_kick;
>   }
>   
> +/**
> + * Validate the transport device features that SVQ can use with the device
> + *
> + * @dev_features  The device features. If success, the acknowledged
features.
> + *
> + * Returns true if SVQ can go with a subset of these, false otherwise.
> + */
> +bool vhost_svq_valid_device_features(uint64_t *dev_features)
> +{
> +    bool r = true;
> +
> +    for (uint64_t b = VIRTIO_TRANSPORT_F_START; b <=
VIRTIO_TRANSPORT_F_END;
> +         ++b) {
> +        switch (b) {
> +        case VIRTIO_F_NOTIFY_ON_EMPTY:
> +        case VIRTIO_F_ANY_LAYOUT:
> +            continue;
> +
> +        case VIRTIO_F_ACCESS_PLATFORM:
> +            /* SVQ does not know how to translate addresses */

I may miss something but any reason that we need to disable 
ACCESS_PLATFORM? I'd expect the vring helper we used for shadow 
virtqueue can deal with vIOMMU perfectly.

> +            if (*dev_features & BIT_ULL(b)) {
> +                clear_bit(b, dev_features);
> +                r = false;
> +            }
> +            break;
> +
> +        case VIRTIO_F_VERSION_1:

I had the same question here.

Thanks

> +            /* SVQ trust that guest vring is little endian */
> +            if (!(*dev_features & BIT_ULL(b))) {
> +                set_bit(b, dev_features);
> +                r = false;
> +            }
> +            continue;
> +
> +        default:
> +            if (*dev_features & BIT_ULL(b)) {
> +                clear_bit(b, dev_features);
> +            }
> +        }
> +    }
> +
> +    return r;
> +}
> +
>   /* Forward guest notifications */
>   static void vhost_handle_guest_kick(EventNotifier *n)
>   {
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index bdb45c8808..9d801cf907 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -855,10 +855,31 @@ static int vhost_vdpa_init_svq(struct vhost_dev
*hdev, struct vhost_vdpa *v,
>       size_t n_svqs = v->shadow_vqs_enabled ? hdev->nvqs : 0;
>       g_autoptr(GPtrArray) shadow_vqs = g_ptr_array_new_full(n_svqs,
>                                                             
vhost_psvq_free);
> +    uint64_t dev_features;
> +    uint64_t svq_features;
> +    int r;
> +    bool ok;
> +
>       if (!v->shadow_vqs_enabled) {
>           goto out;
>       }
>   
> +    r = vhost_vdpa_get_features(hdev, &dev_features);
> +    if (r != 0) {
> +        error_setg(errp, "Can't get vdpa device features, got
(%d)", r);
> +        return r;
> +    }
> +
> +    svq_features = dev_features;
> +    ok = vhost_svq_valid_device_features(&svq_features);
> +    if (unlikely(!ok)) {
> +        error_setg(errp,
> +            "SVQ Invalid device feature flags, offer:
0x%"PRIx64", ok: 0x%"PRIx64,
> +            hdev->features, svq_features);
> +        return -1;
> +    }
> +
> +    shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_psvq_free);
>       for (unsigned n = 0; n < hdev->nvqs; ++n) {
>           VhostShadowVirtqueue *svq = vhost_svq_new();
>