public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kubacki, Michael A" <michael.a.kubacki@intel.com>
To: devel@edk2.groups.io
Cc: Dandan Bi <dandan.bi@intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Eric Dong <eric.dong@intel.com>, Laszlo Ersek <lersek@redhat.com>,
	Liming Gao <liming.gao@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Ray Ni <ray.ni@intel.com>, Jian J Wang <jian.j.wang@intel.com>,
	Hao A Wu <hao.a.wu@intel.com>, Jiewen Yao <jiewen.yao@intel.com>
Subject: [PATCH V1 4/5] MdeModulePkg/Variable: Add RT GetNextVariableName() cache support
Date: Wed, 25 Sep 2019 21:50:45 -0700	[thread overview]
Message-ID: <20190926045046.34592-5-michael.a.kubacki@intel.com> (raw)
In-Reply-To: <20190926045046.34592-1-michael.a.kubacki@intel.com>

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/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 237908e5a2..ff3c390a7d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.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;
   ReleaseLockOnlyAtBootTime (&mVariableServicesLock);
   return Status;
 }
-- 
2.16.2.windows.1


  parent reply	other threads:[~2019-09-26  4:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-26  4:50 [PATCH V1 0/5] UEFI Variable SMI Reduction Kubacki, Michael A
2019-09-26  4:50 ` [PATCH V1 1/5] MdeModulePkg/Variable: Consolidate common parsing functions Kubacki, Michael A
2019-09-27  8:17   ` [edk2-devel] " Wu, Hao A
2019-09-27 17:31     ` Kubacki, Michael A
2019-09-26  4:50 ` [PATCH V1 2/5] MdeModulePkg VariableInfo: Always consider RT DXE and SMM stats Kubacki, Michael A
2019-09-26  4:50 ` [PATCH V1 3/5] MdeModulePkg/Variable: Add RT GetVariable() cache support Kubacki, Michael A
2019-09-26  4:50 ` Kubacki, Michael A [this message]
2019-09-26  4:50 ` [PATCH V1 5/5] MdeModulePkg/VariableSmm: Remove unused SMI handler functions Kubacki, Michael A
2019-09-26 18:23 ` [PATCH V1 0/5] UEFI Variable SMI Reduction Laszlo Ersek
2019-09-26 20:29   ` Kubacki, Michael A
2019-09-26 22:35     ` Kubacki, Michael A
2019-09-30 22:43       ` Laszlo Ersek
2019-09-30 22:47         ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190926045046.34592-5-michael.a.kubacki@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox