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

2018-03-02 Thread Laszlo Ersek
On 03/02/18 06:15, Star Zeng wrote:
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  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 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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),
> +  ,
> +  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, );
> +  CopyGuid (CommBuffer, HandlerType);
>  
>Status = PeiServicesLocatePpi (
>   ,
> @@ -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 ();
>  
>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 ();
> +ASSERT_EFI_ERROR (Status);
> +//
> +// Signal S3SmmInitDone to SMM.
> +//
> +SignalToSmmByCommunication ();
>}
>  
>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 

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org

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

2018-03-01 Thread Yao, Jiewen
Hi Star
Can we keep the human readable debug message?

-  DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Enter\n"));
+  DEBUG ((DEBUG_INFO, "Signal %g to SMM - Enter\n", HandlerType));

Other update is good to me.
Reviewed-by: jiewen@intel.com


Thank you
Yao Jiewen


> -Original Message-
> From: Zeng, Star
> Sent: Friday, March 2, 2018 1:16 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Dong, Eric ; Laszlo Ersek 
> Subject: [PATCH 3/3] UefiCpuPkg S3ResumePei: Signal S3SmmInitDone
> 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  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 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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),
> +  ,
> +  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, );
> +  CopyGuid (CommBuffer, HandlerType);
> 
>Status = PeiServicesLocatePpi (
>   ,
> @@ -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 ();
> 
>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 ();
> +ASSERT_EFI_ERROR (Status);
> +//
> +// Signal S3SmmInitDone to SMM.
> +//
> +SignalToSmmByCommunication ();
>}
> 
>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 #