-----Original Message-----
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 3, 2019 1:05 AM
To: Kubacki, Michael A <michael.a.kubacki@intel.com>;
devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Ard Biesheuvel
<ard.biesheuvel@linaro.org>; Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
<lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; Kinney, Michael
D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [PATCH V2 8/9] MdeModulePkg/Variable: Add RT
GetNextVariableName() cache support
-----Original Message-----
From: Kubacki, Michael A
Sent: Saturday, September 28, 2019 9:47 AM
To: devel@edk2.groups.io
Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming;
Kinney, Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen
Subject: [PATCH V2 8/9] MdeModulePkg/Variable: Add RT
GetNextVariableName() cache support
https://bugzilla.tianocore.org/show_bug.cgi?id=2220
This change implements the Runtime Service GetNextVariableName()
using
the Runtime Cache in VariableSmmRuntimeDxe. Runtime Service calls to
GetNextVariableName() will no longer trigger a SW SMI.
Overall system performance and stability will be improved by
eliminating an SMI for these calls as they typically result in a
relatively large number of invocations to retrieve all variable names
in all variable stores present.
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe
.c | 118 +++++++++-----------
1 file changed, 50 insertions(+), 68 deletions(-)
diff --git
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD
xe.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD
xe.c
index 46f69765a4..bc3b56b0ce 100644
---
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD
xe.c
+++
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD
xe.c
@@ -799,87 +799,69 @@ RuntimeServiceGetNextVariableName (
IN OUT EFI_GUID *VendorGuid
)
{
- EFI_STATUS Status;
- UINTN PayloadSize;
- SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME
*SmmGetNextVariableName;
- UINTN OutVariableNameSize;
- UINTN InVariableNameSize;
+ EFI_STATUS Status;
+ UINTN DelayIndex;
+ UINTN MaxLen;
+ UINTN VarNameSize;
+ VARIABLE_HEADER *VariablePtr;
+ VARIABLE_STORE_HEADER
*VariableStoreHeader[VariableStoreTypeMax];
+
+ Status = EFI_NOT_FOUND;
if (VariableNameSize == NULL || VariableName == NULL || VendorGuid
==
NULL) {
return EFI_INVALID_PARAMETER;
}
- OutVariableNameSize = *VariableNameSize;
- InVariableNameSize = StrSize (VariableName);
- SmmGetNextVariableName = NULL;
-
//
- // If input string exceeds SMM payload limit. Return failure
+ // Calculate the possible maximum length of name string, including
+ the
Null terminator.
//
- if (InVariableNameSize > mVariableBufferPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name))
{
+ MaxLen = *VariableNameSize / sizeof (CHAR16); if ((MaxLen == 0) ||
+ (StrnLenS (VariableName, MaxLen) == MaxLen)) {
+ //
+ // Null-terminator is not found in the first VariableNameSize
+ bytes of the
input VariableName buffer,
+ // follow spec to return EFI_INVALID_PARAMETER.
+ //
return EFI_INVALID_PARAMETER;
}
- AcquireLockOnlyAtBootTime(&mVariableServicesLock);
+ AcquireLockOnlyAtBootTime (&mVariableServicesLock);
- //
- // Init the communicate buffer. The buffer data size is:
- // SMM_COMMUNICATE_HEADER_SIZE +
SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
- //
- if (OutVariableNameSize > mVariableBufferPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name))
{
- //
- // If output buffer exceed SMM payload limit. Trim output buffer to
SMM
payload size
- //
- OutVariableNameSize = mVariableBufferPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
+ for (DelayIndex = 0; mVariableRuntimeCacheReadLock && DelayIndex <
VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT; DelayIndex++) {
+ MicroSecondDelay (10);
}
- //
- // Payload should be Guid + NameSize + MAX of Input & Output buffer
- //
- PayloadSize = OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)
+ MAX
(OutVariableNameSize, InVariableNameSize);
-
- Status = InitCommunicateBuffer ((VOID
**)&SmmGetNextVariableName,
PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
- if (EFI_ERROR (Status)) {
- goto Done;
- }
- ASSERT (SmmGetNextVariableName != NULL);
-
- //
- // SMM comm buffer->NameSize is buffer size for return string
- //
- SmmGetNextVariableName->NameSize = OutVariableNameSize;
-
- CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);
- //
- // Copy whole string
- //
- CopyMem (SmmGetNextVariableName->Name, VariableName,
InVariableNameSize);
- if (OutVariableNameSize > InVariableNameSize) {
- ZeroMem ((UINT8 *) SmmGetNextVariableName->Name +
InVariableNameSize, OutVariableNameSize - InVariableNameSize);
- }
-
- //
- // Send data to SMM
- //
- Status = SendCommunicateBuffer (PayloadSize);
-
- //
- // Get data from SMM.
- //
- if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
- //
- // SMM CommBuffer NameSize can be a trimed value
- // Only update VariableNameSize when needed
- //
- *VariableNameSize = SmmGetNextVariableName->NameSize;
- }
- if (EFI_ERROR (Status)) {
- goto Done;
+ if (DelayIndex < VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT) {
+ ASSERT (!mVariableRuntimeCacheReadLock);
+
+ CheckForRuntimeCacheSync ();
+ mVariableRuntimeCacheReadLock = TRUE;
+
+ if (!mVariableRuntimeCachePendingUpdate) {
+ //
+ // 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;
+
+ Status = GetNextVariableEx (VariableName, VendorGuid,
VariableStoreHeader, &VariablePtr);
+ if (!EFI_ERROR (Status)) {
+ VarNameSize = NameSizeOfVariable (VariablePtr);
+ ASSERT (VarNameSize != 0);
+ if (VarNameSize <= *VariableNameSize) {
+ CopyMem (VariableName, GetVariableNamePtr (VariablePtr),
VarNameSize);
+ CopyMem (VendorGuid, GetVendorGuidPtr (VariablePtr), sizeof
(EFI_GUID));
+ Status = EFI_SUCCESS;
+ } else {
+ Status = EFI_BUFFER_TOO_SMALL;
+ }
+
+ *VariableNameSize = VarNameSize;
+ }
+ }
}
-
- CopyGuid (VendorGuid, &SmmGetNextVariableName->Guid);
- CopyMem (VariableName, SmmGetNextVariableName->Name,
SmmGetNextVariableName->NameSize);
-
-Done:
+ mVariableRuntimeCacheReadLock = FALSE;
Similar to the previous patch (7/9),
if timeout occurs when acquiring the read lock, should this flag be set to
FALSE in such case?
Best Regards,
Hao Wu
ReleaseLockOnlyAtBootTime (&mVariableServicesLock);
return Status;
}
--
2.16.2.windows.1