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