* [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* 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 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
* [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* 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 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
* [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* 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 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 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 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
* [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* 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 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
* [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),
+ ¬ify_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,
- ®ion->mr);
+ memory_region_add_subregion(mr, region->offset, ®ion->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,
®ion->mr);
}
+static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy,
+ VirtIOPCIRegion *region)
+{
+ memory_region_del_subregion(&proxy->io_bar,
+ ®ion->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, ¬ify.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, ¬ify.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,
+ ¬ify_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* 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),
> + ¬ify_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,
> - ®ion->mr);
> + memory_region_add_subregion(mr, region->offset, ®ion->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,
> ®ion->mr);
> }
>
> +static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy,
> + VirtIOPCIRegion *region)
> +{
> + memory_region_del_subregion(&proxy->io_bar,
> + ®ion->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, ¬ify.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, ¬ify.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,
> + ¬ify_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),
> > + ¬ify_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,
> > - ®ion->mr);
> > + memory_region_add_subregion(mr, region->offset, ®ion->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,
> > ®ion->mr);
> > }
> >
> > +static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy,
> > + VirtIOPCIRegion *region)
> > +{
> > + memory_region_del_subregion(&proxy->io_bar,
> > + ®ion->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, ¬ify.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, ¬ify.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,
> > + ¬ify_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-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),
> + ¬ify_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,
> - ®ion->mr);
> + memory_region_add_subregion(mr, region->offset, ®ion->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,
> ®ion->mr);
> }
>
> +static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy,
> + VirtIOPCIRegion *region)
> +{
> + memory_region_del_subregion(&proxy->io_bar,
> + ®ion->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, ¬ify.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, ¬ify.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,
> + ¬ify_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
* [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