Re: [PATCH 3/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxFvSupported

["Wang, Jian J" <jian.j.wang@intel.com>]

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, December 14, 2018 6:29 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>; Ni,
> Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>
> Subject: [PATCH 3/7] MdeModulePkg PeiCore: Remove the using of
> PcdPeiCoreMaxFvSupported
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1405
> 
> Background as below.
> 
> Problem:
> As static configuration from the PCDs, the binary PeiCore (for example
> in FSP binary with dispatch mode) could not predict how many FVs,
> Files or PPIs for different platforms.
> 
> Burden:
> Platform developers need configure the PCDs accordingly for different
> platforms.
> 
> To solve the problem and remove the burden, we can update PeiCore to
> remove the using of PcdPeiCoreMaxFvSupported, PcdPeiCoreMaxPeimPerFv
> and PcdPeiCoreMaxPpiSupported by extending buffer dynamically for FV,
> File and PPI management.
> 
> This patch removes the using of PcdPeiCoreMaxFvSupported in PeiCore.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Pei/FwVol/FwVol.c     | 67
> ++++++++++++++++++++++++++-------
>  MdeModulePkg/Core/Pei/PeiMain.h         | 15 +++++++-
>  MdeModulePkg/Core/Pei/PeiMain.inf       |  1 -
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 16 ++++----
>  4 files changed, 75 insertions(+), 24 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> index 5629c9a1ce20..0a67b96bf1e3 100644
> --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> @@ -503,6 +503,10 @@ PeiInitializeFv (
>                      );
>    ASSERT_EFI_ERROR (Status);
> 
> +  PrivateData->Fv = AllocateZeroPool (sizeof (PEI_CORE_FV_HANDLE) *
> FV_GROWTH_STEP);
> +  ASSERT (PrivateData->Fv != NULL);
> +  PrivateData->MaxFvCount = FV_GROWTH_STEP;
> +
>    //
>    // Update internal PEI_CORE_FV array.
>    //
> @@ -560,6 +564,7 @@ FirmwareVolmeInfoPpiNotifyCallback (
>    VOID                                  *DepexData;
>    BOOLEAN                               IsFvInfo2;
>    UINTN                                 CurFvCount;
> +  VOID                                  *TempPtr;
> 
>    Status       = EFI_SUCCESS;
>    PrivateData  = PEI_CORE_INSTANCE_FROM_PS_THIS (PeiServices);
> @@ -626,10 +631,21 @@ FirmwareVolmeInfoPpiNotifyCallback (
>        }
>      }
> 
> -    if (PrivateData->FvCount >= PcdGet32 (PcdPeiCoreMaxFvSupported)) {
> -      DEBUG ((EFI_D_ERROR, "The number of Fv Images (%d) exceed the max
> supported FVs (%d) in Pei", PrivateData->FvCount + 1, PcdGet32
> (PcdPeiCoreMaxFvSupported)));
> -      DEBUG ((EFI_D_ERROR, "PcdPeiCoreMaxFvSupported value need be
> reconfigurated in DSC"));
> -      ASSERT (FALSE);
> +    if (PrivateData->FvCount >= PrivateData->MaxFvCount) {
> +      //
> +      // Run out of room, grow the buffer.
> +      //
> +      TempPtr = AllocateZeroPool (
> +                  sizeof (PEI_CORE_FV_HANDLE) * (PrivateData->MaxFvCount +
> FV_GROWTH_STEP)
> +                  );
> +      ASSERT (TempPtr != NULL);
> +      CopyMem (
> +        TempPtr,
> +        PrivateData->Fv,
> +        sizeof (PEI_CORE_FV_HANDLE) * PrivateData->MaxFvCount
> +        );
> +      PrivateData->Fv = TempPtr;
> +      PrivateData->MaxFvCount = PrivateData->MaxFvCount +
> FV_GROWTH_STEP;
>      }
> 
>      //
> @@ -2157,7 +2173,6 @@ FindNextCoreFvHandle (
>      }
>    }
> 
> -  ASSERT (Private->FvCount <= PcdGet32 (PcdPeiCoreMaxFvSupported));
>    if (Instance >= Private->FvCount) {
>      return NULL;
>    }
> @@ -2205,7 +2220,7 @@ PeiReinitializeFv (
>    //
>    // Fixup all FvPpi pointers for the implementation in flash to permanent
> memory.
>    //
> -  for (Index = 0; Index < PcdGet32 (PcdPeiCoreMaxFvSupported); Index ++) {
> +  for (Index = 0; Index < PrivateData->FvCount; Index ++) {
>      if (PrivateData->Fv[Index].FvPpi == OldFfsFvPpi) {
>        PrivateData->Fv[Index].FvPpi = &mPeiFfs2FwVol.Fv;
>      }
> @@ -2233,7 +2248,7 @@ PeiReinitializeFv (
>    //
>    // Fixup all FvPpi pointers for the implementation in flash to permanent
> memory.
>    //
> -  for (Index = 0; Index < PcdGet32 (PcdPeiCoreMaxFvSupported); Index ++) {
> +  for (Index = 0; Index < PrivateData->FvCount; Index ++) {
>      if (PrivateData->Fv[Index].FvPpi == OldFfsFvPpi) {
>        PrivateData->Fv[Index].FvPpi = &mPeiFfs3FwVol.Fv;
>      }
> @@ -2263,9 +2278,23 @@ AddUnknownFormatFvInfo (
>    )
>  {
>    PEI_CORE_UNKNOW_FORMAT_FV_INFO    *NewUnknownFv;
> +  VOID                              *TempPtr;
> 
> -  if (PrivateData->UnknownFvInfoCount + 1 >= PcdGet32
> (PcdPeiCoreMaxFvSupported)) {
> -    return EFI_OUT_OF_RESOURCES;
> +  if (PrivateData->UnknownFvInfoCount >= PrivateData-
> >MaxUnknownFvInfoCount) {
> +    //
> +    // Run out of room, grow the buffer.
> +    //
> +    TempPtr = AllocateZeroPool (
> +                sizeof (PEI_CORE_UNKNOW_FORMAT_FV_INFO) * (PrivateData-
> >MaxUnknownFvInfoCount + FV_GROWTH_STEP)
> +                );
> +    ASSERT (TempPtr != NULL);
> +    CopyMem (
> +      TempPtr,
> +      PrivateData->UnknownFvInfo,
> +      sizeof (PEI_CORE_UNKNOW_FORMAT_FV_INFO) * PrivateData-
> >MaxUnknownFvInfoCount
> +      );
> +    PrivateData->UnknownFvInfo = TempPtr;
> +    PrivateData->MaxUnknownFvInfoCount = PrivateData-
> >MaxUnknownFvInfoCount + FV_GROWTH_STEP;
>    }
> 
>    NewUnknownFv = &PrivateData->UnknownFvInfo[PrivateData-
> >UnknownFvInfoCount];
> @@ -2368,6 +2397,7 @@ ThirdPartyFvPpiNotifyCallback (
>    EFI_PEI_FILE_HANDLE          FileHandle;
>    VOID                         *DepexData;
>    UINTN                        CurFvCount;
> +  VOID                         *TempPtr;
> 
>    PrivateData  = PEI_CORE_INSTANCE_FROM_PS_THIS (PeiServices);
>    FvPpi = (EFI_PEI_FIRMWARE_VOLUME_PPI*) Ppi;
> @@ -2403,10 +2433,21 @@ ThirdPartyFvPpiNotifyCallback (
>        continue;
>      }
> 
> -    if (PrivateData->FvCount >= PcdGet32 (PcdPeiCoreMaxFvSupported)) {
> -      DEBUG ((EFI_D_ERROR, "The number of Fv Images (%d) exceed the max
> supported FVs (%d) in Pei", PrivateData->FvCount + 1, PcdGet32
> (PcdPeiCoreMaxFvSupported)));
> -      DEBUG ((EFI_D_ERROR, "PcdPeiCoreMaxFvSupported value need be
> reconfigurated in DSC"));
> -      ASSERT (FALSE);
> +    if (PrivateData->FvCount >= PrivateData->MaxFvCount) {
> +      //
> +      // Run out of room, grow the buffer.
> +      //
> +      TempPtr = AllocateZeroPool (
> +                  sizeof (PEI_CORE_FV_HANDLE) * (PrivateData->MaxFvCount +
> FV_GROWTH_STEP)
> +                  );
> +      ASSERT (TempPtr != NULL);
> +      CopyMem (
> +        TempPtr,
> +        PrivateData->Fv,
> +        sizeof (PEI_CORE_FV_HANDLE) * PrivateData->MaxFvCount
> +        );
> +      PrivateData->Fv = TempPtr;
> +      PrivateData->MaxFvCount = PrivateData->MaxFvCount +
> FV_GROWTH_STEP;
>      }
> 
>      //
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h
> index ab914c7d2e44..b248118087ad 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -107,6 +107,11 @@ typedef struct {
>  #define PEIM_STATE_REGISTER_FOR_SHADOW    0x02
>  #define PEIM_STATE_DONE                   0x03
> 
> +//
> +// Number of FV instances to grow by each time we run out of room
> +//
> +#define FV_GROWTH_STEP 8
> +
>  typedef struct {
>    EFI_FIRMWARE_VOLUME_HEADER          *FvHeader;
>    EFI_PEI_FIRMWARE_VOLUME_PPI         *FvPpi;
> @@ -197,16 +202,22 @@ struct _PEI_CORE_INSTANCE {
>    UINTN                              FvCount;
> 
>    ///
> -  /// Pointer to the buffer with the PcdPeiCoreMaxFvSupported number of
> entries.
> +  /// The max count of FVs which contains FFS and could be dispatched by
> PeiCore.
> +  ///
> +  UINTN                              MaxFvCount;
> +
> +  ///
> +  /// Pointer to the buffer with the MaxFvCount number of entries.
>    /// Each entry is for one FV which contains FFS and could be dispatched by
> PeiCore.
>    ///
>    PEI_CORE_FV_HANDLE                 *Fv;
> 
>    ///
> -  /// Pointer to the buffer with the PcdPeiCoreMaxFvSupported number of
> entries.
> +  /// Pointer to the buffer with the MaxUnknownFvInfoCount number of entries.
>    /// Each entry is for one FV which could not be dispatched by PeiCore.
>    ///
>    PEI_CORE_UNKNOW_FORMAT_FV_INFO     *UnknownFvInfo;
> +  UINTN                              MaxUnknownFvInfoCount;
>    UINTN                              UnknownFvInfoCount;
> 
>    ///
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> b/MdeModulePkg/Core/Pei/PeiMain.inf
> index d106c3606e97..dd41fe41bc89 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> @@ -104,7 +104,6 @@ [Ppis]
>    gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
> 
>  [Pcd]
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported                   ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported                  ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize                  ##
> CONSUMES
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreImageLoaderSearchTeSectionFir
> st  ## CONSUMES
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 52adefeb44b4..4869bf18f005 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -184,13 +184,15 @@ PeiCore (
>        OldCoreData->CpuIo = &OldCoreData->ServiceTableShadow.CpuIo;
>        if (OldCoreData->HeapOffsetPositive) {
>          OldCoreData->HobList.Raw = (VOID *)(OldCoreData->HobList.Raw +
> OldCoreData->HeapOffset);
> -        OldCoreData->UnknownFvInfo        =
> (PEI_CORE_UNKNOW_FORMAT_FV_INFO *) ((UINT8 *) OldCoreData-
> >UnknownFvInfo + OldCoreData->HeapOffset);
> +        if (OldCoreData->UnknownFvInfo != NULL) {
> +          OldCoreData->UnknownFvInfo      =
> (PEI_CORE_UNKNOW_FORMAT_FV_INFO *) ((UINT8 *) OldCoreData-
> >UnknownFvInfo + OldCoreData->HeapOffset);
> +        }
>          if (OldCoreData->CurrentFvFileHandles != NULL) {
>            OldCoreData->CurrentFvFileHandles = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->CurrentFvFileHandles + OldCoreData->HeapOffset);
>          }
>          OldCoreData->PpiData.PpiListPtrs  = (PEI_PPI_LIST_POINTERS *) ((UINT8 *)
> OldCoreData->PpiData.PpiListPtrs + OldCoreData->HeapOffset);
>          OldCoreData->Fv                   = (PEI_CORE_FV_HANDLE *) ((UINT8 *)
> OldCoreData->Fv + OldCoreData->HeapOffset);
> -        for (Index = 0; Index < PcdGet32 (PcdPeiCoreMaxFvSupported); Index ++) {
> +        for (Index = 0; Index < OldCoreData->FvCount; Index ++) {
>            if (OldCoreData->Fv[Index].PeimState != NULL) {
>              OldCoreData->Fv[Index].PeimState     = (UINT8 *) OldCoreData-
> >Fv[Index].PeimState + OldCoreData->HeapOffset;
>            }
> @@ -202,13 +204,15 @@ PeiCore (
>          OldCoreData->TempFileHandles      = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->TempFileHandles + OldCoreData->HeapOffset);
>        } else {
>          OldCoreData->HobList.Raw = (VOID *)(OldCoreData->HobList.Raw -
> OldCoreData->HeapOffset);
> -        OldCoreData->UnknownFvInfo        =
> (PEI_CORE_UNKNOW_FORMAT_FV_INFO *) ((UINT8 *) OldCoreData-
> >UnknownFvInfo - OldCoreData->HeapOffset);
> +        if (OldCoreData->UnknownFvInfo != NULL) {
> +          OldCoreData->UnknownFvInfo      =
> (PEI_CORE_UNKNOW_FORMAT_FV_INFO *) ((UINT8 *) OldCoreData-
> >UnknownFvInfo - OldCoreData->HeapOffset);
> +        }
>          if (OldCoreData->CurrentFvFileHandles != NULL) {
>            OldCoreData->CurrentFvFileHandles = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->CurrentFvFileHandles - OldCoreData->HeapOffset);
>          }
>          OldCoreData->PpiData.PpiListPtrs  = (PEI_PPI_LIST_POINTERS *) ((UINT8 *)
> OldCoreData->PpiData.PpiListPtrs - OldCoreData->HeapOffset);
>          OldCoreData->Fv                   = (PEI_CORE_FV_HANDLE *) ((UINT8 *)
> OldCoreData->Fv - OldCoreData->HeapOffset);
> -        for (Index = 0; Index < PcdGet32 (PcdPeiCoreMaxFvSupported); Index ++) {
> +        for (Index = 0; Index < OldCoreData->FvCount; Index ++) {
>            if (OldCoreData->Fv[Index].PeimState != NULL) {
>              OldCoreData->Fv[Index].PeimState     = (UINT8 *) OldCoreData-
> >Fv[Index].PeimState - OldCoreData->HeapOffset;
>            }
> @@ -339,10 +343,6 @@ PeiCore (
>      //
>      PrivateData.PpiData.PpiListPtrs  = AllocateZeroPool (sizeof
> (PEI_PPI_LIST_POINTERS) * PcdGet32 (PcdPeiCoreMaxPpiSupported));
>      ASSERT (PrivateData.PpiData.PpiListPtrs != NULL);
> -    PrivateData.Fv                   = AllocateZeroPool (sizeof (PEI_CORE_FV_HANDLE)
> * PcdGet32 (PcdPeiCoreMaxFvSupported));
> -    ASSERT (PrivateData.Fv != NULL);
> -    PrivateData.UnknownFvInfo        = AllocateZeroPool (sizeof
> (PEI_CORE_UNKNOW_FORMAT_FV_INFO) * PcdGet32
> (PcdPeiCoreMaxFvSupported));
> -    ASSERT (PrivateData.UnknownFvInfo != NULL);
>    }
>    InitializePpiServices      (&PrivateData,    OldCoreData);
> 
> --
> 2.7.0.windows.1