Re: [SeaBIOS] [RFC v2 1/3] fw/pciinit: Recognize pxb-pcie-dev device

[Marcel Apfelbaum]

On 08/09/2018 08:43 AM, Zihan Yang wrote:
> QEMU q35 uses pxb-pcie-dev to enable multiple host bridges, this patch
> recognizes such devices in seabios and add corresponding e820 entry.
>
> MCFG base and size are already setup in QEMU, we just need to read it
> Signed-off-by: Zihan Yang
>
> src/fw/paravirt.c |  1 -
>   src/fw/pciinit.c  | 17 +++++++++++++++++
>   src/hw/pci_ids.h  |  1 +
>   3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 0770c47..6b14542 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -197,7 +197,6 @@ qemu_platform_setup(void)
>           if (!loader_err)
>               warn_internalerror();
>       }

No need for this chunk.
>   acpi_setup();
>
> }
>
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 3a2f747..6e6a434 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -507,11 +507,28 @@ static void mch_mem_addr_setup(struct pci_device *dev, void *arg)
>           pci_io_low_end = acpi_pm_base;
>   }
>
> +static void pxb_mem_addr_setup(struct pci_device *dev, void *arg)
> +{
> union u64_u32_u mcfg_base;
> mcfg_base.lo = pci_config_readl(dev->bdf, Q35_HOST_BRIDGE_PCIEXBAR);
> mcfg_base.hi = pci_config_readl(dev->bdf, Q35_HOST_BRIDGE_PCIEXBAR + 4);
> // Fix me! Use another meaningful macro
> u32 mcfg_size = pci_config_readl(dev->bdf, Q35_HOST_BRIDGE_PCIEXBAR + 8);

That is indeed strange. If I got it right, this is the address of Q35 
host bridge,
we want other addresses here.
> /* Skip config write here as the qemu-level objects are already setup, we
> * read mcfg_base and mcfg_size from it just now. Instead, we directly add
> * this item to e820 */
> e820_add(mcfg_base.val, mcfg_size, E820_RESERVED);

So did QEMU configure the address or only the size?
> +}
> static const struct pci_device_id pci_platform_tbl[] = {
>     PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
>                i440fx_mem_addr_setup),
>     PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q35_MCH,
>                mch_mem_addr_setup),
> PCI_DEVICE(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_PXB_HOST,
>           pxb_mem_addr_setup),

Will this recognize only the pxb-pcie bridge, or also the legacy pxb one?
Because we plan to support only the pxb-pcie version.

Thanks,
Marcel
>   PCI_DEVICE_END
>
> };
>
> diff --git a/src/hw/pci_ids.h b/src/hw/pci_ids.h
> index 38fa2ca..35096ea 100644
> --- a/src/hw/pci_ids.h
> +++ b/src/hw/pci_ids.h
> @@ -2265,6 +2265,7 @@
>
> #define PCI_VENDOR_ID_REDHAT		0x1b36
>   #define PCI_DEVICE_ID_REDHAT_ROOT_PORT	0x000C
> +#define PCI_DEVICE_ID_REDHAT_PXB_HOST	0x000B
>
> #define PCI_VENDOR_ID_TEKRAM		0x1de1
>   #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29