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 Sent: Friday, May 17, 2024 17:49 To: devel@edk2.groups.io Cc: Ni, Ray ; Liming Gao ; Wu, Jiaxin 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 Cc: Ray Ni Cc: Liming Gao Cc: Jiaxin Wu --- 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] -=-=-=-=-=-=-=-=-=-=-=-