On 05/01/20 22:17, Lendacky, Thomas wrote:
BZ:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2340&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cd2ec699d2c644a55724008d7f141d96f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243137431443098&sdata=oTTju7144KZc8VCmQqu74UilIOzQyji9jlO%2BMJeZYyU%3D&reserved=0
Now that an XCODE5 specific CpuExceptionHandlerLib library is in place,
revert the changes made to the ExceptionHandlerAsm.nasm in commit
2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass XCODE5 tool
chain") so that binary patching of flash code is not performed.
Cc: Eric Dong <eric.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Liming Gao <liming....@intel.com>
Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
---
.../X64/ExceptionHandlerAsm.nasm | 25 +++++--------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 19198f273137..3814f9de3703 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -34,7 +34,7 @@ AsmIdtVectorBegin:
db 0x6a ; push #VectorNum
db ($ - AsmIdtVectorBegin) / ((AsmIdtVectorEnd - AsmIdtVectorBegin)
/ 32) ; VectorNum
push rax
- mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry)
+ mov rax, ASM_PFX(CommonInterruptEntry)
jmp rax
%endrep
AsmIdtVectorEnd:
@@ -44,8 +44,7 @@ HookAfterStubHeaderBegin:
@VectorNum:
db 0 ; 0 will be fixed
push rax
- mov rax, strict qword 0 ; mov rax, HookAfterStubHeaderEnd
-JmpAbsoluteAddress:
+ mov rax, HookAfterStubHeaderEnd
jmp rax
HookAfterStubHeaderEnd:
mov rax, rsp
@@ -257,7 +256,8 @@ HasErrorCode:
; and make sure RSP is 16-byte aligned
;
sub rsp, 4 * 8 + 8
- call ASM_PFX(CommonExceptionHandler)
+ mov rax, ASM_PFX(CommonExceptionHandler)
+ call rax
add rsp, 4 * 8 + 8
cli
@@ -365,24 +365,11 @@ DoIret:
; comments here for definition of address map
global ASM_PFX(AsmGetTemplateAddressMap)
ASM_PFX(AsmGetTemplateAddressMap):
- lea rax, [AsmIdtVectorBegin]
+ mov rax, AsmIdtVectorBegin
mov qword [rcx], rax
mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
- lea rax, [HookAfterStubHeaderBegin]
+ mov rax, HookAfterStubHeaderBegin
mov qword [rcx + 0x10], rax
-
-; Fix up CommonInterruptEntry address
- lea rax, [ASM_PFX(CommonInterruptEntry)]
- lea rcx, [AsmIdtVectorBegin]
-%rep 32
- mov qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin)],
rax
- add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
-%endrep
-; Fix up HookAfterStubHeaderEnd
- lea rax, [HookAfterStubHeaderEnd]
- lea rcx, [JmpAbsoluteAddress]
- mov qword [rcx - 8], rax
-
ret
;-------------------------------------------------------------------------------------
With this patch applied, the differences with the "original" remain:
$ git diff 2db0ccc2d7fe^..HEAD -- \
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index ba8993d84b0b..3814f9de3703 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -1,12 +1,6 @@
;------------------------------------------------------------------------------
;
-; Copyright (c) 2012 - 2014, Intel Corporation. All rights reserved.<BR>
-; This program and the accompanying materials
-; are licensed and made available under the terms and conditions of the BSD
License
-; which accompanies this distribution. The full text of the license may be
found at
-;
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensource.org%2Flicenses%2Fbsd-license.php&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cd2ec699d2c644a55724008d7f141d96f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243137431443098&sdata=SZAc83Y%2BwZauGcj47EDgc10fnxSucy2ljeI9PcaJSvE%3D&reserved=0.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+; Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
;
; Module Name:
;
This is expected.
@@ -189,17 +183,19 @@ HasErrorCode:
push rax
push rax
sidt [rsp]
- xchg rax, [rsp + 2]
- xchg rax, [rsp]
- xchg rax, [rsp + 8]
+ mov bx, word [rsp]
+ mov rax, qword [rsp + 2]
+ mov qword [rsp], rax
+ mov word [rsp + 8], bx
xor rax, rax
push rax
push rax
sgdt [rsp]
- xchg rax, [rsp + 2]
- xchg rax, [rsp]
- xchg rax, [rsp + 8]
+ mov bx, word [rsp]
+ mov rax, qword [rsp + 2]
+ mov qword [rsp], rax
+ mov word [rsp + 8], bx
;; UINT64 Ldtr, Tr;
xor rax, rax
Also expected, from commit f4c898f2b2db
("UefiCpuPkg/CpuExceptionHandlerLib: Fix split lock", 2019-09-20).
Therefore, for this patch:
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
*However*, this revert must be restricted to the original
"SecPeiCpuExceptionHandlerLib.inf" instance, i.e. where binary patching
is not acceptable. (Otherwise, in combination with my request (1) under
patch#1, we'd needlessly break the PEI / DXE / SMM lib instances under
XCODE5.)
(1) Therefore, please insert a new patch between patches #1 and #2, such
that the new patch flip
- PeiCpuExceptionHandlerLib.inf
- DxeCpuExceptionHandlerLib.inf
- SmmCpuExceptionHandlerLib.inf
to using "Xcode5ExceptionHandlerAsm.nasm".
(If you wish, you can squash these modifications into the updated
patch#1, rather than inserting them as a separate patch between #1 and
#2.)
In summary, I suggest the following end-state:
- we should have a self-patching NASM file, and one without
self-patching,
- the self-patching variant should be called
"Xcode5ExceptionHandlerAsm.nasm" (because the *only* reason for the
self-patching is xcode5),
- we should have 5 INF files in total,
- "PeiCpuExceptionHandlerLib.inf", "DxeCpuExceptionHandlerLib.inf",
"SmmCpuExceptionHandlerLib.inf" should use
"Xcode5ExceptionHandlerAsm.nasm" (the self-patching is harmless),
- "SecPeiCpuExceptionHandlerLib.inf" should use
"ExceptionHandlerAsm.nasm" (self-patching is invalid, so don't do it),
- "Xcode5SecPeiCpuExceptionHandlerLib.inf" should use
"Xcode5ExceptionHandlerAsm.nasm" (the self-patching is invalid, but we
can't avoid it when building with XCODE5),
- platforms should resolve the CpuExceptionHandlerLib class to
"Xcode5SecPeiCpuExceptionHandlerLib.inf" only for the XCODE5 toolchain
*and* for the SEC phase.
