OvmfPkg/QemuFwCfgLib: Use BusMasterCommonBuffer to map FW_CFG_DMA_ACCESS

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>
[lersek@redhat.com: convert pointers to UINTN before converting to UINT64]
[lersek@redhat.com: fix argument indentation in multi-line function call]
[lersek@redhat.com: explicitly compare pointers to NULL]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

5 files changed, 367 insertions, 292 deletions

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
index f8eb03b..8b98208 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));

 

-  @param[in]     NumPage  Number of pages.

-  @param[in]     Buffer   DMA Buffer pointer

+  Status = mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping);

+  if (EFI_ERROR (Status)) {

+    DEBUG ((DEBUG_ERROR,

+      "%a:%a failed to UnMap() Mapping 0x%Lx\n", gEfiCallerBaseName,

+      __FUNCTION__, (UINT64)(UINTN)Mapping));

+    ASSERT (FALSE);

+    CpuDeadLoop ();

+  }

+

+  Status = mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, Access);

+  if (EFI_ERROR (Status)) {

+    DEBUG ((DEBUG_ERROR,

+      "%a:%a failed to Free() 0x%Lx\n", gEfiCallerBaseName, __FUNCTION__,

+      (UINT64)(UINTN)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)(UINTN)HostAddress, (UINT64)Size));

+    ASSERT (FALSE);

+    CpuDeadLoop ();

+  }

 

-  ASSERT (mIoMmuProtocol != NULL);

+  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 ();

+  }

 

-  Status = mIoMmuProtocol->FreeBuffer (

-                            mIoMmuProtocol,

-                            NumPages,

-                            Buffer

-                          );

+  *DeviceAddress = PhysicalAddress;

+  *MapInfo = Mapping;

+}

+

+STATIC

+VOID

+UnmapFwCfgDmaDataBuffer (

+  IN  VOID  *Mapping

+  )

+{

+  EFI_STATUS  Status;

+

+  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)(UINTN)Mapping));

     ASSERT (FALSE);

     CpuDeadLoop ();

   }

+}

 

-  DEBUG ((DEBUG_VERBOSE,

-    "%a:%a buffer 0x%Lx Pages %u\n", gEfiCallerBaseName,__FUNCTION__,

-    (UINT64)(UINTN)Buffer, NumPages));

+/**

+  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;

+    }

+  }

+

+  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 != NULL) {

+    FreeFwCfgDmaAccessBuffer ((VOID *)Access, AccessMapping);

+  }

+

+  //

+  // If DataBuffer was mapped then unmap it.

+  //

+  if (DataMapping != NULL) {

+    UnmapFwCfgDmaDataBuffer (DataMapping);

+  }

 }

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
index d3bf754..7f42f38 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/QemuFwCfgLibInternal.h b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
index 8cfa791..327f1d3 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

-  );

-

-/**

- Allocate a bounce buffer for SEV DMA.

+  @param[in]     Size     Size in bytes to transfer or skip.

 

-  @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/QemuFwCfgPei.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
index 40f89c3..471ab03 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 ();

+

+  //

+  // 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));

 

-**/

-VOID

-InternalQemuFwCfgSevDmaFreeBuffer (

-  IN     VOID     *Buffer,

-  IN     UINT32   NumPages

-  )

-{

   //

-  // We should never reach here

+  // Don't look at Access.Control before starting the transfer.

   //

-  ASSERT (FALSE);

-  CpuDeadLoop ();

+  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 ();

 }

+

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
index 071b8d9..1eadba9 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 ();

 }