From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: eric.dong@intel.com) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by groups.io with SMTP; Sun, 09 Jun 2019 19:38:06 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jun 2019 19:38:06 -0700 X-ExtLoop1: 1 Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga006.jf.intel.com with ESMTP; 09 Jun 2019 19:38:05 -0700 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 9 Jun 2019 19:38:05 -0700 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 9 Jun 2019 19:38:05 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.134]) by SHSMSX105.ccr.corp.intel.com ([169.254.11.153]) with mapi id 14.03.0415.000; Mon, 10 Jun 2019 10:38:03 +0800 From: "Dong, Eric" To: "Ni, Ray" , "devel@edk2.groups.io" CC: "Sathyanarayanan, Nandagopal" Subject: Re: [PATCH v2 2/2] UefiCpuPkg/MpInitLib: Decrease NumApsExecuting only for ApInitConfig Thread-Topic: [PATCH v2 2/2] UefiCpuPkg/MpInitLib: Decrease NumApsExecuting only for ApInitConfig Thread-Index: AQHVG2J58em7sj/FX0C4clDixHsx8qaUNEQQ Date: Mon, 10 Jun 2019 02:38:02 +0000 Message-ID: References: <20190605054920.123184-1-ray.ni@intel.com> <20190605054920.123184-3-ray.ni@intel.com> In-Reply-To: <20190605054920.123184-3-ray.ni@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong > -----Original Message----- > From: Ni, Ray > Sent: Wednesday, June 5, 2019 1:49 PM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Sathyanarayanan, Nandagopal > > Subject: [PATCH v2 2/2] UefiCpuPkg/MpInitLib: Decrease NumApsExecuting > only for ApInitConfig >=20 > The patch fixes the bug that the memory under 1MB is modified by firmware > in S3 boot. >=20 > 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. >=20 > #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. >=20 > Since NumApsExecuting is only used when InitFlag =3D=3D ApInitConfig for > counting the processor count. > The patch moves the NumApsExecuting decrease to the path when InitFlag > =3D=3D ApInitConfig. >=20 > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Nandagopal Sathyanarayanan > --- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) >=20 > 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. >=20 > - Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2016 - 2019, Intel Corporation. All rights > + reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -619,6 +619,8 @@ ApWakeupFunction ( > RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters= , > FALSE); > InitializeApData (CpuMpData, ProcessorNumber, BistData, > ApTopOfStack); > ApStartupSignalBuffer =3D 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); >=20 > // > // Place AP is specified loop mode > -- > 2.21.0.windows.1