IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 13 +++++++++++++ IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++ IntelFsp2Pkg/Include/FspEas/FspApi.h | 57 ++++++++++++++++++++++++++++++++++++++------------------- IntelFsp2Pkg/Include/FspGlobalData.h | 53 ++++++++++++++++++++++++++++------------------------- IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 22 +++++++++++++++------- IntelFsp2Pkg/IntelFsp2Pkg.dsc | 1 + IntelFsp2Pkg/Tools/GenCfgOpt.py | 26 ++++++++++++++++---------- IntelFsp2Pkg/Tools/SplitFspBin.py | 6 +++--- 10 files changed, 256 insertions(+), 64 deletions(-)
From: Hongbin1 Zhang <hongbin1.zhang@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
Add FSP-I API entry point for SMM support.
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
Signed-off-by: Hongbin1 Zhang <hongbin1.zhang@intel.com>
---
IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 13 +++++++++++++
IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/Include/FspEas/FspApi.h | 57 ++++++++++++++++++++++++++++++++++++++-------------------
IntelFsp2Pkg/Include/FspGlobalData.h | 53 ++++++++++++++++++++++++++++-------------------------
IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 22 +++++++++++++++-------
IntelFsp2Pkg/IntelFsp2Pkg.dsc | 1 +
IntelFsp2Pkg/Tools/GenCfgOpt.py | 26 ++++++++++++++++----------
IntelFsp2Pkg/Tools/SplitFspBin.py | 6 +++---
10 files changed, 256 insertions(+), 64 deletions(-)
diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
index e22a88cc84..35d223a404 100644
--- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
+++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
@@ -71,6 +71,19 @@ FspApiCallingCheck (
Status = EFI_INVALID_PARAMETER;
}
}
+ } else if (ApiIdx == FspSmmInitApiIndex) {
+ //
+ // FspSmmInitApiIndex check
+ //
+ if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS) || ((UINTN)FspData == MAX_UINT32)) {
+ Status = EFI_UNSUPPORTED;
+ } else {
+ if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
+ Status = EFI_UNSUPPORTED;
+ } else if (EFI_ERROR (FspUpdSignatureCheck (FspSmmInitApiIndex, ApiParam))) {
+ Status = EFI_INVALID_PARAMETER;
+ }
+ }
} else {
Status = EFI_UNSUPPORTED;
}
diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
new file mode 100644
index 0000000000..d31576c00b
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
@@ -0,0 +1,54 @@
+## @file
+# Sec Core for FSP
+#
+# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = FspSecCoreI
+ FILE_GUID = 558782b5-782d-415e-ab9e-0ceb79dc3425
+ MODULE_TYPE = SEC
+ VERSION_STRING = 1.0
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources]
+ SecFspApiChk.c
+ SecFsp.h
+
+[Sources.X64]
+ X64/FspApiEntryI.nasm
+ X64/FspApiEntryCommon.nasm
+ X64/FspHelper.nasm
+
+[Sources.IA32]
+ Ia32/FspApiEntryI.nasm
+ Ia32/FspApiEntryCommon.nasm
+ Ia32/FspHelper.nasm
+
+[Binaries.Ia32]
+ RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
+
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelFsp2Pkg/IntelFsp2Pkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ BaseLib
+ PciCf8Lib
+ SerialPortLib
+ FspSwitchStackLib
+ FspCommonLib
+ FspSecPlatformLib
+
+
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
new file mode 100644
index 0000000000..e9365d6832
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
@@ -0,0 +1,44 @@
+;; @file
+; Provide FSP API entry points.
+;
+; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;;
+
+ SECTION .text
+
+;
+; Following functions will be provided in C
+;
+extern ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspApiCommonContinue API
+;
+; This is the FSP API common entry point to resume the FSP execution
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspApiCommonContinue)
+ASM_PFX(FspApiCommonContinue):
+ jmp $
+
+;----------------------------------------------------------------------------
+; FspSmmInit API
+;
+; This FSP API will notify the FSP about the different phases in the boot
+; process
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspSmmInitApi)
+ASM_PFX(FspSmmInitApi):
+ mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; Module Entrypoint API
+;----------------------------------------------------------------------------
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+ jmp $
+ ; Add reference to APIs so that it will not be optimized by compiler
+ jmp ASM_PFX(FspSmmInitApi)
diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
new file mode 100644
index 0000000000..e9365d6832
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
@@ -0,0 +1,44 @@
+;; @file
+; Provide FSP API entry points.
+;
+; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;;
+
+ SECTION .text
+
+;
+; Following functions will be provided in C
+;
+extern ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspApiCommonContinue API
+;
+; This is the FSP API common entry point to resume the FSP execution
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspApiCommonContinue)
+ASM_PFX(FspApiCommonContinue):
+ jmp $
+
+;----------------------------------------------------------------------------
+; FspSmmInit API
+;
+; This FSP API will notify the FSP about the different phases in the boot
+; process
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspSmmInitApi)
+ASM_PFX(FspSmmInitApi):
+ mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; Module Entrypoint API
+;----------------------------------------------------------------------------
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+ jmp $
+ ; Add reference to APIs so that it will not be optimized by compiler
+ jmp ASM_PFX(FspSmmInitApi)
diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h b/IntelFsp2Pkg/Include/FspEas/FspApi.h
index b36bc2b9ae..1d6c2fb63d 100644
--- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
+++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
@@ -135,18 +135,18 @@ typedef struct {
///
/// Revision of the structure is 2 for this version of the specification.
///
- UINT8 Revision;
- UINT8 Reserved[3];
+ UINT8 Revision;
+ UINT8 Reserved[3];
///
/// Length of the structure in bytes. The current value for this field is 32.
///
- UINT32 Length;
+ UINT32 Length;
///
/// FspDebugHandler Optional debug handler for the bootloader to receive debug messages
/// occurring during FSP execution.
///
- EFI_PHYSICAL_ADDRESS FspDebugHandler;
- UINT8 Reserved1[16];
+ EFI_PHYSICAL_ADDRESS FspDebugHandler;
+ UINT8 Reserved1[16];
} FSPT_ARCH2_UPD;
///
@@ -197,37 +197,37 @@ typedef struct {
///
/// Revision of the structure is 3 for this version of the specification.
///
- UINT8 Revision;
- UINT8 Reserved[3];
+ UINT8 Revision;
+ UINT8 Reserved[3];
///
/// Length of the structure in bytes. The current value for this field is 64.
///
- UINT32 Length;
+ UINT32 Length;
///
/// Pointer to the temporary stack base address to be
/// consumed inside FspMemoryInit() API.
///
- EFI_PHYSICAL_ADDRESS StackBase;
+ EFI_PHYSICAL_ADDRESS StackBase;
///
/// Temporary stack size to be consumed inside
/// FspMemoryInit() API.
///
- UINT64 StackSize;
+ UINT64 StackSize;
///
/// Size of memory to be reserved by FSP below "top
/// of low usable memory" for bootloader usage.
///
- UINT32 BootLoaderTolumSize;
+ UINT32 BootLoaderTolumSize;
///
/// Current boot mode.
///
- UINT32 BootMode;
+ UINT32 BootMode;
///
/// Optional event handler for the bootloader to be informed of events occurring during FSP execution.
/// This value is only valid if Revision is >= 2.
///
- EFI_PHYSICAL_ADDRESS FspEventHandler;
- UINT8 Reserved1[24];
+ EFI_PHYSICAL_ADDRESS FspEventHandler;
+ UINT8 Reserved1[24];
} FSPM_ARCH2_UPD;
///
@@ -266,18 +266,18 @@ typedef struct {
///
/// Revision of the structure is 2 for this version of the specification.
///
- UINT8 Revision;
- UINT8 Reserved[3];
+ UINT8 Revision;
+ UINT8 Reserved[3];
///
/// Length of the structure in bytes. The current value for this field is 32.
///
- UINT32 Length;
+ UINT32 Length;
///
/// FspEventHandler Optional event handler for the bootloader to be informed of events
/// occurring during FSP execution.
///
- EFI_PHYSICAL_ADDRESS FspEventHandler;
- UINT8 Reserved1[16];
+ EFI_PHYSICAL_ADDRESS FspEventHandler;
+ UINT8 Reserved1[16];
} FSPS_ARCH2_UPD;
///
@@ -609,4 +609,23 @@ EFI_STATUS
IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
);
+/**
+ This FSP API initializes SMM and provide any OS runtime silicon services,
+ including Reliability, Availability, and Serviceability (RAS) features implemented by the CPU.
+
+ @param[in] FspiUpdDataPtr Pointer to the FSPI_UPD data structure.
+ If NULL, FSP will use the default parameters.
+
+ @retval EFI_SUCCESS FSP execution environment was initialized successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
+ @retval EFI_DEVICE_ERROR FSP initialization failed.
+ @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status codes will not be returned during S3.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_SMM_INIT)(
+ IN VOID *FspiUpdDataPtr
+ );
+
#endif
diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h b/IntelFsp2Pkg/Include/FspGlobalData.h
index 445540abfa..697b20ed4c 100644
--- a/IntelFsp2Pkg/Include/FspGlobalData.h
+++ b/IntelFsp2Pkg/Include/FspGlobalData.h
@@ -10,9 +10,9 @@
#include <FspEas.h>
-#define FSP_IN_API_MODE 0
-#define FSP_IN_DISPATCH_MODE 1
-#define FSP_GLOBAL_DATA_VERSION 1
+#define FSP_IN_API_MODE 0
+#define FSP_IN_DISPATCH_MODE 1
+#define FSP_GLOBAL_DATA_VERSION 1
#pragma pack(1)
@@ -24,16 +24,17 @@ typedef enum {
TempRamExitApiIndex,
FspSiliconInitApiIndex,
FspMultiPhaseSiInitApiIndex,
+ FspSmmInitApiIndex,
FspApiIndexMax
} FSP_API_INDEX;
typedef struct {
- VOID *DataPtr;
- UINTN MicrocodeRegionBase;
- UINTN MicrocodeRegionSize;
- UINTN CodeRegionBase;
- UINTN CodeRegionSize;
- UINTN Reserved;
+ VOID *DataPtr;
+ UINTN MicrocodeRegionBase;
+ UINTN MicrocodeRegionSize;
+ UINTN CodeRegionBase;
+ UINTN CodeRegionSize;
+ UINTN Reserved;
} FSP_PLAT_DATA;
#define FSP_GLOBAL_DATA_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'D')
@@ -41,28 +42,28 @@ typedef struct {
#define FSP_PERFORMANCE_DATA_TIMER_MASK 0xFFFFFFFFFFFFFF
typedef struct {
- UINT32 Signature;
- UINT8 Version;
- UINT8 Reserved1[3];
+ UINT32 Signature;
+ UINT8 Version;
+ UINT8 Reserved1[3];
///
/// Offset 0x08
///
- UINTN CoreStack;
- UINTN Reserved2;
+ UINTN CoreStack;
+ UINTN Reserved2;
///
/// IA32: Offset 0x10; X64: Offset 0x18
///
- UINT32 StatusCode;
- UINT8 ApiIdx;
+ UINT32 StatusCode;
+ UINT8 ApiIdx;
///
/// 0: FSP in API mode; 1: FSP in DISPATCH mode
///
- UINT8 FspMode;
- UINT8 OnSeparateStack;
- UINT8 Reserved3;
- UINT32 NumberOfPhases;
- UINT32 PhasesExecuted;
- UINT32 Reserved4[8];
+ UINT8 FspMode;
+ UINT8 OnSeparateStack;
+ UINT8 Reserved3;
+ UINT32 NumberOfPhases;
+ UINT32 PhasesExecuted;
+ UINT32 Reserved4[8];
///
/// IA32: Offset 0x40; X64: Offset 0x48
/// Start of UINTN and pointer section
@@ -75,21 +76,23 @@ typedef struct {
VOID *TempRamInitUpdPtr;
VOID *MemoryInitUpdPtr;
VOID *SiliconInitUpdPtr;
+ VOID *SmmInitUpdPtr;
///
- /// IA32: Offset 0x64; X64: Offset 0x90
+ /// IA32: Offset 0x68; X64: Offset 0x98
/// To store function parameters pointer
/// so it can be retrieved after stack switched.
///
VOID *FunctionParameterPtr;
FSP_INFO_HEADER *FspInfoHeader;
VOID *UpdDataPtr;
+ UINTN Reserved5;
///
/// End of UINTN and pointer section
///
- UINT8 Reserved5[16];
+ UINT8 Reserved6[16];
UINT32 PerfSig;
UINT16 PerfLen;
- UINT16 Reserved6;
+ UINT16 Reserved7;
UINT32 PerfIdx;
UINT64 PerfData[32];
} FSP_GLOBAL_DATA;
diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
index c660defac3..c7fb63168f 100644
--- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
+++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
@@ -26,13 +26,13 @@
#define FSP_INFO_HEADER_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'H')
-#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
-#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
-#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
-#define FSP_IA32 0
-#define FSP_X64 1
+#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
+#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
+#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
+#define FSP_IA32 0
+#define FSP_X64 1
-#pragma pack(1)
+ #pragma pack(1)
///
/// FSP Information Header as described in FSP v2.0 Spec section 5.1.1.
@@ -159,6 +159,14 @@ typedef struct {
/// Byte 0x4E: Reserved4.
///
UINT16 Reserved4;
+ ///
+ /// Byte 0x50: Offset for the API for the Multi-Phase memory initialization.
+ ///
+ UINT32 FspMultiPhaseMemInitEntryOffset;
+ ///
+ /// Byte 0x54: Offset for the API to initialize SMM.
+ ///
+ UINT32 FspSmmInitEntryOffset;
} FSP_INFO_HEADER;
///
@@ -240,7 +248,7 @@ typedef struct {
// UINT32 PatchData[];
} FSP_PATCH_TABLE;
-#pragma pack()
+ #pragma pack()
extern EFI_GUID gFspHeaderFileGuid;
diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
index 7cf7e88245..b2d7867880 100644
--- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
@@ -68,6 +68,7 @@
IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
+ IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
index c4fb1f1bb2..128b896592 100644
--- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
+++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
@@ -953,8 +953,8 @@ EndList
return NoFileChange
def CreateSplitUpdTxt (self, UpdTxtFile):
- GuidList = ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID']
- SignatureList = ['0x545F', '0x4D5F','0x535F'] # _T, _M, and _S signature for FSPT, FSPM, FSPS
+ GuidList = ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID','FSP_I_UPD_TOOL_GUID']
+ SignatureList = ['0x545F', '0x4D5F','0x535F','0x495F'] # _T, _M, _S and _I signature for FSPT, FSPM, FSPS, FSPI
for Index in range(len(GuidList)):
UpdTxtFile = ''
FvDir = self._FvDir
@@ -1288,19 +1288,21 @@ EndList
Chars.append(chr(Value & 0xFF))
Value = Value >> 8
SignatureStr = ''.join(Chars)
- # Signature will be _T / _M / _S for FSPT / FSPM / FSPS accordingly
+ # Signature will be _T / _M / _S / _I for FSPT / FSPM / FSPS /FSPI accordingly
if '_T' in SignatureStr[6:6+2]:
TxtBody.append("#define FSPT_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
elif '_M' in SignatureStr[6:6+2]:
TxtBody.append("#define FSPM_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
elif '_S' in SignatureStr[6:6+2]:
TxtBody.append("#define FSPS_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
+ elif '_I' in SignatureStr[6:6+2]:
+ TxtBody.append("#define FSPI_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
TxtBody.append("\n")
for Region in ['UPD']:
UpdOffsetTable = []
- UpdSignature = ['0x545F', '0x4D5F', '0x535F'] #['_T', '_M', '_S'] signature for FSPT, FSPM, FSPS
- UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD']
+ UpdSignature = ['0x545F', '0x4D5F', '0x535F', '0x495F'] #['_T', '_M', '_S', '_I'] signature for FSPT, FSPM, FSPS, FSPI
+ UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD', 'FSPI_UPD']
for Item in self._CfgItemList:
if Item["cname"] == 'Signature' and Item["value"][0:6] in UpdSignature:
Item["offset"] = 0 # re-initialize offset to 0 when new UPD structure starting
@@ -1393,11 +1395,12 @@ EndList
HeaderTFileName = 'FsptUpd.h'
HeaderMFileName = 'FspmUpd.h'
HeaderSFileName = 'FspsUpd.h'
+ HeaderIFileName = 'FspiUpd.h'
- UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS'] # FSPX_UPD_REGION
- UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S'] # FSP_X_CONFIG, FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
- UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE']
- ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD', 'FSPS_ARCH_UPD']
+ UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS', 'FSPI'] # FSPX_UPD_REGION
+ UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S', 'FSP_I'] # FSP_X_CONFIG, FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
+ UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE', 'FSPI_UPD_SIGNATURE']
+ ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD', 'FSPS_ARCH_UPD', 'FSPI_ARCH_UPD']
ExcludedSpecificUpd1 = ['FSPT_ARCH2_UPD', 'FSPM_ARCH2_UPD', 'FSPS_ARCH2_UPD']
IncLines = []
@@ -1420,6 +1423,9 @@ EndList
elif UpdRegionCheck[item] == 'FSPS':
HeaderFd = open(os.path.join(FvDir, HeaderSFileName), "w")
FileBase = os.path.basename(os.path.join(FvDir, HeaderSFileName))
+ elif UpdRegionCheck[item] == 'FSPI':
+ HeaderFd = open(os.path.join(FvDir, HeaderIFileName), "w")
+ FileBase = os.path.basename(os.path.join(FvDir, HeaderIFileName))
FileName = FileBase.replace(".", "_").upper()
HeaderFd.write("%s\n" % (__copyright_h__ % date.today().year))
HeaderFd.write("#ifndef __%s__\n" % FileName)
@@ -1696,7 +1702,7 @@ EndList
def Usage():
- print ("GenCfgOpt Version 0.57")
+ print ("GenCfgOpt Version 0.58")
print ("Usage:")
print (" GenCfgOpt UPDTXT PlatformDscFile BuildFvDir [-D Macros]")
print (" GenCfgOpt HEADER PlatformDscFile BuildFvDir InputHFile [-D Macros]")
diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py b/IntelFsp2Pkg/Tools/SplitFspBin.py
index f9151b5afd..317d9c1fa0 100644
--- a/IntelFsp2Pkg/Tools/SplitFspBin.py
+++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
@@ -1,6 +1,6 @@
## @ SplitFspBin.py
#
-# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -492,7 +492,7 @@ class FspImage:
self.FihOffset = fihoff
self.Offset = offset
self.FvIdxList = []
- self.Type = "XTMSXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) & 0x0F]
+ self.Type = "XTMSIXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) & 0x0F]
self.PatchList = patch
self.PatchList.append(fihoff + 0x1C)
@@ -869,7 +869,7 @@ def main ():
parser_rebase = subparsers.add_parser('rebase', help='rebase a FSP into a new base address')
parser_rebase.set_defaults(which='rebase')
parser_rebase.add_argument('-f', '--fspbin' , dest='FspBinary', type=str, help='FSP binary file path', required = True)
- parser_rebase.add_argument('-c', '--fspcomp', choices=['t','m','s','o'], nargs='+', dest='FspComponent', type=str, help='FSP component to rebase', default = "['t']", required = True)
+ parser_rebase.add_argument('-c', '--fspcomp', choices=['t','m','s','o','i'], nargs='+', dest='FspComponent', type=str, help='FSP component to rebase', default = "['t']", required = True)
parser_rebase.add_argument('-b', '--newbase', dest='FspBase', nargs='+', type=str, help='Rebased FSP binary file name', default = '', required = True)
parser_rebase.add_argument('-o', '--outdir' , dest='OutputDir', type=str, help='Output directory path', default = '.')
parser_rebase.add_argument('-n', '--outfile', dest='OutputFile', type=str, help='Rebased FSP binary file name', default = '')
--
2.35.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91383): https://edk2.groups.io/g/devel/message/91383
Mute This Topic: https://groups.io/mt/92387601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Hi Chasel,
Please see comments inline. Here is a summary of my feedback:
#1) IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm - line 34 - Bug: eax should be rax
#2) IntelFsp2Pkg/Include/FspEas/FspApi.h - Various unnecessary whitespace changes that make the file look worse than before.
#3) IntelFsp2Pkg/Include/Guid/FspHeaderFile.h - Why indent the #pragma lines?
Thanks,
Nate
> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Thursday, July 14, 2022 1:04 PM
> To: devel@edk2.groups.io
> Cc: Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>
> Subject: [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for SMM support.
>
> From: Hongbin1 Zhang <hongbin1.zhang@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> Add FSP-I API entry point for SMM support.
>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> Signed-off-by: Hongbin1 Zhang <hongbin1.zhang@intel.com>
> ---
> IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 13 +++++++++++++
> IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++
> IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++
> IntelFsp2Pkg/Include/FspEas/FspApi.h | 57 ++++++++++++++++++++++++++++++++++++++-------------------
> IntelFsp2Pkg/Include/FspGlobalData.h | 53 ++++++++++++++++++++++++++++-------------------------
> IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 22 +++++++++++++++-------
> IntelFsp2Pkg/IntelFsp2Pkg.dsc | 1 +
> IntelFsp2Pkg/Tools/GenCfgOpt.py | 26 ++++++++++++++++----------
> IntelFsp2Pkg/Tools/SplitFspBin.py | 6 +++---
> 10 files changed, 256 insertions(+), 64 deletions(-)
>
> diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> index e22a88cc84..35d223a404 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> @@ -71,6 +71,19 @@ FspApiCallingCheck (
> Status = EFI_INVALID_PARAMETER;
> }
> }
> + } else if (ApiIdx == FspSmmInitApiIndex) {
> + //
> + // FspSmmInitApiIndex check
> + //
> + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS) || ((UINTN)FspData == MAX_UINT32)) {
> + Status = EFI_UNSUPPORTED;
> + } else {
> + if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
> + Status = EFI_UNSUPPORTED;
> + } else if (EFI_ERROR (FspUpdSignatureCheck (FspSmmInitApiIndex, ApiParam))) {
> + Status = EFI_INVALID_PARAMETER;
> + }
> + }
> } else {
> Status = EFI_UNSUPPORTED;
> }
> diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> new file mode 100644
> index 0000000000..d31576c00b
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> @@ -0,0 +1,54 @@
> +## @file
> +# Sec Core for FSP
> +#
> +# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = FspSecCoreI
> + FILE_GUID = 558782b5-782d-415e-ab9e-0ceb79dc3425
> + MODULE_TYPE = SEC
> + VERSION_STRING = 1.0
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = IA32 X64
> +#
> +
> +[Sources]
> + SecFspApiChk.c
> + SecFsp.h
> +
> +[Sources.X64]
> + X64/FspApiEntryI.nasm
> + X64/FspApiEntryCommon.nasm
> + X64/FspHelper.nasm
> +
> +[Sources.IA32]
> + Ia32/FspApiEntryI.nasm
> + Ia32/FspApiEntryCommon.nasm
> + Ia32/FspHelper.nasm
> +
> +[Binaries.Ia32]
> + RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + IntelFsp2Pkg/IntelFsp2Pkg.dec
> +
> +[LibraryClasses]
> + BaseMemoryLib
> + DebugLib
> + BaseLib
> + PciCf8Lib
> + SerialPortLib
> + FspSwitchStackLib
> + FspCommonLib
> + FspSecPlatformLib
> +
> +
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> new file mode 100644
> index 0000000000..e9365d6832
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> @@ -0,0 +1,44 @@
> +;; @file
> +; Provide FSP API entry points.
> +;
> +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;;
> +
> + SECTION .text
> +
> +;
> +; Following functions will be provided in C
> +;
> +extern ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; FspApiCommonContinue API
> +;
> +; This is the FSP API common entry point to resume the FSP execution
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspApiCommonContinue)
> +ASM_PFX(FspApiCommonContinue):
> + jmp $
> +
> +;----------------------------------------------------------------------------
> +; FspSmmInit API
> +;
> +; This FSP API will notify the FSP about the different phases in the boot
> +; process
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspSmmInitApi)
> +ASM_PFX(FspSmmInitApi):
> + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
> + jmp ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; Module Entrypoint API
> +;----------------------------------------------------------------------------
> +global ASM_PFX(_ModuleEntryPoint)
> +ASM_PFX(_ModuleEntryPoint):
> + jmp $
> + ; Add reference to APIs so that it will not be optimized by compiler
> + jmp ASM_PFX(FspSmmInitApi)
> diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> new file mode 100644
> index 0000000000..e9365d6832
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> @@ -0,0 +1,44 @@
> +;; @file
> +; Provide FSP API entry points.
> +;
> +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;;
> +
> + SECTION .text
> +
> +;
> +; Following functions will be provided in C
> +;
> +extern ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; FspApiCommonContinue API
> +;
> +; This is the FSP API common entry point to resume the FSP execution
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspApiCommonContinue)
> +ASM_PFX(FspApiCommonContinue):
> + jmp $
> +
> +;----------------------------------------------------------------------------
> +; FspSmmInit API
> +;
> +; This FSP API will notify the FSP about the different phases in the boot
> +; process
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspSmmInitApi)
> +ASM_PFX(FspSmmInitApi):
> + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
This is a bug. It should be:
mov rax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
Note that IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm compares ApiIndex numbers using rax, so it is important to make sure that the upper 32-bits are zero:
https://github.com/tianocore/edk2/blob/c966204049f3d5dae6d5e587ddc298c684142c5c/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm#L65
> + jmp ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; Module Entrypoint API
> +;----------------------------------------------------------------------------
> +global ASM_PFX(_ModuleEntryPoint)
> +ASM_PFX(_ModuleEntryPoint):
> + jmp $
> + ; Add reference to APIs so that it will not be optimized by compiler
> + jmp ASM_PFX(FspSmmInitApi)
> diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> index b36bc2b9ae..1d6c2fb63d 100644
> --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> @@ -135,18 +135,18 @@ typedef struct {
> ///
> /// Revision of the structure is 2 for this version of the specification.
> ///
> - UINT8 Revision;
> - UINT8 Reserved[3];
> + UINT8 Revision;
> + UINT8 Reserved[3];
Any reason for these whitespace changes? It looks worse than before.
> ///
> /// Length of the structure in bytes. The current value for this field is 32.
> ///
> - UINT32 Length;
> + UINT32 Length;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// FspDebugHandler Optional debug handler for the bootloader to receive debug messages
> /// occurring during FSP execution.
> ///
> - EFI_PHYSICAL_ADDRESS FspDebugHandler;
> - UINT8 Reserved1[16];
> + EFI_PHYSICAL_ADDRESS FspDebugHandler;
> + UINT8 Reserved1[16];
Any reason for these whitespace changes? It looks worse than before.
> } FSPT_ARCH2_UPD;
>
> ///
> @@ -197,37 +197,37 @@ typedef struct {
> ///
> /// Revision of the structure is 3 for this version of the specification.
> ///
> - UINT8 Revision;
> - UINT8 Reserved[3];
> + UINT8 Revision;
> + UINT8 Reserved[3];
Any reason for these whitespace changes? It looks worse than before.
> ///
> /// Length of the structure in bytes. The current value for this field is 64.
> ///
> - UINT32 Length;
> + UINT32 Length;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Pointer to the temporary stack base address to be
> /// consumed inside FspMemoryInit() API.
> ///
> - EFI_PHYSICAL_ADDRESS StackBase;
> + EFI_PHYSICAL_ADDRESS StackBase;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Temporary stack size to be consumed inside
> /// FspMemoryInit() API.
> ///
> - UINT64 StackSize;
> + UINT64 StackSize;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Size of memory to be reserved by FSP below "top
> /// of low usable memory" for bootloader usage.
> ///
> - UINT32 BootLoaderTolumSize;
> + UINT32 BootLoaderTolumSize;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Current boot mode.
> ///
> - UINT32 BootMode;
> + UINT32 BootMode;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Optional event handler for the bootloader to be informed of events occurring during FSP execution.
> /// This value is only valid if Revision is >= 2.
> ///
> - EFI_PHYSICAL_ADDRESS FspEventHandler;
> - UINT8 Reserved1[24];
> + EFI_PHYSICAL_ADDRESS FspEventHandler;
> + UINT8 Reserved1[24];
Any reason for these whitespace changes? It looks worse than before.
> } FSPM_ARCH2_UPD;
>
> ///
> @@ -266,18 +266,18 @@ typedef struct {
> ///
> /// Revision of the structure is 2 for this version of the specification.
> ///
> - UINT8 Revision;
> - UINT8 Reserved[3];
> + UINT8 Revision;
> + UINT8 Reserved[3];
Any reason for these whitespace changes? It looks worse than before.
> ///
> /// Length of the structure in bytes. The current value for this field is 32.
> ///
> - UINT32 Length;
> + UINT32 Length;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// FspEventHandler Optional event handler for the bootloader to be informed of events
> /// occurring during FSP execution.
> ///
> - EFI_PHYSICAL_ADDRESS FspEventHandler;
> - UINT8 Reserved1[16];
> + EFI_PHYSICAL_ADDRESS FspEventHandler;
> + UINT8 Reserved1[16];
Any reason for these whitespace changes? It looks worse than before.
> } FSPS_ARCH2_UPD;
>
> ///
> @@ -609,4 +609,23 @@ EFI_STATUS
> IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
> );
>
> +/**
> + This FSP API initializes SMM and provide any OS runtime silicon services,
> + including Reliability, Availability, and Serviceability (RAS) features implemented by the CPU.
> +
> + @param[in] FspiUpdDataPtr Pointer to the FSPI_UPD data structure.
> + If NULL, FSP will use the default parameters.
> +
> + @retval EFI_SUCCESS FSP execution environment was initialized successfully.
> + @retval EFI_INVALID_PARAMETER Input parameters are invalid.
> + @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
> + @retval EFI_DEVICE_ERROR FSP initialization failed.
> + @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status codes will not be returned during S3.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *FSP_SMM_INIT)(
> + IN VOID *FspiUpdDataPtr
> + );
> +
> #endif
> diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h b/IntelFsp2Pkg/Include/FspGlobalData.h
> index 445540abfa..697b20ed4c 100644
> --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> @@ -10,9 +10,9 @@
>
> #include <FspEas.h>
>
> -#define FSP_IN_API_MODE 0
> -#define FSP_IN_DISPATCH_MODE 1
> -#define FSP_GLOBAL_DATA_VERSION 1
> +#define FSP_IN_API_MODE 0
> +#define FSP_IN_DISPATCH_MODE 1
> +#define FSP_GLOBAL_DATA_VERSION 1
>
> #pragma pack(1)
>
> @@ -24,16 +24,17 @@ typedef enum {
> TempRamExitApiIndex,
> FspSiliconInitApiIndex,
> FspMultiPhaseSiInitApiIndex,
> + FspSmmInitApiIndex,
> FspApiIndexMax
> } FSP_API_INDEX;
>
> typedef struct {
> - VOID *DataPtr;
> - UINTN MicrocodeRegionBase;
> - UINTN MicrocodeRegionSize;
> - UINTN CodeRegionBase;
> - UINTN CodeRegionSize;
> - UINTN Reserved;
> + VOID *DataPtr;
> + UINTN MicrocodeRegionBase;
> + UINTN MicrocodeRegionSize;
> + UINTN CodeRegionBase;
> + UINTN CodeRegionSize;
> + UINTN Reserved;
> } FSP_PLAT_DATA;
>
> #define FSP_GLOBAL_DATA_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'D')
> @@ -41,28 +42,28 @@ typedef struct {
> #define FSP_PERFORMANCE_DATA_TIMER_MASK 0xFFFFFFFFFFFFFF
>
> typedef struct {
> - UINT32 Signature;
> - UINT8 Version;
> - UINT8 Reserved1[3];
> + UINT32 Signature;
> + UINT8 Version;
> + UINT8 Reserved1[3];
> ///
> /// Offset 0x08
> ///
> - UINTN CoreStack;
> - UINTN Reserved2;
> + UINTN CoreStack;
> + UINTN Reserved2;
> ///
> /// IA32: Offset 0x10; X64: Offset 0x18
> ///
> - UINT32 StatusCode;
> - UINT8 ApiIdx;
> + UINT32 StatusCode;
> + UINT8 ApiIdx;
> ///
> /// 0: FSP in API mode; 1: FSP in DISPATCH mode
> ///
> - UINT8 FspMode;
> - UINT8 OnSeparateStack;
> - UINT8 Reserved3;
> - UINT32 NumberOfPhases;
> - UINT32 PhasesExecuted;
> - UINT32 Reserved4[8];
> + UINT8 FspMode;
> + UINT8 OnSeparateStack;
> + UINT8 Reserved3;
> + UINT32 NumberOfPhases;
> + UINT32 PhasesExecuted;
> + UINT32 Reserved4[8];
> ///
> /// IA32: Offset 0x40; X64: Offset 0x48
> /// Start of UINTN and pointer section
> @@ -75,21 +76,23 @@ typedef struct {
> VOID *TempRamInitUpdPtr;
> VOID *MemoryInitUpdPtr;
> VOID *SiliconInitUpdPtr;
> + VOID *SmmInitUpdPtr;
> ///
> - /// IA32: Offset 0x64; X64: Offset 0x90
> + /// IA32: Offset 0x68; X64: Offset 0x98
> /// To store function parameters pointer
> /// so it can be retrieved after stack switched.
> ///
> VOID *FunctionParameterPtr;
> FSP_INFO_HEADER *FspInfoHeader;
> VOID *UpdDataPtr;
> + UINTN Reserved5;
> ///
> /// End of UINTN and pointer section
> ///
> - UINT8 Reserved5[16];
> + UINT8 Reserved6[16];
> UINT32 PerfSig;
> UINT16 PerfLen;
> - UINT16 Reserved6;
> + UINT16 Reserved7;
> UINT32 PerfIdx;
> UINT64 PerfData[32];
> } FSP_GLOBAL_DATA;
> diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> index c660defac3..c7fb63168f 100644
> --- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> +++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> @@ -26,13 +26,13 @@
>
> #define FSP_INFO_HEADER_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'H')
>
> -#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> -#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> -#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> -#define FSP_IA32 0
> -#define FSP_X64 1
> +#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> +#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> +#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> +#define FSP_IA32 0
> +#define FSP_X64 1
>
> -#pragma pack(1)
> + #pragma pack(1)
Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style guidelines don't require that and it certainly looks worse than before.
>
> ///
> /// FSP Information Header as described in FSP v2.0 Spec section 5.1.1.
> @@ -159,6 +159,14 @@ typedef struct {
> /// Byte 0x4E: Reserved4.
> ///
> UINT16 Reserved4;
> + ///
> + /// Byte 0x50: Offset for the API for the Multi-Phase memory initialization.
> + ///
> + UINT32 FspMultiPhaseMemInitEntryOffset;
> + ///
> + /// Byte 0x54: Offset for the API to initialize SMM.
> + ///
> + UINT32 FspSmmInitEntryOffset;
> } FSP_INFO_HEADER;
>
> ///
> @@ -240,7 +248,7 @@ typedef struct {
> // UINT32 PatchData[];
> } FSP_PATCH_TABLE;
>
> -#pragma pack()
> + #pragma pack()
Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style guidelines don't require that and it certainly looks worse than before.
>
> extern EFI_GUID gFspHeaderFileGuid;
>
> diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> index 7cf7e88245..b2d7867880 100644
> --- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> @@ -68,6 +68,7 @@
> IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
> + IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
>
> diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> index c4fb1f1bb2..128b896592 100644
> --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> @@ -953,8 +953,8 @@ EndList
> return NoFileChange
>
> def CreateSplitUpdTxt (self, UpdTxtFile):
> - GuidList = ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID']
> - SignatureList = ['0x545F', '0x4D5F','0x535F'] # _T, _M, and _S signature for FSPT, FSPM, FSPS
> + GuidList = ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID','FSP_I_UPD_TOOL_GUID']
> + SignatureList = ['0x545F', '0x4D5F','0x535F','0x495F'] # _T, _M, _S and _I signature for FSPT, FSPM, FSPS, FSPI
> for Index in range(len(GuidList)):
> UpdTxtFile = ''
> FvDir = self._FvDir
> @@ -1288,19 +1288,21 @@ EndList
> Chars.append(chr(Value & 0xFF))
> Value = Value >> 8
> SignatureStr = ''.join(Chars)
> - # Signature will be _T / _M / _S for FSPT / FSPM / FSPS accordingly
> + # Signature will be _T / _M / _S / _I for FSPT / FSPM / FSPS /FSPI accordingly
> if '_T' in SignatureStr[6:6+2]:
> TxtBody.append("#define FSPT_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> elif '_M' in SignatureStr[6:6+2]:
> TxtBody.append("#define FSPM_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> elif '_S' in SignatureStr[6:6+2]:
> TxtBody.append("#define FSPS_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> + elif '_I' in SignatureStr[6:6+2]:
> + TxtBody.append("#define FSPI_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> TxtBody.append("\n")
>
> for Region in ['UPD']:
> UpdOffsetTable = []
> - UpdSignature = ['0x545F', '0x4D5F', '0x535F'] #['_T', '_M', '_S'] signature for FSPT, FSPM, FSPS
> - UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD']
> + UpdSignature = ['0x545F', '0x4D5F', '0x535F', '0x495F'] #['_T', '_M', '_S', '_I'] signature for FSPT, FSPM, FSPS, FSPI
> + UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD', 'FSPI_UPD']
> for Item in self._CfgItemList:
> if Item["cname"] == 'Signature' and Item["value"][0:6] in UpdSignature:
> Item["offset"] = 0 # re-initialize offset to 0 when new UPD structure starting
> @@ -1393,11 +1395,12 @@ EndList
> HeaderTFileName = 'FsptUpd.h'
> HeaderMFileName = 'FspmUpd.h'
> HeaderSFileName = 'FspsUpd.h'
> + HeaderIFileName = 'FspiUpd.h'
>
> - UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS'] # FSPX_UPD_REGION
> - UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S'] # FSP_X_CONFIG, FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> - UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE']
> - ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD', 'FSPS_ARCH_UPD']
> + UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS', 'FSPI'] # FSPX_UPD_REGION
> + UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S', 'FSP_I'] # FSP_X_CONFIG, FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> + UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE', 'FSPI_UPD_SIGNATURE']
> + ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD', 'FSPS_ARCH_UPD', 'FSPI_ARCH_UPD']
> ExcludedSpecificUpd1 = ['FSPT_ARCH2_UPD', 'FSPM_ARCH2_UPD', 'FSPS_ARCH2_UPD']
>
> IncLines = []
> @@ -1420,6 +1423,9 @@ EndList
> elif UpdRegionCheck[item] == 'FSPS':
> HeaderFd = open(os.path.join(FvDir, HeaderSFileName), "w")
> FileBase = os.path.basename(os.path.join(FvDir, HeaderSFileName))
> + elif UpdRegionCheck[item] == 'FSPI':
> + HeaderFd = open(os.path.join(FvDir, HeaderIFileName), "w")
> + FileBase = os.path.basename(os.path.join(FvDir, HeaderIFileName))
> FileName = FileBase.replace(".", "_").upper()
> HeaderFd.write("%s\n" % (__copyright_h__ % date.today().year))
> HeaderFd.write("#ifndef __%s__\n" % FileName)
> @@ -1696,7 +1702,7 @@ EndList
>
>
> def Usage():
> - print ("GenCfgOpt Version 0.57")
> + print ("GenCfgOpt Version 0.58")
> print ("Usage:")
> print (" GenCfgOpt UPDTXT PlatformDscFile BuildFvDir [-D Macros]")
> print (" GenCfgOpt HEADER PlatformDscFile BuildFvDir InputHFile [-D Macros]")
> diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py b/IntelFsp2Pkg/Tools/SplitFspBin.py
> index f9151b5afd..317d9c1fa0 100644
> --- a/IntelFsp2Pkg/Tools/SplitFspBin.py
> +++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
> @@ -1,6 +1,6 @@
> ## @ SplitFspBin.py
> #
> -# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -492,7 +492,7 @@ class FspImage:
> self.FihOffset = fihoff
> self.Offset = offset
> self.FvIdxList = []
> - self.Type = "XTMSXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) & 0x0F]
> + self.Type = "XTMSIXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) & 0x0F]
> self.PatchList = patch
> self.PatchList.append(fihoff + 0x1C)
>
> @@ -869,7 +869,7 @@ def main ():
> parser_rebase = subparsers.add_parser('rebase', help='rebase a FSP into a new base address')
> parser_rebase.set_defaults(which='rebase')
> parser_rebase.add_argument('-f', '--fspbin' , dest='FspBinary', type=str, help='FSP binary file path', required = True)
> - parser_rebase.add_argument('-c', '--fspcomp', choices=['t','m','s','o'], nargs='+', dest='FspComponent', type=str, help='FSP component to rebase', default = "['t']", required = True)
> + parser_rebase.add_argument('-c', '--fspcomp', choices=['t','m','s','o','i'], nargs='+', dest='FspComponent', type=str, help='FSP component to rebase', default = "['t']", required = True)
> parser_rebase.add_argument('-b', '--newbase', dest='FspBase', nargs='+', type=str, help='Rebased FSP binary file name', default = '', required = True)
> parser_rebase.add_argument('-o', '--outdir' , dest='OutputDir', type=str, help='Output directory path', default = '.')
> parser_rebase.add_argument('-n', '--outfile', dest='OutputFile', type=str, help='Rebased FSP binary file name', default = '')
> --
> 2.35.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91485): https://edk2.groups.io/g/devel/message/91485
Mute This Topic: https://groups.io/mt/92387601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Nate,
Those whitespace and indents are for passing Uncrustify check.
I notice Uncrustify was enabled in Edk2 merging check and one of my previous patch was failed due to spaces/indents were not agreed by Uncrustify, that's why this time I run the Uncrustify offline before submitting the patch for reviewing.
Wiki link: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#recommended-usage-visual-studio-vs-code-plugin
I will fix ApiIdx issue and resend patch again.
Thanks,
Chasel
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Monday, July 18, 2022 2:41 PM
> To: Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
> Cc: Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for SMM support.
>
> Hi Chasel,
>
> Please see comments inline. Here is a summary of my feedback:
>
> #1) IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm - line 34 - Bug: eax should
> be rax
> #2) IntelFsp2Pkg/Include/FspEas/FspApi.h - Various unnecessary whitespace
> changes that make the file look worse than before.
> #3) IntelFsp2Pkg/Include/Guid/FspHeaderFile.h - Why indent the #pragma lines?
>
> Thanks,
> Nate
>
> > -----Original Message-----
> > From: Chiu, Chasel <chasel.chiu@intel.com>
> > Sent: Thursday, July 14, 2022 1:04 PM
> > To: devel@edk2.groups.io
> > Cc: Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>;
> > Chiu, Chasel <chasel.chiu@intel.com>
> > Subject: [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for SMM support.
> >
> > From: Hongbin1 Zhang <hongbin1.zhang@intel.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> > Add FSP-I API entry point for SMM support.
> >
> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > Signed-off-by: Hongbin1 Zhang <hongbin1.zhang@intel.com>
> > ---
> > IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 13 +++++++++++++
> > IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf | 54
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm | 44
> > ++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/Include/FspEas/FspApi.h | 57
> ++++++++++++++++++++++++++++++++++++++-------------------
> > IntelFsp2Pkg/Include/FspGlobalData.h | 53
> ++++++++++++++++++++++++++++-------------------------
> > IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 22 +++++++++++++++-------
> > IntelFsp2Pkg/IntelFsp2Pkg.dsc | 1 +
> > IntelFsp2Pkg/Tools/GenCfgOpt.py | 26 ++++++++++++++++----------
> > IntelFsp2Pkg/Tools/SplitFspBin.py | 6 +++---
> > 10 files changed, 256 insertions(+), 64 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > index e22a88cc84..35d223a404 100644
> > --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > @@ -71,6 +71,19 @@ FspApiCallingCheck (
> > Status = EFI_INVALID_PARAMETER;
> > }
> > }
> > + } else if (ApiIdx == FspSmmInitApiIndex) {
> > + //
> > + // FspSmmInitApiIndex check
> > + //
> > + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS) ||
> ((UINTN)FspData == MAX_UINT32)) {
> > + Status = EFI_UNSUPPORTED;
> > + } else {
> > + if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
> > + Status = EFI_UNSUPPORTED;
> > + } else if (EFI_ERROR (FspUpdSignatureCheck (FspSmmInitApiIndex,
> ApiParam))) {
> > + Status = EFI_INVALID_PARAMETER;
> > + }
> > + }
> > } else {
> > Status = EFI_UNSUPPORTED;
> > }
> > diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > new file mode 100644
> > index 0000000000..d31576c00b
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > @@ -0,0 +1,54 @@
> > +## @file
> > +# Sec Core for FSP
> > +#
> > +# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR> #
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> > +
> > +[Defines]
> > + INF_VERSION = 0x00010005
> > + BASE_NAME = FspSecCoreI
> > + FILE_GUID = 558782b5-782d-415e-ab9e-0ceb79dc3425
> > + MODULE_TYPE = SEC
> > + VERSION_STRING = 1.0
> > +
> > +#
> > +# The following information is for reference only and not required by the
> build tools.
> > +#
> > +# VALID_ARCHITECTURES = IA32 X64
> > +#
> > +
> > +[Sources]
> > + SecFspApiChk.c
> > + SecFsp.h
> > +
> > +[Sources.X64]
> > + X64/FspApiEntryI.nasm
> > + X64/FspApiEntryCommon.nasm
> > + X64/FspHelper.nasm
> > +
> > +[Sources.IA32]
> > + Ia32/FspApiEntryI.nasm
> > + Ia32/FspApiEntryCommon.nasm
> > + Ia32/FspHelper.nasm
> > +
> > +[Binaries.Ia32]
> > + RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + IntelFsp2Pkg/IntelFsp2Pkg.dec
> > +
> > +[LibraryClasses]
> > + BaseMemoryLib
> > + DebugLib
> > + BaseLib
> > + PciCf8Lib
> > + SerialPortLib
> > + FspSwitchStackLib
> > + FspCommonLib
> > + FspSecPlatformLib
> > +
> > +
> > diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> > b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> > new file mode 100644
> > index 0000000000..e9365d6832
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> > @@ -0,0 +1,44 @@
> > +;; @file
> > +; Provide FSP API entry points.
> > +;
> > +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR> ;
> > +SPDX-License-Identifier: BSD-2-Clause-Patent ;;
> > +
> > + SECTION .text
> > +
> > +;
> > +; Following functions will be provided in C ; extern
> > +ASM_PFX(FspApiCommon)
> > +
> > +;--------------------------------------------------------------------
> > +--------
> > +; FspApiCommonContinue API
> > +;
> > +; This is the FSP API common entry point to resume the FSP execution
> > +;
> > +;--------------------------------------------------------------------
> > +--------
> > +global ASM_PFX(FspApiCommonContinue)
> > +ASM_PFX(FspApiCommonContinue):
> > + jmp $
> > +
> > +;--------------------------------------------------------------------
> > +--------
> > +; FspSmmInit API
> > +;
> > +; This FSP API will notify the FSP about the different phases in the
> > +boot ; process ;
> > +;--------------------------------------------------------------------
> > +--------
> > +global ASM_PFX(FspSmmInitApi)
> > +ASM_PFX(FspSmmInitApi):
> > + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
> > + jmp ASM_PFX(FspApiCommon)
> > +
> > +;--------------------------------------------------------------------
> > +--------
> > +; Module Entrypoint API
> > +;--------------------------------------------------------------------
> > +--------
> > +global ASM_PFX(_ModuleEntryPoint)
> > +ASM_PFX(_ModuleEntryPoint):
> > + jmp $
> > + ; Add reference to APIs so that it will not be optimized by
> > +compiler
> > + jmp ASM_PFX(FspSmmInitApi)
> > diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> > b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> > new file mode 100644
> > index 0000000000..e9365d6832
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> > @@ -0,0 +1,44 @@
> > +;; @file
> > +; Provide FSP API entry points.
> > +;
> > +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR> ;
> > +SPDX-License-Identifier: BSD-2-Clause-Patent ;;
> > +
> > + SECTION .text
> > +
> > +;
> > +; Following functions will be provided in C ; extern
> > +ASM_PFX(FspApiCommon)
> > +
> > +;--------------------------------------------------------------------
> > +--------
> > +; FspApiCommonContinue API
> > +;
> > +; This is the FSP API common entry point to resume the FSP execution
> > +;
> > +;--------------------------------------------------------------------
> > +--------
> > +global ASM_PFX(FspApiCommonContinue)
> > +ASM_PFX(FspApiCommonContinue):
> > + jmp $
> > +
> > +;--------------------------------------------------------------------
> > +--------
> > +; FspSmmInit API
> > +;
> > +; This FSP API will notify the FSP about the different phases in the
> > +boot ; process ;
> > +;--------------------------------------------------------------------
> > +--------
> > +global ASM_PFX(FspSmmInitApi)
> > +ASM_PFX(FspSmmInitApi):
> > + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
>
> This is a bug. It should be:
>
> mov rax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
>
> Note that IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm compares
> ApiIndex numbers using rax, so it is important to make sure that the upper 32-
> bits are zero:
>
> https://github.com/tianocore/edk2/blob/c966204049f3d5dae6d5e587ddc298c6
> 84142c5c/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm#L65
>
> > + jmp ASM_PFX(FspApiCommon)
> > +
> > +;--------------------------------------------------------------------
> > +--------
> > +; Module Entrypoint API
> > +;--------------------------------------------------------------------
> > +--------
> > +global ASM_PFX(_ModuleEntryPoint)
> > +ASM_PFX(_ModuleEntryPoint):
> > + jmp $
> > + ; Add reference to APIs so that it will not be optimized by
> > +compiler
> > + jmp ASM_PFX(FspSmmInitApi)
> > diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > index b36bc2b9ae..1d6c2fb63d 100644
> > --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > @@ -135,18 +135,18 @@ typedef struct {
> > ///
> > /// Revision of the structure is 2 for this version of the specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this field is 32.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// FspDebugHandler Optional debug handler for the bootloader to receive
> debug messages
> > /// occurring during FSP execution.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspDebugHandler;
> > - UINT8 Reserved1[16];
> > + EFI_PHYSICAL_ADDRESS FspDebugHandler;
> > + UINT8 Reserved1[16];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPT_ARCH2_UPD;
> >
> > ///
> > @@ -197,37 +197,37 @@ typedef struct {
> > ///
> > /// Revision of the structure is 3 for this version of the specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this field is 64.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Pointer to the temporary stack base address to be
> > /// consumed inside FspMemoryInit() API.
> > ///
> > - EFI_PHYSICAL_ADDRESS StackBase;
> > + EFI_PHYSICAL_ADDRESS StackBase;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Temporary stack size to be consumed inside
> > /// FspMemoryInit() API.
> > ///
> > - UINT64 StackSize;
> > + UINT64 StackSize;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Size of memory to be reserved by FSP below "top
> > /// of low usable memory" for bootloader usage.
> > ///
> > - UINT32 BootLoaderTolumSize;
> > + UINT32 BootLoaderTolumSize;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Current boot mode.
> > ///
> > - UINT32 BootMode;
> > + UINT32 BootMode;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Optional event handler for the bootloader to be informed of events
> occurring during FSP execution.
> > /// This value is only valid if Revision is >= 2.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspEventHandler;
> > - UINT8 Reserved1[24];
> > + EFI_PHYSICAL_ADDRESS FspEventHandler;
> > + UINT8 Reserved1[24];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPM_ARCH2_UPD;
> >
> > ///
> > @@ -266,18 +266,18 @@ typedef struct {
> > ///
> > /// Revision of the structure is 2 for this version of the specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this field is 32.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// FspEventHandler Optional event handler for the bootloader to be
> informed of events
> > /// occurring during FSP execution.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspEventHandler;
> > - UINT8 Reserved1[16];
> > + EFI_PHYSICAL_ADDRESS FspEventHandler;
> > + UINT8 Reserved1[16];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPS_ARCH2_UPD;
> >
> > ///
> > @@ -609,4 +609,23 @@ EFI_STATUS
> > IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
> > );
> >
> > +/**
> > + This FSP API initializes SMM and provide any OS runtime silicon
> > +services,
> > + including Reliability, Availability, and Serviceability (RAS) features
> implemented by the CPU.
> > +
> > + @param[in] FspiUpdDataPtr Pointer to the FSPI_UPD data structure.
> > + If NULL, FSP will use the default parameters.
> > +
> > + @retval EFI_SUCCESS FSP execution environment was initialized
> successfully.
> > + @retval EFI_INVALID_PARAMETER Input parameters are invalid.
> > + @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
> > + @retval EFI_DEVICE_ERROR FSP initialization failed.
> > + @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status
> codes will not be returned during S3.
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *FSP_SMM_INIT)(
> > + IN VOID *FspiUpdDataPtr
> > + );
> > +
> > #endif
> > diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
> > b/IntelFsp2Pkg/Include/FspGlobalData.h
> > index 445540abfa..697b20ed4c 100644
> > --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> > +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> > @@ -10,9 +10,9 @@
> >
> > #include <FspEas.h>
> >
> > -#define FSP_IN_API_MODE 0
> > -#define FSP_IN_DISPATCH_MODE 1
> > -#define FSP_GLOBAL_DATA_VERSION 1
> > +#define FSP_IN_API_MODE 0
> > +#define FSP_IN_DISPATCH_MODE 1
> > +#define FSP_GLOBAL_DATA_VERSION 1
> >
> > #pragma pack(1)
> >
> > @@ -24,16 +24,17 @@ typedef enum {
> > TempRamExitApiIndex,
> > FspSiliconInitApiIndex,
> > FspMultiPhaseSiInitApiIndex,
> > + FspSmmInitApiIndex,
> > FspApiIndexMax
> > } FSP_API_INDEX;
> >
> > typedef struct {
> > - VOID *DataPtr;
> > - UINTN MicrocodeRegionBase;
> > - UINTN MicrocodeRegionSize;
> > - UINTN CodeRegionBase;
> > - UINTN CodeRegionSize;
> > - UINTN Reserved;
> > + VOID *DataPtr;
> > + UINTN MicrocodeRegionBase;
> > + UINTN MicrocodeRegionSize;
> > + UINTN CodeRegionBase;
> > + UINTN CodeRegionSize;
> > + UINTN Reserved;
> > } FSP_PLAT_DATA;
> >
> > #define FSP_GLOBAL_DATA_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'D')
> > @@ -41,28 +42,28 @@ typedef struct {
> > #define FSP_PERFORMANCE_DATA_TIMER_MASK 0xFFFFFFFFFFFFFF
> >
> > typedef struct {
> > - UINT32 Signature;
> > - UINT8 Version;
> > - UINT8 Reserved1[3];
> > + UINT32 Signature;
> > + UINT8 Version;
> > + UINT8 Reserved1[3];
> > ///
> > /// Offset 0x08
> > ///
> > - UINTN CoreStack;
> > - UINTN Reserved2;
> > + UINTN CoreStack;
> > + UINTN Reserved2;
> > ///
> > /// IA32: Offset 0x10; X64: Offset 0x18
> > ///
> > - UINT32 StatusCode;
> > - UINT8 ApiIdx;
> > + UINT32 StatusCode;
> > + UINT8 ApiIdx;
> > ///
> > /// 0: FSP in API mode; 1: FSP in DISPATCH mode
> > ///
> > - UINT8 FspMode;
> > - UINT8 OnSeparateStack;
> > - UINT8 Reserved3;
> > - UINT32 NumberOfPhases;
> > - UINT32 PhasesExecuted;
> > - UINT32 Reserved4[8];
> > + UINT8 FspMode;
> > + UINT8 OnSeparateStack;
> > + UINT8 Reserved3;
> > + UINT32 NumberOfPhases;
> > + UINT32 PhasesExecuted;
> > + UINT32 Reserved4[8];
> > ///
> > /// IA32: Offset 0x40; X64: Offset 0x48
> > /// Start of UINTN and pointer section @@ -75,21 +76,23 @@ typedef
> > struct {
> > VOID *TempRamInitUpdPtr;
> > VOID *MemoryInitUpdPtr;
> > VOID *SiliconInitUpdPtr;
> > + VOID *SmmInitUpdPtr;
> > ///
> > - /// IA32: Offset 0x64; X64: Offset 0x90
> > + /// IA32: Offset 0x68; X64: Offset 0x98
> > /// To store function parameters pointer
> > /// so it can be retrieved after stack switched.
> > ///
> > VOID *FunctionParameterPtr;
> > FSP_INFO_HEADER *FspInfoHeader;
> > VOID *UpdDataPtr;
> > + UINTN Reserved5;
> > ///
> > /// End of UINTN and pointer section
> > ///
> > - UINT8 Reserved5[16];
> > + UINT8 Reserved6[16];
> > UINT32 PerfSig;
> > UINT16 PerfLen;
> > - UINT16 Reserved6;
> > + UINT16 Reserved7;
> > UINT32 PerfIdx;
> > UINT64 PerfData[32];
> > } FSP_GLOBAL_DATA;
> > diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > index c660defac3..c7fb63168f 100644
> > --- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > +++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > @@ -26,13 +26,13 @@
> >
> > #define FSP_INFO_HEADER_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'H')
> >
> > -#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> > -#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> > -#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> > -#define FSP_IA32 0
> > -#define FSP_X64 1
> > +#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> > +#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> > +#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> > +#define FSP_IA32 0
> > +#define FSP_X64 1
> >
> > -#pragma pack(1)
> > + #pragma pack(1)
>
> Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style
> guidelines don't require that and it certainly looks worse than before.
>
> >
> > ///
> > /// FSP Information Header as described in FSP v2.0 Spec section 5.1.1.
> > @@ -159,6 +159,14 @@ typedef struct {
> > /// Byte 0x4E: Reserved4.
> > ///
> > UINT16 Reserved4;
> > + ///
> > + /// Byte 0x50: Offset for the API for the Multi-Phase memory initialization.
> > + ///
> > + UINT32 FspMultiPhaseMemInitEntryOffset;
> > + ///
> > + /// Byte 0x54: Offset for the API to initialize SMM.
> > + ///
> > + UINT32 FspSmmInitEntryOffset;
> > } FSP_INFO_HEADER;
> >
> > ///
> > @@ -240,7 +248,7 @@ typedef struct {
> > // UINT32 PatchData[];
> > } FSP_PATCH_TABLE;
> >
> > -#pragma pack()
> > + #pragma pack()
>
> Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style
> guidelines don't require that and it certainly looks worse than before.
>
> >
> > extern EFI_GUID gFspHeaderFileGuid;
> >
> > diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > b/IntelFsp2Pkg/IntelFsp2Pkg.dsc index 7cf7e88245..b2d7867880 100644
> > --- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > @@ -68,6 +68,7 @@
> > IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> > IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> > IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
> > + IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> > IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
> >
> > diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > b/IntelFsp2Pkg/Tools/GenCfgOpt.py index c4fb1f1bb2..128b896592 100644
> > --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > @@ -953,8 +953,8 @@ EndList
> > return NoFileChange
> >
> > def CreateSplitUpdTxt (self, UpdTxtFile):
> > - GuidList =
> ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID
> ']
> > - SignatureList = ['0x545F', '0x4D5F','0x535F'] # _T, _M, and _S
> signature for FSPT, FSPM, FSPS
> > + GuidList =
> ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID
> ','FSP_I_UPD_TOOL_GUID']
> > + SignatureList = ['0x545F', '0x4D5F','0x535F','0x495F'] # _T, _M, _S
> and _I signature for FSPT, FSPM, FSPS, FSPI
> > for Index in range(len(GuidList)):
> > UpdTxtFile = ''
> > FvDir = self._FvDir
> > @@ -1288,19 +1288,21 @@ EndList
> > Chars.append(chr(Value & 0xFF))
> > Value = Value >> 8
> > SignatureStr = ''.join(Chars)
> > - # Signature will be _T / _M / _S for FSPT / FSPM / FSPS accordingly
> > + # Signature will be _T / _M / _S / _I for FSPT / FSPM
> > + / FSPS /FSPI accordingly
> > if '_T' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPT_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > elif '_M' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPM_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > elif '_S' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPS_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > + elif '_I' in SignatureStr[6:6+2]:
> > + TxtBody.append("#define FSPI_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > TxtBody.append("\n")
> >
> > for Region in ['UPD']:
> > UpdOffsetTable = []
> > - UpdSignature = ['0x545F', '0x4D5F', '0x535F'] #['_T', '_M', '_S']
> signature for FSPT, FSPM, FSPS
> > - UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD']
> > + UpdSignature = ['0x545F', '0x4D5F', '0x535F', '0x495F'] #['_T', '_M',
> '_S', '_I'] signature for FSPT, FSPM, FSPS, FSPI
> > + UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD',
> > + 'FSPI_UPD']
> > for Item in self._CfgItemList:
> > if Item["cname"] == 'Signature' and Item["value"][0:6] in
> UpdSignature:
> > Item["offset"] = 0 # re-initialize offset to 0
> > when new UPD structure starting @@ -1393,11 +1395,12 @@ EndList
> > HeaderTFileName = 'FsptUpd.h'
> > HeaderMFileName = 'FspmUpd.h'
> > HeaderSFileName = 'FspsUpd.h'
> > + HeaderIFileName = 'FspiUpd.h'
> >
> > - UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS'] # FSPX_UPD_REGION
> > - UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S'] # FSP_X_CONFIG,
> FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> > - UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE',
> 'FSPS_UPD_SIGNATURE']
> > - ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD',
> 'FSPS_ARCH_UPD']
> > + UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS', 'FSPI'] # FSPX_UPD_REGION
> > + UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S', 'FSP_I'] # FSP_X_CONFIG,
> FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> > + UpdSignatureCheck = ['FSPT_UPD_SIGNATURE',
> 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE', 'FSPI_UPD_SIGNATURE']
> > + ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD',
> > + 'FSPS_ARCH_UPD', 'FSPI_ARCH_UPD']
> > ExcludedSpecificUpd1 = ['FSPT_ARCH2_UPD', 'FSPM_ARCH2_UPD',
> > 'FSPS_ARCH2_UPD']
> >
> > IncLines = []
> > @@ -1420,6 +1423,9 @@ EndList
> > elif UpdRegionCheck[item] == 'FSPS':
> > HeaderFd = open(os.path.join(FvDir, HeaderSFileName), "w")
> > FileBase = os.path.basename(os.path.join(FvDir,
> > HeaderSFileName))
> > + elif UpdRegionCheck[item] == 'FSPI':
> > + HeaderFd = open(os.path.join(FvDir, HeaderIFileName), "w")
> > + FileBase = os.path.basename(os.path.join(FvDir,
> > + HeaderIFileName))
> > FileName = FileBase.replace(".", "_").upper()
> > HeaderFd.write("%s\n" % (__copyright_h__ % date.today().year))
> > HeaderFd.write("#ifndef __%s__\n" % FileName)
> > @@ -1696,7 +1702,7 @@ EndList
> >
> >
> > def Usage():
> > - print ("GenCfgOpt Version 0.57")
> > + print ("GenCfgOpt Version 0.58")
> > print ("Usage:")
> > print (" GenCfgOpt UPDTXT PlatformDscFile BuildFvDir [-D
> Macros]")
> > print (" GenCfgOpt HEADER PlatformDscFile BuildFvDir InputHFile [-D
> Macros]")
> > diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py
> > b/IntelFsp2Pkg/Tools/SplitFspBin.py
> > index f9151b5afd..317d9c1fa0 100644
> > --- a/IntelFsp2Pkg/Tools/SplitFspBin.py
> > +++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
> > @@ -1,6 +1,6 @@
> > ## @ SplitFspBin.py
> > #
> > -# Copyright (c) 2015 - 2021, Intel Corporation. All rights
> > reserved.<BR>
> > +# Copyright (c) 2015 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> > # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -492,7
> > +492,7 @@ class FspImage:
> > self.FihOffset = fihoff
> > self.Offset = offset
> > self.FvIdxList = []
> > - self.Type = "XTMSXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) &
> 0x0F]
> > + self.Type = "XTMSIXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) &
> 0x0F]
> > self.PatchList = patch
> > self.PatchList.append(fihoff + 0x1C)
> >
> > @@ -869,7 +869,7 @@ def main ():
> > parser_rebase = subparsers.add_parser('rebase', help='rebase a FSP into a
> new base address')
> > parser_rebase.set_defaults(which='rebase')
> > parser_rebase.add_argument('-f', '--fspbin' , dest='FspBinary', type=str,
> help='FSP binary file path', required = True)
> > - parser_rebase.add_argument('-c', '--fspcomp', choices=['t','m','s','o'],
> nargs='+', dest='FspComponent', type=str, help='FSP component to rebase',
> default = "['t']", required = True)
> > + parser_rebase.add_argument('-c', '--fspcomp',
> > + choices=['t','m','s','o','i'], nargs='+', dest='FspComponent',
> > + type=str, help='FSP component to rebase', default = "['t']",
> > + required = True)
> > parser_rebase.add_argument('-b', '--newbase', dest='FspBase', nargs='+',
> type=str, help='Rebased FSP binary file name', default = '', required = True)
> > parser_rebase.add_argument('-o', '--outdir' , dest='OutputDir', type=str,
> help='Output directory path', default = '.')
> > parser_rebase.add_argument('-n', '--outfile', dest='OutputFile',
> > type=str, help='Rebased FSP binary file name', default = '')
> > --
> > 2.35.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91487): https://edk2.groups.io/g/devel/message/91487
Mute This Topic: https://groups.io/mt/92387601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Mon, Jul 18, 2022 at 10:40 PM Nate DeSimone <
nathaniel.l.desimone@intel.com> wrote:
> Hi Chasel,
>
> Please see comments inline. Here is a summary of my feedback:
>
> #1) IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm - line 34 - Bug: eax
> should be rax
> #2) IntelFsp2Pkg/Include/FspEas/FspApi.h - Various unnecessary whitespace
> changes that make the file look worse than before.
> #3) IntelFsp2Pkg/Include/Guid/FspHeaderFile.h - Why indent the #pragma
> lines?
>
> Thanks,
> Nate
>
> > -----Original Message-----
> > From: Chiu, Chasel <chasel.chiu@intel.com>
> > Sent: Thursday, July 14, 2022 1:04 PM
> > To: devel@edk2.groups.io
> > Cc: Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Chiu,
> > Chasel <chasel.chiu@intel.com>
> > Subject: [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for SMM support.
> >
> > From: Hongbin1 Zhang <hongbin1.zhang@intel.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> > Add FSP-I API entry point for SMM support.
> >
> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > Signed-off-by: Hongbin1 Zhang <hongbin1.zhang@intel.com>
> > ---
> > IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 13 +++++++++++++
> > IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf | 54
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/Include/FspEas/FspApi.h | 57
> ++++++++++++++++++++++++++++++++++++++-------------------
> > IntelFsp2Pkg/Include/FspGlobalData.h | 53
> ++++++++++++++++++++++++++++-------------------------
> > IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 22
> +++++++++++++++-------
> > IntelFsp2Pkg/IntelFsp2Pkg.dsc | 1 +
> > IntelFsp2Pkg/Tools/GenCfgOpt.py | 26
> ++++++++++++++++----------
> > IntelFsp2Pkg/Tools/SplitFspBin.py | 6 +++---
> > 10 files changed, 256 insertions(+), 64 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > index e22a88cc84..35d223a404 100644
> > --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > @@ -71,6 +71,19 @@ FspApiCallingCheck (
> > Status = EFI_INVALID_PARAMETER;
> > }
> > }
> > + } else if (ApiIdx == FspSmmInitApiIndex) {
> > + //
> > + // FspSmmInitApiIndex check
> > + //
> > + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS) ||
> ((UINTN)FspData == MAX_UINT32)) {
> > + Status = EFI_UNSUPPORTED;
> > + } else {
> > + if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
> > + Status = EFI_UNSUPPORTED;
> > + } else if (EFI_ERROR (FspUpdSignatureCheck (FspSmmInitApiIndex,
> ApiParam))) {
> > + Status = EFI_INVALID_PARAMETER;
> > + }
> > + }
> > } else {
> > Status = EFI_UNSUPPORTED;
> > }
> > diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > new file mode 100644
> > index 0000000000..d31576c00b
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > @@ -0,0 +1,54 @@
> > +## @file
> > +# Sec Core for FSP
> > +#
> > +# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +[Defines]
> > + INF_VERSION = 0x00010005
> > + BASE_NAME = FspSecCoreI
> > + FILE_GUID = 558782b5-782d-415e-ab9e-0ceb79dc3425
> > + MODULE_TYPE = SEC
> > + VERSION_STRING = 1.0
> > +
> > +#
> > +# The following information is for reference only and not required by
> the build tools.
> > +#
> > +# VALID_ARCHITECTURES = IA32 X64
> > +#
> > +
> > +[Sources]
> > + SecFspApiChk.c
> > + SecFsp.h
> > +
> > +[Sources.X64]
> > + X64/FspApiEntryI.nasm
> > + X64/FspApiEntryCommon.nasm
> > + X64/FspHelper.nasm
> > +
> > +[Sources.IA32]
> > + Ia32/FspApiEntryI.nasm
> > + Ia32/FspApiEntryCommon.nasm
> > + Ia32/FspHelper.nasm
> > +
> > +[Binaries.Ia32]
> > + RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + IntelFsp2Pkg/IntelFsp2Pkg.dec
> > +
> > +[LibraryClasses]
> > + BaseMemoryLib
> > + DebugLib
> > + BaseLib
> > + PciCf8Lib
> > + SerialPortLib
> > + FspSwitchStackLib
> > + FspCommonLib
> > + FspSecPlatformLib
> > +
> > +
> > diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> > new file mode 100644
> > index 0000000000..e9365d6832
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> > @@ -0,0 +1,44 @@
> > +;; @file
> > +; Provide FSP API entry points.
> > +;
> > +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > +; SPDX-License-Identifier: BSD-2-Clause-Patent
> > +;;
> > +
> > + SECTION .text
> > +
> > +;
> > +; Following functions will be provided in C
> > +;
> > +extern ASM_PFX(FspApiCommon)
> > +
> >
> +;----------------------------------------------------------------------------
> > +; FspApiCommonContinue API
> > +;
> > +; This is the FSP API common entry point to resume the FSP execution
> > +;
> >
> +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspApiCommonContinue)
> > +ASM_PFX(FspApiCommonContinue):
> > + jmp $
> > +
> >
> +;----------------------------------------------------------------------------
> > +; FspSmmInit API
> > +;
> > +; This FSP API will notify the FSP about the different phases in the
> boot
> > +; process
> > +;
> >
> +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspSmmInitApi)
> > +ASM_PFX(FspSmmInitApi):
> > + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
> > + jmp ASM_PFX(FspApiCommon)
> > +
> >
> +;----------------------------------------------------------------------------
> > +; Module Entrypoint API
> >
> +;----------------------------------------------------------------------------
> > +global ASM_PFX(_ModuleEntryPoint)
> > +ASM_PFX(_ModuleEntryPoint):
> > + jmp $
> > + ; Add reference to APIs so that it will not be optimized by compiler
> > + jmp ASM_PFX(FspSmmInitApi)
> > diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> > new file mode 100644
> > index 0000000000..e9365d6832
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> > @@ -0,0 +1,44 @@
> > +;; @file
> > +; Provide FSP API entry points.
> > +;
> > +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > +; SPDX-License-Identifier: BSD-2-Clause-Patent
> > +;;
> > +
> > + SECTION .text
> > +
> > +;
> > +; Following functions will be provided in C
> > +;
> > +extern ASM_PFX(FspApiCommon)
> > +
> >
> +;----------------------------------------------------------------------------
> > +; FspApiCommonContinue API
> > +;
> > +; This is the FSP API common entry point to resume the FSP execution
> > +;
> >
> +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspApiCommonContinue)
> > +ASM_PFX(FspApiCommonContinue):
> > + jmp $
> > +
> >
> +;----------------------------------------------------------------------------
> > +; FspSmmInit API
> > +;
> > +; This FSP API will notify the FSP about the different phases in the
> boot
> > +; process
> > +;
> >
> +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspSmmInitApi)
> > +ASM_PFX(FspSmmInitApi):
> > + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
>
> This is a bug. It should be:
>
> mov rax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
>
Hi Nate,
This is not actually a problem, a 32-bit mov to a 32-bit register in long
mode will zero the upper part.
>
> Note that IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm compares
> ApiIndex numbers using rax, so it is important to make sure that the upper
> 32-bits are zero:
>
>
> https://github.com/tianocore/edk2/blob/c966204049f3d5dae6d5e587ddc298c684142c5c/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm#L65
>
> > + jmp ASM_PFX(FspApiCommon)
> > +
> >
> +;----------------------------------------------------------------------------
> > +; Module Entrypoint API
> >
> +;----------------------------------------------------------------------------
> > +global ASM_PFX(_ModuleEntryPoint)
> > +ASM_PFX(_ModuleEntryPoint):
> > + jmp $
> > + ; Add reference to APIs so that it will not be optimized by compiler
> > + jmp ASM_PFX(FspSmmInitApi)
> > diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > index b36bc2b9ae..1d6c2fb63d 100644
> > --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > @@ -135,18 +135,18 @@ typedef struct {
> > ///
> > /// Revision of the structure is 2 for this version of the
> specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this
> field is 32.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// FspDebugHandler Optional debug handler for the bootloader to
> receive debug messages
> > /// occurring during FSP execution.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspDebugHandler;
> > - UINT8 Reserved1[16];
> > + EFI_PHYSICAL_ADDRESS FspDebugHandler;
> > + UINT8 Reserved1[16];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPT_ARCH2_UPD;
> >
> > ///
> > @@ -197,37 +197,37 @@ typedef struct {
> > ///
> > /// Revision of the structure is 3 for this version of the
> specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this
> field is 64.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Pointer to the temporary stack base address to be
> > /// consumed inside FspMemoryInit() API.
> > ///
> > - EFI_PHYSICAL_ADDRESS StackBase;
> > + EFI_PHYSICAL_ADDRESS StackBase;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Temporary stack size to be consumed inside
> > /// FspMemoryInit() API.
> > ///
> > - UINT64 StackSize;
> > + UINT64 StackSize;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Size of memory to be reserved by FSP below "top
> > /// of low usable memory" for bootloader usage.
> > ///
> > - UINT32 BootLoaderTolumSize;
> > + UINT32 BootLoaderTolumSize;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Current boot mode.
> > ///
> > - UINT32 BootMode;
> > + UINT32 BootMode;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Optional event handler for the bootloader to be informed of
> events occurring during FSP execution.
> > /// This value is only valid if Revision is >= 2.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspEventHandler;
> > - UINT8 Reserved1[24];
> > + EFI_PHYSICAL_ADDRESS FspEventHandler;
> > + UINT8 Reserved1[24];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPM_ARCH2_UPD;
> >
> > ///
> > @@ -266,18 +266,18 @@ typedef struct {
> > ///
> > /// Revision of the structure is 2 for this version of the
> specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this
> field is 32.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// FspEventHandler Optional event handler for the bootloader to be
> informed of events
> > /// occurring during FSP execution.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspEventHandler;
> > - UINT8 Reserved1[16];
> > + EFI_PHYSICAL_ADDRESS FspEventHandler;
> > + UINT8 Reserved1[16];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPS_ARCH2_UPD;
> >
> > ///
> > @@ -609,4 +609,23 @@ EFI_STATUS
> > IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
> > );
> >
> > +/**
> > + This FSP API initializes SMM and provide any OS runtime silicon
> services,
> > + including Reliability, Availability, and Serviceability (RAS)
> features implemented by the CPU.
> > +
> > + @param[in] FspiUpdDataPtr Pointer to the FSPI_UPD data structure.
> > + If NULL, FSP will use the default
> parameters.
> > +
> > + @retval EFI_SUCCESS FSP execution environment was
> initialized successfully.
> > + @retval EFI_INVALID_PARAMETER Input parameters are invalid.
> > + @retval EFI_UNSUPPORTED The FSP calling conditions were
> not met.
> > + @retval EFI_DEVICE_ERROR FSP initialization failed.
> > + @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status
> codes will not be returned during S3.
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *FSP_SMM_INIT)(
> > + IN VOID *FspiUpdDataPtr
> > + );
> > +
> > #endif
> > diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
> b/IntelFsp2Pkg/Include/FspGlobalData.h
> > index 445540abfa..697b20ed4c 100644
> > --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> > +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> > @@ -10,9 +10,9 @@
> >
> > #include <FspEas.h>
> >
> > -#define FSP_IN_API_MODE 0
> > -#define FSP_IN_DISPATCH_MODE 1
> > -#define FSP_GLOBAL_DATA_VERSION 1
> > +#define FSP_IN_API_MODE 0
> > +#define FSP_IN_DISPATCH_MODE 1
> > +#define FSP_GLOBAL_DATA_VERSION 1
> >
> > #pragma pack(1)
> >
> > @@ -24,16 +24,17 @@ typedef enum {
> > TempRamExitApiIndex,
> > FspSiliconInitApiIndex,
> > FspMultiPhaseSiInitApiIndex,
> > + FspSmmInitApiIndex,
> > FspApiIndexMax
> > } FSP_API_INDEX;
> >
> > typedef struct {
> > - VOID *DataPtr;
> > - UINTN MicrocodeRegionBase;
> > - UINTN MicrocodeRegionSize;
> > - UINTN CodeRegionBase;
> > - UINTN CodeRegionSize;
> > - UINTN Reserved;
> > + VOID *DataPtr;
> > + UINTN MicrocodeRegionBase;
> > + UINTN MicrocodeRegionSize;
> > + UINTN CodeRegionBase;
> > + UINTN CodeRegionSize;
> > + UINTN Reserved;
> > } FSP_PLAT_DATA;
> >
> > #define FSP_GLOBAL_DATA_SIGNATURE SIGNATURE_32 ('F', 'S', 'P',
> 'D')
> > @@ -41,28 +42,28 @@ typedef struct {
> > #define FSP_PERFORMANCE_DATA_TIMER_MASK 0xFFFFFFFFFFFFFF
> >
> > typedef struct {
> > - UINT32 Signature;
> > - UINT8 Version;
> > - UINT8 Reserved1[3];
> > + UINT32 Signature;
> > + UINT8 Version;
> > + UINT8 Reserved1[3];
> > ///
> > /// Offset 0x08
> > ///
> > - UINTN CoreStack;
> > - UINTN Reserved2;
> > + UINTN CoreStack;
> > + UINTN Reserved2;
> > ///
> > /// IA32: Offset 0x10; X64: Offset 0x18
> > ///
> > - UINT32 StatusCode;
> > - UINT8 ApiIdx;
> > + UINT32 StatusCode;
> > + UINT8 ApiIdx;
> > ///
> > /// 0: FSP in API mode; 1: FSP in DISPATCH mode
> > ///
> > - UINT8 FspMode;
> > - UINT8 OnSeparateStack;
> > - UINT8 Reserved3;
> > - UINT32 NumberOfPhases;
> > - UINT32 PhasesExecuted;
> > - UINT32 Reserved4[8];
> > + UINT8 FspMode;
> > + UINT8 OnSeparateStack;
> > + UINT8 Reserved3;
> > + UINT32 NumberOfPhases;
> > + UINT32 PhasesExecuted;
> > + UINT32 Reserved4[8];
> > ///
> > /// IA32: Offset 0x40; X64: Offset 0x48
> > /// Start of UINTN and pointer section
> > @@ -75,21 +76,23 @@ typedef struct {
> > VOID *TempRamInitUpdPtr;
> > VOID *MemoryInitUpdPtr;
> > VOID *SiliconInitUpdPtr;
> > + VOID *SmmInitUpdPtr;
> > ///
> > - /// IA32: Offset 0x64; X64: Offset 0x90
> > + /// IA32: Offset 0x68; X64: Offset 0x98
> > /// To store function parameters pointer
> > /// so it can be retrieved after stack switched.
> > ///
> > VOID *FunctionParameterPtr;
> > FSP_INFO_HEADER *FspInfoHeader;
> > VOID *UpdDataPtr;
> > + UINTN Reserved5;
> > ///
> > /// End of UINTN and pointer section
> > ///
> > - UINT8 Reserved5[16];
> > + UINT8 Reserved6[16];
> > UINT32 PerfSig;
> > UINT16 PerfLen;
> > - UINT16 Reserved6;
> > + UINT16 Reserved7;
> > UINT32 PerfIdx;
> > UINT64 PerfData[32];
> > } FSP_GLOBAL_DATA;
> > diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > index c660defac3..c7fb63168f 100644
> > --- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > +++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > @@ -26,13 +26,13 @@
> >
> > #define FSP_INFO_HEADER_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'H')
> >
> > -#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> > -#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> > -#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> > -#define FSP_IA32 0
> > -#define FSP_X64 1
> > +#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> > +#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> > +#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> > +#define FSP_IA32 0
> > +#define FSP_X64 1
> >
> > -#pragma pack(1)
> > + #pragma pack(1)
>
> Why add an indent to the #pragma line? I'm pretty sure that edk2 coding
> style guidelines don't require that and it certainly looks worse than
> before.
>
> >
> > ///
> > /// FSP Information Header as described in FSP v2.0 Spec section 5.1.1.
> > @@ -159,6 +159,14 @@ typedef struct {
> > /// Byte 0x4E: Reserved4.
> > ///
> > UINT16 Reserved4;
> > + ///
> > + /// Byte 0x50: Offset for the API for the Multi-Phase memory
> initialization.
> > + ///
> > + UINT32 FspMultiPhaseMemInitEntryOffset;
> > + ///
> > + /// Byte 0x54: Offset for the API to initialize SMM.
> > + ///
> > + UINT32 FspSmmInitEntryOffset;
> > } FSP_INFO_HEADER;
> >
> > ///
> > @@ -240,7 +248,7 @@ typedef struct {
> > // UINT32 PatchData[];
> > } FSP_PATCH_TABLE;
> >
> > -#pragma pack()
> > + #pragma pack()
>
> Why add an indent to the #pragma line? I'm pretty sure that edk2 coding
> style guidelines don't require that and it certainly looks worse than
> before.
>
> >
> > extern EFI_GUID gFspHeaderFileGuid;
> >
> > diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > index 7cf7e88245..b2d7867880 100644
> > --- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > @@ -68,6 +68,7 @@
> > IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> > IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> > IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
> > + IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> > IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
> >
> > diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > index c4fb1f1bb2..128b896592 100644
> > --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > @@ -953,8 +953,8 @@ EndList
> > return NoFileChange
> >
> > def CreateSplitUpdTxt (self, UpdTxtFile):
> > - GuidList =
> ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID']
> > - SignatureList = ['0x545F', '0x4D5F','0x535F'] # _T, _M,
> and _S signature for FSPT, FSPM, FSPS
> > + GuidList =
> ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID','FSP_I_UPD_TOOL_GUID']
> > + SignatureList = ['0x545F', '0x4D5F','0x535F','0x495F']
> # _T, _M, _S and _I signature for FSPT, FSPM, FSPS, FSPI
> > for Index in range(len(GuidList)):
> > UpdTxtFile = ''
> > FvDir = self._FvDir
> > @@ -1288,19 +1288,21 @@ EndList
> > Chars.append(chr(Value & 0xFF))
> > Value = Value >> 8
> > SignatureStr = ''.join(Chars)
> > - # Signature will be _T / _M / _S for FSPT / FSPM / FSPS
> accordingly
> > + # Signature will be _T / _M / _S / _I for FSPT / FSPM /
> FSPS /FSPI accordingly
> > if '_T' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPT_UPD_SIGNATURE
> %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> > elif '_M' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPM_UPD_SIGNATURE
> %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> > elif '_S' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPS_UPD_SIGNATURE
> %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> > + elif '_I' in SignatureStr[6:6+2]:
> > + TxtBody.append("#define FSPI_UPD_SIGNATURE
> %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> > TxtBody.append("\n")
> >
> > for Region in ['UPD']:
> > UpdOffsetTable = []
> > - UpdSignature = ['0x545F', '0x4D5F', '0x535F'] #['_T',
> '_M', '_S'] signature for FSPT, FSPM, FSPS
> > - UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD']
> > + UpdSignature = ['0x545F', '0x4D5F', '0x535F', '0x495F']
> #['_T', '_M', '_S', '_I'] signature for FSPT, FSPM, FSPS, FSPI
> > + UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD',
> 'FSPI_UPD']
> > for Item in self._CfgItemList:
> > if Item["cname"] == 'Signature' and Item["value"][0:6]
> in UpdSignature:
> > Item["offset"] = 0 # re-initialize offset to 0 when
> new UPD structure starting
> > @@ -1393,11 +1395,12 @@ EndList
> > HeaderTFileName = 'FsptUpd.h'
> > HeaderMFileName = 'FspmUpd.h'
> > HeaderSFileName = 'FspsUpd.h'
> > + HeaderIFileName = 'FspiUpd.h'
> >
> > - UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS'] # FSPX_UPD_REGION
> > - UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S'] # FSP_X_CONFIG,
> FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> > - UpdSignatureCheck = ['FSPT_UPD_SIGNATURE',
> 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE']
> > - ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD',
> 'FSPS_ARCH_UPD']
> > + UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS', 'FSPI'] #
> FSPX_UPD_REGION
> > + UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S', 'FSP_I'] #
> FSP_X_CONFIG, FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> > + UpdSignatureCheck = ['FSPT_UPD_SIGNATURE',
> 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE', 'FSPI_UPD_SIGNATURE']
> > + ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD',
> 'FSPS_ARCH_UPD', 'FSPI_ARCH_UPD']
> > ExcludedSpecificUpd1 = ['FSPT_ARCH2_UPD', 'FSPM_ARCH2_UPD',
> 'FSPS_ARCH2_UPD']
> >
> > IncLines = []
> > @@ -1420,6 +1423,9 @@ EndList
> > elif UpdRegionCheck[item] == 'FSPS':
> > HeaderFd = open(os.path.join(FvDir, HeaderSFileName),
> "w")
> > FileBase = os.path.basename(os.path.join(FvDir,
> HeaderSFileName))
> > + elif UpdRegionCheck[item] == 'FSPI':
> > + HeaderFd = open(os.path.join(FvDir, HeaderIFileName),
> "w")
> > + FileBase = os.path.basename(os.path.join(FvDir,
> HeaderIFileName))
> > FileName = FileBase.replace(".", "_").upper()
> > HeaderFd.write("%s\n" % (__copyright_h__ %
> date.today().year))
> > HeaderFd.write("#ifndef __%s__\n" % FileName)
> > @@ -1696,7 +1702,7 @@ EndList
> >
> >
> > def Usage():
> > - print ("GenCfgOpt Version 0.57")
> > + print ("GenCfgOpt Version 0.58")
> > print ("Usage:")
> > print (" GenCfgOpt UPDTXT PlatformDscFile BuildFvDir
> [-D Macros]")
> > print (" GenCfgOpt HEADER PlatformDscFile BuildFvDir
> InputHFile [-D Macros]")
> > diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py
> b/IntelFsp2Pkg/Tools/SplitFspBin.py
> > index f9151b5afd..317d9c1fa0 100644
> > --- a/IntelFsp2Pkg/Tools/SplitFspBin.py
> > +++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
> > @@ -1,6 +1,6 @@
> > ## @ SplitFspBin.py
> > #
> > -# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR>
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > ##
> > @@ -492,7 +492,7 @@ class FspImage:
> > self.FihOffset = fihoff
> > self.Offset = offset
> > self.FvIdxList = []
> > - self.Type = "XTMSXXXXOXXXXXXX"[(fih.ComponentAttribute >>
> 12) & 0x0F]
> > + self.Type = "XTMSIXXXXOXXXXXXX"[(fih.ComponentAttribute >>
> 12) & 0x0F]
> > self.PatchList = patch
> > self.PatchList.append(fihoff + 0x1C)
> >
> > @@ -869,7 +869,7 @@ def main ():
> > parser_rebase = subparsers.add_parser('rebase', help='rebase a
> FSP into a new base address')
> > parser_rebase.set_defaults(which='rebase')
> > parser_rebase.add_argument('-f', '--fspbin' , dest='FspBinary',
> type=str, help='FSP binary file path', required = True)
> > - parser_rebase.add_argument('-c', '--fspcomp',
> choices=['t','m','s','o'], nargs='+', dest='FspComponent', type=str,
> help='FSP component to rebase', default = "['t']", required = True)
> > + parser_rebase.add_argument('-c', '--fspcomp',
> choices=['t','m','s','o','i'], nargs='+', dest='FspComponent', type=str,
> help='FSP component to rebase', default = "['t']", required = True)
> > parser_rebase.add_argument('-b', '--newbase', dest='FspBase',
> nargs='+', type=str, help='Rebased FSP binary file name', default = '',
> required = True)
> > parser_rebase.add_argument('-o', '--outdir' , dest='OutputDir',
> type=str, help='Output directory path', default = '.')
> > parser_rebase.add_argument('-n', '--outfile', dest='OutputFile',
> type=str, help='Rebased FSP binary file name', default = '')
> > --
> > 2.35.0.windows.1
>
>
>
>
>
>
>
--
Pedro Falcato
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91486): https://edk2.groups.io/g/devel/message/91486
Mute This Topic: https://groups.io/mt/92387601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Pedro,
Wow you are totally right... that is crazy! The exact opposite of the way 32-bit mode works!
Good catch thank you!
Please consider by code review feedback downgraded from a bug to a recommendation to use rax instead of eax to avoid confusion 😊.
Thanks,
Nate
From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: Monday, July 18, 2022 2:50 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2-devel] [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for SMM support.
On Mon, Jul 18, 2022 at 10:40 PM Nate DeSimone <mailto:nathaniel.l.desimone@intel.com> wrote:
Hi Chasel,
Please see comments inline. Here is a summary of my feedback:
#1) IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm - line 34 - Bug: eax should be rax
#2) IntelFsp2Pkg/Include/FspEas/FspApi.h - Various unnecessary whitespace changes that make the file look worse than before.
#3) IntelFsp2Pkg/Include/Guid/FspHeaderFile.h - Why indent the #pragma lines?
Thanks,
Nate
> -----Original Message-----
> From: Chiu, Chasel <mailto:chasel.chiu@intel.com>
> Sent: Thursday, July 14, 2022 1:04 PM
> To: mailto:devel@edk2.groups.io
> Cc: Zhang, Hongbin1 <mailto:hongbin1.zhang@intel.com>; Desimone, Nathaniel L
> <mailto:nathaniel.l.desimone@intel.com>; Zeng, Star <mailto:star.zeng@intel.com>; Chiu,
> Chasel <mailto:chasel.chiu@intel.com>
> Subject: [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for SMM support.
>
> From: Hongbin1 Zhang <mailto:hongbin1.zhang@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> Add FSP-I API entry point for SMM support.
>
> Cc: Nate DeSimone <mailto:nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <mailto:star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <mailto:chasel.chiu@intel.com>
> Signed-off-by: Hongbin1 Zhang <mailto:hongbin1.zhang@intel.com>
> ---
> IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 13 +++++++++++++
> IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++
> IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm | 44 ++++++++++++++++++++++++++++++++++++++++++++
> IntelFsp2Pkg/Include/FspEas/FspApi.h | 57 ++++++++++++++++++++++++++++++++++++++-------------------
> IntelFsp2Pkg/Include/FspGlobalData.h | 53 ++++++++++++++++++++++++++++-------------------------
> IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 22 +++++++++++++++-------
> IntelFsp2Pkg/IntelFsp2Pkg.dsc | 1 +
> IntelFsp2Pkg/Tools/GenCfgOpt.py | 26 ++++++++++++++++----------
> IntelFsp2Pkg/Tools/SplitFspBin.py | 6 +++---
> 10 files changed, 256 insertions(+), 64 deletions(-)
>
> diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> index e22a88cc84..35d223a404 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> @@ -71,6 +71,19 @@ FspApiCallingCheck (
> Status = EFI_INVALID_PARAMETER;
> }
> }
> + } else if (ApiIdx == FspSmmInitApiIndex) {
> + //
> + // FspSmmInitApiIndex check
> + //
> + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS) || ((UINTN)FspData == MAX_UINT32)) {
> + Status = EFI_UNSUPPORTED;
> + } else {
> + if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
> + Status = EFI_UNSUPPORTED;
> + } else if (EFI_ERROR (FspUpdSignatureCheck (FspSmmInitApiIndex, ApiParam))) {
> + Status = EFI_INVALID_PARAMETER;
> + }
> + }
> } else {
> Status = EFI_UNSUPPORTED;
> }
> diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> new file mode 100644
> index 0000000000..d31576c00b
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> @@ -0,0 +1,54 @@
> +## @file
> +# Sec Core for FSP
> +#
> +# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = FspSecCoreI
> + FILE_GUID = 558782b5-782d-415e-ab9e-0ceb79dc3425
> + MODULE_TYPE = SEC
> + VERSION_STRING = 1.0
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = IA32 X64
> +#
> +
> +[Sources]
> + SecFspApiChk.c
> + SecFsp.h
> +
> +[Sources.X64]
> + X64/FspApiEntryI.nasm
> + X64/FspApiEntryCommon.nasm
> + X64/FspHelper.nasm
> +
> +[Sources.IA32]
> + Ia32/FspApiEntryI.nasm
> + Ia32/FspApiEntryCommon.nasm
> + Ia32/FspHelper.nasm
> +
> +[Binaries.Ia32]
> + RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + IntelFsp2Pkg/IntelFsp2Pkg.dec
> +
> +[LibraryClasses]
> + BaseMemoryLib
> + DebugLib
> + BaseLib
> + PciCf8Lib
> + SerialPortLib
> + FspSwitchStackLib
> + FspCommonLib
> + FspSecPlatformLib
> +
> +
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> new file mode 100644
> index 0000000000..e9365d6832
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> @@ -0,0 +1,44 @@
> +;; @file
> +; Provide FSP API entry points.
> +;
> +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;;
> +
> + SECTION .text
> +
> +;
> +; Following functions will be provided in C
> +;
> +extern ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; FspApiCommonContinue API
> +;
> +; This is the FSP API common entry point to resume the FSP execution
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspApiCommonContinue)
> +ASM_PFX(FspApiCommonContinue):
> + jmp $
> +
> +;----------------------------------------------------------------------------
> +; FspSmmInit API
> +;
> +; This FSP API will notify the FSP about the different phases in the boot
> +; process
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspSmmInitApi)
> +ASM_PFX(FspSmmInitApi):
> + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
> + jmp ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; Module Entrypoint API
> +;----------------------------------------------------------------------------
> +global ASM_PFX(_ModuleEntryPoint)
> +ASM_PFX(_ModuleEntryPoint):
> + jmp $
> + ; Add reference to APIs so that it will not be optimized by compiler
> + jmp ASM_PFX(FspSmmInitApi)
> diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> new file mode 100644
> index 0000000000..e9365d6832
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> @@ -0,0 +1,44 @@
> +;; @file
> +; Provide FSP API entry points.
> +;
> +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;;
> +
> + SECTION .text
> +
> +;
> +; Following functions will be provided in C
> +;
> +extern ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; FspApiCommonContinue API
> +;
> +; This is the FSP API common entry point to resume the FSP execution
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspApiCommonContinue)
> +ASM_PFX(FspApiCommonContinue):
> + jmp $
> +
> +;----------------------------------------------------------------------------
> +; FspSmmInit API
> +;
> +; This FSP API will notify the FSP about the different phases in the boot
> +; process
> +;
> +;----------------------------------------------------------------------------
> +global ASM_PFX(FspSmmInitApi)
> +ASM_PFX(FspSmmInitApi):
> + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
This is a bug. It should be:
mov rax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
Hi Nate,
This is not actually a problem, a 32-bit mov to a 32-bit register in long mode will zero the upper part.
Note that IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm compares ApiIndex numbers using rax, so it is important to make sure that the upper 32-bits are zero:
https://github.com/tianocore/edk2/blob/c966204049f3d5dae6d5e587ddc298c684142c5c/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm#L65
> + jmp ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------------
> +; Module Entrypoint API
> +;----------------------------------------------------------------------------
> +global ASM_PFX(_ModuleEntryPoint)
> +ASM_PFX(_ModuleEntryPoint):
> + jmp $
> + ; Add reference to APIs so that it will not be optimized by compiler
> + jmp ASM_PFX(FspSmmInitApi)
> diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> index b36bc2b9ae..1d6c2fb63d 100644
> --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> @@ -135,18 +135,18 @@ typedef struct {
> ///
> /// Revision of the structure is 2 for this version of the specification.
> ///
> - UINT8 Revision;
> - UINT8 Reserved[3];
> + UINT8 Revision;
> + UINT8 Reserved[3];
Any reason for these whitespace changes? It looks worse than before.
> ///
> /// Length of the structure in bytes. The current value for this field is 32.
> ///
> - UINT32 Length;
> + UINT32 Length;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// FspDebugHandler Optional debug handler for the bootloader to receive debug messages
> /// occurring during FSP execution.
> ///
> - EFI_PHYSICAL_ADDRESS FspDebugHandler;
> - UINT8 Reserved1[16];
> + EFI_PHYSICAL_ADDRESS FspDebugHandler;
> + UINT8 Reserved1[16];
Any reason for these whitespace changes? It looks worse than before.
> } FSPT_ARCH2_UPD;
>
> ///
> @@ -197,37 +197,37 @@ typedef struct {
> ///
> /// Revision of the structure is 3 for this version of the specification.
> ///
> - UINT8 Revision;
> - UINT8 Reserved[3];
> + UINT8 Revision;
> + UINT8 Reserved[3];
Any reason for these whitespace changes? It looks worse than before.
> ///
> /// Length of the structure in bytes. The current value for this field is 64.
> ///
> - UINT32 Length;
> + UINT32 Length;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Pointer to the temporary stack base address to be
> /// consumed inside FspMemoryInit() API.
> ///
> - EFI_PHYSICAL_ADDRESS StackBase;
> + EFI_PHYSICAL_ADDRESS StackBase;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Temporary stack size to be consumed inside
> /// FspMemoryInit() API.
> ///
> - UINT64 StackSize;
> + UINT64 StackSize;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Size of memory to be reserved by FSP below "top
> /// of low usable memory" for bootloader usage.
> ///
> - UINT32 BootLoaderTolumSize;
> + UINT32 BootLoaderTolumSize;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Current boot mode.
> ///
> - UINT32 BootMode;
> + UINT32 BootMode;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// Optional event handler for the bootloader to be informed of events occurring during FSP execution.
> /// This value is only valid if Revision is >= 2.
> ///
> - EFI_PHYSICAL_ADDRESS FspEventHandler;
> - UINT8 Reserved1[24];
> + EFI_PHYSICAL_ADDRESS FspEventHandler;
> + UINT8 Reserved1[24];
Any reason for these whitespace changes? It looks worse than before.
> } FSPM_ARCH2_UPD;
>
> ///
> @@ -266,18 +266,18 @@ typedef struct {
> ///
> /// Revision of the structure is 2 for this version of the specification.
> ///
> - UINT8 Revision;
> - UINT8 Reserved[3];
> + UINT8 Revision;
> + UINT8 Reserved[3];
Any reason for these whitespace changes? It looks worse than before.
> ///
> /// Length of the structure in bytes. The current value for this field is 32.
> ///
> - UINT32 Length;
> + UINT32 Length;
Any reason for this whitespace change? It looks worse than before.
> ///
> /// FspEventHandler Optional event handler for the bootloader to be informed of events
> /// occurring during FSP execution.
> ///
> - EFI_PHYSICAL_ADDRESS FspEventHandler;
> - UINT8 Reserved1[16];
> + EFI_PHYSICAL_ADDRESS FspEventHandler;
> + UINT8 Reserved1[16];
Any reason for these whitespace changes? It looks worse than before.
> } FSPS_ARCH2_UPD;
>
> ///
> @@ -609,4 +609,23 @@ EFI_STATUS
> IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
> );
>
> +/**
> + This FSP API initializes SMM and provide any OS runtime silicon services,
> + including Reliability, Availability, and Serviceability (RAS) features implemented by the CPU.
> +
> + @param[in] FspiUpdDataPtr Pointer to the FSPI_UPD data structure.
> + If NULL, FSP will use the default parameters.
> +
> + @retval EFI_SUCCESS FSP execution environment was initialized successfully.
> + @retval EFI_INVALID_PARAMETER Input parameters are invalid.
> + @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
> + @retval EFI_DEVICE_ERROR FSP initialization failed.
> + @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status codes will not be returned during S3.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *FSP_SMM_INIT)(
> + IN VOID *FspiUpdDataPtr
> + );
> +
> #endif
> diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h b/IntelFsp2Pkg/Include/FspGlobalData.h
> index 445540abfa..697b20ed4c 100644
> --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> @@ -10,9 +10,9 @@
>
> #include <FspEas.h>
>
> -#define FSP_IN_API_MODE 0
> -#define FSP_IN_DISPATCH_MODE 1
> -#define FSP_GLOBAL_DATA_VERSION 1
> +#define FSP_IN_API_MODE 0
> +#define FSP_IN_DISPATCH_MODE 1
> +#define FSP_GLOBAL_DATA_VERSION 1
>
> #pragma pack(1)
>
> @@ -24,16 +24,17 @@ typedef enum {
> TempRamExitApiIndex,
> FspSiliconInitApiIndex,
> FspMultiPhaseSiInitApiIndex,
> + FspSmmInitApiIndex,
> FspApiIndexMax
> } FSP_API_INDEX;
>
> typedef struct {
> - VOID *DataPtr;
> - UINTN MicrocodeRegionBase;
> - UINTN MicrocodeRegionSize;
> - UINTN CodeRegionBase;
> - UINTN CodeRegionSize;
> - UINTN Reserved;
> + VOID *DataPtr;
> + UINTN MicrocodeRegionBase;
> + UINTN MicrocodeRegionSize;
> + UINTN CodeRegionBase;
> + UINTN CodeRegionSize;
> + UINTN Reserved;
> } FSP_PLAT_DATA;
>
> #define FSP_GLOBAL_DATA_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'D')
> @@ -41,28 +42,28 @@ typedef struct {
> #define FSP_PERFORMANCE_DATA_TIMER_MASK 0xFFFFFFFFFFFFFF
>
> typedef struct {
> - UINT32 Signature;
> - UINT8 Version;
> - UINT8 Reserved1[3];
> + UINT32 Signature;
> + UINT8 Version;
> + UINT8 Reserved1[3];
> ///
> /// Offset 0x08
> ///
> - UINTN CoreStack;
> - UINTN Reserved2;
> + UINTN CoreStack;
> + UINTN Reserved2;
> ///
> /// IA32: Offset 0x10; X64: Offset 0x18
> ///
> - UINT32 StatusCode;
> - UINT8 ApiIdx;
> + UINT32 StatusCode;
> + UINT8 ApiIdx;
> ///
> /// 0: FSP in API mode; 1: FSP in DISPATCH mode
> ///
> - UINT8 FspMode;
> - UINT8 OnSeparateStack;
> - UINT8 Reserved3;
> - UINT32 NumberOfPhases;
> - UINT32 PhasesExecuted;
> - UINT32 Reserved4[8];
> + UINT8 FspMode;
> + UINT8 OnSeparateStack;
> + UINT8 Reserved3;
> + UINT32 NumberOfPhases;
> + UINT32 PhasesExecuted;
> + UINT32 Reserved4[8];
> ///
> /// IA32: Offset 0x40; X64: Offset 0x48
> /// Start of UINTN and pointer section
> @@ -75,21 +76,23 @@ typedef struct {
> VOID *TempRamInitUpdPtr;
> VOID *MemoryInitUpdPtr;
> VOID *SiliconInitUpdPtr;
> + VOID *SmmInitUpdPtr;
> ///
> - /// IA32: Offset 0x64; X64: Offset 0x90
> + /// IA32: Offset 0x68; X64: Offset 0x98
> /// To store function parameters pointer
> /// so it can be retrieved after stack switched.
> ///
> VOID *FunctionParameterPtr;
> FSP_INFO_HEADER *FspInfoHeader;
> VOID *UpdDataPtr;
> + UINTN Reserved5;
> ///
> /// End of UINTN and pointer section
> ///
> - UINT8 Reserved5[16];
> + UINT8 Reserved6[16];
> UINT32 PerfSig;
> UINT16 PerfLen;
> - UINT16 Reserved6;
> + UINT16 Reserved7;
> UINT32 PerfIdx;
> UINT64 PerfData[32];
> } FSP_GLOBAL_DATA;
> diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> index c660defac3..c7fb63168f 100644
> --- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> +++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> @@ -26,13 +26,13 @@
>
> #define FSP_INFO_HEADER_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'H')
>
> -#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> -#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> -#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> -#define FSP_IA32 0
> -#define FSP_X64 1
> +#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> +#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> +#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> +#define FSP_IA32 0
> +#define FSP_X64 1
>
> -#pragma pack(1)
> + #pragma pack(1)
Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style guidelines don't require that and it certainly looks worse than before.
>
> ///
> /// FSP Information Header as described in FSP v2.0 Spec section 5.1.1.
> @@ -159,6 +159,14 @@ typedef struct {
> /// Byte 0x4E: Reserved4.
> ///
> UINT16 Reserved4;
> + ///
> + /// Byte 0x50: Offset for the API for the Multi-Phase memory initialization.
> + ///
> + UINT32 FspMultiPhaseMemInitEntryOffset;
> + ///
> + /// Byte 0x54: Offset for the API to initialize SMM.
> + ///
> + UINT32 FspSmmInitEntryOffset;
> } FSP_INFO_HEADER;
>
> ///
> @@ -240,7 +248,7 @@ typedef struct {
> // UINT32 PatchData[];
> } FSP_PATCH_TABLE;
>
> -#pragma pack()
> + #pragma pack()
Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style guidelines don't require that and it certainly looks worse than before.
>
> extern EFI_GUID gFspHeaderFileGuid;
>
> diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> index 7cf7e88245..b2d7867880 100644
> --- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> @@ -68,6 +68,7 @@
> IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
> + IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
>
> diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> index c4fb1f1bb2..128b896592 100644
> --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> @@ -953,8 +953,8 @@ EndList
> return NoFileChange
>
> def CreateSplitUpdTxt (self, UpdTxtFile):
> - GuidList = ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID']
> - SignatureList = ['0x545F', '0x4D5F','0x535F'] # _T, _M, and _S signature for FSPT, FSPM, FSPS
> + GuidList = ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID','FSP_I_UPD_TOOL_GUID']
> + SignatureList = ['0x545F', '0x4D5F','0x535F','0x495F'] # _T, _M, _S and _I signature for FSPT, FSPM, FSPS, FSPI
> for Index in range(len(GuidList)):
> UpdTxtFile = ''
> FvDir = self._FvDir
> @@ -1288,19 +1288,21 @@ EndList
> Chars.append(chr(Value & 0xFF))
> Value = Value >> 8
> SignatureStr = ''.join(Chars)
> - # Signature will be _T / _M / _S for FSPT / FSPM / FSPS accordingly
> + # Signature will be _T / _M / _S / _I for FSPT / FSPM / FSPS /FSPI accordingly
> if '_T' in SignatureStr[6:6+2]:
> TxtBody.append("#define FSPT_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> elif '_M' in SignatureStr[6:6+2]:
> TxtBody.append("#define FSPM_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> elif '_S' in SignatureStr[6:6+2]:
> TxtBody.append("#define FSPS_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> + elif '_I' in SignatureStr[6:6+2]:
> + TxtBody.append("#define FSPI_UPD_SIGNATURE %s /* '%s' */\n\n" % (Item['value'], SignatureStr))
> TxtBody.append("\n")
>
> for Region in ['UPD']:
> UpdOffsetTable = []
> - UpdSignature = ['0x545F', '0x4D5F', '0x535F'] #['_T', '_M', '_S'] signature for FSPT, FSPM, FSPS
> - UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD']
> + UpdSignature = ['0x545F', '0x4D5F', '0x535F', '0x495F'] #['_T', '_M', '_S', '_I'] signature for FSPT, FSPM, FSPS, FSPI
> + UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD', 'FSPI_UPD']
> for Item in self._CfgItemList:
> if Item["cname"] == 'Signature' and Item["value"][0:6] in UpdSignature:
> Item["offset"] = 0 # re-initialize offset to 0 when new UPD structure starting
> @@ -1393,11 +1395,12 @@ EndList
> HeaderTFileName = 'FsptUpd.h'
> HeaderMFileName = 'FspmUpd.h'
> HeaderSFileName = 'FspsUpd.h'
> + HeaderIFileName = 'FspiUpd.h'
>
> - UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS'] # FSPX_UPD_REGION
> - UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S'] # FSP_X_CONFIG, FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> - UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE']
> - ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD', 'FSPS_ARCH_UPD']
> + UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS', 'FSPI'] # FSPX_UPD_REGION
> + UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S', 'FSP_I'] # FSP_X_CONFIG, FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> + UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE', 'FSPI_UPD_SIGNATURE']
> + ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD', 'FSPS_ARCH_UPD', 'FSPI_ARCH_UPD']
> ExcludedSpecificUpd1 = ['FSPT_ARCH2_UPD', 'FSPM_ARCH2_UPD', 'FSPS_ARCH2_UPD']
>
> IncLines = []
> @@ -1420,6 +1423,9 @@ EndList
> elif UpdRegionCheck[item] == 'FSPS':
> HeaderFd = open(os.path.join(FvDir, HeaderSFileName), "w")
> FileBase = os.path.basename(os.path.join(FvDir, HeaderSFileName))
> + elif UpdRegionCheck[item] == 'FSPI':
> + HeaderFd = open(os.path.join(FvDir, HeaderIFileName), "w")
> + FileBase = os.path.basename(os.path.join(FvDir, HeaderIFileName))
> FileName = FileBase.replace(".", "_").upper()
> HeaderFd.write("%s\n" % (__copyright_h__ % date.today().year))
> HeaderFd.write("#ifndef __%s__\n" % FileName)
> @@ -1696,7 +1702,7 @@ EndList
>
>
> def Usage():
> - print ("GenCfgOpt Version 0.57")
> + print ("GenCfgOpt Version 0.58")
> print ("Usage:")
> print (" GenCfgOpt UPDTXT PlatformDscFile BuildFvDir [-D Macros]")
> print (" GenCfgOpt HEADER PlatformDscFile BuildFvDir InputHFile [-D Macros]")
> diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py b/IntelFsp2Pkg/Tools/SplitFspBin.py
> index f9151b5afd..317d9c1fa0 100644
> --- a/IntelFsp2Pkg/Tools/SplitFspBin.py
> +++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
> @@ -1,6 +1,6 @@
> ## @ SplitFspBin.py
> #
> -# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -492,7 +492,7 @@ class FspImage:
> self.FihOffset = fihoff
> self.Offset = offset
> self.FvIdxList = []
> - self.Type = "XTMSXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) & 0x0F]
> + self.Type = "XTMSIXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) & 0x0F]
> self.PatchList = patch
> self.PatchList.append(fihoff + 0x1C)
>
> @@ -869,7 +869,7 @@ def main ():
> parser_rebase = subparsers.add_parser('rebase', help='rebase a FSP into a new base address')
> parser_rebase.set_defaults(which='rebase')
> parser_rebase.add_argument('-f', '--fspbin' , dest='FspBinary', type=str, help='FSP binary file path', required = True)
> - parser_rebase.add_argument('-c', '--fspcomp', choices=['t','m','s','o'], nargs='+', dest='FspComponent', type=str, help='FSP component to rebase', default = "['t']", required = True)
> + parser_rebase.add_argument('-c', '--fspcomp', choices=['t','m','s','o','i'], nargs='+', dest='FspComponent', type=str, help='FSP component to rebase', default = "['t']", required = True)
> parser_rebase.add_argument('-b', '--newbase', dest='FspBase', nargs='+', type=str, help='Rebased FSP binary file name', default = '', required = True)
> parser_rebase.add_argument('-o', '--outdir' , dest='OutputDir', type=str, help='Output directory path', default = '.')
> parser_rebase.add_argument('-n', '--outfile', dest='OutputFile', type=str, help='Rebased FSP binary file name', default = '')
> --
> 2.35.0.windows.1
--
Pedro Falcato
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91494): https://edk2.groups.io/g/devel/message/91494
Mute This Topic: https://groups.io/mt/92387601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi,
I have sent V2 to assign ApiIdx to RAX in X64 case to avoid confusion.
Please help to review again,
Thanks,
Chasel
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Monday, July 18, 2022 4:41 PM
> To: Pedro Falcato <pedro.falcato@gmail.com>; edk2-devel-groups-io
> <devel@edk2.groups.io>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Zhang, Hongbin1
> <hongbin1.zhang@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2-devel] [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for
> SMM support.
>
> Hi Pedro,
>
> Wow you are totally right... that is crazy! The exact opposite of the way 32-bit
> mode works!
>
> Good catch thank you!
>
> Please consider by code review feedback downgraded from a bug to a
> recommendation to use rax instead of eax to avoid confusion 😊.
>
> Thanks,
> Nate
>
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Monday, July 18, 2022 2:50 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Zhang, Hongbin1
> <hongbin1.zhang@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2-devel] [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for
> SMM support.
>
>
>
> On Mon, Jul 18, 2022 at 10:40 PM Nate DeSimone
> <mailto:nathaniel.l.desimone@intel.com> wrote:
> Hi Chasel,
>
> Please see comments inline. Here is a summary of my feedback:
>
> #1) IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm - line 34 - Bug: eax should
> be rax
> #2) IntelFsp2Pkg/Include/FspEas/FspApi.h - Various unnecessary whitespace
> changes that make the file look worse than before.
> #3) IntelFsp2Pkg/Include/Guid/FspHeaderFile.h - Why indent the #pragma lines?
>
> Thanks,
> Nate
>
> > -----Original Message-----
> > From: Chiu, Chasel <mailto:chasel.chiu@intel.com>
> > Sent: Thursday, July 14, 2022 1:04 PM
> > To: mailto:devel@edk2.groups.io
> > Cc: Zhang, Hongbin1 <mailto:hongbin1.zhang@intel.com>; Desimone,
> Nathaniel L
> > <mailto:nathaniel.l.desimone@intel.com>; Zeng, Star
> <mailto:star.zeng@intel.com>; Chiu,
> > Chasel <mailto:chasel.chiu@intel.com>
> > Subject: [PATCH] IntelFsp2Pkg/FspSecCore: Add FSP-I API for SMM support.
> >
> > From: Hongbin1 Zhang <mailto:hongbin1.zhang@intel.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> > Add FSP-I API entry point for SMM support.
> >
> > Cc: Nate DeSimone <mailto:nathaniel.l.desimone@intel.com>
> > Cc: Star Zeng <mailto:star.zeng@intel.com>
> > Signed-off-by: Chasel Chiu <mailto:chasel.chiu@intel.com>
> > Signed-off-by: Hongbin1 Zhang <mailto:hongbin1.zhang@intel.com>
> > ---
> > IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 13 +++++++++++++
> > IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf | 54
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> > IntelFsp2Pkg/Include/FspEas/FspApi.h | 57
> ++++++++++++++++++++++++++++++++++++++-------------------
> > IntelFsp2Pkg/Include/FspGlobalData.h | 53
> ++++++++++++++++++++++++++++-------------------------
> > IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 22 +++++++++++++++-------
> > IntelFsp2Pkg/IntelFsp2Pkg.dsc | 1 +
> > IntelFsp2Pkg/Tools/GenCfgOpt.py | 26 ++++++++++++++++----------
> > IntelFsp2Pkg/Tools/SplitFspBin.py | 6 +++---
> > 10 files changed, 256 insertions(+), 64 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > index e22a88cc84..35d223a404 100644
> > --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> > @@ -71,6 +71,19 @@ FspApiCallingCheck (
> > Status = EFI_INVALID_PARAMETER;
> > }
> > }
> > + } else if (ApiIdx == FspSmmInitApiIndex) {
> > + //
> > + // FspSmmInitApiIndex check
> > + //
> > + if ((FspData == NULL) || ((UINTN)FspData == MAX_ADDRESS) ||
> ((UINTN)FspData == MAX_UINT32)) {
> > + Status = EFI_UNSUPPORTED;
> > + } else {
> > + if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
> > + Status = EFI_UNSUPPORTED;
> > + } else if (EFI_ERROR (FspUpdSignatureCheck (FspSmmInitApiIndex,
> ApiParam))) {
> > + Status = EFI_INVALID_PARAMETER;
> > + }
> > + }
> > } else {
> > Status = EFI_UNSUPPORTED;
> > }
> > diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > new file mode 100644
> > index 0000000000..d31576c00b
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > @@ -0,0 +1,54 @@
> > +## @file
> > +# Sec Core for FSP
> > +#
> > +# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +[Defines]
> > + INF_VERSION = 0x00010005
> > + BASE_NAME = FspSecCoreI
> > + FILE_GUID = 558782b5-782d-415e-ab9e-0ceb79dc3425
> > + MODULE_TYPE = SEC
> > + VERSION_STRING = 1.0
> > +
> > +#
> > +# The following information is for reference only and not required by the
> build tools.
> > +#
> > +# VALID_ARCHITECTURES = IA32 X64
> > +#
> > +
> > +[Sources]
> > + SecFspApiChk.c
> > + SecFsp.h
> > +
> > +[Sources.X64]
> > + X64/FspApiEntryI.nasm
> > + X64/FspApiEntryCommon.nasm
> > + X64/FspHelper.nasm
> > +
> > +[Sources.IA32]
> > + Ia32/FspApiEntryI.nasm
> > + Ia32/FspApiEntryCommon.nasm
> > + Ia32/FspHelper.nasm
> > +
> > +[Binaries.Ia32]
> > + RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + IntelFsp2Pkg/IntelFsp2Pkg.dec
> > +
> > +[LibraryClasses]
> > + BaseMemoryLib
> > + DebugLib
> > + BaseLib
> > + PciCf8Lib
> > + SerialPortLib
> > + FspSwitchStackLib
> > + FspCommonLib
> > + FspSecPlatformLib
> > +
> > +
> > diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> > new file mode 100644
> > index 0000000000..e9365d6832
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryI.nasm
> > @@ -0,0 +1,44 @@
> > +;; @file
> > +; Provide FSP API entry points.
> > +;
> > +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > +; SPDX-License-Identifier: BSD-2-Clause-Patent
> > +;;
> > +
> > + SECTION .text
> > +
> > +;
> > +; Following functions will be provided in C
> > +;
> > +extern ASM_PFX(FspApiCommon)
> > +
> > +;----------------------------------------------------------------------------
> > +; FspApiCommonContinue API
> > +;
> > +; This is the FSP API common entry point to resume the FSP execution
> > +;
> > +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspApiCommonContinue)
> > +ASM_PFX(FspApiCommonContinue):
> > + jmp $
> > +
> > +;----------------------------------------------------------------------------
> > +; FspSmmInit API
> > +;
> > +; This FSP API will notify the FSP about the different phases in the boot
> > +; process
> > +;
> > +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspSmmInitApi)
> > +ASM_PFX(FspSmmInitApi):
> > + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
> > + jmp ASM_PFX(FspApiCommon)
> > +
> > +;----------------------------------------------------------------------------
> > +; Module Entrypoint API
> > +;----------------------------------------------------------------------------
> > +global ASM_PFX(_ModuleEntryPoint)
> > +ASM_PFX(_ModuleEntryPoint):
> > + jmp $
> > + ; Add reference to APIs so that it will not be optimized by compiler
> > + jmp ASM_PFX(FspSmmInitApi)
> > diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> > new file mode 100644
> > index 0000000000..e9365d6832
> > --- /dev/null
> > +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryI.nasm
> > @@ -0,0 +1,44 @@
> > +;; @file
> > +; Provide FSP API entry points.
> > +;
> > +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > +; SPDX-License-Identifier: BSD-2-Clause-Patent
> > +;;
> > +
> > + SECTION .text
> > +
> > +;
> > +; Following functions will be provided in C
> > +;
> > +extern ASM_PFX(FspApiCommon)
> > +
> > +;----------------------------------------------------------------------------
> > +; FspApiCommonContinue API
> > +;
> > +; This is the FSP API common entry point to resume the FSP execution
> > +;
> > +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspApiCommonContinue)
> > +ASM_PFX(FspApiCommonContinue):
> > + jmp $
> > +
> > +;----------------------------------------------------------------------------
> > +; FspSmmInit API
> > +;
> > +; This FSP API will notify the FSP about the different phases in the boot
> > +; process
> > +;
> > +;----------------------------------------------------------------------------
> > +global ASM_PFX(FspSmmInitApi)
> > +ASM_PFX(FspSmmInitApi):
> > + mov eax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
>
> This is a bug. It should be:
>
> mov rax, 7 ; FSP_API_INDEX.FspSmmInitApiIndex
>
> Hi Nate,
>
> This is not actually a problem, a 32-bit mov to a 32-bit register in long mode will
> zero the upper part.
>
>
> Note that IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm compares
> ApiIndex numbers using rax, so it is important to make sure that the upper 32-
> bits are zero:
>
> https://github.com/tianocore/edk2/blob/c966204049f3d5dae6d5e587ddc298c6
> 84142c5c/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm#L65
>
> > + jmp ASM_PFX(FspApiCommon)
> > +
> > +;----------------------------------------------------------------------------
> > +; Module Entrypoint API
> > +;----------------------------------------------------------------------------
> > +global ASM_PFX(_ModuleEntryPoint)
> > +ASM_PFX(_ModuleEntryPoint):
> > + jmp $
> > + ; Add reference to APIs so that it will not be optimized by compiler
> > + jmp ASM_PFX(FspSmmInitApi)
> > diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > index b36bc2b9ae..1d6c2fb63d 100644
> > --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > @@ -135,18 +135,18 @@ typedef struct {
> > ///
> > /// Revision of the structure is 2 for this version of the specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this field is 32.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// FspDebugHandler Optional debug handler for the bootloader to receive
> debug messages
> > /// occurring during FSP execution.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspDebugHandler;
> > - UINT8 Reserved1[16];
> > + EFI_PHYSICAL_ADDRESS FspDebugHandler;
> > + UINT8 Reserved1[16];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPT_ARCH2_UPD;
> >
> > ///
> > @@ -197,37 +197,37 @@ typedef struct {
> > ///
> > /// Revision of the structure is 3 for this version of the specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this field is 64.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Pointer to the temporary stack base address to be
> > /// consumed inside FspMemoryInit() API.
> > ///
> > - EFI_PHYSICAL_ADDRESS StackBase;
> > + EFI_PHYSICAL_ADDRESS StackBase;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Temporary stack size to be consumed inside
> > /// FspMemoryInit() API.
> > ///
> > - UINT64 StackSize;
> > + UINT64 StackSize;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Size of memory to be reserved by FSP below "top
> > /// of low usable memory" for bootloader usage.
> > ///
> > - UINT32 BootLoaderTolumSize;
> > + UINT32 BootLoaderTolumSize;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Current boot mode.
> > ///
> > - UINT32 BootMode;
> > + UINT32 BootMode;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// Optional event handler for the bootloader to be informed of events
> occurring during FSP execution.
> > /// This value is only valid if Revision is >= 2.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspEventHandler;
> > - UINT8 Reserved1[24];
> > + EFI_PHYSICAL_ADDRESS FspEventHandler;
> > + UINT8 Reserved1[24];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPM_ARCH2_UPD;
> >
> > ///
> > @@ -266,18 +266,18 @@ typedef struct {
> > ///
> > /// Revision of the structure is 2 for this version of the specification.
> > ///
> > - UINT8 Revision;
> > - UINT8 Reserved[3];
> > + UINT8 Revision;
> > + UINT8 Reserved[3];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > ///
> > /// Length of the structure in bytes. The current value for this field is 32.
> > ///
> > - UINT32 Length;
> > + UINT32 Length;
>
> Any reason for this whitespace change? It looks worse than before.
>
> > ///
> > /// FspEventHandler Optional event handler for the bootloader to be
> informed of events
> > /// occurring during FSP execution.
> > ///
> > - EFI_PHYSICAL_ADDRESS FspEventHandler;
> > - UINT8 Reserved1[16];
> > + EFI_PHYSICAL_ADDRESS FspEventHandler;
> > + UINT8 Reserved1[16];
>
> Any reason for these whitespace changes? It looks worse than before.
>
> > } FSPS_ARCH2_UPD;
> >
> > ///
> > @@ -609,4 +609,23 @@ EFI_STATUS
> > IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
> > );
> >
> > +/**
> > + This FSP API initializes SMM and provide any OS runtime silicon services,
> > + including Reliability, Availability, and Serviceability (RAS) features
> implemented by the CPU.
> > +
> > + @param[in] FspiUpdDataPtr Pointer to the FSPI_UPD data structure.
> > + If NULL, FSP will use the default parameters.
> > +
> > + @retval EFI_SUCCESS FSP execution environment was initialized
> successfully.
> > + @retval EFI_INVALID_PARAMETER Input parameters are invalid.
> > + @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
> > + @retval EFI_DEVICE_ERROR FSP initialization failed.
> > + @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status
> codes will not be returned during S3.
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *FSP_SMM_INIT)(
> > + IN VOID *FspiUpdDataPtr
> > + );
> > +
> > #endif
> > diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
> b/IntelFsp2Pkg/Include/FspGlobalData.h
> > index 445540abfa..697b20ed4c 100644
> > --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> > +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> > @@ -10,9 +10,9 @@
> >
> > #include <FspEas.h>
> >
> > -#define FSP_IN_API_MODE 0
> > -#define FSP_IN_DISPATCH_MODE 1
> > -#define FSP_GLOBAL_DATA_VERSION 1
> > +#define FSP_IN_API_MODE 0
> > +#define FSP_IN_DISPATCH_MODE 1
> > +#define FSP_GLOBAL_DATA_VERSION 1
> >
> > #pragma pack(1)
> >
> > @@ -24,16 +24,17 @@ typedef enum {
> > TempRamExitApiIndex,
> > FspSiliconInitApiIndex,
> > FspMultiPhaseSiInitApiIndex,
> > + FspSmmInitApiIndex,
> > FspApiIndexMax
> > } FSP_API_INDEX;
> >
> > typedef struct {
> > - VOID *DataPtr;
> > - UINTN MicrocodeRegionBase;
> > - UINTN MicrocodeRegionSize;
> > - UINTN CodeRegionBase;
> > - UINTN CodeRegionSize;
> > - UINTN Reserved;
> > + VOID *DataPtr;
> > + UINTN MicrocodeRegionBase;
> > + UINTN MicrocodeRegionSize;
> > + UINTN CodeRegionBase;
> > + UINTN CodeRegionSize;
> > + UINTN Reserved;
> > } FSP_PLAT_DATA;
> >
> > #define FSP_GLOBAL_DATA_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'D')
> > @@ -41,28 +42,28 @@ typedef struct {
> > #define FSP_PERFORMANCE_DATA_TIMER_MASK 0xFFFFFFFFFFFFFF
> >
> > typedef struct {
> > - UINT32 Signature;
> > - UINT8 Version;
> > - UINT8 Reserved1[3];
> > + UINT32 Signature;
> > + UINT8 Version;
> > + UINT8 Reserved1[3];
> > ///
> > /// Offset 0x08
> > ///
> > - UINTN CoreStack;
> > - UINTN Reserved2;
> > + UINTN CoreStack;
> > + UINTN Reserved2;
> > ///
> > /// IA32: Offset 0x10; X64: Offset 0x18
> > ///
> > - UINT32 StatusCode;
> > - UINT8 ApiIdx;
> > + UINT32 StatusCode;
> > + UINT8 ApiIdx;
> > ///
> > /// 0: FSP in API mode; 1: FSP in DISPATCH mode
> > ///
> > - UINT8 FspMode;
> > - UINT8 OnSeparateStack;
> > - UINT8 Reserved3;
> > - UINT32 NumberOfPhases;
> > - UINT32 PhasesExecuted;
> > - UINT32 Reserved4[8];
> > + UINT8 FspMode;
> > + UINT8 OnSeparateStack;
> > + UINT8 Reserved3;
> > + UINT32 NumberOfPhases;
> > + UINT32 PhasesExecuted;
> > + UINT32 Reserved4[8];
> > ///
> > /// IA32: Offset 0x40; X64: Offset 0x48
> > /// Start of UINTN and pointer section
> > @@ -75,21 +76,23 @@ typedef struct {
> > VOID *TempRamInitUpdPtr;
> > VOID *MemoryInitUpdPtr;
> > VOID *SiliconInitUpdPtr;
> > + VOID *SmmInitUpdPtr;
> > ///
> > - /// IA32: Offset 0x64; X64: Offset 0x90
> > + /// IA32: Offset 0x68; X64: Offset 0x98
> > /// To store function parameters pointer
> > /// so it can be retrieved after stack switched.
> > ///
> > VOID *FunctionParameterPtr;
> > FSP_INFO_HEADER *FspInfoHeader;
> > VOID *UpdDataPtr;
> > + UINTN Reserved5;
> > ///
> > /// End of UINTN and pointer section
> > ///
> > - UINT8 Reserved5[16];
> > + UINT8 Reserved6[16];
> > UINT32 PerfSig;
> > UINT16 PerfLen;
> > - UINT16 Reserved6;
> > + UINT16 Reserved7;
> > UINT32 PerfIdx;
> > UINT64 PerfData[32];
> > } FSP_GLOBAL_DATA;
> > diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > index c660defac3..c7fb63168f 100644
> > --- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > +++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> > @@ -26,13 +26,13 @@
> >
> > #define FSP_INFO_HEADER_SIGNATURE SIGNATURE_32 ('F', 'S', 'P', 'H')
> >
> > -#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> > -#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> > -#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> > -#define FSP_IA32 0
> > -#define FSP_X64 1
> > +#define IMAGE_ATTRIBUTE_GRAPHICS_SUPPORT BIT0
> > +#define IMAGE_ATTRIBUTE_DISPATCH_MODE_SUPPORT BIT1
> > +#define IMAGE_ATTRIBUTE_64BIT_MODE_SUPPORT BIT2
> > +#define FSP_IA32 0
> > +#define FSP_X64 1
> >
> > -#pragma pack(1)
> > + #pragma pack(1)
>
> Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style
> guidelines don't require that and it certainly looks worse than before.
>
> >
> > ///
> > /// FSP Information Header as described in FSP v2.0 Spec section 5.1.1.
> > @@ -159,6 +159,14 @@ typedef struct {
> > /// Byte 0x4E: Reserved4.
> > ///
> > UINT16 Reserved4;
> > + ///
> > + /// Byte 0x50: Offset for the API for the Multi-Phase memory initialization.
> > + ///
> > + UINT32 FspMultiPhaseMemInitEntryOffset;
> > + ///
> > + /// Byte 0x54: Offset for the API to initialize SMM.
> > + ///
> > + UINT32 FspSmmInitEntryOffset;
> > } FSP_INFO_HEADER;
> >
> > ///
> > @@ -240,7 +248,7 @@ typedef struct {
> > // UINT32 PatchData[];
> > } FSP_PATCH_TABLE;
> >
> > -#pragma pack()
> > + #pragma pack()
>
> Why add an indent to the #pragma line? I'm pretty sure that edk2 coding style
> guidelines don't require that and it certainly looks worse than before.
>
> >
> > extern EFI_GUID gFspHeaderFileGuid;
> >
> > diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > index 7cf7e88245..b2d7867880 100644
> > --- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> > @@ -68,6 +68,7 @@
> > IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> > IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> > IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
> > + IntelFsp2Pkg/FspSecCore/FspSecCoreI.inf
> > IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> > IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
> >
> > diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > index c4fb1f1bb2..128b896592 100644
> > --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > @@ -953,8 +953,8 @@ EndList
> > return NoFileChange
> >
> > def CreateSplitUpdTxt (self, UpdTxtFile):
> > - GuidList =
> ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID
> ']
> > - SignatureList = ['0x545F', '0x4D5F','0x535F'] # _T, _M, and _S
> signature for FSPT, FSPM, FSPS
> > + GuidList =
> ['FSP_T_UPD_TOOL_GUID','FSP_M_UPD_TOOL_GUID','FSP_S_UPD_TOOL_GUID
> ','FSP_I_UPD_TOOL_GUID']
> > + SignatureList = ['0x545F', '0x4D5F','0x535F','0x495F'] # _T, _M, _S
> and _I signature for FSPT, FSPM, FSPS, FSPI
> > for Index in range(len(GuidList)):
> > UpdTxtFile = ''
> > FvDir = self._FvDir
> > @@ -1288,19 +1288,21 @@ EndList
> > Chars.append(chr(Value & 0xFF))
> > Value = Value >> 8
> > SignatureStr = ''.join(Chars)
> > - # Signature will be _T / _M / _S for FSPT / FSPM / FSPS accordingly
> > + # Signature will be _T / _M / _S / _I for FSPT / FSPM / FSPS /FSPI
> accordingly
> > if '_T' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPT_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > elif '_M' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPM_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > elif '_S' in SignatureStr[6:6+2]:
> > TxtBody.append("#define FSPS_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > + elif '_I' in SignatureStr[6:6+2]:
> > + TxtBody.append("#define FSPI_UPD_SIGNATURE %s /*
> '%s' */\n\n" % (Item['value'], SignatureStr))
> > TxtBody.append("\n")
> >
> > for Region in ['UPD']:
> > UpdOffsetTable = []
> > - UpdSignature = ['0x545F', '0x4D5F', '0x535F'] #['_T', '_M', '_S']
> signature for FSPT, FSPM, FSPS
> > - UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD']
> > + UpdSignature = ['0x545F', '0x4D5F', '0x535F', '0x495F'] #['_T', '_M',
> '_S', '_I'] signature for FSPT, FSPM, FSPS, FSPI
> > + UpdStructure = ['FSPT_UPD', 'FSPM_UPD', 'FSPS_UPD', 'FSPI_UPD']
> > for Item in self._CfgItemList:
> > if Item["cname"] == 'Signature' and Item["value"][0:6] in
> UpdSignature:
> > Item["offset"] = 0 # re-initialize offset to 0 when new UPD
> structure starting
> > @@ -1393,11 +1395,12 @@ EndList
> > HeaderTFileName = 'FsptUpd.h'
> > HeaderMFileName = 'FspmUpd.h'
> > HeaderSFileName = 'FspsUpd.h'
> > + HeaderIFileName = 'FspiUpd.h'
> >
> > - UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS'] # FSPX_UPD_REGION
> > - UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S'] # FSP_X_CONFIG,
> FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> > - UpdSignatureCheck = ['FSPT_UPD_SIGNATURE', 'FSPM_UPD_SIGNATURE',
> 'FSPS_UPD_SIGNATURE']
> > - ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD',
> 'FSPS_ARCH_UPD']
> > + UpdRegionCheck = ['FSPT', 'FSPM', 'FSPS', 'FSPI'] # FSPX_UPD_REGION
> > + UpdConfigCheck = ['FSP_T', 'FSP_M', 'FSP_S', 'FSP_I'] # FSP_X_CONFIG,
> FSP_X_TEST_CONFIG, FSP_X_RESTRICTED_CONFIG
> > + UpdSignatureCheck = ['FSPT_UPD_SIGNATURE',
> 'FSPM_UPD_SIGNATURE', 'FSPS_UPD_SIGNATURE', 'FSPI_UPD_SIGNATURE']
> > + ExcludedSpecificUpd = ['FSPT_ARCH_UPD', 'FSPM_ARCH_UPD',
> 'FSPS_ARCH_UPD', 'FSPI_ARCH_UPD']
> > ExcludedSpecificUpd1 = ['FSPT_ARCH2_UPD', 'FSPM_ARCH2_UPD',
> 'FSPS_ARCH2_UPD']
> >
> > IncLines = []
> > @@ -1420,6 +1423,9 @@ EndList
> > elif UpdRegionCheck[item] == 'FSPS':
> > HeaderFd = open(os.path.join(FvDir, HeaderSFileName), "w")
> > FileBase = os.path.basename(os.path.join(FvDir, HeaderSFileName))
> > + elif UpdRegionCheck[item] == 'FSPI':
> > + HeaderFd = open(os.path.join(FvDir, HeaderIFileName), "w")
> > + FileBase = os.path.basename(os.path.join(FvDir, HeaderIFileName))
> > FileName = FileBase.replace(".", "_").upper()
> > HeaderFd.write("%s\n" % (__copyright_h__ % date.today().year))
> > HeaderFd.write("#ifndef __%s__\n" % FileName)
> > @@ -1696,7 +1702,7 @@ EndList
> >
> >
> > def Usage():
> > - print ("GenCfgOpt Version 0.57")
> > + print ("GenCfgOpt Version 0.58")
> > print ("Usage:")
> > print (" GenCfgOpt UPDTXT PlatformDscFile BuildFvDir [-D
> Macros]")
> > print (" GenCfgOpt HEADER PlatformDscFile BuildFvDir InputHFile [-D
> Macros]")
> > diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py
> b/IntelFsp2Pkg/Tools/SplitFspBin.py
> > index f9151b5afd..317d9c1fa0 100644
> > --- a/IntelFsp2Pkg/Tools/SplitFspBin.py
> > +++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
> > @@ -1,6 +1,6 @@
> > ## @ SplitFspBin.py
> > #
> > -# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR>
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > ##
> > @@ -492,7 +492,7 @@ class FspImage:
> > self.FihOffset = fihoff
> > self.Offset = offset
> > self.FvIdxList = []
> > - self.Type = "XTMSXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) &
> 0x0F]
> > + self.Type = "XTMSIXXXXOXXXXXXX"[(fih.ComponentAttribute >> 12) &
> 0x0F]
> > self.PatchList = patch
> > self.PatchList.append(fihoff + 0x1C)
> >
> > @@ -869,7 +869,7 @@ def main ():
> > parser_rebase = subparsers.add_parser('rebase', help='rebase a FSP into a
> new base address')
> > parser_rebase.set_defaults(which='rebase')
> > parser_rebase.add_argument('-f', '--fspbin' , dest='FspBinary', type=str,
> help='FSP binary file path', required = True)
> > - parser_rebase.add_argument('-c', '--fspcomp',
> choices=['t','m','s','o'], nargs='+', dest='FspComponent', type=str, help='FSP
> component to rebase', default = "['t']", required = True)
> > + parser_rebase.add_argument('-c', '--fspcomp',
> choices=['t','m','s','o','i'], nargs='+', dest='FspComponent', type=str, help='FSP
> component to rebase', default = "['t']", required = True)
> > parser_rebase.add_argument('-b', '--newbase', dest='FspBase', nargs='+',
> type=str, help='Rebased FSP binary file name', default = '', required = True)
> > parser_rebase.add_argument('-o', '--outdir' , dest='OutputDir', type=str,
> help='Output directory path', default = '.')
> > parser_rebase.add_argument('-n', '--outfile', dest='OutputFile', type=str,
> help='Rebased FSP binary file name', default = '')
> > --
> > 2.35.0.windows.1
>
>
>
>
>
>
>
>
> --
> Pedro Falcato
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91545): https://edk2.groups.io/g/devel/message/91545
Mute This Topic: https://groups.io/mt/92387601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.