[Qemu-devel] [PATCH V2 0/8] virtio 1.0 pci optimizations and fixes

[Qemu-devel] [PATCH V2 0/8] virtio 1.0 pci optimizations and fixes

[Jason Wang]

Hi all:

This series tries to fix the following issues:

- qemu abort when trying to adjust endianness for zero length eventfd,
  this prevent fast mmio eventfd from being used in ppc. Fixing by
  skip the endianness adjustment for zero length eventfd.
- 1.0 mmio is slow since it was using datamatch eventfd. Fixing this
  by usinng wildcard mmio eventfd, then we could get speed up through
  kernel fast mmio bus on ept capable machine.
- 1.0 mmio is slow compared to pio (at least on some
  archs/setups). Fixing this by re-introducing pio notification
  capability. This will be useful for the arch/setups that fast mmio
  does not work.
- Some virtio pci 1.0 fields were not migrated. This will cause
  unexpected behaviour if migrate during driver initialization. Fixing
  this by introduce a transport specific callback and get/put
  transport specific fields for virtio 1.0.
- queue_enable read was broken. Fixing by set the queue_enable to true
  during guest write and clear it during reset.

Please review.
Thanks

Changes from V1:
- skip zero length eventfd endianness adjustment
- don't use pci specific name ("modern") in virtio core, using "extra"
  instead and in virtio pci callback, using subsections which could
  allow us to extend the future improvement without changing the core.
- don't check virtio_virtqueue_needed() in virtio_extra_state_needed()
- drop the ppc 2.5 machine type patch
- squash Eduardo's 2.5 machine type patches into this series

Eduardo Habkost (3):
  q35: Move options common to all classes to pc_q35_machine_options()
  q35: Move options common to all classes to pc_i440fx_machine_options()
  pc: Introduce pc-*-2.5 machine classes

Jason Wang (5):
  virtio-pci: fix 1.0 virtqueue migration
  memory: don't try to adjust endianness for zero length eventfd
  virtio-pci: use wildcard mmio eventfd for 1.0 notification cap
  virtio-pci: introduce pio notification capability for modern device
  virtio-pci: unbreak queue_enable read

 hw/i386/pc_piix.c              |  18 ++-
 hw/i386/pc_q35.c               |  20 +++-
 hw/virtio/virtio-pci.c         | 266 +++++++++++++++++++++++++++++++++++++----
 hw/virtio/virtio-pci.h         |  30 +++--
 hw/virtio/virtio.c             |  57 +++++++++
 include/hw/compat.h            |   7 ++
 include/hw/i386/pc.h           |   4 +
 include/hw/virtio/virtio-bus.h |   3 +
 memory.c                       |   8 +-
 9 files changed, 375 insertions(+), 38 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH V2 1/8] q35: Move options common to all classes to pc_q35_machine_options()

[Jason Wang]

From: Eduardo Habkost <ehabkost@redhat.com>

The existing default_machine_opts, default_display, no_floppy, and
no_tco settings will still apply to future machine classes. So it makes
sense to move them to pc_q35_machine_options() instead of keeping them
in a version-specific machine_options function.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_q35.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c07d65b..baae402 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -365,15 +365,15 @@ static void pc_q35_machine_options(MachineClass *m)
     m->desc = "Standard PC (Q35 + ICH9, 2009)";
     m->hot_add_cpu = pc_hot_add_cpu;
     m->units_per_default_bus = 1;
+    m->default_machine_opts = "firmware=bios-256k.bin";
+    m->default_display = "std";
+    m->no_floppy = 1;
+    m->no_tco = 0;
 }
 
 static void pc_q35_2_4_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
-    m->default_machine_opts = "firmware=bios-256k.bin";
-    m->default_display = "std";
-    m->no_floppy = 1;
-    m->no_tco = 0;
     m->alias = "q35";
 }
 
-- 
2.1.4

[Qemu-devel] [PATCH V2 2/8] q35: Move options common to all classes to pc_i440fx_machine_options()

[Jason Wang]

From: Eduardo Habkost <ehabkost@redhat.com>

The existing default_machine_opts and default_display settings will
still apply to future machine classes. So it makes sense to move them to
pc_i440fx_machine_options() instead of keeping them in a
version-specific machine_options function.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9558467..7ca3fcf 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -446,13 +446,13 @@ static void pc_i440fx_machine_options(MachineClass *m)
     m->family = "pc_piix";
     m->desc = "Standard PC (i440FX + PIIX, 1996)";
     m->hot_add_cpu = pc_hot_add_cpu;
+    m->default_machine_opts = "firmware=bios-256k.bin";
+    m->default_display = "std";
 }
 
 static void pc_i440fx_2_4_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
-    m->default_machine_opts = "firmware=bios-256k.bin";
-    m->default_display = "std";
     m->alias = "pc";
     m->is_default = 1;
 }
-- 
2.1.4

[Qemu-devel] [PATCH V2 3/8] pc: Introduce pc-*-2.5 machine classes

[Jason Wang]

From: Eduardo Habkost <ehabkost@redhat.com>

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c    | 14 +++++++++++++-
 hw/i386/pc_q35.c     | 12 +++++++++++-
 include/hw/compat.h  |  3 +++
 include/hw/i386/pc.h |  4 ++++
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7ca3fcf..b40411b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -450,13 +450,25 @@ static void pc_i440fx_machine_options(MachineClass *m)
     m->default_display = "std";
 }
 
-static void pc_i440fx_2_4_machine_options(MachineClass *m)
+static void pc_i440fx_2_5_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
     m->alias = "pc";
     m->is_default = 1;
 }
 
+DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL,
+                      pc_i440fx_2_5_machine_options);
+
+
+static void pc_i440fx_2_4_machine_options(MachineClass *m)
+{
+    pc_i440fx_2_5_machine_options(m);
+    m->alias = NULL;
+    m->is_default = 0;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
+}
+
 DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
                       pc_i440fx_2_4_machine_options)
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index baae402..57af102 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -371,12 +371,22 @@ static void pc_q35_machine_options(MachineClass *m)
     m->no_tco = 0;
 }
 
-static void pc_q35_2_4_machine_options(MachineClass *m)
+static void pc_q35_2_5_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL,
+                   pc_q35_2_5_machine_options);
+
+static void pc_q35_2_4_machine_options(MachineClass *m)
+{
+    pc_q35_2_5_machine_options(m);
+    m->alias = NULL;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
+}
+
 DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
                    pc_q35_2_4_machine_options);
 
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 94c8097..095de5d 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,9 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
+#define HW_COMPAT_2_4 \
+        /* empty */
+
 #define HW_COMPAT_2_3 \
         {\
             .driver   = "virtio-blk-pci",\
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index d0cad87..21c1cff 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -284,7 +284,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+#define PC_COMPAT_2_4 \
+        HW_COMPAT_2_4
+
 #define PC_COMPAT_2_3 \
+        PC_COMPAT_2_4 \
         HW_COMPAT_2_3 \
         {\
             .driver   = TYPE_X86_CPU,\
-- 
2.1.4

[Qemu-devel] [PATCH V2 4/8] virtio-pci: fix 1.0 virtqueue migration

[Jason Wang]

We don't migrate the followings fields for virtio-pci:

uint32_t dfselect;
uint32_t gfselect;
uint32_t guest_features[2];
struct {
    uint16_t num;
    bool enabled;
    uint32_t desc[2];
    uint32_t avail[2];
    uint32_t used[2];
} vqs[VIRTIO_QUEUE_MAX];

This will confuse driver if migrating during initialization. Solves
this issue by:

- introduce transport specific callbacks to load and store extra
  virtqueue states.
- add a new subsection for virtio to migrate transport specific modern
  device state.
- implement pci specific callbacks.
- add a new property for virtio-pci for whether or not to migrate
  extra state.
- compat the migration for 2.4 and elder machine types

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.c         | 129 +++++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio-pci.h         |  20 ++++---
 hw/virtio/virtio.c             |  57 ++++++++++++++++++
 include/hw/compat.h            |   6 +-
 include/hw/virtio/virtio-bus.h |   3 +
 5 files changed, 207 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c024161..a96890b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -86,6 +86,129 @@ static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
         qemu_put_be16(f, vdev->config_vector);
 }
 
+static void virtio_pci_load_modern_queue_state(VirtIOPCIQueue *vq,
+                                               QEMUFile *f)
+{
+    vq->num = qemu_get_be16(f);
+    vq->enabled = qemu_get_be16(f);
+    vq->desc[0] = qemu_get_be32(f);
+    vq->desc[1] = qemu_get_be32(f);
+    vq->avail[0] = qemu_get_be32(f);
+    vq->avail[1] = qemu_get_be32(f);
+    vq->used[0] = qemu_get_be32(f);
+    vq->used[1] = qemu_get_be32(f);
+}
+
+static bool virtio_pci_has_extra_state(DeviceState *d)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;
+}
+
+static int get_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size)
+{
+    VirtIOPCIProxy *proxy = pv;
+    int i;
+
+    proxy->dfselect = qemu_get_be32(f);
+    proxy->gfselect = qemu_get_be32(f);
+    proxy->guest_features[0] = qemu_get_be32(f);
+    proxy->guest_features[1] = qemu_get_be32(f);
+    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+        virtio_pci_load_modern_queue_state(&proxy->vqs[i], f);
+    }
+
+    return 0;
+}
+
+static void virtio_pci_save_modern_queue_state(VirtIOPCIQueue *vq,
+                                               QEMUFile *f)
+{
+    qemu_put_be16(f, vq->num);
+    qemu_put_be16(f, vq->enabled);
+    qemu_put_be32(f, vq->desc[0]);
+    qemu_put_be32(f, vq->desc[1]);
+    qemu_put_be32(f, vq->avail[0]);
+    qemu_put_be32(f, vq->avail[1]);
+    qemu_put_be32(f, vq->used[0]);
+    qemu_put_be32(f, vq->used[1]);
+}
+
+static void put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size)
+{
+    VirtIOPCIProxy *proxy = pv;
+    int i;
+
+    qemu_put_be32(f, proxy->dfselect);
+    qemu_put_be32(f, proxy->gfselect);
+    qemu_put_be32(f, proxy->guest_features[0]);
+    qemu_put_be32(f, proxy->guest_features[1]);
+    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+        virtio_pci_save_modern_queue_state(&proxy->vqs[i], f);
+    }
+}
+
+static VMStateInfo vmstate_info_virtio_pci_modern_state = {
+    .name = "virtqueue_state",
+    .get = get_virtio_pci_modern_state,
+    .put = put_virtio_pci_modern_state,
+};
+
+static bool virtio_pci_modern_state_needed(void *opaque)
+{
+    VirtIOPCIProxy *proxy = opaque;
+
+    return !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
+}
+
+static const VMStateDescription vmstate_virtio_pci_modern_state = {
+    .name = "virtio_pci/modern_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = &virtio_pci_modern_state_needed,
+    .fields = (VMStateField[]) {
+        {
+            .name         = "modern_state",
+            .version_id   = 0,
+            .field_exists = NULL,
+            .size         = 0,
+            .info         = &vmstate_info_virtio_pci_modern_state,
+            .flags        = VMS_SINGLE,
+            .offset       = 0,
+        },
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_virtio_pci = {
+    .name = "virtio_pci",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_virtio_pci_modern_state,
+        NULL
+    }
+};
+
+static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL);
+}
+
+static int virtio_pci_load_extra_state(DeviceState *d, QEMUFile *f)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    return vmstate_load_state(f, &vmstate_virtio_pci, proxy, 1);
+}
+
 static void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
@@ -133,6 +256,7 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
     if (vector != VIRTIO_NO_VECTOR) {
         return msix_vector_use(&proxy->pci_dev, vector);
     }
+
     return 0;
 }
 
@@ -1618,6 +1742,8 @@ static Property virtio_pci_properties[] = {
                     VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, false),
     DEFINE_PROP_BIT("disable-modern", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true),
+    DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2211,6 +2337,9 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->load_config = virtio_pci_load_config;
     k->save_queue = virtio_pci_save_queue;
     k->load_queue = virtio_pci_load_queue;
+    k->save_extra_state = virtio_pci_save_extra_state;
+    k->load_extra_state = virtio_pci_load_extra_state;
+    k->has_extra_state = virtio_pci_has_extra_state;
     k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
     k->set_host_notifier = virtio_pci_set_host_notifier;
     k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index b6c442f..d890009 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -75,6 +75,10 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
 #define VIRTIO_PCI_FLAG_DISABLE_LEGACY (1 << VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT)
 #define VIRTIO_PCI_FLAG_DISABLE_MODERN (1 << VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT)
 
+/* migrate extra state */
+#define VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT 4
+#define VIRTIO_PCI_FLAG_MIGRATE_EXTRA (1 << VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT)
+
 typedef struct {
     MSIMessage msg;
     int virq;
@@ -104,6 +108,14 @@ typedef struct VirtIOPCIRegion {
     uint32_t type;
 } VirtIOPCIRegion;
 
+typedef struct VirtIOPCIQueue {
+  uint16_t num;
+  bool enabled;
+  uint32_t desc[2];
+  uint32_t avail[2];
+  uint32_t used[2];
+} VirtIOPCIQueue;
+
 struct VirtIOPCIProxy {
     PCIDevice pci_dev;
     MemoryRegion bar;
@@ -124,13 +136,7 @@ struct VirtIOPCIProxy {
     uint32_t dfselect;
     uint32_t gfselect;
     uint32_t guest_features[2];
-    struct {
-        uint16_t num;
-        bool enabled;
-        uint32_t desc[2];
-        uint32_t avail[2];
-        uint32_t used[2];
-    } vqs[VIRTIO_QUEUE_MAX];
+    VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX];
 
     bool ioeventfd_disabled;
     bool ioeventfd_started;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 788b556..dcaf022 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1056,6 +1056,16 @@ static bool virtio_virtqueue_needed(void *opaque)
     return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
 }
 
+static bool virtio_extra_state_needed(void *opaque)
+{
+    VirtIODevice *vdev = opaque;
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    return k->has_extra_state &&
+        k->has_extra_state(qbus->parent);
+}
+
 static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size)
 {
     VirtIODevice *vdev = pv;
@@ -1104,6 +1114,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = {
     }
 };
 
+static int get_extra_state(QEMUFile *f, void *pv, size_t size)
+{
+    VirtIODevice *vdev = pv;
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+    int ret = 0;
+
+    ret = k->load_extra_state(qbus->parent, f);
+
+    return ret;
+}
+
+static void put_extra_state(QEMUFile *f, void *pv, size_t size)
+{
+    VirtIODevice *vdev = pv;
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    k->save_extra_state(qbus->parent, f);
+}
+
+static VMStateInfo vmstate_info_extra_state = {
+    .name = "virtqueue_extra_state",
+    .get = get_extra_state,
+    .put = put_extra_state,
+};
+
+static const VMStateDescription vmstate_virtio_modern_state = {
+    .name = "virtio/extra_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = &virtio_extra_state_needed,
+    .fields = (VMStateField[]) {
+        {
+            .name         = "modern_state",
+            .version_id   = 0,
+            .field_exists = NULL,
+            .size         = 0,
+            .info         = &vmstate_info_extra_state,
+            .flags        = VMS_SINGLE,
+            .offset       = 0,
+        },
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_virtio_device_endian = {
     .name = "virtio/device_endian",
     .version_id = 1,
@@ -1138,6 +1194,7 @@ static const VMStateDescription vmstate_virtio = {
         &vmstate_virtio_device_endian,
         &vmstate_virtio_64bit_features,
         &vmstate_virtio_virtqueues,
+        &vmstate_virtio_modern_state,
         NULL
     }
 };
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 095de5d..47faab0 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,11 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_2_4 \
-        /* empty */
+        {\
+            .driver   = "virtio-pci",\
+            .property = "migrate-extra",\
+            .value    = "off",\
+        },
 
 #define HW_COMPAT_2_3 \
         {\
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 8811415..6c3d4cb 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -44,9 +44,12 @@ typedef struct VirtioBusClass {
     void (*notify)(DeviceState *d, uint16_t vector);
     void (*save_config)(DeviceState *d, QEMUFile *f);
     void (*save_queue)(DeviceState *d, int n, QEMUFile *f);
+    void (*save_extra_state)(DeviceState *d, QEMUFile *f);
     int (*load_config)(DeviceState *d, QEMUFile *f);
     int (*load_queue)(DeviceState *d, int n, QEMUFile *f);
     int (*load_done)(DeviceState *d, QEMUFile *f);
+    int (*load_extra_state)(DeviceState *d, QEMUFile *f);
+    bool (*has_extra_state)(DeviceState *d);
     bool (*query_guest_notifiers)(DeviceState *d);
     int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
     int (*set_host_notifier)(DeviceState *d, int n, bool assigned);
-- 
2.1.4

Re: [Qemu-devel] [PATCH V2 4/8] virtio-pci: fix 1.0 virtqueue migration

[Cornelia Huck]

On Wed,  2 Sep 2015 11:25:21 +0800
Jason Wang <jasowang@redhat.com> wrote:

> We don't migrate the followings fields for virtio-pci:
> 
> uint32_t dfselect;
> uint32_t gfselect;
> uint32_t guest_features[2];
> struct {
>     uint16_t num;
>     bool enabled;
>     uint32_t desc[2];
>     uint32_t avail[2];
>     uint32_t used[2];
> } vqs[VIRTIO_QUEUE_MAX];
> 
> This will confuse driver if migrating during initialization. Solves
> this issue by:
> 
> - introduce transport specific callbacks to load and store extra
>   virtqueue states.
> - add a new subsection for virtio to migrate transport specific modern
>   device state.
> - implement pci specific callbacks.
> - add a new property for virtio-pci for whether or not to migrate
>   extra state.
> - compat the migration for 2.4 and elder machine types
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/virtio/virtio-pci.c         | 129 +++++++++++++++++++++++++++++++++++++++++
>  hw/virtio/virtio-pci.h         |  20 ++++---
>  hw/virtio/virtio.c             |  57 ++++++++++++++++++
>  include/hw/compat.h            |   6 +-
>  include/hw/virtio/virtio-bus.h |   3 +
>  5 files changed, 207 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index c024161..a96890b 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -86,6 +86,129 @@ static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
>          qemu_put_be16(f, vdev->config_vector);
>  }
> 
> +static void virtio_pci_load_modern_queue_state(VirtIOPCIQueue *vq,
> +                                               QEMUFile *f)
> +{
> +    vq->num = qemu_get_be16(f);
> +    vq->enabled = qemu_get_be16(f);
> +    vq->desc[0] = qemu_get_be32(f);
> +    vq->desc[1] = qemu_get_be32(f);
> +    vq->avail[0] = qemu_get_be32(f);
> +    vq->avail[1] = qemu_get_be32(f);
> +    vq->used[0] = qemu_get_be32(f);
> +    vq->used[1] = qemu_get_be32(f);
> +}
> +
> +static bool virtio_pci_has_extra_state(DeviceState *d)
> +{
> +    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> +
> +    return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;

Hm, couldn't you already check whether this is a modern device here?
You don't need to save state for a legacy device, do you?

> +}
> +

(...)

> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 788b556..dcaf022 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1056,6 +1056,16 @@ static bool virtio_virtqueue_needed(void *opaque)
>      return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>  }
> 
> +static bool virtio_extra_state_needed(void *opaque)
> +{
> +    VirtIODevice *vdev = opaque;
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    return k->has_extra_state &&
> +        k->has_extra_state(qbus->parent);
> +}
> +
>  static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size)
>  {
>      VirtIODevice *vdev = pv;
> @@ -1104,6 +1114,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = {
>      }
>  };
> 
> +static int get_extra_state(QEMUFile *f, void *pv, size_t size)
> +{
> +    VirtIODevice *vdev = pv;
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +    int ret = 0;
> +
> +    ret = k->load_extra_state(qbus->parent, f);

Should we check for ->load_extra_state() and return failure if it does
not exist?

> +
> +    return ret;
> +}
> +
> +static void put_extra_state(QEMUFile *f, void *pv, size_t size)
> +{
> +    VirtIODevice *vdev = pv;
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    k->save_extra_state(qbus->parent, f);

Maybe add an assert(k->save_extra_state) here, as this means the
transport code is buggy?

> +}
> +
> +static VMStateInfo vmstate_info_extra_state = {
> +    .name = "virtqueue_extra_state",
> +    .get = get_extra_state,
> +    .put = put_extra_state,
> +};
> +
> +static const VMStateDescription vmstate_virtio_modern_state = {

Should probably be named "extra_state".

> +    .name = "virtio/extra_state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = &virtio_extra_state_needed,
> +    .fields = (VMStateField[]) {
> +        {
> +            .name         = "modern_state",

Also here.

> +            .version_id   = 0,
> +            .field_exists = NULL,
> +            .size         = 0,
> +            .info         = &vmstate_info_extra_state,
> +            .flags        = VMS_SINGLE,
> +            .offset       = 0,
> +        },
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_virtio_device_endian = {
>      .name = "virtio/device_endian",
>      .version_id = 1,

Re: [Qemu-devel] [PATCH V2 4/8] virtio-pci: fix 1.0 virtqueue migration

[Jason Wang]

On 09/02/2015 07:06 PM, Cornelia Huck wrote:
> On Wed,  2 Sep 2015 11:25:21 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> We don't migrate the followings fields for virtio-pci:
>>
>> uint32_t dfselect;
>> uint32_t gfselect;
>> uint32_t guest_features[2];
>> struct {
>>     uint16_t num;
>>     bool enabled;
>>     uint32_t desc[2];
>>     uint32_t avail[2];
>>     uint32_t used[2];
>> } vqs[VIRTIO_QUEUE_MAX];
>>
>> This will confuse driver if migrating during initialization. Solves
>> this issue by:
>>
>> - introduce transport specific callbacks to load and store extra
>>   virtqueue states.
>> - add a new subsection for virtio to migrate transport specific modern
>>   device state.
>> - implement pci specific callbacks.
>> - add a new property for virtio-pci for whether or not to migrate
>>   extra state.
>> - compat the migration for 2.4 and elder machine types
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  hw/virtio/virtio-pci.c         | 129 +++++++++++++++++++++++++++++++++++++++++
>>  hw/virtio/virtio-pci.h         |  20 ++++---
>>  hw/virtio/virtio.c             |  57 ++++++++++++++++++
>>  include/hw/compat.h            |   6 +-
>>  include/hw/virtio/virtio-bus.h |   3 +
>>  5 files changed, 207 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index c024161..a96890b 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -86,6 +86,129 @@ static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
>>          qemu_put_be16(f, vdev->config_vector);
>>  }
>>
>> +static void virtio_pci_load_modern_queue_state(VirtIOPCIQueue *vq,
>> +                                               QEMUFile *f)
>> +{
>> +    vq->num = qemu_get_be16(f);
>> +    vq->enabled = qemu_get_be16(f);
>> +    vq->desc[0] = qemu_get_be32(f);
>> +    vq->desc[1] = qemu_get_be32(f);
>> +    vq->avail[0] = qemu_get_be32(f);
>> +    vq->avail[1] = qemu_get_be32(f);
>> +    vq->used[0] = qemu_get_be32(f);
>> +    vq->used[1] = qemu_get_be32(f);
>> +}
>> +
>> +static bool virtio_pci_has_extra_state(DeviceState *d)
>> +{
>> +    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>> +
>> +    return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;
> Hm, couldn't you already check whether this is a modern device here?
> You don't need to save state for a legacy device, do you?

It was prepared for future. If we have something more that needs to be
migrated specifically for virtio-pci. We could just add a new subsection
for vmstate_virtio_pci without touching the core again. So we check the
modern in its subsection instead of here.
 
>
>> +}
>> +
> (...)
>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 788b556..dcaf022 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -1056,6 +1056,16 @@ static bool virtio_virtqueue_needed(void *opaque)
>>      return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>>  }
>>
>> +static bool virtio_extra_state_needed(void *opaque)
>> +{
>> +    VirtIODevice *vdev = opaque;
>> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>> +
>> +    return k->has_extra_state &&
>> +        k->has_extra_state(qbus->parent);
>> +}
>> +
>>  static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size)
>>  {
>>      VirtIODevice *vdev = pv;
>> @@ -1104,6 +1114,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = {
>>      }
>>  };
>>
>> +static int get_extra_state(QEMUFile *f, void *pv, size_t size)
>> +{
>> +    VirtIODevice *vdev = pv;
>> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>> +    int ret = 0;
>> +
>> +    ret = k->load_extra_state(qbus->parent, f);
> Should we check for ->load_extra_state() and return failure if it does
> not exist?

I think checking the existence of has_extra_state() in
virtio_extra_state_needed() is enough for this? Or is there anything I miss?

>
>> +
>> +    return ret;
>> +}
>> +
>> +static void put_extra_state(QEMUFile *f, void *pv, size_t size)
>> +{
>> +    VirtIODevice *vdev = pv;
>> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>> +
>> +    k->save_extra_state(qbus->parent, f);
> Maybe add an assert(k->save_extra_state) here, as this means the
> transport code is buggy?
>
>> +}
>> +
>> +static VMStateInfo vmstate_info_extra_state = {
>> +    .name = "virtqueue_extra_state",
>> +    .get = get_extra_state,
>> +    .put = put_extra_state,
>> +};
>> +
>> +static const VMStateDescription vmstate_virtio_modern_state = {
> Should probably be named "extra_state".

Right.

>
>> +    .name = "virtio/extra_state",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = &virtio_extra_state_needed,
>> +    .fields = (VMStateField[]) {
>> +        {
>> +            .name         = "modern_state",
> Also here.

Will fix this.

Thanks.

>> +            .version_id   = 0,
>> +            .field_exists = NULL,
>> +            .size         = 0,
>> +            .info         = &vmstate_info_extra_state,
>> +            .flags        = VMS_SINGLE,
>> +            .offset       = 0,
>> +        },
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>  static const VMStateDescription vmstate_virtio_device_endian = {
>>      .name = "virtio/device_endian",
>>      .version_id = 1,
>

Re: [Qemu-devel] [PATCH V2 4/8] virtio-pci: fix 1.0 virtqueue migration

[Cornelia Huck]

On Mon, 7 Sep 2015 15:39:59 +0800
Jason Wang <jasowang@redhat.com> wrote:

> On 09/02/2015 07:06 PM, Cornelia Huck wrote:
> > On Wed,  2 Sep 2015 11:25:21 +0800
> > Jason Wang <jasowang@redhat.com> wrote:

> >> +static int get_extra_state(QEMUFile *f, void *pv, size_t size)
> >> +{
> >> +    VirtIODevice *vdev = pv;
> >> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> >> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> >> +    int ret = 0;
> >> +
> >> +    ret = k->load_extra_state(qbus->parent, f);
> > Should we check for ->load_extra_state() and return failure if it does
> > not exist?
> 
> I think checking the existence of has_extra_state() in
> virtio_extra_state_needed() is enough for this? Or is there anything I miss?

The has_extra_state() callback is called by the sender, not by the
receiver.

If the other side sent the extra state but we can't handle it, I think
it would be better to fail the migration than to crash. Still broken
code, but probably easier to debug :)

Re: [Qemu-devel] [PATCH V2 4/8] virtio-pci: fix 1.0 virtqueue migration

[Jason Wang]

On 09/07/2015 04:21 PM, Cornelia Huck wrote:
> On Mon, 7 Sep 2015 15:39:59 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> > On 09/02/2015 07:06 PM, Cornelia Huck wrote:
>>> > > On Wed,  2 Sep 2015 11:25:21 +0800
>>> > > Jason Wang <jasowang@redhat.com> wrote:
>>>> > >> +static int get_extra_state(QEMUFile *f, void *pv, size_t size)
>>>> > >> +{
>>>> > >> +    VirtIODevice *vdev = pv;
>>>> > >> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>>>> > >> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>>>> > >> +    int ret = 0;
>>>> > >> +
>>>> > >> +    ret = k->load_extra_state(qbus->parent, f);
>>> > > Should we check for ->load_extra_state() and return failure if it does
>>> > > not exist?
>> > 
>> > I think checking the existence of has_extra_state() in
>> > virtio_extra_state_needed() is enough for this? Or is there anything I miss?
> The has_extra_state() callback is called by the sender, not by the
> receiver.
>
> If the other side sent the extra state but we can't handle it, I think
> it would be better to fail the migration than to crash. Still broken
> code, but probably easier to debug :)
>

I see so it only happen when transport provides save_extra_state() but
not get_extra_sate(). I'm ok to add the check in next version .

Thanks

Re: [Qemu-devel] [PATCH V2 4/8] virtio-pci: fix 1.0 virtqueue migration

[Michael S. Tsirkin]

On Tue, Sep 08, 2015 at 03:27:59PM +0800, Jason Wang wrote:
> 
> 
> On 09/07/2015 04:21 PM, Cornelia Huck wrote:
> > On Mon, 7 Sep 2015 15:39:59 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> >
> >> > On 09/02/2015 07:06 PM, Cornelia Huck wrote:
> >>> > > On Wed,  2 Sep 2015 11:25:21 +0800
> >>> > > Jason Wang <jasowang@redhat.com> wrote:
> >>>> > >> +static int get_extra_state(QEMUFile *f, void *pv, size_t size)
> >>>> > >> +{
> >>>> > >> +    VirtIODevice *vdev = pv;
> >>>> > >> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> >>>> > >> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> >>>> > >> +    int ret = 0;
> >>>> > >> +
> >>>> > >> +    ret = k->load_extra_state(qbus->parent, f);
> >>> > > Should we check for ->load_extra_state() and return failure if it does
> >>> > > not exist?
> >> > 
> >> > I think checking the existence of has_extra_state() in
> >> > virtio_extra_state_needed() is enough for this? Or is there anything I miss?
> > The has_extra_state() callback is called by the sender, not by the
> > receiver.
> >
> > If the other side sent the extra state but we can't handle it, I think
> > it would be better to fail the migration than to crash. Still broken
> > code, but probably easier to debug :)
> >
> 
> I see so it only happen when transport provides save_extra_state() but
> not get_extra_sate(). I'm ok to add the check in next version .
> 
> Thanks

This seems minor enough. I think I'll merge this as is,
we can tweak things using patches on top.

[Qemu-devel] [PATCH V2 5/8] memory: don't try to adjust endianness for zero length eventfd

[Jason Wang]

There's no need to adjust endianness for zero length eventfd since the
data wrote was actually ignored by kernel. So skip the adjust in this
case to fix a possible crash when trying to use wildcard mmio eventfd
in ppc.

Cc: Greg Kurz <gkurz@linux.vnet.ibm.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 memory.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/memory.c b/memory.c
index 0d8b2d9..de2d999 100644
--- a/memory.c
+++ b/memory.c
@@ -1653,7 +1653,9 @@ void memory_region_add_eventfd(MemoryRegion *mr,
     };
     unsigned i;
 
-    adjust_endianness(mr, &mrfd.data, size);
+    if (size) {
+        adjust_endianness(mr, &mrfd.data, size);
+    }
     memory_region_transaction_begin();
     for (i = 0; i < mr->ioeventfd_nb; ++i) {
         if (memory_region_ioeventfd_before(mrfd, mr->ioeventfds[i])) {
@@ -1686,7 +1688,9 @@ void memory_region_del_eventfd(MemoryRegion *mr,
     };
     unsigned i;
 
-    adjust_endianness(mr, &mrfd.data, size);
+    if (size) {
+        adjust_endianness(mr, &mrfd.data, size);
+    }
     memory_region_transaction_begin();
     for (i = 0; i < mr->ioeventfd_nb; ++i) {
         if (memory_region_ioeventfd_equal(mrfd, mr->ioeventfds[i])) {
-- 
2.1.4

Re: [Qemu-devel] [PATCH V2 5/8] memory: don't try to adjust endianness for zero length eventfd

[Greg Kurz]

On Wed,  2 Sep 2015 11:25:22 +0800
Jason Wang <jasowang@redhat.com> wrote:

> There's no need to adjust endianness for zero length eventfd since the
> data wrote was actually ignored by kernel. So skip the adjust in this
> case to fix a possible crash when trying to use wildcard mmio eventfd
> in ppc.
> 
> Cc: Greg Kurz <gkurz@linux.vnet.ibm.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---

Indeed, this patch prevents the crash to occur on ppc64.

Acked-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

>  memory.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 0d8b2d9..de2d999 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1653,7 +1653,9 @@ void memory_region_add_eventfd(MemoryRegion *mr,
>      };
>      unsigned i;
> 
> -    adjust_endianness(mr, &mrfd.data, size);
> +    if (size) {
> +        adjust_endianness(mr, &mrfd.data, size);
> +    }
>      memory_region_transaction_begin();
>      for (i = 0; i < mr->ioeventfd_nb; ++i) {
>          if (memory_region_ioeventfd_before(mrfd, mr->ioeventfds[i])) {
> @@ -1686,7 +1688,9 @@ void memory_region_del_eventfd(MemoryRegion *mr,
>      };
>      unsigned i;
> 
> -    adjust_endianness(mr, &mrfd.data, size);
> +    if (size) {
> +        adjust_endianness(mr, &mrfd.data, size);
> +    }
>      memory_region_transaction_begin();
>      for (i = 0; i < mr->ioeventfd_nb; ++i) {
>          if (memory_region_ioeventfd_equal(mrfd, mr->ioeventfds[i])) {

[Qemu-devel] [PATCH V2 6/8] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap

[Jason Wang]

We use data match eventfd for 1.0 notification currently. This could
be slow since software decoding is needed for mmio exit. To speed this
up, we can switch to use wild card mmio eventfd for 1.0 notification
since we can examine the queue index directly from the writing
address. KVM kernel module can utilize this by registering it to fast
mmio bus which could be as fast as pio on ept capable machine.

Lots of improvements were seen on a ept capable machine:

Guest RX:(TCP)
size/session/+throughput%/+cpu%/-+per cpu%/
64/1/+1.6807%/[-16.2421%]/[+21.3984%]/
64/2/+0.6091%/[-11.0187%]/[+13.0678%]/
64/4/+0.0553%/[-5.9768%]/[+6.4155%]/
64/8/+0.1206%/[-4.0057%]/[+4.2984%]/
256/1/-0.0031%/[-10.1166%]/[+11.2517%]/
256/2/-0.5058%/[-6.1656%]/+6.0317%]/
...

Guest TX:(TCP)
size/session/+throughput%/+cpu%/-+per cpu%/
64/1/[+18.9183%]/-0.2823%/[+19.2550%]/
64/2/[+13.5714%]/[+2.2675%]/[+11.0533%]/
64/4/[+13.1070%]/[+2.1817%]/[+10.6920%]/
64/8/[+13.0426%]/[+2.0887%]/[+10.7299%]/
256/1/[+36.2761%]/+6.3434%/[+28.1471%]/
...
1024/1/[+44.8873%]/+2.0811%/[+41.9335%]/
...
1024/4/+0.0228%/[-2.2044%]/[+2.2774%]/
...
16384/2/+0.0127%/[-5.0346%]/[+5.3148%]/
...
65535/1/[+0.0062%]/[-4.1183%]/[+4.3017%]/
65535/2/+0.0004%/[-4.2311%]/[+4.4185%]/
65535/4/+0.0107%/[-4.6106%]/[+4.8446%]/
65535/8/-0.0090%/[-5.5178%]/[+5.8306%]/

Latency:(TCP_RR)
size/session/+transaction rate%/+cpu%/-+per cpu%/
64/1/[+6.5248%]/[-9.2882%]/[+17.4322%]/
64/25/[+11.0854%]/[+0.8000%]/[+10.2038%]/
64/50/[+12.1076%]/[+2.4627%]/[+9.4131%]/
256/1/[+5.3677%]/[+10.5669%]/-4.7024%/
256/25/[+5.6402%]/-0.8962%/[+6.5955%]/
256/50/[+5.9685%]/[+1.7766%]/[+4.1188%]/
4096/1/+0.2508%/[-10.4941%]/[+12.0047%]/
4096/25/[+1.8533%]/-0.0273%/+1.8812%/
4096/50/[+1.2156%]/-1.4134%/+2.6667%/

Notes: data with '[]' is the one whose significance is greater than 95%.

Thanks Wenli Quan <wquan@redhat.com> for the benchmarking.

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a96890b..2d00d06 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -286,8 +286,8 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
         }
         virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
         if (modern) {
-            memory_region_add_eventfd(modern_mr, modern_addr, 2,
-                                      true, n, notifier);
+            memory_region_add_eventfd(modern_mr, modern_addr, 0,
+                                      false, n, notifier);
         }
         if (legacy) {
             memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
@@ -295,8 +295,8 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
         }
     } else {
         if (modern) {
-            memory_region_del_eventfd(modern_mr, modern_addr, 2,
-                                      true, n, notifier);
+            memory_region_del_eventfd(modern_mr, modern_addr, 0,
+                                      false, n, notifier);
         }
         if (legacy) {
             memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
-- 
2.1.4

Re: [Qemu-devel] [PATCH V2 6/8] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap

[Michael S. Tsirkin]

On Wed, Sep 02, 2015 at 11:25:23AM +0800, Jason Wang wrote:
> We use data match eventfd for 1.0 notification currently. This could
> be slow since software decoding is needed for mmio exit. To speed this
> up, we can switch to use wild card mmio eventfd for 1.0 notification
> since we can examine the queue index directly from the writing
> address. KVM kernel module can utilize this by registering it to fast
> mmio bus which could be as fast as pio on ept capable machine.
> 
> Lots of improvements were seen on a ept capable machine:
> 
> Guest RX:(TCP)
> size/session/+throughput%/+cpu%/-+per cpu%/
> 64/1/+1.6807%/[-16.2421%]/[+21.3984%]/
> 64/2/+0.6091%/[-11.0187%]/[+13.0678%]/
> 64/4/+0.0553%/[-5.9768%]/[+6.4155%]/
> 64/8/+0.1206%/[-4.0057%]/[+4.2984%]/
> 256/1/-0.0031%/[-10.1166%]/[+11.2517%]/
> 256/2/-0.5058%/[-6.1656%]/+6.0317%]/
> ...
> 
> Guest TX:(TCP)
> size/session/+throughput%/+cpu%/-+per cpu%/
> 64/1/[+18.9183%]/-0.2823%/[+19.2550%]/
> 64/2/[+13.5714%]/[+2.2675%]/[+11.0533%]/
> 64/4/[+13.1070%]/[+2.1817%]/[+10.6920%]/
> 64/8/[+13.0426%]/[+2.0887%]/[+10.7299%]/
> 256/1/[+36.2761%]/+6.3434%/[+28.1471%]/
> ...
> 1024/1/[+44.8873%]/+2.0811%/[+41.9335%]/
> ...
> 1024/4/+0.0228%/[-2.2044%]/[+2.2774%]/
> ...
> 16384/2/+0.0127%/[-5.0346%]/[+5.3148%]/
> ...
> 65535/1/[+0.0062%]/[-4.1183%]/[+4.3017%]/
> 65535/2/+0.0004%/[-4.2311%]/[+4.4185%]/
> 65535/4/+0.0107%/[-4.6106%]/[+4.8446%]/
> 65535/8/-0.0090%/[-5.5178%]/[+5.8306%]/
> 
> Latency:(TCP_RR)
> size/session/+transaction rate%/+cpu%/-+per cpu%/
> 64/1/[+6.5248%]/[-9.2882%]/[+17.4322%]/
> 64/25/[+11.0854%]/[+0.8000%]/[+10.2038%]/
> 64/50/[+12.1076%]/[+2.4627%]/[+9.4131%]/
> 256/1/[+5.3677%]/[+10.5669%]/-4.7024%/
> 256/25/[+5.6402%]/-0.8962%/[+6.5955%]/
> 256/50/[+5.9685%]/[+1.7766%]/[+4.1188%]/
> 4096/1/+0.2508%/[-10.4941%]/[+12.0047%]/
> 4096/25/[+1.8533%]/-0.0273%/+1.8812%/
> 4096/50/[+1.2156%]/-1.4134%/+2.6667%/
> 
> Notes: data with '[]' is the one whose significance is greater than 95%.
> 
> Thanks Wenli Quan <wquan@redhat.com> for the benchmarking.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Thanks a lot.
This makes sense, but I'm afraid this will break on old kernels
which don't allow len == 0.

Maybe we should add a new flag can_ignore_length to memory_region_add_eventfd.
Then it can retry registering with len = 0 and on failure retry with len = 2.

Or maybe we should add a new kvm capability for this, and only
try if it's there.  Will help avoid crashes on broken kernels.

Let's see what does Paolo say when he's back.


> ---
>  hw/virtio/virtio-pci.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index a96890b..2d00d06 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -286,8 +286,8 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
>          }
>          virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
>          if (modern) {
> -            memory_region_add_eventfd(modern_mr, modern_addr, 2,
> -                                      true, n, notifier);
> +            memory_region_add_eventfd(modern_mr, modern_addr, 0,
> +                                      false, n, notifier);
>          }
>          if (legacy) {
>              memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
> @@ -295,8 +295,8 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
>          }
>      } else {
>          if (modern) {
> -            memory_region_del_eventfd(modern_mr, modern_addr, 2,
> -                                      true, n, notifier);
> +            memory_region_del_eventfd(modern_mr, modern_addr, 0,
> +                                      false, n, notifier);
>          }
>          if (legacy) {
>              memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
> -- 
> 2.1.4

[Qemu-devel] [PATCH V2 7/8] virtio-pci: introduce pio notification capability for modern device

[Jason Wang]

We used to use mmio for notification. This could be slow on some arch
(e.g on x86 without EPT). So this patch introduces pio bar and a pio
notification cap for modern device. This ability is enabled through
property "modern-pio-notify" for virtio pci devices and was disabled
by default. Management can enable when it thinks it was needed.

Benchmarks shows almost no obvious difference compared to legacy
device on machines without ept. Thanks Wenli Quan <wquan@redhat.com>
for the benchmarking.

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.c | 122 ++++++++++++++++++++++++++++++++++++++++++-------
 hw/virtio/virtio-pci.h |  10 ++++
 2 files changed, 115 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 2d00d06..bf61acf 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -270,7 +270,9 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
     EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
     bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
     bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
+    bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY;
     MemoryRegion *modern_mr = &proxy->notify.mr;
+    MemoryRegion *modern_notify_mr = &proxy->notify_pio.mr;
     MemoryRegion *legacy_mr = &proxy->bar;
     hwaddr modern_addr = QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
                          virtio_get_queue_index(vq);
@@ -288,6 +290,10 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
         if (modern) {
             memory_region_add_eventfd(modern_mr, modern_addr, 0,
                                       false, n, notifier);
+            if (modern_pio) {
+                memory_region_add_eventfd(modern_notify_mr, 0, 2,
+                                              true, n, notifier);
+            }
         }
         if (legacy) {
             memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
@@ -297,6 +303,10 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
         if (modern) {
             memory_region_del_eventfd(modern_mr, modern_addr, 0,
                                       false, n, notifier);
+            if (modern_pio) {
+                memory_region_del_eventfd(modern_notify_mr, 0, 2,
+                                          true, n, notifier);
+            }
         }
         if (legacy) {
             memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
@@ -1404,6 +1414,17 @@ static void virtio_pci_notify_write(void *opaque, hwaddr addr,
     }
 }
 
+static void virtio_pci_notify_write_pio(void *opaque, hwaddr addr,
+                                        uint64_t val, unsigned size)
+{
+    VirtIODevice *vdev = opaque;
+    unsigned queue = val;
+
+    if (queue < VIRTIO_QUEUE_MAX) {
+        virtio_queue_notify(vdev, queue);
+    }
+}
+
 static uint64_t virtio_pci_isr_read(void *opaque, hwaddr addr,
                                     unsigned size)
 {
@@ -1497,6 +1518,16 @@ static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy)
         },
         .endianness = DEVICE_LITTLE_ENDIAN,
     };
+    static const MemoryRegionOps notify_pio_ops = {
+        .read = virtio_pci_notify_read,
+        .write = virtio_pci_notify_write_pio,
+        .impl = {
+            .min_access_size = 1,
+            .max_access_size = 4,
+        },
+        .endianness = DEVICE_LITTLE_ENDIAN,
+    };
+
 
     memory_region_init_io(&proxy->common.mr, OBJECT(proxy),
                           &common_ops,
@@ -1521,30 +1552,60 @@ static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy)
                           virtio_bus_get_device(&proxy->bus),
                           "virtio-pci-notify",
                           proxy->notify.size);
+
+    memory_region_init_io(&proxy->notify_pio.mr, OBJECT(proxy),
+                          &notify_pio_ops,
+                          virtio_bus_get_device(&proxy->bus),
+                          "virtio-pci-notify-pio",
+                          proxy->notify.size);
 }
 
 static void virtio_pci_modern_region_map(VirtIOPCIProxy *proxy,
                                          VirtIOPCIRegion *region,
-                                         struct virtio_pci_cap *cap)
+                                         struct virtio_pci_cap *cap,
+                                         MemoryRegion *mr,
+                                         uint8_t bar)
 {
-    memory_region_add_subregion(&proxy->modern_bar,
-                                region->offset,
-                                &region->mr);
+    memory_region_add_subregion(mr, region->offset, &region->mr);
 
     cap->cfg_type = region->type;
-    cap->bar = proxy->modern_mem_bar;
+    cap->bar = bar;
     cap->offset = cpu_to_le32(region->offset);
     cap->length = cpu_to_le32(region->size);
     virtio_pci_add_mem_cap(proxy, cap);
+
+}
+
+static void virtio_pci_modern_mem_region_map(VirtIOPCIProxy *proxy,
+                                             VirtIOPCIRegion *region,
+                                             struct virtio_pci_cap *cap)
+{
+    virtio_pci_modern_region_map(proxy, region, cap,
+                                 &proxy->modern_bar, proxy->modern_mem_bar);
 }
 
-static void virtio_pci_modern_region_unmap(VirtIOPCIProxy *proxy,
-                                           VirtIOPCIRegion *region)
+static void virtio_pci_modern_io_region_map(VirtIOPCIProxy *proxy,
+                                            VirtIOPCIRegion *region,
+                                            struct virtio_pci_cap *cap)
+{
+    virtio_pci_modern_region_map(proxy, region, cap,
+                                 &proxy->io_bar, proxy->modern_io_bar);
+}
+
+static void virtio_pci_modern_mem_region_unmap(VirtIOPCIProxy *proxy,
+                                               VirtIOPCIRegion *region)
 {
     memory_region_del_subregion(&proxy->modern_bar,
                                 &region->mr);
 }
 
+static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy,
+                                              VirtIOPCIRegion *region)
+{
+    memory_region_del_subregion(&proxy->io_bar,
+                                &region->mr);
+}
+
 /* This is called by virtio-bus just after the device is plugged. */
 static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
 {
@@ -1552,6 +1613,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
     VirtioBusState *bus = &proxy->bus;
     bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
     bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
+    bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY;
     uint8_t *config;
     uint32_t size;
     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
@@ -1590,16 +1652,31 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
             .cap.cap_len = sizeof cfg,
             .cap.cfg_type = VIRTIO_PCI_CAP_PCI_CFG,
         };
-        struct virtio_pci_cfg_cap *cfg_mask;
+        struct virtio_pci_notify_cap notify_pio = {
+            .cap.cap_len = sizeof notify,
+            .notify_off_multiplier = cpu_to_le32(0x0),
+        };
 
-        /* TODO: add io access for speed */
+        struct virtio_pci_cfg_cap *cfg_mask;
 
         virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);
         virtio_pci_modern_regions_init(proxy);
-        virtio_pci_modern_region_map(proxy, &proxy->common, &cap);
-        virtio_pci_modern_region_map(proxy, &proxy->isr, &cap);
-        virtio_pci_modern_region_map(proxy, &proxy->device, &cap);
-        virtio_pci_modern_region_map(proxy, &proxy->notify, &notify.cap);
+
+        virtio_pci_modern_mem_region_map(proxy, &proxy->common, &cap);
+        virtio_pci_modern_mem_region_map(proxy, &proxy->isr, &cap);
+        virtio_pci_modern_mem_region_map(proxy, &proxy->device, &cap);
+        virtio_pci_modern_mem_region_map(proxy, &proxy->notify, &notify.cap);
+
+        if (modern_pio) {
+            memory_region_init(&proxy->io_bar, OBJECT(proxy),
+                               "virtio-pci-io", 0x4);
+
+            pci_register_bar(&proxy->pci_dev, proxy->modern_io_bar,
+                             PCI_BASE_ADDRESS_SPACE_IO, &proxy->io_bar);
+
+            virtio_pci_modern_io_region_map(proxy, &proxy->notify_pio,
+                                            &notify_pio.cap);
+        }
 
         pci_register_bar(&proxy->pci_dev, proxy->modern_mem_bar,
                          PCI_BASE_ADDRESS_SPACE_MEMORY |
@@ -1652,14 +1729,18 @@ static void virtio_pci_device_unplugged(DeviceState *d)
 {
     VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
     bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
+    bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY;
 
     virtio_pci_stop_ioeventfd(proxy);
 
     if (modern) {
-        virtio_pci_modern_region_unmap(proxy, &proxy->common);
-        virtio_pci_modern_region_unmap(proxy, &proxy->isr);
-        virtio_pci_modern_region_unmap(proxy, &proxy->device);
-        virtio_pci_modern_region_unmap(proxy, &proxy->notify);
+        virtio_pci_modern_mem_region_unmap(proxy, &proxy->common);
+        virtio_pci_modern_mem_region_unmap(proxy, &proxy->isr);
+        virtio_pci_modern_mem_region_unmap(proxy, &proxy->device);
+        virtio_pci_modern_mem_region_unmap(proxy, &proxy->notify);
+        if (modern_pio) {
+            virtio_pci_modern_io_region_unmap(proxy, &proxy->notify_pio);
+        }
     }
 }
 
@@ -1679,6 +1760,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
      */
     proxy->legacy_io_bar  = 0;
     proxy->msix_bar       = 1;
+    proxy->modern_io_bar  = 2;
     proxy->modern_mem_bar = 4;
 
     proxy->common.offset = 0x0;
@@ -1698,6 +1780,10 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
         QEMU_VIRTIO_PCI_QUEUE_MEM_MULT * VIRTIO_QUEUE_MAX;
     proxy->notify.type = VIRTIO_PCI_CAP_NOTIFY_CFG;
 
+    proxy->notify_pio.offset = 0x0;
+    proxy->notify_pio.size = 0x4;
+    proxy->notify_pio.type = VIRTIO_PCI_CAP_NOTIFY_CFG;
+
     /* subclasses can enforce modern, so do this unconditionally */
     memory_region_init(&proxy->modern_bar, OBJECT(proxy), "virtio-pci",
                        2 * QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
@@ -1744,6 +1830,8 @@ static Property virtio_pci_properties[] = {
                     VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true),
     DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true),
+    DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index d890009..3d376f2 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -79,6 +79,13 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
 #define VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT 4
 #define VIRTIO_PCI_FLAG_MIGRATE_EXTRA (1 << VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT)
 
+/* have pio notification for modern device ? */
+#define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT 5
+#define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \
+    (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT)
+
+
+
 typedef struct {
     MSIMessage msg;
     int virq;
@@ -123,11 +130,14 @@ struct VirtIOPCIProxy {
     VirtIOPCIRegion isr;
     VirtIOPCIRegion device;
     VirtIOPCIRegion notify;
+    VirtIOPCIRegion notify_pio;
     MemoryRegion modern_bar;
+    MemoryRegion io_bar;
     MemoryRegion modern_cfg;
     AddressSpace modern_as;
     uint32_t legacy_io_bar;
     uint32_t msix_bar;
+    uint32_t modern_io_bar;
     uint32_t modern_mem_bar;
     int config_cap;
     uint32_t flags;
-- 
2.1.4

[Qemu-devel] [PATCH V2 8/8] virtio-pci: unbreak queue_enable read

[Jason Wang]

Guest always get zero when reading queue_enable. This violates
spec. Fixing this by setting the queue_enable to true during any guest
writing and setting it to zero during reset.

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index bf61acf..7aab6d1 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1372,6 +1372,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
                        proxy->vqs[vdev->queue_sel].avail[0],
                        ((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
                        proxy->vqs[vdev->queue_sel].used[0]);
+        proxy->vqs[vdev->queue_sel].enabled = 1;
         break;
     case VIRTIO_PCI_COMMON_Q_DESCLO:
         proxy->vqs[vdev->queue_sel].desc[0] = val;
@@ -1816,9 +1817,15 @@ static void virtio_pci_reset(DeviceState *qdev)
 {
     VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev);
     VirtioBusState *bus = VIRTIO_BUS(&proxy->bus);
+    int i;
+
     virtio_pci_stop_ioeventfd(proxy);
     virtio_bus_reset(bus);
     msix_unuse_all_vectors(&proxy->pci_dev);
+
+    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+        proxy->vqs[i].enabled = 0;
+    }
 }
 
 static Property virtio_pci_properties[] = {
-- 
2.1.4

Re: [Qemu-devel] [PATCH V2 0/8] virtio 1.0 pci optimizations and fixes

[Michael S. Tsirkin]

On Wed, Sep 02, 2015 at 11:25:17AM +0800, Jason Wang wrote:
> Hi all:
> 
> This series tries to fix the following issues:
> 
> - qemu abort when trying to adjust endianness for zero length eventfd,
>   this prevent fast mmio eventfd from being used in ppc. Fixing by
>   skip the endianness adjustment for zero length eventfd.
> - 1.0 mmio is slow since it was using datamatch eventfd. Fixing this
>   by usinng wildcard mmio eventfd, then we could get speed up through
>   kernel fast mmio bus on ept capable machine.
> - 1.0 mmio is slow compared to pio (at least on some
>   archs/setups). Fixing this by re-introducing pio notification
>   capability. This will be useful for the arch/setups that fast mmio
>   does not work.
> - Some virtio pci 1.0 fields were not migrated. This will cause
>   unexpected behaviour if migrate during driver initialization. Fixing
>   this by introduce a transport specific callback and get/put
>   transport specific fields for virtio 1.0.
> - queue_enable read was broken. Fixing by set the queue_enable to true
>   during guest write and clear it during reset.
> 
> Please review.
> Thanks

Wanted to merge this, but it needs to be rebased to latest master.
Pls do, and repost.

> Changes from V1:
> - skip zero length eventfd endianness adjustment
> - don't use pci specific name ("modern") in virtio core, using "extra"
>   instead and in virtio pci callback, using subsections which could
>   allow us to extend the future improvement without changing the core.
> - don't check virtio_virtqueue_needed() in virtio_extra_state_needed()
> - drop the ppc 2.5 machine type patch
> - squash Eduardo's 2.5 machine type patches into this series
> 
> Eduardo Habkost (3):
>   q35: Move options common to all classes to pc_q35_machine_options()
>   q35: Move options common to all classes to pc_i440fx_machine_options()
>   pc: Introduce pc-*-2.5 machine classes
> 
> Jason Wang (5):
>   virtio-pci: fix 1.0 virtqueue migration
>   memory: don't try to adjust endianness for zero length eventfd
>   virtio-pci: use wildcard mmio eventfd for 1.0 notification cap
>   virtio-pci: introduce pio notification capability for modern device
>   virtio-pci: unbreak queue_enable read
> 
>  hw/i386/pc_piix.c              |  18 ++-
>  hw/i386/pc_q35.c               |  20 +++-
>  hw/virtio/virtio-pci.c         | 266 +++++++++++++++++++++++++++++++++++++----
>  hw/virtio/virtio-pci.h         |  30 +++--
>  hw/virtio/virtio.c             |  57 +++++++++
>  include/hw/compat.h            |   7 ++
>  include/hw/i386/pc.h           |   4 +
>  include/hw/virtio/virtio-bus.h |   3 +
>  memory.c                       |   8 +-
>  9 files changed, 375 insertions(+), 38 deletions(-)
> 
> -- 
> 2.1.4

Re: [Qemu-devel] [PATCH V2 0/8] virtio 1.0 pci optimizations and fixes

[Michael S. Tsirkin]

On Wed, Sep 02, 2015 at 11:25:17AM +0800, Jason Wang wrote:
> Hi all:
> 
> This series tries to fix the following issues:
> 
> - qemu abort when trying to adjust endianness for zero length eventfd,
>   this prevent fast mmio eventfd from being used in ppc. Fixing by
>   skip the endianness adjustment for zero length eventfd.
> - 1.0 mmio is slow since it was using datamatch eventfd. Fixing this
>   by usinng wildcard mmio eventfd, then we could get speed up through
>   kernel fast mmio bus on ept capable machine.
> - 1.0 mmio is slow compared to pio (at least on some
>   archs/setups). Fixing this by re-introducing pio notification
>   capability. This will be useful for the arch/setups that fast mmio
>   does not work.
> - Some virtio pci 1.0 fields were not migrated. This will cause
>   unexpected behaviour if migrate during driver initialization. Fixing
>   this by introduce a transport specific callback and get/put
>   transport specific fields for virtio 1.0.
> - queue_enable read was broken. Fixing by set the queue_enable to true
>   during guest write and clear it during reset.
> 
> Please review.
> Thanks

I expected to get a new version of this one by now.
Can you update and repost pls?

> Changes from V1:
> - skip zero length eventfd endianness adjustment
> - don't use pci specific name ("modern") in virtio core, using "extra"
>   instead and in virtio pci callback, using subsections which could
>   allow us to extend the future improvement without changing the core.
> - don't check virtio_virtqueue_needed() in virtio_extra_state_needed()
> - drop the ppc 2.5 machine type patch
> - squash Eduardo's 2.5 machine type patches into this series
> 
> Eduardo Habkost (3):
>   q35: Move options common to all classes to pc_q35_machine_options()
>   q35: Move options common to all classes to pc_i440fx_machine_options()
>   pc: Introduce pc-*-2.5 machine classes
> 
> Jason Wang (5):
>   virtio-pci: fix 1.0 virtqueue migration
>   memory: don't try to adjust endianness for zero length eventfd
>   virtio-pci: use wildcard mmio eventfd for 1.0 notification cap
>   virtio-pci: introduce pio notification capability for modern device
>   virtio-pci: unbreak queue_enable read
> 
>  hw/i386/pc_piix.c              |  18 ++-
>  hw/i386/pc_q35.c               |  20 +++-
>  hw/virtio/virtio-pci.c         | 266 +++++++++++++++++++++++++++++++++++++----
>  hw/virtio/virtio-pci.h         |  30 +++--
>  hw/virtio/virtio.c             |  57 +++++++++
>  include/hw/compat.h            |   7 ++
>  include/hw/i386/pc.h           |   4 +
>  include/hw/virtio/virtio-bus.h |   3 +
>  memory.c                       |   8 +-
>  9 files changed, 375 insertions(+), 38 deletions(-)
> 
> -- 
> 2.1.4