Change in coreboot[master]: soc/intel/xeon_sp/cpx: find IIO_UDS HOB once when creating DMAR table
Jonathan Zhang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: find IIO_UDS HOB once when creating DMAR table ......................................................................
soc/intel/xeon_sp/cpx: find IIO_UDS HOB once when creating DMAR table
IIO_UDS HOB was found several times during the creation of DMAR table. Reduce it to only once to improve boot time.
Both DRHD and ATSR subtable creations involve addition of PCIe bridge device entries, combine the functions with acpi_create_dmar_ds_pci_br_for_port().
When looping through ports to create PCIe bridge device entries, use MAX_PORTS intead of NUMBER_PORTS_PER_SOCKET to improve boot time.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I469cd8473c50e105daeda6c5607592ae7cef6032 --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 65 insertions(+), 76 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/45376/1
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c index a40d33d..62b1033 100644 --- a/src/soc/intel/xeon_sp/cpx/acpi.c +++ b/src/soc/intel/xeon_sp/cpx/acpi.c @@ -579,26 +579,61 @@ }
/* - * Ports Stack Stack(HOB) IioConfigIou - * ========================================== - * 0 CSTACK stack 0 IOU0 - * 1A..1D PSTACK0 stack 1 IOU1 - * 2A..2D PSTACK1 stack 2 IOU2 - * 3A..3D PSTACK2 stack 4 IOU3 + * This function adds PCIe bridge device entry in DMAR table. If it is called + * in the context of ATSR subtable, it adds ATSR subtable when it is first called. */ -static int get_stack_for_port(int p) +static unsigned long acpi_create_dmar_ds_pci_br_for_port(unsigned long current, + int port, int stack, IIO_RESOURCE_INSTANCE iio_resource, uint32_t pcie_seg, + bool is_atsr, bool *first) { - if (p == 0) - return CSTACK; - else if (p >= PORT_1A && p <= PORT_1D) - return PSTACK0; - else if (p >= PORT_2A && p <= PORT_2D) - return PSTACK1; - else //if (p >= PORT_3A && p <= PORT_3D) - return PSTACK2; + /* + * If the port is not part of the stack, return back. + * Ports Stack Stack(HOB) IioConfigIou + * ========================================== + * 0 CSTACK stack 0 IOU0 + * 1A..1D PSTACK0 stack 1 IOU1 + * 2A..2D PSTACK1 stack 2 IOU2 + * 3A..3D PSTACK2 stack 4 IOU3 + */ + if (port == PORT_0 && stack != CSTACK) + return 0; + else if (port >= PORT_1A && port <= PORT_1D && stack != PSTACK0) + return 0; + else if (port >= PORT_2A && port <= PORT_2D && stack != PSTACK1) + return 0; + else if (port >= PORT_3A && port <= PORT_3D && stack != PSTACK2) + return 0; + else if (port > PORT_3D) + return 0; + + 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); + if (id == 0xffffffff) + return 0; + + unsigned long atsr_size = 0; + unsigned long pci_br_size = 0; + if (is_atsr == true && *first == true) { + printk(BIOS_DEBUG, "[Root Port ATS Capability] Flags: 0x%x, " + "PCI Segment Number: 0x%x\n", 0, pcie_seg); + atsr_size = acpi_create_dmar_atsr(current, 0, pcie_seg); + *first = false; + } + + printk(BIOS_DEBUG, " [PCI Bridge Device] Enumeration ID: 0x%x, " + "PCI Bus Number: 0x%x, PCI Path: 0x%x, 0x%x\n", + 0, bus, dev, func); + pci_br_size = acpi_create_dmar_ds_pci_br(current + atsr_size, bus, dev, func); + + return (atsr_size + pci_br_size); }
-static unsigned long acpi_create_drhd(unsigned long current, int socket, int stack) +static unsigned long acpi_create_drhd(unsigned long current, int socket, + int stack, const IIO_UDS *hob) { int IoApicID[] = { // socket 0 @@ -612,12 +647,6 @@ uint32_t enum_id; unsigned long tmp = current;
- size_t hob_size; - const uint8_t fsp_hob_iio_universal_data_guid[16] = FSP_HOB_IIO_UNIVERSAL_DATA_GUID; - const IIO_UDS *hob = fsp_find_extension_hob_by_guid( - fsp_hob_iio_universal_data_guid, &hob_size); - assert(hob != NULL && hob_size != 0); - uint32_t bus = hob->PlatformData.IIO_resource[socket].StackRes[stack].BusBase; uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment; uint32_t reg_base = @@ -670,24 +699,9 @@ if (socket != 0 || stack != CSTACK) { IIO_RESOURCE_INSTANCE iio_resource = hob->PlatformData.IIO_resource[socket]; - for (int p = 0; p < NUMBER_PORTS_PER_SOCKET; ++p) { - if (get_stack_for_port(p) != stack) - continue; - - uint32_t dev = iio_resource.PcieInfo.PortInfo[p].Device; - uint32_t func = iio_resource.PcieInfo.PortInfo[p].Function; - - uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), - PCI_VENDOR_ID); - if (id == 0xffffffff) - continue; - - printk(BIOS_DEBUG, " [PCI Bridge Device] Enumeration ID: 0x%x, " - "PCI Bus Number: 0x%x, PCI Path: 0x%x, 0x%x\n", - 0, bus, dev, func); - current += acpi_create_dmar_ds_pci_br(current, - bus, dev, func); - } + for (int p = 0; p < MAX_PORTS; ++p) + current += acpi_create_dmar_ds_pci_br_for_port(current, p, stack, + iio_resource, pcie_seg, false, NULL);
// Add VMD if (hob->PlatformData.VMDStackEnable[socket][stack] && @@ -722,13 +736,8 @@ return current; }
-static unsigned long acpi_create_atsr(unsigned long current) +static unsigned long acpi_create_atsr(unsigned long current, const IIO_UDS *hob) { - size_t hob_size; - const uint8_t uds_guid[16] = FSP_HOB_IIO_UNIVERSAL_DATA_GUID; - const IIO_UDS *hob = fsp_find_extension_hob_by_guid(uds_guid, &hob_size); - assert(hob != NULL && hob_size != 0); - for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment; unsigned long tmp = current; @@ -752,32 +761,11 @@ if ((vtd_mmio_cap & 0x4) == 0) // BIT 2 continue;
- for (int p = 0; p < NUMBER_PORTS_PER_SOCKET; ++p) { + for (int p = 0; p < MAX_PORTS; ++p) { if (socket == 0 && p == 0) continue; - if (get_stack_for_port(p) != stack) - continue; - - uint32_t dev = iio_resource.PcieInfo.PortInfo[p].Device; - uint32_t func = iio_resource.PcieInfo.PortInfo[p].Function; - - u32 id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), - PCI_VENDOR_ID); - if (id == 0xffffffff) - continue; - - if (first) { - printk(BIOS_DEBUG, "[Root Port ATS Capability] Flags: 0x%x, " - "PCI Segment Number: 0x%x\n", - 0, pcie_seg); - current += acpi_create_dmar_atsr(current, 0, pcie_seg); - first = 0; - } - - printk(BIOS_DEBUG, " [PCI Bridge Device] Enumeration ID: 0x%x, " - "PCI Bus Number: 0x%x, PCI Path: 0x%x, 0x%x\n", - 0, bus, dev, func); - current += acpi_create_dmar_ds_pci_br(current, bus, dev, func); + current += acpi_create_dmar_ds_pci_br_for_port(current, p, stack, + iio_resource, pcie_seg, true, &first); } } if (tmp != current) @@ -857,20 +845,21 @@ socket = 0;
if (socket == 0) { - for (int stack = 1; stack <= PSTACK2; ++stack) - current = acpi_create_drhd(current, socket, stack); - current = acpi_create_drhd(current, socket, CSTACK); + for (int stack = 1; stack <= PSTACK2; ++stack) { + current = acpi_create_drhd(current, socket, stack, hob); + } + current = acpi_create_drhd(current, socket, CSTACK, hob); } else { for (int stack = 0; stack <= PSTACK2; ++stack) - current = acpi_create_drhd(current, socket, stack); + current = acpi_create_drhd(current, socket, stack, hob); } }
// RMRR current = acpi_create_rmrr(current);
- // ATSR - causes hang - current = acpi_create_atsr(current); + // Root Port ATS Capability + current = acpi_create_atsr(current, hob);
// RHSA current = acpi_create_rhsa(current);
build bot (Jenkins) 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 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45376/1/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45376/1/src/soc/intel/xeon_sp/cpx/a... PS1, Line 767: current += acpi_create_dmar_ds_pci_br_for_port(current, p, stack, line over 96 characters
https://review.coreboot.org/c/coreboot/+/45376/1/src/soc/intel/xeon_sp/cpx/a... PS1, Line 848: for (int stack = 1; stack <= PSTACK2; ++stack) { braces {} are not necessary for single statement blocks
Hello Marc Jones, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45376
to look at the new patch set (#2).
Change subject: soc/intel/xeon_sp/cpx: find IIO_UDS HOB once when creating DMAR table ......................................................................
soc/intel/xeon_sp/cpx: find IIO_UDS HOB once when creating DMAR table
IIO_UDS HOB was found several times during the creation of DMAR table. Reduce it to only once to improve boot time.
Both DRHD and ATSR subtable creations involve addition of PCIe bridge device entries, combine the functions with acpi_create_dmar_ds_pci_br_for_port().
When looping through ports to create PCIe bridge device entries, use MAX_PORTS intead of NUMBER_PORTS_PER_SOCKET to improve boot time.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I469cd8473c50e105daeda6c5607592ae7cef6032 --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 63 insertions(+), 75 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/45376/2
Angel Pons 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: Code-Review+1
(2 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:
if (stack < 0 || get_stack_for_port(port) != stack) return 0;
Since `stack` is an int, `get_stack_for_port` could return -1 for unhandled ports.
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?
Marc Jones 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:
(6 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 608: This is one test and could only have a single return point or keep the get stack for port function as Angel noted above.
if (port == 0 ...) || (port >= 2a ...) || .... return 0
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?
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 620: true This derefrences a nullptr. You need to check it for NULL. Maybe use a different way to track "first".
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
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
https://review.coreboot.org/c/coreboot/+/45376/2/src/soc/intel/xeon_sp/cpx/a... PS2, Line 765: 0 PSTACK0
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
Hello Marc Jones, build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45376
to look at the new patch set (#3).
Change subject: soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table ......................................................................
soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table
IIO_UDS HOB was searched several times during the creation of DMAR table. Reduce it to only once to improve boot time.
Both DRHD and ATSR subtable creations involve addition of PCIe bridge device entries, combine the functions with acpi_create_dmar_ds_pci_br_for_port().
When looping through ports to create PCIe bridge device entries, use MAX_PORTS intead of NUMBER_PORTS_PER_SOCKET to improve boot time.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I469cd8473c50e105daeda6c5607592ae7cef6032 --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 60 insertions(+), 66 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/45376/3
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table ......................................................................
Patch Set 3:
(1 comment)
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 620: true
This derefrences a nullptr. You need to check it for NULL. […]
If is_atsr is true, then first is not NULL. Otherwise first can be NULL since && operator guarantees the second test is not executed. That being said, I added check for NULL.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table ......................................................................
Patch Set 3: Code-Review+1
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table ......................................................................
Patch Set 3: Code-Review+2
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table ......................................................................
Patch Set 3:
(1 comment)
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 615: 0xffffffff
There are multiple possible DEVIDs, I am fine with either way though. […]
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table ......................................................................
soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table
IIO_UDS HOB was searched several times during the creation of DMAR table. Reduce it to only once to improve boot time.
Both DRHD and ATSR subtable creations involve addition of PCIe bridge device entries, combine the functions with acpi_create_dmar_ds_pci_br_for_port().
When looping through ports to create PCIe bridge device entries, use MAX_PORTS intead of NUMBER_PORTS_PER_SOCKET to improve boot time.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I469cd8473c50e105daeda6c5607592ae7cef6032 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45376 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Marc Jones marc@marcjonesconsulting.com --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 60 insertions(+), 66 deletions(-)
Approvals: build bot (Jenkins): Verified Marc Jones: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c index a40d33d..cd497c5 100644 --- a/src/soc/intel/xeon_sp/cpx/acpi.c +++ b/src/soc/intel/xeon_sp/cpx/acpi.c @@ -586,19 +586,60 @@ * 2A..2D PSTACK1 stack 2 IOU2 * 3A..3D PSTACK2 stack 4 IOU3 */ -static int get_stack_for_port(int p) +static int get_stack_for_port(int port) { - if (p == 0) + if (port == PORT_0) return CSTACK; - else if (p >= PORT_1A && p <= PORT_1D) + else if (port >= PORT_1A && port <= PORT_1D) return PSTACK0; - else if (p >= PORT_2A && p <= PORT_2D) + else if (port >= PORT_2A && port <= PORT_2D) return PSTACK1; - else //if (p >= PORT_3A && p <= PORT_3D) + else if (port >= PORT_3A && port <= PORT_3D) return PSTACK2; + else + return -1; }
-static unsigned long acpi_create_drhd(unsigned long current, int socket, int stack) +/* + * This function adds PCIe bridge device entry in DMAR table. If it is called + * in the context of ATSR subtable, it adds ATSR subtable when it is first called. + */ +static unsigned long acpi_create_dmar_ds_pci_br_for_port(unsigned long current, + int port, int stack, IIO_RESOURCE_INSTANCE iio_resource, uint32_t pcie_seg, + bool is_atsr, bool *first) +{ + + if (get_stack_for_port(port) != stack) + return 0; + + const uint32_t bus = iio_resource.StackRes[stack].BusBase; + const uint32_t dev = iio_resource.PcieInfo.PortInfo[port].Device; + const uint32_t func = iio_resource.PcieInfo.PortInfo[port].Function; + + const uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), + PCI_VENDOR_ID); + if (id == 0xffffffff) + return 0; + + unsigned long atsr_size = 0; + unsigned long pci_br_size = 0; + if (is_atsr == true && first && *first == true) { + printk(BIOS_DEBUG, "[Root Port ATS Capability] Flags: 0x%x, " + "PCI Segment Number: 0x%x\n", 0, pcie_seg); + atsr_size = acpi_create_dmar_atsr(current, 0, pcie_seg); + *first = false; + } + + printk(BIOS_DEBUG, " [PCI Bridge Device] Enumeration ID: 0x%x, " + "PCI Bus Number: 0x%x, PCI Path: 0x%x, 0x%x\n", + 0, bus, dev, func); + pci_br_size = acpi_create_dmar_ds_pci_br(current + atsr_size, bus, dev, func); + + return (atsr_size + pci_br_size); +} + +static unsigned long acpi_create_drhd(unsigned long current, int socket, + int stack, const IIO_UDS *hob) { int IoApicID[] = { // socket 0 @@ -612,12 +653,6 @@ uint32_t enum_id; unsigned long tmp = current;
- size_t hob_size; - const uint8_t fsp_hob_iio_universal_data_guid[16] = FSP_HOB_IIO_UNIVERSAL_DATA_GUID; - const IIO_UDS *hob = fsp_find_extension_hob_by_guid( - fsp_hob_iio_universal_data_guid, &hob_size); - assert(hob != NULL && hob_size != 0); - uint32_t bus = hob->PlatformData.IIO_resource[socket].StackRes[stack].BusBase; uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment; uint32_t reg_base = @@ -670,24 +705,9 @@ if (socket != 0 || stack != CSTACK) { IIO_RESOURCE_INSTANCE iio_resource = hob->PlatformData.IIO_resource[socket]; - for (int p = 0; p < NUMBER_PORTS_PER_SOCKET; ++p) { - if (get_stack_for_port(p) != stack) - continue; - - uint32_t dev = iio_resource.PcieInfo.PortInfo[p].Device; - uint32_t func = iio_resource.PcieInfo.PortInfo[p].Function; - - uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), - PCI_VENDOR_ID); - if (id == 0xffffffff) - continue; - - printk(BIOS_DEBUG, " [PCI Bridge Device] Enumeration ID: 0x%x, " - "PCI Bus Number: 0x%x, PCI Path: 0x%x, 0x%x\n", - 0, bus, dev, func); - current += acpi_create_dmar_ds_pci_br(current, - bus, dev, func); - } + for (int p = PORT_0; p < MAX_PORTS; ++p) + current += acpi_create_dmar_ds_pci_br_for_port(current, p, stack, + iio_resource, pcie_seg, false, NULL);
// Add VMD if (hob->PlatformData.VMDStackEnable[socket][stack] && @@ -722,13 +742,8 @@ return current; }
-static unsigned long acpi_create_atsr(unsigned long current) +static unsigned long acpi_create_atsr(unsigned long current, const IIO_UDS *hob) { - size_t hob_size; - const uint8_t uds_guid[16] = FSP_HOB_IIO_UNIVERSAL_DATA_GUID; - const IIO_UDS *hob = fsp_find_extension_hob_by_guid(uds_guid, &hob_size); - assert(hob != NULL && hob_size != 0); - for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment; unsigned long tmp = current; @@ -752,32 +767,11 @@ if ((vtd_mmio_cap & 0x4) == 0) // BIT 2 continue;
- for (int p = 0; p < NUMBER_PORTS_PER_SOCKET; ++p) { - if (socket == 0 && p == 0) + for (int p = PORT_0; p < MAX_PORTS; ++p) { + if (socket == 0 && p == PORT_0) continue; - if (get_stack_for_port(p) != stack) - continue; - - uint32_t dev = iio_resource.PcieInfo.PortInfo[p].Device; - uint32_t func = iio_resource.PcieInfo.PortInfo[p].Function; - - u32 id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), - PCI_VENDOR_ID); - if (id == 0xffffffff) - continue; - - if (first) { - printk(BIOS_DEBUG, "[Root Port ATS Capability] Flags: 0x%x, " - "PCI Segment Number: 0x%x\n", - 0, pcie_seg); - current += acpi_create_dmar_atsr(current, 0, pcie_seg); - first = 0; - } - - printk(BIOS_DEBUG, " [PCI Bridge Device] Enumeration ID: 0x%x, " - "PCI Bus Number: 0x%x, PCI Path: 0x%x, 0x%x\n", - 0, bus, dev, func); - current += acpi_create_dmar_ds_pci_br(current, bus, dev, func); + current += acpi_create_dmar_ds_pci_br_for_port(current, p, + stack, iio_resource, pcie_seg, true, &first); } } if (tmp != current) @@ -858,19 +852,19 @@
if (socket == 0) { for (int stack = 1; stack <= PSTACK2; ++stack) - current = acpi_create_drhd(current, socket, stack); - current = acpi_create_drhd(current, socket, CSTACK); + current = acpi_create_drhd(current, socket, stack, hob); + current = acpi_create_drhd(current, socket, CSTACK, hob); } else { for (int stack = 0; stack <= PSTACK2; ++stack) - current = acpi_create_drhd(current, socket, stack); + current = acpi_create_drhd(current, socket, stack, hob); } }
// RMRR current = acpi_create_rmrr(current);
- // ATSR - causes hang - current = acpi_create_atsr(current); + // Root Port ATS Capability + current = acpi_create_atsr(current, hob);
// RHSA current = acpi_create_rhsa(current);
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45376 )
Change subject: soc/intel/xeon_sp/cpx: search IIO_UDS HOB once when creating DMAR table ......................................................................
Patch Set 4:
Automatic boot test returned (PASS/FAIL/TOTAL): 8/1/9 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/20274 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/20273 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/20272 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/20271 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/20270 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/20278 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/20277 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/20276 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/20275
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
build bot (Jenkins) (Code Review)
-
Jonathan Zhang (Code Review)
-
Marc Jones (Code Review)
-
Patrick Georgi (Code Review)