Re: [edk2-devel][edk2-platforms][PATCH V1 09/11] MinPlatformPkg/SpifvbService: Publish FV based on board request

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

Hi Isaac,

Please update copyright year on SpiFvbServiceCommon.h, SpiFvbServiceCommon.c, SpiFvbServiceSmm.inf, SpiFvbServiceStandaloneMm.inf, and SpiFvbServiceMm.c

Thanks,
Nate

> -----Original Message-----
> From: Oram, Isaac W <isaac.w.oram@intel.com>
> Sent: Friday, October 15, 2021 2:26 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH V1 09/11]
> MinPlatformPkg/SpifvbService: Publish FV based on board request
> 
> Change MM reporting to use ReportFvLib API ReportMmFv (); This removed
> the hardcoded table of FV to publish.
> Also removed the unused "default" table.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
> ---
> 
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCommon.
> c         | 16 --------
> 
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCommon.
> h         |  7 ----
>  Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> | 41 ++++++++++++--------
>  Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> |  1 +
> 
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandalon
> eMm.inf |  1 +
>  5 files changed, 27 insertions(+), 39 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCommo
> n.c
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceComm
> on.c
> index 113c749d04..b8a255dbf4 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCommo
> n.c
> +++
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCom
> +++ mon.c
> @@ -15,22 +15,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  //
>  FVB_GLOBAL   mFvbModuleGlobal;
> 
> -//
> -// This platform driver knows there are multiple FVs on FD.
> -// Now we only provide FVs on Variable region and MicorCode region for
> performance issue.
> -//
> -FV_INFO mPlatformFvBaseAddress[] = {
> -  {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase),
> FixedPcdGet32(PcdFlashNvStorageVariableSize)},
> -  {0, 0}, // {FixedPcdGet32(PcdFlashFvMicrocodeBase),
> FixedPcdGet32(PcdFlashFvMicrocodeSize)},
> -  {0, 0}
> -};
> -
> -FV_INFO mPlatformDefaultBaseAddress[] = {
> -  {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase),
> FixedPcdGet32(PcdFlashNvStorageVariableSize)},
> -  {0, 0}, // {FixedPcdGet32(PcdFlashFvMicrocodeBase),
> FixedPcdGet32(PcdFlashFvMicrocodeSize)},
> -  {0, 0}
> -};
> -
>  FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate = {
>    {
>      {
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCommo
> n.h
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceComm
> on.h
> index e9d69e9858..d1d61b124f 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCommo
> n.h
> +++
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceCom
> +++ mon.h
> @@ -71,11 +71,6 @@ typedef struct {
>    EFI_DEVICE_PATH_PROTOCOL    EndDevPath;
>  } FV_MEMMAP_DEVICE_PATH;
> 
> -typedef struct {
> -  UINT32              FvBase;
> -  UINT32              FvSize;
> -} FV_INFO;
> -
>  //
>  // Protocol APIs
>  //
> @@ -152,7 +147,5 @@ extern FVB_GLOBAL                         mFvbModuleGlobal;
>  extern FV_MEMMAP_DEVICE_PATH
> mFvMemmapDevicePathTemplate;
>  extern FV_PIWG_DEVICE_PATH                mFvPIWGDevicePathTemplate;
>  extern EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL
> mFvbProtocolTemplate;
> -extern FV_INFO                            mPlatformFvBaseAddress[];
> -extern FV_INFO                            mPlatformDefaultBaseAddress[];
> 
>  #endif
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> index 016f19587c..1d8d55b8f2 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> +++
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.
> +++ c
> @@ -11,6 +11,7 @@
>  #include "SpiFvbServiceCommon.h"
>  #include <Library/MmServicesTableLib.h>  #include
> <Library/UefiDriverEntryPoint.h>
> +#include <Library/ReportFvLib.h>
>  #include <Protocol/SmmFirmwareVolumeBlock.h>
> 
>  /**
> @@ -97,8 +98,12 @@ InstallFvbProtocol (
>    The function does the necessary initialization work for
>    Firmware Volume Block Driver.
> 
> +  @param VOID
> +
> +  @retval EFI_SUCCESS             The driver initialized successfully.
> +  @retval EFI_UNSUPPORTED         The driver failed to initialize properly.
>  **/
> -VOID
> +EFI_STATUS
>  FvbInitialize (
>    VOID
>    )
> @@ -113,15 +118,17 @@ FvbInitialize (
>    UINT32                                MaxLbaSize;
>    UINT32                                BytesWritten;
>    UINTN                                 BytesErased;
> +  FV_INFO                               *BoardFvInfoTablePtr;
> 
> -  mPlatformFvBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> -  mPlatformFvBaseAddress[0].FvSize =
> PcdGet32(PcdFlashNvStorageVariableSize);
> -  mPlatformFvBaseAddress[1].FvBase =
> PcdGet32(PcdFlashFvMicrocodeBase);
> -  mPlatformFvBaseAddress[1].FvSize =
> PcdGet32(PcdFlashFvMicrocodeSize);
> -  mPlatformDefaultBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> -  mPlatformDefaultBaseAddress[0].FvSize =
> PcdGet32(PcdFlashNvStorageVariableSize);
> -  mPlatformDefaultBaseAddress[1].FvBase =
> PcdGet32(PcdFlashFvMicrocodeBase);
> -  mPlatformDefaultBaseAddress[1].FvSize =
> PcdGet32(PcdFlashFvMicrocodeSize);
> +  //
> +  // Retrieve the FV Info from the board library  //
> + BoardFvInfoTablePtr = NULL;  ReportMmFv (&BoardFvInfoTablePtr);  if
> + (!BoardFvInfoTablePtr) {
> +    ASSERT (BoardFvInfoTablePtr != NULL);
> +    return EFI_UNSUPPORTED;
> +  }
> 
>    //
>    // We will only continue with FVB installation if the @@ -132,11 +139,11 @@
> FvbInitialize (
>      // Make sure all FVB are valid and/or fix if possible
>      //
>      for (Idx = 0;; Idx++) {
> -      if (mPlatformFvBaseAddress[Idx].FvSize == 0 &&
> mPlatformFvBaseAddress[Idx].FvBase == 0) {
> +      if (BoardFvInfoTablePtr[Idx].FvSize == 0 &&
> + BoardFvInfoTablePtr[Idx].FvBase == 0) {
>          break;
>        }
> 
> -      BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;
> +      BaseAddress = BoardFvInfoTablePtr[Idx].FvBase;
>        FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
> 
>        if (!IsFvHeaderValid (BaseAddress, FvHeader)) { @@ -192,10 +199,10 @@
> FvbInitialize (
>      //
>      BufferSize = 0;
>      for (Idx = 0; ; Idx++) {
> -      if (mPlatformFvBaseAddress[Idx].FvSize == 0 &&
> mPlatformFvBaseAddress[Idx].FvBase == 0) {
> +      if (BoardFvInfoTablePtr[Idx].FvSize == 0 &&
> + BoardFvInfoTablePtr[Idx].FvBase == 0) {
>          break;
>        }
> -      BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;
> +      BaseAddress = BoardFvInfoTablePtr[Idx].FvBase;
>        FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
> 
>        if (!IsFvHeaderValid (BaseAddress, FvHeader)) { @@ -212,7 +219,7 @@
> FvbInitialize (
>      mFvbModuleGlobal.FvbInstance =  (EFI_FVB_INSTANCE *)
> AllocateRuntimeZeroPool (BufferSize);
>      if (mFvbModuleGlobal.FvbInstance == NULL) {
>        ASSERT (FALSE);
> -      return;
> +      return EFI_SUCCESS;
>      }
> 
>      MaxLbaSize      = 0;
> @@ -220,10 +227,10 @@ FvbInitialize (
>      mFvbModuleGlobal.NumFv   = 0;
> 
>      for (Idx = 0; ; Idx++) {
> -      if (mPlatformFvBaseAddress[Idx].FvSize == 0 &&
> mPlatformFvBaseAddress[Idx].FvBase == 0) {
> +      if (BoardFvInfoTablePtr[Idx].FvSize == 0 &&
> + BoardFvInfoTablePtr[Idx].FvBase == 0) {
>          break;
>        }
> -      BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;
> +      BaseAddress = BoardFvInfoTablePtr[Idx].FvBase;
>        FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
> 
>        if (!IsFvHeaderValid (BaseAddress, FvHeader)) { @@ -268,4 +275,6 @@
> FvbInitialize (
> 
>      }
>    }
> +
> +  return EFI_SUCCESS;
>  }
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> index 10e51e1175..7b69bedcdc 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> +++
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm
> +++ .inf
> @@ -34,6 +34,7 @@
>    UefiDriverEntryPoint
>    SpiFlashCommonLib
>    MmServicesTableLib
> +  ReportFvLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.inf
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.inf
> index 9f08d3673f..1dfb2520e7 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.inf
> +++ b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSta
> +++ ndaloneMm.inf
> @@ -33,6 +33,7 @@
>    MmServicesTableLib
>    SpiFlashCommonLib
>    StandaloneMmDriverEntryPoint
> +  ReportFvLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> --
> 2.27.0.windows.1