[edk2-devel] [Patch V2] UefiCpuPkg: Remove ASSERT checking if FinishedCount equal to CpuCount-1

Yuanhao Xie posted 1 patch 6 months ago
Failed in applying to current master (apply log)
UefiCpuPkg/Library/MpInitLib/MpLib.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[edk2-devel] [Patch V2] UefiCpuPkg: Remove ASSERT checking if FinishedCount equal to CpuCount-1
Posted by Yuanhao Xie 6 months ago
Aim:
This patch aims to solve an assertion happens after SwitchApContext()
on checking if CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1).
The assertion issue due to a timing discrepancy between the BSP
completing its Start-up signal checking and the APs incrementing the
FinishedCount.

In the SwitchApContext(), the BSP triggers the startup signal and
checks if the APs have received it to awaken. After completing this
check, the BSP then verifies if the FinishedCount is equal to
CpuCount-1. For the APs, upon receiving the startup signal,
they call the SwitchContextPerAp() and increment the
FinishedCount to indicate their awakening. However, due to a timing
discrepancy, even when all APs have received the startup signal, they
might not have finished incrementing the FinishedCount. This timing
issue results in the triggering of the assertion.

Solution:
Instead of assertion, use while loop to waits until all the APs have
incremented the FinishedCount.

Related Patches:
This patch is a group of following patch series which is to eliminate
the second INIT-SIPI-SIPI sequence. More detials could be seen in the
comments in the following patches:

UefiCpuPkg: Refactor the logic for placing APs in HltLoop.
UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop.
UefiCpuPkg: Create MpHandOff.
UefiCpuPkg: ApWakeupFunction directly use CpuMpData.
UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence.
UefiCpuPkg: Decouple the SEV-ES functionality.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 6f1456cfe1..9a6ec5db5c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -913,8 +913,8 @@ DxeApEntryPoint (
   UINTN  ProcessorNumber;
 
   GetProcessorNumber (CpuMpData, &ProcessorNumber);
-  InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
   RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
+  InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
   PlaceAPInMwaitLoopOrRunLoop (
     CpuMpData->ApLoopMode,
     CpuMpData->CpuData[ProcessorNumber].StartupApSignal,
@@ -2201,7 +2201,12 @@ MpInitLibInitialize (
       // looping process there.
       //
       SwitchApContext (MpHandOff);
-      ASSERT (CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1));
+      //
+      // Wait for all APs finished initialization
+      //
+      while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
+        CpuPause ();
+      }
 
       //
       // Set Apstate as Idle, otherwise Aps cannot be waken-up again.
-- 
2.36.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110035): https://edk2.groups.io/g/devel/message/110035
Mute This Topic: https://groups.io/mt/102175355/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-