From 68305aa3b0fb5e8b3213baa99d48238e57bad2ad Mon Sep 17 00:00:00 2001 From: Ronak Kanabar Date: Thu, 24 Feb 2022 10:09:29 +0530 Subject: [PATCH] soc/intel/common: Remove use of CPUID_EXTENDED_CPU_TOPOLOGY_V2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In x86 processor as per Software Developer's manual there are 2 ways to get CPU topology by querying the processor. BIOS can use CPUID instruction using CPUID_EXTENDED_CPU_TOPOLOGY (0x0B) as input or CPUID_EXTENDED_CPU_TOPOLOGY_V2 (0x1F) as an input. Both will return valid CPU topology data. While CPUID_EXTENDED_CPU_TOPOLOGY (0x0B) returns data related to number of threads, core and package, CPUID_EXTENDED_CPU_TOPOLOGY_V2 (0x1F) provides more granular information regarding Die, package etc. coreboot uses V2 to in order to query and return CPU topology data as of now since that's the highest instruction of CPUID which is supported, there is a mismatch in the way FSP processes the data. FSP queries coreboot MP services to get CPU topology data which uses structure which is either compatible with CPUID_EXTENDED_CPU_TOPOLOGY or CPUID_EXTENDED_CPU_TOPOLOGY_V2. Since coreboot returns V2 data in structure which is expecting data for CPUID_EXTENDED_CPU_TOPOLOGY, there is hang observed on ADL_N CPUs. To solve this problem coreboot should assign CPUID_EXTENDED_CPU_TOPOLOGY data to processor_info_buffer->Location structure so remove use of CPUID_EXTENDED_CPU_TOPOLOGY_V2 Ref EDK2 code: https://github.com/tianocore/edk2/tree/edk2-stable202202 Files: MdePkg/Include/Protocol/MpService.h#L182 UefiCpuPkg/Library/MpInitLib/MpLib.c#L2127 UefiCpuPkg/Library/MpInitLib/MpLib.c#L2120 Ref doc: Software Developer’s Manual volume 3 CH 8.9 BUG=b:220652104 TEST=Build and boot ADL-N RVP with debug FSP and verify CPU topology value and observe system boots (no hang). Change-Id: I1e6832fb03fcc59d33df0ba1664019727185d10a Signed-off-by: Ronak Kanabar Reviewed-on: https://review.coreboot.org/c/coreboot/+/62323 Tested-by: build bot (Jenkins) Reviewed-by: Angel Pons Reviewed-by: Subrata Banik --- src/soc/intel/common/block/cpu/cpulib.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/soc/intel/common/block/cpu/cpulib.c b/src/soc/intel/common/block/cpu/cpulib.c index d4edc8fd98..e1bd471adf 100644 --- a/src/soc/intel/common/block/cpu/cpulib.c +++ b/src/soc/intel/common/block/cpu/cpulib.c @@ -13,7 +13,6 @@ #include #define CPUID_EXTENDED_CPU_TOPOLOGY 0x0b -#define CPUID_EXTENDED_CPU_TOPOLOGY_V2 0x1f #define LEVEL_TYPE_CORE 2 #define LEVEL_TYPE_SMT 1 @@ -443,11 +442,7 @@ static void get_cpu_core_thread_bits(uint32_t *core_bits, uint32_t *thread_bits) /* Assert if extended CPU topology not supported */ assert(cpuid_max_func >= CPUID_EXTENDED_CPU_TOPOLOGY); - /* Check for extended CPU topology CPUID support */ - if (cpuid_max_func >= CPUID_EXTENDED_CPU_TOPOLOGY_V2) - cpu_id_op = CPUID_EXTENDED_CPU_TOPOLOGY_V2; - else if (cpuid_max_func >= CPUID_EXTENDED_CPU_TOPOLOGY) - cpu_id_op = 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);