[edk2-devel] [PATCH v4] UefiCpuPkg/MpInitLib DXE: Add PCD to control AP status check interval
Ni, Ray
ray.ni at intel.com
Wed Mar 25 05:43:48 UTC 2020
Reviewed-by: Ray Ni <ray.ni at intel.com>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu at intel.com>
> Sent: Wednesday, March 25, 2020 1:07 PM
> To: devel at edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu at intel.com>; Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Laszlo Ersek
> <lersek at redhat.com>; Kinney, Michael D <michael.d.kinney at intel.com>; Zeng, Star <star.zeng at intel.com>; Brian J .
> Johnson <brian.johnson at hpe.com>
> Subject: [PATCH v4] UefiCpuPkg/MpInitLib DXE: Add PCD to control AP status check interval
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2627
>
> The commit will introduce a static PCD to specify the periodic interval
> for checking the AP status when MP services StartupAllAPs() and
> StartupThisAP() are being executed in a non-blocking manner. Or in other
> words, specifies the interval for callback function CheckApsStatus().
>
> The purpose is to provide the platform owners with the ability to choose
> the proper interval value to trigger CheckApsStatus() according to:
> A) The number of processors in the system;
> B) How MP services (StartupAllAPs & StartupThisAP) being used.
>
> Setting the PCD to a small value means the AP status check callback will
> be trigerred more frequently, it can benefit the performance for the case
> when the BSP uses WaitForEvent() or uses CheckEvent() in a loop to wait
> for AP(s) to complete the task, especially when the task can be finished
> considerably fast on AP(s).
>
> An example is within function CpuFeaturesInitialize() under
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c,
> where BSP will perform the same task with APs and requires all the
> processors to finish the task before BSP proceeds to its next task.
>
> Setting the PCD to a big value, on the other hand, can reduce the impact
> on BSP by the time being consumed in CheckApsStatus(), especially when the
> number of processors is huge so that the time consumed in CheckApsStatus()
> is not negligible.
>
> The type of the PCD is UINT32, which means the maximum possible interval
> value can be set to:
> 4,294,967,295 microseconds = 4,295 seconds = 71.58 minutes = 1.19 hours
> which should be sufficient for usage.
>
> For least impact, the default value of the new PCD will be the same with
> the current interval value. It will be set to 100,000 microseconds, which
> is 100 milliseconds.
>
> Unitest done:
> A) OS boot successfully;
> B) Use debug message to confirm the 'TriggerTime' parameter for the
> 'SetTimer' service is the same before & after this patch.
>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Cc: Star Zeng <star.zeng at intel.com>
> Cc: Brian J. Johnson <brian.johnson at hpe.com>
> Signed-off-by: Hao A Wu <hao.a.wu at intel.com>
> ---
>
> Notes:
> V4
> Avoiding introducing a local variable in InitMpGlobalData().
>
> V3
> A) Use microseconds, instead of milliseconds as the interval unit;
> B) Use UINT32, instead of UINT64, for the PCD type;
> C) Address the bug that incorrect 'TriggerTime' parameter was passed into
> the 'SetTimer' service call in V2 patch
>
> V2
> Introduce a PCD to specify the AP status check interval.
>
>
> UefiCpuPkg/UefiCpuPkg.dec | 6 ++++++
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 20 ++++++++++----------
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 5 +++--
> UefiCpuPkg/UefiCpuPkg.uni | 5 ++++-
> 4 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index e91dc68cbe..762badf5d2 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -230,6 +230,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> # @Prompt This PCD is the nominal frequency of the core crystal clock in Hz as is CPUID Leaf 0x15:ECX
> gUefiCpuPkgTokenSpaceGuid.PcdCpuCoreCrystalClockFrequency|24000000|UINT64|0x32132113
>
> + ## Specifies the periodic interval value in microseconds for the status check
> + # of APs for StartupAllAPs() and StartupThisAP() executed in non-blocking
> + # mode in DXE phase.
> + # @Prompt Periodic interval value in microseconds for AP status check in DXE.
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuApStatusCheckIntervalInMicroSeconds|100000|UINT32|0x0000001E
> +
> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> ## Specifies max supported number of Logical Processors.
> # @Prompt Configure max supported number of Logical Processors
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> index 45aaa179ff..a51a9ec1d2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> @@ -61,13 +61,13 @@ [Guids]
> gEdkiiMicrocodePatchHobGuid ## SOMETIMES_CONSUMES ## HOB
>
> [Pcd]
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber ## CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## SOMETIMES_CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## SOMETIMES_CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
> -
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber ## CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## SOMETIMES_CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## SOMETIMES_CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuApStatusCheckIntervalInMicroSeconds ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index a987c32109..56b776d3d8 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -15,7 +15,6 @@
>
> #include <Protocol/Timer.h>
>
> -#define AP_CHECK_INTERVAL (EFI_TIMER_PERIOD_MILLISECONDS (100))
> #define AP_SAFE_STACK_SIZE 128
>
> CPU_MP_DATA *mCpuMpData = NULL;
> @@ -451,7 +450,9 @@ InitMpGlobalData (
> Status = gBS->SetTimer (
> mCheckAllApsEvent,
> TimerPeriodic,
> - AP_CHECK_INTERVAL
> + EFI_TIMER_PERIOD_MICROSECONDS (
> + PcdGet32 (PcdCpuApStatusCheckIntervalInMicroSeconds)
> + )
> );
> ASSERT_EFI_ERROR (Status);
>
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> index c0d6ed5136..1780dfdc12 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -3,7 +3,7 @@
> //
> // This Package provides UEFI compatible CPU modules and libraries.
> //
> -// Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
> +// Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.<BR>
> //
> // SPDX-License-Identifier: BSD-2-Clause-Patent
> //
> @@ -275,3 +275,6 @@
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_PROMPT #language en-US "Specify the
> count of pre allocated SMM MP tokens per chunk.\n"
>
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_HELP #language en-US "This value used
> to specify the count of pre allocated SMM MP tokens per chunk.\n"
> +
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApStatusCheckIntervalInMicroSeconds_PROMPT #language en-US
> "Periodic interval value in microseconds for AP status check in DXE.\n"
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApStatusCheckIntervalInMicroSeconds_HELP #language en-US
> "Periodic interval value in microseconds for the status check of APs for StartupAllAPs() and StartupThisAP() executed in
> non-blocking mode in DXE phase.\n"
> --
> 2.12.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56254): https://edk2.groups.io/g/devel/message/56254
Mute This Topic: https://groups.io/mt/72535222/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list