Attention is currently required from: Shelley Chen, Ravi kumar, Paul Menzel, Julius Werner, Arthur Heymans, Kyösti Mälkki, Prasad Malisetty, mturney mturney. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/53903 )
Change subject: libpayload: Add MMIO support in PCI lib ......................................................................
Patch Set 30:
(2 comments)
File payloads/libpayload/drivers/pci.c:
https://review.coreboot.org/c/coreboot/+/53903/comment/9a703c35_7ee1f5b4 PS22, Line 35: #if CONFIG(LP_MMCONF_SUPPORT) : void *cfg_addr = lib_sysinfo.pci_ep_cfg_base + reg; : return read8(cfg_addr); : #endif : outl(device | (reg & ~3), 0xCF8); : return inb(0xCFC + (reg & 3));
Below API's are already present in pci.c file.
These are currently not present in ToT coreboot.
are you referring "pci_read/write_config8/16/32" in pci.c file ?
I meant pci_mmio_{read|write}_config* ops to be defined that perform config space access using MMIO operations.
Is my analysis is right on above comment ? Please let me know, I will incorporate the changes in next version patch.
Yes, your idea about using MMIO ops if MMIO base address is present and fallback to IO ops (only on x86) if no MMIO base address seems right.
File payloads/libpayload/drivers/pci_ops.c:
https://review.coreboot.org/c/coreboot/+/53903/comment/a5e75d7b_2877ef8d PS28, Line 37: u32 devfn = ((dev >> 3) & 0x1f) | (dev & 0x07);
Hi Furquan, […]
Makes sense. I have been also looking at how things are done in the Linux kernel and I like the idea of having a callback in the common code that allows the platform to perform any bus mapping if required. Something like `pci_map_bus()` that can configure the bus as required and return the MMIO address for the 4K config space of the device. For ECAM, this can use MMCONFIG. For other platforms (e.g. QCOM), you can use the callback to perform ATU configuration and return appropriate address. Does this make sense to you?
We would need this change both in coreboot and libpayload.
On that note, I think `MMCONF_SUPPORT` is not really the right option for qualcomm platforms. This is what I am thinking:
* There are two ways to access PCI config space - I/O access and MMIO access * For MMIO access, PCI spec defines ECAM which is what the MMCONF_SUPPORT is about. However, the PCI spec provides flexibility to the platform if it wants to design the configuration window differently.
Thus, for a qualcomm platform, you would need MMIO access mechanism with a customized way of getting to the config window.