sravan/system76-coreboot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Browse Source 
This reverts commit 095c931cf1.

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 <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/69509
Reviewed-by: Subrata Banik <subratabanik@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This commit is contained in:
Arthur Heymans
2022-11-12 18:51:04 +01:00
parent aab91213b2
commit cc22607dbf
5 changed files with 20 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
src/arch/x86/cpu.c
View File

@@ -318,33 +318,9 @@ void arch_bootstate_coreboot_exit(void)
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)
{

21
src/arch/x86/include/arch/cpu.h
View File

@@ -161,6 +161,13 @@ 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;
}
struct cpuinfo_x86 {
uint8_t x86; /* CPU family */
uint8_t x86_vendor; /* CPU vendor */
@@ -212,20 +219,6 @@ 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 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

19
src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
View File

@@ -31,16 +31,17 @@ efi_return_status_t mp_get_processor_info(efi_uintn_t processor_number,
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_get_processor_info(efi_uintn_t processor_number,
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_aps(efi_ap_procedure procedure,
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_all_cpus(efi_ap_procedure procedure,
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())

2
src/soc/intel/xeon_sp/cpx/cpu.c
View File

@@ -79,7 +79,7 @@ static void each_cpu_init(struct device *cpu)
{
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);
/*

2
src/soc/intel/xeon_sp/skx/cpu.c
View File

@@ -59,7 +59,7 @@ static void xeon_sp_core_init(struct device *cpu)
{
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);