[RFT PATCH v3 2/5] UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version


Ard Biesheuvel
 

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@...>
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf=
| 4 +++-
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/{ExceptionHandlerAsm.nasm =
=3D> SecPeiExceptionHandlerAsm.nasm} | 12 ++++++++----
UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerL=
ib.inf | 4 +++-
3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHa=
ndlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException=
HandlerLib.inf
index df44371fe018e06d..885bb6638ab58620 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLi=
b.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLi=
b.inf
@@ -28,7 +28,7 @@ [Sources.Ia32]
Ia32/ArchInterruptDefs.h=0D
=0D
[Sources.X64]=0D
- X64/ExceptionHandlerAsm.nasm=0D
+ X64/SecPeiExceptionHandlerAsm.nasm=0D
X64/ArchExceptionHandler.c=0D
X64/ArchInterruptDefs.h=0D
=0D
@@ -58,3 +58,5 @@ [Pcd]
[FeaturePcd]=0D
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONS=
UMES=0D
=0D
+[BuildOptions]=0D
+ XCODE:*_*_X64_PP_FLAGS =3D -DNO_ABSOLUTE_RELOCS_IN_TEXT=0D
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandler=
Asm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHan=
dlerAsm.nasm
similarity index 94%
rename from UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerA=
sm.nasm
rename to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHand=
lerAsm.nasm
index aaf8d622e6f3b8f1..ec45c60181906c14 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/SecPeiExceptionHandlerA=
sm.nasm
@@ -27,7 +27,9 @@ extern ASM_PFX(CommonExceptionHandler)
SECTION .data=0D
=0D
DEFAULT REL=0D
+#ifndef NO_ABSOLUTE_RELOCS_IN_TEXT=0D
SECTION .text=0D
+#endif=0D
=0D
ALIGN 8=0D
=0D
@@ -51,6 +53,9 @@ HookAfterStubHeaderBegin:
push rax=0D
mov rax, HookAfterStubHeaderEnd=0D
jmp rax=0D
+=0D
+SECTION .text=0D
+=0D
HookAfterStubHeaderEnd:=0D
mov rax, rsp=0D
and sp, 0xfff0 ; make sure 16-byte aligned for exception c=
ontext=0D
@@ -276,8 +281,7 @@ DrFinish:
; and make sure RSP is 16-byte aligned=0D
;=0D
sub rsp, 4 * 8 + 8=0D
- mov rax, ASM_PFX(CommonExceptionHandler)=0D
- call rax=0D
+ call ASM_PFX(CommonExceptionHandler)=0D
add rsp, 4 * 8 + 8=0D
=0D
cli=0D
@@ -384,10 +388,10 @@ DoIret:
; comments here for definition of address map=0D
global ASM_PFX(AsmGetTemplateAddressMap)=0D
ASM_PFX(AsmGetTemplateAddressMap):=0D
- mov rax, AsmIdtVectorBegin=0D
+ lea rax, [AsmIdtVectorBegin]=0D
mov qword [rcx], rax=0D
mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32=
=0D
- mov rax, HookAfterStubHeaderBegin=0D
+ lea rax, [HookAfterStubHeaderBegin]=0D
mov qword [rcx + 0x10], rax=0D
ret=0D
=0D
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExcep=
tionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPei=
CpuExceptionHandlerLib.inf
index 619b39d7f1de9ae3..17f872bb15eb0ff7 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHan=
dlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHan=
dlerLib.inf
@@ -33,7 +33,7 @@ [Sources.Ia32]
Ia32/ArchInterruptDefs.h=0D
=0D
[Sources.X64]=0D
- X64/Xcode5ExceptionHandlerAsm.nasm=0D
+ X64/SecPeiExceptionHandlerAsm.nasm=0D
X64/ArchExceptionHandler.c=0D
X64/ArchInterruptDefs.h=0D
=0D
@@ -63,3 +63,5 @@ [Pcd]
[FeaturePcd]=0D
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONS=
UMES=0D
=0D
+[BuildOptions]=0D
+ XCODE:*_*_X64_PP_FLAGS =3D -DNO_ABSOLUTE_RELOCS_IN_TEXT=0D
--=20
2.39.2


Ni, Ray
 

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@...>
Sent: Friday, March 31, 2023 5:15 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb@...>; Ni, Ray <ray.ni@...>; Andrew
Fish <afish@...>; Kinney, Michael D <michael.d.kinney@...>;
Liu, Zhiguang <zhiguang.liu@...>; Rebecca Cran
<rebecca@...>; Tom Lendacky <thomas.lendacky@...>;
Marvin Häuser <mhaeuser@...>
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@...>
---

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


Ni, Ray
 

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@...>; devel@edk2.groups.io
Cc: Andrew Fish <afish@...>; Kinney, Michael D
<michael.d.kinney@...>; Liu, Zhiguang <zhiguang.liu@...>;
Rebecca Cran <rebecca@...>; Tom Lendacky
<thomas.lendacky@...>; Marvin Häuser <mhaeuser@...>
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@...>
Sent: Friday, March 31, 2023 5:15 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb@...>; Ni, Ray <ray.ni@...>;
Andrew
Fish <afish@...>; Kinney, Michael D <michael.d.kinney@...>;
Liu, Zhiguang <zhiguang.liu@...>; Rebecca Cran
<rebecca@...>; Tom Lendacky <thomas.lendacky@...>;
Marvin Häuser <mhaeuser@...>
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@...>
---

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




Ard Biesheuvel
 

On Fri, 31 Mar 2023 at 11:56, Ni, Ray <ray.ni@...> 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.


Ard Biesheuvel
 

On Fri, 31 Mar 2023 at 11:58, Ni, Ray <ray.ni@...> 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


Ni, Ray
 

So, for nasm advanced macros (rep), use "%". Otherwise, either is fine.

-----Original Message-----
From: Ard Biesheuvel <ardb@...>
Sent: Friday, March 31, 2023 6:14 PM
To: Ni, Ray <ray.ni@...>
Cc: devel@edk2.groups.io; Andrew Fish <afish@...>; Kinney,
Michael D <michael.d.kinney@...>; Liu, Zhiguang
<zhiguang.liu@...>; Rebecca Cran <rebecca@...>; Tom
Lendacky <thomas.lendacky@...>; Marvin Häuser
<mhaeuser@...>
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@...> 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


Ard Biesheuvel
 

On Fri, 31 Mar 2023 at 12:16, Ni, Ray <ray.ni@...> wrote:

So, for nasm advanced macros (rep), use "%". Otherwise, either is fine.
Exactly. These are preprocessor macros, not nasm macros.


Ni, Ray
 

-----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@...>
Cc: Andrew Fish <afish@...>; Kinney, Michael D
<michael.d.kinney@...>; Liu, Zhiguang <zhiguang.liu@...>;
Rebecca Cran <rebecca@...>; Tom Lendacky
<thomas.lendacky@...>; Marvin Häuser <mhaeuser@...>
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@...> 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.


Ard Biesheuvel
 

On Fri, 31 Mar 2023 at 12:19, Ni, Ray <ray.ni@...> 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@...>
Cc: Andrew Fish <afish@...>; Kinney, Michael D
<michael.d.kinney@...>; Liu, Zhiguang <zhiguang.liu@...>;
Rebecca Cran <rebecca@...>; Tom Lendacky
<thomas.lendacky@...>; Marvin Häuser <mhaeuser@...>
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@...> 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.