[edk2-devel][edk2-platforms][PATCH v1 3/5] S3FeaturePkg: Implement working S3 resume

["Benjamin Doron" <benjamin.doron00@gmail.com>]

Follow-up commits to MinPlatform (PeiFspWrapperHobProcessLib for
memory) and FSP-related board libraries (policy overrides)
required for successful S3 resume.

Factored allocation logic into new module to avoid MinPlatform
dependency on S3Feature package.

TODO: Can optimise required size.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ankit Sinha <ankit.sinha@intel.com>
Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../S3FeaturePkg/Include/PostMemory.fdf       |  15 ++
 .../S3FeaturePkg/Include/PreMemory.fdf        |   8 +-
 .../S3FeaturePkg/Include/S3Feature.dsc        |  42 ++++-
 .../S3FeaturePkg/S3Dxe/S3Dxe.c                | 156 ++++++++++++++++++
 .../S3FeaturePkg/S3Dxe/S3Dxe.inf              |  49 ++++++
 .../S3FeaturePkg/S3Pei/S3Pei.c                |  83 +++++++++-
 .../S3FeaturePkg/S3Pei/S3Pei.inf              |   8 +-
 .../Include/AcpiS3MemoryNvData.h              |  22 +++
 8 files changed, 375 insertions(+), 8 deletions(-)
 create mode 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c
 create mode 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.inf
 create mode 100644 Platform/Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h

diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf
index 9e17f853c630..5d3d96f4f317 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf
@@ -2,7 +2,22 @@
 #  FDF file for post-memory S3 advanced feature modules.
 #
 # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2022, Baruch Binyamin Doron.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
+
+## Dependencies
+  INF UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
+  INF MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
+
+## Save-state module stack
+  INF S3FeaturePkg/S3Dxe/S3Dxe.inf
+  INF MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
+  # FSP may perform CPU finalisation, requires CpuInitDxe from closed code
+  # - Presently, PiSmmCpuDxeSmm shall perform finalisation with this data
+  INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
+
+## Restore-state module stack
+  INF MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PreMemory.fdf b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PreMemory.fdf
index fdd16a4e0356..e130fa5f098d 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PreMemory.fdf
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PreMemory.fdf
@@ -2,9 +2,15 @@
 #  FDF file for pre-memory S3 advanced feature modules.
 #
 # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2022, Baruch Binyamin Doron.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
 
-INF S3FeaturePkg/S3Pei/S3Pei.inf
+## Dependencies
+  INF S3FeaturePkg/S3Pei/S3Pei.inf
+  INF UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
+
+## Restore-state module stack
+  INF UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc b/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc
index cc34e785076a..bf45b56258ff 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc
@@ -7,6 +7,7 @@
 # for the build infrastructure.
 #
 # Copyright (c) 2019 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2022, Baruch Binyamin Doron.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -25,6 +26,10 @@
     !error "DXE_ARCH must be specified to build this feature!"
   !endif
 
+[PcdsFixedAtBuild]
+  # Attempts to improve performance at the cost of more DRAM usage
+  gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnS3Boot|TRUE
+
 ################################################################################
 #
 # Library Class section - list of all Library Classes needed by this feature.
@@ -32,7 +37,15 @@
 ################################################################################
 
 [LibraryClasses.common.PEIM]
-  SmmAccessLib|IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.inf
+  #SmmAccessLib|IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.inf
+  SmmControlLib|IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.inf
+  #IntelCompatShimLib|$(PLATFORM_SI_PACKAGE)/Library/BaseIntelCompatShimLibKbl/BaseIntelCompatShimLibKbl.inf
+
+[LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_SMM_DRIVER]
+  #######################################
+  # Edk2 Packages
+  #######################################
+  S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
 
 ################################################################################
 #
@@ -65,3 +78,30 @@
 
   # Add components here that should be included in the package build.
   S3FeaturePkg/S3Pei/S3Pei.inf
+  UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
+  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
+
+#
+# Feature DXE Components
+#
+
+# @todo: Change below line to [Components.$(DXE_ARCH)] after https://bugzilla.tianocore.org/show_bug.cgi?id=2308
+#        is completed.
+[Components.X64]
+  #####################################
+  # S3 Feature Package
+  #####################################
+
+  # Add library instances here that are not included in package components and should be tested
+  # in the package build.
+
+  # Add components here that should be included in the package build.
+  UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
+  MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
+  S3FeaturePkg/S3Dxe/S3Dxe.inf
+  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
+  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
+  # NOTE: RSC will be after end-of-BS, use DebugLibSerialPort
+  # - No gBS in SerialPortInitialize()
+  # - No global assigns after ReadyToLock possible, due to LockBox copy
+  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c b/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c
new file mode 100644
index 000000000000..b3fb63e2bc33
--- /dev/null
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c
@@ -0,0 +1,156 @@
+/** @file
+  Source code file for S3 DXE module
+
+Copyright (c) 2022, Baruch Binyamin Doron.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiDxe.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Guid/AcpiS3Context.h>
+#include <Guid/MemoryTypeInformation.h>
+#include <AcpiS3MemoryNvData.h>
+
+/**
+  Get the mem size in memory type infromation table.
+
+  @return the mem size in memory type infromation table.
+**/
+UINT64
+EFIAPI
+GetMemorySizeInMemoryTypeInformation (
+  VOID
+  )
+{
+  EFI_STATUS                  Status;
+  EFI_MEMORY_TYPE_INFORMATION *MemoryData;
+  UINT8                       Index;
+  UINTN                       TempPageNum;
+
+  Status = EfiGetSystemConfigurationTable (&gEfiMemoryTypeInformationGuid, (VOID **) &MemoryData);
+
+  if (EFI_ERROR (Status) || MemoryData == NULL) {
+    return 0;
+  }
+
+  TempPageNum = 0;
+  for (Index = 0; MemoryData[Index].Type != EfiMaxMemoryType; Index++) {
+    //
+    // Accumulate default memory size requirements
+    //
+    TempPageNum += MemoryData[Index].NumberOfPages;
+  }
+
+  return TempPageNum * EFI_PAGE_SIZE;
+}
+
+/**
+  Get the mem size need to be consumed and reserved for PEI phase resume.
+
+  @return the mem size to be reserved for PEI phase resume.
+**/
+UINT64
+EFIAPI
+GetPeiMemSize (
+  VOID
+  )
+{
+  #define PEI_ADDITIONAL_MEMORY_SIZE    (16 * EFI_PAGE_SIZE)
+
+  UINT64                      Size;
+
+  Size = GetMemorySizeInMemoryTypeInformation ();
+
+  return PcdGet32 (PcdPeiMinMemSize) + Size + PEI_ADDITIONAL_MEMORY_SIZE;
+}
+
+/**
+  Allocate EfiACPIMemoryNVS below 4G memory address.
+
+  This function allocates EfiACPIMemoryNVS below 4G memory address.
+
+  @param  Size         Size of memory to allocate.
+
+  @return Allocated address for output.
+
+**/
+VOID *
+EFIAPI
+AllocateAcpiNvsMemoryBelow4G (
+  IN   UINTN  Size
+  )
+{
+  UINTN                 Pages;
+  EFI_PHYSICAL_ADDRESS  Address;
+  EFI_STATUS            Status;
+  VOID                  *Buffer;
+
+  Pages   = EFI_SIZE_TO_PAGES (Size);
+  Address = 0xffffffff;
+
+  Status = gBS->AllocatePages (
+                  AllocateMaxAddress,
+                  EfiACPIMemoryNVS,
+                  Pages,
+                  &Address
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  Buffer = (VOID *)(UINTN)Address;
+  ZeroMem (Buffer, Size);
+
+  return Buffer;
+}
+
+/**
+  Allocates memory to use on S3 resume.
+
+  @param[in]  ImageHandle          Not used.
+  @param[in]  SystemTable          General purpose services available to every DXE driver.
+
+  @retval     EFI_SUCCESS          The function completes successfully
+  @retval     EFI_OUT_OF_RESOURCES Insufficient resources to create database
+**/
+EFI_STATUS
+EFIAPI
+S3DxeEntryPoint (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  UINT64          S3PeiMemSize;
+  UINT64          S3PeiMemBase;
+  ACPI_S3_MEMORY  S3MemoryInfo;
+  EFI_STATUS      Status;
+
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
+  S3PeiMemSize = GetPeiMemSize ();
+  S3PeiMemBase = (UINTN) AllocateAcpiNvsMemoryBelow4G (S3PeiMemSize);
+  ASSERT (S3PeiMemBase != 0);
+
+  S3MemoryInfo.S3PeiMemBase = S3PeiMemBase;
+  S3MemoryInfo.S3PeiMemSize = S3PeiMemSize;
+
+  DEBUG ((DEBUG_INFO, "S3PeiMemBase: 0x%x\n", S3PeiMemBase));
+  DEBUG ((DEBUG_INFO, "S3PeiMemSize: 0x%x\n", S3PeiMemSize));
+
+  // TODO: LockBox is potentially superior, though requires static location
+  Status = gRT->SetVariable (
+                  ACPI_S3_MEMORY_NV_NAME,
+                  &gEfiAcpiVariableGuid,
+                  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+                  sizeof (S3MemoryInfo),
+                  &S3MemoryInfo
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
+  return EFI_SUCCESS;
+}
diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.inf b/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.inf
new file mode 100644
index 000000000000..28589c2c869b
--- /dev/null
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.inf
@@ -0,0 +1,49 @@
+### @file
+# Component information file for the S3 DXE module.
+#
+# Copyright (c) 2022, Baruch Binyamin Doron.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+###
+
+[Defines]
+  INF_VERSION       = 0x00010017
+  BASE_NAME         = S3Dxe
+  FILE_GUID         = 30926F92-CC83-4381-9F70-AC96EDB5BEE0
+  VERSION_STRING    = 1.0
+  MODULE_TYPE       = DXE_DRIVER
+  ENTRY_POINT       = S3DxeEntryPoint
+
+[LibraryClasses]
+  UefiDriverEntryPoint
+  UefiBootServicesTableLib
+  UefiRuntimeServicesTableLib
+  BaseMemoryLib
+  DebugLib
+  PcdLib
+  UefiLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MinPlatformPkg/MinPlatformPkg.dec
+  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+  S3FeaturePkg/S3FeaturePkg.dec
+
+[Sources]
+  S3Dxe.c
+
+[Pcd]
+  gIntelFsp2WrapperTokenSpaceGuid.PcdPeiMinMemSize
+
+[FeaturePcd]
+  gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable
+
+[Guids]
+  gEfiMemoryTypeInformationGuid  ## CONSUMES
+  gEfiAcpiVariableGuid           ## CONSUMES
+
+[Depex]
+  gEfiVariableArchProtocolGuid      AND
+  gEfiVariableWriteArchProtocolGuid
diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.c b/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.c
index b0aaa04962c8..6acb894b6fc9 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.c
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.c
@@ -2,12 +2,87 @@
   Source code file for S3 PEI module
 
 Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2022, Baruch Binyamin Doron.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <PiPei.h>
+#include <Library/DebugLib.h>
+#include <Library/PciLib.h>
 #include <Library/PeiServicesLib.h>
 #include <Library/SmmAccessLib.h>
+#include <Library/SmmControlLib.h>
+
+// TODO: Finalise implementation factoring
+#define R_SA_PAM0  (0x80)
+#define R_SA_PAM5  (0x85)
+#define R_SA_PAM6  (0x86)
+
+/**
+  This function is called after FspSiliconInitDone installed PPI.
+  For FSP API mode, this is when FSP-M HOBs are installed into EDK2.
+
+  @param[in] PeiServices    Pointer to PEI Services Table.
+  @param[in] NotifyDesc     Pointer to the descriptor for the Notification event that
+                            caused this function to execute.
+  @param[in] Ppi            Pointer to the PPI data associated with this function.
+
+  @retval EFI_STATUS        Always return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+FspSiliconInitDoneNotify (
+  IN EFI_PEI_SERVICES           **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDesc,
+  IN VOID                       *Ppi
+  )
+{
+  EFI_STATUS     Status;
+  EFI_BOOT_MODE  BootMode;
+  UINT64         MchBaseAddress;
+
+  Status = PeiServicesGetBootMode (&BootMode);
+  ASSERT_EFI_ERROR (Status);
+
+  // Enable PAM regions for AP wakeup vector (resume)
+  // - CPU is finalised by PiSmmCpuDxeSmm, not FSP. So, it's safe here?
+  // TODO/TEST: coreboot does this unconditionally, vendor FWs may not (test resume). Should we?
+  // - It is certainly interesting that only PAM0, PAM5 and PAM6 are defined for KabylakeSiliconPkg.
+  // - Also note that 0xA0000-0xFFFFF is marked "reserved" in FSP HOB - this does not mean
+  //   that the memory is unusable, perhaps this is precisely because it will contain
+  //   the AP wakeup vector.
+  if (BootMode == BOOT_ON_S3_RESUME) {
+    MchBaseAddress = PCI_LIB_ADDRESS (0, 0, 0, 0);
+    PciWrite8 (MchBaseAddress + R_SA_PAM0, 0x30);
+    PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 1), 0x33);
+    PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 2), 0x33);
+    PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 3), 0x33);
+    PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 4), 0x33);
+    PciWrite8 (MchBaseAddress + R_SA_PAM5, 0x33);
+    PciWrite8 (MchBaseAddress + R_SA_PAM6, 0x33);
+  }
+
+  //
+  // Install EFI_PEI_MM_ACCESS_PPI for S3 resume case
+  //
+  Status = PeiInstallSmmAccessPpi ();
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Install EFI_PEI_MM_CONTROL_PPI for S3 resume case
+  //
+  Status = PeiInstallSmmControlPpi ();
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
+
+EFI_PEI_NOTIFY_DESCRIPTOR  mFspSiliconInitDoneNotifyDesc = {
+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gFspSiliconInitDonePpiGuid,
+  FspSiliconInitDoneNotify
+};
 
 /**
   S3 PEI module entry point
@@ -25,12 +100,10 @@ S3PeiEntryPoint (
   IN CONST EFI_PEI_SERVICES     **PeiServices
   )
 {
-  EFI_STATUS Status;
+  EFI_STATUS  Status;
 
-  //
-  // Install EFI_PEI_MM_ACCESS_PPI for S3 resume case
-  //
-  Status = PeiInstallSmmAccessPpi ();
+  Status = PeiServicesNotifyPpi (&mFspSiliconInitDoneNotifyDesc);
+  ASSERT_EFI_ERROR (Status);
 
   return Status;
 }
diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.inf b/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.inf
index e485eac9521f..173919bb881e 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.inf
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.inf
@@ -18,10 +18,13 @@
 [LibraryClasses]
   PeimEntryPoint
   PeiServicesLib
+  DebugLib
   SmmAccessLib
+  SmmControlLib
 
 [Packages]
   MdePkg/MdePkg.dec
+  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
   IntelSiliconPkg/IntelSiliconPkg.dec
   S3FeaturePkg/S3FeaturePkg.dec
 
@@ -31,5 +34,8 @@
 [FeaturePcd]
   gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable
 
+[Ppis]
+  gFspSiliconInitDonePpiGuid
+
 [Depex]
-  gEfiPeiMemoryDiscoveredPpiGuid
+  TRUE
diff --git a/Platform/Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h b/Platform/Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h
new file mode 100644
index 000000000000..0d75af8e9a03
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h
@@ -0,0 +1,22 @@
+/** @file
+  Header file for NV data structure definition.
+
+Copyright (c) 2021, Baruch Binyamin Doron
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __ACPI_S3_MEMORY_NV_DATA_H__
+#define __ACPI_S3_MEMORY_NV_DATA_H__
+
+//
+// NV data structure
+//
+typedef struct {
+  UINT64  S3PeiMemBase;
+  UINT64  S3PeiMemSize;
+} ACPI_S3_MEMORY;
+
+#define ACPI_S3_MEMORY_NV_NAME  L"S3MemoryInfo"
+
+#endif
-- 
2.37.2