Re: [edk2] [PATCH 1/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPeimPerFv
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 ; edk2-devel@lists.01.org
Cc: Wu, Hao A ; Gao, Liming ; Ni,
Ruiyu ; Kinney, Michael D ;
Desimone, Nathaniel L ; Chiu, Chasel
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 ; Wang, Jian J
> ; Wu, Hao A ; Gao, Liming
> ; Ni, Ruiyu ; Kinney,
> Michael D ; Desimone, Nathaniel L
> ; Chiu, Chasel
> 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
> Cc: Hao Wu
> Cc: Liming Gao
> Cc: Ruiyu Ni
> Cc: Michael D Kinney
> Cc: Nate DeSimone
> Cc: Chasel Chiu
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng
> ---
> 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_INFOFileInfo;
>
> @@ -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,
> );
> -if (Status != EFI_SUCCESS || PeimCount == PcdGet32
> (PcdPeiCoreMaxPeimPerFv)) {
> - break;
> +if (!EFI_ERROR (Status)) {
> + if (TempPeimCount <
Re: [edk2] [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 ; Wang, Jian J ;
> Wu, Hao A ; Gao, Liming ; Ni,
> Ruiyu ; Kinney, Michael D ;
> Desimone, Nathaniel L ; Chiu, Chasel
>
> 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
> Cc: Hao Wu
> Cc: Liming Gao
> Cc: Ruiyu Ni
> Cc: Michael D Kinney
> Cc: Nate DeSimone
> Cc: Chasel Chiu
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng
> ---
> 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_INFOFileInfo;
>
> @@ -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,
> );
> -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
Re: [edk2] [PATCH 1/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPeimPerFv
Reviewed-by: Chasel Chiu
-Original Message-
From: Zeng, Star
Sent: Friday, December 14, 2018 6:29 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Wang, Jian J ; Wu,
Hao A ; Gao, Liming ; Ni, Ruiyu
; Kinney, Michael D ; Desimone,
Nathaniel L ; Chiu, Chasel
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
Cc: Hao Wu
Cc: Liming Gao
Cc: Ruiyu Ni
Cc: Michael D Kinney
Cc: Nate DeSimone
Cc: Chasel Chiu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng
---
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_INFOFileInfo;
@@ -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,
);
-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);
+
