[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API
From: |
peng.hao2 |
Subject: |
Re: [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API |
Date: |
Fri, 24 Aug 2018 17:18:45 +0800 (CST) |
> On Fri, Aug 24, 2018 at 12:14:47AM +0800, Peng Hao wrote:
> > Signed-off-by: Peng Hao <address@hidden>
> > ---
> > 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: <address@hidden>
> 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 })
- Re: [Qemu-devel] [PATCH V3 2/4] target-i386:add coalesced_pio API,
peng.hao2 <=