Re: [PATCH 3/3] UefiCpuPkg S3ResumePei: Signal S3SmmInitDone

[Laszlo Ersek <lersek@redhat.com>]

On 03/02/18 06:15, Star Zeng wrote:
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 42 +++++++++++++++-------
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  3 ++
>  2 files changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index 4d776891a72d..2cb7b429e59c 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -24,6 +24,7 @@
>  #include <Guid/BootScriptExecutorVariable.h>
>  #include <Guid/ExtendedFirmwarePerformance.h>
>  #include <Guid/EndOfS3Resume.h>
> +#include <Guid/S3SmmInitDone.h>
>  #include <Ppi/ReadOnlyVariable2.h>
>  #include <Ppi/S3Resume2.h>
>  #include <Ppi/SmmAccess.h>
> @@ -259,6 +260,12 @@ EFI_PEI_PPI_DESCRIPTOR mPpiListEndOfPeiTable = {
>    0
>  };
>  
> +EFI_PEI_PPI_DESCRIPTOR mPpiListS3SmmInitDoneTable = {
> +  (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> +  &gEdkiiS3SmmInitDoneGuid,
> +  0
> +};
> +
>  //
>  // Global Descriptor Table (GDT)
>  //
> @@ -322,13 +329,14 @@ IsLongModeWakingVector (
>  }
>  
>  /**
> -  Send EndOfS3Resume event to SmmCore through communication buffer way.
> +  Signal to SMM through communication buffer way.
> +
> +  @param[in]  HandlerType       SMI handler type to be signaled.
>  
> -  @retval  EFI_SUCCESS                 Return send the event success.
>  **/
> -EFI_STATUS
> -SignalEndOfS3Resume (
> -  VOID
> +VOID
> +SignalToSmmByCommunication (
> +  IN EFI_GUID   *HandlerType
>    )
>  {
>    EFI_STATUS                         Status;
> @@ -338,7 +346,7 @@ SignalEndOfS3Resume (
>    SMM_COMMUNICATE_HEADER_64          Header64;
>    VOID                               *CommBuffer;
>  
> -  DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Enter\n"));
> +  DEBUG ((DEBUG_INFO, "Signal %g to SMM - Enter\n", HandlerType));
>  
>    //
>    // This buffer consumed in DXE phase, so base on DXE mode to prepare communicate buffer.
> @@ -355,7 +363,7 @@ SignalEndOfS3Resume (
>      Header32.MessageLength = 0;
>      CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data);
>    }
> -  CopyGuid (CommBuffer, &gEdkiiEndOfS3ResumeGuid);
> +  CopyGuid (CommBuffer, HandlerType);
>  
>    Status = PeiServicesLocatePpi (
>               &gEfiPeiSmmCommunicationPpiGuid,
> @@ -365,7 +373,7 @@ SignalEndOfS3Resume (
>               );
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed (%r)!\n", Status));
> -    return Status;
> +    return;
>    }
>  
>    Status = SmmCommunicationPpi->Communicate (
> @@ -377,8 +385,8 @@ SignalEndOfS3Resume (
>      DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate return failure (%r)!\n", Status));
>    }
>  
> -  DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> -  return Status;
> +  DEBUG ((DEBUG_INFO, "Signal %g to SMM - Exit (%r)\n", HandlerType, Status));
> +  return;
>  }
>  
>  /**
> @@ -464,11 +472,11 @@ S3ResumeBootOs (
>    PERF_END_EX (NULL, "EndOfPeiPpi", NULL, 0, PERF_INMODULE_END_ID);
>  
>    //
> -  // Signal EndOfS3Resume event.
> +  // Signal EndOfS3Resume to SMM.
>    //
>    PERF_START_EX (NULL, "EndOfS3Resume", NULL, 0, PERF_INMODULE_START_ID);
>  
> -  SignalEndOfS3Resume ();
> +  SignalToSmmByCommunication (&gEdkiiEndOfS3ResumeGuid);
>  
>    PERF_END_EX (NULL, "EndOfS3Resume", NULL, 0, PERF_INMODULE_END_ID);
>  
> @@ -787,6 +795,16 @@ S3ResumeExecuteBootScript (
>          Status = SmmAccess->Lock ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
>        }
>      }
> +
> +    //
> +    // Install S3SmmInitDone PPI.
> +    //
> +    Status = PeiServicesInstallPpi (&mPpiListS3SmmInitDoneTable);
> +    ASSERT_EFI_ERROR (Status);
> +    //
> +    // Signal S3SmmInitDone to SMM.
> +    //
> +    SignalToSmmByCommunication (&gEdkiiS3SmmInitDoneGuid);
>    }
>  
>    if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> index 9522ede726f4..47fecd7d6d80 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> @@ -78,6 +78,9 @@ [Guids]
>    gEfiAcpiVariableGuid
>    gEfiAcpiS3ContextGuid                         ## SOMETIMES_CONSUMES ## UNDEFINED # LockBox
>    gEdkiiEndOfS3ResumeGuid                       ## SOMETIMES_CONSUMES ## UNDEFINED # Used to do smm communication
> +  ## SOMETIMES_PRODUCES ## UNDEFINED # Install PPI
> +  ## SOMETIMES_CONSUMES ## UNDEFINED # Used to do smm communication
> +  gEdkiiS3SmmInitDoneGuid
>  
>  [Ppis]
>    gEfiPeiReadOnlyVariable2PpiGuid               ## CONSUMES
> 

OK, this patch seems to preserve the current behavior that the SMM
communication PPI is optional.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo