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.w...@intel.com>
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.d...@intel.com>
> 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...@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> ---
> .../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+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.w...@intel.com]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89979): https://edk2.groups.io/g/devel/message/89979
Mute This Topic: https://groups.io/mt/91231767/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
