* [PATCH v2 0/2] UefiCpuPkg/MpInitLib: introduce PcdCpuBootLogicalProcessorNumber
@ 2019-10-10 11:29 Laszlo Ersek
2019-10-10 11:29 ` [PATCH v2 1/2] UefiCpuPkg/MpInitLib: expand comment on initial AP enumeration Laszlo Ersek
2019-10-10 11:29 ` [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection Laszlo Ersek
0 siblings, 2 replies; 6+ messages in thread
From: Laszlo Ersek @ 2019-10-10 11:29 UTC (permalink / raw)
To: edk2-devel-groups-io; +Cc: Eric Dong, Ray Ni
Repo: https://github.com/lersek/edk2.git
Branch: mpinitlib_bz_1515_v2
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
In version 2, the first patch of the v1 series
[edk2-devel] [PATCH 0/4]
UefiCpuPkg, OvmfPkg: separate PCDs for boot CPU count vs. max CPU count
originally posted at
http://mid.mail-archive.com/20191008112714.6215-1-lersek@redhat.com
https://edk2.groups.io/g/devel/message/48562
has been split out to its own UefiCpuPkg series.
In v2, the current AP enumeration options are documented first (more
extensively than the existent comment). Then the new mode is introduced
(with additional documentation).
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Thanks,
Laszlo
Laszlo Ersek (2):
UefiCpuPkg/MpInitLib: expand comment on initial AP enumeration
UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP
detection
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/MpLib.c | 77 +++++++++++++++-----
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 3 +-
UefiCpuPkg/UefiCpuPkg.dec | 13 ++++
UefiCpuPkg/UefiCpuPkg.uni | 4 +
5 files changed, 80 insertions(+), 18 deletions(-)
--
2.19.1.3.g30247aa5d201
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/2] UefiCpuPkg/MpInitLib: expand comment on initial AP enumeration 2019-10-10 11:29 [PATCH v2 0/2] UefiCpuPkg/MpInitLib: introduce PcdCpuBootLogicalProcessorNumber Laszlo Ersek @ 2019-10-10 11:29 ` Laszlo Ersek 2019-10-11 8:19 ` Ni, Ray 2019-10-10 11:29 ` [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection Laszlo Ersek 1 sibling, 1 reply; 6+ messages in thread From: Laszlo Ersek @ 2019-10-10 11:29 UTC (permalink / raw) To: edk2-devel-groups-io; +Cc: Eric Dong, Ray Ni Before adding another AP enumeration mode, clarify the documentation on the current logic. No functional changes. Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - new patch UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 +++++++++++++++----- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index d6f84c6f45c0..594a035d8b92 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1045,14 +1045,36 @@ WakeUpAP ( } 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. + // The AP enumeration algorithm below is suitable for two use cases. + // + // (1) The check-in time for an individual AP is bounded, and APs run + // through their initialization routines strongly concurrently. In + // particular, the number of concurrently running APs + // ("NumApsExecuting") is never expected to fall to zero + // *temporarily* -- it is expected to fall to zero only when all + // APs have checked-in. + // + // In this case, the platform is supposed to set + // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long + // enough for one AP to start initialization). The timeout will be + // reached soon, and remaining APs are collected by watching + // NumApsExecuting fall to zero. If NumApsExecuting falls to zero + // mid-process, while some APs have not completed initialization, + // the behavior is undefined. + // + // (2) The check-in time for an individual AP is unbounded, and/or APs + // may complete their initializations widely spread out. In + // particular, some APs may finish initialization before some APs + // even start. + // + // In this case, the platform is supposed to set + // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP + // enumeration will always take that long (except when the boot CPU + // count happens to be maximal, that is, + // PcdCpuMaxLogicalProcessorNumber). All APs are expected to + // check-in before the timeout, and NumApsExecuting is assumed zero + // at timeout. APs that miss the time-out may cause undefined + // behavior. // TimedWaitForApFinish ( CpuMpData, -- 2.19.1.3.g30247aa5d201 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] UefiCpuPkg/MpInitLib: expand comment on initial AP enumeration 2019-10-10 11:29 ` [PATCH v2 1/2] UefiCpuPkg/MpInitLib: expand comment on initial AP enumeration Laszlo Ersek @ 2019-10-11 8:19 ` Ni, Ray 0 siblings, 0 replies; 6+ messages in thread From: Ni, Ray @ 2019-10-11 8:19 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel-groups-io; +Cc: Dong, Eric Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Laszlo Ersek <lersek@redhat.com> > Sent: Thursday, October 10, 2019 7:30 PM > To: edk2-devel-groups-io <devel@edk2.groups.io> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com> > Subject: [PATCH v2 1/2] UefiCpuPkg/MpInitLib: expand comment on initial > AP enumeration > > Before adding another AP enumeration mode, clarify the documentation on > the current logic. No functional changes. > > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > v2: > - new patch > > UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 +++++++++++++++----- > 1 file changed, 30 insertions(+), 8 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index d6f84c6f45c0..594a035d8b92 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1045,14 +1045,36 @@ WakeUpAP ( > } > 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. > + // The AP enumeration algorithm below is suitable for two use cases. > + // > + // (1) The check-in time for an individual AP is bounded, and APs run > + // through their initialization routines strongly concurrently. In > + // particular, the number of concurrently running APs > + // ("NumApsExecuting") is never expected to fall to zero > + // *temporarily* -- it is expected to fall to zero only when all > + // APs have checked-in. > + // > + // In this case, the platform is supposed to set > + // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long > + // enough for one AP to start initialization). The timeout will be > + // reached soon, and remaining APs are collected by watching > + // NumApsExecuting fall to zero. If NumApsExecuting falls to zero > + // mid-process, while some APs have not completed initialization, > + // the behavior is undefined. > + // > + // (2) The check-in time for an individual AP is unbounded, and/or APs > + // may complete their initializations widely spread out. In > + // particular, some APs may finish initialization before some APs > + // even start. > + // > + // In this case, the platform is supposed to set > + // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP > + // enumeration will always take that long (except when the boot CPU > + // count happens to be maximal, that is, > + // PcdCpuMaxLogicalProcessorNumber). All APs are expected to > + // check-in before the timeout, and NumApsExecuting is assumed zero > + // at timeout. APs that miss the time-out may cause undefined > + // behavior. > // > TimedWaitForApFinish ( > CpuMpData, > -- > 2.19.1.3.g30247aa5d201 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection 2019-10-10 11:29 [PATCH v2 0/2] UefiCpuPkg/MpInitLib: introduce PcdCpuBootLogicalProcessorNumber Laszlo Ersek 2019-10-10 11:29 ` [PATCH v2 1/2] UefiCpuPkg/MpInitLib: expand comment on initial AP enumeration Laszlo Ersek @ 2019-10-10 11:29 ` Laszlo Ersek 2019-10-11 8:22 ` Ni, Ray 1 sibling, 1 reply; 6+ messages in thread From: Laszlo Ersek @ 2019-10-10 11:29 UTC (permalink / raw) To: edk2-devel-groups-io; +Cc: Eric Dong, Ray Ni - If a platform boots such that the boot CPU count is smaller than PcdCpuMaxLogicalProcessorNumber, then the platform cannot use the "fast AP detection" logic added in commit 6e1987f19af7. (Which has been documented as a subset of use case (2) in the previous patch.) Said logic depends on the boot CPU count being equal to PcdCpuMaxLogicalProcessorNumber. If the equality does not hold, the platform either has to wait too long, or risk missing APs due to an early timeout. - The platform may not be able to use the variant added in commit 0594ec417c89 either. (Which has been documented as use case (1) in the previous patch.) See commit 861218740d6d. When OVMF runs on QEMU/KVM, APs may check in with the BSP in arbitrary order, plus the individual AP may take arbitrarily long to check-in. If "NumApsExecuting" falls to zero mid-enumeration, APs will be missed. Allow platforms to specify the exact boot CPU count, independently of PcdCpuMaxLogicalProcessorNumber. In this mode, the BSP waits for all APs to check-in regardless of timeout. If at least one AP fails to check-in, then the AP enumeration hangs forever. That is the desired behavior when the exact boot CPU count is known in advance. (A hung boot is better than an AP checking-in after timeout, and executing code from released storage.) Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - update commit message - update docs in DEC file - add code comments - no functional changes UefiCpuPkg/UefiCpuPkg.dec | 13 +++ UefiCpuPkg/UefiCpuPkg.uni | 4 + UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 3 +- UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 ++++++++++++-------- 5 files changed, 80 insertions(+), 40 deletions(-) diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index 031a2ccd680a..12f4413ea5b0 100644 --- a/UefiCpuPkg/UefiCpuPkg.dec +++ b/UefiCpuPkg/UefiCpuPkg.dec @@ -227,6 +227,19 @@ [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|UINT32|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) - PcdCpuBootLogicalProcessorNumber is ignored, and + # PcdCpuApInitTimeOutInMicroSeconds limits the initial AP + # detection by the BSP.<BR> + # nonzero - PcdCpuApInitTimeOutInMicroSeconds is ignored. The initial + # AP detection finishes only when the detected CPU count + # (BSP plus APs) reaches the value of + # PcdCpuBootLogicalProcessorNumber, regardless of how long + # that takes.<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|0x00000005 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_HELP #language en-US "Specifies timeout value in microseconds for the BSP to detect all APs for the first time." +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_PROMPT #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 594a035d8b92..622b70ca3c4e 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1044,46 +1044,67 @@ WakeUpAP ( SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart); } if (CpuMpData->InitFlag == ApInitConfig) { - // - // The AP enumeration algorithm below is suitable for two use cases. - // - // (1) The check-in time for an individual AP is bounded, and APs run - // through their initialization routines strongly concurrently. In - // particular, the number of concurrently running APs - // ("NumApsExecuting") is never expected to fall to zero - // *temporarily* -- it is expected to fall to zero only when all - // APs have checked-in. - // - // In this case, the platform is supposed to set - // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long - // enough for one AP to start initialization). The timeout will be - // reached soon, and remaining APs are collected by watching - // NumApsExecuting fall to zero. If NumApsExecuting falls to zero - // mid-process, while some APs have not completed initialization, - // the behavior is undefined. - // - // (2) The check-in time for an individual AP is unbounded, and/or APs - // may complete their initializations widely spread out. In - // particular, some APs may finish initialization before some APs - // even start. - // - // In this case, the platform is supposed to set - // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP - // enumeration will always take that long (except when the boot CPU - // count happens to be maximal, that is, - // PcdCpuMaxLogicalProcessorNumber). All APs are expected to - // check-in before the timeout, and NumApsExecuting is assumed zero - // at timeout. APs that miss the time-out may cause undefined - // behavior. - // - TimedWaitForApFinish ( - CpuMpData, - PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, - PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) - ); + if (PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) { + // + // The AP enumeration algorithm below is suitable only when the + // platform can tell us the *exact* boot CPU count in advance. + // + // The wait below finishes only when the detected AP count reaches + // (PcdCpuBootLogicalProcessorNumber - 1), regardless of how long that + // takes. If at least one AP fails to check in (meaning a platform + // hardware bug), the detection hangs forever, by design. If the actual + // boot CPU count in the system is higher than + // PcdCpuBootLogicalProcessorNumber (meaning a platform + // misconfiguration), then some APs may complete initialization after + // the wait finishes, and cause undefined behavior. + // + TimedWaitForApFinish ( + CpuMpData, + PcdGet32 (PcdCpuBootLogicalProcessorNumber) - 1, + MAX_UINT32 // approx. 71 minutes + ); + } else { + // + // The AP enumeration algorithm below is suitable for two use cases. + // + // (1) The check-in time for an individual AP is bounded, and APs run + // through their initialization routines strongly concurrently. In + // particular, the number of concurrently running APs + // ("NumApsExecuting") is never expected to fall to zero + // *temporarily* -- it is expected to fall to zero only when all + // APs have checked-in. + // + // In this case, the platform is supposed to set + // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long + // enough for one AP to start initialization). The timeout will be + // reached soon, and remaining APs are collected by watching + // NumApsExecuting fall to zero. If NumApsExecuting falls to zero + // mid-process, while some APs have not completed initialization, + // the behavior is undefined. + // + // (2) The check-in time for an individual AP is unbounded, and/or APs + // may complete their initializations widely spread out. In + // particular, some APs may finish initialization before some APs + // even start. + // + // In this case, the platform is supposed to set + // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP + // enumeration will always take that long (except when the boot CPU + // count happens to be maximal, that is, + // PcdCpuMaxLogicalProcessorNumber). All APs are expected to + // check-in before the timeout, and NumApsExecuting is assumed zero + // at timeout. APs that miss the time-out may cause undefined + // behavior. + // + 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection 2019-10-10 11:29 ` [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection Laszlo Ersek @ 2019-10-11 8:22 ` Ni, Ray 2019-10-11 21:31 ` [edk2-devel] " Laszlo Ersek 0 siblings, 1 reply; 6+ messages in thread From: Ni, Ray @ 2019-10-11 8:22 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel-groups-io; +Cc: Dong, Eric Laszlo, the comments couldn't be better! Thanks!! Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Laszlo Ersek <lersek@redhat.com> > Sent: Thursday, October 10, 2019 7:30 PM > To: edk2-devel-groups-io <devel@edk2.groups.io> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com> > Subject: [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot > CPU count in AP detection > > - If a platform boots such that the boot CPU count is smaller than > PcdCpuMaxLogicalProcessorNumber, then the platform cannot use the > "fast > AP detection" logic added in commit 6e1987f19af7. (Which has been > documented as a subset of use case (2) in the previous patch.) > > Said logic depends on the boot CPU count being equal to > PcdCpuMaxLogicalProcessorNumber. If the equality does not hold, the > platform either has to wait too long, or risk missing APs due to an > early timeout. > > - The platform may not be able to use the variant added in commit > 0594ec417c89 either. (Which has been documented as use case (1) in the > previous patch.) > > See commit 861218740d6d. When OVMF runs on QEMU/KVM, APs may > check in > with the BSP in arbitrary order, plus the individual AP may take > arbitrarily long to check-in. If "NumApsExecuting" falls to zero > mid-enumeration, APs will be missed. > > Allow platforms to specify the exact boot CPU count, independently of > PcdCpuMaxLogicalProcessorNumber. In this mode, the BSP waits for all APs > to check-in regardless of timeout. If at least one AP fails to check-in, then the > AP enumeration hangs forever. That is the desired behavior when the exact > boot CPU count is known in advance. (A hung boot is better than an AP > checking-in after timeout, and executing code from released > storage.) > > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > v2: > - update commit message > - update docs in DEC file > - add code comments > - no functional changes > > UefiCpuPkg/UefiCpuPkg.dec | 13 +++ > UefiCpuPkg/UefiCpuPkg.uni | 4 + > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 3 +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 ++++++++++++-------- > 5 files changed, 80 insertions(+), 40 deletions(-) > > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec > index 031a2ccd680a..12f4413ea5b0 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dec > +++ b/UefiCpuPkg/UefiCpuPkg.dec > @@ -227,6 +227,19 @@ [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| > UINT32|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) - > + PcdCpuBootLogicalProcessorNumber is ignored, and > + # PcdCpuApInitTimeOutInMicroSeconds limits the initial AP > + # detection by the BSP.<BR> > + # nonzero - PcdCpuApInitTimeOutInMicroSeconds is ignored. The > initial > + # AP detection finishes only when the detected CPU count > + # (BSP plus APs) reaches the value of > + # PcdCpuBootLogicalProcessorNumber, regardless of how long > + # that takes.<BR> > + # @Prompt Number of Logical Processors available after platform reset. > + > + > gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0|UINT > 32|0x > + 00000008 > ## Specifies the base address of the first microcode Patch in the microcode > Region. > # @Prompt Microcode Region base address. > > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64| > 0x00000005 > 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_ > HELP #language en-US "Specifies timeout value in microseconds for the BSP > to detect all APs for the first time." > > +#string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_PR > OMPT #language en-US "Number of Logical Processors available after > platform reset." > + > +#string > STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_HE > LP #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_PROMP > T #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 594a035d8b92..622b70ca3c4e 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1044,46 +1044,67 @@ WakeUpAP ( > SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart); > } > if (CpuMpData->InitFlag == ApInitConfig) { > - // > - // The AP enumeration algorithm below is suitable for two use cases. > - // > - // (1) The check-in time for an individual AP is bounded, and APs run > - // through their initialization routines strongly concurrently. In > - // particular, the number of concurrently running APs > - // ("NumApsExecuting") is never expected to fall to zero > - // *temporarily* -- it is expected to fall to zero only when all > - // APs have checked-in. > - // > - // In this case, the platform is supposed to set > - // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long > - // enough for one AP to start initialization). The timeout will be > - // reached soon, and remaining APs are collected by watching > - // NumApsExecuting fall to zero. If NumApsExecuting falls to zero > - // mid-process, while some APs have not completed initialization, > - // the behavior is undefined. > - // > - // (2) The check-in time for an individual AP is unbounded, and/or APs > - // may complete their initializations widely spread out. In > - // particular, some APs may finish initialization before some APs > - // even start. > - // > - // In this case, the platform is supposed to set > - // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP > - // enumeration will always take that long (except when the boot CPU > - // count happens to be maximal, that is, > - // PcdCpuMaxLogicalProcessorNumber). All APs are expected to > - // check-in before the timeout, and NumApsExecuting is assumed zero > - // at timeout. APs that miss the time-out may cause undefined > - // behavior. > - // > - TimedWaitForApFinish ( > - CpuMpData, > - PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, > - PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) > - ); > + if (PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) { > + // > + // The AP enumeration algorithm below is suitable only when the > + // platform can tell us the *exact* boot CPU count in advance. > + // > + // The wait below finishes only when the detected AP count reaches > + // (PcdCpuBootLogicalProcessorNumber - 1), regardless of how long > that > + // takes. If at least one AP fails to check in (meaning a platform > + // hardware bug), the detection hangs forever, by design. If the actual > + // boot CPU count in the system is higher than > + // PcdCpuBootLogicalProcessorNumber (meaning a platform > + // misconfiguration), then some APs may complete initialization after > + // the wait finishes, and cause undefined behavior. > + // > + TimedWaitForApFinish ( > + CpuMpData, > + PcdGet32 (PcdCpuBootLogicalProcessorNumber) - 1, > + MAX_UINT32 // approx. 71 minutes > + ); > + } else { > + // > + // The AP enumeration algorithm below is suitable for two use cases. > + // > + // (1) The check-in time for an individual AP is bounded, and APs run > + // through their initialization routines strongly concurrently. In > + // particular, the number of concurrently running APs > + // ("NumApsExecuting") is never expected to fall to zero > + // *temporarily* -- it is expected to fall to zero only when all > + // APs have checked-in. > + // > + // In this case, the platform is supposed to set > + // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long > + // enough for one AP to start initialization). The timeout will be > + // reached soon, and remaining APs are collected by watching > + // NumApsExecuting fall to zero. If NumApsExecuting falls to zero > + // mid-process, while some APs have not completed initialization, > + // the behavior is undefined. > + // > + // (2) The check-in time for an individual AP is unbounded, and/or APs > + // may complete their initializations widely spread out. In > + // particular, some APs may finish initialization before some APs > + // even start. > + // > + // In this case, the platform is supposed to set > + // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP > + // enumeration will always take that long (except when the boot CPU > + // count happens to be maximal, that is, > + // PcdCpuMaxLogicalProcessorNumber). All APs are expected to > + // check-in before the timeout, and NumApsExecuting is assumed > zero > + // at timeout. APs that miss the time-out may cause undefined > + // behavior. > + // > + 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection 2019-10-11 8:22 ` Ni, Ray @ 2019-10-11 21:31 ` Laszlo Ersek 0 siblings, 0 replies; 6+ messages in thread From: Laszlo Ersek @ 2019-10-11 21:31 UTC (permalink / raw) To: devel, ray.ni; +Cc: Dong, Eric On 10/11/19 10:22, Ni, Ray wrote: > Laszlo, the comments couldn't be better! Thanks!! > > Reviewed-by: Ray Ni <ray.ni@intel.com> Thanks, Ray :) Series pushed as commit range a7e2d20193e8..778832bcad33. Cheers! Laszlo > >> -----Original Message----- >> From: Laszlo Ersek <lersek@redhat.com> >> Sent: Thursday, October 10, 2019 7:30 PM >> To: edk2-devel-groups-io <devel@edk2.groups.io> >> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com> >> Subject: [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot >> CPU count in AP detection >> >> - If a platform boots such that the boot CPU count is smaller than >> PcdCpuMaxLogicalProcessorNumber, then the platform cannot use the >> "fast >> AP detection" logic added in commit 6e1987f19af7. (Which has been >> documented as a subset of use case (2) in the previous patch.) >> >> Said logic depends on the boot CPU count being equal to >> PcdCpuMaxLogicalProcessorNumber. If the equality does not hold, the >> platform either has to wait too long, or risk missing APs due to an >> early timeout. >> >> - The platform may not be able to use the variant added in commit >> 0594ec417c89 either. (Which has been documented as use case (1) in the >> previous patch.) >> >> See commit 861218740d6d. When OVMF runs on QEMU/KVM, APs may >> check in >> with the BSP in arbitrary order, plus the individual AP may take >> arbitrarily long to check-in. If "NumApsExecuting" falls to zero >> mid-enumeration, APs will be missed. >> >> Allow platforms to specify the exact boot CPU count, independently of >> PcdCpuMaxLogicalProcessorNumber. In this mode, the BSP waits for all APs >> to check-in regardless of timeout. If at least one AP fails to check-in, then the >> AP enumeration hangs forever. That is the desired behavior when the exact >> boot CPU count is known in advance. (A hung boot is better than an AP >> checking-in after timeout, and executing code from released >> storage.) >> >> Cc: Eric Dong <eric.dong@intel.com> >> Cc: Ray Ni <ray.ni@intel.com> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> >> Notes: >> v2: >> - update commit message >> - update docs in DEC file >> - add code comments >> - no functional changes >> >> UefiCpuPkg/UefiCpuPkg.dec | 13 +++ >> UefiCpuPkg/UefiCpuPkg.uni | 4 + >> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + >> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 3 +- >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 ++++++++++++-------- >> 5 files changed, 80 insertions(+), 40 deletions(-) >> >> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec >> index 031a2ccd680a..12f4413ea5b0 100644 >> --- a/UefiCpuPkg/UefiCpuPkg.dec >> +++ b/UefiCpuPkg/UefiCpuPkg.dec >> @@ -227,6 +227,19 @@ [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| >> UINT32|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) - >> + PcdCpuBootLogicalProcessorNumber is ignored, and >> + # PcdCpuApInitTimeOutInMicroSeconds limits the initial AP >> + # detection by the BSP.<BR> >> + # nonzero - PcdCpuApInitTimeOutInMicroSeconds is ignored. The >> initial >> + # AP detection finishes only when the detected CPU count >> + # (BSP plus APs) reaches the value of >> + # PcdCpuBootLogicalProcessorNumber, regardless of how long >> + # that takes.<BR> >> + # @Prompt Number of Logical Processors available after platform reset. >> + >> + >> gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0|UINT >> 32|0x >> + 00000008 >> ## Specifies the base address of the first microcode Patch in the microcode >> Region. >> # @Prompt Microcode Region base address. >> >> gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64| >> 0x00000005 >> 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_ >> HELP #language en-US "Specifies timeout value in microseconds for the BSP >> to detect all APs for the first time." >> >> +#string >> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_PR >> OMPT #language en-US "Number of Logical Processors available after >> platform reset." >> + >> +#string >> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_HE >> LP #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_PROMP >> T #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 594a035d8b92..622b70ca3c4e 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> @@ -1044,46 +1044,67 @@ WakeUpAP ( >> SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart); >> } >> if (CpuMpData->InitFlag == ApInitConfig) { >> - // >> - // The AP enumeration algorithm below is suitable for two use cases. >> - // >> - // (1) The check-in time for an individual AP is bounded, and APs run >> - // through their initialization routines strongly concurrently. In >> - // particular, the number of concurrently running APs >> - // ("NumApsExecuting") is never expected to fall to zero >> - // *temporarily* -- it is expected to fall to zero only when all >> - // APs have checked-in. >> - // >> - // In this case, the platform is supposed to set >> - // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long >> - // enough for one AP to start initialization). The timeout will be >> - // reached soon, and remaining APs are collected by watching >> - // NumApsExecuting fall to zero. If NumApsExecuting falls to zero >> - // mid-process, while some APs have not completed initialization, >> - // the behavior is undefined. >> - // >> - // (2) The check-in time for an individual AP is unbounded, and/or APs >> - // may complete their initializations widely spread out. In >> - // particular, some APs may finish initialization before some APs >> - // even start. >> - // >> - // In this case, the platform is supposed to set >> - // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP >> - // enumeration will always take that long (except when the boot CPU >> - // count happens to be maximal, that is, >> - // PcdCpuMaxLogicalProcessorNumber). All APs are expected to >> - // check-in before the timeout, and NumApsExecuting is assumed zero >> - // at timeout. APs that miss the time-out may cause undefined >> - // behavior. >> - // >> - TimedWaitForApFinish ( >> - CpuMpData, >> - PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, >> - PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) >> - ); >> + if (PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) { >> + // >> + // The AP enumeration algorithm below is suitable only when the >> + // platform can tell us the *exact* boot CPU count in advance. >> + // >> + // The wait below finishes only when the detected AP count reaches >> + // (PcdCpuBootLogicalProcessorNumber - 1), regardless of how long >> that >> + // takes. If at least one AP fails to check in (meaning a platform >> + // hardware bug), the detection hangs forever, by design. If the actual >> + // boot CPU count in the system is higher than >> + // PcdCpuBootLogicalProcessorNumber (meaning a platform >> + // misconfiguration), then some APs may complete initialization after >> + // the wait finishes, and cause undefined behavior. >> + // >> + TimedWaitForApFinish ( >> + CpuMpData, >> + PcdGet32 (PcdCpuBootLogicalProcessorNumber) - 1, >> + MAX_UINT32 // approx. 71 minutes >> + ); >> + } else { >> + // >> + // The AP enumeration algorithm below is suitable for two use cases. >> + // >> + // (1) The check-in time for an individual AP is bounded, and APs run >> + // through their initialization routines strongly concurrently. In >> + // particular, the number of concurrently running APs >> + // ("NumApsExecuting") is never expected to fall to zero >> + // *temporarily* -- it is expected to fall to zero only when all >> + // APs have checked-in. >> + // >> + // In this case, the platform is supposed to set >> + // PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long >> + // enough for one AP to start initialization). The timeout will be >> + // reached soon, and remaining APs are collected by watching >> + // NumApsExecuting fall to zero. If NumApsExecuting falls to zero >> + // mid-process, while some APs have not completed initialization, >> + // the behavior is undefined. >> + // >> + // (2) The check-in time for an individual AP is unbounded, and/or APs >> + // may complete their initializations widely spread out. In >> + // particular, some APs may finish initialization before some APs >> + // even start. >> + // >> + // In this case, the platform is supposed to set >> + // PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP >> + // enumeration will always take that long (except when the boot CPU >> + // count happens to be maximal, that is, >> + // PcdCpuMaxLogicalProcessorNumber). All APs are expected to >> + // check-in before the timeout, and NumApsExecuting is assumed >> zero >> + // at timeout. APs that miss the time-out may cause undefined >> + // behavior. >> + // >> + 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 > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-11 21:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-10 11:29 [PATCH v2 0/2] UefiCpuPkg/MpInitLib: introduce PcdCpuBootLogicalProcessorNumber Laszlo Ersek 2019-10-10 11:29 ` [PATCH v2 1/2] UefiCpuPkg/MpInitLib: expand comment on initial AP enumeration Laszlo Ersek 2019-10-11 8:19 ` Ni, Ray 2019-10-10 11:29 ` [PATCH v2 2/2] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection Laszlo Ersek 2019-10-11 8:22 ` Ni, Ray 2019-10-11 21:31 ` [edk2-devel] " Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox