[coreboot-gerrit] Change in coreboot[master]: drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` to run serially

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/57343 )

Change subject: drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` to run serially
......................................................................

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 
---
M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
1 file changed, 15 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Maulik V Vaghela: Looks good to me, approved
  Tim Wawrzynczak: Looks good to me, approved

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 3f1e83d..5b537a9 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 @@
    /* 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;
    }

5 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.