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

[Arthur Heymans (Code Review)]

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69509 )

(

4 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: Revert "src/arch/x86: Use core apic id to get cpu_index()"
......................................................................

Revert "src/arch/x86: Use core apic id to get cpu_index()"

This reverts commit 095c931cf12924da9011b47aa64f4a6f11d89f13.

Previously cpu_info() was implemented with a struct on top of an
aligned stack. As FSP changed the stack value cpu_info() could not be
used in FSP context (which PPI is). Now cpu_info() uses GDT segments,
which FSP does not touch so it can be used.

This also exports cpu_infos from cpu.c as it's a convenient way to get
the struct device * for a certain index.

TESTED on aldrvp: FSP-S works and is able to run code on APs.

Change-Id: I3a40156ba275b572d7d1913d8c17c24b4c8f6d78
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/69509
Reviewed-by: Subrata Banik 
Tested-by: build bot (Jenkins) 
---
M src/arch/x86/cpu.c
M src/arch/x86/include/arch/cpu.h
M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
M src/soc/intel/xeon_sp/cpx/cpu.c
M src/soc/intel/xeon_sp/skx/cpu.c
5 files changed, 45 insertions(+), 50 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Subrata Banik: Looks good to me, approved

diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c
index 9fc5233..face248 100644
--- a/src/arch/x86/cpu.c
+++ b/src/arch/x86/cpu.c
@@ -318,33 +318,9 @@
    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 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().
- */
-int 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;
-}
-
 /* cpu_info() looks at address 0 at the base of %gs for a pointer to struct cpu_info */
 static struct per_cpu_segment_data segment_data[CONFIG_MAX_CPUS];
-static struct cpu_info cpu_infos[CONFIG_MAX_CPUS];
+struct cpu_info cpu_infos[CONFIG_MAX_CPUS];

enum cb_err set_cpu_info(unsigned int index, struct device *cpu)
 {
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index c7c6b4e..18bc961 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -161,6 +161,13 @@
    return ci;
 }

+static inline unsigned long cpu_index(void)
+{
+	struct cpu_info *ci;
+	ci = cpu_info();
+	return ci->index;
+}
+
 struct cpuinfo_x86 {
    uint8_t	x86;		/* CPU family */
    uint8_t	x86_vendor;	/* CPU vendor */
@@ -212,20 +219,6 @@
  */
 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 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().
- */
-int cpu_index(void);
-
 #define DETERMINISTIC_CACHE_PARAMETERS_CPUID_IA	0x04
 #define DETERMINISTIC_CACHE_PARAMETERS_CPUID_AMD	0x8000001d

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 66bbc2f..d286ee3 100644
--- a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
+++ b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
@@ -31,16 +31,17 @@
    int apicid;
    uint8_t package, core, thread;

-	if (cpu_index() < 0)
+	if (processor_number >= MIN(get_cpu_count(), CONFIG_MAX_CPUS))
+		return FSP_NOT_FOUND;
+
+	extern struct cpu_info cpu_infos[];
+	struct cpu_info *info = &cpu_infos[processor_number];
+	if (!info)
    	return FSP_DEVICE_ERROR;

if (processor_info_buffer == NULL)
    	return FSP_INVALID_PARAMETER;
-
-	if (processor_number >= get_cpu_count())
-		return FSP_NOT_FOUND;
-
-	apicid = cpu_get_apic_id(processor_number);
+	apicid = info->cpu->path.apic.apic_id;

if (apicid < 0)
    	return FSP_DEVICE_ERROR;
@@ -66,7 +67,7 @@
 efi_return_status_t mp_startup_all_aps(efi_ap_procedure procedure,
    bool run_serial, efi_uintn_t timeout_usec, void *argument)
 {
-	if (cpu_index() < 0)
+	if (!cpu_info())
    	return FSP_DEVICE_ERROR;

if (procedure == NULL)
@@ -84,7 +85,7 @@
 efi_return_status_t mp_startup_all_cpus(efi_ap_procedure procedure,
    efi_uintn_t timeout_usec, void *argument)
 {
-	if (cpu_index() < 0)
+	if (!cpu_info())
    	return FSP_DEVICE_ERROR;

if (procedure == NULL)
@@ -119,7 +120,7 @@
 efi_return_status_t mp_startup_this_ap(efi_ap_procedure procedure,
    efi_uintn_t processor_number, efi_uintn_t timeout_usec, void *argument)
 {
-	if (cpu_index() < 0)
+	if (!cpu_info())
    	return FSP_DEVICE_ERROR;

if (processor_number > get_cpu_count())
diff --git a/src/soc/intel/xeon_sp/cpx/cpu.c b/src/soc/intel/xeon_sp/cpx/cpu.c
index 4ecfad0..8a7dd2a 100644
--- a/src/soc/intel/xeon_sp/cpx/cpu.c
+++ b/src/soc/intel/xeon_sp/cpx/cpu.c
@@ -79,7 +79,7 @@
 {
    msr_t msr;

-	printk(BIOS_SPEW, "%s dev: %s, cpu: %d, apic_id: 0x%x\n",
+	printk(BIOS_SPEW, "%s dev: %s, cpu: %lu, apic_id: 0x%x\n",
    	__func__, dev_path(cpu), cpu_index(), cpu->path.apic.apic_id);

/*
diff --git a/src/soc/intel/xeon_sp/skx/cpu.c b/src/soc/intel/xeon_sp/skx/cpu.c
index d2e4af1..24984e1 100644
--- a/src/soc/intel/xeon_sp/skx/cpu.c
+++ b/src/soc/intel/xeon_sp/skx/cpu.c
@@ -59,7 +59,7 @@
 {
    msr_t msr;

-	printk(BIOS_INFO, "%s dev: %s, cpu: %d, apic_id: 0x%x\n",
+	printk(BIOS_INFO, "%s dev: %s, cpu: %lu, apic_id: 0x%x\n",
    	__func__, dev_path(cpu), cpu_index(), cpu->path.apic.apic_id);
    assert(chip_config);