Nick Vaccaro has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/62907 )
Change subject: Revert "Revert "drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` to run serially"" ......................................................................
Revert "Revert "drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` to run serially""
This reverts a change that was causing hangs and exceptions during boot on an ADL brya4es.
This reverts commit 40ca79714ad7d5f2aa201d83db4d97f21260d924.
BUG=b:224873032 TEST=`emerge-brya coreboot chromeos-bootimage`, flash and verify brya4es is able so successfully reboot 50 times without any issues.
Change-Id: I88c15a51c5d27fbd243478c923e75962d3f8d67d Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c 1 file changed, 17 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/62907/1
diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c index 50e35b0..9aef1b6 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c @@ -94,10 +94,23 @@ /* Run on BSP */ procedure(argument);
- /* Run on APs */ - if (mp_run_on_aps((void *)procedure, argument, - MP_RUN_ON_ALL_CPUS, timeout_usec) != CB_SUCCESS) { - printk(BIOS_ERR, "%s: Exit with Failure\n", __func__); + /* + * Run on APs Serially + * + * FIXME: As per MP service specification, EDK2 is allowed to specify the mode + * in which a 'func' routine should be executed on APs (i.e. execute serially + * or concurrently). + * + * MP service API `StartupAllCPUs` doesn't specify such requirement. + * Hence, running the `CpuCacheInfoCollectCoreAndCacheData` + * (UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c#194) + * simultaneously on APs results in a coherency issue (hang while executing `func`) + * due to lack of acquiring a spin lock while accessing common data structure in + * multiprocessor environment. + */ + if (mp_run_on_all_aps((void *)procedure, argument, timeout_usec, false) != + CB_SUCCESS) { + printk(BIOS_DEBUG, "%s: Exit with Failure\n", __func__); return FSP_NOT_STARTED; }