Hi Ted,
Please see my comments inline below. Thanks, Chasel > -----Original Message----- > From: Kuo, Ted <ted....@intel.com> > Sent: Monday, April 4, 2022 2:23 PM > To: devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Zeng, Star <star.z...@intel.com>; S, > Ashraf Ali <ashraf.al...@intel.com> > Subject: [edk2-devel][PATCH v2 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 <chasel.c...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Ashraf Ali S <ashraf.al...@intel.com> > Signed-off-by: Ted Kuo <ted....@intel.com> > --- > 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 UINT32 BootLoaderStack, > + IN UINTN BootLoaderStack, > IN UINT8 ApiIdx > ) > { > @@ -141,7 +141,7 @@ FspGlobalDataInit ( > // Get FSP Header offset > // It may have multiple FVs, so look into the last one for FSP header > // > - PeiFspData->FspInfoHeader = (FSP_INFO_HEADER > *)AsmGetFspInfoHeader (); > + PeiFspData->FspInfoHeader = (FSP_INFO_HEADER > + *)(UINTN)AsmGetFspInfoHeader (); > SecGetPlatformData (PeiFspData); > > // > @@ -154,7 +154,7 @@ FspGlobalDataInit ( > // > FspmUpdDataPtr = (VOID *)GetFspApiParameter (); > if (FspmUpdDataPtr == NULL) { > - FspmUpdDataPtr = (VOID *)(PeiFspData->FspInfoHeader->ImageBase + > PeiFspData->FspInfoHeader->CfgRegionOffset); > + FspmUpdDataPtr = (VOID > + *)(UINTN)(PeiFspData->FspInfoHeader->ImageBase + > + PeiFspData->FspInfoHeader->CfgRegionOffset); > } > > SetFspUpdDataPointer (FspmUpdDataPtr); diff --git > a/IntelFsp2Pkg/FspSecCore/SecFsp.h b/IntelFsp2Pkg/FspSecCore/SecFsp.h > index 7c9be85fe0..7fb31c3f87 100644 > --- a/IntelFsp2Pkg/FspSecCore/SecFsp.h > +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.h > @@ -48,7 +48,7 @@ FspGetExceptionHandler ( VOID FspGlobalDataInit ( > IN OUT FSP_GLOBAL_DATA *PeiFspData, > - IN UINT32 BootLoaderStack, > + IN UINTN BootLoaderStack, > IN UINT8 ApiIdx > ); > > diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c > b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c > index 7d6ef11fe7..c57247bfaf 100644 > --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c > +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c > @@ -31,7 +31,7 @@ FspApiCallingCheck ( > // > // NotifyPhase check > // > - if ((FspData == NULL) || ((UINT32)FspData == 0xFFFFFFFF)) { > + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS)) { > Status = EFI_UNSUPPORTED; > } else { > if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) { @@ -42,7 > +42,7 @@ FspApiCallingCheck ( > // > // FspMemoryInit check > // > - if ((UINT32)FspData != 0xFFFFFFFF) { > + if ((UINTN)FspData != 0xFFFFFFFF) { Compare with MAX_ADDRESS instead of 0xFFFFFFFF > Status = EFI_UNSUPPORTED; > } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam))) { > Status = EFI_INVALID_PARAMETER; > @@ -51,7 +51,7 @@ FspApiCallingCheck ( > // > // TempRamExit check > // > - if ((FspData == NULL) || ((UINT32)FspData == 0xFFFFFFFF)) { > + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS)) { > Status = EFI_UNSUPPORTED; > } else { > if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) { @@ -62,7 > +62,7 @@ FspApiCallingCheck ( > // > // FspSiliconInit check > // > - if ((FspData == NULL) || ((UINT32)FspData == 0xFFFFFFFF)) { > + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS)) { > Status = EFI_UNSUPPORTED; > } else { > if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) { diff --git > a/IntelFsp2Pkg/FspSecCore/SecMain.c > b/IntelFsp2Pkg/FspSecCore/SecMain.c > index d376fb8361..9e9332ffcd 100644 > --- a/IntelFsp2Pkg/FspSecCore/SecMain.c > +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c > @@ -54,7 +54,7 @@ SecStartup ( > IN UINT32 TempRamBase, > IN VOID *BootFirmwareVolume, > IN PEI_CORE_ENTRY PeiCore, > - IN UINT32 BootLoaderStack, > + IN UINTN BootLoaderStack, > IN UINT32 ApiIdx > ) > { > @@ -233,7 +233,7 @@ SecTemporaryRamSupport ( > GetFspGlobalDataPointer ()->OnSeparateStack = 1; > > if (PcdGet8 (PcdFspHeapSizePercentage) == 0) { > - CurrentStack = AsmReadEsp (); > + CurrentStack = AsmReadStackPointer (); > FspStackBase = (UINTN)GetFspEntryStack (); > > StackSize = FspStackBase - CurrentStack; @@ -292,8 +292,8 @@ > SecTemporaryRamSupport ( > // permanent memory. > // > SecSwitchStack ( > - (UINT32)(UINTN)OldStack, > - (UINT32)(UINTN)NewStack > + (UINTN)OldStack, > + (UINTN)NewStack > ); > > return EFI_SUCCESS; > diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.h > b/IntelFsp2Pkg/FspSecCore/SecMain.h > index 7794255af1..3c60b15f01 100644 > --- a/IntelFsp2Pkg/FspSecCore/SecMain.h > +++ b/IntelFsp2Pkg/FspSecCore/SecMain.h > @@ -51,8 +51,8 @@ typedef struct _SEC_IDT_TABLE { VOID EFIAPI > SecSwitchStack ( > - IN UINT32 TemporaryMemoryBase, > - IN UINT32 PermenentMemoryBase > + IN UINTN TemporaryMemoryBase, > + IN UINTN PermenentMemoryBase > ); > > /** > @@ -104,7 +104,7 @@ SecStartup ( > IN UINT32 TempRamBase, > IN VOID *BootFirmwareVolume, > IN PEI_CORE_ENTRY PeiCore, > - IN UINT32 BootLoaderStack, > + IN UINTN BootLoaderStack, > IN UINT32 ApiIdx > ); > > @@ -127,9 +127,9 @@ ProcessLibraryConstructorList ( > @return value of esp. > > **/ > -UINT32 > +UINTN > EFIAPI > -AsmReadEsp ( > +AsmReadStackPointer ( > VOID > ); > > diff --git > a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/Ia32/Flat32.nasm > b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/Ia32/Flat32.nasm > index aef7f96d1d..7be570c4e5 100644 > --- a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/Ia32/Flat32.nasm > +++ b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/Ia32/Flat32.nasm > @@ -16,7 +16,7 @@ SECTION .text > > %macro RET_ESI 0 > > - movd esi, mm7 ; restore ESP from MM7 > + movd esi, mm7 ; restore EIP from MM7 > jmp esi > > %endmacro > -- > 2.16.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#88458): https://edk2.groups.io/g/devel/message/88458 Mute This Topic: https://groups.io/mt/90235994/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-