vhost-net: cleanup host notifiers at last step

When the vhost notifier is disabled, the userspace handler runs
immediately: virtio_pci_set_host_notifier_internal might
call virtio_queue_notify_vq.
Since the VQ state and the tap backend state aren't
recovered yet, this causes
"Guest moved used index from XXX to YYY" assertions.

The solution is to split out host notifier handling
from vhost VQ setup and disable notifiers as our last step
when we stop vhost-net. For symmetry enable them first thing
on start.

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

diff --git a/hw/vhost.c b/hw/vhost.c
index 18860678ba059ecf70c69caecb16e82b42ab7f9f..0870cb7d8590db00fdc5ec880386665adc4b1a9e 100644 (file)
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -515,11 +515,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
     };
     struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
 
-    if (!vdev->binding->set_host_notifier) {
-        fprintf(stderr, "binding does not support host notifiers\n");
-        return -ENOSYS;
-    }
-
     vq->num = state.num = virtio_queue_get_num(vdev, idx);
     r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state);
     if (r) {
@@ -567,12 +562,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
         r = -errno;
         goto fail_alloc;
     }
-    r = vdev->binding->set_host_notifier(vdev->binding_opaque, idx, true);
-    if (r < 0) {
-        fprintf(stderr, "Error binding host notifier: %d\n", -r);
-        goto fail_host_notifier;
-    }
-
     file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
     r = ioctl(dev->control, VHOST_SET_VRING_KICK, &file);
     if (r) {
@@ -591,8 +580,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
 
 fail_call:
 fail_kick:
-    vdev->binding->set_host_notifier(vdev->binding_opaque, idx, false);
-fail_host_notifier:
 fail_alloc:
     cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx),
                               0, 0);
@@ -618,12 +605,6 @@ static void vhost_virtqueue_cleanup(struct vhost_dev *dev,
         .index = idx,
     };
     int r;
-    r = vdev->binding->set_host_notifier(vdev->binding_opaque, idx, false);
-    if (r < 0) {
-        fprintf(stderr, "vhost VQ %d host cleanup failed: %d\n", idx, r);
-        fflush(stderr);
-    }
-    assert (r >= 0);
     r = ioctl(dev->control, VHOST_GET_VRING_BASE, &state);
     if (r < 0) {
         fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
@@ -697,6 +678,60 @@ bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev)
         hdev->force;
 }
 
+/* Stop processing guest IO notifications in qemu.
+ * Start processing them in vhost in kernel.
+ */
+int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
+{
+    int i, r;
+    if (!vdev->binding->set_host_notifier) {
+        fprintf(stderr, "binding does not support host notifiers\n");
+        r = -ENOSYS;
+        goto fail;
+    }
+
+    for (i = 0; i < hdev->nvqs; ++i) {
+        r = vdev->binding->set_host_notifier(vdev->binding_opaque, i, true);
+        if (r < 0) {
+            fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
+            goto fail_vq;
+        }
+    }
+
+    return 0;
+fail_vq:
+    while (--i >= 0) {
+        r = vdev->binding->set_host_notifier(vdev->binding_opaque, i, false);
+        if (r < 0) {
+            fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
+            fflush(stderr);
+        }
+        assert (r >= 0);
+    }
+fail:
+    return r;
+}
+
+/* Stop processing guest IO notifications in vhost.
+ * Start processing them in qemu.
+ * This might actually run the qemu handlers right away,
+ * so virtio in qemu must be completely setup when this is called.
+ */
+void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
+{
+    int i, r;
+
+    for (i = 0; i < hdev->nvqs; ++i) {
+        r = vdev->binding->set_host_notifier(vdev->binding_opaque, i, false);
+        if (r < 0) {
+            fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
+            fflush(stderr);
+        }
+        assert (r >= 0);
+    }
+}
+
+/* Host notifiers must be enabled at this point. */
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
     int i, r;
@@ -762,6 +797,7 @@ fail:
     return r;
 }
 
+/* Host notifiers must be enabled at this point. */
 void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
     int i, r;

diff --git a/hw/vhost.h b/hw/vhost.h
index c8c595a147d48f36b67795d4f33d735005d5a7ca..c9452f073297f96725635d1cf5fa244e56a3a417 100644 (file)
--- a/hw/vhost.h
+++ b/hw/vhost.h
@@ -46,5 +46,7 @@ void vhost_dev_cleanup(struct vhost_dev *hdev);
 bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev);
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
 void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
+int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
+void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
 
 #endif

diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index a55981200d5cf6c1fd71b5dd3e533e7bb4cfd1ce..950a6b8d990389702164e7ceb7b0565d93f1a2c6 100644 (file)
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -139,16 +139,22 @@ int vhost_net_start(struct vhost_net *net,
 {
     struct vhost_vring_file file = { };
     int r;
+
+    net->dev.nvqs = 2;
+    net->dev.vqs = net->vqs;
+
+    r = vhost_dev_enable_notifiers(&net->dev, dev);
+    if (r < 0) {
+        goto fail_notifiers;
+    }
     if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
         tap_set_vnet_hdr_len(net->vc,
                              sizeof(struct virtio_net_hdr_mrg_rxbuf));
     }
 
-    net->dev.nvqs = 2;
-    net->dev.vqs = net->vqs;
     r = vhost_dev_start(&net->dev, dev);
     if (r < 0) {
-        return r;
+        goto fail_start;
     }
 
     net->vc->info->poll(net->vc, false);
@@ -173,6 +179,9 @@ fail:
     if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
         tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr));
     }
+fail_start:
+    vhost_dev_disable_notifiers(&net->dev, dev);
+fail_notifiers:
     return r;
 }
 
@@ -190,6 +199,7 @@ void vhost_net_stop(struct vhost_net *net,
     if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
         tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr));
     }
+    vhost_dev_disable_notifiers(&net->dev, dev);
 }
 
 void vhost_net_cleanup(struct vhost_net *net)