Re: [PATCH v2] IntelFsp2Pkg: FSPI_UPD is not mandatory.

["Zeng, Star" <star.zeng@intel.com>]

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com> 
Sent: Thursday, July 28, 2022 1:15 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH v2] IntelFsp2Pkg: FSPI_UPD is not mandatory.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
FSPI_UPD is required only When platforms implemented FSP_I component.
Updated the scripts to allow FSPI_UPD not present scenario.
Also fixed FSP_GLOBAL_DATA structure alignment issue and unnecessary
non-backward compatibility change in previous FSP_I patch.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 IntelFsp2Pkg/Include/FspGlobalData.h | 43 +++++++++++++++++++++----------------------
 IntelFsp2Pkg/Tools/GenCfgOpt.py      | 14 ++++++++++----
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h b/IntelFsp2Pkg/Include/FspGlobalData.h
index cf94f7b6a5..32c6d460e4 100644
--- a/IntelFsp2Pkg/Include/FspGlobalData.h
+++ b/IntelFsp2Pkg/Include/FspGlobalData.h
@@ -42,58 +42,57 @@ 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;

+  VOID               *SmmInitUpdPtr;

   ///

   /// 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              Reserved2;

+  UINT32             NumberOfPhases;

+  UINT32             PhasesExecuted;

+  UINT32             Reserved3[8];

   ///

   /// IA32: Offset 0x40; X64: Offset 0x48

   /// Start of UINTN and pointer section

-  /// All UINTN and pointer members must be put in this section

-  /// except CoreStack and Reserved2. In addition, the number of

-  /// UINTN and pointer members must be even for natural alignment

-  /// in both IA32 and X64.

+  /// All UINTN and pointer members are put in this section

+  /// for maintaining natural alignment for both IA32 and X64 builds.

   ///

   FSP_PLAT_DATA      PlatformData;

   VOID               *TempRamInitUpdPtr;

   VOID               *MemoryInitUpdPtr;

   VOID               *SiliconInitUpdPtr;

-  VOID               *SmmInitUpdPtr;

   ///

-  /// IA32: Offset 0x68; X64: Offset 0x98

+  /// IA32: Offset 0x64; X64: Offset 0x90

   /// 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

+  /// At this point, next field offset must be either *0h or *8h to

+  /// meet natural alignment requirement.

   ///

-  UINT8              Reserved6[16];

+  UINT8              Reserved4[16];

   UINT32             PerfSig;

   UINT16             PerfLen;

-  UINT16             Reserved7;

+  UINT16             Reserved5;

   UINT32             PerfIdx;

+  UINT32             Reserved6;

   UINT64             PerfData[32];

 } FSP_GLOBAL_DATA;

 

diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
index 128b896592..71c48f10e0 100644
--- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
+++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
@@ -959,8 +959,13 @@ EndList
             UpdTxtFile = ''

             FvDir = self._FvDir

             if GuidList[Index] not in self._MacroDict:

-                self.Error = "%s definition is missing in DSC file" % (GuidList[Index])

-                return 1

+                NoFSPI = False

+                if GuidList[Index] == 'FSP_I_UPD_TOOL_GUID':

+                    NoFSPI = True

+                    continue

+                else:

+                    self.Error = "%s definition is missing in DSC file" % (GuidList[Index])

+                    return 1

 

             if UpdTxtFile == '':

                 UpdTxtFile = os.path.join(FvDir, self._MacroDict[GuidList[Index]] + '.txt')

@@ -1296,7 +1301,8 @@ EndList
                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))

+                   if NoFSPI == True:

+                       TxtBody.append("#define FSPI_UPD_SIGNATURE               %s        /* '%s' */\n\n" % (Item['value'], SignatureStr))

         TxtBody.append("\n")

 

         for Region in ['UPD']:

@@ -1702,7 +1708,7 @@ EndList
 

 

 def Usage():

-    print ("GenCfgOpt Version 0.58")

+    print ("GenCfgOpt Version 0.59")

     print ("Usage:")

     print ("    GenCfgOpt  UPDTXT  PlatformDscFile BuildFvDir                 [-D Macros]")

     print ("    GenCfgOpt  HEADER  PlatformDscFile BuildFvDir  InputHFile     [-D Macros]")

-- 
2.35.0.windows.1