[Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

This series started as an attempt to always use the dataplane path
for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
was three-fold:

1) to add more coverage for dataplane

2) to remove virtio_add_queue_aio

3) to simplify the dataplane start/stop code

It achieves the first two objectives, and while it doesn't quite
achieve the third it does cleanup the generic ioeventfd code in
virtio-bus more than I expected.  In particular, it reduces the set
of callbacks that transports must implement, and it removes the ugly
case where ioeventfd is started with generic callbacks and then moved
to the dataplane callbacks.  It also enables some simplification of the
functions that deal with host notifiers.

I've tested it with virtio-blk, virtio-scsi and vhost-net.

Patches 1 and 2 are simplifications that are too nice to leave
them for later in the series.

Patch 3 moves some of the ioeventfd code from virtio-bus.c to
virtio.c.  At this point the transition is a bit half-assed, but
this changes as soon as we remove the generic->dataplane
handler transition.

Patches 4 to 6 do exactly that, and then the spring cleaning
begins, lasting for the whole second half of the series.

Opinions, reviews and bug reports?

Thanks,

Paolo

Paolo Bonzini (12):
  virtio: move ioeventfd_disabled flag to VirtioBusState
  virtio: move ioeventfd_started flag to VirtioBusState
  virtio: add start_ioeventfd and stop_ioeventfd to VirtioDeviceClass
  virtio: introduce virtio_device_ioeventfd_enabled
  virtio-blk: always use dataplane path if ioeventfd is active
  virtio-scsi: always use dataplane path if ioeventfd is active
  Revert "virtio: Introduce virtio_add_queue_aio"
  virtio: remove set_handler argument from set_host_notifier_internal
  virtio: remove ioeventfd_disabled altogether
  virtio: do not export set_host_notifier_internal
  virtio: inline virtio_queue_set_host_notifier_fd_handler
  virtio: inline set_host_notifier_internal

 hw/block/dataplane/virtio-blk.c |  67 +++++++++++-------
 hw/block/dataplane/virtio-blk.h |   6 +-
 hw/block/virtio-blk.c           |  16 ++---
 hw/s390x/virtio-ccw.c           |  36 +---------
 hw/s390x/virtio-ccw.h           |   2 -
 hw/scsi/virtio-scsi-dataplane.c |  51 ++++++++------
 hw/scsi/virtio-scsi.c           |  24 +++----
 hw/virtio/vhost.c               |   5 +-
 hw/virtio/virtio-bus.c          | 153 +++++++++++++++-------------------------
 hw/virtio/virtio-mmio.c         |  35 +--------
 hw/virtio/virtio-pci.c          |  32 +--------
 hw/virtio/virtio-pci.h          |   2 -
 hw/virtio/virtio.c              | 139 +++++++++++++++++++++++-------------
 include/hw/virtio/virtio-bus.h  |  27 ++++---
 include/hw/virtio/virtio-scsi.h |   6 +-
 include/hw/virtio/virtio.h      |  11 +--
 16 files changed, 272 insertions(+), 340 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 01/12] virtio: move ioeventfd_disabled flag to VirtioBusState

[Paolo Bonzini]

This simplifies the code and removes the ioeventfd_set_disabled
callback.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/s390x/virtio-ccw.c          | 11 +----------
 hw/s390x/virtio-ccw.h          |  1 -
 hw/virtio/virtio-bus.c         |  4 ++--
 hw/virtio/virtio-mmio.c        | 13 +------------
 hw/virtio/virtio-pci.c         | 11 +----------
 hw/virtio/virtio-pci.h         |  1 -
 include/hw/virtio/virtio-bus.h |  8 ++++++--
 7 files changed, 11 insertions(+), 38 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index a554a24..f3ed9ab 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -82,15 +82,7 @@ static bool virtio_ccw_ioeventfd_disabled(DeviceState *d)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
 
-    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;
+    return !(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD);
 }
 
 static int virtio_ccw_ioeventfd_assign(DeviceState *d, EventNotifier *notifier,
@@ -1599,7 +1591,6 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
     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;
 }
 
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 1c6bc86..1f5fce0 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -83,7 +83,6 @@ struct VirtioCcwDevice {
     uint32_t max_rev;
     VirtioBusState bus;
     bool ioeventfd_started;
-    bool ioeventfd_disabled;
     uint32_t flags;
     uint8_t thinint_isc;
     AdapterRoutes routes;
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index a85b7c8..859b53f 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -193,7 +193,7 @@ void virtio_bus_start_ioeventfd(VirtioBusState *bus)
     if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
         return;
     }
-    if (k->ioeventfd_disabled(proxy)) {
+    if (bus->ioeventfd_disabled || k->ioeventfd_disabled(proxy)) {
         return;
     }
     vdev = virtio_bus_get_device(bus);
@@ -255,7 +255,7 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
     if (!k->ioeventfd_started) {
         return -ENOSYS;
     }
-    k->ioeventfd_set_disabled(proxy, assign);
+    bus->ioeventfd_disabled = assign;
     if (assign) {
         /*
          * Stop using the generic ioeventfd, we are doing eventfd handling
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 13798b3..12a9e79 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -89,7 +89,6 @@ typedef struct {
     uint32_t guest_page_shift;
     /* virtio-bus */
     VirtioBusState bus;
-    bool ioeventfd_disabled;
     bool ioeventfd_started;
     bool format_transport_address;
 } VirtIOMMIOProxy;
@@ -111,16 +110,7 @@ static void virtio_mmio_ioeventfd_set_started(DeviceState *d, bool started,
 
 static bool virtio_mmio_ioeventfd_disabled(DeviceState *d)
 {
-    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
-
-    return !kvm_eventfds_enabled() || proxy->ioeventfd_disabled;
-}
-
-static void virtio_mmio_ioeventfd_set_disabled(DeviceState *d, bool disabled)
-{
-    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
-
-    proxy->ioeventfd_disabled = disabled;
+    return !kvm_eventfds_enabled();
 }
 
 static int virtio_mmio_ioeventfd_assign(DeviceState *d,
@@ -560,7 +550,6 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     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;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 755f921..67a0824 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -281,15 +281,7 @@ 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;
+    return !(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD);
 }
 
 #define QEMU_VIRTIO_PCI_QUEUE_MEM_MULT 0x1000
@@ -2450,7 +2442,6 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     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;
 }
 
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 25fbf8a..1212ac8 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -149,7 +149,6 @@ struct VirtIOPCIProxy {
     uint32_t guest_features[2];
     VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX];
 
-    bool ioeventfd_disabled;
     bool ioeventfd_started;
     VirtIOIRQFD *vector_irqfd;
     int nvqs_with_notifiers;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index f3e5ef3..03a708a 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -83,8 +83,6 @@ typedef struct VirtioBusClass {
     void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
     /* Returns true if the ioeventfd has been disabled for the device. */
     bool (*ioeventfd_disabled)(DeviceState *d);
-    /* Sets the 'ioeventfd disabled' state for the device. */
-    void (*ioeventfd_set_disabled)(DeviceState *d, bool disabled);
     /*
      * Assigns/deassigns the ioeventfd backing for the transport on
      * the device for queue number n. Returns an error value on
@@ -102,6 +100,12 @@ typedef struct VirtioBusClass {
 
 struct VirtioBusState {
     BusState parent_obj;
+
+    /*
+     * Set if the default ioeventfd handlers are disabled by vhost
+     * or dataplane.
+     */
+    bool ioeventfd_disabled;
 };
 
 void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp);
-- 
2.7.4

[Qemu-devel] [PATCH 02/12] virtio: move ioeventfd_started flag to VirtioBusState

[Paolo Bonzini]

This simplifies the code and removes the ioeventfd_started
and ioeventfd_set_started callback.  The only difference is
in how virtio-ccw handles an error---it doesn't disable
ioeventfd forever anymore.  It was the only backend to do
so, and if desired this behavior should be implemented in
virtio-bus.c.

Instead of ioeventfd_started, the ioeventfd_assign callback now
determines whether the virtio bus supports host notifiers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/dataplane/virtio-blk.c |  2 +-
 hw/s390x/virtio-ccw.c           | 21 ---------------------
 hw/s390x/virtio-ccw.h           |  1 -
 hw/scsi/virtio-scsi-dataplane.c |  2 +-
 hw/virtio/vhost.c               |  2 +-
 hw/virtio/virtio-bus.c          | 14 ++++++--------
 hw/virtio/virtio-mmio.c         | 18 ------------------
 hw/virtio/virtio-pci.c          | 17 -----------------
 hw/virtio/virtio-pci.h          |  1 -
 include/hw/virtio/virtio-bus.h  | 17 +++++++----------
 10 files changed, 16 insertions(+), 79 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 704a763..9b268f4 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -93,7 +93,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->ioeventfd_started) {
+    if (!k->set_guest_notifiers || !k->ioeventfd_assign) {
         error_setg(errp,
                    "device is incompatible with dataplane "
                    "(transport does not support notifiers)");
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index f3ed9ab..3e450eb 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -59,25 +59,6 @@ static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
     virtio_bus_stop_ioeventfd(&dev->bus);
 }
 
-static bool virtio_ccw_ioeventfd_started(DeviceState *d)
-{
-    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
-
-    return dev->ioeventfd_started;
-}
-
-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;
-    }
-}
-
 static bool virtio_ccw_ioeventfd_disabled(DeviceState *d)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
@@ -1588,8 +1569,6 @@ 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_assign = virtio_ccw_ioeventfd_assign;
 }
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 1f5fce0..83179c5 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -82,7 +82,6 @@ struct VirtioCcwDevice {
     int revision;
     uint32_t max_rev;
     VirtioBusState bus;
-    bool ioeventfd_started;
     uint32_t flags;
     uint8_t thinint_isc;
     AdapterRoutes routes;
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index b173b94..f537b4e 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -31,7 +31,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->ioeventfd_started) {
+    if (!k->set_guest_notifiers || !k->ioeventfd_assign) {
         fprintf(stderr, "virtio-scsi: Failed to set iothread "
                    "(transport does not support notifiers)");
         exit(1);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 3d0c807..c5a1fe0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1176,7 +1176,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
     int i, r, e;
 
-    if (!k->ioeventfd_started) {
+    if (!k->ioeventfd_assign) {
         error_report("binding does not support host notifiers");
         r = -ENOSYS;
         goto fail;
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 859b53f..64daf34 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -190,10 +190,10 @@ void virtio_bus_start_ioeventfd(VirtioBusState *bus)
     VirtIODevice *vdev;
     int n, r;
 
-    if (!k->ioeventfd_started || k->ioeventfd_started(proxy)) {
+    if (!k->ioeventfd_assign || k->ioeventfd_disabled(proxy)) {
         return;
     }
-    if (bus->ioeventfd_disabled || k->ioeventfd_disabled(proxy)) {
+    if (bus->ioeventfd_started || bus->ioeventfd_disabled) {
         return;
     }
     vdev = virtio_bus_get_device(bus);
@@ -206,7 +206,7 @@ void virtio_bus_start_ioeventfd(VirtioBusState *bus)
             goto assign_error;
         }
     }
-    k->ioeventfd_set_started(proxy, true, false);
+    bus->ioeventfd_started = true;
     return;
 
 assign_error:
@@ -218,18 +218,16 @@ assign_error:
         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)
 {
-    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)) {
+    if (!bus->ioeventfd_started) {
         return;
     }
     vdev = virtio_bus_get_device(bus);
@@ -240,7 +238,7 @@ void virtio_bus_stop_ioeventfd(VirtioBusState *bus)
         r = set_host_notifier_internal(proxy, bus, n, false, false);
         assert(r >= 0);
     }
-    k->ioeventfd_set_started(proxy, false, false);
+    bus->ioeventfd_started = false;
 }
 
 /*
@@ -252,7 +250,7 @@ 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) {
+    if (!k->ioeventfd_assign) {
         return -ENOSYS;
     }
     bus->ioeventfd_disabled = assign;
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 12a9e79..04a9655 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -89,25 +89,9 @@ typedef struct {
     uint32_t guest_page_shift;
     /* virtio-bus */
     VirtioBusState bus;
-    bool ioeventfd_started;
     bool format_transport_address;
 } VirtIOMMIOProxy;
 
-static bool virtio_mmio_ioeventfd_started(DeviceState *d)
-{
-    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
-
-    return proxy->ioeventfd_started;
-}
-
-static void virtio_mmio_ioeventfd_set_started(DeviceState *d, bool started,
-                                              bool err)
-{
-    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
-
-    proxy->ioeventfd_started = started;
-}
-
 static bool virtio_mmio_ioeventfd_disabled(DeviceState *d)
 {
     return !kvm_eventfds_enabled();
@@ -547,8 +531,6 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     k->save_config = virtio_mmio_save_config;
     k->load_config = virtio_mmio_load_config;
     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_assign = virtio_mmio_ioeventfd_assign;
     k->has_variable_vring_alignment = true;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 67a0824..a5d8c8d 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -262,21 +262,6 @@ 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);
@@ -2439,8 +2424,6 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     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_assign = virtio_pci_ioeventfd_assign;
 }
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 1212ac8..1d7063c 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -149,7 +149,6 @@ struct VirtIOPCIProxy {
     uint32_t guest_features[2];
     VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX];
 
-    bool ioeventfd_started;
     VirtIOIRQFD *vector_irqfd;
     int nvqs_with_notifiers;
     VirtioBusState bus;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 03a708a..7031add 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -70,17 +70,9 @@ 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
+     * ioeventfd handling: if the transport implements ioeventfd_assign,
+     * it must implement ioeventfd_disabled as well.
      */
-    /* Returns true if the ioeventfd has been started for the device. */
-    bool (*ioeventfd_started)(DeviceState *d);
-    /*
-     * Sets the 'ioeventfd started' state after the ioeventfd has been
-     * started/stopped for the device. err signifies whether an error
-     * had occurred.
-     */
-    void (*ioeventfd_set_started)(DeviceState *d, bool started, bool err);
     /* Returns true if the ioeventfd has been disabled for the device. */
     bool (*ioeventfd_disabled)(DeviceState *d);
     /*
@@ -106,6 +98,11 @@ struct VirtioBusState {
      * or dataplane.
      */
     bool ioeventfd_disabled;
+
+    /*
+     * Set if ioeventfd has been started.
+     */
+    bool ioeventfd_started;
 };
 
 void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp);
-- 
2.7.4

[Qemu-devel] [PATCH 03/12] virtio: add start_ioeventfd and stop_ioeventfd to VirtioDeviceClass

[Paolo Bonzini]

Allow customization of the start and stop of ioeventfd.  This will
allow direct start of dataplane without passing through the default
ioeventfd handlers, which in turn allows using the dataplane logic
instead of virtio_add_queue_aio.  It will also enable some code
simplification, because the sole entry point to ioeventfd setup
will be virtio_bus_set_host_notifier.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c         | 55 +++++++++++-------------------------
 hw/virtio/virtio.c             | 64 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-bus.h |  7 ++++-
 include/hw/virtio/virtio.h     |  4 +++
 4 files changed, 90 insertions(+), 40 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 64daf34..94e202d 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -152,8 +152,8 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
  * assign: register/deregister ioeventfd with the kernel
  * set_handler: use the generic ioeventfd handler
  */
-static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
-                                      int n, bool assign, bool set_handler)
+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);
@@ -183,61 +183,38 @@ static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
     return r;
 }
 
-void virtio_bus_start_ioeventfd(VirtioBusState *bus)
+int virtio_bus_start_ioeventfd(VirtioBusState *bus)
 {
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
     DeviceState *proxy = DEVICE(BUS(bus)->parent);
-    VirtIODevice *vdev;
-    int n, r;
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+    int r;
 
     if (!k->ioeventfd_assign || k->ioeventfd_disabled(proxy)) {
-        return;
+        return -ENOSYS;
     }
     if (bus->ioeventfd_started || bus->ioeventfd_disabled) {
-        return;
+        return 0;
     }
-    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;
-        }
+    r = vdc->start_ioeventfd(vdev);
+    if (r < 0) {
+        error_report("%s: failed. Fallback to userspace (slower).", __func__);
+        return r;
     }
     bus->ioeventfd_started = true;
-    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);
-    }
-    error_report("%s: failed. Fallback to userspace (slower).", __func__);
+    return 0;
 }
 
 void virtio_bus_stop_ioeventfd(VirtioBusState *bus)
 {
-    DeviceState *proxy = DEVICE(BUS(bus)->parent);
-    VirtIODevice *vdev;
-    int n, r;
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
 
     if (!bus->ioeventfd_started) {
         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, false, false);
-        assert(r >= 0);
-    }
+    vdc->stop_ioeventfd(vdev);
     bus->ioeventfd_started = false;
 }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 15ee3a7..2919113 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1946,15 +1946,79 @@ static Property virtio_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
+{
+    VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    DeviceState *proxy = DEVICE(BUS(qbus)->parent);
+    int n, r, err;
+
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+        r = set_host_notifier_internal(proxy, qbus, n, true, true);
+        if (r < 0) {
+            err = r;
+            goto assign_error;
+        }
+    }
+    return 0;
+
+assign_error:
+    while (--n >= 0) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = set_host_notifier_internal(proxy, qbus, n, false, false);
+        assert(r >= 0);
+    }
+    return err;
+}
+
+int virtio_device_start_ioeventfd(VirtIODevice *vdev)
+{
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusState *vbus = VIRTIO_BUS(qbus);
+
+    return virtio_bus_start_ioeventfd(vbus);
+}
+
+static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
+{
+    VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    DeviceState *proxy = DEVICE(BUS(qbus)->parent);
+    int n, r;
+
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+        r = set_host_notifier_internal(proxy, qbus, n, false, false);
+        assert(r >= 0);
+    }
+}
+
+void virtio_device_stop_ioeventfd(VirtIODevice *vdev)
+{
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusState *vbus = VIRTIO_BUS(qbus);
+
+    virtio_bus_stop_ioeventfd(vbus);
+}
+
 static void virtio_device_class_init(ObjectClass *klass, void *data)
 {
     /* Set the default value here. */
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = virtio_device_realize;
     dc->unrealize = virtio_device_unrealize;
     dc->bus_type = TYPE_VIRTIO_BUS;
     dc->props = virtio_properties;
+    vdc->start_ioeventfd = virtio_device_start_ioeventfd_impl;
+    vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
 }
 
 static const TypeInfo virtio_device_info = {
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 7031add..01b11c5 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -135,10 +135,15 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
 }
 
 /* Start the ioeventfd. */
-void virtio_bus_start_ioeventfd(VirtioBusState *bus);
+int virtio_bus_start_ioeventfd(VirtioBusState *bus);
 /* Stop the ioeventfd. */
 void virtio_bus_stop_ioeventfd(VirtioBusState *bus);
 /* Switch from/to the generic ioeventfd handler */
 int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
 
+/* This is temporary.  It is only needed because virtio_bus_set_host_notifier
+ * sets ioeventfd_disabled but we will shortly get rid of it.  */
+int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
+                               int n, bool assign, bool set_handler);
+
 #endif /* VIRTIO_BUS_H */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d2490c1..f283217 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -124,6 +124,8 @@ typedef struct VirtioDeviceClass {
      * must mask in frontend instead.
      */
     void (*guest_notifier_mask)(VirtIODevice *vdev, int n, bool mask);
+    int (*start_ioeventfd)(VirtIODevice *vdev);
+    void (*stop_ioeventfd)(VirtIODevice *vdev);
     void (*save)(VirtIODevice *vdev, QEMUFile *f);
     int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id);
 } VirtioDeviceClass;
@@ -270,6 +272,8 @@ uint16_t virtio_get_queue_index(VirtQueue *vq);
 EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
 void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                 bool with_irqfd);
+int virtio_device_start_ioeventfd(VirtIODevice *vdev);
+void virtio_device_stop_ioeventfd(VirtIODevice *vdev);
 EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
 void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                bool set_handler);
-- 
2.7.4

[Qemu-devel] [PATCH 04/12] virtio: introduce virtio_device_ioeventfd_enabled

[Paolo Bonzini]

This will be used to forbid iothread configuration when the
proxy does not allow using ioeventfd.  To simplify the implementation,
change the direction of the ioeventfd_disabled callback too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/s390x/virtio-ccw.c          |  6 +++---
 hw/virtio/virtio-bus.c         | 10 +++++++++-
 hw/virtio/virtio-mmio.c        |  6 +++---
 hw/virtio/virtio-pci.c         |  6 +++---
 hw/virtio/virtio.c             |  8 ++++++++
 include/hw/virtio/virtio-bus.h |  8 +++++---
 include/hw/virtio/virtio.h     |  1 +
 7 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 3e450eb..86c1340 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -59,11 +59,11 @@ static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
     virtio_bus_stop_ioeventfd(&dev->bus);
 }
 
-static bool virtio_ccw_ioeventfd_disabled(DeviceState *d)
+static bool virtio_ccw_ioeventfd_enabled(DeviceState *d)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
 
-    return !(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD);
+    return (dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD) != 0;
 }
 
 static int virtio_ccw_ioeventfd_assign(DeviceState *d, EventNotifier *notifier,
@@ -1569,7 +1569,7 @@ 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_disabled = virtio_ccw_ioeventfd_disabled;
+    k->ioeventfd_enabled = virtio_ccw_ioeventfd_enabled;
     k->ioeventfd_assign = virtio_ccw_ioeventfd_assign;
 }
 
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 94e202d..f60efd7 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -191,7 +191,7 @@ int virtio_bus_start_ioeventfd(VirtioBusState *bus)
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
     int r;
 
-    if (!k->ioeventfd_assign || k->ioeventfd_disabled(proxy)) {
+    if (!k->ioeventfd_assign || !k->ioeventfd_enabled(proxy)) {
         return -ENOSYS;
     }
     if (bus->ioeventfd_started || bus->ioeventfd_disabled) {
@@ -218,6 +218,14 @@ void virtio_bus_stop_ioeventfd(VirtioBusState *bus)
     bus->ioeventfd_started = false;
 }
 
+bool virtio_bus_ioeventfd_enabled(VirtIODevice *vdev)
+{
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+    DeviceState *proxy = DEVICE(BUS(bus)->parent);
+
+    return k->ioeventfd_enabled(proxy);
+}
+
 /*
  * This function switches from/to the generic ioeventfd handler.
  * assign==false means 'use generic ioeventfd handler'.
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 04a9655..a30270f 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -92,9 +92,9 @@ typedef struct {
     bool format_transport_address;
 } VirtIOMMIOProxy;
 
-static bool virtio_mmio_ioeventfd_disabled(DeviceState *d)
+static bool virtio_mmio_ioeventfd_enabled(DeviceState *d)
 {
-    return !kvm_eventfds_enabled();
+    return kvm_eventfds_enabled();
 }
 
 static int virtio_mmio_ioeventfd_assign(DeviceState *d,
@@ -531,7 +531,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     k->save_config = virtio_mmio_save_config;
     k->load_config = virtio_mmio_load_config;
     k->set_guest_notifiers = virtio_mmio_set_guest_notifiers;
-    k->ioeventfd_disabled = virtio_mmio_ioeventfd_disabled;
+    k->ioeventfd_enabled = virtio_mmio_ioeventfd_enabled;
     k->ioeventfd_assign = virtio_mmio_ioeventfd_assign;
     k->has_variable_vring_alignment = true;
     bus_class->max_dev = 1;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a5d8c8d..158ffe5 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -262,11 +262,11 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
     return 0;
 }
 
-static bool virtio_pci_ioeventfd_disabled(DeviceState *d)
+static bool virtio_pci_ioeventfd_enabled(DeviceState *d)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
 
-    return !(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD);
+    return (proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD) != 0;
 }
 
 #define QEMU_VIRTIO_PCI_QUEUE_MEM_MULT 0x1000
@@ -2424,7 +2424,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->device_plugged = virtio_pci_device_plugged;
     k->device_unplugged = virtio_pci_device_unplugged;
     k->query_nvectors = virtio_pci_query_nvectors;
-    k->ioeventfd_disabled = virtio_pci_ioeventfd_disabled;
+    k->ioeventfd_enabled = virtio_pci_ioeventfd_enabled;
     k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
 }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2919113..84abd08 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2021,6 +2021,14 @@ static void virtio_device_class_init(ObjectClass *klass, void *data)
     vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
 }
 
+bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
+{
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusState *vbus = VIRTIO_BUS(qbus);
+
+    return virtio_bus_ioeventfd_enabled(vbus);
+}
+
 static const TypeInfo virtio_device_info = {
     .name = TYPE_VIRTIO_DEVICE,
     .parent = TYPE_DEVICE,
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 01b11c5..0abc890 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -71,10 +71,10 @@ typedef struct VirtioBusClass {
     int (*query_nvectors)(DeviceState *d);
     /*
      * ioeventfd handling: if the transport implements ioeventfd_assign,
-     * it must implement ioeventfd_disabled as well.
+     * it must implement ioeventfd_enabled as well.
      */
-    /* Returns true if the ioeventfd has been disabled for the device. */
-    bool (*ioeventfd_disabled)(DeviceState *d);
+    /* Returns true if the ioeventfd is enabled for the device. */
+    bool (*ioeventfd_enabled)(DeviceState *d);
     /*
      * Assigns/deassigns the ioeventfd backing for the transport on
      * the device for queue number n. Returns an error value on
@@ -134,6 +134,8 @@ static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
     return (VirtIODevice *)qdev;
 }
 
+/* Return whether the proxy allows ioeventfd.  */
+bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus);
 /* Start the ioeventfd. */
 int virtio_bus_start_ioeventfd(VirtioBusState *bus);
 /* Stop the ioeventfd. */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index f283217..ec50f7a 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -274,6 +274,7 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                 bool with_irqfd);
 int virtio_device_start_ioeventfd(VirtIODevice *vdev);
 void virtio_device_stop_ioeventfd(VirtIODevice *vdev);
+bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
 EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
 void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                bool set_handler);
-- 
2.7.4

[Qemu-devel] [PATCH 05/12] virtio-blk: always use dataplane path if ioeventfd is active

[Paolo Bonzini]

Override start_ioeventfd and stop_ioeventfd to start/stop the
whole dataplane logic.  This has some positive side effects:

- no need anymore for virtio_add_queue_aio (i.e. a revert of
  commit 0ff841f6d138904d514efa1d885bcaf54583852d)

- no need anymore to switch from generic ioeventfd handlers to
  dataplane

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 67 ++++++++++++++++++++++++-----------------
 hw/block/dataplane/virtio-blk.h |  6 ++--
 hw/block/virtio-blk.c           | 16 ++++------
 hw/virtio/virtio-bus.c          |  2 +-
 4 files changed, 50 insertions(+), 41 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 9b268f4..73781e9 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -83,38 +83,44 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
                                   Error **errp)
 {
     VirtIOBlockDataPlane *s;
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
     *dataplane = NULL;
 
-    if (!conf->iothread) {
+    if (!virtio_device_ioeventfd_enabled(vdev)) {
         return;
     }
 
     /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers || !k->ioeventfd_assign) {
-        error_setg(errp,
-                   "device is incompatible with dataplane "
-                   "(transport does not support notifiers)");
-        return;
-    }
+    if (conf->iothread) {
+        if (!k->set_guest_notifiers || !k->ioeventfd_assign) {
+            error_setg(errp,
+                       "device is incompatible with dataplane "
+                       "(transport does not support notifiers)");
+            return;
+        }
 
-    /* If dataplane is (re-)enabled while the guest is running there could be
-     * block jobs that can conflict.
-     */
-    if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
-        error_prepend(errp, "cannot start dataplane thread: ");
-        return;
+        /* If dataplane is (re-)enabled while the guest is running there could
+         * be block jobs that can conflict.
+         */
+        if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
+            error_prepend(errp, "cannot start dataplane thread: ");
+            return;
+        }
     }
 
     s = g_new0(VirtIOBlockDataPlane, 1);
     s->vdev = vdev;
     s->conf = conf;
 
-    s->iothread = conf->iothread;
-    object_ref(OBJECT(s->iothread));
-    s->ctx = iothread_get_aio_context(s->iothread);
+    if (conf->iothread) {
+        s->iothread = conf->iothread;
+        object_ref(OBJECT(s->iothread));
+        s->ctx = iothread_get_aio_context(s->iothread);
+    } else {
+        s->ctx = qemu_get_aio_context();
+    }
     s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
     s->batch_notify_vqs = bitmap_new(conf->num_queues);
 
@@ -124,14 +130,18 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
 /* Context: QEMU global mutex held */
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
 {
+    VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
+
     if (!s) {
         return;
     }
 
-    virtio_blk_data_plane_stop(s);
+    assert(!vblk->dataplane_started);
     g_free(s->batch_notify_vqs);
     qemu_bh_delete(s->bh);
-    object_unref(OBJECT(s->iothread));
+    if (s->iothread) {
+        object_unref(OBJECT(s->iothread));
+    }
     g_free(s);
 }
 
@@ -147,17 +157,18 @@ static void virtio_blk_data_plane_handle_output(VirtIODevice *vdev,
 }
 
 /* Context: QEMU global mutex held */
-void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
+int virtio_blk_data_plane_start(VirtIODevice *vdev)
 {
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
+    VirtIOBlock *vblk = VIRTIO_BLK(vdev);
+    VirtIOBlockDataPlane *s = vblk->dataplane;
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vblk)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
     unsigned i;
     unsigned nvqs = s->conf->num_queues;
     int r;
 
     if (vblk->dataplane_started || s->starting) {
-        return;
+        return 0;
     }
 
     s->starting = true;
@@ -204,20 +215,22 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
                 virtio_blk_data_plane_handle_output);
     }
     aio_context_release(s->ctx);
-    return;
+    return 0;
 
   fail_guest_notifiers:
     vblk->dataplane_disabled = true;
     s->starting = false;
     vblk->dataplane_started = true;
+    return -ENOSYS;
 }
 
 /* Context: QEMU global mutex held */
-void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
+void virtio_blk_data_plane_stop(VirtIODevice *vdev)
 {
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
+    VirtIOBlock *vblk = VIRTIO_BLK(vdev);
+    VirtIOBlockDataPlane *s = vblk->dataplane;
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vblk));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
     unsigned i;
     unsigned nvqs = s->conf->num_queues;
 
diff --git a/hw/block/dataplane/virtio-blk.h b/hw/block/dataplane/virtio-blk.h
index b1f0b95..db3f47b 100644
--- a/hw/block/dataplane/virtio-blk.h
+++ b/hw/block/dataplane/virtio-blk.h
@@ -23,9 +23,9 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
                                   VirtIOBlockDataPlane **dataplane,
                                   Error **errp);
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s);
-void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s);
-void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s);
-void virtio_blk_data_plane_drain(VirtIOBlockDataPlane *s);
 void virtio_blk_data_plane_notify(VirtIOBlockDataPlane *s, VirtQueue *vq);
 
+int virtio_blk_data_plane_start(VirtIODevice *vdev);
+void virtio_blk_data_plane_stop(VirtIODevice *vdev);
+
 #endif /* HW_DATAPLANE_VIRTIO_BLK_H */
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 331d766..9f09e29 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -604,7 +604,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
         /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
          * dataplane here instead of waiting for .set_status().
          */
-        virtio_blk_data_plane_start(s->dataplane);
+        virtio_device_start_ioeventfd(vdev);
         if (!s->dataplane_disabled) {
             return;
         }
@@ -668,11 +668,9 @@ static void virtio_blk_reset(VirtIODevice *vdev)
         virtio_blk_free_request(req);
     }
 
-    if (s->dataplane) {
-        virtio_blk_data_plane_stop(s->dataplane);
-    }
     aio_context_release(ctx);
 
+    assert(!s->dataplane_started);
     blk_set_enable_write_cache(s->blk, s->original_wce);
 }
 
@@ -770,11 +768,7 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
 {
     VirtIOBlock *s = VIRTIO_BLK(vdev);
 
-    if (s->dataplane && !(status & (VIRTIO_CONFIG_S_DRIVER |
-                                    VIRTIO_CONFIG_S_DRIVER_OK))) {
-        virtio_blk_data_plane_stop(s->dataplane);
-    }
-
+    assert(!s->dataplane_started);
     if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
         return;
     }
@@ -915,7 +909,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     s->sector_mask = (s->conf.conf.logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
     for (i = 0; i < conf->num_queues; i++) {
-        virtio_add_queue_aio(vdev, 128, virtio_blk_handle_output);
+        virtio_add_queue(vdev, 128, virtio_blk_handle_output);
     }
     virtio_blk_data_plane_create(vdev, conf, &s->dataplane, &err);
     if (err != NULL) {
@@ -990,6 +984,8 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data)
     vdc->reset = virtio_blk_reset;
     vdc->save = virtio_blk_save_device;
     vdc->load = virtio_blk_load_device;
+    vdc->start_ioeventfd = virtio_blk_data_plane_start;
+    vdc->stop_ioeventfd = virtio_blk_data_plane_stop;
 }
 
 static const TypeInfo virtio_device_info = {
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index f60efd7..05af529 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -218,7 +218,7 @@ void virtio_bus_stop_ioeventfd(VirtioBusState *bus)
     bus->ioeventfd_started = false;
 }
 
-bool virtio_bus_ioeventfd_enabled(VirtIODevice *vdev)
+bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus)
 {
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
     DeviceState *proxy = DEVICE(BUS(bus)->parent);
-- 
2.7.4

[Qemu-devel] [PATCH 06/12] virtio-scsi: always use dataplane path if ioeventfd is active

[Paolo Bonzini]

Override start_ioeventfd and stop_ioeventfd to start/stop the
whole dataplane logic.  This has some positive side effects:

- no need anymore for virtio_add_queue_aio (i.e. a revert of
  commit 1c627137c10ee2dcf59e0383ade8a9abfa2d4355)

- no need anymore to switch from generic ioeventfd handlers to
  dataplane

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi-dataplane.c | 51 ++++++++++++++++++++++++-----------------
 hw/scsi/virtio-scsi.c           | 24 +++++++++----------
 include/hw/virtio/virtio-scsi.h |  6 ++---
 3 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index f537b4e..619d796 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/virtio/virtio-scsi.h"
 #include "qemu/error-report.h"
 #include "sysemu/block-backend.h"
@@ -21,20 +22,27 @@
 #include "hw/virtio/virtio-access.h"
 
 /* Context: QEMU global mutex held */
-void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
+void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp)
 {
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
-    assert(!s->ctx);
-    s->ctx = iothread_get_aio_context(vs->conf.iothread);
+    if (!virtio_device_ioeventfd_enabled(vdev)) {
+        return;
+    }
 
-    /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers || !k->ioeventfd_assign) {
-        fprintf(stderr, "virtio-scsi: Failed to set iothread "
-                   "(transport does not support notifiers)");
-        exit(1);
+    if (vs->conf.iothread) {
+        if (!k->set_guest_notifiers || !k->ioeventfd_assign) {
+            error_setg(errp,
+                       "device is incompatible with dataplane "
+                       "(transport does not support notifiers)");
+            return;
+        }
+        s->ctx = iothread_get_aio_context(vs->conf.iothread);
+    } else {
+        s->ctx = qemu_get_aio_context();
     }
 }
 
@@ -105,19 +113,19 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
 }
 
 /* Context: QEMU global mutex held */
-void virtio_scsi_dataplane_start(VirtIOSCSI *s)
+int virtio_scsi_dataplane_start(VirtIODevice *vdev)
 {
     int i;
     int rc;
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
+    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
 
     if (s->dataplane_started ||
         s->dataplane_starting ||
-        s->dataplane_fenced ||
-        s->ctx != iothread_get_aio_context(vs->conf.iothread)) {
-        return;
+        s->dataplane_fenced) {
+        return 0;
     }
 
     s->dataplane_starting = true;
@@ -152,7 +160,7 @@ void virtio_scsi_dataplane_start(VirtIOSCSI *s)
     s->dataplane_starting = false;
     s->dataplane_started = true;
     aio_context_release(s->ctx);
-    return;
+    return 0;
 
 fail_vrings:
     virtio_scsi_clear_aio(s);
@@ -165,14 +173,16 @@ fail_guest_notifiers:
     s->dataplane_fenced = true;
     s->dataplane_starting = false;
     s->dataplane_started = true;
+    return -ENOSYS;
 }
 
 /* Context: QEMU global mutex held */
-void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
+void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
 {
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
+    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
     int i;
 
     if (!s->dataplane_started || s->dataplane_stopping) {
@@ -186,7 +196,6 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
         return;
     }
     s->dataplane_stopping = true;
-    assert(s->ctx == iothread_get_aio_context(vs->conf.iothread));
 
     aio_context_acquire(s->ctx);
 
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index ce57ef6..e8eda7c 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -426,7 +426,7 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     VirtIOSCSI *s = (VirtIOSCSI *)vdev;
 
     if (s->ctx) {
-        virtio_scsi_dataplane_start(s);
+        virtio_device_start_ioeventfd(vdev);
         if (!s->dataplane_fenced) {
             return;
         }
@@ -590,7 +590,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
     VirtIOSCSI *s = (VirtIOSCSI *)vdev;
 
     if (s->ctx) {
-        virtio_scsi_dataplane_start(s);
+        virtio_device_start_ioeventfd(vdev);
         if (!s->dataplane_fenced) {
             return;
         }
@@ -648,9 +648,7 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
     VirtIOSCSI *s = VIRTIO_SCSI(vdev);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
 
-    if (s->ctx) {
-        virtio_scsi_dataplane_stop(s);
-    }
+    assert(!s->dataplane_started);
     s->resetting++;
     qbus_reset_all(&s->bus.qbus);
     s->resetting--;
@@ -743,7 +741,7 @@ static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
     VirtIOSCSI *s = VIRTIO_SCSI(vdev);
 
     if (s->ctx) {
-        virtio_scsi_dataplane_start(s);
+        virtio_device_start_ioeventfd(vdev);
         if (!s->dataplane_fenced) {
             return;
         }
@@ -842,14 +840,10 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
     s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
     s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE;
 
-    s->ctrl_vq = virtio_add_queue_aio(vdev, VIRTIO_SCSI_VQ_SIZE, ctrl);
-    s->event_vq = virtio_add_queue_aio(vdev, VIRTIO_SCSI_VQ_SIZE, evt);
+    s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, ctrl);
+    s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, evt);
     for (i = 0; i < s->conf.num_queues; i++) {
-        s->cmd_vqs[i] = virtio_add_queue_aio(vdev, VIRTIO_SCSI_VQ_SIZE, cmd);
-    }
-
-    if (s->conf.iothread) {
-        virtio_scsi_set_iothread(VIRTIO_SCSI(s), s->conf.iothread);
+        s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, cmd);
     }
 }
 
@@ -879,6 +873,8 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
+
+    virtio_scsi_dataplane_setup(s, errp);
 }
 
 static void virtio_scsi_instance_init(Object *obj)
@@ -943,6 +939,8 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
     vdc->set_config = virtio_scsi_set_config;
     vdc->get_features = virtio_scsi_get_features;
     vdc->reset = virtio_scsi_reset;
+    vdc->start_ioeventfd = virtio_scsi_dataplane_start;
+    vdc->stop_ioeventfd = virtio_scsi_dataplane_stop;
     hc->plug = virtio_scsi_hotplug;
     hc->unplug = virtio_scsi_hotunplug;
 }
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index a1e0cfb..9fbc7d7 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -134,9 +134,9 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req);
 void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
                             uint32_t event, uint32_t reason);
 
-void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread);
-void virtio_scsi_dataplane_start(VirtIOSCSI *s);
-void virtio_scsi_dataplane_stop(VirtIOSCSI *s);
+void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp);
+int virtio_scsi_dataplane_start(VirtIODevice *s);
+void virtio_scsi_dataplane_stop(VirtIODevice *s);
 void virtio_scsi_dataplane_notify(VirtIODevice *vdev, VirtIOSCSIReq *req);
 
 #endif /* QEMU_VIRTIO_SCSI_H */
-- 
2.7.4

[Qemu-devel] [PATCH 07/12] Revert "virtio: Introduce virtio_add_queue_aio"

[Paolo Bonzini]

This reverts commit 872dd82c83745a603d2e07a03d34313eb6467ae4.
virtio_add_queue_aio is unused.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio.c         | 38 ++++----------------------------------
 include/hw/virtio/virtio.h |  3 ---
 2 files changed, 4 insertions(+), 37 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 84abd08..d1cc2f5 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -97,7 +97,6 @@ struct VirtQueue
     uint16_t vector;
     VirtIOHandleOutput handle_output;
     VirtIOHandleOutput handle_aio_output;
-    bool use_aio;
     VirtIODevice *vdev;
     EventNotifier guest_notifier;
     EventNotifier host_notifier;
@@ -1141,9 +1140,8 @@ void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector)
     }
 }
 
-static VirtQueue *virtio_add_queue_internal(VirtIODevice *vdev, int queue_size,
-                                            VirtIOHandleOutput handle_output,
-                                            bool use_aio)
+VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
+                            VirtIOHandleOutput handle_output)
 {
     int i;
 
@@ -1160,28 +1158,10 @@ static VirtQueue *virtio_add_queue_internal(VirtIODevice *vdev, int queue_size,
     vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
     vdev->vq[i].handle_output = handle_output;
     vdev->vq[i].handle_aio_output = NULL;
-    vdev->vq[i].use_aio = use_aio;
 
     return &vdev->vq[i];
 }
 
-/* Add a virt queue and mark AIO.
- * An AIO queue will use the AioContext based event interface instead of the
- * default IOHandler and EventNotifier interface.
- */
-VirtQueue *virtio_add_queue_aio(VirtIODevice *vdev, int queue_size,
-                                VirtIOHandleOutput handle_output)
-{
-    return virtio_add_queue_internal(vdev, queue_size, handle_output, true);
-}
-
-/* Add a normal virt queue (on the contrary to the AIO version above. */
-VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
-                            VirtIOHandleOutput handle_output)
-{
-    return virtio_add_queue_internal(vdev, queue_size, handle_output, false);
-}
-
 void virtio_del_queue(VirtIODevice *vdev, int n)
 {
     if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
@@ -1866,21 +1846,11 @@ static void virtio_queue_host_notifier_read(EventNotifier *n)
 void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                bool set_handler)
 {
-    AioContext *ctx = qemu_get_aio_context();
     if (assign && set_handler) {
-        if (vq->use_aio) {
-            aio_set_event_notifier(ctx, &vq->host_notifier, true,
+        event_notifier_set_handler(&vq->host_notifier, true,
                                    virtio_queue_host_notifier_read);
-        } else {
-            event_notifier_set_handler(&vq->host_notifier, true,
-                                       virtio_queue_host_notifier_read);
-        }
     } else {
-        if (vq->use_aio) {
-            aio_set_event_notifier(ctx, &vq->host_notifier, true, NULL);
-        } else {
-            event_notifier_set_handler(&vq->host_notifier, true, NULL);
-        }
+        event_notifier_set_handler(&vq->host_notifier, true, NULL);
     }
     if (!assign) {
         /* Test and clear notifier before after disabling event,
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index ec50f7a..1db4fbf 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -145,9 +145,6 @@ typedef void (*VirtIOHandleOutput)(VirtIODevice *, VirtQueue *);
 VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
                             VirtIOHandleOutput handle_output);
 
-VirtQueue *virtio_add_queue_aio(VirtIODevice *vdev, int queue_size,
-                                VirtIOHandleOutput handle_output);
-
 void virtio_del_queue(VirtIODevice *vdev, int n);
 
 void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num);
-- 
2.7.4

[Qemu-devel] [PATCH 08/12] virtio: remove set_handler argument from set_host_notifier_internal

[Paolo Bonzini]

Make virtio_device_start_ioeventfd_impl use the same logic as
dataplane to set up the host notifier.  This removes the need
for the set_handler argument in set_host_notifier_internal.

This is a first step towards using virtio_bus_set_host_notifier
as the sole entry point to set up ioeventfds.  At least now
the functions have the same interface, but they still differ
in that virtio_bus_set_host_notifier sets ioeventfd_disabled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c | 12 +++---------
 hw/virtio/virtio.c     | 16 +++++++++++++---
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 05af529..c196ba1 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -146,14 +146,8 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
     }
 }
 
-/*
- * This function handles both assigning the ioeventfd handler and
- * registering it with the kernel.
- * assign: register/deregister ioeventfd with the kernel
- * set_handler: use the generic ioeventfd handler
- */
 int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
-                               int n, bool assign, bool set_handler)
+                               int n, bool assign)
 {
     VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
@@ -167,7 +161,7 @@ int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
             error_report("%s: unable to init event notifier: %d", __func__, r);
             return r;
         }
-        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
+        virtio_queue_set_host_notifier_fd_handler(vq, true, false);
         r = k->ioeventfd_assign(proxy, notifier, n, assign);
         if (r < 0) {
             error_report("%s: unable to assign ioeventfd: %d", __func__, r);
@@ -252,7 +246,7 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
          */
         virtio_bus_stop_ioeventfd(bus);
     }
-    return set_host_notifier_internal(proxy, bus, n, assign, false);
+    return set_host_notifier_internal(proxy, bus, n, assign);
 }
 
 static char *virtio_bus_get_dev_path(DeviceState *dev)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d1cc2f5..e7b7d0a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1926,11 +1926,21 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
-        r = set_host_notifier_internal(proxy, qbus, n, true, true);
+        r = set_host_notifier_internal(proxy, qbus, n, true);
         if (r < 0) {
             err = r;
             goto assign_error;
         }
+        virtio_queue_set_host_notifier_fd_handler(&vdev->vq[n], true, true);
+    }
+
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        /* Kick right away to begin processing requests already in vring */
+        VirtQueue *vq = &vdev->vq[n];;
+        if (!vq->vring.num) {
+            continue;
+        }
+        event_notifier_set(&vq->host_notifier);
     }
     return 0;
 
@@ -1940,7 +1950,7 @@ assign_error:
             continue;
         }
 
-        r = set_host_notifier_internal(proxy, qbus, n, false, false);
+        r = set_host_notifier_internal(proxy, qbus, n, false);
         assert(r >= 0);
     }
     return err;
@@ -1964,7 +1974,7 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
-        r = set_host_notifier_internal(proxy, qbus, n, false, false);
+        r = set_host_notifier_internal(proxy, qbus, n, false);
         assert(r >= 0);
     }
 }
-- 
2.7.4

[Qemu-devel] [PATCH 09/12] virtio: remove ioeventfd_disabled altogether

[Paolo Bonzini]

Now that there is not anymore a switch from the generic ioeventfd handler
to the dataplane handler, virtio_bus_set_host_notifier(assign=true) is
always called with !bus->ioeventfd_started, hence virtio_bus_stop_ioeventfd
does nothing in this case.  Move the invocation to vhost.c, which is the
only place that needs it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/vhost.c              |  3 +++
 hw/virtio/virtio-bus.c         | 23 ++++++++---------------
 include/hw/virtio/virtio-bus.h |  6 ------
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index c5a1fe0..284bee1 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1182,6 +1182,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
         goto fail;
     }
 
+    virtio_device_stop_ioeventfd(vdev);
     for (i = 0; i < hdev->nvqs; ++i) {
         r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
                                          true);
@@ -1201,6 +1202,7 @@ fail_vq:
         }
         assert (e >= 0);
     }
+    virtio_device_start_ioeventfd(vdev);
 fail:
     return r;
 }
@@ -1223,6 +1225,7 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
         }
         assert (r >= 0);
     }
+    virtio_device_start_ioeventfd(vdev);
 }
 
 /* Test and clear event pending status.
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index c196ba1..ef8f622 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -188,7 +188,7 @@ int virtio_bus_start_ioeventfd(VirtioBusState *bus)
     if (!k->ioeventfd_assign || !k->ioeventfd_enabled(proxy)) {
         return -ENOSYS;
     }
-    if (bus->ioeventfd_started || bus->ioeventfd_disabled) {
+    if (bus->ioeventfd_started) {
         return 0;
     }
     r = vdc->start_ioeventfd(vdev);
@@ -221,8 +221,8 @@ bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus)
 }
 
 /*
- * This function switches from/to the generic ioeventfd handler.
- * assign==false means 'use generic ioeventfd handler'.
+ * This function switches ioeventfd on/off in the device.
+ * The caller must set or clear the handlers for the EventNotifier.
  */
 int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
 {
@@ -232,19 +232,12 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
     if (!k->ioeventfd_assign) {
         return -ENOSYS;
     }
-    bus->ioeventfd_disabled = assign;
     if (assign) {
-        /*
-         * Stop using the generic ioeventfd, we are doing eventfd handling
-         * ourselves below
-         *
-         * FIXME: We should just switch the handler and not deassign the
-         * ioeventfd.
-         * Otherwise, there's a window where we don't have an
-         * ioeventfd and we may end up with a notification where
-         * we don't expect one.
-         */
-        virtio_bus_stop_ioeventfd(bus);
+        assert(!bus->ioeventfd_started);
+    } else {
+        if (!bus->ioeventfd_started) {
+            return 0;
+        }
     }
     return set_host_notifier_internal(proxy, bus, n, assign);
 }
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 0abc890..b812cad 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -94,12 +94,6 @@ struct VirtioBusState {
     BusState parent_obj;
 
     /*
-     * Set if the default ioeventfd handlers are disabled by vhost
-     * or dataplane.
-     */
-    bool ioeventfd_disabled;
-
-    /*
      * Set if ioeventfd has been started.
      */
     bool ioeventfd_started;
-- 
2.7.4

[Qemu-devel] [PATCH 10/12] virtio: do not export set_host_notifier_internal

[Paolo Bonzini]

ioeventfd_disabled was the only reason for the default
implementation of virtio_device_start_ioeventfd_impl not to use
virtio_bus_set_host_notifier.  This is now fixed, and the sole entry
point to set up ioeventfd can now be virtio_bus_set_host_notifier.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c         | 4 ++--
 hw/virtio/virtio.c             | 8 +++-----
 include/hw/virtio/virtio-bus.h | 5 -----
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index ef8f622..289bf86 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -146,8 +146,8 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
     }
 }
 
-int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
-                               int n, bool assign)
+static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
+                                      int n, bool assign)
 {
     VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index e7b7d0a..8abcd3d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1919,14 +1919,13 @@ static Property virtio_properties[] = {
 static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
 {
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    DeviceState *proxy = DEVICE(BUS(qbus)->parent);
     int n, r, err;
 
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
-        r = set_host_notifier_internal(proxy, qbus, n, true);
+        r = virtio_bus_set_host_notifier(qbus, n, true);
         if (r < 0) {
             err = r;
             goto assign_error;
@@ -1950,7 +1949,7 @@ assign_error:
             continue;
         }
 
-        r = set_host_notifier_internal(proxy, qbus, n, false);
+        r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
     return err;
@@ -1967,14 +1966,13 @@ int virtio_device_start_ioeventfd(VirtIODevice *vdev)
 static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
 {
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    DeviceState *proxy = DEVICE(BUS(qbus)->parent);
     int n, r;
 
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
-        r = set_host_notifier_internal(proxy, qbus, n, false);
+        r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
 }
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index b812cad..209c418 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -137,9 +137,4 @@ void virtio_bus_stop_ioeventfd(VirtioBusState *bus);
 /* Switch from/to the generic ioeventfd handler */
 int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
 
-/* This is temporary.  It is only needed because virtio_bus_set_host_notifier
- * sets ioeventfd_disabled but we will shortly get rid of it.  */
-int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
-                               int n, bool assign, bool set_handler);
-
 #endif /* VIRTIO_BUS_H */
-- 
2.7.4

[Qemu-devel] [PATCH 11/12] virtio: inline virtio_queue_set_host_notifier_fd_handler

[Paolo Bonzini]

Of the three possible parameter combinations for
virtio_queue_set_host_notifier_fd_handler:

- assign=true/set_handler=true is only called from
  virtio_device_start_ioeventfd

- assign=false/set_handler=false is called from
  set_host_notifier_internal but it only does something when
  reached from virtio_device_stop_ioeventfd_impl; otherwise
  there is no EventNotifier set on qemu_get_aio_context().

- assign=true/set_handler=false is called from
  set_host_notifier_internal, but it is not doing anything:
  with the new start_ioeventfd and stop_ioeventfd methods,
  there is never an EventNotifier set on qemu_get_aio_context()
  at this point.  This is enforced by the assertion in
  virtio_bus_set_host_notifier.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c     | 19 +++++++++++--------
 hw/virtio/virtio.c         | 27 +++++++++------------------
 include/hw/virtio/virtio.h |  3 +--
 3 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 289bf86..b8f0527 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -161,19 +161,22 @@ static int set_host_notifier_internal(DeviceState *proxy, VirtioBusState *bus,
             error_report("%s: unable to init event notifier: %d", __func__, r);
             return r;
         }
-        virtio_queue_set_host_notifier_fd_handler(vq, true, false);
-        r = k->ioeventfd_assign(proxy, notifier, n, assign);
+        r = k->ioeventfd_assign(proxy, notifier, n, true);
         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;
+            goto cleanup_event_notifier;
         }
+        return 0;
     } else {
-        k->ioeventfd_assign(proxy, notifier, n, assign);
-        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
-        event_notifier_cleanup(notifier);
+        k->ioeventfd_assign(proxy, notifier, n, false);
     }
+
+cleanup_event_notifier:
+    /* Test and clear notifier before after disabling event,
+     * in case poll callback didn't have time to run.
+     */
+    virtio_queue_host_notifier_read(notifier);
+    event_notifier_cleanup(notifier);
     return r;
 }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8abcd3d..f11ab10 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1835,7 +1835,7 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
     }
 }
 
-static void virtio_queue_host_notifier_read(EventNotifier *n)
+void virtio_queue_host_notifier_read(EventNotifier *n)
 {
     VirtQueue *vq = container_of(n, VirtQueue, host_notifier);
     if (event_notifier_test_and_clear(n)) {
@@ -1843,22 +1843,6 @@ static void virtio_queue_host_notifier_read(EventNotifier *n)
     }
 }
 
-void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
-                                               bool set_handler)
-{
-    if (assign && set_handler) {
-        event_notifier_set_handler(&vq->host_notifier, true,
-                                   virtio_queue_host_notifier_read);
-    } else {
-        event_notifier_set_handler(&vq->host_notifier, true, NULL);
-    }
-    if (!assign) {
-        /* Test and clear notifier before after disabling event,
-         * in case poll callback didn't have time to run. */
-        virtio_queue_host_notifier_read(&vq->host_notifier);
-    }
-}
-
 EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
 {
     return &vq->host_notifier;
@@ -1922,6 +1906,7 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
     int n, r, err;
 
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        VirtQueue *vq = &vdev->vq[n];
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
@@ -1930,7 +1915,8 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
             err = r;
             goto assign_error;
         }
-        virtio_queue_set_host_notifier_fd_handler(&vdev->vq[n], true, true);
+        event_notifier_set_handler(&vq->host_notifier, true,
+                                   virtio_queue_host_notifier_read);
     }
 
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
@@ -1945,10 +1931,12 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
 
 assign_error:
     while (--n >= 0) {
+        VirtQueue *vq = &vdev->vq[n];
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
 
+        event_notifier_set_handler(&vq->host_notifier, true, NULL);
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
@@ -1969,9 +1957,12 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
     int n, r;
 
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        VirtQueue *vq = &vdev->vq[n];
+
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
+        event_notifier_set_handler(&vq->host_notifier, true, NULL);
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 1db4fbf..ac121b1 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -273,8 +273,7 @@ int virtio_device_start_ioeventfd(VirtIODevice *vdev);
 void virtio_device_stop_ioeventfd(VirtIODevice *vdev);
 bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
 EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
-void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
-                                               bool set_handler);
+void virtio_queue_host_notifier_read(EventNotifier *n);
 void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
                                                 void (*fn)(VirtIODevice *,
                                                            VirtQueue *));
-- 
2.7.4

[Qemu-devel] [PATCH 12/12] virtio: inline set_host_notifier_internal

[Paolo Bonzini]

This is only called from virtio_bus_set_host_notifier.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c | 60 +++++++++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 35 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index b8f0527..1f3526d 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -146,40 +146,6 @@ 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)
-{
-    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;
-        }
-        r = k->ioeventfd_assign(proxy, notifier, n, true);
-        if (r < 0) {
-            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
-            goto cleanup_event_notifier;
-        }
-        return 0;
-    } else {
-        k->ioeventfd_assign(proxy, notifier, n, false);
-    }
-
-cleanup_event_notifier:
-    /* Test and clear notifier before after disabling event,
-     * in case poll callback didn't have time to run.
-     */
-    virtio_queue_host_notifier_read(notifier);
-    event_notifier_cleanup(notifier);
-    return r;
-}
-
 int virtio_bus_start_ioeventfd(VirtioBusState *bus)
 {
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
@@ -229,20 +195,44 @@ bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus)
  */
 int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
     DeviceState *proxy = DEVICE(BUS(bus)->parent);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
+    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+    int r = 0;
 
     if (!k->ioeventfd_assign) {
         return -ENOSYS;
     }
+
     if (assign) {
         assert(!bus->ioeventfd_started);
+        r = event_notifier_init(notifier, 1);
+        if (r < 0) {
+            error_report("%s: unable to init event notifier: %d", __func__, r);
+            return r;
+        }
+        r = k->ioeventfd_assign(proxy, notifier, n, true);
+        if (r < 0) {
+            error_report("%s: unable to assign ioeventfd: %d", __func__, r);
+            goto cleanup_event_notifier;
+        }
+        return 0;
     } else {
         if (!bus->ioeventfd_started) {
             return 0;
         }
+        k->ioeventfd_assign(proxy, notifier, n, false);
     }
-    return set_host_notifier_internal(proxy, bus, n, assign);
+
+cleanup_event_notifier:
+    /* Test and clear notifier before after disabling event,
+     * in case poll callback didn't have time to run.
+     */
+    virtio_queue_host_notifier_read(notifier);
+    event_notifier_cleanup(notifier);
+    return r;
 }
 
 static char *virtio_bus_get_dev_path(DeviceState *dev)
-- 
2.7.4

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Christian Borntraeger]

On 09/21/2016 03:18 PM, Paolo Bonzini wrote:
> This series started as an attempt to always use the dataplane path
> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
> was three-fold:
> 
> 1) to add more coverage for dataplane
> 
> 2) to remove virtio_add_queue_aio
> 
> 3) to simplify the dataplane start/stop code
> 
> It achieves the first two objectives, and while it doesn't quite
> achieve the third it does cleanup the generic ioeventfd code in
> virtio-bus more than I expected.  In particular, it reduces the set
> of callbacks that transports must implement, and it removes the ugly
> case where ioeventfd is started with generic callbacks and then moved
> to the dataplane callbacks.  It also enables some simplification of the
> functions that deal with host notifiers.
> 
> I've tested it with virtio-blk, virtio-scsi and vhost-net.
> 
> Patches 1 and 2 are simplifications that are too nice to leave
> them for later in the series.
> 
> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
> virtio.c.  At this point the transition is a bit half-assed, but
> this changes as soon as we remove the generic->dataplane
> handler transition.
> 
> Patches 4 to 6 do exactly that, and then the spring cleaning
> begins, lasting for the whole second half of the series.
> 
> Opinions, reviews and bug reports?

is there a branch?

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

On 21/09/2016 16:01, Christian Borntraeger wrote:
> On 09/21/2016 03:18 PM, Paolo Bonzini wrote:
>> This series started as an attempt to always use the dataplane path
>> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
>> was three-fold:
>>
>> 1) to add more coverage for dataplane
>>
>> 2) to remove virtio_add_queue_aio
>>
>> 3) to simplify the dataplane start/stop code
>>
>> It achieves the first two objectives, and while it doesn't quite
>> achieve the third it does cleanup the generic ioeventfd code in
>> virtio-bus more than I expected.  In particular, it reduces the set
>> of callbacks that transports must implement, and it removes the ugly
>> case where ioeventfd is started with generic callbacks and then moved
>> to the dataplane callbacks.  It also enables some simplification of the
>> functions that deal with host notifiers.
>>
>> I've tested it with virtio-blk, virtio-scsi and vhost-net.
>>
>> Patches 1 and 2 are simplifications that are too nice to leave
>> them for later in the series.
>>
>> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
>> virtio.c.  At this point the transition is a bit half-assed, but
>> this changes as soon as we remove the generic->dataplane
>> handler transition.
>>
>> Patches 4 to 6 do exactly that, and then the spring cleaning
>> begins, lasting for the whole second half of the series.
>>
>> Opinions, reviews and bug reports?
> 
> is there a branch?

ioeventfd-virtio in my github repo.

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Michael S. Tsirkin]

On Wed, Sep 21, 2016 at 03:18:47PM +0200, Paolo Bonzini wrote:
> This series started as an attempt to always use the dataplane path
> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
> was three-fold:
> 
> 1) to add more coverage for dataplane
> 
> 2) to remove virtio_add_queue_aio
> 
> 3) to simplify the dataplane start/stop code
> 
> It achieves the first two objectives, and while it doesn't quite
> achieve the third it does cleanup the generic ioeventfd code in
> virtio-bus more than I expected.  In particular, it reduces the set
> of callbacks that transports must implement, and it removes the ugly
> case where ioeventfd is started with generic callbacks and then moved
> to the dataplane callbacks.  It also enables some simplification of the
> functions that deal with host notifiers.
> 
> I've tested it with virtio-blk, virtio-scsi and vhost-net.
> 
> Patches 1 and 2 are simplifications that are too nice to leave
> them for later in the series.
> 
> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
> virtio.c.  At this point the transition is a bit half-assed, but
> this changes as soon as we remove the generic->dataplane
> handler transition.
> 
> Patches 4 to 6 do exactly that, and then the spring cleaning
> begins, lasting for the whole second half of the series.
> 
> Opinions, reviews and bug reports?
> 
> Thanks,
> 
> Paolo

Overall this looks good, I'll try to review early next week.
Thanks!

> Paolo Bonzini (12):
>   virtio: move ioeventfd_disabled flag to VirtioBusState
>   virtio: move ioeventfd_started flag to VirtioBusState
>   virtio: add start_ioeventfd and stop_ioeventfd to VirtioDeviceClass
>   virtio: introduce virtio_device_ioeventfd_enabled
>   virtio-blk: always use dataplane path if ioeventfd is active
>   virtio-scsi: always use dataplane path if ioeventfd is active
>   Revert "virtio: Introduce virtio_add_queue_aio"
>   virtio: remove set_handler argument from set_host_notifier_internal
>   virtio: remove ioeventfd_disabled altogether
>   virtio: do not export set_host_notifier_internal
>   virtio: inline virtio_queue_set_host_notifier_fd_handler
>   virtio: inline set_host_notifier_internal
> 
>  hw/block/dataplane/virtio-blk.c |  67 +++++++++++-------
>  hw/block/dataplane/virtio-blk.h |   6 +-
>  hw/block/virtio-blk.c           |  16 ++---
>  hw/s390x/virtio-ccw.c           |  36 +---------
>  hw/s390x/virtio-ccw.h           |   2 -
>  hw/scsi/virtio-scsi-dataplane.c |  51 ++++++++------
>  hw/scsi/virtio-scsi.c           |  24 +++----
>  hw/virtio/vhost.c               |   5 +-
>  hw/virtio/virtio-bus.c          | 153 +++++++++++++++-------------------------
>  hw/virtio/virtio-mmio.c         |  35 +--------
>  hw/virtio/virtio-pci.c          |  32 +--------
>  hw/virtio/virtio-pci.h          |   2 -
>  hw/virtio/virtio.c              | 139 +++++++++++++++++++++++-------------
>  include/hw/virtio/virtio-bus.h  |  27 ++++---
>  include/hw/virtio/virtio-scsi.h |   6 +-
>  include/hw/virtio/virtio.h      |  11 +--
>  16 files changed, 272 insertions(+), 340 deletions(-)
> 
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Christian Borntraeger]

On 09/21/2016 06:43 PM, Paolo Bonzini wrote:
> 
> 
> On 21/09/2016 16:01, Christian Borntraeger wrote:
>> On 09/21/2016 03:18 PM, Paolo Bonzini wrote:
>>> This series started as an attempt to always use the dataplane path
>>> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
>>> was three-fold:
>>>
>>> 1) to add more coverage for dataplane
>>>
>>> 2) to remove virtio_add_queue_aio
>>>
>>> 3) to simplify the dataplane start/stop code
>>>
>>> It achieves the first two objectives, and while it doesn't quite
>>> achieve the third it does cleanup the generic ioeventfd code in
>>> virtio-bus more than I expected.  In particular, it reduces the set
>>> of callbacks that transports must implement, and it removes the ugly
>>> case where ioeventfd is started with generic callbacks and then moved
>>> to the dataplane callbacks.  It also enables some simplification of the
>>> functions that deal with host notifiers.
>>>
>>> I've tested it with virtio-blk, virtio-scsi and vhost-net.
>>>
>>> Patches 1 and 2 are simplifications that are too nice to leave
>>> them for later in the series.
>>>
>>> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
>>> virtio.c.  At this point the transition is a bit half-assed, but
>>> this changes as soon as we remove the generic->dataplane
>>> handler transition.
>>>
>>> Patches 4 to 6 do exactly that, and then the spring cleaning
>>> begins, lasting for the whole second half of the series.
>>>
>>> Opinions, reviews and bug reports?
>>
>> is there a branch?
> 
> ioeventfd-virtio in my github repo.

Triggering
qemu-system-s390x: /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:771: virtio_blk_set_status: Assertion `!s->dataplane_started' failed.

Is this based on the old version that still had this bug?

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

On 27/09/2016 16:45, Christian Borntraeger wrote:
> On 09/21/2016 06:43 PM, Paolo Bonzini wrote:
>>
>>
>> On 21/09/2016 16:01, Christian Borntraeger wrote:
>>> On 09/21/2016 03:18 PM, Paolo Bonzini wrote:
>>>> This series started as an attempt to always use the dataplane path
>>>> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
>>>> was three-fold:
>>>>
>>>> 1) to add more coverage for dataplane
>>>>
>>>> 2) to remove virtio_add_queue_aio
>>>>
>>>> 3) to simplify the dataplane start/stop code
>>>>
>>>> It achieves the first two objectives, and while it doesn't quite
>>>> achieve the third it does cleanup the generic ioeventfd code in
>>>> virtio-bus more than I expected.  In particular, it reduces the set
>>>> of callbacks that transports must implement, and it removes the ugly
>>>> case where ioeventfd is started with generic callbacks and then moved
>>>> to the dataplane callbacks.  It also enables some simplification of the
>>>> functions that deal with host notifiers.
>>>>
>>>> I've tested it with virtio-blk, virtio-scsi and vhost-net.
>>>>
>>>> Patches 1 and 2 are simplifications that are too nice to leave
>>>> them for later in the series.
>>>>
>>>> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
>>>> virtio.c.  At this point the transition is a bit half-assed, but
>>>> this changes as soon as we remove the generic->dataplane
>>>> handler transition.
>>>>
>>>> Patches 4 to 6 do exactly that, and then the spring cleaning
>>>> begins, lasting for the whole second half of the series.
>>>>
>>>> Opinions, reviews and bug reports?
>>>
>>> is there a branch?
>>
>> ioeventfd-virtio in my github repo.
> 
> Triggering
> qemu-system-s390x: /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:771: virtio_blk_set_status: Assertion `!s->dataplane_started' failed.
> 
> Is this based on the old version that still had this bug?

No, it's a new assertion (though it looks the same as the old one),
and a stupid one too.  This is hopefully enough:

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9f09e29..52927f7 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -768,7 +768,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
 {
     VirtIOBlock *s = VIRTIO_BLK(vdev);
 
-    assert(!s->dataplane_started);
+    if (!(status & (VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK))) {
+        assert(!s->dataplane_started);
+    }
+
     if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
         return;
     }


Paolo

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Christian Borntraeger]

On 09/27/2016 06:44 PM, Paolo Bonzini wrote:
> 
> 
> On 27/09/2016 16:45, Christian Borntraeger wrote:
>> On 09/21/2016 06:43 PM, Paolo Bonzini wrote:
>>>
>>>
>>> On 21/09/2016 16:01, Christian Borntraeger wrote:
>>>> On 09/21/2016 03:18 PM, Paolo Bonzini wrote:
>>>>> This series started as an attempt to always use the dataplane path
>>>>> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
>>>>> was three-fold:
>>>>>
>>>>> 1) to add more coverage for dataplane
>>>>>
>>>>> 2) to remove virtio_add_queue_aio
>>>>>
>>>>> 3) to simplify the dataplane start/stop code
>>>>>
>>>>> It achieves the first two objectives, and while it doesn't quite
>>>>> achieve the third it does cleanup the generic ioeventfd code in
>>>>> virtio-bus more than I expected.  In particular, it reduces the set
>>>>> of callbacks that transports must implement, and it removes the ugly
>>>>> case where ioeventfd is started with generic callbacks and then moved
>>>>> to the dataplane callbacks.  It also enables some simplification of the
>>>>> functions that deal with host notifiers.
>>>>>
>>>>> I've tested it with virtio-blk, virtio-scsi and vhost-net.
>>>>>
>>>>> Patches 1 and 2 are simplifications that are too nice to leave
>>>>> them for later in the series.
>>>>>
>>>>> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
>>>>> virtio.c.  At this point the transition is a bit half-assed, but
>>>>> this changes as soon as we remove the generic->dataplane
>>>>> handler transition.
>>>>>
>>>>> Patches 4 to 6 do exactly that, and then the spring cleaning
>>>>> begins, lasting for the whole second half of the series.
>>>>>
>>>>> Opinions, reviews and bug reports?
>>>>
>>>> is there a branch?
>>>
>>> ioeventfd-virtio in my github repo.
>>
>> Triggering
>> qemu-system-s390x: /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:771: virtio_blk_set_status: Assertion `!s->dataplane_started' failed.
>>
>> Is this based on the old version that still had this bug?
> 
> No, it's a new assertion (though it looks the same as the old one),
> and a stupid one too.  This is hopefully enough:
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 9f09e29..52927f7 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -768,7 +768,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
>  {
>      VirtIOBlock *s = VIRTIO_BLK(vdev);
> 
> -    assert(!s->dataplane_started);
> +    if (!(status & (VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK))) {
> +        assert(!s->dataplane_started);
> +    }
> +
>      if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>          return;

seems to fix the assert. 

Re: [Qemu-devel] [PATCH 01/12] virtio: move ioeventfd_disabled flag to VirtioBusState

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:48PM +0200, Paolo Bonzini wrote:
> This simplifies the code and removes the ioeventfd_set_disabled
> callback.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/s390x/virtio-ccw.c          | 11 +----------
>  hw/s390x/virtio-ccw.h          |  1 -
>  hw/virtio/virtio-bus.c         |  4 ++--
>  hw/virtio/virtio-mmio.c        | 13 +------------
>  hw/virtio/virtio-pci.c         | 11 +----------
>  hw/virtio/virtio-pci.h         |  1 -
>  include/hw/virtio/virtio-bus.h |  8 ++++++--
>  7 files changed, 11 insertions(+), 38 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 02/12] virtio: move ioeventfd_started flag to VirtioBusState

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:49PM +0200, Paolo Bonzini wrote:
> This simplifies the code and removes the ioeventfd_started
> and ioeventfd_set_started callback.  The only difference is
> in how virtio-ccw handles an error---it doesn't disable
> ioeventfd forever anymore.  It was the only backend to do
> so, and if desired this behavior should be implemented in
> virtio-bus.c.
> 
> Instead of ioeventfd_started, the ioeventfd_assign callback now
> determines whether the virtio bus supports host notifiers.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/block/dataplane/virtio-blk.c |  2 +-
>  hw/s390x/virtio-ccw.c           | 21 ---------------------
>  hw/s390x/virtio-ccw.h           |  1 -
>  hw/scsi/virtio-scsi-dataplane.c |  2 +-
>  hw/virtio/vhost.c               |  2 +-
>  hw/virtio/virtio-bus.c          | 14 ++++++--------
>  hw/virtio/virtio-mmio.c         | 18 ------------------
>  hw/virtio/virtio-pci.c          | 17 -----------------
>  hw/virtio/virtio-pci.h          |  1 -
>  include/hw/virtio/virtio-bus.h  | 17 +++++++----------
>  10 files changed, 16 insertions(+), 79 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 03/12] virtio: add start_ioeventfd and stop_ioeventfd to VirtioDeviceClass

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:50PM +0200, Paolo Bonzini wrote:
> Allow customization of the start and stop of ioeventfd.  This will
> allow direct start of dataplane without passing through the default
> ioeventfd handlers, which in turn allows using the dataplane logic
> instead of virtio_add_queue_aio.  It will also enable some code
> simplification, because the sole entry point to ioeventfd setup
> will be virtio_bus_set_host_notifier.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio/virtio-bus.c         | 55 +++++++++++-------------------------
>  hw/virtio/virtio.c             | 64 ++++++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-bus.h |  7 ++++-
>  include/hw/virtio/virtio.h     |  4 +++
>  4 files changed, 90 insertions(+), 40 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 04/12] virtio: introduce virtio_device_ioeventfd_enabled

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:51PM +0200, Paolo Bonzini wrote:
> This will be used to forbid iothread configuration when the
> proxy does not allow using ioeventfd.  To simplify the implementation,
> change the direction of the ioeventfd_disabled callback too.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/s390x/virtio-ccw.c          |  6 +++---
>  hw/virtio/virtio-bus.c         | 10 +++++++++-
>  hw/virtio/virtio-mmio.c        |  6 +++---
>  hw/virtio/virtio-pci.c         |  6 +++---
>  hw/virtio/virtio.c             |  8 ++++++++
>  include/hw/virtio/virtio-bus.h |  8 +++++---
>  include/hw/virtio/virtio.h     |  1 +
>  7 files changed, 32 insertions(+), 13 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 07/12] Revert "virtio: Introduce virtio_add_queue_aio"

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:54PM +0200, Paolo Bonzini wrote:
> This reverts commit 872dd82c83745a603d2e07a03d34313eb6467ae4.
> virtio_add_queue_aio is unused.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio/virtio.c         | 38 ++++----------------------------------
>  include/hw/virtio/virtio.h |  3 ---
>  2 files changed, 4 insertions(+), 37 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 08/12] virtio: remove set_handler argument from set_host_notifier_internal

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:55PM +0200, Paolo Bonzini wrote:
> Make virtio_device_start_ioeventfd_impl use the same logic as
> dataplane to set up the host notifier.  This removes the need
> for the set_handler argument in set_host_notifier_internal.
> 
> This is a first step towards using virtio_bus_set_host_notifier
> as the sole entry point to set up ioeventfds.  At least now
> the functions have the same interface, but they still differ
> in that virtio_bus_set_host_notifier sets ioeventfd_disabled.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio/virtio-bus.c | 12 +++---------
>  hw/virtio/virtio.c     | 16 +++++++++++++---
>  2 files changed, 16 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 10/12] virtio: do not export set_host_notifier_internal

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:57PM +0200, Paolo Bonzini wrote:
> ioeventfd_disabled was the only reason for the default
> implementation of virtio_device_start_ioeventfd_impl not to use
> virtio_bus_set_host_notifier.  This is now fixed, and the sole entry
> point to set up ioeventfd can now be virtio_bus_set_host_notifier.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio/virtio-bus.c         | 4 ++--
>  hw/virtio/virtio.c             | 8 +++-----
>  include/hw/virtio/virtio-bus.h | 5 -----
>  3 files changed, 5 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 12/12] virtio: inline set_host_notifier_internal

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:59PM +0200, Paolo Bonzini wrote:
> This is only called from virtio_bus_set_host_notifier.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio/virtio-bus.c | 60 +++++++++++++++++++++-----------------------------
>  1 file changed, 25 insertions(+), 35 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Stefan Hajnoczi]

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

On Wed, Sep 21, 2016 at 03:18:47PM +0200, Paolo Bonzini wrote:
> Opinions, reviews and bug reports?

Looks good in general but I've skipped the tricky parts.

Stefan

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

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Michael S. Tsirkin]

On Wed, Sep 21, 2016 at 03:18:47PM +0200, Paolo Bonzini wrote:
> This series started as an attempt to always use the dataplane path
> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
> was three-fold:
> 
> 1) to add more coverage for dataplane
> 
> 2) to remove virtio_add_queue_aio
> 
> 3) to simplify the dataplane start/stop code
> 
> It achieves the first two objectives, and while it doesn't quite
> achieve the third it does cleanup the generic ioeventfd code in
> virtio-bus more than I expected.  In particular, it reduces the set
> of callbacks that transports must implement, and it removes the ugly
> case where ioeventfd is started with generic callbacks and then moved
> to the dataplane callbacks.  It also enables some simplification of the
> functions that deal with host notifiers.
> 
> I've tested it with virtio-blk, virtio-scsi and vhost-net.
> 
> Patches 1 and 2 are simplifications that are too nice to leave
> them for later in the series.
> 
> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
> virtio.c.  At this point the transition is a bit half-assed, but
> this changes as soon as we remove the generic->dataplane
> handler transition.
> 
> Patches 4 to 6 do exactly that, and then the spring cleaning
> begins, lasting for the whole second half of the series.
> 
> Opinions, reviews and bug reports?
> 
> Thanks,
> 
> Paolo

OK, this looks good to me. Can you pls rebase and repost
so I can merge (there was also a bug report I think)?


> Paolo Bonzini (12):
>   virtio: move ioeventfd_disabled flag to VirtioBusState
>   virtio: move ioeventfd_started flag to VirtioBusState
>   virtio: add start_ioeventfd and stop_ioeventfd to VirtioDeviceClass
>   virtio: introduce virtio_device_ioeventfd_enabled
>   virtio-blk: always use dataplane path if ioeventfd is active
>   virtio-scsi: always use dataplane path if ioeventfd is active
>   Revert "virtio: Introduce virtio_add_queue_aio"
>   virtio: remove set_handler argument from set_host_notifier_internal
>   virtio: remove ioeventfd_disabled altogether
>   virtio: do not export set_host_notifier_internal
>   virtio: inline virtio_queue_set_host_notifier_fd_handler
>   virtio: inline set_host_notifier_internal
> 
>  hw/block/dataplane/virtio-blk.c |  67 +++++++++++-------
>  hw/block/dataplane/virtio-blk.h |   6 +-
>  hw/block/virtio-blk.c           |  16 ++---
>  hw/s390x/virtio-ccw.c           |  36 +---------
>  hw/s390x/virtio-ccw.h           |   2 -
>  hw/scsi/virtio-scsi-dataplane.c |  51 ++++++++------
>  hw/scsi/virtio-scsi.c           |  24 +++----
>  hw/virtio/vhost.c               |   5 +-
>  hw/virtio/virtio-bus.c          | 153 +++++++++++++++-------------------------
>  hw/virtio/virtio-mmio.c         |  35 +--------
>  hw/virtio/virtio-pci.c          |  32 +--------
>  hw/virtio/virtio-pci.h          |   2 -
>  hw/virtio/virtio.c              | 139 +++++++++++++++++++++++-------------
>  include/hw/virtio/virtio-bus.h  |  27 ++++---
>  include/hw/virtio/virtio-scsi.h |   6 +-
>  include/hw/virtio/virtio.h      |  11 +--
>  16 files changed, 272 insertions(+), 340 deletions(-)
> 
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

On 10/10/2016 00:01, Michael S. Tsirkin wrote:
> On Wed, Sep 21, 2016 at 03:18:47PM +0200, Paolo Bonzini wrote:
>> This series started as an attempt to always use the dataplane path
>> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
>> was three-fold:
>>
>> 1) to add more coverage for dataplane
>>
>> 2) to remove virtio_add_queue_aio
>>
>> 3) to simplify the dataplane start/stop code
>>
>> It achieves the first two objectives, and while it doesn't quite
>> achieve the third it does cleanup the generic ioeventfd code in
>> virtio-bus more than I expected.  In particular, it reduces the set
>> of callbacks that transports must implement, and it removes the ugly
>> case where ioeventfd is started with generic callbacks and then moved
>> to the dataplane callbacks.  It also enables some simplification of the
>> functions that deal with host notifiers.
>>
>> I've tested it with virtio-blk, virtio-scsi and vhost-net.
>>
>> Patches 1 and 2 are simplifications that are too nice to leave
>> them for later in the series.
>>
>> Patch 3 moves some of the ioeventfd code from virtio-bus.c to
>> virtio.c.  At this point the transition is a bit half-assed, but
>> this changes as soon as we remove the generic->dataplane
>> handler transition.
>>
>> Patches 4 to 6 do exactly that, and then the spring cleaning
>> begins, lasting for the whole second half of the series.
>>
>> Opinions, reviews and bug reports?
>>
>> Thanks,
>>
>> Paolo
> 
> OK, this looks good to me. Can you pls rebase and repost
> so I can merge (there was also a bug report I think)?

Sure, I was going to do that today.  The bug is just too strict an
assertion, I'll fix it on repost.

Paolo

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Cornelia Huck]

On Thu, 20 Oct 2016 18:53:31 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> No, it's because virtio-mmio can be created without a device 
> underneath.  virtio_bus_start_ioeventfd in that case is wrong, but 
> virtio_bus_stop_ioeventfd should be a no-op.  The fix is trivial:

I tend to forget this virtio-mmio speciality...

> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 0479704..bf61f66 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -172,12 +172,15 @@ int virtio_bus_start_ioeventfd(VirtioBusState *bus)
> 
>  void virtio_bus_stop_ioeventfd(VirtioBusState *bus)
>  {
> -    VirtIODevice *vdev = virtio_bus_get_device(bus);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> +    VirtIODevice *vdev;
> +    VirtioDeviceClass *vdc;
> 
>      if (!bus->ioeventfd_started) {
>          return;
>      }
> +
> +    vdev = virtio_bus_get_device(bus);
> +    vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>      vdc->stop_ioeventfd(vdev);
>      bus->ioeventfd_started = false;
>  }

Looks sane, and make check passes with this applied.

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

On 20/10/2016 11:03, Cornelia Huck wrote:
> On Wed, 19 Oct 2016 22:44:18 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> On 19/10/2016 17:38, Cornelia Huck wrote:
>>> - failures are in qom-test for aarch64:
>>> TEST: tests/qom-test... (pid=23997)
>>>   /aarch64/qom/integratorcp:                                           OK
>>>   /aarch64/qom/nuri:                                                   OK
>>>   /aarch64/qom/verdex:                                                 OK
>>>   /aarch64/qom/ast2500-evb:                                            OK
>>>   /aarch64/qom/smdkc210:                                               OK
>>>   /aarch64/qom/collie:                                                 OK
>>>   /aarch64/qom/imx25-pdk:                                              OK
>>>   /aarch64/qom/none:                                                   OK
>>>   /aarch64/qom/spitz:                                                  OK
>>>   /aarch64/qom/realview-pbx-a9:                                        OK
>>>   /aarch64/qom/realview-eb:                                            OK
>>>   /aarch64/qom/versatilepb:                                            OK
>>>   /aarch64/qom/realview-pb-a8:                                         OK
>>>   /aarch64/qom/musicpal:                                               OK
>>>   /aarch64/qom/z2:                                                     OK
>>>   /aarch64/qom/akita:                                                  OK
>>>   /aarch64/qom/virt-2.7:                                               Broken pipe
>>> FAIL
>>> GTester: last random seed: R02Saec62eb6f9ebd3e5bfcbf42d0aaf165a
>>> (pid=24053)
>>>   /aarch64/qom/kzm:                                                    OK
>>>   /aarch64/qom/virt-2.8:                                               Broken pipe
>>> FAIL
>>> GTester: last random seed: R02S3472a1653451d1812262f7d72624492e
>>> (pid=24063)
>>>   /aarch64/qom/realview-eb-mpcore:                                     OK
>>>   /aarch64/qom/sx1:                                                    OK
>>>   /aarch64/qom/sx1-v1:                                                 OK
>>>   /aarch64/qom/virt-2.6:                                               Broken pipe
>>> FAIL
>>> GTester: last random seed: R02Se4b753c5be66c0ef7870bebcca8771f8
>>> (pid=24098)
>>>   /aarch64/qom/cubieboard:                                             OK
>>>   /aarch64/qom/highbank:                                               OK
>>>   /aarch64/qom/raspi2:                                                 OK
>>>   /aarch64/qom/netduino2:                                              OK
>>>   /aarch64/qom/terrier:                                                OK
>>>   /aarch64/qom/n810:                                                   OK
>>>   /aarch64/qom/mainstone:                                              OK
>>>   /aarch64/qom/palmetto-bmc:                                           OK
>>>   /aarch64/qom/sabrelite:                                              OK
>>>   /aarch64/qom/midway:                                                 OK
>>>   /aarch64/qom/cheetah:                                                OK
>>>   /aarch64/qom/tosa:                                                   OK
>>>   /aarch64/qom/borzoi:                                                 OK
>>>   /aarch64/qom/versatileab:                                            OK
>>>   /aarch64/qom/lm3s6965evb:                                            OK
>>>   /aarch64/qom/n800:                                                   OK
>>>   /aarch64/qom/connex:                                                 OK
>>>   /aarch64/qom/xilinx-zynq-a9:                                         OK
>>>   /aarch64/qom/xlnx-ep108:                                             OK
>>>   /aarch64/qom/vexpress-a9:                                            Broken pipe
>>> FAIL
>>> GTester: last random seed: R02Sdf4aceaaef3ceb060fd5996ecfd05bbb
>>> (pid=24180)
>>>   /aarch64/qom/vexpress-a15:                                           Broken pipe
>>> FAIL
>>> GTester: last random seed: R02Sdf4de27065ea3baf0b2acc109af636b8
>>> (pid=24187)
>>>   /aarch64/qom/xlnx-zcu102:                                            OK
>>>   /aarch64/qom/canon-a1100:                                            OK
>>>   /aarch64/qom/lm3s811evb:                                             OK
>>> FAIL: tests/qom-test
>>>
>>> Do these boards maybe have something interesting in common?
>>>
>>> No further time to look into this today, sorry.
>>
>> They use virtio-mmio, probably.  I'll look at it tomorrow.
>>
>> The s390 spew is an abort(), isn't it?
>> Paolo
>>
> 
> virtio-mmio looks like the best candidate (maybe the eventfd is not
> stopped properly and writes happen where they are not supposed to
> anymore?) But staring at the patch didn't make anything jump out at
> me...

No, it's because virtio-mmio can be created without a device 
underneath.  virtio_bus_start_ioeventfd in that case is wrong, but 
virtio_bus_stop_ioeventfd should be a no-op.  The fix is trivial:

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 0479704..bf61f66 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -172,12 +172,15 @@ int virtio_bus_start_ioeventfd(VirtioBusState *bus)
 
 void virtio_bus_stop_ioeventfd(VirtioBusState *bus)
 {
-    VirtIODevice *vdev = virtio_bus_get_device(bus);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+    VirtIODevice *vdev;
+    VirtioDeviceClass *vdc;
 
     if (!bus->ioeventfd_started) {
         return;
     }
+
+    vdev = virtio_bus_get_device(bus);
+    vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
     vdc->stop_ioeventfd(vdev);
     bus->ioeventfd_started = false;
 }


I'll post v3 tomorrow.

Paolo

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Cornelia Huck]

On Wed, 19 Oct 2016 22:44:18 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 19/10/2016 17:38, Cornelia Huck wrote:
> > - failures are in qom-test for aarch64:
> > TEST: tests/qom-test... (pid=23997)
> >   /aarch64/qom/integratorcp:                                           OK
> >   /aarch64/qom/nuri:                                                   OK
> >   /aarch64/qom/verdex:                                                 OK
> >   /aarch64/qom/ast2500-evb:                                            OK
> >   /aarch64/qom/smdkc210:                                               OK
> >   /aarch64/qom/collie:                                                 OK
> >   /aarch64/qom/imx25-pdk:                                              OK
> >   /aarch64/qom/none:                                                   OK
> >   /aarch64/qom/spitz:                                                  OK
> >   /aarch64/qom/realview-pbx-a9:                                        OK
> >   /aarch64/qom/realview-eb:                                            OK
> >   /aarch64/qom/versatilepb:                                            OK
> >   /aarch64/qom/realview-pb-a8:                                         OK
> >   /aarch64/qom/musicpal:                                               OK
> >   /aarch64/qom/z2:                                                     OK
> >   /aarch64/qom/akita:                                                  OK
> >   /aarch64/qom/virt-2.7:                                               Broken pipe
> > FAIL
> > GTester: last random seed: R02Saec62eb6f9ebd3e5bfcbf42d0aaf165a
> > (pid=24053)
> >   /aarch64/qom/kzm:                                                    OK
> >   /aarch64/qom/virt-2.8:                                               Broken pipe
> > FAIL
> > GTester: last random seed: R02S3472a1653451d1812262f7d72624492e
> > (pid=24063)
> >   /aarch64/qom/realview-eb-mpcore:                                     OK
> >   /aarch64/qom/sx1:                                                    OK
> >   /aarch64/qom/sx1-v1:                                                 OK
> >   /aarch64/qom/virt-2.6:                                               Broken pipe
> > FAIL
> > GTester: last random seed: R02Se4b753c5be66c0ef7870bebcca8771f8
> > (pid=24098)
> >   /aarch64/qom/cubieboard:                                             OK
> >   /aarch64/qom/highbank:                                               OK
> >   /aarch64/qom/raspi2:                                                 OK
> >   /aarch64/qom/netduino2:                                              OK
> >   /aarch64/qom/terrier:                                                OK
> >   /aarch64/qom/n810:                                                   OK
> >   /aarch64/qom/mainstone:                                              OK
> >   /aarch64/qom/palmetto-bmc:                                           OK
> >   /aarch64/qom/sabrelite:                                              OK
> >   /aarch64/qom/midway:                                                 OK
> >   /aarch64/qom/cheetah:                                                OK
> >   /aarch64/qom/tosa:                                                   OK
> >   /aarch64/qom/borzoi:                                                 OK
> >   /aarch64/qom/versatileab:                                            OK
> >   /aarch64/qom/lm3s6965evb:                                            OK
> >   /aarch64/qom/n800:                                                   OK
> >   /aarch64/qom/connex:                                                 OK
> >   /aarch64/qom/xilinx-zynq-a9:                                         OK
> >   /aarch64/qom/xlnx-ep108:                                             OK
> >   /aarch64/qom/vexpress-a9:                                            Broken pipe
> > FAIL
> > GTester: last random seed: R02Sdf4aceaaef3ceb060fd5996ecfd05bbb
> > (pid=24180)
> >   /aarch64/qom/vexpress-a15:                                           Broken pipe
> > FAIL
> > GTester: last random seed: R02Sdf4de27065ea3baf0b2acc109af636b8
> > (pid=24187)
> >   /aarch64/qom/xlnx-zcu102:                                            OK
> >   /aarch64/qom/canon-a1100:                                            OK
> >   /aarch64/qom/lm3s811evb:                                             OK
> > FAIL: tests/qom-test
> > 
> > Do these boards maybe have something interesting in common?
> > 
> > No further time to look into this today, sorry.
> 
> They use virtio-mmio, probably.  I'll look at it tomorrow.
> 
> The s390 spew is an abort(), isn't it?
> Paolo
> 

virtio-mmio looks like the best candidate (maybe the eventfd is not
stopped properly and writes happen where they are not supposed to
anymore?) But staring at the patch didn't make anything jump out at
me...

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

On 19/10/2016 17:38, Cornelia Huck wrote:
> - failures are in qom-test for aarch64:
> TEST: tests/qom-test... (pid=23997)
>   /aarch64/qom/integratorcp:                                           OK
>   /aarch64/qom/nuri:                                                   OK
>   /aarch64/qom/verdex:                                                 OK
>   /aarch64/qom/ast2500-evb:                                            OK
>   /aarch64/qom/smdkc210:                                               OK
>   /aarch64/qom/collie:                                                 OK
>   /aarch64/qom/imx25-pdk:                                              OK
>   /aarch64/qom/none:                                                   OK
>   /aarch64/qom/spitz:                                                  OK
>   /aarch64/qom/realview-pbx-a9:                                        OK
>   /aarch64/qom/realview-eb:                                            OK
>   /aarch64/qom/versatilepb:                                            OK
>   /aarch64/qom/realview-pb-a8:                                         OK
>   /aarch64/qom/musicpal:                                               OK
>   /aarch64/qom/z2:                                                     OK
>   /aarch64/qom/akita:                                                  OK
>   /aarch64/qom/virt-2.7:                                               Broken pipe
> FAIL
> GTester: last random seed: R02Saec62eb6f9ebd3e5bfcbf42d0aaf165a
> (pid=24053)
>   /aarch64/qom/kzm:                                                    OK
>   /aarch64/qom/virt-2.8:                                               Broken pipe
> FAIL
> GTester: last random seed: R02S3472a1653451d1812262f7d72624492e
> (pid=24063)
>   /aarch64/qom/realview-eb-mpcore:                                     OK
>   /aarch64/qom/sx1:                                                    OK
>   /aarch64/qom/sx1-v1:                                                 OK
>   /aarch64/qom/virt-2.6:                                               Broken pipe
> FAIL
> GTester: last random seed: R02Se4b753c5be66c0ef7870bebcca8771f8
> (pid=24098)
>   /aarch64/qom/cubieboard:                                             OK
>   /aarch64/qom/highbank:                                               OK
>   /aarch64/qom/raspi2:                                                 OK
>   /aarch64/qom/netduino2:                                              OK
>   /aarch64/qom/terrier:                                                OK
>   /aarch64/qom/n810:                                                   OK
>   /aarch64/qom/mainstone:                                              OK
>   /aarch64/qom/palmetto-bmc:                                           OK
>   /aarch64/qom/sabrelite:                                              OK
>   /aarch64/qom/midway:                                                 OK
>   /aarch64/qom/cheetah:                                                OK
>   /aarch64/qom/tosa:                                                   OK
>   /aarch64/qom/borzoi:                                                 OK
>   /aarch64/qom/versatileab:                                            OK
>   /aarch64/qom/lm3s6965evb:                                            OK
>   /aarch64/qom/n800:                                                   OK
>   /aarch64/qom/connex:                                                 OK
>   /aarch64/qom/xilinx-zynq-a9:                                         OK
>   /aarch64/qom/xlnx-ep108:                                             OK
>   /aarch64/qom/vexpress-a9:                                            Broken pipe
> FAIL
> GTester: last random seed: R02Sdf4aceaaef3ceb060fd5996ecfd05bbb
> (pid=24180)
>   /aarch64/qom/vexpress-a15:                                           Broken pipe
> FAIL
> GTester: last random seed: R02Sdf4de27065ea3baf0b2acc109af636b8
> (pid=24187)
>   /aarch64/qom/xlnx-zcu102:                                            OK
>   /aarch64/qom/canon-a1100:                                            OK
>   /aarch64/qom/lm3s811evb:                                             OK
> FAIL: tests/qom-test
> 
> Do these boards maybe have something interesting in common?
> 
> No further time to look into this today, sorry.

They use virtio-mmio, probably.  I'll look at it tomorrow.

The s390 spew is an abort(), isn't it?
Paolo

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Cornelia Huck]

On Wed, 19 Oct 2016 14:17:59 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Mon, 10 Oct 2016 13:53:28 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > This series started as an attempt to always use the dataplane path
> > for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
> > was three-fold:
> > 
> > 1) to add more coverage for dataplane
> > 
> > 2) to remove virtio_add_queue_aio
> > 
> > 3) to simplify the dataplane start/stop code
> > 
> > It achieves the first two objectives, and while it doesn't quite
> > achieve the third it does cleanup the generic ioeventfd code in
> > virtio-bus more than I expected.  In particular, it reduces the set
> > of callbacks that transports must implement, and it removes the ugly
> > case where ioeventfd is started with generic callbacks and then moved
> > to the dataplane callbacks.  It also enables some simplification of the
> > functions that deal with host notifiers, and detects some configuration
> > errors better.
> > 
> > I've tested it with virtio-blk, virtio-scsi and vhost-net.
> 
> Hm. 'make check' on a s390 host faults in check-qtest-aarch64 (your
> branch rebased to current master; master itself is fine). I'll see if I
> can find out more (probably later today).

I've bisected this to patch 4 ("virtio: add start_ioeventfd and
stop_ioeventfd to VirtioDeviceClass").

More details of the failure:
- host is s390x with Fedora 23
- # Configured with: '../configure' '--target-list=s390x-softmmu s390x-linux-user aarch64-softmmu' '--enable-kvm' '--enable-vhost-net' '--enable-linux-aio'
- dmesg has:
[ 1774.006703] User process fault: interruption code 0010 ilc:3 in qemu-system-aarch64[10000000+7b9000]
[ 1774.006720] Failing address: 0000000000000000 TEID: 0000000000000800
[ 1774.006723] Fault in primary space mode while using user ASCE.
[ 1774.006731] AS:00000000373ac1c7 R3:0000000048148007 S:0000000000000020 
[ 1774.006739] CPU: 1 PID: 24183 Comm: qemu-system-aar Not tainted 4.8.0-20161019.0.eb97ed0.8b99dbe.fc23.s390xdefault #1
[ 1774.006742] Hardware name: IBM              2827 H43              738              (LPAR)
[ 1774.006745] task: 000000004ccb8008 task.stack: 00000000355e0000
[ 1774.006748] User PSW : 0705000180000000 0000000010332380
[ 1774.006753]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:1 AS:0 CC:0 PM:0 RI:0 EA:3
[ 1774.006757] User GPRS: 0000000000000003 0000000000000000 0000000000000000 0000000000000001
[ 1774.006761]            00000000102e1740 0000000000000178 000000001069b2ac 00000000101f8278
[ 1774.006764]            0000000000000000 0000000047471ee0 0000000000000000 00000000101f6600
[ 1774.006766]            0000000000000000 000000004795f788 00000000102e14a0 000003ffe307e988
[ 1774.006774] User Code: 000000001033237a: 07fe		bcr	15,%r14
                          000000001033237c: 0707		bcr	0,%r7
                         #000000001033237e: 0707		bcr	0,%r7
                         >0000000010332380: e32020000004	lg	%r2,0(%r2)
                          0000000010332386: 07fe		bcr	15,%r14
                          0000000010332388: e31020000004	lg	%r1,0(%r2)
                          000000001033238e: e32010500090	llgc	%r2,80(%r1)
                          0000000010332394: 07fe		bcr	15,%r14
[ 1774.006812] Last Breaking-Event-Address:
[ 1774.006816]  [<00000000102e149a>] 0x102e149a

(once for each failure)
- failures are in qom-test for aarch64:
TEST: tests/qom-test... (pid=23997)
  /aarch64/qom/integratorcp:                                           OK
  /aarch64/qom/nuri:                                                   OK
  /aarch64/qom/verdex:                                                 OK
  /aarch64/qom/ast2500-evb:                                            OK
  /aarch64/qom/smdkc210:                                               OK
  /aarch64/qom/collie:                                                 OK
  /aarch64/qom/imx25-pdk:                                              OK
  /aarch64/qom/none:                                                   OK
  /aarch64/qom/spitz:                                                  OK
  /aarch64/qom/realview-pbx-a9:                                        OK
  /aarch64/qom/realview-eb:                                            OK
  /aarch64/qom/versatilepb:                                            OK
  /aarch64/qom/realview-pb-a8:                                         OK
  /aarch64/qom/musicpal:                                               OK
  /aarch64/qom/z2:                                                     OK
  /aarch64/qom/akita:                                                  OK
  /aarch64/qom/virt-2.7:                                               Broken pipe
FAIL
GTester: last random seed: R02Saec62eb6f9ebd3e5bfcbf42d0aaf165a
(pid=24053)
  /aarch64/qom/kzm:                                                    OK
  /aarch64/qom/virt-2.8:                                               Broken pipe
FAIL
GTester: last random seed: R02S3472a1653451d1812262f7d72624492e
(pid=24063)
  /aarch64/qom/realview-eb-mpcore:                                     OK
  /aarch64/qom/sx1:                                                    OK
  /aarch64/qom/sx1-v1:                                                 OK
  /aarch64/qom/virt-2.6:                                               Broken pipe
FAIL
GTester: last random seed: R02Se4b753c5be66c0ef7870bebcca8771f8
(pid=24098)
  /aarch64/qom/cubieboard:                                             OK
  /aarch64/qom/highbank:                                               OK
  /aarch64/qom/raspi2:                                                 OK
  /aarch64/qom/netduino2:                                              OK
  /aarch64/qom/terrier:                                                OK
  /aarch64/qom/n810:                                                   OK
  /aarch64/qom/mainstone:                                              OK
  /aarch64/qom/palmetto-bmc:                                           OK
  /aarch64/qom/sabrelite:                                              OK
  /aarch64/qom/midway:                                                 OK
  /aarch64/qom/cheetah:                                                OK
  /aarch64/qom/tosa:                                                   OK
  /aarch64/qom/borzoi:                                                 OK
  /aarch64/qom/versatileab:                                            OK
  /aarch64/qom/lm3s6965evb:                                            OK
  /aarch64/qom/n800:                                                   OK
  /aarch64/qom/connex:                                                 OK
  /aarch64/qom/xilinx-zynq-a9:                                         OK
  /aarch64/qom/xlnx-ep108:                                             OK
  /aarch64/qom/vexpress-a9:                                            Broken pipe
FAIL
GTester: last random seed: R02Sdf4aceaaef3ceb060fd5996ecfd05bbb
(pid=24180)
  /aarch64/qom/vexpress-a15:                                           Broken pipe
FAIL
GTester: last random seed: R02Sdf4de27065ea3baf0b2acc109af636b8
(pid=24187)
  /aarch64/qom/xlnx-zcu102:                                            OK
  /aarch64/qom/canon-a1100:                                            OK
  /aarch64/qom/lm3s811evb:                                             OK
FAIL: tests/qom-test

Do these boards maybe have something interesting in common?

No further time to look into this today, sorry.

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Cornelia Huck]

On Mon, 10 Oct 2016 13:53:28 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> This series started as an attempt to always use the dataplane path
> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
> was three-fold:
> 
> 1) to add more coverage for dataplane
> 
> 2) to remove virtio_add_queue_aio
> 
> 3) to simplify the dataplane start/stop code
> 
> It achieves the first two objectives, and while it doesn't quite
> achieve the third it does cleanup the generic ioeventfd code in
> virtio-bus more than I expected.  In particular, it reduces the set
> of callbacks that transports must implement, and it removes the ugly
> case where ioeventfd is started with generic callbacks and then moved
> to the dataplane callbacks.  It also enables some simplification of the
> functions that deal with host notifiers, and detects some configuration
> errors better.
> 
> I've tested it with virtio-blk, virtio-scsi and vhost-net.

Hm. 'make check' on a s390 host faults in check-qtest-aarch64 (your
branch rebased to current master; master itself is fine). I'll see if I
can find out more (probably later today).

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

----- Original Message -----
> From: "Cornelia Huck" <cornelia.huck@de.ibm.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: qemu-devel@nongnu.org, stefanha@redhat.com, borntraeger@de.ibm.com, famz@redhat.com, mst@redhat.com
> Sent: Tuesday, October 18, 2016 7:24:45 PM
> Subject: Re: [PATCH 00/12] virtio: cleanup ioeventfd start/stop
> 
> On Mon, 10 Oct 2016 13:53:28 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > This series started as an attempt to always use the dataplane path
> > for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
> > was three-fold:
> > 
> > 1) to add more coverage for dataplane
> > 
> > 2) to remove virtio_add_queue_aio
> > 
> > 3) to simplify the dataplane start/stop code
> > 
> > It achieves the first two objectives, and while it doesn't quite
> > achieve the third it does cleanup the generic ioeventfd code in
> > virtio-bus more than I expected.  In particular, it reduces the set
> > of callbacks that transports must implement, and it removes the ugly
> > case where ioeventfd is started with generic callbacks and then moved
> > to the dataplane callbacks.  It also enables some simplification of the
> > functions that deal with host notifiers, and detects some configuration
> > errors better.
> > 
> > I've tested it with virtio-blk, virtio-scsi and vhost-net.
> 
> Do you have a branch somewhere?

ioeventfd-virtio at git://github.com/bonzini/qemu.git

Paolo

> > 
> > Patch 1 is a bugfix that I found while testing the TCG+dataplane combo.
> > 
> > Patches 2 and 3 are simplifications that are too nice to leave
> > them for later in the series.
> > 
> > Patch 4 moves some of the ioeventfd code from virtio-bus.c to
> > virtio.c.  At this point the transition is a bit half-assed, but
> > this changes as soon as we remove the generic->dataplane
> > handler transition.
> > 
> > Patches 5 to 7 do exactly that, and then the spring cleaning
> > begins, lasting for the whole second half of the series.
> 
> I'll continue to look at the patches from 4 on tomorrow.
> 
> 

Re: [Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Cornelia Huck]

On Mon, 10 Oct 2016 13:53:28 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> This series started as an attempt to always use the dataplane path
> for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
> was three-fold:
> 
> 1) to add more coverage for dataplane
> 
> 2) to remove virtio_add_queue_aio
> 
> 3) to simplify the dataplane start/stop code
> 
> It achieves the first two objectives, and while it doesn't quite
> achieve the third it does cleanup the generic ioeventfd code in
> virtio-bus more than I expected.  In particular, it reduces the set
> of callbacks that transports must implement, and it removes the ugly
> case where ioeventfd is started with generic callbacks and then moved
> to the dataplane callbacks.  It also enables some simplification of the
> functions that deal with host notifiers, and detects some configuration
> errors better.
> 
> I've tested it with virtio-blk, virtio-scsi and vhost-net.

Do you have a branch somewhere?

> 
> Patch 1 is a bugfix that I found while testing the TCG+dataplane combo.
> 
> Patches 2 and 3 are simplifications that are too nice to leave
> them for later in the series.
> 
> Patch 4 moves some of the ioeventfd code from virtio-bus.c to
> virtio.c.  At this point the transition is a bit half-assed, but
> this changes as soon as we remove the generic->dataplane
> handler transition.
> 
> Patches 5 to 7 do exactly that, and then the spring cleaning
> begins, lasting for the whole second half of the series.

I'll continue to look at the patches from 4 on tomorrow.

[Qemu-devel] [PATCH 00/12] virtio: cleanup ioeventfd start/stop

[Paolo Bonzini]

This series started as an attempt to always use the dataplane path
for virtio-blk and virtio-scsi when ioeventfd is active.  The aim
was three-fold:

1) to add more coverage for dataplane

2) to remove virtio_add_queue_aio

3) to simplify the dataplane start/stop code

It achieves the first two objectives, and while it doesn't quite
achieve the third it does cleanup the generic ioeventfd code in
virtio-bus more than I expected.  In particular, it reduces the set
of callbacks that transports must implement, and it removes the ugly
case where ioeventfd is started with generic callbacks and then moved
to the dataplane callbacks.  It also enables some simplification of the
functions that deal with host notifiers, and detects some configuration
errors better.

I've tested it with virtio-blk, virtio-scsi and vhost-net.

Patch 1 is a bugfix that I found while testing the TCG+dataplane combo.

Patches 2 and 3 are simplifications that are too nice to leave
them for later in the series.

Patch 4 moves some of the ioeventfd code from virtio-bus.c to
virtio.c.  At this point the transition is a bit half-assed, but
this changes as soon as we remove the generic->dataplane
handler transition.

Patches 5 to 7 do exactly that, and then the spring cleaning
begins, lasting for the whole second half of the series.

Thanks,

Paolo

Paolo Bonzini (13):
  virtio: disable ioeventfd as early as possible
  virtio: move ioeventfd_disabled flag to VirtioBusState
  virtio: move ioeventfd_started flag to VirtioBusState
  virtio: add start_ioeventfd and stop_ioeventfd to VirtioDeviceClass
  virtio: introduce virtio_device_ioeventfd_enabled
  virtio-blk: always use dataplane path if ioeventfd is active
  virtio-scsi: always use dataplane path if ioeventfd is active
  Revert "virtio: Introduce virtio_add_queue_aio"
  virtio: remove set_handler argument from set_host_notifier_internal
  virtio: remove ioeventfd_disabled altogether
  virtio: use virtio_bus_set_host_notifier to start/stop ioeventfd
  virtio: inline virtio_queue_set_host_notifier_fd_handler
  virtio: inline set_host_notifier_internal

 hw/block/dataplane/virtio-blk.c |  74 +++++++++++--------
 hw/block/dataplane/virtio-blk.h |   6 +-
 hw/block/virtio-blk.c           |  15 ++--
 hw/s390x/virtio-ccw.c           |  44 ++----------
 hw/s390x/virtio-ccw.h           |   2 -
 hw/scsi/virtio-scsi-dataplane.c |  56 +++++++++------
 hw/scsi/virtio-scsi.c           |  24 +++----
 hw/virtio/vhost.c               |   5 +-
 hw/virtio/virtio-bus.c          | 155 +++++++++++++++-------------------------
 hw/virtio/virtio-mmio.c         |  35 +--------
 hw/virtio/virtio-pci.c          |  40 ++---------
 hw/virtio/virtio-pci.h          |   2 -
 hw/virtio/virtio.c              | 139 ++++++++++++++++++++++-------------
 include/hw/virtio/virtio-bus.h  |  27 ++++---
 include/hw/virtio/virtio-scsi.h |   6 +-
 include/hw/virtio/virtio.h      |  11 +--
 16 files changed, 291 insertions(+), 350 deletions(-)

-- 
2.7.4