Attention is currently required from: Furquan Shaikh, Paul Menzel, Tim Wawrzynczak, Angel Pons, Subrata Banik, Andrey Petrov, Patrick Rudolph. Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51085 )
Change subject: driver/intel/fsp2_0: Allow function to run serially on all APs ......................................................................
Patch Set 2:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51085/comment/53e6a01a_d5a05bd4 PS2, Line 9: singlethread
The name seems to be `run_serial`.
Ack
https://review.coreboot.org/c/coreboot/+/51085/comment/d0dadc4f_394209c5 PS2, Line 14:
Please give some numbers, on how much time this actually saves. […]
Just adding a provision for serial run. Only for the were MP programming is recommended to run serially. Time would depend on the procedure to be run and also the timeout param in case it does not go through.
File src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c:
https://review.coreboot.org/c/coreboot/+/51085/comment/c64cf287_c313c538 PS2, Line 67: run_serial
I’d prefer the opposite name `run_in_parallel`.
This is more to align with FSP EFI_PEI_MP_SERVICES_STARTUP_ALL_APS parameters.
https://review.coreboot.org/c/coreboot/+/51085/comment/4c91e32a_f247f5af PS2, Line 77: uint32_t
get_cpu_count() returns an int
Ack
https://review.coreboot.org/c/coreboot/+/51085/comment/a858da71_eea7817c PS2, Line 91: MP_RUN_ON_ALL_CPUS
I think this enum is named incorrectly. https://review.coreboot.org/cgit/coreboot. […]
ok to add a helper in mp init lib, you mean something like this?
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index cca6093458a..8788f650b6b 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -992,6 +992,19 @@ int mp_run_on_aps(void (*func)(void *), void *arg, int logical_cpu_num, return run_ap_work(&lcb, expire_us); }
+int mp_run_on_all_aps(void (*func)(void *), void *arg, long expire_us, bool run_serial) +{ + if (run_serial) { + for (int ap_index = 1; ap_index <= global_aps_num; ap_index++) { + if (mp_run_on_aps(func, arg, ap_index, expire_us)) + printk(BIOS_ERR, "%s: failed running procedure on AP %d\n", + __func__, ap_index); + } + } else { + return mp_run_on_aps(func, arg, 0, expire_us); + } +}
and then call this from fsp driver?