Change in coreboot[master]: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack
Jonathan Zhang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack
This is a temporary patch to unblock test/release. It will be replaced by following patches: * A patch to correct ioapic GSIs. * A patch to skip DRHD generation for non-PCIe stack (this can only be done when the IPS ticket is resolved).
With the patch to correct ioapic GSIs, there are following target OS boot errors: [ 1.098771] IOAPIC[0]: apic_id 8, version 32, address 0xfec00000, GSI 0-119 [ 1.099159] GSI range [24-31] for new IOAPIC conflicts with GSI[0-119]
With the patch, these are the messages: [ 0.399498] IOAPIC[0]: apic_id 8, version 32, address 0xfec00000, GSI 0-119 [ 0.399848] IOAPIC[1]: apic_id 9, version 32, address 0xfec01000, GSI 120-127
I also suspect this causes the reboot instability issue. During some reboots (like once every 30 reboots), following failures happen: [ 4.325795] mce: [Hardware Error]: Machine check events logged [ 4.326597] mce: [Hardware Error]: CPU 0: Machine Check: 0 Bank 9: ee2000000003110a [ 4.327594] mce: [Hardware Error]: TSC 0 ADDR fe9e0000 MISC 228aa040101086 [ 4.328596] mce: [Hardware Error]: PROCESSOR 0:5065b TIME 1601443875 SOCKET 0 APIC 0 microcode 700001d
The MCE errors is happen in bank 9, 10 and 11. The Model specific error code shows it is about SAD_ERR_WB_TO_MMIO error (doc 604926), which means something goes wrong when cache write back to mmio. It is a generic transaction type error in level 2.
Since the error happens during smpboot of PBSP, I suspect this is an issue related to ioapic, hence found the issue with MADT table.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I098605daf12a264f390613581427ec722afcddaf --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 19 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45887/1
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c index cd497c5..8792131 100644 --- a/src/soc/intel/xeon_sp/cpx/acpi.c +++ b/src/soc/intel/xeon_sp/cpx/acpi.c @@ -187,7 +187,7 @@ int cur_index; struct iiostack_resource stack_info = {0};
- int gsi_bases[] = { 0, 0x18, 0x20, 0x28, 0x30, 0x48, 0x50, 0x58, 0x60 }; + int gsi_bases[] = { 0, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0 }; int ioapic_ids[] = { 0x8, 0x9, 0xa, 0xb, 0xc, 0xf, 0x10, 0x11, 0x12 };
/* Local APICs */ @@ -612,9 +612,20 @@ 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; + 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; + + /* TODO: remove this workaround when the IPS ticket is resolved. */ + if (port == PORT_1A || port == PORT_2A || port == PORT_3A) + dev = 0x0; + else if (port == PORT_1B || port == PORT_2B || port == PORT_3B) + dev = 0x1; + else if (port == PORT_1C || port == PORT_2C || port == PORT_3C) + dev = 0x2; + else if (port == PORT_1D || port == PORT_2D || port == PORT_3D) + dev = 0x3; + func = 0x0;
const uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), PCI_VENDOR_ID); @@ -660,6 +671,10 @@ printk(BIOS_SPEW, "%s socket: %d, stack: %d, bus: 0x%x, pcie_seg: 0x%x, reg_base: 0x%x\n", __func__, socket, stack, bus, pcie_seg, reg_base);
+ /* Do not generate DRHD for non-PCIe stack */ + if (reg_base == 0x0) + return current; + // Add DRHD Hardware Unit if (socket == 0 && stack == CSTACK) { printk(BIOS_DEBUG, "[Hardware Unit Definition] Flags: 0x%x, PCI Segment Number: 0x%x, "
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
Patch Set 2:
This change is ready for review.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45887/3/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45887/3/src/soc/intel/xeon_sp/cpx/a... PS3, Line 667: // Add DRHD Hardware Unit : if (socket == 0 && stack == CSTACK) { : printk(BIOS_DEBUG, "[Hardware Unit Definition] Flags: 0x%x, PCI Segment Number: 0x%x, " : "Register Base Address: 0x%x\n", : DRHD_INCLUDE_PCI_ALL, pcie_seg, reg_base); : current += acpi_create_dmar_drhd(current, DRHD_INCLUDE_PCI_ALL, : pcie_seg, reg_base); : } else { : printk(BIOS_DEBUG, "[Hardware Unit Definition] Flags: 0x%x, PCI Segment Number: 0x%x, " : "Register Base Address: 0x%x\n", 0, pcie_seg, reg_base); : current += acpi_create_dmar_drhd(current, 0, pcie_seg, reg_base); : } This is the only place where `reg_base` is used. Is it OK to skip generating the other IOAPIC entries?
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45887/3/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45887/3/src/soc/intel/xeon_sp/cpx/a... PS3, Line 667: // Add DRHD Hardware Unit : if (socket == 0 && stack == CSTACK) { : printk(BIOS_DEBUG, "[Hardware Unit Definition] Flags: 0x%x, PCI Segment Number: 0x%x, " : "Register Base Address: 0x%x\n", : DRHD_INCLUDE_PCI_ALL, pcie_seg, reg_base); : current += acpi_create_dmar_drhd(current, DRHD_INCLUDE_PCI_ALL, : pcie_seg, reg_base); : } else { : printk(BIOS_DEBUG, "[Hardware Unit Definition] Flags: 0x%x, PCI Segment Number: 0x%x, " : "Register Base Address: 0x%x\n", 0, pcie_seg, reg_base); : current += acpi_create_dmar_drhd(current, 0, pcie_seg, reg_base); : }
This is the only place where `reg_base` is used. […]
Yes. When reg_base is 0x0, this stack is not really a PSTACK (or CSTACK), it needs to be skipped during DMAR generation. In CPX-SP, stack 3 is such stack; the mapping is shown in src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_iiouds.h, search for "IIO Stacks".
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Johnny Lin, Jingle Hsu, Angel Pons, Morgan Jang, Bryant Ou, Patrick Rudolph, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45887
to look at the new patch set (#4).
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack
Without skipping of DRHD generation for non-PCIe stack, the OS kernel detects incorrect DMAR table with following messages: [ 0.561817] Your BIOS is broken; DMAR reported at address 0
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I098605daf12a264f390613581427ec722afcddaf --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45887/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
Patch Set 4: Code-Review+2
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Johnny Lin, Jingle Hsu, Angel Pons, Morgan Jang, Bryant Ou, Patrick Rudolph, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45887
to look at the new patch set (#7).
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack
Without skipping of DRHD generation for non-PCIe stack, the OS kernel detects incorrect DMAR table with following messages: [ 0.561817] Your BIOS is broken; DMAR reported at address 0
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I098605daf12a264f390613581427ec722afcddaf --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45887/7
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
Patch Set 7: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
Patch Set 7:
This patch could use a rebase. It is currently held by a merge conflict.
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Johnny Lin, Jingle Hsu, Angel Pons, Morgan Jang, Bryant Ou, Patrick Rudolph, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45887
to look at the new patch set (#8).
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack
Without skipping of DRHD generation for non-PCIe stack, the OS kernel detects incorrect DMAR table with following messages: [ 0.561817] Your BIOS is broken; DMAR reported at address 0
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I098605daf12a264f390613581427ec722afcddaf --- M src/soc/intel/xeon_sp/cpx/soc_acpi.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45887/8
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
Patch Set 8: Code-Review+2
Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45887 )
Change subject: soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack ......................................................................
soc/intel/xeon_sp/cpx: skip DRHD generation for non-PCIe stack
Without skipping of DRHD generation for non-PCIe stack, the OS kernel detects incorrect DMAR table with following messages: [ 0.561817] Your BIOS is broken; DMAR reported at address 0
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I098605daf12a264f390613581427ec722afcddaf Reviewed-on: https://review.coreboot.org/c/coreboot/+/45887 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/xeon_sp/cpx/soc_acpi.c 1 file changed, 4 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/cpx/soc_acpi.c b/src/soc/intel/xeon_sp/cpx/soc_acpi.c index b0352d8..a9af4aa 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/cpx/soc_acpi.c @@ -457,6 +457,10 @@ printk(BIOS_SPEW, "%s socket: %d, stack: %d, bus: 0x%x, pcie_seg: 0x%x, reg_base: 0x%x\n", __func__, socket, stack, bus, pcie_seg, reg_base);
+ /* Do not generate DRHD for non-PCIe stack */ + if (!reg_base) + return current; + // Add DRHD Hardware Unit if (socket == 0 && stack == CSTACK) { printk(BIOS_DEBUG, "[Hardware Unit Definition] Flags: 0x%x, PCI Segment Number: 0x%x, "
-
Angel Pons (Code Review) -
Jonathan Zhang (Code Review)
-
Philipp Deppenwiese (Code Review)