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 <devel@edk2.groups.io>
> Cc: Dong, Eric <eric.d...@intel.com>; Igor Mammedov
> <imamm...@redhat.com>; Ni, Ray <ray...@intel.com>
> Subject: [edk2-devel] [PATCH 1/4] UefiCpuPkg/MpInitLib: honor the platform's
> boot CPU count in AP detection
>
> 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 commit
> 6e1987f19af7. Said logic depends on the boot CPU count being equal to
> PcdCpuMaxLogicalProcessorNumber.
>
> The platform may not be able to use the variant added in commit
> 0594ec417c89 either, for different reasons; see for example commit
> 861218740d6d.
>
> Allow platforms to specify the exact boot CPU count, independently of
> PcdCpuMaxLogicalProcessorNumber.
>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Igor Mammedov <imamm...@redhat.com>
> Cc: Ray Ni <ray...@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
> 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(-)
>
> 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 APs
> for
> the first time.
> # @Prompt Timeout for the BSP to detect all APs for the first time.
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000|UI
> NT32|0x00000004
> + ## Specifies the number of Logical Processors that are available in the
> + # preboot environment after platform reset, including BSP and APs.
> Possible
> + # values:<BR><BR>
> + # zero (default) - This PCD is ignored, and
> + # PcdCpuApInitTimeOutInMicroSeconds limits the initial AP
> + # detection by the BSP.<BR>
> + # nonzero - PcdCpuApInitTimeOutInMicroSeconds is ignored. The
> initial
> + # AP detection finishes when the detected CPU count (BSP
> + # plus APs) reaches the value of this PCD.<BR>
> + # @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 microcode
> Region.
> # @Prompt Microcode Region base address.
>
> 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 @@
>
> #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApInitTimeOutInMicroSeconds_HEL
> P #language en-US "Specifies timeout value in microseconds for the BSP to
> detect all APs for the first time."
>
> +#string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_PRO
> MPT #language en-US "Number of Logical Processors available after platform
> reset."
> +
> +#string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_HELP
> #language en-US "Specifies the number of Logical Processors that are available
> in the preboot environment after platform reset, including BSP and APs."
> +
> #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuMicrocodePatchAddress_PROMPT
> #language en-US "Microcode Region base address."
>
> #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]
>
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ##
> CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ##
> SOMETIMES_CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ##
> CONSUMES
> 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]
>
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ##
> CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ##
> CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber ##
> CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ##
> SOMETIMES_CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ##
> CONSUMES
> 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->BufferStart);
> }
> if (CpuMpData->InitFlag == 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 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)
> - );
> + if (PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) {
> + TimedWaitForApFinish (
> + CpuMpData,
> + PcdGet32 (PcdCpuBootLogicalProcessorNumber) - 1,
> + MAX_UINT32 // approx. 71 minutes
Do you think if the PcdCpuBootLogicalProcessorNumber is not zero, driver should
always wait for all the processors specified by
PcdCpuBootLogicalProcessorNumber 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)
> + );
>
> - while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting != 0) {
> - CpuPause();
> + while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting != 0) {
> + CpuPause();
> + }
> }
> } else {
> //
> --
> 2.19.1.3.g30247aa5d201
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
>
> View/Reply Online (#48563): https://edk2.groups.io/g/devel/message/48563
> Mute This Topic: https://groups.io/mt/34441228/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 (#48616): https://edk2.groups.io/g/devel/message/48616
Mute This Topic: https://groups.io/mt/34441228/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
