Acked-by: Eric Dong <eric.d...@intel.com> 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<mailto:jian.j.w...@intel.com>> Sent: Monday, May 23, 2022 0:40 To: devel@edk2.groups.io<mailto: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<mailto:devel@edk2.groups.io> > <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray > Sent: Friday, May 20, 2022 10:16 PM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Dong, Eric <eric.d...@intel.com<mailto: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<mailto:ray...@intel.com>> > Cc: Eric Dong <eric.d...@intel.com<mailto: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<mailto: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 (#90236): https://edk2.groups.io/g/devel/message/90236 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] -=-=-=-=-=-=-=-=-=-=-=-
