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>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: [PATCH v2 14/15] UefiCpuPkg/PiSmmCpuDxeSmm: remove DBs from SmmRelocationSemaphoreComplete32()
Date: Fri, 23 Mar 2018 22:15:03 +0100	[thread overview]
Message-ID: <20180323211504.22434-15-lersek@redhat.com> (raw)
In-Reply-To: <20180323211504.22434-1-lersek@redhat.com>

(1) SmmRelocationSemaphoreComplete32() runs in 32-bit mode, so wrap it in
    a (BITS 32 ... BITS 64) bracket.

(2) SmmRelocationSemaphoreComplete32() currently compiles to:

> 000002AE  C6050000000001    mov byte [dword 0x0],0x1
> 000002B5  FF2500000000      jmp dword [dword 0x0]

    where the first instruction is patched with the contents of
    "mRebasedFlag" (so that (*mRebasedFlag) is set to 1), and the second
    instruction is patched with the address of
    "mSmmRelocationOriginalAddress" (so that we jump to
    "mSmmRelocationOriginalAddress").

    In its current form the first instruction could not be patched with
    PatchInstructionX86(), given that the operand to patch is not encoded
    in the trailing bytes of the instruction. Therefore, adopt an
    EAX-based version, inspired by both the IA32 and X64 variants of
    SmmRelocationSemaphoreComplete():

> 000002AE  50                push eax
> 000002AF  B800000000        mov eax,0x0
> 000002B4  C60001            mov byte [eax],0x1
> 000002B7  58                pop eax
> 000002B8  FF2500000000      jmp dword [dword 0x0]

    Here both instructions can be patched with PatchInstructionX86(), and
    the DBs can be replaced with native NASM syntax.

(3) Turn the "mRebasedFlagAddr32" and "mSmmRelocationOriginalAddressPtr32"
    variables into markers that suit PatchInstructionX86().

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]
    - drop the claim from the commit message that this patch removes the
      last instructions encoded with DBs from PiSmmCpuDxeSmm (see the next
      patch)

 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm | 24 +++++++++-----------
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c  | 16 +++++++++----
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
index eae14c0549f0..0b0c3f28e53f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
@@ -29,8 +29,8 @@ global ASM_PFX(gPatchSmmInitStack)
 global ASM_PFX(gcSmiInitGdtr)
 global ASM_PFX(gcSmmInitSize)
 global ASM_PFX(gcSmmInitTemplate)
-global ASM_PFX(mRebasedFlagAddr32)
-global ASM_PFX(mSmmRelocationOriginalAddressPtr32)
+global ASM_PFX(gPatchRebasedFlagAddr32)
+global ASM_PFX(gPatchSmmRelocationOriginalAddressPtr32)
 
 %define LONG_MODE_CS 0x38
 
@@ -125,20 +125,18 @@ ASM_PFX(SmmRelocationSemaphoreComplete):
 ;
 ; Semaphore code running in 32-bit mode
 ;
+BITS 32
 global ASM_PFX(SmmRelocationSemaphoreComplete32)
 ASM_PFX(SmmRelocationSemaphoreComplete32):
-    ;
-    ; mov byte ptr [], 1
-    ;
-    db      0xc6, 0x5
-ASM_PFX(mRebasedFlagAddr32): dd 0
-    db      1
-    ;
-    ; jmp dword ptr []
-    ;
-    db      0xff, 0x25
-ASM_PFX(mSmmRelocationOriginalAddressPtr32): dd 0
+    push    eax
+    mov     eax, strict dword 0                ; source operand will be patched
+ASM_PFX(gPatchRebasedFlagAddr32):
+    mov     byte [eax], 1
+    pop     eax
+    jmp     dword [dword 0]                    ; destination will be patched
+ASM_PFX(gPatchSmmRelocationOriginalAddressPtr32):
 
+BITS 64
 global ASM_PFX(PiSmmCpuSmmInitFixupAddress)
 ASM_PFX(PiSmmCpuSmmInitFixupAddress):
     lea    rax, [@LongMode]
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c
index 6dbcb086aa4d..87f595ddb8c3 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c
@@ -15,8 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #include "PiSmmCpuDxeSmm.h"
 
-extern  UINT32    mSmmRelocationOriginalAddressPtr32;
-extern  UINT32    mRebasedFlagAddr32;
+X86_ASSEMBLY_PATCH_LABEL gPatchSmmRelocationOriginalAddressPtr32;
+X86_ASSEMBLY_PATCH_LABEL gPatchRebasedFlagAddr32;
 
 UINTN             mSmmRelocationOriginalAddress;
 volatile BOOLEAN  *mRebasedFlag;
@@ -49,7 +49,11 @@ SemaphoreHook (
   UINTN                 TempValue;
 
   mRebasedFlag       = RebasedFlag;
-  mRebasedFlagAddr32 = (UINT32)(UINTN)mRebasedFlag;
+  PatchInstructionX86 (
+    gPatchRebasedFlagAddr32,
+    (UINT32)(UINTN)mRebasedFlag,
+    4
+    );
 
   CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
   mSmmRelocationOriginalAddress = HookReturnFromSmm (
@@ -63,5 +67,9 @@ SemaphoreHook (
   // Use temp value to fix ICC complier warning
   //
   TempValue = (UINTN)&mSmmRelocationOriginalAddress;
-  mSmmRelocationOriginalAddressPtr32 = (UINT32)TempValue;
+  PatchInstructionX86 (
+    gPatchSmmRelocationOriginalAddressPtr32,
+    (UINT32)TempValue,
+    4
+    );
 }
-- 
2.14.1.3.gb7cf6e02401b




  parent reply	other threads:[~2018-03-23 21:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-23 21:14 [PATCH v2 00/15] rid PiSmmCpuDxeSmm of DB-encoded instructions Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 01/15] MdePkg/BaseLib.h: state preprocessing conditions in comments after #endifs Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 02/15] MdePkg/BaseLib: add PatchInstructionX86() Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 03/15] UefiCpuPkg/PiSmmCpuDxeSmm: remove *.S and *.asm assembly files Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 04/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmbase" with PatchInstructionX86() Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 05/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmiStack" " Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 06/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmiCr3" " Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 07/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "XdSupported" " Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 08/15] UefiCpuPkg/PiSmmCpuDxeSmm: remove unneeded DBs from X64 SmmStartup() Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 09/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmCr3" with PatchInstructionX86() Laszlo Ersek
2018-03-23 21:14 ` [PATCH v2 10/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmCr4" " Laszlo Ersek
2018-03-23 21:15 ` [PATCH v2 11/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmCr0" " Laszlo Ersek
2018-03-23 21:15 ` [PATCH v2 12/15] UefiCpuPkg/PiSmmCpuDxeSmm: eliminate "gSmmJmpAddr" and related DBs Laszlo Ersek
2018-03-23 21:15 ` [PATCH v2 13/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "gSmmInitStack" with PatchInstructionX86() Laszlo Ersek
2018-03-23 21:15 ` Laszlo Ersek [this message]
2018-03-23 21:15 ` [PATCH v2 15/15] UefiCpuPkg/PiSmmCpuDxeSmm: use mnemonics for FXSAVE(64)/FXRSTOR(64) Laszlo Ersek
2018-04-03 12:57 ` [PATCH v2 00/15] rid PiSmmCpuDxeSmm of DB-encoded instructions Laszlo Ersek
2018-04-04  8:56   ` Gao, Liming
2018-04-04 15:02     ` Laszlo Ersek

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=20180323211504.22434-15-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