Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Ronak Kanabar, Andrey Petrov, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57343 )
Change subject: drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` to run serially ......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57343/comment/71dd62e2_ffd1a3f4 PS1, Line 17: But another MP service API `StartupAllCPUs` doesn't specifies any such : requirement. Running the `func` simultaneously on APs results into : coherency issue due to lack of acquire spin lock while accessing common : data structure in multiprocessor environment.
But right here, in the MpServices2 interface (https://github. […]
yes, Tim, I also have same understanding, its upto the routine what we wish to run of APs and how that routine is being written to block access to its data structure. In this case, I felt the implementation owner doesn't pay attention on the merit of the routine and implication how it would execute on APs serially or simultaneously.
I'm thinking on two options 1. as you said, if caller would like to run on all CPUs(BSP + APs) serially or simultaneously then it should use StartupAllAPs => I believe as StartupAllAPs is limited to run only on APs and not on BSP hence they have decided to introduce new API as StartupAllCPUs
2. I would expect EDK2 to provide again one more argument to let caller decides to run routine on CPUs either serially or simultaneously rather fixed design (which is simultaneously, set as FALSE). The suspected routine is being implemented with assumption that it shouldn't run concurrently but its end up doing the same unfortunately