Patch Set 2: Code-Review-1

Patch Set 1:

Patch Set 1:

The number of busses is typically configurable in PCI configuration, which afaik is done by FSP. ACPI has to match the hardware configuration, so the commit message should say if that is the case and how it was tested/verified.

The space used here depends on the assigned bus numbers, no on the enabled root ports.

The coreboot code is in theory free to assign any bus number between 0 and 255 to a bus it configures. There is no dependency on FSP. I also looked and the hardware specification and could not find a way that the bus number assigned can be limited (by masking bits e.g.)

The length of the PCIe configuration space (and therefore the max #busses) is actually configurable in 'PCIEXBAR' offset 0x60 of 00:00.0. It's just bad coreboot (or lack of coreboot code) to not report what is actually programmed in that register.

You are right but this register is set by coreboot depending on the SA_PCIEX_LENGTH setting and indeed within coreboot, there are no further dependencies on this register (regarding bus allocation e.g.). So if the memory mapping is correct I don't see why we shouldn't align the Skylake soc with all other Intel socs using the common system agent code. I will double check if this register is unmodified after post.

View Change

To view, visit change 37704. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8a06b9fba5ad561d8595292a73136091ab532faa
Gerrit-Change-Number: 37704
Gerrit-PatchSet: 2
Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Comment-Date: Fri, 13 Dec 2019 15:47:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment