[Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers

[Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers

[Cornelia Huck]

As discussed in the virtio-blk dataplane start/stop thread, the
various transports contain a lot of similar/identical boilerplate
code for setting up ioeventfds. This makes the code hard to follow.

Let's drag all of the common handling into the virtio-bus layer,
hopefully reducing both the boilerplate code and the number of
indirections.

This RFC patchset is the result of hacking this up quickly, and I
was rather surprised that it did not die immediately on my s390
system (which is all I 'tested'). So be warned :)

Feedback about the approach and the interface would be good.

Cornelia Huck (6):
  virtio-bus: common ioeventfd infrastructure
  virtio-bus: have callers tolerate new host notifier api
  virtio-ccw: convert to ioeventfd callbacks
  virtio-pci: convert to ioeventfd callbacks
  virtio-mmio: convert to ioeventfd callbacks
  virtio-bus: remove old set_host_notifier callback

 hw/block/dataplane/virtio-blk.c |   6 +-
 hw/s390x/virtio-ccw.c           | 133 ++++++++++++++--------------------------
 hw/scsi/virtio-scsi-dataplane.c |   9 ++-
 hw/virtio/vhost.c               |  13 ++--
 hw/virtio/virtio-bus.c          | 108 ++++++++++++++++++++++++++++++++
 hw/virtio/virtio-mmio.c         | 128 +++++++++++++-------------------------
 hw/virtio/virtio-pci.c          | 124 +++++++++++++------------------------
 include/hw/virtio/virtio-bus.h  |  15 ++++-
 8 files changed, 263 insertions(+), 273 deletions(-)

-- 
2.6.5

[Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure

[Cornelia Huck]

Introduce a set of ioeventfd callbacks on the virtio-bus level
that can be implemented by the individual transports. At the
virtio-bus level, do common handling for host notifiers (which
is actually most of it).

Two things of note:
- We always iterate over all possible virtio queues, even though
ccw (currently) has a lower limit. It does not really matter in
this place.
- We allow for the virtio-bus caller to pass an "assign" argument
down when stopping ioeventfd, which the old interface did not allow.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-bus.h |  14 ++++++
 2 files changed, 122 insertions(+)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 574f0e2..501300f 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -146,6 +146,114 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
     }
 }
 
+static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
+                                      int n, bool assign, bool set_handler)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
+    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+    int r = 0;
+
+    if (assign) {
+        r = event_notifier_init(notifier, 1);
+        if (r < 0) {
+            error_report("%s: unable to init event notifier: %d", __func__, r);
+            return r;
+        }
+        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
+        r = k->ioeventfd_assign(proxy, notifier, n, assign);
+        if (r < 0) {
+            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
+            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+            event_notifier_cleanup(notifier);
+            return r;
+        }
+    } else {
+        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+        k->ioeventfd_assign(proxy, notifier, n, assign);
+        event_notifier_cleanup(notifier);
+    }
+    return r;
+}
+
+void virtio_bus_start_ioeventfd(VirtioBusState *bus)
+{
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    DeviceState *proxy = DEVICE(BUS(bus)->parent);
+    VirtIODevice *vdev;
+    int n, r;
+
+    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
+        return;
+    }
+    if (!k->ioeventfd_disabled(proxy)) {
+        return;
+    }
+    vdev = virtio_bus_get_device(bus);
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+        r = set_host_notifier_internal(proxy, bus, n, true, true);
+        if (r < 0) {
+            goto assign_error;
+        }
+    }
+    k->ioeventfd_set_started(proxy, true, false);
+    return;
+
+assign_error:
+    while (--n >= 0) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = set_host_notifier_internal(proxy, bus, n, false, false);
+        assert(r >= 0);
+    }
+    k->ioeventfd_set_started(proxy, false, true);
+    error_report("%s: failed. Fallback to userspace (slower).", __func__);
+}
+
+void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
+{
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    DeviceState *proxy = DEVICE(BUS(bus)->parent);
+    VirtIODevice *vdev;
+    int n, r;
+
+    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
+        return;
+    }
+    vdev = virtio_bus_get_device(bus);
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+        r = set_host_notifier_internal(proxy, bus, n, assign, false);
+        assert(r >= 0);
+    }
+    k->ioeventfd_set_started(proxy, false, false);
+}
+
+int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
+{
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    DeviceState *proxy = DEVICE(BUS(bus)->parent);
+
+    if (!k->ioeventfd_started) {
+        return -ENOSYS;
+    }
+    /* Stop using the generic ioeventfd, we are doing eventfd handling
+     * ourselves below */
+    k->ioeventfd_set_disabled(proxy, assign);
+    if (assign) {
+        virtio_bus_stop_ioeventfd(bus, assign);
+    }
+    return set_host_notifier_internal(proxy, bus, n, assign, false);
+}
+
 static char *virtio_bus_get_dev_path(DeviceState *dev)
 {
     BusState *bus = qdev_get_parent_bus(dev);
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 3f2c136..0281cbf 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
     void (*device_unplugged)(DeviceState *d);
     int (*query_nvectors)(DeviceState *d);
     /*
+     * ioeventfd handling: if the transport implements ioeventfd_started,
+     * it must implement the other ioeventfd callbacks as well
+     */
+    bool (*ioeventfd_started)(DeviceState *d);
+    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
+    bool (*ioeventfd_disabled)(DeviceState *d);
+    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
+    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
+                            int n, bool assign);
+    /*
      * Does the transport have variable vring alignment?
      * (ie can it ever call virtio_queue_set_align()?)
      * Note that changing this will break migration for this transport.
@@ -111,4 +121,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
     return (VirtIODevice *)qdev;
 }
 
+void virtio_bus_start_ioeventfd(VirtioBusState *bus);
+void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
+int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
+
 #endif /* VIRTIO_BUS_H */
-- 
2.6.5

[Qemu-devel] [PATCH RFC 2/6] virtio-bus: have callers tolerate new host notifier api

[Cornelia Huck]

Have vhost and dataplane use the new api for transports that
have been converted.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/block/dataplane/virtio-blk.c | 14 +++++++++++---
 hw/scsi/virtio-scsi-dataplane.c | 20 +++++++++++++++-----
 hw/virtio/vhost.c               | 20 ++++++++++++++++----
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 36f3d2b..9ca41d7 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -132,7 +132,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
     }
 
     /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers || !k->set_host_notifier) {
+    if (!k->set_guest_notifiers ||
+        (!k->set_host_notifier && !k->ioeventfd_started)) {
         error_setg(errp,
                    "device is incompatible with dataplane "
                    "(transport does not support notifiers)");
@@ -209,7 +210,10 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     s->guest_notifier = virtio_queue_get_guest_notifier(s->vq);
 
     /* Set up virtqueue notify */
-    r = k->set_host_notifier(qbus->parent, 0, true);
+    r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, true);
+    if (r == -ENOSYS) {
+        r = k->set_host_notifier(qbus->parent, 0, true);
+    }
     if (r != 0) {
         fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
         goto fail_host_notifier;
@@ -244,6 +248,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
+    int r;
 
     if (!vblk->dataplane_started || s->stopping) {
         return;
@@ -268,7 +273,10 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
 
     aio_context_release(s->ctx);
 
-    k->set_host_notifier(qbus->parent, 0, false);
+    r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
+    if (r == -ENOSYS) {
+        k->set_host_notifier(qbus->parent, 0, false);
+    }
 
     /* Clean up guest notifier (irq) */
     k->set_guest_notifiers(qbus->parent, 1, false);
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 367e476..88c3abd 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -32,7 +32,8 @@ void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
     s->ctx = iothread_get_aio_context(vs->conf.iothread);
 
     /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers || !k->set_host_notifier) {
+    if (!k->set_guest_notifiers ||
+        (!k->set_host_notifier && !k->ioeventfd_started)) {
         fprintf(stderr, "virtio-scsi: Failed to set iothread "
                    "(transport does not support notifiers)");
         exit(1);
@@ -46,7 +47,10 @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n)
     int rc;
 
     /* Set up virtqueue notify */
-    rc = k->set_host_notifier(qbus->parent, n, true);
+    rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), n, true);
+    if (rc == -ENOSYS) {
+        rc = k->set_host_notifier(qbus->parent, n, true);
+    }
     if (rc != 0) {
         fprintf(stderr, "virtio-scsi: Failed to set host notifier (%d)\n",
                 rc);
@@ -129,7 +133,10 @@ fail_vrings:
     virtio_scsi_clear_aio(s);
     aio_context_release(s->ctx);
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
-        k->set_host_notifier(qbus->parent, i, false);
+        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+        if (rc == -ENOSYS) {
+            k->set_host_notifier(qbus->parent, i, false);
+        }
     }
     k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
 fail_guest_notifiers:
@@ -144,7 +151,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
-    int i;
+    int i, rc;
 
     if (!s->dataplane_started || s->dataplane_stopping) {
         return;
@@ -168,7 +175,10 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
     aio_context_release(s->ctx);
 
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
-        k->set_host_notifier(qbus->parent, i, false);
+        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+        if (rc == -ENOSYS) {
+            k->set_host_notifier(qbus->parent, i, false);
+        }
     }
 
     /* Clean up guest notifier (irq) */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 392d848..38f54a2 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1113,14 +1113,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     VirtioBusState *vbus = VIRTIO_BUS(qbus);
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
     int i, r, e;
-    if (!k->set_host_notifier) {
+    if (!k->set_host_notifier || !k->ioeventfd_started) {
         fprintf(stderr, "binding does not support host notifiers\n");
         r = -ENOSYS;
         goto fail;
     }
 
     for (i = 0; i < hdev->nvqs; ++i) {
-        r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
+        r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
+                                         true);
+        if (r == -ENOSYS) {
+            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
+        }
         if (r < 0) {
             fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
             goto fail_vq;
@@ -1130,7 +1134,11 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     return 0;
 fail_vq:
     while (--i >= 0) {
-        e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
+                                         false);
+        if (e == -ENOSYS) {
+            e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        }
         if (e < 0) {
             fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
             fflush(stderr);
@@ -1154,7 +1162,11 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     int i, r;
 
     for (i = 0; i < hdev->nvqs; ++i) {
-        r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
+                                         false);
+        if (r == -ENOSYS) {
+            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        }
         if (r < 0) {
             fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
             fflush(stderr);
-- 
2.6.5

[Qemu-devel] [PATCH RFC 3/6] virtio-ccw: convert to ioeventfd callbacks

[Cornelia Huck]

Use the new interface.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 133 +++++++++++++++++---------------------------------
 1 file changed, 45 insertions(+), 88 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cb887ba..8732b57 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -67,92 +67,58 @@ VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
     return vdev;
 }
 
-static int virtio_ccw_set_guest2host_notifier(VirtioCcwDevice *dev, int n,
-                                              bool assign, bool set_handler)
+static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
 {
-    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
-    VirtQueue *vq = virtio_get_queue(vdev, n);
-    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
-    int r = 0;
-    SubchDev *sch = dev->sch;
-    uint32_t sch_id = (css_build_subchannel_id(sch) << 16) | sch->schid;
+    virtio_bus_start_ioeventfd(&dev->bus);
+}
 
-    if (assign) {
-        r = event_notifier_init(notifier, 1);
-        if (r < 0) {
-            error_report("%s: unable to init event notifier: %d", __func__, r);
-            return r;
-        }
-        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
-        r = s390_assign_subch_ioeventfd(notifier, sch_id, n, assign);
-        if (r < 0) {
-            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
-            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
-            event_notifier_cleanup(notifier);
-            return r;
-        }
-    } else {
-        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
-        s390_assign_subch_ioeventfd(notifier, sch_id, n, assign);
-        event_notifier_cleanup(notifier);
-    }
-    return r;
+static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
+{
+    virtio_bus_start_ioeventfd(&dev->bus);
 }
 
-static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
+static bool virtio_ccw_ioeventfd_started(DeviceState *d)
 {
-    VirtIODevice *vdev;
-    int n, r;
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
 
-    if (!(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD) ||
-        dev->ioeventfd_disabled ||
-        dev->ioeventfd_started) {
-        return;
-    }
-    vdev = virtio_bus_get_device(&dev->bus);
-    for (n = 0; n < VIRTIO_CCW_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
-        r = virtio_ccw_set_guest2host_notifier(dev, n, true, true);
-        if (r < 0) {
-            goto assign_error;
-        }
-    }
-    dev->ioeventfd_started = true;
-    return;
+    return dev->ioeventfd_started;
+}
 
-  assign_error:
-    while (--n >= 0) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
-        r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
-        assert(r >= 0);
+static void virtio_ccw_ioeventfd_set_started(DeviceState *d, bool started,
+                                             bool err)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+
+    dev->ioeventfd_started = started;
+    if (err) {
+        /* Disable ioeventfd for this device. */
+        dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
     }
-    dev->ioeventfd_started = false;
-    /* Disable ioeventfd for this device. */
-    dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
-    error_report("%s: failed. Fallback to userspace (slower).", __func__);
 }
 
-static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
+static bool virtio_ccw_ioeventfd_disabled(DeviceState *d)
 {
-    VirtIODevice *vdev;
-    int n, r;
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
 
-    if (!dev->ioeventfd_started) {
-        return;
-    }
-    vdev = virtio_bus_get_device(&dev->bus);
-    for (n = 0; n < VIRTIO_CCW_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
-        r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
-        assert(r >= 0);
-    }
-    dev->ioeventfd_started = false;
+    return dev->ioeventfd_disabled ||
+        !(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD);
+}
+
+static void virtio_ccw_ioeventfd_set_disabled(DeviceState *d, bool disabled)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+
+    dev->ioeventfd_disabled = disabled;
+}
+
+static int virtio_ccw_ioeventfd_assign(DeviceState *d, EventNotifier *notifier,
+                                       int n, bool assign)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+    SubchDev *sch = dev->sch;
+    uint32_t sch_id = (css_build_subchannel_id(sch) << 16) | sch->schid;
+
+    return s390_assign_subch_ioeventfd(notifier, sch_id, n, assign);
 }
 
 VirtualCssBus *virtual_css_bus_init(void)
@@ -1193,19 +1159,6 @@ static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
     return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
 }
 
-static int virtio_ccw_set_host_notifier(DeviceState *d, int n, bool assign)
-{
-    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
-
-    /* Stop using the generic ioeventfd, we are doing eventfd handling
-     * ourselves below */
-    dev->ioeventfd_disabled = assign;
-    if (assign) {
-        virtio_ccw_stop_ioeventfd(dev);
-    }
-    return virtio_ccw_set_guest2host_notifier(dev, n, assign, false);
-}
-
 static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
 {
     int r;
@@ -1834,7 +1787,6 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
     k->notify = virtio_ccw_notify;
     k->vmstate_change = virtio_ccw_vmstate_change;
     k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
-    k->set_host_notifier = virtio_ccw_set_host_notifier;
     k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
     k->save_queue = virtio_ccw_save_queue;
     k->load_queue = virtio_ccw_load_queue;
@@ -1843,6 +1795,11 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
     k->device_plugged = virtio_ccw_device_plugged;
     k->post_plugged = virtio_ccw_post_plugged;
     k->device_unplugged = virtio_ccw_device_unplugged;
+    k->ioeventfd_started = virtio_ccw_ioeventfd_started;
+    k->ioeventfd_set_started = virtio_ccw_ioeventfd_set_started;
+    k->ioeventfd_disabled = virtio_ccw_ioeventfd_disabled;
+    k->ioeventfd_set_disabled = virtio_ccw_ioeventfd_set_disabled;
+    k->ioeventfd_assign = virtio_ccw_ioeventfd_assign;
 }
 
 static const TypeInfo virtio_ccw_bus_info = {
-- 
2.6.5

[Qemu-devel] [PATCH RFC 4/6] virtio-pci: convert to ioeventfd callbacks

[Cornelia Huck]

Convert to new interface.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/virtio/virtio-pci.c | 124 ++++++++++++++++---------------------------------
 1 file changed, 41 insertions(+), 83 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 0dadb66..b5e59c6 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -261,14 +261,44 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
     return 0;
 }
 
+static bool virtio_pci_ioeventfd_started(DeviceState *d)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    return proxy->ioeventfd_started;
+}
+
+static void virtio_pci_ioeventfd_set_started(DeviceState *d, bool started,
+                                             bool err)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    proxy->ioeventfd_started = started;
+}
+
+static bool virtio_pci_ioeventfd_disabled(DeviceState *d)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    return proxy->ioeventfd_disabled ||
+        !(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD);
+}
+
+static void virtio_pci_ioeventfd_set_disabled(DeviceState *d, bool disabled)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    proxy->ioeventfd_disabled = disabled;
+}
+
 #define QEMU_VIRTIO_PCI_QUEUE_MEM_MULT 0x1000
 
-static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
-                                                 int n, bool assign, bool set_handler)
+static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier *notifier,
+                                       int n, bool assign)
 {
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtQueue *vq = virtio_get_queue(vdev, n);
-    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
     bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
     bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
     bool fast_mmio = kvm_ioeventfd_any_length_enabled();
@@ -279,16 +309,8 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
     hwaddr modern_addr = QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
                          virtio_get_queue_index(vq);
     hwaddr legacy_addr = VIRTIO_PCI_QUEUE_NOTIFY;
-    int r = 0;
 
     if (assign) {
-        r = event_notifier_init(notifier, 1);
-        if (r < 0) {
-            error_report("%s: unable to init event notifier: %d",
-                         __func__, r);
-            return r;
-        }
-        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
         if (modern) {
             if (fast_mmio) {
                 memory_region_add_eventfd(modern_mr, modern_addr, 0,
@@ -324,68 +346,18 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
             memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
                                       true, n, notifier);
         }
-        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
-        event_notifier_cleanup(notifier);
     }
-    return r;
+    return 0;
 }
 
 static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
 {
-    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
-    int n, r;
-
-    if (!(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD) ||
-        proxy->ioeventfd_disabled ||
-        proxy->ioeventfd_started) {
-        return;
-    }
-
-    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
-
-        r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
-        if (r < 0) {
-            goto assign_error;
-        }
-    }
-    proxy->ioeventfd_started = true;
-    return;
-
-assign_error:
-    while (--n >= 0) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
-
-        r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
-        assert(r >= 0);
-    }
-    proxy->ioeventfd_started = false;
-    error_report("%s: failed. Fallback to a userspace (slower).", __func__);
+    virtio_bus_start_ioeventfd(&proxy->bus);
 }
 
 static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
 {
-    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
-    int r;
-    int n;
-
-    if (!proxy->ioeventfd_started) {
-        return;
-    }
-
-    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
-
-        r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
-        assert(r >= 0);
-    }
-    proxy->ioeventfd_started = false;
+    virtio_bus_stop_ioeventfd(&proxy->bus, false);
 }
 
 static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
@@ -1111,24 +1083,6 @@ assign_error:
     return r;
 }
 
-static int virtio_pci_set_host_notifier(DeviceState *d, int n, bool assign)
-{
-    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
-
-    /* Stop using ioeventfd for virtqueue kick if the device starts using host
-     * notifiers.  This makes it easy to avoid stepping on each others' toes.
-     */
-    proxy->ioeventfd_disabled = assign;
-    if (assign) {
-        virtio_pci_stop_ioeventfd(proxy);
-    }
-    /* We don't need to start here: it's not needed because backend
-     * currently only stops on status change away from ok,
-     * reset, vmstop and such. If we do add code to start here,
-     * need to check vmstate, device state etc. */
-    return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
-}
-
 static void virtio_pci_vmstate_change(DeviceState *d, bool running)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
@@ -2489,12 +2443,16 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->load_extra_state = virtio_pci_load_extra_state;
     k->has_extra_state = virtio_pci_has_extra_state;
     k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
-    k->set_host_notifier = virtio_pci_set_host_notifier;
     k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
     k->vmstate_change = virtio_pci_vmstate_change;
     k->device_plugged = virtio_pci_device_plugged;
     k->device_unplugged = virtio_pci_device_unplugged;
     k->query_nvectors = virtio_pci_query_nvectors;
+    k->ioeventfd_started = virtio_pci_ioeventfd_started;
+    k->ioeventfd_set_started = virtio_pci_ioeventfd_set_started;
+    k->ioeventfd_disabled = virtio_pci_ioeventfd_disabled;
+    k->ioeventfd_set_disabled = virtio_pci_ioeventfd_set_disabled;
+    k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
-- 
2.6.5

[Qemu-devel] [PATCH RFC 5/6] virtio-mmio: convert to ioeventfd callbacks

[Cornelia Huck]

Convert to the new interface.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/virtio/virtio-mmio.c | 128 ++++++++++++++++--------------------------------
 1 file changed, 41 insertions(+), 87 deletions(-)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index d4cd91f..d2accbc 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -93,90 +93,59 @@ typedef struct {
     bool ioeventfd_started;
 } VirtIOMMIOProxy;
 
-static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
-                                                  int n, bool assign,
-                                                  bool set_handler)
+static bool virtio_mmio_ioeventfd_started(DeviceState *d)
 {
-    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
-    VirtQueue *vq = virtio_get_queue(vdev, n);
-    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
-    int r = 0;
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
 
-    if (assign) {
-        r = event_notifier_init(notifier, 1);
-        if (r < 0) {
-            error_report("%s: unable to init event notifier: %d",
-                         __func__, r);
-            return r;
-        }
-        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
-        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
-                                  true, n, notifier);
-    } else {
-        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
-                                  true, n, notifier);
-        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
-        event_notifier_cleanup(notifier);
-    }
-    return r;
+    return proxy->ioeventfd_started;
 }
 
-static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy)
+static void virtio_mmio_ioeventfd_set_started(DeviceState *d, bool started,
+                                              bool err)
 {
-    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
-    int n, r;
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
 
-    if (!kvm_eventfds_enabled() ||
-        proxy->ioeventfd_disabled ||
-        proxy->ioeventfd_started) {
-        return;
-    }
+    proxy->ioeventfd_started = started;
+}
 
-    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
+static bool virtio_mmio_ioeventfd_disabled(DeviceState *d)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
 
-        r = virtio_mmio_set_host_notifier_internal(proxy, n, true, true);
-        if (r < 0) {
-            goto assign_error;
-        }
-    }
-    proxy->ioeventfd_started = true;
-    return;
+    return !kvm_eventfds_enabled() || proxy->ioeventfd_disabled;
+}
 
-assign_error:
-    while (--n >= 0) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
+static void virtio_mmio_ioeventfd_set_disabled(DeviceState *d, bool disabled)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
 
-        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
-        assert(r >= 0);
-    }
-    proxy->ioeventfd_started = false;
-    error_report("%s: failed. Fallback to a userspace (slower).", __func__);
+    proxy->ioeventfd_disabled = disabled;
 }
 
-static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+static int virtio_mmio_ioeventfd_assign(DeviceState *d,
+                                        EventNotifier *notifier,
+                                        int n, bool assign)
 {
-    int r;
-    int n;
-    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
 
-    if (!proxy->ioeventfd_started) {
-        return;
+    if (assign) {
+        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+                                  true, n, notifier);
+    } else {
+        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+                                  true, n, notifier);
     }
+    return 0;
+}
 
-    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(vdev, n)) {
-            continue;
-        }
+static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+    virtio_bus_start_ioeventfd(&proxy->bus);
+}
 
-        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
-        assert(r >= 0);
-    }
-    proxy->ioeventfd_started = false;
+static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+    virtio_bus_stop_ioeventfd(&proxy->bus, false);
 }
 
 static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
@@ -498,25 +467,6 @@ assign_error:
     return r;
 }
 
-static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n,
-                                         bool assign)
-{
-    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
-
-    /* Stop using ioeventfd for virtqueue kick if the device starts using host
-     * notifiers.  This makes it easy to avoid stepping on each others' toes.
-     */
-    proxy->ioeventfd_disabled = assign;
-    if (assign) {
-        virtio_mmio_stop_ioeventfd(proxy);
-    }
-    /* We don't need to start here: it's not needed because backend
-     * currently only stops on status change away from ok,
-     * reset, vmstop and such. If we do add code to start here,
-     * need to check vmstate, device state etc. */
-    return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false);
-}
-
 /* virtio-mmio device */
 
 static void virtio_mmio_realizefn(DeviceState *d, Error **errp)
@@ -558,8 +508,12 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     k->notify = virtio_mmio_update_irq;
     k->save_config = virtio_mmio_save_config;
     k->load_config = virtio_mmio_load_config;
-    k->set_host_notifier = virtio_mmio_set_host_notifier;
     k->set_guest_notifiers = virtio_mmio_set_guest_notifiers;
+    k->ioeventfd_started = virtio_mmio_ioeventfd_started;
+    k->ioeventfd_set_started = virtio_mmio_ioeventfd_set_started;
+    k->ioeventfd_disabled = virtio_mmio_ioeventfd_disabled;
+    k->ioeventfd_set_disabled = virtio_mmio_ioeventfd_set_disabled;
+    k->ioeventfd_assign = virtio_mmio_ioeventfd_assign;
     k->has_variable_vring_alignment = true;
     bus_class->max_dev = 1;
 }
-- 
2.6.5

[Qemu-devel] [PATCH RFC 6/6] virtio-bus: remove old set_host_notifier callback

[Cornelia Huck]

All users have been converted to the new ioevent callbacks.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/block/dataplane/virtio-blk.c | 12 ++----------
 hw/scsi/virtio-scsi-dataplane.c | 19 ++++---------------
 hw/virtio/vhost.c               | 13 +------------
 include/hw/virtio/virtio-bus.h  |  1 -
 4 files changed, 7 insertions(+), 38 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 9ca41d7..1b2d5fa 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -132,8 +132,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
     }
 
     /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers ||
-        (!k->set_host_notifier && !k->ioeventfd_started)) {
+    if (!k->set_guest_notifiers || !k->ioeventfd_started) {
         error_setg(errp,
                    "device is incompatible with dataplane "
                    "(transport does not support notifiers)");
@@ -211,9 +210,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 
     /* Set up virtqueue notify */
     r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, true);
-    if (r == -ENOSYS) {
-        r = k->set_host_notifier(qbus->parent, 0, true);
-    }
     if (r != 0) {
         fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
         goto fail_host_notifier;
@@ -248,7 +244,6 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
-    int r;
 
     if (!vblk->dataplane_started || s->stopping) {
         return;
@@ -273,10 +268,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
 
     aio_context_release(s->ctx);
 
-    r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
-    if (r == -ENOSYS) {
-        k->set_host_notifier(qbus->parent, 0, false);
-    }
+    virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
 
     /* Clean up guest notifier (irq) */
     k->set_guest_notifiers(qbus->parent, 1, false);
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 88c3abd..88773d2 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -32,8 +32,7 @@ void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
     s->ctx = iothread_get_aio_context(vs->conf.iothread);
 
     /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers ||
-        (!k->set_host_notifier && !k->ioeventfd_started)) {
+    if (!k->set_guest_notifiers || !k->ioeventfd_started) {
         fprintf(stderr, "virtio-scsi: Failed to set iothread "
                    "(transport does not support notifiers)");
         exit(1);
@@ -43,14 +42,10 @@ void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
 static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n)
 {
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     int rc;
 
     /* Set up virtqueue notify */
     rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), n, true);
-    if (rc == -ENOSYS) {
-        rc = k->set_host_notifier(qbus->parent, n, true);
-    }
     if (rc != 0) {
         fprintf(stderr, "virtio-scsi: Failed to set host notifier (%d)\n",
                 rc);
@@ -133,10 +128,7 @@ fail_vrings:
     virtio_scsi_clear_aio(s);
     aio_context_release(s->ctx);
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
-        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
-        if (rc == -ENOSYS) {
-            k->set_host_notifier(qbus->parent, i, false);
-        }
+        virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
     }
     k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
 fail_guest_notifiers:
@@ -151,7 +143,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
-    int i, rc;
+    int i;
 
     if (!s->dataplane_started || s->dataplane_stopping) {
         return;
@@ -175,10 +167,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
     aio_context_release(s->ctx);
 
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
-        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
-        if (rc == -ENOSYS) {
-            k->set_host_notifier(qbus->parent, i, false);
-        }
+        virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
     }
 
     /* Clean up guest notifier (irq) */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 38f54a2..90febf2 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1113,7 +1113,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     VirtioBusState *vbus = VIRTIO_BUS(qbus);
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
     int i, r, e;
-    if (!k->set_host_notifier || !k->ioeventfd_started) {
+    if (!k->ioeventfd_started) {
         fprintf(stderr, "binding does not support host notifiers\n");
         r = -ENOSYS;
         goto fail;
@@ -1122,9 +1122,6 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     for (i = 0; i < hdev->nvqs; ++i) {
         r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
                                          true);
-        if (r == -ENOSYS) {
-            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
-        }
         if (r < 0) {
             fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
             goto fail_vq;
@@ -1136,9 +1133,6 @@ fail_vq:
     while (--i >= 0) {
         e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
                                          false);
-        if (e == -ENOSYS) {
-            e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
-        }
         if (e < 0) {
             fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
             fflush(stderr);
@@ -1157,16 +1151,11 @@ fail:
 void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    VirtioBusState *vbus = VIRTIO_BUS(qbus);
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
     int i, r;
 
     for (i = 0; i < hdev->nvqs; ++i) {
         r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +i,
                                          false);
-        if (r == -ENOSYS) {
-            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
-        }
         if (r < 0) {
             fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
             fflush(stderr);
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 0281cbf..cbe279f 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -52,7 +52,6 @@ typedef struct VirtioBusClass {
     bool (*has_extra_state)(DeviceState *d);
     bool (*query_guest_notifiers)(DeviceState *d);
     int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
-    int (*set_host_notifier)(DeviceState *d, int n, bool assigned);
     void (*vmstate_change)(DeviceState *d, bool running);
     /*
      * transport independent init function.
-- 
2.6.5

Re: [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]

On Thu, Mar 17, 2016 at 11:01:27AM +0100, Cornelia Huck wrote:
> As discussed in the virtio-blk dataplane start/stop thread, the
> various transports contain a lot of similar/identical boilerplate
> code for setting up ioeventfds. This makes the code hard to follow.
> 
> Let's drag all of the common handling into the virtio-bus layer,
> hopefully reducing both the boilerplate code and the number of
> indirections.
> 
> This RFC patchset is the result of hacking this up quickly, and I
> was rather surprised that it did not die immediately on my s390
> system (which is all I 'tested'). So be warned :)
> 
> Feedback about the approach and the interface would be good.
> 
> Cornelia Huck (6):
>   virtio-bus: common ioeventfd infrastructure
>   virtio-bus: have callers tolerate new host notifier api
>   virtio-ccw: convert to ioeventfd callbacks
>   virtio-pci: convert to ioeventfd callbacks
>   virtio-mmio: convert to ioeventfd callbacks
>   virtio-bus: remove old set_host_notifier callback
> 
>  hw/block/dataplane/virtio-blk.c |   6 +-
>  hw/s390x/virtio-ccw.c           | 133 ++++++++++++++--------------------------
>  hw/scsi/virtio-scsi-dataplane.c |   9 ++-
>  hw/virtio/vhost.c               |  13 ++--
>  hw/virtio/virtio-bus.c          | 108 ++++++++++++++++++++++++++++++++
>  hw/virtio/virtio-mmio.c         | 128 +++++++++++++-------------------------
>  hw/virtio/virtio-pci.c          | 124 +++++++++++++------------------------
>  include/hw/virtio/virtio-bus.h  |  15 ++++-
>  8 files changed, 263 insertions(+), 273 deletions(-)

I haven't reviewed it line-by-line but overall it looks good.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure

[Fam Zheng]

On Thu, 03/17 11:01, Cornelia Huck wrote:
> Introduce a set of ioeventfd callbacks on the virtio-bus level
> that can be implemented by the individual transports. At the
> virtio-bus level, do common handling for host notifiers (which
> is actually most of it).
> 
> Two things of note:
> - We always iterate over all possible virtio queues, even though
> ccw (currently) has a lower limit. It does not really matter in
> this place.
> - We allow for the virtio-bus caller to pass an "assign" argument
> down when stopping ioeventfd, which the old interface did not allow.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-bus.h |  14 ++++++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 574f0e2..501300f 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -146,6 +146,114 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
>      }
>  }
>  
> +static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
> +                                      int n, bool assign, bool set_handler)
> +{
> +    VirtIODevice *vdev = virtio_bus_get_device(bus);
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    VirtQueue *vq = virtio_get_queue(vdev, n);
> +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> +    int r = 0;
> +
> +    if (assign) {
> +        r = event_notifier_init(notifier, 1);
> +        if (r < 0) {
> +            error_report("%s: unable to init event notifier: %d", __func__, r);
> +            return r;
> +        }
> +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> +        r = k->ioeventfd_assign(proxy, notifier, n, assign);
> +        if (r < 0) {
> +            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
> +            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +            event_notifier_cleanup(notifier);
> +            return r;
> +        }
> +    } else {
> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +        k->ioeventfd_assign(proxy, notifier, n, assign);
> +        event_notifier_cleanup(notifier);
> +    }
> +    return r;
> +}
> +
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    if (!k->ioeventfd_disabled(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, true, true);
> +        if (r < 0) {
> +            goto assign_error;
> +        }
> +    }
> +    k->ioeventfd_set_started(proxy, true, false);
> +    return;
> +
> +assign_error:
> +    while (--n >= 0) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +
> +        r = set_host_notifier_internal(proxy, bus, n, false, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, true);
> +    error_report("%s: failed. Fallback to userspace (slower).", __func__);
> +}
> +
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, assign, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, false);
> +}
> +
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +
> +    if (!k->ioeventfd_started) {
> +        return -ENOSYS;
> +    }
> +    /* Stop using the generic ioeventfd, we are doing eventfd handling
> +     * ourselves below */
> +    k->ioeventfd_set_disabled(proxy, assign);
> +    if (assign) {
> +        virtio_bus_stop_ioeventfd(bus, assign);
> +    }
> +    return set_host_notifier_internal(proxy, bus, n, assign, false);
> +}
> +
>  static char *virtio_bus_get_dev_path(DeviceState *dev)
>  {
>      BusState *bus = qdev_get_parent_bus(dev);
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 3f2c136..0281cbf 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
>      void (*device_unplugged)(DeviceState *d);
>      int (*query_nvectors)(DeviceState *d);
>      /*
> +     * ioeventfd handling: if the transport implements ioeventfd_started,
> +     * it must implement the other ioeventfd callbacks as well
> +     */
> +    bool (*ioeventfd_started)(DeviceState *d);
> +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> +    bool (*ioeventfd_disabled)(DeviceState *d);
> +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> +                            int n, bool assign);

Maybe we should consider documenting these operations and parameters?

> +    /*
>       * Does the transport have variable vring alignment?
>       * (ie can it ever call virtio_queue_set_align()?)
>       * Note that changing this will break migration for this transport.
> @@ -111,4 +121,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
>      return (VirtIODevice *)qdev;
>  }
>  
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus);
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
> +
>  #endif /* VIRTIO_BUS_H */
> -- 
> 2.6.5
> 

Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure

[Cornelia Huck]

On Tue, 22 Mar 2016 08:24:33 +0800
Fam Zheng <famz@redhat.com> wrote:

> On Thu, 03/17 11:01, Cornelia Huck wrote:

> > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> > index 3f2c136..0281cbf 100644
> > --- a/include/hw/virtio/virtio-bus.h
> > +++ b/include/hw/virtio/virtio-bus.h
> > @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
> >      void (*device_unplugged)(DeviceState *d);
> >      int (*query_nvectors)(DeviceState *d);
> >      /*
> > +     * ioeventfd handling: if the transport implements ioeventfd_started,
> > +     * it must implement the other ioeventfd callbacks as well
> > +     */
> > +    bool (*ioeventfd_started)(DeviceState *d);
> > +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> > +    bool (*ioeventfd_disabled)(DeviceState *d);
> > +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> > +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> > +                            int n, bool assign);
> 
> Maybe we should consider documenting these operations and parameters?

Yes, we should :) I just wanted to make sure first that this is the way
to go.

Re: [Qemu-devel] [PATCH RFC 0/6] virtio: refactor host notifiers

[Fam Zheng]

On Thu, 03/17 11:01, Cornelia Huck wrote:
> As discussed in the virtio-blk dataplane start/stop thread, the
> various transports contain a lot of similar/identical boilerplate
> code for setting up ioeventfds. This makes the code hard to follow.
> 
> Let's drag all of the common handling into the virtio-bus layer,
> hopefully reducing both the boilerplate code and the number of
> indirections.
> 
> This RFC patchset is the result of hacking this up quickly, and I
> was rather surprised that it did not die immediately on my s390
> system (which is all I 'tested'). So be warned :)
> 
> Feedback about the approach and the interface would be good.

I tested this series with the two assertions (Paolo's "test" reentrancy
detection and my "assert(is_in_iothread())", in virtio-blk request handler),
and it seems to fix the crash . (A few seconds to crash on master vs.  several
hours survival).

Fam

> 
> Cornelia Huck (6):
>   virtio-bus: common ioeventfd infrastructure
>   virtio-bus: have callers tolerate new host notifier api
>   virtio-ccw: convert to ioeventfd callbacks
>   virtio-pci: convert to ioeventfd callbacks
>   virtio-mmio: convert to ioeventfd callbacks
>   virtio-bus: remove old set_host_notifier callback
> 
>  hw/block/dataplane/virtio-blk.c |   6 +-
>  hw/s390x/virtio-ccw.c           | 133 ++++++++++++++--------------------------
>  hw/scsi/virtio-scsi-dataplane.c |   9 ++-
>  hw/virtio/vhost.c               |  13 ++--
>  hw/virtio/virtio-bus.c          | 108 ++++++++++++++++++++++++++++++++
>  hw/virtio/virtio-mmio.c         | 128 +++++++++++++-------------------------
>  hw/virtio/virtio-pci.c          | 124 +++++++++++++------------------------
>  include/hw/virtio/virtio-bus.h  |  15 ++++-
>  8 files changed, 263 insertions(+), 273 deletions(-)
> 
> -- 
> 2.6.5
> 

Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 11:01:28AM +0100, Cornelia Huck wrote:
> Introduce a set of ioeventfd callbacks on the virtio-bus level
> that can be implemented by the individual transports. At the
> virtio-bus level, do common handling for host notifiers (which
> is actually most of it).
> 
> Two things of note:
> - We always iterate over all possible virtio queues, even though
> ccw (currently) has a lower limit. It does not really matter in
> this place.
> - We allow for the virtio-bus caller to pass an "assign" argument
> down when stopping ioeventfd, which the old interface did not allow.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

So it sounds like this is supposed to fix races
in current code, pls document how.


> ---
>  hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-bus.h |  14 ++++++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 574f0e2..501300f 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -146,6 +146,114 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
>      }
>  }
>  
> +static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
> +                                      int n, bool assign, bool set_handler)
> +{
> +    VirtIODevice *vdev = virtio_bus_get_device(bus);
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    VirtQueue *vq = virtio_get_queue(vdev, n);
> +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> +    int r = 0;
> +
> +    if (assign) {
> +        r = event_notifier_init(notifier, 1);
> +        if (r < 0) {
> +            error_report("%s: unable to init event notifier: %d", __func__, r);
> +            return r;
> +        }
> +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> +        r = k->ioeventfd_assign(proxy, notifier, n, assign);
> +        if (r < 0) {
> +            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
> +            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +            event_notifier_cleanup(notifier);
> +            return r;
> +        }
> +    } else {
> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +        k->ioeventfd_assign(proxy, notifier, n, assign);
> +        event_notifier_cleanup(notifier);
> +    }
> +    return r;
> +}
> +
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    if (!k->ioeventfd_disabled(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, true, true);
> +        if (r < 0) {
> +            goto assign_error;
> +        }
> +    }
> +    k->ioeventfd_set_started(proxy, true, false);
> +    return;
> +
> +assign_error:
> +    while (--n >= 0) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +
> +        r = set_host_notifier_internal(proxy, bus, n, false, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, true);
> +    error_report("%s: failed. Fallback to userspace (slower).", __func__);
> +}
> +
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +    VirtIODevice *vdev;
> +    int n, r;
> +
> +    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
> +        return;
> +    }
> +    vdev = virtio_bus_get_device(bus);
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +        r = set_host_notifier_internal(proxy, bus, n, assign, false);
> +        assert(r >= 0);
> +    }
> +    k->ioeventfd_set_started(proxy, false, false);
> +}
> +
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)


Could you add documentation for what does "assign" mean here?
It was there as an internal API, but now it's external
so needs better docs.

> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +
> +    if (!k->ioeventfd_started) {
> +        return -ENOSYS;
> +    }
> +    /* Stop using the generic ioeventfd, we are doing eventfd handling
> +     * ourselves below */
> +    k->ioeventfd_set_disabled(proxy, assign);
> +    if (assign) {
> +        virtio_bus_stop_ioeventfd(bus, assign);
> +    }

Need comment to explain why don't we start on !assign.

> +    return set_host_notifier_internal(proxy, bus, n, assign, false);
> +}
> +
>  static char *virtio_bus_get_dev_path(DeviceState *dev)
>  {
>      BusState *bus = qdev_get_parent_bus(dev);
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 3f2c136..0281cbf 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
>      void (*device_unplugged)(DeviceState *d);
>      int (*query_nvectors)(DeviceState *d);
>      /*
> +     * ioeventfd handling: if the transport implements ioeventfd_started,
> +     * it must implement the other ioeventfd callbacks as well
> +     */
> +    bool (*ioeventfd_started)(DeviceState *d);
> +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> +    bool (*ioeventfd_disabled)(DeviceState *d);
> +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> +                            int n, bool assign);
> +    /*
>       * Does the transport have variable vring alignment?
>       * (ie can it ever call virtio_queue_set_align()?)
>       * Note that changing this will break migration for this transport.
> @@ -111,4 +121,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
>      return (VirtIODevice *)qdev;
>  }
>  
> +void virtio_bus_start_ioeventfd(VirtioBusState *bus);
> +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
> +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
> +
>  #endif /* VIRTIO_BUS_H */
> -- 
> 2.6.5

Re: [Qemu-devel] [PATCH RFC 1/6] virtio-bus: common ioeventfd infrastructure

[Cornelia Huck]

On Thu, 24 Mar 2016 13:20:34 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Mar 17, 2016 at 11:01:28AM +0100, Cornelia Huck wrote:
> > Introduce a set of ioeventfd callbacks on the virtio-bus level
> > that can be implemented by the individual transports. At the
> > virtio-bus level, do common handling for host notifiers (which
> > is actually most of it).
> > 
> > Two things of note:
> > - We always iterate over all possible virtio queues, even though
> > ccw (currently) has a lower limit. It does not really matter in
> > this place.
> > - We allow for the virtio-bus caller to pass an "assign" argument
> > down when stopping ioeventfd, which the old interface did not allow.
> > 
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> So it sounds like this is supposed to fix races
> in current code, pls document how.

Yes, final version will have more description + comments.

> 
> 
> > ---
> >  hw/virtio/virtio-bus.c         | 108 +++++++++++++++++++++++++++++++++++++++++
> >  include/hw/virtio/virtio-bus.h |  14 ++++++
> >  2 files changed, 122 insertions(+)
> > 
> > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> > index 574f0e2..501300f 100644
> > --- a/hw/virtio/virtio-bus.c
> > +++ b/hw/virtio/virtio-bus.c
> > @@ -146,6 +146,114 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
> >      }
> >  }
> >  
> > +static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
> > +                                      int n, bool assign, bool set_handler)
> > +{
> > +    VirtIODevice *vdev = virtio_bus_get_device(bus);
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    VirtQueue *vq = virtio_get_queue(vdev, n);
> > +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> > +    int r = 0;
> > +
> > +    if (assign) {
> > +        r = event_notifier_init(notifier, 1);
> > +        if (r < 0) {
> > +            error_report("%s: unable to init event notifier: %d", __func__, r);
> > +            return r;
> > +        }
> > +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> > +        r = k->ioeventfd_assign(proxy, notifier, n, assign);
> > +        if (r < 0) {
> > +            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
> > +            virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> > +            event_notifier_cleanup(notifier);
> > +            return r;
> > +        }
> > +    } else {
> > +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> > +        k->ioeventfd_assign(proxy, notifier, n, assign);
> > +        event_notifier_cleanup(notifier);
> > +    }
> > +    return r;
> > +}

Note that I'm currently trying to disentangle ioeventfd registration
and handler assignment, so this will look a bit different in the final
patch.

> > +
> > +void virtio_bus_start_ioeventfd(VirtioBusState *bus)
> > +{
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> > +    VirtIODevice *vdev;
> > +    int n, r;
> > +
> > +    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
> > +        return;
> > +    }
> > +    if (!k->ioeventfd_disabled(proxy)) {

And this one is actually the wrong way around, as mentioned in the
other thread. Will be fixed.

> > +        return;
> > +    }
> > +    vdev = virtio_bus_get_device(bus);
> > +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> > +        if (!virtio_queue_get_num(vdev, n)) {
> > +            continue;
> > +        }
> > +        r = set_host_notifier_internal(proxy, bus, n, true, true);
> > +        if (r < 0) {
> > +            goto assign_error;
> > +        }
> > +    }
> > +    k->ioeventfd_set_started(proxy, true, false);
> > +    return;
> > +
> > +assign_error:
> > +    while (--n >= 0) {
> > +        if (!virtio_queue_get_num(vdev, n)) {
> > +            continue;
> > +        }
> > +
> > +        r = set_host_notifier_internal(proxy, bus, n, false, false);
> > +        assert(r >= 0);
> > +    }
> > +    k->ioeventfd_set_started(proxy, false, true);
> > +    error_report("%s: failed. Fallback to userspace (slower).", __func__);
> > +}
> > +
> > +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign)
> > +{
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> > +    VirtIODevice *vdev;
> > +    int n, r;
> > +
> > +    if (!k->ioeventfd_started || !k->ioeventfd_started(proxy)) {
> > +        return;
> > +    }
> > +    vdev = virtio_bus_get_device(bus);
> > +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> > +        if (!virtio_queue_get_num(vdev, n)) {
> > +            continue;
> > +        }
> > +        r = set_host_notifier_internal(proxy, bus, n, assign, false);
> > +        assert(r >= 0);
> > +    }
> > +    k->ioeventfd_set_started(proxy, false, false);
> > +}
> > +
> > +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
> 
> 
> Could you add documentation for what does "assign" mean here?
> It was there as an internal API, but now it's external
> so needs better docs.

On my list.

> 
> > +{
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> > +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> > +
> > +    if (!k->ioeventfd_started) {
> > +        return -ENOSYS;
> > +    }
> > +    /* Stop using the generic ioeventfd, we are doing eventfd handling
> > +     * ourselves below */
> > +    k->ioeventfd_set_disabled(proxy, assign);
> > +    if (assign) {
> > +        virtio_bus_stop_ioeventfd(bus, assign);
> > +    }
> 
> Need comment to explain why don't we start on !assign.

That's a part I'm currently reworking anyway, as we don't want to try
to assign ioeventfds twice, but just keep them. It will be hopefully
clearer then.

> 
> > +    return set_host_notifier_internal(proxy, bus, n, assign, false);
> > +}
> > +
> >  static char *virtio_bus_get_dev_path(DeviceState *dev)
> >  {
> >      BusState *bus = qdev_get_parent_bus(dev);
> > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> > index 3f2c136..0281cbf 100644
> > --- a/include/hw/virtio/virtio-bus.h
> > +++ b/include/hw/virtio/virtio-bus.h
> > @@ -71,6 +71,16 @@ typedef struct VirtioBusClass {
> >      void (*device_unplugged)(DeviceState *d);
> >      int (*query_nvectors)(DeviceState *d);
> >      /*
> > +     * ioeventfd handling: if the transport implements ioeventfd_started,
> > +     * it must implement the other ioeventfd callbacks as well
> > +     */
> > +    bool (*ioeventfd_started)(DeviceState *d);
> > +    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
> > +    bool (*ioeventfd_disabled)(DeviceState *d);
> > +    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
> > +    int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
> > +                            int n, bool assign);

I've also added a short description for the callbacks in the meantime.

> > +    /*
> >       * Does the transport have variable vring alignment?
> >       * (ie can it ever call virtio_queue_set_align()?)
> >       * Note that changing this will break migration for this transport.
> > @@ -111,4 +121,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
> >      return (VirtIODevice *)qdev;
> >  }
> >  
> > +void virtio_bus_start_ioeventfd(VirtioBusState *bus);
> > +void virtio_bus_stop_ioeventfd(VirtioBusState *bus, bool assign);
> > +int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
> > +
> >  #endif /* VIRTIO_BUS_H */
> > -- 
> > 2.6.5
>