On 5/6/20 2:01 PM, Laszlo Ersek via groups.io wrote: > On 05/06/20 18:33, Tom Lendacky 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%7C3c47ab80a1554f27afd608d7f1eff6e8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243885258646451&sdata=Xh7E1EB%2B1oOCtc2jgtJ8lsjgcwxgswIbgpL4%2BUwpoOw%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 a new SEC INF >> file for the XCODE5 version of CpuExceptionHandlerLib. >> >> Since binary patching is allowed when running outside of flash, switch >> the Dxe, Pei and Smm versions of the CpuExceptionHandlerLib over to use >> the Xcode5ExceptionHandlerAsm.nasm file to retain current functionality. >> >> Cc: Eric Dong <eric.d...@intel.com> >> Cc: Ray Ni <ray...@intel.com> >> Cc: Laszlo Ersek <ler...@redhat.com> >> Cc: Liming Gao <liming....@intel.com> >> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com> >> --- >> UefiCpuPkg/UefiCpuPkg.dsc | 5 + >> .../DxeCpuExceptionHandlerLib.inf | 2 +- >> .../PeiCpuExceptionHandlerLib.inf | 2 +- >> .../SmmCpuExceptionHandlerLib.inf | 2 +- >> .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 54 +++ >> .../X64/Xcode5ExceptionHandlerAsm.nasm | 396 ++++++++++++++++++ >> .../Xcode5SecPeiCpuExceptionHandlerLib.uni | 17 + >> 7 files changed, 475 insertions(+), 3 deletions(-) >> create mode 100644 >> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf >> create mode 100644 >> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm >> create mode 100644 >> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni >> >> diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc >> index d28cb5cccb52..264e5a787bce 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/Xcode5SecPeiCpuExceptionHandlerLib.inf >> +!else >> >> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf >> +!endif >> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf >> >> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf >> >> MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf >> @@ -126,6 +130,7 @@ [Components.IA32, Components.X64] >> >> UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf > > (1) I think this lib instance ("SecPeiCpuExceptionHandlerLib.inf") may > not build with XCODE5 at the end of the series, even in stand-alone > mode. Thus I think it should be conditionalized with > > !if $(TOOL_CHAIN_TAG) != "XCODE5" > ... > !endif > > When using XCODE5, we should only build > "Xcode5SecPeiCpuExceptionHandlerLib.inf"; otherwise, we should build > *both* "SecPeiCpuExceptionHandlerLib.inf" and > ".inf". >
This is the area that was resulting in the error when I used the pull request to run the integration tests. I was using an if/else originally, I'll try just the if != XCODE5 around SecPeiCpuExceptionHandlerLib.inf and see if that goes through. If not, Brett Barkelew responded with a suggestion to add the Xcode5SecPeiCpuExceptionHandlerLib.inf to the ignore list in the CI yaml file. For the next version, under [Components], it will look like: @@ -123,9 +127,12 @@ [Components.IA32, Components.X64] UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +!if $(TOOL_CHAIN_TAG) != "XCODE5" UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf +!endif UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf + UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf >> UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf >> UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf >> + >> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf >> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf >> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf >> UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf >> diff --git >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf >> index e41383573043..61e2ec30b089 100644 >> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf >> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf >> @@ -28,7 +28,7 @@ [Sources.Ia32] >> Ia32/ArchInterruptDefs.h >> >> [Sources.X64] >> - X64/ExceptionHandlerAsm.nasm >> + X64/Xcode5ExceptionHandlerAsm.nasm >> X64/ArchExceptionHandler.c >> X64/ArchInterruptDefs.h >> >> diff --git >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf >> index f31423ac0f91..093374944df6 100644 >> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf >> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf >> @@ -28,7 +28,7 @@ [Sources.Ia32] >> Ia32/ArchInterruptDefs.h >> >> [Sources.X64] >> - X64/ExceptionHandlerAsm.nasm >> + X64/Xcode5ExceptionHandlerAsm.nasm >> X64/ArchExceptionHandler.c >> X64/ArchInterruptDefs.h >> >> diff --git >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf >> index 66c7f59e3c91..2ffbbccc302f 100644 >> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf >> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf >> @@ -28,7 +28,7 @@ [Sources.Ia32] >> Ia32/ArchInterruptDefs.h >> >> [Sources.X64] >> - X64/ExceptionHandlerAsm.nasm >> + X64/Xcode5ExceptionHandlerAsm.nasm >> X64/ArchExceptionHandler.c >> X64/ArchInterruptDefs.h >> >> diff --git >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf >> >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf >> new file mode 100644 >> index 000000000000..3ed1378d6fa6 >> --- /dev/null >> +++ >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf >> @@ -0,0 +1,54 @@ >> +## @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 > > (2) This is a customized copy of "SecPeiCpuExceptionHandlerLib.inf"; I > think you should prepend your (C) notice. Ok. > >> +# >> +# This is the XCODE5 variant of the SEC/PEI CpuExceptionHandlerLib. This >> +# variant performs binary patching to fix up addresses that allow the >> +# XCODE5 toolchain to be used. >> +# >> +## >> + >> +[Defines] >> + INF_VERSION = 0x00010005 >> + BASE_NAME = Xcode5SecPeiCpuExceptionHandlerLib >> + MODULE_UNI_FILE = Xcode5SecPeiCpuExceptionHandlerLib.uni >> + FILE_GUID = 49C481AF-1621-42F3-8FA1-27C64143E304 >> + MODULE_TYPE = PEIM >> + VERSION_STRING = 1.1 >> + LIBRARY_CLASS = CpuExceptionHandlerLib|SEC PEI_CORE PEIM >> + <... SNIP ...> >> +global ASM_PFX(AsmVectorNumFixup) >> +ASM_PFX(AsmVectorNumFixup): >> + mov rax, rdx >> + mov [rcx + (@VectorNum - HookAfterStubHeaderBegin)], al >> + ret >> + >> diff --git >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni >> >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni >> new file mode 100644 >> index 000000000000..be69992cef09 >> --- /dev/null >> +++ >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni >> @@ -0,0 +1,17 @@ >> +// /** @file >> +// XCODE5 CPU Exception Handler library instance for SEC/PEI modules. >> +// >> +// CPU Exception Handler library instance for SEC/PEI modules when built >> +// using the XCODE5 toolchain. >> +// >> +// Copyright (c) 2012 - 2014, Intel Corporation. All rights reserved.<BR> >> +// >> +// SPDX-License-Identifier: BSD-2-Clause-Patent >> +// >> +// **/ >> + >> + >> +#string STR_MODULE_ABSTRACT #language en-US "CPU Exception >> Handler library instance for SEC/PEI modules with the XCODE5 toolchain." >> + >> +#string STR_MODULE_DESCRIPTION #language en-US "CPU Exception >> Handler library instance for SEC/PEI modules with the XCODE5 toolchain." >> + >> > > (3) This is a brand new file; I think you should prepend your (C) notice. Ok. > > Meta-hint: with patches like this, it sometimes makes sense to format > the series for posting with "--find-copies-harder". Ah, I'll do that on the next version. Thanks, Tom > > With (1) through (3) updated: > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > > Thanks, > Laszlo > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#58746): https://edk2.groups.io/g/devel/message/58746 Mute This Topic: https://groups.io/mt/74032331/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
