From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.6172.1574946941158570747 for ; Thu, 28 Nov 2019 05:15:41 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RJQg9jro; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: philmd@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574946940; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nPf67NfgH8FGgTCpDUctB8ghJaRwkt/DYryUnRKnW0c=; b=RJQg9jroYebCWanEamWuqO+1xvQlcSAmwcl85d2VVmRWkfhroMIrDkiG3BSpj7AjPVqSGC byMT5cBmj3j3sMdL20dVYPVO+j+NhHrmBwMUxesQgEFcJw/6jhkcF812lKccYmP2OlTENU 62zMtFSAsyT+5YxqTGsDRC0IgTecle4= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-78-hZTj_AR6NqKBvBMuPxe1mg-1; Thu, 28 Nov 2019 08:15:37 -0500 Received: by mail-wr1-f71.google.com with SMTP id b2so12478312wrj.9 for ; Thu, 28 Nov 2019 05:15:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=nPf67NfgH8FGgTCpDUctB8ghJaRwkt/DYryUnRKnW0c=; b=AdZBs7vTq2Jrlp/rwl7b6bYOD7kZytWMrMLqnGgOhmoyi2YNFnWHkxKhtKCtCAVvPi w3KFg2h5tNoniXhzyS0xC0MxWUtyL3/4UWQeVqhLS/FVf5DYQ3rBkM7TIfOp6tMnsuUk 88Bt1IW2HrjrCMZv+mVFEqWLNZcxMXi/WY7UB/+6yHxid5Rtl0yuor5WjsH9+OPJlmU7 PpxX6ErndPzVmKQpWj/wvJi4EQ0lpzHo5t7psL0++bdbLFWh34R7sGxLXh614UoDVD8f FuYaESciCstLohBcMl8fVQZp7fkmc6DHd+CDzcTyfFjzf8ooHQQb4xYNsuLvBvqr03nR Mwug== X-Gm-Message-State: APjAAAXb5uzXGCwoKDSIUh9A7P8Pnue+MgcliO3+EQX6to4wxH67JWvW WZEEc4lTXHqDuasZ5hMulEXxpPRvInJ2ZxlDLi+ttippXpLoTTD6zZdtvmpYyFdcw9rgIPzIl88 /twD/gNm5W9o49A== X-Received: by 2002:adf:ec48:: with SMTP id w8mr294643wrn.19.1574946936489; Thu, 28 Nov 2019 05:15:36 -0800 (PST) X-Google-Smtp-Source: APXvYqwfO79RmsxD2Ick8j4R1vyJQgl++CofIzv4/qx/Xo0CjukaEnBoaGPZftWSDFwMFIYr1ytsNw== X-Received: by 2002:adf:ec48:: with SMTP id w8mr294608wrn.19.1574946936205; Thu, 28 Nov 2019 05:15:36 -0800 (PST) Return-Path: Received: from [192.168.1.35] (182.red-88-21-103.staticip.rima-tde.net. [88.21.103.182]) by smtp.gmail.com with ESMTPSA id l26sm10464192wmj.48.2019.11.28.05.15.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 28 Nov 2019 05:15:35 -0800 (PST) Subject: Re: [edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate Token every time. To: devel@edk2.groups.io, eric.dong@intel.com Cc: Ray Ni , Laszlo Ersek References: <20191128061716.196-1-eric.dong@intel.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Message-ID: <515bc9a6-106f-4e8d-c5da-e8197990fe13@redhat.com> Date: Thu, 28 Nov 2019 14:15:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 MIME-Version: 1.0 In-Reply-To: <20191128061716.196-1-eric.dong@intel.com> X-MC-Unique: hZTj_AR6NqKBvBMuPxe1mg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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 > Cc: Ray Ni > Cc: Laszlo Ersek > --- > 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; >