From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lj1-f169.google.com (mail-lj1-f169.google.com [209.85.208.169]) by mx.groups.io with SMTP id smtpd.web08.11052.1662911204337560536 for ; Sun, 11 Sep 2022 08:46:44 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=fpbKzTvM; spf=pass (domain: gmail.com, ip: 209.85.208.169, mailfrom: benjamin.doron00@gmail.com) Received: by mail-lj1-f169.google.com with SMTP id p5so7819888ljc.13 for ; Sun, 11 Sep 2022 08:46:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=CkQ6TkTvdrP4J8DTskTPE6b2jNfMNVowU1cKsu41clE=; b=fpbKzTvMmpNBSAOGwdfo3EnNo/sotQ2NKVi+gWSTPfh/A/slgzGFt8DbRBdaMmycgp rYoen6BUYBDz6vGWc3SPTOTfM382zrOaAGkPpXplY/QHcBVzRPpeNNKUCqbzxbimNBQe GGpZNWRAoT72yIcaJijJj9yksqucccXJlwpHdAhnMmOVW10ce5pmsVjjwZ/8yLPune8E NVpLM/CA/sgwGeT7SoIlplCdU/KCON3ZzzfYygly5rAt8Pa9W4DTL3kUa60KftDip4xu AgbckljfO1wenVcMOWvUESukmwTQVfTHRQo7hpkTg8R5k6fb9+NnIJC7DWb6rggtIUvv FTKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=CkQ6TkTvdrP4J8DTskTPE6b2jNfMNVowU1cKsu41clE=; b=XMGRSZX44ZbpMIHRuCcIi3AcIyhqaOXGGe31Vqtx8WQKYxYaf3NvdYaLXCvshBB9fB FlPe4Noi8+8Mu8+TFXF6gYODHnbL9jpHkNcIJ8H6VRp3T/edm5XsDrKpbkeMqeEjP4Qu hOnruF3bRmknbw+8FvTLOkH/FVx2/bFDmw45HqBTC8fJXH4EasB8duZhbEIrRe+svvrx IOKkrs5kmZzBlhQXO4dkRImkEK7HpM66g4ai+B2dbeVEbMWy29cVayWMW5SosSthZ29+ PbZ9f9coJ491PhvAIQ900f8WQV/OQP1B1pPSDapBrzB+BABVozm3jGohagLMjuqvWeoD ESDg== X-Gm-Message-State: ACgBeo060own11UwSlPZhaZ/DflK/OF5v1UODriwKhNXi59/mLxXVTUz 8TqkieF3fkj/MGmBBrU8cTBUdHoQVwN1pSwUHqU= X-Google-Smtp-Source: AA6agR4eZBKnDgkhU2ARe217+8PA2EyEG1g//OP4Qv28eN4CGOxx7Wb3nVqvLSes+F2TjBpuq4o3lQ9qdO1mszCo7EA= X-Received: by 2002:a05:651c:158e:b0:26b:46a6:bf63 with SMTP id h14-20020a05651c158e00b0026b46a6bf63mr5034614ljq.21.1662911202441; Sun, 11 Sep 2022 08:46:42 -0700 (PDT) MIME-Version: 1.0 References: <7984cecbfb1970a28dc483788a7a8a9dc4175ccd.1662483691.git.benjamin.doron00@gmail.com> In-Reply-To: From: "Benjamin Doron" Date: Sun, 11 Sep 2022 11:46:31 -0400 Message-ID: Subject: Re: [edk2-devel][edk2-platforms][PATCH v2 4/6] S3FeaturePkg: Implement working S3 resume To: "Oram, Isaac W" Cc: "devel@edk2.groups.io" , "Chaganty, Rangasai V" , "Desimone, Nathaniel L" , "Sinha, Ankit" , "Gao, Liming" Content-Type: multipart/alternative; boundary="00000000000083c0c305e868affa" --00000000000083c0c305e868affa Content-Type: text/plain; charset="UTF-8" Hi Isaac, These SMM communication modules are S3 specific: - PiSmmCommunicationPei exits with EFI_UNSUPPORTED if BootMode != BOOT_ON_S3_RESUME ( https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.c#L380-L383 ). - PiSmmCommunicationSmm installs an SMI handler that can be found in the SMM config table, GUID-ed as "Pei...Ppi" ( https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.c#L38-L43 ). So, those modules are for S3 resume. I can push a patch that adds SmmLockBox to MinPlatform. Maybe stage 5 makes sense? Best regards, Benjamin On Wed, 7 Sept 2022 at 20:38, Oram, Isaac W wrote: > It seems like: > UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf > UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf > MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf > > Are not S3 specific and belong with common stage 4 or 5 content. It seems > many features could require them. DSC can deal with duplicates, but FDF > would fail if there were collisions. > > S3Feature.dsc > - Remove commented out code > > Regards, > Isaac > > -----Original Message----- > From: Benjamin Doron > Sent: Tuesday, September 6, 2022 10:02 AM > To: devel@edk2.groups.io > Cc: Desimone, Nathaniel L ; Sinha, Ankit < > ankit.sinha@intel.com>; Chaganty, Rangasai V < > rangasai.v.chaganty@intel.com>; Oram, Isaac W ; > Gao, Liming > Subject: [edk2-devel][edk2-platforms][PATCH v2 4/6] S3FeaturePkg: > Implement working S3 resume > > 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 > Cc: Ankit Sinha > Cc: Sai Chaganty > Cc: Isaac Oram > Cc: Liming Gao > Signed-off-by: Benjamin Doron > --- > .../S3FeaturePkg/Include/PostMemory.fdf | 13 ++ > .../S3FeaturePkg/Include/PreMemory.fdf | 8 +- > .../S3FeaturePkg/Include/S3Feature.dsc | 38 ++++- > .../S3FeaturePkg/S3Dxe/S3Dxe.c | 155 ++++++++++++++++++ > .../S3FeaturePkg/S3Dxe/S3Dxe.inf | 49 ++++++ > .../S3FeaturePkg/S3Pei/S3Pei.c | 83 +++++++++- > .../S3FeaturePkg/S3Pei/S3Pei.inf | 8 +- > .../Include/AcpiS3MemoryNvData.h | 22 +++ > 8 files changed, 365 insertions(+), 11 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..7f630908fa2c 100644 > --- a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf > +++ b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf > @@ -2,7 +2,20 @@ > # FDF file for post-memory S3 advanced feature modules. # # Copyright > (c) 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2022, > Baruch Binyamin Doron.
# # 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+ INF > UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf++## Restore-state module stack+ > INF > MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.infdiff > --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.
+# Copyright (c) 2022, > Baruch Binyamin Doron.
# # 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.infdiff --git > a/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc > b/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc > index cc34e785076a..d8bfc7909413 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.
+# Copyright (c) 2022, Baruch Binyamin > Doron.
# # 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,14 @@ > ################################################################################ > [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++[LibraryClasses.common.DXE_DRIVER, > LibraryClasses.common.DXE_SMM_DRIVER]+ > #######################################+ # Edk2 Packages+ > #######################################+ > S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > ################################################################################ > #@@ -60,8 +72,26 @@ > # 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. 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 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+ > MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.infdiff > --git a/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c > b/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c > new file mode 100644 > index 000000000000..1a7ccb8eedab > --- /dev/null > +++ b/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c > @@ -0,0 +1,155 @@ > +/** @file+ Source code file for S3 DXE module++Copyright (c) 2022, > Baruch Binyamin Doron.
+SPDX-License-Identifier: > BSD-2-Clause-Patent++**/++#include +#include > +#include +#include > +#include +#include > +#include > +#include > +#include +#include > ++#define PEI_ADDITIONAL_MEMORY_SIZE (16 * > EFI_PAGE_SIZE)++/**+ Get the mem size in memory type information table.++ > @return the mem size in memory type information > 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+ )+{+ 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));++ 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.
+#+# 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+ > gEfiVariableWriteArchProtocolGuiddiff --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.
+Copyright (c) 2022, Baruch Binyamin > Doron.
SPDX-License-Identifier: BSD-2-Clause-Patent **/ +#include > +#include +#include > #include #include > +#include ++// 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+ > TRUEdiff --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 > > --00000000000083c0c305e868affa Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Isaac,
These SMM communication modules a= re S3 specific:
- PiSmmCommunicationPei exits with EFI_UNSUPPORTE= D if BootMode !=3D BOOT_ON_S3_RESUME (https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/PiSmmCom= munication/PiSmmCommunicationPei.c#L380-L383).
- PiSmmCommuni= cationSmm installs an SMI handler that can be found in the SMM config table= , GUID-ed as "Pei...Ppi" (https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/PiSmmCommuni= cation/PiSmmCommunicationSmm.c#L38-L43).

S= o, those modules are for S3 resume. I can push a patch that adds SmmLockBox= to MinPlatform. Maybe stage 5 makes sense?

Best regards,
Benjamin


On Wed, 7 Sept 2022 at 20:38, Oram, Isaac W <isaac.w.oram@intel.com> wrote:
It seems like:
=C2=A0 UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
=C2=A0 UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
=C2=A0 MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf

Are not S3 specific and belong with common stage 4 or 5 content.=C2=A0 It s= eems many features could require them.=C2=A0 DSC can deal with duplicates, = but FDF would fail if there were collisions.

S3Feature.dsc
- Remove commented out code

Regards,
Isaac

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com>
Sent: Tuesday, September 6, 2022 10:02 AM
To: devel@edk2.gr= oups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Sinha, Ankit= <ankit.sinha= @intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>;= Oram, Isaac W <isaac.w.oram@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: [edk2-devel][edk2-platforms][PATCH v2 4/6] S3FeaturePkg: Implement= working S3 resume

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

Factored allocation logic into new module to avoid MinPlatform dependency o= n 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>
---
=C2=A0.../S3FeaturePkg/Include/PostMemory.fdf=C2=A0 =C2=A0 =C2=A0 =C2=A0|= =C2=A0 13 ++
=C2=A0.../S3FeaturePkg/Include/PreMemory.fdf=C2=A0 =C2=A0 =C2=A0 =C2=A0 |= =C2=A0 =C2=A08 +-
=C2=A0.../S3FeaturePkg/Include/S3Feature.dsc=C2=A0 =C2=A0 =C2=A0 =C2=A0 |= =C2=A0 38 ++++-
=C2=A0.../S3FeaturePkg/S3Dxe/S3Dxe.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 | 155 ++++++++++++++++++
=C2=A0.../S3FeaturePkg/S3Dxe/S3Dxe.inf=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 |=C2=A0 49 ++++++
=C2=A0.../S3FeaturePkg/S3Pei/S3Pei.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 |=C2=A0 83 +++++++++-
=C2=A0.../S3FeaturePkg/S3Pei/S3Pei.inf=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 |=C2=A0 =C2=A08 +-
=C2=A0.../Include/AcpiS3MemoryNvData.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 |=C2=A0 22 +++
=C2=A08 files changed, 365 insertions(+), 11 deletions(-)=C2=A0 create mode= 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c
=C2=A0create mode 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/= S3Dxe.inf
=C2=A0create mode 100644 Platform/Intel/MinPlatformPkg/Include/AcpiS3Memory= NvData.h

diff --git a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory= .fdf b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf index 9e17f853c630..7f630908fa2c 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf +++ b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf @@ -2,7 +2,20 @@
=C2=A0#=C2=A0 FDF file for post-memory S3 advanced feature modules. # # Cop= yright (c) 2019, Intel Corporation. All rights reserved.<BR>+# Copyri= ght (c) 2022, Baruch Binyamin Doron.<BR> # # SPDX-License-Identifier:= BSD-2-Clause-Patent # ##++## Dependencies+=C2=A0 INF UefiCpuPkg/PiSmmCommu= nication/PiSmmCommunicationSmm.inf+=C2=A0 INF MdeModulePkg/Universal/LockBo= x/SmmLockBox/SmmLockBox.inf++## Save-state module stack+=C2=A0 INF S3Featur= ePkg/S3Dxe/S3Dxe.inf+=C2=A0 INF MdeModulePkg/Universal/Acpi/S3SaveStateDxe/= S3SaveStateDxe.inf+=C2=A0 INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf++## = Restore-state module stack+=C2=A0 INF MdeModulePkg/Universal/Acpi/BootScrip= tExecutorDxe/BootScriptExecutorDxe.infdiff --git a/Features/Intel/PowerMana= gement/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 @@
=C2=A0#=C2=A0 FDF file for pre-memory S3 advanced feature modules. # # Copy= right (c) 2019, Intel Corporation. All rights reserved.<BR>+# Copyrig= ht (c) 2022, Baruch Binyamin Doron.<BR> # # SPDX-License-Identifier: = BSD-2-Clause-Patent # ## -INF S3FeaturePkg/S3Pei/S3Pei.inf+## Dependencies+= =C2=A0 INF S3FeaturePkg/S3Pei/S3Pei.inf+=C2=A0 INF UefiCpuPkg/PiSmmCommunic= ation/PiSmmCommunicationPei.inf++## Restore-state module stack+=C2=A0 INF U= efiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.infdiff --git a/Features= /Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc b/Features/Intel/= PowerManagement/S3FeaturePkg/Include/S3Feature.dsc
index cc34e785076a..d8bfc7909413 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc
@@ -7,6 +7,7 @@
=C2=A0# for the build infrastructure. # # Copyright (c) 2019 - 2021, Intel = Corporation. All rights reserved.<BR>+# Copyright (c) 2022, Baruch Bi= nyamin Doron.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent #@= @ -25,6 +26,10 @@
=C2=A0 =C2=A0 =C2=A0!error "DXE_ARCH must be specified to build this f= eature!"=C2=A0 =C2=A0!endif +[PcdsFixedAtBuild]+=C2=A0 # Attempts to i= mprove performance at the cost of more DRAM usage+=C2=A0 gEfiMdeModulePkgTo= kenSpaceGuid.PcdShadowPeimOnS3Boot|TRUE+ ##################################= ############################################## # # Library Class section - = list of all Library Classes needed by this feature.@@ -32,7 +37,14 @@
=C2=A0#####################################################################= ###########=C2=A0 [LibraryClasses.common.PEIM]-=C2=A0 SmmAccessLib|IntelSil= iconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.inf+=C2= =A0 #SmmAccessLib|IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib= Smramc/PeiSmmAccessLib.inf+=C2=A0 SmmControlLib|IntelSiliconPkg/Feature/Smm= Control/Library/PeiSmmControlLib/PeiSmmControlLib.inf++[LibraryClasses.comm= on.DXE_DRIVER, LibraryClasses.common.DXE_SMM_DRIVER]+=C2=A0 ###############= ########################+=C2=A0 # Edk2 Packages+=C2=A0 ####################= ###################+=C2=A0 S3BootScriptLib|MdeModulePkg/Library/PiDxeS3Boot= ScriptLib/DxeS3BootScriptLib.inf=C2=A0 ####################################= ############################################ #@@ -60,8 +72,26 @@
=C2=A0 =C2=A0# S3 Feature Package=C2=A0 =C2=A0#############################= ######## -=C2=A0 # Add library instances here that are not included in pack= age components and should be tested-=C2=A0 # in the package build.-=C2=A0 = =C2=A0# Add components here that should be included in the package build.= =C2=A0 =C2=A0S3FeaturePkg/S3Pei/S3Pei.inf+=C2=A0 UefiCpuPkg/PiSmmCommunicat= ion/PiSmmCommunicationPei.inf+=C2=A0 UefiCpuPkg/Universal/Acpi/S3Resume2Pei= /S3Resume2Pei.inf++#+# Feature DXE Components+#++# @todo: Change below line= to [Components.$(DXE_ARCH)] after https://bu= gzilla.tianocore.org/show_bug.cgi?id=3D2308+#=C2=A0 =C2=A0 =C2=A0 =C2= =A0 is completed.+[Components.X64]+=C2=A0 #################################= ####+=C2=A0 # S3 Feature Package+=C2=A0 ###################################= ##++=C2=A0 # Add components here that should be included in the package bui= ld.+=C2=A0 UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf+=C2=A0 M= deModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf+=C2=A0 S3FeaturePkg= /S3Dxe/S3Dxe.inf+=C2=A0 MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveSt= ateDxe.inf+=C2=A0 UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf+=C2=A0 MdeModule= Pkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.infdiff --gi= t a/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c b/Features/In= tel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c
new file mode 100644
index 000000000000..1a7ccb8eedab
--- /dev/null
+++ b/Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c
@@ -0,0 +1,155 @@
+/** @file+=C2=A0 Source code file for S3 DXE module++Copyright (c) 2022, B= aruch Binyamin Doron.<BR>+SPDX-License-Identifier: BSD-2-Clause-Paten= t++**/++#include <PiDxe.h>+#include <Library/BaseMemoryLib.h>+#= include <Library/DebugLib.h>+#include <Library/PcdLib.h>+#inclu= de <Library/UefiLib.h>+#include <Library/UefiBootServicesTableLib.= h>+#include <Library/UefiRuntimeServicesTableLib.h>+#include <G= uid/AcpiS3Context.h>+#include <Guid/MemoryTypeInformation.h>+#incl= ude <AcpiS3MemoryNvData.h>++#define PEI_ADDITIONAL_MEMORY_SIZE=C2=A0 = =C2=A0 (16 * EFI_PAGE_SIZE)++/**+=C2=A0 Get the mem size in memory type inf= ormation table.++=C2=A0 @return the mem size in memory type information tab= le.+**/+UINT64+EFIAPI+GetMemorySizeInMemoryTypeInformation (+=C2=A0 VOID+= =C2=A0 )+{+=C2=A0 EFI_STATUS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 Status;+=C2=A0 EFI_MEMORY_TYPE_INFORMATION *MemoryData;+= =C2=A0 UINT8=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0Index;+=C2=A0 UINTN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0TempPageNum;++=C2=A0 Status= =3D EfiGetSystemConfigurationTable (&gEfiMemoryTypeInformationGuid, (V= OID **) &MemoryData);++=C2=A0 if (EFI_ERROR (Status) || MemoryData =3D= =3D NULL) {+=C2=A0 =C2=A0 return 0;+=C2=A0 }++=C2=A0 TempPageNum =3D 0;+=C2= =A0 for (Index =3D 0; MemoryData[Index].Type !=3D EfiMaxMemoryType; Index++= ) {+=C2=A0 =C2=A0 //+=C2=A0 =C2=A0 // Accumulate default memory size requir= ements+=C2=A0 =C2=A0 //+=C2=A0 =C2=A0 TempPageNum +=3D MemoryData[Index].Nu= mberOfPages;+=C2=A0 }++=C2=A0 return TempPageNum * EFI_PAGE_SIZE;+}++/**+= =C2=A0 Get the mem size need to be consumed and reserved for PEI phase resu= me.++=C2=A0 @return the mem size to be reserved for PEI phase resume.+**/+U= INT64+EFIAPI+GetPeiMemSize (+=C2=A0 VOID+=C2=A0 )+{+=C2=A0 UINT64=C2=A0 Siz= e;++=C2=A0 Size =3D GetMemorySizeInMemoryTypeInformation ();++=C2=A0 return= PcdGet32 (PcdPeiMinMemSize) + Size + PEI_ADDITIONAL_MEMORY_SIZE;+}++/**+= =C2=A0 Allocate EfiACPIMemoryNVS below 4G memory address.++=C2=A0 This func= tion allocates EfiACPIMemoryNVS below 4G memory address.++=C2=A0 @param=C2= =A0 Size=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Size of memory to allocate.++=C2= =A0 @return Allocated address for output.++**/+VOID *+EFIAPI+AllocateAcpiNv= sMemoryBelow4G (+=C2=A0 IN=C2=A0 =C2=A0UINTN=C2=A0 Size+=C2=A0 )+{+=C2=A0 U= INTN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Pages;+= =C2=A0 EFI_PHYSICAL_ADDRESS=C2=A0 Address;+=C2=A0 EFI_STATUS=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 Status;+=C2=A0 VOID=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *Buffer;++=C2=A0 Pages=C2=A0 =C2=A0=3D E= FI_SIZE_TO_PAGES (Size);+=C2=A0 Address =3D 0xffffffff;++=C2=A0 Status =3D = gBS->AllocatePages (+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 AllocateMaxAddress,+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 EfiACPIMemoryNVS,+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 Pages,+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 &Address+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 );+=C2=A0 ASSERT_EFI_ERROR (Status);++=C2=A0 Bu= ffer =3D (VOID *)(UINTN)Address;+=C2=A0 ZeroMem (Buffer, Size);++=C2=A0 ret= urn Buffer;+}++/**+=C2=A0 Allocates memory to use on S3 resume.++=C2=A0 @pa= ram[in]=C2=A0 ImageHandle=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Not used.+=C2= =A0 @param[in]=C2=A0 SystemTable=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 General = purpose services available to every DXE driver.++=C2=A0 @retval=C2=A0 =C2= =A0 =C2=A0EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 The function comple= tes successfully+=C2=A0 @retval=C2=A0 =C2=A0 =C2=A0EFI_OUT_OF_RESOURCES Ins= ufficient resources to create database+**/+EFI_STATUS+EFIAPI+S3DxeEntryPoin= t (+=C2=A0 IN EFI_HANDLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 ImageHandle,+=C2=A0 IN = EFI_SYSTEM_TABLE=C2=A0 *SystemTable+=C2=A0 )+{+=C2=A0 UINT64=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 S3PeiMemSize;+=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 S3PeiMemBase;+=C2=A0 ACPI_S3_MEMORY=C2=A0 S3MemoryInfo;+=C2=A0 EFI_= STATUS=C2=A0 =C2=A0 =C2=A0 Status;++=C2=A0 DEBUG ((DEBUG_INFO, "%a() S= tart\n", __FUNCTION__));++=C2=A0 S3PeiMemSize =3D GetPeiMemSize ();+= =C2=A0 S3PeiMemBase =3D (UINTN) AllocateAcpiNvsMemoryBelow4G (S3PeiMemSize)= ;+=C2=A0 ASSERT (S3PeiMemBase !=3D 0);++=C2=A0 S3MemoryInfo.S3PeiMemBase = =3D S3PeiMemBase;+=C2=A0 S3MemoryInfo.S3PeiMemSize =3D S3PeiMemSize;++=C2= =A0 DEBUG ((DEBUG_INFO, "S3PeiMemBase: 0x%x\n", S3PeiMemBase));+= =C2=A0 DEBUG ((DEBUG_INFO, "S3PeiMemSize: 0x%x\n", S3PeiMemSize))= ;++=C2=A0 Status =3D gRT->SetVariable (+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ACPI_S3_MEMORY_NV_NAME,+=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &gEfiAcpiVariableGuid,+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 EFI_VARIABLE= _NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,+=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeof (S3MemoryInfo),+=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &S3MemoryInfo+=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 );+=C2=A0 ASSERT_EF= I_ERROR (Status);++=C2=A0 DEBUG ((DEBUG_INFO, "%a() End\n", __FUN= CTION__));+=C2=A0 return EFI_SUCCESS;+}diff --git a/Features/Intel/PowerMan= agement/S3FeaturePkg/S3Dxe/S3Dxe.inf b/Features/Intel/PowerManagement/S3Fea= turePkg/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.+#+# Copyrigh= t (c) 2022, Baruch Binyamin Doron.<BR>+#+# SPDX-License-Identifier: B= SD-2-Clause-Patent+#+###++[Defines]+=C2=A0 INF_VERSION=C2=A0 =C2=A0 =C2=A0 = =C2=A0=3D 0x00010017+=C2=A0 BASE_NAME=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D = S3Dxe+=C2=A0 FILE_GUID=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D 30926F92-CC83-4= 381-9F70-AC96EDB5BEE0+=C2=A0 VERSION_STRING=C2=A0 =C2=A0 =3D 1.0+=C2=A0 MOD= ULE_TYPE=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D DXE_DRIVER+=C2=A0 ENTRY_POINT=C2=A0 = =C2=A0 =C2=A0 =C2=A0=3D S3DxeEntryPoint++[LibraryClasses]+=C2=A0 UefiDriver= EntryPoint+=C2=A0 UefiBootServicesTableLib+=C2=A0 UefiRuntimeServicesTableL= ib+=C2=A0 BaseMemoryLib+=C2=A0 DebugLib+=C2=A0 PcdLib+=C2=A0 UefiLib++[Pack= ages]+=C2=A0 MdePkg/MdePkg.dec+=C2=A0 MdeModulePkg/MdeModulePkg.dec+=C2=A0 = MinPlatformPkg/MinPlatformPkg.dec+=C2=A0 IntelFsp2WrapperPkg/IntelFsp2Wrapp= erPkg.dec+=C2=A0 S3FeaturePkg/S3FeaturePkg.dec++[Sources]+=C2=A0 S3Dxe.c++[= Pcd]+=C2=A0 gIntelFsp2WrapperTokenSpaceGuid.PcdPeiMinMemSize++[FeaturePcd]+= =C2=A0 gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable++[Guids]+=C2=A0 gEfiM= emoryTypeInformationGuid=C2=A0 ## CONSUMES+=C2=A0 gEfiAcpiVariableGuid=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0## CONSUMES++[Depex]+=C2=A0 gEfiVaria= bleArchProtocolGuid=C2=A0 =C2=A0 =C2=A0 AND+=C2=A0 gEfiVariableWriteArchPro= tocolGuiddiff --git a/Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3P= ei.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 @@
=C2=A0 =C2=A0Source code file for S3 PEI module=C2=A0 Copyright (c) 2019, I= ntel Corporation. All rights reserved.<BR>+Copyright (c) 2022, Baruch= Binyamin Doron.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent=C2= =A0 **/ +#include <PiPei.h>+#include <Library/DebugLib.h>+#incl= ude <Library/PciLib.h> #include <Library/PeiServicesLib.h> #inc= lude <Library/SmmAccessLib.h>+#include <Library/SmmControlLib.h>= ;++// TODO: Finalise implementation factoring+#define R_SA_PAM0=C2=A0 (0x80= )+#define R_SA_PAM5=C2=A0 (0x85)+#define R_SA_PAM6=C2=A0 (0x86)++/**+=C2=A0= This function is called after FspSiliconInitDone installed PPI.+=C2=A0 For= FSP API mode, this is when FSP-M HOBs are installed into EDK2.++=C2=A0 @pa= ram[in] PeiServices=C2=A0 =C2=A0 Pointer to PEI Services Table.+=C2=A0 @par= am[in] NotifyDesc=C2=A0 =C2=A0 =C2=A0Pointer to the descriptor for the Noti= fication event that+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 caused this function to execute.= +=C2=A0 @param[in] Ppi=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Pointer to = the PPI data associated with this function.++=C2=A0 @retval EFI_STATUS=C2= =A0 =C2=A0 =C2=A0 =C2=A0 Always return EFI_SUCCESS+**/+EFI_STATUS+EFIAPI+Fs= pSiliconInitDoneNotify (+=C2=A0 IN EFI_PEI_SERVICES=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0**PeiServices,+=C2=A0 IN EFI_PEI_NOTIFY_DESCRIPTOR=C2=A0 *= NotifyDesc,+=C2=A0 IN VOID=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*Ppi+=C2=A0 )+{+=C2=A0 EFI_STATUS=C2=A0 = =C2=A0 =C2=A0Status;+=C2=A0 EFI_BOOT_MODE=C2=A0 BootMode;+=C2=A0 UINT64=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0MchBaseAddress;++=C2=A0 Status =3D PeiServic= esGetBootMode (&BootMode);+=C2=A0 ASSERT_EFI_ERROR (Status);++=C2=A0 //= Enable PAM regions for AP wakeup vector (resume)+=C2=A0 // - CPU is finali= sed by PiSmmCpuDxeSmm, not FSP. So, it's safe here?+=C2=A0 // TODO/TEST= : coreboot does this unconditionally, vendor FWs may not (test resume). Sho= uld we?+=C2=A0 // - It is certainly interesting that only PAM0, PAM5 and PA= M6 are defined for KabylakeSiliconPkg.+=C2=A0 // - Also note that 0xA0000-0= xFFFFF is marked "reserved" in FSP HOB - this does not mean+=C2= =A0 //=C2=A0 =C2=A0that the memory is unusable, perhaps this is precisely b= ecause it will contain+=C2=A0 //=C2=A0 =C2=A0the AP wakeup vector.+=C2=A0 i= f (BootMode =3D=3D BOOT_ON_S3_RESUME) {+=C2=A0 =C2=A0 MchBaseAddress =3D PC= I_LIB_ADDRESS (0, 0, 0, 0);+=C2=A0 =C2=A0 PciWrite8 (MchBaseAddress + R_SA_= PAM0, 0x30);+=C2=A0 =C2=A0 PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 1), 0x3= 3);+=C2=A0 =C2=A0 PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 2), 0x33);+=C2= =A0 =C2=A0 PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 3), 0x33);+=C2=A0 =C2= =A0 PciWrite8 (MchBaseAddress + (R_SA_PAM0 + 4), 0x33);+=C2=A0 =C2=A0 PciWr= ite8 (MchBaseAddress + R_SA_PAM5, 0x33);+=C2=A0 =C2=A0 PciWrite8 (MchBaseAd= dress + R_SA_PAM6, 0x33);+=C2=A0 }++=C2=A0 //+=C2=A0 // Install EFI_PEI_MM_= ACCESS_PPI for S3 resume case+=C2=A0 //+=C2=A0 Status =3D PeiInstallSmmAcce= ssPpi ();+=C2=A0 ASSERT_EFI_ERROR (Status);++=C2=A0 //+=C2=A0 // Install EF= I_PEI_MM_CONTROL_PPI for S3 resume case+=C2=A0 //+=C2=A0 Status =3D PeiInst= allSmmControlPpi ();+=C2=A0 ASSERT_EFI_ERROR (Status);++=C2=A0 return Statu= s;+}++EFI_PEI_NOTIFY_DESCRIPTOR=C2=A0 mFspSiliconInitDoneNotifyDesc =3D {+= =C2=A0 (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TER= MINATE_LIST),+=C2=A0 &gFspSiliconInitDonePpiGuid,+=C2=A0 FspSiliconInit= DoneNotify+};=C2=A0 /**=C2=A0 =C2=A0S3 PEI module entry point@@ -25,12 +100= ,10 @@ S3PeiEntryPoint (
=C2=A0 =C2=A0IN CONST EFI_PEI_SERVICES=C2=A0 =C2=A0 =C2=A0**PeiServices=C2= =A0 =C2=A0) {-=C2=A0 EFI_STATUS Status;+=C2=A0 EFI_STATUS=C2=A0 Status; -= =C2=A0 //-=C2=A0 // Install EFI_PEI_MM_ACCESS_PPI for S3 resume case-=C2=A0= //-=C2=A0 Status =3D PeiInstallSmmAccessPpi ();+=C2=A0 Status =3D PeiServi= cesNotifyPpi (&mFspSiliconInitDoneNotifyDesc);+=C2=A0 ASSERT_EFI_ERROR = (Status);=C2=A0 =C2=A0 return Status; }diff --git a/Features/Intel/PowerMan= agement/S3FeaturePkg/S3Pei/S3Pei.inf b/Features/Intel/PowerManagement/S3Fea= turePkg/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 @@
=C2=A0[LibraryClasses]=C2=A0 =C2=A0PeimEntryPoint=C2=A0 =C2=A0PeiServicesLi= b+=C2=A0 DebugLib=C2=A0 =C2=A0SmmAccessLib+=C2=A0 SmmControlLib=C2=A0 [Pack= ages]=C2=A0 =C2=A0MdePkg/MdePkg.dec+=C2=A0 IntelFsp2WrapperPkg/IntelFsp2Wra= pperPkg.dec=C2=A0 =C2=A0IntelSiliconPkg/IntelSiliconPkg.dec=C2=A0 =C2=A0S3F= eaturePkg/S3FeaturePkg.dec @@ -31,5 +34,8 @@
=C2=A0[FeaturePcd]=C2=A0 =C2=A0gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnab= le +[Ppis]+=C2=A0 gFspSiliconInitDonePpiGuid+ [Depex]-=C2=A0 gEfiPeiMemoryD= iscoveredPpiGuid+=C2=A0 TRUEdiff --git a/Platform/Intel/MinPlatformPkg/Incl= ude/AcpiS3MemoryNvData.h b/Platform/Intel/MinPlatformPkg/Include/AcpiS3Memo= ryNvData.h
new file mode 100644
index 000000000000..0d75af8e9a03
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h
@@ -0,0 +1,22 @@
+/** @file
+=C2=A0 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 {
+=C2=A0 UINT64=C2=A0 S3PeiMemBase;
+=C2=A0 UINT64=C2=A0 S3PeiMemSize;
+} ACPI_S3_MEMORY;
+
+#define ACPI_S3_MEMORY_NV_NAME=C2=A0 L"S3MemoryInfo"
+
+#endif
--
2.37.2

--00000000000083c0c305e868affa--