[PATCH 0/6] OvmfPkg: AcpiPlatformDxe, QemuFwCfg: small cleanups after WRITE_POINTER

[PATCH 0/6] OvmfPkg: AcpiPlatformDxe, QemuFwCfg: small cleanups after WRITE_POINTER

[Laszlo Ersek]

Most of these were suggested by Jordan.

Cc: Jordan Justen <jordan.l.justen@intel.com>

Repo:   https://github.com/lersek/edk2.git
Branch: write_pointer_followup

Thanks!
Laszlo

Laszlo Ersek (6):
  OvmfPkg/AcpiPlatformDxe: drop double right shift in ADD/WRITE POINTER
    cmds
  OvmfPkg/AcpiPlatformDxe: update PointerValue comments in
    "BootScript.c"
  OvmfPkg/QemuFwCfgLib: move types/macros from lib class to
    IndustryStandard
  OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_SELECTOR, adapt the package
  OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DATA, adapt the package
  OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DMA_ADDRESS, adapt the package

 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h | 104 ++++++++++++++++++++
 OvmfPkg/Include/Library/QemuFwCfgLib.h       |  70 +------------
 OvmfPkg/AcpiPlatformDxe/BootScript.c         |  18 ++--
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c      |   8 +-
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c  |  12 +--
 OvmfPkg/SmmControl2Dxe/SmiFeatures.c         |   6 +-
 6 files changed, 128 insertions(+), 90 deletions(-)
 create mode 100644 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h

-- 
2.9.3

[PATCH 1/6] OvmfPkg/AcpiPlatformDxe: drop double right shift in ADD/WRITE POINTER cmds

[Laszlo Ersek]

The Count parameter of RShiftU64() must be strictly smaller than 64.
ProcessCmdAddPointer() and ProcessCmdWritePointer() currently ensure this
by "cleverly" breaking the last bit of a potentially 8-byte right shift
out to a separate operation.

Instead, exclude the Count==64 case explicitly (in which case the
preexistent outer RShiftU64() would return 0), and keep only the inner
RShiftU64(), with the direct Count however.

This is not a functional change, just style improvement.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index eadd690bef4e..6a0ecd1ad962 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -277,8 +277,8 @@ ProcessCmdAddPointer (
   ASSERT ((UINTN)Blob2->Base <= MAX_ADDRESS - Blob2->Size);
 
   PointerValue += (UINT64)(UINTN)Blob2->Base;
-  if (RShiftU64 (
-        RShiftU64 (PointerValue, AddPointer->PointerSize * 8 - 1), 1) != 0) {
+  if (AddPointer->PointerSize < 8 &&
+      RShiftU64 (PointerValue, AddPointer->PointerSize * 8) != 0) {
     DEBUG ((EFI_D_ERROR, "%a: relocated pointer value unrepresentable in "
       "\"%a\"\n", __FUNCTION__, AddPointer->PointerFile));
     return EFI_PROTOCOL_ERROR;
@@ -438,8 +438,8 @@ ProcessCmdWritePointer (
   ASSERT ((UINTN)PointeeBlob->Base <= MAX_ADDRESS - PointeeBlob->Size);
 
   PointerValue += (UINT64)(UINTN)PointeeBlob->Base;
-  if (RShiftU64 (
-        RShiftU64 (PointerValue, WritePointer->PointerSize * 8 - 1), 1) != 0) {
+  if (WritePointer->PointerSize < 8 &&
+      RShiftU64 (PointerValue, WritePointer->PointerSize * 8) != 0) {
     DEBUG ((DEBUG_ERROR, "%a: pointer value unrepresentable in \"%a\"\n",
       __FUNCTION__, WritePointer->PointerFile));
     return EFI_PROTOCOL_ERROR;
-- 
2.9.3

[PATCH 2/6] OvmfPkg/AcpiPlatformDxe: update PointerValue comments in "BootScript.c"

[Laszlo Ersek]

Commit df73df138d9d ("OvmfPkg/AcpiPlatformDxe: replay
QEMU_LOADER_WRITE_POINTER commands at S3", 2017-02-09) added
"BootScript.c" with such comments on the PointerValue field of
CONDENSED_WRITE_POINTER, and on the corresponding PointerValue parameter
of SaveCondensedWritePointerToS3Context(), that did not consider the
then-latest update of the QEMU_LOADER_WRITE_POINTER structure. (Namely,
the introduction of the PointeeOffset field.)

The code is fine as-is -- ProcessCmdWritePointer() already calls
SaveCondensedWritePointerToS3Context() correctly, and "BootScript.c"
itself is indifferent to the exact values --, but the comments in
"BootScript.c" should match reality too. Update them.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/AcpiPlatformDxe/BootScript.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/BootScript.c b/OvmfPkg/AcpiPlatformDxe/BootScript.c
index b7a7f270f223..1ad468e2f834 100644
--- a/OvmfPkg/AcpiPlatformDxe/BootScript.c
+++ b/OvmfPkg/AcpiPlatformDxe/BootScript.c
@@ -29,6 +29,7 @@ typedef struct {
   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
+                        //   and QEMU_LOADER_WRITE_POINTER.PointeeOffset
 } CONDENSED_WRITE_POINTER;
 
 
@@ -159,7 +160,8 @@ ReleaseS3Context (
 
   @param[in] PointerValue   The base address of the allocated / downloaded
                             fw_cfg blob that is identified by
-                            QEMU_LOADER_WRITE_POINTER.PointeeFile.
+                            QEMU_LOADER_WRITE_POINTER.PointeeFile, plus
+                            QEMU_LOADER_WRITE_POINTER.PointeeOffset.
 
   @retval EFI_SUCCESS           The information derived from
                                 QEMU_LOADER_WRITE_POINTER has been successfully
@@ -271,9 +273,9 @@ TransferS3ContextToBootScript (
   // (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),
+  //     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.
   //
@@ -346,8 +348,8 @@ TransferS3ContextToBootScript (
     //
     // (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),
+    //     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);
-- 
2.9.3

[PATCH 3/6] OvmfPkg/QemuFwCfgLib: move types/macros from lib class to IndustryStandard

[Laszlo Ersek]

Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h | 96 ++++++++++++++++++++
 OvmfPkg/Include/Library/QemuFwCfgLib.h       | 70 +-------------
 2 files changed, 97 insertions(+), 69 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
new file mode 100644
index 000000000000..c7e9b5c382a5
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
@@ -0,0 +1,96 @@
+/** @file
+  Macro and type definitions corresponding to the QEMU fw_cfg interface.
+
+  Refer to "docs/specs/fw_cfg.txt" in the QEMU source directory.
+
+  Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
+  Copyright (C) 2013 - 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.
+**/
+
+#ifndef __FW_CFG_H__
+#define __FW_CFG_H__
+
+#include <Base.h>
+
+//
+// The size, in bytes, of names of firmware configuration files, including at
+// least one terminating NUL byte.
+//
+#define QEMU_FW_CFG_FNAME_SIZE 56
+
+//
+// If the following bit is set in the UINT32 fw_cfg revision / feature bitmap
+// -- read from key 0x0001 with the basic IO Port or MMIO method --, then the
+// DMA interface is available.
+//
+#define FW_CFG_F_DMA BIT1
+
+//
+// Macros for the FW_CFG_DMA_ACCESS.Control bitmap (in native encoding).
+//
+#define FW_CFG_DMA_CTL_ERROR  BIT0
+#define FW_CFG_DMA_CTL_READ   BIT1
+#define FW_CFG_DMA_CTL_SKIP   BIT2
+#define FW_CFG_DMA_CTL_SELECT BIT3
+#define FW_CFG_DMA_CTL_WRITE  BIT4
+
+//
+// Numerically defined keys.
+//
+typedef enum {
+  QemuFwCfgItemSignature            = 0x0000,
+  QemuFwCfgItemInterfaceVersion     = 0x0001,
+  QemuFwCfgItemSystemUuid           = 0x0002,
+  QemuFwCfgItemRamSize              = 0x0003,
+  QemuFwCfgItemGraphicsEnabled      = 0x0004,
+  QemuFwCfgItemSmpCpuCount          = 0x0005,
+  QemuFwCfgItemMachineId            = 0x0006,
+  QemuFwCfgItemKernelAddress        = 0x0007,
+  QemuFwCfgItemKernelSize           = 0x0008,
+  QemuFwCfgItemKernelCommandLine    = 0x0009,
+  QemuFwCfgItemInitrdAddress        = 0x000a,
+  QemuFwCfgItemInitrdSize           = 0x000b,
+  QemuFwCfgItemBootDevice           = 0x000c,
+  QemuFwCfgItemNumaData             = 0x000d,
+  QemuFwCfgItemBootMenu             = 0x000e,
+  QemuFwCfgItemMaximumCpuCount      = 0x000f,
+  QemuFwCfgItemKernelEntry          = 0x0010,
+  QemuFwCfgItemKernelData           = 0x0011,
+  QemuFwCfgItemInitrdData           = 0x0012,
+  QemuFwCfgItemCommandLineAddress   = 0x0013,
+  QemuFwCfgItemCommandLineSize      = 0x0014,
+  QemuFwCfgItemCommandLineData      = 0x0015,
+  QemuFwCfgItemKernelSetupAddress   = 0x0016,
+  QemuFwCfgItemKernelSetupSize      = 0x0017,
+  QemuFwCfgItemKernelSetupData      = 0x0018,
+  QemuFwCfgItemFileDir              = 0x0019,
+
+  QemuFwCfgItemX86AcpiTables        = 0x8000,
+  QemuFwCfgItemX86SmbiosTables      = 0x8001,
+  QemuFwCfgItemX86Irq0Override      = 0x8002,
+  QemuFwCfgItemX86E820Table         = 0x8003,
+  QemuFwCfgItemX86HpetData          = 0x8004,
+
+} FIRMWARE_CONFIG_ITEM;
+
+//
+// Communication structure for the DMA access method. All fields are encoded in
+// big endian.
+//
+#pragma pack (1)
+typedef struct {
+  UINT32 Control;
+  UINT32 Length;
+  UINT64 Address;
+} FW_CFG_DMA_ACCESS;
+#pragma pack ()
+
+#endif
diff --git a/OvmfPkg/Include/Library/QemuFwCfgLib.h b/OvmfPkg/Include/Library/QemuFwCfgLib.h
index 41c3817470a2..2a1261327b01 100644
--- a/OvmfPkg/Include/Library/QemuFwCfgLib.h
+++ b/OvmfPkg/Include/Library/QemuFwCfgLib.h
@@ -17,75 +17,7 @@
 #ifndef __FW_CFG_LIB__
 #define __FW_CFG_LIB__
 
-//
-// The size, in bytes, of names of firmware configuration files, including at
-// least one terminating NUL byte.
-//
-#define QEMU_FW_CFG_FNAME_SIZE 56
-
-//
-// If the following bit is set in the UINT32 fw_cfg revision / feature bitmap
-// -- read from key 0x0001 with the basic IO Port or MMIO method --, then the
-// DMA interface is available.
-//
-#define FW_CFG_F_DMA BIT1
-
-//
-// Macros for the FW_CFG_DMA_ACCESS.Control bitmap (in native encoding).
-//
-#define FW_CFG_DMA_CTL_ERROR  BIT0
-#define FW_CFG_DMA_CTL_READ   BIT1
-#define FW_CFG_DMA_CTL_SKIP   BIT2
-#define FW_CFG_DMA_CTL_SELECT BIT3
-#define FW_CFG_DMA_CTL_WRITE  BIT4
-
-typedef enum {
-  QemuFwCfgItemSignature            = 0x0000,
-  QemuFwCfgItemInterfaceVersion     = 0x0001,
-  QemuFwCfgItemSystemUuid           = 0x0002,
-  QemuFwCfgItemRamSize              = 0x0003,
-  QemuFwCfgItemGraphicsEnabled      = 0x0004,
-  QemuFwCfgItemSmpCpuCount          = 0x0005,
-  QemuFwCfgItemMachineId            = 0x0006,
-  QemuFwCfgItemKernelAddress        = 0x0007,
-  QemuFwCfgItemKernelSize           = 0x0008,
-  QemuFwCfgItemKernelCommandLine    = 0x0009,
-  QemuFwCfgItemInitrdAddress        = 0x000a,
-  QemuFwCfgItemInitrdSize           = 0x000b,
-  QemuFwCfgItemBootDevice           = 0x000c,
-  QemuFwCfgItemNumaData             = 0x000d,
-  QemuFwCfgItemBootMenu             = 0x000e,
-  QemuFwCfgItemMaximumCpuCount      = 0x000f,
-  QemuFwCfgItemKernelEntry          = 0x0010,
-  QemuFwCfgItemKernelData           = 0x0011,
-  QemuFwCfgItemInitrdData           = 0x0012,
-  QemuFwCfgItemCommandLineAddress   = 0x0013,
-  QemuFwCfgItemCommandLineSize      = 0x0014,
-  QemuFwCfgItemCommandLineData      = 0x0015,
-  QemuFwCfgItemKernelSetupAddress   = 0x0016,
-  QemuFwCfgItemKernelSetupSize      = 0x0017,
-  QemuFwCfgItemKernelSetupData      = 0x0018,
-  QemuFwCfgItemFileDir              = 0x0019,
-
-  QemuFwCfgItemX86AcpiTables        = 0x8000,
-  QemuFwCfgItemX86SmbiosTables      = 0x8001,
-  QemuFwCfgItemX86Irq0Override      = 0x8002,
-  QemuFwCfgItemX86E820Table         = 0x8003,
-  QemuFwCfgItemX86HpetData          = 0x8004,
-
-} FIRMWARE_CONFIG_ITEM;
-
-//
-// Communication structure for the DMA access method. All fields are encoded in
-// big endian.
-//
-#pragma pack (1)
-typedef struct {
-  UINT32 Control;
-  UINT32 Length;
-  UINT64 Address;
-} FW_CFG_DMA_ACCESS;
-#pragma pack ()
+#include <IndustryStandard/QemuFwCfg.h>
 
 /**
   Returns a boolean indicating if the firmware configuration interface
-- 
2.9.3

[PATCH 4/6] OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_SELECTOR, adapt the package

[Laszlo Ersek]

Introduce the FW_CFG_IO_SELECTOR macro for IO Port 0x510 (the Selector
Register), and update all references in OvmfPkg.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h | 6 ++++++
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c  | 2 +-
 OvmfPkg/SmmControl2Dxe/SmiFeatures.c         | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
index c7e9b5c382a5..776bfe88ae2b 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
@@ -43,6 +43,12 @@
 #define FW_CFG_DMA_CTL_WRITE  BIT4
 
 //
+// The fw_cfg registers can be found at these IO Ports, on the IO-mapped
+// platforms (Ia32 and X64).
+//
+#define FW_CFG_IO_SELECTOR    0x510
+
+//
 // Numerically defined keys.
 //
 typedef enum {
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
index 7744873217fe..1387ea85f3f0 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
@@ -42,7 +42,7 @@ QemuFwCfgSelectItem (
   )
 {
   DEBUG ((EFI_D_INFO, "Select Item: 0x%x\n", (UINT16)(UINTN) QemuFwCfgItem));
-  IoWrite16 (0x510, (UINT16)(UINTN) QemuFwCfgItem);
+  IoWrite16 (FW_CFG_IO_SELECTOR, (UINT16)(UINTN) QemuFwCfgItem);
 }
 
 
diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index e070969065c0..352ffa017373 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -281,7 +281,7 @@ SaveSmiFeatures (
                           S3SaveState,                     // This
                           EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode
                           EfiBootScriptWidthUint16,        // Width
-                          (UINT64)0x510,                   // Address
+                          (UINT64)FW_CFG_IO_SELECTOR,      // Address
                           (UINTN)1,                        // Count
                           &FeaturesOkItemAsUint16          // Buffer
                           );
-- 
2.9.3

[PATCH 5/6] OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DATA, adapt the package

[Laszlo Ersek]

Introduce the FW_CFG_IO_DATA macro for IO Port 0x511 (the Data Register),
and update all references in OvmfPkg.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h | 1 +
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c  | 6 +++---
 OvmfPkg/SmmControl2Dxe/SmiFeatures.c         | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
index 776bfe88ae2b..5da6b456febe 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
@@ -47,6 +47,7 @@
 // platforms (Ia32 and X64).
 //
 #define FW_CFG_IO_SELECTOR    0x510
+#define FW_CFG_IO_DATA        0x511
 
 //
 // Numerically defined keys.
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
index 1387ea85f3f0..d79d0a444ca7 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
@@ -135,7 +135,7 @@ InternalQemuFwCfgReadBytes (
     InternalQemuFwCfgDmaBytes ((UINT32)Size, Buffer, FW_CFG_DMA_CTL_READ);
     return;
   }
-  IoReadFifo8 (0x511, Size, Buffer);
+  IoReadFifo8 (FW_CFG_IO_DATA, Size, Buffer);
 }
 
 
@@ -187,7 +187,7 @@ QemuFwCfgWriteBytes (
       InternalQemuFwCfgDmaBytes ((UINT32)Size, Buffer, FW_CFG_DMA_CTL_WRITE);
       return;
     }
-    IoWriteFifo8 (0x511, Size, Buffer);
+    IoWriteFifo8 (FW_CFG_IO_DATA, Size, Buffer);
   }
 }
 
@@ -230,7 +230,7 @@ QemuFwCfgSkipBytes (
   //
   while (Size > 0) {
     ChunkSize = MIN (Size, sizeof SkipBuffer);
-    IoReadFifo8 (0x511, ChunkSize, SkipBuffer);
+    IoReadFifo8 (FW_CFG_IO_DATA, ChunkSize, SkipBuffer);
     Size -= ChunkSize;
   }
 }
diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index 352ffa017373..73c29848b334 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -303,7 +303,7 @@ SaveSmiFeatures (
                      S3SaveState,                    // This
                      EFI_BOOT_SCRIPT_IO_POLL_OPCODE, // OpCode
                      EfiBootScriptWidthUint8,        // Width
-                     (UINT64)(UINTN)0x511,           // Address
+                     (UINT64)(UINTN)FW_CFG_IO_DATA,  // Address
                      &FeaturesOkData,                // Data
                      &FeaturesOkMask,                // DataMask
                      MAX_UINT64                      // Delay
-- 
2.9.3

[PATCH 6/6] OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DMA_ADDRESS, adapt the package

[Laszlo Ersek]

Introduce the FW_CFG_IO_DMA_ADDRESS macro for IO Ports 0x514 and 0x518
(most significant and least significant halves of the DMA Address
Register, respectively), and update all references in OvmfPkg.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h | 1 +
 OvmfPkg/AcpiPlatformDxe/BootScript.c         | 4 ++--
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c  | 4 ++--
 OvmfPkg/SmmControl2Dxe/SmiFeatures.c         | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
index 5da6b456febe..8c32f83e8e16 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
@@ -48,6 +48,7 @@
 //
 #define FW_CFG_IO_SELECTOR    0x510
 #define FW_CFG_IO_DATA        0x511
+#define FW_CFG_IO_DMA_ADDRESS 0x514
 
 //
 // Numerically defined keys.
diff --git a/OvmfPkg/AcpiPlatformDxe/BootScript.c b/OvmfPkg/AcpiPlatformDxe/BootScript.c
index 1ad468e2f834..bff42ad8b9b0 100644
--- a/OvmfPkg/AcpiPlatformDxe/BootScript.c
+++ b/OvmfPkg/AcpiPlatformDxe/BootScript.c
@@ -317,7 +317,7 @@ TransferS3ContextToBootScript (
                             S3SaveState,                     // This
                             EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode
                             EfiBootScriptWidthUint32,        // Width
-                            (UINT64)0x514,                   // Address
+                            (UINT64)FW_CFG_IO_DMA_ADDRESS,   // Address
                             (UINTN)2,                        // Count
                             &BigEndianAddressOfAccess        // Buffer
                             );
@@ -376,7 +376,7 @@ TransferS3ContextToBootScript (
                             S3SaveState,                     // This
                             EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode
                             EfiBootScriptWidthUint32,        // Width
-                            (UINT64)0x514,                   // Address
+                            (UINT64)FW_CFG_IO_DMA_ADDRESS,   // Address
                             (UINTN)2,                        // Count
                             &BigEndianAddressOfAccess        // Buffer
                             );
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
index d79d0a444ca7..3dd55ba5042e 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
@@ -94,8 +94,8 @@ InternalQemuFwCfgDmaBytes (
   //
   AccessHigh = (UINT32)RShiftU64 ((UINTN)&Access, 32);
   AccessLow  = (UINT32)(UINTN)&Access;
-  IoWrite32 (0x514, SwapBytes32 (AccessHigh));
-  IoWrite32 (0x518, SwapBytes32 (AccessLow));
+  IoWrite32 (FW_CFG_IO_DMA_ADDRESS,     SwapBytes32 (AccessHigh));
+  IoWrite32 (FW_CFG_IO_DMA_ADDRESS + 4, SwapBytes32 (AccessLow));
 
   //
   // Don't look at Access.Control before starting the transfer.
diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index 73c29848b334..bd257f15d955 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -240,7 +240,7 @@ SaveSmiFeatures (
                           S3SaveState,                     // This
                           EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode
                           EfiBootScriptWidthUint32,        // Width
-                          (UINT64)0x514,                   // Address
+                          (UINT64)FW_CFG_IO_DMA_ADDRESS,   // Address
                           (UINTN)2,                        // Count
                           &AccessAddress                   // Buffer
                           );
-- 
2.9.3

Re: [PATCH 0/6] OvmfPkg: AcpiPlatformDxe, QemuFwCfg: small cleanups after WRITE_POINTER

[Jordan Justen]

Good cleanups. Thanks for taking the extra time sort these out!

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Unfortunately, https://bugzilla.tianocore.org/show_bug.cgi?id=394 may
not be quite so 'fun'. :)

-Jordan

On 2017-02-21 07:38:06, Laszlo Ersek wrote:
> Most of these were suggested by Jordan.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: write_pointer_followup
> 
> Thanks!
> Laszlo
> 
> Laszlo Ersek (6):
>   OvmfPkg/AcpiPlatformDxe: drop double right shift in ADD/WRITE POINTER
>     cmds
>   OvmfPkg/AcpiPlatformDxe: update PointerValue comments in
>     "BootScript.c"
>   OvmfPkg/QemuFwCfgLib: move types/macros from lib class to
>     IndustryStandard
>   OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_SELECTOR, adapt the package
>   OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DATA, adapt the package
>   OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DMA_ADDRESS, adapt the package
> 
>  OvmfPkg/Include/IndustryStandard/QemuFwCfg.h | 104 ++++++++++++++++++++
>  OvmfPkg/Include/Library/QemuFwCfgLib.h       |  70 +------------
>  OvmfPkg/AcpiPlatformDxe/BootScript.c         |  18 ++--
>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c      |   8 +-
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c  |  12 +--
>  OvmfPkg/SmmControl2Dxe/SmiFeatures.c         |   6 +-
>  6 files changed, 128 insertions(+), 90 deletions(-)
>  create mode 100644 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
> 
> -- 
> 2.9.3
> 

Re: [PATCH 0/6] OvmfPkg: AcpiPlatformDxe, QemuFwCfg: small cleanups after WRITE_POINTER

[Laszlo Ersek]

On 02/22/17 02:58, Jordan Justen wrote:
> Good cleanups. Thanks for taking the extra time sort these out!

You made good points, so thanks for those :) Also, stale comments are
sometimes worse than no comments, so I wanted to clean up those too.

> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Ha! I for one welcome another one of our ultra fast reviewer overlords! :)

Series committed as b9d56d0b93ae..ed1a2d42d5d5.

> Unfortunately, https://bugzilla.tianocore.org/show_bug.cgi?id=394 may
> not be quite so 'fun'. :)

Stay tuned, I'm already working on that :)

Thanks!
Laszlo

> -Jordan
> 
> On 2017-02-21 07:38:06, Laszlo Ersek wrote:
>> Most of these were suggested by Jordan.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: write_pointer_followup
>>
>> Thanks!
>> Laszlo
>>
>> Laszlo Ersek (6):
>>   OvmfPkg/AcpiPlatformDxe: drop double right shift in ADD/WRITE POINTER
>>     cmds
>>   OvmfPkg/AcpiPlatformDxe: update PointerValue comments in
>>     "BootScript.c"
>>   OvmfPkg/QemuFwCfgLib: move types/macros from lib class to
>>     IndustryStandard
>>   OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_SELECTOR, adapt the package
>>   OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DATA, adapt the package
>>   OvmfPkg/QemuFwCfg: introduce FW_CFG_IO_DMA_ADDRESS, adapt the package
>>
>>  OvmfPkg/Include/IndustryStandard/QemuFwCfg.h | 104 ++++++++++++++++++++
>>  OvmfPkg/Include/Library/QemuFwCfgLib.h       |  70 +------------
>>  OvmfPkg/AcpiPlatformDxe/BootScript.c         |  18 ++--
>>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c      |   8 +-
>>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c  |  12 +--
>>  OvmfPkg/SmmControl2Dxe/SmiFeatures.c         |   6 +-
>>  6 files changed, 128 insertions(+), 90 deletions(-)
>>  create mode 100644 OvmfPkg/Include/IndustryStandard/QemuFwCfg.h
>>
>> -- 
>> 2.9.3
>>