From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.50]) by mx.groups.io with SMTP id smtpd.web12.2789.1662147088897724664 for ; Fri, 02 Sep 2022 12:31:29 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=bPkpEOw1; spf=pass (domain: gmail.com, ip: 209.85.167.50, mailfrom: benjamin.doron00@gmail.com) Received: by mail-lf1-f50.google.com with SMTP id q7so4671591lfu.5 for ; Fri, 02 Sep 2022 12:31:28 -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=n9vHMWqW+rgK4kBLhyMMmXTBzrqVrRCz5Mf+rbSUAtY=; b=bPkpEOw1u07hZXkKsrlXScIMYoafcszZCcDIYtFmu+0lYdvSuQTszJcWf65fMKEWCr Zqq7TB1F04Pt/wjcGoq18bhm2kM8rNzo82ORcRirSsZgVBtrI5dv0JBbRV+zJMKj137H Hj3J7x39i6A8hVGl9zpreAvSE2qiZ/hIdLlyNca3hmnPeRHJCndu39QDYcTNfPndsp2u IiyZQsQVORL0R0OtLkUayZEnrWwbahG1Nr5pfOIkLNLC7NFePLXlQfWYI6AhVAUAQ2NL nVzcCOhjlf7LbgeOFhNmnqB4xnwX9cvgswFfaBBeNRdpwJsNqZQjWvXMw089C1snygVE nOhA== 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=n9vHMWqW+rgK4kBLhyMMmXTBzrqVrRCz5Mf+rbSUAtY=; b=HxJPltvuwzb/ueepTGUzR7bQreNO/KJhHKYjCIxK8BtLouHqLD1HNHwHuPnnVygIfY HhpYNTWl70eg0GrLrsXeLQebHcCEqOpB5TmssT8HK/CcJtYKMYUNwtVXOBQqiqktvvG9 Zrs87wQxencJ+M5L6jumVsqWvxKaTmF/aZBhOJePLbXXfjSyCyACMM0hyE9/w6/D4CWX quWJ26EGe28mEU03qx3395klFAgTcfmbdsWCo/RH7HSX0KgqYpAbdAo1Rbpil+jOj9Mo b4CJR1LoVOI1iO/ATBDMvqI1x3rS7IiDuKw8H+dB9mWcH1ZJKG1baklMCtIqSSMyzi7b s7Ww== X-Gm-Message-State: ACgBeo1Pa5Zwjf/jo6b3gD60naj6k6cxEZTN29WA8uzJoUBLlgmLwWEF 1bs15ctBsHA6tOOPhPdaFvWCalFLv0FMYv7awE0= X-Google-Smtp-Source: AA6agR42qCglZdEJrNmcCBKpd08fO6StHhEvIRRjGiIkkqwQXn3JCxFtYi5q3yxl1uJ0TK7XGzTnGVrsBPpSCxCvq84= X-Received: by 2002:a05:6512:694:b0:494:9a24:3243 with SMTP id t20-20020a056512069400b004949a243243mr3373926lfe.673.1662147086780; Fri, 02 Sep 2022 12:31:26 -0700 (PDT) MIME-Version: 1.0 References: <23fd6d63ba743cfbfa994dda115437719dbc2b4f.1661799519.git.benjamin.doron00@gmail.com> In-Reply-To: From: "Benjamin Doron" Date: Fri, 2 Sep 2022 15:31:15 -0400 Message-ID: Subject: Re: [edk2-devel][edk2-platforms][PATCH v1 3/5] S3FeaturePkg: Implement working S3 resume To: "Oram, Isaac W" Cc: "devel@edk2.groups.io" , "Desimone, Nathaniel L" , "Sinha, Ankit" , "Chaganty, Rangasai V" , "Gao, Liming" Content-Type: multipart/alternative; boundary="000000000000ac0f0305e7b6c677" --000000000000ac0f0305e7b6c677 Content-Type: text/plain; charset="UTF-8" > > S3Feature.dsc > - remove commented out code Regarding the SmmAccess library definition, which is the safer default? There is a build failure for the package if nothing is defined, but a platform should be required to define its correct library. Currently, that's the SMRAMC instance for platforms other than Tigerlake. I'm thinking that maybe this definition should be moved to S3FeaturePkg.dsc, so that this package can build, but boards will define the platform appropriate one. # NOTE: RSC will be after end-of-BS, use DebugLibSerialPort > # - No gBS in SerialPortInitialize() > # - No global assigns after ReadyToLock possible, due to LockBox copy > - These comments also don't make sense to me > Just some notes I made on the specific requirements for a debug library stack in BootScriptExecutorDxe. I elaborated in the cover letter, but they shouldn't need to go to the tree, so I'll drop them here. I intend to push a v2 soon with everything fixed. Best regards, Benjamin On Mon, 29 Aug 2022 at 20:19, Oram, Isaac W wrote: > S3Feature.dsc > - remove commented out code > > # Add library instances here that are not included in package components > and should be tested > # in the package build. > - These comments don't make a lot of sense to me in the feature include > DSC. Looks like cut and paste propagation that is not appropriate here. > Note instances in Components.IA32 and Components.X64 sections. > > # NOTE: RSC will be after end-of-BS, use DebugLibSerialPort > # - No gBS in SerialPortInitialize() > # - No global assigns after ReadyToLock possible, due to LockBox copy > - These comments also don't make sense to me > > S3Dxe.c > - infromation should be "information" in multiple instances > - GetPeiMemSize - Generally we would prefer #defines in .h or near top of > the file after includes and before function prototypes and implementations. > - Resolve ToDo > > S3Pei.c > - Resolve ToDo and TODO/TEST > > Regards, > Isaac > > -----Original Message----- > From: Benjamin Doron > Sent: Monday, August 29, 2022 1:36 PM > 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 v1 3/5] 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 | 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.
+# 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+ # 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.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..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.
+# 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,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.infdiff > --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.
+SPDX-License-Identifier: > BSD-2-Clause-Patent++**/++#include +#include > +#include +#include > +#include +#include > +#include > +#include > +#include +#include > ++/**+ 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.
+#+# 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 > > --000000000000ac0f0305e7b6c677 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
S3Featur= e.dsc
- remove commented out code
=C2=A0
Regarding the= SmmAccess library definition, which is the safer default? There is a build= failure for the package if nothing is defined, but a platform should be re= quired to define its correct library. Currently, that's the SMRAMC inst= ance for platforms other than Tigerlake. I'm thinking that maybe this d= efinition should be moved to S3FeaturePkg.dsc, so that this package can bui= ld, but boards will define the platform appropriate one.

=C2=A0 # NOTE: RSC will be after end-of-BS, use DebugLibSerialPort
=C2=A0 # - No gBS in SerialPortInitialize()
=C2=A0 # - No global assigns after ReadyToLock possible, due to LockBox cop= y
 - These comments also don't make sense to me
Just some notes I made on the specific requirements for a debug= library stack in BootScriptExecutorDxe. I elaborated in the cover letter, = but they shouldn't need to go to the tree, so I'll drop them here.<= /div>

I intend to push a v2 soon with everything fixed.<= br>

Best regards,
Benjamin


On Mon, 29 Aug 2022 at 20:19, Oram, = Isaac W <isaac.w.oram@intel.co= m> wrote:
S3Feature.dsc
- remove commented out code

=C2=A0 # Add library instances here that are not included in package compon= ents and should be tested
=C2=A0 # in the package build.
- These comments don't make a lot of sense to me in the feature include= DSC.=C2=A0 Looks like cut and paste propagation that is not appropriate he= re.=C2=A0 Note instances in Components.IA32 and Components.X64 sections.
=C2=A0 # NOTE: RSC will be after end-of-BS, use DebugLibSerialPort
=C2=A0 # - No gBS in SerialPortInitialize()
=C2=A0 # - No global assigns after ReadyToLock possible, due to LockBox cop= y
- These comments also don't make sense to me

S3Dxe.c
- infromation should be "information" in multiple instances
- GetPeiMemSize - Generally we would prefer #defines in .h or near top of t= he file after includes and before function prototypes and implementations.<= br> - Resolve ToDo

S3Pei.c
- Resolve ToDo and TODO/TEST

Regards,
Isaac

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com>
Sent: Monday, August 29, 2022 1:36 PM
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 v1 3/5] 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 15 ++
=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 42 ++++-
=C2=A0.../S3FeaturePkg/S3Dxe/S3Dxe.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 | 156 ++++++++++++++++++
=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, 375 insertions(+), 8 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..5d3d96f4f317 100644
--- a/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf +++ b/Features/Intel/PowerManagement/S3FeaturePkg/Include/PostMemory.fdf @@ -2,7 +2,22 @@
=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 # FSP may perform CPU finalisation, requires CpuI= nitDxe from closed code+=C2=A0 # - Presently, PiSmmCpuDxeSmm shall perform = finalisation with this data+=C2=A0 INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe= .inf++## Restore-state module stack+=C2=A0 INF MdeModulePkg/Universal/Acpi/= BootScriptExecutorDxe/BootScriptExecutorDxe.infdiff --git a/Features/Intel/= PowerManagement/S3FeaturePkg/Include/PreMemory.fdf b/Features/Intel/PowerMa= nagement/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..bf45b56258ff 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,15 @@
=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+=C2=A0 #IntelCompatSh= imLib|$(PLATFORM_SI_PACKAGE)/Library/BaseIntelCompatShimLibKbl/BaseIntelCom= patShimLibKbl.inf++[LibraryClasses.common.DXE_DRIVER, LibraryClasses.common= .DXE_SMM_DRIVER]+=C2=A0 #######################################+=C2=A0 # Ed= k2 Packages+=C2=A0 #######################################+=C2=A0 S3BootScr= iptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf=C2= =A0 #######################################################################= ######### #@@ -65,3 +78,30 @@
=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/PiSmmComm= unication/PiSmmCommunicationPei.inf+=C2=A0 UefiCpuPkg/Universal/Acpi/S3Resu= me2Pei/S3Resume2Pei.inf++#+# Feature DXE Components+#++# @todo: Change belo= w line to [Components.$(DXE_ARCH)] after http= s://bugzilla.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 library instances here that are not included in packag= e components and should be tested+=C2=A0 # in the package build.++=C2=A0 # = Add components here that should be included in the package build.+=C2=A0 Ue= fiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf+=C2=A0 MdeModulePkg/U= niversal/LockBox/SmmLockBox/SmmLockBox.inf+=C2=A0 S3FeaturePkg/S3Dxe/S3Dxe.= inf+=C2=A0 MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf+= =C2=A0 UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf+=C2=A0 # NOTE: RSC will be = after end-of-BS, use DebugLibSerialPort+=C2=A0 # - No gBS in SerialPortInit= ialize()+=C2=A0 # - No global assigns after ReadyToLock possible, due to Lo= ckBox copy+=C2=A0 MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScr= iptExecutorDxe.infdiff --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+=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>++/**+=C2=A0 Get the mem size in memory typ= e infromation table.++=C2=A0 @return the mem size in memory type infromatio= n table.+**/+UINT64+EFIAPI+GetMemorySizeInMemoryTypeInformation (+=C2=A0 VO= ID+=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 St= atus =3D EfiGetSystemConfigurationTable (&gEfiMemoryTypeInformationGuid= , (VOID **) &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; Inde= x++) {+=C2=A0 =C2=A0 //+=C2=A0 =C2=A0 // Accumulate default memory size req= uirements+=C2=A0 =C2=A0 //+=C2=A0 =C2=A0 TempPageNum +=3D MemoryData[Index]= .NumberOfPages;+=C2=A0 }++=C2=A0 return TempPageNum * EFI_PAGE_SIZE;+}++/**= +=C2=A0 Get the mem size need to be consumed and reserved for PEI phase res= ume.++=C2=A0 @return the mem size to be reserved for PEI phase resume.+**/+= UINT64+EFIAPI+GetPeiMemSize (+=C2=A0 VOID+=C2=A0 )+{+=C2=A0 #define PEI_ADD= ITIONAL_MEMORY_SIZE=C2=A0 =C2=A0 (16 * EFI_PAGE_SIZE)++=C2=A0 UINT64=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Size;= ++=C2=A0 Size =3D GetMemorySizeInMemoryTypeInformation ();++=C2=A0 return P= cdGet32 (PcdPeiMinMemSize) + Size + PEI_ADDITIONAL_MEMORY_SIZE;+}++/**+=C2= =A0 Allocate EfiACPIMemoryNVS below 4G memory address.++=C2=A0 This functio= n 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+AllocateAcpiNvsMemo= ryBelow4G (+=C2=A0 IN=C2=A0 =C2=A0UINTN=C2=A0 Size+=C2=A0 )+{+=C2=A0 UINTN= =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 EFI_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 Buffer = =3D (VOID *)(UINTN)Address;+=C2=A0 ZeroMem (Buffer, Size);++=C2=A0 return B= uffer;+}++/**+=C2=A0 Allocates memory to use on S3 resume.++=C2=A0 @param[i= n]=C2=A0 ImageHandle=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Not used.+=C2=A0 @pa= ram[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 completes suc= cessfully+=C2=A0 @retval=C2=A0 =C2=A0 =C2=A0EFI_OUT_OF_RESOURCES Insufficie= nt resources to create database+**/+EFI_STATUS+EFIAPI+S3DxeEntryPoint (+=C2= =A0 IN EFI_HANDLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 ImageHandle,+=C2=A0 IN EFI_SYS= TEM_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() Start\n= ", __FUNCTION__));++=C2=A0 S3PeiMemSize =3D GetPeiMemSize ();+=C2=A0 S= 3PeiMemBase =3D (UINTN) AllocateAcpiNvsMemoryBelow4G (S3PeiMemSize);+=C2=A0= ASSERT (S3PeiMemBase !=3D 0);++=C2=A0 S3MemoryInfo.S3PeiMemBase =3D S3PeiM= emBase;+=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 // = TODO: LockBox is potentially superior, though requires static location+=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_VO= LATILE | 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_EFI_ERROR= (Status);++=C2=A0 DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__= ));+=C2=A0 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.+#+# 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

--000000000000ac0f0305e7b6c677--