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.
    1. 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) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_