Change in coreboot[master]: Revert "Revert "drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` t...

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change. ( 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.

The hang (or APIC exception) occurs at what appears to be the FSP MP
initialization sequence, prior to the "Display FSP Version Info HOB"
log being displayed :

[DEBUG]  Detected 10 core, 12 thread CPU.
  [DEBUG]  Display FSP Version Info HOB

This reverts commit 40ca79714ad7d5f2aa201d83db4d97f21260d924.

BUG=b:224873032
TEST=`emerge-brya coreboot chromeos-bootimage`, flash and verify brya4es
is able to successfully reboot 200 times without any issues.

Change-Id: I88c15a51c5d27fbd243478c923e75962d3f8d67d
Signed-off-by: Nick Vaccaro 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62907
Tested-by: build bot (Jenkins) 
Reviewed-by: Subrata Banik 
Reviewed-by: Ronak Kanabar 
Reviewed-by: Tim Wawrzynczak 
---
M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
1 file changed, 17 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Ronak Kanabar: Looks good to me, approved
  Tim Wawrzynczak: Looks good to me, approved
  Subrata Banik: 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 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;
    }