Attention is currently required from: Nico Huber, Jonathan Zhang, David Hendricks, Rocky Phagura, Angel Pons. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52518 )
Change subject: src/cpu/x86/smm: move console init to thread safe section ......................................................................
Patch Set 1:
(1 comment)
File src/cpu/x86/smm/smm_module_handler.c:
https://review.coreboot.org/c/coreboot/+/52518/comment/83c98969_9915e0c8 PS1, Line 149: if (cpu > CONFIG_MAX_CPUS) {
So I'm still not convinced. Regardless of the configuration and where its detected, this code is not thread safe. It causes hangs and makes Coreboot less robust. At the time of the hang, there is no console messages available and the hang occurs within in SMM. These requires a hardware debugger to root cause the issue.
If max CPU config is checked earlier during MP init and its an illegal config, then the system should be halted at that point, thus making this code not applicable.
The stack of a CPU is decided at runtime in the stub. Having a cpu > CONFIG_MAX_CPU would have that stack smashing the code, so returning should probably happen before the stack is set up. I don't if the CPU setup will relocate SMM for cpu > CONFIG_MAX_CPU, so it might be a NOOP.