qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes
@ 2015-08-21  9:05 Jason Wang
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type Jason Wang
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-21  9:05 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: Jason Wang

Hi all:

This series tries to fix the following issues:

- 1.0 mmio is slow. 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. 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

Jason Wang (6):
  pc: introduce 2.5 machine type
  ppc: spapr: introduce 2.5 machine type
  virtio-pci: fix 1.0 virtqueue migration
  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              |  21 ++++-
 hw/i386/pc_q35.c               |  23 ++++-
 hw/ppc/spapr.c                 |  40 +++++++-
 hw/virtio/virtio-pci.c         | 206 ++++++++++++++++++++++++++++++++++++-----
 hw/virtio/virtio-pci.h         |  30 ++++--
 hw/virtio/virtio.c             |  58 ++++++++++++
 include/hw/compat.h            |   7 ++
 include/hw/i386/pc.h           |   3 +
 include/hw/virtio/virtio-bus.h |   3 +
 9 files changed, 357 insertions(+), 34 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type
  2015-08-21  9:05 [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes Jason Wang
@ 2015-08-21  9:05 ` Jason Wang
  2015-08-21 15:47   ` Eduardo Habkost
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 2/6] ppc: spapr: " Jason Wang
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2015-08-21  9:05 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: Paolo Bonzini, Jason Wang, Eduardo Habkost, Richard Henderson

This will be used by virtio 1.0 virtio-pci virtqueue migration
backward compatibility.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/pc_piix.c    | 21 +++++++++++++++++++--
 hw/i386/pc_q35.c     | 23 +++++++++++++++++++++--
 include/hw/compat.h  |  3 +++
 include/hw/i386/pc.h |  3 +++
 4 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a896624..2a7b7d9 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -304,9 +304,15 @@ static void pc_init1(MachineState *machine)
     }
 }
 
+static void pc_compat_2_4(MachineState *machine)
+{
+}
+
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -477,7 +483,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
     m->hot_add_cpu = pc_hot_add_cpu;
 }
 
-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->default_machine_opts = "firmware=bios-256k.bin";
@@ -486,7 +492,18 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
     m->is_default = 1;
 }
 
-DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
+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_machine_options(m);
+    m->default_machine_opts = "firmware=bios-256k.bin";
+    m->default_display = "std";
+    SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
+}
+
+DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", pc_compat_2_4,
                       pc_i440fx_2_4_machine_options)
 
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 974aead..88d3076 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -287,9 +287,15 @@ static void pc_q35_init(MachineState *machine)
     }
 }
 
+static void pc_compat_2_4(MachineState *machine)
+{
+}
+
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -392,7 +398,7 @@ static void pc_q35_machine_options(MachineClass *m)
     m->units_per_default_bus = 1;
 }
 
-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->default_machine_opts = "firmware=bios-256k.bin";
@@ -402,7 +408,20 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
     m->alias = "q35";
 }
 
-DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
+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_machine_options(m);
+    m->default_machine_opts = "firmware=bios-256k.bin";
+    m->default_display = "std";
+    m->no_floppy = 1;
+    m->no_tco = 0;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
+}
+
+DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", pc_compat_2_4,
                    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 954203d..156b4ef 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -292,6 +292,9 @@ 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 \
         HW_COMPAT_2_3 \
         {\
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 2/6] ppc: spapr: introduce 2.5 machine type
  2015-08-21  9:05 [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes Jason Wang
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type Jason Wang
@ 2015-08-21  9:05 ` Jason Wang
  2015-08-22  0:10   ` David Gibson
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration Jason Wang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2015-08-21  9:05 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: Jason Wang, qemu-ppc, Alexander Graf, David Gibson

This will be used by virtio 1.0 virtio-pci virtqueue migration
backward compatibility.

Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/ppc/spapr.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a6f1947..7d5b0db 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1855,7 +1855,11 @@ static const TypeInfo spapr_machine_info = {
     },
 };
 
+#define SPAPR_COMPAT_2_4 \
+        HW_COMPAT_2_4
+
 #define SPAPR_COMPAT_2_3 \
+        SPAPR_COMPAT_2_4 \
         HW_COMPAT_2_3 \
         {\
             .driver   = "spapr-pci-host-bridge",\
@@ -1876,8 +1880,13 @@ static const TypeInfo spapr_machine_info = {
         SPAPR_COMPAT_2_2 \
         HW_COMPAT_2_1
 
+static void spapr_compat_2_4(Object *obj)
+{
+}
+
 static void spapr_compat_2_3(Object *obj)
 {
+    spapr_compat_2_4(obj);
     savevm_skip_section_footers();
     global_state_set_optional();
 }
@@ -1892,6 +1901,12 @@ static void spapr_compat_2_1(Object *obj)
     spapr_compat_2_2(obj);
 }
 
+static void spapr_machine_2_4_instance_init(Object *obj)
+{
+    spapr_compat_2_4(obj);
+    spapr_machine_initfn(obj);
+}
+
 static void spapr_machine_2_3_instance_init(Object *obj)
 {
     spapr_compat_2_3(obj);
@@ -1972,18 +1987,38 @@ static const TypeInfo spapr_machine_2_3_info = {
 
 static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
 {
+    static GlobalProperty compat_props[] = {
+        SPAPR_COMPAT_2_4
+        { /* end of list */ }
+    };
     MachineClass *mc = MACHINE_CLASS(oc);
 
     mc->name = "pseries-2.4";
     mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
-    mc->alias = "pseries";
-    mc->is_default = 1;
+    mc->compat_props = compat_props;
 }
 
 static const TypeInfo spapr_machine_2_4_info = {
     .name          = TYPE_SPAPR_MACHINE "2.4",
     .parent        = TYPE_SPAPR_MACHINE,
     .class_init    = spapr_machine_2_4_class_init,
+    .instance_init = spapr_machine_2_4_instance_init,
+};
+
+static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->name = "pseries-2.5";
+    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
+    mc->alias = "pseries";
+    mc->is_default = 1;
+}
+
+static const TypeInfo spapr_machine_2_5_info = {
+    .name          = TYPE_SPAPR_MACHINE "2.5",
+    .parent        = TYPE_SPAPR_MACHINE,
+    .class_init    = spapr_machine_2_5_class_init,
 };
 
 static void spapr_machine_register_types(void)
@@ -1993,6 +2028,7 @@ static void spapr_machine_register_types(void)
     type_register_static(&spapr_machine_2_2_info);
     type_register_static(&spapr_machine_2_3_info);
     type_register_static(&spapr_machine_2_4_info);
+    type_register_static(&spapr_machine_2_5_info);
 }
 
 type_init(spapr_machine_register_types)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration
  2015-08-21  9:05 [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes Jason Wang
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type Jason Wang
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 2/6] ppc: spapr: " Jason Wang
@ 2015-08-21  9:05 ` Jason Wang
  2015-08-21  9:43   ` Cornelia Huck
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 4/6] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap Jason Wang
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2015-08-21  9:05 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: 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 modern
  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
  modern 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         | 69 ++++++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio-pci.h         | 20 +++++++-----
 hw/virtio/virtio.c             | 58 +++++++++++++++++++++++++++++++++++
 include/hw/compat.h            |  6 +++-
 include/hw/virtio/virtio-bus.h |  3 ++
 5 files changed, 148 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c024161..d785623 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -86,6 +86,69 @@ 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_modern_state(DeviceState *d)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+
+    return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_MODERN;
+}
+
+static int virtio_pci_load_modern_state(DeviceState *d, QEMUFile *f)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    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 virtio_pci_save_modern_state(DeviceState *d, QEMUFile *f)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    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 void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
@@ -133,6 +196,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 +1682,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-modern", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_MIGRATE_MODERN_BIT, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2211,6 +2277,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_modern_state = virtio_pci_save_modern_state;
+    k->load_modern_state = virtio_pci_load_modern_state;
+    k->has_modern_state = virtio_pci_has_modern_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..fd2e889 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 modern state ? */
+#define VIRTIO_PCI_FLAG_MIGRATE_MODERN_BIT 4
+#define VIRTIO_PCI_FLAG_MIGRATE_MODERN (1 << VIRTIO_PCI_FLAG_MIGRATE_MODERN_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..c971ba2 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1056,6 +1056,17 @@ static bool virtio_virtqueue_needed(void *opaque)
     return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
 }
 
+static bool virtio_modern_state_needed(void *opaque)
+{
+    VirtIODevice *vdev = opaque;
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    return virtio_virtqueue_needed(opaque) &&
+        k->has_modern_state &&
+        k->has_modern_state(qbus->parent);
+}
+
 static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size)
 {
     VirtIODevice *vdev = pv;
@@ -1104,6 +1115,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = {
     }
 };
 
+static int get_modern_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_modern_state(qbus->parent, f);
+
+    return ret;
+}
+
+static void put_modern_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_modern_state(qbus->parent, f);
+}
+
+static VMStateInfo vmstate_info_modern_state = {
+    .name = "virtqueue_state",
+    .get = get_modern_state,
+    .put = put_modern_state,
+};
+
+static const VMStateDescription vmstate_virtio_modern_state = {
+    .name = "virtio/modern_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = &virtio_modern_state_needed,
+    .fields = (VMStateField[]) {
+        {
+            .name         = "modern_state",
+            .version_id   = 0,
+            .field_exists = NULL,
+            .size         = 0,
+            .info         = &vmstate_info_modern_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 +1195,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..f468880 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-modern",\
+            .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..30e6b07 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_modern_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_modern_state)(DeviceState *d, QEMUFile *f);
+    bool (*has_modern_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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 4/6] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap
  2015-08-21  9:05 [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes Jason Wang
                   ` (2 preceding siblings ...)
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration Jason Wang
@ 2015-08-21  9:05 ` Jason Wang
  2015-08-24 16:30   ` Greg Kurz
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device Jason Wang
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 6/6] virtio-pci: unbreak queue_enable read Jason Wang
  5 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2015-08-21  9:05 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: 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 d785623..fbd1f1f 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -226,8 +226,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,
@@ -235,8 +235,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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device
  2015-08-21  9:05 [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes Jason Wang
                   ` (3 preceding siblings ...)
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 4/6] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap Jason Wang
@ 2015-08-21  9:05 ` Jason Wang
  2015-08-24 14:52   ` Greg Kurz
  2015-08-25 11:48   ` Michael S. Tsirkin
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 6/6] virtio-pci: unbreak queue_enable read Jason Wang
  5 siblings, 2 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-21  9:05 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: 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 with legacy device.
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 fbd1f1f..e399565 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -210,7 +210,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);
@@ -228,6 +230,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,
@@ -237,6 +243,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,
@@ -1344,6 +1354,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)
 {
@@ -1437,6 +1458,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,
@@ -1461,30 +1492,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)
 {
@@ -1492,6 +1553,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);
@@ -1530,16 +1592,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 |
@@ -1592,14 +1669,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);
+        }
     }
 }
 
@@ -1619,6 +1700,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;
@@ -1638,6 +1720,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 *
@@ -1684,6 +1770,8 @@ static Property virtio_pci_properties[] = {
                     VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true),
     DEFINE_PROP_BIT("migrate-modern", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_MIGRATE_MODERN_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 fd2e889..491edfd 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_MODERN_BIT 4
 #define VIRTIO_PCI_FLAG_MIGRATE_MODERN (1 << VIRTIO_PCI_FLAG_MIGRATE_MODERN_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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 6/6] virtio-pci: unbreak queue_enable read
  2015-08-21  9:05 [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes Jason Wang
                   ` (4 preceding siblings ...)
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device Jason Wang
@ 2015-08-21  9:05 ` Jason Wang
  5 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-21  9:05 UTC (permalink / raw)
  To: qemu-devel, mst; +Cc: 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 e399565..6cb51de 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1312,6 +1312,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;
@@ -1756,9 +1757,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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration Jason Wang
@ 2015-08-21  9:43   ` Cornelia Huck
  2015-08-24  5:37     ` Jason Wang
  0 siblings, 1 reply; 21+ messages in thread
From: Cornelia Huck @ 2015-08-21  9:43 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, mst

On Fri, 21 Aug 2015 17:05:47 +0800
Jason Wang <jasowang@redhat.com> wrote:

> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 788b556..c971ba2 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1056,6 +1056,17 @@ static bool virtio_virtqueue_needed(void *opaque)
>      return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>  }
> 
> +static bool virtio_modern_state_needed(void *opaque)
> +{
> +    VirtIODevice *vdev = opaque;
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    return virtio_virtqueue_needed(opaque) &&

Why does core want to check that? Shouldn't that be done by the class
instead (but see below)?

> +        k->has_modern_state &&
> +        k->has_modern_state(qbus->parent);
> +}

I don't really like this "modern_state" stuff (which is pci specific)
creeping into core.

How about introducing "extra_state" and/or "extra_queue_state" (or
something like that) instead?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type Jason Wang
@ 2015-08-21 15:47   ` Eduardo Habkost
  2015-08-24  5:45     ` Jason Wang
  0 siblings, 1 reply; 21+ messages in thread
From: Eduardo Habkost @ 2015-08-21 15:47 UTC (permalink / raw)
  To: Jason Wang; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel, mst

On Fri, Aug 21, 2015 at 05:05:45PM +0800, Jason Wang wrote:
[...]
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a896624..2a7b7d9 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -304,9 +304,15 @@ static void pc_init1(MachineState *machine)
>      }
>  }
>  
> +static void pc_compat_2_4(MachineState *machine)
> +{
> +}
> +
>  static void pc_compat_2_3(MachineState *machine)
>  {
>      PCMachineState *pcms = PC_MACHINE(machine);
> +
> +    pc_compat_2_4(machine);
>      savevm_skip_section_footers();
>      if (kvm_enabled()) {
>          pcms->smm = ON_OFF_AUTO_OFF;
> @@ -477,7 +483,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
>      m->hot_add_cpu = pc_hot_add_cpu;
>  }
>  
> -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->default_machine_opts = "firmware=bios-256k.bin";
> @@ -486,7 +492,18 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
>      m->is_default = 1;
>  }
>  
> -DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
> +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_machine_options(m);
> +    m->default_machine_opts = "firmware=bios-256k.bin";
> +    m->default_display = "std";
> +    SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);

Please don't duplicate pc_i440fx_2_5_machine_options() code here. Call
pc_i440fx_2_5_machine_options() instead.

Otherwise pc-2.4 will never inherit the compat settings from pc-2.8,
pc-2.7, pc-2.6, when we create those machines.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 2/6] ppc: spapr: introduce 2.5 machine type
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 2/6] ppc: spapr: " Jason Wang
@ 2015-08-22  0:10   ` David Gibson
  2015-08-24  5:37     ` Jason Wang
  0 siblings, 1 reply; 21+ messages in thread
From: David Gibson @ 2015-08-22  0:10 UTC (permalink / raw)
  To: Jason Wang; +Cc: Alexander Graf, qemu-ppc, qemu-devel, mst

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

On Fri, Aug 21, 2015 at 05:05:46PM +0800, Jason Wang wrote:
> This will be used by virtio 1.0 virtio-pci virtqueue migration
> backward compatibility.
> 
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Jason Wang <jasowang@redhat.com>

There's already a more or less equivalent patch in the spapr-next
branch.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration
  2015-08-21  9:43   ` Cornelia Huck
@ 2015-08-24  5:37     ` Jason Wang
  2015-08-24 14:14       ` Cornelia Huck
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2015-08-24  5:37 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-devel, mst



On 08/21/2015 05:43 PM, Cornelia Huck wrote:
> On Fri, 21 Aug 2015 17:05:47 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 788b556..c971ba2 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -1056,6 +1056,17 @@ static bool virtio_virtqueue_needed(void *opaque)
>>      return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>>  }
>>
>> +static bool virtio_modern_state_needed(void *opaque)
>> +{
>> +    VirtIODevice *vdev = opaque;
>> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>> +
>> +    return virtio_virtqueue_needed(opaque) &&
> Why does core want to check that? Shouldn't that be done by the class
> instead (but see below)?

Re-think about this, it should be ok to get rid of
virtio_virtioqueue_needed() here.

>> +        k->has_modern_state &&
>> +        k->has_modern_state(qbus->parent);
>> +}
> I don't really like this "modern_state" stuff (which is pci specific)
> creeping into core.
>
> How about introducing "extra_state" and/or "extra_queue_state" (or
> something like that) instead?
>

Ok, if you don't like pci specific name, maybe something like
"virtio_1_state" is better?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 2/6] ppc: spapr: introduce 2.5 machine type
  2015-08-22  0:10   ` David Gibson
@ 2015-08-24  5:37     ` Jason Wang
  0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-24  5:37 UTC (permalink / raw)
  To: David Gibson; +Cc: Alexander Graf, qemu-ppc, qemu-devel, mst



On 08/22/2015 08:10 AM, David Gibson wrote:
> On Fri, Aug 21, 2015 at 05:05:46PM +0800, Jason Wang wrote:
>> This will be used by virtio 1.0 virtio-pci virtqueue migration
>> backward compatibility.
>>
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: qemu-ppc@nongnu.org
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> There's already a more or less equivalent patch in the spapr-next
> branch.
>

Will drop this from the series. Thanks

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type
  2015-08-21 15:47   ` Eduardo Habkost
@ 2015-08-24  5:45     ` Jason Wang
  0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-24  5:45 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, mst, qemu-devel, Richard Henderson



On 08/21/2015 11:47 PM, Eduardo Habkost wrote:
> On Fri, Aug 21, 2015 at 05:05:45PM +0800, Jason Wang wrote:
> [...]
>> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> > index a896624..2a7b7d9 100644
>> > --- a/hw/i386/pc_piix.c
>> > +++ b/hw/i386/pc_piix.c
>> > @@ -304,9 +304,15 @@ static void pc_init1(MachineState *machine)
>> >      }
>> >  }
>> >  
>> > +static void pc_compat_2_4(MachineState *machine)
>> > +{
>> > +}
>> > +
>> >  static void pc_compat_2_3(MachineState *machine)
>> >  {
>> >      PCMachineState *pcms = PC_MACHINE(machine);
>> > +
>> > +    pc_compat_2_4(machine);
>> >      savevm_skip_section_footers();
>> >      if (kvm_enabled()) {
>> >          pcms->smm = ON_OFF_AUTO_OFF;
>> > @@ -477,7 +483,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
>> >      m->hot_add_cpu = pc_hot_add_cpu;
>> >  }
>> >  
>> > -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->default_machine_opts = "firmware=bios-256k.bin";
>> > @@ -486,7 +492,18 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
>> >      m->is_default = 1;
>> >  }
>> >  
>> > -DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
>> > +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_machine_options(m);
>> > +    m->default_machine_opts = "firmware=bios-256k.bin";
>> > +    m->default_display = "std";
>> > +    SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
> Please don't duplicate pc_i440fx_2_5_machine_options() code here. Call
> pc_i440fx_2_5_machine_options() instead.
>
> Otherwise pc-2.4 will never inherit the compat settings from pc-2.8,
> pc-2.7, pc-2.6, when we create those machines.

Right, will fix this in V2.

Thanks

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration
  2015-08-24  5:37     ` Jason Wang
@ 2015-08-24 14:14       ` Cornelia Huck
  2015-08-25  3:14         ` Jason Wang
  0 siblings, 1 reply; 21+ messages in thread
From: Cornelia Huck @ 2015-08-24 14:14 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, mst

On Mon, 24 Aug 2015 13:37:06 +0800
Jason Wang <jasowang@redhat.com> wrote:

> On 08/21/2015 05:43 PM, Cornelia Huck wrote:
> > On Fri, 21 Aug 2015 17:05:47 +0800
> > Jason Wang <jasowang@redhat.com> wrote:

> >> +        k->has_modern_state &&
> >> +        k->has_modern_state(qbus->parent);
> >> +}
> > I don't really like this "modern_state" stuff (which is pci specific)
> > creeping into core.
> >
> > How about introducing "extra_state" and/or "extra_queue_state" (or
> > something like that) instead?
> >
> 
> Ok, if you don't like pci specific name, maybe something like
> "virtio_1_state" is better?

I was thinking more along the lines of "transport wants to save/restore
additional state for the device" - which is not neccessarily depending
on virtio-1. It would be good if a transport can extend the state
without needlessly introducing incompatibilities.

pci can handle its modern state via this then and encapsulate it.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device Jason Wang
@ 2015-08-24 14:52   ` Greg Kurz
  2015-08-24 16:29     ` Greg Kurz
  2015-08-25 11:48   ` Michael S. Tsirkin
  1 sibling, 1 reply; 21+ messages in thread
From: Greg Kurz @ 2015-08-24 14:52 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, mst

On Fri, 21 Aug 2015 17:05:49 +0800
Jason Wang <jasowang@redhat.com> wrote:

> 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 with legacy device.
> 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 fbd1f1f..e399565 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -210,7 +210,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);
> @@ -228,6 +230,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);
> +            }


This calls for the following change in memory.c:

 static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
 {
-    if (memory_region_wrong_endianness(mr)) {
+    if (size && memory_region_wrong_endianness(mr)) {


otherwise we abort on PPC64.

>          }
>          if (legacy) {
>              memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
> @@ -237,6 +243,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,
> @@ -1344,6 +1354,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)
>  {
> @@ -1437,6 +1458,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,
> @@ -1461,30 +1492,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)
>  {
> @@ -1492,6 +1553,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);
> @@ -1530,16 +1592,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 |
> @@ -1592,14 +1669,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);
> +        }
>      }
>  }
> 
> @@ -1619,6 +1700,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;
> @@ -1638,6 +1720,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 *
> @@ -1684,6 +1770,8 @@ static Property virtio_pci_properties[] = {
>                      VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true),
>      DEFINE_PROP_BIT("migrate-modern", VirtIOPCIProxy, flags,
>                      VIRTIO_PCI_FLAG_MIGRATE_MODERN_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 fd2e889..491edfd 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_MODERN_BIT 4
>  #define VIRTIO_PCI_FLAG_MIGRATE_MODERN (1 << VIRTIO_PCI_FLAG_MIGRATE_MODERN_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;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device
  2015-08-24 14:52   ` Greg Kurz
@ 2015-08-24 16:29     ` Greg Kurz
  0 siblings, 0 replies; 21+ messages in thread
From: Greg Kurz @ 2015-08-24 16:29 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, mst

On Mon, 24 Aug 2015 16:52:45 +0200
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:

> On Fri, 21 Aug 2015 17:05:49 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
> > 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 with legacy device.
> > 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 fbd1f1f..e399565 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -210,7 +210,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);
> > @@ -228,6 +230,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);
> > +            }
> 
> 
> This calls for the following change in memory.c:
> 
>  static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
>  {
> -    if (memory_region_wrong_endianness(mr)) {
> +    if (size && memory_region_wrong_endianness(mr)) {
> 

Oops... this comment was for patch 4/6... Please ignore.

> 
> otherwise we abort on PPC64.
> 
> >          }
> >          if (legacy) {
> >              memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
> > @@ -237,6 +243,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,
> > @@ -1344,6 +1354,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)
> >  {
> > @@ -1437,6 +1458,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,
> > @@ -1461,30 +1492,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)
> >  {
> > @@ -1492,6 +1553,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);
> > @@ -1530,16 +1592,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 |
> > @@ -1592,14 +1669,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);
> > +        }
> >      }
> >  }
> > 
> > @@ -1619,6 +1700,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;
> > @@ -1638,6 +1720,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 *
> > @@ -1684,6 +1770,8 @@ static Property virtio_pci_properties[] = {
> >                      VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true),
> >      DEFINE_PROP_BIT("migrate-modern", VirtIOPCIProxy, flags,
> >                      VIRTIO_PCI_FLAG_MIGRATE_MODERN_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 fd2e889..491edfd 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_MODERN_BIT 4
> >  #define VIRTIO_PCI_FLAG_MIGRATE_MODERN (1 << VIRTIO_PCI_FLAG_MIGRATE_MODERN_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;
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 4/6] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap Jason Wang
@ 2015-08-24 16:30   ` Greg Kurz
  2015-08-25  3:21     ` Jason Wang
  0 siblings, 1 reply; 21+ messages in thread
From: Greg Kurz @ 2015-08-24 16:30 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, mst

On Fri, 21 Aug 2015 17:05:48 +0800
Jason Wang <jasowang@redhat.com> 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>
> ---
>  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 d785623..fbd1f1f 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -226,8 +226,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);

This calls for the following change in memory.c:

 static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
 {
-    if (memory_region_wrong_endianness(mr)) {
+    if (size && memory_region_wrong_endianness(mr)) {


otherwise we abort on PPC64.

>          }
>          if (legacy) {
>              memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
> @@ -235,8 +235,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,

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration
  2015-08-24 14:14       ` Cornelia Huck
@ 2015-08-25  3:14         ` Jason Wang
  0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-25  3:14 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-devel, mst



On 08/24/2015 10:14 PM, Cornelia Huck wrote:
> On Mon, 24 Aug 2015 13:37:06 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> On 08/21/2015 05:43 PM, Cornelia Huck wrote:
>>> On Fri, 21 Aug 2015 17:05:47 +0800
>>> Jason Wang <jasowang@redhat.com> wrote:
>>>> +        k->has_modern_state &&
>>>> +        k->has_modern_state(qbus->parent);
>>>> +}
>>> I don't really like this "modern_state" stuff (which is pci specific)
>>> creeping into core.
>>>
>>> How about introducing "extra_state" and/or "extra_queue_state" (or
>>> something like that) instead?
>>>
>> Ok, if you don't like pci specific name, maybe something like
>> "virtio_1_state" is better?
> I was thinking more along the lines of "transport wants to save/restore
> additional state for the device" - which is not neccessarily depending
> on virtio-1. It would be good if a transport can extend the state
> without needlessly introducing incompatibilities.

I see.

>
> pci can handle its modern state via this then and encapsulate it.
>

Ok. Will have a try.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap
  2015-08-24 16:30   ` Greg Kurz
@ 2015-08-25  3:21     ` Jason Wang
  0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-25  3:21 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-devel, mst



On 08/25/2015 12:30 AM, Greg Kurz wrote:
> On Fri, 21 Aug 2015 17:05:48 +0800
> Jason Wang <jasowang@redhat.com> 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>
>> > ---
>> >  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 d785623..fbd1f1f 100644
>> > --- a/hw/virtio/virtio-pci.c
>> > +++ b/hw/virtio/virtio-pci.c
>> > @@ -226,8 +226,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);
> This calls for the following change in memory.c:
>
>  static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
>  {
> -    if (memory_region_wrong_endianness(mr)) {
> +    if (size && memory_region_wrong_endianness(mr)) {
>
>
> otherwise we abort on PPC64.
>

Right, will fix this in V2.

Thanks

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device
  2015-08-21  9:05 ` [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device Jason Wang
  2015-08-24 14:52   ` Greg Kurz
@ 2015-08-25 11:48   ` Michael S. Tsirkin
  2015-08-26  5:29     ` Jason Wang
  1 sibling, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-08-25 11:48 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel

On Fri, Aug 21, 2015 at 05:05:49PM +0800, Jason Wang wrote:
> 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 with legacy device.
> Thanks Wenli Quan <wquan@redhat.com> for the benchmarking.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I don't really care much about non-EPT hosts, but if you propose
a patch to optimize them, it should be accompanied by numbers
showing the performance difference.

> ---
>  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 fbd1f1f..e399565 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -210,7 +210,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);
> @@ -228,6 +230,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,
> @@ -237,6 +243,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,
> @@ -1344,6 +1354,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)
>  {
> @@ -1437,6 +1458,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,
> @@ -1461,30 +1492,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)
>  {
> @@ -1492,6 +1553,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);
> @@ -1530,16 +1592,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 |
> @@ -1592,14 +1669,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);
> +        }
>      }
>  }
>  
> @@ -1619,6 +1700,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;
> @@ -1638,6 +1720,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 *
> @@ -1684,6 +1770,8 @@ static Property virtio_pci_properties[] = {
>                      VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true),
>      DEFINE_PROP_BIT("migrate-modern", VirtIOPCIProxy, flags,
>                      VIRTIO_PCI_FLAG_MIGRATE_MODERN_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 fd2e889..491edfd 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_MODERN_BIT 4
>  #define VIRTIO_PCI_FLAG_MIGRATE_MODERN (1 << VIRTIO_PCI_FLAG_MIGRATE_MODERN_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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device
  2015-08-25 11:48   ` Michael S. Tsirkin
@ 2015-08-26  5:29     ` Jason Wang
  0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2015-08-26  5:29 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel



On 08/25/2015 07:48 PM, Michael S. Tsirkin wrote:
> On Fri, Aug 21, 2015 at 05:05:49PM +0800, Jason Wang wrote:
>> > 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 with legacy device.
>> > Thanks Wenli Quan <wquan@redhat.com> for the benchmarking.
>> > 
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> I don't really care much about non-EPT hosts, but if you propose
> a patch to optimize them, it should be accompanied by numbers
> showing the performance difference.
>

According to the test, PIO is a little bit faster than fast mmio in some
specific TCP_RR case:

modern device fast mmio vs modern device pio:

TCP_RR:

size/session/+transaction rate%/+cpu%/-+per cpu%/
64/1/[+1.5646%]/+5.6604%/-4.3415%/  
64/25/+0.3003%/-0.4517%/+0.7486%/
...
256/1/[+1.0046%]/[-6.5238%]/[+7.0673%]/

So the improvement is almost as much as previous patch.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2015-08-26  5:29 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-21  9:05 [Qemu-devel] [PATCH 0/6] virtio pci 1.0 optimizations and fixes Jason Wang
2015-08-21  9:05 ` [Qemu-devel] [PATCH 1/6] pc: introduce 2.5 machine type Jason Wang
2015-08-21 15:47   ` Eduardo Habkost
2015-08-24  5:45     ` Jason Wang
2015-08-21  9:05 ` [Qemu-devel] [PATCH 2/6] ppc: spapr: " Jason Wang
2015-08-22  0:10   ` David Gibson
2015-08-24  5:37     ` Jason Wang
2015-08-21  9:05 ` [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration Jason Wang
2015-08-21  9:43   ` Cornelia Huck
2015-08-24  5:37     ` Jason Wang
2015-08-24 14:14       ` Cornelia Huck
2015-08-25  3:14         ` Jason Wang
2015-08-21  9:05 ` [Qemu-devel] [PATCH 4/6] virtio-pci: use wildcard mmio eventfd for 1.0 notification cap Jason Wang
2015-08-24 16:30   ` Greg Kurz
2015-08-25  3:21     ` Jason Wang
2015-08-21  9:05 ` [Qemu-devel] [PATCH 5/6] virtio-pci: introduce pio notification capability for modern device Jason Wang
2015-08-24 14:52   ` Greg Kurz
2015-08-24 16:29     ` Greg Kurz
2015-08-25 11:48   ` Michael S. Tsirkin
2015-08-26  5:29     ` Jason Wang
2015-08-21  9:05 ` [Qemu-devel] [PATCH 6/6] virtio-pci: unbreak queue_enable read Jason Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).