Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: find IIO_UDS HOB once when creating DMAR table ......................................................................
Patch Set 2:
(7 comments)
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 598: if (port == PORT_0 && stack != CSTACK)
I'd keep the `get_stack_for_port` function: […]
Done
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 608:
This is one test and could only have a single return point or keep the get stack for port function a […]
Done
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 609: uint32_t bus = iio_resource.StackRes[stack].BusBase; : uint32_t dev = iio_resource.PcieInfo.PortInfo[port].Device; : uint32_t func = iio_resource.PcieInfo.PortInfo[port].Function; : : uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), : PCI_VENDOR_ID);
Make these const?
Done
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 615: 0xffffffff
Should this check for an expected DEVID instead of just checking if a device is present?
There are multiple possible DEVIDs, I am fine with either way though. [root@localhost ~]# lspci -d 8086:2033 15:03.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port 1D (rev 0b) 63:03.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port 1D (rev 0b) b2:03.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port 1D (rev 0b) [root@localhost ~]# lspci -d 8086:2030 15:00.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port 1A (rev 0b) 63:00.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port 1A (rev 0b) [root@localhost ~]# lspci -d 8086:203f b2:00.0 PCI bridge: Intel Corporation Device 203f (rev 0b)
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 702:
Try to use the enums consistently p = PSTACK0
Done
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 764: 0
Try to use the enums consistently p = PSTACK0
Done
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 765: 0
PSTACK0
Done