From 456c4ccab27b79f1b9c8d13ddcb5ed5453c09c53 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Fri, 2 Feb 2018 04:12:51 +0100 Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: eliminate "gSmmJmpAddr" and related DBs The IA32 version of "SmmInit.nasm" does not need "gSmmJmpAddr" at all (its PiSmmCpuSmmInitFixupAddress() variant doesn't do anything either). We can simply use the NASM syntax for the following Mixed-Size Jump: > jmp PROTECT_MODE_CS : dword @32bit The generated object code for the instruction is unchanged: > 00000182 66EA5A0000000800 jmp dword 0x8:0x5a (The NASM manual explains that putting the DWORD prefix after the colon ":" reflects the intent better, since it is the offset that is a DWORD. Thus, that's what I used. However, both syntaxes are interchangeable, hence the ndisasm output.) The X64 version of "SmmInit.nasm" appears to require "gSmmJmpAddr"; however that's accidental, not inherent: - Bring LONG_MODE_CODE_SEGMENT from "UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h" to "SmmInit.nasm" as LONG_MODE_CS, same as PROTECT_MODE_CODE_SEGMENT was brought to the IA32 version as PROTECT_MODE_CS earlier. - Apply the NASM-native Mixed-Size Jump syntax again, but jump to the fixed zero offset in LONG_MODE_CS. This will produce no relocation record at all. Add a label after the instruction. - Modify PiSmmCpuSmmInitFixupAddress() to patch the jump target backwards from the label. Because we modify the DWORD offset with a DWORD access, the segment selector is unharmed in the instruction, and we need not set it from PiCpuSmmEntry(). According to "objdump --reloc", the X64 version undergoes only the following relocations, after this patch: > RELOCATION RECORDS FOR [.text]: > OFFSET TYPE VALUE > 0000000000000095 R_X86_64_PC32 SmmInitHandler-0x0000000000000004 > 00000000000000e0 R_X86_64_PC32 mRebasedFlag-0x0000000000000004 > 00000000000000ea R_X86_64_PC32 mSmmRelocationOriginalAddress-0x0000000000000004 Therefore the patch does not regress ("Enable XCODE5 tool chain for UefiCpuPkg with nasm source code"). Cc: Eric Dong Cc: Michael D Kinney Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=866 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek Reviewed-by: Liming Gao --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 6 +----- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 7 ------- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 11 ----------- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm | 11 ++++++----- 4 files changed, 7 insertions(+), 28 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm index 0f62fe4487..f59413d9d4 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm @@ -25,7 +25,6 @@ extern ASM_PFX(mSmmRelocationOriginalAddress) global ASM_PFX(gPatchSmmCr3) global ASM_PFX(gPatchSmmCr4) global ASM_PFX(gPatchSmmCr0) -global ASM_PFX(gSmmJmpAddr) global ASM_PFX(gSmmInitStack) global ASM_PFX(gcSmiInitGdtr) global ASM_PFX(gcSmmInitSize) @@ -64,10 +63,7 @@ ASM_PFX(gPatchSmmCr4): ASM_PFX(gPatchSmmCr0): mov di, PROTECT_MODE_DS mov cr0, eax - DB 0x66, 0xea ; jmp far [ptr48] -ASM_PFX(gSmmJmpAddr): - DD @32bit - DW PROTECT_MODE_CS + jmp PROTECT_MODE_CS : dword @32bit BITS 32 @32bit: diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c index f602d86d51..0c8a4543d8 100755 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c @@ -569,13 +569,6 @@ PiCpuSmmEntry ( EFI_COMPUTING_UNIT_HOST_PROCESSOR | EFI_CU_HP_PC_SMM_INIT ); - // - // Fix segment address of the long-mode-switch jump - // - if (sizeof (UINTN) == sizeof (UINT64)) { - gSmmJmpAddr.Segment = LONG_MODE_CODE_SEGMENT; - } - // // Find out SMRR Base and SMRR Size // diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h index 8344e0653a..d897d4353e 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -295,17 +295,6 @@ WriteSaveStateRegister ( IN CONST VOID *Buffer ); -// -// -// -typedef struct { - UINT32 Offset; - UINT16 Segment; - UINT16 Reserved; -} IA32_FAR_ADDRESS; - -extern IA32_FAR_ADDRESS gSmmJmpAddr; - extern CONST UINT8 gcSmmInitTemplate[]; extern CONST UINT16 gcSmmInitSize; X86_ASSEMBLY_PATCH_LABEL gPatchSmmCr0; diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm index 1a0667bd97..2460e1eb2d 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm @@ -25,7 +25,6 @@ extern ASM_PFX(mSmmRelocationOriginalAddress) global ASM_PFX(gPatchSmmCr3) global ASM_PFX(gPatchSmmCr4) global ASM_PFX(gPatchSmmCr0) -global ASM_PFX(gSmmJmpAddr) global ASM_PFX(gSmmInitStack) global ASM_PFX(gcSmiInitGdtr) global ASM_PFX(gcSmmInitSize) @@ -33,6 +32,8 @@ global ASM_PFX(gcSmmInitTemplate) global ASM_PFX(mRebasedFlagAddr32) global ASM_PFX(mSmmRelocationOriginalAddressPtr32) +%define LONG_MODE_CS 0x38 + DEFAULT REL SECTION .text @@ -66,8 +67,8 @@ ASM_PFX(gPatchSmmCr4): 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 + jmp LONG_MODE_CS : dword 0 ; offset will be patched to @LongMode +@PatchLongModeOffset: BITS 64 @LongMode: ; long-mode starts here @@ -141,8 +142,8 @@ ASM_PFX(mSmmRelocationOriginalAddressPtr32): dd 0 global ASM_PFX(PiSmmCpuSmmInitFixupAddress) ASM_PFX(PiSmmCpuSmmInitFixupAddress): lea rax, [@LongMode] - lea rcx, [ASM_PFX(gSmmJmpAddr)] - mov qword [rcx], rax + lea rcx, [@PatchLongModeOffset - 6] + mov dword [rcx], eax lea rax, [ASM_PFX(SmmStartup)] lea rcx, [@L1] -- 2.39.2