Change in coreboot[master]: soc/intel/xeon_sp/acpi.c Look over HOB stack for MADT generation
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47300 )
Change subject: soc/intel/xeon_sp/acpi.c Look over HOB stack for MADT generation ......................................................................
soc/intel/xeon_sp/acpi.c Look over HOB stack for MADT generation
To align MADT generation with DMAR, we loop over HOB entries instead of over copied HOB entries fetched from get_iio_stacks(). This makes it easier to see what is going on.
Tested on ocp/deltalake
Change-Id: I8ffe0322bb182b7ec5887354ec801e1f9f3d3288 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/xeon_sp/acpi.c 1 file changed, 26 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/47300/1
diff --git a/src/soc/intel/xeon_sp/acpi.c b/src/soc/intel/xeon_sp/acpi.c index b7e20d4..fcd5f00 100644 --- a/src/soc/intel/xeon_sp/acpi.c +++ b/src/soc/intel/xeon_sp/acpi.c @@ -72,12 +72,12 @@ return current; }
-static unsigned long add_madt_ioapic(unsigned long current, int stack, int ioapic_id, +static unsigned long add_madt_ioapic(unsigned long current, int socket, int stack, int ioapic_id, uint32_t ioapic_base, int gsi_base) { - printk(BIOS_DEBUG, "Adding MADT IOAPIC for stack: %d, ioapic_id: 0x%x, " + printk(BIOS_DEBUG, "Adding MADT IOAPIC for socket: %d, stack: %d, ioapic_id: 0x%x, " "ioapic_base: 0x%x, gsi_base: 0x%x\n", - stack, ioapic_id, ioapic_base, gsi_base); + socket, stack, ioapic_id, ioapic_base, gsi_base); return acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, ioapic_id, ioapic_base, gsi_base); } @@ -85,7 +85,7 @@ unsigned long acpi_fill_madt(unsigned long current) { int cur_index; - struct iiostack_resource stack_info = {0}; + const IIO_UDS *hob = get_iio_uds();
/* With XEON-SP FSP, PCH IOAPIC is allocated with first 120 GSIs. */ #if (CONFIG(SOC_INTEL_COOPERLAKE_SP)) @@ -101,30 +101,32 @@ current = xeonsp_acpi_create_madt_lapics(current);
cur_index = 0; - get_iiostack_info(&stack_info);
- 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]; - current += add_madt_ioapic(current, stack, 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 */ + for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { + for (int stack = 0; stack < MAX_IIO_STACK; ++stack) { + const STACK_RES *ri = &hob->PlatformData.IIO_resource[socket].StackRes[stack]; + if (ri->Personality != TYPE_UBOX_IIO) + continue; 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]; - current += add_madt_ioapic(current, stack, ioapic_id, - ri->IoApicBase + 0x1000, gsi_base); + int ioapic_id = ioapic_ids[cur_index]; + int gsi_base = gsi_bases[cur_index]; + current += add_madt_ioapic(current, socket, stack, 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 && socket == 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]; + current += add_madt_ioapic(current, socket, stack, ioapic_id, + ri->IoApicBase + 0x1000, gsi_base); + ++cur_index; + } } }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47300 )
Change subject: soc/intel/xeon_sp/acpi.c Look over HOB stack for MADT generation ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47300/1/src/soc/intel/xeon_sp/acpi.... File src/soc/intel/xeon_sp/acpi.c:
https://review.coreboot.org/c/coreboot/+/47300/1/src/soc/intel/xeon_sp/acpi.... PS1, Line 75: static unsigned long add_madt_ioapic(unsigned long current, int socket, int stack, int ioapic_id, line over 96 characters
https://review.coreboot.org/c/coreboot/+/47300/1/src/soc/intel/xeon_sp/acpi.... PS1, Line 107: const STACK_RES *ri = &hob->PlatformData.IIO_resource[socket].StackRes[stack]; line over 96 characters
https://review.coreboot.org/c/coreboot/+/47300/1/src/soc/intel/xeon_sp/acpi.... PS1, Line 114: current += add_madt_ioapic(current, socket, stack, ioapic_id, ri->IoApicBase, gsi_base); line over 96 characters
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47300 )
Change subject: soc/intel/xeon_sp/acpi.c Look over HOB stack for MADT generation ......................................................................
Patch Set 1: Code-Review+2
Hello Marc Jones, build bot (Jenkins), Jonathan Zhang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47300
to look at the new patch set (#2).
Change subject: soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation ......................................................................
soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation
To align MADT generation with DMAR, we loop over HOB entries instead of over copied HOB entries fetched from get_iio_stacks(). This makes it easier to see what is going on.
Tested on ocp/deltalake
Change-Id: I8ffe0322bb182b7ec5887354ec801e1f9f3d3288 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/xeon_sp/acpi.c 1 file changed, 29 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/47300/2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47300 )
Change subject: soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47300/3/src/soc/intel/xeon_sp/acpi.... File src/soc/intel/xeon_sp/acpi.c:
https://review.coreboot.org/c/coreboot/+/47300/3/src/soc/intel/xeon_sp/acpi.... PS3, Line 110: TYPE_UBOX_IIO looks like this is not defined on skx. Is just looping over entries where the IoApicBase != 0 ok?
Hello build bot (Jenkins), Marc Jones, Jonathan Zhang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47300
to look at the new patch set (#5).
Change subject: soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation ......................................................................
soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation
To align MADT generation with DMAR, we loop over HOB entries instead of over copied HOB entries fetched from get_iio_stacks(). This makes it easier to see what is going on.
Tested on ocp/deltalake
Change-Id: I8ffe0322bb182b7ec5887354ec801e1f9f3d3288 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/xeon_sp/acpi.c 1 file changed, 29 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/47300/5
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47300 )
Change subject: soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation ......................................................................
Patch Set 6: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47300 )
Change subject: soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47300/3/src/soc/intel/xeon_sp/acpi.... File src/soc/intel/xeon_sp/acpi.c:
https://review.coreboot.org/c/coreboot/+/47300/3/src/soc/intel/xeon_sp/acpi.... PS3, Line 110: TYPE_UBOX_IIO
looks like this is not defined on skx. […]
Done
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47300 )
Change subject: soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation ......................................................................
soc/intel/xeon_sp/acpi.c Loop over HOB stack for MADT generation
To align MADT generation with DMAR, we loop over HOB entries instead of over copied HOB entries fetched from get_iio_stacks(). This makes it easier to see what is going on.
Tested on ocp/deltalake
Change-Id: I8ffe0322bb182b7ec5887354ec801e1f9f3d3288 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/47300 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marc Jones marc@marcjonesconsulting.com --- M src/soc/intel/xeon_sp/acpi.c 1 file changed, 29 insertions(+), 25 deletions(-)
Approvals: build bot (Jenkins): Verified Marc Jones: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/acpi.c b/src/soc/intel/xeon_sp/acpi.c index 5a77bf3..f747f87 100644 --- a/src/soc/intel/xeon_sp/acpi.c +++ b/src/soc/intel/xeon_sp/acpi.c @@ -73,12 +73,12 @@ return current; }
-static unsigned long add_madt_ioapic(unsigned long current, int stack, int ioapic_id, - uint32_t ioapic_base, int gsi_base) +static unsigned long add_madt_ioapic(unsigned long current, int socket, int stack, + int ioapic_id, uint32_t ioapic_base, int gsi_base) { - printk(BIOS_DEBUG, "Adding MADT IOAPIC for stack: %d, ioapic_id: 0x%x, " + printk(BIOS_DEBUG, "Adding MADT IOAPIC for socket: %d, stack: %d, ioapic_id: 0x%x, " "ioapic_base: 0x%x, gsi_base: 0x%x\n", - stack, ioapic_id, ioapic_base, gsi_base); + socket, stack, ioapic_id, ioapic_base, gsi_base); return acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, ioapic_id, ioapic_base, gsi_base); } @@ -86,7 +86,7 @@ unsigned long acpi_fill_madt(unsigned long current) { int cur_index; - struct iiostack_resource stack_info = {0}; + const IIO_UDS *hob = get_iio_uds();
/* With XEON-SP FSP, PCH IOAPIC is allocated with first 120 GSIs. */ #if (CONFIG(SOC_INTEL_COOPERLAKE_SP)) @@ -102,30 +102,34 @@ current = xeonsp_acpi_create_madt_lapics(current);
cur_index = 0; - get_iiostack_info(&stack_info);
- 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]; - current += add_madt_ioapic(current, stack, 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 */ + for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { + for (int stack = 0; stack < MAX_IIO_STACK; ++stack) { + const STACK_RES *ri = + &hob->PlatformData.IIO_resource[socket].StackRes[stack]; + if (!is_iio_stack_res(ri)) + continue; 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]; - current += add_madt_ioapic(current, stack, ioapic_id, - ri->IoApicBase + 0x1000, gsi_base); + int ioapic_id = ioapic_ids[cur_index]; + int gsi_base = gsi_bases[cur_index]; + current += add_madt_ioapic(current, socket, stack, 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 && socket == 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]; + current += add_madt_ioapic(current, socket, stack, ioapic_id, + ri->IoApicBase + 0x1000, gsi_base); + ++cur_index; + } } }
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
Marc Jones (Code Review)