[PATCH v3 1/1] OvmfPkg/QemuFwCfgLib: Use BusMasterCommonBuffer to map FW_CFG_DMA_ACCESS

[Brijesh Singh <brijesh.singh@amd.com>]

Commit 09719a01b11b (OvmfPkg/QemuFwCfgLib: Implement SEV internal function
for Dxe phase) uses IOMMU protocol to allocate and free FW_CFG_DMA_ACCESS
buffer when SEV is active. During initial commits we made assumption that
IOMMU.AllocateBuffer() will provide PlainTextAddress (i.e C-bit cleared).
This assumption was wrong, the AllocateBuffer() protocol member is not
expected to produce a buffer that is immediatly usable, and client is
required to call Map() uncondtionally with BusMasterCommonBuffer[64] to
get a mapping which is accessable by both host and device.

The patch refactors code a bit and add the support to Map()
FW_CFG_DMA_ACCESS buffer using BusMasterCommonBuffer operation after
allocation and Unamp() before free.

The complete discussion about this and recommendation from Laszlo can be
found here [1]

[1] https://lists.01.org/pipermail/edk2-devel/2017-July/012652.html

Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h |  42 +--
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c         | 330 ++++++++++++++++----
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c         | 130 --------
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c         |  99 +++---
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c         |  58 +---
 5 files changed, 367 insertions(+), 292 deletions(-)

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
index 8cfa7913ffae..327f1d37e17d 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
@@ -45,39 +45,25 @@ InternalQemuFwCfgDmaIsAvailable (
   );
 
 /**
- Returns a boolean indicating whether SEV support is enabled
+  Transfer an array of bytes, or skip a number of bytes, using the DMA
+  interface.
 
- @retval    TRUE    SEV is enabled
- @retval    FALSE   SEV is disabled
-**/
-BOOLEAN
-InternalQemuFwCfgSevIsEnabled (
-  VOID
-  );
+  @param[in]     Size     Size in bytes to transfer or skip.
 
-/**
- Allocate a bounce buffer for SEV DMA.
-
-  @param[out]    Buffer   Allocated DMA Buffer pointer
-  @param[in]     NumPage  Number of pages.
+  @param[in,out] Buffer   Buffer to read data into or write data from. Ignored,
+                          and may be NULL, if Size is zero, or Control is
+                          FW_CFG_DMA_CTL_SKIP.
 
+  @param[in]     Control  One of the following:
+                          FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
+                          FW_CFG_DMA_CTL_READ  - read from fw_cfg into Buffer.
+                          FW_CFG_DMA_CTL_SKIP  - skip bytes in fw_cfg.
 **/
 VOID
-InternalQemuFwCfgSevDmaAllocateBuffer (
-  OUT    VOID     **Buffer,
-  IN     UINT32   NumPages
+InternalQemuFwCfgDmaBytes (
+  IN     UINT32   Size,
+  IN OUT VOID     *Buffer OPTIONAL,
+  IN     UINT32   Control
   );
 
-/**
- Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
-
-  @param[in]     NumPage  Number of pages.
-  @param[in]     Buffer   DMA Buffer pointer
-
-**/
-VOID
-InternalQemuFwCfgSevDmaFreeBuffer (
-  IN     VOID     *Buffer,
-  IN     UINT32   NumPages
-  );
 #endif
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
index f8eb03bc3a9a..576941749cf2 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
@@ -20,6 +20,7 @@
 #include <Protocol/IoMmu.h>
 
 #include <Library/BaseLib.h>
+#include <Library/IoLib.h>
 #include <Library/DebugLib.h>
 #include <Library/QemuFwCfgLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -31,20 +32,6 @@ STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
 STATIC BOOLEAN mQemuFwCfgDmaSupported;
 
 STATIC EDKII_IOMMU_PROTOCOL        *mIoMmuProtocol;
-/**
-
- Returns a boolean indicating whether SEV is enabled
-
- @retval    TRUE    SEV is enabled
- @retval    FALSE   SEV is disabled
-**/
-BOOLEAN
-InternalQemuFwCfgSevIsEnabled (
-  VOID
-  )
-{
-  return MemEncryptSevIsEnabled ();
-}
 
 /**
   Returns a boolean indicating if the firmware configuration interface
@@ -110,7 +97,8 @@ QemuFwCfgInitialize (
     // IoMmuDxe driver must have installed the IOMMU protocol. If we are not
     // able to locate the protocol then something must have gone wrong.
     //
-    Status = gBS->LocateProtocol (&gEdkiiIoMmuProtocolGuid, NULL, (VOID **)&mIoMmuProtocol);
+    Status = gBS->LocateProtocol (&gEdkiiIoMmuProtocolGuid, NULL,
+                    (VOID **)&mIoMmuProtocol);
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR,
         "QemuFwCfgSevDma %a:%a Failed to locate IOMMU protocol.\n",
@@ -157,74 +145,308 @@ InternalQemuFwCfgDmaIsAvailable (
 }
 
 /**
- Allocate a bounce buffer for SEV DMA.
-
-  @param[in]     NumPage  Number of pages.
-  @param[out]    Buffer   Allocated DMA Buffer pointer
+  Function is used for allocating a bi-directional FW_CFG_DMA_ACCESS used
+  between Host and device to exchange the information. The buffer must be free'd
+  using FreeFwCfgDmaAccessBuffer ().
 
 **/
+STATIC
 VOID
-InternalQemuFwCfgSevDmaAllocateBuffer (
-  OUT    VOID     **Buffer,
-  IN     UINT32   NumPages
+AllocFwCfgDmaAccessBuffer (
+  OUT   VOID     **Access,
+  OUT   VOID     **MapInfo
   )
 {
-  EFI_STATUS    Status;
+  UINTN                 Size;
+  UINTN                 NumPages;
+  EFI_STATUS            Status;
+  VOID                  *HostAddress;
+  EFI_PHYSICAL_ADDRESS  DmaAddress;
+  VOID                  *Mapping;
 
-  ASSERT (mIoMmuProtocol != NULL);
+  Size = sizeof (FW_CFG_DMA_ACCESS);
+  NumPages = EFI_SIZE_TO_PAGES (Size);
 
+  //
+  // As per UEFI spec, in order to map a host address with
+  // BusMasterCommomBuffer64, the buffer must be allocated using the IOMMU
+  // AllocateBuffer()
+  //
   Status = mIoMmuProtocol->AllocateBuffer (
-                            mIoMmuProtocol,
-                            0,
-                            EfiBootServicesData,
-                            NumPages,
-                            Buffer,
-                            EDKII_IOMMU_ATTRIBUTE_MEMORY_CACHED
-                          );
+                             mIoMmuProtocol,
+                             AllocateAnyPages,
+                             EfiBootServicesData,
+                             NumPages,
+                             &HostAddress,
+                             EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE
+                             );
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR,
-      "%a:%a failed to allocate %u pages\n", gEfiCallerBaseName, __FUNCTION__,
-      NumPages));
+      "%a:%a failed to allocate FW_CFG_DMA_ACCESS\n", gEfiCallerBaseName,
+      __FUNCTION__));
     ASSERT (FALSE);
     CpuDeadLoop ();
   }
 
-  DEBUG ((DEBUG_VERBOSE,
-    "%a:%a buffer 0x%Lx Pages %u\n", gEfiCallerBaseName, __FUNCTION__,
-    (UINT64)(UINTN)Buffer, NumPages));
+  //
+  // Map the host buffer with BusMasterCommonBuffer64
+  //
+  Status = mIoMmuProtocol->Map (
+                             mIoMmuProtocol,
+                             EdkiiIoMmuOperationBusMasterCommonBuffer64,
+                             HostAddress,
+                             &Size,
+                             &DmaAddress,
+                             &Mapping
+                             );
+  if (EFI_ERROR (Status)) {
+    mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, HostAddress);
+    DEBUG ((DEBUG_ERROR,
+      "%a:%a failed to Map() FW_CFG_DMA_ACCESS\n", gEfiCallerBaseName,
+      __FUNCTION__));
+    ASSERT (FALSE);
+    CpuDeadLoop ();
+  }
+
+  if (Size < sizeof (FW_CFG_DMA_ACCESS)) {
+    mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping);
+    mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, HostAddress);
+    DEBUG ((DEBUG_ERROR,
+      "%a:%a failed to Map() - requested 0x%Lx got 0x%Lx\n", gEfiCallerBaseName,
+      __FUNCTION__, (UINT64)sizeof (FW_CFG_DMA_ACCESS), (UINT64)Size));
+    ASSERT (FALSE);
+    CpuDeadLoop ();
+  }
+
+  *Access = HostAddress;
+  *MapInfo = Mapping;
 }
 
 /**
- Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+  Function is to used for freeing the Access buffer allocated using
+  AllocFwCfgDmaAccessBuffer()
+
+**/
+STATIC
+VOID
+FreeFwCfgDmaAccessBuffer (
+  IN  VOID    *Access,
+  IN  VOID    *Mapping
+  )
+{
+  UINTN       NumPages;
+  EFI_STATUS  Status;
+
+  NumPages = EFI_SIZE_TO_PAGES (sizeof (FW_CFG_DMA_ACCESS));
+
+  Status = mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a:%a failed to UnMap() Mapping 0x%Lx\n", gEfiCallerBaseName,
+      __FUNCTION__, (UINT64)Mapping));
+    ASSERT (FALSE);
+    CpuDeadLoop ();
+  }
 
-  @param[in]     NumPage  Number of pages.
-  @param[in]     Buffer   DMA Buffer pointer
+  Status = mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, Access);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a:%a failed to Free() 0x%Lx\n", gEfiCallerBaseName, __FUNCTION__,
+      (UINT64) Access));
+    ASSERT (FALSE);
+    CpuDeadLoop ();
+  }
+}
+
+/**
+  Function is used for mapping host address to device address. The buffer must
+  be unmapped with UnmapDmaDataBuffer ().
 
 **/
+STATIC
 VOID
-InternalQemuFwCfgSevDmaFreeBuffer (
-  IN     VOID     *Buffer,
-  IN     UINT32   NumPages
+MapFwCfgDmaDataBuffer (
+  IN  BOOLEAN               IsWrite,
+  IN  VOID                  *HostAddress,
+  IN  UINT32                Size,
+  OUT EFI_PHYSICAL_ADDRESS  *DeviceAddress,
+  OUT VOID                  **MapInfo
   )
 {
-  EFI_STATUS    Status;
+  EFI_STATUS              Status;
+  UINTN                   NumberOfBytes;
+  VOID                    *Mapping;
+  EFI_PHYSICAL_ADDRESS    PhysicalAddress;
+
+  NumberOfBytes = Size;
+  Status = mIoMmuProtocol->Map (
+                             mIoMmuProtocol,
+                             (IsWrite ?
+                              EdkiiIoMmuOperationBusMasterRead64 :
+                              EdkiiIoMmuOperationBusMasterWrite64),
+                             HostAddress,
+                             &NumberOfBytes,
+                             &PhysicalAddress,
+                             &Mapping
+                             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a:%a failed to Map() Address 0x%Lx Size 0x%Lx\n", gEfiCallerBaseName,
+      __FUNCTION__, (UINT64) HostAddress, (UINT64)Size));
+    ASSERT (FALSE);
+    CpuDeadLoop ();
+  }
+
+  if (NumberOfBytes < Size) {
+    mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping);
+    DEBUG ((DEBUG_ERROR,
+      "%a:%a failed to Map() - requested 0x%x got 0x%Lx\n", gEfiCallerBaseName,
+      __FUNCTION__, Size, (UINT64)NumberOfBytes));
+    ASSERT (FALSE);
+    CpuDeadLoop ();
+  }
+
+  *DeviceAddress = PhysicalAddress;
+  *MapInfo = Mapping;
+}
 
-  ASSERT (mIoMmuProtocol != NULL);
+STATIC
+VOID
+UnmapFwCfgDmaDataBuffer (
+  IN  VOID  *Mapping
+  )
+{
+  EFI_STATUS  Status;
 
-  Status = mIoMmuProtocol->FreeBuffer (
-                            mIoMmuProtocol,
-                            NumPages,
-                            Buffer
-                          );
+  Status = mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR,
-      "%a:%a failed to free buffer 0x%Lx pages %u\n", gEfiCallerBaseName,
-      __FUNCTION__, (UINT64)(UINTN)Buffer, NumPages));
+      "%a:%a failed to UnMap() Mapping 0x%Lx\n", gEfiCallerBaseName,
+      __FUNCTION__, (UINT64)Mapping));
     ASSERT (FALSE);
     CpuDeadLoop ();
   }
+}
+
+/**
+  Transfer an array of bytes, or skip a number of bytes, using the DMA
+  interface.
+
+  @param[in]     Size     Size in bytes to transfer or skip.
+
+  @param[in,out] Buffer   Buffer to read data into or write data from. Ignored,
+                          and may be NULL, if Size is zero, or Control is
+                          FW_CFG_DMA_CTL_SKIP.
+
+  @param[in]     Control  One of the following:
+                          FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
+                          FW_CFG_DMA_CTL_READ  - read from fw_cfg into Buffer.
+                          FW_CFG_DMA_CTL_SKIP  - skip bytes in fw_cfg.
+**/
+VOID
+InternalQemuFwCfgDmaBytes (
+  IN     UINT32   Size,
+  IN OUT VOID     *Buffer OPTIONAL,
+  IN     UINT32   Control
+  )
+{
+  volatile FW_CFG_DMA_ACCESS LocalAccess;
+  volatile FW_CFG_DMA_ACCESS *Access;
+  UINT32                     AccessHigh, AccessLow;
+  UINT32                     Status;
+  VOID                       *AccessMapping, *DataMapping;
+  VOID                       *DataBuffer;
+
+  ASSERT (Control == FW_CFG_DMA_CTL_WRITE || Control == FW_CFG_DMA_CTL_READ ||
+    Control == FW_CFG_DMA_CTL_SKIP);
+
+  if (Size == 0) {
+    return;
+  }
+
+  Access = &LocalAccess;
+  AccessMapping = NULL;
+  DataMapping = NULL;
+  DataBuffer = Buffer;
+
+  //
+  // When SEV is enabled, map Buffer to DMA address before issuing the DMA
+  // request
+  //
+  if (MemEncryptSevIsEnabled ()) {
+    VOID                  *AccessBuffer;
+    EFI_PHYSICAL_ADDRESS  DataBufferAddress;
+
+    //
+    // Allocate DMA Access buffer
+    //
+    AllocFwCfgDmaAccessBuffer (&AccessBuffer, &AccessMapping);
+
+    Access = AccessBuffer;
+
+    //
+    // Map actual data buffer
+    //
+    if (Control != FW_CFG_DMA_CTL_SKIP) {
+      MapFwCfgDmaDataBuffer (
+                             Control == FW_CFG_DMA_CTL_WRITE,
+                             Buffer,
+                             Size,
+                             &DataBufferAddress,
+                             &DataMapping
+                             );
+
+      DataBuffer = (VOID *) (UINTN) DataBufferAddress;
+    }
+  }
 
-  DEBUG ((DEBUG_VERBOSE,
-    "%a:%a buffer 0x%Lx Pages %u\n", gEfiCallerBaseName,__FUNCTION__,
-    (UINT64)(UINTN)Buffer, NumPages));
+  Access->Control = SwapBytes32 (Control);
+  Access->Length  = SwapBytes32 (Size);
+  Access->Address = SwapBytes64 ((UINTN)DataBuffer);
+
+  //
+  // Delimit the transfer from (a) modifications to Access, (b) in case of a
+  // write, from writes to Buffer by the caller.
+  //
+  MemoryFence ();
+
+  //
+  // Start the transfer.
+  //
+  AccessHigh = (UINT32)RShiftU64 ((UINTN)Access, 32);
+  AccessLow  = (UINT32)(UINTN)Access;
+  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.
+  //
+  MemoryFence ();
+
+  //
+  // Wait for the transfer to complete.
+  //
+  do {
+    Status = SwapBytes32 (Access->Control);
+    ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
+  } while (Status != 0);
+
+  //
+  // After a read, the caller will want to use Buffer.
+  //
+  MemoryFence ();
+
+  //
+  // If Access buffer was dynamically allocated then free it.
+  //
+  if (AccessMapping) {
+    FreeFwCfgDmaAccessBuffer ((VOID *)Access, AccessMapping);
+  }
+
+  //
+  // If DataBuffer was mapped then unmap it.
+  //
+  if (DataMapping) {
+    UnmapFwCfgDmaDataBuffer (DataMapping);
+  }
 }
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
index d3bf75498d60..7f42f38d1c05 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
@@ -45,136 +45,6 @@ QemuFwCfgSelectItem (
   IoWrite16 (FW_CFG_IO_SELECTOR, (UINT16)(UINTN) QemuFwCfgItem);
 }
 
-
-/**
-  Transfer an array of bytes, or skip a number of bytes, using the DMA
-  interface.
-
-  @param[in]     Size     Size in bytes to transfer or skip.
-
-  @param[in,out] Buffer   Buffer to read data into or write data from. Ignored,
-                          and may be NULL, if Size is zero, or Control is
-                          FW_CFG_DMA_CTL_SKIP.
-
-  @param[in]     Control  One of the following:
-                          FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
-                          FW_CFG_DMA_CTL_READ  - read from fw_cfg into Buffer.
-                          FW_CFG_DMA_CTL_SKIP  - skip bytes in fw_cfg.
-**/
-VOID
-InternalQemuFwCfgDmaBytes (
-  IN     UINT32   Size,
-  IN OUT VOID     *Buffer OPTIONAL,
-  IN     UINT32   Control
-  )
-{
-  volatile FW_CFG_DMA_ACCESS LocalAccess;
-  volatile FW_CFG_DMA_ACCESS *Access;
-  UINT32                     AccessHigh, AccessLow;
-  UINT32                     Status;
-  UINT32                     NumPages;
-  VOID                       *DmaBuffer, *BounceBuffer;
-
-  ASSERT (Control == FW_CFG_DMA_CTL_WRITE || Control == FW_CFG_DMA_CTL_READ ||
-    Control == FW_CFG_DMA_CTL_SKIP);
-
-  if (Size == 0) {
-    return;
-  }
-
-  //
-  // set NumPages to suppress incorrect compiler/analyzer warnings
-  //
-  NumPages = 0;
-
-  //
-  // When SEV is enabled then allocate DMA bounce buffer
-  //
-  if (InternalQemuFwCfgSevIsEnabled ()) {
-    UINTN  TotalSize;
-
-    TotalSize = sizeof (*Access);
-    //
-    // Skip operation does not need buffer
-    //
-    if (Control != FW_CFG_DMA_CTL_SKIP) {
-      TotalSize += Size;
-    }
-
-    //
-    // Allocate SEV DMA buffer
-    //
-    NumPages = (UINT32)EFI_SIZE_TO_PAGES (TotalSize);
-    InternalQemuFwCfgSevDmaAllocateBuffer (&BounceBuffer, NumPages);
-
-    Access = BounceBuffer;
-    DmaBuffer = (UINT8*)BounceBuffer + sizeof (*Access);
-
-    //
-    //  Decrypt data from encrypted guest buffer into DMA buffer
-    //
-    if (Control == FW_CFG_DMA_CTL_WRITE) {
-      CopyMem (DmaBuffer, Buffer, Size);
-    }
-  } else {
-    Access = &LocalAccess;
-    DmaBuffer = Buffer;
-    BounceBuffer = NULL;
-  }
-
-  Access->Control = SwapBytes32 (Control);
-  Access->Length  = SwapBytes32 (Size);
-  Access->Address = SwapBytes64 ((UINTN)DmaBuffer);
-
-  //
-  // Delimit the transfer from (a) modifications to Access, (b) in case of a
-  // write, from writes to Buffer by the caller.
-  //
-  MemoryFence ();
-
-  //
-  // Start the transfer.
-  //
-  AccessHigh = (UINT32)RShiftU64 ((UINTN)Access, 32);
-  AccessLow  = (UINT32)(UINTN)Access;
-  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.
-  //
-  MemoryFence ();
-
-  //
-  // Wait for the transfer to complete.
-  //
-  do {
-    Status = SwapBytes32 (Access->Control);
-    ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
-  } while (Status != 0);
-
-  //
-  // After a read, the caller will want to use Buffer.
-  //
-  MemoryFence ();
-
-  //
-  // If Bounce buffer was allocated then copy the data into guest buffer and
-  // free the bounce buffer
-  //
-  if (BounceBuffer != NULL) {
-    //
-    //  Encrypt the data from DMA buffer into guest buffer
-    //
-    if (Control == FW_CFG_DMA_CTL_READ) {
-      CopyMem (Buffer, DmaBuffer, Size);
-    }
-
-    InternalQemuFwCfgSevDmaFreeBuffer (BounceBuffer, NumPages);
-  }
-}
-
-
 /**
   Reads firmware configuration bytes into a buffer
 
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
index 40f89c3b53e2..471ab0335e64 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
@@ -16,6 +16,7 @@
 **/
 
 #include <Library/BaseLib.h>
+#include <Library/IoLib.h>
 #include <Library/DebugLib.h>
 #include <Library/QemuFwCfgLib.h>
 #include <Library/MemEncryptSevLib.h>
@@ -85,7 +86,7 @@ QemuFwCfgInitialize (
     // (which need to allocate dynamic memory and allocating a PAGE size'd
     // buffer can be challenge in PEI phase)
     //
-    if (InternalQemuFwCfgSevIsEnabled ()) {
+    if (MemEncryptSevIsEnabled ()) {
       DEBUG ((DEBUG_INFO, "SEV: QemuFwCfg fallback to IO Port interface.\n"));
     } else {
       mQemuFwCfgDmaSupported = TRUE;
@@ -129,56 +130,78 @@ InternalQemuFwCfgDmaIsAvailable (
 }
 
 /**
+  Transfer an array of bytes, or skip a number of bytes, using the DMA
+  interface.
 
- Returns a boolean indicating whether SEV is enabled
+  @param[in]     Size     Size in bytes to transfer or skip.
 
- @retval    TRUE    SEV is enabled
- @retval    FALSE   SEV is disabled
+  @param[in,out] Buffer   Buffer to read data into or write data from. Ignored,
+                          and may be NULL, if Size is zero, or Control is
+                          FW_CFG_DMA_CTL_SKIP.
+
+  @param[in]     Control  One of the following:
+                          FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
+                          FW_CFG_DMA_CTL_READ  - read from fw_cfg into Buffer.
+                          FW_CFG_DMA_CTL_SKIP  - skip bytes in fw_cfg.
 **/
-BOOLEAN
-InternalQemuFwCfgSevIsEnabled (
-  VOID
+VOID
+InternalQemuFwCfgDmaBytes (
+  IN     UINT32   Size,
+  IN OUT VOID     *Buffer OPTIONAL,
+  IN     UINT32   Control
   )
 {
-  return MemEncryptSevIsEnabled ();
-}
+  volatile FW_CFG_DMA_ACCESS Access;
+  UINT32                     AccessHigh, AccessLow;
+  UINT32                     Status;
 
-/**
- Allocate a bounce buffer for SEV DMA.
+  ASSERT (Control == FW_CFG_DMA_CTL_WRITE || Control == FW_CFG_DMA_CTL_READ ||
+    Control == FW_CFG_DMA_CTL_SKIP);
 
-  @param[in]     NumPage  Number of pages.
-  @param[out]    Buffer   Allocated DMA Buffer pointer
+  if (Size == 0) {
+    return;
+  }
 
-**/
-VOID
-InternalQemuFwCfgSevDmaAllocateBuffer (
-  OUT    VOID     **Buffer,
-  IN     UINT32   NumPages
-  )
-{
   //
-  // We should never reach here
+  // SEV does not support DMA operations in PEI stage, we should
+  // not have reached here.
   //
-  ASSERT (FALSE);
-  CpuDeadLoop ();
-}
+  ASSERT (!MemEncryptSevIsEnabled ());
 
-/**
- Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+  Access.Control = SwapBytes32 (Control);
+  Access.Length  = SwapBytes32 (Size);
+  Access.Address = SwapBytes64 ((UINTN)Buffer);
 
-  @param[in]     NumPage  Number of pages.
-  @param[in]     Buffer   DMA Buffer pointer
+  //
+  // Delimit the transfer from (a) modifications to Access, (b) in case of a
+  // write, from writes to Buffer by the caller.
+  //
+  MemoryFence ();
 
-**/
-VOID
-InternalQemuFwCfgSevDmaFreeBuffer (
-  IN     VOID     *Buffer,
-  IN     UINT32   NumPages
-  )
-{
   //
-  // We should never reach here
+  // Start the transfer.
+  //
+  AccessHigh = (UINT32)RShiftU64 ((UINTN)&Access, 32);
+  AccessLow  = (UINT32)(UINTN)&Access;
+  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.
+  //
+  MemoryFence ();
+
+  //
+  // Wait for the transfer to complete.
+  //
+  do {
+    Status = SwapBytes32 (Access.Control);
+    ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
+  } while (Status != 0);
+
+  //
+  // After a read, the caller will want to use Buffer.
   //
-  ASSERT (FALSE);
-  CpuDeadLoop ();
+  MemoryFence ();
 }
+
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
index 071b8d9b91d4..1eadba99e1b3 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
@@ -17,6 +17,7 @@
   WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 **/
 
+#include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 #include <Library/QemuFwCfgLib.h>
 
@@ -97,57 +98,30 @@ InternalQemuFwCfgDmaIsAvailable (
 }
 
 /**
+  Transfer an array of bytes, or skip a number of bytes, using the DMA
+  interface.
 
- Returns a boolean indicating whether SEV is enabled
+  @param[in]     Size     Size in bytes to transfer or skip.
 
- @retval    TRUE    SEV is enabled
- @retval    FALSE   SEV is disabled
-**/
-BOOLEAN
-InternalQemuFwCfgSevIsEnabled (
-  VOID
-  )
-{
-  //
-  // DMA is not supported in SEC phase hence SEV support is irrelevant
-  //
-  return FALSE;
-}
-
-/**
- Allocate a bounce buffer for SEV DMA.
-
-  @param[in]     NumPage  Number of pages.
-  @param[out]    Buffer   Allocated DMA Buffer pointer
-
-**/
-VOID
-InternalQemuFwCfgSevDmaAllocateBuffer (
-  OUT    VOID     **Buffer,
-  IN     UINT32   NumPages
-  )
-{
-  //
-  // We should never reach here
-  //
-  ASSERT (FALSE);
-}
-
-/**
- Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
-
-  @param[in]     NumPage  Number of pages.
-  @param[in]     Buffer   DMA Buffer pointer
+  @param[in,out] Buffer   Buffer to read data into or write data from. Ignored,
+                          and may be NULL, if Size is zero, or Control is
+                          FW_CFG_DMA_CTL_SKIP.
 
+  @param[in]     Control  One of the following:
+                          FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
+                          FW_CFG_DMA_CTL_READ  - read from fw_cfg into Buffer.
+                          FW_CFG_DMA_CTL_SKIP  - skip bytes in fw_cfg.
 **/
 VOID
-InternalQemuFwCfgSevDmaFreeBuffer (
-  IN     VOID     *Buffer,
-  IN     UINT32   NumPages
+InternalQemuFwCfgDmaBytes (
+  IN     UINT32   Size,
+  IN OUT VOID     *Buffer OPTIONAL,
+  IN     UINT32   Control
   )
 {
   //
   // We should never reach here
   //
   ASSERT (FALSE);
+  CpuDeadLoop ();
 }
-- 
2.7.4