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

["Kun Qin via groups.io" <Kun.Qin=microsoft.com@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 --]