Re: [PATCH] IntelFsp2WrapperPkg: Support UPD allocation outside FspWrapper

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Chasel,
> Chiu
> Sent: Thursday, November 23, 2017 10:30 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [edk2] [PATCH] IntelFsp2WrapperPkg: Support UPD allocation outside
> FspWrapper
> 
> UPD allocation and patching can be done outside FspWrapper
> as implementation choice so adding a PCD to select between
> original FspWrapper allocation model or outside model
> 
> Cc: Jiewen Yao <Jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c              | 25 ++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf            |  3 +-
>  .../FspsWrapperPeim/FspsWrapperPeim.c              | 84
> +++++++++++-----------
>  .../FspsWrapperPeim/FspsWrapperPeim.inf            |  3 +-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec        | 13 +++-
>  5 files changed, 76 insertions(+), 52 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index f1d1cd6421..7b7c5f5d86 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
>    which accompanies this distribution.  The full text of the license may be
> found at
> @@ -63,20 +63,29 @@ PeiFspMemoryInit (
>    DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
> 
>    FspHobListPtr = NULL;
> +  FspmUpdDataPtr = NULL;
> 
> -  //
> -  // Copy default FSP-M UPD data from Flash
> -  //
>    FspmHeaderPtr = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32
> (PcdFspmBaseAddress));
>    DEBUG ((DEBUG_INFO, "FspmHeaderPtr - 0x%x\n", FspmHeaderPtr));
>    if (FspmHeaderPtr == NULL) {
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  FspmUpdDataPtr = (FSPM_UPD_COMMON *)AllocateZeroPool
> ((UINTN)FspmHeaderPtr->CfgRegionSize);
> -  ASSERT (FspmUpdDataPtr != NULL);
> -  SourceData = (UINTN *)((UINTN)FspmHeaderPtr->ImageBase +
> (UINTN)FspmHeaderPtr->CfgRegionOffset);
> -  CopyMem (FspmUpdDataPtr, SourceData,
> (UINTN)FspmHeaderPtr->CfgRegionSize);
> +  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 &&
> (FspmHeaderPtr->CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset !=
> 0)) {
> +    //
> +    // Copy default FSP-M UPD data from Flash
> +    //
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *)AllocateZeroPool
> ((UINTN)FspmHeaderPtr->CfgRegionSize);
> +    ASSERT (FspmUpdDataPtr != NULL);
> +    SourceData = (UINTN *)((UINTN)FspmHeaderPtr->ImageBase +
> (UINTN)FspmHeaderPtr->CfgRegionOffset);
> +    CopyMem (FspmUpdDataPtr, SourceData,
> (UINTN)FspmHeaderPtr->CfgRegionSize);
> +  } else {
> +    //
> +    // External UPD is ready, get the buffer from PCD pointer.
> +    //
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    ASSERT (FspmUpdDataPtr != NULL);
> +  }
> 
>    DEBUG ((DEBUG_INFO, "UpdateFspmUpdData enter\n"));
>    UpdateFspmUpdData ((VOID *)FspmUpdDataPtr);
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 2b3d240d08..542356b582 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -59,7 +59,8 @@
>    IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> 
>  [Pcd]
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress  ## CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress     ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress  ##
> CONSUMES
> 
>  [Sources]
>    FspmWrapperPeim.c
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index ddc19c7e8f..70dac7a414 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
>    which accompanies this distribution.  The full text of the license may be
> found at
> @@ -44,14 +44,14 @@ extern EFI_PEI_NOTIFY_DESCRIPTOR
> mS3EndOfPeiNotifyDesc;
>  extern EFI_GUID                  gFspHobGuid;
> 
>  /**
> -This function handles S3 resume task at the end of PEI
> +  This function handles S3 resume task at the end of PEI
> 
> -@param[in] PeiServices    Pointer to PEI Services Table.
> -@param[in] NotifyDesc     Pointer to the descriptor for the Notification event
> that
> -caused this function to execute.
> -@param[in] Ppi            Pointer to the PPI data associated with this
> function.
> +  @param[in] PeiServices    Pointer to PEI Services Table.
> +  @param[in] NotifyDesc     Pointer to the descriptor for the Notification
> event that
> +                            caused this function to execute.
> +  @param[in] Ppi            Pointer to the PPI data associated with this
> function.
> 
> -@retval EFI_STATUS        Always return EFI_SUCCESS
> +  @retval EFI_STATUS        Always return EFI_SUCCESS
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -68,14 +68,14 @@ EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc
> = {
>  };
> 
>  /**
> -This function handles S3 resume task at the end of PEI
> +  This function handles S3 resume task at the end of PEI
> 
> -@param[in] PeiServices    Pointer to PEI Services Table.
> -@param[in] NotifyDesc     Pointer to the descriptor for the Notification event
> that
> -caused this function to execute.
> -@param[in] Ppi            Pointer to the PPI data associated with this
> function.
> +  @param[in] PeiServices    Pointer to PEI Services Table.
> +  @param[in] NotifyDesc     Pointer to the descriptor for the Notification
> event that
> +                            caused this function to execute.
> +  @param[in] Ppi            Pointer to the PPI data associated with this
> function.
> 
> -@retval EFI_STATUS        Always return EFI_SUCCESS
> +  @retval EFI_STATUS        Always return EFI_SUCCESS
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -130,13 +130,13 @@ S3EndOfPeiNotify(
>  }
> 
>  /**
> -Return Hob list produced by FSP.
> +  Return Hob list produced by FSP.
> 
> -@param[in]  PeiServices  The pointer to the PEI Services Table.
> -@param[in]  This         The pointer to this instance of this PPI.
> -@param[out] FspHobList   The pointer to Hob list produced by FSP.
> +  @param[in]  PeiServices  The pointer to the PEI Services Table.
> +  @param[in]  This         The pointer to this instance of this PPI.
> +  @param[out] FspHobList   The pointer to Hob list produced by FSP.
> 
> -@return EFI_SUCCESS FReturn Hob list produced by FSP successfully.
> +  @return EFI_SUCCESS FReturn Hob list produced by FSP successfully.
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -157,13 +157,13 @@ EFI_PEI_PPI_DESCRIPTOR
> mPeiFspSiliconInitDonePpi = {
>  };
> 
>  /**
> -Return Hob list produced by FSP.
> +  Return Hob list produced by FSP.
> 
> -@param[in]  PeiServices  The pointer to the PEI Services Table.
> -@param[in]  This         The pointer to this instance of this PPI.
> -@param[out] FspHobList   The pointer to Hob list produced by FSP.
> +  @param[in]  PeiServices  The pointer to the PEI Services Table.
> +  @param[in]  This         The pointer to this instance of this PPI.
> +  @param[out] FspHobList   The pointer to Hob list produced by FSP.
> 
> -@return EFI_SUCCESS FReturn Hob list produced by FSP successfully.
> +  @return EFI_SUCCESS FReturn Hob list produced by FSP successfully.
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -209,14 +209,14 @@ EFI_PEI_NOTIFY_DESCRIPTOR
> mPeiMemoryDiscoveredNotifyDesc = {
>  };
> 
>  /**
> -This function is called after PEI core discover memory and finish migration.
> +  This function is called after PEI core discover memory and finish migration.
> 
> -@param[in] PeiServices    Pointer to PEI Services Table.
> -@param[in] NotifyDesc     Pointer to the descriptor for the Notification event
> that
> -caused this function to execute.
> -@param[in] Ppi            Pointer to the PPI data associated with this
> function.
> +  @param[in] PeiServices    Pointer to PEI Services Table.
> +  @param[in] NotifyDesc     Pointer to the descriptor for the Notification
> event that
> +                            caused this function to execute.
> +  @param[in] Ppi            Pointer to the PPI data associated with this
> function.
> 
> -@retval EFI_STATUS        Always return EFI_SUCCESS
> +  @retval EFI_STATUS        Always return EFI_SUCCESS
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -234,22 +234,27 @@ PeiMemoryDiscoveredNotify (
>    FSPS_UPD_COMMON           *FspsUpdDataPtr;
>    UINTN                     *SourceData;
> 
> -
>    DEBUG ((DEBUG_INFO, "PeiMemoryDiscoveredNotify enter\n"));
> -
> -  //
> -  // Copy default FSP-S UPD data from Flash
> -  //
> +  FspsUpdDataPtr = NULL;
> +
>    FspsHeaderPtr = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32
> (PcdFspsBaseAddress));
>    DEBUG ((DEBUG_INFO, "FspsHeaderPtr - 0x%x\n", FspsHeaderPtr));
>    if (FspsHeaderPtr == NULL) {
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  FspsUpdDataPtr = (FSPS_UPD_COMMON *)AllocateZeroPool
> ((UINTN)FspsHeaderPtr->CfgRegionSize);
> -  ASSERT (FspsUpdDataPtr != NULL);
> -  SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase +
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
> -  CopyMem (FspsUpdDataPtr, SourceData,
> (UINTN)FspsHeaderPtr->CfgRegionSize);
> +  if (PcdGet32 (PcdFspsUpdDataAddress) == 0 &&
> (FspsHeaderPtr->CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0))
> {
> +    //
> +    // Copy default FSP-S UPD data from Flash
> +    //
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *)AllocateZeroPool
> ((UINTN)FspsHeaderPtr->CfgRegionSize);
> +    ASSERT (FspsUpdDataPtr != NULL);
> +    SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase +
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
> +    CopyMem (FspsUpdDataPtr, SourceData,
> (UINTN)FspsHeaderPtr->CfgRegionSize);
> +  } else {
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    ASSERT (FspsUpdDataPtr != NULL);
> +  }
> 
>    UpdateFspsUpdData ((VOID *)FspsUpdDataPtr);
> 
> @@ -314,7 +319,7 @@ FspsWrapperInit (
>    //
>    Status = PeiServicesNotifyPpi (&mPeiMemoryDiscoveredNotifyDesc);
>    ASSERT_EFI_ERROR (Status);
> -
> +
>    //
>    // Register EndOfPei Notify for S3 to run FSP NotifyPhase
>    //
> @@ -342,7 +347,6 @@ FspsWrapperPeimEntryPoint (
>    IN CONST EFI_PEI_SERVICES     **PeiServices
>    )
>  {
> -
>    DEBUG ((DEBUG_INFO, "FspsWrapperPeimEntryPoint\n"));
> 
>    FspsWrapperInit ();
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index c858e7097d..cd87a99c40 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -66,7 +66,8 @@
>    gEfiPeiMemoryDiscoveredPpiGuid        ## NOTIFY
> 
>  [Pcd]
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress     ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress  ##
> CONSUMES
> 
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index c0881852c5..7634619f80 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -1,7 +1,7 @@
>  ## @file
>  # Provides drivers and definitions to support fsp in EDKII bios.
>  #
> -# Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.<BR>
>  # This program and the accompanying materials are licensed and made
> available under
>  # the terms and conditions of the BSD License that accompanies this
> distribution.
>  # The full text of the license may be found at
> @@ -95,4 +95,13 @@
> 
>  [PcdsFixedAtBuild, PcdsPatchableInModule,PcdsDynamic,PcdsDynamicEx]
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress|0x00000000|UINT32|
> 0x00001001
> -
> \ No newline at end of file
> +  #
> +  # To provide flexibility for platform to pre-allocate FSP UPD buffer
> +  #
> +  # The PCDs define the pre-allocated FSPM and FSPS UPD Data Buffer
> Address.
> +  # 0x00000000 - Platform will not pre-allocate UPD buffer before
> FspWrapper module
> +  # non-zero   - Platform will pre-allocate UPD buffer and patch this value to
> +  #              buffer address before FspWrapper module executing.
> +  #
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UI
> NT32|0x50000000
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UIN
> T32|0x50000001
> \ No newline at end of file
> --
> 2.13.3.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel