On 10/29/15 02:32, Jordan Justen wrote:
> Since the CpuDxe has a fixed maximum number of APs that are controlled
> with PcdCpuMaxLogicalProcessorNumber, if the maximum number of APs are
> awake, then the BSP should be able to continue.
>
> Since the APs may be updating the mMpSystemData.NumberOfProcessors
> during this early init phase, we use SynchronizationLib to update this
> variable on the AP and to read it on the BSP.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Jeff Fan <jeff....@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> ---
> UefiCpuPkg/CpuDxe/CpuMp.c | 15 ++++++++++++---
> UefiCpuPkg/CpuDxe/CpuMp.h | 2 +-
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
> index 6a22b9d..1094292 100644
> --- a/UefiCpuPkg/CpuDxe/CpuMp.c
> +++ b/UefiCpuPkg/CpuDxe/CpuMp.c
> @@ -1474,7 +1474,7 @@ ApEntryPointInC (
> // Store the Stack address, when reset the AP, We can found the original
> address.
> //
> mMpSystemData.CpuDatas[mMpSystemData.NumberOfProcessors].TopOfStack =
> TopOfApStack;
> - mMpSystemData.NumberOfProcessors++;
> + InterlockedIncrement (&mMpSystemData.NumberOfProcessors);
> mMpSystemData.NumberOfEnabledProcessors++;
> } else {
> Status = WhoAmI (&mMpServicesTemplate, &ProcessorNumber);
> @@ -1749,9 +1749,18 @@ InitializeMpSupport (
> StartApsStackless ();
>
> //
> - // Wait for APs to arrive at the ApEntryPoint routine
> + // Wait for all APs to start
> //
> - MicroSecondDelay (PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds));
> + Timeout = 0;
> + do {
> + if (InterlockedCompareExchange32 (
> + &mMpSystemData.NumberOfProcessors, 0, 0) >=
> + gMaxLogicalProcessorNumber) {
> + break;
> + }
I suggest to add a CpuPause() call here. I think it might help on
physical platforms too, but it definitely helps on hypervisors / PCPUs
that support Pause Loop Exiting.
The CpuPause() call will likely make each iteration of this loop to take
longer, but gPollInterval below is a lower bound anyway.
Anyway, this is just an idea. With or without CpuPause():
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks
Laszlo
> + MicroSecondDelay (gPollInterval);
> + Timeout += gPollInterval;
> + } while (Timeout <= PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds));
> }
>
> DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n",
> mMpSystemData.NumberOfProcessors));
> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h
> index 503f3ae..ab7a7f5 100644
> --- a/UefiCpuPkg/CpuDxe/CpuMp.h
> +++ b/UefiCpuPkg/CpuDxe/CpuMp.h
> @@ -114,7 +114,7 @@ typedef struct {
> **/
> typedef struct {
> CPU_DATA_BLOCK *CpuDatas;
> - UINTN NumberOfProcessors;
> + UINT32 NumberOfProcessors;
> UINTN NumberOfEnabledProcessors;
>
> EFI_AP_PROCEDURE Procedure;
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
