[Qemu-devel] [PATCH V3 0/6] virtio 1.0 pci optimizations and fixes

[Qemu-devel] [PATCH V3 0/6] 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 zero length 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 V2:
- rebase to master
- conditionally use zero length io eventfd based on new kvm cap
- replace "modern_state" with "extra_state" in patch 1
- check for the existence of k->load_extra_state in patch 1

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

Jason Wang (6):
  virtio-pci: fix 1.0 virtqueue migration
  memory: don't try to adjust endianness for zero length eventfd
  KVM: add support for any length io eventfd
  virtio-pci: use zero length mmio eventfd for 1.0 notification cap when
    possible
  virtio-pci: introduce pio notification capability for modern device
  virtio-pci: unbreak queue_enable read

 hw/virtio/virtio-pci.c         | 277 +++++++++++++++++++++++++++++++++++++----
 hw/virtio/virtio-pci.h         |  28 +++--
 hw/virtio/virtio.c             |  58 +++++++++
 include/hw/compat.h            |   4 +
 include/hw/virtio/virtio-bus.h |   3 +
 include/sysemu/kvm.h           |   8 ++
 kvm-all.c                      |   4 +
 kvm-stub.c                     |   1 +
 memory.c                       |   8 +-
 9 files changed, 361 insertions(+), 30 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH V3 1/6] 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             |  58 ++++++++++++++++++
 include/hw/compat.h            |   4 ++
 include/hw/virtio/virtio-bus.h |   3 +
 5 files changed, 207 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index f55dd2b..ab2a372 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 const 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;
 }
 
@@ -1622,6 +1746,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(),
 };
 
@@ -2212,6 +2338,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 801c23a..58679f7 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 939f802..1edef59 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1116,6 +1116,16 @@ static bool virtio_ringsize_needed(void *opaque)
     return false;
 }
 
+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;
@@ -1210,6 +1220,53 @@ static const VMStateDescription vmstate_virtio_ringsize = {
     }
 };
 
+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);
+
+    if (!k->load_extra_state) {
+        return -1;
+    } else {
+        return k->load_extra_state(qbus->parent, f);
+    }
+}
+
+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 const VMStateInfo vmstate_info_extra_state = {
+    .name = "virtqueue_extra_state",
+    .get = get_extra_state,
+    .put = put_extra_state,
+};
+
+static const VMStateDescription vmstate_virtio_extra_state = {
+    .name = "virtio/extra_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = &virtio_extra_state_needed,
+    .fields = (VMStateField[]) {
+        {
+            .name         = "extra_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,
@@ -1245,6 +1302,7 @@ static const VMStateDescription vmstate_virtio = {
         &vmstate_virtio_64bit_features,
         &vmstate_virtio_virtqueues,
         &vmstate_virtio_ringsize,
+        &vmstate_virtio_extra_state,
         NULL
     }
 };
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 93e71af..65799c1 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -6,6 +6,10 @@
             .driver   = "virtio-blk-device",\
             .property = "scsi",\
             .value    = "true",\
+        },{\
+            .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

[Qemu-devel] [PATCH V3 2/6] 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>
Acked-by: Greg Kurz <gkurz@linux.vnet.ibm.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 c435c88..e193658 100644
--- a/memory.c
+++ b/memory.c
@@ -1688,7 +1688,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])) {
@@ -1721,7 +1723,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

[Qemu-devel] [PATCH V3 3/6] KVM: add support for any length io eventfd

[Jason Wang]

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/sysemu/kvm.h | 8 ++++++++
 kvm-all.c            | 4 ++++
 kvm-stub.c           | 1 +
 3 files changed, 13 insertions(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 4ac6176..b31f325 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -53,6 +53,7 @@ extern bool kvm_gsi_routing_allowed;
 extern bool kvm_gsi_direct_mapping;
 extern bool kvm_readonly_mem_allowed;
 extern bool kvm_direct_msi_allowed;
+extern bool kvm_ioeventfd_any_length_allowed;
 
 #if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled()           (kvm_allowed)
@@ -153,6 +154,12 @@ extern bool kvm_direct_msi_allowed;
  */
 #define kvm_direct_msi_enabled() (kvm_direct_msi_allowed)
 
+/**
+ * kvm_ioeventfd_any_length_enabled:
+ * Returns: true if KVM allows any length io eventfd.
+ */
+#define kvm_ioeventfd_any_length_enabled() (kvm_ioeventfd_any_length_allowed)
+
 #else
 #define kvm_enabled()           (0)
 #define kvm_irqchip_in_kernel() (false)
@@ -166,6 +173,7 @@ extern bool kvm_direct_msi_allowed;
 #define kvm_gsi_direct_mapping() (false)
 #define kvm_readonly_mem_enabled() (false)
 #define kvm_direct_msi_enabled() (false)
+#define kvm_ioeventfd_any_length_enabled() (false)
 #endif
 
 struct kvm_run;
diff --git a/kvm-all.c b/kvm-all.c
index 1bc1273..f4e2cf0 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -109,6 +109,7 @@ bool kvm_allowed;
 bool kvm_readonly_mem_allowed;
 bool kvm_vm_attributes_allowed;
 bool kvm_direct_msi_allowed;
+bool kvm_ioeventfd_any_length_allowed;
 
 static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_INFO(USER_MEMORY),
@@ -1612,6 +1613,9 @@ static int kvm_init(MachineState *ms)
     kvm_vm_attributes_allowed =
         (kvm_check_extension(s, KVM_CAP_VM_ATTRIBUTES) > 0);
 
+    kvm_ioeventfd_any_length_allowed =
+        (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0);
+
     ret = kvm_arch_init(ms, s);
     if (ret < 0) {
         goto err;
diff --git a/kvm-stub.c b/kvm-stub.c
index a5051f7..dc97a5e 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -30,6 +30,7 @@ bool kvm_gsi_routing_allowed;
 bool kvm_gsi_direct_mapping;
 bool kvm_allowed;
 bool kvm_readonly_mem_allowed;
+bool kvm_ioeventfd_any_length_allowed;
 
 int kvm_init_vcpu(CPUState *cpu)
 {
-- 
2.1.4

[Qemu-devel] [PATCH V3 4/6] virtio-pci: use zero length mmio eventfd for 1.0 notification cap when possible

[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 zero length 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 when
fast mmio is supported by host kernel.

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 | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ab2a372..9c3f6e3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -270,6 +270,7 @@ 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 fast_mmio = kvm_ioeventfd_any_length_enabled();
     MemoryRegion *modern_mr = &proxy->notify.mr;
     MemoryRegion *legacy_mr = &proxy->bar;
     hwaddr modern_addr = QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
@@ -286,8 +287,13 @@ 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);
+            if (fast_mmio) {
+                memory_region_add_eventfd(modern_mr, modern_addr, 0,
+                                          false, n, notifier);
+            } else {
+                memory_region_add_eventfd(modern_mr, modern_addr, 2,
+                                          false, n, notifier);
+            }
         }
         if (legacy) {
             memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
@@ -295,8 +301,13 @@ 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);
+            if (fast_mmio) {
+                memory_region_del_eventfd(modern_mr, modern_addr, 0,
+                                          false, n, notifier);
+            } else {
+                memory_region_del_eventfd(modern_mr, modern_addr, 2,
+                                          false, n, notifier);
+            }
         }
         if (legacy) {
             memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
-- 
2.1.4

[Qemu-devel] [PATCH V3 5/6] 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 |   8 ++++
 2 files changed, 113 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 9c3f6e3..e4449da 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -271,7 +271,9 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
     bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
     bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
     bool fast_mmio = kvm_ioeventfd_any_length_enabled();
+    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);
@@ -294,6 +296,10 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
                 memory_region_add_eventfd(modern_mr, modern_addr, 2,
                                           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,
@@ -308,6 +314,10 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
                 memory_region_del_eventfd(modern_mr, modern_addr, 2,
                                           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,
@@ -1416,6 +1426,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)
 {
@@ -1509,6 +1530,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,
@@ -1533,30 +1564,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)
 {
@@ -1564,6 +1625,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);
@@ -1602,16 +1664,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 |
@@ -1667,14 +1744,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);
+        }
     }
 }
 
@@ -1694,6 +1775,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;
@@ -1713,6 +1795,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 *
@@ -1759,6 +1845,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 58679f7..460c3c9 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -79,6 +79,11 @@ 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 +128,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 V3 6/6] 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 e4449da..52a9e33 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1384,6 +1384,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;
@@ -1831,9 +1832,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 V3 1/6] virtio-pci: fix 1.0 virtqueue migration

[Cornelia Huck]

On Fri,  6 Nov 2015 16:02:44 +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             |  58 ++++++++++++++++++
>  include/hw/compat.h            |   4 ++
>  include/hw/virtio/virtio-bus.h |   3 +
>  5 files changed, 207 insertions(+), 7 deletions(-)

> @@ -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);
>      }
> +

whitespace change :)

>      return 0;
>  }
> 

(...)

> +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);
> +

I'd probably put an 'assert(k->save_extra_state)' here. (I think we
have discussed this before, but I can't recall the outcome.)

> +    k->save_extra_state(qbus->parent, f);
> +}

Otherwise, this looks good to me. With or with out the assert,

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [Qemu-devel] [PATCH V3 6/6] virtio-pci: unbreak queue_enable read

[Cornelia Huck]

On Fri,  6 Nov 2015 16:02:49 +0800
Jason Wang <jasowang@redhat.com> wrote:

> 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 e4449da..52a9e33 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1384,6 +1384,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;

Hm. Spec says that the driver MUST NOT write 0 here. Does this need a
check?

<Also, I think you can remove the 'TODO' comment - this should be
handled now.>

>          break;
>      case VIRTIO_PCI_COMMON_Q_DESCLO:
>          proxy->vqs[vdev->queue_sel].desc[0] = val;
> @@ -1831,9 +1832,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[] = {

Otherwise, this looks good to me.

Re: [Qemu-devel] [PATCH V3 1/6] virtio-pci: fix 1.0 virtqueue migration

[Jason Wang]

On 11/09/2015 08:56 PM, Cornelia Huck wrote:
> On Fri,  6 Nov 2015 16:02:44 +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             |  58 ++++++++++++++++++
>>  include/hw/compat.h            |   4 ++
>>  include/hw/virtio/virtio-bus.h |   3 +
>>  5 files changed, 207 insertions(+), 7 deletions(-)
>> @@ -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);
>>      }
>> +
> whitespace change :)

Right, I will wait for other comments to see if v4 is needed (and fix
there).

>>      return 0;
>>  }
>>
> (...)
>
>> +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);
>> +
> I'd probably put an 'assert(k->save_extra_state)' here. (I think we
> have discussed this before, but I can't recall the outcome.)

Yes, may help to hunt bugs earlier.

>
>> +    k->save_extra_state(qbus->parent, f);
>> +}
> Otherwise, this looks good to me. With or with out the assert,
>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>
>

Re: [Qemu-devel] [PATCH V3 6/6] virtio-pci: unbreak queue_enable read

[Jason Wang]

On 11/09/2015 09:14 PM, Cornelia Huck wrote:
> On Fri,  6 Nov 2015 16:02:49 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> 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 e4449da..52a9e33 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -1384,6 +1384,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;
> Hm. Spec says that the driver MUST NOT write 0 here. 

Yes.

> Does this need a
> check?

Add warn in this case? I'm not sure, I read the spec as "writing any
value implies enabling". (And the patch itself does not conflict the spec).

>
> <Also, I think you can remove the 'TODO' comment - this should be
> handled now.>

Yes

>
>>          break;
>>      case VIRTIO_PCI_COMMON_Q_DESCLO:
>>          proxy->vqs[vdev->queue_sel].desc[0] = val;
>> @@ -1831,9 +1832,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[] = {
> Otherwise, this looks good to me.
>
>

Thanks

Re: [Qemu-devel] [PATCH V3 3/6] KVM: add support for any length io eventfd

[Paolo Bonzini]

On 06/11/2015 09:02, Jason Wang wrote:
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  include/sysemu/kvm.h | 8 ++++++++
>  kvm-all.c            | 4 ++++
>  kvm-stub.c           | 1 +
>  3 files changed, 13 insertions(+)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 4ac6176..b31f325 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -53,6 +53,7 @@ extern bool kvm_gsi_routing_allowed;
>  extern bool kvm_gsi_direct_mapping;
>  extern bool kvm_readonly_mem_allowed;
>  extern bool kvm_direct_msi_allowed;
> +extern bool kvm_ioeventfd_any_length_allowed;
>  
>  #if defined CONFIG_KVM || !defined NEED_CPU_H
>  #define kvm_enabled()           (kvm_allowed)
> @@ -153,6 +154,12 @@ extern bool kvm_direct_msi_allowed;
>   */
>  #define kvm_direct_msi_enabled() (kvm_direct_msi_allowed)
>  
> +/**
> + * kvm_ioeventfd_any_length_enabled:
> + * Returns: true if KVM allows any length io eventfd.
> + */
> +#define kvm_ioeventfd_any_length_enabled() (kvm_ioeventfd_any_length_allowed)
> +
>  #else
>  #define kvm_enabled()           (0)
>  #define kvm_irqchip_in_kernel() (false)
> @@ -166,6 +173,7 @@ extern bool kvm_direct_msi_allowed;
>  #define kvm_gsi_direct_mapping() (false)
>  #define kvm_readonly_mem_enabled() (false)
>  #define kvm_direct_msi_enabled() (false)
> +#define kvm_ioeventfd_any_length_enabled() (false)
>  #endif
>  
>  struct kvm_run;
> diff --git a/kvm-all.c b/kvm-all.c
> index 1bc1273..f4e2cf0 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -109,6 +109,7 @@ bool kvm_allowed;
>  bool kvm_readonly_mem_allowed;
>  bool kvm_vm_attributes_allowed;
>  bool kvm_direct_msi_allowed;
> +bool kvm_ioeventfd_any_length_allowed;
>  
>  static const KVMCapabilityInfo kvm_required_capabilites[] = {
>      KVM_CAP_INFO(USER_MEMORY),
> @@ -1612,6 +1613,9 @@ static int kvm_init(MachineState *ms)
>      kvm_vm_attributes_allowed =
>          (kvm_check_extension(s, KVM_CAP_VM_ATTRIBUTES) > 0);
>  
> +    kvm_ioeventfd_any_length_allowed =
> +        (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0);
> +
>      ret = kvm_arch_init(ms, s);
>      if (ret < 0) {
>          goto err;
> diff --git a/kvm-stub.c b/kvm-stub.c
> index a5051f7..dc97a5e 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -30,6 +30,7 @@ bool kvm_gsi_routing_allowed;
>  bool kvm_gsi_direct_mapping;
>  bool kvm_allowed;
>  bool kvm_readonly_mem_allowed;
> +bool kvm_ioeventfd_any_length_allowed;
>  
>  int kvm_init_vcpu(CPUState *cpu)
>  {
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>