qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Ladi Prosek <lprosek@redhat.com>
To: qemu-devel@nongnu.org
Cc: mst@redhat.com, vrozenfe@redhat.com, lprosek@redhat.com
Subject: [Qemu-devel] [RFC PATCH] virtio: set ISR status when delivering queue MSI
Date: Thu,  5 May 2016 11:13:37 +0200	[thread overview]
Message-ID: <1462439617-11387-1-git-send-email-lprosek@redhat.com> (raw)

There is a discrepancy between dataplane and no-dataplane virtio
behavior with respect to the ISR status register and MSI-X
capability.

Without dataplane the Queue interrupt ISR status bit is set
regardless of how the notification is delivered to the guest.

With dataplane the Queue interrupt ISR status bit is set only
if the notification is delivered as an IRQ.

While both conform to the spec, QEMU should be consistent to
minimize surprises with broken drivers.

This RFC patch shows the relevant code location and contains a
possible fix which makes QEMU set the bit on the MSI dataplane
code path.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 hw/virtio/virtio.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 08275a9..4053313 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1192,7 +1192,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
     }
 
     trace_virtio_notify(vdev, vq);
-    vdev->isr |= 0x01;
+    vdev->isr |= 0x01; // <= here we set ISR status even if we don't raise IRQ
     virtio_notify_vector(vdev, vq->vector);
 }
 
@@ -1757,16 +1757,25 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
     }
 }
 
+static void virtio_queue_guest_notifier_read_irqfd(EventNotifier *n)
+{
+    VirtQueue *vq = container_of(n, VirtQueue, guest_notifier);
+    if (event_notifier_test_and_clear(n)) {
+        vq->vdev->isr |= 0x01;
+    }
+}
+
 void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                 bool with_irqfd)
 {
-    if (assign && !with_irqfd) {
+    if (assign) {
         event_notifier_set_handler(&vq->guest_notifier,
-                                   virtio_queue_guest_notifier_read);
+             with_irqfd ?
+                virtio_queue_guest_notifier_read_irqfd :
+                virtio_queue_guest_notifier_read);
     } else {
         event_notifier_set_handler(&vq->guest_notifier, NULL);
-    }
-    if (!assign) {
+
         /* Test and clear notifier before closing it,
          * in case poll callback didn't have time to run. */
         virtio_queue_guest_notifier_read(&vq->guest_notifier);
-- 
2.5.5

             reply	other threads:[~2016-05-05  9:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-05  9:13 Ladi Prosek [this message]
2016-05-05 13:36 ` [Qemu-devel] [RFC PATCH] virtio: set ISR status when delivering queue MSI Michael S. Tsirkin
2016-05-05 14:59   ` Ladi Prosek
2016-05-05 15:02     ` Michael S. Tsirkin
2016-05-05 15:15       ` Ladi Prosek
2016-05-05 15:20         ` Michael S. Tsirkin
2016-05-05 15:27           ` Ladi Prosek
2016-05-05 16:04             ` Michael S. Tsirkin
2016-05-05 16:29               ` Ladi Prosek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1462439617-11387-1-git-send-email-lprosek@redhat.com \
    --to=lprosek@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vrozenfe@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).