[edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib
Michael Kubacki
mikuback at linux.microsoft.com
Tue Apr 6 22:01:03 UTC 2021
Hi Nate,
Feedback is inline.
Please carry over applicable feedback to LargeVariableWriteLib, I did
not duplicate the response there.
Thanks,
Michael
On 4/6/2021 12:24 PM, Nate DeSimone wrote:
> LargeVariableReadLib is used to retrieve large data sets using
> the UEFI Variable Services. At time of writting, most UEFI
> Variable Services implementations to not allow more than 64KB
> of data to be stored in a single UEFI variable. This library
> will split data sets across multiple variables as needed.
>
> It adds the GetLargeVariable() API to provide this service.
>
> The primary use for this library is to create binary compatible
> drivers and OpROMs which need to work both with TianoCore and
> other UEFI PI implementations. When customizing and recompiling
> the platform firmware image is possible, adjusting the value of
> PcdMaxVariableSize may provide a simpler solution to this
> problem.
>
> Cc: Chasel Chiu <chasel.chiu at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Michael Kubacki <michael.kubacki at microsoft.com>
> Cc: Isaac Oram <isaac.w.oram at intel.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone at intel.com>
> Reviewed-by: Isaac Oram <isaac.w.oram at intel.com>
> ---
> .../Include/Dsc/CoreCommonLib.dsc | 5 +-
> .../Include/Library/LargeVariableReadLib.h | 56 +++++
> .../BaseLargeVariableReadLib.inf | 50 +++++
> .../LargeVariableReadLib.c | 199 ++++++++++++++++++
> 4 files changed, 308 insertions(+), 2 deletions(-)
> create mode 100644 Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
> create mode 100644 Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
> create mode 100644 Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c
>
> diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> index cf2940cf02..5f2ad3f0f0 100644
> --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> @@ -135,13 +135,14 @@
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
> -
> +
> !if gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable == TRUE
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> !endif
>
> SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
> + LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
>
> #
> # CryptLib
> @@ -165,4 +166,4 @@
>
> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> - VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
> \ No newline at end of file
> + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
> diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
> new file mode 100644
> index 0000000000..5579492727
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
> @@ -0,0 +1,56 @@
> +/** @file
> + Large Variable Read Lib
> +
> + This library is used to retrieve large data sets using the UEFI Variable
> + Services. At time of writing, most UEFI Variable Services implementations to
> + not allow more than 64KB of data to be stored in a single UEFI variable. This
> + library will split data sets across multiple variables as needed.
> +
> + In the case where more than one variable is needed to store the data, an
> + integer number will be added to the end of the variable name. This number
> + will be incremented for each variable as needed to retrieve the entire data
> + set.
> +
> + The primary use for this library is to create binary compatible drivers
> + and OpROMs which need to work both with TianoCore and other UEFI PI
> + implementations. When customizing and recompiling the platform firmware image
> + is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
> + solution to this problem.
> +
> + Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
Similar comment to VariableReadLib/VariableWriteLib, header guard is
missing.
> +#include <Uefi/UefiBaseType.h>
> +
> +/**
> + Returns the value of a large variable.
> +
> + @param[in] VariableName A Null-terminated string that is the name of the vendor's
> + variable.
> + @param[in] VendorGuid A unique identifier for the vendor.
> + @param[in, out] DataSize On input, the size in bytes of the return Data buffer.
> + On output the size of data returned in Data.
> + @param[out] Data The buffer to return the contents of the variable. May be NULL
> + with a zero DataSize in order to determine the size buffer needed.
> +
> + @retval EFI_SUCCESS The function completed successfully.
> + @retval EFI_NOT_FOUND The variable was not found.
> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the result.
> + @retval EFI_INVALID_PARAMETER VariableName is NULL.
> + @retval EFI_INVALID_PARAMETER VendorGuid is NULL.
> + @retval EFI_INVALID_PARAMETER DataSize is NULL.
> + @retval EFI_INVALID_PARAMETER The DataSize is not too small and Data is NULL.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
> + @retval EFI_SECURITY_VIOLATION The variable could not be retrieved due to an authentication failure.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetLargeVariable (
> + IN CHAR16 *VariableName,
> + IN EFI_GUID *VendorGuid,
> + IN OUT UINTN *DataSize,
> + OUT VOID *Data OPTIONAL
> + );
> diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
> new file mode 100644
> index 0000000000..822febd62b
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
> @@ -0,0 +1,50 @@
> +## @file
> +# Component description file for Large Variable Read Library
> +#
> +# This library is used to retrieve large data sets using the UEFI Variable
> +# Services. At time of writing, most UEFI Variable Services implementations to
> +# not allow more than 64KB of data to be stored in a single UEFI variable. This
> +# library will split data sets across multiple variables as needed.
> +#
> +# In the case where more than one variable is needed to store the data, an
> +# integer number will be added to the end of the variable name. This number
> +# will be incremented for each variable as needed to retrieve the entire data
> +# set.
> +#
> +# The primary use for this library is to create binary compatible drivers
> +# and OpROMs which need to work both with TianoCore and other UEFI PI
> +# implementations. When customizing and recompiling the platform firmware image
> +# is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
> +# solution to this problem.
> +#
> +# Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = BaseLargeVariableReadLib
> + FILE_GUID = 4E9D7D31-A7A0-4004-AE93-D12F1AB08730
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = LargeVariableReadLib
> +
> +#
> +# VALID_ARCHITECTURES = IA32 X64 EBC
> +#
> +
> +[Sources]
> + LargeVariableReadLib.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + MinPlatformPkg/MinPlatformPkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + BaseMemoryLib
> + DebugLib
> + PrintLib
> + VariableReadLib
> diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c
> new file mode 100644
> index 0000000000..115f3aeb17
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c
> @@ -0,0 +1,199 @@
> +/** @file
> + Large Variable Read Lib
> +
> + This library is used to retrieve large data sets using the UEFI Variable
> + Services. At time of writing, most UEFI Variable Services implementations to
> + not allow more than 64KB of data to be stored in a single UEFI variable. This
> + library will split data sets across multiple variables as needed.
> +
> + In the case where more than one variable is needed to store the data, an
> + integer number will be added to the end of the variable name. This number
> + will be incremented for each variable as needed to retrieve the entire data
> + set.
> +
> + Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/VariableReadLib.h>
> +
> +//
> +// 1024 was choosen because this is the size of the SMM communication buffer
> +// used by VariableDxeSmm to transfer the VariableName from DXE to SMM. Choosing
> +// the same size will prevent this library from limiting variable names any
> +// more than the MdeModulePkg implementation of UEFI Variable Services does.
> +//
> +#define MAX_VARIABLE_NAME_SIZE 1024
> +
It would be nice to share a definition of these values with
LargeVariableWriteLib to prevent the two from potentially getting out of
sync.
> +//
> +// The 2012 Windows Hardware Requirements specified a minimum variable size of
> +// 32KB. By setting the maximum allowed number of variables to 0x20000, this
> +// allows up to 4GB of data to be stored on most UEFI implementations in
> +// existence. Older UEFI implementations were known to only provide 8KB per
> +// variable. In this case, up to 1GB can be stored. Since 1GB vastly exceeds the
> +// size of any known NvStorage FV, choosing this number should effectively
> +// enable all available NvStorage space to be used to store the given data.
> +//
> +#define MAX_VARIABLE_SPLIT 131072 // 0x20000
> +
> +//
> +// There are 6 digits in the number 131072, which means the length of the string
> +// representation of this number will be at most 6 characters long.
> +//
> +#define MAX_VARIABLE_SPLIT_DIGITS 6
> +
> +/**
> + Returns the value of a large variable.
> +
> + @param[in] VariableName A Null-terminated string that is the name of the vendor's
> + variable.
> + @param[in] VendorGuid A unique identifier for the vendor.
> + @param[in, out] DataSize On input, the size in bytes of the return Data buffer.
> + On output the size of data returned in Data.
> + @param[out] Data The buffer to return the contents of the variable. May be NULL
> + with a zero DataSize in order to determine the size buffer needed.
> +
> + @retval EFI_SUCCESS The function completed successfully.
> + @retval EFI_NOT_FOUND The variable was not found.
> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the result.
> + @retval EFI_INVALID_PARAMETER VariableName is NULL.
> + @retval EFI_INVALID_PARAMETER VendorGuid is NULL.
> + @retval EFI_INVALID_PARAMETER DataSize is NULL.
> + @retval EFI_INVALID_PARAMETER The DataSize is not too small and Data is NULL.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
> + @retval EFI_SECURITY_VIOLATION The variable could not be retrieved due to an authentication failure.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetLargeVariable (
> + IN CHAR16 *VariableName,
> + IN EFI_GUID *VendorGuid,
> + IN OUT UINTN *DataSize,
> + OUT VOID *Data OPTIONAL
> + )
> +{
> + CHAR16 TempVariableName[MAX_VARIABLE_NAME_SIZE];
> + EFI_STATUS Status;
> + UINTN TotalSize;
> + UINTN VarDataSize;
> + UINTN Index;
> + UINTN VariableSize;
> + UINTN BytesRemaining;
> + UINT8 *OffsetPtr;
> +
> + VarDataSize = 0;
> +
> + //
> + // First check if a variable with the given name exists
> + //
> + Status = VarLibGetVariable (VariableName, VendorGuid, NULL, &VarDataSize, NULL);
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + if (*DataSize >= VarDataSize) {
> + if (Data == NULL) {
> + Status = EFI_INVALID_PARAMETER;
> + goto Done;
> + }
> + DEBUG ((DEBUG_INFO, "GetLargeVariable: Single Variable Found\n"));
This is somewhat subjective but do you think the message above should
remain DEBUG_INFO? It seems like DEBUG_VERBOSE might be appropriate
(same to counterpart messages in other places).
> + Status = VarLibGetVariable (VariableName, VendorGuid, NULL, DataSize, Data);
> + goto Done;
> + } else {
> + *DataSize = VarDataSize;
> + Status = EFI_BUFFER_TOO_SMALL;
> + goto Done;
> + }
> +
> + } else if (Status == EFI_NOT_FOUND) {
> + //
> + // Check if the first variable of a multi-variable set exists
> + //
> + if (StrLen (VariableName) >= (MAX_VARIABLE_NAME_SIZE - MAX_VARIABLE_SPLIT_DIGITS)) {
> + DEBUG ((DEBUG_ERROR, "GetLargeVariable: Variable name too long\n"));
> + Status = EFI_OUT_OF_RESOURCES;
> + goto Done;
> + }
> +
> + VarDataSize = 0;
> + Index = 0;
> + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
> + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", VariableName, Index);
> + Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL, &VarDataSize, NULL);
> +
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + //
> + // The first variable exists. Calculate the total size of all the variables.
> + //
> + DEBUG ((DEBUG_INFO, "GetLargeVariable: Multiple Variables Found\n"));
> + TotalSize = 0;
> + for (Index = 0; Index < MAX_VARIABLE_SPLIT; Index++) {
> + VarDataSize = 0;
> + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
> + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", VariableName, Index);
> + Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL, &VarDataSize, NULL);
> + if (Status != EFI_BUFFER_TOO_SMALL) {
> + break;
> + }
> + TotalSize += VarDataSize;
> + }
> + DEBUG ((DEBUG_INFO, "TotalSize = %d, NumVariables = %d\n", TotalSize, Index));
> +
> + //
> + // Check if the user provided a large enough buffer
> + //
> + if (*DataSize >= TotalSize) {
> + if (Data == NULL) {
> + Status = EFI_INVALID_PARAMETER;
> + goto Done;
> + }
> +
> + //
> + // Read the data from all variables
> + //
> + OffsetPtr = (UINT8 *) Data;
> + BytesRemaining = *DataSize;
> + for (Index = 0; Index < MAX_VARIABLE_SPLIT; Index++) {
> + VarDataSize = 0;
> + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
> + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", VariableName, Index);
> + VariableSize = BytesRemaining;
> + DEBUG ((DEBUG_INFO, "Reading %s, Guid = %g,", TempVariableName, VendorGuid));
> + Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL, &VariableSize, (VOID *) OffsetPtr);
> + DEBUG ((DEBUG_INFO, " Size %d\n", VariableSize));
> + if (EFI_ERROR (Status)) {
> + if (Status == EFI_NOT_FOUND) {
> + DEBUG ((DEBUG_INFO, "No more variables found\n"));
> + Status = EFI_SUCCESS; // The end has been reached
> + }
> + goto Done;
> + }
> +
> + if (VariableSize < BytesRemaining) {
> + BytesRemaining -= VariableSize;
> + OffsetPtr += VariableSize;
> + } else {
> + DEBUG ((DEBUG_INFO, "All data has been read\n"));
> + BytesRemaining = 0;
> + break;
> + }
> + } //End of for loop
> +
> + goto Done;
> + } else {
> + *DataSize = TotalSize;
> + Status = EFI_BUFFER_TOO_SMALL;
> + goto Done;
> + }
> + } else {
> + Status = EFI_NOT_FOUND;
> + }
> + }
> +
> +Done:
> + DEBUG ((DEBUG_ERROR, "GetLargeVariable: Status = %r\n", Status));
> + return Status;
> +}
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73747): https://edk2.groups.io/g/devel/message/73747
Mute This Topic: https://groups.io/mt/81898861/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list