summaryrefslogtreecommitdiffstats
path: root/OvmfPkg/AcpiPlatformDxe
diff options
context:
space:
mode:
authorLaszlo Ersek <lersek@redhat.com>2017-08-06 11:20:02 +0200
committerLaszlo Ersek <lersek@redhat.com>2017-08-08 15:10:45 +0200
commit9e2a8e928995c3b1bb664b73fd59785055c6b5f6 (patch)
tree78a3ee066e47430a61b2cfc69d6b8fc6629fb7b9 /OvmfPkg/AcpiPlatformDxe
parent4cf3f37c87ba1f9d58072444bd735e40e4779e70 (diff)
downloadedk2-9e2a8e928995c3b1bb664b73fd59785055c6b5f6.tar.gz
edk2-9e2a8e928995c3b1bb664b73fd59785055c6b5f6.tar.bz2
edk2-9e2a8e928995c3b1bb664b73fd59785055c6b5f6.zip
OvmfPkg/AcpiPlatformDxe: short-circuit the transfer of an empty S3_CONTEXT
In commit 805762252733 ("OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib", 2017-02-23), we replaced the explicit S3 boot script manipulation in TransferS3ContextToBootScript() with a call to QemuFwCfgS3CallWhenBootScriptReady(). (Passing AppendFwCfgBootScript() as callback.) QemuFwCfgS3CallWhenBootScriptReady() checks for fw_cfg DMA up-front, and bails with RETURN_NOT_FOUND if fw_cfg DMA is missing. (This is justified as the goal of QemuFwCfgS3Lib is to "enable[] driver modules [...] to produce fw_cfg DMA operations that are to be replayed at S3 resume time".) In turn, if QemuFwCfgS3CallWhenBootScriptReady() fails, then OvmfPkg/AcpiPlatformDxe rolls back any earlier linker/loader script processing, and falls back to the built-in ACPI tables. (This is also justified because failure to save WRITE_POINTER commands for replaying at S3 resume implies failure to process the linker/loader script comprehensively.) Calling QemuFwCfgS3CallWhenBootScriptReady() from TransferS3ContextToBootScript() *unconditionally* is wrong however. For the case when the linker/loader script contains no WRITE_POINTER commands, the call perpetuated an earlier side effect, and introduced another one: (1) On machine types that provide fw_cfg DMA (i.e., 2.5+), QemuFwCfgS3CallWhenBootScriptReady() would succeed, and allocate workspace for the boot script opcodes in reserved memory. However, no opcodes would actually be produced in the AppendFwCfgBootScript() callback, due to lack of any WRITE_POINTER commands. This waste of reserved memory had been introduced in earlier commit df73df138d9d ("OvmfPkg/AcpiPlatformDxe: replay QEMU_LOADER_WRITE_POINTER commands at S3", 2017-02-09). (2) On machine types that lack fw_cfg DMA (i.e., 2.4 and earlier), TransferS3ContextToBootScript() would now fail the linker/loader script for no reason. (Note that QEMU itself prevents adding devices that depend on WRITE_POINTER if the machine type lacks fw_cfg DMA: $ qemu-system-x86_64 -M pc-q35-2.4 -device vmgenid qemu-system-x86_64: -device vmgenid: vmgenid requires DMA write support in fw_cfg, which this machine type does not provide) Short-circuit an empty S3_CONTEXT in TransferS3ContextToBootScript() by dropping S3_CONTEXT on the floor. This is compatible with the current contract of the function as it constitutes a transfer of ownership. Regression (2) was found and reported by Dhiru Kholia as an OSX guest boot failure on the "pc-q35-2.4" machine type: http://mid.mail-archive.com/CANO7a6x6EaWNZ8y=MvLU=w_LjRLXserO3NmsgHvaYE0aUCCWzg@mail.gmail.com Dhiru bisected the issue to commit 805762252733. Cc: Dhiru Kholia <dhiru.kholia@gmail.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Fixes: df73df138d9d53f7f7570f4fe97a6cde941a2656 Fixes: 805762252733bb67bc5157f0137c64e010724c77 Reported-by: Dhiru Kholia <dhiru.kholia@gmail.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Dhiru Kholia <dhiru.kholia@gmail.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Diffstat (limited to 'OvmfPkg/AcpiPlatformDxe')
-rw-r--r--OvmfPkg/AcpiPlatformDxe/BootScript.c10
1 files changed, 9 insertions, 1 deletions
diff --git a/OvmfPkg/AcpiPlatformDxe/BootScript.c b/OvmfPkg/AcpiPlatformDxe/BootScript.c
index a7d2f9e38f..1a188bfd61 100644
--- a/OvmfPkg/AcpiPlatformDxe/BootScript.c
+++ b/OvmfPkg/AcpiPlatformDxe/BootScript.c
@@ -249,7 +249,10 @@ FatalError:
because the ownership of S3Context has been transfered.
@retval EFI_SUCCESS The translation of S3Context to ACPI S3 Boot Script
- opcodes has been successfully executed or queued.
+ opcodes has been successfully executed or queued. (This
+ includes the case when S3Context was empty on input and
+ no ACPI S3 Boot Script opcodes have been necessary to
+ produce.)
@return Error codes from underlying functions.
**/
@@ -260,6 +263,11 @@ TransferS3ContextToBootScript (
{
RETURN_STATUS Status;
+ if (S3Context->Used == 0) {
+ ReleaseS3Context (S3Context);
+ return EFI_SUCCESS;
+ }
+
Status = QemuFwCfgS3CallWhenBootScriptReady (AppendFwCfgBootScript,
S3Context, sizeof (SCRATCH_BUFFER));
return (EFI_STATUS)Status;