Re: [edk2-devel] [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only

["Ni, Ray" <ray.ni@intel.com>]

I like this approach that allows "relocation entry guided" fixups done by either GenFv or PE loader.

Only concern is which to use between "#" and "%".

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Friday, March 31, 2023 5:15 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb@kernel.org>; Ni, Ray <ray.ni@intel.com>; Andrew
> Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Liu, Zhiguang <zhiguang.liu@intel.com>; Rebecca Cran
> <rebecca@bsdio.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> Marvin Häuser <mhaeuser@posteo.de>
> Subject: [edk2-devel] [RFT PATCH v3 3/5]
> UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
> 
> The CPU exception handler library code was rewritten at some point to
> populate the vector code templates with absolute references at runtime,
> given that the XCODE linker does not permit absolute references in
> executable code when creating PIE executables.
> 
> This is rather unfortunate, as this prevents us from using strict
> permissions on the memory mappings, given that the .text section needs
> to be writable at runtime for this arrangement to work.
> 
> So let's make this hack XCODE-only, by setting a preprocessor #define
> from the command line when using the XCODE toolchain, and only including
> the runtime fixup code when the macro is defined.
> 
> While at it, rename the Xcode5ExceptionHandlerAsm.nasm source file and
> drop the Xcode5 prefix: this code is used by other toolchains too.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.in
> f                                    |  5 ++++-
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
> |  4 +++-
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.i
> nf                                    |  4 +++-
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandler
> Asm.nasm => ExceptionHandlerAsm.nasm} | 10 ++++++++++
>  4 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.
> inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib
> .inf
> index d0f82095cf926e99..ee9df805c05df4f7 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/Xcode5ExceptionHandlerAsm.nasm
> 
> +  X64/ExceptionHandlerAsm.nasm
> 
>    X64/ArchExceptionHandler.c
> 
>    X64/ArchInterruptDefs.h
> 
> 
> 
> @@ -61,3 +61,6 @@ [LibraryClasses]
>    MemoryAllocationLib
> 
>    DebugLib
> 
>    CcExitLib
> 
> +
> 
> +[BuildOptions]
> 
> +  XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> index 5339f8e604045801..83970c54712f22a2 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> @@ -28,7 +28,7 @@ [Sources.Ia32]
>    Ia32/ArchInterruptDefs.h
> 
> 
> 
>  [Sources.X64]
> 
> -  X64/Xcode5ExceptionHandlerAsm.nasm
> 
> +  X64/ExceptionHandlerAsm.nasm
> 
>    X64/ArchExceptionHandler.c
> 
>    X64/ArchInterruptDefs.h
> 
> 
> 
> @@ -62,3 +62,5 @@ [Pcd]
>  [FeaturePcd]
> 
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard                    ##
> CONSUMES
> 
> 
> 
> +[BuildOptions]
> 
> +  XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> index 8f8a5dab79303f87..acd2936aef4490a5 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> @@ -28,7 +28,7 @@ [Sources.Ia32]
>    Ia32/ArchInterruptDefs.h
> 
> 
> 
>  [Sources.X64]
> 
> -  X64/Xcode5ExceptionHandlerAsm.nasm
> 
> +  X64/ExceptionHandlerAsm.nasm
> 
>    X64/ArchExceptionHandler.c
> 
>    X64/ArchInterruptDefs.h
> 
> 
> 
> @@ -61,3 +61,5 @@ [Pcd]
>  [FeaturePcd]
> 
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard                    ##
> CONSUMES
> 
> 
> 
> +[BuildOptions]
> 
> +  XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> similarity index 95%
> rename from
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerA
> sm.nasm
> rename to
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
> m
> index 957478574253e619..3823656ea7d4c3b8 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> @@ -59,7 +59,11 @@ AsmIdtVectorBegin:
>  %rep  256
> 
>      push    strict dword %[Vector] ; This instruction pushes sign-extended 8-
> byte value on stack
> 
>      push    rax
> 
> +#ifdef NO_ABSOLUTE_RELOCS_IN_TEXT
> 
>      mov     rax, strict qword 0    ; mov     rax, ASM_PFX(CommonInterruptEntry)
> 
> +#else
> 
> +    mov     rax, ASM_PFX(CommonInterruptEntry)
> 
> +#endif
> 
>      jmp     rax
> 
>  %assign Vector Vector+1
> 
>  %endrep
> 
> @@ -69,8 +73,12 @@ HookAfterStubHeaderBegin:
>      push    strict dword 0      ; 0 will be fixed
> 
>  VectorNum:
> 
>      push    rax
> 
> +#ifdef NO_ABSOLUTE_RELOCS_IN_TEXT
> 
>      mov     rax, strict qword 0 ;     mov     rax, HookAfterStubHeaderEnd
> 
>  JmpAbsoluteAddress:
> 
> +#else
> 
> +    mov     rax, HookAfterStubHeaderEnd
> 
> +#endif
> 
>      jmp     rax
> 
>  HookAfterStubHeaderEnd:
> 
>      mov     rax, rsp
> 
> @@ -457,6 +465,7 @@ ASM_PFX(AsmGetTemplateAddressMap):
>      lea     rax, [HookAfterStubHeaderBegin]
> 
>      mov     qword [rcx + 0x10], rax
> 
> 
> 
> +#ifdef NO_ABSOLUTE_RELOCS_IN_TEXT
> 
>  ; Fix up CommonInterruptEntry address
> 
>      lea    rax, [ASM_PFX(CommonInterruptEntry)]
> 
>      lea    rcx, [AsmIdtVectorBegin]
> 
> @@ -468,6 +477,7 @@ ASM_PFX(AsmGetTemplateAddressMap):
>      lea    rax, [HookAfterStubHeaderEnd]
> 
>      lea    rcx, [JmpAbsoluteAddress]
> 
>      mov    qword [rcx - 8], rax
> 
> +#endif
> 
> 
> 
>      ret
> 
> 
> 
> --
> 2.39.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#102258):
> https://edk2.groups.io/g/devel/message/102258
> Mute This Topic: https://groups.io/mt/97969651/1712937
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
>