Re: [PATCH v2] virtio: notify virtqueue via host notifier when available

[Michael S. Tsirkin]

On Tue, Nov 05, 2019 at 03:09:46PM +0100, Stefan Hajnoczi wrote:
> Host notifiers are used in several cases:
> 1. Traditional ioeventfd where virtqueue notifications are handled in
>    the main loop thread.
> 2. IOThreads (aio_handle_output) where virtqueue notifications are
>    handled in an IOThread AioContext.
> 3. vhost where virtqueue notifications are handled by kernel vhost or
>    a vhost-user device backend.
> 
> Most virtqueue notifications from the guest use the ioeventfd mechanism,
> but there are corner cases where QEMU code calls virtio_queue_notify().
> This currently honors the host notifier for the IOThreads
> aio_handle_output case, but not for the vhost case.  The result is that
> vhost does not receive virtqueue notifications from QEMU when
> virtio_queue_notify() is called.
> 
> This patch extends virtio_queue_notify() to set the host notifier
> whenever it is enabled instead of calling the vq->(aio_)handle_output()
> function directly.  We track the host notifier state for each virtqueue
> separately since some devices may use it only for certain virtqueues.
> 
> This fixes the vhost case although it does add a trip through the
> eventfd for the traditional ioeventfd case.  I don't think it's worth
> adding a fast path for the traditional ioeventfd case because calling
> virtio_queue_notify() is rare when ioeventfd is enabled.
> 
> Reported-by: Felipe Franciosi <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>

queued, thanks!

> ---
> v2:
>  * Track host notifier enabled/disabled state per virtqueue [Yongji Xie]
>  * Tested with contrib/vhost-user-blk and contrib/vhost-user-scsi
> 
>  hw/virtio/virtio-bus.c     | 4 ++++
>  hw/virtio/virtio.c         | 9 ++++++++-
>  include/hw/virtio/virtio.h | 1 +
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index b2c804292e..d6332d45c3 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -288,6 +288,10 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, 
> int n, bool assign)
>          k->ioeventfd_assign(proxy, notifier, n, false);
>      }
>  
> +    if (r == 0) {
> +        virtio_queue_set_host_notifier_enabled(vq, assign);
> +    }
> +
>      return r;
>  }
>  
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 762df12f4c..04716b5f6c 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -128,6 +128,7 @@ struct VirtQueue
>      VirtIODevice *vdev;
>      EventNotifier guest_notifier;
>      EventNotifier host_notifier;
> +    bool host_notifier_enabled;
>      QLIST_ENTRY(VirtQueue) node;
>  };
>  
> @@ -2271,7 +2272,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
>      }
>  
>      trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
> -    if (vq->handle_aio_output) {
> +    if (vq->host_notifier_enabled) {
>          event_notifier_set(&vq->host_notifier);
>      } else if (vq->handle_output) {
>          vq->handle_output(vdev, vq);
> @@ -3145,6 +3146,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
>          vdev->vq[i].vector = VIRTIO_NO_VECTOR;
>          vdev->vq[i].vdev = vdev;
>          vdev->vq[i].queue_index = i;
> +        vdev->vq[i].host_notifier_enabled = false;
>      }
>  
>      vdev->name = name;
> @@ -3436,6 +3438,11 @@ EventNotifier 
> *virtio_queue_get_host_notifier(VirtQueue *vq)
>      return &vq->host_notifier;
>  }
>  
> +void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
> +{
> +    vq->host_notifier_enabled = enabled;
> +}
> +
>  int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
>                                        MemoryRegion *mr, bool assign)
>  {
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 3448d67d2a..c32a815303 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -312,6 +312,7 @@ int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
>  void virtio_device_release_ioeventfd(VirtIODevice *vdev);
>  bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
>  EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> +void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
>  void virtio_queue_host_notifier_read(EventNotifier *n);
>  void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext 
> *ctx,
>                                                  VirtIOHandleAIOOutput 
> handle_output);
> -- 
> 2.23.0