Re: [Qemu-devel] [PATCHv5 07/12] libqos: Implement mmio accessors in terms of mem{read, write}

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3359 bytes --]

On Tue, Oct 25, 2016 at 05:47:43PM +1100, Alexey Kardashevskiy wrote:
> On 24/10/16 15:59, David Gibson wrote:
> > In the libqos PCI code we now have accessors both for registers (byte
> > significance preserving) and for streaming data (byte address order
> > preserving).  These exist in both the interface for qtest drivers and in
> > the machine specific backends.
> > 
> > However, the register-style accessors aren't actually necessary in the
> > backend.  They can be implemented in terms of the byte address order
> > preserving accessors by the libqos wrappers.  This works because PCI is
> > always little endian.
> > 
> > This does assume that the back end byte address order preserving accessors
> > will perform the equivalent of a single bus transaction for short lengths.
> > This is the case, and in fact they currently end up using the same
> > cpu_physical_memory_rw() implementation within the qtest accelerator.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> >  tests/libqos/pci-pc.c    | 38 --------------------------------------
> >  tests/libqos/pci-spapr.c | 44 --------------------------------------------
> >  tests/libqos/pci.c       | 20 ++++++++++++++------
> >  tests/libqos/pci.h       |  8 --------
> >  4 files changed, 14 insertions(+), 96 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
> > index 2b08362..ce6ed08 100644
> > --- a/tests/libqos/pci.h
> > +++ b/tests/libqos/pci.h
> > @@ -27,18 +27,10 @@ struct QPCIBus {
> >      uint16_t (*pio_readw)(QPCIBus *bus, uint32_t addr);
> >      uint32_t (*pio_readl)(QPCIBus *bus, uint32_t addr);
> >  
> > -    uint8_t (*mmio_readb)(QPCIBus *bus, uint32_t addr);
> > -    uint16_t (*mmio_readw)(QPCIBus *bus, uint32_t addr);
> > -    uint32_t (*mmio_readl)(QPCIBus *bus, uint32_t addr);
> > -
> >      void (*pio_writeb)(QPCIBus *bus, uint32_t addr, uint8_t value);
> >      void (*pio_writew)(QPCIBus *bus, uint32_t addr, uint16_t value);
> >      void (*pio_writel)(QPCIBus *bus, uint32_t addr, uint32_t value);
> >  
> > -    void (*mmio_writeb)(QPCIBus *bus, uint32_t addr, uint8_t value);
> > -    void (*mmio_writew)(QPCIBus *bus, uint32_t addr, uint16_t value);
> > -    void (*mmio_writel)(QPCIBus *bus, uint32_t addr, uint32_t value);
> > -
> >      void (*memread)(QPCIBus *bus, uint32_t addr, void *buf, size_t len);
> >      void (*memwrite)(QPCIBus *bus, uint32_t addr, const void *buf, size_t len);
> >  
> > 
> 
> You added them in "libqos: Handle PCI IO de-multiplexing in common code"
> (few patched before) and removing them now - if you moved this patch
> earlier, it would reduce the series, or what do I miss?

Well, it can't go before the PIO / MMIO split, because on x86 the PIO
part is implemented with inw/outw instead of readw/writew, and those
don't have a memread/memwrite equivalent.

The change could go at the same time, but my feeling was that logical
separation of the steps was worth a bit of temporary extra code.

-- 
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]