1. Topics
  2. [PATCH 1/5] CpuException: Avoid allocating code pages for DXE instance

[PATCH 1/5] CpuException: Avoid allocating code pages for DXE instance

Ni, Ray
 

Today the DXE instance allocates code page and then copies the IDT
vectors to the allocated code page. Then it fixes up the vector number
in the IDT vector.

But if we update the NASM file to generate 256 IDT vectors, there is
no need to do the copy and fix-up.

A side effect is up to 4096 bytes (HOOKAFTER_STUB_SIZE * 256) is
used for 256 IDT vectors. While 32 IDT vectors only require 512 bytes.

But considering the code logic simplification, 3.5K space is not a big
deal. SEC instance still generates 32 IDT vectors so no impact to SEC.
If 3.5K is too much a waste in PEI phase, we can enhance the code
further to generate 32 vectors for PEI.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
---
.../CpuExceptionHandlerLib/DxeException.c | 22 -------------------
.../Ia32/ExceptionHandlerAsm.nasm | 4 ++--
.../X64/ExceptionHandlerAsm.nasm | 2 ++
.../X64/Xcode5ExceptionHandlerAsm.nasm | 9 ++++----
4 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c b/Uef=
iCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index 61f11e98f8..5083c4b8e8 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -95,9 +95,6 @@ InitializeCpuInterruptHandlers (
IA32_DESCRIPTOR IdtDescriptor;=0D
UINTN IdtEntryCount;=0D
EXCEPTION_HANDLER_TEMPLATE_MAP TemplateMap;=0D
- UINTN Index;=0D
- UINTN InterruptEntry;=0D
- UINT8 *InterruptEntryCode;=0D
RESERVED_VECTORS_DATA *ReservedVectors;=0D
EFI_CPU_INTERRUPT_HANDLER *ExternalInterruptHandler;=0D
=0D
@@ -138,25 +135,6 @@ InitializeCpuInterruptHandlers (
AsmGetTemplateAddressMap (&TemplateMap);=0D
ASSERT (TemplateMap.ExceptionStubHeaderSize <=3D HOOKAFTER_STUB_SIZE);=0D
=0D
- Status =3D gBS->AllocatePool (=0D
- EfiBootServicesCode,=0D
- TemplateMap.ExceptionStubHeaderSize * CPU_INTERRUPT_NUM,=
=0D
- (VOID **)&InterruptEntryCode=0D
- );=0D
- ASSERT (!EFI_ERROR (Status) && InterruptEntryCode !=3D NULL);=0D
-=0D
- InterruptEntry =3D (UINTN)InterruptEntryCode;=0D
- for (Index =3D 0; Index < CPU_INTERRUPT_NUM; Index++) {=0D
- CopyMem (=0D
- (VOID *)InterruptEntry,=0D
- (VOID *)TemplateMap.ExceptionStart,=0D
- TemplateMap.ExceptionStubHeaderSize=0D
- );=0D
- AsmVectorNumFixup ((VOID *)InterruptEntry, (UINT8)Index, (VOID *)Templ=
ateMap.ExceptionStart);=0D
- InterruptEntry +=3D TemplateMap.ExceptionStubHeaderSize;=0D
- }=0D
-=0D
- TemplateMap.ExceptionStart =3D (UINTN)InterruptEntry=
Code;=0D
mExceptionHandlerData.IdtEntryCount =3D CPU_INTERRUPT_NUM;=0D
mExceptionHandlerData.ReservedVectors =3D ReservedVectors;=0D
mExceptionHandlerData.ExternalInterruptHandler =3D ExternalInterruptHand=
ler;=0D
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandle=
rAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandler=
Asm.nasm
index 3fe9aed1e8..8ed2b8f455 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.na=
sm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.na=
sm
@@ -33,7 +33,7 @@ ALIGN 8
;=0D
AsmIdtVectorBegin:=0D
%assign Vector 0=0D
-%rep 32=0D
+%rep 256=0D
push byte %[Vector];=0D
push eax=0D
mov eax, ASM_PFX(CommonInterruptEntry)=0D
@@ -439,7 +439,7 @@ ASM_PFX(AsmGetTemplateAddressMap):
=0D
mov ebx, dword [ebp + 0x8]=0D
mov dword [ebx], AsmIdtVectorBegin=0D
- mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32=0D
+ mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256=0D
mov dword [ebx + 0x8], HookAfterStubBegin=0D
=0D
popad=0D
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandler=
Asm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAs=
m.nasm
index 9a806d1f86..aaf8d622e6 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -31,6 +31,8 @@ SECTION .text
=0D
ALIGN 8=0D
=0D
+; Generate 32 IDT vectors.=0D
+; 32 IDT vectors are enough because interrupts (32+) are not enabled in SE=
C and PEI phase.=0D
AsmIdtVectorBegin:=0D
%assign Vector 0=0D
%rep 32=0D
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionH=
andlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5Except=
ionHandlerAsm.nasm
index 9c72fa5815..7c0e3d3b0b 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerA=
sm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerA=
sm.nasm
@@ -53,9 +53,10 @@ SECTION .text
=0D
ALIGN 8=0D
=0D
+; Generate 256 IDT vectors.=0D
AsmIdtVectorBegin:=0D
%assign Vector 0=0D
-%rep 32=0D
+%rep 256=0D
push byte %[Vector]=0D
push rax=0D
mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptE=
ntry)=0D
@@ -453,16 +454,16 @@ global ASM_PFX(AsmGetTemplateAddressMap)
ASM_PFX(AsmGetTemplateAddressMap):=0D
lea rax, [AsmIdtVectorBegin]=0D
mov qword [rcx], rax=0D
- mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32=
=0D
+ mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 25=
6=0D
lea rax, [HookAfterStubHeaderBegin]=0D
mov qword [rcx + 0x10], rax=0D
=0D
; Fix up CommonInterruptEntry address=0D
lea rax, [ASM_PFX(CommonInterruptEntry)]=0D
lea rcx, [AsmIdtVectorBegin]=0D
-%rep 32=0D
+%rep 256=0D
mov qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin=
)], rax=0D
- add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32=0D
+ add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256=0D
%endrep=0D
; Fix up HookAfterStubHeaderEnd=0D
lea rax, [HookAfterStubHeaderEnd]=0D
--=20
2.35.1.windows.2
Wang, Jian J
 

Ray,

You changed "%rep 32" to "%rep 256" in Ia32/ExceptionHandlerAsm.nasm.
According to my understanding and your comments, this should be done
only to X64 code, right?

Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Friday, May 20, 2022 10:16 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>
Subject: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code pages
for DXE instance

Today the DXE instance allocates code page and then copies the IDT
vectors to the allocated code page. Then it fixes up the vector number
in the IDT vector.

But if we update the NASM file to generate 256 IDT vectors, there is
no need to do the copy and fix-up.

A side effect is up to 4096 bytes (HOOKAFTER_STUB_SIZE * 256) is
used for 256 IDT vectors. While 32 IDT vectors only require 512 bytes.

But considering the code logic simplification, 3.5K space is not a big
deal. SEC instance still generates 32 IDT vectors so no impact to SEC.
If 3.5K is too much a waste in PEI phase, we can enhance the code
further to generate 32 vectors for PEI.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
---
.../CpuExceptionHandlerLib/DxeException.c | 22 -------------------
.../Ia32/ExceptionHandlerAsm.nasm | 4 ++--
.../X64/ExceptionHandlerAsm.nasm | 2 ++
.../X64/Xcode5ExceptionHandlerAsm.nasm | 9 ++++----
4 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index 61f11e98f8..5083c4b8e8 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -95,9 +95,6 @@ InitializeCpuInterruptHandlers (
IA32_DESCRIPTOR IdtDescriptor;

UINTN IdtEntryCount;

EXCEPTION_HANDLER_TEMPLATE_MAP TemplateMap;

- UINTN Index;

- UINTN InterruptEntry;

- UINT8 *InterruptEntryCode;

RESERVED_VECTORS_DATA *ReservedVectors;

EFI_CPU_INTERRUPT_HANDLER *ExternalInterruptHandler;



@@ -138,25 +135,6 @@ InitializeCpuInterruptHandlers (
AsmGetTemplateAddressMap (&TemplateMap);

ASSERT (TemplateMap.ExceptionStubHeaderSize <= HOOKAFTER_STUB_SIZE);



- Status = gBS->AllocatePool (

- EfiBootServicesCode,

- TemplateMap.ExceptionStubHeaderSize * CPU_INTERRUPT_NUM,

- (VOID **)&InterruptEntryCode

- );

- ASSERT (!EFI_ERROR (Status) && InterruptEntryCode != NULL);

-

- InterruptEntry = (UINTN)InterruptEntryCode;

- for (Index = 0; Index < CPU_INTERRUPT_NUM; Index++) {

- CopyMem (

- (VOID *)InterruptEntry,

- (VOID *)TemplateMap.ExceptionStart,

- TemplateMap.ExceptionStubHeaderSize

- );

- AsmVectorNumFixup ((VOID *)InterruptEntry, (UINT8)Index, (VOID
*)TemplateMap.ExceptionStart);

- InterruptEntry += TemplateMap.ExceptionStubHeaderSize;

- }

-

- TemplateMap.ExceptionStart = (UINTN)InterruptEntryCode;

mExceptionHandlerData.IdtEntryCount = CPU_INTERRUPT_NUM;

mExceptionHandlerData.ReservedVectors = ReservedVectors;

mExceptionHandlerData.ExternalInterruptHandler = ExternalInterruptHandler;

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
index 3fe9aed1e8..8ed2b8f455 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
@@ -33,7 +33,7 @@ ALIGN 8
;

AsmIdtVectorBegin:

%assign Vector 0

-%rep 32

+%rep 256

push byte %[Vector];

push eax

mov eax, ASM_PFX(CommonInterruptEntry)

@@ -439,7 +439,7 @@ ASM_PFX(AsmGetTemplateAddressMap):


mov ebx, dword [ebp + 0x8]

mov dword [ebx], AsmIdtVectorBegin

- mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

mov dword [ebx + 0x8], HookAfterStubBegin



popad

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 9a806d1f86..aaf8d622e6 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -31,6 +31,8 @@ SECTION .text


ALIGN 8



+; Generate 32 IDT vectors.

+; 32 IDT vectors are enough because interrupts (32+) are not enabled in SEC and
PEI phase.

AsmIdtVectorBegin:

%assign Vector 0

%rep 32

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
index 9c72fa5815..7c0e3d3b0b 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
@@ -53,9 +53,10 @@ SECTION .text


ALIGN 8



+; Generate 256 IDT vectors.

AsmIdtVectorBegin:

%assign Vector 0

-%rep 32

+%rep 256

push byte %[Vector]

push rax

mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry)

@@ -453,16 +454,16 @@ global ASM_PFX(AsmGetTemplateAddressMap)
ASM_PFX(AsmGetTemplateAddressMap):

lea rax, [AsmIdtVectorBegin]

mov qword [rcx], rax

- mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

lea rax, [HookAfterStubHeaderBegin]

mov qword [rcx + 0x10], rax



; Fix up CommonInterruptEntry address

lea rax, [ASM_PFX(CommonInterruptEntry)]

lea rcx, [AsmIdtVectorBegin]

-%rep 32

+%rep 256

mov qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin)],
rax

- add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

%endrep

; Fix up HookAfterStubHeaderEnd

lea rax, [HookAfterStubHeaderEnd]

--
2.35.1.windows.2



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89916): https://edk2.groups.io/g/devel/message/89916
Mute This Topic: https://groups.io/mt/91231767/1768734
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.wang@...]
-=-=-=-=-=-=
Ni, Ray
 

Jian,
Ia32/ExceptionHandlerAsm.nasm is used by 32bit DxeCpuExceptionHandlerLib instance.

I agree the commit message is not correct. The commit message says
SEC still creates 32 entries but 32bit SEC creates 256 entries.

I will update the commit message to align to code behavior.

Thanks,
Ray

________________________________________
From: Wang, Jian J <jian.j.wang@...>
Sent: Monday, May 23, 2022 0:40
To: devel@edk2.groups.io; Ni, Ray
Cc: Dong, Eric
Subject: RE: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code pages for DXE instance

Ray,

You changed "%rep 32" to "%rep 256" in Ia32/ExceptionHandlerAsm.nasm.
According to my understanding and your comments, this should be done
only to X64 code, right?

Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Friday, May 20, 2022 10:16 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>
Subject: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code pages
for DXE instance

Today the DXE instance allocates code page and then copies the IDT
vectors to the allocated code page. Then it fixes up the vector number
in the IDT vector.

But if we update the NASM file to generate 256 IDT vectors, there is
no need to do the copy and fix-up.

A side effect is up to 4096 bytes (HOOKAFTER_STUB_SIZE * 256) is
used for 256 IDT vectors. While 32 IDT vectors only require 512 bytes.

But considering the code logic simplification, 3.5K space is not a big
deal. SEC instance still generates 32 IDT vectors so no impact to SEC.
If 3.5K is too much a waste in PEI phase, we can enhance the code
further to generate 32 vectors for PEI.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
---
.../CpuExceptionHandlerLib/DxeException.c | 22 -------------------
.../Ia32/ExceptionHandlerAsm.nasm | 4 ++--
.../X64/ExceptionHandlerAsm.nasm | 2 ++
.../X64/Xcode5ExceptionHandlerAsm.nasm | 9 ++++----
4 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index 61f11e98f8..5083c4b8e8 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -95,9 +95,6 @@ InitializeCpuInterruptHandlers (
IA32_DESCRIPTOR IdtDescriptor;

UINTN IdtEntryCount;

EXCEPTION_HANDLER_TEMPLATE_MAP TemplateMap;

- UINTN Index;

- UINTN InterruptEntry;

- UINT8 *InterruptEntryCode;

RESERVED_VECTORS_DATA *ReservedVectors;

EFI_CPU_INTERRUPT_HANDLER *ExternalInterruptHandler;



@@ -138,25 +135,6 @@ InitializeCpuInterruptHandlers (
AsmGetTemplateAddressMap (&TemplateMap);

ASSERT (TemplateMap.ExceptionStubHeaderSize <= HOOKAFTER_STUB_SIZE);



- Status = gBS->AllocatePool (

- EfiBootServicesCode,

- TemplateMap.ExceptionStubHeaderSize * CPU_INTERRUPT_NUM,

- (VOID **)&InterruptEntryCode

- );

- ASSERT (!EFI_ERROR (Status) && InterruptEntryCode != NULL);

-

- InterruptEntry = (UINTN)InterruptEntryCode;

- for (Index = 0; Index < CPU_INTERRUPT_NUM; Index++) {

- CopyMem (

- (VOID *)InterruptEntry,

- (VOID *)TemplateMap.ExceptionStart,

- TemplateMap.ExceptionStubHeaderSize

- );

- AsmVectorNumFixup ((VOID *)InterruptEntry, (UINT8)Index, (VOID
*)TemplateMap.ExceptionStart);

- InterruptEntry += TemplateMap.ExceptionStubHeaderSize;

- }

-

- TemplateMap.ExceptionStart = (UINTN)InterruptEntryCode;

mExceptionHandlerData.IdtEntryCount = CPU_INTERRUPT_NUM;

mExceptionHandlerData.ReservedVectors = ReservedVectors;

mExceptionHandlerData.ExternalInterruptHandler = ExternalInterruptHandler;

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
index 3fe9aed1e8..8ed2b8f455 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
@@ -33,7 +33,7 @@ ALIGN 8
;

AsmIdtVectorBegin:

%assign Vector 0

-%rep 32

+%rep 256

push byte %[Vector];

push eax

mov eax, ASM_PFX(CommonInterruptEntry)

@@ -439,7 +439,7 @@ ASM_PFX(AsmGetTemplateAddressMap):


mov ebx, dword [ebp + 0x8]

mov dword [ebx], AsmIdtVectorBegin

- mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

mov dword [ebx + 0x8], HookAfterStubBegin



popad

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 9a806d1f86..aaf8d622e6 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -31,6 +31,8 @@ SECTION .text


ALIGN 8



+; Generate 32 IDT vectors.

+; 32 IDT vectors are enough because interrupts (32+) are not enabled in SEC and
PEI phase.

AsmIdtVectorBegin:

%assign Vector 0

%rep 32

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
index 9c72fa5815..7c0e3d3b0b 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
@@ -53,9 +53,10 @@ SECTION .text


ALIGN 8



+; Generate 256 IDT vectors.

AsmIdtVectorBegin:

%assign Vector 0

-%rep 32

+%rep 256

push byte %[Vector]

push rax

mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry)

@@ -453,16 +454,16 @@ global ASM_PFX(AsmGetTemplateAddressMap)
ASM_PFX(AsmGetTemplateAddressMap):

lea rax, [AsmIdtVectorBegin]

mov qword [rcx], rax

- mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

lea rax, [HookAfterStubHeaderBegin]

mov qword [rcx + 0x10], rax



; Fix up CommonInterruptEntry address

lea rax, [ASM_PFX(CommonInterruptEntry)]

lea rcx, [AsmIdtVectorBegin]

-%rep 32

+%rep 256

mov qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin)],
rax

- add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

%endrep

; Fix up HookAfterStubHeaderEnd

lea rax, [HookAfterStubHeaderEnd]

--
2.35.1.windows.2



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89916): https://edk2.groups.io/g/devel/message/89916
Mute This Topic: https://groups.io/mt/91231767/1768734
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.wang@...]
-=-=-=-=-=-=
Wang, Jian J
 

I see. With its addressed,

Reviewed-by: Jian J Wang <jian.j.wang@...>

Regards,
Jian

-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Tuesday, May 24, 2022 4:02 PM
To: Wang, Jian J <jian.j.wang@...>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>
Subject: Re: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code
pages for DXE instance

Jian,
Ia32/ExceptionHandlerAsm.nasm is used by 32bit DxeCpuExceptionHandlerLib
instance.

I agree the commit message is not correct. The commit message says
SEC still creates 32 entries but 32bit SEC creates 256 entries.

I will update the commit message to align to code behavior.

Thanks,
Ray

________________________________________
From: Wang, Jian J <jian.j.wang@...>
Sent: Monday, May 23, 2022 0:40
To: devel@edk2.groups.io; Ni, Ray
Cc: Dong, Eric
Subject: RE: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code
pages for DXE instance

Ray,

You changed "%rep 32" to "%rep 256" in Ia32/ExceptionHandlerAsm.nasm.
According to my understanding and your comments, this should be done
only to X64 code, right?

Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Friday, May 20, 2022 10:16 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>
Subject: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code pages
for DXE instance

Today the DXE instance allocates code page and then copies the IDT
vectors to the allocated code page. Then it fixes up the vector number
in the IDT vector.

But if we update the NASM file to generate 256 IDT vectors, there is
no need to do the copy and fix-up.

A side effect is up to 4096 bytes (HOOKAFTER_STUB_SIZE * 256) is
used for 256 IDT vectors. While 32 IDT vectors only require 512 bytes.

But considering the code logic simplification, 3.5K space is not a big
deal. SEC instance still generates 32 IDT vectors so no impact to SEC.
If 3.5K is too much a waste in PEI phase, we can enhance the code
further to generate 32 vectors for PEI.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
---
.../CpuExceptionHandlerLib/DxeException.c | 22 -------------------
.../Ia32/ExceptionHandlerAsm.nasm | 4 ++--
.../X64/ExceptionHandlerAsm.nasm | 2 ++
.../X64/Xcode5ExceptionHandlerAsm.nasm | 9 ++++----
4 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index 61f11e98f8..5083c4b8e8 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -95,9 +95,6 @@ InitializeCpuInterruptHandlers (
IA32_DESCRIPTOR IdtDescriptor;

UINTN IdtEntryCount;

EXCEPTION_HANDLER_TEMPLATE_MAP TemplateMap;

- UINTN Index;

- UINTN InterruptEntry;

- UINT8 *InterruptEntryCode;

RESERVED_VECTORS_DATA *ReservedVectors;

EFI_CPU_INTERRUPT_HANDLER *ExternalInterruptHandler;



@@ -138,25 +135,6 @@ InitializeCpuInterruptHandlers (
AsmGetTemplateAddressMap (&TemplateMap);

ASSERT (TemplateMap.ExceptionStubHeaderSize <= HOOKAFTER_STUB_SIZE);



- Status = gBS->AllocatePool (

- EfiBootServicesCode,

- TemplateMap.ExceptionStubHeaderSize * CPU_INTERRUPT_NUM,

- (VOID **)&InterruptEntryCode

- );

- ASSERT (!EFI_ERROR (Status) && InterruptEntryCode != NULL);

-

- InterruptEntry = (UINTN)InterruptEntryCode;

- for (Index = 0; Index < CPU_INTERRUPT_NUM; Index++) {

- CopyMem (

- (VOID *)InterruptEntry,

- (VOID *)TemplateMap.ExceptionStart,

- TemplateMap.ExceptionStubHeaderSize

- );

- AsmVectorNumFixup ((VOID *)InterruptEntry, (UINT8)Index, (VOID
*)TemplateMap.ExceptionStart);

- InterruptEntry += TemplateMap.ExceptionStubHeaderSize;

- }

-

- TemplateMap.ExceptionStart = (UINTN)InterruptEntryCode;

mExceptionHandlerData.IdtEntryCount = CPU_INTERRUPT_NUM;

mExceptionHandlerData.ReservedVectors = ReservedVectors;

mExceptionHandlerData.ExternalInterruptHandler =
ExternalInterruptHandler;

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
index 3fe9aed1e8..8ed2b8f455 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
m
@@ -33,7 +33,7 @@ ALIGN 8
;

AsmIdtVectorBegin:

%assign Vector 0

-%rep 32

+%rep 256

push byte %[Vector];

push eax

mov eax, ASM_PFX(CommonInterruptEntry)

@@ -439,7 +439,7 @@ ASM_PFX(AsmGetTemplateAddressMap):


mov ebx, dword [ebp + 0x8]

mov dword [ebx], AsmIdtVectorBegin

- mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

mov dword [ebx + 0x8], HookAfterStubBegin



popad

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 9a806d1f86..aaf8d622e6 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -31,6 +31,8 @@ SECTION .text


ALIGN 8



+; Generate 32 IDT vectors.

+; 32 IDT vectors are enough because interrupts (32+) are not enabled in SEC
and
PEI phase.

AsmIdtVectorBegin:

%assign Vector 0

%rep 32

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
index 9c72fa5815..7c0e3d3b0b 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
m.nasm
@@ -53,9 +53,10 @@ SECTION .text


ALIGN 8



+; Generate 256 IDT vectors.

AsmIdtVectorBegin:

%assign Vector 0

-%rep 32

+%rep 256

push byte %[Vector]

push rax

mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry)

@@ -453,16 +454,16 @@ global ASM_PFX(AsmGetTemplateAddressMap)
ASM_PFX(AsmGetTemplateAddressMap):

lea rax, [AsmIdtVectorBegin]

mov qword [rcx], rax

- mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

lea rax, [HookAfterStubHeaderBegin]

mov qword [rcx + 0x10], rax



; Fix up CommonInterruptEntry address

lea rax, [ASM_PFX(CommonInterruptEntry)]

lea rcx, [AsmIdtVectorBegin]

-%rep 32

+%rep 256

mov qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin)],
rax

- add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32

+ add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256

%endrep

; Fix up HookAfterStubHeaderEnd

lea rax, [HookAfterStubHeaderEnd]

--
2.35.1.windows.2



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89916): https://edk2.groups.io/g/devel/message/89916
Mute This Topic: https://groups.io/mt/91231767/1768734
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.wang@...]
-=-=-=-=-=-=
Dong, Eric
 

Acked-by: Eric Dong <eric.dong@...>

 

From: Ni, Ray <ray.ni@...>
Sent: Tuesday, May 24, 2022 4:02 PM
To: Wang, Jian J <jian.j.wang@...>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>
Subject: Re: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code pages for DXE instance

 

Jian,
Ia32/ExceptionHandlerAsm.nasm is used by 32bit DxeCpuExceptionHandlerLib instance.

I agree the commit message is not correct. The commit message says
SEC still creates 32 entries but 32bit SEC creates 256 entries.

I will update the commit message to align to code behavior.

Thanks,
Ray

________________________________________
From: Wang, Jian J <jian.j.wang@...>
Sent: Monday, May 23, 2022 0:40
To: devel@edk2.groups.io; Ni, Ray
Cc: Dong, Eric
Subject: RE: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code pages for DXE instance

Ray,

You changed "%rep 32" to "%rep 256" in Ia32/ExceptionHandlerAsm.nasm.
According to my understanding and your comments, this should be done
only to X64 code, right?

Regards,
Jian

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Friday, May 20, 2022 10:16 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@...>
> Subject: [edk2-devel] [PATCH 1/5] CpuException: Avoid allocating code pages
> for DXE instance
>
> Today the DXE instance allocates code page and then copies the IDT
> vectors to the allocated code page. Then it fixes up the vector number
> in the IDT vector.
>
> But if we update the NASM file to generate 256 IDT vectors, there is
> no need to do the copy and fix-up.
>
> A side effect is up to 4096 bytes (HOOKAFTER_STUB_SIZE * 256) is
> used for 256 IDT vectors. While 32 IDT vectors only require 512 bytes.
>
> But considering the code logic simplification, 3.5K space is not a big
> deal. SEC instance still generates 32 IDT vectors so no impact to SEC.
> If 3.5K is too much a waste in PEI phase, we can enhance the code
> further to generate 32 vectors for PEI.
>
> Signed-off-by: Ray Ni <ray.ni@...>
> Cc: Eric Dong <eric.dong@...>
> ---
>  .../CpuExceptionHandlerLib/DxeException.c     | 22 -------------------
>  .../Ia32/ExceptionHandlerAsm.nasm             |  4 ++--
>  .../X64/ExceptionHandlerAsm.nasm              |  2 ++
>  .../X64/Xcode5ExceptionHandlerAsm.nasm        |  9 ++++----
>  4 files changed, 9 insertions(+), 28 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> index 61f11e98f8..5083c4b8e8 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> @@ -95,9 +95,6 @@ InitializeCpuInterruptHandlers (
>    IA32_DESCRIPTOR                 IdtDescriptor;
>
>    UINTN                           IdtEntryCount;
>
>    EXCEPTION_HANDLER_TEMPLATE_MAP  TemplateMap;
>
> -  UINTN                           Index;
>
> -  UINTN                           InterruptEntry;
>
> -  UINT8                           *InterruptEntryCode;
>
>    RESERVED_VECTORS_DATA           *ReservedVectors;
>
>    EFI_CPU_INTERRUPT_HANDLER       *ExternalInterruptHandler;
>
>
>
> @@ -138,25 +135,6 @@ InitializeCpuInterruptHandlers (
>    AsmGetTemplateAddressMap (&TemplateMap);
>
>    ASSERT (TemplateMap.ExceptionStubHeaderSize <= HOOKAFTER_STUB_SIZE);
>
>
>
> -  Status = gBS->AllocatePool (
>
> -                  EfiBootServicesCode,
>
> -                  TemplateMap.ExceptionStubHeaderSize * CPU_INTERRUPT_NUM,
>
> -                  (VOID **)&InterruptEntryCode
>
> -                  );
>
> -  ASSERT (!EFI_ERROR (Status) && InterruptEntryCode != NULL);
>
> -
>
> -  InterruptEntry = (UINTN)InterruptEntryCode;
>
> -  for (Index = 0; Index < CPU_INTERRUPT_NUM; Index++) {
>
> -    CopyMem (
>
> -      (VOID *)InterruptEntry,
>
> -      (VOID *)TemplateMap.ExceptionStart,
>
> -      TemplateMap.ExceptionStubHeaderSize
>
> -      );
>
> -    AsmVectorNumFixup ((VOID *)InterruptEntry, (UINT8)Index, (VOID
> *)TemplateMap.ExceptionStart);
>
> -    InterruptEntry += TemplateMap.ExceptionStubHeaderSize;
>
> -  }
>
> -
>
> -  TemplateMap.ExceptionStart                     = (UINTN)InterruptEntryCode;
>
>    mExceptionHandlerData.IdtEntryCount            = CPU_INTERRUPT_NUM;
>
>    mExceptionHandlerData.ReservedVectors          = ReservedVectors;
>
>    mExceptionHandlerData.ExternalInterruptHandler = ExternalInterruptHandler;
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
> m
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
> m
> index 3fe9aed1e8..8ed2b8f455 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
> m
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nas
> m
> @@ -33,7 +33,7 @@ ALIGN   8
>  ;
>
>  AsmIdtVectorBegin:
>
>  %assign Vector 0
>
> -%rep  32
>
> +%rep  256
>
>      push    byte %[Vector];
>
>      push    eax
>
>      mov     eax, ASM_PFX(CommonInterruptEntry)
>
> @@ -439,7 +439,7 @@ ASM_PFX(AsmGetTemplateAddressMap):
>
>
>      mov ebx, dword [ebp + 0x8]
>
>      mov dword [ebx],      AsmIdtVectorBegin
>
> -    mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
>
> +    mov dword [ebx + 0x4], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256
>
>      mov dword [ebx + 0x8], HookAfterStubBegin
>
>
>
>      popad
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> index 9a806d1f86..aaf8d622e6 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> @@ -31,6 +31,8 @@ SECTION .text
>
>
>  ALIGN   8
>
>
>
> +; Generate 32 IDT vectors.
>
> +; 32 IDT vectors are enough because interrupts (32+) are not enabled in SEC and
> PEI phase.
>
>  AsmIdtVectorBegin:
>
>  %assign Vector 0
>
>  %rep  32
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
> m.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
> m.nasm
> index 9c72fa5815..7c0e3d3b0b 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
> m.nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
> m.nasm
> @@ -53,9 +53,10 @@ SECTION .text
>
>
>  ALIGN   8
>
>
>
> +; Generate 256 IDT vectors.
>
>  AsmIdtVectorBegin:
>
>  %assign Vector 0
>
> -%rep  32
>
> +%rep  256
>
>      push    byte %[Vector]
>
>      push    rax
>
>      mov     rax, strict qword 0 ;    mov     rax, ASM_PFX(CommonInterruptEntry)
>
> @@ -453,16 +454,16 @@ global ASM_PFX(AsmGetTemplateAddressMap)
>  ASM_PFX(AsmGetTemplateAddressMap):
>
>      lea     rax, [AsmIdtVectorBegin]
>
>      mov     qword [rcx], rax
>
> -    mov     qword [rcx + 0x8],  (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
>
> +    mov     qword [rcx + 0x8],  (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256
>
>      lea     rax, [HookAfterStubHeaderBegin]
>
>      mov     qword [rcx + 0x10], rax
>
>
>
>  ; Fix up CommonInterruptEntry address
>
>      lea    rax, [ASM_PFX(CommonInterruptEntry)]
>
>      lea    rcx, [AsmIdtVectorBegin]
>
> -%rep  32
>
> +%rep  256
>
>      mov    qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin)],
> rax
>
> -    add    rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
>
> +    add    rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 256
>
>  %endrep
>
>  ; Fix up HookAfterStubHeaderEnd
>
>      lea    rax, [HookAfterStubHeaderEnd]
>
> --
> 2.35.1.windows.2
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#89916): https://edk2.groups.io/g/devel/message/89916
> Mute This Topic: https://groups.io/mt/91231767/1768734
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.wang@...]
> -=-=-=-=-=-=
>
1 - 5 of 5