[edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

Nate DeSimone posted 4 patches 1 month ago

[edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

Posted by Nate DeSimone 1 month ago
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@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
Reviewed-by: Isaac Oram <isaac.w.oram@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
+
+**/
+
+#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
+
+//
+// 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"));
+      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;
+}
-- 
2.27.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73739): https://edk2.groups.io/g/devel/message/73739
Mute This Topic: https://groups.io/mt/81898861/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

Posted by Michael Kubacki 1 month ago
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@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Reviewed-by: Isaac Oram <isaac.w.oram@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

Posted by Nate DeSimone 1 month ago
Hi Michael,

Thank you for the great feedback! I believe I have all of it addressed in V4.

Thanks,
Nate

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
Sent: Tuesday, April 6, 2021 3:01 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Michael Kubacki <michael.kubacki@microsoft.com>; Oram, Isaac W <isaac.w.oram@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

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@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Reviewed-by: Isaac Oram <isaac.w.oram@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/LargeVa
> riableReadLib.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/AuthVariableL
> ibNull.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/VariablePolic
> yLib.inf
> -  
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V
> ariablePolicyHelperLib.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/LargeVariableReadL
> +++ ib.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/BaseL
> argeVariableReadLib.inf 
> b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseL
> argeVariableReadLib.inf
> new file mode 100644
> index 0000000000..822febd62b
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/B
> +++ aseLargeVariableReadLib.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/Large
> VariableReadLib.c 
> b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/Large
> VariableReadLib.c
> new file mode 100644
> index 0000000000..115f3aeb17
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/L
> +++ argeVariableReadLib.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 (#73754): https://edk2.groups.io/g/devel/message/73754
Mute This Topic: https://groups.io/mt/81898861/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-