Re: [PATCH 04/31] vdpa: Add vhost_svq_set_svq_kick_fd

[Jason Wang]

在 2022/1/22 上午4:27, Eugenio Pérez 写道:
> This function allows the vhost-vdpa backend to override kick_fd.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   hw/virtio/vhost-shadow-virtqueue.h |  1 +
>   hw/virtio/vhost-shadow-virtqueue.c | 45 ++++++++++++++++++++++++++++++
>   2 files changed, 46 insertions(+)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
> b/hw/virtio/vhost-shadow-virtqueue.h
> index 400effd9f2..a56ecfc09d 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -15,6 +15,7 @@
>   
>   typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
>   
> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
>   const EventNotifier *vhost_svq_get_dev_kick_notifier(
>                                                 const VhostShadowVirtqueue 
> *svq);
>   
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index bd87110073..21534bc94d 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -11,6 +11,7 @@
>   #include "hw/virtio/vhost-shadow-virtqueue.h"
>   
>   #include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
>   
>   /* Shadow virtqueue to relay notifications */
>   typedef struct VhostShadowVirtqueue {
> @@ -18,8 +19,20 @@ typedef struct VhostShadowVirtqueue {
>       EventNotifier hdev_kick;
>       /* Shadow call notifier, sent to vhost */
>       EventNotifier hdev_call;
> +
> +    /*
> +     * Borrowed virtqueue's guest to host notifier.
> +     * To borrow it in this event notifier allows to register on the event
> +     * loop and access the associated shadow virtqueue easily. If we use the
> +     * VirtQueue, we don't have an easy way to retrieve it.
> +     *
> +     * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
> +     */
> +    EventNotifier svq_kick;
>   } VhostShadowVirtqueue;
>   
> +#define INVALID_SVQ_KICK_FD -1
> +
>   /**
>    * The notifier that SVQ will use to notify the device.
>    */
> @@ -29,6 +42,35 @@ const EventNotifier *vhost_svq_get_dev_kick_notifier(
>       return &svq->hdev_kick;
>   }
>   
> +/**
> + * Set a new file descriptor for the guest to kick SVQ and notify for avail
> + *
> + * @svq          The svq
> + * @svq_kick_fd  The new svq kick fd
> + */
> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
> +{
> +    EventNotifier tmp;
> +    bool check_old = INVALID_SVQ_KICK_FD !=
> +                     event_notifier_get_fd(&svq->svq_kick);
> +
> +    if (check_old) {
> +        event_notifier_set_handler(&svq->svq_kick, NULL);
> +        event_notifier_init_fd(&tmp, event_notifier_get_fd(&svq->svq_kick));
> +    }


It looks to me we don't do similar things in vhost-net. Any reason for 
caring about the old svq_kick?


> +
> +    /*
> +     * event_notifier_set_handler already checks for guest's notifications if
> +     * they arrive to the new file descriptor in the switch, so there is no
> +     * need to explicitely check for them.
> +     */
> +    event_notifier_init_fd(&svq->svq_kick, svq_kick_fd);
> +
> +    if (!check_old || event_notifier_test_and_clear(&tmp)) {
> +        event_notifier_set(&svq->hdev_kick);


Any reason we need to kick the device directly here?

Thanks


> +    }
> +}
> +
>   /**
>    * Creates vhost shadow virtqueue, and instruct vhost device to use the shadow
>    * methods and file descriptors.
> @@ -52,6 +94,9 @@ VhostShadowVirtqueue *vhost_svq_new(void)
>           goto err_init_hdev_call;
>       }
>   
> +    /* Placeholder descriptor, it should be deleted at set_kick_fd */
> +    event_notifier_init_fd(&svq->svq_kick, INVALID_SVQ_KICK_FD);
> +
>       return g_steal_pointer(&svq);
>   
>   err_init_hdev_call: