qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Ladi Prosek <lprosek@redhat.com>
To: qemu-devel@nongnu.org
Cc: geoff@hostfission.com, pbonzini@redhat.com, armbru@redhat.com,
	marcandre.lureau@redhat.com
Subject: [Qemu-devel] [PATCH 2/3] ivshmem: Always remove irqfd notifiers
Date: Fri, 10 Nov 2017 18:34:20 +0100	[thread overview]
Message-ID: <20171110173421.17904-3-lprosek@redhat.com> (raw)
In-Reply-To: <20171110173421.17904-1-lprosek@redhat.com>

As of commit 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications"),
QEMU crashes with:

ivshmem: msix_set_vector_notifiers failed
msix_unset_vector_notifiers: Assertion `dev->msix_vector_use_notifier && dev->msix_vector_release_notifier' failed.

if MSI-X is repeatedly enabled and disabled on the ivshmem device, for example
by loading and unloading the Windows ivshmem driver. This is because
msix_unset_vector_notifiers() doesn't call any of the release notifier callbacks
since MSI-X is already disabled at that point (msix_enabled() returning false
is how this transition is detected in the first place). Thus ivshmem_vector_mask()
doesn't run and when MSI-X is subsequently enabled again ivshmem_vector_unmask()
fails.

This is fixed by keeping track of unmasked vectors and making sure that
ivshmem_vector_mask() always runs on MSI-X disable.

Fixes: 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications")
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 hw/misc/ivshmem.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 6e46669744..493a5030a1 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -77,6 +77,7 @@ typedef struct Peer {
 typedef struct MSIVector {
     PCIDevice *pdev;
     int virq;
+    bool unmasked;
 } MSIVector;
 
 typedef struct IVShmemState {
@@ -321,6 +322,7 @@ static int ivshmem_vector_unmask(PCIDevice *dev, unsigned vector,
         error_report("ivshmem: vector %d route does not exist", vector);
         return -EINVAL;
     }
+    assert(!v->unmasked);
 
     ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg, dev);
     if (ret < 0) {
@@ -328,7 +330,11 @@ static int ivshmem_vector_unmask(PCIDevice *dev, unsigned vector,
     }
     kvm_irqchip_commit_routes(kvm_state);
 
-    return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, v->virq);
+    ret = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, v->virq);
+    if (ret == 0) {
+        v->unmasked = true;
+    }
+    return ret;
 }
 
 static void ivshmem_vector_mask(PCIDevice *dev, unsigned vector)
@@ -343,9 +349,12 @@ static void ivshmem_vector_mask(PCIDevice *dev, unsigned vector)
         error_report("ivshmem: vector %d route does not exist", vector);
         return;
     }
+    assert(v->unmasked);
 
     ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, v->virq);
-    if (ret != 0) {
+    if (ret == 0) {
+        v->unmasked = false;
+    } else {
         error_report("remove_irqfd_notifier_gsi failed");
     }
 }
@@ -817,11 +826,20 @@ static void ivshmem_disable_irqfd(IVShmemState *s)
     PCIDevice *pdev = PCI_DEVICE(s);
     int i;
 
-    for (i = 0; i < s->peers[s->vm_id].nb_eventfds; i++) {
-        ivshmem_remove_kvm_msi_virq(s, i);
-    }
-
     msix_unset_vector_notifiers(pdev);
+
+    for (i = 0; i < s->peers[s->vm_id].nb_eventfds; i++) {
+        /*
+         * MSI-X is already disabled here so msix_unset_vector_notifiers
+         * didn't call our release notifier. Do it now to keep our masks and
+         * unmasks balanced.
+         */
+        if (s->msi_vectors[i].unmasked) {
+            ivshmem_vector_mask(pdev, i);
+        }
+        ivshmem_remove_kvm_msi_virq(s, i);
+    }
+
 }
 
 static void ivshmem_write_config(PCIDevice *pdev, uint32_t address,
-- 
2.13.5

  parent reply	other threads:[~2017-11-10 17:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-10 17:34 [Qemu-devel] [PATCH 0/3] ivshmem: MSI bug fixes Ladi Prosek
2017-11-10 17:34 ` [Qemu-devel] [PATCH 1/3] ivshmem: Don't update non-existent MSI routes Ladi Prosek
2017-11-10 18:10   ` Marc-André Lureau
2017-11-10 20:04   ` geoff
2017-11-13 14:22   ` Markus Armbruster
2017-11-13 14:38     ` Ladi Prosek
2017-11-13 17:26       ` Markus Armbruster
2017-11-10 17:34 ` Ladi Prosek [this message]
2017-11-13 14:36   ` [Qemu-devel] [PATCH 2/3] ivshmem: Always remove irqfd notifiers Markus Armbruster
2017-11-13 14:40     ` Ladi Prosek
2017-11-10 17:34 ` [Qemu-devel] [PATCH 3/3] ivshmem: Improve MSI irqfd error handling Ladi Prosek
2017-11-13 17:27   ` Markus Armbruster
2017-11-13 17:47     ` geoff
2017-11-14  7:37       ` Markus Armbruster

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=20171110173421.17904-3-lprosek@redhat.com \
    --to=lprosek@redhat.com \
    --cc=armbru@redhat.com \
    --cc=geoff@hostfission.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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).