Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34570 )
Change subject: cpu/x86/mp_init: Wait longer with serial enabled ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 1:
Patch Set 1: Code-Review-1
While a bigger timeout fixes the issue I'm seeing and it's related to slow serial, the fix is incorrect. The main issue is that `mp_run_on_all_cpus` doesn't wait for CPUs to finish execution.
That means consecutive calls to mp_run_on_all_cpus expect all CPUs to pick-up the work, but they are still busy with the old job (due to high core count and console spinlock).
I guess that's the reason mp_park_aps has this big timeout.
An idea how to fix it: Wait it mp_run_on_aps for all APs to signal idle state before trying to launch a new job.
we need to think do we really need to have all serial print in multi-core environment for example, i'm running same function foo() [where i'm having bunch of serial debug print] in all cores (BSP + AP) and expecting to see all those log in "n" numbers of time. What is the benefit in that approach. if we could only limit serial log for BSP then we can avoid such race case where due to slower serial uart ultimately my AP function giving error, there is no issue with my cores (APs) only due to slower uart, AP has to compromise.
if we write something like this for all functions suppose to run in multi threaded envionment as below
function foo() { if(bsp) printk(...) }
thoughts ?
That works, but:
- Requires lots of changes in common code
- Only fixes current code, but doesn't prevent future developers from doing the same error again
- Might break again as soon as more code (delay) as added on secondary APs
- Might break again as soon as core count increases
Alternative solution: https://review.coreboot.org/c/coreboot/+/34587