[M] Change in coreboot[master]: Revert "drivers/fsp2_0/mp_service_ppi: Use struct device to fill in b...
Attention is currently required from: Patrick Rudolph, Maximilian Brune, Arthur Heymans, Lean Sheng Tan, Werner Zeh, Andrey Petrov, Felix Held.
Hello build bot (Jenkins), Patrick Rudolph, Maximilian Brune, Arthur Heymans, Lean Sheng Tan, Werner Zeh, Andrey Petrov, Felix Held,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/73907
to review the following change.
Change subject: Revert "drivers/fsp2_0/mp_service_ppi: Use struct device to fill in buffer" ......................................................................
Revert "drivers/fsp2_0/mp_service_ppi: Use struct device to fill in buffer"
This reverts commit 8b8400a889abadbbd2156d4a35a27203068766f1.
Reason for revert: This patch results into the CPU exception while booting google/rex.
Detailed Failure Log:
[DEBUG] FSP MultiPhaseSiInit src/soc/intel/meteorlake/fsp_params.c/platform_fsp_multi_phase_init_cb called [DEBUG] Detected 10 core, 12 thread CPU. ... ... [DEBUG] Detected 10 core, 12 thread CPU. [EMERG] CPU Index 2 - APIC 0 Unexpected Exception:13 @ 10:75f3e34e - Halting [EMERG] Code: 0 eflags: 00010012 cr2: 00000000 [EMERG] eax: 00000001 ebx: 00000640 ecx: 0000004e edx: 00000000 [EMERG] edi: 75f313d8 esi: 0000004e ebp: 75b13218 esp: 75b131c0
[EMERG] 0x75f3e308: c4 0c c7 45 b4 00 00 00 [EMERG] 0x75f3e310: 00 f7 d0 56 21 d8 ff 75 [EMERG] 0x75f3e318: b4 89 45 b0 ff 75 b0 e8 [EMERG] 0x75f3e320: 9c 13 00 00 83 c4 10 39 [EMERG] 0x75f3e328: 57 14 75 09 39 47 10 0f [EMERG] 0x75f3e330: 84 98 02 00 00 8a 4f 09 [EMERG] 0x75f3e338: 8b 47 10 8b 57 14 89 45 [EMERG] 0x75f3e340: c0 89 55 c4 8b 77 04 80 [EMERG] 0x75f3e348: f9 3f 76 09 89 f1 0f 30 [EMERG] 0x75f3e350: e9 78 02 00 00 83 ec 0c [EMERG] 0x75f3e358: 88 4d ac 0f b6 5f 08 56 [EMERG] 0x75f3e360: e8 9f e0 ff ff ff 75 c4 [EMERG] 0x75f3e368: ff 75 c0 0f b6 4d ac 6a [EMERG] 0x75f3e370: 00 6a 00 8d 4c 0b ff 51 [EMERG] 0x75f3e378: 53 52 50 e8 c4 f7 ff ff [EMERG] 0x75f3e380: 89 f1 0f 30 83 c4 30 e9 [EMERG] 0x75b1323c: 0x00000002 [EMERG] 0x75b13238: 0x75cf44a8 [EMERG] 0x75b13234: 0x75f3e156 [EMERG] 0x75b13230: 0x00000006 [EMERG] 0x75b1322c: 0x00000000 [EMERG] 0x75b13228: 0x0000000c [EMERG] 0x75b13224: 0x75afd5b4 [EMERG] 0x75b13220: 0x75cf44a8 [EMERG] 0x75b1321c: 0x75acbab3 [EMERG] 0x75b13218: 0x00000000 <-ebp [EMERG] 0x75b13214: 0x00000002 [EMERG] 0x75b13210: 0x75b091e8 [EMERG] 0x75b1320c: 0x00000002 [EMERG] 0x75b13208: 0x00000000 [EMERG] 0x75b13204: 0x00000000 [EMERG] 0x75b13200: 0x000009ff [EMERG] 0x75b131fc: 0xbfebfbff [EMERG] 0x75b131f8: 0x77fafbff [EMERG] 0x75b131f4: 0x000a06a1 [EMERG] 0x75b131f0: 0x00000199 [EMERG] 0x75b131ec: 0x75f3a000 [EMERG] 0x75b131e8: 0x75f38018 [EMERG] 0x75b131e4: 0x75f39030 [EMERG] 0x75b131e0: 0x00000029 [EMERG] 0x75b131dc: 0x00000000 [EMERG] 0x75b131d8: 0x00000001 [EMERG] 0x75b131d4: 0x75f3b0c0 [EMERG] 0x75b131d0: 0x00000002 [EMERG] 0x75b131cc: 0x00000000 [EMERG] 0x75b131c8: 0x00000000 [EMERG] 0x75b131c4: 0x75f3b001 [EMERG] 0x75b131c0: 0x75cf5434 <-esp
Change-Id: I4ffea2b3ecce818a7874f95d50bb6fc0ceb59cc4 Signed-off-by: subratabanik subratabanik@google.com --- M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c M src/soc/intel/common/block/cpu/cpulib.c M src/soc/intel/common/block/include/intelblocks/cpulib.h 3 files changed, 157 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/73907/1
diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c index a891ba0..d286ee3 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c @@ -28,6 +28,9 @@ efi_return_status_t mp_get_processor_info(efi_uintn_t processor_number, efi_processor_information *processor_info_buffer) { + int apicid; + uint8_t package, core, thread; + if (processor_number >= MIN(get_cpu_count(), CONFIG_MAX_CPUS)) return FSP_NOT_FOUND;
@@ -36,10 +39,14 @@ if (!info) return FSP_DEVICE_ERROR;
- struct device *dev = info->cpu; - if (processor_info_buffer == NULL) return FSP_INVALID_PARAMETER; + apicid = info->cpu->path.apic.apic_id; + + if (apicid < 0) + return FSP_DEVICE_ERROR; + + processor_info_buffer->ProcessorId = apicid;
processor_info_buffer->StatusFlag = PROCESSOR_HEALTH_STATUS_BIT | PROCESSOR_ENABLED_BIT; @@ -47,10 +54,12 @@ if (processor_number == BSP_CPU_SLOT) processor_info_buffer->StatusFlag |= PROCESSOR_AS_BSP_BIT;
- processor_info_buffer->ProcessorId = dev->path.apic.apic_id; - processor_info_buffer->Location.Package = dev->path.apic.package_id; - processor_info_buffer->Location.Core = dev->path.apic.core_id; - processor_info_buffer->Location.Thread = dev->path.apic.thread_id; + /* Fill EFI_CPU_PHYSICAL_LOCATION structure information */ + get_cpu_topology_from_apicid(apicid, &package, &core, &thread); + + processor_info_buffer->Location.Package = package; + processor_info_buffer->Location.Core = core; + processor_info_buffer->Location.Thread = thread;
return FSP_SUCCESS; } diff --git a/src/soc/intel/common/block/cpu/cpulib.c b/src/soc/intel/common/block/cpu/cpulib.c index c317e05..3f0ee67 100644 --- a/src/soc/intel/common/block/cpu/cpulib.c +++ b/src/soc/intel/common/block/cpu/cpulib.c @@ -15,6 +15,23 @@ #include <soc/soc_chip.h> #include <types.h>
+#define CPUID_EXTENDED_CPU_TOPOLOGY 0x0b +#define LEVEL_TYPE_CORE 2 +#define LEVEL_TYPE_SMT 1 + +#define CPUID_CPU_TOPOLOGY(x, val) \ + (((val) >> CPUID_CPU_TOPOLOGY_##x##_SHIFT) & CPUID_CPU_TOPOLOGY_##x##_MASK) + +#define CPUID_CPU_TOPOLOGY_LEVEL_TYPE_SHIFT 0x8 +#define CPUID_CPU_TOPOLOGY_LEVEL_TYPE_MASK 0xff +#define CPUID_CPU_TOPOLOGY_LEVEL(res) CPUID_CPU_TOPOLOGY(LEVEL_TYPE, (res).ecx) + +#define CPUID_CPU_TOPOLOGY_LEVEL_BITS_SHIFT 0x0 +#define CPUID_CPU_TOPOLOGY_LEVEL_BITS_MASK 0x1f +#define CPUID_CPU_TOPOLOGY_THREAD_BITS(res) CPUID_CPU_TOPOLOGY(LEVEL_BITS, (res).eax) +#define CPUID_CPU_TOPOLOGY_CORE_BITS(res, threadbits) \ + ((CPUID_CPU_TOPOLOGY(LEVEL_BITS, (res).eax)) - threadbits) + #define CPUID_PROCESSOR_FREQUENCY 0X16 #define CPUID_HYBRID_INFORMATION 0x1a
@@ -464,6 +481,52 @@ return valid_size; }
+/* Get number of bits for core ID and SMT ID */ +static void get_cpu_core_thread_bits(uint32_t *core_bits, uint32_t *thread_bits) +{ + struct cpuid_result cpuid_regs; + int level_num, cpu_id_op = 0; + const uint32_t cpuid_max_func = cpuid_get_max_func(); + + /* Assert if extended CPU topology not supported */ + assert(cpuid_max_func >= CPUID_EXTENDED_CPU_TOPOLOGY); + + cpu_id_op = CPUID_EXTENDED_CPU_TOPOLOGY; + + *core_bits = level_num = 0; + cpuid_regs = cpuid_ext(cpu_id_op, level_num); + + /* Sub-leaf index 0 enumerates SMT level, if not assert */ + assert(CPUID_CPU_TOPOLOGY_LEVEL(cpuid_regs) == LEVEL_TYPE_SMT); + + *thread_bits = CPUID_CPU_TOPOLOGY_THREAD_BITS(cpuid_regs); + do { + level_num++; + cpuid_regs = cpuid_ext(cpu_id_op, level_num); + if (CPUID_CPU_TOPOLOGY_LEVEL(cpuid_regs) == LEVEL_TYPE_CORE) { + *core_bits = CPUID_CPU_TOPOLOGY_CORE_BITS(cpuid_regs, *thread_bits); + break; + } + /* Stop when level type is invalid i.e 0 */ + } while (CPUID_CPU_TOPOLOGY_LEVEL(cpuid_regs)); +} + +void get_cpu_topology_from_apicid(uint32_t apicid, uint8_t *package, + uint8_t *core, uint8_t *thread) +{ + + uint32_t core_bits, thread_bits; + + get_cpu_core_thread_bits(&core_bits, &thread_bits); + + if (package) + *package = apicid >> (thread_bits + core_bits); + if (core) + *core = (apicid >> thread_bits) & ((1 << core_bits) - 1); + if (thread) + *thread = apicid & ((1 << thread_bits) - 1); +} + static void sync_core_prmrr(void) { static msr_t msr_base, msr_mask; diff --git a/src/soc/intel/common/block/include/intelblocks/cpulib.h b/src/soc/intel/common/block/include/intelblocks/cpulib.h index cbc9e44..38c43d8 100644 --- a/src/soc/intel/common/block/include/intelblocks/cpulib.h +++ b/src/soc/intel/common/block/include/intelblocks/cpulib.h @@ -183,6 +183,10 @@ */ void enable_pm_timer_emulation(void);
+/* Derive core, package and thread information from lapic ID */ +void get_cpu_topology_from_apicid(uint32_t apicid, uint8_t *package, + uint8_t *core, uint8_t *thread); + /* * Initialize core PRMRR *
-
Subrata Banik (Code Review)