Re: [Qemu-devel] [PATCHv4 04/11] libqos: Better handling of PCI legacy IO

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

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

On Fri, Oct 21, 2016 at 12:05:06PM +0200, Greg Kurz wrote:
> On Fri, 21 Oct 2016 12:19:45 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The usual model for PCI IO with libqos is to use qpci_iomap() to map a
> > specific BAR for a PCI device, then perform IOs within that BAR using
> > qpci_io_{read,write}*().
> > 
> > However, certain devices also have legacy PCI IO.  In this case, instead of
> > (or as well as) being accessed via PCI BARs, the device can be accessed
> > via certain well-known, fixed addresses in PCI IO space.
> > 
> > Two existing tests use legacy PCI IO, and take different flawed approaches
> > to it:
> >     * tco-test manually constructs a tco_io_base value instead of calling
> >       qpci_iomap(), which assumes internal knowledge of the structure of
> >       the value it shouldn't have
> >     * ide-test uses direct in*() and out*() calls instead of using
> >       qpci_io_*() accessors, meaning it's not portable to non-x86 machine
> >       types.
> > 
> > This patch implements a new qpci_iomap_legacy() interface which gets a
> > handle in the same format as qpci_iomap() but refers to a region in
> > the legacy PIO space.  For a device which has the same registers
> > available both in a BAR and in legacy space (quite common), this
> > allows the same test code to test both options with just a different
> > iomap() at the beginning.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> >  tests/libqos/pci.c | 5 +++++
> >  tests/libqos/pci.h | 1 +
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
> > index bf1c532..98a2e56 100644
> > --- a/tests/libqos/pci.c
> > +++ b/tests/libqos/pci.c
> > @@ -350,6 +350,11 @@ void qpci_iounmap(QPCIDevice *dev, void *data)
> >      /* FIXME */
> >  }
> >  
> > +void *qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr)
> > +{
> > +    return (void *)(uintptr_t)addr;
> > +}
> > +
> 
> Since both tco-test and ide-test are hardcoded for x86 machine types, is
> it right for this implementation to sit in common PCI code instead of
> the PC specific one ?

Those tests are x86 specific for now.  tco-test is of some ICH9
internal device so it probably always will be.  But it's absolutely
possible to have a legacy IDE controller on another platform.

Even if those specific tests don't happen, it's likely someone will
want to test some sort of legacy device on another platform at some
point.

[Even though the "legacy" in question is old PCs, it's not uncommon to
see these devices on other platforms - a PC-style southbridge chip
including an ISA bridge and legacy devices is a cheap way of getting a
bunch of useful gadgets on your board, so it's pretty common on many
archs]

> >  void qpci_plug_device_test(const char *driver, const char *id,
> >                             uint8_t slot, const char *opts)
> >  {
> > diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
> > index f6f916d..b6f855e 100644
> > --- a/tests/libqos/pci.h
> > +++ b/tests/libqos/pci.h
> > @@ -94,6 +94,7 @@ void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value);
> >  
> >  void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr);
> >  void qpci_iounmap(QPCIDevice *dev, void *data);
> > +void *qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr);
> >  
> >  void qpci_plug_device_test(const char *driver, const char *id,
> >                             uint8_t slot, const char *opts);
> 

-- 
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 --]