tyler.durden/audk
1
0
Fork 0
mirror of https://github.com/acidanthera/audk.git synced 2025-04-07 19:45:07 +02:00
Code Issues Packages Projects Releases Wiki Activity

OvmfPkg/AcpiPlatformDxe: short-circuit the transfer of an empty S3_CONTEXT

Browse Source 
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>
This commit is contained in:
Laszlo Ersek 2017-08-06 11:20:02 +02:00
parent 4cf3f37c87
commit 9e2a8e9289
1 changed files with 9 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
OvmfPkg/AcpiPlatformDxe/BootScript.c
View File

@ -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;