public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: "Wang, Jian J" <jian.j.wang@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "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>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH 1/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPeimPerFv
Date: Tue, 18 Dec 2018 02:45:28 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB04831040220C40@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <D827630B58408649ACB04F44C510003624EC1B29@SHSMSX103.ccr.corp.intel.com>

Good suggestion. It will make the buffer extension log consistent.
I will send V2 to cover it with some test.

Thanks,
Star
-----Original Message-----
From: Wang, Jian J 
Sent: Tuesday, December 18, 2018 10:04 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: 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: RE: [PATCH 1/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPeimPerFv

Star,

Just one place which might need refine. Please see it below.

Regards,
Jian


> -----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 1/7] MdeModulePkg PeiCore: Remove the using of 
> PcdPeiCoreMaxPeimPerFv
> 
> 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 code 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 PcdPeiCoreMaxPeimPerFv 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/Dispatcher/Dispatcher.c | 199 
> ++++++++++++++++----
> ------
>  MdeModulePkg/Core/Pei/PeiMain.h               |  17 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf             |   1 -
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c       |  48 +++----
>  4 files changed, 157 insertions(+), 108 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> index f6bb35a5fe8d..71440bab9488 100644
> --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> @@ -41,7 +41,8 @@ DiscoverPeimsAndOrderWithApriori (
>    UINTN                               PeimCount;
>    EFI_GUID                            *Guid;
>    EFI_PEI_FILE_HANDLE                 *TempFileHandles;
> -  EFI_GUID                            *FileGuid;
> +  EFI_GUID                            *TempFileGuid;
> +  UINTN                               TempPeimCount;
>    EFI_PEI_FIRMWARE_VOLUME_PPI         *FvPpi;
>    EFI_FV_FILE_INFO                    FileInfo;
> 
> @@ -51,38 +52,106 @@ DiscoverPeimsAndOrderWithApriori (
>    // Walk the FV and find all the PEIMs and the Apriori file.
>    //
>    AprioriFileHandle = NULL;
> -  Private->CurrentFvFileHandles[0] = NULL;
> +  Private->CurrentFvFileHandles = NULL;
>    Guid = NULL;
> -  FileHandle = NULL;
> -  TempFileHandles = Private->FileHandles;
> -  FileGuid        = Private->FileGuid;
> 
>    //
> -  // If the current Fv has been scanned, directly get its cachable record.
> +  // If the current Fv has been scanned, directly get its cached records.
>    //
> -  if (Private->Fv[Private->CurrentPeimFvCount].ScanFv) {
> -    CopyMem (Private->CurrentFvFileHandles, Private->Fv[Private-
> >CurrentPeimFvCount].FvFileHandles, sizeof (EFI_PEI_FILE_HANDLE) * 
> >PcdGet32
> (PcdPeiCoreMaxPeimPerFv));
> +  if (CoreFileHandle->ScanFv) {
> +    Private->CurrentFvFileHandles = CoreFileHandle->FvFileHandles;
>      return;
>    }
> 
> +  if (Private->TempPeimCount == 0) {
> +    //
> +    // Initialize the temp buffers.
> +    //
> +    Private->TempPeimCount = 32;
> +    Private->TempFileHandles = AllocatePool (sizeof 
> + (EFI_PEI_FILE_HANDLE) *
> 32);
> +    ASSERT (Private->TempFileHandles != NULL);
> +    Private->TempFileGuid    = AllocatePool (sizeof (EFI_GUID) * 32);
> +    ASSERT (Private->TempFileGuid != NULL);  }  TempFileHandles = 
> + Private->TempFileHandles;
> +  TempFileGuid    = Private->TempFileGuid;
> +
>    //
> -  // Go ahead to scan this Fv, and cache FileHandles within it.
> +  // Go ahead to scan this Fv, get PeimCount and cache FileHandles 
> + within it to
> TempFileHandles.
>    //
> -  Status = EFI_NOT_FOUND;
> -  for (PeimCount = 0; PeimCount <= PcdGet32 (PcdPeiCoreMaxPeimPerFv);
> PeimCount++) {
> +  PeimCount = 0;
> +  FileHandle = NULL;
> +  TempPeimCount = 0;
> +  do {
>      Status = FvPpi->FindFileByType (FvPpi, 
> PEI_CORE_INTERNAL_FFS_FILE_DISPATCH_TYPE, CoreFileHandle->FvHandle, 
> &FileHandle);
> -    if (Status != EFI_SUCCESS || PeimCount == PcdGet32
> (PcdPeiCoreMaxPeimPerFv)) {
> -      break;
> +    if (!EFI_ERROR (Status)) {
> +      if (TempPeimCount < Private->TempPeimCount) {
> +        TempFileHandles[TempPeimCount] = FileHandle;
> +        TempPeimCount++;
> +      }
> +      PeimCount++;
>      }
> +  } while (!EFI_ERROR (Status));
> 
> -    Private->CurrentFvFileHandles[PeimCount] = FileHandle;
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "%a(): Found 0x%x PEI FFS files in the %dth FV\n",
> +    __FUNCTION__,
> +    PeimCount,
> +    Private->CurrentPeimFvCount
> +    ));
> +
> +  if (PeimCount == 0) {
> +    //
> +    // No PEIM FFS file is found, set ScanFv flag and return.
> +    //
> +    CoreFileHandle->ScanFv = TRUE;
> +    return;
> +  }
> +
> +  if (PeimCount > Private->TempPeimCount) {
> +    //
> +    // The temp buffers are too small, allocate bigger ones.
> +    //
> +    TempFileHandles = AllocatePool (sizeof (EFI_PEI_FILE_HANDLE) *
> PeimCount);
> +    ASSERT (TempFileHandles != NULL);
> +    CopyMem (
> +      TempFileHandles,
> +      Private->TempFileHandles,
> +      sizeof (EFI_PEI_FILE_HANDLE) * Private->TempPeimCount
> +      );
> +    Private->TempFileHandles = TempFileHandles;
> +    TempFileGuid = AllocatePool (sizeof (EFI_GUID) * PeimCount);
> +    ASSERT (TempFileGuid != NULL);
> +    CopyMem (
> +      TempFileGuid,
> +      Private->TempFileGuid,
> +      sizeof (EFI_GUID) * Private->TempPeimCount
> +      );
> +    Private->TempFileGuid = TempFileGuid;
> +    Private->TempPeimCount = PeimCount;
>    }
> 
>    //
> -  // Check whether the count of files exceeds the max support files 
> in a FV image
> -  // If more files are required in a FV image, PcdPeiCoreMaxPeimPerFv 
> can be set to a larger value in DSC file.
> +  // Record PeimCount, allocate buffer for PeimState and FvFileHandles.
> +  //
> +  CoreFileHandle->PeimCount = PeimCount;  CoreFileHandle->PeimState = 
> + AllocateZeroPool (sizeof (UINT8) * PeimCount);  ASSERT 
> + (CoreFileHandle->PeimState != NULL);  CoreFileHandle->FvFileHandles 
> + = AllocateZeroPool (sizeof
> (EFI_PEI_FILE_HANDLE) * PeimCount);
> +  ASSERT (CoreFileHandle->FvFileHandles != NULL);
> +
>    //
> -  ASSERT ((Status != EFI_SUCCESS) || (PeimCount < PcdGet32 
> (PcdPeiCoreMaxPeimPerFv)));
> +  // TempFileHandles may be not big enough in last scan.
> +  // Go ahead to rescan this Fv, and cache remaining FileHandles 
> + within it to
> TempFileHandles.
> +  //
> +  FileHandle = TempFileHandles[TempPeimCount - 1];  for (; 
> + TempPeimCount < PeimCount; TempPeimCount++) {
> +    Status = FvPpi->FindFileByType (FvPpi,
> PEI_CORE_INTERNAL_FFS_FILE_DISPATCH_TYPE, CoreFileHandle->FvHandle, 
> &FileHandle);
> +    ASSERT_EFI_ERROR (Status);
> +
> +    TempFileHandles[TempPeimCount] = FileHandle;  }
> 

Above logic looks have some redundant code. And the FV might be re-scanned if there're more than 32 PEIMs in it. If the scan is time consuming task, maybe it'd be better to refine the logic. Maybe something like below (just show the idea, not verified yet).

  PeimCount       = 0;
  FileHandle      = NULL;
  TempPeimCount   = 0;
  TempFileHandles         = Private->TempFileHandles;
  TempFileGuid            = Private->TempFileGuid;
  do {
    Status = FvPpi->FindFileByType(FvPpi, PEI_CORE_INTERNAL_FFS_FILE_DISPATCH_TYPE, CoreFileHandle->FvHandle, &FileHandle);
    if (!EFI_ERROR (Status)) {
      PeimCount++;

      //
      // Have enought buffer?
      //
      if (Private->TempPeimCount < PeimCount) {
        Private->TempPeimCount  += 32;
        TempFileHandles         = Private->TempFileHandles;
        TempFileGuid            = Private->TempFileGuid;

        Private->TempFileHandles = AllocatePool (sizeof (EFI_PEI_FILE_HANDLE) * Private->TempPeimCount);
        ASSERT (Private->TempFileHandles != NULL);
        Private->TempFileGuid    = AllocatePool (sizeof (EFI_GUID) * Private->TempPeimCount);
        ASSERT (Private->TempFileGuid != NULL);

        if (TempFileHandles != NULL) {
          CopyMem (
            Private->TempFileHandles,
            TempFileHandles,
            sizeof (EFI_PEI_FILE_HANDLE) * (PeimCount - 1)
            );
        }

        if (TempFileGuid != NULL) {
          CopyMem (
            Private->TempFileGuid,
            TempFileGuid,
            sizeof (EFI_GUID) * (PeimCount - 1)
            );
        }

        TempPeimCount   = 0;
        TempFileHandles = Private->TempFileHandles + PeimCount - 1;
        TempFileGuid    = Private->TempFileGuid + PeimCount - 1;
      }

      TempFileHandles[TempPeimCount++] = FileHandle;
    }
  } while (!EFI_ERROR (Status));


>    //
>    // Get Apriori File handle
> @@ -96,7 +165,7 @@ DiscoverPeimsAndOrderWithApriori (
>      Status = FvPpi->FindSectionByType (FvPpi, EFI_SECTION_RAW, 
> AprioriFileHandle, (VOID **) &Apriori);
>      if (!EFI_ERROR (Status)) {
>        //
> -      // Calculate the number of PEIMs in the A Priori list
> +      // Calculate the number of PEIMs in the Apriori file
>        //
>        Status = FvPpi->GetFileInfo (FvPpi, AprioriFileHandle, &FileInfo);
>        ASSERT_EFI_ERROR (Status);
> @@ -113,71 +182,55 @@ DiscoverPeimsAndOrderWithApriori (
>          // Make an array of file name guids that matches the 
> FileHandle array so we can convert
>          // quickly from file name to file handle
>          //
> -        Status = FvPpi->GetFileInfo (FvPpi, Private->CurrentFvFileHandles[Index],
> &FileInfo);
> -        CopyMem (&FileGuid[Index], &FileInfo.FileName, sizeof(EFI_GUID));
> +        Status = FvPpi->GetFileInfo (FvPpi, TempFileHandles[Index], &FileInfo);
> +        ASSERT_EFI_ERROR (Status);
> +        CopyMem (&TempFileGuid[Index], &FileInfo.FileName, 
> + sizeof(EFI_GUID));
>        }
> 
>        //
> -      // Walk through FileGuid array to find out who is invalid PEIM guid in Apriori
> file.
> -      // Add available PEIMs in Apriori file into TempFileHandles array at first.
> +      // Walk through TempFileGuid array to find out who is invalid 
> + PEIM guid in
> Apriori file.
> +      // Add available PEIMs in Apriori file into FvFileHandles array.
>        //
> -      Index2 = 0;
> -      for (Index = 0; Index2 < Private->AprioriCount; Index++) {
> -        while (Index2 < Private->AprioriCount) {
> -          Guid = ScanGuid (FileGuid, PeimCount * sizeof (EFI_GUID),
> &Apriori[Index2++]);
> -          if (Guid != NULL) {
> -            break;
> -          }
> -        }
> -        if (Guid == NULL) {
> -          break;
> -        }
> -        PeimIndex = ((UINTN)Guid - (UINTN)&FileGuid[0])/sizeof (EFI_GUID);
> -        TempFileHandles[Index] = Private->CurrentFvFileHandles[PeimIndex];
> +      Index = 0;
> +      for (Index2 = 0; Index2 < Private->AprioriCount; Index2++) {
> +        Guid = ScanGuid (TempFileGuid, PeimCount * sizeof (EFI_GUID),
> &Apriori[Index2]);
> +        if (Guid != NULL) {
> +          PeimIndex = ((UINTN)Guid - (UINTN)&TempFileGuid[0])/sizeof (EFI_GUID);
> +          CoreFileHandle->FvFileHandles[Index++] = 
> + TempFileHandles[PeimIndex];
> 
> -        //
> -        // Since we have copied the file handle we can remove it from this list.
> -        //
> -        Private->CurrentFvFileHandles[PeimIndex] = NULL;
> +          //
> +          // Since we have copied the file handle we can remove it from this list.
> +          //
> +          TempFileHandles[PeimIndex] = NULL;
> +        }
>        }
> 
>        //
> -      // Update valid Aprioricount
> +      // Update valid AprioriCount
>        //
>        Private->AprioriCount = Index;
> 
>        //
>        // Add in any PEIMs not in the Apriori file
>        //
> -      for (;Index < PeimCount; Index++) {
> -        for (Index2 = 0; Index2 < PeimCount; Index2++) {
> -          if (Private->CurrentFvFileHandles[Index2] != NULL) {
> -            TempFileHandles[Index] = Private->CurrentFvFileHandles[Index2];
> -            Private->CurrentFvFileHandles[Index2] = NULL;
> -            break;
> -          }
> +      for (Index2 = 0; Index2 < PeimCount; Index2++) {
> +        if (TempFileHandles[Index2] != NULL) {
> +          CoreFileHandle->FvFileHandles[Index++] = TempFileHandles[Index2];
> +          TempFileHandles[Index2] = NULL;
>          }
>        }
> -      //
> -      //Index the end of array contains re-range Pei moudle.
> -      //
> -      TempFileHandles[Index] = NULL;
> -
> -      //
> -      // Private->CurrentFvFileHandles is currently in PEIM in the FV order.
> -      // We need to update it to start with files in the A Priori list and
> -      // then the remaining files in PEIM order.
> -      //
> -      CopyMem (Private->CurrentFvFileHandles, TempFileHandles, sizeof
> (EFI_PEI_FILE_HANDLE) * PcdGet32 (PcdPeiCoreMaxPeimPerFv));
> +      ASSERT (Index == PeimCount);
>      }
> +  } else {
> +    CopyMem (CoreFileHandle->FvFileHandles, TempFileHandles, sizeof
> (EFI_PEI_FILE_HANDLE) * PeimCount);
>    }
> +
>    //
> -  // Cache the current Fv File Handle. So that we don't have to scan the Fv again.
> -  // Instead, we can retrieve the file handles within this Fv from cachable data.
> +  // The current Fv File Handles have been cached. So that we don't 
> + have to
> scan the Fv again.
> +  // Instead, we can retrieve the file handles within this Fv from cached records.
>    //
> -  Private->Fv[Private->CurrentPeimFvCount].ScanFv = TRUE;
> -  CopyMem (Private->Fv[Private->CurrentPeimFvCount].FvFileHandles, 
> Private-
> >CurrentFvFileHandles, sizeof (EFI_PEI_FILE_HANDLE) * PcdGet32
> (PcdPeiCoreMaxPeimPerFv));
> -
> +  CoreFileHandle->ScanFv = TRUE;
> +  Private->CurrentFvFileHandles = CoreFileHandle->FvFileHandles;
>  }
> 
>  //
> @@ -977,7 +1030,7 @@ PeiDispatcher (
>      SaveCurrentFileHandle =  Private->CurrentFileHandle;
> 
>      for (Index1 = 0; Index1 < Private->FvCount; Index1++) {
> -      for (Index2 = 0; (Index2 < PcdGet32 (PcdPeiCoreMaxPeimPerFv)) &&
> (Private->Fv[Index1].FvFileHandles[Index2] != NULL); Index2++) {
> +      for (Index2 = 0; Index2 < Private->Fv[Index1].PeimCount; 
> + Index2++) {
>          if (Private->Fv[Index1].PeimState[Index2] ==
> PEIM_STATE_REGISTER_FOR_SHADOW) {
>            PeimFileHandle = Private->Fv[Index1].FvFileHandles[Index2];
>            Private->CurrentFileHandle   = PeimFileHandle;
> @@ -1063,7 +1116,7 @@ PeiDispatcher (
>        // Start to dispatch all modules within the current Fv.
>        //
>        for (PeimCount = Private->CurrentPeimCount;
> -           (PeimCount < PcdGet32 (PcdPeiCoreMaxPeimPerFv)) && (Private-
> >CurrentFvFileHandles[PeimCount] != NULL);
> +           PeimCount < Private->Fv[FvCount].PeimCount;
>             PeimCount++) {
>          Private->CurrentPeimCount  = PeimCount;
>          PeimFileHandle = Private->CurrentFileHandle = Private-
> >CurrentFvFileHandles[PeimCount];
> @@ -1207,21 +1260,17 @@ PeiDispatcher (
>        }
> 
>        //
> -      // We set to NULL here to optimize the 2nd entry to this routine after
> -      //  memory is found. This reprevents rescanning of the FV. We set to
> -      //  NULL here so we start at the begining of the next FV
> +      // Before walking through the next FV, we should set them to NULL/0 to
> +      // start at the begining of the next FV.
>        //
>        Private->CurrentFileHandle = NULL;
>        Private->CurrentPeimCount = 0;
> -      //
> -      // Before walking through the next FV,Private-
> >CurrentFvFileHandles[]should set to NULL
> -      //
> -      SetMem (Private->CurrentFvFileHandles, sizeof (EFI_PEI_FILE_HANDLE) *
> PcdGet32 (PcdPeiCoreMaxPeimPerFv), 0);
> +      Private->CurrentFvFileHandles = NULL;
>      }
> 
>      //
> -    // Before making another pass, we should set Private->CurrentPeimFvCount
> =0 to go
> -    // through all the FV.
> +    // Before making another pass, we should set it to 0 to
> +    // go through all the FVs.
>      //
>      Private->CurrentPeimFvCount = 0;
> 
> @@ -1300,7 +1349,7 @@ DepexSatisfied (
> 
>    if (PeimCount < Private->AprioriCount) {
>      //
> -    // If its in the A priori file then we set Depex to TRUE
> +    // If it's in the Apriori file then we set Depex to TRUE
>      //
>      DEBUG ((DEBUG_DISPATCH, "  RESULT = TRUE (Apriori)\n"));
>      return TRUE;
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h 
> b/MdeModulePkg/Core/Pei/PeiMain.h index 6469436b8f79..ab914c7d2e44 
> 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -111,12 +111,13 @@ typedef struct {
>    EFI_FIRMWARE_VOLUME_HEADER          *FvHeader;
>    EFI_PEI_FIRMWARE_VOLUME_PPI         *FvPpi;
>    EFI_PEI_FV_HANDLE                   FvHandle;
> +  UINTN                               PeimCount;
>    //
> -  // Ponter to the buffer with the PcdPeiCoreMaxPeimPerFv number of Entries.
> +  // Ponter to the buffer with the PeimCount number of Entries.
>    //
>    UINT8                               *PeimState;
>    //
> -  // Ponter to the buffer with the PcdPeiCoreMaxPeimPerFv number of Entries.
> +  // Ponter to the buffer with the PeimCount number of Entries.
>    //
>    EFI_PEI_FILE_HANDLE                 *FvFileHandles;
>    BOOLEAN                             ScanFv;
> @@ -209,7 +210,7 @@ struct _PEI_CORE_INSTANCE {
>    UINTN                              UnknownFvInfoCount;
> 
>    ///
> -  /// Pointer to the buffer with the PcdPeiCoreMaxPeimPerFv number of entries.
> +  /// Pointer to the buffer FvFileHandlers in PEI_CORE_FV_HANDLE 
> + specified by
> CurrentPeimFvCount.
>    ///
>    EFI_PEI_FILE_HANDLE                *CurrentFvFileHandles;
>    UINTN                              AprioriCount;
> @@ -256,14 +257,16 @@ struct _PEI_CORE_INSTANCE {
>    //
>    PE_COFF_LOADER_READ_FILE          ShadowedImageRead;
> 
> +  UINTN                             TempPeimCount;
> +
>    //
> -  // Pointer to the temp buffer with the PcdPeiCoreMaxPeimPerFv + 1 
> number of entries.
> +  // Pointer to the temp buffer with the TempPeimCount number of entries.
>    //
> -  EFI_PEI_FILE_HANDLE               *FileHandles;
> +  EFI_PEI_FILE_HANDLE               *TempFileHandles;
>    //
> -  // Pointer to the temp buffer with the PcdPeiCoreMaxPeimPerFv 
> number of entries.
> +  // Pointer to the temp buffer with the TempPeimCount number of entries.
>    //
> -  EFI_GUID                          *FileGuid;
> +  EFI_GUID                          *TempFileGuid;
> 
>    //
>    // Temp Memory Range is not covered by PeiTempMem and Stack.
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> b/MdeModulePkg/Core/Pei/PeiMain.inf
> index 4e1581a926d9..d106c3606e97 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> @@ -105,7 +105,6 @@ [Ppis]
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported                   ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv                     ##
> 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 e3a301dfe0f2..52adefeb44b4 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -185,27 +185,39 @@ PeiCore (
>        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);
> -        OldCoreData->CurrentFvFileHandles = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->CurrentFvFileHandles + 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 ++) {
> -          OldCoreData->Fv[Index].PeimState     = (UINT8 *) OldCoreData-
> >Fv[Index].PeimState + OldCoreData->HeapOffset;
> -          OldCoreData->Fv[Index].FvFileHandles = (EFI_PEI_FILE_HANDLE *)
> ((UINT8 *) OldCoreData->Fv[Index].FvFileHandles + 
> OldCoreData->HeapOffset);
> +          if (OldCoreData->Fv[Index].PeimState != NULL) {
> +            OldCoreData->Fv[Index].PeimState     = (UINT8 *) OldCoreData-
> >Fv[Index].PeimState + OldCoreData->HeapOffset;
> +          }
> +          if (OldCoreData->Fv[Index].FvFileHandles != NULL) {
> +            OldCoreData->Fv[Index].FvFileHandles = 
> + (EFI_PEI_FILE_HANDLE *)
> ((UINT8 *) OldCoreData->Fv[Index].FvFileHandles + 
> OldCoreData->HeapOffset);
> +          }
>          }
> -        OldCoreData->FileGuid             = (EFI_GUID *) ((UINT8 *) OldCoreData-
> >FileGuid + OldCoreData->HeapOffset);
> -        OldCoreData->FileHandles          = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->FileHandles + OldCoreData->HeapOffset);
> +        OldCoreData->TempFileGuid         = (EFI_GUID *) ((UINT8 *) OldCoreData-
> >TempFileGuid + OldCoreData->HeapOffset);
> +        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);
> -        OldCoreData->CurrentFvFileHandles = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->CurrentFvFileHandles - 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 ++) {
> -          OldCoreData->Fv[Index].PeimState     = (UINT8 *) OldCoreData-
> >Fv[Index].PeimState - OldCoreData->HeapOffset;
> -          OldCoreData->Fv[Index].FvFileHandles = (EFI_PEI_FILE_HANDLE *)
> ((UINT8 *) OldCoreData->Fv[Index].FvFileHandles - 
> OldCoreData->HeapOffset);
> +          if (OldCoreData->Fv[Index].PeimState != NULL) {
> +            OldCoreData->Fv[Index].PeimState     = (UINT8 *) OldCoreData-
> >Fv[Index].PeimState - OldCoreData->HeapOffset;
> +          }
> +          if (OldCoreData->Fv[Index].FvFileHandles != NULL) {
> +            OldCoreData->Fv[Index].FvFileHandles = 
> + (EFI_PEI_FILE_HANDLE *)
> ((UINT8 *) OldCoreData->Fv[Index].FvFileHandles - 
> OldCoreData->HeapOffset);
> +          }
>          }
> -        OldCoreData->FileGuid             = (EFI_GUID *) ((UINT8 *) OldCoreData-
> >FileGuid - OldCoreData->HeapOffset);
> -        OldCoreData->FileHandles          = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->FileHandles - OldCoreData->HeapOffset);
> +        OldCoreData->TempFileGuid         = (EFI_GUID *) ((UINT8 *) OldCoreData-
> >TempFileGuid - OldCoreData->HeapOffset);
> +        OldCoreData->TempFileHandles      = (EFI_PEI_FILE_HANDLE *) ((UINT8 *)
> OldCoreData->TempFileHandles - OldCoreData->HeapOffset);
>        }
> 
>        //
> @@ -320,7 +332,7 @@ PeiCore (
>    //
>    // Initialize PEI Core Services
>    //
> -  InitializeMemoryServices   (&PrivateData,    SecCoreData, OldCoreData);
> +  InitializeMemoryServices   (&PrivateData, SecCoreData, OldCoreData);
>    if (OldCoreData == NULL) {
>      //
>      // Initialize PEI Core Private Data Buffer @@ -329,22 +341,8 @@ 
> PeiCore (
>      ASSERT (PrivateData.PpiData.PpiListPtrs != NULL);
>      PrivateData.Fv                   = AllocateZeroPool (sizeof (PEI_CORE_FV_HANDLE)
> * PcdGet32 (PcdPeiCoreMaxFvSupported));
>      ASSERT (PrivateData.Fv != NULL);
> -    PrivateData.Fv[0].PeimState      = AllocateZeroPool (sizeof (UINT8) *
> PcdGet32 (PcdPeiCoreMaxPeimPerFv) * PcdGet32 (PcdPeiCoreMaxFvSupported));
> -    ASSERT (PrivateData.Fv[0].PeimState != NULL);
> -    PrivateData.Fv[0].FvFileHandles  = AllocateZeroPool (sizeof
> (EFI_PEI_FILE_HANDLE) * PcdGet32 (PcdPeiCoreMaxPeimPerFv) * PcdGet32 
> (PcdPeiCoreMaxFvSupported));
> -    ASSERT (PrivateData.Fv[0].FvFileHandles != NULL);
> -    for (Index = 1; Index < PcdGet32 (PcdPeiCoreMaxFvSupported); Index ++) {
> -      PrivateData.Fv[Index].PeimState     = PrivateData.Fv[Index - 1].PeimState +
> PcdGet32 (PcdPeiCoreMaxPeimPerFv);
> -      PrivateData.Fv[Index].FvFileHandles = PrivateData.Fv[Index -
> 1].FvFileHandles + PcdGet32 (PcdPeiCoreMaxPeimPerFv);
> -    }
>      PrivateData.UnknownFvInfo        = AllocateZeroPool (sizeof
> (PEI_CORE_UNKNOW_FORMAT_FV_INFO) * PcdGet32 
> (PcdPeiCoreMaxFvSupported));
>      ASSERT (PrivateData.UnknownFvInfo != NULL);
> -    PrivateData.CurrentFvFileHandles = AllocateZeroPool (sizeof
> (EFI_PEI_FILE_HANDLE) * PcdGet32 (PcdPeiCoreMaxPeimPerFv));
> -    ASSERT (PrivateData.CurrentFvFileHandles != NULL);
> -    PrivateData.FileGuid             = AllocatePool (sizeof (EFI_GUID) * PcdGet32
> (PcdPeiCoreMaxPeimPerFv));
> -    ASSERT (PrivateData.FileGuid != NULL);
> -    PrivateData.FileHandles          = AllocatePool (sizeof (EFI_PEI_FILE_HANDLE) *
> (PcdGet32 (PcdPeiCoreMaxPeimPerFv) + 1));
> -    ASSERT (PrivateData.FileHandles != NULL);
>    }
>    InitializePpiServices      (&PrivateData,    OldCoreData);
> 
> --
> 2.7.0.windows.1



  reply	other threads:[~2018-12-18  2:46 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-14 10:28 [PATCH 0/7] Remove PcdPeiCoreMaxXXX PCDs Star Zeng
2018-12-14 10:28 ` [PATCH 1/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPeimPerFv Star Zeng
2018-12-17 23:20   ` Chiu, Chasel
2018-12-18  2:04   ` Wang, Jian J
2018-12-18  2:45     ` Zeng, Star [this message]
2018-12-14 10:28 ` [PATCH 2/7] SecurityPkg Tcg(2)Pei: Remove the using of PcdPeiCoreMaxFvSupported Star Zeng
2018-12-17  5:06   ` Zhang, Chao B
2018-12-14 10:28 ` [PATCH 3/7] MdeModulePkg PeiCore: " Star Zeng
2018-12-17 23:20   ` Chiu, Chasel
2018-12-18  2:10   ` Wang, Jian J
2018-12-14 10:28 ` [PATCH 4/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPpiSupported Star Zeng
2018-12-17 23:20   ` Chiu, Chasel
2018-12-18  2:24   ` Wang, Jian J
2018-12-18  3:12     ` Zeng, Star
2018-12-14 10:28 ` [PATCH 5/7] OvmfPkg: Remove PcdPeiCoreMaxXXX PCDs' statement Star Zeng
2018-12-14 10:57   ` Ard Biesheuvel
2018-12-14 13:20   ` Laszlo Ersek
2018-12-14 10:28 ` [PATCH 6/7] Vlv2TbltDevicePkg: " Star Zeng
     [not found]   ` <A6DBF04CB9C4D045926EB50EBFFE5A2950865373@shsmsx102.ccr.corp.intel.com>
2018-12-18 11:00     ` Zeng, Star
     [not found]       ` <A6DBF04CB9C4D045926EB50EBFFE5A29508656F9@shsmsx102.ccr.corp.intel.com>
2018-12-19  1:23         ` Zeng, Star
     [not found]           ` <A6DBF04CB9C4D045926EB50EBFFE5A2950865882@shsmsx102.ccr.corp.intel.com>
2018-12-19  3:12             ` Sun, Zailiang
2018-12-14 10:28 ` [PATCH 7/7] MdeModulePkg: Remove PcdPeiCoreMaxXXX PCDs Star Zeng
2018-12-17 23:19   ` Chiu, Chasel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0C09AFA07DD0434D9E2A0C6AEB04831040220C40@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox