[Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.

[Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.

[Eric Dong]

Add gEdkiiSmmEndOfS3ResumeProtocolGuid which used by SmmCore to
notify smm drives that S3 resume has finished.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h | 26 ++++++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                    |  3 +++
 2 files changed, 29 insertions(+)
 create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h

diff --git a/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
new file mode 100644
index 0000000..9716f6a
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
@@ -0,0 +1,26 @@
+/** @file
+  This Protocol will be installed at the end of S3 resume phase in SMM environment. 
+  It allows for smm drivers to hook this point and do the requried tasks.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  This program and the accompanying materials                          
+  are licensed and made available under the terms and conditions of the BSD License         
+  which accompanies this distribution.  The full text of the license may be found at        
+  http://opensource.org/licenses/bsd-license.php                                            
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,                     
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.             
+
+**/
+
+#ifndef __SMM_END_OF_S3_RESUME_H__
+#define __SMM_END_OF_S3_RESUME_H__
+
+#define EDKII_SMM_END_OF_S3_RESUME_PROTOCOL_GUID \
+  { \
+    0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } \
+  }
+
+extern EFI_GUID gEdkiiSmmEndOfS3ResumeProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a3c0633..216e4f9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -556,6 +556,9 @@
   ## Include/Protocol/IoMmu.h
   gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 0x1e } }
 
+  ## Include/Protocol/SmmEndofS3Resume.h
+  gEdkiiSmmEndOfS3ResumeProtocolGuid = { 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } }
+
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
-- 
2.7.0.windows.1

[Patch v3 0/3] Add SmmEndOfS3Resume event.

[Eric Dong]

This patch series add new SmmEndOfS3Resume event which required by some
SMM drivers. 
It will implmented by SmmCore to install the gEdkiiSmmEndOfS3ResumeProtocolGuid
Protocol. Smm drivers can install this protocol's notification functions to 
hoot this envet.
It will be trigged right after the EndOfPei event in S3 resume phase.

V2 Changes: 
  Only change patch 2/3
  1. Change structures name to avoid they start with EFI_.
  2. Base on DXE phase bits to provide communication buffer, current implement
     check both PEI and DXE phase.

V3 Changes:
  for 2/3 patch:UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
    1. Change structure name for better understanding.
    2. Enhance communication buffer calculate logic to more accurate.

  for 3/3 patch: MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.
    1. Uninstall the protocol right after install it to avoid run out of memory.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>

Eric Dong (3):
  MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
  UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
  MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c            | 55 ++++++++++++--
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h            | 24 ++++++
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf          |  1 +
 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h   | 31 ++++++++
 MdeModulePkg/MdeModulePkg.dec                      |  3 +
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 85 ++++++++++++++++++++++
 .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
 7 files changed, 196 insertions(+), 7 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h

-- 
2.7.0.windows.1

[Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.

[Eric Dong]

Add gEdkiiSmmEndOfS3ResumeProtocolGuid which used by SmmCore to
notify smm drives that S3 resume has finished.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h | 26 ++++++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                    |  3 +++
 2 files changed, 29 insertions(+)
 create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h

diff --git a/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
new file mode 100644
index 0000000..9716f6a
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
@@ -0,0 +1,26 @@
+/** @file
+  This Protocol will be installed at the end of S3 resume phase in SMM environment. 
+  It allows for smm drivers to hook this point and do the requried tasks.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  This program and the accompanying materials                          
+  are licensed and made available under the terms and conditions of the BSD License         
+  which accompanies this distribution.  The full text of the license may be found at        
+  http://opensource.org/licenses/bsd-license.php                                            
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,                     
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.             
+
+**/
+
+#ifndef __SMM_END_OF_S3_RESUME_H__
+#define __SMM_END_OF_S3_RESUME_H__
+
+#define EDKII_SMM_END_OF_S3_RESUME_PROTOCOL_GUID \
+  { \
+    0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } \
+  }
+
+extern EFI_GUID gEdkiiSmmEndOfS3ResumeProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a3c0633..216e4f9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -556,6 +556,9 @@
   ## Include/Protocol/IoMmu.h
   gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 0x1e } }
 
+  ## Include/Protocol/SmmEndofS3Resume.h
+  gEdkiiSmmEndOfS3ResumeProtocolGuid = { 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } }
+
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
-- 
2.7.0.windows.1

[Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.

[Eric Dong]

Driver will send S3 resume finished event to SmmCore through communicate
buffer after it signals EndOfPei event.

V2 Changes:
1. Change structures name to avoid they start with EFI_.
2. Base on DXE phase bits to provide communication buffer, current implement
check both PEI and DXE phase.

V3 Changes:
1. Change structure name for better understanding.
2. Enhance communication buffer calculate logic to more accurate.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 87 ++++++++++++++++++++++
 .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
 2 files changed, 91 insertions(+)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index e53ed21..c2171cb 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -28,6 +28,9 @@
 #include <Ppi/SmmAccess.h>
 #include <Ppi/PostBootScriptTable.h>
 #include <Ppi/EndOfPeiPhase.h>
+#include <Ppi/SmmCommunication.h>
+
+#include <Protocol/SmmEndOfS3Resume.h>
 
 #include <Library/DebugLib.h>
 #include <Library/BaseLib.h>
@@ -151,6 +154,22 @@ typedef union {
   UINT64    Uint64;
 } PAGE_TABLE_1G_ENTRY;
 
+//
+// Define two type of smm communicate headers.
+// One for 32 bits  PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits DXE case.
+//
+typedef struct {
+  EFI_GUID  HeaderGuid;
+  UINT32    MessageLength;
+  UINT8     Data[1];
+} SMM_COMMUNICATE_HEADER_32;
+
+typedef struct {
+  EFI_GUID  HeaderGuid;
+  UINT64    MessageLength;
+  UINT8     Data[1];
+} SMM_COMMUNICATE_HEADER_64;
+
 #pragma pack()
 
 //
@@ -430,6 +449,68 @@ IsLongModeWakingVector (
 }
 
 /**
+  Send EndOfS3Resume event to SmmCore through communication buffer way.
+
+  @retval  EFI_SUCCESS                 Return send the event success.
+**/
+EFI_STATUS
+SignalEndOfS3Resume (
+  VOID
+  )
+{
+  EFI_STATUS                         Status;
+  EFI_PEI_SMM_COMMUNICATION_PPI      *SmmCommunicationPpi;
+  UINTN                              CommSize;
+  SMM_COMMUNICATE_HEADER_32          Header32;
+  SMM_COMMUNICATE_HEADER_64          Header64;
+  VOID                               *CommBuffer;
+
+  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
+
+  //
+  // This buffer consumed in DXE phase, so base on DXE mode to prepare communicate buffer.
+  // Detect whether DXE is 64 bits mode.
+  // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE also 64 bits.
+  // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64 bits.
+  //
+  if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet (PcdDxeIplSwitchToLongMode))) {
+    CommBuffer = &Header64;
+    Header64.MessageLength = 0;
+    CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_64, Data);
+  } else {
+    CommBuffer = &Header32;
+    Header32.MessageLength = 0;
+    CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data);
+  }
+  CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
+
+  //
+  // Get needed resource
+  //
+  Status = PeiServicesLocatePpi (
+             &gEfiPeiSmmCommunicationPpiGuid,
+             0,
+             NULL,
+             (VOID **)&SmmCommunicationPpi
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Send command
+  //
+  Status = SmmCommunicationPpi->Communicate (
+                                  SmmCommunicationPpi,
+                                  (VOID *)CommBuffer,
+                                  &CommSize
+                                  );
+  ASSERT_EFI_ERROR (Status);
+
+  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
+
+  return Status;
+}
+
+/**
   Jump to OS waking vector.
   The function will install boot script done PPI, report S3 resume status code, and then jump to OS waking vector.
 
@@ -504,6 +585,12 @@ S3ResumeBootOs (
   ASSERT_EFI_ERROR (Status);
 
   //
+  // Signal EndOfS3Resume event.
+  //
+  Status = SignalEndOfS3Resume ();
+  ASSERT_EFI_ERROR (Status);
+
+  //
   // report status code on S3 resume
   //
   REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_PC_OS_WAKE);
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
index d514523..943f114 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
@@ -85,6 +85,10 @@
   gPeiSmmAccessPpiGuid                          ## SOMETIMES_CONSUMES
   gPeiPostScriptTablePpiGuid                    ## SOMETIMES_PRODUCES
   gEfiEndOfPeiSignalPpiGuid                     ## SOMETIMES_PRODUCES
+  gEfiPeiSmmCommunicationPpiGuid                ## SOMETIMES_CONSUMES
+
+[Protocols]
+  gEdkiiSmmEndOfS3ResumeProtocolGuid            ## SOMETIMES_CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode         ## CONSUMES
-- 
2.7.0.windows.1

[Patch v3 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.

[Eric Dong]

Install EdkiiSmmEndOfS3ResumeProtocol when S3 resume finished.
S3ResumePei will send S3 resume finished event to SmmCore through
communication buffer.

V2 change:
 None.

V3 change:
1. Uninstall the protocol right after install it to avoid run out of memory.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c   | 69 +++++++++++++++++++++++++++----
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h   | 24 +++++++++++
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf |  1 +
 3 files changed, 87 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 9e4390e..b833763 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -75,13 +75,14 @@ BOOLEAN  mInLegacyBoot = FALSE;
 // Table of SMI Handlers that are registered by the SMM Core when it is initialized
 //
 SMM_CORE_SMI_HANDLERS  mSmmCoreSmiHandlers[] = {
-  { SmmDriverDispatchHandler,   &gEfiEventDxeDispatchGuid,          NULL, TRUE  },
-  { SmmReadyToLockHandler,      &gEfiDxeSmmReadyToLockProtocolGuid, NULL, TRUE }, 
-  { SmmLegacyBootHandler,       &gEfiEventLegacyBootGuid,           NULL, FALSE },
-  { SmmExitBootServicesHandler, &gEfiEventExitBootServicesGuid,     NULL, FALSE },
-  { SmmReadyToBootHandler,      &gEfiEventReadyToBootGuid,          NULL, FALSE },
-  { SmmEndOfDxeHandler,         &gEfiEndOfDxeEventGroupGuid,        NULL, TRUE },
-  { NULL,                       NULL,                               NULL, FALSE }
+  { SmmDriverDispatchHandler,   &gEfiEventDxeDispatchGuid,           NULL, TRUE  },
+  { SmmReadyToLockHandler,      &gEfiDxeSmmReadyToLockProtocolGuid,  NULL, TRUE }, 
+  { SmmLegacyBootHandler,       &gEfiEventLegacyBootGuid,            NULL, FALSE },
+  { SmmExitBootServicesHandler, &gEfiEventExitBootServicesGuid,      NULL, FALSE },
+  { SmmReadyToBootHandler,      &gEfiEventReadyToBootGuid,           NULL, FALSE },
+  { SmmEndOfDxeHandler,         &gEfiEndOfDxeEventGroupGuid,         NULL, TRUE },
+  { SmmEndOfS3ResumeHandler,    &gEdkiiSmmEndOfS3ResumeProtocolGuid, NULL, FALSE },
+  { NULL,                       NULL,                                NULL, FALSE }
 };
 
 UINTN                           mFullSmramRangeCount;
@@ -383,6 +384,60 @@ SmmEndOfDxeHandler (
 }
 
 /**
+  Software SMI handler that is called when the EndOfS3Resume event is trigged.
+  This function installs the SMM EndOfS3Resume Protocol so SMM Drivers are informed that
+  S3 resume has finished.
+
+  @param  DispatchHandle  The unique handle assigned to this handler by SmiHandlerRegister().
+  @param  Context         Points to an optional handler context which was specified when the handler was registered.
+  @param  CommBuffer      A pointer to a collection of data in memory that will
+                          be conveyed from a non-SMM environment into an SMM environment.
+  @param  CommBufferSize  The size of the CommBuffer.
+
+  @return Status Code
+
+**/
+EFI_STATUS
+EFIAPI
+SmmEndOfS3ResumeHandler (
+  IN     EFI_HANDLE  DispatchHandle,
+  IN     CONST VOID  *Context,        OPTIONAL
+  IN OUT VOID        *CommBuffer,     OPTIONAL
+  IN OUT UINTN       *CommBufferSize  OPTIONAL
+  )
+{
+  EFI_STATUS  Status;
+  EFI_HANDLE  SmmHandle;
+
+  DEBUG ((EFI_D_INFO, "SmmEndOfS3ResumeHandler\n"));
+
+  //
+  // Install SMM EndOfS3Resume protocol
+  //
+  SmmHandle = NULL;
+  Status = SmmInstallProtocolInterface (
+             &SmmHandle,
+             &gEdkiiSmmEndOfS3ResumeProtocolGuid,
+             EFI_NATIVE_INTERFACE,
+             NULL
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Uninstall the protocol here because the comsume just hook the
+  // installation event.
+  //
+  Status = SmmUninstallProtocolInterface (
+           SmmHandle,
+           &gEdkiiSmmEndOfS3ResumeProtocolGuid,
+           NULL
+           );
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
+
+/**
   Determine if two buffers overlap in memory.
 
   @param[in] Buff1  Pointer to first buffer
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index b6f815c..6cc824b 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -32,6 +32,7 @@
 #include <Protocol/SmmExitBootServices.h>
 #include <Protocol/SmmLegacyBoot.h>
 #include <Protocol/SmmReadyToBoot.h>
+#include <Protocol/SmmEndOfS3Resume.h>
 
 #include <Guid/Apriori.h>
 #include <Guid/EventGroup.h>
@@ -802,6 +803,29 @@ SmmReadyToBootHandler (
   );
 
 /**
+  Software SMI handler that is called when the EndOfS3Resume event is trigged.
+  This function installs the SMM EndOfS3Resume Protocol so SMM Drivers are informed that
+  S3 resume has finished.
+
+  @param  DispatchHandle  The unique handle assigned to this handler by SmiHandlerRegister().
+  @param  Context         Points to an optional handler context which was specified when the handler was registered.
+  @param  CommBuffer      A pointer to a collection of data in memory that will
+                          be conveyed from a non-SMM environment into an SMM environment.
+  @param  CommBufferSize  The size of the CommBuffer.
+
+  @return Status Code
+
+**/
+EFI_STATUS
+EFIAPI
+SmmEndOfS3ResumeHandler (
+  IN     EFI_HANDLE  DispatchHandle,
+  IN     CONST VOID  *Context,        OPTIONAL
+  IN OUT VOID        *CommBuffer,     OPTIONAL
+  IN OUT UINTN       *CommBufferSize  OPTIONAL
+  );
+
+/**
   Place holder function until all the SMM System Table Service are available.
 
   @param  Arg1                   Undefined
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
index 95e34bd..a724189 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
@@ -79,6 +79,7 @@
   gEdkiiSmmExitBootServicesProtocolGuid         ## SOMETIMES_PRODUCES
   gEdkiiSmmLegacyBootProtocolGuid               ## SOMETIMES_PRODUCES
   gEdkiiSmmReadyToBootProtocolGuid              ## PRODUCES
+  gEdkiiSmmEndOfS3ResumeProtocolGuid            ## SOMETIMES_PRODUCES
 
   gEfiSmmSwDispatch2ProtocolGuid                ## SOMETIMES_CONSUMES
   gEfiSmmSxDispatch2ProtocolGuid                ## SOMETIMES_CONSUMES
-- 
2.7.0.windows.1

Re: [Patch v3 0/3] Add SmmEndOfS3Resume event.

[Yao, Jiewen]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 4:22 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [Patch v3 0/3] Add SmmEndOfS3Resume event.
> 
> This patch series add new SmmEndOfS3Resume event which required by some
> SMM drivers.
> It will implmented by SmmCore to install the
> gEdkiiSmmEndOfS3ResumeProtocolGuid
> Protocol. Smm drivers can install this protocol's notification functions to
> hoot this envet.
> It will be trigged right after the EndOfPei event in S3 resume phase.
> 
> V2 Changes:
>   Only change patch 2/3
>   1. Change structures name to avoid they start with EFI_.
>   2. Base on DXE phase bits to provide communication buffer, current implement
>      check both PEI and DXE phase.
> 
> V3 Changes:
>   for 2/3 patch:UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to
> SmmCore.
>     1. Change structure name for better understanding.
>     2. Enhance communication buffer calculate logic to more accurate.
> 
>   for 3/3 patch: MdeModulePkg/PiSmmCore: Install Protocol when S3 resume
> finished.
>     1. Uninstall the protocol right after install it to avoid run out of memory.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> 
> Eric Dong (3):
>   MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
>   UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
>   MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.
> 
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c            | 55
> ++++++++++++--
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h            | 24 ++++++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf          |  1 +
>  MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h   | 31 ++++++++
>  MdeModulePkg/MdeModulePkg.dec                      |  3 +
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 85
> ++++++++++++++++++++++
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
>  7 files changed, 196 insertions(+), 7 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
> 
> --
> 2.7.0.windows.1

Re: [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.

[Laszlo Ersek]

Hi,

I'm sorry that I couldn't comment on this patch in time.

However, I must observe that version 1 of the patch,

http://mid.mail-archive.com/1507688554-10264-3-git-send-email-eric.dong@intel.com

was sent out at 04:22 AM on 11 Oct, in my time zone; and version 3 of
the patch was committed at 04:17 AM on 12 Oct in my time zone.

That's not a lot of time for reviewers to comment -- I was very busy
with other stuff yesterday. I even set up an auto-reply about that,
asking for a bit of patience.

Of course the auto-reply didn't help, because I wasn't CC'd on the patch
in the first place; despite my earlier request to be CC'd on all SMM-
and/or S3-related patches.

So, after this introduction:

On 10/11/17 10:22, Eric Dong wrote:
> Driver will send S3 resume finished event to SmmCore through communicate
> buffer after it signals EndOfPei event.
> 
> V2 Changes:
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current implement
> check both PEI and DXE phase.
> 
> V3 Changes:
> 1. Change structure name for better understanding.
> 2. Enhance communication buffer calculate logic to more accurate.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 87 ++++++++++++++++++++++
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
>  2 files changed, 91 insertions(+)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index e53ed21..c2171cb 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -28,6 +28,9 @@
>  #include <Ppi/SmmAccess.h>
>  #include <Ppi/PostBootScriptTable.h>
>  #include <Ppi/EndOfPeiPhase.h>
> +#include <Ppi/SmmCommunication.h>
> +
> +#include <Protocol/SmmEndOfS3Resume.h>
>  
>  #include <Library/DebugLib.h>
>  #include <Library/BaseLib.h>
> @@ -151,6 +154,22 @@ typedef union {
>    UINT64    Uint64;
>  } PAGE_TABLE_1G_ENTRY;
>  
> +//
> +// Define two type of smm communicate headers.
> +// One for 32 bits  PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits DXE case.
> +//
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT32    MessageLength;
> +  UINT8     Data[1];
> +} SMM_COMMUNICATE_HEADER_32;
> +
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT64    MessageLength;
> +  UINT8     Data[1];
> +} SMM_COMMUNICATE_HEADER_64;
> +
>  #pragma pack()
>  
>  //
> @@ -430,6 +449,68 @@ IsLongModeWakingVector (
>  }
>  
>  /**
> +  Send EndOfS3Resume event to SmmCore through communication buffer way.
> +
> +  @retval  EFI_SUCCESS                 Return send the event success.
> +**/
> +EFI_STATUS
> +SignalEndOfS3Resume (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                         Status;
> +  EFI_PEI_SMM_COMMUNICATION_PPI      *SmmCommunicationPpi;
> +  UINTN                              CommSize;
> +  SMM_COMMUNICATE_HEADER_32          Header32;
> +  SMM_COMMUNICATE_HEADER_64          Header64;
> +  VOID                               *CommBuffer;
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
> +
> +  //
> +  // This buffer consumed in DXE phase, so base on DXE mode to prepare communicate buffer.
> +  // Detect whether DXE is 64 bits mode.
> +  // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE also 64 bits.
> +  // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64 bits.
> +  //
> +  if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet (PcdDxeIplSwitchToLongMode))) {
> +    CommBuffer = &Header64;
> +    Header64.MessageLength = 0;
> +    CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_64, Data);
> +  } else {
> +    CommBuffer = &Header32;
> +    Header32.MessageLength = 0;
> +    CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data);
> +  }
> +  CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
> +
> +  //
> +  // Get needed resource
> +  //
> +  Status = PeiServicesLocatePpi (
> +             &gEfiPeiSmmCommunicationPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **)&SmmCommunicationPpi
> +             );
> +  ASSERT_EFI_ERROR (Status);

This patch breaks S3 resume with OVMF+SMM. OVMF does not have or install
an EFI_PEI_SMM_COMMUNICATION_PPI. We discussed this ~2 years ago, when
SMM was being added to KVM, QEMU, and OVMF.

OVMF would have needed EFI_PEI_SMM_COMMUNICATION_PPI only for
SmmLockBoxPeiLib. But we agreed that SmmLockBoxPeiLib would work without
this PPI; exactly the same as if the PPI existed, but Communicate()
returned EFI_NOT_STARTED.

Namely, if RestoreLockBox() failed to locate
EFI_PEI_SMM_COMMUNICATION_PPI, then InternalRestoreLockBoxFromSmram()
would be called instead, which only needed PEI_SMM_ACCESS_PPI. And OVMF
would provide that, so everything would work fine.

Please see the following edk2 commits:

  https://github.com/tianocore/edk2/commit/bd3afeb1d62c
  https://github.com/tianocore/edk2/commit/9d560947f6d3

Eric, can you please modify SignalEndOfS3Resume() as follows:

- locate EFI_PEI_SMM_COMMUNICATION_PPI as first action,
- if it is not found, return EFI_NOT_FOUND gracefully,
- in S3ResumeBootOs(), simply log the return value, and proceed.

Currently, during S3 resume (with SMM), I get

> SignalEndOfS3Resume - Enter

> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c(496):
!EFI_ERROR (Status)

Thanks,
Laszlo

> +
> +  //
> +  // Send command
> +  //
> +  Status = SmmCommunicationPpi->Communicate (
> +                                  SmmCommunicationPpi,
> +                                  (VOID *)CommBuffer,
> +                                  &CommSize
> +                                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> +
> +  return Status;
> +}
> +
> +/**
>    Jump to OS waking vector.
>    The function will install boot script done PPI, report S3 resume status code, and then jump to OS waking vector.
>  
> @@ -504,6 +585,12 @@ S3ResumeBootOs (
>    ASSERT_EFI_ERROR (Status);
>  
>    //
> +  // Signal EndOfS3Resume event.
> +  //
> +  Status = SignalEndOfS3Resume ();
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
>    // report status code on S3 resume
>    //
>    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_PC_OS_WAKE);
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> index d514523..943f114 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> @@ -85,6 +85,10 @@
>    gPeiSmmAccessPpiGuid                          ## SOMETIMES_CONSUMES
>    gPeiPostScriptTablePpiGuid                    ## SOMETIMES_PRODUCES
>    gEfiEndOfPeiSignalPpiGuid                     ## SOMETIMES_PRODUCES
> +  gEfiPeiSmmCommunicationPpiGuid                ## SOMETIMES_CONSUMES
> +
> +[Protocols]
> +  gEdkiiSmmEndOfS3ResumeProtocolGuid            ## SOMETIMES_CONSUMES
>  
>  [FeaturePcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode         ## CONSUMES
> 

Re: [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.

[Dong, Eric]

Hi Laszlo,

Sorry for this patch break OVMF. Platform without this Ppi is not what we expected before. So we just add ASSERT here. 

I will send a patch ASAP to fix this ASSERT case.   Also we will provide another patch later to handle the platform without this Ppi case.

Thanks,
Eric

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Thursday, October 12, 2017 4:01 PM
To: Dong, Eric <eric.dong@intel.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [edk2] [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.

Hi,

I'm sorry that I couldn't comment on this patch in time.

However, I must observe that version 1 of the patch,

http://mid.mail-archive.com/1507688554-10264-3-git-send-email-eric.dong@intel.com

was sent out at 04:22 AM on 11 Oct, in my time zone; and version 3 of the patch was committed at 04:17 AM on 12 Oct in my time zone.

That's not a lot of time for reviewers to comment -- I was very busy with other stuff yesterday. I even set up an auto-reply about that, asking for a bit of patience.

Of course the auto-reply didn't help, because I wasn't CC'd on the patch in the first place; despite my earlier request to be CC'd on all SMM- and/or S3-related patches.

So, after this introduction:

On 10/11/17 10:22, Eric Dong wrote:
> Driver will send S3 resume finished event to SmmCore through 
> communicate buffer after it signals EndOfPei event.
> 
> V2 Changes:
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current 
> implement check both PEI and DXE phase.
> 
> V3 Changes:
> 1. Change structure name for better understanding.
> 2. Enhance communication buffer calculate logic to more accurate.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c  | 87 ++++++++++++++++++++++
>  .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf   |  4 +
>  2 files changed, 91 insertions(+)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c 
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index e53ed21..c2171cb 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -28,6 +28,9 @@
>  #include <Ppi/SmmAccess.h>
>  #include <Ppi/PostBootScriptTable.h>
>  #include <Ppi/EndOfPeiPhase.h>
> +#include <Ppi/SmmCommunication.h>
> +
> +#include <Protocol/SmmEndOfS3Resume.h>
>  
>  #include <Library/DebugLib.h>
>  #include <Library/BaseLib.h>
> @@ -151,6 +154,22 @@ typedef union {
>    UINT64    Uint64;
>  } PAGE_TABLE_1G_ENTRY;
>  
> +//
> +// Define two type of smm communicate headers.
> +// One for 32 bits  PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits DXE case.
> +//
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT32    MessageLength;
> +  UINT8     Data[1];
> +} SMM_COMMUNICATE_HEADER_32;
> +
> +typedef struct {
> +  EFI_GUID  HeaderGuid;
> +  UINT64    MessageLength;
> +  UINT8     Data[1];
> +} SMM_COMMUNICATE_HEADER_64;
> +
>  #pragma pack()
>  
>  //
> @@ -430,6 +449,68 @@ IsLongModeWakingVector (  }
>  
>  /**
> +  Send EndOfS3Resume event to SmmCore through communication buffer way.
> +
> +  @retval  EFI_SUCCESS                 Return send the event success.
> +**/
> +EFI_STATUS
> +SignalEndOfS3Resume (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                         Status;
> +  EFI_PEI_SMM_COMMUNICATION_PPI      *SmmCommunicationPpi;
> +  UINTN                              CommSize;
> +  SMM_COMMUNICATE_HEADER_32          Header32;
> +  SMM_COMMUNICATE_HEADER_64          Header64;
> +  VOID                               *CommBuffer;
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
> +
> +  //
> +  // This buffer consumed in DXE phase, so base on DXE mode to prepare communicate buffer.
> +  // Detect whether DXE is 64 bits mode.
> +  // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE also 64 bits.
> +  // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64 bits.
> +  //
> +  if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet (PcdDxeIplSwitchToLongMode))) {
> +    CommBuffer = &Header64;
> +    Header64.MessageLength = 0;
> +    CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_64, Data);  } else {
> +    CommBuffer = &Header32;
> +    Header32.MessageLength = 0;
> +    CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data);  }  
> + CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
> +
> +  //
> +  // Get needed resource
> +  //
> +  Status = PeiServicesLocatePpi (
> +             &gEfiPeiSmmCommunicationPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **)&SmmCommunicationPpi
> +             );
> +  ASSERT_EFI_ERROR (Status);

This patch breaks S3 resume with OVMF+SMM. OVMF does not have or install an EFI_PEI_SMM_COMMUNICATION_PPI. We discussed this ~2 years ago, when SMM was being added to KVM, QEMU, and OVMF.

OVMF would have needed EFI_PEI_SMM_COMMUNICATION_PPI only for SmmLockBoxPeiLib. But we agreed that SmmLockBoxPeiLib would work without this PPI; exactly the same as if the PPI existed, but Communicate() returned EFI_NOT_STARTED.

Namely, if RestoreLockBox() failed to locate EFI_PEI_SMM_COMMUNICATION_PPI, then InternalRestoreLockBoxFromSmram() would be called instead, which only needed PEI_SMM_ACCESS_PPI. And OVMF would provide that, so everything would work fine.

Please see the following edk2 commits:

  https://github.com/tianocore/edk2/commit/bd3afeb1d62c
  https://github.com/tianocore/edk2/commit/9d560947f6d3

Eric, can you please modify SignalEndOfS3Resume() as follows:

- locate EFI_PEI_SMM_COMMUNICATION_PPI as first action,
- if it is not found, return EFI_NOT_FOUND gracefully,
- in S3ResumeBootOs(), simply log the return value, and proceed.

Currently, during S3 resume (with SMM), I get

> SignalEndOfS3Resume - Enter

> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c(496):
!EFI_ERROR (Status)

Thanks,
Laszlo

> +
> +  //
> +  // Send command
> +  //
> +  Status = SmmCommunicationPpi->Communicate (
> +                                  SmmCommunicationPpi,
> +                                  (VOID *)CommBuffer,
> +                                  &CommSize
> +                                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> +
> +  return Status;
> +}
> +
> +/**
>    Jump to OS waking vector.
>    The function will install boot script done PPI, report S3 resume status code, and then jump to OS waking vector.
>  
> @@ -504,6 +585,12 @@ S3ResumeBootOs (
>    ASSERT_EFI_ERROR (Status);
>  
>    //
> +  // Signal EndOfS3Resume event.
> +  //
> +  Status = SignalEndOfS3Resume ();
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
>    // report status code on S3 resume
>    //
>    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE | 
> EFI_SW_PEI_PC_OS_WAKE); diff --git 
> a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf 
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> index d514523..943f114 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> @@ -85,6 +85,10 @@
>    gPeiSmmAccessPpiGuid                          ## SOMETIMES_CONSUMES
>    gPeiPostScriptTablePpiGuid                    ## SOMETIMES_PRODUCES
>    gEfiEndOfPeiSignalPpiGuid                     ## SOMETIMES_PRODUCES
> +  gEfiPeiSmmCommunicationPpiGuid                ## SOMETIMES_CONSUMES
> +
> +[Protocols]
> +  gEdkiiSmmEndOfS3ResumeProtocolGuid            ## SOMETIMES_CONSUMES
>  
>  [FeaturePcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode         ## CONSUMES
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel