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. (stored across multiple variables) 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 | 109 ++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------- Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c | 2 +- Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------- Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c | 4 ++-- Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf | 8 ++++++-- Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc | 1 + Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h | 40 +++++++++++++++++++++++++++++++++++----- Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.inf | 4 +++- Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec | 1 + 9 files changed, 176 insertions(+), 82 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c index 41ed2550bd..820585f676 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 **/ @@ -16,7 +16,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include <Guid/GlobalVariable.h> #include <Library/MemoryAllocationLib.h> #include <Library/BaseMemoryLib.h> -#include <Protocol/VariableLock.h> +#include <Library/LargeVariableReadLib.h> +#include <Library/LargeVariableWriteLib.h> +#include <Guid/FspNonVolatileStorageHob2.h> /** This is the standard EFI driver point that detects whether there is a @@ -40,86 +42,71 @@ SaveMemoryConfigEntryPoint ( VOID *VariableData; UINTN DataSize; UINTN BufferSize; - EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock; + BOOLEAN DataIsIdentical; - DataSize = 0; - VariableData = NULL; - GuidHob = NULL; - HobData = NULL; + DataSize = 0; + BufferSize = 0; + VariableData = NULL; + GuidHob = NULL; + HobData = NULL; + DataIsIdentical = FALSE; // // 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 + // Check if the presently saved data is identical to the data given by MRC/FSP // - BufferSize = DataSize; - VariableData = AllocatePool (BufferSize); - if (VariableData == NULL) { - return EFI_UNSUPPORTED; - } - Status = gRT->GetVariable ( - L"MemoryConfig", - &gFspNonVolatileStorageHobGuid, - NULL, - &BufferSize, - VariableData - ); - + Status = GetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, &BufferSize, NULL); if (Status == EFI_BUFFER_TOO_SMALL) { - FreePool (VariableData); - VariableData = AllocatePool (BufferSize); - if (VariableData == NULL) { - return EFI_UNSUPPORTED; + if (BufferSize == DataSize) { + VariableData = AllocatePool (BufferSize); + if (VariableData != NULL) { + Status = GetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, &BufferSize, VariableData); + if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 == CompareMem (HobData, VariableData, DataSize))) { + DataIsIdentical = TRUE; + } + FreePool (VariableData); + } } - - Status = gRT->GetVariable ( - L"MemoryConfig", - &gFspNonVolatileStorageHobGuid, - NULL, - &BufferSize, - VariableData - ); } + Status = EFI_SUCCESS; - if ( (EFI_ERROR(Status)) || BufferSize != DataSize || 0 != CompareMem (HobData, VariableData, DataSize)) { - Status = gRT->SetVariable ( - L"MemoryConfig", - &gFspNonVolatileStorageHobGuid, - (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS), - DataSize, - HobData - ); + if (!DataIsIdentical) { + Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, TRUE, DataSize, HobData); ASSERT_EFI_ERROR (Status); - - DEBUG((DEBUG_INFO, "Restored Size is 0x%x\n", DataSize)); - } - - // - // Mark MemoryConfig to read-only if the Variable Lock protocol exists - // - Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&VariableLock); - if (!EFI_ERROR(Status)) { - Status = VariableLock->RequestToLock(VariableLock, L"MemoryConfig", &gFspNonVolatileStorageHobGuid); - ASSERT_EFI_ERROR(Status); + DEBUG ((DEBUG_INFO, "Saved size of FSP / MRC Training Data: 0x%x\n", DataSize)); + } else { + DEBUG ((DEBUG_INFO, "FSP / MRC Training Data is identical to data from last boot, no need to save.\n")); } - - FreePool (VariableData); - } else { - DEBUG((DEBUG_INFO, "Memory save size is %d\n", DataSize)); } } else { DEBUG((DEBUG_ERROR, "Memory S3 Data HOB was not found\n")); } // - // This driver does not produce any protocol services, so always unload it. + // This driver cannot be unloaded because DxeRuntimeVariableWriteLib constructor will register ExitBootServices callback. // - return EFI_REQUEST_UNLOAD_IMAGE; + return EFI_SUCCESS; } diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c index 703aca6fd8..e4b97ef1df 100644 --- a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c @@ -343,7 +343,7 @@ SetLargeVariable ( // Check that it is possible to store the data using less than // MAX_VARIABLE_SPLIT variables // - if ((DataSize / (VariableSplitSize - MAX_VARIABLE_SPLIT_DIGITS)) > MAX_VARIABLE_SPLIT) { + if ((DataSize / ((UINTN) VariableSplitSize - MAX_VARIABLE_SPLIT_DIGITS)) > MAX_VARIABLE_SPLIT) { DEBUG (( DEBUG_ERROR, "SetLargeVariable: More than %d variables are needed to store the data, which exceeds the maximum supported\n", diff --git a/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c b/Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c index 96dfd588dc..3f8cf761a7 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,13 +9,17 @@ 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> /** - Returns the status whether get the variable success. The function retrieves - variable through the ReadOnlyVariable2 PPI GetVariable(). The - returned buffer is allocated using AllocatePool(). The caller is responsible - for freeing this buffer with FreePool(). + Returns the status whether get the variable success. The function retrieves + variable through the ReadOnlyVariable2 PPI GetVariable(). + + If the *Size is 0, the returned buffer is allocated using AllocatePool(). + The buffer is not expected to be freed as PEI does not support a FreePool(). + + If the *Size is non-0, this function just uses caller allocated *Value. If Name is NULL, then ASSERT(). If Guid is NULL, then ASSERT(). @@ -108,6 +112,71 @@ PeiGetVariable ( return Status; } +/** + This function returns a "large variable". A large variable is stored across multiple + UEFI Variables. This function retrieves the multiple UEFI Variables using + ReadOnlyVariable2 PPI GetVariable(). + The function uses AllocatePages () to allocate the buffer. + The caller is responsible for freeing this buffer with FreePages(). + + 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 @@ -139,7 +208,7 @@ InternalGetFfsHandleFromAnyFv ( /** Finds the file in any FV and gets file Address and Size - + @param[in] NameGuid File GUID @param[out] Address Pointer to the File Address @param[out] Size Pointer to File Size @@ -162,7 +231,7 @@ PeiGetFfsFromAnyFv ( if (FfsHandle == NULL) { return EFI_NOT_FOUND; } - + // // Need get size // @@ -185,7 +254,7 @@ PeiGetFfsFromAnyFv ( @param[in] SectionInstance The Instance of Section to be found @param[out] OutSectionBuffer The section found, including SECTION_HEADER @param[out] OutSectionSize The size of section found, including SECTION_HEADER - + @retval EFI_SUCCESS Successfull in reading the section from FV **/ EFI_STATUS @@ -263,7 +332,7 @@ PeiGetSectionFromAnyFv ( EFI_COMMON_SECTION_HEADER *Section; VOID *FileBuffer; UINTN FileBufferSize; - + Status = PeiGetFfsFromAnyFv (NameGuid, &FileBuffer, &FileBufferSize); if (EFI_ERROR(Status)) { return Status; @@ -282,6 +351,6 @@ PeiGetSectionFromAnyFv ( *Size = SECTION_SIZE(Section) - sizeof (EFI_COMMON_SECTION_HEADER); *Address = (UINT8 *)*Address + sizeof (EFI_COMMON_SECTION_HEADER); } - + return EFI_SUCCESS; } 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..e2dbd2fb49 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 # @@ -23,11 +23,14 @@ DebugLib MemoryAllocationLib BaseMemoryLib + LargeVariableReadLib + LargeVariableWriteLib [Packages] MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec IntelFsp2Pkg/IntelFsp2Pkg.dec + MinPlatformPkg/MinPlatformPkg.dec [Sources] SaveMemoryConfig.c @@ -35,10 +38,11 @@ [Protocols] gEfiVariableArchProtocolGuid ## CONSUMES gEfiVariableWriteArchProtocolGuid ## CONSUMES - gEdkiiVariableLockProtocolGuid [Guids] gFspNonVolatileStorageHobGuid ## CONSUMES + gFspNonVolatileStorageHob2Guid ## CONSUMES + gFspNvsBufferVariableGuid ## PRODUCES [Depex] gEfiVariableArchProtocolGuid AND diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc index d3c668d441..c12189bd9a 100644 --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc @@ -41,6 +41,7 @@ DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf !endif PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf + VariableReadLib|MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.inf [LibraryClasses.common.PEI_CORE] TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h b/Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h index d8b1a47c58..eed6502d84 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 **/ @@ -11,11 +11,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include <PiPei.h> /** - Returns the status whether get the variable success. The function retrieves + Returns the status whether get the variable success. The function retrieves variable through the ReadOnlyVariable2 PPI GetVariable(). If the *Size is 0, the returned buffer is allocated using AllocatePool(). - The caller is responsible for freeing this buffer with FreePool(). + The buffer is not expected to be freed as PEI does not support a FreePool(). If the *Size is non-0, this function just uses caller allocated *Value. @@ -38,9 +38,39 @@ PeiGetVariable ( OUT UINTN *Size ); +/** + This function returns a "large variable". A large variable is stored across multiple + UEFI Variables. This function retrieves the multiple UEFI Variables using + ReadOnlyVariable2 PPI GetVariable(). + The function uses AllocatePages () to allocate the buffer. + The caller is responsible for freeing this buffer with FreePages(). + + 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 - + @param[in] NameGuid File GUID @param[out] Address Pointer to the File Address @param[out] Size Pointer to File Size @@ -57,7 +87,7 @@ PeiGetFfsFromAnyFv ( /** Finds the section in any FV and gets section Address and Size - + @param[in] NameGuid File GUID @param[in] SectionType The SectionType of Section to be found @param[in] SectionInstance The Instance of Section to be found 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 (#82012): https://edk2.groups.io/g/devel/message/82012 Mute This Topic: https://groups.io/mt/86308172/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-