[PATCHv2] qemu-kvm: fix crash on reboot with vhost-net

[PATCHv2] qemu-kvm: fix crash on reboot with vhost-net

[Michael S. Tsirkin]

When vhost-net is disabled on reboot, we set msix mask notifier
to NULL to disable further mask/unmask notifications.
Code currently tries to pass this NULL to notifier,
leading to a crash.  The right thing to do is
to add explicit APIs to enable/disable notifications.
Now when disabling notifications:
- if vector is masked, we don't need to notify backend,
  just disable future notifications
- if vector is unmasked, invoke callback to unassign backend,
  then disable future notifications

This patch also polls notifier before closing it,
to make sure we don't lose events if poll callback
didn't have time to run.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Changes from v1:
	Separate APIs to set and unset notifiers
	Test and clear notifier before destroying it

 hw/msix.c       |   40 +++++++++++++++++++++++++++++++++++-----
 hw/msix.h       |    1 +
 hw/virtio-pci.c |    7 +++++--
 3 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 3ec8805..8f9a621 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -609,14 +609,44 @@ void msix_unuse_all_vectors(PCIDevice *dev)
 
 int msix_set_mask_notifier(PCIDevice *dev, unsigned vector, void *opaque)
 {
+    int r;
+    if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector])
+        return 0;
+
+    assert(dev->msix_mask_notifier);
+    assert(opaque);
+    assert(!dev->msix_mask_notifier_opaque[vector]);
+
+    if (msix_is_masked(dev, vector)) {
+        return 0;
+    }
+    r = dev->msix_mask_notifier(dev, vector, opaque,
+                                msix_is_masked(dev, vector));
+    if (r < 0) {
+        return r;
+    }
+    dev->msix_mask_notifier_opaque[vector] = opaque;
+    return r;
+}
+
+int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector)
+{
     int r = 0;
     if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector])
         return 0;
 
-    if (dev->msix_mask_notifier)
-        r = dev->msix_mask_notifier(dev, vector, opaque,
-                                    msix_is_masked(dev, vector));
-    if (r >= 0)
-        dev->msix_mask_notifier_opaque[vector] = opaque;
+    assert(dev->msix_mask_notifier);
+    assert(dev->msix_mask_notifier_opaque[vector]);
+
+    if (msix_is_masked(dev, vector)) {
+        return 0;
+    }
+    r = dev->msix_mask_notifier(dev, vector,
+                                dev->msix_mask_notifier_opaque[vector],
+                                msix_is_masked(dev, vector));
+    if (r < 0) {
+        return r;
+    }
+    dev->msix_mask_notifier_opaque[vector] = NULL;
     return r;
 }
diff --git a/hw/msix.h b/hw/msix.h
index f167231..6b21ffb 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -34,4 +34,5 @@ void msix_reset(PCIDevice *dev);
 extern int msix_supported;
 
 int msix_set_mask_notifier(PCIDevice *dev, unsigned vector, void *opaque);
+int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector);
 #endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 99a588c..c4bc633 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -462,10 +462,13 @@ static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign)
         msix_set_mask_notifier(&proxy->pci_dev,
                                virtio_queue_vector(proxy->vdev, n), vq);
     } else {
-        msix_set_mask_notifier(&proxy->pci_dev,
-                               virtio_queue_vector(proxy->vdev, n), NULL);
+        msix_unset_mask_notifier(&proxy->pci_dev,
+				 virtio_queue_vector(proxy->vdev, n));
         qemu_set_fd_handler(event_notifier_get_fd(notifier),
                             NULL, NULL, NULL);
+        /* Test and clear notifier before closing it,
+         * in case poll callback didn't have time to run. */
+        virtio_pci_guest_notifier_read(vq);
         event_notifier_cleanup(notifier);
     }
 
-- 
1.7.1.rc1.22.g3163

Re: [PATCHv2] qemu-kvm: fix crash on reboot with vhost-net

[Juan Quintela]

"Michael S. Tsirkin" <mst@redhat.com> wrote:
> When vhost-net is disabled on reboot, we set msix mask notifier
> to NULL to disable further mask/unmask notifications.
> Code currently tries to pass this NULL to notifier,
> leading to a crash.  The right thing to do is
> to add explicit APIs to enable/disable notifications.
> Now when disabling notifications:
> - if vector is masked, we don't need to notify backend,
>   just disable future notifications
> - if vector is unmasked, invoke callback to unassign backend,
>   then disable future notifications
>
> This patch also polls notifier before closing it,
> to make sure we don't lose events if poll callback
> didn't have time to run.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Changes from v1:
> 	Separate APIs to set and unset notifiers
> 	Test and clear notifier before destroying it
>
>  hw/msix.c       |   40 +++++++++++++++++++++++++++++++++++-----
>  hw/msix.h       |    1 +
>  hw/virtio-pci.c |    7 +++++--
>  3 files changed, 41 insertions(+), 7 deletions(-)

Acked-by: Juan Quintela <quintela@redhat.com>

This patch addresses the issues that I pointed in previous mail and
fixes the problem at hand.

Creating a better API for interaction between virtio-pci <-> msix still
needs more work/thought, already discussed that with mst.

Later, Juan.

Re: [PATCHv2] qemu-kvm: fix crash on reboot with vhost-net

[Marcelo Tosatti]

On Wed, Apr 28, 2010 at 12:27:38PM +0300, Michael S. Tsirkin wrote:
> When vhost-net is disabled on reboot, we set msix mask notifier
> to NULL to disable further mask/unmask notifications.
> Code currently tries to pass this NULL to notifier,
> leading to a crash.  The right thing to do is
> to add explicit APIs to enable/disable notifications.
> Now when disabling notifications:
> - if vector is masked, we don't need to notify backend,
>   just disable future notifications
> - if vector is unmasked, invoke callback to unassign backend,
>   then disable future notifications
> 
> This patch also polls notifier before closing it,
> to make sure we don't lose events if poll callback
> didn't have time to run.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Applied, thanks.

Re: [PATCHv2] qemu-kvm: fix crash on reboot with vhost-net

[Michael S. Tsirkin]

On Wed, Apr 28, 2010 at 02:43:05PM -0300, Marcelo Tosatti wrote:
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Applied, thanks.

Can you tell me what commit id it has pls (for backport to rhel6).