UefiCpuPkg/PiSmmCpuDxeSmm: Decrease mNumberToFinish in AP safe code

We will put APs into hlt-loop in safe code. But we decrease mNumberToFinish
before APs enter into the safe code. Paolo pointed out this gap.

This patch is to move mNumberToFinish decreasing to the safe code. It could
make sure BSP could wait for all APs are running in safe code.

https://bugzilla.tianocore.org/show_bug.cgi?id=216

Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index ad7dc412fb40af19881d58485fedf5f481888028..3fb686439aeda027d073edf9977f37dd077cbc94 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -79,9 +79,11 @@ BOOLEAN                      mAcpiS3Enable = TRUE;
 \r
 UINT8                        *mApHltLoopCode = NULL;\r
 UINT8                        mApHltLoopCodeTemplate[] = {\r
-                               0xFA,        // cli\r
-                               0xF4,        // hlt\r
-                               0xEB, 0xFC   // jmp $-2\r
+                               0x8B, 0x44, 0x24, 0x04,  // mov  eax, dword ptr [esp+4]\r
+                               0xF0, 0xFF, 0x08,        // lock dec  dword ptr [eax]\r
+                               0xFA,                    // cli\r
+                               0xF4,                    // hlt\r
+                               0xEB, 0xFC               // jmp $-2\r
                                };\r
 \r
 /**\r
@@ -399,17 +401,12 @@ MPRendezvousProcedure (
   }\r
 \r
   //\r
-  // Count down the number with lock mechanism.\r
-  //\r
-  InterlockedDecrement (&mNumberToFinish);\r
-\r
-  //\r
-  // Place AP into the safe code\r
+  // Place AP into the safe code, count down the number with lock mechanism in the safe code.\r
   //\r
   TopOfStack  = (UINT32) (UINTN) Stack + sizeof (Stack);\r
   TopOfStack &= ~(UINT32) (CPU_STACK_ALIGNMENT - 1);\r
   CopyMem ((VOID *) (UINTN) mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate));\r
-  TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack);\r
+  TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack, &mNumberToFinish);\r
 }\r
 \r
 /**\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
index 8b880d6ab76368b5c6df856fde3745818ee77f73..9760373b4740f055f0170cfe5e1fd3f3144c941f 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
@@ -100,17 +100,19 @@ InitGdt (
 \r
   @param[in] ApHltLoopCode    The 32-bit address of the safe hlt-loop function.\r
   @param[in] TopOfStack       A pointer to the new stack to use for the ApHltLoopCode.\r
+  @param[in] NumberToFinish   Semaphore of APs finish count.\r
 \r
 **/\r
 VOID\r
 TransferApToSafeState (\r
   IN UINT32             ApHltLoopCode,\r
-  IN UINT32             TopOfStack\r
+  IN UINT32             TopOfStack,\r
+  IN UINT32             *NumberToFinish\r
   )\r
 {\r
   SwitchStack (\r
     (SWITCH_STACK_ENTRY_POINT) (UINTN) ApHltLoopCode,\r
-    NULL,\r
+    NumberToFinish,\r
     NULL,\r
     (VOID *) (UINTN) TopOfStack\r
     );\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 6c98659389ec8da8beebd7bec46cbb7b57ed3e3c..88d9c85db6b775f24d0a7b2ff0afeefd4c8b18cf 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -830,12 +830,14 @@ GetAcpiS3EnableFlag (
 \r
   @param[in] ApHltLoopCode    The 32-bit address of the safe hlt-loop function.\r
   @param[in] TopOfStack       A pointer to the new stack to use for the ApHltLoopCode.\r
+  @param[in] NumberToFinish   Semaphore of APs finish count.\r
 \r
 **/\r
 VOID\r
 TransferApToSafeState (\r
   IN UINT32             ApHltLoopCode,\r
-  IN UINT32             TopOfStack\r
+  IN UINT32             TopOfStack,\r
+  IN UINT32             *NumberToFinish\r
   );\r
 \r
 #endif\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index 62338b79bd1f32d3c99886920d1c62130ec84b86..6844c3fc499d810bb041a38ae04084f934f090e7 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -105,18 +105,20 @@ GetProtectedModeCS (
 \r
   @param[in] ApHltLoopCode    The 32-bit address of the safe hlt-loop function.\r
   @param[in] TopOfStack       A pointer to the new stack to use for the ApHltLoopCode.\r
+  @param[in] NumberToFinish   Semaphore of APs finish count.\r
 \r
 **/\r
 VOID\r
 TransferApToSafeState (\r
   IN UINT32             ApHltLoopCode,\r
-  IN UINT32             TopOfStack\r
+  IN UINT32             TopOfStack,\r
+  IN UINT32             *NumberToFinish\r
   )\r
 {\r
   AsmDisablePaging64 (\r
     GetProtectedModeCS (),\r
     (UINT32) (UINTN) ApHltLoopCode,\r
-    0,\r
+    (UINT32) (UINTN) NumberToFinish,\r
     0,\r
     TopOfStack\r
     );\r