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

["Ni, Ray" <ray.ni@intel.com>]

[-- 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 --]