From: Yuanhao Xie <yuanhao.xie@intel.com>

This patch synchronizes the No-Execute bit in the IA32_EFER
register for the APs before the RestoreVolatileRegisters operation.

The commit 964a4f0, titled "Eliminate the second INIT-SIPI-SIPI
sequence," replaces the second INIT-SIPI-SIPI sequence with the BSP
calling the SwitchApContext function to initiate a specialized start-up
signal, waking up APs in the DXE instead of using INIT-SIPI-SIPI.

Due to this change, the logic for "Enable execute disable bit" in
MpFuncs.nasm is no longer executed. However, to ensure the proper setup
of the page table, it is necessary to synchronize the IA32_EFER.NXE for
APs before executing RestoreVolatileRegisters .

Based on SDM:
If IA32_EFER.NXE is set to 1, it signifies execute-disable, meaning
instruction fetches are not allowed from the 4-KByte page controlled by
this entry. Conversely, if it is set to 0, it is reserved.

Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 33 ++++++++++++++++++----------
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  1 +
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5c..a5ff7ef91f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -906,21 +906,29 @@ ApWakeupFunction (
 **/
 VOID
 EFIAPI
-DxeApEntryPoint (
-  CPU_MP_DATA  *CpuMpData
+DxeApEntryPoint(
+  CPU_MP_DATA *CpuMpData
   )
 {
-  UINTN  ProcessorNumber;
+  UINTN ProcessorNumber;
+  MSR_IA32_EFER_REGISTER EferMsr;
 
-  GetProcessorNumber (CpuMpData, &ProcessorNumber);
-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
-  InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
-  PlaceAPInMwaitLoopOrRunLoop (
-    CpuMpData->ApLoopMode,
-    CpuMpData->CpuData[ProcessorNumber].StartupApSignal,
-    CpuMpData->ApTargetCState
-    );
-  ApWakeupFunction (CpuMpData, ProcessorNumber);
+  GetProcessorNumber(CpuMpData, &ProcessorNumber);
+  InterlockedIncrement((UINT32 *)&CpuMpData->FinishedCount);
+
+  if (CpuMpData->EnableExecuteDisableForSwitchContext)
+  {
+    EferMsr.Uint64 = AsmReadMsr64(MSR_IA32_EFER);
+    EferMsr.Bits.NXE = 1;
+    AsmWriteMsr64(MSR_IA32_EFER, EferMsr.Uint64);
+  }
+
+  RestoreVolatileRegisters(&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
+  PlaceAPInMwaitLoopOrRunLoop(
+      CpuMpData->ApLoopMode,
+      CpuMpData->CpuData[ProcessorNumber].StartupApSignal,
+      CpuMpData->ApTargetCState);
+  ApWakeupFunction(CpuMpData, ProcessorNumber);
 }
 
 /**
@@ -2190,6 +2198,7 @@ MpInitLibInitialize (
 
       CpuMpData->FinishedCount = 0;
       CpuMpData->InitFlag      = ApInitDone;
+      CpuMpData->EnableExecuteDisableForSwitchContext = IsBspExecuteDisableEnabled ();
       SaveCpuMpData (CpuMpData);
       //
       // In scenarios where both the PEI and DXE phases run in the same
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index d2f411f006..bf86a13820 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -265,6 +265,7 @@ struct _CPU_MP_DATA {
   UINT64                           TotalTime;
   EFI_EVENT                        WaitEvent;
   UINTN                            **FailedCpuList;
+  BOOLEAN                          EnableExecuteDisableForSwitchContext;
 
   AP_INIT_STATE                    InitFlag;
   BOOLEAN                          SwitchBspFlag;
-- 
2.39.1.windows.1



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

  Hi,

> -  GetProcessorNumber (CpuMpData, &ProcessorNumber);
> -  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
> -  InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
> -  PlaceAPInMwaitLoopOrRunLoop (
> -    CpuMpData->ApLoopMode,
> -    CpuMpData->CpuData[ProcessorNumber].StartupApSignal,
> -    CpuMpData->ApTargetCState
> -    );
> -  ApWakeupFunction (CpuMpData, ProcessorNumber);
> +  GetProcessorNumber(CpuMpData, &ProcessorNumber);
> +  InterlockedIncrement((UINT32 *)&CpuMpData->FinishedCount);

You have some whitespace changes here makes the patch hard to read and
I also doubt this will pass the uncrustify check in CI.

take care,
  Gerd



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