public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Eric Dong <eric.dong@intel.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Liming Gao <liming.gao@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Ruiyu Ni <ruiyu.ni@intel.com>
Subject: [PATCH 07/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "XdSupported" with PatchInstructionX86()
Date: Fri,  2 Feb 2018 15:39:47 +0100	[thread overview]
Message-ID: <20180202143954.7357-8-lersek@redhat.com> (raw)
In-Reply-To: <20180202143954.7357-1-lersek@redhat.com>

"mXdSupported" is a global BOOLEAN variable, initialized to TRUE. The
CheckFeatureSupported() function is executed on all processors (not
concurrently though), called from SmmInitHandler(). If XD support is found
to be missing on any CPU, then "mXdSupported" is set to FALSE, and further
processors omit the check. Afterwards, "mXdSupported" is read by several
assembly and C code locations.

The tricky part is *where* "mXdSupported" is allocated (defined):

- Before commit 717fb60443fb ("UefiCpuPkg/PiSmmCpuDxeSmm: Add paging
  protection.", 2016-11-17), it used to be a normal global variable,
  defined (allocated) in "SmmProfile.c".

- With said commit, we moved the definition (allocation) of "mXdSupported"
  into "SmiEntry.nasm". The variable was defined over the last byte of a
  "mov al, 1" instruction, so that setting it to FALSE in
  CheckFeatureSupported() would patch the instruction to "mov al, 0". The
  subsequent conditional jump would change behavior, plus all further read
  references to "mXdSupported" (in C and assembly code) would read back
  the source (imm8) operand of the patched MOV instruction as data.

  This trick required that the MOV instruction be encoded with DB.

In order to get rid of the DB, we have to split both roles: we need a
helper variable for the code patching, and "mXdSupported" has to be
defined (allocated) independently of the code patching. Of course, their
values must always remain in sync.

(1) Reinstate the "mXdSupported" definition and initialization in
    "SmmProfile.c" from before commit 717fb60443fb. Change the assembly
    language definition ("global") to a declaration ("extern").

(2) Define the "gPatchXdSupported" helper (type UINT8) in "SmiEntry.nasm",
    and add the C-language declaration to "SmmProfileInternal.h". Replace
    the DB with the MOV mnemonic (keeping the imm8 source operand with
    value 1).

(3) In CheckFeatureSupported(), whenever "mXdSupported" is set to FALSE,
    patch the assembly code in sync, with PatchInstructionX86().

Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@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>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c         | 7 +++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   | 7 ++++---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm    | 7 ++++---
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
index a21689145bb4..1c8b899f9455 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
@@ -100,6 +100,7 @@ typedef struct {
 extern SMM_S3_RESUME_STATE       *mSmmS3ResumeState;
 extern UINTN                     gSmiExceptionHandlers[];
 extern BOOLEAN                   mXdSupported;
+extern UINT8                     gPatchXdSupported;
 extern UINTN                     *mPFEntryCount;
 extern UINT64                    (*mLastPFEntryValue)[MAX_PF_ENTRY_COUNT];
 extern UINT64                    *(*mLastPFEntryPointer)[MAX_PF_ENTRY_COUNT];
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 9588eaf02964..fb0d0b365e05 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -31,6 +31,11 @@ UINTN                     mSmmProfileSize;
 //
 UINTN                     mMsrDsAreaSize   = SMM_PROFILE_DTS_SIZE;
 
+//
+// The flag indicates if execute-disable is supported by processor.
+//
+BOOLEAN                   mXdSupported     = TRUE;
+
 //
 // The flag indicates if execute-disable is enabled on processor.
 //
@@ -1010,6 +1015,7 @@ CheckFeatureSupported (
       // Extended CPUID functions are not supported on this processor.
       //
       mXdSupported = FALSE;
+      PatchInstructionX86 (&gPatchXdSupported, mXdSupported, 1);
     }
 
     AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
@@ -1018,6 +1024,7 @@ CheckFeatureSupported (
       // Execute Disable Bit feature is not supported on this processor.
       //
       mXdSupported = FALSE;
+      PatchInstructionX86 (&gPatchXdSupported, mXdSupported, 1);
     }
   }
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 0023cb328d6a..509e7a0a665f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -47,7 +47,8 @@ global ASM_PFX(gcSmiHandlerSize)
 global ASM_PFX(gPatchSmiCr3)
 global ASM_PFX(gPatchSmiStack)
 global ASM_PFX(gPatchSmbase)
-global ASM_PFX(mXdSupported)
+extern ASM_PFX(mXdSupported)
+global ASM_PFX(gPatchXdSupported)
 extern ASM_PFX(gSmiHandlerIdtr)
 
     SECTION .text
@@ -133,8 +134,8 @@ ASM_PFX(gPatchSmiCr3):
 .6:
 
 ; enable NXE if supported
-    DB      0b0h                        ; mov al, imm8
-ASM_PFX(mXdSupported):     DB      1
+    mov     al, strict byte 1           ; source operand may be patched
+ASM_PFX(gPatchXdSupported):
     cmp     al, 0
     jz      @SkipXd
 ;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index 9971ae6f064a..5d731e228095 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -54,7 +54,8 @@ extern ASM_PFX(CpuSmmDebugEntry)
 extern ASM_PFX(CpuSmmDebugExit)
 
 global ASM_PFX(gPatchSmbase)
-global ASM_PFX(mXdSupported)
+extern ASM_PFX(mXdSupported)
+global ASM_PFX(gPatchXdSupported)
 global ASM_PFX(gPatchSmiStack)
 global ASM_PFX(gPatchSmiCr3)
 global ASM_PFX(gcSmiHandlerTemplate)
@@ -118,8 +119,8 @@ ASM_PFX(gPatchSmiCr3):
     ltr     ax
 
 ; enable NXE if supported
-    DB      0xb0                        ; mov al, imm8
-ASM_PFX(mXdSupported):     DB      1
+    mov     al, strict byte 1           ; source operand may be patched
+ASM_PFX(gPatchXdSupported):
     cmp     al, 0
     jz      @SkipXd
 ;
-- 
2.14.1.3.gb7cf6e02401b




  parent reply	other threads:[~2018-02-02 14:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-02 14:39 [PATCH 00/14] rid PiSmmCpuDxeSmm of DB-encoded instructions Laszlo Ersek
2018-02-02 14:39 ` [PATCH 01/14] MdePkg/BaseLib.h: state preprocessing conditions in comments after #endifs Laszlo Ersek
2018-02-02 14:39 ` [PATCH 02/14] MdePkg/BaseLib: add PatchInstructionX86() Laszlo Ersek
2018-02-02 14:39 ` [PATCH 03/14] UefiCpuPkg/PiSmmCpuDxeSmm: remove *.S and *.asm assembly files Laszlo Ersek
2018-03-22 23:45   ` Laszlo Ersek
2018-02-02 14:39 ` [PATCH 04/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmbase" with PatchInstructionX86() Laszlo Ersek
2018-02-02 14:39 ` [PATCH 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmiStack" " Laszlo Ersek
2018-02-02 14:39 ` [PATCH 06/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmiCr3" " Laszlo Ersek
2018-02-02 14:39 ` Laszlo Ersek [this message]
2018-02-02 14:39 ` [PATCH 08/14] UefiCpuPkg/PiSmmCpuDxeSmm: remove unneeded DBs from X64 SmmStartup() Laszlo Ersek
2018-02-02 14:39 ` [PATCH 09/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmCr3" with PatchInstructionX86() Laszlo Ersek
2018-02-02 14:39 ` [PATCH 10/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmCr4" " Laszlo Ersek
2018-02-02 14:39 ` [PATCH 11/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmCr0" " Laszlo Ersek
2018-02-02 14:39 ` [PATCH 12/14] UefiCpuPkg/PiSmmCpuDxeSmm: eliminate "gSmmJmpAddr" and related DBs Laszlo Ersek
2018-02-02 14:39 ` [PATCH 13/14] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmInitStack" with PatchInstructionX86() Laszlo Ersek
2018-02-02 14:39 ` [PATCH 14/14] UefiCpuPkg/PiSmmCpuDxeSmm: remove DBs from SmmRelocationSemaphoreComplete32() Laszlo Ersek
2018-02-03  0:45 ` [PATCH 00/14] rid PiSmmCpuDxeSmm of DB-encoded instructions Kinney, Michael D
2018-02-05 10:28   ` Laszlo Ersek
2018-02-05 18:22     ` Kinney, Michael D
2018-02-05 19:23       ` Laszlo Ersek
2018-03-23  0:29         ` Kinney, Michael D

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180202143954.7357-8-lersek@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox