Re: [PATCH] IntelFsp2Pkg: Add FSPI_ARCH_UPD.

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

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