Hi Chasel, Please see feedback inline.
Thanks, Nate > -----Original Message----- > From: Chiu, Chasel <chasel.c...@intel.com> > Sent: Thursday, October 7, 2021 11:43 PM > To: devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Liming Gao > <gaolim...@byosoft.com.cn>; Dong, Eric <eric.d...@intel.com> > Subject: [edk2-platforms: PATCH v3 1/9] MinPlatformPkg: Support FSP 2.3 > FSP_NON_VOLATILE_STORAGE_HOB2. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3678 > > Implementation should search FSP_NON_VOLATILE_STORAGE_HOB2 firstly > and only search FSP_NON_VOLATILE_STORAGE_HOB when former one is > not found. > > Also added PeiGetLargeVariable () to support the scenarios where the > variable data size is bigger than a single variable size limit. > > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Eric Dong <eric.d...@intel.com> > Signed-off-by: Chasel Chiu <chasel.c...@intel.com> > --- > Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c > | 39 ++++++++++++++++++++++++++++----------- > Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c > | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > > Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c > | 4 ++-- > > Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf > | 5 ++++- > Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h > | 33 ++++++++++++++++++++++++++++++++- > Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.inf > | 4 +++- > Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > | 1 + > 7 files changed, 138 insertions(+), 17 deletions(-) > > diff --git > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c > > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c > index 41ed2550bd..401e02a7a9 100644 > --- > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c > +++ > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c > @@ -2,7 +2,7 @@ > This is the driver that locates the MemoryConfigurationData HOB, if it > exists, and saves the data to nvRAM. > > -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -17,6 +17,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include <Library/MemoryAllocationLib.h> > #include <Library/BaseMemoryLib.h> > #include <Protocol/VariableLock.h> > +#include <Guid/FspNonVolatileStorageHob2.h> > > /** > This is the standard EFI driver point that detects whether there is a > @@ -50,11 +51,27 @@ SaveMemoryConfigEntryPoint ( > // > // Search for the Memory Configuration GUID HOB. If it is not present, > then > // there's nothing we can do. It may not exist on the update path. > + // Firstly check version2 FspNvsHob. > // > - GuidHob = GetFirstGuidHob (&gFspNonVolatileStorageHobGuid); > + GuidHob = GetFirstGuidHob (&gFspNonVolatileStorageHob2Guid); > if (GuidHob != NULL) { > - HobData = GET_GUID_HOB_DATA (GuidHob); > - DataSize = GET_GUID_HOB_DATA_SIZE(GuidHob); > + HobData = (VOID *) (UINTN) ((FSP_NON_VOLATILE_STORAGE_HOB2 *) (UINTN) > GuidHob)->NvsDataPtr; > + DataSize = (UINTN) ((FSP_NON_VOLATILE_STORAGE_HOB2 *) (UINTN) > GuidHob)->NvsDataLength; > + } else { > + // > + // Fall back to version1 FspNvsHob > + // > + GuidHob = GetFirstGuidHob (&gFspNonVolatileStorageHobGuid); > + if (GuidHob != NULL) { > + HobData = GET_GUID_HOB_DATA (GuidHob); > + DataSize = GET_GUID_HOB_DATA_SIZE (GuidHob); > + } > + } > + > + if (HobData != NULL) { > + DEBUG ((DEBUG_INFO, "FspNvsHob.NvsDataLength:%d\n", DataSize)); > + DEBUG ((DEBUG_INFO, "FspNvsHob.NvsDataPtr : 0x%x\n", HobData)); > + > if (DataSize > 0) { > // > // Use the HOB to save Memory Configuration Data > @@ -65,8 +82,8 @@ SaveMemoryConfigEntryPoint ( > return EFI_UNSUPPORTED; > } > Status = gRT->GetVariable ( > - L"MemoryConfig", > - &gFspNonVolatileStorageHobGuid, > + L"FspNvsBuffer", > + &gFspNvsBufferVariableGuid, This needs to be updated to use LargeVariableLib as well. > NULL, > &BufferSize, > VariableData > @@ -80,8 +97,8 @@ SaveMemoryConfigEntryPoint ( > } > > Status = gRT->GetVariable ( > - L"MemoryConfig", > - &gFspNonVolatileStorageHobGuid, > + L"FspNvsBuffer", > + &gFspNvsBufferVariableGuid, > NULL, > &BufferSize, > VariableData > @@ -90,8 +107,8 @@ SaveMemoryConfigEntryPoint ( > > if ( (EFI_ERROR(Status)) || BufferSize != DataSize || 0 != CompareMem > (HobData, VariableData, DataSize)) { > Status = gRT->SetVariable ( > - L"MemoryConfig", > - &gFspNonVolatileStorageHobGuid, > + L"FspNvsBuffer", > + &gFspNvsBufferVariableGuid, > (EFI_VARIABLE_NON_VOLATILE | > EFI_VARIABLE_BOOTSERVICE_ACCESS), > DataSize, > HobData > @@ -106,7 +123,7 @@ SaveMemoryConfigEntryPoint ( > // > Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL, > (VOID **)&VariableLock); > if (!EFI_ERROR(Status)) { > - Status = VariableLock->RequestToLock(VariableLock, L"MemoryConfig", > &gFspNonVolatileStorageHobGuid); > + Status = VariableLock->RequestToLock(VariableLock, L"FspNvsBuffer", > &gFspNvsBufferVariableGuid); > ASSERT_EFI_ERROR(Status); > } > > diff --git a/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c > b/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c > index 96dfd588dc..9e92387761 100644 > --- a/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c > +++ b/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c > @@ -1,6 +1,6 @@ > /** @file > > -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include <Library/DebugLib.h> > #include <Library/PeiServicesLib.h> > #include <Library/MemoryAllocationLib.h> > +#include <Library/LargeVariableReadLib.h> > #include <Ppi/ReadOnlyVariable2.h> > > /** > @@ -108,6 +109,72 @@ PeiGetVariable ( > return Status; > } > > +/** > + Returns the status whether get the large variable success. When variable > size > + is bigger than single UEFI variable size limit, The variable will be saved > in > + multiple UEFI variables. This function retrieves such large variable > through the > + ReadOnlyVariable2 PPI GetVariable(). > + The returned buffer is allocated using AllocatePage() to support > 64KB > and this > + buffer cannot be Free(). > + > + If Name is NULL, then ASSERT(). > + If Guid is NULL, then ASSERT(). > + If Value is NULL, then ASSERT(). > + > + @param[in] Name The pointer to a Null-terminated Unicode string. > + @param[in] Guid The pointer to an EFI_GUID structure > + @param[out] Value The buffer point saved the variable info. > + @param[out] Size The buffer size of the variable. > + > + @return EFI_OUT_OF_RESOURCES Allocate buffer failed. > + @return EFI_SUCCESS Find the specified variable. > + @return Others Errors Return errors from call to > gRT->GetVariable. > + > +**/ > +EFI_STATUS > +EFIAPI > +PeiGetLargeVariable ( > + IN CHAR16 *Name, > + IN EFI_GUID *Guid, > + OUT VOID **Value, > + OUT UINTN *Size OPTIONAL > + ) > +{ > + EFI_STATUS Status; > + UINTN VariableSize; > + VOID *VariableData; > + > + ASSERT (Name != NULL); > + ASSERT (Guid != NULL); > + ASSERT (Value != NULL); > + > + VariableSize = 0; > + VariableData = NULL; > + Status = GetLargeVariable (Name, Guid, &VariableSize, NULL); > + if (Status == EFI_BUFFER_TOO_SMALL) { > + VariableData = AllocatePages (EFI_SIZE_TO_PAGES (VariableSize)); > + if (VariableData == NULL) { > + DEBUG ((DEBUG_ERROR, "Error: Cannot create VariableData, out of > memory!\n")); > + ASSERT (FALSE); > + return EFI_OUT_OF_RESOURCES; > + } > + Status = GetLargeVariable (Name, Guid, &VariableSize, VariableData); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Error: Unable to read UEFI variable Status: > %r\n", Status)); > + ASSERT_EFI_ERROR (Status); > + return Status; > + } > + if (Value != NULL) { > + *Value = VariableData; > + } > + if (Size != NULL) { > + *Size = VariableSize; > + } > + return EFI_SUCCESS; > + } > + return Status; > +} > + > EFI_PEI_FILE_HANDLE > InternalGetFfsHandleFromAnyFv ( > IN CONST EFI_GUID *NameGuid > diff --git > a/Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c > > b/Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c > index 5eeee12a3c..b7885dd6c2 100644 > --- > a/Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c > +++ > b/Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c > @@ -67,7 +67,7 @@ VarLibGetVariable ( > &gEfiPeiReadOnlyVariable2PpiGuid, > 0, > NULL, > - &VariablePpi > + (VOID **) &VariablePpi > ); > ASSERT_EFI_ERROR (Status); > if (EFI_ERROR (Status)) { > @@ -134,7 +134,7 @@ VarLibGetNextVariableName ( > &gEfiPeiReadOnlyVariable2PpiGuid, > 0, > NULL, > - &VariablePpi > + (VOID **) &VariablePpi > ); > ASSERT_EFI_ERROR (Status); > if (EFI_ERROR (Status)) { > diff --git > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf > > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf > index 0c8689a6f6..eac0880504 100644 > --- > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf > +++ > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf > @@ -1,7 +1,7 @@ > ### @file > # Component information file for SaveMemoryConfig module > # > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -28,6 +28,7 @@ > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > IntelFsp2Pkg/IntelFsp2Pkg.dec > + MinPlatformPkg/MinPlatformPkg.dec > > [Sources] > SaveMemoryConfig.c > @@ -39,6 +40,8 @@ > > [Guids] > gFspNonVolatileStorageHobGuid ## CONSUMES > + gFspNonVolatileStorageHob2Guid ## CONSUMES > + gFspNvsBufferVariableGuid ## PRODUCES > > [Depex] > gEfiVariableArchProtocolGuid AND > diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h > b/Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h > index d8b1a47c58..86ffb378ec 100644 > --- a/Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h > +++ b/Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h > @@ -1,6 +1,6 @@ > /** @file > > -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -38,6 +38,37 @@ PeiGetVariable ( > OUT UINTN *Size > ); > > +/** > + Returns the status whether get the large variable success. When variable > size > + is bigger than single UEFI variable size limit, The variable will be saved > in > + multiple UEFI variables. This function retrieves such large variable > through the > + ReadOnlyVariable2 PPI GetVariable(). > + The returned buffer is allocated using AllocatePage() to support > 64KB > and this > + buffer cannot be Free(). > + > + If Name is NULL, then ASSERT(). > + If Guid is NULL, then ASSERT(). > + If Value is NULL, then ASSERT(). > + > + @param[in] Name The pointer to a Null-terminated Unicode string. > + @param[in] Guid The pointer to an EFI_GUID structure > + @param[out] Value The buffer point saved the variable info. > + @param[out] Size The buffer size of the variable. > + > + @return EFI_OUT_OF_RESOURCES Allocate buffer failed. > + @return EFI_SUCCESS Find the specified variable. > + @return Others Errors Return errors from call to > gRT->GetVariable. > + > +**/ > +EFI_STATUS > +EFIAPI > +PeiGetLargeVariable ( > + IN CHAR16 *Name, > + IN EFI_GUID *Guid, > + OUT VOID **Value, > + OUT UINTN *Size OPTIONAL > + ); > + > /** > Finds the file in any FV and gets file Address and Size > > diff --git a/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.inf > b/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.inf > index 7e740172a0..bd57cf7870 100644 > --- a/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.inf > +++ b/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component information file for Board Init Test Library > # > -# Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -20,9 +20,11 @@ > PeiServicesLib > MemoryAllocationLib > DebugLib > + LargeVariableReadLib > > [Packages] > MdePkg/MdePkg.dec > + MinPlatformPkg/MinPlatformPkg.dec > > [Sources] > PeiLib.c > diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > index bcb42f0ef9..d6e80a66ce 100644 > --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > @@ -50,6 +50,7 @@ > gBdsEventBeforeConsoleAfterTrustedConsoleGuid = {0x51e49ff5, 0x28a9, > 0x4159, { 0xac, 0x8a, 0xb8, 0xc4, 0x88, 0xa7, 0xfd, 0xee}} > gBdsEventBeforeConsoleBeforeEndOfDxeGuid = {0xfcf26e41, 0xbda6, > 0x4633, { 0xb5, 0x73, 0xd4, 0xb8, 0x0e, 0x6d, 0xd0, 0x78}} > gBdsEventAfterConsoleReadyBeforeBootOptionGuid = {0x8eb3d5dc, 0xf4e7, > 0x4b57, { 0xa9, 0xe7, 0x27, 0x39, 0x10, 0xf2, 0x18, 0x9f}} > + gFspNvsBufferVariableGuid = {0x9c7715cd, 0x8d66, > 0x4d2a, { 0x90, 0x0d, 0x01, 0x45, 0x9a, 0x57, 0x59, 0x6b}} > > [LibraryClasses] > > -- > 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#81769): https://edk2.groups.io/g/devel/message/81769 Mute This Topic: https://groups.io/mt/86164705/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
