| Message ID | 1334327272-31278-2-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Apr 13, 2012 at 14:27, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> 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 <stefanha@linux.vnet.ibm.com> > --- > 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 <stefanha@linux.vnet.ibm.com> > + * > + * 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 <stefanha@linux.vnet.ibm.com> > + * > + * 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 > >
On Sat, Apr 14, 2012 at 12:32:00PM +0000, Blue Swirl wrote: > On Fri, Apr 13, 2012 at 14:27, Stefan Hajnoczi > <stefanha@linux.vnet.ibm.com> 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? Probably not given how small these functions are and how they use glib assert calls because they are part of tests. > > +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. Yes, it's only i440fx for now. I think it makes sense to move it later since we have no non-x86 qtests yet. > > + > > +/* 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. Sorry this comment is unclear. I meant initializing the "PciDevice" struct, not the physical PCI device. Will change. Stefan
Am 16.04.2012 11:14, schrieb Stefan Hajnoczi: > On Sat, Apr 14, 2012 at 12:32:00PM +0000, Blue Swirl wrote: >> On Fri, Apr 13, 2012 at 14:27, Stefan Hajnoczi >> <stefanha@linux.vnet.ibm.com> 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? > > Probably not given how small these functions are and how they use glib > assert calls because they are part of tests. > >>> +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. > > Yes, it's only i440fx for now. I think it makes sense to move it later > since we have no non-x86 qtests yet. As stated before, I'm very grateful of your work in this area. I'd be very interested in having PCI-based tests for the PReP devices we're introducing - some outline of how non-x86 targets are supposed to fit in here would be appreciated. For example, would we want to rename the file to libpci-i440fx.c and have libpci.h be a common interface for multiple implementations? Or are you expecting some #ifdef TARGET_FOO inside libpci.c? Thanks, Andreas
On Mon, Apr 16, 2012 at 12:20 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 16.04.2012 11:14, schrieb Stefan Hajnoczi: >> On Sat, Apr 14, 2012 at 12:32:00PM +0000, Blue Swirl wrote: >>> On Fri, Apr 13, 2012 at 14:27, Stefan Hajnoczi >>> <stefanha@linux.vnet.ibm.com> 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? >> >> Probably not given how small these functions are and how they use glib >> assert calls because they are part of tests. >> >>>> +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. >> >> Yes, it's only i440fx for now. I think it makes sense to move it later >> since we have no non-x86 qtests yet. > > As stated before, I'm very grateful of your work in this area. I'd be > very interested in having PCI-based tests for the PReP devices we're > introducing - some outline of how non-x86 targets are supposed to fit in > here would be appreciated. For example, would we want to rename the file > to libpci-i440fx.c and have libpci.h be a common interface for multiple > implementations? Or are you expecting some #ifdef TARGET_FOO inside > libpci.c? I think it's cleanest to have libpci-i440fx.c. When support for the next PCI controller gets added we can decide the details of how to split it. Stefan
On 04/16/2012 04:14 AM, Stefan Hajnoczi wrote: > On Sat, Apr 14, 2012 at 12:32:00PM +0000, Blue Swirl wrote: >> On Fri, Apr 13, 2012 at 14:27, Stefan Hajnoczi >> <stefanha@linux.vnet.ibm.com> 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? > > Probably not given how small these functions are and how they use glib > assert calls because they are part of tests. > >>> +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. > > Yes, it's only i440fx for now. I think it makes sense to move it later > since we have no non-x86 qtests yet. Note that this isn't i440fx specific at all. cf8 is the standard port for PCI on the PC. I think what we'll want to eventually do is make a pci_config_read/pci_write() set of function pointers that can be overloaded. But definitely don't name it libqtest-i440fx. At least call it libqtest-pc.c Regards, Anthony Liguori > >>> + >>> +/* 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. > > Sorry this comment is unclear. I meant initializing the "PciDevice" > struct, not the physical PCI device. Will change. > > Stefan > >
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 <stefanha@linux.vnet.ibm.com> + * + * 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); +} + +/* Initialize a PciDevice if a device is present on the bus */ +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 <stefanha@linux.vnet.ibm.com> + * + * 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 */
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. Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> --- 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