Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40335 )
Change subject: soc/intel/common/block/acpi: Add provision for multiple PCI segments ......................................................................
Patch Set 5:
Patch Set 5:
(2 comments)
I've left some inline comments. But before we contiue discussing the implementation, IMHO, it's better to discuss some design decisions around it.
First of all, I don't think we should make the number of segment groups follow the SA_PCIEX_LENGTH Kconfig. Instead, I would prefer another Kconfig, to be set by the mainboard or soc code, that sets the number of segment groups. If that is > 1, then SA_PCIEX_LENGTH should be increased automatically. Let's call this PCI_SEGMENT_GROUPS.
We can then add additional segment groups to the DSDT, according to PCI_SEGMENT_GROUPS. If we should do that with an SSDT generator, or in static .asl files, at the soc level or at the mainboard level, depends a lot on the actual use case. Which I know next to nothing about.
Only when patches to add additional segment groups to the DSDT, along with their _SEG numbers have landed, this change should land. cf. PCI Firmware Specification "MCFG Table Description"
Most of design i agree with except that we need a dedicated Kconfig for knowing PCI segment PCI_SEGMENT_GROUPS because EDS never talks about how many segment present, its deduced based on PCI BUS number SA_PCIEX_LENGTH we have.
Can't we use SA_PCIEX_LENGTH to calculate segment number dynamically (rather doing opposite?) as we are doing now and then pass those information from .c file .asl (northbridge.asl) to create additional capability for new segment (for _SEG 1). The reason is knowing SA_PCIEX_LENGTH and PCI_SEGMENT_GROUPS are too much information we expect from soc/mb user to define SoC programming logic.