[Qemu-devel] [PATCH] virtio: always use handle_aio_output if registered

Paolo Bonzini posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170228132132.20231-1-pbonzini@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/virtio/virtio.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] virtio: always use handle_aio_output if registered
Posted by Paolo Bonzini 7 years, 1 month ago
Commit ad07cd6 ("virtio-scsi: always use dataplane path if ioeventfd is
active", 2016-10-30) and 9ffe337 ("virtio-blk: always use dataplane
path if ioeventfd is active", 2016-10-30) broke the virtio 1.0
indirect access registers.

The indirect access registers bypass the ioeventfd, so that virtio-blk
and virtio-scsi now repeatedly try to initialize dataplane instead of
triggering the guest->host EventNotifier.  Detect the situation by
checking vq->handle_aio_output; if it is not NULL, trigger the
EventNotifier, which is how the device expects to get notifications
and in fact the only thread-safe manner to deliver them.

Fixes: ad07cd6
Fixes: 9ffe337
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 23483c7..1206f6c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1458,7 +1458,18 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
 
 void virtio_queue_notify(VirtIODevice *vdev, int n)
 {
-    virtio_queue_notify_vq(&vdev->vq[n]);
+    VirtQueue *vq = &vdev->vq[n];
+
+    if (unlikely(!vq->vring.desc || vdev->broken)) {
+        return;
+    }
+
+    trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
+    if (vq->handle_aio_output) {
+        event_notifier_set(&vq->host_notifier);
+    } else if (vq->handle_output) {
+        vq->handle_output(vdev, vq);
+    }
 }
 
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n)
-- 
2.9.3


Re: [Qemu-devel] [Qemu-stable] [PATCH] virtio: always use handle_aio_output if registered
Posted by Michael Roth 7 years ago
Quoting Paolo Bonzini (2017-02-28 07:21:32)
> Commit ad07cd6 ("virtio-scsi: always use dataplane path if ioeventfd is
> active", 2016-10-30) and 9ffe337 ("virtio-blk: always use dataplane
> path if ioeventfd is active", 2016-10-30) broke the virtio 1.0
> indirect access registers.
> 
> The indirect access registers bypass the ioeventfd, so that virtio-blk
> and virtio-scsi now repeatedly try to initialize dataplane instead of
> triggering the guest->host EventNotifier.  Detect the situation by
> checking vq->handle_aio_output; if it is not NULL, trigger the
> EventNotifier, which is how the device expects to get notifications
> and in fact the only thread-safe manner to deliver them.
> 
> Fixes: ad07cd6
> Fixes: 9ffe337
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Ping for 2.9 / 2.8.1

> ---
>  hw/virtio/virtio.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 23483c7..1206f6c 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1458,7 +1458,18 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
> 
>  void virtio_queue_notify(VirtIODevice *vdev, int n)
>  {
> -    virtio_queue_notify_vq(&vdev->vq[n]);
> +    VirtQueue *vq = &vdev->vq[n];
> +
> +    if (unlikely(!vq->vring.desc || vdev->broken)) {
> +        return;
> +    }
> +
> +    trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
> +    if (vq->handle_aio_output) {
> +        event_notifier_set(&vq->host_notifier);
> +    } else if (vq->handle_output) {
> +        vq->handle_output(vdev, vq);
> +    }
>  }
> 
>  uint16_t virtio_queue_vector(VirtIODevice *vdev, int n)
> -- 
> 2.9.3
> 
> 


Re: [Qemu-devel] [PATCH] virtio: always use handle_aio_output if registered
Posted by Stefan Hajnoczi 7 years ago
On Tue, Feb 28, 2017 at 02:21:32PM +0100, Paolo Bonzini wrote:
> Commit ad07cd6 ("virtio-scsi: always use dataplane path if ioeventfd is
> active", 2016-10-30) and 9ffe337 ("virtio-blk: always use dataplane
> path if ioeventfd is active", 2016-10-30) broke the virtio 1.0
> indirect access registers.
> 
> The indirect access registers bypass the ioeventfd, so that virtio-blk
> and virtio-scsi now repeatedly try to initialize dataplane instead of
> triggering the guest->host EventNotifier.  Detect the situation by
> checking vq->handle_aio_output; if it is not NULL, trigger the
> EventNotifier, which is how the device expects to get notifications
> and in fact the only thread-safe manner to deliver them.
> 
> Fixes: ad07cd6
> Fixes: 9ffe337
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio/virtio.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>