UefiCpuPkg/S3: Skip CR3 modification in S3Resume for 64-bit PEI

Previously, when PEI was 32-bit and DXE was 64-bit, S3 resume code had to set or change the CR3 register before executing 64-bit code. However, with both PEI and DXE now may being 64-bit, this modification is unnecessary as PEI already utilizes sufficiently large page tables. Additionally, there is a bug in the current implementation where the changed CR3 during S3 resume could map only below 4G MMIO, which could lead to issues if end of PEI notify attempts to access above 4G. Overall, skipping the CR3 modification in S3Resume when PEI is 64-bit can fix the bug and also avoid unnecessary logic. Signed-off-by: Zhiguang Liu <[email protected]>
mxu9 · Oct 10, 2024 · 6f17bd5 · 6f17bd5
1 parent da1c6dd
commit 6f17bd5
Showing 1 changed file with 5 additions and 3 deletions.
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -871,7 +871,7 @@ S3ResumeExecuteBootScript (
     SignalToSmmByCommunication (&gEdkiiS3SmmInitDoneGuid);
   }
 
-  if ((FeaturePcdGet (PcdDxeIplSwitchToLongMode)) || (sizeof (UINTN) == sizeof (UINT64))) {
+  if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
     AsmWriteCr3 ((UINTN)AcpiS3Context->S3NvsPageTableAddress);
   }
 
@@ -1083,7 +1083,7 @@ S3RestoreConfig2 (
     CpuDeadLoop ();
   }
 
-  if ((FeaturePcdGet (PcdDxeIplSwitchToLongMode)) || (sizeof (UINTN) == sizeof (UINT64))) {
+  if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
     //
     // Need reconstruct page table here, since we do not trust ACPINvs.
     //
@@ -1217,7 +1217,9 @@ S3RestoreConfig2 (
         AsmWriteCr0 (Cr0.UintN);
       }
 
-      AsmWriteCr3 ((UINTN)SmmS3ResumeState->SmmS3Cr3);
+      if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
+        AsmWriteCr3 ((UINTN)SmmS3ResumeState->SmmS3Cr3);
+      }
 
       //
       // Disable interrupt of Debug timer, since IDT table cannot work in long mode.