Attention is currently required from: Raul Rangel, Mariusz Szafrański, Jonathan Zhang, Paul Menzel, Stefan Reinauer, Kyösti Mälkki, Andrey Petrov, Patrick Rudolph, Nico Huber, Anjaneya "Reddy" Chagam, Johnny Lin, Suresh Bellampalli, Morgan Jang, Michal Motyl, Alexander Couzens, Felix Held, Shelley Chen, Angel Pons, Lance Zhao, Jason Glenesk, Martin Roth, Damien Zammit, Lee Leahy, Marshall Dawson, Tim Wawrzynczak, Vanessa Eusebio, Huang Jin. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57333 )
Change subject: Rename MMCONF Kconfigs to ECAM_MMCONF ......................................................................
Patch Set 5:
(13 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57333/comment/e3dc103b_c5612532 PS3, Line 11: which applies only to x86 platforms Actually, this applies to platforms that support ACPI.
https://review.coreboot.org/c/coreboot/+/57333/comment/652b14d9_31b0d602 PS3, Line 11: This can be confusing as : other platforms, such as ARM, use a different way of mapping the pci : config space to memory.
My understanding so far (for QC IIUC) is that there is a single 4K window in MMIO space, and some ot […]
That sounds right for QC platform. Other platforms could implement this in other ways too.
https://review.coreboot.org/c/coreboot/+/57333/comment/3fb8efc2_9dc97bca PS3, Line 16: NO_ECAM_MMCONF_SUPPORT Reading the PCI firmware specification again, ECAM and MMCONF actually go hand in hand.
So,
Commit Message:
https://review.coreboot.org/c/coreboot/+/57333/comment/ceb292de_5a3d19da PS4, Line 9: the MMCONF Kconfigs only use the Enhanced Configuration : Access mechanism (ECAM) method Reading following two docs again: * PCI base express specification * PCI firmware specification
This is what I understand: ECAM and MMCONFIG go hand in hand. PCI base express specification mentions that platforms that support ACPI must implement Enhanced configuration access mechanism (ECAM). This allows PCI configuration space access using memory primitives instead of I/O-based primitives (CF8/CFC). In order to report the memory mapped configuration (MMCONFIG) space base address, MCFG table is used in ACPI.
Given that, I think we need to set up things such that:
There are two types of PCI ops supported: * PCI_IO_OPS * PCI_MMIO_OPS
For PCI_MMIO_OPS, there will be PCI_ECAM which will be selected by default if PCI and ACPI are both selected for a platform. If ACPI is not selected, then the platform can either choose to select PCI_ECAM or it can provide its own access mechanisms.
I think the current MMCONF Kconfigs are really a mix of MMIO ops and ECAM. So, we will have to inspect and create two different Kconfigs i.e. PCI_MMIO_OPS and PCI_ECAM and accordingly update the current configs.
PCI_MMIO_OPS must be selected by default if PCI is selected unless a platform selects PCI_NO_MMIO_OPS.
https://review.coreboot.org/c/coreboot/+/57333/comment/a6cb1a94_48b06689 PS4, Line 11: which applies only to x86 platforms Actually, this applies to platforms that support ACPI. So, if there are non-x86 platforms that support ACPI, they will support ECAM.
https://review.coreboot.org/c/coreboot/+/57333/comment/fae77218_acefbd93 PS4, Line 16: NO_ECAM_MMCONF_SUPPORT I think this will have to be renamed to `PCI_NO_MMIO_OPS`
https://review.coreboot.org/c/coreboot/+/57333/comment/be0532ce_e5c93a8b PS4, Line 17: ECAM_MMCONF_SUPPORT This will really be either `PCI_ECAM` or `PCI_MMIO_OPS` depending upon how it is used.
https://review.coreboot.org/c/coreboot/+/57333/comment/41e121a3_ffd3a241 PS4, Line 18: ECAM_MMCONF_BASE_ADDRESS I think `MMCONF_BASE_ADDRESS` is fine here because that is how the PCI spec uses it. Probably this can be renamed to PCI_MMCONF_BASE_ADDRESS and add dependency on `PCI_ECAM`.
https://review.coreboot.org/c/coreboot/+/57333/comment/a74c46cc_cb899b24 PS4, Line 19: MMCONF_BUS_NUMBER --> ECAM_MMCONF_BUS_NUMBER : MMCONF_LENGTH --> ECAM_MMCONF_LENGTH Same for these two Kconfigs as `MMCONF_BASE_ADDRESS`.
File src/arch/x86/include/arch/pci_io_cfg.h:
https://review.coreboot.org/c/coreboot/+/57333/comment/e50876be_5806b636 PS4, Line 72: ECAM_MMCONF_SUPPORT This should be `!CONFIG(PCI_MMIO_OPS)`
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/57333/comment/8eda7e1c_0a70d7ce PS4, Line 502: NO_ECAM_MMCONF_SUPPORT Posted a comment on the commit message about the Kconfigs.
File src/northbridge/intel/e7505/Kconfig:
https://review.coreboot.org/c/coreboot/+/57333/comment/711d9d6d_ab3d04c9 PS4, Line 10: NO_ECAM_MMCONF_SUPPORT This will have to be changed to PCI_NO_MMIO_OPS
File src/soc/intel/quark/Kconfig:
https://review.coreboot.org/c/coreboot/+/57333/comment/63335c3b_310ce292 PS4, Line 13: NO_ECAM_MMCONF_SUPPORT This should be PCI_NO_MMIO_OPS