From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1C0DD2098C8B0 for ; Thu, 19 Jul 2018 05:09:53 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Jul 2018 05:09:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,374,1526367600"; d="scan'208";a="73698116" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga001.fm.intel.com with ESMTP; 19 Jul 2018 05:09:53 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 19 Jul 2018 05:09:52 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 19 Jul 2018 05:09:52 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.124]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.100]) with mapi id 14.03.0319.002; Thu, 19 Jul 2018 20:09:50 +0800 From: "Dong, Eric" To: Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" Thread-Topic: [edk2] UefiCpuPkg/MpInitLib: Fix S3 resume hang issue. Thread-Index: AQHUHlWl6jaChLkNoUae79wugoCosaSV7GOAgACJb8A= Date: Thu, 19 Jul 2018 12:09:49 +0000 Message-ID: References: <20180718051009.19748-1-eric.dong@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: UefiCpuPkg/MpInitLib: Fix S3 resume hang issue. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Jul 2018 12:09:54 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Laszlo, > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Laszlo Ersek > Sent: Thursday, July 19, 2018 7:57 PM > To: Dong, Eric ; edk2-devel@lists.01.org > Cc: Ni, Ruiyu > Subject: Re: [edk2] UefiCpuPkg/MpInitLib: Fix S3 resume hang issue. >=20 > Hi, >=20 > On 07/18/18 07:10, Eric Dong wrote: > > When resume from S3 and CPU loop mode is MWait mode, if driver calls > > APs to do task at EndOfPei point, the APs can't been wake up and bios > > hang at that point. > > > > The root cause is PiSmmCpuDxeSmm driver wakes up APs with HLT mode > > during S3 resume phase to do SMM relocation. > > After this task, PiSmmCpuDxeSmm driver not restore APs context which > > make CpuMpPei driver saved wake up buffer not works. > > > > The solution for this issue is let CpuMpPei driver hook S3SmmInitDone > > ppi notification. In this notify function, it check whether Cpu Loop > > mode is not HLT mode. If yes, CpuMpPei driver will set a flag to force > > BSP use INIT-SIPI -SIPI command to wake up the APs. > > > > Cc: Laszlo Ersek > > Cc: Ruiyu Ni > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Eric Dong > > --- > > UefiCpuPkg/Library/MpInitLib/MpLib.c | 16 ++++- > > UefiCpuPkg/Library/MpInitLib/MpLib.h | 9 +++ > > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 ++ > > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 89 > +++++++++++++++++++++++++++ > > 4 files changed, 116 insertions(+), 2 deletions(-) >=20 > I see this patch is already committed (58942277bcbf); I have two comments= in > retrospect: >=20 > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > > index 722db2a01f..e5c701ddeb 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > > @@ -985,13 +985,15 @@ WakeUpAP ( > > CpuMpData->FinishedCount =3D 0; > > ResetVectorRequired =3D FALSE; > > > > - if (CpuMpData->ApLoopMode =3D=3D ApInHltLoop || > > + if (CpuMpData->WakeUpByInitSipiSipi || > > CpuMpData->InitFlag !=3D ApInitDone) { > > ResetVectorRequired =3D TRUE; > > AllocateResetVector (CpuMpData); > > FillExchangeInfoData (CpuMpData); > > SaveLocalApicTimerSetting (CpuMpData); > > - } else if (CpuMpData->ApLoopMode =3D=3D ApInMwaitLoop) { > > + } > > + > > + if (CpuMpData->ApLoopMode =3D=3D ApInMwaitLoop) { > > // > > // Get AP target C-state each time when waking up AP, > > // for it maybe updated by platform again @@ -1076,6 +1078,13 @@ > > WakeUpAP ( > > if (ResetVectorRequired) { > > FreeResetVector (CpuMpData); > > } > > + > > + // > > + // After one round of Wakeup Ap actions, need to re-sync ApLoopMode > > + with // WakeUpByInitSipiSipi flag. WakeUpByInitSipiSipi flag maybe > > + changed by // S3SmmInitDone Ppi. > > + // > > + CpuMpData->WakeUpByInitSipiSipi =3D (CpuMpData->ApLoopMode =3D=3D > > + ApInHltLoop); > > } > > > > /** > > @@ -1648,6 +1657,9 @@ MpInitLibInitialize ( > > // > > CpuMpData->ApLoopMode =3D ApLoopMode; > > DEBUG ((DEBUG_INFO, "AP Loop Mode is %d\n", > > CpuMpData->ApLoopMode)); > > + > > + CpuMpData->WakeUpByInitSipiSipi =3D (CpuMpData->ApLoopMode =3D=3D > > + ApInHltLoop); > > + > > // > > // Set up APs wakeup signal buffer > > // > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h > > b/UefiCpuPkg/Library/MpInitLib/MpLib.h > > index 6958080ac1..9d0b866d09 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h > > @@ -250,6 +250,15 @@ struct _CPU_MP_DATA { > > UINT32 ProcessorFlags; > > UINT64 MicrocodeDataAddress; > > UINT32 MicrocodeRevision; > > + > > + // > > + // Whether need to use Init-Sipi-Sipi to wake up the APs. > > + // Two cases need to set this value to TRUE. One is in HLT // loop > > + mode, the other is resume from S3 which loop mode // will be > > + hardcode change to HLT mode by PiSmmCpuDxeSmm // driver. > > + // > > + BOOLEAN WakeUpByInitSipiSipi; > > }; > > > > extern EFI_GUID mCpuInitMpLibHobGuid; > > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > > index fa84e392af..43a3b3b036 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > > @@ -44,6 +44,7 @@ > > [Packages] > > MdePkg/MdePkg.dec > > UefiCpuPkg/UefiCpuPkg.dec > > + MdeModulePkg/MdeModulePkg.dec > > > > [LibraryClasses] > > BaseLib > > @@ -54,6 +55,7 @@ > > CpuLib > > UefiCpuLib > > SynchronizationLib > > + PeiServicesLib > > > > [Pcd] > > gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## > CONSUMES > > @@ -64,3 +66,5 @@ > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## > CONSUMES > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## > SOMETIMES_CONSUMES > > > > +[Guids] > > + gEdkiiS3SmmInitDoneGuid > > \ No newline at end of file > > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > > index 92f28681e4..06d966b227 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > > @@ -13,6 +13,87 @@ > > **/ > > > > #include "MpLib.h" > > +#include > > +#include > > + > > +/** > > + S3 SMM Init Done notification function. > > + > > + @param PeiServices Indirect reference to the PEI Services Tabl= e. > > + @param NotifyDesc Address of the notification descriptor data > structure. > > + @param InvokePpi Address of the PPI that was invoked. > > + > > + @retval EFI_SUCCESS The function completes successfully. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +NotifyOnS3SmmInitDonePpi ( > > + IN EFI_PEI_SERVICES **PeiServices, > > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc, > > + IN VOID *InvokePpi > > + ); > > + > > + > > +// > > +// Global function > > +// > > +EFI_PEI_NOTIFY_DESCRIPTOR mS3SmmInitDoneNotifyDesc =3D { > > + EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST, > > + &gEdkiiS3SmmInitDoneGuid, > > + NotifyOnS3SmmInitDonePpi > > +}; > > + > > +/** > > + The function prototype for invoking a function on an Application > Processor. > > + > > + This definition is used by the UEFI MP Serices Protocol, and the > > + PI SMM System Table. > > + > > + @param[in,out] Buffer The pointer to private data buffer. > > +**/ > > +VOID > > +EmptyApProcedure ( > > + IN OUT VOID * Buffer > > + ) > > +{ > > +} > > + >=20 > (1) This function seems useless -- can you please submit a patch to > remove it? >=20 Yes. > > +/** > > + S3 SMM Init Done notification function. > > + > > + @param PeiServices Indirect reference to the PEI Services Tabl= e. > > + @param NotifyDesc Address of the notification descriptor data > structure. > > + @param InvokePpi Address of the PPI that was invoked. > > + > > + @retval EFI_SUCCESS The function completes successfully. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +NotifyOnS3SmmInitDonePpi ( > > + IN EFI_PEI_SERVICES **PeiServices, > > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc, > > + IN VOID *InvokePpi > > + ) > > +{ > > + CPU_MP_DATA *CpuMpData; > > + > > + CpuMpData =3D GetCpuMpData (); > > + > > + // > > + // PiSmmCpuDxeSmm driver hardcode change the loop mode to HLT > mode. > > + // So in this notify function, code need to check the current loop > > + // mode, if it is not HLT mode, code need to change loop mode back > > + // to the original mode. > > + // > > + if (CpuMpData->ApLoopMode !=3D ApInHltLoop) { > > + CpuMpData->WakeUpByInitSipiSipi =3D TRUE; > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > > > /** > > Enable Debug Agent to support source debugging on AP function. > > @@ -240,7 +321,15 @@ InitMpGlobalData ( > > IN CPU_MP_DATA *CpuMpData > > ) > > { > > + EFI_STATUS Status; > > + > > SaveCpuMpData (CpuMpData); > > + > > + /// > > + /// Install Notify > > + /// > > + Status =3D PeiServicesNotifyPpi (&mS3SmmInitDoneNotifyDesc); > > + ASSERT_EFI_ERROR (Status); > > } > > > > /** > > >=20 > (2) Because OVMF uses the "UefiCpuPkg.dec" default for PcdCpuApLoopMode > (value 1, "Place AP in the Hlt-Loop state"), this patch should mostly be > a no-op for OVMF; WakeUpByInitSipiSipi is always TRUE (and > NotifyOnS3SmmInitDonePpi() basically does nothing). >=20 > So, I regression-tested this change, and it's fine. I see that the PPI > notify is registered, and then called, but there's no other change to S3 > resume behavior: >=20 > > Loading PEIM at 0x0000086FCC0 EntryPoint=3D0x00000875367 CpuMpPei.efi > > AP Loop Mode is 1 > > WakeupBufferStart =3D 9F000, WakeupBufferSize =3D 0 > > TimedWaitForApFinish: reached FinishedApLimit=3D7 in 0 microseconds > > APIC MODE is 1 > > MpInitLib: Find 8 processors in system. > > Register PPI Notify: [EdkiiS3SmmInitDone] > > Does not find any stored CPU BIST information from PPI! > > APICID - 0x00000000, BIST - 0x00000000 > > APICID - 0x00000001, BIST - 0x00000000 > > APICID - 0x00000002, BIST - 0x00000000 > > APICID - 0x00000003, BIST - 0x00000000 > > APICID - 0x00000004, BIST - 0x00000000 > > APICID - 0x00000005, BIST - 0x00000000 > > APICID - 0x00000006, BIST - 0x00000000 > > APICID - 0x00000007, BIST - 0x00000000 > > Install PPI: [EfiSecPlatformInformation2Ppi] > > Install PPI: [EfiPeiMpServicesPpi] > > [...] > > Call AsmDisablePaging64() to return to S3 Resume in PEI Phase > > S3ResumeExecuteBootScript() > > Close all SMRAM regions before executing boot script > > Lock all SMRAM regions before executing boot script > > Signal S3SmmInitDone > > Install PPI: [EdkiiS3SmmInitDone] > > Notify: PPI Guid: [EdkiiS3SmmInitDone], Peim notify entry point: 87674B > > Signal [EdkiiS3SmmInitDone] to SMM - Enter > > Locate Smm Communicate Ppi failed (Not Found)! > > PeiS3ResumeState - 7EF62118 > > Enable X64 and transfer control to Standalone Boot Script Executor > > [...] >=20 > Regression-tested-by: Laszlo Ersek >=20 > Thanks > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel