Re: [edk2-devel][edk2-platforms][PATCH v2 5/6] MinPlatformPkg: Implement working S3 resume

["Isaac Oram" <isaac.w.oram@intel.com>]

DSC and FDF should wrap performance PEIM with !if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE to be consistent with the MinPlatformPkg DSC implementations.  Or you could take a look at what we did in WhitleyOpenBoardPkg/Include/Dsc/ EnablePerformanceMonitoringInfrastructure.dsc.  But I don't think that this is quite ready for promotion to wider use.  It might fit better as an advanced feature, but I haven't had time to play with it.

That plus adding the UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf to CorePostMemoryInclude.fdf
And
  UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
  MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
To CoreOsBootInclude.fdf.

I think that those are the best places.  They don't need any conditional logic because they have dependencies that should enable the desired execution timing.

Regards,
Isaac

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com> 
Sent: Tuesday, September 6, 2022 10:02 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [edk2-devel][edk2-platforms][PATCH v2 5/6] MinPlatformPkg: Implement working S3 resume

Consume S3 resume memory allocation on resume flow.

Also, include complementary FirmwarePerformanceDataTablePei module in MinPlatform FV for S3 resume performance measurement.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ankit Sinha <ankit.sinha@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../FspWrapperHobProcessLib.c                 | 69 ++++++++++++++++++-
 .../PeiFspWrapperHobProcessLib.inf            |  2 +
 .../Include/Dsc/CorePeiInclude.dsc            |  2 +
 .../Include/Fdf/CorePostMemoryInclude.fdf     |  2 +
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/FspWrapperHobProcessLib.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/FspWrapperHobProcessLib.c
index 7ee4d3a31c49..9bd6fe6290c5 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/FspWrapperHobProcessLib.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobP
+++ rocessLib/FspWrapperHobProcessLib.c
@@ -16,14 +16,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include <Library/HobLib.h> #include <Library/PcdLib.h> #include <Library/FspWrapperPlatformLib.h>+#include <Guid/AcpiS3Context.h> #include <Guid/GuidHobFspEas.h> #include <Guid/MemoryTypeInformation.h> #include <Guid/GraphicsInfoHob.h> #include <Guid/PcdDataBaseHobGuid.h> #include <Guid/ZeroGuid.h> #include <Ppi/Capsule.h>+#include <Ppi/ReadOnlyVariable2.h>  #include <FspEas.h>+#include <AcpiS3MemoryNvData.h>  // // Additional pages are used by DXE memory manager.@@ -130,6 +133,55 @@ GetPeiMemSize (
   return MinSize + Size + PEI_ADDITIONAL_MEMORY_SIZE; } +/**+  Get S3 PEI memory information.++  @note At this point, memory is ready, and PeiServices are available to use.+  Platform can get some data from SMRAM directly.++  @param[out] S3PeiMemSize  PEI memory size to be installed in S3 phase.+  @param[out] S3PeiMemBase  PEI memory base to be installed in S3 phase.++  @return If S3 PEI memory information is got successfully.+**/+EFI_STATUS+EFIAPI+GetS3MemoryInfo (+  OUT UINT64                *S3PeiMemSize,+  OUT EFI_PHYSICAL_ADDRESS  *S3PeiMemBase+  )+{+  EFI_STATUS                       Status;+  EFI_PEI_READ_ONLY_VARIABLE2_PPI  *VariablePpi;+  UINTN                            DataSize;+  ACPI_S3_MEMORY                   S3MemoryInfo;++  *S3PeiMemBase = 0;+  *S3PeiMemSize = 0;++  Status = PeiServicesLocatePpi (&gEfiPeiReadOnlyVariable2PpiGuid, 0, NULL, (VOID **) &VariablePpi);+  ASSERT_EFI_ERROR (Status);++  DataSize = sizeof (S3MemoryInfo);+  Status = VariablePpi->GetVariable (+                          VariablePpi,+                          ACPI_S3_MEMORY_NV_NAME,+                          &gEfiAcpiVariableGuid,+                          NULL,+                          &DataSize,+                          &S3MemoryInfo+                          );+  ASSERT_EFI_ERROR (Status);++  if (EFI_ERROR (Status)) {+    return Status;+  }++  *S3PeiMemBase = S3MemoryInfo.S3PeiMemBase;+  *S3PeiMemSize = S3MemoryInfo.S3PeiMemSize;+  return EFI_SUCCESS;+}+ /**   Post FSP-M HOB process for Memory Resource Descriptor. @@ -280,7 +332,7 @@ PostFspmHobProcess (
     0x1000     ); -+  if (BootMode != BOOT_ON_S3_RESUME) {     //     // Capsule mode     //@@ -337,7 +389,22 @@ PostFspmHobProcess (
     if (Capsule != NULL) {       Status = Capsule->CreateState ((EFI_PEI_SERVICES **)PeiServices, CapsuleBuffer, CapsuleBufferLength);     }+  } else {+    Status = GetS3MemoryInfo (&PeiMemSize, &PeiMemBase);+    ASSERT_EFI_ERROR (Status); +    DEBUG ((DEBUG_INFO, "S3 resume PeiMemBase        : 0x%08x\n", PeiMemBase));+    DEBUG ((DEBUG_INFO, "S3 resume PeiMemSize        : 0x%08x\n", PeiMemSize));++    //+    // Install efi memory+    //+    Status = PeiServicesInstallPeiMemory (+               PeiMemBase,+               PeiMemSize+               );+    ASSERT_EFI_ERROR (Status);+  }    //   // Create a memory allocation HOB at fixed location for MP Services PPI AP wait loop.diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf
index b846e7af1d2d..e2aac36bf018 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobP
+++ rocessLib/PeiFspWrapperHobProcessLib.inf
@@ -75,7 +75,9 @@
   gZeroGuid   gEfiGraphicsInfoHobGuid   gEfiGraphicsDeviceInfoHobGuid+  gEfiAcpiVariableGuid  [Ppis]   gEfiPeiCapsulePpiGuid                                   ## CONSUMES+  gEfiPeiReadOnlyVariable2PpiGuid                         ## CONSUMES   gEdkiiSiliconInitializedPpiGuid                         ## PRODUCESdiff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiInclude.dsc b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiInclude.dsc
index 08e50cac075f..f271fb26b189 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiInclude.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiInclude.dsc
@@ -41,3 +41,5 @@
       NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf   } !endif++  MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTablePei/FirmwarePerformancePei.infdiff --git a/Platform/Intel/MinPlatformPkg/Include/Fdf/CorePostMemoryInclude.fdf b/Platform/Intel/MinPlatformPkg/Include/Fdf/CorePostMemoryInclude.fdf
index 3c2716d6728a..160f02650a8c 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Fdf/CorePostMemoryInclude.fdf
+++ b/Platform/Intel/MinPlatformPkg/Include/Fdf/CorePostMemoryInclude.fd
+++ f
@@ -6,3 +6,5 @@
 # SPDX-License-Identifier: BSD-2-Clause-Patent # ##++  INF  MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTablePei/FirmwarePerformancePei.inf--
2.37.2