Re: UefiCpuPkg/MpInitLib: Fix S3 resume hang issue.

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Reviewed-by: Ruiyu Ni <Ruiyu.nI@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Dong, Eric
> Sent: Wednesday, July 18, 2018 1:10 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: UefiCpuPkg/MpInitLib: Fix S3 resume hang issue.
> 
> 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 <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  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(-)
> 
> 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 = 0;
>    ResetVectorRequired = FALSE;
> 
> -  if (CpuMpData->ApLoopMode == ApInHltLoop ||
> +  if (CpuMpData->WakeUpByInitSipiSipi ||
>        CpuMpData->InitFlag   != ApInitDone) {
>      ResetVectorRequired = TRUE;
>      AllocateResetVector (CpuMpData);
>      FillExchangeInfoData (CpuMpData);
>      SaveLocalApicTimerSetting (CpuMpData);
> -  } else if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
> +  }
> +
> +  if (CpuMpData->ApLoopMode == 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 = (CpuMpData->ApLoopMode ==
> + ApInHltLoop);
>  }
> 
>  /**
> @@ -1648,6 +1657,9 @@ MpInitLibInitialize (
>    //
>    CpuMpData->ApLoopMode = ApLoopMode;
>    DEBUG ((DEBUG_INFO, "AP Loop Mode is %d\n", CpuMpData-
> >ApLoopMode));
> +
> +  CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode ==
> + 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 <Library/PeiServicesLib.h>
> +#include <Guid/S3SmmInitDone.h>
> +
> +/**
> +  S3 SMM Init Done notification function.
> +
> +  @param  PeiServices      Indirect reference to the PEI Services Table.
> +  @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 = {
> +  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
> +  )
> +{
> +}
> +
> +/**
> +  S3 SMM Init Done notification function.
> +
> +  @param  PeiServices      Indirect reference to the PEI Services Table.
> +  @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 = 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 != ApInHltLoop) {
> +    CpuMpData->WakeUpByInitSipiSipi = 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 = PeiServicesNotifyPpi (&mS3SmmInitDoneNotifyDesc);
> + ASSERT_EFI_ERROR (Status);
>  }
> 
>  /**
> --
> 2.15.0.windows.1