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....@microsoft.com> Sent: Saturday, May 18, 2024 1:09 To: Ni, Ray <ray...@intel.com>; Tan, Dun <dun....@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io> Cc: Liming Gao <gaolim...@byosoft.com.cn>; Sean Brogan <sean.bro...@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...@intel.com> Sent: Friday, May 17, 2024 5:10 AM To: Tan, Dun <dun....@intel.com>; devel@edk2.groups.io Cc: Liming Gao <gaolim...@byosoft.com.cn>; Kun Qin <kun....@microsoft.com>; Sean Brogan <sean.bro...@microsoft.com> Subject: [EXTERNAL] Re: [PATCH 7/9] MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid Reviewed-by: Ray Ni <ray...@intel.com<mailto:ray...@intel.com>> Thanks, Ray ________________________________ From: Tan, Dun <dun....@intel.com<mailto:dun....@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...@intel.com<mailto:ray...@intel.com>>; Liming Gao <gaolim...@byosoft.com.cn<mailto:gaolim...@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....@intel.com<mailto:dun....@intel.com>> Cc: Ray Ni <ray...@intel.com<mailto:ray...@intel.com>> Cc: Liming Gao <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>> Cc: Jiaxin Wu <jiaxin...@intel.com<mailto:jiaxin...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-