Reviewed-by: Eric Dong <eric.d...@intel.com>
> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, June 5, 2019 1:49 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>; Sathyanarayanan, Nandagopal
> <nandagopal.sathyanaraya...@intel.com>
> Subject: [PATCH v2 2/2] UefiCpuPkg/MpInitLib: Decrease NumApsExecuting
> only for ApInitConfig
>
> The patch fixes the bug that the memory under 1MB is modified by firmware
> in S3 boot.
>
> Root cause is a racing condition in MpInitLib:
> 1. BSP: WakeUpByInitSipiSipi is set by NotifyOnS3SmmInitDonePpi() 2. BSP:
> WakeUpAP() wakes all APs to run certain procedure.
> 2.1. AllocateResetVector() uses <1MB memory for wake up vector.
> 2.1. FillExchangeInfoData() resets NumApsExecuting to 0.
> 2.2. WaitApWakeup() waits AP to clear WAKEUP_AP_SIGNAL.
> 3. AP: ApWakeupFunction() clears WAKEUP_AP_SIGNAL to inform BSP.
> 5. BSP: FreeResetVector() restores the <1MB memory 4. AP:
> ApWakeupFunction() calls the certain procedure.
> 4.1. NumApsExecuting is decreased.
>
> #4.1 happens after the 1MB memory is restored so the result is memory
> below 1MB is changed by #4.1 It happens only when the AP executes
> procedure a bit longer.
> AP returns back to ApWakeupFunction() from procedure after BSP restores
> the <1MB memory.
>
> Since NumApsExecuting is only used when InitFlag == ApInitConfig for
> counting the processor count.
> The patch moves the NumApsExecuting decrease to the path when InitFlag
> == ApInitConfig.
>
> Signed-off-by: Ray Ni <ray...@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Nandagopal Sathyanarayanan <nandagopal.sathyanaraya...@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 3337488892..6f51bc4ebf 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1,7 +1,7 @@
> /** @file
> CPU MP Initialize Library common functions.
>
> - Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2016 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -619,6 +619,8 @@ ApWakeupFunction (
> RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters,
> FALSE);
> InitializeApData (CpuMpData, ProcessorNumber, BistData,
> ApTopOfStack);
> ApStartupSignalBuffer = CpuMpData-
> >CpuData[ProcessorNumber].StartupApSignal;
> +
> + InterlockedDecrement ((UINT32 *)
> + &CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
> } else {
> //
> // Execute AP function if AP is ready @@ -698,7 +700,6 @@
> ApWakeupFunction (
> // AP finished executing C code
> //
> InterlockedIncrement ((UINT32 *) &CpuMpData->FinishedCount);
> - InterlockedDecrement ((UINT32 *) &CpuMpData->MpCpuExchangeInfo-
> >NumApsExecuting);
>
> //
> // Place AP is specified loop mode
> --
> 2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#42051): https://edk2.groups.io/g/devel/message/42051
Mute This Topic: https://groups.io/mt/31934506/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
