[SeaBIOS] Re: [PATCH] pci: add mmconfig support

[Kevin O'Connor]

On Thu, Mar 19, 2020 at 10:21:55AM +0100, Gerd Hoffmann wrote:
> Add support for pci config space access via mmconfig bar.  Enable for
> qemu q35 chipset.  Main advantage is that we need only one instead of
> two io operations per config space access, which translates to one
> instead of two vmexits for virtualization.
> Signed-off-by: Gerd Hoffmann
>
> src/hw/pci.h     |  1 +
>  src/fw/pciinit.c |  1 +
>  src/hw/pci.c     | 54 ++++++++++++++++++++++++++++++++++++++++++------
>  3 files changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/src/hw/pci.h b/src/hw/pci.h
> index 2e30e28918a0..01c51f705a00 100644
> --- a/src/hw/pci.h
> +++ b/src/hw/pci.h
> @@ -39,6 +39,7 @@ u32 pci_config_readl(u16 bdf, u32 addr);
>  u16 pci_config_readw(u16 bdf, u32 addr);
>  u8 pci_config_readb(u16 bdf, u32 addr);
>  void pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on);
> +void pci_enable_mmconfig(u64 addr, const char *name);
>  u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap);
>  int pci_next(int bdf, int bus);
>  int pci_probe_host(void);
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index d5e87f00f164..d25931bb0573 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -480,6 +480,7 @@ static void mch_mmconfig_setup(u16 bdf)
>      pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR, 0);
>      pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR + 4, upper);
>      pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR, lower);
> pci_enable_mmconfig(Q35_HOST_BRIDGE_PCIEXBAR_ADDR, "q35");
>
> }
>
> static void mch_mem_addr_setup(struct pci_device *dev, void *arg)
> diff --git a/src/hw/pci.c b/src/hw/pci.c
> index 9855badbc7da..3b8f43cbe5dc 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -14,38 +14,71 @@
>  #define PORT_PCI_CMD           0x0cf8
>  #define PORT_PCI_DATA          0x0cfc
>
> +static u32 mmconfig;
>
> +static void *pci_mmconfig_addr(u16 bdf, u32 addr)
> +{
> if (!mmconfig)
>    return NULL;
> return (void*)(mmconfig + ((u32)bdf << 12) + addr);
>
> +}
>
> void pci_config_writel(u16 bdf, u32 addr, u32 val)
>  {
> outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
> outl(val, PORT_PCI_DATA);
> void *mmcfg = pci_mmconfig_addr(bdf, addr);
> if (mmcfg) {
>    writel(mmcfg, val);
> } else {
>    outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
>    outl(val, PORT_PCI_DATA);
> }
>
> }

The pci_config_writeX() functions are called from 16-bit mode and
32-big segment mode, so this doesn't look correct to me.

What device would call the pci_config_writeX() functions frequently
enough that performance would matter?

Thanks,
-Kevin
> void pci_config_writew(u16 bdf, u32 addr, u16 val)
>  {
> outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
> outw(val, PORT_PCI_DATA + (addr & 2));
> void *mmcfg = pci_mmconfig_addr(bdf, addr);
> if (mmcfg) {
>    writew(mmcfg, val);
> } else {
>    outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
>    outw(val, PORT_PCI_DATA + (addr & 2));
> }
>
> }
>
> void pci_config_writeb(u16 bdf, u32 addr, u8 val)
>  {
> outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
> outb(val, PORT_PCI_DATA + (addr & 3));
> void *mmcfg = pci_mmconfig_addr(bdf, addr);
> if (mmcfg) {
>    writeb(mmcfg, val);
> } else {
>    outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
>    outb(val, PORT_PCI_DATA + (addr & 3));
> }
>
> }
>
> u32 pci_config_readl(u16 bdf, u32 addr)
>  {
> void *mmcfg = pci_mmconfig_addr(bdf, addr);
> if (mmcfg)
>    return readl(mmcfg);
> outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
>    return inl(PORT_PCI_DATA);
>
> }
>
> u16 pci_config_readw(u16 bdf, u32 addr)
>  {
> void *mmcfg = pci_mmconfig_addr(bdf, addr);
> if (mmcfg)
>    return readw(mmcfg);
> outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
>    return inw(PORT_PCI_DATA + (addr & 2));
>
> }
>
> u8 pci_config_readb(u16 bdf, u32 addr)
>  {
> void *mmcfg = pci_mmconfig_addr(bdf, addr);
> if (mmcfg)
>    return readb(mmcfg);
> outl(0x80000000 | (bdf << 8) | (addr & 0xfc), PORT_PCI_CMD);
>    return inb(PORT_PCI_DATA + (addr & 3));
>
> }
> @@ -58,6 +91,15 @@ pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on)
>      pci_config_writew(bdf, addr, val);
>  }
>
> +void
> +pci_enable_mmconfig(u64 addr, const char *name)
> +{
> if (addr >= 0x100000000ll)
>    return;
> dprintf(1, "PCIe: using %s mmconfig at 0x%llx\n", name, addr);
> mmconfig = addr;
>
> +}
>
> u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap)
>  {
>      int i;
> -- 
> 2.18.2
> _______________________________________________
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]