Date
1 - 8 of 8
[RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
|
Ard Biesheuvel
We have a special version of CpuExceptionHandlerLib for XCODE5, whose=0D
linker (LLD) does not permit absolute symbol references in read-only=0D sections.=0D =0D Instead of fixing this up at runtime for all toolchains (which is done=0D by writing the fixed up values to the .text section, which we'd prefer=0D to avoid), tweak the SEC/PEI version so it does not need this, and=0D update the remaining versions to only incorporate this logic when using=0D the XCODE toolchain.=0D =0D Changes since v3:=0D - As Marvin points out, using '-read_only_relocs suppress' with the X64=0D XCODE linker is a terrible idea, as it corrupts the resulting PE=0D binaries, so instead, let's do the following:=0D . tweak the SEC/PEI version of the library so the relocs are emitted=0D into .data when using XCODE;=0D . tweak the other versions so the runtime fixups are only done when=0D using XCODE=0D - add acks from Jiewen and Ray=0D =0D Changes since v2:=0D - pass linker switches to permit absolute relocations in read-only=0D regions, and keep all code in .text=0D =0D Cc: "Ni, Ray" <ray.ni@...>=0D Cc: Andrew Fish <afish@...>=0D Cc: "Kinney, Michael D" <michael.d.kinney@...>=0D Cc: "Liu, Zhiguang" <zhiguang.liu@...>=0D Cc: Rebecca Cran <rebecca@...>=0D Cc: Tom Lendacky <thomas.lendacky@...>=0D Cc: Marvin H=C3=A4user <mhaeuser@...>=0D =0D Ard Biesheuvel (5):=0D BaseTools/tools_def CLANGDWARF: Permit text relocations=0D UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version=0D UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only=0D OvmfPkg: Drop special Xcode5 version of exception handler library=0D UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version=0D =0D BaseTools/Conf/tools_def.template = | 2 +-=0D OvmfPkg/AmdSev/AmdSevX64.dsc = | 4 -=0D OvmfPkg/CloudHv/CloudHvX64.dsc = | 4 -=0D OvmfPkg/IntelTdx/IntelTdxX64.dsc = | 4 -=0D OvmfPkg/Microvm/MicrovmX64.dsc = | 4 -=0D OvmfPkg/OvmfPkgIa32.dsc = | 4 -=0D OvmfPkg/OvmfPkgIa32X64.dsc = | 4 -=0D OvmfPkg/OvmfPkgX64.dsc = | 4 -=0D OvmfPkg/OvmfXen.dsc = | 4 -=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf = | 5 +-=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf = | 4 +-=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf= | 4 +-=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf = | 4 +-=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm = | 116 +++++++++++++++++---=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandlerAsm.n= asm =3D> SecPeiExceptionHandlerAsm.nasm} | 108 +++---------------=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerL= ib.inf | 65 -----------=0D UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerL= ib.uni | 18 ---=0D UefiCpuPkg/UefiCpuPkg.dsc = | 7 --=0D 18 files changed, 133 insertions(+), 232 deletions(-)=0D rename UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandl= erAsm.nasm =3D> SecPeiExceptionHandlerAsm.nasm} (70%)=0D delete mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiC= puExceptionHandlerLib.inf=0D delete mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiC= puExceptionHandlerLib.uni=0D =0D -- =0D 2.39.2=0D =0D |
|
|
|
Ni, Ray
Ard,
toggle quoted message
Show quoted text
What does "-read_only_relocs suppress" control? Linker doesn't produce relocation entries that modifies .text section silently so the final .text just cannot run at all? Thanks, Ray -----Original Message----- |
|
|
|
Ard Biesheuvel
On Fri, 31 Mar 2023 at 12:09, Ni, Ray <ray.ni@...> wrote:
Yeah, good question. So this is why I dropped this now - it doesn't work as expected. |
|
|
|
Marvin Häuser
Hi Ray,
On 31. Mar 2023, at 12:09, Ni, Ray <ray.ni@...> wrote:It controls whether relocs that target read-only segments yield a build error or not. I think lld uses “-z notext”. Linker doesn't produce relocation entries that modifies .text section silentlyCould you please rephrase? I’m not sure I understand, but I think it’s important everyone understands the issues at play to make a good judgment call. Best regards, Marvin
|
|
|
|
Ard Biesheuvel
On Fri, 31 Mar 2023 at 12:41, Marvin Häuser <mhaeuser@...> wrote:
As *I* understood it, it means suppress the *warning* not suppress the *relocation* But the resulting binaries are broken, so it doesn't really matter. |
|
|
|
Marvin Häuser
On 31. Mar 2023, at 13:03, Ard Biesheuvel <ardb@...> wrote:Correct.
|
|
|
|
Ni, Ray
toggle quoted message
Show quoted text
-----Original Message-----What the meaning of "suppress relocation"? Why the final binaries are not executable?
|
|
|
|
Marvin Häuser
The option naming is just a bit odd, it suppresses the warning about relocations to read-only segments, not the relocations themselves.
I explained that here: https://edk2.groups.io/g/devel/message/102219 TL;dr: Relocations are relative to the first writable segment (thus usually __DATA), so relocations to preceding segments (usually __TEXT) will underflow and thus get corrupted offsets. Best regards, Marvin
|
|
|