From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web12.1317.1575449771972221233 for ; Wed, 04 Dec 2019 00:56:12 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: eric.dong@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Dec 2019 00:56:11 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,276,1571727600"; d="scan'208";a="213711332" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga006.jf.intel.com with ESMTP; 04 Dec 2019 00:56:11 -0800 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 4 Dec 2019 00:56:10 -0800 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 4 Dec 2019 00:56:10 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.109]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.71]) with mapi id 14.03.0439.000; Wed, 4 Dec 2019 16:56:08 +0800 From: "Dong, Eric" To: "devel@edk2.groups.io" , "Dong, Eric" CC: "Ni, Ray" , Laszlo Ersek Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate Token every time Thread-Topic: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate Token every time Thread-Index: AQHVqnmtnwaJLIU06E+w5cRWR3xBc6eprCig Date: Wed, 4 Dec 2019 08:56:08 +0000 Message-ID: References: <15DD1C76ED7F5B8C.17294@groups.io> In-Reply-To: <15DD1C76ED7F5B8C.17294@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi all, Please ignore this version change which based on old codebase. Will send ne= w 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 ; Laszlo Ersek > Subject: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid > allocate Token every time >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2388 >=20 > 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. >=20 > Signed-off-by: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > --- >=20 > V3 changes: >=20 > Introduce PCD to control the pre allocated toke buffer size. >=20 >=20 >=20 > v2 changes: >=20 > Minor update based on comments. >=20 >=20 > 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(-) >=20 > 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 ( > { >=20 > LIST_ENTRY *Link; >=20 > PROCEDURE_TOKEN *ProcToken; >=20 > + TOKEN_BUFFER *TokenBuf; >=20 > + >=20 > + // >=20 > + // Not free the buffer, just clear the UsedTokenNum. In order to >=20 > + // avoid triggering allocate action when we need to use the token, >=20 > + // do not free the buffer. >=20 > + // >=20 > + gSmmCpuPrivate->UsedTokenNum =3D 0; >=20 > + >=20 > + Link =3D GetFirstNode (&gSmmCpuPrivate->OldTokenBufList); >=20 > + while (!IsNull (&gSmmCpuPrivate->OldTokenBufList, Link)) { >=20 > + TokenBuf =3D TOKEN_BUFFER_FROM_LINK (Link); >=20 > + >=20 > + Link =3D RemoveEntryList (&TokenBuf->Link); >=20 > + >=20 > + FreePool (TokenBuf->Buffer); >=20 > + FreePool (TokenBuf); >=20 > + } >=20 >=20 >=20 > while (!IsListEmpty (&gSmmCpuPrivate->TokenList)) { >=20 > Link =3D GetFirstNode (&gSmmCpuPrivate->TokenList); >=20 > @@ -499,7 +517,6 @@ FreeTokens ( >=20 >=20 > RemoveEntryList (&ProcToken->Link); >=20 >=20 >=20 > - FreePool ((VOID *)ProcToken->ProcedureToken); >=20 > FreePool (ProcToken); >=20 > } >=20 > } >=20 > @@ -1115,13 +1132,37 @@ CreateToken ( > VOID >=20 > ) >=20 > { >=20 > - PROCEDURE_TOKEN *ProcToken; >=20 > + PROCEDURE_TOKEN *ProcToken; >=20 > SPIN_LOCK *CpuToken; >=20 > UINTN SpinLockSize; >=20 > + TOKEN_BUFFER *TokenBuf; >=20 > + UINT32 TokenCountPerChunk; >=20 >=20 >=20 > SpinLockSize =3D GetSpinLockProperties (); >=20 > - CpuToken =3D AllocatePool (SpinLockSize); >=20 > - ASSERT (CpuToken !=3D NULL); >=20 > + TokenCountPerChunk =3D PcdGet32 (PcdTokenCountPerChunk); >=20 > + >=20 > + if (gSmmCpuPrivate->UsedTokenNum =3D=3D TokenCountPerChunk) { >=20 > + DEBUG ((DEBUG_VERBOSE, "CpuSmm: No free token buffer, allocate > new buffer!\n")); >=20 > + >=20 > + // >=20 > + // Record current token buffer for later free action usage. >=20 > + // Current used token buffer not in this list. >=20 > + // >=20 > + TokenBuf =3D AllocatePool (sizeof (TOKEN_BUFFER)); >=20 > + ASSERT (TokenBuf !=3D NULL); >=20 > + TokenBuf->Signature =3D TOKEN_BUFFER_SIGNATURE; >=20 > + TokenBuf->Buffer =3D gSmmCpuPrivate->CurrentTokenBuf; >=20 > + >=20 > + InsertTailList (&gSmmCpuPrivate->OldTokenBufList, &TokenBuf->Link); >=20 > + >=20 > + gSmmCpuPrivate->CurrentTokenBuf =3D AllocatePool (SpinLockSize * > TokenCountPerChunk); >=20 > + ASSERT (gSmmCpuPrivate->CurrentTokenBuf !=3D NULL); >=20 > + gSmmCpuPrivate->UsedTokenNum =3D 0; >=20 > + } >=20 > + >=20 > + CpuToken =3D (SPIN_LOCK *)(gSmmCpuPrivate->CurrentTokenBuf + > SpinLockSize * gSmmCpuPrivate->UsedTokenNum); >=20 > + gSmmCpuPrivate->UsedTokenNum++; >=20 > + >=20 > InitializeSpinLock (CpuToken); >=20 > AcquireSpinLock (CpuToken); >=20 >=20 >=20 > @@ -1737,10 +1778,28 @@ InitializeDataForMmMp ( > VOID >=20 > ) >=20 > { >=20 > + UINTN SpinLockSize; >=20 > + UINT32 TokenCountPerChunk; >=20 > + >=20 > + SpinLockSize =3D GetSpinLockProperties (); >=20 > + TokenCountPerChunk =3D PcdGet32 (PcdTokenCountPerChunk); >=20 > + ASSERT_EFI_ERROR (TokenCountPerChunk !=3D 0); >=20 > + if (TokenCountPerChunk =3D=3D 0) { >=20 > + DEBUG ((EFI_D_ERROR, "PcdTokenCountPerChunk should not be > Zero!\n")); >=20 > + CpuDeadLoop (); >=20 > + } >=20 > + DEBUG ((DEBUG_INFO, "CpuSmm: SpinLock Size =3D 0x%x, > PreAllocateTokenNum =3D 0x%x\n", SpinLockSize, TokenCountPerChunk)); >=20 > + >=20 > + gSmmCpuPrivate->CurrentTokenBuf =3D AllocatePool (SpinLockSize * > TokenCountPerChunk); >=20 > + ASSERT (gSmmCpuPrivate->CurrentTokenBuf !=3D NULL); >=20 > + >=20 > + gSmmCpuPrivate->UsedTokenNum =3D 0; >=20 > + >=20 > gSmmCpuPrivate->ApWrapperFunc =3D AllocatePool (sizeof > (PROCEDURE_WRAPPER) * gSmmCpuPrivate- > >SmmCoreEntryContext.NumberOfCpus); >=20 > ASSERT (gSmmCpuPrivate->ApWrapperFunc !=3D NULL); >=20 >=20 >=20 > InitializeListHead (&gSmmCpuPrivate->TokenList); >=20 > + InitializeListHead (&gSmmCpuPrivate->OldTokenBufList); >=20 > } >=20 >=20 >=20 > /** >=20 > 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 { >=20 >=20 > #define PROCEDURE_TOKEN_FROM_LINK(a) CR (a, PROCEDURE_TOKEN, > Link, PROCEDURE_TOKEN_SIGNATURE) >=20 >=20 >=20 > +#define TOKEN_BUFFER_SIGNATURE SIGNATURE_32 ('T', 'K', 'B', 'S') >=20 > + >=20 > +typedef struct { >=20 > + UINTN Signature; >=20 > + LIST_ENTRY Link; >=20 > + >=20 > + UINT8 *Buffer; >=20 > +} TOKEN_BUFFER; >=20 > + >=20 > +#define TOKEN_BUFFER_FROM_LINK(a) CR (a, TOKEN_BUFFER, Link, > TOKEN_BUFFER_SIGNATURE) >=20 > + >=20 > // >=20 > // Private structure for the SMM CPU module that is stored in DXE Runtim= e > memory >=20 > // Contains the SMM Configuration Protocols that is produced. >=20 > @@ -243,6 +254,10 @@ typedef struct { > PROCEDURE_WRAPPER *ApWrapperFunc; >=20 > LIST_ENTRY TokenList; >=20 >=20 >=20 > + LIST_ENTRY OldTokenBufList; >=20 > + >=20 > + UINT8 *CurrentTokenBuf; >=20 > + UINTN UsedTokenNum; // Only record token= s used in > CurrentTokenBuf. >=20 > } SMM_CPU_PRIVATE_DATA; >=20 >=20 >=20 > extern SMM_CPU_PRIVATE_DATA *gSmmCpuPrivate; >=20 > 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 @@ >=20 > gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask > ## CONSUMES >=20 > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask > ## CONSUMES >=20 > gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask > ## CONSUMES >=20 > + gUefiCpuPkgTokenSpaceGuid.PcdTokenCountPerChunk = ## > CONSUMES >=20 >=20 >=20 > [Depex] >=20 > gEfiMpServiceProtocolGuid >=20 > 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. >=20 >=20 > gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize|2048|UINT32|0 > x30002001 >=20 >=20 >=20 > + ## Size of pre allocated token count per chunk. >=20 > + # @Prompt Specify the size of pre allocated token count per chunk. >=20 > + > gUefiCpuPkgTokenSpaceGuid.PcdTokenCountPerChunk|64|UINT32|0x3000 > 2002 >=20 > + >=20 > [PcdsFixedAtBuild, PcdsPatchableInModule] >=20 > ## This value is the CPU Local APIC base address, which aligns the add= ress > on a 4-KByte boundary. >=20 > # @Prompt Configure base address of CPU Local APIC >=20 > 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).
\n" >=20 > = "19200000 - Intel Atom > processors based on Goldmont Microarchitecture with CPUID signature > 06_5CH(19.2MHz).
\n" >=20 >=20 >=20 > +#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 >=20 >=20 > -=3D-=3D-=3D-=3D-=3D-=3D > Groups.io Links: You receive all messages sent to this group. >=20 > View/Reply Online (#51721): https://edk2.groups.io/g/devel/message/51721 > Mute This Topic: https://groups.io/mt/66393846/1768733 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [eric.dong@intel.com] > -=3D-=3D-=3D-=3D-=3D-=3D