On 19/10/2016 14:25, David Gibson 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.
>
> tco_test uses the libqos PCI code to access the device. This makes perfect
> sense for the PCI config space accesses. However for IO, rather than the
> usual PCI approach of mapping a PCI BAR, then accessing that, it instead
> uses the legacy approach of fixed, known addresses in PCI IO space.
>
> That doesn't work very well with the qpci_io_{read,write} functions because
> we never use qpci_iomap() and so have to make assumptions about the
> internal encoding of the address tokens iomap() returns.
>
> This patch avoids that, by directly using the bus's pio_{read,write}
> callbacks, which are defined to take addresses within the PCI IO space.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
This patch makes sense but it is not obvious (at least for me) why you
are doing this if I don't read patch 11/11 before...
Reviewed-by: Laurent Vivier <lviv...@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;
> +}
> +
> 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);
>
