From 6af980a2aeca9b8cedfb3d7734389e6e36099c88 Mon Sep 17 00:00:00 2001 From: Subrata Banik Date: Thu, 2 Sep 2021 21:05:29 +0530 Subject: [PATCH] drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` to run serially As per MP service specification, EDK2 is allowed to specify the mode in which a 'func' routine should be executed on APs. `SingleThread` sets to 'true' meaning to execute the function one by one (serially) or sets to 'false' meaning to execute the function simultaneously. MP service API `StartupAllAPs` was designed to pass such options as part of function argument. But another MP service API `StartupAllCPUs` doesn't specify any such requirement. Running the `func` 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. BUG=b:199246420 Change-Id: Ia95d11408f663212fd40daa9fd9b0881a07f1ce7 Signed-off-by: Subrata Banik Signed-off-by: Ronak Kanabar Reviewed-on: https://review.coreboot.org/c/coreboot/+/57343 Tested-by: build bot (Jenkins) Reviewed-by: Maulik V Vaghela Reviewed-by: Tim Wawrzynczak --- src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) 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 3f1e83d541..5b537a9531 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c @@ -94,9 +94,21 @@ efi_return_status_t mp_startup_all_cpus(efi_ap_procedure procedure, /* 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) { + /* + * 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)) { printk(BIOS_DEBUG, "%s: Exit with Failure\n", __func__); return FSP_NOT_STARTED; }