* Re: [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API
@ 2018-08-24 9:18 peng.hao2
2018-08-24 10:42 ` Eduardo Habkost
0 siblings, 1 reply; 4+ messages in thread
From: peng.hao2 @ 2018-08-24 9:18 UTC (permalink / raw)
To: ehabkost; +Cc: pbonzini, mst, rkrcmar, kvm, qemu-devel, zhong.weidong
> On Fri, Aug 24, 2018 at 12:14:47AM +0800, Peng Hao wrote:
> > Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> > ---
> > accel/kvm/kvm-all.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-----
> > include/exec/memattrs.h | 2 +-
> > 2 files changed, 53 insertions(+), 7 deletions(-)
> >
>> @@ -536,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);
> > }
> > @@ -553,12 +554,51 @@ 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);
> > }
> > }
>
> The 2 hunks above need to go to patch 1/4 to avoid breaking the
> build after applying 1/4.
>
> However, patch 1/4 is just a header update and I don't think
> running update-linux-headers.sh was supposed to break the build.
>
> Radim asked about this at:
> Date: Wed, 18 Jul 2018 16:43:11 +0200
> Subject: Re: [PATCH v2] KVM: Add coalesced PIO support
> Message-ID: <20180718144311.GB6348@flask>
> Paolo, do you have a suggestion on how to update the struct
> without making a header update break the build?
>
> > +static MemoryListener kvm_coalesced_io_listener = {
> > + .coalesced_mmio_add = kvm_coalesce_io_add,
> > + .coalesced_mmio_del = kvm_coalesce_io_del,
> Nit: I would rename the fields to "coalesced_mmio_{add,del}" to
> coalesced_io_*, because they are not specific to MMIO anymore.
> I would also name the functions kvm_coalesce_pio_{add,del}
> because the are specific to pio.
I will modify in next patch.
> > +};
> > +
> > int kvm_check_extension(KVMState *s, unsigned int extension)
> > {
> > int ret;
> > @@ -1615,7 +1655,8 @@ static int kvm_init(MachineState *ms)
> >
> > 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_NONE, ent->data, ent->len, true);
> Why exactly MEMTXATTRS_NONE is the right attrs argument here?
> Why MEMTXATTRS_UNSPECIFIED wouldn't work?
I didn't notice MEMTXATTRS_NONE is the same as MEMTXATTRS_UNSPECIFIED.
MEMTXATTRS_NONE is redundant.
>> + } 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/memattrs.h b/include/exec/memattrs.h
>> index d4a1642..12fd64f 100644
>> --- a/include/exec/memattrs.h
>> +++ b/include/exec/memattrs.h
>> @@ -45,7 +45,7 @@ typedef struct MemTxAttrs {
>> * from "didn't specify" if necessary).
>> */
>> #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
>> -
>> +#define MEMTXATTRS_NONE ((MemTxAttrs) { 0 })
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API
2018-08-24 9:18 [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API peng.hao2
@ 2018-08-24 10:42 ` Eduardo Habkost
0 siblings, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2018-08-24 10:42 UTC (permalink / raw)
To: peng.hao2; +Cc: pbonzini, mst, rkrcmar, kvm, qemu-devel, zhong.weidong
On Fri, Aug 24, 2018 at 05:18:45PM +0800, peng.hao2@zte.com.cn wrote:
[...]
> > > - 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_NONE, ent->data, ent->len, true);
>
> > Why exactly MEMTXATTRS_NONE is the right attrs argument here?
> > Why MEMTXATTRS_UNSPECIFIED wouldn't work?
>
> I didn't notice MEMTXATTRS_NONE is the same as MEMTXATTRS_UNSPECIFIED.
> MEMTXATTRS_NONE is redundant.
They are not exactly the same (see below), but in either case I'm
not sure it would make any difference for PIO.
> >> + } 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/memattrs.h b/include/exec/memattrs.h
> >> index d4a1642..12fd64f 100644
> >> --- a/include/exec/memattrs.h
> >> +++ b/include/exec/memattrs.h
> >> @@ -45,7 +45,7 @@ typedef struct MemTxAttrs {
> >> * from "didn't specify" if necessary).
> >> */
> >> #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
> >> -
> >> +#define MEMTXATTRS_NONE ((MemTxAttrs) { 0 })
--
Eduardo
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH V3 0/4] introduce coalesced pio support
@ 2018-08-23 16:14 Peng Hao
2018-08-23 16:14 ` [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API Peng Hao
0 siblings, 1 reply; 4+ messages in thread
From: Peng Hao @ 2018-08-23 16:14 UTC (permalink / raw)
To: pbonzini, mst, ehabkost, rkrcmar; +Cc: kvm, qemu-devel, zhong.weidong, Peng Hao
Coalesced pio is base on coalesced mmio and can be used for some port
like rtc port, pci-host config port, virtio-pci 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.
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.
I think there is a need for further consideration in changing coalesecd_mmio's
naming in kernel and qemu together.So maybe another patch does it.
Peng Hao (4):
target-i386: introduce coalesced_pio kvm header update
target-i386: add coalesced_pio API
target-i386: add rtc 0x70 port as coalesced_pio
target-i386: add i440fx 0xcf8 port as coalesced_pio
accel/kvm/kvm-all.c | 58 ++++++++++++++++++++++++++++++++++++++++++-----
hw/pci-host/piix.c | 4 ++++
hw/timer/mc146818rtc.c | 8 +++++++
include/exec/memattrs.h | 2 +-
linux-headers/linux/kvm.h | 5 ++--
5 files changed, 68 insertions(+), 9 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API
2018-08-23 16:14 [Qemu-devel] [PATCH V3 0/4] introduce coalesced pio support Peng Hao
@ 2018-08-23 16:14 ` Peng Hao
2018-08-24 3:58 ` Eduardo Habkost
0 siblings, 1 reply; 4+ messages in thread
From: Peng Hao @ 2018-08-23 16:14 UTC (permalink / raw)
To: pbonzini, mst, ehabkost, rkrcmar; +Cc: kvm, qemu-devel, zhong.weidong, Peng Hao
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
accel/kvm/kvm-all.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-----
include/exec/memattrs.h | 2 +-
2 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index eb7db92..830745e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -78,6 +78,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;
@@ -536,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);
}
@@ -553,12 +554,51 @@ 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);
}
}
+static void kvm_coalesce_io_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_io_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_io_listener = {
+ .coalesced_mmio_add = kvm_coalesce_io_add,
+ .coalesced_mmio_del = kvm_coalesce_io_del,
+};
+
int kvm_check_extension(KVMState *s, unsigned int extension)
{
int ret;
@@ -1615,7 +1655,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);
#endif
@@ -1694,7 +1735,8 @@ static int kvm_init(MachineState *ms)
&address_space_memory, 0);
memory_listener_register(&kvm_io_listener,
&address_space_io);
-
+ memory_listener_register(&kvm_coalesced_io_listener,
+ &address_space_io);
s->many_ioeventfds = kvm_check_many_ioeventfds();
s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
@@ -1775,8 +1817,12 @@ void kvm_flush_coalesced_mmio_buffer(void)
struct kvm_coalesced_mmio *ent;
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_NONE, 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/memattrs.h b/include/exec/memattrs.h
index d4a1642..12fd64f 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -45,7 +45,7 @@ typedef struct MemTxAttrs {
* from "didn't specify" if necessary).
*/
#define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
-
+#define MEMTXATTRS_NONE ((MemTxAttrs) { 0 })
/* New-style MMIO accessors can indicate that the transaction failed.
* A zero (MEMTX_OK) response means success; anything else is a failure
* of some kind. The memory subsystem will bitwise-OR together results
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API
2018-08-23 16:14 ` [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API Peng Hao
@ 2018-08-24 3:58 ` Eduardo Habkost
0 siblings, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2018-08-24 3:58 UTC (permalink / raw)
To: Peng Hao; +Cc: pbonzini, mst, rkrcmar, kvm, qemu-devel, zhong.weidong
On Fri, Aug 24, 2018 at 12:14:47AM +0800, Peng Hao wrote:
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
> accel/kvm/kvm-all.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-----
> include/exec/memattrs.h | 2 +-
> 2 files changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index eb7db92..830745e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -78,6 +78,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;
> @@ -536,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);
> }
> @@ -553,12 +554,51 @@ 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);
> }
> }
The 2 hunks above need to go to patch 1/4 to avoid breaking the
build after applying 1/4.
However, patch 1/4 is just a header update and I don't think
running update-linux-headers.sh was supposed to break the build.
Radim asked about this at:
Date: Wed, 18 Jul 2018 16:43:11 +0200
Subject: Re: [PATCH v2] KVM: Add coalesced PIO support
Message-ID: <20180718144311.GB6348@flask>
Paolo, do you have a suggestion on how to update the struct
without making a header update break the build?
>
> +static void kvm_coalesce_io_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_io_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_io_listener = {
> + .coalesced_mmio_add = kvm_coalesce_io_add,
> + .coalesced_mmio_del = kvm_coalesce_io_del,
Nit: I would rename the fields to "coalesced_mmio_{add,del}" to
coalesced_io_*, because they are not specific to MMIO anymore.
I would also name the functions kvm_coalesce_pio_{add,del}
because the are specific to pio.
> +};
> +
> int kvm_check_extension(KVMState *s, unsigned int extension)
> {
> int ret;
> @@ -1615,7 +1655,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);
Nit: I would keep the blank line here.
> #ifdef KVM_CAP_VCPU_EVENTS
> s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
> #endif
> @@ -1694,7 +1735,8 @@ static int kvm_init(MachineState *ms)
> &address_space_memory, 0);
> memory_listener_register(&kvm_io_listener,
> &address_space_io);
> -
> + memory_listener_register(&kvm_coalesced_io_listener,
> + &address_space_io);
Nit: I would keep the blank line here.
> s->many_ioeventfds = kvm_check_many_ioeventfds();
>
> s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
> @@ -1775,8 +1817,12 @@ void kvm_flush_coalesced_mmio_buffer(void)
> struct kvm_coalesced_mmio *ent;
>
> 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_NONE, ent->data, ent->len, true);
Why exactly MEMTXATTRS_NONE is the right attrs argument here?
Why MEMTXATTRS_UNSPECIFIED wouldn't work?
> + } 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/memattrs.h b/include/exec/memattrs.h
> index d4a1642..12fd64f 100644
> --- a/include/exec/memattrs.h
> +++ b/include/exec/memattrs.h
> @@ -45,7 +45,7 @@ typedef struct MemTxAttrs {
> * from "didn't specify" if necessary).
> */
> #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
> -
> +#define MEMTXATTRS_NONE ((MemTxAttrs) { 0 })
Nit: I would keep the blank line here.
> /* New-style MMIO accessors can indicate that the transaction failed.
> * A zero (MEMTX_OK) response means success; anything else is a failure
> * of some kind. The memory subsystem will bitwise-OR together results
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-24 10:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-24 9:18 [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API peng.hao2
2018-08-24 10:42 ` Eduardo Habkost
-- strict thread matches above, loose matches on Subject: below --
2018-08-23 16:14 [Qemu-devel] [PATCH V3 0/4] introduce coalesced pio support Peng Hao
2018-08-23 16:14 ` [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API Peng Hao
2018-08-24 3:58 ` Eduardo Habkost
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).