public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v1 0/3] Add Variable Flash Info HOB
@ 2022-04-06 16:26 Michael Kubacki
  2022-04-06 16:26 ` [PATCH v1 1/3] MdeModulePkg: " Michael Kubacki
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Michael Kubacki @ 2022-04-06 16:26 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao

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

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

The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
VariableStandaloneMm, etc. (and their dependent protocol/library
stack), typically acquire UEFI variable store flash information
with PCDs declared in MdeModulePkg.

For example:
[Pcd]
  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize

These PCDs work as-is in the StandaloneMm driver if they are not
dynamic such as Dynamic or DynamicEx because PCD services are not
readily available in the Standalone MM environment. Platforms that
use Standalone MM today, must define these PCDs as FixedAtBuild in
their platform build. However, the PCDs do allow platforms to treat
the PCDs as Dynamic/DynamicEx and being able to support that is
currently a gap for Standalone MM.

This patch series introduces a HOB that can be produced by the
platform to provide the same information. The HOB list is
available to Standalone MM.

The PCD declarations are left as-is in MdeModulePkg for backward
compatibility. This means unless a platform wants to use the HOB,
their code will continue to work with no change (they do not need
to produce the HOB). Only if the HOB is found, is its value used
instead of the PCDs.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com

Michael Kubacki (3):
  MdeModulePkg: Add Variable Flash Info HOB
  MdeModulePkg/Variable: Consume Variable Info HOB
  MdeModulePkg/FaultTolerantWrite: Consume Variable Info HOB

 MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c                          | 111 +++++++++++++++++---
 MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c               |   7 +-
 MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c            |  92 ++++++++++++++--
 MdeModulePkg/Universal/Variable/Pei/Variable.c                                  |  66 +++++++++++-
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                           |  42 ++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c                        |  25 ++++-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c                |  20 +++-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c                        |  20 +++-
 MdeModulePkg/Include/Guid/VariableFlashInfo.h                                   |  36 +++++++
 MdeModulePkg/MdeModulePkg.dec                                                   |   4 +
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h               |   9 +-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf          |  11 +-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf          |  11 +-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf |  11 +-
 MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf          |  10 +-
 MdeModulePkg/Universal/Variable/Pei/Variable.h                                  |   2 +
 MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                             |   6 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                           |  17 +++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf               |   6 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                      |   6 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf             |   6 +-
 21 files changed, 455 insertions(+), 63 deletions(-)
 create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h

-- 
2.28.0.windows.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/3] MdeModulePkg: Add Variable Flash Info HOB
  2022-04-06 16:26 [PATCH v1 0/3] Add Variable Flash Info HOB Michael Kubacki
@ 2022-04-06 16:26 ` Michael Kubacki
  2022-04-06 16:26 ` [PATCH v1 2/3] MdeModulePkg/Variable: Consume Variable " Michael Kubacki
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Michael Kubacki @ 2022-04-06 16:26 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao

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

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

Adds a new GUID that is used to identify a HOB that passes variable
flash information to UEFI variable drivers in HOB consumption phases
such as DXE, Traditional MM, and Standalone MM.

This information was previously passed directly with PCDs such
as EfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
and gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize.

However, the Standalone MM variable driver instance does not have
direct access to the PCD database. Therefore, this HOB will first
be considered as the source for variable flash information and
if platforms do not produce the HOB, reading the information from
the PCDs directly will be a backup to provide backward
compatibility.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 MdeModulePkg/Include/Guid/VariableFlashInfo.h | 36 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                 |  4 +++
 2 files changed, 40 insertions(+)

diff --git a/MdeModulePkg/Include/Guid/VariableFlashInfo.h b/MdeModulePkg/Include/Guid/VariableFlashInfo.h
new file mode 100644
index 000000000000..07aaa69fb748
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/VariableFlashInfo.h
@@ -0,0 +1,36 @@
+/** @file
+  This file defines the GUID and data structure used to pass information about
+  a variable store mapped on flash (i.e. a MMIO firmware volume) to the DXE and MM environment.
+
+  Copyright (c) Microsoft Corporation.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef VARIABLE_FLASH_INFO_H_
+#define VARIABLE_FLASH_INFO_H_
+
+#define VARIABLE_FLASH_INFO_HOB_GUID \
+  { 0x5d11c653, 0x8154, 0x4ac3, { 0xa8, 0xc2, 0xfb, 0xa2, 0x89, 0x20, 0xfc, 0x90 }}
+
+extern EFI_GUID  gVariableFlashInfoHobGuid;
+
+#pragma pack (push, 1)
+
+///
+/// This structure can be used to describe UEFI variable
+/// flash information.
+///
+typedef struct {
+  EFI_PHYSICAL_ADDRESS    NvStorageBaseAddress;
+  UINT64                  NvStorageLength;
+  EFI_PHYSICAL_ADDRESS    FtwSpareBaseAddress;
+  UINT64                  FtwSpareLength;
+  EFI_PHYSICAL_ADDRESS    FtwWorkingBaseAddress;
+  UINT64                  FtwWorkingLength;
+} VARIABLE_FLASH_INFO;
+
+#pragma pack (pop)
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index cf79292ec877..4e82f5836096 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -226,6 +226,10 @@ [Guids]
   #  Include/Guid/SmmVariableCommon.h
   gSmmVariableWriteGuid  = { 0x93ba1826, 0xdffb, 0x45dd, { 0x82, 0xa7, 0xe7, 0xdc, 0xaa, 0x3b, 0xbd, 0xf3 }}
 
+  ## Guid of the variable flash information HOB.
+  #  Include/Guid/VariableFlashInfo.h
+  gVariableFlashInfoHobGuid = { 0x5d11c653, 0x8154, 0x4ac3, { 0xa8, 0xc2, 0xfb, 0xa2, 0x89, 0x20, 0xfc, 0x90 }}
+
   ## Performance protocol guid that also acts as the performance HOB guid and performance variable GUID
   #  Include/Guid/Performance.h
   gPerformanceProtocolGuid       = { 0x76B6BDFA, 0x2ACD, 0x4462, { 0x9E, 0x3F, 0xCB, 0x58, 0xC9, 0x69, 0xD9, 0x37 } }
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 2/3] MdeModulePkg/Variable: Consume Variable Info HOB
  2022-04-06 16:26 [PATCH v1 0/3] Add Variable Flash Info HOB Michael Kubacki
  2022-04-06 16:26 ` [PATCH v1 1/3] MdeModulePkg: " Michael Kubacki
@ 2022-04-06 16:26 ` Michael Kubacki
  2022-04-07  3:31   ` [edk2-devel] " Wu, Hao A
  2022-04-06 16:26 ` [PATCH v1 3/3] MdeModulePkg/FaultTolerantWrite: " Michael Kubacki
  2022-04-07  3:31 ` [edk2-devel] [PATCH v1 0/3] Add Variable Flash " Wu, Hao A
  3 siblings, 1 reply; 9+ messages in thread
From: Michael Kubacki @ 2022-04-06 16:26 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao

From: Michael Kubacki <michael.kubacki@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 HOB.

If the HOB is not present, the drivers will continue to directly
read flash information from PCDs.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 MdeModulePkg/Universal/Variable/Pei/Variable.c                      | 66 ++++++++++++++++++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c               | 42 +++++++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c            | 25 ++++++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c    | 20 ++++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c            | 20 +++++-
 MdeModulePkg/Universal/Variable/Pei/Variable.h                      |  2 +
 MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                 |  6 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h               | 17 +++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf   |  6 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf          |  6 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf |  6 +-
 11 files changed, 194 insertions(+), 22 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index b36dd0de67b2..b19a26965ef2 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -553,6 +553,48 @@ GetHobVariableStore (
   }
 }
 
+/**
+  Get the HOB that contains variable flash information.
+
+  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable flash information structure.
+
+  @retval EFI_SUCCESS             Variable flash information was found successfully.
+  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is NULL.
+  @retval EFI_NOT_FOUND           Variable flash information could not be found.
+
+**/
+EFI_STATUS
+GetVariableFlashInfo (
+  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
+  )
+{
+  EFI_HOB_GUID_TYPE  *GuidHob;
+
+  if (VariableFlashInfo == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
+  if (GuidHob == NULL) {
+    return EFI_NOT_FOUND;
+  }
+
+  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
+
+  //
+  // Assert if more than one variable flash information HOB is present.
+  //
+  DEBUG_CODE (
+    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information HOBs\n"));
+    ASSERT (FALSE);
+  }
+
+    );
+
+  return EFI_SUCCESS;
+}
+
 /**
   Return the variable store header and the store info based on the Index.
 
@@ -567,8 +609,10 @@ GetVariableStore (
   OUT VARIABLE_STORE_INFO  *StoreInfo
   )
 {
+  EFI_STATUS                            Status;
   EFI_HOB_GUID_TYPE                     *GuidHob;
   EFI_FIRMWARE_VOLUME_HEADER            *FvHeader;
+  VARIABLE_FLASH_INFO                   *VariableFlashInfo;
   VARIABLE_STORE_HEADER                 *VariableStoreHeader;
   EFI_PHYSICAL_ADDRESS                  NvStorageBase;
   UINT32                                NvStorageSize;
@@ -591,11 +635,23 @@ GetVariableStore (
         // Emulated non-volatile variable mode is not enabled.
         //
 
-        NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
-        NvStorageBase = (EFI_PHYSICAL_ADDRESS)(PcdGet64 (PcdFlashNvStorageVariableBase64) != 0 ?
-                                               PcdGet64 (PcdFlashNvStorageVariableBase64) :
-                                               PcdGet32 (PcdFlashNvStorageVariableBase)
-                                               );
+        Status = GetVariableFlashInfo (&VariableFlashInfo);
+        if (!EFI_ERROR (Status)) {
+          NvStorageBase = VariableFlashInfo->NvStorageBaseAddress;
+          Status        = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength, &NvStorageSize);
+          // This driver currently assumes the size will be UINT32 so only accept
+          // that for now.
+          ASSERT_EFI_ERROR (Status);
+        }
+
+        if (EFI_ERROR (Status)) {
+          NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
+          NvStorageBase = (EFI_PHYSICAL_ADDRESS)(PcdGet64 (PcdFlashNvStorageVariableBase64) != 0 ?
+                                                 PcdGet64 (PcdFlashNvStorageVariableBase64) :
+                                                 PcdGet32 (PcdFlashNvStorageVariableBase)
+                                                 );
+        }
+
         ASSERT (NvStorageBase != 0);
 
         //
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6c1a3440ac8c..6ab4efd62a7e 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -3701,6 +3701,48 @@ GetHobVariableStore (
   return EFI_SUCCESS;
 }
 
+/**
+  Get the HOB that contains variable flash information.
+
+  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable flash information structure.
+
+  @retval EFI_SUCCESS             Variable flash information was found successfully.
+  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is NULL.
+  @retval EFI_NOT_FOUND           Variable flash information could not be found.
+
+**/
+EFI_STATUS
+GetVariableFlashInfo (
+  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
+  )
+{
+  EFI_HOB_GUID_TYPE  *GuidHob;
+
+  if (VariableFlashInfo == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
+  if (GuidHob == NULL) {
+    return EFI_NOT_FOUND;
+  }
+
+  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
+
+  //
+  // Assert if more than one variable flash information HOB is present.
+  //
+  DEBUG_CODE (
+    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information HOBs\n"));
+    ASSERT (FALSE);
+  }
+
+    );
+
+  return EFI_SUCCESS;
+}
+
 /**
   Initializes variable store area for non-volatile and volatile variable.
 
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 03fec3048dc4..1dc121d78e35 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -416,6 +416,7 @@ FtwNotificationEvent (
   EFI_STATUS                          Status;
   EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *FvbProtocol;
   EFI_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;
+  VARIABLE_FLASH_INFO                 *VariableFlashInfo;
   EFI_PHYSICAL_ADDRESS                NvStorageVariableBase;
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR     GcdDescriptor;
   EFI_PHYSICAL_ADDRESS                BaseAddress;
@@ -423,6 +424,7 @@ FtwNotificationEvent (
   EFI_PHYSICAL_ADDRESS                VariableStoreBase;
   UINT64                              VariableStoreLength;
   UINTN                               FtwMaxBlockSize;
+  UINT32                              NvStorageVariableSize;
 
   //
   // Ensure FTW protocol is installed.
@@ -432,14 +434,29 @@ FtwNotificationEvent (
     return;
   }
 
+  Status = GetVariableFlashInfo (&VariableFlashInfo);
+  if (!EFI_ERROR (Status)) {
+    NvStorageVariableBase = VariableFlashInfo->NvStorageBaseAddress;
+    Status                = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength, &NvStorageVariableSize);
+    // This driver currently assumes the size will be UINT32 so only accept
+    // that for now.
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  if (EFI_ERROR (Status)) {
+    NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
+    NvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
+  }
+
+  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/RuntimeDxe/VariableNonVolatile.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
index 5e9d40b67ac2..e424c210248a 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
@@ -137,6 +137,7 @@ InitRealNonVolatileVariableStore (
 {
   EFI_FIRMWARE_VOLUME_HEADER            *FvHeader;
   VARIABLE_STORE_HEADER                 *VariableStore;
+  VARIABLE_FLASH_INFO                   *VariableFlashInfo;
   UINT32                                VariableStoreLength;
   EFI_HOB_GUID_TYPE                     *GuidHob;
   EFI_PHYSICAL_ADDRESS                  NvStorageBase;
@@ -153,19 +154,30 @@ InitRealNonVolatileVariableStore (
 
   mVariableModuleGlobal->FvbInstance = NULL;
 
+  Status = GetVariableFlashInfo (&VariableFlashInfo);
+  if (!EFI_ERROR (Status)) {
+    NvStorageBase = VariableFlashInfo->NvStorageBaseAddress;
+    Status        = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength, &NvStorageSize);
+    // This driver currently assumes the size will be UINT32 so only accept that for now.
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  if (EFI_ERROR (Status)) {
+    NvStorageBase = NV_STORAGE_VARIABLE_BASE;
+    NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
+  }
+
+  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..c8667dd6ca34 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -1082,8 +1082,10 @@ SmmFtwNotificationEvent (
   EFI_PHYSICAL_ADDRESS                    VariableStoreBase;
   EFI_SMM_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *FvbProtocol;
   EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;
+  VARIABLE_FLASH_INFO                     *VariableFlashInfo;
   EFI_PHYSICAL_ADDRESS                    NvStorageVariableBase;
   UINTN                                   FtwMaxBlockSize;
+  UINT32                                  NvStorageVariableSize;
 
   if (mVariableModuleGlobal->FvbInstance != NULL) {
     return EFI_SUCCESS;
@@ -1097,9 +1099,25 @@ SmmFtwNotificationEvent (
     return Status;
   }
 
+  Status = GetVariableFlashInfo (&VariableFlashInfo);
+  if (!EFI_ERROR (Status)) {
+    NvStorageVariableBase = VariableFlashInfo->NvStorageBaseAddress;
+    Status                = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength, &NvStorageVariableSize);
+    // This driver currently assumes the size will be UINT32 so only accept that for now.
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  if (EFI_ERROR (Status)) {
+    NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
+    NvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
+  }
+
+  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;
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.h b/MdeModulePkg/Universal/Variable/Pei/Variable.h
index 7f9ad5bfc357..b2ba30b841c1 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.h
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.h
@@ -20,7 +20,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/BaseMemoryLib.h>
 #include <Library/PeiServicesTablePointerLib.h>
 #include <Library/PeiServicesLib.h>
+#include <Library/SafeIntLib.h>
 
+#include <Guid/VariableFlashInfo.h>
 #include <Guid/VariableFormat.h>
 #include <Guid/VariableIndexTable.h>
 #include <Guid/SystemNvDataGuid.h>
diff --git a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
index 7cbdd2385e8f..2557219f9b30 100644
--- a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+++ b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
@@ -39,6 +39,7 @@ [LibraryClasses]
   DebugLib
   PeiServicesTablePointerLib
   PeiServicesLib
+  SafeIntLib
 
 [Guids]
   ## CONSUMES             ## GUID # Variable store header
@@ -54,14 +55,15 @@ [Guids]
   ## SOMETIMES_CONSUMES   ## HOB
   ## CONSUMES             ## GUID # Dependence
   gEdkiiFaultTolerantWriteGuid
+  gVariableFlashInfoHobGuid   ## CONSUMES   ## HOB
 
 [Ppis]
   gEfiPeiReadOnlyVariable2PpiGuid   ## PRODUCES
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable         ## SOMETIMES_CONSUMES
 
 [Depex]
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index 31e408976a35..c9bbd1568567 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
@@ -31,12 +31,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/MemoryAllocationLib.h>
 #include <Library/AuthVariableLib.h>
 #include <Library/VarCheckLib.h>
+#include <Library/SafeIntLib.h>
 #include <Guid/GlobalVariable.h>
 #include <Guid/EventGroup.h>
 #include <Guid/VariableFormat.h>
 #include <Guid/SystemNvDataGuid.h>
 #include <Guid/FaultTolerantWrite.h>
 #include <Guid/VarErrorFlag.h>
+#include <Guid/VariableFlashInfo.h>
 
 #include "PrivilegePolymorphic.h"
 
@@ -126,6 +128,21 @@ typedef struct {
   EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL    *FvbInstance;
 } VARIABLE_MODULE_GLOBAL;
 
+/**
+  Get the HOB that contains variable flash information.
+
+  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable flash information structure.
+
+  @retval EFI_SUCCESS             Variable flash information was found successfully.
+  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is NULL.
+  @retval EFI_NOT_FOUND           Variable flash information could not be found.
+
+**/
+EFI_STATUS
+GetVariableFlashInfo (
+  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
+  );
+
 /**
   Flush the HOB variable to flash.
 
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index c9434df631ee..6df350afa260 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -73,6 +73,7 @@ [LibraryClasses]
   VarCheckLib
   VariablePolicyLib
   VariablePolicyHelperLib
+  SafeIntLib
 
 [Protocols]
   gEfiFirmwareVolumeBlockProtocolGuid           ## CONSUMES
@@ -114,6 +115,7 @@ [Guids]
   gEfiSystemNvDataFvGuid                        ## CONSUMES             ## GUID
   gEfiEndOfDxeEventGroupGuid                    ## CONSUMES             ## Event
   gEdkiiFaultTolerantWriteGuid                  ## SOMETIMES_CONSUMES   ## HOB
+  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
 
   ## SOMETIMES_CONSUMES   ## Variable:L"VarErrorFlag"
   ## SOMETIMES_PRODUCES   ## Variable:L"VarErrorFlag"
@@ -125,9 +127,9 @@ [Guids]
   gEfiImageSecurityDatabaseGuid
 
 [Pcd]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ## SOMETIMES_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..398e7d9060ab 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -82,6 +82,7 @@ [LibraryClasses]
   UefiBootServicesTableLib
   VariablePolicyLib
   VariablePolicyHelperLib
+  SafeIntLib
 
 [Protocols]
   gEfiSmmFirmwareVolumeBlockProtocolGuid        ## CONSUMES
@@ -121,15 +122,16 @@ [Guids]
   gSmmVariableWriteGuid                         ## PRODUCES             ## GUID # Install protocol
   gEfiSystemNvDataFvGuid                        ## CONSUMES             ## GUID
   gEdkiiFaultTolerantWriteGuid                  ## SOMETIMES_CONSUMES   ## HOB
+  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
 
   ## SOMETIMES_CONSUMES   ## Variable:L"VarErrorFlag"
   ## SOMETIMES_PRODUCES   ## Variable:L"VarErrorFlag"
   gEdkiiVarErrorFlagGuid
 
 [Pcd]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase       ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ## SOMETIMES_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..efc9290ae759 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
@@ -73,6 +73,7 @@ [LibraryClasses]
   HobLib
   MemoryAllocationLib
   MmServicesTableLib
+  SafeIntLib
   StandaloneMmDriverEntryPoint
   SynchronizationLib
   VarCheckLib
@@ -114,6 +115,7 @@ [Guids]
 
   gEfiSystemNvDataFvGuid                        ## CONSUMES             ## GUID
   gEdkiiFaultTolerantWriteGuid                  ## SOMETIMES_CONSUMES   ## HOB
+  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
 
   ## SOMETIMES_CONSUMES   ## Variable:L"VarErrorFlag"
   ## SOMETIMES_PRODUCES   ## Variable:L"VarErrorFlag"
@@ -121,8 +123,8 @@ [Guids]
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase       ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize                  ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize              ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize          ## CONSUMES
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 3/3] MdeModulePkg/FaultTolerantWrite: Consume Variable Info HOB
  2022-04-06 16:26 [PATCH v1 0/3] Add Variable Flash Info HOB Michael Kubacki
  2022-04-06 16:26 ` [PATCH v1 1/3] MdeModulePkg: " Michael Kubacki
  2022-04-06 16:26 ` [PATCH v1 2/3] MdeModulePkg/Variable: Consume Variable " Michael Kubacki
@ 2022-04-06 16:26 ` Michael Kubacki
  2022-04-07  3:31   ` [edk2-devel] " Wu, Hao A
  2022-04-07  3:31 ` [edk2-devel] [PATCH v1 0/3] Add Variable Flash " Wu, Hao A
  3 siblings, 1 reply; 9+ messages in thread
From: Michael Kubacki @ 2022-04-06 16:26 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao

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

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

Adds support to the UEFI variable fault tolerant write (FTW) drivers
to receive FTW base and size information dynamically via the Variable
Flash Information HOB.

This supplements support added earlier for the variable store base
address and size passed with the Variable Flash Information HOB.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c                          | 111 +++++++++++++++++---
 MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c               |   7 +-
 MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c            |  92 ++++++++++++++--
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h               |   9 +-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf          |  11 +-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf          |  11 +-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf |  11 +-
 MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf          |  10 +-
 8 files changed, 221 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
index 661e1487673b..5066fe0d3711 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
@@ -972,6 +972,48 @@ GetPreviousRecordOfWrites (
   return EFI_SUCCESS;
 }
 
+/**
+  Get the HOB that contains variable flash information.
+
+  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable flash information structure.
+
+  @retval EFI_SUCCESS             Variable flash information was found successfully.
+  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is NULL.
+  @retval EFI_NOT_FOUND           Variable flash information could not be found.
+
+**/
+EFI_STATUS
+GetVariableFlashInfo (
+  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
+  )
+{
+  EFI_HOB_GUID_TYPE  *GuidHob;
+
+  if (VariableFlashInfo == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
+  if (GuidHob == NULL) {
+    return EFI_NOT_FOUND;
+  }
+
+  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
+
+  //
+  // Assert if more than one variable flash information HOB is present.
+  //
+  DEBUG_CODE (
+    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information HOBs\n"));
+    ASSERT (FALSE);
+  }
+
+    );
+
+  return EFI_SUCCESS;
+}
+
 /**
   Allocate private data for FTW driver and initialize it.
 
@@ -987,22 +1029,71 @@ InitFtwDevice (
   OUT EFI_FTW_DEVICE  **FtwData
   )
 {
-  EFI_FTW_DEVICE  *FtwDevice;
+  EFI_STATUS           Status;
+  EFI_STATUS           VariableFlashInfoStatus;
+  UINTN                FtwWorkingSize;
+  VARIABLE_FLASH_INFO  *VariableFlashInfo;
+  EFI_FTW_DEVICE       *FtwDevice;
+
+  FtwWorkingSize          = 0;
+  VariableFlashInfoStatus = GetVariableFlashInfo (&VariableFlashInfo);
+  if (!EFI_ERROR (VariableFlashInfoStatus)) {
+    Status = SafeUint64ToUintn (VariableFlashInfo->FtwWorkingLength, &FtwWorkingSize);
+    if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
+      FtwWorkingSize = 0;
+    }
+  }
+
+  if (FtwWorkingSize == 0) {
+    FtwWorkingSize = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+  }
 
   //
   // Allocate private data of this driver,
   // Including the FtwWorkSpace[FTW_WORK_SPACE_SIZE].
   //
-  FtwDevice = AllocateZeroPool (sizeof (EFI_FTW_DEVICE) + PcdGet32 (PcdFlashNvStorageFtwWorkingSize));
+  FtwDevice = AllocateZeroPool (sizeof (EFI_FTW_DEVICE) + FtwWorkingSize);
   if (FtwDevice == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
+  FtwDevice->WorkSpaceLength = FtwWorkingSize;
+
+  if (!EFI_ERROR (VariableFlashInfoStatus)) {
+    // This driver currently assumes the size will be UINT32 so only accept
+    // that for now.
+    Status = SafeUint64ToUintn (VariableFlashInfo->FtwSpareLength, &FtwDevice->SpareAreaLength);
+    if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
+      FtwDevice->SpareAreaLength = 0;
+    }
+
+    FtwDevice->SpareAreaAddress = VariableFlashInfo->FtwSpareBaseAddress;
+    FtwDevice->WorkSpaceAddress = VariableFlashInfo->FtwWorkingBaseAddress;
+  }
+
+  if (FtwDevice->SpareAreaLength == 0) {
+    FtwDevice->SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  }
+
+  if (FtwDevice->SpareAreaAddress == 0) {
+    FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwSpareBase64);
+    if (FtwDevice->SpareAreaAddress == 0) {
+      FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwSpareBase);
+    }
+  }
+
+  if (FtwDevice->WorkSpaceAddress == 0) {
+    FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwWorkingBase64);
+    if (FtwDevice->WorkSpaceAddress == 0) {
+      FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
+    }
+  }
+
   //
   // Initialize other parameters, and set WorkSpace as FTW_ERASED_BYTE.
   //
-  FtwDevice->WorkSpaceLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
-  FtwDevice->SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
   if ((FtwDevice->WorkSpaceLength == 0) || (FtwDevice->SpareAreaLength == 0)) {
     DEBUG ((DEBUG_ERROR, "Ftw: Workspace or Spare block does not exist!\n"));
     FreePool (FtwDevice);
@@ -1015,16 +1106,6 @@ InitFtwDevice (
   FtwDevice->FtwWorkSpaceLba = (EFI_LBA)(-1);
   FtwDevice->FtwSpareLba     = (EFI_LBA)(-1);
 
-  FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwWorkingBase64);
-  if (FtwDevice->WorkSpaceAddress == 0) {
-    FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
-  }
-
-  FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwSpareBase64);
-  if (FtwDevice->SpareAreaAddress == 0) {
-    FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwSpareBase);
-  }
-
   *FtwData = FtwDevice;
   return EFI_SUCCESS;
 }
@@ -1277,7 +1358,7 @@ InitFtwProtocol (
   FtwDevice->FtwLastWriteHeader = NULL;
   FtwDevice->FtwLastWriteRecord = NULL;
 
-  InitializeLocalWorkSpaceHeader ();
+  InitializeLocalWorkSpaceHeader (FtwDevice->WorkSpaceLength);
 
   //
   // Refresh the working space data from working block
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
index 61e7a92ccea1..fd563643eb63 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
@@ -16,10 +16,13 @@ EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER  mWorkingBlockHeader = { ZERO_GUID, 0, 0
 
   Since Signature and WriteQueueSize have been known, Crc can be calculated out,
   then the work space header will be fixed.
+
+  @param[in]  WorkSpaceLength     Length in bytes of the FTW workspace area.
+
 **/
 VOID
 InitializeLocalWorkSpaceHeader (
-  VOID
+  IN  UINTN  WorkSpaceLength
   )
 {
   //
@@ -46,7 +49,7 @@ InitializeLocalWorkSpaceHeader (
     &gEdkiiWorkingBlockSignatureGuid,
     sizeof (EFI_GUID)
     );
-  mWorkingBlockHeader.WriteQueueSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize) - sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER);
+  mWorkingBlockHeader.WriteQueueSize = WorkSpaceLength - sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER);
 
   //
   // Crc is calculated with all the fields except Crc and STATE, so leave them as FTW_ERASED_BYTE.
diff --git a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
index 15543f12ed29..7abf8f72d569 100644
--- a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
+++ b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
@@ -11,11 +11,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <Guid/SystemNvDataGuid.h>
 #include <Guid/FaultTolerantWrite.h>
+#include <Guid/VariableFlashInfo.h>
 #include <Library/PeiServicesLib.h>
 #include <Library/PcdLib.h>
 #include <Library/DebugLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/HobLib.h>
+#include <Library/SafeIntLib.h>
 
 EFI_PEI_PPI_DESCRIPTOR  mPpiListVariable = {
   (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
@@ -186,6 +188,48 @@ IsValidWorkSpace (
   return TRUE;
 }
 
+/**
+  Get the HOB that contains variable flash information.
+
+  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable flash information structure.
+
+  @retval EFI_SUCCESS             Variable flash information was found successfully.
+  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is NULL.
+  @retval EFI_NOT_FOUND           Variable flash information could not be found.
+
+**/
+EFI_STATUS
+GetVariableFlashInfo (
+  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
+  )
+{
+  EFI_HOB_GUID_TYPE  *GuidHob;
+
+  if (VariableFlashInfo == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
+  if (GuidHob == NULL) {
+    return EFI_NOT_FOUND;
+  }
+
+  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
+
+  //
+  // Assert if more than one variable flash information HOB is present.
+  //
+  DEBUG_CODE (
+    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information HOBs\n"));
+    ASSERT (FALSE);
+  }
+
+    );
+
+  return EFI_SUCCESS;
+}
+
 /**
   Main entry for Fault Tolerant Write PEIM.
 
@@ -207,6 +251,7 @@ PeimFaultTolerantWriteInitialize (
   EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER  *FtwWorkingBlockHeader;
   EFI_FAULT_TOLERANT_WRITE_HEADER          *FtwLastWriteHeader;
   EFI_FAULT_TOLERANT_WRITE_RECORD          *FtwLastWriteRecord;
+  VARIABLE_FLASH_INFO                      *VariableFlashInfo;
   EFI_PHYSICAL_ADDRESS                     WorkSpaceAddress;
   UINTN                                    WorkSpaceLength;
   EFI_PHYSICAL_ADDRESS                     SpareAreaAddress;
@@ -218,19 +263,52 @@ PeimFaultTolerantWriteInitialize (
   FtwLastWriteHeader    = NULL;
   FtwLastWriteRecord    = NULL;
 
-  WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwWorkingBase64);
-  if (WorkSpaceAddress == 0) {
-    WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
+  SpareAreaAddress = 0;
+  SpareAreaLength  = 0;
+  WorkSpaceAddress = 0;
+  WorkSpaceLength  = 0;
+
+  Status = GetVariableFlashInfo (&VariableFlashInfo);
+  if (!EFI_ERROR (Status)) {
+    // This driver currently assumes the size will be UINT32 so only accept
+    // that for now.
+    Status = SafeUint64ToUintn (VariableFlashInfo->FtwSpareLength, &SpareAreaLength);
+    if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
+      SpareAreaLength = 0;
+    }
+
+    Status = SafeUint64ToUintn (VariableFlashInfo->FtwWorkingLength, &WorkSpaceLength);
+    if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
+      WorkSpaceLength = 0;
+    }
+
+    SpareAreaAddress = VariableFlashInfo->FtwSpareBaseAddress;
+    WorkSpaceAddress = VariableFlashInfo->FtwWorkingBaseAddress;
   }
 
-  WorkSpaceLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+  if (SpareAreaLength == 0) {
+    SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  }
+
+  if (WorkSpaceLength == 0) {
+    WorkSpaceLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+  }
 
-  SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwSpareBase64);
   if (SpareAreaAddress == 0) {
-    SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwSpareBase);
+    SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwSpareBase64);
+    if (SpareAreaAddress == 0) {
+      SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwSpareBase);
+    }
   }
 
-  SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  if (WorkSpaceAddress == 0) {
+    WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwWorkingBase64);
+    if (WorkSpaceAddress == 0) {
+      WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
+    }
+  }
 
   //
   // The address of FTW working base and spare base must not be 0.
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
index c14e47b5c7b2..40ef44c069eb 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
@@ -14,11 +14,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <PiDxe.h>
 
 #include <Guid/SystemNvDataGuid.h>
+#include <Guid/VariableFlashInfo.h>
 #include <Guid/ZeroGuid.h>
 #include <Protocol/FaultTolerantWrite.h>
 #include <Protocol/FirmwareVolumeBlock.h>
 #include <Protocol/SwapAddressRange.h>
 
+#include <Library/HobLib.h>
+#include <Library/SafeIntLib.h>
 #include <Library/PcdLib.h>
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
@@ -706,12 +709,16 @@ InitFtwProtocol (
 /**
   Initialize a local work space header.
 
+
   Since Signature and WriteQueueSize have been known, Crc can be calculated out,
   then the work space header will be fixed.
+
+  @param[in]  WorkSpaceLength     Length in bytes of the FTW workspace area.
+
 **/
 VOID
 InitializeLocalWorkSpaceHeader (
-  VOID
+  IN  UINTN  WorkSpaceLength
   );
 
 /**
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
index 96165614d178..923f0231a047 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
@@ -46,6 +46,8 @@ [LibraryClasses]
   UefiLib
   PcdLib
   ReportStatusCodeLib
+  HobLib
+  SafeIntLib
 
 [Guids]
   #
@@ -54,6 +56,7 @@ [Guids]
   ## CONSUMES           ## GUID
   ## PRODUCES           ## GUID
   gEdkiiWorkingBlockSignatureGuid
+  gVariableFlashInfoHobGuid       ## CONSUMES   ## HOB
 
 [Protocols]
   gEfiSwapAddressRangeProtocolGuid | gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable ## SOMETIMES_CONSUMES
@@ -67,11 +70,11 @@ [FeaturePcd]
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## SOMETIMES_CONSUMES
 
 #
 # gBS->CalculateCrc32() is consumed in EntryPoint.
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
index 8cc6028470d8..bbed17e4b611 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
@@ -52,6 +52,8 @@ [LibraryClasses]
   ReportStatusCodeLib
   SmmMemLib
   BaseLib
+  HobLib
+  SafeIntLib
 
 [Guids]
   #
@@ -60,6 +62,7 @@ [Guids]
   ## CONSUMES           ## GUID
   ## PRODUCES           ## GUID
   gEdkiiWorkingBlockSignatureGuid
+  gVariableFlashInfoHobGuid       ## CONSUMES   ## HOB
 
 [Protocols]
   gEfiSmmSwapAddressRangeProtocolGuid | gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ## SOMETIMES_CONSUMES
@@ -76,11 +79,11 @@ [FeaturePcd]
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## SOMETIMES_CONSUMES
 
 #
 # gBS->CalculateCrc32() is consumed in EntryPoint.
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
index d0fab7d9414f..0b394b04da7b 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
@@ -46,10 +46,12 @@ [LibraryClasses]
   BaseLib
   BaseMemoryLib
   DebugLib
+  HobLib
   MemoryAllocationLib
   MmServicesTableLib
   PcdLib
   ReportStatusCodeLib
+  SafeIntLib
   StandaloneMmDriverEntryPoint
 
 [Guids]
@@ -59,6 +61,7 @@ [Guids]
   ## CONSUMES           ## GUID
   ## PRODUCES           ## GUID
   gEdkiiWorkingBlockSignatureGuid
+  gVariableFlashInfoHobGuid       ## CONSUMES   ## HOB
 
 [Protocols]
   gEfiSmmSwapAddressRangeProtocolGuid | gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ## SOMETIMES_CONSUMES
@@ -75,11 +78,11 @@ [FeaturePcd]
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## SOMETIMES_CONSUMES
 
 [Depex]
   TRUE
diff --git a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
index f90892ad4493..ff9d53bf7040 100644
--- a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
@@ -39,6 +39,7 @@ [LibraryClasses]
   HobLib
   BaseMemoryLib
   PcdLib
+  SafeIntLib
 
 [Guids]
   ## SOMETIMES_PRODUCES   ## HOB
@@ -46,14 +47,15 @@ [Guids]
   gEdkiiFaultTolerantWriteGuid
   gEdkiiWorkingBlockSignatureGuid               ## SOMETIMES_CONSUMES   ## GUID
   gEfiSystemNvDataFvGuid                        ## SOMETIMES_CONSUMES   ## GUID
+  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## SOMETIMES_CONSUMES
 
 [Depex]
   TRUE
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/3] Add Variable Flash Info HOB
  2022-04-06 16:26 [PATCH v1 0/3] Add Variable Flash Info HOB Michael Kubacki
                   ` (2 preceding siblings ...)
  2022-04-06 16:26 ` [PATCH v1 3/3] MdeModulePkg/FaultTolerantWrite: " Michael Kubacki
@ 2022-04-07  3:31 ` Wu, Hao A
  2022-04-07 23:56   ` Michael Kubacki
  3 siblings, 1 reply; 9+ messages in thread
From: Wu, Hao A @ 2022-04-07  3:31 UTC (permalink / raw)
  To: devel@edk2.groups.io, mikuback@linux.microsoft.com
  Cc: Wang, Jian J, Gao, Liming

Sorry for a question:
GetVariableFlashInfo() seems being defined multiple times across modules, do you see value in abstracting it as a library API?

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Thursday, April 7, 2022 12:27 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH v1 0/3] Add Variable Flash Info HOB
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
> 
> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
> VariableStandaloneMm, etc. (and their dependent protocol/library
> stack), typically acquire UEFI variable store flash information
> with PCDs declared in MdeModulePkg.
> 
> For example:
> [Pcd]
>   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> 
> These PCDs work as-is in the StandaloneMm driver if they are not
> dynamic such as Dynamic or DynamicEx because PCD services are not
> readily available in the Standalone MM environment. Platforms that
> use Standalone MM today, must define these PCDs as FixedAtBuild in
> their platform build. However, the PCDs do allow platforms to treat
> the PCDs as Dynamic/DynamicEx and being able to support that is
> currently a gap for Standalone MM.
> 
> This patch series introduces a HOB that can be produced by the
> platform to provide the same information. The HOB list is
> available to Standalone MM.
> 
> The PCD declarations are left as-is in MdeModulePkg for backward
> compatibility. This means unless a platform wants to use the HOB,
> their code will continue to work with no change (they do not need
> to produce the HOB). Only if the HOB is found, is its value used
> instead of the PCDs.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com
> 
> Michael Kubacki (3):
>   MdeModulePkg: Add Variable Flash Info HOB
>   MdeModulePkg/Variable: Consume Variable Info HOB
>   MdeModulePkg/FaultTolerantWrite: Consume Variable Info HOB
> 
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c                          |
> 111 +++++++++++++++++---
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> |   7 +-
>  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> |  92 ++++++++++++++--
>  MdeModulePkg/Universal/Variable/Pei/Variable.c                                  |  66
> +++++++++++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                           |  42
> ++++++++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c                        |
> 25 ++++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
> |  20 +++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c                        |
> 20 +++-
>  MdeModulePkg/Include/Guid/VariableFlashInfo.h                                   |  36
> +++++++
>  MdeModulePkg/MdeModulePkg.dec                                                   |   4 +
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
> |   9 +-
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> |  11 +-
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> |  11 +-
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon
> eMm.inf |  11 +-
>  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> |  10 +-
>  MdeModulePkg/Universal/Variable/Pei/Variable.h                                  |   2 +
>  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                             |   6 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                           |  17
> +++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |   6 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                      |
> 6 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> |   6 +-
>  21 files changed, 455 insertions(+), 63 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
> 
> --
> 2.28.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#88461): https://edk2.groups.io/g/devel/message/88461
> Mute This Topic: https://groups.io/mt/90293658/1768737
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-=-=-=-=
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH v1 2/3] MdeModulePkg/Variable: Consume Variable Info HOB
  2022-04-06 16:26 ` [PATCH v1 2/3] MdeModulePkg/Variable: Consume Variable " Michael Kubacki
@ 2022-04-07  3:31   ` Wu, Hao A
  0 siblings, 0 replies; 9+ messages in thread
From: Wu, Hao A @ 2022-04-07  3:31 UTC (permalink / raw)
  To: devel@edk2.groups.io, mikuback@linux.microsoft.com
  Cc: Wang, Jian J, Gao, Liming

One inline comment below:


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Thursday, April 7, 2022 12:27 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH v1 2/3] MdeModulePkg/Variable: Consume
> Variable Info HOB
> 
> From: Michael Kubacki <michael.kubacki@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 HOB.
> 
> If the HOB is not present, the drivers will continue to directly
> read flash information from PCDs.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  MdeModulePkg/Universal/Variable/Pei/Variable.c                      | 66
> ++++++++++++++++++--
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c               | 42
> +++++++++++++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c            | 25
> ++++++--
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c    | 20
> ++++--
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c            | 20
> +++++-
>  MdeModulePkg/Universal/Variable/Pei/Variable.h                      |  2 +
>  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                 |  6 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h               | 17 +++++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf   |  6
> +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf          |  6 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf |  6
> +-
>  11 files changed, 194 insertions(+), 22 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> index b36dd0de67b2..b19a26965ef2 100644
> --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> @@ -553,6 +553,48 @@ GetHobVariableStore (
>    }
>  }
> 
> +/**
> +  Get the HOB that contains variable flash information.
> +
> +  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable
> flash information structure.
> +
> +  @retval EFI_SUCCESS             Variable flash information was found successfully.
> +  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is
> NULL.
> +  @retval EFI_NOT_FOUND           Variable flash information could not be found.
> +
> +**/
> +EFI_STATUS
> +GetVariableFlashInfo (
> +  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
> +  )
> +{
> +  EFI_HOB_GUID_TYPE  *GuidHob;
> +
> +  if (VariableFlashInfo == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
> +  if (GuidHob == NULL) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
> +
> +  //
> +  // Assert if more than one variable flash information HOB is present.
> +  //
> +  DEBUG_CODE (
> +    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB
> (GuidHob)) != NULL)) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information
> HOBs\n"));
> +    ASSERT (FALSE);
> +  }
> +
> +    );
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Return the variable store header and the store info based on the Index.
> 
> @@ -567,8 +609,10 @@ GetVariableStore (
>    OUT VARIABLE_STORE_INFO  *StoreInfo
>    )
>  {
> +  EFI_STATUS                            Status;
>    EFI_HOB_GUID_TYPE                     *GuidHob;
>    EFI_FIRMWARE_VOLUME_HEADER            *FvHeader;
> +  VARIABLE_FLASH_INFO                   *VariableFlashInfo;
>    VARIABLE_STORE_HEADER                 *VariableStoreHeader;
>    EFI_PHYSICAL_ADDRESS                  NvStorageBase;
>    UINT32                                NvStorageSize;
> @@ -591,11 +635,23 @@ GetVariableStore (
>          // Emulated non-volatile variable mode is not enabled.
>          //
> 
> -        NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
> -        NvStorageBase = (EFI_PHYSICAL_ADDRESS)(PcdGet64
> (PcdFlashNvStorageVariableBase64) != 0 ?
> -                                               PcdGet64 (PcdFlashNvStorageVariableBase64) :
> -                                               PcdGet32 (PcdFlashNvStorageVariableBase)
> -                                               );
> +        Status = GetVariableFlashInfo (&VariableFlashInfo);
> +        if (!EFI_ERROR (Status)) {
> +          NvStorageBase = VariableFlashInfo->NvStorageBaseAddress;
> +          Status        = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength,
> &NvStorageSize);
> +          // This driver currently assumes the size will be UINT32 so only accept
> +          // that for now.
> +          ASSERT_EFI_ERROR (Status);
> +        }
> +
> +        if (EFI_ERROR (Status)) {
> +          NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
> +          NvStorageBase = (EFI_PHYSICAL_ADDRESS)(PcdGet64
> (PcdFlashNvStorageVariableBase64) != 0 ?
> +                                                 PcdGet64 (PcdFlashNvStorageVariableBase64) :
> +                                                 PcdGet32 (PcdFlashNvStorageVariableBase)
> +                                                 );
> +        }
> +
>          ASSERT (NvStorageBase != 0);
> 
>          //
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 6c1a3440ac8c..6ab4efd62a7e 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -3701,6 +3701,48 @@ GetHobVariableStore (
>    return EFI_SUCCESS;
>  }
> 
> +/**
> +  Get the HOB that contains variable flash information.
> +
> +  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable
> flash information structure.
> +
> +  @retval EFI_SUCCESS             Variable flash information was found successfully.
> +  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is
> NULL.
> +  @retval EFI_NOT_FOUND           Variable flash information could not be found.
> +
> +**/
> +EFI_STATUS
> +GetVariableFlashInfo (
> +  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
> +  )
> +{
> +  EFI_HOB_GUID_TYPE  *GuidHob;
> +
> +  if (VariableFlashInfo == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
> +  if (GuidHob == NULL) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
> +
> +  //
> +  // Assert if more than one variable flash information HOB is present.
> +  //
> +  DEBUG_CODE (
> +    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB
> (GuidHob)) != NULL)) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information
> HOBs\n"));
> +    ASSERT (FALSE);
> +  }
> +
> +    );
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Initializes variable store area for non-volatile and volatile variable.
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 03fec3048dc4..1dc121d78e35 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -416,6 +416,7 @@ FtwNotificationEvent (
>    EFI_STATUS                          Status;
>    EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *FvbProtocol;
>    EFI_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;
> +  VARIABLE_FLASH_INFO                 *VariableFlashInfo;
>    EFI_PHYSICAL_ADDRESS                NvStorageVariableBase;
>    EFI_GCD_MEMORY_SPACE_DESCRIPTOR     GcdDescriptor;
>    EFI_PHYSICAL_ADDRESS                BaseAddress;
> @@ -423,6 +424,7 @@ FtwNotificationEvent (
>    EFI_PHYSICAL_ADDRESS                VariableStoreBase;
>    UINT64                              VariableStoreLength;
>    UINTN                               FtwMaxBlockSize;
> +  UINT32                              NvStorageVariableSize;
> 
>    //
>    // Ensure FTW protocol is installed.
> @@ -432,14 +434,29 @@ FtwNotificationEvent (
>      return;
>    }
> 
> +  Status = GetVariableFlashInfo (&VariableFlashInfo);
> +  if (!EFI_ERROR (Status)) {
> +    NvStorageVariableBase = VariableFlashInfo->NvStorageBaseAddress;
> +    Status                = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength,
> &NvStorageVariableSize);
> +    // This driver currently assumes the size will be UINT32 so only accept
> +    // that for now.
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  if (EFI_ERROR (Status)) {
> +    NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
> +    NvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
> +  }
> +
> +  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/RuntimeDxe/VariableNonVolatile.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
> index 5e9d40b67ac2..e424c210248a 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
> @@ -137,6 +137,7 @@ InitRealNonVolatileVariableStore (
>  {
>    EFI_FIRMWARE_VOLUME_HEADER            *FvHeader;
>    VARIABLE_STORE_HEADER                 *VariableStore;
> +  VARIABLE_FLASH_INFO                   *VariableFlashInfo;
>    UINT32                                VariableStoreLength;
>    EFI_HOB_GUID_TYPE                     *GuidHob;
>    EFI_PHYSICAL_ADDRESS                  NvStorageBase;
> @@ -153,19 +154,30 @@ InitRealNonVolatileVariableStore (
> 
>    mVariableModuleGlobal->FvbInstance = NULL;
> 
> +  Status = GetVariableFlashInfo (&VariableFlashInfo);
> +  if (!EFI_ERROR (Status)) {
> +    NvStorageBase = VariableFlashInfo->NvStorageBaseAddress;
> +    Status        = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength,
> &NvStorageSize);
> +    // This driver currently assumes the size will be UINT32 so only accept that
> for now.
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  if (EFI_ERROR (Status)) {
> +    NvStorageBase = NV_STORAGE_VARIABLE_BASE;
> +    NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
> +  }
> +
> +  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..c8667dd6ca34 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -1082,8 +1082,10 @@ SmmFtwNotificationEvent (
>    EFI_PHYSICAL_ADDRESS                    VariableStoreBase;
>    EFI_SMM_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *FvbProtocol;
>    EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;
> +  VARIABLE_FLASH_INFO                     *VariableFlashInfo;
>    EFI_PHYSICAL_ADDRESS                    NvStorageVariableBase;
>    UINTN                                   FtwMaxBlockSize;
> +  UINT32                                  NvStorageVariableSize;
> 
>    if (mVariableModuleGlobal->FvbInstance != NULL) {
>      return EFI_SUCCESS;
> @@ -1097,9 +1099,25 @@ SmmFtwNotificationEvent (
>      return Status;
>    }
> 
> +  Status = GetVariableFlashInfo (&VariableFlashInfo);
> +  if (!EFI_ERROR (Status)) {
> +    NvStorageVariableBase = VariableFlashInfo->NvStorageBaseAddress;
> +    Status                = SafeUint64ToUint32 (VariableFlashInfo->NvStorageLength,
> &NvStorageVariableSize);
> +    // This driver currently assumes the size will be UINT32 so only accept that
> for now.
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  if (EFI_ERROR (Status)) {
> +    NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
> +    NvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
> +  }
> +
> +  ASSERT (NvStorageVariableBase != 0);
> +  VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache-
> >HeaderLength;


It seems to me that the above line is not needed. Already done in existing code several lines below.

Best Regards,
Hao Wu


> +
>    Status = FtwProtocol->GetMaxBlockSize (FtwProtocol, &FtwMaxBlockSize);
>    if (!EFI_ERROR (Status)) {
> -    ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <= FtwMaxBlockSize);
> +    ASSERT (NvStorageVariableSize <= FtwMaxBlockSize);
>    }
> 
>    NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
> diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.h
> b/MdeModulePkg/Universal/Variable/Pei/Variable.h
> index 7f9ad5bfc357..b2ba30b841c1 100644
> --- a/MdeModulePkg/Universal/Variable/Pei/Variable.h
> +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.h
> @@ -20,7 +20,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/PeiServicesTablePointerLib.h>
>  #include <Library/PeiServicesLib.h>
> +#include <Library/SafeIntLib.h>
> 
> +#include <Guid/VariableFlashInfo.h>
>  #include <Guid/VariableFormat.h>
>  #include <Guid/VariableIndexTable.h>
>  #include <Guid/SystemNvDataGuid.h>
> diff --git a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> index 7cbdd2385e8f..2557219f9b30 100644
> --- a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> +++ b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> @@ -39,6 +39,7 @@ [LibraryClasses]
>    DebugLib
>    PeiServicesTablePointerLib
>    PeiServicesLib
> +  SafeIntLib
> 
>  [Guids]
>    ## CONSUMES             ## GUID # Variable store header
> @@ -54,14 +55,15 @@ [Guids]
>    ## SOMETIMES_CONSUMES   ## HOB
>    ## CONSUMES             ## GUID # Dependence
>    gEdkiiFaultTolerantWriteGuid
> +  gVariableFlashInfoHobGuid   ## CONSUMES   ## HOB
> 
>  [Ppis]
>    gEfiPeiReadOnlyVariable2PpiGuid   ## PRODUCES
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable         ##
> SOMETIMES_CONSUMES
> 
>  [Depex]
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> index 31e408976a35..c9bbd1568567 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> @@ -31,12 +31,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/AuthVariableLib.h>
>  #include <Library/VarCheckLib.h>
> +#include <Library/SafeIntLib.h>
>  #include <Guid/GlobalVariable.h>
>  #include <Guid/EventGroup.h>
>  #include <Guid/VariableFormat.h>
>  #include <Guid/SystemNvDataGuid.h>
>  #include <Guid/FaultTolerantWrite.h>
>  #include <Guid/VarErrorFlag.h>
> +#include <Guid/VariableFlashInfo.h>
> 
>  #include "PrivilegePolymorphic.h"
> 
> @@ -126,6 +128,21 @@ typedef struct {
>    EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL    *FvbInstance;
>  } VARIABLE_MODULE_GLOBAL;
> 
> +/**
> +  Get the HOB that contains variable flash information.
> +
> +  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable
> flash information structure.
> +
> +  @retval EFI_SUCCESS             Variable flash information was found successfully.
> +  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is
> NULL.
> +  @retval EFI_NOT_FOUND           Variable flash information could not be found.
> +
> +**/
> +EFI_STATUS
> +GetVariableFlashInfo (
> +  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
> +  );
> +
>  /**
>    Flush the HOB variable to flash.
> 
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> index c9434df631ee..6df350afa260 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> @@ -73,6 +73,7 @@ [LibraryClasses]
>    VarCheckLib
>    VariablePolicyLib
>    VariablePolicyHelperLib
> +  SafeIntLib
> 
>  [Protocols]
>    gEfiFirmwareVolumeBlockProtocolGuid           ## CONSUMES
> @@ -114,6 +115,7 @@ [Guids]
>    gEfiSystemNvDataFvGuid                        ## CONSUMES             ## GUID
>    gEfiEndOfDxeEventGroupGuid                    ## CONSUMES             ## Event
>    gEdkiiFaultTolerantWriteGuid                  ## SOMETIMES_CONSUMES   ## HOB
> +  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
> 
>    ## SOMETIMES_CONSUMES   ## Variable:L"VarErrorFlag"
>    ## SOMETIMES_PRODUCES   ## Variable:L"VarErrorFlag"
> @@ -125,9 +127,9 @@ [Guids]
>    gEfiImageSecurityDatabaseGuid
> 
>  [Pcd]
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64    ##
> SOMETIMES_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..398e7d9060ab 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -82,6 +82,7 @@ [LibraryClasses]
>    UefiBootServicesTableLib
>    VariablePolicyLib
>    VariablePolicyHelperLib
> +  SafeIntLib
> 
>  [Protocols]
>    gEfiSmmFirmwareVolumeBlockProtocolGuid        ## CONSUMES
> @@ -121,15 +122,16 @@ [Guids]
>    gSmmVariableWriteGuid                         ## PRODUCES             ## GUID # Install
> protocol
>    gEfiSystemNvDataFvGuid                        ## CONSUMES             ## GUID
>    gEdkiiFaultTolerantWriteGuid                  ## SOMETIMES_CONSUMES   ## HOB
> +  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
> 
>    ## SOMETIMES_CONSUMES   ## Variable:L"VarErrorFlag"
>    ## SOMETIMES_PRODUCES   ## Variable:L"VarErrorFlag"
>    gEdkiiVarErrorFlagGuid
> 
>  [Pcd]
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase       ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ##
> SOMETIMES_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..efc9290ae759 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> @@ -73,6 +73,7 @@ [LibraryClasses]
>    HobLib
>    MemoryAllocationLib
>    MmServicesTableLib
> +  SafeIntLib
>    StandaloneMmDriverEntryPoint
>    SynchronizationLib
>    VarCheckLib
> @@ -114,6 +115,7 @@ [Guids]
> 
>    gEfiSystemNvDataFvGuid                        ## CONSUMES             ## GUID
>    gEdkiiFaultTolerantWriteGuid                  ## SOMETIMES_CONSUMES   ## HOB
> +  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
> 
>    ## SOMETIMES_CONSUMES   ## Variable:L"VarErrorFlag"
>    ## SOMETIMES_PRODUCES   ## Variable:L"VarErrorFlag"
> @@ -121,8 +123,8 @@ [Guids]
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase       ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64     ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize       ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize                  ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize              ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize          ##
> CONSUMES
> --
> 2.28.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#88463): https://edk2.groups.io/g/devel/message/88463
> Mute This Topic: https://groups.io/mt/90293666/1768737
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-=-=-=-=
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/FaultTolerantWrite: Consume Variable Info HOB
  2022-04-06 16:26 ` [PATCH v1 3/3] MdeModulePkg/FaultTolerantWrite: " Michael Kubacki
@ 2022-04-07  3:31   ` Wu, Hao A
  0 siblings, 0 replies; 9+ messages in thread
From: Wu, Hao A @ 2022-04-07  3:31 UTC (permalink / raw)
  To: devel@edk2.groups.io, mikuback@linux.microsoft.com
  Cc: Wang, Jian J, Gao, Liming

Two minor inline comments:


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Thursday, April 7, 2022 12:27 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/FaultTolerantWrite:
> Consume Variable Info HOB
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
> 
> Adds support to the UEFI variable fault tolerant write (FTW) drivers
> to receive FTW base and size information dynamically via the Variable
> Flash Information HOB.
> 
> This supplements support added earlier for the variable store base
> address and size passed with the Variable Flash Information HOB.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c                          |
> 111 +++++++++++++++++---
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> |   7 +-
>  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> |  92 ++++++++++++++--
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
> |   9 +-
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> |  11 +-
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> |  11 +-
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon
> eMm.inf |  11 +-
>  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> |  10 +-
>  8 files changed, 221 insertions(+), 41 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
> index 661e1487673b..5066fe0d3711 100644
> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
> @@ -972,6 +972,48 @@ GetPreviousRecordOfWrites (
>    return EFI_SUCCESS;
>  }
> 
> +/**
> +  Get the HOB that contains variable flash information.
> +
> +  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable
> flash information structure.
> +
> +  @retval EFI_SUCCESS             Variable flash information was found successfully.
> +  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is
> NULL.
> +  @retval EFI_NOT_FOUND           Variable flash information could not be found.
> +
> +**/
> +EFI_STATUS
> +GetVariableFlashInfo (
> +  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
> +  )
> +{
> +  EFI_HOB_GUID_TYPE  *GuidHob;
> +
> +  if (VariableFlashInfo == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
> +  if (GuidHob == NULL) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
> +
> +  //
> +  // Assert if more than one variable flash information HOB is present.
> +  //
> +  DEBUG_CODE (
> +    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB
> (GuidHob)) != NULL)) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information
> HOBs\n"));
> +    ASSERT (FALSE);
> +  }
> +
> +    );
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Allocate private data for FTW driver and initialize it.
> 
> @@ -987,22 +1029,71 @@ InitFtwDevice (
>    OUT EFI_FTW_DEVICE  **FtwData
>    )
>  {
> -  EFI_FTW_DEVICE  *FtwDevice;
> +  EFI_STATUS           Status;
> +  EFI_STATUS           VariableFlashInfoStatus;
> +  UINTN                FtwWorkingSize;
> +  VARIABLE_FLASH_INFO  *VariableFlashInfo;
> +  EFI_FTW_DEVICE       *FtwDevice;
> +
> +  FtwWorkingSize          = 0;
> +  VariableFlashInfoStatus = GetVariableFlashInfo (&VariableFlashInfo);
> +  if (!EFI_ERROR (VariableFlashInfoStatus)) {
> +    Status = SafeUint64ToUintn (VariableFlashInfo->FtwWorkingLength,
> &FtwWorkingSize);
> +    if (EFI_ERROR (Status)) {
> +      ASSERT_EFI_ERROR (Status);
> +      FtwWorkingSize = 0;
> +    }
> +  }
> +
> +  if (FtwWorkingSize == 0) {
> +    FtwWorkingSize = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
> +  }
> 
>    //
>    // Allocate private data of this driver,
>    // Including the FtwWorkSpace[FTW_WORK_SPACE_SIZE].
>    //
> -  FtwDevice = AllocateZeroPool (sizeof (EFI_FTW_DEVICE) + PcdGet32
> (PcdFlashNvStorageFtwWorkingSize));
> +  FtwDevice = AllocateZeroPool (sizeof (EFI_FTW_DEVICE) + FtwWorkingSize);
>    if (FtwDevice == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> 
> +  FtwDevice->WorkSpaceLength = FtwWorkingSize;
> +
> +  if (!EFI_ERROR (VariableFlashInfoStatus)) {
> +    // This driver currently assumes the size will be UINT32 so only accept


"the size will be UINT32", UINTN to be more precise.


> +    // that for now.
> +    Status = SafeUint64ToUintn (VariableFlashInfo->FtwSpareLength,
> &FtwDevice->SpareAreaLength);
> +    if (EFI_ERROR (Status)) {
> +      ASSERT_EFI_ERROR (Status);
> +      FtwDevice->SpareAreaLength = 0;
> +    }
> +
> +    FtwDevice->SpareAreaAddress = VariableFlashInfo->FtwSpareBaseAddress;
> +    FtwDevice->WorkSpaceAddress = VariableFlashInfo-
> >FtwWorkingBaseAddress;
> +  }
> +
> +  if (FtwDevice->SpareAreaLength == 0) {
> +    FtwDevice->SpareAreaLength = (UINTN)PcdGet32
> (PcdFlashNvStorageFtwSpareSize);
> +  }
> +
> +  if (FtwDevice->SpareAreaAddress == 0) {
> +    FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwSpareBase64);
> +    if (FtwDevice->SpareAreaAddress == 0) {
> +      FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwSpareBase);
> +    }
> +  }
> +
> +  if (FtwDevice->WorkSpaceAddress == 0) {
> +    FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwWorkingBase64);
> +    if (FtwDevice->WorkSpaceAddress == 0) {
> +      FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwWorkingBase);
> +    }
> +  }
> +
>    //
>    // Initialize other parameters, and set WorkSpace as FTW_ERASED_BYTE.
>    //
> -  FtwDevice->WorkSpaceLength = (UINTN)PcdGet32
> (PcdFlashNvStorageFtwWorkingSize);
> -  FtwDevice->SpareAreaLength = (UINTN)PcdGet32
> (PcdFlashNvStorageFtwSpareSize);
>    if ((FtwDevice->WorkSpaceLength == 0) || (FtwDevice->SpareAreaLength == 0))
> {
>      DEBUG ((DEBUG_ERROR, "Ftw: Workspace or Spare block does not exist!\n"));
>      FreePool (FtwDevice);
> @@ -1015,16 +1106,6 @@ InitFtwDevice (
>    FtwDevice->FtwWorkSpaceLba = (EFI_LBA)(-1);
>    FtwDevice->FtwSpareLba     = (EFI_LBA)(-1);
> 
> -  FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwWorkingBase64);
> -  if (FtwDevice->WorkSpaceAddress == 0) {
> -    FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwWorkingBase);
> -  }
> -
> -  FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwSpareBase64);
> -  if (FtwDevice->SpareAreaAddress == 0) {
> -    FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwSpareBase);
> -  }
> -
>    *FtwData = FtwDevice;
>    return EFI_SUCCESS;
>  }
> @@ -1277,7 +1358,7 @@ InitFtwProtocol (
>    FtwDevice->FtwLastWriteHeader = NULL;
>    FtwDevice->FtwLastWriteRecord = NULL;
> 
> -  InitializeLocalWorkSpaceHeader ();
> +  InitializeLocalWorkSpaceHeader (FtwDevice->WorkSpaceLength);
> 
>    //
>    // Refresh the working space data from working block
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> index 61e7a92ccea1..fd563643eb63 100644
> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> @@ -16,10 +16,13 @@ EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER
> mWorkingBlockHeader = { ZERO_GUID, 0, 0
> 
>    Since Signature and WriteQueueSize have been known, Crc can be calculated
> out,
>    then the work space header will be fixed.
> +
> +  @param[in]  WorkSpaceLength     Length in bytes of the FTW workspace area.
> +
>  **/
>  VOID
>  InitializeLocalWorkSpaceHeader (
> -  VOID
> +  IN  UINTN  WorkSpaceLength
>    )
>  {
>    //
> @@ -46,7 +49,7 @@ InitializeLocalWorkSpaceHeader (
>      &gEdkiiWorkingBlockSignatureGuid,
>      sizeof (EFI_GUID)
>      );
> -  mWorkingBlockHeader.WriteQueueSize = PcdGet32
> (PcdFlashNvStorageFtwWorkingSize) - sizeof
> (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER);
> +  mWorkingBlockHeader.WriteQueueSize = WorkSpaceLength - sizeof
> (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER);
> 
>    //
>    // Crc is calculated with all the fields except Crc and STATE, so leave them as
> FTW_ERASED_BYTE.
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> index 15543f12ed29..7abf8f72d569 100644
> --- a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> +++ b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> @@ -11,11 +11,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #include <Guid/SystemNvDataGuid.h>
>  #include <Guid/FaultTolerantWrite.h>
> +#include <Guid/VariableFlashInfo.h>
>  #include <Library/PeiServicesLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/HobLib.h>
> +#include <Library/SafeIntLib.h>
> 
>  EFI_PEI_PPI_DESCRIPTOR  mPpiListVariable = {
>    (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> @@ -186,6 +188,48 @@ IsValidWorkSpace (
>    return TRUE;
>  }
> 
> +/**
> +  Get the HOB that contains variable flash information.
> +
> +  @param[out] VariableFlashInfo   Pointer to a pointer to set to the variable
> flash information structure.
> +
> +  @retval EFI_SUCCESS             Variable flash information was found successfully.
> +  @retval EFI_INVALID_PARAMETER   The VariableFlashInfo pointer given is
> NULL.
> +  @retval EFI_NOT_FOUND           Variable flash information could not be found.
> +
> +**/
> +EFI_STATUS
> +GetVariableFlashInfo (
> +  OUT VARIABLE_FLASH_INFO  **VariableFlashInfo
> +  )
> +{
> +  EFI_HOB_GUID_TYPE  *GuidHob;
> +
> +  if (VariableFlashInfo == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
> +  if (GuidHob == NULL) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
> +
> +  //
> +  // Assert if more than one variable flash information HOB is present.
> +  //
> +  DEBUG_CODE (
> +    if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB
> (GuidHob)) != NULL)) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information
> HOBs\n"));
> +    ASSERT (FALSE);
> +  }
> +
> +    );
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Main entry for Fault Tolerant Write PEIM.
> 
> @@ -207,6 +251,7 @@ PeimFaultTolerantWriteInitialize (
>    EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER  *FtwWorkingBlockHeader;
>    EFI_FAULT_TOLERANT_WRITE_HEADER          *FtwLastWriteHeader;
>    EFI_FAULT_TOLERANT_WRITE_RECORD          *FtwLastWriteRecord;
> +  VARIABLE_FLASH_INFO                      *VariableFlashInfo;
>    EFI_PHYSICAL_ADDRESS                     WorkSpaceAddress;
>    UINTN                                    WorkSpaceLength;
>    EFI_PHYSICAL_ADDRESS                     SpareAreaAddress;
> @@ -218,19 +263,52 @@ PeimFaultTolerantWriteInitialize (
>    FtwLastWriteHeader    = NULL;
>    FtwLastWriteRecord    = NULL;
> 
> -  WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwWorkingBase64);
> -  if (WorkSpaceAddress == 0) {
> -    WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwWorkingBase);
> +  SpareAreaAddress = 0;
> +  SpareAreaLength  = 0;
> +  WorkSpaceAddress = 0;
> +  WorkSpaceLength  = 0;
> +
> +  Status = GetVariableFlashInfo (&VariableFlashInfo);
> +  if (!EFI_ERROR (Status)) {
> +    // This driver currently assumes the size will be UINT32 so only accept


"the size will be UINT32", UINTN to be more precise.

Best Regards,
Hao Wu


> +    // that for now.
> +    Status = SafeUint64ToUintn (VariableFlashInfo->FtwSpareLength,
> &SpareAreaLength);
> +    if (EFI_ERROR (Status)) {
> +      ASSERT_EFI_ERROR (Status);
> +      SpareAreaLength = 0;
> +    }
> +
> +    Status = SafeUint64ToUintn (VariableFlashInfo->FtwWorkingLength,
> &WorkSpaceLength);
> +    if (EFI_ERROR (Status)) {
> +      ASSERT_EFI_ERROR (Status);
> +      WorkSpaceLength = 0;
> +    }
> +
> +    SpareAreaAddress = VariableFlashInfo->FtwSpareBaseAddress;
> +    WorkSpaceAddress = VariableFlashInfo->FtwWorkingBaseAddress;
>    }
> 
> -  WorkSpaceLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
> +  if (SpareAreaLength == 0) {
> +    SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
> +  }
> +
> +  if (WorkSpaceLength == 0) {
> +    WorkSpaceLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
> +  }
> 
> -  SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwSpareBase64);
>    if (SpareAreaAddress == 0) {
> -    SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwSpareBase);
> +    SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwSpareBase64);
> +    if (SpareAreaAddress == 0) {
> +      SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwSpareBase);
> +    }
>    }
> 
> -  SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
> +  if (WorkSpaceAddress == 0) {
> +    WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdFlashNvStorageFtwWorkingBase64);
> +    if (WorkSpaceAddress == 0) {
> +      WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32
> (PcdFlashNvStorageFtwWorkingBase);
> +    }
> +  }
> 
>    //
>    // The address of FTW working base and spare base must not be 0.
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
> index c14e47b5c7b2..40ef44c069eb 100644
> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
> @@ -14,11 +14,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <PiDxe.h>
> 
>  #include <Guid/SystemNvDataGuid.h>
> +#include <Guid/VariableFlashInfo.h>
>  #include <Guid/ZeroGuid.h>
>  #include <Protocol/FaultTolerantWrite.h>
>  #include <Protocol/FirmwareVolumeBlock.h>
>  #include <Protocol/SwapAddressRange.h>
> 
> +#include <Library/HobLib.h>
> +#include <Library/SafeIntLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/UefiLib.h>
> @@ -706,12 +709,16 @@ InitFtwProtocol (
>  /**
>    Initialize a local work space header.
> 
> +
>    Since Signature and WriteQueueSize have been known, Crc can be calculated
> out,
>    then the work space header will be fixed.
> +
> +  @param[in]  WorkSpaceLength     Length in bytes of the FTW workspace area.
> +
>  **/
>  VOID
>  InitializeLocalWorkSpaceHeader (
> -  VOID
> +  IN  UINTN  WorkSpaceLength
>    );
> 
>  /**
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> index 96165614d178..923f0231a047 100644
> ---
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +++
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> @@ -46,6 +46,8 @@ [LibraryClasses]
>    UefiLib
>    PcdLib
>    ReportStatusCodeLib
> +  HobLib
> +  SafeIntLib
> 
>  [Guids]
>    #
> @@ -54,6 +56,7 @@ [Guids]
>    ## CONSUMES           ## GUID
>    ## PRODUCES           ## GUID
>    gEdkiiWorkingBlockSignatureGuid
> +  gVariableFlashInfoHobGuid       ## CONSUMES   ## HOB
> 
>  [Protocols]
>    gEfiSwapAddressRangeProtocolGuid |
> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable ##
> SOMETIMES_CONSUMES
> @@ -67,11 +70,11 @@ [FeaturePcd]
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> SOMETIMES_CONSUMES
> 
>  #
>  # gBS->CalculateCrc32() is consumed in EntryPoint.
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> index 8cc6028470d8..bbed17e4b611 100644
> ---
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> +++
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> @@ -52,6 +52,8 @@ [LibraryClasses]
>    ReportStatusCodeLib
>    SmmMemLib
>    BaseLib
> +  HobLib
> +  SafeIntLib
> 
>  [Guids]
>    #
> @@ -60,6 +62,7 @@ [Guids]
>    ## CONSUMES           ## GUID
>    ## PRODUCES           ## GUID
>    gEdkiiWorkingBlockSignatureGuid
> +  gVariableFlashInfoHobGuid       ## CONSUMES   ## HOB
> 
>  [Protocols]
>    gEfiSmmSwapAddressRangeProtocolGuid |
> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##
> SOMETIMES_CONSUMES
> @@ -76,11 +79,11 @@ [FeaturePcd]
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> SOMETIMES_CONSUMES
> 
>  #
>  # gBS->CalculateCrc32() is consumed in EntryPoint.
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.inf
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.inf
> index d0fab7d9414f..0b394b04da7b 100644
> ---
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.inf
> +++
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.inf
> @@ -46,10 +46,12 @@ [LibraryClasses]
>    BaseLib
>    BaseMemoryLib
>    DebugLib
> +  HobLib
>    MemoryAllocationLib
>    MmServicesTableLib
>    PcdLib
>    ReportStatusCodeLib
> +  SafeIntLib
>    StandaloneMmDriverEntryPoint
> 
>  [Guids]
> @@ -59,6 +61,7 @@ [Guids]
>    ## CONSUMES           ## GUID
>    ## PRODUCES           ## GUID
>    gEdkiiWorkingBlockSignatureGuid
> +  gVariableFlashInfoHobGuid       ## CONSUMES   ## HOB
> 
>  [Protocols]
>    gEfiSmmSwapAddressRangeProtocolGuid |
> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##
> SOMETIMES_CONSUMES
> @@ -75,11 +78,11 @@ [FeaturePcd]
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> SOMETIMES_CONSUMES
> 
>  [Depex]
>    TRUE
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> index f90892ad4493..ff9d53bf7040 100644
> --- a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> +++
> b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> @@ -39,6 +39,7 @@ [LibraryClasses]
>    HobLib
>    BaseMemoryLib
>    PcdLib
> +  SafeIntLib
> 
>  [Guids]
>    ## SOMETIMES_PRODUCES   ## HOB
> @@ -46,14 +47,15 @@ [Guids]
>    gEdkiiFaultTolerantWriteGuid
>    gEdkiiWorkingBlockSignatureGuid               ## SOMETIMES_CONSUMES   ##
> GUID
>    gEfiSystemNvDataFvGuid                        ## SOMETIMES_CONSUMES   ## GUID
> +  gVariableFlashInfoHobGuid                     ## CONSUMES             ## HOB
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> ## SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ##
> SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##
> SOMETIMES_CONSUMES
> 
>  [Depex]
>    TRUE
> --
> 2.28.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#88464): https://edk2.groups.io/g/devel/message/88464
> Mute This Topic: https://groups.io/mt/90293667/1768737
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-=-=-=-=
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/3] Add Variable Flash Info HOB
  2022-04-07  3:31 ` [edk2-devel] [PATCH v1 0/3] Add Variable Flash " Wu, Hao A
@ 2022-04-07 23:56   ` Michael Kubacki
  2022-04-08 21:20     ` Michael Kubacki
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Kubacki @ 2022-04-07 23:56 UTC (permalink / raw)
  To: devel, hao.a.wu; +Cc: Wang, Jian J, Gao, Liming

Yes, I considered that as well but I was not sure if it was worth adding 
a new library just for that. I agree though that the code should not be 
duplicated. I will look into it and include your other suggestions in a 
v2 series.

Thanks,
Michael

On 4/6/2022 11:31 PM, Wu, Hao A wrote:
> Sorry for a question:
> GetVariableFlashInfo() seems being defined multiple times across modules, do you see value in abstracting it as a library API?
> 
> Best Regards,
> Hao Wu
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
>> Kubacki
>> Sent: Thursday, April 7, 2022 12:27 AM
>> To: devel@edk2.groups.io
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>> Gao, Liming <gaoliming@byosoft.com.cn>
>> Subject: [edk2-devel] [PATCH v1 0/3] Add Variable Flash Info HOB
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>
>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
>> VariableStandaloneMm, etc. (and their dependent protocol/library
>> stack), typically acquire UEFI variable store flash information
>> with PCDs declared in MdeModulePkg.
>>
>> For example:
>> [Pcd]
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>>
>> These PCDs work as-is in the StandaloneMm driver if they are not
>> dynamic such as Dynamic or DynamicEx because PCD services are not
>> readily available in the Standalone MM environment. Platforms that
>> use Standalone MM today, must define these PCDs as FixedAtBuild in
>> their platform build. However, the PCDs do allow platforms to treat
>> the PCDs as Dynamic/DynamicEx and being able to support that is
>> currently a gap for Standalone MM.
>>
>> This patch series introduces a HOB that can be produced by the
>> platform to provide the same information. The HOB list is
>> available to Standalone MM.
>>
>> The PCD declarations are left as-is in MdeModulePkg for backward
>> compatibility. This means unless a platform wants to use the HOB,
>> their code will continue to work with no change (they do not need
>> to produce the HOB). Only if the HOB is found, is its value used
>> instead of the PCDs.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com
>>
>> Michael Kubacki (3):
>>    MdeModulePkg: Add Variable Flash Info HOB
>>    MdeModulePkg/Variable: Consume Variable Info HOB
>>    MdeModulePkg/FaultTolerantWrite: Consume Variable Info HOB
>>
>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c                          |
>> 111 +++++++++++++++++---
>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>> |   7 +-
>>   MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
>> |  92 ++++++++++++++--
>>   MdeModulePkg/Universal/Variable/Pei/Variable.c                                  |  66
>> +++++++++++-
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                           |  42
>> ++++++++
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c                        |
>> 25 ++++-
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
>> |  20 +++-
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c                        |
>> 20 +++-
>>   MdeModulePkg/Include/Guid/VariableFlashInfo.h                                   |  36
>> +++++++
>>   MdeModulePkg/MdeModulePkg.dec                                                   |   4 +
>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
>> |   9 +-
>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>> |  11 +-
>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
>> |  11 +-
>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon
>> eMm.inf |  11 +-
>>   MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
>> |  10 +-
>>   MdeModulePkg/Universal/Variable/Pei/Variable.h                                  |   2 +
>>   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                             |   6 +-
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                           |  17
>> +++
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> |   6 +-
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                      |
>> 6 +-
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>> |   6 +-
>>   21 files changed, 455 insertions(+), 63 deletions(-)
>>   create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>
>> --
>> 2.28.0.windows.1
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#88461): https://edk2.groups.io/g/devel/message/88461
>> Mute This Topic: https://groups.io/mt/90293658/1768737
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
>> -=-=-=-=-=-=
>>
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/3] Add Variable Flash Info HOB
  2022-04-07 23:56   ` Michael Kubacki
@ 2022-04-08 21:20     ` Michael Kubacki
  0 siblings, 0 replies; 9+ messages in thread
From: Michael Kubacki @ 2022-04-08 21:20 UTC (permalink / raw)
  To: devel, hao.a.wu; +Cc: Wang, Jian J, Gao, Liming

Hi Hao,

I addressed all of your feedback including abstraction of the 
information with a library API in v2. It did help clean up the 
consumption code quite a bit.

https://edk2.groups.io/g/devel/message/88649

Thanks,
Michael

On 4/7/2022 7:56 PM, Michael Kubacki wrote:
> Yes, I considered that as well but I was not sure if it was worth adding 
> a new library just for that. I agree though that the code should not be 
> duplicated. I will look into it and include your other suggestions in a 
> v2 series.
> 
> Thanks,
> Michael
> 
> On 4/6/2022 11:31 PM, Wu, Hao A wrote:
>> Sorry for a question:
>> GetVariableFlashInfo() seems being defined multiple times across 
>> modules, do you see value in abstracting it as a library API?
>>
>> Best Regards,
>> Hao Wu
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
>>> Kubacki
>>> Sent: Thursday, April 7, 2022 12:27 AM
>>> To: devel@edk2.groups.io
>>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A 
>>> <hao.a.wu@intel.com>;
>>> Gao, Liming <gaoliming@byosoft.com.cn>
>>> Subject: [edk2-devel] [PATCH v1 0/3] Add Variable Flash Info HOB
>>>
>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>
>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>>
>>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
>>> VariableStandaloneMm, etc. (and their dependent protocol/library
>>> stack), typically acquire UEFI variable store flash information
>>> with PCDs declared in MdeModulePkg.
>>>
>>> For example:
>>> [Pcd]
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>>>
>>> These PCDs work as-is in the StandaloneMm driver if they are not
>>> dynamic such as Dynamic or DynamicEx because PCD services are not
>>> readily available in the Standalone MM environment. Platforms that
>>> use Standalone MM today, must define these PCDs as FixedAtBuild in
>>> their platform build. However, the PCDs do allow platforms to treat
>>> the PCDs as Dynamic/DynamicEx and being able to support that is
>>> currently a gap for Standalone MM.
>>>
>>> This patch series introduces a HOB that can be produced by the
>>> platform to provide the same information. The HOB list is
>>> available to Standalone MM.
>>>
>>> The PCD declarations are left as-is in MdeModulePkg for backward
>>> compatibility. This means unless a platform wants to use the HOB,
>>> their code will continue to work with no change (they do not need
>>> to produce the HOB). Only if the HOB is found, is its value used
>>> instead of the PCDs.
>>>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com
>>>
>>> Michael Kubacki (3):
>>>    MdeModulePkg: Add Variable Flash Info HOB
>>>    MdeModulePkg/Variable: Consume Variable Info HOB
>>>    MdeModulePkg/FaultTolerantWrite: Consume Variable Info HOB
>>>
>>>   
>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c                          
>>> |
>>> 111 +++++++++++++++++---
>>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>>> |   7 +-
>>>   MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
>>> |  92 ++++++++++++++--
>>>   
>>> MdeModulePkg/Universal/Variable/Pei/Variable.c                                  
>>> |  66
>>> +++++++++++-
>>>   
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                           
>>> |  42
>>> ++++++++
>>>   
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c                        
>>> |
>>> 25 ++++-
>>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
>>> |  20 +++-
>>>   
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c                        
>>> |
>>> 20 +++-
>>>   
>>> MdeModulePkg/Include/Guid/VariableFlashInfo.h                                   
>>> |  36
>>> +++++++
>>>   
>>> MdeModulePkg/MdeModulePkg.dec                                                   
>>> |   4 +
>>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
>>> |   9 +-
>>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>>> |  11 +-
>>>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
>>> |  11 +-
>>>
>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon
>>> eMm.inf |  11 +-
>>>   MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
>>> |  10 +-
>>>   
>>> MdeModulePkg/Universal/Variable/Pei/Variable.h                                  
>>> |   2 +
>>>   
>>> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                             
>>> |   6 +-
>>>   
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                           
>>> |  17
>>> +++
>>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>>> |   6 +-
>>>   
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                      
>>> |
>>> 6 +-
>>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>>> |   6 +-
>>>   21 files changed, 455 insertions(+), 63 deletions(-)
>>>   create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>>
>>> -- 
>>> 2.28.0.windows.1
>>>
>>>
>>>
>>> -=-=-=-=-=-=
>>> Groups.io Links: You receive all messages sent to this group.
>>> View/Reply Online (#88461): https://edk2.groups.io/g/devel/message/88461
>>> Mute This Topic: https://groups.io/mt/90293658/1768737
>>> Group Owner: devel+owner@edk2.groups.io
>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
>>> -=-=-=-=-=-=
>>>
>>
>>
>>
>> 
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-04-08 21:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-06 16:26 [PATCH v1 0/3] Add Variable Flash Info HOB Michael Kubacki
2022-04-06 16:26 ` [PATCH v1 1/3] MdeModulePkg: " Michael Kubacki
2022-04-06 16:26 ` [PATCH v1 2/3] MdeModulePkg/Variable: Consume Variable " Michael Kubacki
2022-04-07  3:31   ` [edk2-devel] " Wu, Hao A
2022-04-06 16:26 ` [PATCH v1 3/3] MdeModulePkg/FaultTolerantWrite: " Michael Kubacki
2022-04-07  3:31   ` [edk2-devel] " Wu, Hao A
2022-04-07  3:31 ` [edk2-devel] [PATCH v1 0/3] Add Variable Flash " Wu, Hao A
2022-04-07 23:56   ` Michael Kubacki
2022-04-08 21:20     ` Michael Kubacki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox