[edk2-devel][PATCH v2 1/8] IntelFsp2Pkg: X64 compatible changes to support PEI in 64bit
Chiu, Chasel
chasel.chiu at intel.com
Wed Apr 6 14:54:02 UTC 2022
Hi Ted,
Please see my comments inline below.
Thanks,
Chasel
> -----Original Message-----
> From: Kuo, Ted <ted.kuo at intel.com>
> Sent: Monday, April 4, 2022 2:23 PM
> To: devel at edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Zeng, Star <star.zeng at intel.com>; S,
> Ashraf Ali <ashraf.ali.s at 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.chiu at intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> Cc: Star Zeng <star.zeng at intel.com>
> Cc: Ashraf Ali S <ashraf.ali.s at intel.com>
> Signed-off-by: Ted Kuo <ted.kuo at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list