From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AFF8E225E4020 for ; Fri, 23 Mar 2018 14:08:49 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 835C9722D5; Fri, 23 Mar 2018 21:15:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-180.rdu2.redhat.com [10.10.120.180]) by smtp.corp.redhat.com (Postfix) with ESMTP id CD5DC2166BAE; Fri, 23 Mar 2018 21:15:21 +0000 (UTC) From: Laszlo Ersek To: edk2-devel-01 Cc: Eric Dong , Michael D Kinney Date: Fri, 23 Mar 2018 22:14:56 +0100 Message-Id: <20180323211504.22434-8-lersek@redhat.com> In-Reply-To: <20180323211504.22434-1-lersek@redhat.com> References: <20180323211504.22434-1-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 23 Mar 2018 21:15:22 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 23 Mar 2018 21:15:22 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: [PATCH v2 07/15] UefiCpuPkg/PiSmmCpuDxeSmm: patch "XdSupported" with PatchInstructionX86() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Mar 2018 21:08:50 -0000 "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 label 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" label (type X86_ASSEMBLY_PATCH_LABEL) 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 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 --- Notes: v2: - use the X86_ASSEMBLY_PATCH_LABEL type rather than UINT8 [Mike] UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 1 + UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 7 ++++--- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 7 ++++--- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 7 +++++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h index a21689145bb4..1613e9cd5cb9 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; +X86_ASSEMBLY_PATCH_LABEL gPatchXdSupported; extern UINTN *mPFEntryCount; extern UINT64 (*mLastPFEntryValue)[MAX_PF_ENTRY_COUNT]; extern UINT64 *(*mLastPFEntryPointer)[MAX_PF_ENTRY_COUNT]; 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 ; diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c index c90167f16060..b4fe0bc23b6c 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); } } -- 2.14.1.3.gb7cf6e02401b