public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jordan Justen <jordan.l.justen@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	edk2-devel-01 <edk2-devel@lists.01.org>
Subject: Re: [PATCH 5/5] OvmfPkg/AcpiPlatformDxe: replay QEMU_LOADER_WRITE_POINTER commands at S3
Date: Fri, 17 Feb 2017 13:25:49 -0800	[thread overview]
Message-ID: <148736674932.16600.18157429547029640715@jljusten-ivb> (raw)
In-Reply-To: <20170216204137.30221-6-lersek@redhat.com>

On 2017-02-16 12:41:37, Laszlo Ersek wrote:
> Ultimately, each QEMU_LOADER_WRITE_POINTER command creates a guest memory
> reference in some QEMU device. When the virtual machine is reset, the
> device willfully forgets the guest address, since the guest memory is
> wholly invalidated during platform reset.
> 
> ... Unless the reset is part of S3 resume. Then the guest memory is
> preserved intact, and the firmware must reprogram those devices with the
> original guest memory allocation addresses.
> 
> This patch accumulates the fw_cfg select, skip and write operations of
> ProcessCmdWritePointer() in a validated / condensed form, and turns them
> into an ACPI S3 Boot Script fragment at the very end of
> InstallQemuFwCfgTables().
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=359
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf          |   2 +
>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |   2 +
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h               |  27 ++
>  OvmfPkg/AcpiPlatformDxe/BootScript.c                 | 414 ++++++++++++++++++++
>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c              |  70 +++-
>  5 files changed, 510 insertions(+), 5 deletions(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> index 654d3a03905d..bb5f14e0fc7a 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> @@ -31,10 +31,11 @@ [Sources]
>    Qemu.c
>    QemuFwCfgAcpi.c
>    Xen.c
>    EntryPoint.c
>    PciDecoding.c
> +  BootScript.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    OvmfPkg/OvmfPkg.dec
> @@ -57,10 +58,11 @@ [LibraryClasses]
>    OrderedCollectionLib
>  
>  [Protocols]
>    gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
>    gEfiPciIoProtocolGuid                         # PROTOCOL SOMETIMES_CONSUMED
> +  gEfiS3SaveStateProtocolGuid                   # PROTOCOL SOMETIMES_CONSUMED
>  
>  [Guids]
>    gEfiXenInfoGuid
>    gRootBridgesConnectedEventGroupGuid
>  
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
> index d99f2d5a95c7..e550ff5a4714 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
> @@ -29,10 +29,11 @@ [Defines]
>  [Sources]
>    QemuFwCfgAcpiPlatform.c
>    QemuFwCfgAcpi.c
>    EntryPoint.c
>    PciDecoding.c
> +  BootScript.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    OvmfPkg/OvmfPkg.dec
> @@ -47,10 +48,11 @@ [LibraryClasses]
>    UefiDriverEntryPoint
>  
>  [Protocols]
>    gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
>    gEfiPciIoProtocolGuid                         # PROTOCOL SOMETIMES_CONSUMED
> +  gEfiS3SaveStateProtocolGuid                   # PROTOCOL SOMETIMES_CONSUMED
>  
>  [Guids]
>    gRootBridgesConnectedEventGroupGuid
>  
>  [Pcd]
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> index 08dd7f8f7dd7..0f035a0d5751 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> @@ -31,10 +31,12 @@
>  typedef struct {
>    EFI_PCI_IO_PROTOCOL *PciIo;
>    UINT64              PciAttributes;
>  } ORIGINAL_ATTRIBUTES;
>  
> +typedef struct S3_CONTEXT S3_CONTEXT;
> +
>  EFI_STATUS
>  EFIAPI
>  InstallAcpiTable (
>    IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol,
>    IN   VOID                          *AcpiTableBuffer,
> @@ -89,7 +91,32 @@ VOID
>  RestorePciDecoding (
>    IN ORIGINAL_ATTRIBUTES *OriginalAttributes,
>    IN UINTN               Count
>    );
>  
> +EFI_STATUS
> +AllocateS3Context (
> +  OUT S3_CONTEXT **S3Context,
> +  IN  UINTN      WritePointerCount
> +  );
> +
> +VOID
> +ReleaseS3Context (
> +  IN S3_CONTEXT *S3Context
> +  );
> +
> +EFI_STATUS
> +SaveCondensedWritePointerToS3Context (
> +  IN OUT S3_CONTEXT *S3Context,
> +  IN     UINT16     PointerItem,
> +  IN     UINT8      PointerSize,
> +  IN     UINT32     PointerOffset,
> +  IN     UINT64     PointerValue
> +  );
> +
> +EFI_STATUS
> +TransferS3ContextToBootScript (
> +  IN CONST S3_CONTEXT *S3Context
> +  );
> +
>  #endif
>  
> diff --git a/OvmfPkg/AcpiPlatformDxe/BootScript.c b/OvmfPkg/AcpiPlatformDxe/BootScript.c
> new file mode 100644
> index 000000000000..b7a7f270f223
> --- /dev/null
> +++ b/OvmfPkg/AcpiPlatformDxe/BootScript.c
> @@ -0,0 +1,414 @@
> +/** @file
> +  Append an ACPI S3 Boot Script fragment from the QEMU_LOADER_WRITE_POINTER
> +  commands of QEMU's fully processed table linker/loader script.
> +
> +  Copyright (C) 2017, Red Hat, Inc.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/QemuFwCfgLib.h>
> +#include <Protocol/S3SaveState.h>
> +
> +#include "AcpiPlatform.h"
> +
> +
> +//
> +// Condensed structure for capturing the fw_cfg operations -- select, skip,
> +// write -- inherent in executing a QEMU_LOADER_WRITE_POINTER command.
> +//
> +typedef struct {
> +  UINT16 PointerItem;   // resolved from QEMU_LOADER_WRITE_POINTER.PointerFile
> +  UINT8  PointerSize;   // copied as-is from QEMU_LOADER_WRITE_POINTER
> +  UINT32 PointerOffset; // copied as-is from QEMU_LOADER_WRITE_POINTER
> +  UINT64 PointerValue;  // resolved from QEMU_LOADER_WRITE_POINTER.PointeeFile
> +} CONDENSED_WRITE_POINTER;
> +
> +
> +//
> +// Context structure to accumulate CONDENSED_WRITE_POINTER objects from
> +// QEMU_LOADER_WRITE_POINTER commands.
> +//
> +// Any pointers in this structure own the pointed-to objects; that is, when the
> +// context structure is released, all pointed-to objects must be released too.
> +//
> +struct S3_CONTEXT {
> +  CONDENSED_WRITE_POINTER *WritePointers; // one array element per processed
> +                                          //   QEMU_LOADER_WRITE_POINTER
> +                                          //   command
> +  UINTN                   Allocated;      // number of elements allocated for
> +                                          //   WritePointers
> +  UINTN                   Used;           // number of elements populated in
> +                                          //   WritePointers
> +};
> +
> +
> +//
> +// 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.
> +//
> +#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
> +} SCRATCH_BUFFER;
> +#pragma pack ()
> +
> +
> +/**
> +  Allocate an S3_CONTEXT object.
> +
> +  @param[out] S3Context         The allocated S3_CONTEXT object is returned
> +                                through this parameter.
> +
> +  @param[in] WritePointerCount  Number of CONDENSED_WRITE_POINTER elements to
> +                                allocate room for. WritePointerCount must be
> +                                positive.
> +
> +  @retval EFI_SUCCESS            Allocation successful.
> +
> +  @retval EFI_OUT_OF_RESOURCES   Out of memory.
> +
> +  @retval EFI_INVALID_PARAMETER  WritePointerCount is zero.
> +**/
> +EFI_STATUS
> +AllocateS3Context (
> +  OUT S3_CONTEXT **S3Context,
> +  IN  UINTN      WritePointerCount
> +  )
> +{
> +  EFI_STATUS Status;
> +  S3_CONTEXT *Context;
> +
> +  if (WritePointerCount == 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Context = AllocateZeroPool (sizeof *Context);
> +  if (Context == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Context->WritePointers = AllocatePool (WritePointerCount *
> +                             sizeof *Context->WritePointers);
> +  if (Context->WritePointers == NULL) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto FreeContext;
> +  }
> +
> +  Context->Allocated = WritePointerCount;
> +  *S3Context = Context;
> +  return EFI_SUCCESS;
> +
> +FreeContext:
> +  FreePool (Context);
> +
> +  return Status;
> +}
> +
> +
> +/**
> +  Release an S3_CONTEXT object.
> +
> +  @param[in] S3Context  The object to release.
> +**/
> +VOID
> +ReleaseS3Context (
> +  IN S3_CONTEXT *S3Context
> +  )
> +{
> +  FreePool (S3Context->WritePointers);
> +  FreePool (S3Context);
> +}
> +
> +
> +/**
> +  Save the information necessary to replicate a QEMU_LOADER_WRITE_POINTER
> +  command during S3 resume, in condensed format.
> +
> +  This function is to be called from ProcessCmdWritePointer(), after all the
> +  sanity checks have passed, and before the fw_cfg operations are performed.
> +
> +  @param[in,out] S3Context  The S3_CONTEXT object into which the caller wants
> +                            to save the information that was derived from
> +                            QEMU_LOADER_WRITE_POINTER.
> +
> +  @param[in] PointerItem    The FIRMWARE_CONFIG_ITEM that
> +                            QEMU_LOADER_WRITE_POINTER.PointerFile was resolved
> +                            to, expressed as a UINT16 value.
> +
> +  @param[in] PointerSize    Copied directly from
> +                            QEMU_LOADER_WRITE_POINTER.PointerSize.
> +
> +  @param[in] PointerOffset  Copied directly from
> +                            QEMU_LOADER_WRITE_POINTER.PointerOffset.
> +
> +  @param[in] PointerValue   The base address of the allocated / downloaded
> +                            fw_cfg blob that is identified by
> +                            QEMU_LOADER_WRITE_POINTER.PointeeFile.
> +
> +  @retval EFI_SUCCESS           The information derived from
> +                                QEMU_LOADER_WRITE_POINTER has been successfully
> +                                absorbed into S3Context.
> +
> +  @retval EFI_OUT_OF_RESOURCES  No room available in S3Context.
> +**/
> +EFI_STATUS
> +SaveCondensedWritePointerToS3Context (
> +  IN OUT S3_CONTEXT *S3Context,
> +  IN     UINT16     PointerItem,
> +  IN     UINT8      PointerSize,
> +  IN     UINT32     PointerOffset,
> +  IN     UINT64     PointerValue
> +  )
> +{
> +  CONDENSED_WRITE_POINTER *Condensed;
> +
> +  if (S3Context->Used == S3Context->Allocated) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +  Condensed = S3Context->WritePointers + S3Context->Used;
> +  Condensed->PointerItem   = PointerItem;
> +  Condensed->PointerSize   = PointerSize;
> +  Condensed->PointerOffset = PointerOffset;
> +  Condensed->PointerValue  = PointerValue;
> +  DEBUG ((DEBUG_VERBOSE, "%a: 0x%04x/[0x%08x+%d] := 0x%Lx (%Lu)\n",
> +    __FUNCTION__, PointerItem, PointerOffset, PointerSize, PointerValue,
> +    (UINT64)S3Context->Used));
> +  ++S3Context->Used;
> +  return EFI_SUCCESS;
> +}
> +
> +
> +/**
> +  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.
> +**/
> +EFI_STATUS
> +TransferS3ContextToBootScript (
> +  IN CONST S3_CONTEXT *S3Context
> +  )
> +{
> +  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;
> +  }
> +
> +  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),
> +  //     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;
> +
> +    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)0x514,                   // Address

It's unfortunate that the boot script makes us add fw-cfg low level
details into a module besides QemuFwCfgLib.

We probably should add OvmfPkg/Include/IndustryStandard/QemuFwCfg.h to
define the IA32/X64 I/O ports used by fw-cfg. Maybe move some other
items from QemuFwCfgLib.h too.

This is using the DMA access, right? Does something prevent adding
this boot script entry when the DMA interface is not supported?

-Jordan

> +                            (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), 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)0x514,                   // 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));
> +      goto FatalError;
> +    }
> +  }
> +
> +  DEBUG ((DEBUG_VERBOSE, "%a: boot script fragment saved, ScratchBuffer=%p\n",
> +    __FUNCTION__, (VOID *)ScratchBuffer));
> +  return EFI_SUCCESS;
> +
> +FatalError:
> +  ASSERT (FALSE);
> +  CpuDeadLoop ();
> +  return Status;
> +}
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> index de827c2df204..eadd690bef4e 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> @@ -358,26 +358,39 @@ ProcessCmdAddChecksum (
>    @param[in] WritePointer   The QEMU_LOADER_WRITE_POINTER command to process.
>  
>    @param[in] Tracker        The ORDERED_COLLECTION tracking the BLOB user
>                              structures created thus far.
>  
> +  @param[in,out] S3Context  The S3_CONTEXT object capturing the fw_cfg actions
> +                            of successfully processed QEMU_LOADER_WRITE_POINTER
> +                            commands, to be replayed at S3 resume. S3Context
> +                            may be NULL if S3 is disabled.
> +
>    @retval EFI_PROTOCOL_ERROR  Malformed fw_cfg file name(s) have been found in
>                                WritePointer. Or, the WritePointer command
>                                references a file unknown to Tracker or the
>                                fw_cfg directory. Or, the pointer object to
>                                rewrite has invalid location, size, or initial
>                                relative value. Or, the pointer value to store
>                                does not fit in the given pointer size.
>  
>    @retval EFI_SUCCESS         The pointer object inside the writeable fw_cfg
> -                              file has been written.
> +                              file has been written. If S3Context is not NULL,
> +                              then WritePointer has been condensed into
> +                              S3Context.
> +
> +  @return                     Error codes propagated from
> +                              SaveCondensedWritePointerToS3Context(). The
> +                              pointer object inside the writeable fw_cfg file
> +                              has not been written.
>  **/
>  STATIC
>  EFI_STATUS
>  ProcessCmdWritePointer (
>    IN     CONST QEMU_LOADER_WRITE_POINTER *WritePointer,
> -  IN     CONST ORDERED_COLLECTION        *Tracker
> +  IN     CONST ORDERED_COLLECTION        *Tracker,
> +  IN OUT       S3_CONTEXT                *S3Context OPTIONAL
>    )
>  {
>    RETURN_STATUS            Status;
>    FIRMWARE_CONFIG_ITEM     PointerItem;
>    UINTN                    PointerItemSize;
> @@ -430,10 +443,29 @@ ProcessCmdWritePointer (
>      DEBUG ((DEBUG_ERROR, "%a: pointer value unrepresentable in \"%a\"\n",
>        __FUNCTION__, WritePointer->PointerFile));
>      return EFI_PROTOCOL_ERROR;
>    }
>  
> +  //
> +  // If S3 is enabled, we have to capture the below fw_cfg actions in condensed
> +  // form, to be replayed during S3 resume.
> +  //
> +  if (S3Context != NULL) {
> +    EFI_STATUS SaveStatus;
> +
> +    SaveStatus = SaveCondensedWritePointerToS3Context (
> +                   S3Context,
> +                   (UINT16)PointerItem,
> +                   WritePointer->PointerSize,
> +                   WritePointer->PointerOffset,
> +                   PointerValue
> +                   );
> +    if (EFI_ERROR (SaveStatus)) {
> +      return SaveStatus;
> +    }
> +  }
> +
>    QemuFwCfgSelectItem (PointerItem);
>    QemuFwCfgSkipBytes (WritePointer->PointerOffset);
>    QemuFwCfgWriteBytes (WritePointer->PointerSize, &PointerValue);
>  
>    //
> @@ -699,10 +731,11 @@ InstallQemuFwCfgTables (
>    QEMU_LOADER_ENTRY        *LoaderStart;
>    CONST QEMU_LOADER_ENTRY  *LoaderEntry, *LoaderEnd;
>    CONST QEMU_LOADER_ENTRY  *WritePointerSubsetEnd;
>    ORIGINAL_ATTRIBUTES      *OriginalPciAttributes;
>    UINTN                    OriginalPciAttributesCount;
> +  S3_CONTEXT               *S3Context;
>    ORDERED_COLLECTION       *Tracker;
>    UINTN                    *InstalledKey;
>    INT32                    Installed;
>    ORDERED_COLLECTION_ENTRY *TrackerEntry, *TrackerEntry2;
>  
> @@ -724,14 +757,26 @@ InstallQemuFwCfgTables (
>    QemuFwCfgSelectItem (FwCfgItem);
>    QemuFwCfgReadBytes (FwCfgSize, LoaderStart);
>    RestorePciDecoding (OriginalPciAttributes, OriginalPciAttributesCount);
>    LoaderEnd = LoaderStart + FwCfgSize / sizeof *LoaderEntry;
>  
> +  S3Context = NULL;
> +  if (QemuFwCfgS3Enabled ()) {
> +    //
> +    // Size the allocation pessimistically, assuming that all commands in the
> +    // script are QEMU_LOADER_WRITE_POINTER commands.
> +    //
> +    Status = AllocateS3Context (&S3Context, LoaderEnd - LoaderStart);
> +    if (EFI_ERROR (Status)) {
> +      goto FreeLoader;
> +    }
> +  }
> +
>    Tracker = OrderedCollectionInit (BlobCompare, BlobKeyCompare);
>    if (Tracker == NULL) {
>      Status = EFI_OUT_OF_RESOURCES;
> -    goto FreeLoader;
> +    goto FreeS3Context;
>    }
>  
>    //
>    // first pass: process the commands
>    //
> @@ -756,11 +801,11 @@ InstallQemuFwCfgTables (
>                   Tracker);
>        break;
>  
>      case QemuLoaderCmdWritePointer:
>          Status = ProcessCmdWritePointer (&LoaderEntry->Command.WritePointer,
> -                   Tracker);
> +                   Tracker, S3Context);
>          if (!EFI_ERROR (Status)) {
>            WritePointerSubsetEnd = LoaderEntry + 1;
>          }
>          break;
>  
> @@ -788,15 +833,25 @@ InstallQemuFwCfgTables (
>    for (LoaderEntry = LoaderStart; LoaderEntry < LoaderEnd; ++LoaderEntry) {
>      if (LoaderEntry->Type == QemuLoaderCmdAddPointer) {
>        Status = Process2ndPassCmdAddPointer (&LoaderEntry->Command.AddPointer,
>                   Tracker, AcpiProtocol, InstalledKey, &Installed);
>        if (EFI_ERROR (Status)) {
> -        break;
> +        goto UninstallAcpiTables;
>        }
>      }
>    }
>  
> +  //
> +  // Translating the condensed QEMU_LOADER_WRITE_POINTER commands to ACPI S3
> +  // Boot Script opcodes has to be the last operation in this function, because
> +  // if it succeeds, it cannot be undone.
> +  //
> +  if (S3Context != NULL) {
> +    Status = TransferS3ContextToBootScript (S3Context);
> +  }
> +
> +UninstallAcpiTables:
>    if (EFI_ERROR (Status)) {
>      //
>      // roll back partial installation
>      //
>      while (Installed > 0) {
> @@ -845,10 +900,15 @@ RollbackWritePointersAndFreeTracker:
>      }
>      FreePool (Blob);
>    }
>    OrderedCollectionUninit (Tracker);
>  
> +FreeS3Context:
> +  if (S3Context != NULL) {
> +    ReleaseS3Context (S3Context);
> +  }
> +
>  FreeLoader:
>    FreePool (LoaderStart);
>  
>    return Status;
>  }
> -- 
> 2.9.3
> 


  reply	other threads:[~2017-02-17 21:25 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-16 20:41 [PATCH 0/5] OvmfPkg: support QEMU_LOADER_WRITE_POINTER Laszlo Ersek
2017-02-16 20:41 ` [PATCH 1/5] OvmfPkg/AcpiPlatformDxe: prepare for QEMU_LOADER_WRITE_POINTER definitions Laszlo Ersek
2017-02-16 20:41 ` [PATCH 2/5] OvmfPkg/AcpiPlatformDxe: add " Laszlo Ersek
2017-02-16 20:41 ` [PATCH 3/5] OvmfPkg/AcpiPlatformDxe: rewrap license block in "QemuFwCfgAcpi.c" Laszlo Ersek
2017-02-16 20:41 ` [PATCH 4/5] OvmfPkg/AcpiPlatformDxe: implement the QEMU_LOADER_WRITE_POINTER command Laszlo Ersek
2017-02-17 19:34   ` Jordan Justen
2017-02-17 20:51     ` Laszlo Ersek
2017-02-16 20:41 ` [PATCH 5/5] OvmfPkg/AcpiPlatformDxe: replay QEMU_LOADER_WRITE_POINTER commands at S3 Laszlo Ersek
2017-02-17 21:25   ` Jordan Justen [this message]
2017-02-17 22:41     ` Laszlo Ersek
2017-02-17 22:48       ` Laszlo Ersek
2017-02-21  0:17         ` Jordan Justen
2017-02-21 12:15           ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=148736674932.16600.18157429547029640715@jljusten-ivb \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox