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.