From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web12.399.1570582631749649046 for ; Tue, 08 Oct 2019 17:57:11 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: eric.dong@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Oct 2019 17:57:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,273,1566889200"; d="scan'208";a="394860021" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga006.fm.intel.com with ESMTP; 08 Oct 2019 17:57:11 -0700 Received: from fmsmsx163.amr.corp.intel.com (10.18.125.72) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 8 Oct 2019 17:57:11 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx163.amr.corp.intel.com (10.18.125.72) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 8 Oct 2019 17:57:10 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.176]) by SHSMSX151.ccr.corp.intel.com ([10.239.6.50]) with mapi id 14.03.0439.000; Wed, 9 Oct 2019 08:57:09 +0800 From: "Dong, Eric" To: "devel@edk2.groups.io" , "lersek@redhat.com" CC: Igor Mammedov , "Ni, Ray" Subject: Re: [edk2-devel] [PATCH 1/4] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection Thread-Topic: [edk2-devel] [PATCH 1/4] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection Thread-Index: AQHVfctcC6uYn5sgXky2skUovv/r2adRe9Mg Date: Wed, 9 Oct 2019 00:57:09 +0000 Message-ID: References: <20191008112714.6215-1-lersek@redhat.com> <20191008112714.6215-2-lersek@redhat.com> In-Reply-To: <20191008112714.6215-2-lersek@redhat.com> 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 Laszlo, > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Laszlo Ersek > Sent: Tuesday, October 8, 2019 7:27 PM > To: edk2-devel-groups-io > Cc: Dong, Eric ; Igor Mammedov > ; Ni, Ray > Subject: [edk2-devel] [PATCH 1/4] UefiCpuPkg/MpInitLib: honor the platfor= m's > boot CPU count in AP detection >=20 > If a platform boots with a CPU topology that is not fully populated (that > is, the boot CPU count is smaller than PcdCpuMaxLogicalProcessorNumber), > then the platform cannot use the "fast AP detection" logic added in commi= t > 6e1987f19af7. Said logic depends on the boot CPU count being equal to > PcdCpuMaxLogicalProcessorNumber. >=20 > The platform may not be able to use the variant added in commit > 0594ec417c89 either, for different reasons; see for example commit > 861218740d6d. >=20 > Allow platforms to specify the exact boot CPU count, independently of > PcdCpuMaxLogicalProcessorNumber. >=20 > Cc: Eric Dong > Cc: Igor Mammedov > Cc: Ray Ni > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1515 > Signed-off-by: Laszlo Ersek > --- > UefiCpuPkg/UefiCpuPkg.dec | 11 +++++ > UefiCpuPkg/UefiCpuPkg.uni | 4 ++ > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 3 +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 42 ++++++++++++-------- > 5 files changed, 43 insertions(+), 18 deletions(-) >=20 > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec > index 031a2ccd680a..d6b33fd9b465 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dec > +++ b/UefiCpuPkg/UefiCpuPkg.dec > @@ -227,6 +227,17 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, > PcdsDynamic, PcdsDynamicEx] > ## Specifies timeout value in microseconds for the BSP to detect all A= Ps for > the first time. > # @Prompt Timeout for the BSP to detect all APs for the first time. >=20 > gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000|UI > NT32|0x00000004 > + ## Specifies the number of Logical Processors that are available in th= e > + # preboot environment after platform reset, including BSP and APs. Po= ssible > + # values:

> + # zero (default) - This PCD is ignored, and > + # PcdCpuApInitTimeOutInMicroSeconds limits the initi= al AP > + # detection by the BSP.
> + # nonzero - PcdCpuApInitTimeOutInMicroSeconds is ignored. The = initial > + # AP detection finishes when the detected CPU count = (BSP > + # plus APs) reaches the value of this PCD.
> + # @Prompt Number of Logical Processors available after platform reset. > + > gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0|UINT32 > |0x00000008 > ## Specifies the base address of the first microcode Patch in the micr= ocode > Region. > # @Prompt Microcode Region base address. >=20 > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64|0x > 00000005 > diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni > index fbf768072668..a7e279c5cb14 100644 > --- a/UefiCpuPkg/UefiCpuPkg.uni > +++ b/UefiCpuPkg/UefiCpuPkg.uni > @@ -37,6 +37,10 @@ >=20 > #string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApInitTimeOutInMicroSeconds_HEL > P #language en-US "Specifies timeout value in microseconds for the BSP t= o > detect all APs for the first time." >=20 > +#string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_PRO > MPT #language en-US "Number of Logical Processors available after platfo= rm > reset." > + > +#string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_HELP > #language en-US "Specifies the number of Logical Processors that are avai= lable > in the preboot environment after platform reset, including BSP and APs." > + > #string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuMicrocodePatchAddress_PROMPT > #language en-US "Microcode Region base address." >=20 > #string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuMicrocodePatchAddress_HELP > #language en-US "Specifies the base address of the first microcode Patch = in > the microcode Region." > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > index 37b3f64e578a..cd912ab0c5ee 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > @@ -61,6 +61,7 @@ [Guids] >=20 > [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## > CONSUMES > + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## > SOMETIMES_CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CO= NSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## > CONSUMES > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > index 82b77b63ea87..1538185ef99a 100644 > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > @@ -53,7 +53,8 @@ [LibraryClasses] >=20 > [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## > CONSUMES > - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## > CONSUMES > + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber ## > CONSUMES > + gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## > SOMETIMES_CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CO= NSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## > CONSUMES > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index d6f84c6f45c0..f1bf8a7ba7cf 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1044,24 +1044,32 @@ WakeUpAP ( > SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferSta= rt); > } > if (CpuMpData->InitFlag =3D=3D ApInitConfig) { > - // > - // Here support two methods to collect AP count through adjust > - // PcdCpuApInitTimeOutInMicroSeconds values. > - // > - // one way is set a value to just let the first AP to start the > - // initialization, then through the later while loop to wait all A= ps > - // finsh the initialization. > - // The other way is set a value to let all APs finished the initia= lzation. > - // In this case, the later while loop is useless. > - // > - TimedWaitForApFinish ( > - CpuMpData, > - PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, > - PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) > - ); > + if (PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) { > + TimedWaitForApFinish ( > + CpuMpData, > + PcdGet32 (PcdCpuBootLogicalProcessorNumber) - 1, > + MAX_UINT32 // approx. 71 minutes Do you think if the PcdCpuBootLogicalProcessorNumber is not zero, driver sh= ould always wait for all the processors specified by PcdCpuBootLogicalProce= ssorNumber ready? I just have little concern about the MAX_UINT32 usage. I = not sure whether there has a case that some processors can't wake up? Thanks, Eric > + ); > + } else { > + // > + // Here support two methods to collect AP count through adjust > + // PcdCpuApInitTimeOutInMicroSeconds values. > + // > + // one way is set a value to just let the first AP to start the > + // initialization, then through the later while loop to wait all= Aps > + // finsh the initialization. > + // The other way is set a value to let all APs finished the > + // initialzation. In this case, the later while loop is useless. > + // > + TimedWaitForApFinish ( > + CpuMpData, > + PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, > + PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) > + ); >=20 > - while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting !=3D 0) { > - CpuPause(); > + while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting !=3D 0) { > + CpuPause(); > + } > } > } else { > // > -- > 2.19.1.3.g30247aa5d201 >=20 >=20 >=20 > -=3D-=3D-=3D-=3D-=3D-=3D > Groups.io Links: You receive all messages sent to this group. >=20 > View/Reply Online (#48563): https://edk2.groups.io/g/devel/message/48563 > Mute This Topic: https://groups.io/mt/34441228/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