On 2015/9/15 17:28, Laszlo Ersek wrote:
On 09/14/15 15:58, Star Zeng wrote:
What to do:
1. Remove a hidden assumption "No SMM driver writes BootScript between
SmmReadyToLock and S3SleepEntryCallback".
  1.1. Use SmmExitBootServices and SmmLegacyBoot notification to record
       AtRuntime flag.
  1.2. Use mBootScriptDataBootTimeGuid LockBox to save boot time boot
       script data to handle potential INSERT boot script at runtime in SMM.
2. Do not depend on OS to help restore ACPINvs data and use
EfiReservedMemoryType instead of EfiACPIMemoryNVS.
  2.1. Use mBootScriptSmmPrivateDataGuid LockBox to save boot script
       SMM private data with BackFromS3 = TRUE at runtime. S3 resume
       will help restore it to tell the Library the system is back
       from S3.

Why to do:
1. The hidden assumption "No SMM driver writes BootScript between
SmmReadyToLock and S3SleepEntryCallback" will cause confusion to
the library's consumer and block the usage of "SMM driver writes
BootScript after SmmReadyToLock". So Remove the assumption.
2. In original code, there might be a corner case that malicious
code patch ACPINvs boot TableLength field same as SMM boot script.
So that it can skip the table restore. The impact is that BootScript
in SMM may be overridden by malicious code.
--------------------
     CopyMem ((VOID*)&TableHeader, (VOID*)mS3BootScriptTablePtr->TableBase, 
sizeof(EFI_BOOT_SCRIPT_TABLE_HEADER));
     if (mS3BootScriptTablePtr->TableLength + sizeof(EFI_BOOT_SCRIPT_TERMINATE) 
!= TableHeader.TableLength) { // TableLength is in NVS
       ......
       //
       // NOTE: We should NOT use TableHeader.TableLength, because it is 
already updated to be whole length.
       //
       mS3BootScriptTablePtr->TableLength = (UINT32)(mLockBoxLength - 
sizeof(EFI_BOOT_SCRIPT_TERMINATE)); ? This line can be skipped.
--------------------
So use EfiReservedMemoryType instead of EfiACPIMemoryNVS as the code
has been updated to not depend on OS to help restore ACPINvs data.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.z...@intel.com>
Reviewed-by: Jiewen Yao <jiewen....@intel.com>
---
  .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  | 477 ++++++++++++++-------
  .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf    |   4 +-
  .../PiDxeS3BootScriptLib/InternalBootScriptLib.h   |  17 +-
  MdeModulePkg/MdeModulePkg.dec                      |   8 +-
  MdeModulePkg/MdeModulePkg.uni                      | Bin 183916 -> 184014 
bytes
  5 files changed, 345 insertions(+), 161 deletions(-)

I've regression tested this (... it has been committed to SVN anyway)
with OVMF's S3, both with the code that is in the tree right now (ie.
without SMM support), and with my pending SMM series.

The SMM-less code uses just one opcode in the S3 boot script (an
information opcode). The SMM series adds two more opcodes (one
EFI_BOOT_SCRIPT_IO_READ_WRITE_OPCODE, and one
EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE). Both flavors are similar
insofar that they only add opcodes at boot time; no S3 bootscript
commands are added at runtime (by any SMM driver).

Everything seems to work fine; I didn't witness any regressions.

Very good information, thanks for that. ^_^

BTW: When will the SMM support be ready on OVMF?
I can image that we can develop SMM code on OVMF if no hardware board available.

Thanks,
Star

Thanks
Laszlo


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to