Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Tim Chu.
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76559?usp=email )
Change subject: soc/intel/xeon_sp: Skip empty sockets ......................................................................
soc/intel/xeon_sp: Skip empty sockets
The current SaphireRappids code assumes that all sockets have working CPUs. On multi-socket platforms a CPU might be missing or was disabled due to an error. The variable PlatformData.numofIIO and the variable SystemStatus.numCpus reflect the working CPUs, but not the actual socket count.
Update the code to iterate over sockets until PlatformData.numofIIO IIOs have been found. This is required as FSP doesn't sort IIOs by working/non working status. This resolves invalid ACPI table generation and it fixes a crash as commands were sent to a disabled CPU.
TEST: Disabled Socket1 on IBM/SBP1.
Change-Id: I237b6392764bbdb3b96013f577a10a4394ba9c6e Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/xeon_sp/acpi.c M src/soc/intel/xeon_sp/cpx/chip.c M src/soc/intel/xeon_sp/cpx/soc_util.c M src/soc/intel/xeon_sp/memmap.c M src/soc/intel/xeon_sp/skx/soc_acpi.c M src/soc/intel/xeon_sp/spr/chip.c M src/soc/intel/xeon_sp/spr/numa.c M src/soc/intel/xeon_sp/spr/soc_acpi.c M src/soc/intel/xeon_sp/spr/soc_util.c M src/soc/intel/xeon_sp/uncore_acpi.c M src/soc/intel/xeon_sp/util.c 11 files changed, 59 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/76559/1
diff --git a/src/soc/intel/xeon_sp/acpi.c b/src/soc/intel/xeon_sp/acpi.c index b452ab3..519d6c7 100644 --- a/src/soc/intel/xeon_sp/acpi.c +++ b/src/soc/intel/xeon_sp/acpi.c @@ -97,7 +97,10 @@
*ioapic_bases = xeonsp_ioapic_bases;
- for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { + for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; socket++) { + if (!soc_cpu_is_enabled(socket)) + continue; + iio++; for (int stack = 0; stack < MAX_IIO_STACK; ++stack) { const STACK_RES *ri = &hob->PlatformData.IIO_resource[socket].StackRes[stack]; diff --git a/src/soc/intel/xeon_sp/cpx/chip.c b/src/soc/intel/xeon_sp/cpx/chip.c index a3d15ab..dd9c113 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.c +++ b/src/soc/intel/xeon_sp/cpx/chip.c @@ -115,7 +115,9 @@
static void set_pcu_locks(void) { - for (uint32_t socket = 0; socket < soc_get_num_cpus(); ++socket) { + for (uint32_t socket = 0; socket < CONFIG_MAX_SOCKET; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; uint32_t bus = get_socket_stack_busno(socket, PCU_IIO_STACK);
/* configure PCU_CR0_FUN csrs */ diff --git a/src/soc/intel/xeon_sp/cpx/soc_util.c b/src/soc/intel/xeon_sp/cpx/soc_util.c index 031b2bb..64efb61 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_util.c +++ b/src/soc/intel/xeon_sp/cpx/soc_util.c @@ -45,7 +45,7 @@ { const IIO_UDS *hob = get_iio_uds();
- assert(socket < hob->SystemStatus.numCpus && stack < MAX_LOGIC_IIO_STACK); + assert(socket < CONFIG_MAX_SOCKET && stack < MAX_LOGIC_IIO_STACK);
return hob->PlatformData.IIO_resource[socket].StackRes[stack].BusBase; } diff --git a/src/soc/intel/xeon_sp/memmap.c b/src/soc/intel/xeon_sp/memmap.c index c256484..8562eb1 100644 --- a/src/soc/intel/xeon_sp/memmap.c +++ b/src/soc/intel/xeon_sp/memmap.c @@ -67,7 +67,10 @@ dpr.raw = pci_read_config32(dev, VTD_LTDPR);
/* Compare the LTDPR register on all iio stacks */ - for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { + for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; + iio++; for (int stack = 0; stack < MAX_IIO_STACK; ++stack) { const STACK_RES *ri = &hob->PlatformData.IIO_resource[socket].StackRes[stack]; diff --git a/src/soc/intel/xeon_sp/skx/soc_acpi.c b/src/soc/intel/xeon_sp/skx/soc_acpi.c index 33e7c1a..d572d4b 100644 --- a/src/soc/intel/xeon_sp/skx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/skx/soc_acpi.c @@ -76,7 +76,10 @@ return;
acpigen_write_scope("\_SB"); - for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { + for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; + iio++; IIO_RESOURCE_INSTANCE iio_resource = hob->PlatformData.IIO_resource[socket]; for (int stack = 0; stack <= PSTACK2; ++stack) { diff --git a/src/soc/intel/xeon_sp/spr/chip.c b/src/soc/intel/xeon_sp/spr/chip.c index 16e059b..f9c43db 100644 --- a/src/soc/intel/xeon_sp/spr/chip.c +++ b/src/soc/intel/xeon_sp/spr/chip.c @@ -92,7 +92,9 @@
static void set_pcu_locks(void) { - for (uint32_t socket = 0; socket < soc_get_num_cpus(); ++socket) { + for (uint32_t socket = 0; socket < CONFIG_MAX_SOCKET; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; const uint32_t bus = get_ubox_busno(socket, UNCORE_BUS_1);
/* configure PCU_CR0_FUN csrs */ diff --git a/src/soc/intel/xeon_sp/spr/numa.c b/src/soc/intel/xeon_sp/spr/numa.c index d49dcb8..ef1c153 100644 --- a/src/soc/intel/xeon_sp/spr/numa.c +++ b/src/soc/intel/xeon_sp/spr/numa.c @@ -50,11 +50,13 @@ memset(pds.pds, 0, sizeof(struct proximity_domain) * pds.num_pds);
/* Fill in processor domains */ - uint8_t i, j; + uint8_t i, j, socket; struct device *dev; - for (i = 0; i < num_sockets; i++) { + for (socket = 0, i = 0; i < num_sockets; socket++) { + if (!soc_cpu_is_enabled(socket)) + continue; pds.pds[i].pd_type = PD_TYPE_PROCESSOR; - pds.pds[i].socket_bitmap = 1 << hob->PlatformData.IIO_resource[i].SocketID; + pds.pds[i].socket_bitmap = 1 << hob->PlatformData.IIO_resource[socket].SocketID; pds.pds[i].distances = malloc(sizeof(uint8_t) * pds.num_pds); if (!pds.pds[i].distances) die("%s %d out of memory.", __FILE__, __LINE__); @@ -65,6 +67,7 @@ else pds.pds[i].distances[j] = 0x0e; } + i++; }
/* If there are no CXL nodes, we are done */ diff --git a/src/soc/intel/xeon_sp/spr/soc_acpi.c b/src/soc/intel/xeon_sp/spr/soc_acpi.c index 2e77de2..2b820f1 100644 --- a/src/soc/intel/xeon_sp/spr/soc_acpi.c +++ b/src/soc/intel/xeon_sp/spr/soc_acpi.c @@ -558,13 +558,17 @@ cxl_uid.byte0 = 'C'; cxl_uid.byte1 = 'X'; /* Loop through all sockets and stacks, add CHBS for each CXL IIO stack */ - for (uint8_t s = 0; s < hob->PlatformData.numofIIO; ++s) { + for (uint8_t socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; + iio++; for (int x = 0; x < MAX_LOGIC_IIO_STACK; ++x) { - const STACK_RES *ri = &hob->PlatformData.IIO_resource[s].StackRes[x]; + const STACK_RES *ri; + ri = &hob->PlatformData.IIO_resource[socket].StackRes[x]; if (!is_iio_cxl_stack_res(ri)) continue; /* uid needs to match with ACPI CXL device ID, eg. acpi/iiostack.asl */ - cxl_uid.byte2 = s + '0'; + cxl_uid.byte2 = socket + '0'; cxl_uid.byte3 = x + '0'; cxl_ver = ACPI_CEDT_CHBS_CXL_VER_1_1; base = ri->Mmio32Base; /* DP RCRB base */ diff --git a/src/soc/intel/xeon_sp/spr/soc_util.c b/src/soc/intel/xeon_sp/spr/soc_util.c index 67cb063..f352145 100644 --- a/src/soc/intel/xeon_sp/spr/soc_util.c +++ b/src/soc/intel/xeon_sp/spr/soc_util.c @@ -129,7 +129,7 @@ { const IIO_UDS *hob = get_iio_uds();
- assert(socket < hob->SystemStatus.numCpus && stack < MAX_LOGIC_IIO_STACK); + assert(socket < CONFIG_MAX_SOCKET && stack < MAX_LOGIC_IIO_STACK);
return hob->PlatformData.IIO_resource[socket].StackRes[stack].BusBase; } @@ -138,7 +138,7 @@ { const IIO_UDS *hob = get_iio_uds();
- assert(socket < hob->SystemStatus.numCpus); + assert(socket < CONFIG_MAX_SOCKET); for (int stack = 0; stack < MAX_LOGIC_IIO_STACK; ++stack) { if (hob->PlatformData.IIO_resource[socket].StackRes[stack].Personality == TYPE_UBOX) diff --git a/src/soc/intel/xeon_sp/uncore_acpi.c b/src/soc/intel/xeon_sp/uncore_acpi.c index 2cc3c8f..e2b47329 100644 --- a/src/soc/intel/xeon_sp/uncore_acpi.c +++ b/src/soc/intel/xeon_sp/uncore_acpi.c @@ -384,7 +384,11 @@
static unsigned long acpi_create_atsr(unsigned long current, const IIO_UDS *hob) { - for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { + for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; + iio++; + uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment; unsigned long tmp = current; bool first = true; @@ -463,7 +467,11 @@ { const IIO_UDS *hob = get_iio_uds();
- for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { + for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; + iio++; + IIO_RESOURCE_INSTANCE iio_resource = hob->PlatformData.IIO_resource[socket]; for (int stack = 0; stack < MAX_LOGIC_IIO_STACK; ++stack) { @@ -504,14 +512,15 @@ /* SoC Integrated Address Translation Cache */ static unsigned long acpi_create_satc(unsigned long current, const IIO_UDS *hob) { - const unsigned long tmp = current;
// Add the SATC header current += acpi_create_dmar_satc(current, 0, 0);
// Find the DINO devices on each socket - for (int socket = (hob->PlatformData.numofIIO - 1); socket >= 0; --socket) { + for (int socket = CONFIG_MAX_SOCKET - 1; socket >= 0; --socket) { + if (!soc_cpu_is_enabled(socket)) + continue; for (int stack = (MAX_LOGIC_IIO_STACK - 1); stack >= 0; --stack) { const STACK_RES *ri = &hob->PlatformData.IIO_resource[socket].StackRes[stack]; // Add the DINO ATS devices to the SATC @@ -530,7 +539,9 @@ const IIO_UDS *hob = get_iio_uds();
// DRHD - socket 0 stack 0 must be the last DRHD entry. - for (int socket = (hob->PlatformData.numofIIO - 1); socket >= 0; --socket) { + for (int socket = (CONFIG_MAX_SOCKET - 1); socket >= 0; --socket) { + if (!soc_cpu_is_enabled(socket)) + continue; for (int stack = (MAX_LOGIC_IIO_STACK - 1); stack >= 0; --stack) current = acpi_create_drhd(current, socket, stack, hob); } diff --git a/src/soc/intel/xeon_sp/util.c b/src/soc/intel/xeon_sp/util.c index f9efe58..adcc9b5 100644 --- a/src/soc/intel/xeon_sp/util.c +++ b/src/soc/intel/xeon_sp/util.c @@ -107,9 +107,13 @@
// copy IIO Stack info from FSP HOB info->no_of_stacks = 0; - for (int s = 0; s < hob->PlatformData.numofIIO; ++s) { + for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; + iio++; for (int x = 0; x < MAX_IIO_STACK; ++x) { - const STACK_RES *ri = &hob->PlatformData.IIO_resource[s].StackRes[x]; + const STACK_RES *ri; + ri = &hob->PlatformData.IIO_resource[socket].StackRes[x]; if (!is_iio_stack_res(ri)) continue; assert(info->no_of_stacks < (CONFIG_MAX_SOCKET * MAX_IIO_STACK)); @@ -249,7 +253,6 @@
void set_bios_init_completion(void) { - /* FIXME: This may need to be changed for multi-socket platforms */ uint32_t sbsp_socket_id = 0;
/* @@ -257,7 +260,9 @@ * to receive the BIOS init completion message. So, we send it to all non-SBSP * sockets first. */ - for (uint32_t socket = 0; socket < soc_get_num_cpus(); ++socket) { + for (uint32_t socket = 0; socket < CONFIG_MAX_SOCKET; ++socket) { + if (!soc_cpu_is_enabled(socket)) + continue; if (socket == sbsp_socket_id) continue; set_bios_init_completion_for_package(socket);
-
Patrick Rudolph (Code Review)