[edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[duntan]

This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB is used to store the address and size of the buffer that will be used for variable runtime service when the PcdEnableVariableRuntimeCache is TRUE.
In following patches, when PcdEnableVariableRuntimeCache is TRUE, VariablePei will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate the needed buffer for different type variable runtime cache and build the HOB.
Then VariableSmmRuntimeDxe driver will consume gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable runtime cache related content. The code to allocate and unblock the runtime cache buffer in VariableSmmRuntimeDxe is also removed in this patc set.

PR for review: https://github.com/tianocore/edk2/pull/5607

Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>

Dun Tan (9):
  MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid
  ArmVirtPkg: Add MmUnblockMemoryLib in DSC
  EmulatorPkg: Add MmUnblockMemoryLib in DSC
  OvmfPkg: Add MmUnblockMemoryLib in DSC
  MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid
  MdeModulePkg:Remove unnecessary global variable
  MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid
  MdeModulePkg: Refine InitVariableCache()
  MdeModulePkg:Add global variable mVariableRtCacheInfo

 ArmVirtPkg/ArmVirtCloudHv.dsc                                        |   2 ++
 EmulatorPkg/EmulatorPkg.dsc                                          |   1 +
 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h                 |  65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                                        |   3 +++
 MdeModulePkg/Universal/Variable/Pei/Variable.c                       | 298 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 MdeModulePkg/Universal/Variable/Pei/Variable.h                       |   3 +++
 MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                  |   8 +++++++-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 293 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--
 OvmfPkg/OvmfPkgIa32X64.dsc                                           |   2 +-
 10 files changed, 506 insertions(+), 174 deletions(-)
 create mode 100644 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h

-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119017): https://edk2.groups.io/g/devel/message/119017
Mute This Topic: https://groups.io/mt/106150796/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/9] MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid

[duntan]

This commit defines VARIABLE_RUNTIME_CACHE_INFO HOB.
The HOB is used to store the address and size of the
buffer that will be used for variable runtime service
when the PcdEnableVariableRuntimeCache is TRUE.

In following patches, when PcdEnableVariableRuntimeCache
is TRUE, VariablePei module will install a callback of
gEfiPeiMemoryDiscoveredPpiGuid to allocate needed buffer
for different type cache, unblock the buffer and build HOB.
Then VariableSmmRuntimeDxe driver will consume the
gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the
variable runtime cache related content.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                        |  3 +++
 2 files changed, 68 insertions(+)

diff --git a/MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h b/MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
new file mode 100644
index 0000000000..c2a8b77945
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
@@ -0,0 +1,65 @@
+/** @file
+  This Variable Runtime Cache Info HOB is used to store the address
+  and the size of the buffer that will be used for variable runtime
+  service when the PcdEnableVariableRuntimeCache is TRUE.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef VARIABLE_RUNTIME_CACHE_INFO_H_
+#define VARIABLE_RUNTIME_CACHE_INFO_H_
+
+#include <PiPei.h>
+
+#define VARIABLE_RUNTIME_CACHE_INFO_HOB_REVISION  1
+
+#define VARIABLE_RUNTIME_CACHE_INFO_GUID \
+  { \
+    0x0f472f7d, 0x6713, 0x4915, {0x96, 0x14, 0x5d, 0xda, 0x28, 0x40, 0x10, 0x56}  \
+  }
+
+#pragma pack(1)
+typedef struct {
+  ///
+  /// TRUE indicates GetVariable () or GetNextVariable () is being called.
+  /// When the value is FALSE, the given update (and any other pending updates)
+  /// can be flushed to the runtime cache.
+  ///
+  BOOLEAN    ReadLock;
+  ///
+  /// TRUE indicates there is pending update for the given variable store needed
+  /// to be flushed to the runtime cache.
+  ///
+  BOOLEAN    PendingUpdate;
+  ///
+  /// TRUE indicates all HOB variables have been flushed in flash.
+  ///
+  BOOLEAN    HobFlushComplete;
+} CACHE_INFO_FLAG;
+
+typedef struct {
+  CACHE_INFO_FLAG    *CacheInfoFlag;
+  ///
+  /// Buffer reserved for runtime Hob cache
+  ///
+  UINT64             RuntimeHobCacheBuffer;
+  UINTN              RuntimeHobCachePages;
+  ///
+  /// Buffer reserved for Non-Volatile runtime cache
+  ///
+  UINT64             RuntimeNvCacheBuffer;
+  UINTN              RuntimeNvCachePages;
+  ///
+  /// Buffer reserved for Volatile runtime cache
+  ///
+  UINT64             RuntimeVolatileCacheBuffer;
+  UINTN              RuntimeVolatileCachePages;
+} VARIABLE_RUNTIME_CACHE_INFO;
+#pragma pack()
+
+extern EFI_GUID  gEdkiiVariableRuntimeCacheInfoHobGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index f7339f0aec..1bf5e31b7c 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -465,6 +465,9 @@
   gEdk2JedecSfdpSpiDxeDriverGuid  = { 0xBE71701E, 0xB63C, 0x4574, { 0x9C, 0x5C, 0x36, 0x29, 0xE8, 0xEA, 0xC4, 0x14 }}
   gEdk2JedecSfdpSpiSmmDriverGuid  = { 0x95A1E915, 0x195C, 0x477C, { 0x92, 0x6F, 0x7E, 0x24, 0x67, 0xC1, 0xB3, 0x1F }}
 
+  ## Include/Guid/VariableRuntimeCacheInfo.h
+  gEdkiiVariableRuntimeCacheInfoHobGuid = { 0x0f472f7d, 0x6713, 0x4915, { 0x96, 0x14, 0x5d, 0xda, 0x28, 0x40, 0x10, 0x56 }}
+
 [Ppis]
   ## Include/Ppi/FirmwareVolumeShadowPpi.h
   gEdkiiPeiFirmwareVolumeShadowPpiGuid = { 0x7dfe756c, 0xed8d, 0x4d77, {0x9e, 0xc4, 0x39, 0x9a, 0x8a, 0x81, 0x51, 0x16 } }
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119018): https://edk2.groups.io/g/devel/message/119018
Mute This Topic: https://groups.io/mt/106150797/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/9] ArmVirtPkg: Add MmUnblockMemoryLib in DSC

[duntan]

Add MmUnblockMemoryLib in ArmVirtCloudHv.dsc.
This lib will be required by VariablePei in
following commits.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 ArmVirtPkg/ArmVirtCloudHv.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index 5cb2a609b1..a8ede49ef9 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -57,6 +57,8 @@
   TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
   TpmPlatformHierarchyLib|SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLibNull/PeiDxeTpmPlatformHierarchyLib.inf
 
+  MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
+
 !include MdePkg/MdeLibs.dsc.inc
 
 [LibraryClasses.common.PEIM]
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119019): https://edk2.groups.io/g/devel/message/119019
Mute This Topic: https://groups.io/mt/106150798/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/9] EmulatorPkg: Add MmUnblockMemoryLib in DSC

[duntan]

Add MmUnblockMemoryLib in EmulatorPkg.dsc.
This lib will be required by VariablePei in
following commits.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
---
 EmulatorPkg/EmulatorPkg.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 5fa1ed345a..0e15dafb5c 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -127,6 +127,7 @@
   ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   ImagePropertiesRecordLib|MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.inf
+  MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   RngLib|MdeModulePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119020): https://edk2.groups.io/g/devel/message/119020
Mute This Topic: https://groups.io/mt/106150799/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 4/9] OvmfPkg: Add MmUnblockMemoryLib in DSC

[duntan]

Add MmUnblockMemoryLib in OvmfPkgIa32X64.dsc.
This lib will be required by VariablePei in
following commits.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index d27a4c7278..cc03104aac 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -233,7 +233,7 @@
   VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
   VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
   VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
-
+  MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
 
   #
   # Network libraries
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119021): https://edk2.groups.io/g/devel/message/119021
Mute This Topic: https://groups.io/mt/106150800/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 5/9] MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid

[duntan]

Install the callback of gEfiPeiMemoryDiscoveredPpiGuid
to create gEdkiiVariableRuntimeCacheInfoHobGuid in
VariablePei module. When PcdEnableVariableRuntimeCache
is TRUE, the callback will be installed to allocate
the needed buffer for different type variable runtime
cache, unblock the buffer and build this HOB. Then the
runtime cache buffer address and size will be saved in
the HOB content.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/Pei/Variable.c      | 298 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 MdeModulePkg/Universal/Variable/Pei/Variable.h      |   3 +++
 MdeModulePkg/Universal/Variable/Pei/VariablePei.inf |   8 +++++++-
 3 files changed, 307 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index 26a4c73b45..15419eb437 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -2,7 +2,7 @@
   Implement ReadOnly Variable Services required by PEIM and install
   PEI ReadOnly Varaiable2 PPI. These services operates the non volatile storage space.
 
-Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.<BR>
 Copyright (c) Microsoft Corporation.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -24,6 +24,31 @@ EFI_PEI_PPI_DESCRIPTOR  mPpiListVariable = {
   &mVariablePpi
 };
 
+/**
+  Build gEdkiiVariableRuntimeCacheInfoHobGuid.
+
+  @param[in] PeiServices          General purpose services available to every PEIM.
+  @param[in] NotifyDescriptor     The notification structure this PEIM registered on install.
+  @param[in] Ppi                  The memory discovered PPI.  Not used.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval others                  Failed to build VariableRuntimeCacheInfo Hob.
+
+**/
+EFI_STATUS
+EFIAPI
+BuildVariableRuntimeCacheInfoHob (
+  IN EFI_PEI_SERVICES           **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID                       *Ppi
+  );
+
+EFI_PEI_NOTIFY_DESCRIPTOR  mPostMemNotifyList = {
+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEfiPeiMemoryDiscoveredPpiGuid,
+  BuildVariableRuntimeCacheInfoHob
+};
+
 /**
   Provide the functionality of the variable services.
 
@@ -41,6 +66,10 @@ PeimInitializeVariableServices (
   IN CONST EFI_PEI_SERVICES     **PeiServices
   )
 {
+  if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
+    PeiServicesNotifyPpi (&mPostMemNotifyList);
+  }
+
   return PeiServicesInstallPpi (&mPpiListVariable);
 }
 
@@ -1250,3 +1279,270 @@ PeiGetNextVariableName (
     }
   }
 }
+
+/**
+  Calculate the auth variable storage size converted from normal variable storage.
+
+  @param[in]  StoreInfo         Pointer to the store info
+  @param[in]  NormalHobVarStorage  Pointer to the normal variable storage header
+
+  @retval the auth variable storage size
+**/
+UINTN
+CalculateAuthVarStorageSize (
+  IN  VARIABLE_STORE_INFO    *StoreInfo,
+  IN  VARIABLE_STORE_HEADER  *NormalHobVarStorage
+  )
+{
+  VARIABLE_HEADER  *StartPtr;
+  VARIABLE_HEADER  *EndPtr;
+  UINTN            AuthVarStroageSize;
+
+  AuthVarStroageSize = sizeof (VARIABLE_STORE_HEADER);
+
+  //
+  // Calculate Auth Variable Storage Size
+  //
+  StartPtr = GetStartPointer (NormalHobVarStorage);
+  EndPtr   = GetEndPointer (NormalHobVarStorage);
+  while (StartPtr < EndPtr) {
+    if (StartPtr->State == VAR_ADDED) {
+      AuthVarStroageSize  = HEADER_ALIGN (AuthVarStroageSize);
+      AuthVarStroageSize += sizeof (AUTHENTICATED_VARIABLE_HEADER);
+      AuthVarStroageSize += StartPtr->NameSize + GET_PAD_SIZE (StartPtr->NameSize);
+      AuthVarStroageSize += StartPtr->DataSize + GET_PAD_SIZE (StartPtr->DataSize);
+    }
+
+    StartPtr = GetNextVariablePtr (StoreInfo, StartPtr, StartPtr);
+  }
+
+  return AuthVarStroageSize;
+}
+
+/**
+  Calculate Hob variable cache size.
+
+  @param[in]  NvAuthFlag   If the NV variable store is Auth.
+
+  @retval Maximum of Nv variable cache size.
+
+**/
+UINTN
+CalculateHobVariableCacheSize (
+  IN BOOLEAN  NvAuthFlag
+  )
+{
+  VARIABLE_STORE_INFO    StoreInfo;
+  VARIABLE_STORE_HEADER  *VariableStoreHeader;
+
+  VariableStoreHeader = NULL;
+  GetHobVariableStore (&StoreInfo, &VariableStoreHeader);
+
+  if (VariableStoreHeader == NULL) {
+    return 0;
+  }
+
+  if (NvAuthFlag == StoreInfo.AuthFlag) {
+    return VariableStoreHeader->Size;
+  } else {
+    //
+    // Normal NV variable store + Auth HOB variable store is not supported
+    //
+    ASSERT (NvAuthFlag && (!StoreInfo.AuthFlag));
+
+    //
+    // Need to calculate auth variable storage size converted from normal variable storage
+    //
+    return CalculateAuthVarStorageSize (&StoreInfo, VariableStoreHeader);
+  }
+}
+
+/**
+  Calculate Nv variable cache size.
+
+  @param[out]  NvAuthFlag   If the NV variable store is Auth.
+
+  @retval Maximum of Nv variable cache size.
+
+**/
+UINTN
+CalculateNvVariableCacheSize (
+  OUT BOOLEAN  *NvAuthFlag
+  )
+{
+  EFI_STATUS                            Status;
+  EFI_HOB_GUID_TYPE                     *GuidHob;
+  EFI_FIRMWARE_VOLUME_HEADER            *FvHeader;
+  VARIABLE_STORE_HEADER                 *VariableStoreHeader;
+  EFI_PHYSICAL_ADDRESS                  NvStorageBase;
+  UINT32                                NvStorageSize;
+  UINT64                                NvStorageSize64;
+  FAULT_TOLERANT_WRITE_LAST_WRITE_DATA  *FtwLastWriteData;
+
+  if (PcdGetBool (PcdEmuVariableNvModeEnable)) {
+    return PcdGet32 (PcdVariableStoreSize);
+  }
+
+  Status = GetVariableFlashNvStorageInfo (&NvStorageBase, &NvStorageSize64);
+  ASSERT_EFI_ERROR (Status);
+
+  Status = SafeUint64ToUint32 (NvStorageSize64, &NvStorageSize);
+  ASSERT_EFI_ERROR (Status);
+  ASSERT (NvStorageBase != 0);
+  FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)NvStorageBase;
+
+  //
+  // Check the FTW last write data hob.
+  //
+  GuidHob = GetFirstGuidHob (&gEdkiiFaultTolerantWriteGuid);
+  if (GuidHob != NULL) {
+    FtwLastWriteData = (FAULT_TOLERANT_WRITE_LAST_WRITE_DATA *)GET_GUID_HOB_DATA (GuidHob);
+    if (FtwLastWriteData->TargetAddress == NvStorageBase) {
+      //
+      // Let FvHeader point to spare block.
+      //
+      FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FtwLastWriteData->SpareAddress;
+    }
+  }
+
+  VariableStoreHeader = (VARIABLE_STORE_HEADER *)((UINT8 *)FvHeader + FvHeader->HeaderLength);
+  *NvAuthFlag         = (BOOLEAN)(CompareGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid));
+
+  return NvStorageSize - FvHeader->HeaderLength;
+}
+
+/**
+  Build gEdkiiVariableRuntimeCacheInfoHobGuid.
+
+  @param[in] PeiServices          General purpose services available to every PEIM.
+  @param[in] NotifyDescriptor     The notification structure this PEIM registered on install.
+  @param[in] Ppi                  The memory discovered PPI.  Not used.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval others                  Failed to build VariableRuntimeCacheInfo Hob.
+
+**/
+EFI_STATUS
+EFIAPI
+BuildVariableRuntimeCacheInfoHob (
+  IN EFI_PEI_SERVICES           **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID                       *Ppi
+  )
+{
+  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;
+  EFI_STATUS                   Status;
+  VOID                         *Buffer;
+  UINTN                        BufferSize;
+  BOOLEAN                      NvAuthFlag;
+  UINTN                        Pages;
+
+  VariableRuntimeCacheHob = BuildGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
+  ASSERT (VariableRuntimeCacheHob != NULL);
+  ZeroMem (VariableRuntimeCacheHob, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
+
+  //
+  // AllocateRuntimePages for CACHE_INFO_FLAG and unblock it.
+  //
+  Pages  = EFI_SIZE_TO_PAGES (sizeof (CACHE_INFO_FLAG));
+  Buffer = AllocateRuntimePages (Pages);
+  ASSERT (Buffer != NULL);
+  Status = MmUnblockMemoryRequest (
+             (EFI_PHYSICAL_ADDRESS)(UINTN)Buffer,
+             Pages
+             );
+  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  VariableRuntimeCacheHob->CacheInfoFlag = (CACHE_INFO_FLAG *)(UINTN)Buffer;
+  DEBUG ((
+    DEBUG_INFO,
+    "PeiVariable: CACHE_INFO_FLAG Buffer is: 0x%x, number of pages is: 0x%x\n",
+    (UINTN)(VariableRuntimeCacheHob->CacheInfoFlag),
+    Pages
+    ));
+
+  //
+  // AllocateRuntimePages for VolatileCache and unblock it.
+  //
+  BufferSize = PcdGet32 (PcdVariableStoreSize);
+  if (BufferSize > 0) {
+    Pages  = EFI_SIZE_TO_PAGES (BufferSize);
+    Buffer = AllocateRuntimePages (Pages);
+    ASSERT (Buffer != NULL);
+    Status = MmUnblockMemoryRequest (
+               (EFI_PHYSICAL_ADDRESS)(UINTN)Buffer,
+               Pages
+               );
+    if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer = (UINTN)Buffer;
+    VariableRuntimeCacheHob->RuntimeVolatileCachePages  = Pages;
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "PeiVariable: Volatile cache Buffer is: 0x%x, number of pages is: 0x%x\n",
+    VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer,
+    VariableRuntimeCacheHob->RuntimeVolatileCachePages
+    ));
+
+  //
+  // AllocateRuntimePages for NVCache and unblock it.
+  //
+  BufferSize = CalculateNvVariableCacheSize (&NvAuthFlag);
+  if (BufferSize > 0) {
+    Pages  = EFI_SIZE_TO_PAGES (BufferSize);
+    Buffer = AllocateRuntimePages (Pages);
+    ASSERT (Buffer != NULL);
+    Status = MmUnblockMemoryRequest (
+               (EFI_PHYSICAL_ADDRESS)(UINTN)Buffer,
+               Pages
+               );
+    if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    VariableRuntimeCacheHob->RuntimeNvCacheBuffer = (UINTN)Buffer;
+    VariableRuntimeCacheHob->RuntimeNvCachePages  = Pages;
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "PeiVariable: NV cache Buffer is: 0x%x, number of pages is: 0x%x\n",
+    VariableRuntimeCacheHob->RuntimeNvCacheBuffer,
+    VariableRuntimeCacheHob->RuntimeNvCachePages
+    ));
+
+  //
+  // AllocateRuntimePages for HobCache and unblock it.
+  //
+  BufferSize = CalculateHobVariableCacheSize (NvAuthFlag);
+  if (BufferSize > 0) {
+    Pages  = EFI_SIZE_TO_PAGES (BufferSize);
+    Buffer = AllocateRuntimePages (Pages);
+    ASSERT (Buffer != NULL);
+    Status = MmUnblockMemoryRequest (
+               (EFI_PHYSICAL_ADDRESS)(UINTN)Buffer,
+               Pages
+               );
+    if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    VariableRuntimeCacheHob->RuntimeHobCacheBuffer = (UINTN)Buffer;
+    VariableRuntimeCacheHob->RuntimeHobCachePages  = Pages;
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "PeiVariable: HOB cache Buffer is: 0x%x, number of pages is: 0x%x\n",
+    VariableRuntimeCacheHob->RuntimeHobCacheBuffer,
+    VariableRuntimeCacheHob->RuntimeHobCachePages
+    ));
+
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.h b/MdeModulePkg/Universal/Variable/Pei/Variable.h
index 51effbf799..aa0d79f166 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.h
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.h
@@ -22,11 +22,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/PeiServicesLib.h>
 #include <Library/SafeIntLib.h>
 #include <Library/VariableFlashInfoLib.h>
+#include <Library/MmUnblockMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
 
 #include <Guid/VariableFormat.h>
 #include <Guid/VariableIndexTable.h>
 #include <Guid/SystemNvDataGuid.h>
 #include <Guid/FaultTolerantWrite.h>
+#include <Guid/VariableRuntimeCacheInfo.h>
 
 typedef enum {
   VariableStoreTypeHob,
diff --git a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
index 7264a24bdf..f2dc7c042c 100644
--- a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+++ b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
@@ -3,7 +3,7 @@
 #
 #  This module implements ReadOnly Variable Services required by PEIM and installs PEI ReadOnly Varaiable2 PPI.
 #
-#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -41,6 +41,8 @@
   PeiServicesLib
   SafeIntLib
   VariableFlashInfoLib
+  MmUnblockMemoryLib
+  MemoryAllocationLib
 
 [Guids]
   ## CONSUMES             ## GUID # Variable store header
@@ -56,12 +58,16 @@
   ## SOMETIMES_CONSUMES   ## HOB
   ## CONSUMES             ## GUID # Dependence
   gEdkiiFaultTolerantWriteGuid
+  gEdkiiVariableRuntimeCacheInfoHobGuid
 
 [Ppis]
   gEfiPeiReadOnlyVariable2PpiGuid   ## PRODUCES
+  gEfiPeiMemoryDiscoveredPpiGuid    ## CONSUMES
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable         ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize               ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache      ## CONSUMES
 
 [Depex]
   gEdkiiFaultTolerantWriteGuid
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119022): https://edk2.groups.io/g/devel/message/119022
Mute This Topic: https://groups.io/mt/106150802/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 6/9] MdeModulePkg:Remove unnecessary global variable

[duntan]

Remove the two unnecessary global variables and
replace them by two local variables:
mVariableRuntimeNvCacheBufferSize
mVariableRuntimeVolatileCacheBufferSize

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 6930875e9f..8b42ae7d72 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -13,7 +13,7 @@
 
   InitCommunicateBuffer() is really function to check the variable data size.
 
-Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 Copyright (c) Microsoft Corporation.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -55,8 +55,6 @@ VARIABLE_STORE_HEADER           *mVariableRuntimeNvCacheBuffer       = NULL;
 VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
 UINTN                           mVariableBufferSize;
 UINTN                           mVariableRuntimeHobCacheBufferSize;
-UINTN                           mVariableRuntimeNvCacheBufferSize;
-UINTN                           mVariableRuntimeVolatileCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
 BOOLEAN                         mVariableRuntimeCachePendingUpdate;
 BOOLEAN                         mVariableRuntimeCacheReadLock;
@@ -1691,6 +1689,8 @@ SmmVariableReady (
   )
 {
   EFI_STATUS  Status;
+  UINTN       RuntimeNvCacheSize;
+  UINTN       RuntimeVolatileCacheSize;
 
   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1721,16 +1721,16 @@ SmmVariableReady (
     //
     Status =  GetRuntimeCacheInfo (
                 &mVariableRuntimeHobCacheBufferSize,
-                &mVariableRuntimeNvCacheBufferSize,
-                &mVariableRuntimeVolatileCacheBufferSize,
+                &RuntimeNvCacheSize,
+                &RuntimeVolatileCacheSize,
                 &mVariableAuthFormat
                 );
     if (!EFI_ERROR (Status)) {
       Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
       if (!EFI_ERROR (Status)) {
-        Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &mVariableRuntimeNvCacheBufferSize);
+        Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
         if (!EFI_ERROR (Status)) {
-          Status = InitVariableCache (&mVariableRuntimeVolatileCacheBuffer, &mVariableRuntimeVolatileCacheBufferSize);
+          Status = InitVariableCache (&mVariableRuntimeVolatileCacheBuffer, &RuntimeVolatileCacheSize);
           if (!EFI_ERROR (Status)) {
             Status = SendRuntimeVariableCacheContextToSmm ();
             if (!EFI_ERROR (Status)) {
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119023): https://edk2.groups.io/g/devel/message/119023
Mute This Topic: https://groups.io/mt/106150803/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

[duntan]

Consume gEdkiiVariableRuntimeCacheInfoHobGuid in

VariableSmmRuntimeDxe driver to initialize the following

variable cache related buffer:
  *mVariableRuntimeHobCacheBuffer
  *mVariableRuntimeNvCacheBuffer
  *mVariableRuntimeVolatileCacheBuffer
  *mVariableRuntimeCachePendingUpdate
  *mVariableRuntimeCacheReadLock
  *mHobFlushComplete

The code to to allocate 
and unblock the buffer for
different type cache in VariableSmmRuntimeDxe is also
removed in this commit.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--
 2 files changed, 52 insertions(+), 73 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 8b42ae7d72..68a249c5ac 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -35,10 +35,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
 #include <Library/BaseLib.h>
-#include <Library/MmUnblockMemoryLib.h>
+#include <Library/HobLib.h>
 
 #include <Guid/EventGroup.h>
 #include <Guid/SmmVariableCommon.h>
+#include <Guid/VariableRuntimeCacheInfo.h>
 
 #include "PrivilegePolymorphic.h"
 #include "VariableParsing.h"
@@ -56,10 +57,10 @@ VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
 UINTN                           mVariableBufferSize;
 UINTN                           mVariableRuntimeHobCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
-BOOLEAN                         mVariableRuntimeCachePendingUpdate;
-BOOLEAN                         mVariableRuntimeCacheReadLock;
+BOOLEAN                         *mVariableRuntimeCachePendingUpdate;
+BOOLEAN                         *mVariableRuntimeCacheReadLock;
 BOOLEAN                         mVariableAuthFormat;
-BOOLEAN                         mHobFlushComplete;
+BOOLEAN                         *mHobFlushComplete;
 EFI_LOCK                        mVariableServicesLock;
 EDKII_VARIABLE_LOCK_PROTOCOL    mVariableLock;
 EDKII_VAR_CHECK_PROTOCOL        mVarCheck;
@@ -180,27 +181,6 @@ InitVariableCache (
 
   *TotalVariableCacheSize = ALIGN_VALUE (*TotalVariableCacheSize, sizeof (UINT32));
 
-  //
-  // Allocate NV Storage Cache and initialize it to all 1's (like an erased FV)
-  //
-  *VariableCacheBuffer =  (VARIABLE_STORE_HEADER *)AllocateRuntimePages (
-                                                     EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-                                                     );
-  if (*VariableCacheBuffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // Request to unblock the newly allocated cache region to be accessible from inside MM
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)(UINTN)*VariableCacheBuffer,
-             EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    return Status;
-  }
-
   VariableCacheStorePtr = *VariableCacheBuffer;
   SetMem32 ((VOID *)VariableCacheStorePtr, *TotalVariableCacheSize, (UINT32)0xFFFFFFFF);
 
@@ -568,16 +548,16 @@ CheckForRuntimeCacheSync (
   VOID
   )
 {
-  if (mVariableRuntimeCachePendingUpdate) {
+  if (*mVariableRuntimeCachePendingUpdate) {
     SyncRuntimeCache ();
   }
 
-  ASSERT (!mVariableRuntimeCachePendingUpdate);
+  ASSERT (!(*mVariableRuntimeCachePendingUpdate));
 
   //
   // The HOB variable data may have finished being flushed in the runtime cache sync update
   //
-  if (mHobFlushComplete && (mVariableRuntimeHobCacheBuffer != NULL)) {
+  if ((*mHobFlushComplete) && (mVariableRuntimeHobCacheBuffer != NULL)) {
     if (!EfiAtRuntime ()) {
       FreePages (mVariableRuntimeHobCacheBuffer, EFI_SIZE_TO_PAGES (mVariableRuntimeHobCacheBufferSize));
     }
@@ -633,12 +613,12 @@ FindVariableInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));
 
-  mVariableRuntimeCacheReadLock = TRUE;
+  *mVariableRuntimeCacheReadLock = TRUE;
   CheckForRuntimeCacheSync ();
 
-  if (!mVariableRuntimeCachePendingUpdate) {
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -698,7 +678,7 @@ Done:
     }
   }
 
-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;
 
   return Status;
 }
@@ -921,12 +901,12 @@ GetNextVariableNameInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));
 
   CheckForRuntimeCacheSync ();
 
-  mVariableRuntimeCacheReadLock = TRUE;
-  if (!mVariableRuntimeCachePendingUpdate) {
+  *mVariableRuntimeCacheReadLock = TRUE;
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -958,7 +938,7 @@ GetNextVariableNameInRuntimeCache (
     }
   }
 
-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;
 
   return Status;
 }
@@ -1622,37 +1602,9 @@ SendRuntimeVariableCacheContextToSmm (
   SmmRuntimeVarCacheContext->RuntimeHobCache      = mVariableRuntimeHobCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeVolatileCache = mVariableRuntimeVolatileCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeNvCache       = mVariableRuntimeNvCacheBuffer;
-  SmmRuntimeVarCacheContext->PendingUpdate        = &mVariableRuntimeCachePendingUpdate;
-  SmmRuntimeVarCacheContext->ReadLock             = &mVariableRuntimeCacheReadLock;
-  SmmRuntimeVarCacheContext->HobFlushComplete     = &mHobFlushComplete;
-
-  //
-  // Request to unblock this region to be accessible from inside MM environment
-  // These fields "should" be all on the same page, but just to be on the safe side...
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->PendingUpdate - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCachePendingUpdate))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->ReadLock - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCacheReadLock))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->HobFlushComplete - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mHobFlushComplete))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
+  SmmRuntimeVarCacheContext->PendingUpdate        = mVariableRuntimeCachePendingUpdate;
+  SmmRuntimeVarCacheContext->ReadLock             = mVariableRuntimeCacheReadLock;
+  SmmRuntimeVarCacheContext->HobFlushComplete     = mHobFlushComplete;
 
   //
   // Send data to SMM.
@@ -1688,9 +1640,14 @@ SmmVariableReady (
   IN  VOID       *Context
   )
 {
-  EFI_STATUS  Status;
-  UINTN       RuntimeNvCacheSize;
-  UINTN       RuntimeVolatileCacheSize;
+  EFI_STATUS                   Status;
+  UINTN                        RuntimeNvCacheSize;
+  UINTN                        RuntimeVolatileCacheSize;
+  UINTN                        AllocatedHobCacheSize;
+  UINTN                        AllocatedNvCacheSize;
+  UINTN                        AllocatedVolatileCacheSize;
+  EFI_HOB_GUID_TYPE            *GuidHob;
+  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;
 
   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1717,7 +1674,7 @@ SmmVariableReady (
   if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
     DEBUG ((DEBUG_INFO, "Variable driver runtime cache is enabled.\n"));
     //
-    // Allocate runtime variable cache memory buffers.
+    // Get needed runtime cache buffer size and check if auth variables are to be used from SMM
     //
     Status =  GetRuntimeCacheInfo (
                 &mVariableRuntimeHobCacheBufferSize,
@@ -1726,6 +1683,27 @@ SmmVariableReady (
                 &mVariableAuthFormat
                 );
     if (!EFI_ERROR (Status)) {
+      GuidHob = GetFirstGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid);
+      ASSERT (GuidHob != NULL);
+      VariableRuntimeCacheHob    = GET_GUID_HOB_DATA (GuidHob);
+      AllocatedHobCacheSize      = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeHobCachePages);
+      AllocatedNvCacheSize       = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeNvCachePages);
+      AllocatedVolatileCacheSize = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeVolatileCachePages);
+
+      ASSERT (
+        (AllocatedHobCacheSize >= mVariableRuntimeHobCacheBufferSize) &&
+        (AllocatedNvCacheSize >= RuntimeNvCacheSize) &&
+        (AllocatedVolatileCacheSize >= RuntimeVolatileCacheSize)
+        );
+
+      mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
+      mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
+      mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
+      mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
+      mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
+      mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
+      mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
+
       Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
       if (!EFI_ERROR (Status)) {
         Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index e1085653fe..2d16f28674 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -13,7 +13,7 @@
 #  may not be modified without authorization. If platform fails to protect these resources,
 #  the authentication service provided in this driver will be broken, and the behavior is undefined.
 #
-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) Microsoft Corporation.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -60,7 +60,7 @@
   TpmMeasurementLib
   SafeIntLib
   PcdLib
-  MmUnblockMemoryLib
+  HobLib
 
 [Protocols]
   gEfiVariableWriteArchProtocolGuid             ## PRODUCES
@@ -113,6 +113,7 @@
   gVarCheckPolicyLibMmiHandlerGuid
   gEfiEndOfDxeEventGroupGuid
   gEfiDeviceSignatureDatabaseGuid
+  gEdkiiVariableRuntimeCacheInfoHobGuid
 
 [Depex]
   gEfiMmCommunication2ProtocolGuid
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119024): https://edk2.groups.io/g/devel/message/119024
Mute This Topic: https://groups.io/mt/106150805/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 8/9] MdeModulePkg: Refine InitVariableCache()

[duntan]

Refine the code logic in InitVariableCache().
In this commit, three times calling of
InitVariableCache() for different type cache are
merged into one calling. This commit is to make
the code looks cleaner and doesn't change any
code functionality.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------
 1 file changed, 95 insertions(+), 103 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 68a249c5ac..6efe5cee10 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -144,54 +144,6 @@ AtRuntime (
   return EfiAtRuntime ();
 }
 
-/**
-  Initialize the variable cache buffer as an empty variable store.
-
-  @param[out]     VariableCacheBuffer     A pointer to pointer of a cache variable store.
-  @param[in,out]  TotalVariableCacheSize  On input, the minimum size needed for the UEFI variable store cache
-                                          buffer that is allocated. On output, the actual size of the buffer allocated.
-                                          If TotalVariableCacheSize is zero, a buffer will not be allocated and the
-                                          function will return with EFI_SUCCESS.
-
-  @retval EFI_SUCCESS             The variable cache was allocated and initialized successfully.
-  @retval EFI_INVALID_PARAMETER   A given pointer is NULL or an invalid variable store size was specified.
-  @retval EFI_OUT_OF_RESOURCES    Insufficient resources are available to allocate the variable store cache buffer.
-
-**/
-EFI_STATUS
-InitVariableCache (
-  OUT    VARIABLE_STORE_HEADER  **VariableCacheBuffer,
-  IN OUT UINTN                  *TotalVariableCacheSize
-  )
-{
-  VARIABLE_STORE_HEADER  *VariableCacheStorePtr;
-  EFI_STATUS             Status;
-
-  if (TotalVariableCacheSize == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  if (*TotalVariableCacheSize == 0) {
-    return EFI_SUCCESS;
-  }
-
-  if ((VariableCacheBuffer == NULL) || (*TotalVariableCacheSize < sizeof (VARIABLE_STORE_HEADER))) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  *TotalVariableCacheSize = ALIGN_VALUE (*TotalVariableCacheSize, sizeof (UINT32));
-
-  VariableCacheStorePtr = *VariableCacheBuffer;
-  SetMem32 ((VOID *)VariableCacheStorePtr, *TotalVariableCacheSize, (UINT32)0xFFFFFFFF);
-
-  ZeroMem ((VOID *)VariableCacheStorePtr, sizeof (VARIABLE_STORE_HEADER));
-  VariableCacheStorePtr->Size   = (UINT32)*TotalVariableCacheSize;
-  VariableCacheStorePtr->Format = VARIABLE_STORE_FORMATTED;
-  VariableCacheStorePtr->State  = VARIABLE_STORE_HEALTHY;
-
-  return EFI_SUCCESS;
-}
-
 /**
   Initialize the communicate buffer using DataSize and Function.
 
@@ -1554,6 +1506,92 @@ Done:
   return Status;
 }
 
+/**
+  Initialize the variable cache buffer as an empty variable store.
+
+  @param[in]  VariableCacheBuffer     A pointer to pointer of a cache variable store.
+  @param[in]  TotalVariableCacheSize  The size needed for the UEFI variable store cache buffer that is allocated.
+
+**/
+VOID
+InitVariableStoreHeader (
+  IN  VARIABLE_STORE_HEADER  *VariableCacheBuffer,
+  IN  UINTN                  TotalVariableCacheSize
+  )
+{
+  if (TotalVariableCacheSize > 0) {
+    ASSERT ((VariableCacheBuffer != NULL) && (TotalVariableCacheSize >= sizeof (VARIABLE_STORE_HEADER)));
+
+    SetMem32 ((VOID *)VariableCacheBuffer, TotalVariableCacheSize, (UINT32)0xFFFFFFFF);
+    ZeroMem ((VOID *)VariableCacheBuffer, sizeof (VARIABLE_STORE_HEADER));
+    VariableCacheBuffer->Size   = (UINT32)TotalVariableCacheSize;
+    VariableCacheBuffer->Format = VARIABLE_STORE_FORMATTED;
+    VariableCacheBuffer->State  = VARIABLE_STORE_HEALTHY;
+  }
+}
+
+/**
+  Initialize the runtime variable cache related content.
+
+  @retval EFI_SUCCESS    Initialize the runtime variable cache related content successfully.
+  @retval Others         Could not initialize the runtime variable cache related content successfully.
+
+**/
+EFI_STATUS
+InitVariableCache (
+  VOID
+  )
+{
+  EFI_STATUS                   Status;
+  UINTN                        ExpectedHobCacheSize;
+  UINTN                        ExpectedNvCacheSize;
+  UINTN                        ExpectedVolatileCacheSize;
+  UINTN                        AllocatedHobCacheSize;
+  UINTN                        AllocatedNvCacheSize;
+  UINTN                        AllocatedVolatileCacheSize;
+  EFI_HOB_GUID_TYPE            *GuidHob;
+  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;
+
+  DEBUG ((DEBUG_INFO, "Variable driver runtime cache is enabled.\n"));
+  //
+  // Get needed runtime cache buffer size and check if auth variables are to be used from SMM
+  //
+  Status =  GetRuntimeCacheInfo (
+              &ExpectedHobCacheSize,
+              &ExpectedNvCacheSize,
+              &ExpectedVolatileCacheSize,
+              &mVariableAuthFormat
+              );
+  if (!EFI_ERROR (Status)) {
+    GuidHob = GetFirstGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid);
+    ASSERT (GuidHob != NULL);
+    VariableRuntimeCacheHob    = GET_GUID_HOB_DATA (GuidHob);
+    AllocatedHobCacheSize      = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeHobCachePages);
+    AllocatedNvCacheSize       = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeNvCachePages);
+    AllocatedVolatileCacheSize = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeVolatileCachePages);
+
+    ASSERT (
+      (AllocatedHobCacheSize >= ExpectedHobCacheSize) &&
+      (AllocatedNvCacheSize >= ExpectedNvCacheSize) &&
+      (AllocatedVolatileCacheSize >= ExpectedVolatileCacheSize)
+      );
+
+    mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
+    mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
+    mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
+    mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
+    mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
+    mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
+    mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
+
+    InitVariableStoreHeader (mVariableRuntimeHobCacheBuffer, AllocatedHobCacheSize);
+    InitVariableStoreHeader (mVariableRuntimeNvCacheBuffer, AllocatedNvCacheSize);
+    InitVariableStoreHeader (mVariableRuntimeVolatileCacheBuffer, AllocatedVolatileCacheSize);
+  }
+
+  return Status;
+}
+
 /**
   Sends the runtime variable cache context information to SMM.
 
@@ -1640,14 +1678,7 @@ SmmVariableReady (
   IN  VOID       *Context
   )
 {
-  EFI_STATUS                   Status;
-  UINTN                        RuntimeNvCacheSize;
-  UINTN                        RuntimeVolatileCacheSize;
-  UINTN                        AllocatedHobCacheSize;
-  UINTN                        AllocatedNvCacheSize;
-  UINTN                        AllocatedVolatileCacheSize;
-  EFI_HOB_GUID_TYPE            *GuidHob;
-  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;
+  EFI_STATUS  Status;
 
   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1672,57 +1703,18 @@ SmmVariableReady (
   mVariableBufferPhysical = mVariableBuffer;
 
   if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
-    DEBUG ((DEBUG_INFO, "Variable driver runtime cache is enabled.\n"));
-    //
-    // Get needed runtime cache buffer size and check if auth variables are to be used from SMM
-    //
-    Status =  GetRuntimeCacheInfo (
-                &mVariableRuntimeHobCacheBufferSize,
-                &RuntimeNvCacheSize,
-                &RuntimeVolatileCacheSize,
-                &mVariableAuthFormat
-                );
+    Status = InitVariableCache ();
     if (!EFI_ERROR (Status)) {
-      GuidHob = GetFirstGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid);
-      ASSERT (GuidHob != NULL);
-      VariableRuntimeCacheHob    = GET_GUID_HOB_DATA (GuidHob);
-      AllocatedHobCacheSize      = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeHobCachePages);
-      AllocatedNvCacheSize       = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeNvCachePages);
-      AllocatedVolatileCacheSize = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeVolatileCachePages);
-
-      ASSERT (
-        (AllocatedHobCacheSize >= mVariableRuntimeHobCacheBufferSize) &&
-        (AllocatedNvCacheSize >= RuntimeNvCacheSize) &&
-        (AllocatedVolatileCacheSize >= RuntimeVolatileCacheSize)
-        );
-
-      mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
-      mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
-      mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
-      mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
-      mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
-      mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
-      mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
-
-      Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
+      Status = SendRuntimeVariableCacheContextToSmm ();
       if (!EFI_ERROR (Status)) {
-        Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
-        if (!EFI_ERROR (Status)) {
-          Status = InitVariableCache (&mVariableRuntimeVolatileCacheBuffer, &RuntimeVolatileCacheSize);
-          if (!EFI_ERROR (Status)) {
-            Status = SendRuntimeVariableCacheContextToSmm ();
-            if (!EFI_ERROR (Status)) {
-              SyncRuntimeCache ();
-            }
-          }
-        }
+        SyncRuntimeCache ();
       }
+    }
 
-      if (EFI_ERROR (Status)) {
-        mVariableRuntimeHobCacheBuffer      = NULL;
-        mVariableRuntimeNvCacheBuffer       = NULL;
-        mVariableRuntimeVolatileCacheBuffer = NULL;
-      }
+    if (EFI_ERROR (Status)) {
+      mVariableRuntimeHobCacheBuffer      = NULL;
+      mVariableRuntimeNvCacheBuffer       = NULL;
+      mVariableRuntimeVolatileCacheBuffer = NULL;
     }
 
     ASSERT_EFI_ERROR (Status);
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119025): https://edk2.groups.io/g/devel/message/119025
Mute This Topic: https://groups.io/mt/106150806/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 9/9] MdeModulePkg:Add global variable mVariableRtCacheInfo

[duntan]

Add global variable mVariableRtCacheInfo to save the
content in gEdkiiVariableRuntimeCacheInfoHobGuid. With
this new global variable, 7 global variables can be
removed.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 97 +++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------
 1 file changed, 41 insertions(+), 56 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 6efe5cee10..de39462d68 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -44,26 +44,20 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "PrivilegePolymorphic.h"
 #include "VariableParsing.h"
 
-EFI_HANDLE                      mHandle                              = NULL;
-EFI_SMM_VARIABLE_PROTOCOL       *mSmmVariable                        = NULL;
-EFI_EVENT                       mVirtualAddressChangeEvent           = NULL;
-EFI_MM_COMMUNICATION2_PROTOCOL  *mMmCommunication2                   = NULL;
-UINT8                           *mVariableBuffer                     = NULL;
-UINT8                           *mVariableBufferPhysical             = NULL;
-VARIABLE_INFO_ENTRY             *mVariableInfo                       = NULL;
-VARIABLE_STORE_HEADER           *mVariableRuntimeHobCacheBuffer      = NULL;
-VARIABLE_STORE_HEADER           *mVariableRuntimeNvCacheBuffer       = NULL;
-VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
+EFI_HANDLE                      mHandle                    = NULL;
+EFI_SMM_VARIABLE_PROTOCOL       *mSmmVariable              = NULL;
+EFI_EVENT                       mVirtualAddressChangeEvent = NULL;
+EFI_MM_COMMUNICATION2_PROTOCOL  *mMmCommunication2         = NULL;
+UINT8                           *mVariableBuffer           = NULL;
+UINT8                           *mVariableBufferPhysical   = NULL;
+VARIABLE_INFO_ENTRY             *mVariableInfo             = NULL;
 UINTN                           mVariableBufferSize;
-UINTN                           mVariableRuntimeHobCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
-BOOLEAN                         *mVariableRuntimeCachePendingUpdate;
-BOOLEAN                         *mVariableRuntimeCacheReadLock;
 BOOLEAN                         mVariableAuthFormat;
-BOOLEAN                         *mHobFlushComplete;
 EFI_LOCK                        mVariableServicesLock;
 EDKII_VARIABLE_LOCK_PROTOCOL    mVariableLock;
 EDKII_VAR_CHECK_PROTOCOL        mVarCheck;
+VARIABLE_RUNTIME_CACHE_INFO     mVariableRtCacheInfo;
 
 /**
   The logic to initialize the VariablePolicy engine is in its own file.
@@ -500,21 +494,21 @@ CheckForRuntimeCacheSync (
   VOID
   )
 {
-  if (*mVariableRuntimeCachePendingUpdate) {
+  if (mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate) {
     SyncRuntimeCache ();
   }
 
-  ASSERT (!(*mVariableRuntimeCachePendingUpdate));
+  ASSERT (!(mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate));
 
   //
   // The HOB variable data may have finished being flushed in the runtime cache sync update
   //
-  if ((*mHobFlushComplete) && (mVariableRuntimeHobCacheBuffer != NULL)) {
+  if ((mVariableRtCacheInfo.CacheInfoFlag->HobFlushComplete) && (mVariableRtCacheInfo.RuntimeHobCacheBuffer != 0)) {
     if (!EfiAtRuntime ()) {
-      FreePages (mVariableRuntimeHobCacheBuffer, EFI_SIZE_TO_PAGES (mVariableRuntimeHobCacheBufferSize));
+      FreePages ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer, mVariableRtCacheInfo.RuntimeHobCachePages);
     }
 
-    mVariableRuntimeHobCacheBuffer = NULL;
+    mVariableRtCacheInfo.RuntimeHobCacheBuffer = 0;
   }
 }
 
@@ -565,20 +559,20 @@ FindVariableInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!(*mVariableRuntimeCacheReadLock));
+  ASSERT (!(mVariableRtCacheInfo.CacheInfoFlag->ReadLock));
 
-  *mVariableRuntimeCacheReadLock = TRUE;
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = TRUE;
   CheckForRuntimeCacheSync ();
 
-  if (!(*mVariableRuntimeCachePendingUpdate)) {
+  if (!(mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
     // makes use of this mapping to implement search algorithm.
     //
-    VariableStoreList[VariableStoreTypeVolatile] = mVariableRuntimeVolatileCacheBuffer;
-    VariableStoreList[VariableStoreTypeHob]      = mVariableRuntimeHobCacheBuffer;
-    VariableStoreList[VariableStoreTypeNv]       = mVariableRuntimeNvCacheBuffer;
+    VariableStoreList[VariableStoreTypeVolatile] = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer;
+    VariableStoreList[VariableStoreTypeHob]      = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer;
+    VariableStoreList[VariableStoreTypeNv]       = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer;
 
     for (StoreType = (VARIABLE_STORE_TYPE)0; StoreType < VariableStoreTypeMax; StoreType++) {
       if (VariableStoreList[StoreType] == NULL) {
@@ -630,7 +624,7 @@ Done:
     }
   }
 
-  *mVariableRuntimeCacheReadLock = FALSE;
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = FALSE;
 
   return Status;
 }
@@ -853,20 +847,20 @@ GetNextVariableNameInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!(*mVariableRuntimeCacheReadLock));
+  ASSERT (!(mVariableRtCacheInfo.CacheInfoFlag->ReadLock));
 
   CheckForRuntimeCacheSync ();
 
-  *mVariableRuntimeCacheReadLock = TRUE;
-  if (!(*mVariableRuntimeCachePendingUpdate)) {
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = TRUE;
+  if (!(mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
     // makes use of this mapping to implement search algorithm.
     //
-    VariableStoreHeader[VariableStoreTypeVolatile] = mVariableRuntimeVolatileCacheBuffer;
-    VariableStoreHeader[VariableStoreTypeHob]      = mVariableRuntimeHobCacheBuffer;
-    VariableStoreHeader[VariableStoreTypeNv]       = mVariableRuntimeNvCacheBuffer;
+    VariableStoreHeader[VariableStoreTypeVolatile] = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer;
+    VariableStoreHeader[VariableStoreTypeHob]      = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer;
+    VariableStoreHeader[VariableStoreTypeNv]       = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer;
 
     Status =  VariableServiceGetNextVariableInternal (
                 VariableName,
@@ -890,7 +884,7 @@ GetNextVariableNameInRuntimeCache (
     }
   }
 
-  *mVariableRuntimeCacheReadLock = FALSE;
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = FALSE;
 
   return Status;
 }
@@ -1345,9 +1339,9 @@ VariableAddressChangeEvent (
 {
   EfiConvertPointer (0x0, (VOID **)&mVariableBuffer);
   EfiConvertPointer (0x0, (VOID **)&mMmCommunication2);
-  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRuntimeHobCacheBuffer);
-  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRuntimeNvCacheBuffer);
-  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRuntimeVolatileCacheBuffer);
+  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRtCacheInfo.RuntimeHobCacheBuffer);
+  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRtCacheInfo.RuntimeNvCacheBuffer);
+  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRtCacheInfo.RuntimeVolatileCacheBuffer);
 }
 
 /**
@@ -1576,17 +1570,10 @@ InitVariableCache (
       (AllocatedVolatileCacheSize >= ExpectedVolatileCacheSize)
       );
 
-    mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
-    mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
-    mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
-    mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
-    mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
-    mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
-    mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
-
-    InitVariableStoreHeader (mVariableRuntimeHobCacheBuffer, AllocatedHobCacheSize);
-    InitVariableStoreHeader (mVariableRuntimeNvCacheBuffer, AllocatedNvCacheSize);
-    InitVariableStoreHeader (mVariableRuntimeVolatileCacheBuffer, AllocatedVolatileCacheSize);
+    CopyMem (&mVariableRtCacheInfo, VariableRuntimeCacheHob, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
+    InitVariableStoreHeader ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer, AllocatedHobCacheSize);
+    InitVariableStoreHeader ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer, AllocatedNvCacheSize);
+    InitVariableStoreHeader ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer, AllocatedVolatileCacheSize);
   }
 
   return Status;
@@ -1637,12 +1624,12 @@ SendRuntimeVariableCacheContextToSmm (
   SmmVariableFunctionHeader->Function = SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT;
   SmmRuntimeVarCacheContext           = (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT *)SmmVariableFunctionHeader->Data;
 
-  SmmRuntimeVarCacheContext->RuntimeHobCache      = mVariableRuntimeHobCacheBuffer;
-  SmmRuntimeVarCacheContext->RuntimeVolatileCache = mVariableRuntimeVolatileCacheBuffer;
-  SmmRuntimeVarCacheContext->RuntimeNvCache       = mVariableRuntimeNvCacheBuffer;
-  SmmRuntimeVarCacheContext->PendingUpdate        = mVariableRuntimeCachePendingUpdate;
-  SmmRuntimeVarCacheContext->ReadLock             = mVariableRuntimeCacheReadLock;
-  SmmRuntimeVarCacheContext->HobFlushComplete     = mHobFlushComplete;
+  SmmRuntimeVarCacheContext->RuntimeHobCache      = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer;
+  SmmRuntimeVarCacheContext->RuntimeVolatileCache = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer;
+  SmmRuntimeVarCacheContext->RuntimeNvCache       = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer;
+  SmmRuntimeVarCacheContext->PendingUpdate        = &mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate;
+  SmmRuntimeVarCacheContext->ReadLock             = &mVariableRtCacheInfo.CacheInfoFlag->ReadLock;
+  SmmRuntimeVarCacheContext->HobFlushComplete     = &mVariableRtCacheInfo.CacheInfoFlag->HobFlushComplete;
 
   //
   // Send data to SMM.
@@ -1712,9 +1699,7 @@ SmmVariableReady (
     }
 
     if (EFI_ERROR (Status)) {
-      mVariableRuntimeHobCacheBuffer      = NULL;
-      mVariableRuntimeNvCacheBuffer       = NULL;
-      mVariableRuntimeVolatileCacheBuffer = NULL;
+      ZeroMem (&mVariableRtCacheInfo, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
     }
 
     ASSERT_EFI_ERROR (Status);
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119026): https://edk2.groups.io/g/devel/message/119026
Mute This Topic: https://groups.io/mt/106150807/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/9] MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 4914 bytes --]

There is no need to use pack(1).
Explicit pack(1) is needed when you really need to save spaces or the data is saved in some NV storage to be accessed by a different software component.

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 1/9] MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid

This commit defines VARIABLE_RUNTIME_CACHE_INFO HOB.
The HOB is used to store the address and size of the
buffer that will be used for variable runtime service
when the PcdEnableVariableRuntimeCache is TRUE.

In following patches, when PcdEnableVariableRuntimeCache
is TRUE, VariablePei module will install a callback of
gEfiPeiMemoryDiscoveredPpiGuid to allocate needed buffer
for different type cache, unblock the buffer and build HOB.
Then VariableSmmRuntimeDxe driver will consume the
gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the
variable runtime cache related content.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                        |  3 +++
 2 files changed, 68 insertions(+)

diff --git a/MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h b/MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
new file mode 100644
index 0000000000..c2a8b77945
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
@@ -0,0 +1,65 @@
+/** @file
+  This Variable Runtime Cache Info HOB is used to store the address
+  and the size of the buffer that will be used for variable runtime
+  service when the PcdEnableVariableRuntimeCache is TRUE.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef VARIABLE_RUNTIME_CACHE_INFO_H_
+#define VARIABLE_RUNTIME_CACHE_INFO_H_
+
+#include <PiPei.h>
+
+#define VARIABLE_RUNTIME_CACHE_INFO_HOB_REVISION  1
+
+#define VARIABLE_RUNTIME_CACHE_INFO_GUID \
+  { \
+    0x0f472f7d, 0x6713, 0x4915, {0x96, 0x14, 0x5d, 0xda, 0x28, 0x40, 0x10, 0x56}  \
+  }
+
+#pragma pack(1)
+typedef struct {
+  ///
+  /// TRUE indicates GetVariable () or GetNextVariable () is being called.
+  /// When the value is FALSE, the given update (and any other pending updates)
+  /// can be flushed to the runtime cache.
+  ///
+  BOOLEAN    ReadLock;
+  ///
+  /// TRUE indicates there is pending update for the given variable store needed
+  /// to be flushed to the runtime cache.
+  ///
+  BOOLEAN    PendingUpdate;
+  ///
+  /// TRUE indicates all HOB variables have been flushed in flash.
+  ///
+  BOOLEAN    HobFlushComplete;
+} CACHE_INFO_FLAG;
+
+typedef struct {
+  CACHE_INFO_FLAG    *CacheInfoFlag;
+  ///
+  /// Buffer reserved for runtime Hob cache
+  ///
+  UINT64             RuntimeHobCacheBuffer;
+  UINTN              RuntimeHobCachePages;
+  ///
+  /// Buffer reserved for Non-Volatile runtime cache
+  ///
+  UINT64             RuntimeNvCacheBuffer;
+  UINTN              RuntimeNvCachePages;
+  ///
+  /// Buffer reserved for Volatile runtime cache
+  ///
+  UINT64             RuntimeVolatileCacheBuffer;
+  UINTN              RuntimeVolatileCachePages;
+} VARIABLE_RUNTIME_CACHE_INFO;
+#pragma pack()
+
+extern EFI_GUID  gEdkiiVariableRuntimeCacheInfoHobGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index f7339f0aec..1bf5e31b7c 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -465,6 +465,9 @@
   gEdk2JedecSfdpSpiDxeDriverGuid  = { 0xBE71701E, 0xB63C, 0x4574, { 0x9C, 0x5C, 0x36, 0x29, 0xE8, 0xEA, 0xC4, 0x14 }}
   gEdk2JedecSfdpSpiSmmDriverGuid  = { 0x95A1E915, 0x195C, 0x477C, { 0x92, 0x6F, 0x7E, 0x24, 0x67, 0xC1, 0xB3, 0x1F }}

+  ## Include/Guid/VariableRuntimeCacheInfo.h
+  gEdkiiVariableRuntimeCacheInfoHobGuid = { 0x0f472f7d, 0x6713, 0x4915, { 0x96, 0x14, 0x5d, 0xda, 0x28, 0x40, 0x10, 0x56 }}
+
 [Ppis]
   ## Include/Ppi/FirmwareVolumeShadowPpi.h
   gEdkiiPeiFirmwareVolumeShadowPpiGuid = { 0x7dfe756c, 0xed8d, 0x4d77, {0x9e, 0xc4, 0x39, 0x9a, 0x8a, 0x81, 0x51, 0x16 } }
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119040): https://edk2.groups.io/g/devel/message/119040
Mute This Topic: https://groups.io/mt/106150797/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 8153 bytes --]

Re: [edk2-devel] [PATCH 2/9] ArmVirtPkg: Add MmUnblockMemoryLib in DSC

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]


+  MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
[Ray] Do you really need this line as the next "!include" already includes the NULL instance lib?


+
 !include MdePkg/MdeLibs.dsc.inc


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119041): https://edk2.groups.io/g/devel/message/119041
Mute This Topic: https://groups.io/mt/106150798/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 2028 bytes --]

Re: [edk2-devel] [PATCH 3/9] EmulatorPkg: Add MmUnblockMemoryLib in DSC

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 1637 bytes --]

It's not needed.

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH 3/9] EmulatorPkg: Add MmUnblockMemoryLib in DSC

Add MmUnblockMemoryLib in EmulatorPkg.dsc.
This lib will be required by VariablePei in
following commits.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
---
 EmulatorPkg/EmulatorPkg.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 5fa1ed345a..0e15dafb5c 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -127,6 +127,7 @@
   ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   ImagePropertiesRecordLib|MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.inf
+  MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf

 !if $(SECURE_BOOT_ENABLE) == TRUE
   RngLib|MdeModulePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119042): https://edk2.groups.io/g/devel/message/119042
Mute This Topic: https://groups.io/mt/106150799/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3546 bytes --]

Re: [edk2-devel] [PATCH 4/9] OvmfPkg: Add MmUnblockMemoryLib in DSC

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

it's not needed.

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
Subject: [PATCH 4/9] OvmfPkg: Add MmUnblockMemoryLib in DSC

Add MmUnblockMemoryLib in OvmfPkgIa32X64.dsc.
This lib will be required by VariablePei in
following commits.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index d27a4c7278..cc03104aac 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -233,7 +233,7 @@
   VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
   VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
   VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
-
+  MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf

   #
   # Network libraries
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119043): https://edk2.groups.io/g/devel/message/119043
Mute This Topic: https://groups.io/mt/106150800/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3742 bytes --]

Re: [edk2-devel] [PATCH 5/9] MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]


+  VariableRuntimeCacheHob = BuildGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
+  ASSERT (VariableRuntimeCacheHob != NULL);
+  ZeroMem (VariableRuntimeCacheHob, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
+
+  //
+  // AllocateRuntimePages for CACHE_INFO_FLAG and unblock it.
+  //
+  Pages  = EFI_SIZE_TO_PAGES (sizeof (CACHE_INFO_FLAG));
+  Buffer = AllocateRuntimePages (Pages);
+  ASSERT (Buffer != NULL);
+  Status = MmUnblockMemoryRequest (
+             (EFI_PHYSICAL_ADDRESS)(UINTN)Buffer,
+             Pages
+             );
+  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
+    return Status;

[Ray.1] The GUID hob is created already. Maybe you should defer the HOB creation to later phase.

+  }
+
+  VariableRuntimeCacheHob->CacheInfoFlag = (CACHE_INFO_FLAG *)(UINTN)Buffer;
+  DEBUG ((
+    DEBUG_INFO,
+    "PeiVariable: CACHE_INFO_FLAG Buffer is: 0x%x, number of pages is: 0x%x\n",
[Ray.2] please use "%p" for pointer dump. "%x" only prints "int" type value.


[Ray.3]I think you should create HOB at this point.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119044): https://edk2.groups.io/g/devel/message/119044
Mute This Topic: https://groups.io/mt/106150802/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3973 bytes --]

Re: [edk2-devel] [PATCH 6/9] MdeModulePkg:Remove unnecessary global variable

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 4026 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 6/9] MdeModulePkg:Remove unnecessary global variable

Remove the two unnecessary global variables and
replace them by two local variables:
mVariableRuntimeNvCacheBufferSize
mVariableRuntimeVolatileCacheBufferSize

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 6930875e9f..8b42ae7d72 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -13,7 +13,7 @@

   InitCommunicateBuffer() is really function to check the variable data size.

-Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 Copyright (c) Microsoft Corporation.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -55,8 +55,6 @@ VARIABLE_STORE_HEADER           *mVariableRuntimeNvCacheBuffer       = NULL;
 VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
 UINTN                           mVariableBufferSize;
 UINTN                           mVariableRuntimeHobCacheBufferSize;
-UINTN                           mVariableRuntimeNvCacheBufferSize;
-UINTN                           mVariableRuntimeVolatileCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
 BOOLEAN                         mVariableRuntimeCachePendingUpdate;
 BOOLEAN                         mVariableRuntimeCacheReadLock;
@@ -1691,6 +1689,8 @@ SmmVariableReady (
   )
 {
   EFI_STATUS  Status;
+  UINTN       RuntimeNvCacheSize;
+  UINTN       RuntimeVolatileCacheSize;

   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1721,16 +1721,16 @@ SmmVariableReady (
     //
     Status =  GetRuntimeCacheInfo (
                 &mVariableRuntimeHobCacheBufferSize,
-                &mVariableRuntimeNvCacheBufferSize,
-                &mVariableRuntimeVolatileCacheBufferSize,
+                &RuntimeNvCacheSize,
+                &RuntimeVolatileCacheSize,
                 &mVariableAuthFormat
                 );
     if (!EFI_ERROR (Status)) {
       Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
       if (!EFI_ERROR (Status)) {
-        Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &mVariableRuntimeNvCacheBufferSize);
+        Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
         if (!EFI_ERROR (Status)) {
-          Status = InitVariableCache (&mVariableRuntimeVolatileCacheBuffer, &mVariableRuntimeVolatileCacheBufferSize);
+          Status = InitVariableCache (&mVariableRuntimeVolatileCacheBuffer, &RuntimeVolatileCacheSize);
           if (!EFI_ERROR (Status)) {
             Status = SendRuntimeVariableCacheContextToSmm ();
             if (!EFI_ERROR (Status)) {
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119045): https://edk2.groups.io/g/devel/message/119045
Mute This Topic: https://groups.io/mt/106150803/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 8407 bytes --]

Re: [edk2-devel] [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 13204 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
Subject: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

Consume gEdkiiVariableRuntimeCacheInfoHobGuid in

VariableSmmRuntimeDxe driver to initialize the following

variable cache related buffer:
  *mVariableRuntimeHobCacheBuffer
  *mVariableRuntimeNvCacheBuffer
  *mVariableRuntimeVolatileCacheBuffer
  *mVariableRuntimeCachePendingUpdate
  *mVariableRuntimeCacheReadLock
  *mHobFlushComplete

The code to to allocate
and unblock the buffer for
different type cache in VariableSmmRuntimeDxe is also
removed in this commit.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--
 2 files changed, 52 insertions(+), 73 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 8b42ae7d72..68a249c5ac 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -35,10 +35,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
 #include <Library/BaseLib.h>
-#include <Library/MmUnblockMemoryLib.h>
+#include <Library/HobLib.h>

 #include <Guid/EventGroup.h>
 #include <Guid/SmmVariableCommon.h>
+#include <Guid/VariableRuntimeCacheInfo.h>

 #include "PrivilegePolymorphic.h"
 #include "VariableParsing.h"
@@ -56,10 +57,10 @@ VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
 UINTN                           mVariableBufferSize;
 UINTN                           mVariableRuntimeHobCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
-BOOLEAN                         mVariableRuntimeCachePendingUpdate;
-BOOLEAN                         mVariableRuntimeCacheReadLock;
+BOOLEAN                         *mVariableRuntimeCachePendingUpdate;
+BOOLEAN                         *mVariableRuntimeCacheReadLock;
 BOOLEAN                         mVariableAuthFormat;
-BOOLEAN                         mHobFlushComplete;
+BOOLEAN                         *mHobFlushComplete;
 EFI_LOCK                        mVariableServicesLock;
 EDKII_VARIABLE_LOCK_PROTOCOL    mVariableLock;
 EDKII_VAR_CHECK_PROTOCOL        mVarCheck;
@@ -180,27 +181,6 @@ InitVariableCache (

   *TotalVariableCacheSize = ALIGN_VALUE (*TotalVariableCacheSize, sizeof (UINT32));

-  //
-  // Allocate NV Storage Cache and initialize it to all 1's (like an erased FV)
-  //
-  *VariableCacheBuffer =  (VARIABLE_STORE_HEADER *)AllocateRuntimePages (
-                                                     EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-                                                     );
-  if (*VariableCacheBuffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // Request to unblock the newly allocated cache region to be accessible from inside MM
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)(UINTN)*VariableCacheBuffer,
-             EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    return Status;
-  }
-
   VariableCacheStorePtr = *VariableCacheBuffer;
   SetMem32 ((VOID *)VariableCacheStorePtr, *TotalVariableCacheSize, (UINT32)0xFFFFFFFF);

@@ -568,16 +548,16 @@ CheckForRuntimeCacheSync (
   VOID
   )
 {
-  if (mVariableRuntimeCachePendingUpdate) {
+  if (*mVariableRuntimeCachePendingUpdate) {
     SyncRuntimeCache ();
   }

-  ASSERT (!mVariableRuntimeCachePendingUpdate);
+  ASSERT (!(*mVariableRuntimeCachePendingUpdate));

   //
   // The HOB variable data may have finished being flushed in the runtime cache sync update
   //
-  if (mHobFlushComplete && (mVariableRuntimeHobCacheBuffer != NULL)) {
+  if ((*mHobFlushComplete) && (mVariableRuntimeHobCacheBuffer != NULL)) {
     if (!EfiAtRuntime ()) {
       FreePages (mVariableRuntimeHobCacheBuffer, EFI_SIZE_TO_PAGES (mVariableRuntimeHobCacheBufferSize));
     }
@@ -633,12 +613,12 @@ FindVariableInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

-  mVariableRuntimeCacheReadLock = TRUE;
+  *mVariableRuntimeCacheReadLock = TRUE;
   CheckForRuntimeCacheSync ();

-  if (!mVariableRuntimeCachePendingUpdate) {
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -698,7 +678,7 @@ Done:
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -921,12 +901,12 @@ GetNextVariableNameInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

   CheckForRuntimeCacheSync ();

-  mVariableRuntimeCacheReadLock = TRUE;
-  if (!mVariableRuntimeCachePendingUpdate) {
+  *mVariableRuntimeCacheReadLock = TRUE;
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -958,7 +938,7 @@ GetNextVariableNameInRuntimeCache (
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -1622,37 +1602,9 @@ SendRuntimeVariableCacheContextToSmm (
   SmmRuntimeVarCacheContext->RuntimeHobCache      = mVariableRuntimeHobCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeVolatileCache = mVariableRuntimeVolatileCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeNvCache       = mVariableRuntimeNvCacheBuffer;
-  SmmRuntimeVarCacheContext->PendingUpdate        = &mVariableRuntimeCachePendingUpdate;
-  SmmRuntimeVarCacheContext->ReadLock             = &mVariableRuntimeCacheReadLock;
-  SmmRuntimeVarCacheContext->HobFlushComplete     = &mHobFlushComplete;
-
-  //
-  // Request to unblock this region to be accessible from inside MM environment
-  // These fields "should" be all on the same page, but just to be on the safe side...
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->PendingUpdate - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCachePendingUpdate))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->ReadLock - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCacheReadLock))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->HobFlushComplete - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mHobFlushComplete))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
+  SmmRuntimeVarCacheContext->PendingUpdate        = mVariableRuntimeCachePendingUpdate;
+  SmmRuntimeVarCacheContext->ReadLock             = mVariableRuntimeCacheReadLock;
+  SmmRuntimeVarCacheContext->HobFlushComplete     = mHobFlushComplete;

   //
   // Send data to SMM.
@@ -1688,9 +1640,14 @@ SmmVariableReady (
   IN  VOID       *Context
   )
 {
-  EFI_STATUS  Status;
-  UINTN       RuntimeNvCacheSize;
-  UINTN       RuntimeVolatileCacheSize;
+  EFI_STATUS                   Status;
+  UINTN                        RuntimeNvCacheSize;
+  UINTN                        RuntimeVolatileCacheSize;
+  UINTN                        AllocatedHobCacheSize;
+  UINTN                        AllocatedNvCacheSize;
+  UINTN                        AllocatedVolatileCacheSize;
+  EFI_HOB_GUID_TYPE            *GuidHob;
+  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;

   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1717,7 +1674,7 @@ SmmVariableReady (
   if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
     DEBUG ((DEBUG_INFO, "Variable driver runtime cache is enabled.\n"));
     //
-    // Allocate runtime variable cache memory buffers.
+    // Get needed runtime cache buffer size and check if auth variables are to be used from SMM
     //
     Status =  GetRuntimeCacheInfo (
                 &mVariableRuntimeHobCacheBufferSize,
@@ -1726,6 +1683,27 @@ SmmVariableReady (
                 &mVariableAuthFormat
                 );
     if (!EFI_ERROR (Status)) {
+      GuidHob = GetFirstGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid);
+      ASSERT (GuidHob != NULL);
+      VariableRuntimeCacheHob    = GET_GUID_HOB_DATA (GuidHob);
+      AllocatedHobCacheSize      = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeHobCachePages);
+      AllocatedNvCacheSize       = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeNvCachePages);
+      AllocatedVolatileCacheSize = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeVolatileCachePages);
+
+      ASSERT (
+        (AllocatedHobCacheSize >= mVariableRuntimeHobCacheBufferSize) &&
+        (AllocatedNvCacheSize >= RuntimeNvCacheSize) &&
+        (AllocatedVolatileCacheSize >= RuntimeVolatileCacheSize)
+        );
+
+      mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
+      mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
+      mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
+      mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
+      mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
+      mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
+      mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
+
       Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
       if (!EFI_ERROR (Status)) {
         Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index e1085653fe..2d16f28674 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -13,7 +13,7 @@
 #  may not be modified without authorization. If platform fails to protect these resources,
 #  the authentication service provided in this driver will be broken, and the behavior is undefined.
 #
-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) Microsoft Corporation.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -60,7 +60,7 @@
   TpmMeasurementLib
   SafeIntLib
   PcdLib
-  MmUnblockMemoryLib
+  HobLib

 [Protocols]
   gEfiVariableWriteArchProtocolGuid             ## PRODUCES
@@ -113,6 +113,7 @@
   gVarCheckPolicyLibMmiHandlerGuid
   gEfiEndOfDxeEventGroupGuid
   gEfiDeviceSignatureDatabaseGuid
+  gEdkiiVariableRuntimeCacheInfoHobGuid

 [Depex]
   gEfiMmCommunication2ProtocolGuid
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119046): https://edk2.groups.io/g/devel/message/119046
Mute This Topic: https://groups.io/mt/106150805/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 22859 bytes --]

Re: [edk2-devel] [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 13309 bytes --]

Dun,
All the 6 new pointers that are accessed at runtime should be converted through ConvertPointer().
Please ignore my previous Reviewed-by.

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
Subject: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

Consume gEdkiiVariableRuntimeCacheInfoHobGuid in

VariableSmmRuntimeDxe driver to initialize the following

variable cache related buffer:
  *mVariableRuntimeHobCacheBuffer
  *mVariableRuntimeNvCacheBuffer
  *mVariableRuntimeVolatileCacheBuffer
  *mVariableRuntimeCachePendingUpdate
  *mVariableRuntimeCacheReadLock
  *mHobFlushComplete

The code to to allocate
and unblock the buffer for
different type cache in VariableSmmRuntimeDxe is also
removed in this commit.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--
 2 files changed, 52 insertions(+), 73 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 8b42ae7d72..68a249c5ac 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -35,10 +35,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
 #include <Library/BaseLib.h>
-#include <Library/MmUnblockMemoryLib.h>
+#include <Library/HobLib.h>

 #include <Guid/EventGroup.h>
 #include <Guid/SmmVariableCommon.h>
+#include <Guid/VariableRuntimeCacheInfo.h>

 #include "PrivilegePolymorphic.h"
 #include "VariableParsing.h"
@@ -56,10 +57,10 @@ VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
 UINTN                           mVariableBufferSize;
 UINTN                           mVariableRuntimeHobCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
-BOOLEAN                         mVariableRuntimeCachePendingUpdate;
-BOOLEAN                         mVariableRuntimeCacheReadLock;
+BOOLEAN                         *mVariableRuntimeCachePendingUpdate;
+BOOLEAN                         *mVariableRuntimeCacheReadLock;
 BOOLEAN                         mVariableAuthFormat;
-BOOLEAN                         mHobFlushComplete;
+BOOLEAN                         *mHobFlushComplete;
 EFI_LOCK                        mVariableServicesLock;
 EDKII_VARIABLE_LOCK_PROTOCOL    mVariableLock;
 EDKII_VAR_CHECK_PROTOCOL        mVarCheck;
@@ -180,27 +181,6 @@ InitVariableCache (

   *TotalVariableCacheSize = ALIGN_VALUE (*TotalVariableCacheSize, sizeof (UINT32));

-  //
-  // Allocate NV Storage Cache and initialize it to all 1's (like an erased FV)
-  //
-  *VariableCacheBuffer =  (VARIABLE_STORE_HEADER *)AllocateRuntimePages (
-                                                     EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-                                                     );
-  if (*VariableCacheBuffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // Request to unblock the newly allocated cache region to be accessible from inside MM
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)(UINTN)*VariableCacheBuffer,
-             EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    return Status;
-  }
-
   VariableCacheStorePtr = *VariableCacheBuffer;
   SetMem32 ((VOID *)VariableCacheStorePtr, *TotalVariableCacheSize, (UINT32)0xFFFFFFFF);

@@ -568,16 +548,16 @@ CheckForRuntimeCacheSync (
   VOID
   )
 {
-  if (mVariableRuntimeCachePendingUpdate) {
+  if (*mVariableRuntimeCachePendingUpdate) {
     SyncRuntimeCache ();
   }

-  ASSERT (!mVariableRuntimeCachePendingUpdate);
+  ASSERT (!(*mVariableRuntimeCachePendingUpdate));

   //
   // The HOB variable data may have finished being flushed in the runtime cache sync update
   //
-  if (mHobFlushComplete && (mVariableRuntimeHobCacheBuffer != NULL)) {
+  if ((*mHobFlushComplete) && (mVariableRuntimeHobCacheBuffer != NULL)) {
     if (!EfiAtRuntime ()) {
       FreePages (mVariableRuntimeHobCacheBuffer, EFI_SIZE_TO_PAGES (mVariableRuntimeHobCacheBufferSize));
     }
@@ -633,12 +613,12 @@ FindVariableInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

-  mVariableRuntimeCacheReadLock = TRUE;
+  *mVariableRuntimeCacheReadLock = TRUE;
   CheckForRuntimeCacheSync ();

-  if (!mVariableRuntimeCachePendingUpdate) {
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -698,7 +678,7 @@ Done:
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -921,12 +901,12 @@ GetNextVariableNameInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

   CheckForRuntimeCacheSync ();

-  mVariableRuntimeCacheReadLock = TRUE;
-  if (!mVariableRuntimeCachePendingUpdate) {
+  *mVariableRuntimeCacheReadLock = TRUE;
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -958,7 +938,7 @@ GetNextVariableNameInRuntimeCache (
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -1622,37 +1602,9 @@ SendRuntimeVariableCacheContextToSmm (
   SmmRuntimeVarCacheContext->RuntimeHobCache      = mVariableRuntimeHobCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeVolatileCache = mVariableRuntimeVolatileCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeNvCache       = mVariableRuntimeNvCacheBuffer;
-  SmmRuntimeVarCacheContext->PendingUpdate        = &mVariableRuntimeCachePendingUpdate;
-  SmmRuntimeVarCacheContext->ReadLock             = &mVariableRuntimeCacheReadLock;
-  SmmRuntimeVarCacheContext->HobFlushComplete     = &mHobFlushComplete;
-
-  //
-  // Request to unblock this region to be accessible from inside MM environment
-  // These fields "should" be all on the same page, but just to be on the safe side...
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->PendingUpdate - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCachePendingUpdate))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->ReadLock - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCacheReadLock))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->HobFlushComplete - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mHobFlushComplete))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
+  SmmRuntimeVarCacheContext->PendingUpdate        = mVariableRuntimeCachePendingUpdate;
+  SmmRuntimeVarCacheContext->ReadLock             = mVariableRuntimeCacheReadLock;
+  SmmRuntimeVarCacheContext->HobFlushComplete     = mHobFlushComplete;

   //
   // Send data to SMM.
@@ -1688,9 +1640,14 @@ SmmVariableReady (
   IN  VOID       *Context
   )
 {
-  EFI_STATUS  Status;
-  UINTN       RuntimeNvCacheSize;
-  UINTN       RuntimeVolatileCacheSize;
+  EFI_STATUS                   Status;
+  UINTN                        RuntimeNvCacheSize;
+  UINTN                        RuntimeVolatileCacheSize;
+  UINTN                        AllocatedHobCacheSize;
+  UINTN                        AllocatedNvCacheSize;
+  UINTN                        AllocatedVolatileCacheSize;
+  EFI_HOB_GUID_TYPE            *GuidHob;
+  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;

   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1717,7 +1674,7 @@ SmmVariableReady (
   if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
     DEBUG ((DEBUG_INFO, "Variable driver runtime cache is enabled.\n"));
     //
-    // Allocate runtime variable cache memory buffers.
+    // Get needed runtime cache buffer size and check if auth variables are to be used from SMM
     //
     Status =  GetRuntimeCacheInfo (
                 &mVariableRuntimeHobCacheBufferSize,
@@ -1726,6 +1683,27 @@ SmmVariableReady (
                 &mVariableAuthFormat
                 );
     if (!EFI_ERROR (Status)) {
+      GuidHob = GetFirstGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid);
+      ASSERT (GuidHob != NULL);
+      VariableRuntimeCacheHob    = GET_GUID_HOB_DATA (GuidHob);
+      AllocatedHobCacheSize      = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeHobCachePages);
+      AllocatedNvCacheSize       = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeNvCachePages);
+      AllocatedVolatileCacheSize = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeVolatileCachePages);
+
+      ASSERT (
+        (AllocatedHobCacheSize >= mVariableRuntimeHobCacheBufferSize) &&
+        (AllocatedNvCacheSize >= RuntimeNvCacheSize) &&
+        (AllocatedVolatileCacheSize >= RuntimeVolatileCacheSize)
+        );
+
+      mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
+      mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
+      mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
+      mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
+      mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
+      mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
+      mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
+
       Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
       if (!EFI_ERROR (Status)) {
         Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index e1085653fe..2d16f28674 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -13,7 +13,7 @@
 #  may not be modified without authorization. If platform fails to protect these resources,
 #  the authentication service provided in this driver will be broken, and the behavior is undefined.
 #
-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) Microsoft Corporation.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -60,7 +60,7 @@
   TpmMeasurementLib
   SafeIntLib
   PcdLib
-  MmUnblockMemoryLib
+  HobLib

 [Protocols]
   gEfiVariableWriteArchProtocolGuid             ## PRODUCES
@@ -113,6 +113,7 @@
   gVarCheckPolicyLibMmiHandlerGuid
   gEfiEndOfDxeEventGroupGuid
   gEfiDeviceSignatureDatabaseGuid
+  gEdkiiVariableRuntimeCacheInfoHobGuid

 [Depex]
   gEfiMmCommunication2ProtocolGuid
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119047): https://edk2.groups.io/g/devel/message/119047
Mute This Topic: https://groups.io/mt/106150805/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 23308 bytes --]

Re: [edk2-devel] [PATCH 9/9] MdeModulePkg:Add global variable mVariableRtCacheInfo

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 13134 bytes --]

I didn't read through all changes.
2 comments:

  1.
Please do not remember to call ConvertPointer() for any pointers accessed at runtime. I think your patch contains 4 pointers in the mVariableRtCacheInfo structure.
  2.
Please do not call FreePages() to free a PEI-allocated runtime memory. It won't succeed as PEI and DXE memory services do not share one database.
     *
The FreePages() call should be removed in earlier patch when you let the Variable driver consume the PEI-allocated memory.

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 9/9] MdeModulePkg:Add global variable mVariableRtCacheInfo

Add global variable mVariableRtCacheInfo to save the
content in gEdkiiVariableRuntimeCacheInfoHobGuid. With
this new global variable, 7 global variables can be
removed.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 97 +++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------
 1 file changed, 41 insertions(+), 56 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 6efe5cee10..de39462d68 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -44,26 +44,20 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "PrivilegePolymorphic.h"
 #include "VariableParsing.h"

-EFI_HANDLE                      mHandle                              = NULL;
-EFI_SMM_VARIABLE_PROTOCOL       *mSmmVariable                        = NULL;
-EFI_EVENT                       mVirtualAddressChangeEvent           = NULL;
-EFI_MM_COMMUNICATION2_PROTOCOL  *mMmCommunication2                   = NULL;
-UINT8                           *mVariableBuffer                     = NULL;
-UINT8                           *mVariableBufferPhysical             = NULL;
-VARIABLE_INFO_ENTRY             *mVariableInfo                       = NULL;
-VARIABLE_STORE_HEADER           *mVariableRuntimeHobCacheBuffer      = NULL;
-VARIABLE_STORE_HEADER           *mVariableRuntimeNvCacheBuffer       = NULL;
-VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
+EFI_HANDLE                      mHandle                    = NULL;
+EFI_SMM_VARIABLE_PROTOCOL       *mSmmVariable              = NULL;
+EFI_EVENT                       mVirtualAddressChangeEvent = NULL;
+EFI_MM_COMMUNICATION2_PROTOCOL  *mMmCommunication2         = NULL;
+UINT8                           *mVariableBuffer           = NULL;
+UINT8                           *mVariableBufferPhysical   = NULL;
+VARIABLE_INFO_ENTRY             *mVariableInfo             = NULL;
 UINTN                           mVariableBufferSize;
-UINTN                           mVariableRuntimeHobCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
-BOOLEAN                         *mVariableRuntimeCachePendingUpdate;
-BOOLEAN                         *mVariableRuntimeCacheReadLock;
 BOOLEAN                         mVariableAuthFormat;
-BOOLEAN                         *mHobFlushComplete;
 EFI_LOCK                        mVariableServicesLock;
 EDKII_VARIABLE_LOCK_PROTOCOL    mVariableLock;
 EDKII_VAR_CHECK_PROTOCOL        mVarCheck;
+VARIABLE_RUNTIME_CACHE_INFO     mVariableRtCacheInfo;

 /**
   The logic to initialize the VariablePolicy engine is in its own file.
@@ -500,21 +494,21 @@ CheckForRuntimeCacheSync (
   VOID
   )
 {
-  if (*mVariableRuntimeCachePendingUpdate) {
+  if (mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate) {
     SyncRuntimeCache ();
   }

-  ASSERT (!(*mVariableRuntimeCachePendingUpdate));
+  ASSERT (!(mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate));

   //
   // The HOB variable data may have finished being flushed in the runtime cache sync update
   //
-  if ((*mHobFlushComplete) && (mVariableRuntimeHobCacheBuffer != NULL)) {
+  if ((mVariableRtCacheInfo.CacheInfoFlag->HobFlushComplete) && (mVariableRtCacheInfo.RuntimeHobCacheBuffer != 0)) {
     if (!EfiAtRuntime ()) {
-      FreePages (mVariableRuntimeHobCacheBuffer, EFI_SIZE_TO_PAGES (mVariableRuntimeHobCacheBufferSize));
+      FreePages ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer, mVariableRtCacheInfo.RuntimeHobCachePages);
     }

-    mVariableRuntimeHobCacheBuffer = NULL;
+    mVariableRtCacheInfo.RuntimeHobCacheBuffer = 0;
   }
 }

@@ -565,20 +559,20 @@ FindVariableInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!(*mVariableRuntimeCacheReadLock));
+  ASSERT (!(mVariableRtCacheInfo.CacheInfoFlag->ReadLock));

-  *mVariableRuntimeCacheReadLock = TRUE;
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = TRUE;
   CheckForRuntimeCacheSync ();

-  if (!(*mVariableRuntimeCachePendingUpdate)) {
+  if (!(mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
     // makes use of this mapping to implement search algorithm.
     //
-    VariableStoreList[VariableStoreTypeVolatile] = mVariableRuntimeVolatileCacheBuffer;
-    VariableStoreList[VariableStoreTypeHob]      = mVariableRuntimeHobCacheBuffer;
-    VariableStoreList[VariableStoreTypeNv]       = mVariableRuntimeNvCacheBuffer;
+    VariableStoreList[VariableStoreTypeVolatile] = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer;
+    VariableStoreList[VariableStoreTypeHob]      = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer;
+    VariableStoreList[VariableStoreTypeNv]       = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer;

     for (StoreType = (VARIABLE_STORE_TYPE)0; StoreType < VariableStoreTypeMax; StoreType++) {
       if (VariableStoreList[StoreType] == NULL) {
@@ -630,7 +624,7 @@ Done:
     }
   }

-  *mVariableRuntimeCacheReadLock = FALSE;
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = FALSE;

   return Status;
 }
@@ -853,20 +847,20 @@ GetNextVariableNameInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!(*mVariableRuntimeCacheReadLock));
+  ASSERT (!(mVariableRtCacheInfo.CacheInfoFlag->ReadLock));

   CheckForRuntimeCacheSync ();

-  *mVariableRuntimeCacheReadLock = TRUE;
-  if (!(*mVariableRuntimeCachePendingUpdate)) {
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = TRUE;
+  if (!(mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
     // makes use of this mapping to implement search algorithm.
     //
-    VariableStoreHeader[VariableStoreTypeVolatile] = mVariableRuntimeVolatileCacheBuffer;
-    VariableStoreHeader[VariableStoreTypeHob]      = mVariableRuntimeHobCacheBuffer;
-    VariableStoreHeader[VariableStoreTypeNv]       = mVariableRuntimeNvCacheBuffer;
+    VariableStoreHeader[VariableStoreTypeVolatile] = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer;
+    VariableStoreHeader[VariableStoreTypeHob]      = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer;
+    VariableStoreHeader[VariableStoreTypeNv]       = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer;

     Status =  VariableServiceGetNextVariableInternal (
                 VariableName,
@@ -890,7 +884,7 @@ GetNextVariableNameInRuntimeCache (
     }
   }

-  *mVariableRuntimeCacheReadLock = FALSE;
+  mVariableRtCacheInfo.CacheInfoFlag->ReadLock = FALSE;

   return Status;
 }
@@ -1345,9 +1339,9 @@ VariableAddressChangeEvent (
 {
   EfiConvertPointer (0x0, (VOID **)&mVariableBuffer);
   EfiConvertPointer (0x0, (VOID **)&mMmCommunication2);
-  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRuntimeHobCacheBuffer);
-  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRuntimeNvCacheBuffer);
-  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRuntimeVolatileCacheBuffer);
+  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRtCacheInfo.RuntimeHobCacheBuffer);
+  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRtCacheInfo.RuntimeNvCacheBuffer);
+  EfiConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mVariableRtCacheInfo.RuntimeVolatileCacheBuffer);
 }

 /**
@@ -1576,17 +1570,10 @@ InitVariableCache (
       (AllocatedVolatileCacheSize >= ExpectedVolatileCacheSize)
       );

-    mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
-    mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
-    mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
-    mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
-    mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
-    mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
-    mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
-
-    InitVariableStoreHeader (mVariableRuntimeHobCacheBuffer, AllocatedHobCacheSize);
-    InitVariableStoreHeader (mVariableRuntimeNvCacheBuffer, AllocatedNvCacheSize);
-    InitVariableStoreHeader (mVariableRuntimeVolatileCacheBuffer, AllocatedVolatileCacheSize);
+    CopyMem (&mVariableRtCacheInfo, VariableRuntimeCacheHob, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
+    InitVariableStoreHeader ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer, AllocatedHobCacheSize);
+    InitVariableStoreHeader ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer, AllocatedNvCacheSize);
+    InitVariableStoreHeader ((VOID *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer, AllocatedVolatileCacheSize);
   }

   return Status;
@@ -1637,12 +1624,12 @@ SendRuntimeVariableCacheContextToSmm (
   SmmVariableFunctionHeader->Function = SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT;
   SmmRuntimeVarCacheContext           = (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT *)SmmVariableFunctionHeader->Data;

-  SmmRuntimeVarCacheContext->RuntimeHobCache      = mVariableRuntimeHobCacheBuffer;
-  SmmRuntimeVarCacheContext->RuntimeVolatileCache = mVariableRuntimeVolatileCacheBuffer;
-  SmmRuntimeVarCacheContext->RuntimeNvCache       = mVariableRuntimeNvCacheBuffer;
-  SmmRuntimeVarCacheContext->PendingUpdate        = mVariableRuntimeCachePendingUpdate;
-  SmmRuntimeVarCacheContext->ReadLock             = mVariableRuntimeCacheReadLock;
-  SmmRuntimeVarCacheContext->HobFlushComplete     = mHobFlushComplete;
+  SmmRuntimeVarCacheContext->RuntimeHobCache      = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeHobCacheBuffer;
+  SmmRuntimeVarCacheContext->RuntimeVolatileCache = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeVolatileCacheBuffer;
+  SmmRuntimeVarCacheContext->RuntimeNvCache       = (VARIABLE_STORE_HEADER *)(UINTN)mVariableRtCacheInfo.RuntimeNvCacheBuffer;
+  SmmRuntimeVarCacheContext->PendingUpdate        = &mVariableRtCacheInfo.CacheInfoFlag->PendingUpdate;
+  SmmRuntimeVarCacheContext->ReadLock             = &mVariableRtCacheInfo.CacheInfoFlag->ReadLock;
+  SmmRuntimeVarCacheContext->HobFlushComplete     = &mVariableRtCacheInfo.CacheInfoFlag->HobFlushComplete;

   //
   // Send data to SMM.
@@ -1712,9 +1699,7 @@ SmmVariableReady (
     }

     if (EFI_ERROR (Status)) {
-      mVariableRuntimeHobCacheBuffer      = NULL;
-      mVariableRuntimeNvCacheBuffer       = NULL;
-      mVariableRuntimeVolatileCacheBuffer = NULL;
+      ZeroMem (&mVariableRtCacheInfo, sizeof (VARIABLE_RUNTIME_CACHE_INFO));
     }

     ASSERT_EFI_ERROR (Status);
--
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119048): https://edk2.groups.io/g/devel/message/119048
Mute This Topic: https://groups.io/mt/106150807/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 23565 bytes --]

Re: [edk2-devel] [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

[Kun Qin via groups.io]

[-- Attachment #1: Type: text/plain, Size: 14229 bytes --]

Hi Ray & Dun,

Thanks for adding me to the patch. I think the proposed solution should work. One question, which is actually on the hob creator patch (https://edk2.groups.io/g/devel/message/119022), is that I understand the hob creation depends on "gEfiPeiMemoryDiscoveredPpiGuid", but does this routine still run properly if variable PEIM is loaded much later than memory discovered?

Regards,
Kun

From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, May 17, 2024 5:10 AM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Kun Qin <Kun.Qin@microsoft.com>; Sean Brogan <sean.brogan@microsoft.com>
Subject: [EXTERNAL] Re: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

Reviewed-by: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Subject: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

Consume gEdkiiVariableRuntimeCacheInfoHobGuid in

VariableSmmRuntimeDxe driver to initialize the following

variable cache related buffer:
  *mVariableRuntimeHobCacheBuffer
  *mVariableRuntimeNvCacheBuffer
  *mVariableRuntimeVolatileCacheBuffer
  *mVariableRuntimeCachePendingUpdate
  *mVariableRuntimeCacheReadLock
  *mHobFlushComplete

The code to to allocate
and unblock the buffer for
different type cache in VariableSmmRuntimeDxe is also
removed in this commit.

Signed-off-by: Dun Tan <dun.tan@intel.com<mailto:dun.tan@intel.com>>
Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>

Cc: Jiaxin Wu <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--
 2 files changed, 52 insertions(+), 73 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 8b42ae7d72..68a249c5ac 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -35,10 +35,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
 #include <Library/BaseLib.h>
-#include <Library/MmUnblockMemoryLib.h>
+#include <Library/HobLib.h>

 #include <Guid/EventGroup.h>
 #include <Guid/SmmVariableCommon.h>
+#include <Guid/VariableRuntimeCacheInfo.h>

 #include "PrivilegePolymorphic.h"
 #include "VariableParsing.h"
@@ -56,10 +57,10 @@ VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
 UINTN                           mVariableBufferSize;
 UINTN                           mVariableRuntimeHobCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
-BOOLEAN                         mVariableRuntimeCachePendingUpdate;
-BOOLEAN                         mVariableRuntimeCacheReadLock;
+BOOLEAN                         *mVariableRuntimeCachePendingUpdate;
+BOOLEAN                         *mVariableRuntimeCacheReadLock;
 BOOLEAN                         mVariableAuthFormat;
-BOOLEAN                         mHobFlushComplete;
+BOOLEAN                         *mHobFlushComplete;
 EFI_LOCK                        mVariableServicesLock;
 EDKII_VARIABLE_LOCK_PROTOCOL    mVariableLock;
 EDKII_VAR_CHECK_PROTOCOL        mVarCheck;
@@ -180,27 +181,6 @@ InitVariableCache (

   *TotalVariableCacheSize = ALIGN_VALUE (*TotalVariableCacheSize, sizeof (UINT32));

-  //
-  // Allocate NV Storage Cache and initialize it to all 1's (like an erased FV)
-  //
-  *VariableCacheBuffer =  (VARIABLE_STORE_HEADER *)AllocateRuntimePages (
-                                                     EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-                                                     );
-  if (*VariableCacheBuffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // Request to unblock the newly allocated cache region to be accessible from inside MM
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)(UINTN)*VariableCacheBuffer,
-             EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    return Status;
-  }
-
   VariableCacheStorePtr = *VariableCacheBuffer;
   SetMem32 ((VOID *)VariableCacheStorePtr, *TotalVariableCacheSize, (UINT32)0xFFFFFFFF);

@@ -568,16 +548,16 @@ CheckForRuntimeCacheSync (
   VOID
   )
 {
-  if (mVariableRuntimeCachePendingUpdate) {
+  if (*mVariableRuntimeCachePendingUpdate) {
     SyncRuntimeCache ();
   }

-  ASSERT (!mVariableRuntimeCachePendingUpdate);
+  ASSERT (!(*mVariableRuntimeCachePendingUpdate));

   //
   // The HOB variable data may have finished being flushed in the runtime cache sync update
   //
-  if (mHobFlushComplete && (mVariableRuntimeHobCacheBuffer != NULL)) {
+  if ((*mHobFlushComplete) && (mVariableRuntimeHobCacheBuffer != NULL)) {
     if (!EfiAtRuntime ()) {
       FreePages (mVariableRuntimeHobCacheBuffer, EFI_SIZE_TO_PAGES (mVariableRuntimeHobCacheBufferSize));
     }
@@ -633,12 +613,12 @@ FindVariableInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

-  mVariableRuntimeCacheReadLock = TRUE;
+  *mVariableRuntimeCacheReadLock = TRUE;
   CheckForRuntimeCacheSync ();

-  if (!mVariableRuntimeCachePendingUpdate) {
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -698,7 +678,7 @@ Done:
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -921,12 +901,12 @@ GetNextVariableNameInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

   CheckForRuntimeCacheSync ();

-  mVariableRuntimeCacheReadLock = TRUE;
-  if (!mVariableRuntimeCachePendingUpdate) {
+  *mVariableRuntimeCacheReadLock = TRUE;
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -958,7 +938,7 @@ GetNextVariableNameInRuntimeCache (
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -1622,37 +1602,9 @@ SendRuntimeVariableCacheContextToSmm (
   SmmRuntimeVarCacheContext->RuntimeHobCache      = mVariableRuntimeHobCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeVolatileCache = mVariableRuntimeVolatileCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeNvCache       = mVariableRuntimeNvCacheBuffer;
-  SmmRuntimeVarCacheContext->PendingUpdate        = &mVariableRuntimeCachePendingUpdate;
-  SmmRuntimeVarCacheContext->ReadLock             = &mVariableRuntimeCacheReadLock;
-  SmmRuntimeVarCacheContext->HobFlushComplete     = &mHobFlushComplete;
-
-  //
-  // Request to unblock this region to be accessible from inside MM environment
-  // These fields "should" be all on the same page, but just to be on the safe side...
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->PendingUpdate - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCachePendingUpdate))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->ReadLock - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCacheReadLock))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->HobFlushComplete - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mHobFlushComplete))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
+  SmmRuntimeVarCacheContext->PendingUpdate        = mVariableRuntimeCachePendingUpdate;
+  SmmRuntimeVarCacheContext->ReadLock             = mVariableRuntimeCacheReadLock;
+  SmmRuntimeVarCacheContext->HobFlushComplete     = mHobFlushComplete;

   //
   // Send data to SMM.
@@ -1688,9 +1640,14 @@ SmmVariableReady (
   IN  VOID       *Context
   )
 {
-  EFI_STATUS  Status;
-  UINTN       RuntimeNvCacheSize;
-  UINTN       RuntimeVolatileCacheSize;
+  EFI_STATUS                   Status;
+  UINTN                        RuntimeNvCacheSize;
+  UINTN                        RuntimeVolatileCacheSize;
+  UINTN                        AllocatedHobCacheSize;
+  UINTN                        AllocatedNvCacheSize;
+  UINTN                        AllocatedVolatileCacheSize;
+  EFI_HOB_GUID_TYPE            *GuidHob;
+  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;

   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1717,7 +1674,7 @@ SmmVariableReady (
   if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
     DEBUG ((DEBUG_INFO, "Variable driver runtime cache is enabled.\n"));
     //
-    // Allocate runtime variable cache memory buffers.
+    // Get needed runtime cache buffer size and check if auth variables are to be used from SMM
     //
     Status =  GetRuntimeCacheInfo (
                 &mVariableRuntimeHobCacheBufferSize,
@@ -1726,6 +1683,27 @@ SmmVariableReady (
                 &mVariableAuthFormat
                 );
     if (!EFI_ERROR (Status)) {
+      GuidHob = GetFirstGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid);
+      ASSERT (GuidHob != NULL);
+      VariableRuntimeCacheHob    = GET_GUID_HOB_DATA (GuidHob);
+      AllocatedHobCacheSize      = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeHobCachePages);
+      AllocatedNvCacheSize       = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeNvCachePages);
+      AllocatedVolatileCacheSize = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeVolatileCachePages);
+
+      ASSERT (
+        (AllocatedHobCacheSize >= mVariableRuntimeHobCacheBufferSize) &&
+        (AllocatedNvCacheSize >= RuntimeNvCacheSize) &&
+        (AllocatedVolatileCacheSize >= RuntimeVolatileCacheSize)
+        );
+
+      mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
+      mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
+      mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
+      mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
+      mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
+      mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
+      mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
+
       Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
       if (!EFI_ERROR (Status)) {
         Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index e1085653fe..2d16f28674 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -13,7 +13,7 @@
 #  may not be modified without authorization. If platform fails to protect these resources,
 #  the authentication service provided in this driver will be broken, and the behavior is undefined.
 #
-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) Microsoft Corporation.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -60,7 +60,7 @@
   TpmMeasurementLib
   SafeIntLib
   PcdLib
-  MmUnblockMemoryLib
+  HobLib

 [Protocols]
   gEfiVariableWriteArchProtocolGuid             ## PRODUCES
@@ -113,6 +113,7 @@
   gVarCheckPolicyLibMmiHandlerGuid
   gEfiEndOfDxeEventGroupGuid
   gEfiDeviceSignatureDatabaseGuid
+  gEdkiiVariableRuntimeCacheInfoHobGuid

 [Depex]
   gEfiMmCommunication2ProtocolGuid
--
2.31.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119055): https://edk2.groups.io/g/devel/message/119055
Mute This Topic: https://groups.io/mt/106150805/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 28180 bytes --]

Re: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[Nhi Pham via groups.io]

On 5/17/2024 4:49 PM, duntan via groups.io wrote:
> This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB is used to store the address and size of the buffer that will be used for variable runtime service when the PcdEnableVariableRuntimeCache is TRUE.
> In following patches, when PcdEnableVariableRuntimeCache is TRUE, VariablePei will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate the needed buffer for different type variable runtime cache and build the HOB.
> Then VariableSmmRuntimeDxe driver will consume gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable runtime cache related content. The code to allocate and unblock the runtime cache buffer in VariableSmmRuntimeDxe is also removed in this patc set.
> 
> PR for review: https://github.com/tianocore/edk2/pull/5607

Per design, SMM or StandaloneMM needs to access these runtime cache 
buffers for cache coherency. I'm not sure how to implement the 
MmUnblockMemoryLib for ARM to dynamically request mapping of the 
non-secure runtime cache buffers in StandaloneMM (Secure World). Is it 
possible to have these runtime buffers allocated statically with 
predefined PCD at build time. On ARM, they can also define the buffers 
in device tree (manifest)?

Thanks,
Nhi


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119068): https://edk2.groups.io/g/devel/message/119068
Mute This Topic: https://groups.io/mt/106150796/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[gaoliming via groups.io]

Dun:
  Is there a Bugzilla for this change?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> 发送时间: 2024年5月17日 17:49
> 收件人: devel@edk2.groups.io
> 抄送: Ray Ni <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Jiaxin Wu <jiaxin.wu@intel.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>;
> Leif Lindholm <quic_llindhol@quicinc.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>; Andrew Fish
> <afish@apple.com>; Jiewen Yao <jiewen.yao@intel.com>
> 主题: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache
> buffer in PEI
> 
> This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB is
> used to store the address and size of the buffer that will be used for
variable
> runtime service when the PcdEnableVariableRuntimeCache is TRUE.
> In following patches, when PcdEnableVariableRuntimeCache is TRUE,
VariablePei
> will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate the
needed
> buffer for different type variable runtime cache and build the HOB.
> Then VariableSmmRuntimeDxe driver will consume
> gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable runtime
cache
> related content. The code to allocate and unblock the runtime cache buffer
in
> VariableSmmRuntimeDxe is also removed in this patc set.
> 
> PR for review: https://github.com/tianocore/edk2/pull/5607
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> 
> Dun Tan (9):
>   MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid
>   ArmVirtPkg: Add MmUnblockMemoryLib in DSC
>   EmulatorPkg: Add MmUnblockMemoryLib in DSC
>   OvmfPkg: Add MmUnblockMemoryLib in DSC
>   MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid
>   MdeModulePkg:Remove unnecessary global variable
>   MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid
>   MdeModulePkg: Refine InitVariableCache()
>   MdeModulePkg:Add global variable mVariableRtCacheInfo
> 
>  ArmVirtPkg/ArmVirtCloudHv.dsc
> |   2 ++
>  EmulatorPkg/EmulatorPkg.dsc
> |   1 +
>  MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
> |  65
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++
>  MdeModulePkg/MdeModulePkg.dec
> |   3 +++
>  MdeModulePkg/Universal/Variable/Pei/Variable.c                       |
> 298
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  MdeModulePkg/Universal/Variable/Pei/Variable.h                       |
> 3 +++
>  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                  |
> 8 +++++++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> | 293
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
++--------------------------------------------------------------------------
-------------------------
> ----------------------------------------------------------------------
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |
> 5 +++--
>  OvmfPkg/OvmfPkgIa32X64.dsc
> |   2 +-
>  10 files changed, 506 insertions(+), 174 deletions(-)
>  create mode 100644
> MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119070): https://edk2.groups.io/g/devel/message/119070
Mute This Topic: https://groups.io/mt/106196627/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 2953 bytes --]

I remember ARM platform could have a PEI-less design so that SEC directly invokes DXE.

So I can imagine that a SEC logic to create the VARIABLE_RUNTIME_CACHE_INFO HOB.

Then it comes to how to calculate the size before bios boots.
I think it's doable.
There are 3 caches. Volatile cache size is hardcode by a PCD. NV cache size can equal to the bios NV variable region size. HOB cache size can be calculated by:

  1.
collect all default values in IFR/VFR
  2.
convert the default variable to auth/non-auth type depending on the BIOS NV variable region format.

Both steps can be performed in bios-build phase. There is no runtime information needed.

Thanks,
Ray
________________________________
From: Nhi Pham <nhi@os.amperecomputing.com>
Sent: Monday, May 20, 2024 9:01
To: devel@edk2.groups.io <devel@edk2.groups.io>; Tan, Dun <dun.tan@intel.com>
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <quic_llindhol@quicinc.com>; Sami Mujawar <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>; Andrew Fish <afish@apple.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

On 5/17/2024 4:49 PM, duntan via groups.io wrote:
> This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB is used to store the address and size of the buffer that will be used for variable runtime service when the PcdEnableVariableRuntimeCache is TRUE.
> In following patches, when PcdEnableVariableRuntimeCache is TRUE, VariablePei will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate the needed buffer for different type variable runtime cache and build the HOB.
> Then VariableSmmRuntimeDxe driver will consume gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable runtime cache related content. The code to allocate and unblock the runtime cache buffer in VariableSmmRuntimeDxe is also removed in this patc set.
>
> PR for review: https://github.com/tianocore/edk2/pull/5607

Per design, SMM or StandaloneMM needs to access these runtime cache
buffers for cache coherency. I'm not sure how to implement the
MmUnblockMemoryLib for ARM to dynamically request mapping of the
non-secure runtime cache buffers in StandaloneMM (Secure World). Is it
possible to have these runtime buffers allocated statically with
predefined PCD at build time. On ARM, they can also define the buffers
in device tree (manifest)?

Thanks,
Nhi


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119072): https://edk2.groups.io/g/devel/message/119072
Mute This Topic: https://groups.io/mt/106150796/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 6958 bytes --]

Re: [edk2-devel] [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 14916 bytes --]

Kun,
Good to know that you have no concerns on the patch. The patch set aims to finalize the unblock memory regions before standalone MM env is launched.
The PeiNotifyPpi() can still notify the PPI callback when the PPI has been installed already.

Thanks,
Ray
________________________________
From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Saturday, May 18, 2024 1:09
To: Ni, Ray <ray.ni@intel.com>; Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Sean Brogan <sean.brogan@microsoft.com>
Subject: RE: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid


Hi Ray & Dun,



Thanks for adding me to the patch. I think the proposed solution should work. One question, which is actually on the hob creator patch (https://edk2.groups.io/g/devel/message/119022), is that I understand the hob creation depends on “gEfiPeiMemoryDiscoveredPpiGuid”, but does this routine still run properly if variable PEIM is loaded much later than memory discovered?



Regards,

Kun



From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, May 17, 2024 5:10 AM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Kun Qin <Kun.Qin@microsoft.com>; Sean Brogan <sean.brogan@microsoft.com>
Subject: [EXTERNAL] Re: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid



Reviewed-by: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>



Thanks,

Ray

________________________________

From: Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>
Sent: Friday, May 17, 2024 17:49
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Subject: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid



Consume gEdkiiVariableRuntimeCacheInfoHobGuid in

VariableSmmRuntimeDxe driver to initialize the following

variable cache related buffer:
  *mVariableRuntimeHobCacheBuffer
  *mVariableRuntimeNvCacheBuffer
  *mVariableRuntimeVolatileCacheBuffer
  *mVariableRuntimeCachePendingUpdate
  *mVariableRuntimeCacheReadLock
  *mHobFlushComplete

The code to to allocate
and unblock the buffer for
different type cache in VariableSmmRuntimeDxe is also
removed in this commit.

Signed-off-by: Dun Tan <dun.tan@intel.com<mailto:dun.tan@intel.com>>
Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>

Cc: Jiaxin Wu <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--
 2 files changed, 52 insertions(+), 73 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 8b42ae7d72..68a249c5ac 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -35,10 +35,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
 #include <Library/BaseLib.h>
-#include <Library/MmUnblockMemoryLib.h>
+#include <Library/HobLib.h>

 #include <Guid/EventGroup.h>
 #include <Guid/SmmVariableCommon.h>
+#include <Guid/VariableRuntimeCacheInfo.h>

 #include "PrivilegePolymorphic.h"
 #include "VariableParsing.h"
@@ -56,10 +57,10 @@ VARIABLE_STORE_HEADER           *mVariableRuntimeVolatileCacheBuffer = NULL;
 UINTN                           mVariableBufferSize;
 UINTN                           mVariableRuntimeHobCacheBufferSize;
 UINTN                           mVariableBufferPayloadSize;
-BOOLEAN                         mVariableRuntimeCachePendingUpdate;
-BOOLEAN                         mVariableRuntimeCacheReadLock;
+BOOLEAN                         *mVariableRuntimeCachePendingUpdate;
+BOOLEAN                         *mVariableRuntimeCacheReadLock;
 BOOLEAN                         mVariableAuthFormat;
-BOOLEAN                         mHobFlushComplete;
+BOOLEAN                         *mHobFlushComplete;
 EFI_LOCK                        mVariableServicesLock;
 EDKII_VARIABLE_LOCK_PROTOCOL    mVariableLock;
 EDKII_VAR_CHECK_PROTOCOL        mVarCheck;
@@ -180,27 +181,6 @@ InitVariableCache (

   *TotalVariableCacheSize = ALIGN_VALUE (*TotalVariableCacheSize, sizeof (UINT32));

-  //
-  // Allocate NV Storage Cache and initialize it to all 1's (like an erased FV)
-  //
-  *VariableCacheBuffer =  (VARIABLE_STORE_HEADER *)AllocateRuntimePages (
-                                                     EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-                                                     );
-  if (*VariableCacheBuffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // Request to unblock the newly allocated cache region to be accessible from inside MM
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)(UINTN)*VariableCacheBuffer,
-             EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    return Status;
-  }
-
   VariableCacheStorePtr = *VariableCacheBuffer;
   SetMem32 ((VOID *)VariableCacheStorePtr, *TotalVariableCacheSize, (UINT32)0xFFFFFFFF);

@@ -568,16 +548,16 @@ CheckForRuntimeCacheSync (
   VOID
   )
 {
-  if (mVariableRuntimeCachePendingUpdate) {
+  if (*mVariableRuntimeCachePendingUpdate) {
     SyncRuntimeCache ();
   }

-  ASSERT (!mVariableRuntimeCachePendingUpdate);
+  ASSERT (!(*mVariableRuntimeCachePendingUpdate));

   //
   // The HOB variable data may have finished being flushed in the runtime cache sync update
   //
-  if (mHobFlushComplete && (mVariableRuntimeHobCacheBuffer != NULL)) {
+  if ((*mHobFlushComplete) && (mVariableRuntimeHobCacheBuffer != NULL)) {
     if (!EfiAtRuntime ()) {
       FreePages (mVariableRuntimeHobCacheBuffer, EFI_SIZE_TO_PAGES (mVariableRuntimeHobCacheBufferSize));
     }
@@ -633,12 +613,12 @@ FindVariableInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

-  mVariableRuntimeCacheReadLock = TRUE;
+  *mVariableRuntimeCacheReadLock = TRUE;
   CheckForRuntimeCacheSync ();

-  if (!mVariableRuntimeCachePendingUpdate) {
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -698,7 +678,7 @@ Done:
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -921,12 +901,12 @@ GetNextVariableNameInRuntimeCache (
   // a GetVariable () or GetNextVariable () call from being issued until a prior call has returned. The runtime
   // cache read lock should always be free when entering this function.
   //
-  ASSERT (!mVariableRuntimeCacheReadLock);
+  ASSERT (!(*mVariableRuntimeCacheReadLock));

   CheckForRuntimeCacheSync ();

-  mVariableRuntimeCacheReadLock = TRUE;
-  if (!mVariableRuntimeCachePendingUpdate) {
+  *mVariableRuntimeCacheReadLock = TRUE;
+  if (!(*mVariableRuntimeCachePendingUpdate)) {
     //
     // 0: Volatile, 1: HOB, 2: Non-Volatile.
     // The index and attributes mapping must be kept in this order as FindVariable
@@ -958,7 +938,7 @@ GetNextVariableNameInRuntimeCache (
     }
   }

-  mVariableRuntimeCacheReadLock = FALSE;
+  *mVariableRuntimeCacheReadLock = FALSE;

   return Status;
 }
@@ -1622,37 +1602,9 @@ SendRuntimeVariableCacheContextToSmm (
   SmmRuntimeVarCacheContext->RuntimeHobCache      = mVariableRuntimeHobCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeVolatileCache = mVariableRuntimeVolatileCacheBuffer;
   SmmRuntimeVarCacheContext->RuntimeNvCache       = mVariableRuntimeNvCacheBuffer;
-  SmmRuntimeVarCacheContext->PendingUpdate        = &mVariableRuntimeCachePendingUpdate;
-  SmmRuntimeVarCacheContext->ReadLock             = &mVariableRuntimeCacheReadLock;
-  SmmRuntimeVarCacheContext->HobFlushComplete     = &mHobFlushComplete;
-
-  //
-  // Request to unblock this region to be accessible from inside MM environment
-  // These fields "should" be all on the same page, but just to be on the safe side...
-  //
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->PendingUpdate - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCachePendingUpdate))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->ReadLock - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mVariableRuntimeCacheReadLock))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
-
-  Status = MmUnblockMemoryRequest (
-             (EFI_PHYSICAL_ADDRESS)ALIGN_VALUE ((UINTN)SmmRuntimeVarCacheContext->HobFlushComplete - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
-             EFI_SIZE_TO_PAGES (sizeof (mHobFlushComplete))
-             );
-  if ((Status != EFI_UNSUPPORTED) && EFI_ERROR (Status)) {
-    goto Done;
-  }
+  SmmRuntimeVarCacheContext->PendingUpdate        = mVariableRuntimeCachePendingUpdate;
+  SmmRuntimeVarCacheContext->ReadLock             = mVariableRuntimeCacheReadLock;
+  SmmRuntimeVarCacheContext->HobFlushComplete     = mHobFlushComplete;

   //
   // Send data to SMM.
@@ -1688,9 +1640,14 @@ SmmVariableReady (
   IN  VOID       *Context
   )
 {
-  EFI_STATUS  Status;
-  UINTN       RuntimeNvCacheSize;
-  UINTN       RuntimeVolatileCacheSize;
+  EFI_STATUS                   Status;
+  UINTN                        RuntimeNvCacheSize;
+  UINTN                        RuntimeVolatileCacheSize;
+  UINTN                        AllocatedHobCacheSize;
+  UINTN                        AllocatedNvCacheSize;
+  UINTN                        AllocatedVolatileCacheSize;
+  EFI_HOB_GUID_TYPE            *GuidHob;
+  VARIABLE_RUNTIME_CACHE_INFO  *VariableRuntimeCacheHob;

   Status = gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)&mSmmVariable);
   if (EFI_ERROR (Status)) {
@@ -1717,7 +1674,7 @@ SmmVariableReady (
   if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
     DEBUG ((DEBUG_INFO, "Variable driver runtime cache is enabled.\n"));
     //
-    // Allocate runtime variable cache memory buffers.
+    // Get needed runtime cache buffer size and check if auth variables are to be used from SMM
     //
     Status =  GetRuntimeCacheInfo (
                 &mVariableRuntimeHobCacheBufferSize,
@@ -1726,6 +1683,27 @@ SmmVariableReady (
                 &mVariableAuthFormat
                 );
     if (!EFI_ERROR (Status)) {
+      GuidHob = GetFirstGuidHob (&gEdkiiVariableRuntimeCacheInfoHobGuid);
+      ASSERT (GuidHob != NULL);
+      VariableRuntimeCacheHob    = GET_GUID_HOB_DATA (GuidHob);
+      AllocatedHobCacheSize      = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeHobCachePages);
+      AllocatedNvCacheSize       = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeNvCachePages);
+      AllocatedVolatileCacheSize = EFI_PAGES_TO_SIZE (VariableRuntimeCacheHob->RuntimeVolatileCachePages);
+
+      ASSERT (
+        (AllocatedHobCacheSize >= mVariableRuntimeHobCacheBufferSize) &&
+        (AllocatedNvCacheSize >= RuntimeNvCacheSize) &&
+        (AllocatedVolatileCacheSize >= RuntimeVolatileCacheSize)
+        );
+
+      mVariableRuntimeHobCacheBuffer      = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeHobCacheBuffer;
+      mVariableRuntimeNvCacheBuffer       = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeNvCacheBuffer;
+      mVariableRuntimeVolatileCacheBuffer = (VARIABLE_STORE_HEADER *)(UINTN)VariableRuntimeCacheHob->RuntimeVolatileCacheBuffer;
+      mVariableRuntimeCachePendingUpdate  = &VariableRuntimeCacheHob->CacheInfoFlag->PendingUpdate;
+      mVariableRuntimeCacheReadLock       = &VariableRuntimeCacheHob->CacheInfoFlag->ReadLock;
+      mHobFlushComplete                   = &VariableRuntimeCacheHob->CacheInfoFlag->HobFlushComplete;
+      mVariableRuntimeHobCacheBufferSize  = AllocatedHobCacheSize;
+
       Status = InitVariableCache (&mVariableRuntimeHobCacheBuffer, &mVariableRuntimeHobCacheBufferSize);
       if (!EFI_ERROR (Status)) {
         Status = InitVariableCache (&mVariableRuntimeNvCacheBuffer, &RuntimeNvCacheSize);
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index e1085653fe..2d16f28674 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -13,7 +13,7 @@
 #  may not be modified without authorization. If platform fails to protect these resources,
 #  the authentication service provided in this driver will be broken, and the behavior is undefined.
 #
-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) Microsoft Corporation.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -60,7 +60,7 @@
   TpmMeasurementLib
   SafeIntLib
   PcdLib
-  MmUnblockMemoryLib
+  HobLib

 [Protocols]
   gEfiVariableWriteArchProtocolGuid             ## PRODUCES
@@ -113,6 +113,7 @@
   gVarCheckPolicyLibMmiHandlerGuid
   gEfiEndOfDxeEventGroupGuid
   gEfiDeviceSignatureDatabaseGuid
+  gEdkiiVariableRuntimeCacheInfoHobGuid

 [Depex]
   gEfiMmCommunication2ProtocolGuid
--
2.31.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119074): https://edk2.groups.io/g/devel/message/119074
Mute This Topic: https://groups.io/mt/106150805/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 29104 bytes --]

Re: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[duntan]

Hi Liming,

I haven't created a Bugzilla for this change.
Is a bugzila needed for this patch set? I can create one if needed.

Thanks,
Dun

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Monday, May 20, 2024 9:41 AM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Ni, Ray <ray.ni@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; 'Ard Biesheuvel' <ardb+tianocore@kernel.org>; 'Leif Lindholm' <quic_llindhol@quicinc.com>; 'Sami Mujawar' <sami.mujawar@arm.com>; 'Gerd Hoffmann' <kraxel@redhat.com>; 'Andrew Fish' <afish@apple.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: 回复: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

Dun:
  Is there a Bugzilla for this change?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> 发送时间: 2024年5月17日 17:49
> 收件人: devel@edk2.groups.io
> 抄送: Ray Ni <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; 
> Jiaxin Wu <jiaxin.wu@intel.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>;
> Leif Lindholm <quic_llindhol@quicinc.com>; Sami Mujawar 
> <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>; Andrew Fish 
> <afish@apple.com>; Jiewen Yao <jiewen.yao@intel.com>
> 主题: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime 
> cache buffer in PEI
> 
> This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB 
> is used to store the address and size of the buffer that will be used 
> for
variable
> runtime service when the PcdEnableVariableRuntimeCache is TRUE.
> In following patches, when PcdEnableVariableRuntimeCache is TRUE,
VariablePei
> will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate 
> the
needed
> buffer for different type variable runtime cache and build the HOB.
> Then VariableSmmRuntimeDxe driver will consume 
> gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable 
> runtime
cache
> related content. The code to allocate and unblock the runtime cache 
> buffer
in
> VariableSmmRuntimeDxe is also removed in this patc set.
> 
> PR for review: https://github.com/tianocore/edk2/pull/5607
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> 
> Dun Tan (9):
>   MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid
>   ArmVirtPkg: Add MmUnblockMemoryLib in DSC
>   EmulatorPkg: Add MmUnblockMemoryLib in DSC
>   OvmfPkg: Add MmUnblockMemoryLib in DSC
>   MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid
>   MdeModulePkg:Remove unnecessary global variable
>   MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid
>   MdeModulePkg: Refine InitVariableCache()
>   MdeModulePkg:Add global variable mVariableRtCacheInfo
> 
>  ArmVirtPkg/ArmVirtCloudHv.dsc
> |   2 ++
>  EmulatorPkg/EmulatorPkg.dsc
> |   1 +
>  MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
> |  65
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++
>  MdeModulePkg/MdeModulePkg.dec
> |   3 +++
>  MdeModulePkg/Universal/Variable/Pei/Variable.c                       |
> 298
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  MdeModulePkg/Universal/Variable/Pei/Variable.h                       |
> 3 +++
>  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                  |
> 8 +++++++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> | 293
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
++----------------------------------------------------------------------
++----
-------------------------
> ----------------------------------------------------------------------
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf 
> |
> 5 +++--
>  OvmfPkg/OvmfPkgIa32X64.dsc
> |   2 +-
>  10 files changed, 506 insertions(+), 174 deletions(-)  create mode 
> 100644 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119075): https://edk2.groups.io/g/devel/message/119075
Mute This Topic: https://groups.io/mt/106199240/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[Sean]

[-- Attachment #1: Type: text/plain, Size: 6022 bytes --]

I can't find patch 1 in the series in my email so putting a few comments 
here.  I really hope this patch series can wait for PRs so code comments 
can more easily be correlated with code change.

Looking at your PR with commit: Allocate Varaible cache buffer in PEI by 
td36 · Pull Request #5607 · tianocore/edk2 (github.com) 
<https://github.com/tianocore/edk2/pull/5607/commits/36c2cac5fa4196be7fc85d842e8056e376010479>

A few comments for now.

Variable is spelled incorrectly in commit message.

I would really prefer to not mix PCDs and Hobs.  It is really confusing 
what has to be turned on and set to what to get the different 
behaviors.  For a hob producer it is OK to take that info from PCDs but 
lets not mix in the consumer code PCDs and hob data.  The HOB definition 
should not reference a PCD and a HOB definition should not be focused on 
producer/consumer but on the data.

The existence of a hob is also a good indicator that a feature is used 
so you may not need to have "enable" PCDs anymore.

Please don't include other header files in public header files 
(especially for super common headers like PiPei.h.  i know over the last 
few years this practice has become more common but it just creates pain 
when debugging build errors.  The trade off is not worth it.

Hobs really shouldn't use UINTN sizes.  Since hobs helps transfer config 
state across phases where the size of UINTN may be different this causes 
problems.

Hobs used for cross phase sharing shouldn't have pointers for the reason.

Better and more complete comments on the different field members would 
be helpful.  I assume the "Buffer" fields are physical addresses and the 
"Pages" are number of 4K pages.   I would suggest EFI_PHYSICAL_ADDRESS 
for addresses and UINT64 for lengths.

More details on what the three cache's are would be helpful or at least 
reference the feature of the cache they are supporting.

This hob definition file isn't perfect and I believe some of the comment 
in the file header belong in different places, it is at least a good 
template for a hob definition.

edk2/MdeModulePkg/Include/Guid/VariableFlashInfo.h at 
284dbac43da752ee34825c8b3f6f9e8281cb5a19 · tianocore/edk2 (github.com) 
<https://github.com/tianocore/edk2/blob/284dbac43da752ee34825c8b3f6f9e8281cb5a19/MdeModulePkg/Include/Guid/VariableFlashInfo.h>


Thanks

Sean



On 5/17/2024 2:49 AM, duntan wrote:
> This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB is used to store the address and size of the buffer that will be used for variable runtime service when the PcdEnableVariableRuntimeCache is TRUE.
> In following patches, when PcdEnableVariableRuntimeCache is TRUE, VariablePei will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate the needed buffer for different type variable runtime cache and build the HOB.
> Then VariableSmmRuntimeDxe driver will consume gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable runtime cache related content. The code to allocate and unblock the runtime cache buffer in VariableSmmRuntimeDxe is also removed in this patc set.
>
> PR for review:https://github.com/tianocore/edk2/pull/5607
>
> Cc: Ray Ni<ray.ni@intel.com>
> Cc: Liming Gao<gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu<jiaxin.wu@intel.com>
> Cc: Ard Biesheuvel<ardb+tianocore@kernel.org>
> Cc: Leif Lindholm<quic_llindhol@quicinc.com>
> Cc: Sami Mujawar<sami.mujawar@arm.com>
> Cc: Gerd Hoffmann<kraxel@redhat.com>
> Cc: Andrew Fish<afish@apple.com>
> Cc: Jiewen Yao<jiewen.yao@intel.com>
>
> Dun Tan (9):
>    MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid
>    ArmVirtPkg: Add MmUnblockMemoryLib in DSC
>    EmulatorPkg: Add MmUnblockMemoryLib in DSC
>    OvmfPkg: Add MmUnblockMemoryLib in DSC
>    MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid
>    MdeModulePkg:Remove unnecessary global variable
>    MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid
>    MdeModulePkg: Refine InitVariableCache()
>    MdeModulePkg:Add global variable mVariableRtCacheInfo
>
>   ArmVirtPkg/ArmVirtCloudHv.dsc                                        |   2 ++
>   EmulatorPkg/EmulatorPkg.dsc                                          |   1 +
>   MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h                 |  65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   MdeModulePkg/MdeModulePkg.dec                                        |   3 +++
>   MdeModulePkg/Universal/Variable/Pei/Variable.c                       | 298 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   MdeModulePkg/Universal/Variable/Pei/Variable.h                       |   3 +++
>   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                  |   8 +++++++-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 293 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--
>   OvmfPkg/OvmfPkgIa32X64.dsc                                           |   2 +-
>   10 files changed, 506 insertions(+), 174 deletions(-)
>   create mode 100644 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119089): https://edk2.groups.io/g/devel/message/119089
Mute This Topic: https://groups.io/mt/106150796/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 8056 bytes --]

Re: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI

[duntan]

[-- Attachment #1: Type: text/plain, Size: 7428 bytes --]

Hi Sean,

I saw the PR contribution process proposal in community. It looks good.

I agree that the existence of the HOB can indicate if the feature is used or not on the consumer side. Will move the reference to PcdEnableVariableRuntimeCache on the HOB consumer side. But I think PcdEnableVariableRuntimeCache is still needed to decide whether we need to build HOB or not.

For other comments, I’ll modify the patch series based on your comments and the sample you mentioned. Thanks for you detailed comments.

Thanks,
Dun

From: Sean Brogan <spbrogan@outlook.com>
Sent: Tuesday, May 21, 2024 2:17 AM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <quic_llindhol@quicinc.com>; Sami Mujawar <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>; Andrew Fish <afish@apple.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/9] Allocate and unblock variable runtime cache buffer in PEI


I can't find patch 1 in the series in my email so putting a few comments here.  I really hope this patch series can wait for PRs so code comments can more easily be correlated with code change.

Looking at your PR with commit: Allocate Varaible cache buffer in PEI by td36 · Pull Request #5607 · tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/5607/commits/36c2cac5fa4196be7fc85d842e8056e376010479>

A few comments for now.

Variable is spelled incorrectly in commit message.

I would really prefer to not mix PCDs and Hobs.  It is really confusing what has to be turned on and set to what to get the different behaviors.  For a hob producer it is OK to take that info from PCDs but lets not mix in the consumer code PCDs and hob data.  The HOB definition should not reference a PCD and a HOB definition should not be focused on producer/consumer but on the data.

The existence of a hob is also a good indicator that a feature is used so you may not need to have "enable" PCDs anymore.

Please don't include other header files in public header files (especially for super common headers like PiPei.h.  i know over the last few years this practice has become more common but it just creates pain when debugging build errors.  The trade off is not worth it.

Hobs really shouldn't use UINTN sizes.  Since hobs helps transfer config state across phases where the size of UINTN may be different this causes problems.

Hobs used for cross phase sharing shouldn't have pointers for the reason.

Better and more complete comments on the different field members would be helpful.  I assume the "Buffer" fields are physical addresses and the "Pages" are number of 4K pages.   I would suggest EFI_PHYSICAL_ADDRESS for addresses and UINT64 for lengths.

More details on what the three cache's are would be helpful or at least reference the feature of the cache they are supporting.

This hob definition file isn't perfect and I believe some of the comment in the file header belong in different places, it is at least a good template for a hob definition.

edk2/MdeModulePkg/Include/Guid/VariableFlashInfo.h at 284dbac43da752ee34825c8b3f6f9e8281cb5a19 · tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/blob/284dbac43da752ee34825c8b3f6f9e8281cb5a19/MdeModulePkg/Include/Guid/VariableFlashInfo.h>



Thanks

Sean




On 5/17/2024 2:49 AM, duntan wrote:

This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB is used to store the address and size of the buffer that will be used for variable runtime service when the PcdEnableVariableRuntimeCache is TRUE.

In following patches, when PcdEnableVariableRuntimeCache is TRUE, VariablePei will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate the needed buffer for different type variable runtime cache and build the HOB.

Then VariableSmmRuntimeDxe driver will consume gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable runtime cache related content. The code to allocate and unblock the runtime cache buffer in VariableSmmRuntimeDxe is also removed in this patc set.



PR for review: https://github.com/tianocore/edk2/pull/5607



Cc: Ray Ni <ray.ni@intel.com><mailto:ray.ni@intel.com>

Cc: Liming Gao <gaoliming@byosoft.com.cn><mailto:gaoliming@byosoft.com.cn>

Cc: Jiaxin Wu <jiaxin.wu@intel.com><mailto:jiaxin.wu@intel.com>

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org><mailto:ardb+tianocore@kernel.org>

Cc: Leif Lindholm <quic_llindhol@quicinc.com><mailto:quic_llindhol@quicinc.com>

Cc: Sami Mujawar <sami.mujawar@arm.com><mailto:sami.mujawar@arm.com>

Cc: Gerd Hoffmann <kraxel@redhat.com><mailto:kraxel@redhat.com>

Cc: Andrew Fish <afish@apple.com><mailto:afish@apple.com>

Cc: Jiewen Yao <jiewen.yao@intel.com><mailto:jiewen.yao@intel.com>



Dun Tan (9):

  MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid

  ArmVirtPkg: Add MmUnblockMemoryLib in DSC

  EmulatorPkg: Add MmUnblockMemoryLib in DSC

  OvmfPkg: Add MmUnblockMemoryLib in DSC

  MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid

  MdeModulePkg:Remove unnecessary global variable

  MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

  MdeModulePkg: Refine InitVariableCache()

  MdeModulePkg:Add global variable mVariableRtCacheInfo



 ArmVirtPkg/ArmVirtCloudHv.dsc                                        |   2 ++

 EmulatorPkg/EmulatorPkg.dsc                                          |   1 +

 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h                 |  65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 MdeModulePkg/MdeModulePkg.dec                                        |   3 +++

 MdeModulePkg/Universal/Variable/Pei/Variable.c                       | 298 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-

 MdeModulePkg/Universal/Variable/Pei/Variable.h                       |   3 +++

 MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                  |   8 +++++++-

 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 293 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------

 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   5 +++--

 OvmfPkg/OvmfPkgIa32X64.dsc                                           |   2 +-

 10 files changed, 506 insertions(+), 174 deletions(-)

 create mode 100644 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119100): https://edk2.groups.io/g/devel/message/119100
Mute This Topic: https://groups.io/mt/106150796/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 13552 bytes --]

Re: [edk2-devel] [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid

[Kenneth Lautner]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

Hi Dun, Ray and Kun,

Something to consider is that Test Point in edk2_platforms/MinPlatformPkg looks for contiguous memory for all of the memory types that persist into the OS.  This runtime allocation in PEI will cause fragmentation that will lead to that test failing.

Thanks,

Ken


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119110): https://edk2.groups.io/g/devel/message/119110
Mute This Topic: https://groups.io/mt/106150805/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 1113 bytes --]