Re: [edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit
Hi Andrew, Please see my inline comment. Thanks, Ted -Original Message- From: Andrew Fish Sent: Thursday, April 7, 2022 8:06 AM To: devel@edk2.groups.io; Ni, Ray Cc: Kuo, Ted ; Chiu, Chasel ; Desimone, Nathaniel L ; Zeng, Star ; S, Ashraf Ali Subject: Re: [edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit > On Apr 6, 2022, at 4:57 PM, Ni, Ray wrote: > >> -; UINT32 >> +; UINTN >> ; EFIAPI >> -; AsmReadEsp ( >> +; AsmReadStackPointer ( >> ; VOID >> ; ); >> ;-- >> -global ASM_PFX(AsmReadEsp) >> -ASM_PFX(AsmReadEsp): >> +global ASM_PFX(AsmReadStackPointer) >> +ASM_PFX(AsmReadStackPointer): >> mov eax, esp >> ret >> > > I guess it's possible that bootloader sets up the stack above 4G. > If that's the case, above code doesn't work. > > Is there an issue with the SecSwitchStack too? [Ted]: I believe we already handled it in IntelFsp2Pkg/FspSecCore/X64/Stack.nasm in the patch [edk2-devel][PATCH v3 4/8] IntelFsp2Pkg: FspSecCore support for X64. Thanks, Andrew Fish > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#88495): https://edk2.groups.io/g/devel/message/88495 Mute This Topic: https://groups.io/mt/90294483/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit
Hi Ray, Please see my inline comment. Thanks, Ted -Original Message- From: Ni, Ray Sent: Thursday, April 7, 2022 7:58 AM To: devel@edk2.groups.io; Kuo, Ted Cc: Chiu, Chasel ; Desimone, Nathaniel L ; Zeng, Star ; S, Ashraf Ali Subject: RE: [edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit > -; UINT32 > +; UINTN > ; EFIAPI > -; AsmReadEsp ( > +; AsmReadStackPointer ( > ; VOID > ; ); > > ;-- > -global ASM_PFX(AsmReadEsp) > -ASM_PFX(AsmReadEsp): > +global ASM_PFX(AsmReadStackPointer) > +ASM_PFX(AsmReadStackPointer): > mov eax, esp > ret > I guess it's possible that bootloader sets up the stack above 4G. If that's the case, above code doesn't work. [Ted]: The above code is for IA32 only. We have a separate ReadRsp.nasm for X64. You can find it in the patch [edk2-devel][PATCH v3 4/8] IntelFsp2Pkg: FspSecCore support for X64. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#88494): https://edk2.groups.io/g/devel/message/88494 Mute This Topic: https://groups.io/mt/90294483/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit
> >> -; UINT32 > >> +; UINTN > >> ; EFIAPI > >> -; AsmReadEsp ( > >> +; AsmReadStackPointer ( > >> ; VOID > >> ; ); > >> ;-- > >> -global ASM_PFX(AsmReadEsp) > >> -ASM_PFX(AsmReadEsp): > >> +global ASM_PFX(AsmReadStackPointer) > >> +ASM_PFX(AsmReadStackPointer): > >> mov eax, esp > >> ret > >> > > > > I guess it's possible that bootloader sets up the stack above 4G. > > If that's the case, above code doesn't work. > > > > > > Is there an issue with the SecSwitchStack too? I didn't read the whole patch😊. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#88485): https://edk2.groups.io/g/devel/message/88485 Mute This Topic: https://groups.io/mt/90294483/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit
> On Apr 6, 2022, at 4:57 PM, Ni, Ray wrote: > >> -; UINT32 >> +; UINTN >> ; EFIAPI >> -; AsmReadEsp ( >> +; AsmReadStackPointer ( >> ; VOID >> ; ); >> ;-- >> -global ASM_PFX(AsmReadEsp) >> -ASM_PFX(AsmReadEsp): >> +global ASM_PFX(AsmReadStackPointer) >> +ASM_PFX(AsmReadStackPointer): >> mov eax, esp >> ret >> > > I guess it's possible that bootloader sets up the stack above 4G. > If that's the case, above code doesn't work. > > Is there an issue with the SecSwitchStack too? Thanks, Andrew Fish > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#88483): https://edk2.groups.io/g/devel/message/88483 Mute This Topic: https://groups.io/mt/90294483/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit
> -; UINT32 > +; UINTN > ; EFIAPI > -; AsmReadEsp ( > +; AsmReadStackPointer ( > ; VOID > ; ); > > ;-- > -global ASM_PFX(AsmReadEsp) > -ASM_PFX(AsmReadEsp): > +global ASM_PFX(AsmReadStackPointer) > +ASM_PFX(AsmReadStackPointer): > mov eax, esp > ret > I guess it's possible that bootloader sets up the stack above 4G. If that's the case, above code doesn't work. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#88482): https://edk2.groups.io/g/devel/message/88482 Mute This Topic: https://groups.io/mt/90294483/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel][PATCH v3 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3893 1.Added EFIAPI to FspNotifyPhasePeimEntryPoint. 2.Changed AsmReadEsp to AsmReadStackPointer. 3.Changed the type of the return value of AsmReadStackPointer from UINT32 to UINTN. 4.Changed the type of TemporaryMemoryBase, PermenentMemoryBase and BootLoaderStack from UINT32 to UINTN. 5.Some type casting to pointers are UINT32. Changed them to UINTN to accommodate both IA32 and X64. 6.Corrected some typos. Cc: Chasel Chiu Cc: Nate DeSimone Cc: Star Zeng Cc: Ashraf Ali S Signed-off-by: Ted Kuo --- IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c | 1 + IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm | 8 IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm| 10 +- IntelFsp2Pkg/FspSecCore/SecFsp.c | 8 IntelFsp2Pkg/FspSecCore/SecFsp.h | 2 +- IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 8 IntelFsp2Pkg/FspSecCore/SecMain.c | 8 IntelFsp2Pkg/FspSecCore/SecMain.h | 10 +- IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/Ia32/Flat32.nasm | 2 +- 9 files changed, 29 insertions(+), 28 deletions(-) diff --git a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c index 88f5540fef..66d39cc70c 100644 --- a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c +++ b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c @@ -112,6 +112,7 @@ WaitForNotify ( @retval EFI_OUT_OF_RESOURCES Insufficient resources to create database **/ EFI_STATUS +EFIAPI FspNotifyPhasePeimEntryPoint ( IN EFI_PEI_FILE_HANDLE FileHandle, IN CONST EFI_PEI_SERVICES **PeiServices diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm index 8046b43745..d40dad5a52 100644 --- a/IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm +++ b/IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm @@ -9,14 +9,14 @@ SECTION .text ;-- -; UINT32 +; UINTN ; EFIAPI -; AsmReadEsp ( +; AsmReadStackPointer ( ; VOID ; ); ;-- -global ASM_PFX(AsmReadEsp) -ASM_PFX(AsmReadEsp): +global ASM_PFX(AsmReadStackPointer) +ASM_PFX(AsmReadStackPointer): mov eax, esp ret diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm index 5a7e27c240..ce20639890 100644 --- a/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm +++ b/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm @@ -9,20 +9,20 @@ ; ;-- -SECTION .text +SECTION .text ;-- ; VOID ; EFIAPI ; SecSwitchStack ( ; UINT32 TemporaryMemoryBase, -; UINT32 PermenentMemoryBase +; UINT32 PermanentMemoryBase ; ); ;-- global ASM_PFX(SecSwitchStack) ASM_PFX(SecSwitchStack): ; -; Save three register: eax, ebx, ecx +; Save four register: eax, ebx, ecx, edx ; push eax push ebx @@ -55,7 +55,7 @@ ASM_PFX(SecSwitchStack): mov dword [eax + 12], edx mov edx, dword [esp + 16]; Update this function's return address into permanent memory mov dword [eax + 16], edx -mov esp, eax ; From now, esp is pointed to permanent memory +mov esp, eax ; From now, esp is pointed to permanent memory ; ; Fixup the ebp point to permanent memory @@ -63,7 +63,7 @@ ASM_PFX(SecSwitchStack): mov eax, ebp sub eax, ebx add eax, ecx -mov ebp, eax; From now, ebp is pointed to permanent memory +mov ebp, eax ; From now, ebp is pointed to permanent memory pop edx pop ecx diff --git a/IntelFsp2Pkg/FspSecCore/SecFsp.c b/IntelFsp2Pkg/FspSecCore/SecFsp.c index 68e588dd41..85fbc7664c 100644 --- a/IntelFsp2Pkg/FspSecCore/SecFsp.c +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.c @@ -26,7 +26,7 @@ FspGetExceptionHandler ( IA32_IDT_GATE_DESCRIPTOR *IdtGateDescriptor; FSP_INFO_HEADER *FspInfoHeader; - FspInfoHeader = (FSP_INFO_HEADER *)AsmGetFspInfoHeader (); + FspInfoHeader = (FSP_INFO_HEADER *)(UINTN)AsmGetFspInfoHeader (); ExceptionHandler = IdtEntryTemplate; IdtGateDescriptor = (IA32_IDT_GATE_DESCRIPTOR *)&ExceptionHandler; Entry = (IdtGateDescriptor->Bits.OffsetHigh << 16) | IdtGateDescriptor->Bits.OffsetLow; @@ -115,7 +115,7 @@ SecGetPlatformData ( VOID FspGlobalDataInit ( IN OUT FSP_GLOBAL_DATA *PeiFspData, - IN UINT
