[coreboot-gerrit] Change in coreboot[master]: coreboot_tables: Add PCIe info to coreboot table

[Nico Huber (Code Review)]

Attention is currently required from: Shelley Chen, Ravi Kumar Bokka, Hung-Te Lin, Tim Wawrzynczak, Paul Menzel, Rex-BC Chen, Julius Werner, Arthur Heymans, Yu-Ping Wu, Jianjun Wang.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63251 )

Change subject: coreboot_tables: Add PCIe info to coreboot table
......................................................................

Patch Set 20:

(1 comment)

Patchset:

PS9:
> I really don't understand where the 1MB number is coming from here.

Wild guess, maybe it was meant as a feature but turned out to be too
subtle to be useful. Usually 1MiB covers the config space for all devices
on a single bus, as the calculation above suggests. However, there are
several clues why this might not work as expected:

1. On the PCI side of the ATU, we don't have 4KiB per function as PCIe
needs but 64KiB instead (BDF on the PCI-side address is covers bits
31..16). Assuming the ATU translates this 1:1, there's only space for
16 functions instead of the expected 256.

2. The ATU sharing with the I/O-port mapping. It kind of implies that
one can't leave the PCI config space mapped for future access. And
without proper locking, it's even dangerous (a concurrent PCI driver
trying to write I/O ports might write to random config space instead).
This wouldn't matter for coreboot, but if the code was originally tested
for Linux, who knows...

It's all probably hard to test. One would need a legacy PCI bridge or
a PCIe switch device. A single device with multiple functions could
serve to test at least multiple functions.

If one has address space to waste and the secondary bus at the root
port can be configured, one could also test a bigger mapping some-
where else (after all the ATU is configured with the CPU-side address
and size). Then configure the secondary bus >1, map 00:00.0 and see
if the downstream device shows up anywhere in the mapped space.
> If you want to access PCIe config space registers for the controller itself, you have to go over the DBI (data bus interface), where those 4096 bytes are mapped into the lower 4096 bytes of the DBI space.

This is also what I read in the kernel but couldn't find in coreboot
code yet.