From 805762252733bb67bc5157f0137c64e010724c77 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 23 Feb 2017 00:22:17 +0100 Subject: [PATCH] OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib Drop the explicit S3SaveState protocol and opcode management; instead, create ACPI S3 Boot Script opcodes for the WRITE_POINTER commands with QemuFwCfgS3Lib functions. In this case, we have a dynamically allocated Context structure, hence the patch demonstrates how the FW_CFG_BOOT_SCRIPT_CALLBACK_FUNCTION takes ownership of Context. Cc: Jordan Justen Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jordan Justen --- OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 2 +- OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 1 - OvmfPkg/AcpiPlatformDxe/BootScript.c | 262 ++++-------------- OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 7 + .../QemuFwCfgAcpiPlatformDxe.inf | 1 - 5 files changed, 64 insertions(+), 209 deletions(-) diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h index 0f035a0d57..83b981ee00 100644 --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h @@ -115,7 +115,7 @@ SaveCondensedWritePointerToS3Context ( EFI_STATUS TransferS3ContextToBootScript ( - IN CONST S3_CONTEXT *S3Context + IN S3_CONTEXT *S3Context ); #endif diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf index 42edc97b3d..9a9b2e6bb2 100644 --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf @@ -61,7 +61,6 @@ [Protocols] gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CONSUMED - gEfiS3SaveStateProtocolGuid # PROTOCOL SOMETIMES_CONSUMED [Guids] gEfiXenInfoGuid diff --git a/OvmfPkg/AcpiPlatformDxe/BootScript.c b/OvmfPkg/AcpiPlatformDxe/BootScript.c index bff42ad8b9..a7d2f9e38f 100644 --- a/OvmfPkg/AcpiPlatformDxe/BootScript.c +++ b/OvmfPkg/AcpiPlatformDxe/BootScript.c @@ -15,7 +15,7 @@ #include #include -#include +#include #include "AcpiPlatform.h" @@ -53,19 +53,11 @@ struct S3_CONTEXT { // // Scratch buffer, allocated in EfiReservedMemoryType type memory, for the ACPI -// S3 Boot Script opcodes to work on. We use the buffer to compose and to -// replay several fw_cfg select+skip and write operations, using the DMA access -// method. The fw_cfg operations will implement the actions dictated by -// CONDENSED_WRITE_POINTER objects. +// S3 Boot Script opcodes to work on. // #pragma pack (1) -typedef struct { - FW_CFG_DMA_ACCESS Access; // filled in from - // CONDENSED_WRITE_POINTER.PointerItem, - // CONDENSED_WRITE_POINTER.PointerSize, - // CONDENSED_WRITE_POINTER.PointerOffset - UINT64 PointerValue; // filled in from - // CONDENSED_WRITE_POINTER.PointerValue +typedef union { + UINT64 PointerValue; // filled in from CONDENSED_WRITE_POINTER.PointerValue } SCRATCH_BUFFER; #pragma pack () @@ -197,220 +189,78 @@ SaveCondensedWritePointerToS3Context ( /** - Translate and append the information from an S3_CONTEXT object to the ACPI S3 - Boot Script. - - The effects of a successful call to this function cannot be undone. - - @param[in] S3Context The S3_CONTEXT object to translate to ACPI S3 Boot - Script opcodes. - - @retval EFI_OUT_OF_RESOURCES Out of memory. - - @retval EFI_SUCCESS The translation of S3Context to ACPI S3 Boot - Script opcodes has been successful. - - @return Error codes from underlying functions. + FW_CFG_BOOT_SCRIPT_CALLBACK_FUNCTION provided to QemuFwCfgS3Lib. **/ -EFI_STATUS -TransferS3ContextToBootScript ( - IN CONST S3_CONTEXT *S3Context +STATIC +VOID +EFIAPI +AppendFwCfgBootScript ( + IN OUT VOID *Context, OPTIONAL + IN OUT VOID *ExternalScratchBuffer ) { - EFI_STATUS Status; - EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState; - SCRATCH_BUFFER *ScratchBuffer; - FW_CFG_DMA_ACCESS *Access; - UINT64 BigEndianAddressOfAccess; - UINT32 ControlPollData; - UINT32 ControlPollMask; - UINTN Index; - - // - // If the following protocol lookup fails, it shall not happen due to an - // unexpected DXE driver dispatch order. - // - // Namely, this function is only invoked on QEMU. Therefore it is only - // reached after Platform BDS signals gRootBridgesConnectedEventGroupGuid - // (see OnRootBridgesConnected() in "EntryPoint.c"). Hence, because - // TransferS3ContextToBootScript() is invoked in BDS, all DXE drivers, - // including S3SaveStateDxe (producing EFI_S3_SAVE_STATE_PROTOCOL), have been - // dispatched by the time we get here. (S3SaveStateDxe is not expected to - // have any stricter-than-TRUE DEPEX -- not a DEPEX that gets unblocked only - // within BDS anyway.) - // - // Reaching this function also depends on QemuFwCfgS3Enabled(). That implies - // S3SaveStateDxe has not exited immediately due to S3 being disabled. Thus - // EFI_S3_SAVE_STATE_PROTOCOL can only be missing for genuinely unforeseeable - // reasons. - // - Status = gBS->LocateProtocol (&gEfiS3SaveStateProtocolGuid, - NULL /* Registration */, (VOID **)&S3SaveState); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: LocateProtocol(): %r\n", __FUNCTION__, Status)); - return Status; - } + S3_CONTEXT *S3Context; + SCRATCH_BUFFER *ScratchBuffer; + UINTN Index; - ScratchBuffer = AllocateReservedPool (sizeof *ScratchBuffer); - if (ScratchBuffer == NULL) { - return EFI_OUT_OF_RESOURCES; - } + S3Context = Context; + ScratchBuffer = ExternalScratchBuffer; - // - // Set up helper variables that we'll use identically for all - // CONDENSED_WRITE_POINTER elements. - // - Access = &ScratchBuffer->Access; - BigEndianAddressOfAccess = SwapBytes64 ((UINTN)Access); - ControlPollData = 0; - ControlPollMask = MAX_UINT32; - - // - // For each CONDENSED_WRITE_POINTER, we need six ACPI S3 Boot Script opcodes: - // (1) restore an FW_CFG_DMA_ACCESS object in reserved memory that selects - // the writeable fw_cfg file PointerFile (through PointerItem), and skips - // to PointerOffset in it, - // (2) call QEMU with the FW_CFG_DMA_ACCESS object, - // (3) wait for the select+skip to finish, - // (4) restore a SCRATCH_BUFFER object in reserved memory that writes - // PointerValue (base address of the allocated / downloaded PointeeFile, - // plus PointeeOffset), of size PointerSize, into the fw_cfg file - // selected in (1), at the offset sought to in (1), - // (5) call QEMU with the FW_CFG_DMA_ACCESS object, - // (6) wait for the write to finish. - // - // EFI_S3_SAVE_STATE_PROTOCOL does not allow rolling back opcode additions, - // therefore we treat any failure here as fatal. - // for (Index = 0; Index < S3Context->Used; ++Index) { CONST CONDENSED_WRITE_POINTER *Condensed; + RETURN_STATUS Status; Condensed = &S3Context->WritePointers[Index]; - // - // (1) restore an FW_CFG_DMA_ACCESS object in reserved memory that selects - // the writeable fw_cfg file PointerFile (through PointerItem), and - // skips to PointerOffset in it, - // - Access->Control = SwapBytes32 ((UINT32)Condensed->PointerItem << 16 | - FW_CFG_DMA_CTL_SELECT | FW_CFG_DMA_CTL_SKIP); - Access->Length = SwapBytes32 (Condensed->PointerOffset); - Access->Address = 0; - Status = S3SaveState->Write ( - S3SaveState, // This - EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE, // OpCode - EfiBootScriptWidthUint8, // Width - (UINT64)(UINTN)Access, // Address - sizeof *Access, // Count - Access // Buffer - ); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Index %Lu opcode 1: %r\n", __FUNCTION__, - (UINT64)Index, Status)); - goto FatalError; - } - - // - // (2) call QEMU with the FW_CFG_DMA_ACCESS object, - // - Status = S3SaveState->Write ( - S3SaveState, // This - EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode - EfiBootScriptWidthUint32, // Width - (UINT64)FW_CFG_IO_DMA_ADDRESS, // Address - (UINTN)2, // Count - &BigEndianAddressOfAccess // Buffer - ); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Index %Lu opcode 2: %r\n", __FUNCTION__, - (UINT64)Index, Status)); + Status = QemuFwCfgS3ScriptSkipBytes (Condensed->PointerItem, + Condensed->PointerOffset); + if (RETURN_ERROR (Status)) { goto FatalError; } - // - // (3) wait for the select+skip to finish, - // - Status = S3SaveState->Write ( - S3SaveState, // This - EFI_BOOT_SCRIPT_MEM_POLL_OPCODE, // OpCode - EfiBootScriptWidthUint32, // Width - (UINT64)(UINTN)&Access->Control, // Address - &ControlPollData, // Data - &ControlPollMask, // DataMask - MAX_UINT64 // Delay - ); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Index %Lu opcode 3: %r\n", __FUNCTION__, - (UINT64)Index, Status)); - goto FatalError; - } - - // - // (4) restore a SCRATCH_BUFFER object in reserved memory that writes - // PointerValue (base address of the allocated / downloaded - // PointeeFile, plus PointeeOffset), of size PointerSize, into the - // fw_cfg file selected in (1), at the offset sought to in (1), - // - Access->Control = SwapBytes32 (FW_CFG_DMA_CTL_WRITE); - Access->Length = SwapBytes32 (Condensed->PointerSize); - Access->Address = SwapBytes64 ((UINTN)&ScratchBuffer->PointerValue); ScratchBuffer->PointerValue = Condensed->PointerValue; - Status = S3SaveState->Write ( - S3SaveState, // This - EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE, // OpCode - EfiBootScriptWidthUint8, // Width - (UINT64)(UINTN)ScratchBuffer, // Address - sizeof *ScratchBuffer, // Count - ScratchBuffer // Buffer - ); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Index %Lu opcode 4: %r\n", __FUNCTION__, - (UINT64)Index, Status)); - goto FatalError; - } - - // - // (5) call QEMU with the FW_CFG_DMA_ACCESS object, - // - Status = S3SaveState->Write ( - S3SaveState, // This - EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode - EfiBootScriptWidthUint32, // Width - (UINT64)FW_CFG_IO_DMA_ADDRESS, // Address - (UINTN)2, // Count - &BigEndianAddressOfAccess // Buffer - ); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Index %Lu opcode 5: %r\n", __FUNCTION__, - (UINT64)Index, Status)); - goto FatalError; - } - - // - // (6) wait for the write to finish. - // - Status = S3SaveState->Write ( - S3SaveState, // This - EFI_BOOT_SCRIPT_MEM_POLL_OPCODE, // OpCode - EfiBootScriptWidthUint32, // Width - (UINT64)(UINTN)&Access->Control, // Address - &ControlPollData, // Data - &ControlPollMask, // DataMask - MAX_UINT64 // Delay - ); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Index %Lu opcode 6: %r\n", __FUNCTION__, - (UINT64)Index, Status)); + Status = QemuFwCfgS3ScriptWriteBytes (-1, Condensed->PointerSize); + if (RETURN_ERROR (Status)) { goto FatalError; } } - DEBUG ((DEBUG_VERBOSE, "%a: boot script fragment saved, ScratchBuffer=%p\n", - __FUNCTION__, (VOID *)ScratchBuffer)); - return EFI_SUCCESS; + DEBUG ((DEBUG_VERBOSE, "%a: boot script fragment saved\n", __FUNCTION__)); + + ReleaseS3Context (S3Context); + return; FatalError: ASSERT (FALSE); CpuDeadLoop (); - return Status; +} + + +/** + Translate and append the information from an S3_CONTEXT object to the ACPI S3 + Boot Script. + + The effects of a successful call to this function cannot be undone. + + @param[in] S3Context The S3_CONTEXT object to translate to ACPI S3 Boot + Script opcodes. If the function returns successfully, + the caller must set the S3Context pointer -- originally + returned by AllocateS3Context() -- immediately to NULL, + 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. + + @return Error codes from underlying functions. +**/ +EFI_STATUS +TransferS3ContextToBootScript ( + IN S3_CONTEXT *S3Context + ) +{ + RETURN_STATUS Status; + + Status = QemuFwCfgS3CallWhenBootScriptReady (AppendFwCfgBootScript, + S3Context, sizeof (SCRATCH_BUFFER)); + return (EFI_STATUS)Status; } diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c index 76512534f5..7bb2e3f218 100644 --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c @@ -848,6 +848,13 @@ InstallQemuFwCfgTables ( // if (S3Context != NULL) { Status = TransferS3ContextToBootScript (S3Context); + if (EFI_ERROR (Status)) { + goto UninstallAcpiTables; + } + // + // Ownership of S3Context has been transfered. + // + S3Context = NULL; } UninstallAcpiTables: diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf index a935054021..adc50cfd9f 100644 --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf @@ -51,7 +51,6 @@ [Protocols] gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CONSUMED - gEfiS3SaveStateProtocolGuid # PROTOCOL SOMETIMES_CONSUMED [Guids] gRootBridgesConnectedEventGroupGuid -- 2.39.2