Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32701
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
arch/cpu: Rename mp_get_apic_id() function
This patch renames mp_get_apic_id() to cpu_get_apic_id() in order access it outside CONFIG_PARALLEL_MP kconfig scope.
Also make below changes 1. Make sure add_cpu_map_entry() function saves default_apic_id before calling SMM relocation. 2. Use dev_find_path() to get cpu device path. 3. Remove redundent add_cpu_map_entry() call from BSP and AP. Move this function call into cpu_initialize()
Change-Id: I6a6c85df055bc0b5fc8c850cfa04d50859067088 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/cpu.c M src/cpu/x86/mp_init.c M src/include/cpu/cpu.h M src/include/cpu/x86/mp.h 4 files changed, 32 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/32701/1
diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c index 80d4d0d..f1b14d5 100644 --- a/src/arch/x86/cpu.c +++ b/src/arch/x86/cpu.c @@ -219,6 +219,30 @@ cpu->ops = driver ? driver->ops : NULL; }
+struct cpu_map { + struct device *dev; + /* Keep track of default apic ids for SMM. */ + int default_apic_id; +}; + +/* Keep track of APIC and device structure for each CPU. */ +static struct cpu_map cpus[CONFIG_MAX_CPUS]; + +static inline void add_cpu_map_entry(const struct cpu_info *info) +{ + cpus[info->index].dev = info->cpu; + cpus[info->index].default_apic_id = cpuid_ebx(1) >> 24; +} + +/* Returns APIC id for coreboot CPU number or < 0 on failure. */ +int cpu_get_apic_id(int logical_cpu) +{ + if (logical_cpu >= CONFIG_MAX_CPUS || logical_cpu < 0) + return -1; + + return cpus[logical_cpu].default_apic_id; +} + void cpu_initialize(unsigned int index) { /* Because we busy wait at the printk spinlock. @@ -231,6 +255,8 @@ struct cpuinfo_x86 c;
info = cpu_info(); + /* Track cpus in cpu_map structures. */ + add_cpu_map_entry(info);
printk(BIOS_INFO, "Initializing CPU #%d\n", index);
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 9d51b3e..28e50d0 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -135,21 +135,6 @@ static int global_num_aps; static struct mp_flight_plan mp_info;
-struct cpu_map { - struct device *dev; - /* Keep track of default apic ids for SMM. */ - int default_apic_id; -}; - -/* Keep track of APIC and device structure for each CPU. */ -static struct cpu_map cpus[CONFIG_MAX_CPUS]; - -static inline void add_cpu_map_entry(const struct cpu_info *info) -{ - cpus[info->index].dev = info->cpu; - cpus[info->index].default_apic_id = cpuid_ebx(1) >> 24; -} - static inline void barrier_wait(atomic_t *b) { while (atomic_read(b) == 0) @@ -212,9 +197,8 @@
info = cpu_info(); info->index = cpu; - info->cpu = cpus[cpu].dev; + info->cpu = dev_find_path(NULL, DEVICE_PATH_CPU_CLUSTER);
- add_cpu_map_entry(info); thread_init_cpu_info_non_bsp(info);
/* Fix up APIC id with reality. */ @@ -411,7 +395,6 @@ continue; } new->name = processor_name; - cpus[i].dev = new; }
return max_cpus; @@ -587,9 +570,6 @@
if (info->index != 0) printk(BIOS_CRIT, "BSP index(%d) != 0!\n", info->index); - - /* Track BSP in cpu_map structures. */ - add_cpu_map_entry(info); }
/* @@ -667,15 +647,6 @@ cpu_initialize(info->index); }
-/* Returns APIC id for coreboot CPU number or < 0 on failure. */ -int mp_get_apic_id(int logical_cpu) -{ - if (logical_cpu >= CONFIG_MAX_CPUS || logical_cpu < 0) - return -1; - - return cpus[logical_cpu].default_apic_id; -} - void smm_initiate_relocation_parallel(void) { if ((lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY)) { @@ -769,7 +740,7 @@ struct smm_runtime *runtime = smm_params->runtime;
for (i = 0; i < CONFIG_MAX_CPUS; i++) - runtime->apic_id_to_cpu[i] = mp_get_apic_id(i); + runtime->apic_id_to_cpu[i] = cpu_get_apic_id(i); }
static int install_relocation_handler(int num_cpus, size_t save_state_size) @@ -1001,12 +972,12 @@ }
static struct mp_flight_record mp_steps[] = { + /* Initialize each CPU through the driver framework. */ + MP_FR_BLOCK_APS(mp_initialize_cpu, mp_initialize_cpu), /* Once the APs are up load the SMM handlers. */ MP_FR_BLOCK_APS(NULL, load_smm_handlers), /* Perform SMM relocation. */ MP_FR_NOBLOCK_APS(trigger_smm_relocation, trigger_smm_relocation), - /* Initialize each CPU through the driver framework. */ - MP_FR_BLOCK_APS(mp_initialize_cpu, mp_initialize_cpu), /* Wait for APs to finish then optionally start looking for work. */ MP_FR_BLOCK_APS(ap_wait_for_instruction, NULL), }; diff --git a/src/include/cpu/cpu.h b/src/include/cpu/cpu.h index 60940f0..7cdbbd3 100644 --- a/src/include/cpu/cpu.h +++ b/src/include/cpu/cpu.h @@ -5,6 +5,8 @@
#if !defined(__ROMCC__) void cpu_initialize(unsigned int cpu_index); +/* Returns APIC id for coreboot CPU number or < 0 on failure. */ +int cpu_get_apic_id(int logical_cpu); struct bus; void initialize_cpus(struct bus *cpu_bus); asmlinkage void secondary_cpu_init(unsigned int cpu_index); diff --git a/src/include/cpu/x86/mp.h b/src/include/cpu/x86/mp.h index 9789910..c04252e 100644 --- a/src/include/cpu/x86/mp.h +++ b/src/include/cpu/x86/mp.h @@ -145,9 +145,6 @@ */ int mp_park_aps(void);
-/* Returns APIC id for coreboot CPU number or < 0 on failure. */ -int mp_get_apic_id(int logical_cpu); - /* * SMM helpers to use with initializing CPUs. */
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(2 comments)
Before going deeper into this, what platforms (of interest) still don't use PARALLEL_MP? Would it be worth to port them forward instead?
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@200 PS1, Line 200: info->cpu = dev_find_path(NULL, DEVICE_PATH_CPU_CLUSTER); I fear, this sets the wrong device, so the APIC id (line 205) would not be updated correctly.
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@976 PS1, Line 976: MP_FR_BLOCK_APS(mp_initialize_cpu, mp_initialize_cpu), Order is critical here. For instance many CPUs do a second microcode load here that has to happen after SMM is set up.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(2 comments)
Before going deeper into this, what platforms (of interest) still don't use PARALLEL_MP? Would it be worth to port them forward instead?
List of board doesn't use PARALLEL_MP https://qa.coreboot.org/job/coreboot-gerrit/93546/testReport/junit/(root)/bo...
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@200 PS1, Line 200: info->cpu = dev_find_path(NULL, DEVICE_PATH_CPU_CLUSTER);
I fear, this sets the wrong device, so the APIC id (line 205) would not […]
i can verify and get back
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@976 PS1, Line 976: MP_FR_BLOCK_APS(mp_initialize_cpu, mp_initialize_cpu),
Order is critical here. For instance many CPUs do a second microcode […]
do you mean SGX flow to load second microcode ?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@a142 PS1, Line 142: I would split this up. It seems `dev` is only used locally here to provide this pointer to the AP, once. `default_apic_id` is what would be useful in a more common place, for your cpu_index() implementation.
Maybe wait on Aaron's feedback on this.
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@a592 PS1, Line 592: I would leave this here and add a similar call in `lapic_cpu_init.c`. Then cpu_initialize() wouldn't be affected and there'd be no need to change the flight plan.
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@976 PS1, Line 976: MP_FR_BLOCK_APS(mp_initialize_cpu, mp_initialize_cpu),
do you mean SGX flow to load second microcode ?
No, there is a much older step that we do since Sandy Bridge, IIRC. See for instance 550049 "Figure 6-2. MP Initialization Overview".
And that was just an example. I don't think we can change order here.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@a592 PS1, Line 592:
I would leave this here and add a similar call in `lapic_cpu_init.c`. […]
do you recommend add_cpu_map_entry to move into cpu.c and call the same from here and lapic_cpu_init.c so the flow remain same.
Also lets move cpu_get_apic_id() into cpu.c ?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@a592 PS1, Line 592:
do you recommend add_cpu_map_entry to move into cpu. […]
Yes, sounds like a plan.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@a592 PS1, Line 592:
Yes, sounds like a plan.
thanks Nico, i will modify. also will wait for Aaron's feedback.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() function ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/32701/1/src/arch/x86/cpu.c File src/arch/x86/cpu.c:
https://review.coreboot.org/#/c/32701/1/src/arch/x86/cpu.c@237 PS1, Line 237: APIC id This is the default apic id that comes out of reset, correct? No the current one?
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@200 PS1, Line 200: info->cpu = dev_find_path(NULL, DEVICE_PATH_CPU_CLUSTER);
i can verify and get back
I would definitely prefer to keep the ordering of dev assignment. Can't we just expose a routine that does the dev assignment such that the order is consistent in how it is here?
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@976 PS1, Line 976: MP_FR_BLOCK_APS(mp_initialize_cpu, mp_initialize_cpu),
No, there is a much older step that we do since Sandy Bridge, IIRC. […]
Ya. I don't think we should have so much churn in this change. We should be able to expose an API to utilize at the key parts such that it's only a few line change.
Hello Aaron Durbin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32701
to look at the new patch set (#2).
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function
This patch renames mp_get_apic_id() to cpu_get_apic_id() and add_cpu_map_entry() to cpu_add_map_entry() in order access it outside CONFIG_PARALLEL_MP kconfig scope.
Also make below changes - Make cpu_add_map_entry() function available externally to call it from mp_init.c and lapic_cpu_init.c.
BRANCH=none BUG=b:79562868
Change-Id: I6a6c85df055bc0b5fc8c850cfa04d50859067088 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/cpu.c M src/cpu/x86/mp_init.c M src/include/cpu/cpu.h M src/include/cpu/x86/mp.h 4 files changed, 35 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/32701/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c File src/arch/x86/cpu.c:
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@222 PS2, Line 222: struct cpu_map { Doesn't need to be a struct anymore.
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@231 PS2, Line 231: const struct cpu_info *info nit: could just pass only `index`
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@233 PS2, Line 233: cpuid_ebx(1) >> 24 Maybe add a function for this? e.g. initial_lapicid()?
https://review.coreboot.org/#/c/32701/2/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/2/src/cpu/x86/mp_init.c@138 PS2, Line 138: struct cpu_device_map { Doesn't need to be a struct anymore.
Hello Aaron Durbin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32701
to look at the new patch set (#3).
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function
This patch renames mp_get_apic_id() to cpu_get_apic_id() and add_cpu_map_entry() to cpu_add_map_entry() in order access it outside CONFIG_PARALLEL_MP kconfig scope.
Also make below changes - Make cpu_add_map_entry() function available externally to call it from mp_init.c and lapic_cpu_init.c.
BRANCH=none BUG=b:79562868
Change-Id: I6a6c85df055bc0b5fc8c850cfa04d50859067088 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/cpu.c M src/cpu/x86/mp_init.c M src/include/cpu/cpu.h M src/include/cpu/x86/mp.h 4 files changed, 40 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/32701/3
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c File src/arch/x86/cpu.c:
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@222 PS2, Line 222: struct cpu_map {
Doesn't need to be a struct anymore.
Done
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@231 PS2, Line 231: const struct cpu_info *info
nit: could just pass only `index`
Done
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@233 PS2, Line 233: cpuid_ebx(1) >> 24
Maybe add a function for this? e.g. […]
Done
https://review.coreboot.org/#/c/32701/2/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/2/src/cpu/x86/mp_init.c@138 PS2, Line 138: struct cpu_device_map {
Doesn't need to be a struct anymore.
Done
Subrata Banik has removed build bot (Jenkins) from this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
Removed reviewer build bot (Jenkins).
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
Patch Set 3: Code-Review+2
(8 comments)
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c File src/arch/x86/cpu.c:
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@222 PS2, Line 222: struct cpu_map {
Done
Ack
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@231 PS2, Line 231: const struct cpu_info *info
Done
Ack
https://review.coreboot.org/#/c/32701/2/src/arch/x86/cpu.c@233 PS2, Line 233: cpuid_ebx(1) >> 24
Done
Ack
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@a142 PS1, Line 142:
I would split this up. It seems `dev` is only used locally here to provide […]
Done
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@a592 PS1, Line 592:
thanks Nico, i will modify. […]
Done
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@200 PS1, Line 200: info->cpu = dev_find_path(NULL, DEVICE_PATH_CPU_CLUSTER);
I would definitely prefer to keep the ordering of dev assignment. […]
Done (sort of)
https://review.coreboot.org/#/c/32701/1/src/cpu/x86/mp_init.c@976 PS1, Line 976: MP_FR_BLOCK_APS(mp_initialize_cpu, mp_initialize_cpu),
Ya. I don't think we should have so much churn in this change. […]
Done
https://review.coreboot.org/#/c/32701/2/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/32701/2/src/cpu/x86/mp_init.c@138 PS2, Line 138: struct cpu_device_map {
Done
Ack
Hello Aaron Durbin, Aamir Bohra, Rizwan Qureshi, build bot (Jenkins), Nico Huber, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32701
to look at the new patch set (#4).
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function
This patch renames mp_get_apic_id() to cpu_get_apic_id() and add_cpu_map_entry() to cpu_add_map_entry() in order access it outside CONFIG_PARALLEL_MP kconfig scope.
Also make below changes - Make cpu_add_map_entry() function available externally to call it from mp_init.c and lapic_cpu_init.c.
BRANCH=none BUG=b:79562868
Change-Id: I6a6c85df055bc0b5fc8c850cfa04d50859067088 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/cpu.c M src/cpu/x86/mp_init.c M src/include/cpu/cpu.h M src/include/cpu/x86/mp.h 4 files changed, 40 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/32701/4
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32701 )
Change subject: arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function ......................................................................
arch/cpu: Rename mp_get_apic_id() and add_cpu_map_entry() function
This patch renames mp_get_apic_id() to cpu_get_apic_id() and add_cpu_map_entry() to cpu_add_map_entry() in order access it outside CONFIG_PARALLEL_MP kconfig scope.
Also make below changes - Make cpu_add_map_entry() function available externally to call it from mp_init.c and lapic_cpu_init.c.
BRANCH=none BUG=b:79562868
Change-Id: I6a6c85df055bc0b5fc8c850cfa04d50859067088 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32701 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/arch/x86/cpu.c M src/cpu/x86/mp_init.c M src/include/cpu/cpu.h M src/include/cpu/x86/mp.h 4 files changed, 40 insertions(+), 31 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 80d4d0d..f19b389 100644 --- a/src/arch/x86/cpu.c +++ b/src/arch/x86/cpu.c @@ -219,6 +219,35 @@ cpu->ops = driver ? driver->ops : NULL; }
+/* Keep track of default apic ids for SMM. */ +static int cpus_default_apic_id[CONFIG_MAX_CPUS]; + +/* + * When CPUID executes with EAX set to 1, additional processor identification + * information is returned to EBX register: + * Default APIC ID: EBX[31-24] - this number is the 8 bit ID that is assigned + * to the local APIC on the processor during power on. + */ +static int initial_lapicid(void) +{ + return cpuid_ebx(1) >> 24; +} + +/* Function to keep track of cpu default apic_id */ +void cpu_add_map_entry(unsigned int index) +{ + cpus_default_apic_id[index] = initial_lapicid(); +} + +/* Returns default APIC id based on logical_cpu number or < 0 on failure. */ +int cpu_get_apic_id(int logical_cpu) +{ + if (logical_cpu >= CONFIG_MAX_CPUS || logical_cpu < 0) + return -1; + + return cpus_default_apic_id[logical_cpu]; +} + void cpu_initialize(unsigned int index) { /* Because we busy wait at the printk spinlock. diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 9d51b3e..8957515 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -135,20 +135,8 @@ static int global_num_aps; static struct mp_flight_plan mp_info;
-struct cpu_map { - struct device *dev; - /* Keep track of default apic ids for SMM. */ - int default_apic_id; -}; - -/* Keep track of APIC and device structure for each CPU. */ -static struct cpu_map cpus[CONFIG_MAX_CPUS]; - -static inline void add_cpu_map_entry(const struct cpu_info *info) -{ - cpus[info->index].dev = info->cpu; - cpus[info->index].default_apic_id = cpuid_ebx(1) >> 24; -} +/* Keep track of device structure for each CPU. */ +static struct device *cpus_dev[CONFIG_MAX_CPUS];
static inline void barrier_wait(atomic_t *b) { @@ -212,9 +200,9 @@
info = cpu_info(); info->index = cpu; - info->cpu = cpus[cpu].dev; + info->cpu = cpus_dev[cpu];
- add_cpu_map_entry(info); + cpu_add_map_entry(info->index); thread_init_cpu_info_non_bsp(info);
/* Fix up APIC id with reality. */ @@ -411,7 +399,7 @@ continue; } new->name = processor_name; - cpus[i].dev = new; + cpus_dev[i] = new; }
return max_cpus; @@ -589,7 +577,7 @@ printk(BIOS_CRIT, "BSP index(%d) != 0!\n", info->index);
/* Track BSP in cpu_map structures. */ - add_cpu_map_entry(info); + cpu_add_map_entry(info->index); }
/* @@ -667,15 +655,6 @@ cpu_initialize(info->index); }
-/* Returns APIC id for coreboot CPU number or < 0 on failure. */ -int mp_get_apic_id(int logical_cpu) -{ - if (logical_cpu >= CONFIG_MAX_CPUS || logical_cpu < 0) - return -1; - - return cpus[logical_cpu].default_apic_id; -} - void smm_initiate_relocation_parallel(void) { if ((lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY)) { @@ -769,7 +748,7 @@ struct smm_runtime *runtime = smm_params->runtime;
for (i = 0; i < CONFIG_MAX_CPUS; i++) - runtime->apic_id_to_cpu[i] = mp_get_apic_id(i); + runtime->apic_id_to_cpu[i] = cpu_get_apic_id(i); }
static int install_relocation_handler(int num_cpus, size_t save_state_size) diff --git a/src/include/cpu/cpu.h b/src/include/cpu/cpu.h index 60940f0..9a28373 100644 --- a/src/include/cpu/cpu.h +++ b/src/include/cpu/cpu.h @@ -5,6 +5,10 @@
#if !defined(__ROMCC__) void cpu_initialize(unsigned int cpu_index); +/* Returns default APIC id based on logical_cpu number or < 0 on failure. */ +int cpu_get_apic_id(int logical_cpu); +/* Function to keep track of cpu default apic_id */ +void cpu_add_map_entry(unsigned int index); struct bus; void initialize_cpus(struct bus *cpu_bus); asmlinkage void secondary_cpu_init(unsigned int cpu_index); diff --git a/src/include/cpu/x86/mp.h b/src/include/cpu/x86/mp.h index 9789910..c04252e 100644 --- a/src/include/cpu/x86/mp.h +++ b/src/include/cpu/x86/mp.h @@ -145,9 +145,6 @@ */ int mp_park_aps(void);
-/* Returns APIC id for coreboot CPU number or < 0 on failure. */ -int mp_get_apic_id(int logical_cpu); - /* * SMM helpers to use with initializing CPUs. */
-
Aaron Durbin (Code Review) -
Nico Huber (Code Review)
-
Subrata Banik (Code Review)