Reviewed-by: Star Zeng <star.z...@intel.com> > -----Original Message----- > From: Wu, Hao A > Sent: Wednesday, March 25, 2020 1:07 PM > To: devel@edk2.groups.io > Cc: Wu, Hao A <hao.a...@intel.com>; Dong, Eric <eric.d...@intel.com>; Ni, > Ray <ray...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Kinney, Michael > D <michael.d.kin...@intel.com>; Zeng, Star <star.z...@intel.com>; Brian J . > Johnson <brian.john...@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.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Brian J. Johnson <brian.john...@hpe.com> > Signed-off-by: Hao A Wu <hao.a...@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| > 10 > + 0000|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_PR > OMPT #language en-US "Specify the count of pre allocated SMM MP tokens > per chunk.\n" > > #string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_HE > LP #language en-US "This value used to specify the count of pre allocated > SMM MP tokens per chunk.\n" > + > +#string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApStatusCheckIntervalInMicroSec > onds_PROMPT #language en-US "Periodic interval value in microseconds for > AP status check in DXE.\n" > +#string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApStatusCheckIntervalInMicroSec > onds_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 (#56253): https://edk2.groups.io/g/devel/message/56253 Mute This Topic: https://groups.io/mt/72535222/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
