[edk2-devel] [PATCH v2 07/25] UefiCpuPkg: Always Set Stack Guard in MpPei Init

[Taylor Beebe]

Memory protection is not set in PEI and ingested during and
after DXE handoff. This paradigm means that the platform cannot
reliably query the stack guard setting during MpInit. Because the
execution path of PEI consistent and no third party
code is executed, setting the stack guard in MpInit on every
boot should be fine.

Signed-off-by: Taylor Beebe <taylor.d.beebe at gmail.com>
Cc: Eric Dong <eric.dong at intel.com>
Cc: Ray Ni <ray.ni at intel.com>
Cc: Rahul Kumar <rahul1.kumar at intel.com> 
Cc: Gerd Hoffmann <kraxel at redhat.com>
---
 UefiCpuPkg/CpuMpPei/CpuMpPei.c   |  8 +++-----
 UefiCpuPkg/CpuMpPei/CpuPaging.c  | 16 ++++++++--------
 UefiCpuPkg/CpuMpPei/CpuMpPei.h   |  3 ++-
 UefiCpuPkg/CpuMpPei/CpuMpPei.inf |  1 -
 4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index b504bea3cfeb..ca0c6bdb4b21 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -469,10 +469,6 @@ InitializeMpExceptionStackSwitchHandlers (
   EFI_STATUS                      Status;
   UINT8                           *Buffer;
 
-  if (!PcdGetBool (PcdCpuStackGuard)) {
-    return;
-  }
-
   Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
   ASSERT_EFI_ERROR (Status);
 
@@ -589,7 +585,9 @@ InitializeCpuMpWorker (
   //
   // Special initialization for the sake of Stack Guard
   //
-  InitializeMpExceptionStackSwitchHandlers ();
+  if (mInitStackGuard) {
+    InitializeMpExceptionStackSwitchHandlers ();
+  }
 
   //
   // Update and publish CPU BIST information
diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index b7ddb0005b6f..0ab8ceeee8a6 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -68,6 +68,8 @@ EFI_PEI_NOTIFY_DESCRIPTOR  mPostMemNotifyList[] = {
   }
 };
 
+BOOLEAN  mInitStackGuard = FALSE;
+
 /**
   The function will check if IA32 PAE is supported.
 
@@ -532,7 +534,7 @@ SetupStackGuardPage (
 }
 
 /**
-  Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
+  Enable/setup stack guard for each processor.
 
   Doing this in the memory-discovered callback is to make sure the Stack Guard
   feature to cover as most PEI code as possible.
@@ -553,7 +555,6 @@ MemoryDiscoveredPpiNotifyCallback (
   )
 {
   EFI_STATUS              Status;
-  BOOLEAN                 InitStackGuard;
   EDKII_MIGRATED_FV_INFO  *MigratedFvInfo;
   EFI_PEI_HOB_POINTERS    Hob;
   IA32_CR0                Cr0;
@@ -563,11 +564,10 @@ MemoryDiscoveredPpiNotifyCallback (
   // initialization later will not contain paging information and then fail
   // the task switch (for the sake of stack switch).
   //
-  InitStackGuard = FALSE;
-  Hob.Raw        = NULL;
+  Hob.Raw = NULL;
   if (IsIa32PaeSupported ()) {
-    Hob.Raw        = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
-    InitStackGuard = PcdGetBool (PcdCpuStackGuard);
+    Hob.Raw         = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
+    mInitStackGuard = TRUE;
   }
 
   //
@@ -575,7 +575,7 @@ MemoryDiscoveredPpiNotifyCallback (
   // is to enable paging if it is not enabled (only in 32bit mode).
   //
   Cr0.UintN = AsmReadCr0 ();
-  if ((Cr0.Bits.PG == 0) && (InitStackGuard || (Hob.Raw != NULL))) {
+  if ((Cr0.Bits.PG == 0) && (mInitStackGuard || (Hob.Raw != NULL))) {
     ASSERT (sizeof (UINTN) == sizeof (UINT32));
 
     Status = EnablePaePageTable ();
@@ -588,7 +588,7 @@ MemoryDiscoveredPpiNotifyCallback (
   Status = InitializeCpuMpWorker ((CONST EFI_PEI_SERVICES **)PeiServices);
   ASSERT_EFI_ERROR (Status);
 
-  if (InitStackGuard) {
+  if (mInitStackGuard) {
     SetupStackGuardPage ();
   }
 
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index 1b9a94e18fdf..d0db4e480e13 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -31,6 +31,7 @@
 #include <Library/CpuPageTableLib.h>
 
 extern EFI_PEI_PPI_DESCRIPTOR  mPeiCpuMpPpiDesc;
+extern BOOLEAN                 mInitStackGuard;
 
 /**
   This service retrieves the number of logical processor in the platform
@@ -426,7 +427,7 @@ InitializeCpuMpWorker (
   );
 
 /**
-  Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
+  Enable/setup stack guard for each processor.
 
   Doing this in the memory-discovered callback is to make sure the Stack Guard
   feature to cover as most PEI code as possible.
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
index 865be5627e85..6a987754120a 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
@@ -64,7 +64,6 @@ [Ppis]
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask    ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                       ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList              ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize                    ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize                           ## SOMETIMES_CONSUMES
-- 
2.41.0.windows.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107862): https://edk2.groups.io/g/devel/message/107862
Mute This Topic: https://groups.io/mt/100830905/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-