Hi all, Please ignore this version change which based on old codebase. Will send new change soon.
Thanks, Eric > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Dong, Eric > Sent: Wednesday, December 4, 2019 4:05 PM > To: devel@edk2.groups.io > Cc: Ni, Ray <ray...@intel.com>; Laszlo Ersek <ler...@redhat.com> > Subject: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid > allocate Token every time > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388 > > Token is new introduced by MM MP Protocol. 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> > --- > > V3 changes: > > Introduce PCD to control the pre allocated toke buffer size. > > > > v2 changes: > > Minor update based on comments. > > > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 67 > ++++++++++++++++++-- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 +++++ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + > UefiCpuPkg/UefiCpuPkg.dec | 4 ++ > UefiCpuPkg/UefiCpuPkg.uni | 1 + > 5 files changed, 84 insertions(+), 4 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index d8d2b6f444..33aad3f3e9 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -492,6 +492,24 @@ FreeTokens ( > { > > LIST_ENTRY *Link; > > PROCEDURE_TOKEN *ProcToken; > > + TOKEN_BUFFER *TokenBuf; > > + > > + // > > + // Not free the buffer, just clear the UsedTokenNum. In order to > > + // avoid triggering allocate action when we need to use the token, > > + // do not free the buffer. > > + // > > + 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 +517,6 @@ FreeTokens ( > > > RemoveEntryList (&ProcToken->Link); > > > > - FreePool ((VOID *)ProcToken->ProcedureToken); > > FreePool (ProcToken); > > } > > } > > @@ -1115,13 +1132,37 @@ CreateToken ( > VOID > > ) > > { > > - PROCEDURE_TOKEN *ProcToken; > > + PROCEDURE_TOKEN *ProcToken; > > SPIN_LOCK *CpuToken; > > UINTN SpinLockSize; > > + TOKEN_BUFFER *TokenBuf; > > + UINT32 TokenCountPerChunk; > > > > SpinLockSize = GetSpinLockProperties (); > > - CpuToken = AllocatePool (SpinLockSize); > > - ASSERT (CpuToken != NULL); > > + TokenCountPerChunk = PcdGet32 (PcdTokenCountPerChunk); > > + > > + if (gSmmCpuPrivate->UsedTokenNum == TokenCountPerChunk) { > > + DEBUG ((DEBUG_VERBOSE, "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 = AllocatePool (SpinLockSize * > TokenCountPerChunk); > > + ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL); > > + gSmmCpuPrivate->UsedTokenNum = 0; > > + } > > + > > + CpuToken = (SPIN_LOCK *)(gSmmCpuPrivate->CurrentTokenBuf + > SpinLockSize * gSmmCpuPrivate->UsedTokenNum); > > + gSmmCpuPrivate->UsedTokenNum++; > > + > > InitializeSpinLock (CpuToken); > > AcquireSpinLock (CpuToken); > > > > @@ -1737,10 +1778,28 @@ InitializeDataForMmMp ( > VOID > > ) > > { > > + UINTN SpinLockSize; > > + UINT32 TokenCountPerChunk; > > + > > + SpinLockSize = GetSpinLockProperties (); > > + TokenCountPerChunk = PcdGet32 (PcdTokenCountPerChunk); > > + ASSERT_EFI_ERROR (TokenCountPerChunk != 0); > > + if (TokenCountPerChunk == 0) { > > + DEBUG ((EFI_D_ERROR, "PcdTokenCountPerChunk should not be > Zero!\n")); > > + CpuDeadLoop (); > > + } > > + DEBUG ((DEBUG_INFO, "CpuSmm: SpinLock Size = 0x%x, > PreAllocateTokenNum = 0x%x\n", SpinLockSize, TokenCountPerChunk)); > > + > > + gSmmCpuPrivate->CurrentTokenBuf = AllocatePool (SpinLockSize * > TokenCountPerChunk); > > + 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..08ef8d2e15 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -217,6 +217,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 +254,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; > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > index 851a8cb258..8b6c71b697 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > @@ -140,6 +140,7 @@ > > gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask > ## CONSUMES > > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask > ## CONSUMES > > gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask > ## CONSUMES > > + gUefiCpuPkgTokenSpaceGuid.PcdTokenCountPerChunk ## > CONSUMES > > > > [Depex] > > gEfiMpServiceProtocolGuid > > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec > index 83acd33612..8ec2153459 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dec > +++ b/UefiCpuPkg/UefiCpuPkg.dec > @@ -147,6 +147,10 @@ > # @Prompt Specify size of good stack of exception which need switching > stack. > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize|2048|UINT32|0 > x30002001 > > > > + ## Size of pre allocated token count per chunk. > > + # @Prompt Specify the size of pre allocated token count per chunk. > > + > gUefiCpuPkgTokenSpaceGuid.PcdTokenCountPerChunk|64|UINT32|0x3000 > 2002 > > + > > [PcdsFixedAtBuild, PcdsPatchableInModule] > > ## This value is the CPU Local APIC base address, which aligns the address > on a 4-KByte boundary. > > # @Prompt Configure base address of CPU Local APIC > > diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni > index fbf7680726..3bb951cc72 100644 > --- a/UefiCpuPkg/UefiCpuPkg.uni > +++ b/UefiCpuPkg/UefiCpuPkg.uni > @@ -252,3 +252,4 @@ > > "24000000 - 6th and 7th > generation Intel Core processors and Intel Xeon W Processor > Family(24MHz).<BR>\n" > > > "19200000 - Intel Atom > processors based on Goldmont Microarchitecture with CPUID signature > 06_5CH(19.2MHz).<BR>\n" > > > > +#string > STR_gUefiCpuPkgTokenSpaceGuid_PcdTokenCountPerChunk_PROMPT > #language en-US "Specify the size of pre allocated token count per chunk.\n" > \ No newline at end of file > -- > 2.23.0.windows.1 > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#51721): https://edk2.groups.io/g/devel/message/51721 > Mute This Topic: https://groups.io/mt/66393846/1768733 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [eric.d...@intel.com] > -=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#51725): https://edk2.groups.io/g/devel/message/51725 Mute This Topic: https://groups.io/mt/66393846/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
