On 09/15/15 11:38, Zeng, Star wrote:
> 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?
Hey, that's a question that I ask Intel, and not the other way around! :)
Seriously though, Jordan has started reviewing the SMM series. A good
part of that series is OVMF specific, so for those patches it's "only" a
matter of time and the usual patch review process.
Another big part of the series is a port of several SMM-related drivers
from the Quark distribution into OvmfPkg. As far as I understand, Mike
Kinney and others have been looking into incorporating the same
SMM-related drivers in edk2, in a more central location, like
MdeModulePkg. For a number of drivers this actually means "open
sourcing", because e.g. the X64 SMM entry vector is not open source yet.
(I absolutely needed something to work with, which is why I followed
Jiewen's great suggestion to look at Quark. In any case, 64-bit support,
and multiple VCPU support too, are not optional for enterprise use; so
those remain to be developed / integrated with OVMF later. The current
series is "just" a starting point.)
... I believe that incorporation is happening, but it takes time.
To prevent regressions from creeping into my work in the meantime, I
keep the series in my own repo up-to-date. Whenever I pull from SVN, I
rebase, rebuild, and retest the series. Thankfully there have not been
many conflicts since I posted the it.
> I can image that we can develop SMM code on OVMF if no hardware board
> available.
Yes, I think that should be possible for higher-level SMM drivers (ie.
those that "just" need the SMM_CORE to be present and functioning, and
some SMRAM available).
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
