public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@ml01.01.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Subject: [PATCH 11/12] OvmfPkg/SmmControl2Dxe: save fw_cfg boot script with QemuFwCfgS3Lib
Date: Thu, 23 Feb 2017 02:48:13 +0100	[thread overview]
Message-ID: <20170223014814.10937-12-lersek@redhat.com> (raw)
In-Reply-To: <20170223014814.10937-1-lersek@redhat.com>

We cannot entirely eliminate the manual boot script building in this
driver, as it also programs lower-level chipset registers (SMI_EN,
GEN_PMCON_1) at S3 resume, not just registers exposed via fw_cfg.

We can nonetheless replace the manually built opcodes for the latter class
of registers with QemuFwCfgS3Lib function calls. We preserve the ordering
between the two sets of registers (low-level chipset first, fw_cfg
second).

This patch demonstrates that manual handling of S3SaveState protocol
installation can be combined with QemuFwCfgS3Lib, even without upsetting
the original order between boot script fragments. An S3SaveState notify
function running at TPL_CALLBACK can safely queue another S3SaveState
notify function at TPL_CALLBACK with QemuFwCfgS3TransferOwnership().

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/SmmControl2Dxe/SmiFeatures.h    |   5 +-
 OvmfPkg/SmmControl2Dxe/SmiFeatures.c    | 224 +++++++-------------
 OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c |   5 +-
 3 files changed, 77 insertions(+), 157 deletions(-)

diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.h b/OvmfPkg/SmmControl2Dxe/SmiFeatures.h
index 9d5f1dbcb57e..3f3a5d3ea9b1 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.h
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.h
@@ -37,13 +37,10 @@ NegotiateSmiFeatures (
 /**
   Append a boot script fragment that will re-select the previously negotiated
   SMI features during S3 resume.
-
-  @param[in] S3SaveState  The EFI_S3_SAVE_STATE_PROTOCOL instance to append to
-                          the S3 boot script with.
 **/
 VOID
 SaveSmiFeatures (
-  IN EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState
+  VOID
   );
 
 #endif
diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index bd257f15d955..ecdab2fd9a86 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -18,6 +18,7 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/QemuFwCfgLib.h>
+#include <Library/QemuFwCfgS3Lib.h>
 
 #include "SmiFeatures.h"
 
@@ -29,29 +30,26 @@
 
 //
 // Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
-// for the S3 boot script fragment to write to and read from. The buffer
-// captures a combined fw_cfg item selection + write command using the DMA
-// access method. Note that we don't trust the runtime OS to preserve the
-// contents of the buffer, the boot script will first rewrite it.
+// for the S3 boot script fragment to write to and read from.
 //
 #pragma pack (1)
-typedef struct {
-  FW_CFG_DMA_ACCESS Access;
-  UINT64            Features;
+typedef union {
+  UINT64 Features;
+  UINT8  FeaturesOk;
 } SCRATCH_BUFFER;
 #pragma pack ()
 
 //
 // These carry the selector keys of the "etc/smi/requested-features" and
 // "etc/smi/features-ok" fw_cfg files from NegotiateSmiFeatures() to
-// SaveSmiFeatures().
+// AppendFwCfgBootScript().
 //
 STATIC FIRMWARE_CONFIG_ITEM mRequestedFeaturesItem;
 STATIC FIRMWARE_CONFIG_ITEM mFeaturesOkItem;
 
 //
 // Carries the negotiated SMI features from NegotiateSmiFeatures() to
-// SaveSmiFeatures().
+// AppendFwCfgBootScript().
 //
 STATIC UINT64 mSmiFeatures;
 
@@ -168,157 +166,81 @@ FatalError:
 }
 
 /**
+  FW_CFG_BOOT_SCRIPT_APPEND_FUNCTION provided to QemuFwCfgS3Lib.
+**/
+STATIC
+VOID
+EFIAPI
+AppendFwCfgBootScript (
+  IN OUT VOID *Context,              OPTIONAL
+  IN OUT VOID *ExternalScratchBuffer
+  )
+{
+  SCRATCH_BUFFER *ScratchBuffer;
+  RETURN_STATUS  Status;
+
+  ScratchBuffer = ExternalScratchBuffer;
+
+  //
+  // Write the negotiated feature bitmap into "etc/smi/requested-features".
+  //
+  ScratchBuffer->Features = mSmiFeatures;
+  Status = QemuFwCfgS3WriteBytes (mRequestedFeaturesItem,
+             sizeof ScratchBuffer->Features);
+  if (RETURN_ERROR (Status)) {
+    goto FatalError;
+  }
+
+  //
+  // Read back "etc/smi/features-ok". This invokes the feature validation &
+  // lockdown. (The validation succeeded at first boot.)
+  //
+  Status = QemuFwCfgS3ReadBytes (mFeaturesOkItem,
+             sizeof ScratchBuffer->FeaturesOk);
+  if (RETURN_ERROR (Status)) {
+    goto FatalError;
+  }
+
+  //
+  // If "etc/smi/features-ok" read as 1, we're good. Otherwise, hang the S3
+  // resume process.
+  //
+  Status = QemuFwCfgS3CheckValue (&ScratchBuffer->FeaturesOk,
+             sizeof ScratchBuffer->FeaturesOk, MAX_UINT8, 1);
+  if (RETURN_ERROR (Status)) {
+    goto FatalError;
+  }
+
+  DEBUG ((DEBUG_VERBOSE, "%a: SMI feature negotiation boot script saved\n",
+    __FUNCTION__));
+  return;
+
+FatalError:
+  ASSERT (FALSE);
+  CpuDeadLoop ();
+}
+
+
+/**
   Append a boot script fragment that will re-select the previously negotiated
   SMI features during S3 resume.
-
-  @param[in] S3SaveState  The EFI_S3_SAVE_STATE_PROTOCOL instance to append to
-                          the S3 boot script with.
 **/
 VOID
 SaveSmiFeatures (
-  IN EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState
+  VOID
   )
 {
-  SCRATCH_BUFFER *ScratchBuffer;
-  EFI_STATUS     Status;
-  UINT64         AccessAddress;
-  UINT32         ControlPollData;
-  UINT32         ControlPollMask;
-  UINT16         FeaturesOkItemAsUint16;
-  UINT8          FeaturesOkData;
-  UINT8          FeaturesOkMask;
+  RETURN_STATUS Status;
 
-  ScratchBuffer = AllocateReservedPool (sizeof *ScratchBuffer);
-  if (ScratchBuffer == NULL) {
-    DEBUG ((DEBUG_ERROR, "%a: scratch buffer allocation failed\n",
-      __FUNCTION__));
-    goto FatalError;
-  }
-
-  //
-  // Populate the scratch buffer with a select + write fw_cfg DMA command that
-  // will write the negotiated feature bitmap into
-  // "etc/smi/requested-features".
-  //
-  ScratchBuffer->Access.Control = SwapBytes32 (
-                                    (UINT32)mRequestedFeaturesItem << 16 |
-                                    FW_CFG_DMA_CTL_SELECT |
-                                    FW_CFG_DMA_CTL_WRITE
-                                    );
-  ScratchBuffer->Access.Length = SwapBytes32 (
-                                   (UINT32)sizeof ScratchBuffer->Features);
-  ScratchBuffer->Access.Address = SwapBytes64 (
-                                    (UINTN)&ScratchBuffer->Features);
-  ScratchBuffer->Features       = mSmiFeatures;
-
-  //
-  // Copy the scratch buffer into the boot script. When replayed, this
-  // EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE will restore the current contents of the
-  // scratch buffer, in-place.
   //
-  Status = S3SaveState->Write (
-                          S3SaveState,                      // This
-                          EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE, // OpCode
-                          EfiBootScriptWidthUint8,          // Width
-                          (UINT64)(UINTN)ScratchBuffer,     // Address
-                          sizeof *ScratchBuffer,            // Count
-                          (VOID*)ScratchBuffer              // Buffer
-                          );
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE: %r\n",
-      __FUNCTION__, __LINE__, Status));
-    goto FatalError;
-  }
-
-  //
-  // Append an opcode that will write the address of the scratch buffer to the
-  // fw_cfg DMA address register, which consists of two 32-bit IO ports. The
-  // second (highest address, least significant) write will start the transfer.
+  // We are already running at TPL_CALLBACK, on the stack of
+  // OnS3SaveStateInstalled(). But that's okay, we can easily queue more
+  // notification functions while executing a notification function.
   //
-  AccessAddress = SwapBytes64 ((UINTN)&ScratchBuffer->Access);
-  Status = S3SaveState->Write (
-                          S3SaveState,                     // This
-                          EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode
-                          EfiBootScriptWidthUint32,        // Width
-                          (UINT64)FW_CFG_IO_DMA_ADDRESS,   // Address
-                          (UINTN)2,                        // Count
-                          &AccessAddress                   // Buffer
-                          );
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_IO_WRITE_OPCODE: %r\n",
-      __FUNCTION__, __LINE__, Status));
-    goto FatalError;
+  Status = QemuFwCfgS3TransferOwnership (AppendFwCfgBootScript, NULL,
+             sizeof (SCRATCH_BUFFER));
+  if (RETURN_ERROR (Status)) {
+    ASSERT (FALSE);
+    CpuDeadLoop ();
   }
-
-  //
-  // The EFI_BOOT_SCRIPT_MEM_POLL_OPCODE will wait until the Control word reads
-  // as zero (transfer complete). As timeout we use MAX_UINT64 * 100ns, which
-  // is approximately 58494 years.
-  //
-  ControlPollData = 0;
-  ControlPollMask = MAX_UINT32;
-  Status = S3SaveState->Write (
-                     S3SaveState,                                   // This
-                     EFI_BOOT_SCRIPT_MEM_POLL_OPCODE,               // OpCode
-                     EfiBootScriptWidthUint32,                      // Width
-                     (UINT64)(UINTN)&ScratchBuffer->Access.Control, // Address
-                     &ControlPollData,                              // Data
-                     &ControlPollMask,                              // DataMask
-                     MAX_UINT64                                     // Delay
-                     );
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_MEM_POLL_OPCODE: %r\n",
-      __FUNCTION__, __LINE__, Status));
-    goto FatalError;
-  }
-
-  //
-  // Select the "etc/smi/features-ok" fw_cfg file, which invokes the feature
-  // validation & lockdown. (The validation succeeded at first boot.)
-  //
-  FeaturesOkItemAsUint16 = (UINT16)mFeaturesOkItem;
-  Status = S3SaveState->Write (
-                          S3SaveState,                     // This
-                          EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode
-                          EfiBootScriptWidthUint16,        // Width
-                          (UINT64)FW_CFG_IO_SELECTOR,      // Address
-                          (UINTN)1,                        // Count
-                          &FeaturesOkItemAsUint16          // Buffer
-                          );
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_IO_WRITE_OPCODE: %r\n",
-      __FUNCTION__, __LINE__, Status));
-    goto FatalError;
-  }
-
-  //
-  // Read the contents (one byte) of "etc/smi/features-ok". If the value is
-  // one, we're good. Otherwise, continue reading the data port: QEMU returns 0
-  // past the end of the fw_cfg item, so this will hang the resume process,
-  // which matches our intent.
-  //
-  FeaturesOkData = 1;
-  FeaturesOkMask = MAX_UINT8;
-  Status = S3SaveState->Write (
-                     S3SaveState,                    // This
-                     EFI_BOOT_SCRIPT_IO_POLL_OPCODE, // OpCode
-                     EfiBootScriptWidthUint8,        // Width
-                     (UINT64)(UINTN)FW_CFG_IO_DATA,  // Address
-                     &FeaturesOkData,                // Data
-                     &FeaturesOkMask,                // DataMask
-                     MAX_UINT64                      // Delay
-                     );
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_IO_POLL_OPCODE: %r\n",
-      __FUNCTION__, __LINE__, Status));
-    goto FatalError;
-  }
-
-  DEBUG ((DEBUG_VERBOSE, "%a: ScratchBuffer@%p\n", __FUNCTION__,
-    (VOID *)ScratchBuffer));
-  return;
-
-FatalError:
-  ASSERT (FALSE);
-  CpuDeadLoop ();
 }
diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c
index bb79fce0855b..e1cd3d02ac36 100644
--- a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c
+++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c
@@ -378,14 +378,15 @@ OnS3SaveStateInstalled (
     CpuDeadLoop ();
   }
 
+  DEBUG ((DEBUG_VERBOSE, "%a: chipset boot script saved\n", __FUNCTION__));
+
   //
   // Append a boot script fragment that re-selects the negotiated SMI features.
   //
   if (mSmiFeatureNegotiation) {
-    SaveSmiFeatures (S3SaveState);
+    SaveSmiFeatures ();
   }
 
-  DEBUG ((EFI_D_VERBOSE, "%a: boot script fragment saved\n", __FUNCTION__));
   gBS->CloseEvent (Event);
   mS3SaveStateInstalled = NULL;
 }
-- 
2.9.3




  parent reply	other threads:[~2017-02-23  1:48 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-23  1:48 [PATCH 00/12] ArmVirtPkg, OvmfPkg: factor out QemuFwCfgS3Lib Laszlo Ersek
2017-02-23  1:48 ` [PATCH 01/12] OvmfPkg: introduce QemuFwCfgS3Lib class Laszlo Ersek
2017-02-23  1:48 ` [PATCH 02/12] OvmfPkg/QemuFwCfgS3Lib: add initial Base Null library instance Laszlo Ersek
2017-02-23  1:48 ` [PATCH 03/12] OvmfPkg/QemuFwCfgS3Lib: add initial PEI and DXE fw_cfg library instances Laszlo Ersek
2017-02-23  1:48 ` [PATCH 04/12] ArmVirtPkg: resolve QemuFwCfgS3Lib Laszlo Ersek
2017-02-23 11:18   ` Ard Biesheuvel
2017-02-23  1:48 ` [PATCH 05/12] OvmfPkg: " Laszlo Ersek
2017-02-23  1:48 ` [PATCH 06/12] ArmVirtPkg, OvmfPkg: retire QemuFwCfgS3Enabled() from QemuFwCfgLib Laszlo Ersek
2017-02-23 11:18   ` Ard Biesheuvel
2017-02-23  1:48 ` [PATCH 07/12] OvmfPkg/QemuFwCfgS3Lib: add boot script opcode generation APIs to libclass Laszlo Ersek
2017-03-10  9:58   ` Jordan Justen
2017-03-10 19:14     ` Laszlo Ersek
2017-02-23  1:48 ` [PATCH 08/12] OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for Base Null instance Laszlo Ersek
2017-02-23  1:48 ` [PATCH 09/12] OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for PEI fw_cfg instance Laszlo Ersek
2017-02-23  1:48 ` [PATCH 10/12] OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for DXE " Laszlo Ersek
2017-02-23  1:48 ` Laszlo Ersek [this message]
2017-03-10 10:14   ` [PATCH 11/12] OvmfPkg/SmmControl2Dxe: save fw_cfg boot script with QemuFwCfgS3Lib Jordan Justen
2017-03-10 19:36     ` Laszlo Ersek
2017-03-13 21:32       ` Jordan Justen
2017-03-13 22:12         ` Laszlo Ersek
2017-03-13 22:43           ` Jordan Justen
2017-03-14  1:58             ` Kinney, Michael D
2017-03-14 13:48               ` Laszlo Ersek
2017-03-14 20:38                 ` Laszlo Ersek
2017-02-23  1:48 ` [PATCH 12/12] OvmfPkg/AcpiPlatformDxe: " Laszlo Ersek
2017-02-23 16:59 ` [PATCH 00/12] ArmVirtPkg, OvmfPkg: factor out QemuFwCfgS3Lib Jordan Justen
2017-02-23 17:22   ` Laszlo Ersek
2017-03-06  9:19 ` Jordan Justen
2017-03-06 18:41   ` Laszlo Ersek
2017-03-10  9:55     ` Jordan Justen
2017-03-10 20:16       ` Laszlo Ersek
2017-03-11  3:17         ` 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=20170223014814.10937-12-lersek@redhat.com \
    --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