Variable: Consume Variable Flash Info

Sami Mujawar Mon, 25 Apr 2022 09:05:58 -0700

Hi Michael,

Thank you for this patch.


These changes look good to me.

Reviewed-by: Sami Mujawar <sami.muja...@arm.com>

Regards,

Sami Mujawar

On 12/04/2022 05:29 pm, Michael Kubacki via groups.io wrote:

From: Michael Kubacki <michael.kuba...@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479

Updates VariableRuntimeDxe, VariableSmm, and VariableStandaloneMm
to acquire variable flash information from the Variable Flash
Information library.

Cc: Jian J Wang <jian.j.w...@intel.com>
Cc: Hao A Wu <hao.a...@intel.com>
Cc: Liming Gao <gaolim...@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com>
---
  MdeModulePkg/Universal/Variable/Pei/Variable.c                      | 14 
+++++++++-----
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c            | 16 
++++++++++++----
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c    | 14 
++++++++++----
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c            | 17 
+++++++++++++----
  MdeModulePkg/Universal/Variable/Pei/Variable.h                      |  2 ++
  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                 |  5 ++---
  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h               |  7 
++-----
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf   |  5 ++---
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf          |  5 ++---
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf |  5 ++---
  10 files changed, 56 insertions(+), 34 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c 
b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index b36dd0de67b2..26a4c73b45a5 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -567,11 +567,13 @@ GetVariableStore (
    OUT VARIABLE_STORE_INFO  *StoreInfo
    )
  {
+  EFI_STATUS                            Status;
    EFI_HOB_GUID_TYPE                     *GuidHob;
    EFI_FIRMWARE_VOLUME_HEADER            *FvHeader;
    VARIABLE_STORE_HEADER                 *VariableStoreHeader;
    EFI_PHYSICAL_ADDRESS                  NvStorageBase;
    UINT32                                NvStorageSize;
+  UINT64                                NvStorageSize64;
    FAULT_TOLERANT_WRITE_LAST_WRITE_DATA  *FtwLastWriteData;
    UINT32                                BackUpOffset;

@@ -591,11 +593,13 @@ GetVariableStore (

          // Emulated non-volatile variable mode is not enabled.
          //

- NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);

-        NvStorageBase = (EFI_PHYSICAL_ADDRESS)(PcdGet64 
(PcdFlashNvStorageVariableBase64) != 0 ?
-                                               PcdGet64 
(PcdFlashNvStorageVariableBase64) :
-                                               PcdGet32 
(PcdFlashNvStorageVariableBase)
-                                               );
+        Status = GetVariableFlashNvStorageInfo (&NvStorageBase, 
&NvStorageSize64);
+        ASSERT_EFI_ERROR (Status);
+
+        Status = SafeUint64ToUint32 (NvStorageSize64, &NvStorageSize);
+        // This driver currently assumes the size will be UINT32 so assert the 
value is safe for now.
+        ASSERT_EFI_ERROR (Status);
+
          ASSERT (NvStorageBase != 0);

//

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 03fec3048dc4..d5c409c914d1 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -423,6 +423,8 @@ FtwNotificationEvent (
    EFI_PHYSICAL_ADDRESS                VariableStoreBase;
    UINT64                              VariableStoreLength;
    UINTN                               FtwMaxBlockSize;
+  UINT32                              NvStorageVariableSize;
+  UINT64                              NvStorageVariableSize64;

//

    // Ensure FTW protocol is installed.
@@ -432,14 +434,20 @@ FtwNotificationEvent (
      return;
    }

+ Status = GetVariableFlashNvStorageInfo (&NvStorageVariableBase, &NvStorageVariableSize64);

+  ASSERT_EFI_ERROR (Status);
+
+  Status = SafeUint64ToUint32 (NvStorageVariableSize64, 
&NvStorageVariableSize);
+  // This driver currently assumes the size will be UINT32 so assert the value 
is safe for now.
+  ASSERT_EFI_ERROR (Status);
+
+  VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache->HeaderLength;
+
    Status = FtwProtocol->GetMaxBlockSize (FtwProtocol, &FtwMaxBlockSize);
    if (!EFI_ERROR (Status)) {
-    ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <= FtwMaxBlockSize);
+    ASSERT (NvStorageVariableSize <= FtwMaxBlockSize);
    }

- NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;

-  VariableStoreBase     = NvStorageVariableBase + 
mNvFvHeaderCache->HeaderLength;
-
    //
    // Let NonVolatileVariableBase point to flash variable store base directly 
after FTW ready.
    //
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
index 5e9d40b67ac2..9e2d8fe0fe0c 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
@@ -142,6 +142,7 @@ InitRealNonVolatileVariableStore (
    EFI_PHYSICAL_ADDRESS                  NvStorageBase;
    UINT8                                 *NvStorageData;
    UINT32                                NvStorageSize;
+  UINT64                                NvStorageSize64;
    FAULT_TOLERANT_WRITE_LAST_WRITE_DATA  *FtwLastWriteData;
    UINT32                                BackUpOffset;
    UINT32                                BackUpSize;
@@ -153,19 +154,24 @@ InitRealNonVolatileVariableStore (

mVariableModuleGlobal->FvbInstance = NULL;+ Status = GetVariableFlashNvStorageInfo (&NvStorageBase, &NvStorageSize64);

+  ASSERT_EFI_ERROR (Status);
+
+  Status = SafeUint64ToUint32 (NvStorageSize64, &NvStorageSize);
+  // This driver currently assumes the size will be UINT32 so assert the value 
is safe for now.
+  ASSERT_EFI_ERROR (Status);
+
+  ASSERT (NvStorageBase != 0);
+
    //
    // Allocate runtime memory used for a memory copy of the FLASH region.
    // Keep the memory and the FLASH in sync as updates occur.
    //
-  NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
    NvStorageData = AllocateRuntimeZeroPool (NvStorageSize);
    if (NvStorageData == NULL) {
      return EFI_OUT_OF_RESOURCES;
    }

- NvStorageBase = NV_STORAGE_VARIABLE_BASE;

-  ASSERT (NvStorageBase != 0);
-
    //
    // Copy NV storage data to the memory buffer.
    //
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 517cae7b00f8..5253c328dcd9 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -1084,6 +1084,8 @@ SmmFtwNotificationEvent (
    EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;
    EFI_PHYSICAL_ADDRESS                    NvStorageVariableBase;
    UINTN                                   FtwMaxBlockSize;
+  UINT32                                  NvStorageVariableSize;
+  UINT64                                  NvStorageVariableSize64;

if (mVariableModuleGlobal->FvbInstance != NULL) {

      return EFI_SUCCESS;
@@ -1097,14 +1099,21 @@ SmmFtwNotificationEvent (
      return Status;
    }

+ Status = GetVariableFlashNvStorageInfo (&NvStorageVariableBase, &NvStorageVariableSize64);

+  ASSERT_EFI_ERROR (Status);
+
+  Status = SafeUint64ToUint32 (NvStorageVariableSize64, 
&NvStorageVariableSize);
+  // This driver currently assumes the size will be UINT32 so assert the value 
is safe for now.
+  ASSERT_EFI_ERROR (Status);
+
+  ASSERT (NvStorageVariableBase != 0);
+  VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache->HeaderLength;
+
    Status = FtwProtocol->GetMaxBlockSize (FtwProtocol, &FtwMaxBlockSize);
    if (!EFI_ERROR (Status)) {
-    ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <= FtwMaxBlockSize);
+    ASSERT (NvStorageVariableSize <= FtwMaxBlockSize);
    }

- NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;

-  VariableStoreBase     = NvStorageVariableBase + 
mNvFvHeaderCache->HeaderLength;
-
    //
    // Let NonVolatileVariableBase point to flash variable store base directly 
after FTW ready.
    //
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.h 
b/MdeModulePkg/Universal/Variable/Pei/Variable.h
index 7f9ad5bfc357..51effbf79987 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.h
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.h
@@ -20,6 +20,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
  #include <Library/BaseMemoryLib.h>
  #include <Library/PeiServicesTablePointerLib.h>
  #include <Library/PeiServicesLib.h>
+#include <Library/SafeIntLib.h>
+#include <Library/VariableFlashInfoLib.h>

#include <Guid/VariableFormat.h>

  #include <Guid/VariableIndexTable.h>
diff --git a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf 
b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
index 7cbdd2385e8f..7264a24bdf71 100644
--- a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+++ b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
@@ -39,6 +39,8 @@ [LibraryClasses]
    DebugLib
    PeiServicesTablePointerLib
    PeiServicesLib
+  SafeIntLib
+  VariableFlashInfoLib

[Guids]

    ## CONSUMES             ## GUID # Variable store header
@@ -59,9 +61,6 @@ [Ppis]
    gEfiPeiReadOnlyVariable2PpiGuid   ## PRODUCES

[Pcd]

-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ## 
SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable         ## 
SOMETIMES_CONSUMES

[Depex]

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index 31e408976a35..a668abb82b15 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
@@ -31,6 +31,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
  #include <Library/MemoryAllocationLib.h>
  #include <Library/AuthVariableLib.h>
  #include <Library/VarCheckLib.h>
+#include <Library/VariableFlashInfoLib.h>
+#include <Library/SafeIntLib.h>
  #include <Guid/GlobalVariable.h>
  #include <Guid/EventGroup.h>
  #include <Guid/VariableFormat.h>
@@ -40,11 +42,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

#include "PrivilegePolymorphic.h"-#define NV_STORAGE_VARIABLE_BASE (EFI_PHYSICAL_ADDRESS)\

-                                   (PcdGet64 (PcdFlashNvStorageVariableBase64) 
!= 0 ? \
-                                    PcdGet64 (PcdFlashNvStorageVariableBase64) 
: \
-                                    PcdGet32 (PcdFlashNvStorageVariableBase))
-
  #define EFI_VARIABLE_ATTRIBUTES_MASK  (EFI_VARIABLE_NON_VOLATILE |\
                                        EFI_VARIABLE_BOOTSERVICE_ACCESS | \
                                        EFI_VARIABLE_RUNTIME_ACCESS | \
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index c9434df631ee..3858adf6739d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -71,8 +71,10 @@ [LibraryClasses]
    TpmMeasurementLib
    AuthVariableLib
    VarCheckLib
+  VariableFlashInfoLib
    VariablePolicyLib
    VariablePolicyHelperLib
+  SafeIntLib

[Protocols]

    gEfiFirmwareVolumeBlockProtocolGuid           ## CONSUMES
@@ -125,9 +127,6 @@ [Guids]
    gEfiImageSecurityDatabaseGuid

[Pcd]

-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ## 
SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ## CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize                 ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize             ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize         ## 
CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index eaa97a01c6e5..8c552b87e080 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -80,8 +80,10 @@ [LibraryClasses]
    AuthVariableLib
    VarCheckLib
    UefiBootServicesTableLib
+  VariableFlashInfoLib
    VariablePolicyLib
    VariablePolicyHelperLib
+  SafeIntLib

[Protocols]

    gEfiSmmFirmwareVolumeBlockProtocolGuid        ## CONSUMES
@@ -127,9 +129,6 @@ [Guids]
    gEdkiiVarErrorFlagGuid

[Pcd]

-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase       ## 
SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize                  ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize              ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize          ## 
CONSUMES
diff --git 
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
index d8c4f77e7f1f..f09bed40cf51 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
@@ -73,9 +73,11 @@ [LibraryClasses]
    HobLib
    MemoryAllocationLib
    MmServicesTableLib
+  SafeIntLib
    StandaloneMmDriverEntryPoint
    SynchronizationLib
    VarCheckLib
+  VariableFlashInfoLib
    VariablePolicyLib
    VariablePolicyHelperLib

@@ -120,9 +122,6 @@ [Guids]

    gEdkiiVarErrorFlagGuid

[Pcd]

-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase       ## 
SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize                  ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize              ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize          ## 
CONSUMES



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89275): https://edk2.groups.io/g/devel/message/89275
Mute This Topic: https://groups.io/mt/90421973/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 3/8] MdeModulePkg/Variable: Consume Variable Flash Info

Reply via email to