* [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
@ 2023-03-31 9:14 Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 1/5] BaseTools/tools_def CLANGDWARF: Permit text relocations Ard Biesheuvel
` (5 more replies)
0 siblings, 6 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 9:14 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ni, Ray, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4418 bytes --]
We have a special version of CpuExceptionHandlerLib for XCODE5, whose
linker (LLD) does not permit absolute symbol references in read-only
sections.
Instead of fixing this up at runtime for all toolchains (which is done
by writing the fixed up values to the .text section, which we'd prefer
to avoid), tweak the SEC/PEI version so it does not need this, and
update the remaining versions to only incorporate this logic when using
the XCODE toolchain.
Changes since v3:
- As Marvin points out, using '-read_only_relocs suppress' with the X64
XCODE linker is a terrible idea, as it corrupts the resulting PE
binaries, so instead, let's do the following:
. tweak the SEC/PEI version of the library so the relocs are emitted
into .data when using XCODE;
. tweak the other versions so the runtime fixups are only done when
using XCODE
- add acks from Jiewen and Ray
Changes since v2:
- pass linker switches to permit absolute relocations in read-only
regions, and keep all code in .text
Cc: "Ni, Ray" <ray.ni@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Liu, Zhiguang" <zhiguang.liu@intel.com>
Cc: Rebecca Cran <rebecca@bsdio.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Marvin Häuser <mhaeuser@posteo.de>
Ard Biesheuvel (5):
BaseTools/tools_def CLANGDWARF: Permit text relocations
UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
OvmfPkg: Drop special Xcode5 version of exception handler library
UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version
BaseTools/Conf/tools_def.template | 2 +-
OvmfPkg/AmdSev/AmdSevX64.dsc | 4 -
OvmfPkg/CloudHv/CloudHvX64.dsc | 4 -
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 4 -
OvmfPkg/Microvm/MicrovmX64.dsc | 4 -
OvmfPkg/OvmfPkgIa32.dsc | 4 -
OvmfPkg/OvmfPkgIa32X64.dsc | 4 -
OvmfPkg/OvmfPkgX64.dsc | 4 -
OvmfPkg/OvmfXen.dsc | 4 -
UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf | 5 +-
UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf | 4 +-
UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf | 4 +-
UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf | 4 +-
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm | 116 +++++++++++++++++---
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandlerAsm.nasm => SecPeiExceptionHandlerAsm.nasm} | 108 +++---------------
UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf | 65 -----------
UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni | 18 ---
UefiCpuPkg/UefiCpuPkg.dsc | 7 --
18 files changed, 133 insertions(+), 232 deletions(-)
rename UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandlerAsm.nasm => SecPeiExceptionHandlerAsm.nasm} (70%)
delete mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
delete mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni
--
2.39.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFT PATCH v3 1/5] BaseTools/tools_def CLANGDWARF: Permit text relocations
2023-03-31 9:14 [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ard Biesheuvel
@ 2023-03-31 9:14 ` Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version Ard Biesheuvel
` (4 subsequent siblings)
5 siblings, 0 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 9:14 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ni, Ray, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
We rely on PIE executables to get the codegen that is suitable for
PE/COFF conversion where the resulting executables can be loaded
anywhere in the address space.
However, ELF linkers may default to disallowing text relocations in PIE
executables, as this would require text segments to be updated at
runtime, which is bad for security and increases the copy-on-write
footprint of ELF executables and shared libraries.
However, none of those concerns apply to PE/COFF executables in the
context of EFI, which are copied into memory rather than mmap()'ed, and
fixed up by the loader before launch.
So pass -z notext to the LLD linker to permit runtime relocations in
read-only sections.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
BaseTools/Conf/tools_def.template | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index ae43101853870c6d..5a3af55bfb09d753 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -2870,7 +2870,7 @@ DEFINE CLANGDWARF_X64_PREFIX = ENV(CLANG_BIN)
DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON = -nostdlib -Wl,-q,--gc-sections -z max-page-size=0x40
DEFINE CLANGDWARF_DLINK2_FLAGS_COMMON = -Wl,--script=$(EDK_TOOLS_PATH)/Scripts/ClangBase.lds
DEFINE CLANGDWARF_IA32_X64_ASLDLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) -Wl,--defsym=PECOFF_HEADER_SIZE=0 DEF(CLANGDWARF_DLINK2_FLAGS_COMMON) -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable
-DEFINE CLANGDWARF_IA32_X64_DLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map,--whole-archive
+DEFINE CLANGDWARF_IA32_X64_DLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map,--whole-archive -Wl,-z,notext
DEFINE CLANGDWARF_IA32_DLINK2_FLAGS = -Wl,--defsym=PECOFF_HEADER_SIZE=0x220 DEF(CLANGDWARF_DLINK2_FLAGS_COMMON)
DEFINE CLANGDWARF_X64_DLINK2_FLAGS = -Wl,--defsym=PECOFF_HEADER_SIZE=0x228 DEF(CLANGDWARF_DLINK2_FLAGS_COMMON)
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 9:14 [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 1/5] BaseTools/tools_def CLANGDWARF: Permit text relocations Ard Biesheuvel
@ 2023-03-31 9:14 ` Ard Biesheuvel
2023-03-31 9:56 ` Ni, Ray
[not found] ` <17517877FE72B326.27612@groups.io>
2023-03-31 9:14 ` [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only Ard Biesheuvel
` (3 subsequent siblings)
5 siblings, 2 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 9:14 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ni, Ray, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
Currently, we use the non-Xcode5 version of ExceptionHandlerAsm.nasm
only for the SEC and PEI phases, and this version was not compatible
with the XCODE or LLD linkers, which do not permit absolute relocations
in read-only sections.
Given that SEC and PEI code typically executes in place from flash and
does not use page alignment for sections, we can simply emit the code
carrying the absolute symbol references into the .data segment instead.
This works around the linker's objections, and the resulting image will
be mapped executable in its entirety anyway. Since this is only needed
for XCODE, let's make this change conditionally using a preprocessor
macro.
Let's rename the .nasm file to reflect the fact that is used for the
SecPei flavor of this library only, and while at it, remove some
unnecessary absolute references.
Also update the Xcode specific version of this library, and use this
source file instead. This is necesessary, as the Xcode specific version
modifies its own code at runtime, which is not permitted in SEC or PEI.
Note that this also removes CET support from the Xcode5 specific build
of the SEC/PEI version of this library, but this is not needed this
early in any case, and this aligns it with other toolchains, which use
this version of the library, which does not have CET support either.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf | 4 +++-
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{ExceptionHandlerAsm.nasm => SecPeiExceptionHandlerAsm.nasm} | 12 ++++++++----
UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf | 4 +++-
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
index df44371fe018e06d..885bb6638ab58620 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
@@ -28,7 +28,7 @@ [Sources.Ia32]
Ia32/ArchInterruptDefs.h
[Sources.X64]
- X64/ExceptionHandlerAsm.nasm
+ X64/SecPeiExceptionHandlerAsm.nasm
X64/ArchExceptionHandler.c
X64/ArchInterruptDefs.h
@@ -58,3 +58,5 @@ [Pcd]
[FeaturePcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES
+[BuildOptions]
+ XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandlerAsm.nasm
similarity index 94%
rename from UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
rename to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandlerAsm.nasm
index aaf8d622e6f3b8f1..ec45c60181906c14 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandlerAsm.nasm
@@ -27,7 +27,9 @@ extern ASM_PFX(CommonExceptionHandler)
SECTION .data
DEFAULT REL
+#ifndef NO_ABSOLUTE_RELOCS_IN_TEXT
SECTION .text
+#endif
ALIGN 8
@@ -51,6 +53,9 @@ HookAfterStubHeaderBegin:
push rax
mov rax, HookAfterStubHeaderEnd
jmp rax
+
+SECTION .text
+
HookAfterStubHeaderEnd:
mov rax, rsp
and sp, 0xfff0 ; make sure 16-byte aligned for exception context
@@ -276,8 +281,7 @@ DrFinish:
; and make sure RSP is 16-byte aligned
;
sub rsp, 4 * 8 + 8
- mov rax, ASM_PFX(CommonExceptionHandler)
- call rax
+ call ASM_PFX(CommonExceptionHandler)
add rsp, 4 * 8 + 8
cli
@@ -384,10 +388,10 @@ DoIret:
; comments here for definition of address map
global ASM_PFX(AsmGetTemplateAddressMap)
ASM_PFX(AsmGetTemplateAddressMap):
- mov rax, AsmIdtVectorBegin
+ lea rax, [AsmIdtVectorBegin]
mov qword [rcx], rax
mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
- mov rax, HookAfterStubHeaderBegin
+ lea rax, [HookAfterStubHeaderBegin]
mov qword [rcx + 0x10], rax
ret
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
index 619b39d7f1de9ae3..17f872bb15eb0ff7 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
@@ -33,7 +33,7 @@ [Sources.Ia32]
Ia32/ArchInterruptDefs.h
[Sources.X64]
- X64/Xcode5ExceptionHandlerAsm.nasm
+ X64/SecPeiExceptionHandlerAsm.nasm
X64/ArchExceptionHandler.c
X64/ArchInterruptDefs.h
@@ -63,3 +63,5 @@ [Pcd]
[FeaturePcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES
+[BuildOptions]
+ XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
2023-03-31 9:14 [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 1/5] BaseTools/tools_def CLANGDWARF: Permit text relocations Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version Ard Biesheuvel
@ 2023-03-31 9:14 ` Ard Biesheuvel
2023-03-31 10:03 ` [edk2-devel] " Ni, Ray
2023-03-31 10:20 ` Ni, Ray
2023-03-31 9:14 ` [RFT PATCH v3 4/5] OvmfPkg: Drop special Xcode5 version of exception handler library Ard Biesheuvel
` (2 subsequent siblings)
5 siblings, 2 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 9:14 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ni, Ray, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
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.inf | 5 ++++-
UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf | 4 +++-
UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf | 4 +++-
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandlerAsm.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.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
index 5339f8e604045801..83970c54712f22a2 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/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/SmmCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
index 8f8a5dab79303f87..acd2936aef4490a5 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/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/Xcode5ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
similarity index 95%
rename from UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
rename to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 957478574253e619..3823656ea7d4c3b8 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFT PATCH v3 4/5] OvmfPkg: Drop special Xcode5 version of exception handler library
2023-03-31 9:14 [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ard Biesheuvel
` (2 preceding siblings ...)
2023-03-31 9:14 ` [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only Ard Biesheuvel
@ 2023-03-31 9:14 ` Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 5/5] UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version Ard Biesheuvel
2023-03-31 10:08 ` [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ni, Ray
5 siblings, 0 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 9:14 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ni, Ray, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser,
Jiewen Yao
The generic and XCODE5 versions of this library are now identical, so
drop the special case. The library will be removed entirely in a
subsequent patch.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Jiewen Yao <jiewen.yao@intel.com>
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 4 ----
OvmfPkg/CloudHv/CloudHvX64.dsc | 4 ----
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 4 ----
OvmfPkg/Microvm/MicrovmX64.dsc | 4 ----
OvmfPkg/OvmfPkgIa32.dsc | 4 ----
OvmfPkg/OvmfPkgIa32X64.dsc | 4 ----
OvmfPkg/OvmfPkgX64.dsc | 4 ----
OvmfPkg/OvmfXen.dsc | 4 ----
8 files changed, 32 deletions(-)
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index c005e474dd826759..943c4eed9831a1c5 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -224,11 +224,7 @@ [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/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
CcExitLib|OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index b9820cc14bee0693..cc2dd925bc940ea8 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -270,11 +270,7 @@ [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/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
CcExitLib|OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 5c56858d063b96bf..f734409055400859 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -238,11 +238,7 @@ [LibraryClasses.common.SEC]
ExtractGuidedSectionLib|MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
-!if $(TOOL_CHAIN_TAG) == "XCODE5"
- CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
CcExitLib|OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
PrePiHobListPointerLib|OvmfPkg/IntelTdx/PrePiHobListPointerLibTdx/PrePiHobListPointerLibTdx.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 384b0b7afc74e90f..e9aab515592ffcec 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -272,11 +272,7 @@ [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/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
CcExitLib|OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index a6db902f54ece86f..86177bb948999435 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -275,11 +275,7 @@ [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/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
[LibraryClasses.common.PEI_CORE]
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 076fc0353de02aaa..065b5445064712d9 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -280,11 +280,7 @@ [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/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
[LibraryClasses.common.PEI_CORE]
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index b2f3d14cd94d5fff..3d405cd4ade07900 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -297,11 +297,7 @@ [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" || $(TOOL_CHAIN_TAG) == "CLANGDWARF"
- CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
CcExitLib|OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
CcProbeLib|OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 990225d2dd05d3a8..8bfc16c2d3d6aabe 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -247,11 +247,7 @@ [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/Xcode5SecPeiCpuExceptionHandlerLib.inf
-!else
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
-!endif
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
[LibraryClasses.common.PEI_CORE]
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFT PATCH v3 5/5] UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version
2023-03-31 9:14 [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ard Biesheuvel
` (3 preceding siblings ...)
2023-03-31 9:14 ` [RFT PATCH v3 4/5] OvmfPkg: Drop special Xcode5 version of exception handler library Ard Biesheuvel
@ 2023-03-31 9:14 ` Ard Biesheuvel
2023-03-31 10:08 ` [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ni, Ray
5 siblings, 0 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 9:14 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ni, Ray, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
This library is no longer used or needed, so let's remove it.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Ray Ni <ray.ni@intel.com>
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf | 67 --------------------
UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni | 18 ------
UefiCpuPkg/UefiCpuPkg.dsc | 7 --
3 files changed, 92 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
deleted file mode 100644
index 17f872bb15eb0ff7..0000000000000000
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
+++ /dev/null
@@ -1,67 +0,0 @@
-## @file
-# CPU Exception Handler library instance for SEC/PEI modules.
-#
-# Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
-# Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved.<BR>
-# SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-# 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
-
-#
-# 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
-
-[Sources.X64]
- X64/SecPeiExceptionHandlerAsm.nasm
- X64/ArchExceptionHandler.c
- X64/ArchInterruptDefs.h
-
-[Sources.common]
- CpuExceptionCommon.h
- CpuExceptionCommon.c
- SecPeiCpuException.c
-
-[Packages]
- MdePkg/MdePkg.dec
- MdeModulePkg/MdeModulePkg.dec
- UefiCpuPkg/UefiCpuPkg.dec
-
-[LibraryClasses]
- BaseLib
- SerialPortLib
- PrintLib
- LocalApicLib
- PeCoffGetEntryPointLib
- CcExitLib
-
-[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
- gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList
- gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize
-
-[FeaturePcd]
- gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES
-
-[BuildOptions]
- XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni
deleted file mode 100644
index a63b25f39d992775..0000000000000000
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.uni
+++ /dev/null
@@ -1,18 +0,0 @@
-// /** @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) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
-// 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."
-
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index a7318d3fe9db0ec4..d85d56916f2cdbce 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -69,11 +69,7 @@ [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
@@ -145,12 +141,9 @@ [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
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 9:14 ` [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version Ard Biesheuvel
@ 2023-03-31 9:56 ` Ni, Ray
2023-03-31 10:12 ` [edk2-devel] " Ard Biesheuvel
[not found] ` <17517877FE72B326.27612@groups.io>
1 sibling, 1 reply; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 9:56 UTC (permalink / raw)
To: Ard Biesheuvel, devel@edk2.groups.io
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky, Marvin Häuser
Ard,
Thanks for the detailed commit messages. That really helps me to understand why XCODE version
was needed.
However, I feel it would be great if you can "highlight" what are changed by this patch.
The following is just an example. You can reword as you like.
1. Change for non-XCODE SecPeiCpuExceptionHandlerLib:
* Use SecPeiExceptionHandlerAsm.nasm (renamed from ExceptionHandlerAsm.nasm)
* Removed some unnecessary absolute references
* (32 IDT stubs are still in .text.)
2. Change for XCODE SecPeiCpuExceptionHandlerLib:
* Use SecPeiExceptionHandlerAsm.nasm instead of Xcode5ExceptionHandlerAsm.nasm
* CET logic is not in SecPeiExceptionHandlerAsm.nasm (but aligns to non-XCODE lib instance)
* Fixed a bug that does runtime fixup in TEXT section in SPI flash.
* Emitted the code carrying the absolute symbol references into the .data which XCODE or
LLD linkers allow.
Then fixup can be done by other build tools such as GenFv if the code runs in SPI flash,
or by PE coff loader if the code is loaded to memory.
Again, thanks for the quick patches just because I asked some XCODE questions.
Thanks,
Ray
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> 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: [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single
> SEC/PEI version
>
> Currently, we use the non-Xcode5 version of ExceptionHandlerAsm.nasm
> only for the SEC and PEI phases, and this version was not compatible
> with the XCODE or LLD linkers, which do not permit absolute relocations
> in read-only sections.
>
> Given that SEC and PEI code typically executes in place from flash and
> does not use page alignment for sections, we can simply emit the code
> carrying the absolute symbol references into the .data segment instead.
> This works around the linker's objections, and the resulting image will
> be mapped executable in its entirety anyway. Since this is only needed
> for XCODE, let's make this change conditionally using a preprocessor
> macro.
>
> Let's rename the .nasm file to reflect the fact that is used for the
> SecPei flavor of this library only, and while at it, remove some
> unnecessary absolute references.
>
> Also update the Xcode specific version of this library, and use this
> source file instead. This is necesessary, as the Xcode specific version
> modifies its own code at runtime, which is not permitted in SEC or PEI.
> Note that this also removes CET support from the Xcode5 specific build
> of the SEC/PEI version of this library, but this is not needed this
> early in any case, and this aligns it with other toolchains, which use
> this version of the library, which does not have CET support either.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib
> .inf | 4 +++-
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{ExceptionHandlerAsm.na
> sm => SecPeiExceptionHandlerAsm.nasm} | 12 ++++++++----
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
> ndlerLib.inf | 4 +++-
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> index df44371fe018e06d..885bb6638ab58620 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> @@ -28,7 +28,7 @@ [Sources.Ia32]
> Ia32/ArchInterruptDefs.h
>
>
>
> [Sources.X64]
>
> - X64/ExceptionHandlerAsm.nasm
>
> + X64/SecPeiExceptionHandlerAsm.nasm
>
> X64/ArchExceptionHandler.c
>
> X64/ArchInterruptDefs.h
>
>
>
> @@ -58,3 +58,5 @@ [Pcd]
> [FeaturePcd]
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ##
> CONSUMES
>
>
>
> +[BuildOptions]
>
> + XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandler
> Asm.nasm
> similarity index 94%
> rename from
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
> m
> rename to
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandlerA
> sm.nasm
> index aaf8d622e6f3b8f1..ec45c60181906c14 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandler
> Asm.nasm
> @@ -27,7 +27,9 @@ extern ASM_PFX(CommonExceptionHandler)
> SECTION .data
>
>
>
> DEFAULT REL
>
> +#ifndef NO_ABSOLUTE_RELOCS_IN_TEXT
>
> SECTION .text
>
> +#endif
>
>
>
> ALIGN 8
>
>
>
> @@ -51,6 +53,9 @@ HookAfterStubHeaderBegin:
> push rax
>
> mov rax, HookAfterStubHeaderEnd
>
> jmp rax
>
> +
>
> +SECTION .text
>
> +
>
> HookAfterStubHeaderEnd:
>
> mov rax, rsp
>
> and sp, 0xfff0 ; make sure 16-byte aligned for exception context
>
> @@ -276,8 +281,7 @@ DrFinish:
> ; and make sure RSP is 16-byte aligned
>
> ;
>
> sub rsp, 4 * 8 + 8
>
> - mov rax, ASM_PFX(CommonExceptionHandler)
>
> - call rax
>
> + call ASM_PFX(CommonExceptionHandler)
>
> add rsp, 4 * 8 + 8
>
>
>
> cli
>
> @@ -384,10 +388,10 @@ DoIret:
> ; comments here for definition of address map
>
> global ASM_PFX(AsmGetTemplateAddressMap)
>
> ASM_PFX(AsmGetTemplateAddressMap):
>
> - mov rax, AsmIdtVectorBegin
>
> + lea rax, [AsmIdtVectorBegin]
>
> mov qword [rcx], rax
>
> mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
>
> - mov rax, HookAfterStubHeaderBegin
>
> + lea rax, [HookAfterStubHeaderBegin]
>
> mov qword [rcx + 0x10], rax
>
> ret
>
>
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> index 619b39d7f1de9ae3..17f872bb15eb0ff7 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> @@ -33,7 +33,7 @@ [Sources.Ia32]
> Ia32/ArchInterruptDefs.h
>
>
>
> [Sources.X64]
>
> - X64/Xcode5ExceptionHandlerAsm.nasm
>
> + X64/SecPeiExceptionHandlerAsm.nasm
>
> X64/ArchExceptionHandler.c
>
> X64/ArchInterruptDefs.h
>
>
>
> @@ -63,3 +63,5 @@ [Pcd]
> [FeaturePcd]
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ##
> CONSUMES
>
>
>
> +[BuildOptions]
>
> + XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
>
> --
> 2.39.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
[not found] ` <17517877FE72B326.27612@groups.io>
@ 2023-03-31 9:58 ` Ni, Ray
2023-03-31 10:14 ` Ard Biesheuvel
0 siblings, 1 reply; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 9:58 UTC (permalink / raw)
To: devel@edk2.groups.io, Ni, Ray, Ard Biesheuvel
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky, Marvin Häuser
By the way, which ("%" or "#") should be used for def check in NASM?
I thought we need to use "%" but your patch uses "#".
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Friday, March 31, 2023 5:56 PM
> To: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io
> Cc: 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: Re: [edk2-devel] [RFT PATCH v3 2/5]
> UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
>
> Ard,
> Thanks for the detailed commit messages. That really helps me to
> understand why XCODE version
> was needed.
>
> However, I feel it would be great if you can "highlight" what are changed by
> this patch.
> The following is just an example. You can reword as you like.
>
> 1. Change for non-XCODE SecPeiCpuExceptionHandlerLib:
> * Use SecPeiExceptionHandlerAsm.nasm (renamed from
> ExceptionHandlerAsm.nasm)
> * Removed some unnecessary absolute references
> * (32 IDT stubs are still in .text.)
> 2. Change for XCODE SecPeiCpuExceptionHandlerLib:
> * Use SecPeiExceptionHandlerAsm.nasm instead of
> Xcode5ExceptionHandlerAsm.nasm
> * CET logic is not in SecPeiExceptionHandlerAsm.nasm (but aligns to non-
> XCODE lib instance)
> * Fixed a bug that does runtime fixup in TEXT section in SPI flash.
> * Emitted the code carrying the absolute symbol references into the .data
> which XCODE or
> LLD linkers allow.
> Then fixup can be done by other build tools such as GenFv if the code runs
> in SPI flash,
> or by PE coff loader if the code is loaded to memory.
>
> Again, thanks for the quick patches just because I asked some XCODE
> questions.
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > 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: [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use
> single
> > SEC/PEI version
> >
> > Currently, we use the non-Xcode5 version of ExceptionHandlerAsm.nasm
> > only for the SEC and PEI phases, and this version was not compatible
> > with the XCODE or LLD linkers, which do not permit absolute relocations
> > in read-only sections.
> >
> > Given that SEC and PEI code typically executes in place from flash and
> > does not use page alignment for sections, we can simply emit the code
> > carrying the absolute symbol references into the .data segment instead.
> > This works around the linker's objections, and the resulting image will
> > be mapped executable in its entirety anyway. Since this is only needed
> > for XCODE, let's make this change conditionally using a preprocessor
> > macro.
> >
> > Let's rename the .nasm file to reflect the fact that is used for the
> > SecPei flavor of this library only, and while at it, remove some
> > unnecessary absolute references.
> >
> > Also update the Xcode specific version of this library, and use this
> > source file instead. This is necesessary, as the Xcode specific version
> > modifies its own code at runtime, which is not permitted in SEC or PEI.
> > Note that this also removes CET support from the Xcode5 specific build
> > of the SEC/PEI version of this library, but this is not needed this
> > early in any case, and this aligns it with other toolchains, which use
> > this version of the library, which does not have CET support either.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib
> > .inf | 4 +++-
> >
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{ExceptionHandlerAsm.na
> > sm => SecPeiExceptionHandlerAsm.nasm} | 12 ++++++++----
> >
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
> > ndlerLib.inf | 4 +++-
> > 3 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> > Lib.inf
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> > Lib.inf
> > index df44371fe018e06d..885bb6638ab58620 100644
> > ---
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> > Lib.inf
> > +++
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> > Lib.inf
> > @@ -28,7 +28,7 @@ [Sources.Ia32]
> > Ia32/ArchInterruptDefs.h
> >
> >
> >
> > [Sources.X64]
> >
> > - X64/ExceptionHandlerAsm.nasm
> >
> > + X64/SecPeiExceptionHandlerAsm.nasm
> >
> > X64/ArchExceptionHandler.c
> >
> > X64/ArchInterruptDefs.h
> >
> >
> >
> > @@ -58,3 +58,5 @@ [Pcd]
> > [FeaturePcd]
> >
> > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ##
> > CONSUMES
> >
> >
> >
> > +[BuildOptions]
> >
> > + XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
> >
> > diff --git
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> > asm
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandler
> > Asm.nasm
> > similarity index 94%
> > rename from
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
> > m
> > rename to
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandlerA
> > sm.nasm
> > index aaf8d622e6f3b8f1..ec45c60181906c14 100644
> > ---
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> > asm
> > +++
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandler
> > Asm.nasm
> > @@ -27,7 +27,9 @@ extern ASM_PFX(CommonExceptionHandler)
> > SECTION .data
> >
> >
> >
> > DEFAULT REL
> >
> > +#ifndef NO_ABSOLUTE_RELOCS_IN_TEXT
> >
> > SECTION .text
> >
> > +#endif
> >
> >
> >
> > ALIGN 8
> >
> >
> >
> > @@ -51,6 +53,9 @@ HookAfterStubHeaderBegin:
> > push rax
> >
> > mov rax, HookAfterStubHeaderEnd
> >
> > jmp rax
> >
> > +
> >
> > +SECTION .text
> >
> > +
> >
> > HookAfterStubHeaderEnd:
> >
> > mov rax, rsp
> >
> > and sp, 0xfff0 ; make sure 16-byte aligned for exception context
> >
> > @@ -276,8 +281,7 @@ DrFinish:
> > ; and make sure RSP is 16-byte aligned
> >
> > ;
> >
> > sub rsp, 4 * 8 + 8
> >
> > - mov rax, ASM_PFX(CommonExceptionHandler)
> >
> > - call rax
> >
> > + call ASM_PFX(CommonExceptionHandler)
> >
> > add rsp, 4 * 8 + 8
> >
> >
> >
> > cli
> >
> > @@ -384,10 +388,10 @@ DoIret:
> > ; comments here for definition of address map
> >
> > global ASM_PFX(AsmGetTemplateAddressMap)
> >
> > ASM_PFX(AsmGetTemplateAddressMap):
> >
> > - mov rax, AsmIdtVectorBegin
> >
> > + lea rax, [AsmIdtVectorBegin]
> >
> > mov qword [rcx], rax
> >
> > mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
> >
> > - mov rax, HookAfterStubHeaderBegin
> >
> > + lea rax, [HookAfterStubHeaderBegin]
> >
> > mov qword [rcx + 0x10], rax
> >
> > ret
> >
> >
> >
> > diff --git
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> > HandlerLib.inf
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> > HandlerLib.inf
> > index 619b39d7f1de9ae3..17f872bb15eb0ff7 100644
> > ---
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> > HandlerLib.inf
> > +++
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> > HandlerLib.inf
> > @@ -33,7 +33,7 @@ [Sources.Ia32]
> > Ia32/ArchInterruptDefs.h
> >
> >
> >
> > [Sources.X64]
> >
> > - X64/Xcode5ExceptionHandlerAsm.nasm
> >
> > + X64/SecPeiExceptionHandlerAsm.nasm
> >
> > X64/ArchExceptionHandler.c
> >
> > X64/ArchInterruptDefs.h
> >
> >
> >
> > @@ -63,3 +63,5 @@ [Pcd]
> > [FeaturePcd]
> >
> > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ##
> > CONSUMES
> >
> >
> >
> > +[BuildOptions]
> >
> > + XCODE:*_*_X64_PP_FLAGS = -DNO_ABSOLUTE_RELOCS_IN_TEXT
> >
> > --
> > 2.39.2
>
>
>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
2023-03-31 9:14 ` [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only Ard Biesheuvel
@ 2023-03-31 10:03 ` Ni, Ray
2023-03-31 10:20 ` Ni, Ray
1 sibling, 0 replies; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 10:03 UTC (permalink / raw)
To: devel@edk2.groups.io, ardb@kernel.org
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky, Marvin Häuser
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]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
2023-03-31 9:14 [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ard Biesheuvel
` (4 preceding siblings ...)
2023-03-31 9:14 ` [RFT PATCH v3 5/5] UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version Ard Biesheuvel
@ 2023-03-31 10:08 ` Ni, Ray
2023-03-31 10:15 ` Ard Biesheuvel
2023-03-31 10:41 ` Marvin Häuser
5 siblings, 2 replies; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 10:08 UTC (permalink / raw)
To: devel@edk2.groups.io, ardb@kernel.org
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky, Marvin Häuser
Ard,
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-----
> 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 0/5] UefiCpuPkg, OvmfPkg: Simplify
> CpuExceptionHandlerLib
>
> We have a special version of CpuExceptionHandlerLib for XCODE5, whose
>
> linker (LLD) does not permit absolute symbol references in read-only
>
> sections.
>
>
>
> Instead of fixing this up at runtime for all toolchains (which is done
>
> by writing the fixed up values to the .text section, which we'd prefer
>
> to avoid), tweak the SEC/PEI version so it does not need this, and
>
> update the remaining versions to only incorporate this logic when using
>
> the XCODE toolchain.
>
>
>
> Changes since v3:
>
> - As Marvin points out, using '-read_only_relocs suppress' with the X64
>
> XCODE linker is a terrible idea, as it corrupts the resulting PE
>
> binaries, so instead, let's do the following:
>
> . tweak the SEC/PEI version of the library so the relocs are emitted
>
> into .data when using XCODE;
>
> . tweak the other versions so the runtime fixups are only done when
>
> using XCODE
>
> - add acks from Jiewen and Ray
>
>
>
> Changes since v2:
>
> - pass linker switches to permit absolute relocations in read-only
>
> regions, and keep all code in .text
>
>
>
> Cc: "Ni, Ray" <ray.ni@intel.com>
>
> Cc: Andrew Fish <afish@apple.com>
>
> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
>
> Cc: "Liu, Zhiguang" <zhiguang.liu@intel.com>
>
> Cc: Rebecca Cran <rebecca@bsdio.com>
>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>
> Cc: Marvin Häuser <mhaeuser@posteo.de>
>
>
>
> Ard Biesheuvel (5):
>
> BaseTools/tools_def CLANGDWARF: Permit text relocations
>
> UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
>
> UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
>
> OvmfPkg: Drop special Xcode5 version of exception handler library
>
> UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version
>
>
>
> BaseTools/Conf/tools_def.template
> | 2 +-
>
> OvmfPkg/AmdSev/AmdSevX64.dsc
> | 4 -
>
> OvmfPkg/CloudHv/CloudHvX64.dsc
> | 4 -
>
> OvmfPkg/IntelTdx/IntelTdxX64.dsc |
> 4 -
>
> OvmfPkg/Microvm/MicrovmX64.dsc
> | 4 -
>
> OvmfPkg/OvmfPkgIa32.dsc | 4
> -
>
> OvmfPkg/OvmfPkgIa32X64.dsc |
> 4 -
>
> OvmfPkg/OvmfPkgX64.dsc | 4
> -
>
> OvmfPkg/OvmfXen.dsc | 4 -
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.in
> f | 5 +-
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
> | 4 +-
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib
> .inf | 4 +-
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.i
> nf | 4 +-
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
> m | 116 +++++++++++++++++---
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandler
> Asm.nasm => SecPeiExceptionHandlerAsm.nasm} | 108 +++---------------
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
> ndlerLib.inf | 65 -----------
>
>
> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
> ndlerLib.uni | 18 ---
>
> UefiCpuPkg/UefiCpuPkg.dsc | 7
> --
>
> 18 files changed, 133 insertions(+), 232 deletions(-)
>
> rename
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandler
> Asm.nasm => SecPeiExceptionHandlerAsm.nasm} (70%)
>
> delete mode 100644
> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
> ndlerLib.inf
>
> delete mode 100644
> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
> ndlerLib.uni
>
>
>
> --
>
> 2.39.2
>
>
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#102255):
> https://edk2.groups.io/g/devel/message/102255
> Mute This Topic: https://groups.io/mt/97969646/1712937
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 9:56 ` Ni, Ray
@ 2023-03-31 10:12 ` Ard Biesheuvel
2023-03-31 10:19 ` Ni, Ray
0 siblings, 1 reply; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 10:12 UTC (permalink / raw)
To: devel, ray.ni
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky, Marvin Häuser
On Fri, 31 Mar 2023 at 11:56, Ni, Ray <ray.ni@intel.com> wrote:
>
> Ard,
> Thanks for the detailed commit messages. That really helps me to understand why XCODE version
> was needed.
>
> However, I feel it would be great if you can "highlight" what are changed by this patch.
> The following is just an example. You can reword as you like.
>
> 1. Change for non-XCODE SecPeiCpuExceptionHandlerLib:
> * Use SecPeiExceptionHandlerAsm.nasm (renamed from ExceptionHandlerAsm.nasm)
> * Removed some unnecessary absolute references
> * (32 IDT stubs are still in .text.)
Indeed
> 2. Change for XCODE SecPeiCpuExceptionHandlerLib:
> * Use SecPeiExceptionHandlerAsm.nasm instead of Xcode5ExceptionHandlerAsm.nasm
> * CET logic is not in SecPeiExceptionHandlerAsm.nasm (but aligns to non-XCODE lib instance)
No, this does not actually change in this patch. The CET logic does
not exist in the generic SecPei version either before or after this
patch.
Only the Xcode version is changed, because that version uses the same
as the Dxe/Smm version, which does have the CET code.
> * Fixed a bug that does runtime fixup in TEXT section in SPI flash.
> * Emitted the code carrying the absolute symbol references into the .data which XCODE or
> LLD linkers allow.
> Then fixup can be done by other build tools such as GenFv if the code runs in SPI flash,
> or by PE coff loader if the code is loaded to memory.
>
> Again, thanks for the quick patches just because I asked some XCODE questions.
>
No problem. I'd like to get this fixed too for OVMF.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 9:58 ` Ni, Ray
@ 2023-03-31 10:14 ` Ard Biesheuvel
2023-03-31 10:16 ` Ni, Ray
0 siblings, 1 reply; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 10:14 UTC (permalink / raw)
To: Ni, Ray
Cc: devel@edk2.groups.io, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
On Fri, 31 Mar 2023 at 11:58, Ni, Ray <ray.ni@intel.com> wrote:
>
> By the way, which ("%" or "#") should be used for def check in NASM?
> I thought we need to use "%" but your patch uses "#".
>
The build rule for NASM files is
Trim --asm-file -o ${d_path}(+)${s_base}.i -i $(INC_LIST) ${src}
"$(PP)" $(DEPS_FLAGS) $(PP_FLAGS) $(INC) ${src} >
${d_path}(+)${s_base}.ii
Trim --trim-long --source-code -o ${d_path}(+)${s_base}.iii
${d_path}(+)${s_base}.ii
"$(NASM)" -I${s_path}(+) $(NASM_INC) $(NASM_FLAGS) -o $dst
${d_path}(+)${s_base}.iii
So the preprocessor $(PP) is executed first, which takes care of the #ifdefs
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
2023-03-31 10:08 ` [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ni, Ray
@ 2023-03-31 10:15 ` Ard Biesheuvel
2023-03-31 10:41 ` Marvin Häuser
1 sibling, 0 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 10:15 UTC (permalink / raw)
To: Ni, Ray
Cc: devel@edk2.groups.io, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
On Fri, 31 Mar 2023 at 12:09, Ni, Ray <ray.ni@intel.com> wrote:
>
> Ard,
> 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?
>
Yeah, good question. So this is why I dropped this now - it doesn't
work as expected.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 10:14 ` Ard Biesheuvel
@ 2023-03-31 10:16 ` Ni, Ray
2023-03-31 10:19 ` Ard Biesheuvel
0 siblings, 1 reply; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 10:16 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel@edk2.groups.io, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
So, for nasm advanced macros (rep), use "%". Otherwise, either is fine.
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Friday, March 31, 2023 6:14 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; 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: Re: [edk2-devel] [RFT PATCH v3 2/5]
> UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
>
> On Fri, 31 Mar 2023 at 11:58, Ni, Ray <ray.ni@intel.com> wrote:
> >
> > By the way, which ("%" or "#") should be used for def check in NASM?
> > I thought we need to use "%" but your patch uses "#".
> >
>
> The build rule for NASM files is
>
> Trim --asm-file -o ${d_path}(+)${s_base}.i -i $(INC_LIST) ${src}
> "$(PP)" $(DEPS_FLAGS) $(PP_FLAGS) $(INC) ${src} >
> ${d_path}(+)${s_base}.ii
> Trim --trim-long --source-code -o ${d_path}(+)${s_base}.iii
> ${d_path}(+)${s_base}.ii
> "$(NASM)" -I${s_path}(+) $(NASM_INC) $(NASM_FLAGS) -o $dst
> ${d_path}(+)${s_base}.iii
>
> So the preprocessor $(PP) is executed first, which takes care of the #ifdefs
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 10:16 ` Ni, Ray
@ 2023-03-31 10:19 ` Ard Biesheuvel
0 siblings, 0 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 10:19 UTC (permalink / raw)
To: Ni, Ray
Cc: devel@edk2.groups.io, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
On Fri, 31 Mar 2023 at 12:16, Ni, Ray <ray.ni@intel.com> wrote:
>
> So, for nasm advanced macros (rep), use "%". Otherwise, either is fine.
>
Exactly. These are preprocessor macros, not nasm macros.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 10:12 ` [edk2-devel] " Ard Biesheuvel
@ 2023-03-31 10:19 ` Ni, Ray
2023-03-31 10:49 ` Ard Biesheuvel
0 siblings, 1 reply; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 10:19 UTC (permalink / raw)
To: devel@edk2.groups.io, ardb@kernel.org
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky, Marvin Häuser
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Friday, March 31, 2023 6:13 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
> Cc: 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: Re: [edk2-devel] [RFT PATCH v3 2/5]
> UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
>
> On Fri, 31 Mar 2023 at 11:56, Ni, Ray <ray.ni@intel.com> wrote:
> >
> > Ard,
> > Thanks for the detailed commit messages. That really helps me to
> understand why XCODE version
> > was needed.
> >
> > However, I feel it would be great if you can "highlight" what are changed by
> this patch.
> > The following is just an example. You can reword as you like.
> >
> > 1. Change for non-XCODE SecPeiCpuExceptionHandlerLib:
> > * Use SecPeiExceptionHandlerAsm.nasm (renamed from
> ExceptionHandlerAsm.nasm)
> > * Removed some unnecessary absolute references
> > * (32 IDT stubs are still in .text.)
>
> Indeed
>
> > 2. Change for XCODE SecPeiCpuExceptionHandlerLib:
> > * Use SecPeiExceptionHandlerAsm.nasm instead of
> Xcode5ExceptionHandlerAsm.nasm
> > * CET logic is not in SecPeiExceptionHandlerAsm.nasm (but aligns to non-
> XCODE lib instance)
>
> No, this does not actually change in this patch. The CET logic does
> not exist in the generic SecPei version either before or after this
> patch.
Because of this patch, CET logic is removed from XCODE SecPeiCpuExceptionHandlerLib.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
2023-03-31 9:14 ` [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only Ard Biesheuvel
2023-03-31 10:03 ` [edk2-devel] " Ni, Ray
@ 2023-03-31 10:20 ` Ni, Ray
1 sibling, 0 replies; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 10:20 UTC (permalink / raw)
To: devel@edk2.groups.io, ardb@kernel.org
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky, Marvin Häuser
Ok I see that the macro is added to PP flags.
I thought it's added to NASM build flags.
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----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]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
2023-03-31 10:08 ` [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ni, Ray
2023-03-31 10:15 ` Ard Biesheuvel
@ 2023-03-31 10:41 ` Marvin Häuser
2023-03-31 11:03 ` Ard Biesheuvel
1 sibling, 1 reply; 23+ messages in thread
From: Marvin Häuser @ 2023-03-31 10:41 UTC (permalink / raw)
To: Ni, Ray
Cc: devel, ardb, Andrew Fish, Kinney, Michael D, Liu, Zhiguang,
Rebecca Cran, Tom Lendacky
Hi Ray,
> On 31. Mar 2023, at 12:09, Ni, Ray <ray.ni@intel.com> wrote:
>
> Ard,
> What does "-read_only_relocs suppress" control?
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 silently
> so the final .text just cannot run at all?
Could 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
>
> Thanks,
> Ray
>
>> -----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 0/5] UefiCpuPkg, OvmfPkg: Simplify
>> CpuExceptionHandlerLib
>>
>> We have a special version of CpuExceptionHandlerLib for XCODE5, whose
>>
>> linker (LLD) does not permit absolute symbol references in read-only
>>
>> sections.
>>
>>
>>
>> Instead of fixing this up at runtime for all toolchains (which is done
>>
>> by writing the fixed up values to the .text section, which we'd prefer
>>
>> to avoid), tweak the SEC/PEI version so it does not need this, and
>>
>> update the remaining versions to only incorporate this logic when using
>>
>> the XCODE toolchain.
>>
>>
>>
>> Changes since v3:
>>
>> - As Marvin points out, using '-read_only_relocs suppress' with the X64
>>
>> XCODE linker is a terrible idea, as it corrupts the resulting PE
>>
>> binaries, so instead, let's do the following:
>>
>> . tweak the SEC/PEI version of the library so the relocs are emitted
>>
>> into .data when using XCODE;
>>
>> . tweak the other versions so the runtime fixups are only done when
>>
>> using XCODE
>>
>> - add acks from Jiewen and Ray
>>
>>
>>
>> Changes since v2:
>>
>> - pass linker switches to permit absolute relocations in read-only
>>
>> regions, and keep all code in .text
>>
>>
>>
>> Cc: "Ni, Ray" <ray.ni@intel.com>
>>
>> Cc: Andrew Fish <afish@apple.com>
>>
>> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
>>
>> Cc: "Liu, Zhiguang" <zhiguang.liu@intel.com>
>>
>> Cc: Rebecca Cran <rebecca@bsdio.com>
>>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> Cc: Marvin Häuser <mhaeuser@posteo.de>
>>
>>
>>
>> Ard Biesheuvel (5):
>>
>> BaseTools/tools_def CLANGDWARF: Permit text relocations
>>
>> UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
>>
>> UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
>>
>> OvmfPkg: Drop special Xcode5 version of exception handler library
>>
>> UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version
>>
>>
>>
>> BaseTools/Conf/tools_def.template
>> | 2 +-
>>
>> OvmfPkg/AmdSev/AmdSevX64.dsc
>> | 4 -
>>
>> OvmfPkg/CloudHv/CloudHvX64.dsc
>> | 4 -
>>
>> OvmfPkg/IntelTdx/IntelTdxX64.dsc |
>> 4 -
>>
>> OvmfPkg/Microvm/MicrovmX64.dsc
>> | 4 -
>>
>> OvmfPkg/OvmfPkgIa32.dsc | 4
>> -
>>
>> OvmfPkg/OvmfPkgIa32X64.dsc |
>> 4 -
>>
>> OvmfPkg/OvmfPkgX64.dsc | 4
>> -
>>
>> OvmfPkg/OvmfXen.dsc | 4 -
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.in
>> f | 5 +-
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
>> | 4 +-
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib
>> .inf | 4 +-
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.i
>> nf | 4 +-
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
>> m | 116 +++++++++++++++++---
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandler
>> Asm.nasm => SecPeiExceptionHandlerAsm.nasm} | 108 +++---------------
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
>> ndlerLib.inf | 65 -----------
>>
>>
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
>> ndlerLib.uni | 18 ---
>>
>> UefiCpuPkg/UefiCpuPkg.dsc | 7
>> --
>>
>> 18 files changed, 133 insertions(+), 232 deletions(-)
>>
>> rename
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{Xcode5ExceptionHandler
>> Asm.nasm => SecPeiExceptionHandlerAsm.nasm} (70%)
>>
>> delete mode 100644
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
>> ndlerLib.inf
>>
>> delete mode 100644
>> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
>> ndlerLib.uni
>>
>>
>>
>> --
>>
>> 2.39.2
>>
>>
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#102255):
>> https://edk2.groups.io/g/devel/message/102255
>> Mute This Topic: https://groups.io/mt/97969646/1712937
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
>> -=-=-=-=-=-=
>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
2023-03-31 10:19 ` Ni, Ray
@ 2023-03-31 10:49 ` Ard Biesheuvel
0 siblings, 0 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 10:49 UTC (permalink / raw)
To: Ni, Ray
Cc: devel@edk2.groups.io, Andrew Fish, Kinney, Michael D,
Liu, Zhiguang, Rebecca Cran, Tom Lendacky, Marvin Häuser
On Fri, 31 Mar 2023 at 12:19, Ni, Ray <ray.ni@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > Biesheuvel
> > Sent: Friday, March 31, 2023 6:13 PM
> > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
> > Cc: 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: Re: [edk2-devel] [RFT PATCH v3 2/5]
> > UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
> >
> > On Fri, 31 Mar 2023 at 11:56, Ni, Ray <ray.ni@intel.com> wrote:
> > >
> > > Ard,
> > > Thanks for the detailed commit messages. That really helps me to
> > understand why XCODE version
> > > was needed.
> > >
> > > However, I feel it would be great if you can "highlight" what are changed by
> > this patch.
> > > The following is just an example. You can reword as you like.
> > >
> > > 1. Change for non-XCODE SecPeiCpuExceptionHandlerLib:
> > > * Use SecPeiExceptionHandlerAsm.nasm (renamed from
> > ExceptionHandlerAsm.nasm)
> > > * Removed some unnecessary absolute references
> > > * (32 IDT stubs are still in .text.)
> >
> > Indeed
> >
> > > 2. Change for XCODE SecPeiCpuExceptionHandlerLib:
> > > * Use SecPeiExceptionHandlerAsm.nasm instead of
> > Xcode5ExceptionHandlerAsm.nasm
> > > * CET logic is not in SecPeiExceptionHandlerAsm.nasm (but aligns to non-
> > XCODE lib instance)
> >
> > No, this does not actually change in this patch. The CET logic does
> > not exist in the generic SecPei version either before or after this
> > patch.
>
> Because of this patch, CET logic is removed from XCODE SecPeiCpuExceptionHandlerLib.
Indeed - I will make that clear in the commit log.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
2023-03-31 10:41 ` Marvin Häuser
@ 2023-03-31 11:03 ` Ard Biesheuvel
2023-03-31 11:09 ` Marvin Häuser
0 siblings, 1 reply; 23+ messages in thread
From: Ard Biesheuvel @ 2023-03-31 11:03 UTC (permalink / raw)
To: Marvin Häuser
Cc: Ni, Ray, devel, Andrew Fish, Kinney, Michael D, Liu, Zhiguang,
Rebecca Cran, Tom Lendacky
On Fri, 31 Mar 2023 at 12:41, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
> Hi Ray,
>
> > On 31. Mar 2023, at 12:09, Ni, Ray <ray.ni@intel.com> wrote:
> >
> > Ard,
> > What does "-read_only_relocs suppress" control?
>
> 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 silently
> > so the final .text just cannot run at all?
>
> Could 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.
>
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
2023-03-31 11:03 ` Ard Biesheuvel
@ 2023-03-31 11:09 ` Marvin Häuser
2023-03-31 14:39 ` Ni, Ray
0 siblings, 1 reply; 23+ messages in thread
From: Marvin Häuser @ 2023-03-31 11:09 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Ni, Ray, devel, Andrew Fish, Kinney, Michael D, Liu, Zhiguang,
Rebecca Cran, Tom Lendacky
> On 31. Mar 2023, at 13:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 31 Mar 2023 at 12:41, Marvin Häuser <mhaeuser@posteo.de> wrote:
>>
>> Hi Ray,
>>
>>>> On 31. Mar 2023, at 12:09, Ni, Ray <ray.ni@intel.com> wrote:
>>>
>>> Ard,
>>> What does "-read_only_relocs suppress" control?
>>
>> 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 silently
>>> so the final .text just cannot run at all?
>>
>> Could 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.
>>
>
> As *I* understood it, it means suppress the *warning* not suppress the
> *relocation*
Correct.
>
> But the resulting binaries are broken, so it doesn't really matter.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
2023-03-31 11:09 ` Marvin Häuser
@ 2023-03-31 14:39 ` Ni, Ray
2023-03-31 14:42 ` Marvin Häuser
0 siblings, 1 reply; 23+ messages in thread
From: Ni, Ray @ 2023-03-31 14:39 UTC (permalink / raw)
To: devel@edk2.groups.io, mhaeuser@posteo.de, Ard Biesheuvel
Cc: Andrew Fish, Kinney, Michael D, Liu, Zhiguang, Rebecca Cran,
Tom Lendacky
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin
> Häuser
> Sent: Friday, March 31, 2023 7:10 PM
> To: Ard Biesheuvel <ardb@kernel.org>
> Cc: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; 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>
> Subject: Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify
> CpuExceptionHandlerLib
>
>
> > On 31. Mar 2023, at 13:03, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 31 Mar 2023 at 12:41, Marvin Häuser <mhaeuser@posteo.de> wrote:
> >>
> >> Hi Ray,
> >>
> >>>> On 31. Mar 2023, at 12:09, Ni, Ray <ray.ni@intel.com> wrote:
> >>>
> >>> Ard,
> >>> What does "-read_only_relocs suppress" control?
> >>
> >> 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
> silently
> >>> so the final .text just cannot run at all?
> >>
> >> Could 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.
> >>
> >
> > As *I* understood it, it means suppress the *warning* not suppress the
> > *relocation*
What the meaning of "suppress relocation"?
Why the final binaries are not executable?
>
> Correct.
>
> >
> > But the resulting binaries are broken, so it doesn't really matter.
>
>
>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib
2023-03-31 14:39 ` Ni, Ray
@ 2023-03-31 14:42 ` Marvin Häuser
0 siblings, 0 replies; 23+ messages in thread
From: Marvin Häuser @ 2023-03-31 14:42 UTC (permalink / raw)
To: Ni, Ray
Cc: devel@edk2.groups.io, Ard Biesheuvel, Andrew Fish,
Kinney, Michael D, Liu, Zhiguang, Rebecca Cran, Tom Lendacky
[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]
> On 31. Mar 2023, at 16:39, Ni, Ray <ray.ni@intel.com> wrote:
>
>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin
>> Häuser
>> Sent: Friday, March 31, 2023 7:10 PM
>> To: Ard Biesheuvel <ardb@kernel.org>
>> Cc: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; 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>
>> Subject: Re: [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify
>> CpuExceptionHandlerLib
>>
>>
>>> On 31. Mar 2023, at 13:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>
>>> On Fri, 31 Mar 2023 at 12:41, Marvin Häuser <mhaeuser@posteo.de> wrote:
>>>>
>>>> Hi Ray,
>>>>
>>>>>> On 31. Mar 2023, at 12:09, Ni, Ray <ray.ni@intel.com> wrote:
>>>>>
>>>>> Ard,
>>>>> What does "-read_only_relocs suppress" control?
>>>>
>>>> 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
>> silently
>>>>> so the final .text just cannot run at all?
>>>>
>>>> Could 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.
>>>>
>>>
>>> As *I* understood it, it means suppress the *warning* not suppress the
>>> *relocation*
>
> What the meaning of "suppress relocation"?
The option naming is just a bit odd, it suppresses the warning about relocations to read-only segments, not the relocations themselves.
> Why the final binaries are not executable?
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
>
>>
>> Correct.
>>
>>>
>>> But the resulting binaries are broken, so it doesn't really matter.
>>
>>
>>
>>
>>
>
[-- Attachment #2: Type: text/html, Size: 3351 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2023-03-31 14:43 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-31 9:14 [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 1/5] BaseTools/tools_def CLANGDWARF: Permit text relocations Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version Ard Biesheuvel
2023-03-31 9:56 ` Ni, Ray
2023-03-31 10:12 ` [edk2-devel] " Ard Biesheuvel
2023-03-31 10:19 ` Ni, Ray
2023-03-31 10:49 ` Ard Biesheuvel
[not found] ` <17517877FE72B326.27612@groups.io>
2023-03-31 9:58 ` Ni, Ray
2023-03-31 10:14 ` Ard Biesheuvel
2023-03-31 10:16 ` Ni, Ray
2023-03-31 10:19 ` Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 3/5] UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only Ard Biesheuvel
2023-03-31 10:03 ` [edk2-devel] " Ni, Ray
2023-03-31 10:20 ` Ni, Ray
2023-03-31 9:14 ` [RFT PATCH v3 4/5] OvmfPkg: Drop special Xcode5 version of exception handler library Ard Biesheuvel
2023-03-31 9:14 ` [RFT PATCH v3 5/5] UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version Ard Biesheuvel
2023-03-31 10:08 ` [edk2-devel] [RFT PATCH v3 0/5] UefiCpuPkg, OvmfPkg: Simplify CpuExceptionHandlerLib Ni, Ray
2023-03-31 10:15 ` Ard Biesheuvel
2023-03-31 10:41 ` Marvin Häuser
2023-03-31 11:03 ` Ard Biesheuvel
2023-03-31 11:09 ` Marvin Häuser
2023-03-31 14:39 ` Ni, Ray
2023-03-31 14:42 ` Marvin Häuser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox