public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel][edk2-platforms][PATCH v2 0/6] Implement S3 resume
@ 2022-09-06 17:02 Benjamin Doron
  2022-09-06 17:02 ` [edk2-devel][edk2-platforms][PATCH v2 1/6] {Platform,Silicon}/Intel: Move PcdAcpiBaseAddress definition Benjamin Doron
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:02 UTC (permalink / raw)
  To: devel

MinPlatform is an open-source EDK2 firmware project that can boot some
mainstream boards. However, it lacked working support for S3 resume, an
important feature for mobile platforms, which means that its
applicability as-is to mainstream use is limited. Therefore, I have now
implemented working S3 resume support on MinPlatform. This patch series
comprises a majority of one of my work products for GSoC 2022.

The BootScript-related modules are EDK2 open-source and fairly
straightforward to include. However, the partial dependency
PiSmmCommunicationPei (for signalling SMM) creates a dependency on both
the SmmAccess and SmmControl PPIs, so these are implemented here as
libraries, ported from the DXE drivers. As the register definitions are
generic, one library shim is implemented for compatibility, so the
library can be generic too. SmmAccess shall be required regardless of
SMM signalling, for the LockBox.

Like all boots, S3 resume will require working DRAM. To my
understanding, we do not need to parse the memory map HOBs again, so
some memory is allocated in DXE phase (to reserve it), the address
stashed in a variable and consumed on S3 resume flows. Some
optimisation can be performed here, regarding how much is necessary.
Stashing in a variable is imperfect, but the details must be available
without DRAM. As the FSP HOBs are published later, SmmAccess cannot be
used to retrieve from the LockBox.

Per my suspicions, notes from my mentors, Nate and Ankit, and the
coreboot code, the PAMs are opened for access to the AP wake vectors.
Presently, all are opened, more research can be performed here.

As noted, either FSP or PiSmmCpuDxeSmm can apply boot CPU structures
(GDT, IDT, MTRRs, etc). Per my research and findings by my mentors, the
closed-source module CpuInitDxe would be required, so this
implementation includes CpuS3DataDxe and open-source code will perform
this task instead.

Unfortunately, board-specific code has some tasks to perform too. I've
implemented these for Kabylake, the platform I can test. This includes
detecting the boot mode, policy (memory overwrite is contraindicated,
do not pass a VBT so FSP does not initialise graphics again) and
ensuring a special provision, if desired, for debugging
BootScriptExecutorDxe.

One major bug that blocked progress was simply specific to debugging.
DebugLibReportStatusCode should not be used for that module, because
RSC has uninstalled the serial port handler at end-of-BS. Furthermore,
it's obvious that the boot services are now unavailable. So,
DebugLibSerialPort should be used. As an aside, due to AutoGen ordering,
gBS cannot be used in SerialPortInitialize(). What really makes this
module a unique case is the fact that it's behaviour is very like
runtime drivers. For the integrity of the platform's security, the
module is copied to the LockBox at DxeSmmReadyToLock. This caused one
major bug that was blocking progress: libraries cannot attempt to
modify globals (the data section) with end-of-BS events; they will
never reach the true copy. So, rather than using flags to control code
flow, it must be coded this way instead. For instance, there is now a
special variant of I2cHdmiDebugSerialPortLib that does not use gBS in
SerialPortWrite().

Previous revisions of this patch-set tested on my Aspire VN7-572G
(Skylake). It's my belief and intention that this implementation be
ready for other platforms too (some Intel-specific assumptions made),
with a minimum of porting effort, though readying for some debugging is
recommended.

Some potential bugs include:
- Power failure is being set (PMC PWR_FLR), so BootMode == 0x0. This bit
  is RW/1C, so mitigate it. Until finalised and I close the laptop
  chassis, I don't know if this is a bug in Kabylake's PchPmcLib.
- Very early in testing I saw a memory init error, which means that
  self-refresh failed. A BaseMemoryTest() predictably failed too,
  inserted before the PEI core installs memory. Either this was fixed
  in the code as I finished the implementation, or it's a bug. A major
  difference in build options is SerialPortSpiFlash ->
  I2cHdmiDebugSerialPortLib, but this seemed irrelevant. If it's simply
  the finalised implementation, I think this isn't worth a diff against
  the reflog.

Benjamin Doron (6):
  {Platform,Silicon}/Intel: Move PcdAcpiBaseAddress definition
  IntelSiliconPkg/Feature/SmmAccess: Implement PPI with chipset support
  IntelSiliconPkg/Feature/SmmControl: Implement PPI with chipset support
  S3FeaturePkg: Implement working S3 resume
  MinPlatformPkg: Implement working S3 resume
  KabylakeOpenBoardPkg: Example of board S3

 .../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 +-
 .../CometlakeURvp/OpenBoardPkgPcd.dsc         |   1 +
 .../Features/Tbt/TbtInit/Smm/TbtSmm.inf       |   2 +-
 .../PeiFspMiscUpdUpdateLib.c                  |  12 +-
 .../PeiSaPolicyUpdate.c                       |  12 +-
 .../PeiAspireVn7Dash572GInitPreMemLib.c       |  61 ++-
 .../BoardInitLib/PeiBoardInitPreMemLib.inf    |   3 +
 .../AspireVn7Dash572G/OpenBoardPkg.dsc        |  21 +
 .../AspireVn7Dash572G/OpenBoardPkgPcd.dsc     |  17 +-
 .../PeiSiliconPolicyUpdateLib.c               |  11 +-
 .../PeiSiliconPolicyUpdateLib.inf             |   1 +
 .../Features/Tbt/TbtInit/Smm/TbtSmm.inf       |   2 +-
 .../PeiFspMiscUpdUpdateLib.c                  |  11 +-
 .../PeiSaPolicyUpdate.c                       |  12 +-
 .../BasePlatformHookLib.inf                   |   2 +-
 .../BoardInitLib/PeiBoardInitPreMemLib.inf    |   1 +
 .../BoardInitLib/PeiGalagoPro3InitPreMemLib.c |  27 +-
 .../PeiMultiBoardInitPreMemLib.inf            |   1 +
 .../GalagoPro3/OpenBoardPkg.dsc               |  15 +
 .../GalagoPro3/OpenBoardPkgPcd.dsc            |   3 +-
 .../PeiFspMiscUpdUpdateLib.c                  |  12 +-
 .../PeiSaPolicyUpdate.c                       |  12 +-
 .../BasePlatformHookLib.inf                   |   2 +-
 .../BoardInitLib/PeiBoardInitPreMemLib.inf    |   1 +
 .../PeiKabylakeRvp3InitPreMemLib.c            |  27 +-
 .../PeiMultiBoardInitPreMemLib.inf            |   1 +
 .../KabylakeRvp3/OpenBoardPkg.dsc             |  12 +
 .../KabylakeRvp3/OpenBoardPkgPcd.dsc          |   3 +-
 .../PeiSiliconPolicyUpdateLib.c               |  11 +-
 .../PeiSiliconPolicyUpdateLib.inf             |   1 +
 .../FspWrapperHobProcessLib.c                 |  69 ++-
 .../PeiFspWrapperHobProcessLib.inf            |   2 +
 .../Include/AcpiS3MemoryNvData.h              |  22 +
 .../Include/Dsc/CorePeiInclude.dsc            |   2 +
 .../Include/Fdf/CorePostMemoryInclude.fdf     |   2 +
 .../TigerlakeURvp/OpenBoardPkgPcd.dsc         |   1 +
 .../Features/Tbt/TbtInit/Smm/TbtSmm.inf       |   2 +-
 .../UpXtreme/OpenBoardPkgPcd.dsc              |   1 +
 .../WhiskeylakeURvp/OpenBoardPkgPcd.dsc       |   1 +
 .../CoffeelakeSiliconPkg.dsc                  |   1 +
 .../PeiSiliconInitLib/PeiSiliconInitLib.inf   |   5 +-
 .../PeiDxeSmmPmcLib/PeiDxeSmmPmcLib.inf       |   3 +-
 .../PeiDxeSmmPmcPrivateLibCnl.inf             |   2 +-
 Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec  |   1 -
 .../PeiSmmAccessLibSmramc/PeiSmmAccessLib.c   | 430 ++++++++++++++++++
 .../PeiSmmAccessLibSmramc/PeiSmmAccessLib.inf |  36 ++
 .../PeiSmmControlLib/PeiSmmControlLib.c       | 309 +++++++++++++
 .../PeiSmmControlLib/PeiSmmControlLib.inf     |  34 ++
 .../Include/Library/SmmControlLib.h           |  26 ++
 .../Intel/IntelSiliconPkg/IntelSiliconPkg.dec |   8 +
 .../KabylakeSiliconPkg/KabylakeSiliconPkg.dsc |   1 +
 .../PeiSiliconInitLib/PeiSiliconInitLib.inf   |   2 +-
 .../PeiDxeSmmPchPmcLib/PeiDxeSmmPchPmcLib.inf |   3 +-
 .../PeiPchPolicyLib/PeiPchPolicyLib.inf       |   2 +-
 .../Pch/Library/PeiSpiLib/PeiSpiLib.inf       |   2 +-
 Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec    |   1 -
 .../PeiDxeSmmPmcLib/PeiDxeSmmPmcLib.inf       |   3 +-
 .../PeiDxeSmmPmcPrivateLibVer2.inf            |   3 +-
 Silicon/Intel/TigerlakeSiliconPkg/SiPkg.dec   |   1 -
 .../TigerlakeSiliconPkg.dsc                   |   1 +
 66 files changed, 1557 insertions(+), 70 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
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.c
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.inf
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.c
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.inf
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Library/SmmControlLib.h

-- 
2.37.2


^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-09-11 15:46 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-06 17:02 [edk2-devel][edk2-platforms][PATCH v2 0/6] Implement S3 resume Benjamin Doron
2022-09-06 17:02 ` [edk2-devel][edk2-platforms][PATCH v2 1/6] {Platform,Silicon}/Intel: Move PcdAcpiBaseAddress definition Benjamin Doron
2022-09-07 22:50   ` Isaac Oram
2022-09-07 22:57   ` Chaganty, Rangasai V
2022-09-09 21:11     ` Isaac Oram
2022-09-06 17:02 ` [edk2-devel][edk2-platforms][PATCH v2 2/6] IntelSiliconPkg/Feature/SmmAccess: Implement PPI with chipset support Benjamin Doron
2022-09-07 23:49   ` Isaac Oram
     [not found]   ` <1712B8F6079EA3A9.20240@groups.io>
2022-09-09 21:13     ` Isaac Oram
2022-09-06 17:02 ` [edk2-devel][edk2-platforms][PATCH v2 3/6] IntelSiliconPkg/Feature/SmmControl: " Benjamin Doron
2022-09-07 23:50   ` Isaac Oram
     [not found]   ` <1712B905B760092F.20378@groups.io>
2022-09-09 21:16     ` Isaac Oram
2022-09-06 17:02 ` [edk2-devel][edk2-platforms][PATCH v2 4/6] S3FeaturePkg: Implement working S3 resume Benjamin Doron
2022-09-08  0:38   ` Isaac Oram
2022-09-11 15:46     ` Benjamin Doron
2022-09-06 17:02 ` [edk2-devel][edk2-platforms][PATCH v2 5/6] MinPlatformPkg: " Benjamin Doron
2022-09-08  2:46   ` Isaac Oram
2022-09-06 17:02 ` [edk2-devel][edk2-platforms][PATCH v2 6/6] KabylakeOpenBoardPkg: Example of board S3 Benjamin Doron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox