[edk2] [PATCH 12/12] OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib

Laszlo Ersek posted 12 patches 7 years, 8 months ago
There is a newer version of this series
[edk2] [PATCH 12/12] OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib
Posted by Laszlo Ersek 7 years, 8 months ago
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_APPEND_FUNCTION takes
ownership of Context.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf          |   1 -
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |   1 -
 OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h               |   2 +-
 OvmfPkg/AcpiPlatformDxe/BootScript.c                 | 262 +++++---------------
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c              |   7 +
 5 files changed, 64 insertions(+), 209 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 42edc97b3da2..9a9b2e6bb2e5 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -61,7 +61,6 @@ [LibraryClasses]
 [Protocols]
   gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
   gEfiPciIoProtocolGuid                         # PROTOCOL SOMETIMES_CONSUMED
-  gEfiS3SaveStateProtocolGuid                   # PROTOCOL SOMETIMES_CONSUMED
 
 [Guids]
   gEfiXenInfoGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
index a9350540215d..adc50cfd9f76 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
@@ -51,7 +51,6 @@ [LibraryClasses]
 [Protocols]
   gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
   gEfiPciIoProtocolGuid                         # PROTOCOL SOMETIMES_CONSUMED
-  gEfiS3SaveStateProtocolGuid                   # PROTOCOL SOMETIMES_CONSUMED
 
 [Guids]
   gRootBridgesConnectedEventGroupGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
index 0f035a0d5751..83b981ee005d 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/BootScript.c b/OvmfPkg/AcpiPlatformDxe/BootScript.c
index bff42ad8b9b0..424b4b3ee2bd 100644
--- a/OvmfPkg/AcpiPlatformDxe/BootScript.c
+++ b/OvmfPkg/AcpiPlatformDxe/BootScript.c
@@ -15,7 +15,7 @@
 
 #include <Library/MemoryAllocationLib.h>
 #include <Library/QemuFwCfgLib.h>
-#include <Protocol/S3SaveState.h>
+#include <Library/QemuFwCfgS3Lib.h>
 
 #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_APPEND_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;
+  S3_CONTEXT     *S3Context;
+  SCRATCH_BUFFER *ScratchBuffer;
+  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;
-  }
+  S3Context = Context;
+  ScratchBuffer = ExternalScratchBuffer;
 
-  ScratchBuffer = AllocateReservedPool (sizeof *ScratchBuffer);
-  if (ScratchBuffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // 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));
+    Status = QemuFwCfgS3SkipBytes (Condensed->PointerItem,
+               Condensed->PointerOffset);
+    if (RETURN_ERROR (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));
-      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 = QemuFwCfgS3WriteBytes (-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 = QemuFwCfgS3TransferOwnership (AppendFwCfgBootScript, S3Context,
+             sizeof (SCRATCH_BUFFER));
+  return (EFI_STATUS)Status;
 }
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index 76512534f5e0..7bb2e3f21821 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:
-- 
2.9.3

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