Change in coreboot[master]: intel/fsp2_0: Fix the mp_get_processor_info

[Aamir Bohra (Code Review)]

Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/50880 )

Change subject: intel/fsp2_0: Fix the mp_get_processor_info
......................................................................

intel/fsp2_0: Fix the mp_get_processor_info

FSP expects mp_get_processor_info to give processor specfic apic ID,
core(zero-indexed), package(zero-indexed) and thread(zero-indexed) info.
This function is run from BSP for all logical processor, with current
implementation the location information returned is incorrect per logical
processor. Also the processor id returned does not corresponds to the
processor index, rather is returned only for the BSP.

This implementation adds support for logical processor specific info as per
processor index. It derives core, package and thread ID from apic id using
CPUID extended topology(eax = 0xb).

Also the function is modified to run the procedure on APs first and then on
BSP, which aligns to corresponding edk2 API implementation.

BUG=b:179113790

Signed-off-by: Aamir Bohra 
Change-Id: Ief8677e4830a765af61a0df9621ecaa372730fca
---
M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
1 file changed, 50 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/50880/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 87056a5..7d1acd3 100644
--- a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
+++ b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
@@ -1,6 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

+#include <assert.h>
 #include <console/console.h>
+#include <cpu/cpu.h>
 #include <cpu/x86/mp.h>
 #include <cpu/x86/lapic.h>
 #include <cpu/intel/microcode.h>
@@ -10,7 +12,41 @@
 #include <intelblocks/mp_init.h>

#define BSP_CPU_SLOT 0
-#define SINGLE_CHIP_PACKAGE 0
+
+static void get_core_thread_bits(uint32_t *core_bits, uint32_t *thread_bits)
+{
+	register int ecx;
+	struct cpuid_result cpuid_regs;
+
+	/* get max index of CPUID */
+	cpuid_regs = cpuid(0);
+	assert(cpuid_regs.eax >= 0xb); /* cpuid_regs.eax is max input value for cpuid */
+
+	*thread_bits = *core_bits = 0;
+	ecx = 0;
+	while (1) {
+		cpuid_regs = cpuid_ext(0xb, ecx);
+		if (ecx == 0) {
+			*thread_bits = (cpuid_regs.eax & 0x1f);
+		} else {
+			*core_bits = (cpuid_regs.eax & 0x1f) - *thread_bits;
+			break;
+		}
+		ecx++;
+	}
+}
+
+static void get_cpu_info_from_apicid(uint32_t apicid, uint32_t core_bits, uint32_t thread_bits,
+	uint8_t *package, uint8_t *core, uint8_t *thread)
+{
+	if (package != NULL)
+		*package = (apicid >> (thread_bits + core_bits));
+	if (core != NULL)
+		*core = (uint32_t)((apicid >> thread_bits) & ~((~0) << core_bits));
+	if (thread != NULL)
+		*thread = (uint32_t)(apicid & ~((~0) << thread_bits));
+}
+

efi_return_status_t mp_get_number_of_processors(efi_uintn_t *number_of_processors,
    efi_uintn_t *number_of_enabled_processors)
@@ -28,7 +64,8 @@
 efi_return_status_t mp_get_processor_info(efi_uintn_t processor_number,
    efi_processor_information *processor_info_buffer)
 {
-	unsigned int num_virt_cores, num_phys_cores;
+	uint32_t apicid, core_bits, thread_bits;
+	uint8_t package, core, thread;

if (cpu_index() < 0)
    	return FSP_DEVICE_ERROR;
@@ -39,7 +76,8 @@
    if (processor_number >= get_cpu_count())
    	return FSP_NOT_FOUND;

-	processor_info_buffer->ProcessorId = lapicid();
+	apicid = cpu_get_apic_id(processor_number);
+	processor_info_buffer->ProcessorId = apicid;

processor_info_buffer->StatusFlag = PROCESSOR_HEALTH_STATUS_BIT
    		| PROCESSOR_ENABLED_BIT;
@@ -48,12 +86,14 @@
    	processor_info_buffer->StatusFlag |= PROCESSOR_AS_BSP_BIT;

/* Fill EFI_CPU_PHYSICAL_LOCATION structure information */
-	cpu_read_topology(&num_phys_cores, &num_virt_cores);
+	get_core_thread_bits(&core_bits, &thread_bits);
+	get_cpu_info_from_apicid(apicid, core_bits, thread_bits,
+		&package, &core, &thread);

/* FSP will add one to the value in this Package field */
-	processor_info_buffer->Location.Package = SINGLE_CHIP_PACKAGE;
-	processor_info_buffer->Location.Core = num_phys_cores;
-	processor_info_buffer->Location.Thread = num_virt_cores;
+	processor_info_buffer->Location.Package = package;
+	processor_info_buffer->Location.Core = core;
+	processor_info_buffer->Location.Thread = thread;

return FSP_SUCCESS;
 }
@@ -85,9 +125,6 @@
    if (procedure == NULL)
    	return FSP_INVALID_PARAMETER;

-	/* Run on BSP */
-	procedure(argument);
-
    /* Run on APs */
    if (mp_run_on_aps((void *)procedure, argument,
    		MP_RUN_ON_ALL_CPUS, timeout_usec)) {
@@ -95,6 +132,9 @@
    	return FSP_NOT_STARTED;
    }

+	/* Run on BSP */
+	procedure(argument);
+
    return FSP_SUCCESS;
 }