Like "gSmmCr4" in the previous patch, "gSmmCr0" is not only used for
machine code patching, but also as a means to communicate the initial CR0
value from SmmRelocateBases() to InitSmmS3ResumeState(). In other words,
the last four bytes of the "mov eax, Cr0Value" instruction's binary
representation are utilized as normal data too.

In order to get rid of the DB for "mov eax, Cr0Value", we have to split
both roles, patching and data flow. Introduce the "mSmmCr0" global (SMRAM)
variable for the data flow purpose. Rename the "gSmmCr0" variable to
"gPatchSmmCr0" so that its association with PatchInstructionX86() is clear
from the declaration, change its type to X86_ASSEMBLY_PATCH_LABEL, and
patch it with PatchInstructionX86(), to the value now contained in
"mSmmCr0".

This lets us remove the binary (DB) encoding of "mov eax, Cr0Value" in
"SmmInit.nasm".

Cc: Eric Dong <eric.dong@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=866
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - use the X86_ASSEMBLY_PATCH_LABEL type rather than UINT8 [Mike]

 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h  | 3 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 6 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm  | 6 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c           | 2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c  | 4 +++-
 5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index f6eddf7e0199..62e4e38e9f2a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -308,7 +308,8 @@ extern IA32_FAR_ADDRESS             gSmmJmpAddr;
 
 extern CONST UINT8                  gcSmmInitTemplate[];
 extern CONST UINT16                 gcSmmInitSize;
-extern UINT32                       gSmmCr0;
+X86_ASSEMBLY_PATCH_LABEL            gPatchSmmCr0;
+extern UINT32                       mSmmCr0;
 X86_ASSEMBLY_PATCH_LABEL            gPatchSmmCr3;
 extern UINT32                       mSmmCr4;
 X86_ASSEMBLY_PATCH_LABEL            gPatchSmmCr4;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
index bd07a6e4f536..0f62fe448712 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
@@ -24,7 +24,7 @@ extern ASM_PFX(mSmmRelocationOriginalAddress)
 
 global ASM_PFX(gPatchSmmCr3)
 global ASM_PFX(gPatchSmmCr4)
-global ASM_PFX(gSmmCr0)
+global ASM_PFX(gPatchSmmCr0)
 global ASM_PFX(gSmmJmpAddr)
 global ASM_PFX(gSmmInitStack)
 global ASM_PFX(gcSmiInitGdtr)
@@ -60,8 +60,8 @@ ASM_PFX(gPatchSmmCr4):
     rdmsr
     or      eax, ebx                    ; set NXE bit if NX is available
     wrmsr
-    DB      0x66, 0xb8                  ; mov eax, imm32
-ASM_PFX(gSmmCr0): DD 0
+    mov     eax, strict dword 0         ; source operand will be patched
+ASM_PFX(gPatchSmmCr0):
     mov     di, PROTECT_MODE_DS
     mov     cr0, eax
     DB      0x66, 0xea                  ; jmp far [ptr48]
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
index 971bd118132f..1a0667bd97ba 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
@@ -24,7 +24,7 @@ extern ASM_PFX(mSmmRelocationOriginalAddress)
 
 global ASM_PFX(gPatchSmmCr3)
 global ASM_PFX(gPatchSmmCr4)
-global ASM_PFX(gSmmCr0)
+global ASM_PFX(gPatchSmmCr0)
 global ASM_PFX(gSmmJmpAddr)
 global ASM_PFX(gSmmInitStack)
 global ASM_PFX(gcSmiInitGdtr)
@@ -63,8 +63,8 @@ ASM_PFX(gPatchSmmCr4):
     or      ah, BIT3                    ; set NXE bit
 .1:
     wrmsr
-    DB      0x66, 0xb8                   ; mov eax, imm32
-ASM_PFX(gSmmCr0): DD 0
+    mov     eax, strict dword 0         ; source operand will be patched
+ASM_PFX(gPatchSmmCr0):
     mov     cr0, eax                    ; enable protected mode & paging
     DB      0x66, 0xea                   ; far jmp to long mode
 ASM_PFX(gSmmJmpAddr): DQ 0;@LongMode
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index b4ed0a56a814..0b8ef7035903 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -744,7 +744,7 @@ InitSmmS3ResumeState (
       SmmS3ResumeState->SmmS3StackSize = 0;
     }
 
-    SmmS3ResumeState->SmmS3Cr0 = gSmmCr0;
+    SmmS3ResumeState->SmmS3Cr0 = mSmmCr0;
     SmmS3ResumeState->SmmS3Cr3 = Cr3;
     SmmS3ResumeState->SmmS3Cr4 = mSmmCr4;
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index a3fd796dba4a..f602d86d51a1 100755
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -128,6 +128,7 @@ UINT8                    mPhysicalAddressBits;
 //
 // Control register contents saved for SMM S3 resume state initialization.
 //
+UINT32                   mSmmCr0;
 UINT32                   mSmmCr4;
 
 /**
@@ -410,7 +411,8 @@ SmmRelocateBases (
   //
   // Patch ASM code template with current CR0, CR3, and CR4 values
   //
-  gSmmCr0 = (UINT32)AsmReadCr0 ();
+  mSmmCr0 = (UINT32)AsmReadCr0 ();
+  PatchInstructionX86 (gPatchSmmCr0, mSmmCr0, 4);
   PatchInstructionX86 (gPatchSmmCr3, AsmReadCr3 (), 4);
   mSmmCr4 = (UINT32)AsmReadCr4 ();
   PatchInstructionX86 (gPatchSmmCr4, mSmmCr4, 4);
-- 
2.14.1.3.gb7cf6e02401b


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel