Hi Eric,

On 11/28/19 7:17 AM, Dong, Eric via Groups.Io wrote:
v2 changes:
   Minor update based on comments.

v1 changes:

Nitpick, previous comments should not go into the commit description, either on a cover letter, or after the '---' marker so they get stripped out by git-am.

Another nitpick, remove the trailing dot in commit subject.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388

Current logic allocate Token every time when need to use it. The logic
caused SMI latency raised to very high. Update logic to allocate Token
buffer at driver's entry point. Later use the token from the allocated
token buffer. Only when all the buffer have been used, then need to
allocate new buffer.

Signed-off-by: Eric Dong <eric.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
---
  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c      | 56 ++++++++++++++++++++--
  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16 +++++++
  2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index d8d2b6f444..4632e5b0c2 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -492,6 +492,23 @@ FreeTokens (
  {
    LIST_ENTRY            *Link;
    PROCEDURE_TOKEN       *ProcToken;
+  TOKEN_BUFFER          *TokenBuf;
+
+  //
+  // Not free the buffer, just clear the UsedTokenNum. In order to
+  // avoid later trig allcate action again when need to use token.

Maybe: "In order to avoid triggering allocate action when we need to use the token, do not free the buffer."

Patch looks good otherwise.

+  //
+  gSmmCpuPrivate->UsedTokenNum = 0;
+
+  Link = GetFirstNode (&gSmmCpuPrivate->OldTokenBufList);
+  while (!IsNull (&gSmmCpuPrivate->OldTokenBufList, Link)) {
+    TokenBuf = TOKEN_BUFFER_FROM_LINK (Link);
+
+    Link = RemoveEntryList (&TokenBuf->Link);
+
+    FreePool (TokenBuf->Buffer);
+    FreePool (TokenBuf);
+  }
while (!IsListEmpty (&gSmmCpuPrivate->TokenList)) {
      Link = GetFirstNode (&gSmmCpuPrivate->TokenList);
@@ -499,7 +516,6 @@ FreeTokens (
RemoveEntryList (&ProcToken->Link); - FreePool ((VOID *)ProcToken->ProcedureToken);
      FreePool (ProcToken);
    }
  }
@@ -1115,13 +1131,35 @@ CreateToken (
    VOID
    )
  {
-  PROCEDURE_TOKEN    *ProcToken;
+  PROCEDURE_TOKEN     *ProcToken;
    SPIN_LOCK           *CpuToken;
    UINTN               SpinLockSize;
+  TOKEN_BUFFER        *TokenBuf;
SpinLockSize = GetSpinLockProperties ();
-  CpuToken = AllocatePool (SpinLockSize);
-  ASSERT (CpuToken != NULL);
+
+  if (gSmmCpuPrivate->UsedTokenNum == MAX_PRE_RESERVE_TOKEN_COUNT) {
+    DEBUG((DEBUG_INFO, "CpuSmm: No free token buffer, allocate new 
buffer!\n"));
+
+    //
+    // Record current token buffer for later free action usage.
+    // Current used token buffer not in this list.
+    //
+    TokenBuf = AllocatePool (sizeof (TOKEN_BUFFER));
+    ASSERT (TokenBuf != NULL);
+    TokenBuf->Signature = TOKEN_BUFFER_SIGNATURE;
+    TokenBuf->Buffer  = gSmmCpuPrivate->CurrentTokenBuf;
+
+    InsertTailList (&gSmmCpuPrivate->OldTokenBufList, &TokenBuf->Link);
+
+    gSmmCpuPrivate->CurrentTokenBuf   = AllocateZeroPool (SpinLockSize * 
MAX_PRE_RESERVE_TOKEN_COUNT);
+    ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
+    gSmmCpuPrivate->UsedTokenNum = 0;
+  }
+
+  CpuToken = (SPIN_LOCK *)(gSmmCpuPrivate->CurrentTokenBuf + SpinLockSize * 
gSmmCpuPrivate->UsedTokenNum);
+  gSmmCpuPrivate->UsedTokenNum++;
+
    InitializeSpinLock (CpuToken);
    AcquireSpinLock (CpuToken);
@@ -1737,10 +1775,20 @@ InitializeDataForMmMp (
    VOID
    )
  {
+  UINTN              SpinLockSize;
+
+  SpinLockSize = GetSpinLockProperties ();
+  DEBUG((DEBUG_INFO, "CpuSmm: SpinLock Size = 0x%x\n", SpinLockSize));
+
+  gSmmCpuPrivate->CurrentTokenBuf = AllocateZeroPool (SpinLockSize * 
MAX_PRE_RESERVE_TOKEN_COUNT);
+  ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
+  gSmmCpuPrivate->UsedTokenNum = 0;
+
    gSmmCpuPrivate->ApWrapperFunc = AllocatePool (sizeof (PROCEDURE_WRAPPER) * 
gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
    ASSERT (gSmmCpuPrivate->ApWrapperFunc != NULL);
InitializeListHead (&gSmmCpuPrivate->TokenList);
+  InitializeListHead (&gSmmCpuPrivate->OldTokenBufList);
  }
/**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 8c29f1a558..36fd0dae92 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -198,6 +198,7 @@ typedef UINT32                              
SMM_CPU_ARRIVAL_EXCEPTIONS;
  #define ARRIVAL_EXCEPTION_DELAYED           0x2
  #define ARRIVAL_EXCEPTION_SMI_DISABLED      0x4
+#define MAX_PRE_RESERVE_TOKEN_COUNT 0x512
  //
  // Wrapper used to convert EFI_AP_PROCEDURE2 and EFI_AP_PROCEDURE.
  //
@@ -217,6 +218,17 @@ typedef struct {
#define PROCEDURE_TOKEN_FROM_LINK(a) CR (a, PROCEDURE_TOKEN, Link, PROCEDURE_TOKEN_SIGNATURE) +#define TOKEN_BUFFER_SIGNATURE SIGNATURE_32 ('T', 'K', 'B', 'S')
+
+typedef struct {
+  UINTN                   Signature;
+  LIST_ENTRY              Link;
+
+  UINT8                   *Buffer;
+} TOKEN_BUFFER;
+
+#define TOKEN_BUFFER_FROM_LINK(a)  CR (a, TOKEN_BUFFER, Link, 
TOKEN_BUFFER_SIGNATURE)
+
  //
  // Private structure for the SMM CPU module that is stored in DXE Runtime 
memory
  // Contains the SMM Configuration Protocols that is produced.
@@ -243,6 +255,10 @@ typedef struct {
    PROCEDURE_WRAPPER               *ApWrapperFunc;
    LIST_ENTRY                      TokenList;
+ LIST_ENTRY OldTokenBufList;
+
+  UINT8                           *CurrentTokenBuf;
+  UINTN                           UsedTokenNum;     // Only record tokens used 
in CurrentTokenBuf.
  } SMM_CPU_PRIVATE_DATA;
extern SMM_CPU_PRIVATE_DATA *gSmmCpuPrivate;



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51452): https://edk2.groups.io/g/devel/message/51452
Mute This Topic: https://groups.io/mt/63850169/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to