[Qemu-devel] [PATCH 0/5] target/i386: introduce coalesced pio

[Qemu-devel] [PATCH 0/5] target/i386: introduce coalesced pio

[Peng Hao]

Coalesced pio is base on coalesced mmio and can be used for some port
like rtc port, pci-host config port and so on.

Specially in case of rtc as coalesced pio, some versions of windows guest
access rtc frequently because of rtc as system tick. guest access rtc like
this: write register index to 0x70, then write or read data from 0x71.
writing 0x70 port is just as index and do nothing else. So we can use
coalesced pio to handle this scene to reduce VM-EXIT time.

When it starts and closes the virtual machine, it will access pci-host config
port or virtio-pci config port frequently. So setting these port as
coalesced pio can reduce startup and shutdown time. In qemu I just realize
piixfx's pci-host, it is convenient for other pci-host type implementations.

without my patch, get the vm-exit time of accessing rtc 0x70 and piix 0xcf8
using perf tools: (guest OS : windows 7 64bit)
IO Port Access  Samples Samples%  Time%  Min Time  Max Time  Avg time
0x70:POUT        86     30.99%    74.59%   9us      29us    10.75us (+- 3.41%)
0xcf8:POUT     1119     2.60%     2.12%   2.79us    56.83us 3.41us (+- 2.23%)

with my patch
IO Port Access  Samples Samples%  Time%   Min Time  Max Time   Avg time
0x70:POUT       106    32.02%    29.47%    0us      10us     1.57us (+- 7.38%)
0xcf8:POUT      1065    1.67%     0.28%   0.41us    65.44us   0.66us (+- 10.55%)

These are just qemu's patches, another patches are for kernel.

Changes v4 --> v5:
    update kvm header, improve compatibility.

Changes v3 --> v4
    modify coalesced_mmio_{add|del} to coalesced_io_{add|del}
    delete unnecessary macro define

Peng Hao (5):
  target-i386: add rtc 0x70 port as coalesced_pio
  target-i386: add i440fx 0xcf8 port as coalesced_pio
  target-i386: add q35 0xcf8 port as coalesced_pio
  target/i386: add coalesced pio support
  target-i386: add coalesced_pio API

 accel/kvm/kvm-all.c       | 60 +++++++++++++++++++++++++++++++++++++++++++----
 hw/pci-host/piix.c        |  4 ++++
 hw/pci-host/q35.c         |  4 ++++
 hw/timer/mc146818rtc.c    |  8 +++++++
 include/exec/memory.h     |  4 ++--
 linux-headers/linux/kvm.h |  9 ++++++-
 memory.c                  |  4 ++--
 7 files changed, 83 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH V6 1/5] target/i386 : add coalesced pio support

[Peng Hao]

add coalesced_pio's struct and KVM_CAP_COALESCED_PIO header.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 accel/kvm/kvm-all.c       | 4 ++--
 linux-headers/linux/kvm.h | 9 ++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index de12f78..29d208d 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -537,7 +537,7 @@ static void kvm_coalesce_mmio_region(MemoryListener *listener,
 
         zone.addr = start;
         zone.size = size;
-        zone.pad = 0;
+        zone.pio = 0;
 
         (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
     }
@@ -554,7 +554,7 @@ static void kvm_uncoalesce_mmio_region(MemoryListener *listener,
 
         zone.addr = start;
         zone.size = size;
-        zone.pad = 0;
+        zone.pio = 0;
 
         (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
     }
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 83ba4eb..b5d4289 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -420,13 +420,19 @@ struct kvm_run {
 struct kvm_coalesced_mmio_zone {
 	__u64 addr;
 	__u32 size;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 pio;
+	};
 };
 
 struct kvm_coalesced_mmio {
 	__u64 phys_addr;
 	__u32 len;
+	unino {
 	__u32 pad;
+	__u32 pio;
+	};
 	__u8  data[8];
 };
 
@@ -953,6 +959,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_NESTED_STATE 157
 #define KVM_CAP_ARM_INJECT_SERROR_ESR 158
 #define KVM_CAP_MSR_PLATFORM_INFO 159
+#define KVM_CAP_COALESCED_PIO 160
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V6 1/5] target/i386 : add coalesced pio support

[Vitaly Kuznetsov]

Peng Hao <peng.hao2@zte.com.cn> writes:

> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 83ba4eb..b5d4289 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -420,13 +420,19 @@ struct kvm_run {
>  struct kvm_coalesced_mmio_zone {
>  	__u64 addr;
>  	__u32 size;
> -	__u32 pad;
> +	union {
> +		__u32 pad;
> +		__u32 pio;
> +	};
>  };

I was just passing by, but what's the point in keeping 'pad'?

>  
>  struct kvm_coalesced_mmio {
>  	__u64 phys_addr;
>  	__u32 len;
> +	unino {

'union' I would guess? How does it compile?

>  	__u32 pad;
> +	__u32 pio;
> +	};
>  	__u8  data[8];
>  };
>  
> @@ -953,6 +959,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_NESTED_STATE 157
>  #define KVM_CAP_ARM_INJECT_SERROR_ESR 158
>  #define KVM_CAP_MSR_PLATFORM_INFO 159
> +#define KVM_CAP_COALESCED_PIO 160
>  
>  #ifdef KVM_CAP_IRQ_ROUTING

-- 
Vitaly

Re: [Qemu-devel] [PATCH V6 1/5] target/i386 : add coalesced pio support

[Paolo Bonzini]

On 17/10/2018 17:04, Vitaly Kuznetsov wrote:
>> +	union {
>> +		__u32 pad;
>> +		__u32 pio;
>> +	};
>>  };
> I was just passing by, but what's the point in keeping 'pad'?
> 

Backwards compatibility.  QEMU does have a couple instances of "x.pad =
0", so...

Paolo

[Qemu-devel] [PATCH V6 2/5] target-i386 : add coalesced_pio API

[Peng Hao]

the primary API realization.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
 accel/kvm/kvm-all.c   | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---
 include/exec/memory.h |  4 ++--
 memory.c              |  4 ++--
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 29d208d..7724055 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -79,6 +79,7 @@ struct KVMState
     int fd;
     int vmfd;
     int coalesced_mmio;
+    int coalesced_pio;
     struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
     bool coalesced_flush_in_progress;
     int vcpu_events;
@@ -560,6 +561,45 @@ static void kvm_uncoalesce_mmio_region(MemoryListener *listener,
     }
 }
 
+static void kvm_coalesce_pio_add(MemoryListener *listener,
+                                MemoryRegionSection *section,
+                                hwaddr start, hwaddr size)
+{
+    KVMState *s = kvm_state;
+
+    if (s->coalesced_pio) {
+        struct kvm_coalesced_mmio_zone zone;
+
+        zone.addr = start;
+        zone.size = size;
+        zone.pio = 1;
+
+        (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
+    }
+}
+
+static void kvm_coalesce_pio_del(MemoryListener *listener,
+                                MemoryRegionSection *section,
+                                hwaddr start, hwaddr size)
+{
+    KVMState *s = kvm_state;
+
+    if (s->coalesced_pio) {
+        struct kvm_coalesced_mmio_zone zone;
+
+        zone.addr = start;
+        zone.size = size;
+        zone.pio = 1;
+
+        (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
+     }
+}
+
+static MemoryListener kvm_coalesced_pio_listener = {
+    .coalesced_io_add = kvm_coalesce_pio_add,
+    .coalesced_io_del = kvm_coalesce_pio_del,
+};
+
 int kvm_check_extension(KVMState *s, unsigned int extension)
 {
     int ret;
@@ -1616,6 +1656,8 @@ static int kvm_init(MachineState *ms)
     }
 
     s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
+    s->coalesced_pio = s->coalesced_mmio &&
+                       kvm_check_extension(s, KVM_CAP_COALESCED_PIO);
 
 #ifdef KVM_CAP_VCPU_EVENTS
     s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
@@ -1686,13 +1728,15 @@ static int kvm_init(MachineState *ms)
         s->memory_listener.listener.eventfd_add = kvm_mem_ioeventfd_add;
         s->memory_listener.listener.eventfd_del = kvm_mem_ioeventfd_del;
     }
-    s->memory_listener.listener.coalesced_mmio_add = kvm_coalesce_mmio_region;
-    s->memory_listener.listener.coalesced_mmio_del = kvm_uncoalesce_mmio_region;
+    s->memory_listener.listener.coalesced_io_add = kvm_coalesce_mmio_region;
+    s->memory_listener.listener.coalesced_io_del = kvm_uncoalesce_mmio_region;
 
     kvm_memory_listener_register(s, &s->memory_listener,
                                  &address_space_memory, 0);
     memory_listener_register(&kvm_io_listener,
                              &address_space_io);
+    memory_listener_register(&kvm_coalesced_pio_listener,
+                             &address_space_io);
 
     s->many_ioeventfds = kvm_check_many_ioeventfds();
 
@@ -1778,7 +1822,13 @@ void kvm_flush_coalesced_mmio_buffer(void)
 
             ent = &ring->coalesced_mmio[ring->first];
 
-            cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
+            if (ent->pio == 1) {
+                address_space_rw(&address_space_io, ent->phys_addr,
+                                 MEMTXATTRS_UNSPECIFIED, ent->data,
+                                 ent->len, true);
+            } else {
+                cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
+            }
             smp_wmb();
             ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
         }
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 3a427aa..667466b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -419,9 +419,9 @@ struct MemoryListener {
                         bool match_data, uint64_t data, EventNotifier *e);
     void (*eventfd_del)(MemoryListener *listener, MemoryRegionSection *section,
                         bool match_data, uint64_t data, EventNotifier *e);
-    void (*coalesced_mmio_add)(MemoryListener *listener, MemoryRegionSection *section,
+    void (*coalesced_io_add)(MemoryListener *listener, MemoryRegionSection *section,
                                hwaddr addr, hwaddr len);
-    void (*coalesced_mmio_del)(MemoryListener *listener, MemoryRegionSection *section,
+    void (*coalesced_io_del)(MemoryListener *listener, MemoryRegionSection *section,
                                hwaddr addr, hwaddr len);
     /* Lower = earlier (during add), later (during del) */
     unsigned priority;
diff --git a/memory.c b/memory.c
index d852f11..51204aa 100644
--- a/memory.c
+++ b/memory.c
@@ -2129,7 +2129,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
                 .size = fr->addr.size,
             };
 
-            MEMORY_LISTENER_CALL(as, coalesced_mmio_del, Reverse, &section,
+            MEMORY_LISTENER_CALL(as, coalesced_io_del, Reverse, &section,
                                  int128_get64(fr->addr.start),
                                  int128_get64(fr->addr.size));
             QTAILQ_FOREACH(cmr, &mr->coalesced, link) {
@@ -2140,7 +2140,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
                     continue;
                 }
                 tmp = addrrange_intersection(tmp, fr->addr);
-                MEMORY_LISTENER_CALL(as, coalesced_mmio_add, Forward, &section,
+                MEMORY_LISTENER_CALL(as, coalesced_io_add, Forward, &section,
                                      int128_get64(tmp.start),
                                      int128_get64(tmp.size));
             }
-- 
1.8.3.1

[Qemu-devel] [PATCH V6 3/5] target-i386: add rtc 0x70 port as coalesced_pio

[Peng Hao]

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 hw/timer/mc146818rtc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index acee47d..808a212 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -70,6 +70,7 @@ typedef struct RTCState {
     ISADevice parent_obj;
 
     MemoryRegion io;
+    MemoryRegion coalesced_io;
     uint8_t cmos_data[128];
     uint8_t cmos_index;
     int32_t base_year;
@@ -990,6 +991,13 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2);
     isa_register_ioport(isadev, &s->io, base);
 
+    /* register rtc 0x70 port as coalesced_pio */
+    memory_region_set_flush_coalesced(&s->io);
+    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
+                          s, "rtc1", 1);
+    isa_register_ioport(isadev, &s->coalesced_io, base);
+    memory_region_add_coalescing(&s->coalesced_io, 0, 1);
+
     qdev_set_legacy_instance_id(dev, base, 3);
     qemu_register_reset(rtc_reset, s);
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V6 3/5] target-i386: add rtc 0x70 port as coalesced_pio

[Paolo Bonzini]

On 17/10/2018 18:52, Peng Hao wrote:
> +    /* register rtc 0x70 port as coalesced_pio */
> +    memory_region_set_flush_coalesced(&s->io);
> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
> +                          s, "rtc1", 1);
> +    isa_register_ioport(isadev, &s->coalesced_io, base);

I think instead of isa_register_ioport you can use
memory_region_add_subregion, so that s->coalesced_io is added below s->io?

This way, you don't rely on the behavior of overlapping regions.

Paolo

> +    memory_region_add_coalescing(&s->coalesced_io, 0, 1);
> +

Re: [Qemu-devel] [PATCH V6 3/5] target-i386: add rtc 0x70 port as coalesced_pio

[peng.hao2]

>On 17/10/2018 18:52, Peng Hao wrote:
>> +    /* register rtc 0x70 port as coalesced_pio */
>> +    memory_region_set_flush_coalesced(&s->io);
>> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
>> +                          s, "rtc1", 1);
>> +    isa_register_ioport(isadev, &s->coalesced_io, base);
>
>I think instead of isa_register_ioport you can use
>memory_region_add_subregion, so that s->coalesced_io is added below s->io?
>
isa_register_ioport also called memory_region_add_subregion.
modify code  like this:
  //isa_register_ioport(isadev, &s->coalesced_io, base);
    memory_region_add_subregion(get_system_io(),0x70,&s->coalesced_io);
The regional distribution before and after code modification is consistent.

   0000000000000070-0000000000000070 (prio 0, i/o): rtc1
    0000000000000070-0000000000000071 (prio 0, i/o): rtc
>This way, you don't rely on the behavior of overlapping regions.
>
>Paolo
>
>> +    memory_region_add_coalescing(&s->coalesced_io, 0, 1);
>> +

Re: [Qemu-devel] [PATCH V6 3/5] target-i386: add rtc 0x70 port as coalesced_pio

[Paolo Bonzini]

On 18/10/2018 10:03, peng.hao2@zte.com.cn wrote:
>>> +    /* register rtc 0x70 port as coalesced_pio */
>>> +    memory_region_set_flush_coalesced(&s->io);
>>> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
>>> +                          s, "rtc1", 1);
>>> +    isa_register_ioport(isadev, &s->coalesced_io, base);
>> I think instead of isa_register_ioport you can use
>> memory_region_add_subregion, so that s->coalesced_io is added below s->io?
>>
> isa_register_ioport also called memory_region_add_subregion.
> modify code  like this:
>   //isa_register_ioport(isadev, &s->coalesced_io, base);
>     memory_region_add_subregion(get_system_io(),0x70,&s->coalesced_io);
> The regional distribution before and after code modification is consistent.

Right, but I'd rather add s->coalesced_io subregion as a subregion of
s->io at offset 0.

Paolo

>    0000000000000070-0000000000000070 (prio 0, i/o): rtc1
>     0000000000000070-0000000000000071 (prio 0, i/o): rtc

Re: [Qemu-devel] [PATCH V6 3/5] target-i386: add rtc 0x70 port as coalesced_pio

[peng.hao2]

>On 18/10/2018 10:03, peng.hao2@zte.com.cn wrote:
>>>> +    /* register rtc 0x70 port as coalesced_pio */
>>>> +    memory_region_set_flush_coalesced(&s->io);
>>>> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
>>>> +                          s, "rtc1", 1);
>>>> +    isa_register_ioport(isadev, &s->coalesced_io, base);
>>> I think instead of isa_register_ioport you can use
>>> memory_region_add_subregion, so that s->coalesced_io is added below s->io?
>>>
>> isa_register_ioport also called memory_region_add_subregion.
>> modify code  like this:
>>   //isa_register_ioport(isadev, &s->coalesced_io, base);
>>     memory_region_add_subregion(get_system_io(),0x70,&s->coalesced_io);
>> The regional distribution before and after code modification is consistent.
>
>Right, but I'd rather add s->coalesced_io subregion as a subregion of
>s->io at offset 0.
Ok， I will modify it and resubmit the patch.
By the way , I find that  when I adjusted the format of the patch "PATCH V6 1/5", I made a mistake to modify a code character. I also resubmit the patch.
Thanks.
>
>Paolo
>
>>    0000000000000070-0000000000000070 (prio 0, i/o): rtc1
>>     0000000000000070-0000000000000071 (prio 0, i/o): rtc

[Qemu-devel] [PATCH V6 4/5] target-i386: add i440fx 0xcf8 port as coalesced_pio

[Peng Hao]

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 hw/pci-host/piix.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0e60834..da73743 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -327,6 +327,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
 
     sysbus_add_io(sbd, 0xcfc, &s->data_mem);
     sysbus_init_ioports(sbd, 0xcfc, 4);
+
+    /* register i440fx 0xcf8 port as coalesced pio */
+    memory_region_set_flush_coalesced(&s->data_mem);
+    memory_region_add_coalescing(&s->conf_mem, 0, 4);
 }
 
 static void i440fx_realize(PCIDevice *dev, Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PATCH V6 5/5] target-i386: add q35 0xcf8 port as coalesced_pio

[Peng Hao]

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 hw/pci-host/q35.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 02f9576..8ce1e09 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -51,6 +51,10 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
+    /* register q35 0xcf8 port as coalesced pio */
+    memory_region_set_flush_coalesced(&pci->data_mem);
+    memory_region_add_coalescing(&pci->conf_mem, 0, 4);
+
     pci->bus = pci_root_bus_new(DEVICE(s), "pcie.0",
                                 s->mch.pci_address_space,
                                 s->mch.address_space_io,
-- 
1.8.3.1