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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Signed-off-by: Ted Kuo <ted.kuo@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..a746e8b64e 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 == 0xFFFFFFFF)) {
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) {
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 == 0xFFFFFFFF)) {
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 == 0xFFFFFFFF)) {
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 (#88467): https://edk2.groups.io/g/devel/message/88467
Mute This Topic: https://groups.io/mt/90294483/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-> -; 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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> On Apr 6, 2022, at 4:57 PM, Ni, Ray <ray.ni@intel.com> 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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> >> -; 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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Andrew, Please see my inline comment. Thanks, Ted -----Original Message----- From: Andrew Fish <afish@apple.com> Sent: Thursday, April 7, 2022 8:06 AM To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com> Cc: Kuo, Ted <ted.kuo@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com> 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 <ray.ni@intel.com> 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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Ray, Please see my inline comment. Thanks, Ted -----Original Message----- From: Ni, Ray <ray.ni@intel.com> Sent: Thursday, April 7, 2022 7:58 AM To: devel@edk2.groups.io; Kuo, Ted <ted.kuo@intel.com> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com> 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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.