From 095c931cf12924da9011b47aa64f4a6f11d89f13 Mon Sep 17 00:00:00 2001 From: Subrata Banik Date: Mon, 6 May 2019 19:51:34 +0530 Subject: [PATCH] src/arch/x86: Use core apic id to get cpu_index() This cpu_index() implementation assumes that cpu_index() function might always getting called from coreboot context (ESP stack pointer will always refer to coreboot). This might not be true in case of proposed PI spec MP_SERVICES_PPI implementation, where FSP context (stack pointer refers to fsp) will request to get cpu_index(), natural alignment logic will use ESP and retrieve struct cpu_info *ci from (stack_top - 8 byte). This is not the place where cpu_index is actually stored by ramstage c_start.S Hence this patch tries to remove those dependencies while retrieving cpu_index(), rather it uses cpuid to fetch lapic id and matches with cpus_default_apic_id[] variable to return correct cpu_index(). BRANCH=none BUG=b:79562868 TEST=Ensures functions can be run on APs without any failure and cpu_index() also provides correct index number. Change-Id: I55023a3e0cf42f0496d45bc6af8ead447f402350 Signed-off-by: Subrata Banik Reviewed-on: https://review.coreboot.org/c/coreboot/+/26346 Reviewed-by: Nico Huber Tested-by: build bot (Jenkins) --- src/arch/x86/cpu.c | 24 ++++++++++++++++++++++++ src/arch/x86/include/arch/cpu.h | 21 ++++++++++++++------- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c index f19b389441..fb4c7b6cfe 100644 --- a/src/arch/x86/cpu.c +++ b/src/arch/x86/cpu.c @@ -337,3 +337,27 @@ void arch_bootstate_coreboot_exit(void) /* APs are waiting for work. Last thing to do is park them. */ mp_park_aps(); } + +/* + * Previously cpu_index() implementation assumes that cpu_index() + * function will always getting called from coreboot context + * (ESP stack pointer will always refer to coreboot). + * + * But with FSP_USES_MP_SERVICES_PPI implementation in coreboot this + * assumption might not be true, where FSP context (stack pointer refers + * to FSP) will request to get cpu_index(). + * + * Hence new logic to use cpuid to fetch lapic id and matches with + * cpus_default_apic_id[] variable to return correct cpu_index(). + */ +unsigned long cpu_index(void) +{ + int i; + int lapic_id = initial_lapicid(); + + for (i = 0; i < CONFIG_MAX_CPUS; i++) { + if (cpu_get_apic_id(i) == lapic_id) + return i; + } + return -1; +} diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 61b17a6d2a..481ee9d8e0 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -261,13 +261,6 @@ static inline struct cpu_info *cpu_info(void) ); return ci; } - -static inline unsigned long cpu_index(void) -{ - struct cpu_info *ci; - ci = cpu_info(); - return ci->index; -} #endif #ifndef __ROMCC__ // romcc is segfaulting in some cases @@ -374,4 +367,18 @@ uint32_t cpu_get_feature_flags_ecx(void); */ uint32_t cpu_get_feature_flags_edx(void); +/* + * Previously cpu_index() implementation assumes that cpu_index() + * function will always getting called from coreboot context + * (ESP stack pointer will always refer to coreboot). + * + * But with FSP_USES_MP_SERVICES_PPI implementation in coreboot this + * assumption might not be true, where FSP context (stack pointer refers + * to FSP) will request to get cpu_index(). + * + * Hence new logic to use cpuid to fetch lapic id and matches with + * cpus_default_apic_id[] variable to return correct cpu_index(). + */ +unsigned long cpu_index(void); + #endif /* ARCH_CPU_H */