From: Fan Jeff <vanjeff_919@hotmail.com>
To: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
"Dong, Eric" <eric.dong@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: 答复: [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for AP initialization logic.
Date: Tue, 24 Oct 2017 07:32:27 +0000 [thread overview]
Message-ID: <CY1PR19MB02830BE0107B6571066D8C7DD7470@CY1PR19MB0283.namprd19.prod.outlook.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BAA36D7@SHSMSX104.ccr.corp.intel.com>
Sure. Done.
发件人: Ni, Ruiyu<mailto:ruiyu.ni@intel.com>
发送时间: 2017年10月24日 15:18
收件人: Fan Jeff<mailto:vanjeff_919@hotmail.com>; Dong, Eric<mailto:eric.dong@intel.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
主题: RE: [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for AP initialization logic.
Jeff,
I see. Thanks for mentioning that😊
Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com>
Do you mind to also give a r-b?
Thanks/Ray
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Fan Jeff
> Sent: Tuesday, October 24, 2017 2:27 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; edk2-
> devel@lists.01.org
> Subject: [edk2] 答复: [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for
> AP initialization logic.
>
> Ray,
>
> MpCpuExchangeInfo was declared as volatile type. It may not be necessary
> to add volatile for NumApsExecuting.
>
> Jeff
>
> 发件人: Ni, Ruiyu<mailto:ruiyu.ni@intel.com>
> 发送时间: 2017年10月24日 14:03
> 收件人: Dong, Eric<mailto:eric.dong@intel.com>; edk2-
> devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> 主题: Re: [edk2] [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for AP
> initialization logic.
>
> You need to have "volatile" for "UINTN NumApsExecuting;".
> Otherwise, compiler may optimize the code to cause below code wait
> forever:
> while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting != 0) {
> CpuPause();
> }
>
>
> Thanks/Ray
>
> > -----Original Message-----
> > From: Dong, Eric
> > Sent: Monday, October 23, 2017 3:23 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for AP
> > initialization logic.
> >
> > Current logic always waiting for a specific value to collect all APs
> > count. This logic may caused some platforms cost too much time to wait for
> time out.
> > This patch add new logic to collect APs count. It adds new variable
> > NumApsExecuting to detect whether all APs have finished initialization.
> > Each AP let NumApsExecuting++ when begin to initialize itself and let
> > NumApsExecuting-- when it finish the initialization. BSP base on
> > whether NumApsExecuting == 0 to finished the collect AP process.
> >
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > ---
> > UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 1 +
> > UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 6 ++++++
> > UefiCpuPkg/Library/MpInitLib/MpLib.c | 20 ++++++++++++++------
> > UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 +
> > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 3 ++-
> > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 6 ++++++
> > 6 files changed, 30 insertions(+), 7 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> > b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> > index 976af1f..bdfe0d3 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> > @@ -40,4 +40,5 @@ EnableExecuteDisableLocation equ LockLocation +
> > 30h
> > Cr3Location equ LockLocation + 34h
> > InitFlagLocation equ LockLocation + 38h
> > CpuInfoLocation equ LockLocation + 3Ch
> > +NumApsExecutingLocation equ LockLocation + 40h
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> > b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> > index 1b9c6a6..2b6c27d 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> > @@ -86,6 +86,12 @@ Flat32Start: ; protected mode entry
> > point
> >
> > mov esi, ebx
> >
> > + ; Increment the number of APs executing here as early as possible
> > + ; This is decremented in C code when AP is finished executing
> > + mov edi, esi
> > + add edi, NumApsExecutingLocation
> > + lock inc dword [edi]
> > +
> > mov edi, esi
> > add edi, EnableExecuteDisableLocation
> > cmp byte [edi], 0
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index db923c9..48f930b 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -662,6 +662,7 @@ ApWakeupFunction (
> > // AP finished executing C code
> > //
> > InterlockedIncrement ((UINT32 *) &CpuMpData->FinishedCount);
> > + InterlockedDecrement ((UINT32 *)
> > + &CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
> >
> > //
> > // Place AP is specified loop mode @@ -765,6 +766,7 @@
> > FillExchangeInfoData (
> >
> > ExchangeInfo->CFunction = (UINTN) ApWakeupFunction;
> > ExchangeInfo->ApIndex = 0;
> > + ExchangeInfo->NumApsExecuting = 0;
> > ExchangeInfo->InitFlag = (UINTN) CpuMpData->InitFlag;
> > ExchangeInfo->CpuInfo = (CPU_INFO_IN_HOB *) (UINTN)
> CpuMpData-
> > >CpuInfoInHob;
> > ExchangeInfo->CpuMpData = CpuMpData;
> > @@ -934,13 +936,19 @@ WakeUpAP (
> > }
> > if (CpuMpData->InitFlag == ApInitConfig) {
> > //
> > - // Wait for all potential APs waken up in one specified period
> > + // Wait for one potential AP waken up in one specified period
> > //
> > - TimedWaitForApFinish (
> > - CpuMpData,
> > - PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,
> > - PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)
> > - );
> > + if (CpuMpData->CpuCount == 0) {
> > + TimedWaitForApFinish (
> > + CpuMpData,
> > + PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,
> > + PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)
> > + );
> > + }
> > +
> > + while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting != 0) {
> > + CpuPause();
> > + }
> > } else {
> > //
> > // Wait all APs waken up if this is not the 1st broadcast of
> > SIPI diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > index e41d2db..d13d5c0 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > @@ -176,6 +176,7 @@ typedef struct {
> > UINTN Cr3;
> > UINTN InitFlag;
> > CPU_INFO_IN_HOB *CpuInfo;
> > + UINTN NumApsExecuting;
> > CPU_MP_DATA *CpuMpData;
> > UINTN InitializeFloatingPointUnitsAddress;
> > } MP_CPU_EXCHANGE_INFO;
> > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> > b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> > index 114f4e0..d255ca5 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> > @@ -40,5 +40,6 @@ EnableExecuteDisableLocation equ LockLocation +
> > 5Ch
> > Cr3Location equ LockLocation + 64h
> > InitFlagLocation equ LockLocation + 6Ch
> > CpuInfoLocation equ LockLocation + 74h
> > -InitializeFloatingPointUnitsAddress equ LockLocation + 84h
> > +NumApsExecutingLocation equ LockLocation + 7Ch
> > +InitializeFloatingPointUnitsAddress equ LockLocation + 8Ch
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > index 4ada649..21d2786 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > @@ -124,6 +124,12 @@ LongModeStart:
> > cmp qword [edi], 1 ; ApInitConfig
> > jnz GetApicId
> >
> > + ; Increment the number of APs executing here as early as possible
> > + ; This is decremented in C code when AP is finished executing
> > + mov edi, esi
> > + add edi, NumApsExecutingLocation
> > + lock inc dword [edi]
> > +
> > ; AP init
> > mov edi, esi
> > add edi, LockLocation
> > --
> > 2.7.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2017-10-24 7:28 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-23 7:22 [Patch 0/2] Enhance collect AP Count logic Eric Dong
2017-10-23 7:22 ` [Patch 1/2] UefiCpuPkg/MpInitLib: Change AP Index variable name Eric Dong
2017-10-24 6:00 ` Ni, Ruiyu
2017-10-23 7:22 ` [Patch 2/2] UefiCpuPkg/MpInitLib: Enhance waiting for AP initialization logic Eric Dong
2017-10-24 6:02 ` Ni, Ruiyu
2017-10-24 6:27 ` 答复: " Fan Jeff
2017-10-24 7:18 ` Ni, Ruiyu
2017-10-24 7:32 ` Fan Jeff [this message]
2017-10-24 10:15 ` Laszlo Ersek
2017-10-24 14:24 ` 答复: " Fan Jeff
2017-10-24 16:29 ` Laszlo Ersek
2017-10-24 15:23 ` Dong, Eric
2017-10-24 15:40 ` Dong, Eric
2017-10-24 17:40 ` Laszlo Ersek
2017-10-24 22:30 ` Brian J. Johnson
2017-10-25 5:35 ` 答复: " Fan Jeff
2017-10-25 5:32 ` Fan Jeff
2017-10-25 5:42 ` Dong, Eric
2017-10-25 15:07 ` Laszlo Ersek
2017-10-26 1:13 ` Dong, Eric
2017-10-26 20:48 ` Brian J. Johnson
2017-10-27 1:31 ` Dong, Eric
2017-10-24 7:31 ` 答复: [Patch 0/2] Enhance collect AP Count logic Fan Jeff
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=CY1PR19MB02830BE0107B6571066D8C7DD7470@CY1PR19MB0283.namprd19.prod.outlook.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