I see. With its addressed,
Reviewed-by: Jian J Wang <jian.j.w...@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Ni, Ray <ray...@intel.com>
> Sent: Tuesday, May 24, 2022 4:02 PM
> To: Wang, Jian J <jian.j.w...@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>
> 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.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 (#89986): https://edk2.groups.io/g/devel/message/89986
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]
-=-=-=-=-=-=-=-=-=-=-=-
