[PATCH v2 1/1] OvmfPkg : QemuFwCfgLib: Use BusMasterCommandBuffer 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         | 247 ++++++++++++++++----
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c         | 130 -----------
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c         | 101 +++++---
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c         |  56 ++---
 5 files changed, 292 insertions(+), 284 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..e03647bae3bd 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>
@@ -157,74 +158,228 @@ 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 ().
 
 **/
-VOID
-InternalQemuFwCfgSevDmaAllocateBuffer (
-  OUT    VOID     **Buffer,
-  IN     UINT32   NumPages
+STATIC
+EFI_STATUS
+AllocFwCfgDmaAccessBuffer (
+  OUT   VOID     **Access,
+  OUT   VOID     **Mapping
   )
 {
-  EFI_STATUS    Status;
+  UINTN                 Size;
+  UINTN                 NumPages;
+  EFI_STATUS            Status;
+  VOID                  *HostAddress;
+  EFI_PHYSICAL_ADDRESS  DmaAddress;
 
-  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_MEMORY_CACHED
                           );
+
+  //
+  // Map the host buffer with BusMasterCommonBuffer64
+  //
+  Status = mIoMmuProtocol->Map (
+                             mIoMmuProtocol,
+                             EdkiiIoMmuOperationBusMasterCommonBuffer64,
+                             HostAddress,
+                             &Size,
+                             &DmaAddress,
+                             Mapping
+                             );
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR,
-      "%a:%a failed to allocate %u pages\n", gEfiCallerBaseName, __FUNCTION__,
-      NumPages));
-    ASSERT (FALSE);
-    CpuDeadLoop ();
+    mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, HostAddress);
   }
 
-  DEBUG ((DEBUG_VERBOSE,
-    "%a:%a buffer 0x%Lx Pages %u\n", gEfiCallerBaseName, __FUNCTION__,
-    (UINT64)(UINTN)Buffer, NumPages));
+  *Access = HostAddress;
+  return Status;
+}
+
+/**
+  Function is to used for freeing the Access buffer allocated using
+  AllocFwCfgDmaAccessBuffer()
+
+**/
+STATIC
+VOID
+FreeFwCfgDmaAccessBuffer (
+  IN  VOID    *Access,
+  IN  VOID    *Mapping
+  )
+{
+  UINTN   NumPages;
+
+  NumPages = EFI_SIZE_TO_PAGES (sizeof (FW_CFG_DMA_ACCESS));
+
+  mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping);
+
+  mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, Access);
+}
+
+/**
+  Function is used for mapping host address to device address. The buffer must
+  be unmapped with UnmapDmaDataBuffer ().
+
+**/
+STATIC
+EFI_STATUS
+MapFwCfgDmaDataBuffer (
+  IN  BOOLEAN               IsWrite,
+  IN  VOID                  *HostAddress,
+  IN  UINT32                Size,
+  OUT EFI_PHYSICAL_ADDRESS  *DeviceAddress,
+  OUT VOID                  **Mapping
+  )
+{
+  EFI_STATUS            Status;
+  UINTN                 NumberOfBytes;
+
+  NumberOfBytes = Size;
+  Status = mIoMmuProtocol->Map (
+                             mIoMmuProtocol,
+                             IsWrite ? EdkiiIoMmuOperationBusMasterRead64 : EdkiiIoMmuOperationBusMasterWrite64,
+                             HostAddress,
+                             &NumberOfBytes,
+                             DeviceAddress,
+                             Mapping
+                             );
+  return Status;
+}
+
+EFI_STATUS
+UnmapFwCfgDmaDataBuffer (
+  IN  VOID  *Mapping
+  )
+{
+  return mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping);
 }
 
 /**
- Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+  Transfer an array of bytes, or skip a number of bytes, using the DMA
+  interface.
 
-  @param[in]     NumPage  Number of pages.
-  @param[in]     Buffer   DMA Buffer pointer
+  @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
-InternalQemuFwCfgSevDmaFreeBuffer (
-  IN     VOID     *Buffer,
-  IN     UINT32   NumPages
+InternalQemuFwCfgDmaBytes (
+  IN     UINT32   Size,
+  IN OUT VOID     *Buffer OPTIONAL,
+  IN     UINT32   Control
   )
 {
-  EFI_STATUS    Status;
+  volatile FW_CFG_DMA_ACCESS LocalAccess;
+  volatile FW_CFG_DMA_ACCESS *Access;
+  UINT32                     AccessHigh, AccessLow;
+  UINT32                     Status;
+  VOID                       *AccessMapping, *DataMapping;
 
-  ASSERT (mIoMmuProtocol != NULL);
+  ASSERT (Control == FW_CFG_DMA_CTL_WRITE || Control == FW_CFG_DMA_CTL_READ ||
+    Control == FW_CFG_DMA_CTL_SKIP);
 
-  Status = mIoMmuProtocol->FreeBuffer (
-                            mIoMmuProtocol,
-                            NumPages,
-                            Buffer
-                          );
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR,
-      "%a:%a failed to free buffer 0x%Lx pages %u\n", gEfiCallerBaseName,
-      __FUNCTION__, (UINT64)(UINTN)Buffer, NumPages));
-    ASSERT (FALSE);
-    CpuDeadLoop ();
+  if (Size == 0) {
+    return;
   }
 
-  DEBUG ((DEBUG_VERBOSE,
-    "%a:%a buffer 0x%Lx Pages %u\n", gEfiCallerBaseName,__FUNCTION__,
-    (UINT64)(UINTN)Buffer, NumPages));
+  //
+  // When SEV is enabled, map Buffer to DMA address before issuing the DMA request
+  //
+  if (MemEncryptSevIsEnabled ()) {
+    VOID                  *AccessBuffer;
+    EFI_PHYSICAL_ADDRESS  DataBuffer;
+
+    //
+    // Allocate DMA Access buffer
+    //
+    Status = AllocFwCfgDmaAccessBuffer (&AccessBuffer, &AccessMapping);
+    ASSERT_EFI_ERROR (Status);
+
+    Access = AccessBuffer;
+
+    //
+    // Map actual data buffer
+    //
+    if (Buffer) {
+      Status = MapFwCfgDmaDataBuffer (Control == FW_CFG_DMA_CTL_WRITE ? 1 : 0,
+                Buffer, Size, &DataBuffer, &DataMapping);
+      ASSERT_EFI_ERROR (Status);
+
+      Buffer = (VOID *) (UINTN) DataBuffer;
+    }
+  } else {
+    Access = &LocalAccess;
+    AccessMapping = NULL;
+    DataMapping = NULL;
+  }
+
+  Access->Control = SwapBytes32 (Control);
+  Access->Length  = SwapBytes32 (Size);
+  Access->Address = SwapBytes64 ((UINTN)Buffer);
+
+  //
+  // 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 (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..bc649b40aec3 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,80 @@ 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;
+  }
+
+  if (MemEncryptSevIsEnabled ()) {
+    //
+    // SEV does not support DMA operations in PEI stage, we should
+    // not have reached here.
+    //
+    ASSERT (FALSE);
+  }
+
+  Access.Control = SwapBytes32 (Control);
+  Access.Length  = SwapBytes32 (Size);
+  Access.Address = SwapBytes64 ((UINTN)Buffer);
 
-**/
-VOID
-InternalQemuFwCfgSevDmaAllocateBuffer (
-  OUT    VOID     **Buffer,
-  IN     UINT32   NumPages
-  )
-{
   //
-  // We should never reach here
+  // Delimit the transfer from (a) modifications to Access, (b) in case of a
+  // write, from writes to Buffer by the caller.
   //
-  ASSERT (FALSE);
-  CpuDeadLoop ();
-}
+  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));
 
-/**
- Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+  //
+  // Don't look at Access.Control before starting the transfer.
+  //
+  MemoryFence ();
 
-  @param[in]     NumPage  Number of pages.
-  @param[in]     Buffer   DMA Buffer pointer
+  //
+  // Wait for the transfer to complete.
+  //
+  do {
+    Status = SwapBytes32 (Access.Control);
+    ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
+  } while (Status != 0);
 
-**/
-VOID
-InternalQemuFwCfgSevDmaFreeBuffer (
-  IN     VOID     *Buffer,
-  IN     UINT32   NumPages
-  )
-{
   //
-  // We should never reach here
+  // 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..62ddb69d3b4d 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
@@ -97,53 +97,25 @@ 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
   )
 {
   //
-- 
2.7.4