* [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
* 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
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).