Re: [Qemu-devel] [RFC PATCH 1/3] tests: add libpci qtest library

[Blue Swirl]

On Fri, Apr 13, 2012 at 14:27, Stefan Hajnoczi
<address@hidden> wrote:
> This patch adds a common PCI bus driver library which works for
> i386/x86-64 targets.  Tests can use the library to probe for PCI
> devices, map BARs, and access configuration space.

I guess we have almost identical code in SeaBIOS, OpenBIOS, various OS
and maybe userland PCI tools. Would it be possible to reduce NIH
somewhere?

>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  tests/libpci.c |  106 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libpci.h |   41 ++++++++++++++++++++++
>  2 files changed, 147 insertions(+)
>  create mode 100644 tests/libpci.c
>  create mode 100644 tests/libpci.h
>
> diff --git a/tests/libpci.c b/tests/libpci.c
> new file mode 100644
> index 0000000..24d4d8b
> --- /dev/null
> +++ b/tests/libpci.c
> @@ -0,0 +1,106 @@
> +/*
> + * QTest PCI library
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Stefan Hajnoczi <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <glib.h>
> +#include "bswap.h"
> +#include "libqtest.h"
> +#include "libpci.h"
> +
> +enum {
> +    /* PCI controller I/O ports */
> +    PCI_CONFIG_ADDR = 0xcf8,
> +    PCI_CONFIG_DATA = 0xcfc,
> +};
> +
> +static void pci_config_setup(PciDevice *dev, unsigned int offset)
> +{
> +    outl(PCI_CONFIG_ADDR, 0x80000000 | (dev->devfn << 8) | (offset & ~3));
> +}
> +
> +uint8_t pci_config_readb(PciDevice *dev, unsigned int offset)
> +{
> +    pci_config_setup(dev, offset);
> +    return inb(PCI_CONFIG_DATA + (offset & 3));
> +}
> +
> +void pci_config_writeb(PciDevice *dev, unsigned int offset, uint8_t b)
> +{
> +    pci_config_setup(dev, offset);
> +    outb(PCI_CONFIG_DATA + (offset & 3), b);
> +}
> +
> +uint16_t pci_config_readw(PciDevice *dev, unsigned int offset)
> +{
> +    pci_config_setup(dev, offset);
> +    return inw(PCI_CONFIG_DATA + (offset & 2));
> +}
> +
> +void pci_config_writew(PciDevice *dev, unsigned int offset, uint16_t w)
> +{
> +    pci_config_setup(dev, offset);
> +    outw(PCI_CONFIG_DATA + (offset & 2), w);
> +}
> +
> +uint32_t pci_config_readl(PciDevice *dev, unsigned int offset)
> +{
> +    pci_config_setup(dev, offset);
> +    return inl(PCI_CONFIG_DATA);
> +}
> +
> +void pci_config_writel(PciDevice *dev, unsigned int offset, uint32_t l)
> +{
> +    pci_config_setup(dev, offset);
> +    outl(PCI_CONFIG_DATA, l);
> +}

All code above is specific to i440fx or similar PCI bridges, other
bridges may use different config space access methods. If we want to
share the rest for example with Sparc64 or PPC, the above would need
to be changed. How about splitting the above to a separate file? It
could be done later too.

> +
> +/* Initialize a PciDevice if a device is present on the bus */

The comment talks about initalizing instead of only probing which is
actually what the code does, please fix.

> +bool pci_probe(PciDevice *dev, unsigned int slot, unsigned int func)
> +{
> +    uint16_t vendor;
> +
> +    dev->devfn = (slot << 3) | func;
> +    vendor = pci_config_readw(dev, PCI_VENDOR_ID);
> +    if (vendor == 0xffff || vendor == 0x0) {
> +        return false;
> +    }
> +    return true;
> +}
> +
> +/* Map an I/O BAR to a specific address */
> +void pci_map_bar_io(PciDevice *dev, unsigned int bar, uint16_t addr)
> +{
> +    uint32_t old_bar;
> +
> +    old_bar = pci_config_readl(dev, bar);
> +    g_assert_cmphex(old_bar & PCI_BASE_ADDRESS_SPACE, ==,
> +                    PCI_BASE_ADDRESS_SPACE_IO);
> +
> +    /* Address must be valid */
> +    g_assert_cmphex(addr & ~PCI_BASE_ADDRESS_IO_MASK, ==, 0);
> +
> +    pci_config_writel(dev, bar, addr);
> +
> +    /* BAR must have accepted address */
> +    old_bar = pci_config_readl(dev, bar);
> +    g_assert_cmphex(old_bar & PCI_BASE_ADDRESS_IO_MASK, ==, addr);
> +}
> +
> +/* Enable memory and I/O decoding so BARs can be accessed */
> +void pci_enable(PciDevice *dev)
> +{
> +    uint16_t cmd;
> +
> +    cmd = pci_config_readw(dev, PCI_COMMAND);
> +    pci_config_writew(dev, PCI_COMMAND,
> +                      cmd | PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
> +}
> diff --git a/tests/libpci.h b/tests/libpci.h
> new file mode 100644
> index 0000000..1b3103d
> --- /dev/null
> +++ b/tests/libpci.h
> @@ -0,0 +1,41 @@
> +/*
> + * QTest PCI library
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Stefan Hajnoczi <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef LIBPCI_H
> +#define LIBPCI_H
> +
> +#include <stdbool.h>
> +#include "hw/pci_regs.h"
> +
> +/* A PCI Device
> + *
> + * Set up a PciDevice instance with pci_probe().  The device can then be used
> + * for configuration space access and other operations.
> + */
> +typedef struct {
> +    uint8_t devfn;
> +} PciDevice;
> +
> +bool pci_probe(PciDevice *dev, unsigned int slot, unsigned int func);
> +void pci_map_bar_io(PciDevice *dev, unsigned int bar, uint16_t addr);
> +void pci_enable(PciDevice *dev);
> +
> +/* Configuration space access */
> +uint8_t pci_config_readb(PciDevice *dev, unsigned int offset);
> +void pci_config_writeb(PciDevice *dev, unsigned int offset, uint8_t b);
> +uint16_t pci_config_readw(PciDevice *dev, unsigned int offset);
> +void pci_config_writew(PciDevice *dev, unsigned int offset, uint16_t w);
> +uint32_t pci_config_readl(PciDevice *dev, unsigned int offset);
> +void pci_config_writel(PciDevice *dev, unsigned int offset, uint32_t l);
> +
> +#endif /* LIBPCI_H */
> --
> 1.7.9.5
>
>