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

Star Zeng posted 3 patches 6 years, 8 months ago
[edk2] [PATCH 3/3] UefiCpuPkg S3ResumePei: Signal S3SmmInitDone
Posted by Star Zeng 6 years, 8 months ago
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
-- 
2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] UefiCpuPkg S3ResumePei: Signal S3SmmInitDone
Posted by Yao, Jiewen 6 years, 8 months ago
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.yao@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 <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH 3/3] UefiCpuPkg S3ResumePei: Signal S3SmmInitDone
> 
> 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
> --
> 2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] UefiCpuPkg S3ResumePei: Signal S3SmmInitDone
Posted by Laszlo Ersek 6 years, 8 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel