* [PATCH 0/4] XCODE5 toolchain binary patching fix
@ 2020-05-01 20:17 Lendacky, Thomas
2020-05-01 20:17 ` [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific Lendacky, Thomas
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Lendacky, Thomas @ 2020-05-01 20:17 UTC (permalink / raw)
To: devel
Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao,
Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You,
Guo Dong, Julien Grall, Maurice Ma, Andrew Fish, Hao A Wu,
Jian J Wang, Michael D Kinney
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340
Commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass
XCODE5 tool chain") introduced binary patching in the
ExceptionHandlerAsm.nasm in order to support the XCODE5 toolchain.
However, the CpuExceptionHandlerLib can be used during SEC phase which
would result in binary patching of flash.
This series creates a new CpuExceptionHandlerLib file to support
the required binary patching for the XCODE5 toolchain, while reverting
the changes from commit 2db0ccc2d7fe in the standard file.
This is accomplished in phases:
- Create a new XCODE5 specific version of the ExceptionHandlerAsm.nasm
file
- Update the DSC files that use the CpuExceptionHandlerLib library to
to use the XCODE5 version of the library when the XCODE5 toolchain
is used.
- Revert the changes made by commit 2db0ccc2d7fe in the standard file.
I don't have access to an XCODE5 toolchain setup, so I have not tested
this with XCODE5. I would like to request that someone who does please
test this.
---
These patches are based on commit:
e54310451f1a ("OvmfPkg: Add VBE2 mode info structure to LegacyVgaBios.h")
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien@xen.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Tom Lendacky (4):
UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific
UefiPayloadPkg: Use toolchain appropriate CpuExceptionHandlerLib
OvmfPkg: Use toolchain appropriate CpuExceptionHandlerLib
UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard
CpuExceptionHandlerLib
OvmfPkg/OvmfPkgIa32.dsc | 20 +
OvmfPkg/OvmfPkgIa32X64.dsc | 20 +
OvmfPkg/OvmfPkgX64.dsc | 20 +
OvmfPkg/OvmfXen.dsc | 16 +
UefiCpuPkg/UefiCpuPkg.dsc | 23 +
UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 8 +
.../Xcode5DxeCpuExceptionHandlerLib.inf | 64 +++
.../Xcode5PeiCpuExceptionHandlerLib.inf | 63 +++
.../Xcode5SecPeiCpuExceptionHandlerLib.inf | 55 +++
.../Xcode5SmmCpuExceptionHandlerLib.inf | 59 +++
.../X64/ExceptionHandlerAsm.nasm | 25 +-
.../X64/Xcode5ExceptionHandlerAsm.nasm | 413 ++++++++++++++++++
12 files changed, 767 insertions(+), 19 deletions(-)
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
--
2.17.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific 2020-05-01 20:17 [PATCH 0/4] XCODE5 toolchain binary patching fix Lendacky, Thomas @ 2020-05-01 20:17 ` Lendacky, Thomas 2020-05-05 21:39 ` [edk2-devel] " Laszlo Ersek 2020-05-01 20:17 ` [PATCH 2/4] UefiPayloadPkg: Use toolchain appropriate CpuExceptionHandlerLib Lendacky, Thomas ` (3 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-01 20:17 UTC (permalink / raw) To: devel Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 Commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass XCODE5 tool chain") introduced binary patching into the exception handling support. CPU exception handling is allowed during SEC and this results in binary patching of flash, which should not be done. Separate the changes from commit 2db0ccc2d7fe into an XCODE5 toolchain specific file, Xcode5ExceptionHandlerAsm.nasm, and create new INF files for an XCODE5 version of CpuExceptionHandlerLib. Update the UefiCpuPkg.dsc file to use the new files when the XCODE5 toolchain is used. Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> --- UefiCpuPkg/UefiCpuPkg.dsc | 23 + .../Xcode5DxeCpuExceptionHandlerLib.inf | 64 +++ .../Xcode5PeiCpuExceptionHandlerLib.inf | 63 +++ .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 55 +++ .../Xcode5SmmCpuExceptionHandlerLib.inf | 59 +++ .../X64/Xcode5ExceptionHandlerAsm.nasm | 413 ++++++++++++++++++ 6 files changed, 677 insertions(+) create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc index d28cb5cccb52..ad298011232d 100644 --- a/UefiCpuPkg/UefiCpuPkg.dsc +++ b/UefiCpuPkg/UefiCpuPkg.dsc @@ -59,7 +59,11 @@ [LibraryClasses] [LibraryClasses.common.SEC] PlatformSecLib|UefiCpuPkg/Library/PlatformSecLibNull/PlatformSecLibNull.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf +!endif HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf @@ -73,12 +77,20 @@ [LibraryClasses.common.PEIM] [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM] PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf +!endif [LibraryClasses.common.DXE_DRIVER] MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf RegisterCpuFeaturesLib|UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf @@ -86,7 +98,11 @@ [LibraryClasses.common.DXE_SMM_DRIVER] SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf +!endif [LibraryClasses.common.UEFI_APPLICATION] UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf @@ -122,10 +138,17 @@ [Components.IA32, Components.X64] UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf +!else + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf +!endif UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf new file mode 100644 index 000000000000..ef37efec6246 --- /dev/null +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf @@ -0,0 +1,64 @@ +## @file +# CPU Exception Handler library instance for DXE modules. +# +# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = Xcode5DxeCpuExceptionHandlerLib + MODULE_UNI_FILE = DxeCpuExceptionHandlerLib.uni + FILE_GUID = B6E9835A-EDCF-4748-98A8-27D3C722E02D + MODULE_TYPE = DXE_DRIVER + VERSION_STRING = 1.1 + LIBRARY_CLASS = CpuExceptionHandlerLib|DXE_CORE DXE_DRIVER UEFI_APPLICATION + +# +# The following information is for reference only and not required by the build tools. +# +# VALID_ARCHITECTURES = IA32 X64 +# + +[Sources.Ia32] + Ia32/ExceptionHandlerAsm.nasm + Ia32/ExceptionTssEntryAsm.nasm + Ia32/ArchExceptionHandler.c + Ia32/ArchInterruptDefs.h + Ia32/ArchAMDSevVcHandler.c + +[Sources.X64] + X64/Xcode5ExceptionHandlerAsm.nasm + X64/ArchExceptionHandler.c + X64/ArchInterruptDefs.h + X64/ArchAMDSevVcHandler.c + +[Sources.common] + CpuExceptionCommon.h + CpuExceptionCommon.c + PeiDxeSmmCpuException.c + DxeException.c + AMDSevVcHandler.c + AMDSevVcCommon.h + +[Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard + gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList + gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + UefiCpuPkg/UefiCpuPkg.dec + +[LibraryClasses] + BaseLib + SerialPortLib + PrintLib + SynchronizationLib + LocalApicLib + PeCoffGetEntryPointLib + MemoryAllocationLib + DebugLib + VmgExitLib diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf new file mode 100644 index 000000000000..830ed1eb8bad --- /dev/null +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf @@ -0,0 +1,63 @@ +## @file +# CPU Exception Handler library instance for PEI module. +# +# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR> +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = Xcode5PeiCpuExceptionHandlerLib + MODULE_UNI_FILE = PeiCpuExceptionHandlerLib.uni + FILE_GUID = 980DDA67-44A6-4897-99E6-275290B71F9E + MODULE_TYPE = PEIM + VERSION_STRING = 1.1 + LIBRARY_CLASS = CpuExceptionHandlerLib|PEI_CORE PEIM + +# +# The following information is for reference only and not required by the build tools. +# +# VALID_ARCHITECTURES = IA32 X64 +# + +[Sources.Ia32] + Ia32/ExceptionHandlerAsm.nasm + Ia32/ExceptionTssEntryAsm.nasm + Ia32/ArchExceptionHandler.c + Ia32/ArchInterruptDefs.h + Ia32/ArchAMDSevVcHandler.c + +[Sources.X64] + X64/Xcode5ExceptionHandlerAsm.nasm + X64/ArchExceptionHandler.c + X64/ArchInterruptDefs.h + X64/ArchAMDSevVcHandler.c + +[Sources.common] + CpuExceptionCommon.h + CpuExceptionCommon.c + PeiCpuException.c + PeiDxeSmmCpuException.c + AMDSevVcHandler.c + AMDSevVcCommon.h + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + UefiCpuPkg/UefiCpuPkg.dec + +[LibraryClasses] + BaseLib + SerialPortLib + PrintLib + LocalApicLib + PeCoffGetEntryPointLib + HobLib + MemoryAllocationLib + SynchronizationLib + VmgExitLib + +[Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard # CONSUMES + diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf new file mode 100644 index 000000000000..36420be22faa --- /dev/null +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf @@ -0,0 +1,55 @@ +## @file +# CPU Exception Handler library instance for SEC/PEI modules. +# +# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR> +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = Xcode5SecPeiCpuExceptionHandlerLib + MODULE_UNI_FILE = SecPeiCpuExceptionHandlerLib.uni + FILE_GUID = CA4BBC99-DFC6-4234-B553-8B6586B7B113 + MODULE_TYPE = PEIM + VERSION_STRING = 1.1 + LIBRARY_CLASS = CpuExceptionHandlerLib|SEC PEI_CORE PEIM + +# +# The following information is for reference only and not required by the build tools. +# +# VALID_ARCHITECTURES = IA32 X64 +# + +[Sources.Ia32] + Ia32/ExceptionHandlerAsm.nasm + Ia32/ExceptionTssEntryAsm.nasm + Ia32/ArchExceptionHandler.c + Ia32/ArchInterruptDefs.h + Ia32/ArchAMDSevVcHandler.c + +[Sources.X64] + X64/Xcode5ExceptionHandlerAsm.nasm + X64/ArchExceptionHandler.c + X64/ArchInterruptDefs.h + X64/ArchAMDSevVcHandler.c + +[Sources.common] + CpuExceptionCommon.h + CpuExceptionCommon.c + SecPeiCpuException.c + AMDSevVcHandler.c + AMDSevVcCommon.h + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + UefiCpuPkg/UefiCpuPkg.dec + +[LibraryClasses] + BaseLib + SerialPortLib + PrintLib + LocalApicLib + PeCoffGetEntryPointLib + VmgExitLib diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf new file mode 100644 index 000000000000..8f71a45c86d5 --- /dev/null +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf @@ -0,0 +1,59 @@ +## @file +# CPU Exception Handler library instance for SMM modules. +# +# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = Xcode5SmmCpuExceptionHandlerLib + MODULE_UNI_FILE = SmmCpuExceptionHandlerLib.uni + FILE_GUID = 8D2C439B-3981-42ff-9CE5-1B50ECA502D6 + MODULE_TYPE = DXE_SMM_DRIVER + VERSION_STRING = 1.1 + LIBRARY_CLASS = CpuExceptionHandlerLib|DXE_SMM_DRIVER + +# +# The following information is for reference only and not required by the build tools. +# +# VALID_ARCHITECTURES = IA32 X64 +# + +[Sources.Ia32] + Ia32/ExceptionHandlerAsm.nasm + Ia32/ExceptionTssEntryAsm.nasm + Ia32/ArchExceptionHandler.c + Ia32/ArchInterruptDefs.h + Ia32/ArchAMDSevVcHandler.c + +[Sources.X64] + X64/Xcode5ExceptionHandlerAsm.nasm + X64/ArchExceptionHandler.c + X64/ArchInterruptDefs.h + X64/ArchAMDSevVcHandler.c + +[Sources.common] + CpuExceptionCommon.h + CpuExceptionCommon.c + PeiDxeSmmCpuException.c + SmmException.c + AMDSevVcHandler.c + AMDSevVcCommon.h + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + UefiCpuPkg/UefiCpuPkg.dec + +[LibraryClasses] + BaseLib + SerialPortLib + PrintLib + SynchronizationLib + LocalApicLib + PeCoffGetEntryPointLib + DebugLib + VmgExitLib + diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm new file mode 100644 index 000000000000..26cae56cc5cf --- /dev/null +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm @@ -0,0 +1,413 @@ +;------------------------------------------------------------------------------ ; +; Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR> +; SPDX-License-Identifier: BSD-2-Clause-Patent +; +; Module Name: +; +; ExceptionHandlerAsm.Asm +; +; Abstract: +; +; x64 CPU Exception Handler +; +; Notes: +; +;------------------------------------------------------------------------------ + +; +; CommonExceptionHandler() +; + +%define VC_EXCEPTION 29 + +extern ASM_PFX(mErrorCodeFlag) ; Error code flags for exceptions +extern ASM_PFX(mDoFarReturnFlag) ; Do far return flag +extern ASM_PFX(CommonExceptionHandler) + +SECTION .data + +DEFAULT REL +SECTION .text + +ALIGN 8 + +AsmIdtVectorBegin: +%rep 32 + db 0x6a ; push #VectorNum + db ($ - AsmIdtVectorBegin) / ((AsmIdtVectorEnd - AsmIdtVectorBegin) / 32) ; VectorNum + push rax + mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry) + jmp rax +%endrep +AsmIdtVectorEnd: + +HookAfterStubHeaderBegin: + db 0x6a ; push +@VectorNum: + db 0 ; 0 will be fixed + push rax + mov rax, strict qword 0 ; mov rax, HookAfterStubHeaderEnd +JmpAbsoluteAddress: + jmp rax +HookAfterStubHeaderEnd: + mov rax, rsp + and sp, 0xfff0 ; make sure 16-byte aligned for exception context + sub rsp, 0x18 ; reserve room for filling exception data later + push rcx + mov rcx, [rax + 8] + bt [ASM_PFX(mErrorCodeFlag)], ecx + jnc .0 + push qword [rsp] ; push additional rcx to make stack alignment +.0: + xchg rcx, [rsp] ; restore rcx, save Exception Number in stack + push qword [rax] ; push rax into stack to keep code consistence + +;---------------------------------------; +; CommonInterruptEntry ; +;---------------------------------------; +; The follow algorithm is used for the common interrupt routine. +; Entry from each interrupt with a push eax and eax=interrupt number +; Stack frame would be as follows as specified in IA32 manuals: +; +; +---------------------+ <-- 16-byte aligned ensured by processor +; + Old SS + +; +---------------------+ +; + Old RSP + +; +---------------------+ +; + RFlags + +; +---------------------+ +; + CS + +; +---------------------+ +; + RIP + +; +---------------------+ +; + Error Code + +; +---------------------+ +; + Vector Number + +; +---------------------+ +; + RBP + +; +---------------------+ <-- RBP, 16-byte aligned +; The follow algorithm is used for the common interrupt routine. +global ASM_PFX(CommonInterruptEntry) +ASM_PFX(CommonInterruptEntry): + cli + pop rax + ; + ; All interrupt handlers are invoked through interrupt gates, so + ; IF flag automatically cleared at the entry point + ; + xchg rcx, [rsp] ; Save rcx into stack and save vector number into rcx + and rcx, 0xFF + cmp ecx, 32 ; Intel reserved vector for exceptions? + jae NoErrorCode + bt [ASM_PFX(mErrorCodeFlag)], ecx + jc HasErrorCode + +NoErrorCode: + + ; + ; Push a dummy error code on the stack + ; to maintain coherent stack map + ; + push qword [rsp] + mov qword [rsp + 8], 0 +HasErrorCode: + push rbp + mov rbp, rsp + push 0 ; clear EXCEPTION_HANDLER_CONTEXT.OldIdtHandler + push 0 ; clear EXCEPTION_HANDLER_CONTEXT.ExceptionDataFlag + + ; + ; Stack: + ; +---------------------+ <-- 16-byte aligned ensured by processor + ; + Old SS + + ; +---------------------+ + ; + Old RSP + + ; +---------------------+ + ; + RFlags + + ; +---------------------+ + ; + CS + + ; +---------------------+ + ; + RIP + + ; +---------------------+ + ; + Error Code + + ; +---------------------+ + ; + RCX / Vector Number + + ; +---------------------+ + ; + RBP + + ; +---------------------+ <-- RBP, 16-byte aligned + ; + + ; + ; Since here the stack pointer is 16-byte aligned, so + ; EFI_FX_SAVE_STATE_X64 of EFI_SYSTEM_CONTEXT_x64 + ; is 16-byte aligned + ; + +;; UINT64 Rdi, Rsi, Rbp, Rsp, Rbx, Rdx, Rcx, Rax; +;; UINT64 R8, R9, R10, R11, R12, R13, R14, R15; + push r15 + push r14 + push r13 + push r12 + push r11 + push r10 + push r9 + push r8 + push rax + push qword [rbp + 8] ; RCX + push rdx + push rbx + push qword [rbp + 48] ; RSP + push qword [rbp] ; RBP + push rsi + push rdi + +;; UINT64 Gs, Fs, Es, Ds, Cs, Ss; insure high 16 bits of each is zero + movzx rax, word [rbp + 56] + push rax ; for ss + movzx rax, word [rbp + 32] + push rax ; for cs + mov rax, ds + push rax + mov rax, es + push rax + mov rax, fs + push rax + mov rax, gs + push rax + + mov [rbp + 8], rcx ; save vector number + +;; UINT64 Rip; + push qword [rbp + 24] + +;; UINT64 Gdtr[2], Idtr[2]; + xor rax, rax + push rax + push rax + sidt [rsp] + 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] + mov bx, word [rsp] + mov rax, qword [rsp + 2] + mov qword [rsp], rax + mov word [rsp + 8], bx + +;; UINT64 Ldtr, Tr; + xor rax, rax + str ax + push rax + sldt ax + push rax + +;; UINT64 RFlags; + push qword [rbp + 40] + +;; UINT64 Cr0, Cr1, Cr2, Cr3, Cr4, Cr8; + mov rax, cr8 + push rax + mov rax, cr4 + or rax, 0x208 + mov cr4, rax + push rax + mov rax, cr3 + push rax + mov rax, cr2 + push rax + xor rax, rax + push rax + mov rax, cr0 + push rax + +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7; + cmp qword [rbp + 8], VC_EXCEPTION + je VcDebugRegs ; For SEV-ES (#VC) Debug registers ignored + + mov rax, dr7 + push rax + mov rax, dr6 + push rax + mov rax, dr3 + push rax + mov rax, dr2 + push rax + mov rax, dr1 + push rax + mov rax, dr0 + push rax + jmp DrFinish + +VcDebugRegs: +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception recursion + xor rax, rax + push rax + push rax + push rax + push rax + push rax + push rax + +DrFinish: +;; FX_SAVE_STATE_X64 FxSaveState; + sub rsp, 512 + mov rdi, rsp + db 0xf, 0xae, 0x7 ;fxsave [rdi] + +;; UEFI calling convention for x64 requires that Direction flag in EFLAGs is clear + cld + +;; UINT32 ExceptionData; + push qword [rbp + 16] + +;; Prepare parameter and call + mov rcx, [rbp + 8] + mov rdx, rsp + ; + ; Per X64 calling convention, allocate maximum parameter stack space + ; and make sure RSP is 16-byte aligned + ; + sub rsp, 4 * 8 + 8 + call ASM_PFX(CommonExceptionHandler) + add rsp, 4 * 8 + 8 + + cli +;; UINT64 ExceptionData; + add rsp, 8 + +;; FX_SAVE_STATE_X64 FxSaveState; + + mov rsi, rsp + db 0xf, 0xae, 0xE ; fxrstor [rsi] + add rsp, 512 + +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7; +;; Skip restoration of DRx registers to support in-circuit emualators +;; or debuggers set breakpoint in interrupt/exception context + add rsp, 8 * 6 + +;; UINT64 Cr0, Cr1, Cr2, Cr3, Cr4, Cr8; + pop rax + mov cr0, rax + add rsp, 8 ; not for Cr1 + pop rax + mov cr2, rax + pop rax + mov cr3, rax + pop rax + mov cr4, rax + pop rax + mov cr8, rax + +;; UINT64 RFlags; + pop qword [rbp + 40] + +;; UINT64 Ldtr, Tr; +;; UINT64 Gdtr[2], Idtr[2]; +;; Best not let anyone mess with these particular registers... + add rsp, 48 + +;; UINT64 Rip; + pop qword [rbp + 24] + +;; UINT64 Gs, Fs, Es, Ds, Cs, Ss; + pop rax + ; mov gs, rax ; not for gs + pop rax + ; mov fs, rax ; not for fs + ; (X64 will not use fs and gs, so we do not restore it) + pop rax + mov es, rax + pop rax + mov ds, rax + pop qword [rbp + 32] ; for cs + pop qword [rbp + 56] ; for ss + +;; UINT64 Rdi, Rsi, Rbp, Rsp, Rbx, Rdx, Rcx, Rax; +;; UINT64 R8, R9, R10, R11, R12, R13, R14, R15; + pop rdi + pop rsi + add rsp, 8 ; not for rbp + pop qword [rbp + 48] ; for rsp + pop rbx + pop rdx + pop rcx + pop rax + pop r8 + pop r9 + pop r10 + pop r11 + pop r12 + pop r13 + pop r14 + pop r15 + + mov rsp, rbp + pop rbp + add rsp, 16 + cmp qword [rsp - 32], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler + jz DoReturn + cmp qword [rsp - 40], 1 ; check EXCEPTION_HANDLER_CONTEXT.ExceptionDataFlag + jz ErrorCode + jmp qword [rsp - 32] +ErrorCode: + sub rsp, 8 + jmp qword [rsp - 24] + +DoReturn: + cmp qword [ASM_PFX(mDoFarReturnFlag)], 0 ; Check if need to do far return instead of IRET + jz DoIret + push rax + mov rax, rsp ; save old RSP to rax + mov rsp, [rsp + 0x20] + push qword [rax + 0x10] ; save CS in new location + push qword [rax + 0x8] ; save EIP in new location + push qword [rax + 0x18] ; save EFLAGS in new location + mov rax, [rax] ; restore rax + popfq ; restore EFLAGS + DB 0x48 ; prefix to composite "retq" with next "retf" + retf ; far return +DoIret: + iretq + +;------------------------------------------------------------------------------------- +; GetTemplateAddressMap (&AddressMap); +;------------------------------------------------------------------------------------- +; comments here for definition of address map +global ASM_PFX(AsmGetTemplateAddressMap) +ASM_PFX(AsmGetTemplateAddressMap): + lea rax, [AsmIdtVectorBegin] + mov qword [rcx], rax + mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32 + lea 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 + +;------------------------------------------------------------------------------------- +; AsmVectorNumFixup (*NewVectorAddr, VectorNum, *OldVectorAddr); +;------------------------------------------------------------------------------------- +global ASM_PFX(AsmVectorNumFixup) +ASM_PFX(AsmVectorNumFixup): + mov rax, rdx + mov [rcx + (@VectorNum - HookAfterStubHeaderBegin)], al + ret + -- 2.17.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific 2020-05-01 20:17 ` [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific Lendacky, Thomas @ 2020-05-05 21:39 ` Laszlo Ersek 2020-05-05 22:09 ` Lendacky, Thomas 0 siblings, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2020-05-05 21:39 UTC (permalink / raw) To: devel, thomas.lendacky Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish Hi Tom, On 05/01/20 22:17, Lendacky, Thomas wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 > > Commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass > XCODE5 tool chain") introduced binary patching into the exception handling > support. CPU exception handling is allowed during SEC and this results in > binary patching of flash, which should not be done. > > Separate the changes from commit 2db0ccc2d7fe into an XCODE5 toolchain > specific file, Xcode5ExceptionHandlerAsm.nasm, and create new INF files > for an XCODE5 version of CpuExceptionHandlerLib. Update the UefiCpuPkg.dsc > file to use the new files when the XCODE5 toolchain is used. > > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Liming Gao <liming.gao@intel.com> > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > UefiCpuPkg/UefiCpuPkg.dsc | 23 + > .../Xcode5DxeCpuExceptionHandlerLib.inf | 64 +++ > .../Xcode5PeiCpuExceptionHandlerLib.inf | 63 +++ > .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 55 +++ > .../Xcode5SmmCpuExceptionHandlerLib.inf | 59 +++ I don't think that paralleling all the existent INF files is necessary for XCODE5. The binary patching is a problem when the UEFI module containing the self-patching CpuExceptionHandlerLib instance executes in-place from flash. That applies to: (a) SEC modules, (b) PEI modules that do *not* have a DEPEX on "gEfiPeiMemoryDiscoveredPpiGuid". (PEIMs that do have a DEPEX on that PPI GUID are only dispatched after the permanent PEI RAM has been discovered / published, so they run out of normal RAM.) Reviewing the existent INF files, we have: - DxeCpuExceptionHandlerLib.inf: for DXE_CORE, DXE_DRIVER, UEFI_APPLICATION modules. Self-patching is fine. - SmmCpuExceptionHandlerLib.inf: for DXE_SMM_DRIVER modules. Self-patching is fine. - SecPeiCpuExceptionHandlerLib.inf: SEC is listed explicitly, so here we certainly need an alternative. - PeiCpuExceptionHandlerLib.inf: unfortunately, the differences of this library instance with "SecPeiCpuExceptionHandlerLib.inf" is not obvious; only SEC's absence is easily visible. If we look at the commit that introduced this lib instance (a81abf161666, "UefiCpuPkg/ExceptionLib: Import PeiCpuExceptionHandlerLib module", 2016-06-01), we find: > This module could be linked by CpuMpPei driver to handle reserved vector list > and provide spin lock for BSP/APs to prevent dump message corrupted. So the library was added explicitly for CpuMpPei's sake -- which looks promising, because CpuMpPei certainly depends on "gEfiPeiMemoryDiscoveredPpiGuid", as it needs a bunch of RAM for offering the multi-processing PPI. That suggests the self-patching is OK in "PeiCpuExceptionHandlerLib.inf" too. The CpuMpPei DEPEX in question was replaced with a PPI notify callback in commit 0a0d5296e448 ("UefiCpuPkg/CpuMpPei: support stack guard feature", 2018-09-10). This would be a problem if the self-patching in the PeiCpuExceptionHandlerLib instance occurred in the library constructor, because the CpuMpPei can now actually be dispatched before permanent PEI RAM is available -- and the constructor would run immediately. Luckily, the lib instance has no CONSTRUCTOR at all, and CpuMpPei calls InitializeCpuExceptionHandlers() explicitly in InitializeCpuMpWorker(), which is the PPI notify in question. (And per <https://bugzilla.tianocore.org/show_bug.cgi?id=2340#c0>, the self-patching occurs in InitializeCpuExceptionHandlers().) Therefore, having two variants of PeiCpuExceptionHandlerLib.inf is also unnecessary. (1) We only need two variants for "SecPeiCpuExceptionHandlerLib.inf", in my opinion. (Note: if we check OVMF, we see that PeiCpuExceptionHandlerLib.inf is used universally for PEIMs. That's because OVMF is special -- its PEI phase runs entirely out of RAM. See also commit f0e6a56a9a2f, "OvmfPkg: include UefiCpuPkg/CpuMpPei", 2016-07-15.) --*-- With this patch applied: $ diff -u \ SecPeiCpuExceptionHandlerLib.inf \ Xcode5SecPeiCpuExceptionHandlerLib.inf > --- SecPeiCpuExceptionHandlerLib.inf 2020-05-05 18:36:12.813156743 +0200 > +++ Xcode5SecPeiCpuExceptionHandlerLib.inf 2020-05-05 23:25:24.578572971 +0200 > @@ -8,7 +8,7 @@ > > [Defines] > INF_VERSION = 0x00010005 > - BASE_NAME = SecPeiCpuExceptionHandlerLib > + BASE_NAME = Xcode5SecPeiCpuExceptionHandlerLib OK > MODULE_UNI_FILE = SecPeiCpuExceptionHandlerLib.uni (2) We'll need a separate UNI file here -- also we should customize the file-top comment in the INF file -- that explains the difference between the XCODE5 and non-XCODE5 variants, briefly. > FILE_GUID = CA4BBC99-DFC6-4234-B553-8B6586B7B113 (3) Please generate a new FILE_GUID with "uuidgen". > MODULE_TYPE = PEIM > @@ -26,16 +26,20 @@ > Ia32/ExceptionTssEntryAsm.nasm > Ia32/ArchExceptionHandler.c > Ia32/ArchInterruptDefs.h > + Ia32/ArchAMDSevVcHandler.c (4) Even though the blurb says that this series is based on edk2 commit e54310451f1a, some SEV-ES specific parts remain in this patch, and should be eliminated. The first example is above. > > [Sources.X64] > - X64/ExceptionHandlerAsm.nasm > + X64/Xcode5ExceptionHandlerAsm.nasm > X64/ArchExceptionHandler.c > X64/ArchInterruptDefs.h > + X64/ArchAMDSevVcHandler.c (5) Another SEV-ES change. > > [Sources.common] > CpuExceptionCommon.h > CpuExceptionCommon.c > SecPeiCpuException.c > + AMDSevVcHandler.c > + AMDSevVcCommon.h (6) ditto > > [Packages] > MdePkg/MdePkg.dec > @@ -48,3 +52,4 @@ > PrintLib > LocalApicLib > PeCoffGetEntryPointLib > + VmgExitLib (7) ditto Furthermore: $ diff -u \ ExceptionHandlerAsm.nasm \ Xcode5ExceptionHandlerAsm.nasm > --- ExceptionHandlerAsm.nasm 2020-05-05 23:26:30.941784203 +0200 > +++ Xcode5ExceptionHandlerAsm.nasm 2020-05-05 23:25:24.578572971 +0200 > @@ -18,6 +18,8 @@ > ; CommonExceptionHandler() > ; > > +%define VC_EXCEPTION 29 > + > extern ASM_PFX(mErrorCodeFlag) ; Error code flags for exceptions > extern ASM_PFX(mDoFarReturnFlag) ; Do far return flag > extern ASM_PFX(CommonExceptionHandler) > @@ -225,6 +227,9 @@ > push rax > > ;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7; > + cmp qword [rbp + 8], VC_EXCEPTION > + je VcDebugRegs ; For SEV-ES (#VC) Debug registers ignored > + > mov rax, dr7 > push rax > mov rax, dr6 > @@ -237,7 +242,19 @@ > push rax > mov rax, dr0 > push rax > + jmp DrFinish > + > +VcDebugRegs: > +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception recursion > + xor rax, rax > + push rax > + push rax > + push rax > + push rax > + push rax > + push rax > > +DrFinish: > ;; FX_SAVE_STATE_X64 FxSaveState; > sub rsp, 512 > mov rdi, rsp (8) All of these should be removed -- they should be part of your SEV-ES series, on top of this set. Thanks, Laszlo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific 2020-05-05 21:39 ` [edk2-devel] " Laszlo Ersek @ 2020-05-05 22:09 ` Lendacky, Thomas 0 siblings, 0 replies; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-05 22:09 UTC (permalink / raw) To: Laszlo Ersek, devel Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish On 5/5/20 4:39 PM, Laszlo Ersek wrote: > Hi Tom, > > 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%7C4d398c73a4bc4674d36608d7f13cce1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243115777198329&sdata=YA9qmWhZDrwGCchGXKDmOQPFqXdDztokQSZeZIq6u18%3D&reserved=0 >> >> Commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass >> XCODE5 tool chain") introduced binary patching into the exception handling >> support. CPU exception handling is allowed during SEC and this results in >> binary patching of flash, which should not be done. >> >> Separate the changes from commit 2db0ccc2d7fe into an XCODE5 toolchain >> specific file, Xcode5ExceptionHandlerAsm.nasm, and create new INF files >> for an XCODE5 version of CpuExceptionHandlerLib. Update the UefiCpuPkg.dsc >> file to use the new files when the XCODE5 toolchain is used. >> >> Cc: Eric Dong <eric.dong@intel.com> >> Cc: Ray Ni <ray.ni@intel.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Liming Gao <liming.gao@intel.com> >> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> >> --- >> UefiCpuPkg/UefiCpuPkg.dsc | 23 + >> .../Xcode5DxeCpuExceptionHandlerLib.inf | 64 +++ >> .../Xcode5PeiCpuExceptionHandlerLib.inf | 63 +++ >> .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 55 +++ >> .../Xcode5SmmCpuExceptionHandlerLib.inf | 59 +++ > > I don't think that paralleling all the existent INF files is necessary > for XCODE5. > > The binary patching is a problem when the UEFI module containing the > self-patching CpuExceptionHandlerLib instance executes in-place from > flash. That applies to: (a) SEC modules, (b) PEI modules that do *not* > have a DEPEX on "gEfiPeiMemoryDiscoveredPpiGuid". (PEIMs that do have a > DEPEX on that PPI GUID are only dispatched after the permanent PEI RAM > has been discovered / published, so they run out of normal RAM.) > > Reviewing the existent INF files, we have: > > - DxeCpuExceptionHandlerLib.inf: for DXE_CORE, DXE_DRIVER, > UEFI_APPLICATION modules. Self-patching is fine. > > - SmmCpuExceptionHandlerLib.inf: for DXE_SMM_DRIVER modules. > Self-patching is fine. > > - SecPeiCpuExceptionHandlerLib.inf: SEC is listed explicitly, so here we > certainly need an alternative. > > - PeiCpuExceptionHandlerLib.inf: unfortunately, the differences of this > library instance with "SecPeiCpuExceptionHandlerLib.inf" is not obvious; > only SEC's absence is easily visible. > > If we look at the commit that introduced this lib instance > (a81abf161666, "UefiCpuPkg/ExceptionLib: Import > PeiCpuExceptionHandlerLib module", 2016-06-01), we find: > >> This module could be linked by CpuMpPei driver to handle reserved vector list >> and provide spin lock for BSP/APs to prevent dump message corrupted. > > So the library was added explicitly for CpuMpPei's sake -- which looks > promising, because CpuMpPei certainly depends on > "gEfiPeiMemoryDiscoveredPpiGuid", as it needs a bunch of RAM for > offering the multi-processing PPI. That suggests the self-patching is OK > in "PeiCpuExceptionHandlerLib.inf" too. > > The CpuMpPei DEPEX in question was replaced with a PPI notify callback > in commit 0a0d5296e448 ("UefiCpuPkg/CpuMpPei: support stack guard > feature", 2018-09-10). This would be a problem if the self-patching in > the PeiCpuExceptionHandlerLib instance occurred in the library > constructor, because the CpuMpPei can now actually be dispatched before > permanent PEI RAM is available -- and the constructor would run > immediately. > > Luckily, the lib instance has no CONSTRUCTOR at all, and CpuMpPei calls > InitializeCpuExceptionHandlers() explicitly in InitializeCpuMpWorker(), > which is the PPI notify in question. (And per > <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2340%23c0&data=02%7C01%7Cthomas.lendacky%40amd.com%7C4d398c73a4bc4674d36608d7f13cce1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243115777198329&sdata=1ucjYymvr7VAF2d2QGyCxQhE8hGe8AFMaW8EYbUUkdM%3D&reserved=0>, the > self-patching occurs in InitializeCpuExceptionHandlers().) > > Therefore, having two variants of PeiCpuExceptionHandlerLib.inf is also > unnecessary. > > (1) We only need two variants for "SecPeiCpuExceptionHandlerLib.inf", in > my opinion. Ok, I'll rework it to have variants for just SecPeiCpuExceptionHandlerLib. > > (Note: if we check OVMF, we see that PeiCpuExceptionHandlerLib.inf is > used universally for PEIMs. That's because OVMF is special -- its PEI > phase runs entirely out of RAM. See also commit f0e6a56a9a2f, "OvmfPkg: > include UefiCpuPkg/CpuMpPei", 2016-07-15.) > > --*-- > > With this patch applied: > > $ diff -u \ > SecPeiCpuExceptionHandlerLib.inf \ > Xcode5SecPeiCpuExceptionHandlerLib.inf > >> --- SecPeiCpuExceptionHandlerLib.inf 2020-05-05 18:36:12.813156743 +0200 >> +++ Xcode5SecPeiCpuExceptionHandlerLib.inf 2020-05-05 23:25:24.578572971 +0200 >> @@ -8,7 +8,7 @@ >> >> [Defines] >> INF_VERSION = 0x00010005 >> - BASE_NAME = SecPeiCpuExceptionHandlerLib >> + BASE_NAME = Xcode5SecPeiCpuExceptionHandlerLib > > OK > >> MODULE_UNI_FILE = SecPeiCpuExceptionHandlerLib.uni > > (2) We'll need a separate UNI file here -- also we should customize the > file-top comment in the INF file -- that explains the difference between > the XCODE5 and non-XCODE5 variants, briefly. Ok, will do. > >> FILE_GUID = CA4BBC99-DFC6-4234-B553-8B6586B7B113 > > (3) Please generate a new FILE_GUID with "uuidgen". Ok, thanks, that answers my question that I asked in another email. > >> MODULE_TYPE = PEIM >> @@ -26,16 +26,20 @@ >> Ia32/ExceptionTssEntryAsm.nasm >> Ia32/ArchExceptionHandler.c >> Ia32/ArchInterruptDefs.h >> + Ia32/ArchAMDSevVcHandler.c > > (4) Even though the blurb says that this series is based on edk2 commit > e54310451f1a, some SEV-ES specific parts remain in this patch, and > should be eliminated. The first example is above. Ugh. I thought I had that all cleaned up before sending. My bad, I'll fix that in the next version. Thanks, Tom > >> >> [Sources.X64] >> - X64/ExceptionHandlerAsm.nasm >> + X64/Xcode5ExceptionHandlerAsm.nasm >> X64/ArchExceptionHandler.c >> X64/ArchInterruptDefs.h >> + X64/ArchAMDSevVcHandler.c > > (5) Another SEV-ES change. > >> >> [Sources.common] >> CpuExceptionCommon.h >> CpuExceptionCommon.c >> SecPeiCpuException.c >> + AMDSevVcHandler.c >> + AMDSevVcCommon.h > > (6) ditto > >> >> [Packages] >> MdePkg/MdePkg.dec >> @@ -48,3 +52,4 @@ >> PrintLib >> LocalApicLib >> PeCoffGetEntryPointLib >> + VmgExitLib > > (7) ditto > > Furthermore: > > $ diff -u \ > ExceptionHandlerAsm.nasm \ > Xcode5ExceptionHandlerAsm.nasm > >> --- ExceptionHandlerAsm.nasm 2020-05-05 23:26:30.941784203 +0200 >> +++ Xcode5ExceptionHandlerAsm.nasm 2020-05-05 23:25:24.578572971 +0200 >> @@ -18,6 +18,8 @@ >> ; CommonExceptionHandler() >> ; >> >> +%define VC_EXCEPTION 29 >> + >> extern ASM_PFX(mErrorCodeFlag) ; Error code flags for exceptions >> extern ASM_PFX(mDoFarReturnFlag) ; Do far return flag >> extern ASM_PFX(CommonExceptionHandler) >> @@ -225,6 +227,9 @@ >> push rax >> >> ;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7; >> + cmp qword [rbp + 8], VC_EXCEPTION >> + je VcDebugRegs ; For SEV-ES (#VC) Debug registers ignored >> + >> mov rax, dr7 >> push rax >> mov rax, dr6 >> @@ -237,7 +242,19 @@ >> push rax >> mov rax, dr0 >> push rax >> + jmp DrFinish >> + >> +VcDebugRegs: >> +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception recursion >> + xor rax, rax >> + push rax >> + push rax >> + push rax >> + push rax >> + push rax >> + push rax >> >> +DrFinish: >> ;; FX_SAVE_STATE_X64 FxSaveState; >> sub rsp, 512 >> mov rdi, rsp > > (8) All of these should be removed -- they should be part of your SEV-ES > series, on top of this set. > > Thanks, > Laszlo > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] UefiPayloadPkg: Use toolchain appropriate CpuExceptionHandlerLib 2020-05-01 20:17 [PATCH 0/4] XCODE5 toolchain binary patching fix Lendacky, Thomas 2020-05-01 20:17 ` [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific Lendacky, Thomas @ 2020-05-01 20:17 ` Lendacky, Thomas 2020-05-05 22:19 ` [edk2-devel] " Laszlo Ersek 2020-05-01 20:17 ` [PATCH 3/4] OvmfPkg: " Lendacky, Thomas ` (2 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-01 20:17 UTC (permalink / raw) To: devel Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 Use the XCODE5 CpuExceptionHandlerLib library in place of the standard library when building with the XCODE5 toolchain. The XCODE5 version of the library performs binary patching and should only be used when building with the XCODE5 toolchain. Cc: Maurice Ma <maurice.ma@intel.com> Cc: Guo Dong <guo.dong@intel.com> Cc: Benjamin You <benjamin.you@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> --- UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/UefiPayloadPkg/UefiPayloadPkgIa32.dsc b/UefiPayloadPkg/UefiPayloadPkgIa32.dsc index d52945442e0e..2bf7aafd8c77 100644 --- a/UefiPayloadPkg/UefiPayloadPkgIa32.dsc +++ b/UefiPayloadPkg/UefiPayloadPkgIa32.dsc @@ -232,7 +232,11 @@ [LibraryClasses.common.DXE_CORE] !if $(SOURCE_DEBUG_ENABLE) DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif [LibraryClasses.common.DXE_DRIVER] PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf @@ -243,7 +247,11 @@ [LibraryClasses.common.DXE_DRIVER] !if $(SOURCE_DEBUG_ENABLE) DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf [LibraryClasses.common.DXE_RUNTIME_DRIVER] -- 2.17.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 2/4] UefiPayloadPkg: Use toolchain appropriate CpuExceptionHandlerLib 2020-05-01 20:17 ` [PATCH 2/4] UefiPayloadPkg: Use toolchain appropriate CpuExceptionHandlerLib Lendacky, Thomas @ 2020-05-05 22:19 ` Laszlo Ersek 0 siblings, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2020-05-05 22:19 UTC (permalink / raw) To: devel, thomas.lendacky Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish On 05/01/20 22:17, Lendacky, Thomas wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 > > Use the XCODE5 CpuExceptionHandlerLib library in place of the standard > library when building with the XCODE5 toolchain. The XCODE5 version of > the library performs binary patching and should only be used when building > with the XCODE5 toolchain. > > Cc: Maurice Ma <maurice.ma@intel.com> > Cc: Guo Dong <guo.dong@intel.com> > Cc: Benjamin You <benjamin.you@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/UefiPayloadPkg/UefiPayloadPkgIa32.dsc b/UefiPayloadPkg/UefiPayloadPkgIa32.dsc > index d52945442e0e..2bf7aafd8c77 100644 > --- a/UefiPayloadPkg/UefiPayloadPkgIa32.dsc > +++ b/UefiPayloadPkg/UefiPayloadPkgIa32.dsc > @@ -232,7 +232,11 @@ [LibraryClasses.common.DXE_CORE] > !if $(SOURCE_DEBUG_ENABLE) > DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf > !endif > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > +!else > + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf > +!endif > > [LibraryClasses.common.DXE_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -243,7 +247,11 @@ [LibraryClasses.common.DXE_DRIVER] > !if $(SOURCE_DEBUG_ENABLE) > DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf > !endif > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > +!else > + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf > +!endif > MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > [LibraryClasses.common.DXE_RUNTIME_DRIVER] > This patch should be dropped, as it only modifies the lib class resolutions for DXE_CORE and DXE_DRIVER modules; in those modules, the self-patching is harmless. Thanks Laszlo ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] OvmfPkg: Use toolchain appropriate CpuExceptionHandlerLib 2020-05-01 20:17 [PATCH 0/4] XCODE5 toolchain binary patching fix Lendacky, Thomas 2020-05-01 20:17 ` [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific Lendacky, Thomas 2020-05-01 20:17 ` [PATCH 2/4] UefiPayloadPkg: Use toolchain appropriate CpuExceptionHandlerLib Lendacky, Thomas @ 2020-05-01 20:17 ` Lendacky, Thomas 2020-05-05 21:49 ` [edk2-devel] " Laszlo Ersek 2020-05-01 20:17 ` [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib Lendacky, Thomas [not found] ` <160B00E54624ADAA.10991@groups.io> 4 siblings, 1 reply; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-01 20:17 UTC (permalink / raw) To: devel Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish, Ard Biesheuvel BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 Use the XCODE5 CpuExceptionHandlerLib library in place of the standard library when building with the XCODE5 toolchain. The XCODE5 version of the library performs binary patching and should only be used when building with the XCODE5 toolchain. Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Anthony Perard <anthony.perard@citrix.com> Cc: Julien Grall <julien@xen.org> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> --- OvmfPkg/OvmfPkgIa32.dsc | 20 ++++++++++++++++++++ OvmfPkg/OvmfPkgIa32X64.dsc | 20 ++++++++++++++++++++ OvmfPkg/OvmfPkgX64.dsc | 20 ++++++++++++++++++++ OvmfPkg/OvmfXen.dsc | 16 ++++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index fcd9779b5ba2..f27fcd7e1087 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -245,7 +245,11 @@ [LibraryClasses.common.SEC] PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf +!endif [LibraryClasses.common.PEI_CORE] HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf @@ -283,7 +287,11 @@ [LibraryClasses.common.PEIM] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf +!endif MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf @@ -309,7 +317,11 @@ [LibraryClasses.common.DXE_CORE] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf [LibraryClasses.common.DXE_RUNTIME_DRIVER] @@ -362,7 +374,11 @@ [LibraryClasses.common.DXE_DRIVER] PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif !if $(SMM_REQUIRE) == TRUE LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf !else @@ -413,7 +429,11 @@ [LibraryClasses.common.DXE_SMM_DRIVER] !else DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf +!endif !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf !endif diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 1626d2415a2c..0d3c3559d15f 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -249,7 +249,11 @@ [LibraryClasses.common.SEC] PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf +!endif [LibraryClasses.common.PEI_CORE] HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf @@ -287,7 +291,11 @@ [LibraryClasses.common.PEIM] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf +!endif MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf @@ -313,7 +321,11 @@ [LibraryClasses.common.DXE_CORE] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf [LibraryClasses.common.DXE_RUNTIME_DRIVER] @@ -366,7 +378,11 @@ [LibraryClasses.common.DXE_DRIVER] PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif !if $(SMM_REQUIRE) == TRUE LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf !else @@ -417,7 +433,11 @@ [LibraryClasses.common.DXE_SMM_DRIVER] !else DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf +!endif !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf !endif diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 65cfe957761b..c713e65db6f7 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -249,7 +249,11 @@ [LibraryClasses.common.SEC] PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf +!endif [LibraryClasses.common.PEI_CORE] HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf @@ -287,7 +291,11 @@ [LibraryClasses.common.PEIM] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf +!endif MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf @@ -313,7 +321,11 @@ [LibraryClasses.common.DXE_CORE] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf [LibraryClasses.common.DXE_RUNTIME_DRIVER] @@ -366,7 +378,11 @@ [LibraryClasses.common.DXE_DRIVER] PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif !if $(SMM_REQUIRE) == TRUE LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf !else @@ -417,7 +433,11 @@ [LibraryClasses.common.DXE_SMM_DRIVER] !else DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf +!endif !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf !endif diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc index 8b3615e0b07e..4f1a4f7980a3 100644 --- a/OvmfPkg/OvmfXen.dsc +++ b/OvmfPkg/OvmfXen.dsc @@ -228,7 +228,11 @@ [LibraryClasses.common.SEC] PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf +!endif [LibraryClasses.common.PEI_CORE] HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf @@ -256,7 +260,11 @@ [LibraryClasses.common.PEIM] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf +!endif MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf @@ -271,7 +279,11 @@ [LibraryClasses.common.DXE_CORE] !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf !endif +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf [LibraryClasses.common.DXE_RUNTIME_DRIVER] @@ -306,7 +318,11 @@ [LibraryClasses.common.DXE_DRIVER] PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!else + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf +!endif LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf !if $(SOURCE_DEBUG_ENABLE) == TRUE DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf -- 2.17.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 3/4] OvmfPkg: Use toolchain appropriate CpuExceptionHandlerLib 2020-05-01 20:17 ` [PATCH 3/4] OvmfPkg: " Lendacky, Thomas @ 2020-05-05 21:49 ` Laszlo Ersek 0 siblings, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2020-05-05 21:49 UTC (permalink / raw) To: devel, thomas.lendacky Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish, Ard Biesheuvel On 05/01/20 22:17, Lendacky, Thomas wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 > > Use the XCODE5 CpuExceptionHandlerLib library in place of the standard > library when building with the XCODE5 toolchain. The XCODE5 version of > the library performs binary patching and should only be used when building > with the XCODE5 toolchain. > > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> > Cc: Anthony Perard <anthony.perard@citrix.com> > Cc: Julien Grall <julien@xen.org> > Cc: Liming Gao <liming.gao@intel.com> > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > OvmfPkg/OvmfPkgIa32.dsc | 20 ++++++++++++++++++++ > OvmfPkg/OvmfPkgIa32X64.dsc | 20 ++++++++++++++++++++ > OvmfPkg/OvmfPkgX64.dsc | 20 ++++++++++++++++++++ > OvmfPkg/OvmfXen.dsc | 16 ++++++++++++++++ > 4 files changed, 76 insertions(+) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index fcd9779b5ba2..f27fcd7e1087 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -245,7 +245,11 @@ [LibraryClasses.common.SEC] > PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf > PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf > MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf > +!else > + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > +!endif (1) In accordance with my point (1) under patch#1, please keep the SEC phase hunks only, for all four DSC files, and drop the rest of the hunks. Only SecPeiCpuExceptionHandlerLib needs to be conditionalized. (2) Style request: given that we have an "!else" branch too, please use the "==" operator with the "!if". I think that the double-negation arising from testing "!=" first, and then having an "!else", is hard to read. Thanks! Laszlo ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-01 20:17 [PATCH 0/4] XCODE5 toolchain binary patching fix Lendacky, Thomas ` (2 preceding siblings ...) 2020-05-01 20:17 ` [PATCH 3/4] OvmfPkg: " Lendacky, Thomas @ 2020-05-01 20:17 ` Lendacky, Thomas 2020-05-01 20:49 ` [EXTERNAL] [edk2-devel] " Bret Barkelew 2020-05-05 22:15 ` Laszlo Ersek [not found] ` <160B00E54624ADAA.10991@groups.io> 4 siblings, 2 replies; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-01 20:17 UTC (permalink / raw) To: devel Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 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.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Tom Lendacky <thomas.lendacky@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 ;------------------------------------------------------------------------------------- -- 2.17.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [EXTERNAL] [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-01 20:17 ` [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib Lendacky, Thomas @ 2020-05-01 20:49 ` Bret Barkelew 2020-05-05 22:15 ` Laszlo Ersek 1 sibling, 0 replies; 17+ messages in thread From: Bret Barkelew @ 2020-05-01 20:49 UTC (permalink / raw) To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish [-- Attachment #1: Type: text/plain, Size: 4258 bytes --] Acked-by: Bret Barkelew <bret.barkelew@microsoft.com> - Bret ________________________________ From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Lendacky, Thomas via groups.io <thomas.lendacky=amd.com@groups.io> Sent: Friday, May 1, 2020 1:17:41 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: Jordan Justen <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Liming Gao <liming.gao@intel.com>; Eric Dong <eric.dong@intel.com>; Ray Ni <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Anthony Perard <anthony.perard@citrix.com>; Benjamin You <benjamin.you@intel.com>; Guo Dong <guo.dong@intel.com>; Julien Grall <julien@xen.org>; Maurice Ma <maurice.ma@intel.com>; Andrew Fish <afish@apple.com> Subject: [EXTERNAL] [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib BZ: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2340&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc233bc94949a424f66bf08d7ee0cc626%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637239610952998859&sdata=rm%2FpiCDgatUHtU9PEQGWTzFT70XmkpbIukV7eiHWHeo%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.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Tom Lendacky <thomas.lendacky@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 ;------------------------------------------------------------------------------------- -- 2.17.1 [-- Attachment #2: Type: text/html, Size: 8304 bytes --] ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-01 20:17 ` [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib Lendacky, Thomas 2020-05-01 20:49 ` [EXTERNAL] [edk2-devel] " Bret Barkelew @ 2020-05-05 22:15 ` Laszlo Ersek 2020-05-06 14:35 ` Lendacky, Thomas 1 sibling, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2020-05-05 22:15 UTC (permalink / raw) To: devel, thomas.lendacky Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish On 05/01/20 22:17, Lendacky, Thomas wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340 > > 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.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Liming Gao <liming.gao@intel.com> > Signed-off-by: Tom Lendacky <thomas.lendacky@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 > -; http://opensource.org/licenses/bsd-license.php. > -; > -; 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 <lersek@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. Thanks, Laszlo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-05 22:15 ` Laszlo Ersek @ 2020-05-06 14:35 ` Lendacky, Thomas 2020-05-06 14:53 ` Liming Gao 2020-05-06 16:33 ` Laszlo Ersek 0 siblings, 2 replies; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-06 14:35 UTC (permalink / raw) To: devel, lersek Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish On 5/5/20 5:15 PM, Laszlo Ersek via groups.io wrote: > 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.dong@intel.com> >> Cc: Ray Ni <ray.ni@intel.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Liming Gao <liming.gao@intel.com> >> Signed-off-by: Tom Lendacky <thomas.lendacky@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 <lersek@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. Ok, I have v2 ready to go, but when I ran it through the integration tests using a pull request I received some errors (see https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=6516&view=results). The error is the same in all cases and the error message is: CRITICAL - UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf not in UefiCpuPkg/UefiCpuPkg.dsc Any idea about the reason for this message? Is it due to the [Components] section of the UefiCpuPkg/UefiCpuPkg.dsc file? Should that section not use the !if check and just list both .inf files (SecPeiCpuExceptionHandlerLib.inf and Xcode5SecPeiCpuExceptionHandlerLib.inf)? Thanks, Tom > > Thanks, > Laszlo > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-06 14:35 ` Lendacky, Thomas @ 2020-05-06 14:53 ` Liming Gao 2020-05-06 16:33 ` Laszlo Ersek 1 sibling, 0 replies; 17+ messages in thread From: Liming Gao @ 2020-05-06 14:53 UTC (permalink / raw) To: devel@edk2.groups.io, thomas.lendacky@amd.com, lersek@redhat.com Cc: Justen, Jordan L, Ard Biesheuvel, Dong, Eric, Ni, Ray, Brijesh Singh, Anthony Perard, You, Benjamin, Dong, Guo, Julien Grall, Ma, Maurice, Andrew Fish Thomas: > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Lendacky, Thomas > Sent: Wednesday, May 6, 2020 10:35 PM > To: devel@edk2.groups.io; lersek@redhat.com > Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gao, Liming <liming.gao@intel.com>; > Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Anthony Perard > <anthony.perard@citrix.com>; You, Benjamin <benjamin.you@intel.com>; Dong, Guo <guo.dong@intel.com>; Julien Grall > <julien@xen.org>; Ma, Maurice <maurice.ma@intel.com>; Andrew Fish <afish@apple.com> > Subject: Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib > > On 5/5/20 5:15 PM, Laszlo Ersek via groups.io wrote: > > 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&d > ata=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.dong@intel.com> > >> Cc: Ray Ni <ray.ni@intel.com> > >> Cc: Laszlo Ersek <lersek@redhat.com> > >> Cc: Liming Gao <liming.gao@intel.com> > >> Signed-off-by: Tom Lendacky <thomas.lendacky@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%7C3dd8961fe4884e608e > 11a82d994e183d%7C0%7C0%7C637243137431443098&sdata=SZAc83Y%2BwZauGcj47EDgc10fnxSucy2ljeI9PcaJSvE%3D&reser > ved=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 <lersek@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. > > Ok, I have v2 ready to go, but when I ran it through the integration tests > using a pull request I received some errors (see > https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=6516&view=results). > The error is the same in all cases and the error message is: > > CRITICAL - > UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > not in UefiCpuPkg/UefiCpuPkg.dsc > > Any idea about the reason for this message? Is it due to the [Components] > section of the UefiCpuPkg/UefiCpuPkg.dsc file? Should that section not use > the !if check and just list both .inf files > (SecPeiCpuExceptionHandlerLib.inf and Xcode5SecPeiCpuExceptionHandlerLib.inf)? > Yes. Package DSC [Components] section should list all module INF files, then verify their build. Thanks Liming > Thanks, > Tom > > > > > Thanks, > > Laszlo > > > > > > > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-06 14:35 ` Lendacky, Thomas 2020-05-06 14:53 ` Liming Gao @ 2020-05-06 16:33 ` Laszlo Ersek 2020-05-06 18:07 ` [EXTERNAL] " Bret Barkelew 1 sibling, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2020-05-06 16:33 UTC (permalink / raw) To: Tom Lendacky, devel Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish On 05/06/20 16:35, Tom Lendacky wrote: > On 5/5/20 5:15 PM, Laszlo Ersek via groups.io wrote: >> 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.dong@intel.com> >>> Cc: Ray Ni <ray.ni@intel.com> >>> Cc: Laszlo Ersek <lersek@redhat.com> >>> Cc: Liming Gao <liming.gao@intel.com> >>> Signed-off-by: Tom Lendacky <thomas.lendacky@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 <lersek@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. > > Ok, I have v2 ready to go, but when I ran it through the integration > tests using a pull request I received some errors (see > https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=6516&view=results). > The error is the same in all cases and the error message is: > > CRITICAL - > UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > not in UefiCpuPkg/UefiCpuPkg.dsc > > Any idea about the reason for this message? The reason is that the CI code found a library instance (INF) in UefiCpuPkg that cannot be built *stand-alone* (i.e. without being consumed by a different UEFI module / INF file). In core packages, library instances should be buildable stand-alone with the "-m" build flag, and for that, the lib instances need to be listed in the [Components] section. > Is it due to the > [Components] section of the UefiCpuPkg/UefiCpuPkg.dsc file? Yes. > Should that > section not use the !if check and just list both .inf files > (SecPeiCpuExceptionHandlerLib.inf and > Xcode5SecPeiCpuExceptionHandlerLib.inf)? Hmmm, this is a very good point; after all, the updated (=reverted) "SecPeiCpuExceptionHandlerLib.inf" instance will not build with XCODE5. Therefore we should list both lib instance INF files under [Components], but make "SecPeiCpuExceptionHandlerLib.inf" conditional on non-XCODE5. I wonder how the CI logic will cope with this! Thanks, Laszlo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [EXTERNAL] Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-06 16:33 ` Laszlo Ersek @ 2020-05-06 18:07 ` Bret Barkelew 2020-05-06 19:51 ` Lendacky, Thomas 0 siblings, 1 reply; 17+ messages in thread From: Bret Barkelew @ 2020-05-06 18:07 UTC (permalink / raw) To: devel@edk2.groups.io, lersek@redhat.com, Tom Lendacky Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish [-- Attachment #1: Type: text/plain, Size: 11812 bytes --] <Quote> > Should that > section not use the !if check and just list both .inf files > (SecPeiCpuExceptionHandlerLib.inf and > Xcode5SecPeiCpuExceptionHandlerLib.inf)? Hmmm, this is a very good point; after all, the updated (=reverted) "SecPeiCpuExceptionHandlerLib.inf" instance will not build with XCODE5. Therefore we should list both lib instance INF files under [Components], but make "SecPeiCpuExceptionHandlerLib.inf" conditional on non-XCODE5. </Quote> Xcode5SecPeiCpuExceptionHandlerLib.inf could be added to the ignore list in the CI yaml file. PR gating CI currently only uses GCC and VS2017/19 and shouldn’t have a problem with the reverted lib. This makes Xcode5SecPeiCpuExceptionHandlerLib the exception which can be documented in the ignore list (why it’s being ignored). Thoughts? - Bret From: Laszlo Ersek via groups.io<mailto:lersek=redhat.com@groups.io> Sent: Wednesday, May 6, 2020 9:33 AM To: Tom Lendacky<mailto:thomas.lendacky@amd.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Jordan Justen<mailto:jordan.l.justen@intel.com>; Ard Biesheuvel<mailto:ard.biesheuvel@linaro.org>; Liming Gao<mailto:liming.gao@intel.com>; Eric Dong<mailto:eric.dong@intel.com>; Ray Ni<mailto:ray.ni@intel.com>; Brijesh Singh<mailto:brijesh.singh@amd.com>; Anthony Perard<mailto:anthony.perard@citrix.com>; Benjamin You<mailto:benjamin.you@intel.com>; Guo Dong<mailto:guo.dong@intel.com>; Julien Grall<mailto:julien@xen.org>; Maurice Ma<mailto:maurice.ma@intel.com>; Andrew Fish<mailto:afish@apple.com> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib On 05/06/20 16:35, Tom Lendacky wrote: > On 5/5/20 5:15 PM, Laszlo Ersek via groups.io wrote: >> On 05/01/20 22:17, Lendacky, Thomas wrote: >>> BZ: >>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2340&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca9365fbefc53477c9e3308d7f1db4560%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637243796372782584&sdata=R%2B5IgncDzx7viv3yIwX3MloX1QlzuUJ4bZlKnc%2B0xoI%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.dong@intel.com> >>> Cc: Ray Ni <ray.ni@intel.com> >>> Cc: Laszlo Ersek <lersek@redhat.com> >>> Cc: Liming Gao <liming.gao@intel.com> >>> Signed-off-by: Tom Lendacky <thomas.lendacky@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://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensource.org%2Flicenses%2Fbsd-license.php&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca9365fbefc53477c9e3308d7f1db4560%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637243796372792580&sdata=CZovXRJ6HURgo6sz%2FDi55SUy9IsrJ2pFzcHX%2Bp%2Fv8qA%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 <lersek@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. > > Ok, I have v2 ready to go, but when I ran it through the integration > tests using a pull request I received some errors (see > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.com%2Ftianocore%2Fedk2-ci%2F_build%2Fresults%3FbuildId%3D6516%26view%3Dresults&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca9365fbefc53477c9e3308d7f1db4560%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637243796372792580&sdata=N2aYxzfr%2BNw7hkhBTEg0C%2Fm4Hv00xXBZhSz3%2FFgiW1s%3D&reserved=0). > The error is the same in all cases and the error message is: > > CRITICAL - > UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > not in UefiCpuPkg/UefiCpuPkg.dsc > > Any idea about the reason for this message? The reason is that the CI code found a library instance (INF) in UefiCpuPkg that cannot be built *stand-alone* (i.e. without being consumed by a different UEFI module / INF file). In core packages, library instances should be buildable stand-alone with the "-m" build flag, and for that, the lib instances need to be listed in the [Components] section. > Is it due to the > [Components] section of the UefiCpuPkg/UefiCpuPkg.dsc file? Yes. > Should that > section not use the !if check and just list both .inf files > (SecPeiCpuExceptionHandlerLib.inf and > Xcode5SecPeiCpuExceptionHandlerLib.inf)? Hmmm, this is a very good point; after all, the updated (=reverted) "SecPeiCpuExceptionHandlerLib.inf" instance will not build with XCODE5. Therefore we should list both lib instance INF files under [Components], but make "SecPeiCpuExceptionHandlerLib.inf" conditional on non-XCODE5. I wonder how the CI logic will cope with this! Thanks, Laszlo [-- Attachment #2: Type: text/html, Size: 20879 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [EXTERNAL] Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib 2020-05-06 18:07 ` [EXTERNAL] " Bret Barkelew @ 2020-05-06 19:51 ` Lendacky, Thomas 0 siblings, 0 replies; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-06 19:51 UTC (permalink / raw) To: devel, bret.barkelew, lersek@redhat.com Cc: Jordan Justen, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish On 5/6/20 1:07 PM, Bret Barkelew via groups.io wrote: > <Quote> > > > Should that > > section not use the !if check and just list both .inf files > > (SecPeiCpuExceptionHandlerLib.inf and > > Xcode5SecPeiCpuExceptionHandlerLib.inf)? > > Hmmm, this is a very good point; after all, the updated (=reverted) > "SecPeiCpuExceptionHandlerLib.inf" instance will not build with XCODE5. > Therefore we should list both lib instance INF files under [Components], > but make "SecPeiCpuExceptionHandlerLib.inf" conditional on non-XCODE5. > > </Quote> > > Xcode5SecPeiCpuExceptionHandlerLib.inf could be added to the ignore list > in the CI yaml file. PR gating CI currently only uses GCC and VS2017/19 > and shouldn’t have a problem with the reverted lib. This makes > Xcode5SecPeiCpuExceptionHandlerLib the exception which can be documented > in the ignore list (why it’s being ignored). > > Thoughts? I'll give those suggestions a try and see how they work. Thanks! Tom > > - Bret > > *From: *Laszlo Ersek via groups.io <mailto:lersek=redhat.com@groups.io> > *Sent: *Wednesday, May 6, 2020 9:33 AM > *To: *Tom Lendacky <mailto:thomas.lendacky@amd.com>; devel@edk2.groups.io > <mailto:devel@edk2.groups.io> > *Cc: *Jordan Justen <mailto:jordan.l.justen@intel.com>; Ard Biesheuvel > <mailto:ard.biesheuvel@linaro.org>; Liming Gao > <mailto:liming.gao@intel.com>; Eric Dong <mailto:eric.dong@intel.com>; Ray > Ni <mailto:ray.ni@intel.com>; Brijesh Singh > <mailto:brijesh.singh@amd.com>; Anthony Perard > <mailto:anthony.perard@citrix.com>; Benjamin You > <mailto:benjamin.you@intel.com>; Guo Dong <mailto:guo.dong@intel.com>; > Julien Grall <mailto:julien@xen.org>; Maurice Ma > <mailto:maurice.ma@intel.com>; Andrew Fish <mailto:afish@apple.com> > *Subject: *[EXTERNAL] Re: [edk2-devel] [PATCH 4/4] > UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard > CpuExceptionHandlerLib > > On 05/06/20 16:35, Tom Lendacky wrote: > > On 5/5/20 5:15 PM, Laszlo Ersek via groups.io wrote: > >> On 05/01/20 22:17, Lendacky, Thomas wrote: > >>> BZ: > >>> > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2340&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca9365fbefc53477c9e3308d7f1db4560%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637243796372782584&sdata=R%2B5IgncDzx7viv3yIwX3MloX1QlzuUJ4bZlKnc%2B0xoI%3D&reserved=0 > <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%7C4d968ebc52544accf1b608d7f1e865f8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243852756214038&sdata=p9f6gdfmOnbTMx7u7Ao4jw6jEqcZ2DVXENwHhwcETjo%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.dong@intel.com> > >>> Cc: Ray Ni <ray.ni@intel.com> > >>> Cc: Laszlo Ersek <lersek@redhat.com> > >>> Cc: Liming Gao <liming.gao@intel.com> > >>> Signed-off-by: Tom Lendacky <thomas.lendacky@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://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensource.org%2Flicenses%2Fbsd-license.php&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca9365fbefc53477c9e3308d7f1db4560%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637243796372792580&sdata=CZovXRJ6HURgo6sz%2FDi55SUy9IsrJ2pFzcHX%2Bp%2Fv8qA%3D&reserved=0 > <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensource.org%2Flicenses%2Fbsd-license.php&data=02%7C01%7Cthomas.lendacky%40amd.com%7C4d968ebc52544accf1b608d7f1e865f8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243852756214038&sdata=6TFEj80GpLSavcQeRvxn2uCPLVK6HxGN1yKB2PPS7Yk%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 <lersek@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. > > > > Ok, I have v2 ready to go, but when I ran it through the integration > > tests using a pull request I received some errors (see > > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.com%2Ftianocore%2Fedk2-ci%2F_build%2Fresults%3FbuildId%3D6516%26view%3Dresults&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca9365fbefc53477c9e3308d7f1db4560%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637243796372792580&sdata=N2aYxzfr%2BNw7hkhBTEg0C%2Fm4Hv00xXBZhSz3%2FFgiW1s%3D&reserved=0 > <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.com%2Ftianocore%2Fedk2-ci%2F_build%2Fresults%3FbuildId%3D6516%26view%3Dresults&data=02%7C01%7Cthomas.lendacky%40amd.com%7C4d968ebc52544accf1b608d7f1e865f8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243852756224001&sdata=xccjYXSqyhRJoGrINVWuLVsOql8pAWt5YQjU%2FSsZHWU%3D&reserved=0>). > > The error is the same in all cases and the error message is: > > > > CRITICAL - > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > > not in UefiCpuPkg/UefiCpuPkg.dsc > > > > Any idea about the reason for this message? > > The reason is that the CI code found a library instance (INF) in > UefiCpuPkg that cannot be built *stand-alone* (i.e. without being > consumed by a different UEFI module / INF file). > > In core packages, library instances should be buildable stand-alone with > the "-m" build flag, and for that, the lib instances need to be listed > in the [Components] section. > > > Is it due to the > > [Components] section of the UefiCpuPkg/UefiCpuPkg.dsc file? > > Yes. > > > Should that > > section not use the !if check and just list both .inf files > > (SecPeiCpuExceptionHandlerLib.inf and > > Xcode5SecPeiCpuExceptionHandlerLib.inf)? > > Hmmm, this is a very good point; after all, the updated (=reverted) > "SecPeiCpuExceptionHandlerLib.inf" instance will not build with XCODE5. > Therefore we should list both lib instance INF files under [Components], > but make "SecPeiCpuExceptionHandlerLib.inf" conditional on non-XCODE5. > > I wonder how the CI logic will cope with this! > > Thanks, > Laszlo > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <160B00E54624ADAA.10991@groups.io>]
* Re: [edk2-devel] [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific [not found] ` <160B00E54624ADAA.10991@groups.io> @ 2020-05-05 18:50 ` Lendacky, Thomas 0 siblings, 0 replies; 17+ messages in thread From: Lendacky, Thomas @ 2020-05-05 18:50 UTC (permalink / raw) To: devel Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao, Eric Dong, Ray Ni, Brijesh Singh, Anthony Perard, Benjamin You, Guo Dong, Julien Grall, Maurice Ma, Andrew Fish On 5/1/20 3:17 PM, Lendacky, Thomas via groups.io 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%7Ca1268b8e6d554b6c55ca08d7ee0cbf84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637239610850064292&sdata=soYzRSutJ%2Bepugf2XnzRUpMCLa1GaKoBD%2FX1F4HPCt8%3D&reserved=0 > > Commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass > XCODE5 tool chain") introduced binary patching into the exception handling > support. CPU exception handling is allowed during SEC and this results in > binary patching of flash, which should not be done. > > Separate the changes from commit 2db0ccc2d7fe into an XCODE5 toolchain > specific file, Xcode5ExceptionHandlerAsm.nasm, and create new INF files > for an XCODE5 version of CpuExceptionHandlerLib. Update the UefiCpuPkg.dsc > file to use the new files when the XCODE5 toolchain is used. I used the same FILE_GUID for the new INF files as the old ones. I wasn't sure if they should get new GUIDs or use the same GUID as the original since only one of the libraries will/should be used at a time. Let me know if they need new GUIDs and I'll update and repost the series. Thanks, Tom > > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Liming Gao <liming.gao@intel.com> > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > UefiCpuPkg/UefiCpuPkg.dsc | 23 + > .../Xcode5DxeCpuExceptionHandlerLib.inf | 64 +++ > .../Xcode5PeiCpuExceptionHandlerLib.inf | 63 +++ > .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 55 +++ > .../Xcode5SmmCpuExceptionHandlerLib.inf | 59 +++ > .../X64/Xcode5ExceptionHandlerAsm.nasm | 413 ++++++++++++++++++ > 6 files changed, 677 insertions(+) > create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf > create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf > create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf > create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm > > diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc > index d28cb5cccb52..ad298011232d 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dsc > +++ b/UefiCpuPkg/UefiCpuPkg.dsc > @@ -59,7 +59,11 @@ [LibraryClasses] > > [LibraryClasses.common.SEC] > PlatformSecLib|UefiCpuPkg/Library/PlatformSecLibNull/PlatformSecLibNull.inf > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf > +!else > + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > +!endif > HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf > PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf > MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf > @@ -73,12 +77,20 @@ [LibraryClasses.common.PEIM] > > [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM] > PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf > +!else > + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf > +!endif > > [LibraryClasses.common.DXE_DRIVER] > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > +!else > + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf > +!endif > MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > RegisterCpuFeaturesLib|UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf > > @@ -86,7 +98,11 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf > +!else > + CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf > +!endif > > [LibraryClasses.common.UEFI_APPLICATION] > UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf > @@ -122,10 +138,17 @@ [Components.IA32, Components.X64] > UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf > UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf > UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf > +!if $(TOOL_CHAIN_TAG) != "XCODE5" > UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf > UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf > UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf > +!else > + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf > + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf > + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf > +!endif > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf > new file mode 100644 > index 000000000000..ef37efec6246 > --- /dev/null > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf > @@ -0,0 +1,64 @@ > +## @file > +# CPU Exception Handler library instance for DXE modules. > +# > +# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = Xcode5DxeCpuExceptionHandlerLib > + MODULE_UNI_FILE = DxeCpuExceptionHandlerLib.uni > + FILE_GUID = B6E9835A-EDCF-4748-98A8-27D3C722E02D > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.1 > + LIBRARY_CLASS = CpuExceptionHandlerLib|DXE_CORE DXE_DRIVER UEFI_APPLICATION > + > +# > +# The following information is for reference only and not required by the build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 > +# > + > +[Sources.Ia32] > + Ia32/ExceptionHandlerAsm.nasm > + Ia32/ExceptionTssEntryAsm.nasm > + Ia32/ArchExceptionHandler.c > + Ia32/ArchInterruptDefs.h > + Ia32/ArchAMDSevVcHandler.c > + > +[Sources.X64] > + X64/Xcode5ExceptionHandlerAsm.nasm > + X64/ArchExceptionHandler.c > + X64/ArchInterruptDefs.h > + X64/ArchAMDSevVcHandler.c > + > +[Sources.common] > + CpuExceptionCommon.h > + CpuExceptionCommon.c > + PeiDxeSmmCpuException.c > + DxeException.c > + AMDSevVcHandler.c > + AMDSevVcCommon.h > + > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard > + gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList > + gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > + > +[LibraryClasses] > + BaseLib > + SerialPortLib > + PrintLib > + SynchronizationLib > + LocalApicLib > + PeCoffGetEntryPointLib > + MemoryAllocationLib > + DebugLib > + VmgExitLib > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf > new file mode 100644 > index 000000000000..830ed1eb8bad > --- /dev/null > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf > @@ -0,0 +1,63 @@ > +## @file > +# CPU Exception Handler library instance for PEI module. > +# > +# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR> > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = Xcode5PeiCpuExceptionHandlerLib > + MODULE_UNI_FILE = PeiCpuExceptionHandlerLib.uni > + FILE_GUID = 980DDA67-44A6-4897-99E6-275290B71F9E > + MODULE_TYPE = PEIM > + VERSION_STRING = 1.1 > + LIBRARY_CLASS = CpuExceptionHandlerLib|PEI_CORE PEIM > + > +# > +# The following information is for reference only and not required by the build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 > +# > + > +[Sources.Ia32] > + Ia32/ExceptionHandlerAsm.nasm > + Ia32/ExceptionTssEntryAsm.nasm > + Ia32/ArchExceptionHandler.c > + Ia32/ArchInterruptDefs.h > + Ia32/ArchAMDSevVcHandler.c > + > +[Sources.X64] > + X64/Xcode5ExceptionHandlerAsm.nasm > + X64/ArchExceptionHandler.c > + X64/ArchInterruptDefs.h > + X64/ArchAMDSevVcHandler.c > + > +[Sources.common] > + CpuExceptionCommon.h > + CpuExceptionCommon.c > + PeiCpuException.c > + PeiDxeSmmCpuException.c > + AMDSevVcHandler.c > + AMDSevVcCommon.h > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > + > +[LibraryClasses] > + BaseLib > + SerialPortLib > + PrintLib > + LocalApicLib > + PeCoffGetEntryPointLib > + HobLib > + MemoryAllocationLib > + SynchronizationLib > + VmgExitLib > + > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard # CONSUMES > + > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > new file mode 100644 > index 000000000000..36420be22faa > --- /dev/null > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf > @@ -0,0 +1,55 @@ > +## @file > +# CPU Exception Handler library instance for SEC/PEI modules. > +# > +# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR> > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = Xcode5SecPeiCpuExceptionHandlerLib > + MODULE_UNI_FILE = SecPeiCpuExceptionHandlerLib.uni > + FILE_GUID = CA4BBC99-DFC6-4234-B553-8B6586B7B113 > + MODULE_TYPE = PEIM > + VERSION_STRING = 1.1 > + LIBRARY_CLASS = CpuExceptionHandlerLib|SEC PEI_CORE PEIM > + > +# > +# The following information is for reference only and not required by the build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 > +# > + > +[Sources.Ia32] > + Ia32/ExceptionHandlerAsm.nasm > + Ia32/ExceptionTssEntryAsm.nasm > + Ia32/ArchExceptionHandler.c > + Ia32/ArchInterruptDefs.h > + Ia32/ArchAMDSevVcHandler.c > + > +[Sources.X64] > + X64/Xcode5ExceptionHandlerAsm.nasm > + X64/ArchExceptionHandler.c > + X64/ArchInterruptDefs.h > + X64/ArchAMDSevVcHandler.c > + > +[Sources.common] > + CpuExceptionCommon.h > + CpuExceptionCommon.c > + SecPeiCpuException.c > + AMDSevVcHandler.c > + AMDSevVcCommon.h > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > + > +[LibraryClasses] > + BaseLib > + SerialPortLib > + PrintLib > + LocalApicLib > + PeCoffGetEntryPointLib > + VmgExitLib > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf > new file mode 100644 > index 000000000000..8f71a45c86d5 > --- /dev/null > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf > @@ -0,0 +1,59 @@ > +## @file > +# CPU Exception Handler library instance for SMM modules. > +# > +# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = Xcode5SmmCpuExceptionHandlerLib > + MODULE_UNI_FILE = SmmCpuExceptionHandlerLib.uni > + FILE_GUID = 8D2C439B-3981-42ff-9CE5-1B50ECA502D6 > + MODULE_TYPE = DXE_SMM_DRIVER > + VERSION_STRING = 1.1 > + LIBRARY_CLASS = CpuExceptionHandlerLib|DXE_SMM_DRIVER > + > +# > +# The following information is for reference only and not required by the build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 > +# > + > +[Sources.Ia32] > + Ia32/ExceptionHandlerAsm.nasm > + Ia32/ExceptionTssEntryAsm.nasm > + Ia32/ArchExceptionHandler.c > + Ia32/ArchInterruptDefs.h > + Ia32/ArchAMDSevVcHandler.c > + > +[Sources.X64] > + X64/Xcode5ExceptionHandlerAsm.nasm > + X64/ArchExceptionHandler.c > + X64/ArchInterruptDefs.h > + X64/ArchAMDSevVcHandler.c > + > +[Sources.common] > + CpuExceptionCommon.h > + CpuExceptionCommon.c > + PeiDxeSmmCpuException.c > + SmmException.c > + AMDSevVcHandler.c > + AMDSevVcCommon.h > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > + > +[LibraryClasses] > + BaseLib > + SerialPortLib > + PrintLib > + SynchronizationLib > + LocalApicLib > + PeCoffGetEntryPointLib > + DebugLib > + VmgExitLib > + > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm > new file mode 100644 > index 000000000000..26cae56cc5cf > --- /dev/null > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm > @@ -0,0 +1,413 @@ > +;------------------------------------------------------------------------------ ; > +; Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR> > +; SPDX-License-Identifier: BSD-2-Clause-Patent > +; > +; Module Name: > +; > +; ExceptionHandlerAsm.Asm > +; > +; Abstract: > +; > +; x64 CPU Exception Handler > +; > +; Notes: > +; > +;------------------------------------------------------------------------------ > + > +; > +; CommonExceptionHandler() > +; > + > +%define VC_EXCEPTION 29 > + > +extern ASM_PFX(mErrorCodeFlag) ; Error code flags for exceptions > +extern ASM_PFX(mDoFarReturnFlag) ; Do far return flag > +extern ASM_PFX(CommonExceptionHandler) > + > +SECTION .data > + > +DEFAULT REL > +SECTION .text > + > +ALIGN 8 > + > +AsmIdtVectorBegin: > +%rep 32 > + db 0x6a ; push #VectorNum > + db ($ - AsmIdtVectorBegin) / ((AsmIdtVectorEnd - AsmIdtVectorBegin) / 32) ; VectorNum > + push rax > + mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry) > + jmp rax > +%endrep > +AsmIdtVectorEnd: > + > +HookAfterStubHeaderBegin: > + db 0x6a ; push > +@VectorNum: > + db 0 ; 0 will be fixed > + push rax > + mov rax, strict qword 0 ; mov rax, HookAfterStubHeaderEnd > +JmpAbsoluteAddress: > + jmp rax > +HookAfterStubHeaderEnd: > + mov rax, rsp > + and sp, 0xfff0 ; make sure 16-byte aligned for exception context > + sub rsp, 0x18 ; reserve room for filling exception data later > + push rcx > + mov rcx, [rax + 8] > + bt [ASM_PFX(mErrorCodeFlag)], ecx > + jnc .0 > + push qword [rsp] ; push additional rcx to make stack alignment > +.0: > + xchg rcx, [rsp] ; restore rcx, save Exception Number in stack > + push qword [rax] ; push rax into stack to keep code consistence > + > +;---------------------------------------; > +; CommonInterruptEntry ; > +;---------------------------------------; > +; The follow algorithm is used for the common interrupt routine. > +; Entry from each interrupt with a push eax and eax=interrupt number > +; Stack frame would be as follows as specified in IA32 manuals: > +; > +; +---------------------+ <-- 16-byte aligned ensured by processor > +; + Old SS + > +; +---------------------+ > +; + Old RSP + > +; +---------------------+ > +; + RFlags + > +; +---------------------+ > +; + CS + > +; +---------------------+ > +; + RIP + > +; +---------------------+ > +; + Error Code + > +; +---------------------+ > +; + Vector Number + > +; +---------------------+ > +; + RBP + > +; +---------------------+ <-- RBP, 16-byte aligned > +; The follow algorithm is used for the common interrupt routine. > +global ASM_PFX(CommonInterruptEntry) > +ASM_PFX(CommonInterruptEntry): > + cli > + pop rax > + ; > + ; All interrupt handlers are invoked through interrupt gates, so > + ; IF flag automatically cleared at the entry point > + ; > + xchg rcx, [rsp] ; Save rcx into stack and save vector number into rcx > + and rcx, 0xFF > + cmp ecx, 32 ; Intel reserved vector for exceptions? > + jae NoErrorCode > + bt [ASM_PFX(mErrorCodeFlag)], ecx > + jc HasErrorCode > + > +NoErrorCode: > + > + ; > + ; Push a dummy error code on the stack > + ; to maintain coherent stack map > + ; > + push qword [rsp] > + mov qword [rsp + 8], 0 > +HasErrorCode: > + push rbp > + mov rbp, rsp > + push 0 ; clear EXCEPTION_HANDLER_CONTEXT.OldIdtHandler > + push 0 ; clear EXCEPTION_HANDLER_CONTEXT.ExceptionDataFlag > + > + ; > + ; Stack: > + ; +---------------------+ <-- 16-byte aligned ensured by processor > + ; + Old SS + > + ; +---------------------+ > + ; + Old RSP + > + ; +---------------------+ > + ; + RFlags + > + ; +---------------------+ > + ; + CS + > + ; +---------------------+ > + ; + RIP + > + ; +---------------------+ > + ; + Error Code + > + ; +---------------------+ > + ; + RCX / Vector Number + > + ; +---------------------+ > + ; + RBP + > + ; +---------------------+ <-- RBP, 16-byte aligned > + ; > + > + ; > + ; Since here the stack pointer is 16-byte aligned, so > + ; EFI_FX_SAVE_STATE_X64 of EFI_SYSTEM_CONTEXT_x64 > + ; is 16-byte aligned > + ; > + > +;; UINT64 Rdi, Rsi, Rbp, Rsp, Rbx, Rdx, Rcx, Rax; > +;; UINT64 R8, R9, R10, R11, R12, R13, R14, R15; > + push r15 > + push r14 > + push r13 > + push r12 > + push r11 > + push r10 > + push r9 > + push r8 > + push rax > + push qword [rbp + 8] ; RCX > + push rdx > + push rbx > + push qword [rbp + 48] ; RSP > + push qword [rbp] ; RBP > + push rsi > + push rdi > + > +;; UINT64 Gs, Fs, Es, Ds, Cs, Ss; insure high 16 bits of each is zero > + movzx rax, word [rbp + 56] > + push rax ; for ss > + movzx rax, word [rbp + 32] > + push rax ; for cs > + mov rax, ds > + push rax > + mov rax, es > + push rax > + mov rax, fs > + push rax > + mov rax, gs > + push rax > + > + mov [rbp + 8], rcx ; save vector number > + > +;; UINT64 Rip; > + push qword [rbp + 24] > + > +;; UINT64 Gdtr[2], Idtr[2]; > + xor rax, rax > + push rax > + push rax > + sidt [rsp] > + 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] > + mov bx, word [rsp] > + mov rax, qword [rsp + 2] > + mov qword [rsp], rax > + mov word [rsp + 8], bx > + > +;; UINT64 Ldtr, Tr; > + xor rax, rax > + str ax > + push rax > + sldt ax > + push rax > + > +;; UINT64 RFlags; > + push qword [rbp + 40] > + > +;; UINT64 Cr0, Cr1, Cr2, Cr3, Cr4, Cr8; > + mov rax, cr8 > + push rax > + mov rax, cr4 > + or rax, 0x208 > + mov cr4, rax > + push rax > + mov rax, cr3 > + push rax > + mov rax, cr2 > + push rax > + xor rax, rax > + push rax > + mov rax, cr0 > + push rax > + > +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7; > + cmp qword [rbp + 8], VC_EXCEPTION > + je VcDebugRegs ; For SEV-ES (#VC) Debug registers ignored > + > + mov rax, dr7 > + push rax > + mov rax, dr6 > + push rax > + mov rax, dr3 > + push rax > + mov rax, dr2 > + push rax > + mov rax, dr1 > + push rax > + mov rax, dr0 > + push rax > + jmp DrFinish > + > +VcDebugRegs: > +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception recursion > + xor rax, rax > + push rax > + push rax > + push rax > + push rax > + push rax > + push rax > + > +DrFinish: > +;; FX_SAVE_STATE_X64 FxSaveState; > + sub rsp, 512 > + mov rdi, rsp > + db 0xf, 0xae, 0x7 ;fxsave [rdi] > + > +;; UEFI calling convention for x64 requires that Direction flag in EFLAGs is clear > + cld > + > +;; UINT32 ExceptionData; > + push qword [rbp + 16] > + > +;; Prepare parameter and call > + mov rcx, [rbp + 8] > + mov rdx, rsp > + ; > + ; Per X64 calling convention, allocate maximum parameter stack space > + ; and make sure RSP is 16-byte aligned > + ; > + sub rsp, 4 * 8 + 8 > + call ASM_PFX(CommonExceptionHandler) > + add rsp, 4 * 8 + 8 > + > + cli > +;; UINT64 ExceptionData; > + add rsp, 8 > + > +;; FX_SAVE_STATE_X64 FxSaveState; > + > + mov rsi, rsp > + db 0xf, 0xae, 0xE ; fxrstor [rsi] > + add rsp, 512 > + > +;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7; > +;; Skip restoration of DRx registers to support in-circuit emualators > +;; or debuggers set breakpoint in interrupt/exception context > + add rsp, 8 * 6 > + > +;; UINT64 Cr0, Cr1, Cr2, Cr3, Cr4, Cr8; > + pop rax > + mov cr0, rax > + add rsp, 8 ; not for Cr1 > + pop rax > + mov cr2, rax > + pop rax > + mov cr3, rax > + pop rax > + mov cr4, rax > + pop rax > + mov cr8, rax > + > +;; UINT64 RFlags; > + pop qword [rbp + 40] > + > +;; UINT64 Ldtr, Tr; > +;; UINT64 Gdtr[2], Idtr[2]; > +;; Best not let anyone mess with these particular registers... > + add rsp, 48 > + > +;; UINT64 Rip; > + pop qword [rbp + 24] > + > +;; UINT64 Gs, Fs, Es, Ds, Cs, Ss; > + pop rax > + ; mov gs, rax ; not for gs > + pop rax > + ; mov fs, rax ; not for fs > + ; (X64 will not use fs and gs, so we do not restore it) > + pop rax > + mov es, rax > + pop rax > + mov ds, rax > + pop qword [rbp + 32] ; for cs > + pop qword [rbp + 56] ; for ss > + > +;; UINT64 Rdi, Rsi, Rbp, Rsp, Rbx, Rdx, Rcx, Rax; > +;; UINT64 R8, R9, R10, R11, R12, R13, R14, R15; > + pop rdi > + pop rsi > + add rsp, 8 ; not for rbp > + pop qword [rbp + 48] ; for rsp > + pop rbx > + pop rdx > + pop rcx > + pop rax > + pop r8 > + pop r9 > + pop r10 > + pop r11 > + pop r12 > + pop r13 > + pop r14 > + pop r15 > + > + mov rsp, rbp > + pop rbp > + add rsp, 16 > + cmp qword [rsp - 32], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler > + jz DoReturn > + cmp qword [rsp - 40], 1 ; check EXCEPTION_HANDLER_CONTEXT.ExceptionDataFlag > + jz ErrorCode > + jmp qword [rsp - 32] > +ErrorCode: > + sub rsp, 8 > + jmp qword [rsp - 24] > + > +DoReturn: > + cmp qword [ASM_PFX(mDoFarReturnFlag)], 0 ; Check if need to do far return instead of IRET > + jz DoIret > + push rax > + mov rax, rsp ; save old RSP to rax > + mov rsp, [rsp + 0x20] > + push qword [rax + 0x10] ; save CS in new location > + push qword [rax + 0x8] ; save EIP in new location > + push qword [rax + 0x18] ; save EFLAGS in new location > + mov rax, [rax] ; restore rax > + popfq ; restore EFLAGS > + DB 0x48 ; prefix to composite "retq" with next "retf" > + retf ; far return > +DoIret: > + iretq > + > +;------------------------------------------------------------------------------------- > +; GetTemplateAddressMap (&AddressMap); > +;------------------------------------------------------------------------------------- > +; comments here for definition of address map > +global ASM_PFX(AsmGetTemplateAddressMap) > +ASM_PFX(AsmGetTemplateAddressMap): > + lea rax, [AsmIdtVectorBegin] > + mov qword [rcx], rax > + mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32 > + lea 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 > + > +;------------------------------------------------------------------------------------- > +; AsmVectorNumFixup (*NewVectorAddr, VectorNum, *OldVectorAddr); > +;------------------------------------------------------------------------------------- > +global ASM_PFX(AsmVectorNumFixup) > +ASM_PFX(AsmVectorNumFixup): > + mov rax, rdx > + mov [rcx + (@VectorNum - HookAfterStubHeaderBegin)], al > + ret > + > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-05-06 19:51 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-01 20:17 [PATCH 0/4] XCODE5 toolchain binary patching fix Lendacky, Thomas
2020-05-01 20:17 ` [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific Lendacky, Thomas
2020-05-05 21:39 ` [edk2-devel] " Laszlo Ersek
2020-05-05 22:09 ` Lendacky, Thomas
2020-05-01 20:17 ` [PATCH 2/4] UefiPayloadPkg: Use toolchain appropriate CpuExceptionHandlerLib Lendacky, Thomas
2020-05-05 22:19 ` [edk2-devel] " Laszlo Ersek
2020-05-01 20:17 ` [PATCH 3/4] OvmfPkg: " Lendacky, Thomas
2020-05-05 21:49 ` [edk2-devel] " Laszlo Ersek
2020-05-01 20:17 ` [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib Lendacky, Thomas
2020-05-01 20:49 ` [EXTERNAL] [edk2-devel] " Bret Barkelew
2020-05-05 22:15 ` Laszlo Ersek
2020-05-06 14:35 ` Lendacky, Thomas
2020-05-06 14:53 ` Liming Gao
2020-05-06 16:33 ` Laszlo Ersek
2020-05-06 18:07 ` [EXTERNAL] " Bret Barkelew
2020-05-06 19:51 ` Lendacky, Thomas
[not found] ` <160B00E54624ADAA.10991@groups.io>
2020-05-05 18:50 ` [edk2-devel] [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific Lendacky, Thomas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox