Change in coreboot[master]: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46503 )

Change subject: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR
......................................................................

soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR

In MADT loop over each stack and compare it against the same IO APIC
ID table as during the DMAR table generation.

This fixes Linux (5.8) warning:
"DMAR-IR: [Firmware Bug]: ioapic 12 has no mapping iommu, interrupt
remapping will be disabled"

Change-Id: I32caf9036f0d548520b30b82324265cd4d0c3b8f
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/xeon_sp/cpx/acpi.c
1 file changed, 42 insertions(+), 35 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/46503/1

diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c
index a1ebc4a..c7172b2 100644
--- a/src/soc/intel/xeon_sp/cpx/acpi.c
+++ b/src/soc/intel/xeon_sp/cpx/acpi.c
@@ -78,49 +78,56 @@

unsigned long acpi_fill_madt(unsigned long current)
 {
-	int cur_index;
-	struct iiostack_resource stack_info = {0};
+	int cur_index = 0;
+	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);

/* With CPX-SP FSP, PCH IOAPIC is allocated with first 120 GSIs. */
-	int gsi_bases[] = { 0, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0 };
-	int ioapic_ids[] = { 0x8, 0x9, 0xa, 0xb, 0xc, 0xf, 0x10, 0x11, 0x12 };
+	const int gsi_bases[] = { 0, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0 };
+	const int IoApicID[] = {
+		// socket 0
+		PC00_IOAPIC_ID, PC01_IOAPIC_ID, PC02_IOAPIC_ID, PC03_IOAPIC_ID,
+		PC04_IOAPIC_ID, PC05_IOAPIC_ID,
+		// socket 1
+		PC06_IOAPIC_ID, PC07_IOAPIC_ID, PC08_IOAPIC_ID, PC09_IOAPIC_ID,
+		PC10_IOAPIC_ID, PC11_IOAPIC_ID,
+	};
+
+	assert(hob != NULL && hob_size != 0);

/* Local APICs */
    current = xeonsp_acpi_create_madt_lapics(current);

-	cur_index = 0;
-	get_iiostack_info(&stack_info);
+	/*
+	 * Stack 0 has non-PCH IOAPIC and PCH IOAPIC.
+	 * Add entry for PCH IOAPIC.
+	 */
+	{
+		const STACK_RES *ri = &hob->PlatformData.IIO_resource[0].StackRes[0];
+		const int gsi_base = gsi_bases[cur_index];
+		printk(BIOS_DEBUG, "Adding MADT IOAPIC for PCH: ioapic_id: 0x%x, "
+		       "ioapic_base: 0x%x, gsi_base: 0x%x\n",
+		       PCH_IOAPIC_ID, ri->IoApicBase, gsi_base);
+		current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, PCH_IOAPIC_ID,
+					   ri->IoApicBase, gsi_base);
+		cur_index++;
+	}

-	for (int stack = 0; stack < stack_info.no_of_stacks; ++stack) {
-		const STACK_RES *ri = &stack_info.res[stack];
-		assert(cur_index < ARRAY_SIZE(ioapic_ids));
-		assert(cur_index < ARRAY_SIZE(gsi_bases));
-		int ioapic_id = ioapic_ids[cur_index];
-		int gsi_base = gsi_bases[cur_index];
-		printk(BIOS_DEBUG, "Adding MADT IOAPIC for stack: %d, ioapic_id: 0x%x, "
-			"ioapic_base: 0x%x, gsi_base: 0x%x\n",
-			stack,  ioapic_id, ri->IoApicBase, gsi_base);
-		current += acpi_create_madt_ioapic(
-			(acpi_madt_ioapic_t *)current,
-			ioapic_id, ri->IoApicBase, gsi_base);
-		++cur_index;

-		/*
-		 * Stack 0 has non-PCH IOAPIC and PCH IOAPIC.
-		 * Add entry for PCH IOAPIC.
-		 */
-		if (stack == 0) { /* PCH IOAPIC */
-			assert(cur_index < ARRAY_SIZE(ioapic_ids));
-			assert(cur_index < ARRAY_SIZE(gsi_bases));
-			ioapic_id = ioapic_ids[cur_index];
-			gsi_base = gsi_bases[cur_index];
-			printk(BIOS_DEBUG, "Adding MADT IOAPIC for stack: %d, ioapic_id: 0x%x, "
-				"ioapic_base: 0x%x, gsi_base: 0x%x\n",
-				stack,  ioapic_id,
-				ri->IoApicBase + 0x1000, gsi_base);
-			current += acpi_create_madt_ioapic(
-				(acpi_madt_ioapic_t *)current,
-				ioapic_id, ri->IoApicBase + 0x1000, gsi_base);
+	// DRHD
+	for (int iio = 0; iio < hob->PlatformData.numofIIO; ++iio) {
+		for (int stack = 0; stack < MAX_IIO_STACK; stack++) {
+			const STACK_RES *ri = &hob->PlatformData.IIO_resource[iio].StackRes[stack];
+			if (ri->Personality != TYPE_UBOX_IIO)
+				continue;
+			const int ioapic_id = IoApicID[(iio * MAX_IIO_STACK) + stack];
+			const int gsi_base = gsi_bases[cur_index];
+			uint32_t apic_base = ri->IoApicBase;
+			if (iio == 0 && stack == 0)
+				apic_base += 0x1000;
+			current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current,
+							   ioapic_id, apic_base, gsi_base);
    		++cur_index;
    	}
    }

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46503 )

Change subject: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46503/1/src/soc/intel/xeon_sp/cpx/a... 
File src/soc/intel/xeon_sp/cpx/acpi.c:

https://review.coreboot.org/c/coreboot/+/46503/1/src/soc/intel/xeon_sp/cpx/a... 
PS1, Line 121: 			const STACK_RES *ri = &hob->PlatformData.IIO_resource[iio].StackRes[stack];
line over 96 characters

[Arthur Heymans (Code Review)]

Hello Jonathan Zhang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46503

to look at the new patch set (#2).

Change subject: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR
......................................................................

soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR

In MADT generation loop over each stack and compare it against the
same ACPI IOAPIC ID table as during the DMAR table generation.

This fixes Linux (5.8) warning:
"DMAR-IR: [Firmware Bug]: ioapic 12 has no mapping iommu, interrupt
remapping will be disabled"

Change-Id: I32caf9036f0d548520b30b82324265cd4d0c3b8f
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/xeon_sp/cpx/acpi.c
1 file changed, 42 insertions(+), 35 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/46503/2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46503 )

Change subject: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR
......................................................................

Patch Set 2: Code-Review+1

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46503 )

Change subject: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR
......................................................................

Patch Set 2:

(2 comments)

Thanks for fixing this, Arthur.
I added some minor comments.

https://review.coreboot.org/c/coreboot/+/46503/2/src/soc/intel/xeon_sp/cpx/a... 
File src/soc/intel/xeon_sp/cpx/acpi.c:

https://review.coreboot.org/c/coreboot/+/46503/2/src/soc/intel/xeon_sp/cpx/a... 
PS2, Line 87: 	const int gsi_bases[] = { 0, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0 };
It would be good to increase gsi_bases[] to match the number of IOAPICs, to take care of 2 socket case.

https://review.coreboot.org/c/coreboot/+/46503/2/src/soc/intel/xeon_sp/cpx/a... 
PS2, Line 129: 			current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current,
It would be good to add a print statement here to facilitate debugging.

[Rocky Phagura (Code Review)]

Attention is currently required from: Arthur Heymans.
Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46503 )

Change subject: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR
......................................................................

Patch Set 2:

(1 comment)

Patchset:

PS2: 
Arthur,
Any updates to this patch? Thx

[Arthur Heymans (Code Review)]

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/46503 )

Change subject: soc/intel/xeon_sp/cpx/acpi: Align MADT IO APIC ID's with DMAR
......................................................................

Abandoned

This was moved and is fixed.