Change in ...coreboot[master]: src/arch/x86: Use core apic id to get cpu_index()

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/26346 )

Change subject: src/arch/x86: Use core apic id to get cpu_index()
......................................................................

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) 
---
M src/arch/x86/cpu.c
M src/arch/x86/include/arch/cpu.h
2 files changed, 38 insertions(+), 7 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c
index f19b389..fb4c7b6 100644
--- a/src/arch/x86/cpu.c
+++ b/src/arch/x86/cpu.c
@@ -337,3 +337,27 @@
    /* 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 61b17a6..481ee9d 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -261,13 +261,6 @@
    );
    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_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 */