Re: [PATCH] IntelFsp2Pkg: Add FSPI_ARCH_UPD.

["Chiu, Chasel" <chasel.chiu@intel.com>]

Thanks Nate! I have sent V2 to add this comment, please review again.

> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Wednesday, July 20, 2022 1:52 PM
> To: Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH] IntelFsp2Pkg: Add FSPI_ARCH_UPD.
> 
> Hi Chasel,
> 
> Please see feedback inline.
> 
> Thanks,
> Nate
> 
> > -----Original Message-----
> > From: Chiu, Chasel <chasel.chiu@intel.com>
> > Sent: Wednesday, July 20, 2022 12:02 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] IntelFsp2Pkg: Add FSPI_ARCH_UPD.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> >
> > Adding the missing FSPI_ARCH_UPD, FSP_GLOBAL_DATA_VERSION bumpping
> up,
> > and some comments for clarification.
> >
> > 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/FspEas/FspApi.h | 69
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> -----
> >  IntelFsp2Pkg/Include/FspGlobalData.h |  2 +-
> >  2 files changed, 63 insertions(+), 8 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > index bf46f13f73..ad5d62c385 100644
> > --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> > @@ -1,6 +1,6 @@
> >  /** @file
> >    Intel FSP API definition from Intel Firmware Support Package
> > External
> > -  Architecture Specification v2.0 - v2.2
> > +  Architecture Specification v2.0 and above.
> >
> >    Copyright (c) 2014 - 2022, Intel Corporation. All rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent @@ -100,13 +100,14 @@
> > typedef struct {
> >    /// "XXXXXX_T" for FSP-T
> >    /// "XXXXXX_M" for FSP-M
> >    /// "XXXXXX_S" for FSP-S
> > +  /// "XXXXXX_I" for FSP-I
> >    /// Where XXXXXX is an unique signature
> >    ///
> >    UINT64    Signature;
> >    ///
> >    /// Revision of the Data structure.
> > -  ///   For FSP spec 2.0/2.1 value is 1.
> > -  ///   For FSP spec 2.2 value is 2.
> > +  ///   For FSP spec 2.0/2.1, this value is 1 and only FSPM_UPD having
> ARCH_UPD.
> > +  ///   For FSP spec 2.2 and above, this value is 2 and ARCH_UPD present in all
> UPD structures.
> >    ///
> >    UINT8     Revision;
> >    UINT8     Reserved[23];
> > @@ -134,7 +135,7 @@ typedef struct {
> >  } FSPT_ARCH_UPD;
> >
> >  ///
> > -/// FSPT_ARCH2_UPD Configuration.
> > +/// FSPT_ARCH2_UPD Configuration for FSP 2.4 and above.
> >  ///
> >  typedef struct {
> >    ///
> > @@ -196,7 +197,7 @@ typedef struct {
> >  } FSPM_ARCH_UPD;
> >
> >  ///
> > -/// FSPM_ARCH2_UPD Configuration.
> > +/// FSPM_ARCH2_UPD Configuration for FSP 2.4 and above.
> >  ///
> >  typedef struct {
> >    ///
> > @@ -209,6 +210,11 @@ typedef struct {
> >    ///
> >    UINT32                  Length;
> >    ///
> > +  /// Pointer to the non-volatile storage (NVS) data buffer.
> > +  /// If it is NULL it indicates the NVS data is not available.
> 
> Please add the following to this comment:
> 
> This value is deprecated starting with v2.4 of the FSP specification and will be
> removed in an upcoming version of the FSP specification.
> 
> > +  ///
> > +  EFI_PHYSICAL_ADDRESS    NvsBufferPtr;
> > +  ///
> >    /// Pointer to the temporary stack base address to be
> >    /// consumed inside FspMemoryInit() API.
> >    ///
> > @@ -232,7 +238,7 @@ typedef struct {
> >    /// This value is only valid if Revision is >= 2.
> >    ///
> >    EFI_PHYSICAL_ADDRESS    FspEventHandler;
> > -  UINT8                   Reserved1[24];
> > +  UINT8                   Reserved1[16];
> >  } FSPM_ARCH2_UPD;
> >
> >  ///
> > @@ -265,7 +271,7 @@ typedef struct {
> >  } FSPS_ARCH_UPD;
> >
> >  ///
> > -/// FSPS_ARCH2_UPD Configuration.
> > +/// FSPS_ARCH2_UPD Configuration for FSP 2.4 and above.
> >  ///
> >  typedef struct {
> >    ///
> > @@ -285,6 +291,40 @@ typedef struct {
> >    UINT8                   Reserved1[16];
> >  } FSPS_ARCH2_UPD;
> >
> > +///
> > +/// FSPI_ARCH_UPD Configuration.
> > +///
> > +typedef struct {
> > +  ///
> > +  /// Revision of the structure is 1 for this version of the specification.
> > +  ///
> > +  UINT8                   Revision;
> > +  UINT8                   Reserved[3];
> > +  ///
> > +  /// Length of the structure in bytes. The current value for this field is 32.
> > +  ///
> > +  UINT32                  Length;
> > +  ///
> > +  /// The physical memory-mapped base address of the bootloader SMM
> firmware volume (FV).
> > +  ///
> > +  EFI_PHYSICAL_ADDRESS    BootloaderSmmFvBaseAddress;
> > +  ///
> > +  /// The length in bytes of the bootloader SMM firmware volume (FV).
> > +  ///
> > +  UINT64                  BootloaderSmmFvLength;
> > +  ///
> > +  /// The physical memory-mapped base address of the bootloader SMM FV
> context data.
> > +  /// This data is provided to bootloader SMM drivers through a HOB by the
> FSP MM Foundation.
> > +  ///
> > +  EFI_PHYSICAL_ADDRESS    BootloaderSmmFvContextData;
> > +  ///
> > +  /// The length in bytes of the bootloader SMM FV context data.
> > +  /// This data is provided to bootloader SMM drivers through a HOB by the
> FSP MM Foundation.
> > +  ///
> > +  UINT16                  BootloaderSmmFvContextDataLength;
> > +  UINT8                   Reserved1[24];
> > +} FSPI_ARCH_UPD;
> > +
> >  ///
> >  /// FSPT_UPD_COMMON Configuration.
> >  ///
> > @@ -393,6 +433,21 @@ typedef struct {
> >    FSPS_ARCH2_UPD    FspsArchUpd;
> >  } FSPS_UPD_COMMON_FSP24;
> >
> > +///
> > +/// FSPI_UPD_COMMON Configuration.
> > +///
> > +typedef struct {
> > +  ///
> > +  /// FSP_UPD_HEADER Configuration.
> > +  ///
> > +  FSP_UPD_HEADER    FspUpdHeader;
> > +
> > +  ///
> > +  /// FSPI_ARCH_UPD Configuration.
> > +  ///
> > +  FSPI_ARCH_UPD     FspiArchUpd;
> > +} FSPI_UPD_COMMON;
> > +
> >  ///
> >  /// Enumeration of FSP_INIT_PHASE for NOTIFY_PHASE.
> >  ///
> > diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
> > b/IntelFsp2Pkg/Include/FspGlobalData.h
> > index 697b20ed4c..cf94f7b6a5 100644
> > --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> > +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> > @@ -12,7 +12,7 @@
> >
> >  #define FSP_IN_API_MODE          0
> >  #define FSP_IN_DISPATCH_MODE     1
> > -#define FSP_GLOBAL_DATA_VERSION  1
> > +#define FSP_GLOBAL_DATA_VERSION  0x2
> >
> >  #pragma pack(1)
> >
> > --
> > 2.35.0.windows.1