[PATCH v3 13/23] OvmfPkg/VirtioBlkDxe: map host address to device address

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

The SynchronousRequest() function, programs the vring descriptor with the
buffers pointed-by virtio-blk requests, status and memory that is
referenced inside the request header.

The patch uses VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() function to map
host address to device address and programs the vring descriptor with
device addresses.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/VirtioBlkDxe/VirtioBlk.h |   1 +
 OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 201 +++++++++++++++++---
 2 files changed, 180 insertions(+), 22 deletions(-)

diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h
index 6c402ca88ea4..9ec0b956b818 100644
--- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h
+++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h
@@ -41,6 +41,7 @@ typedef struct {
   VRING                  Ring;                 // VirtioRingInit      2
   EFI_BLOCK_IO_PROTOCOL  BlockIo;              // VirtioBlkInit       1
   EFI_BLOCK_IO_MEDIA     BlockIoMedia;         // VirtioBlkInit       1
+  VOID                   *RingMap;             // VirtioRingMap       2
 } VBLK_DEV;
 
 #define VIRTIO_BLK_FROM_BLOCK_IO(BlockIoPointer) \
diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
index bff15fe3add1..e23762743f75 100644
--- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
+++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
@@ -232,7 +232,8 @@ VerifyReadWriteRequest (
 
   @retval EFI_DEVICE_ERROR     Failed to notify host side via VirtIo write, or
                                unable to parse host response, or host response
-                               is not VIRTIO_BLK_S_OK.
+                               is not VIRTIO_BLK_S_OK or failed to map Buffer
+                               for a bus master operation.
 
 **/
 
@@ -249,8 +250,16 @@ SynchronousRequest (
 {
   UINT32                  BlockSize;
   volatile VIRTIO_BLK_REQ Request;
-  volatile UINT8          HostStatus;
+  volatile UINT8          *HostStatus;
+  VOID                    *HostStatusBuffer;
   DESC_INDICES            Indices;
+  VOID                    *RequestMapping;
+  VOID                    *StatusMapping;
+  VOID                    *BufferMapping;
+  EFI_PHYSICAL_ADDRESS    BufferDeviceAddress;
+  EFI_PHYSICAL_ADDRESS    HostStatusDeviceAddress;
+  EFI_PHYSICAL_ADDRESS    RequestDeviceAddress;
+  EFI_STATUS              Status, Ret;
 
   BlockSize = Dev->BlockIoMedia.BlockSize;
 
@@ -278,9 +287,88 @@ SynchronousRequest (
   VirtioPrepare (&Dev->Ring, &Indices);
 
   //
+  // Host status is bi-directional (we preset with a value and expect the device
+  // to update it). Allocate a host status buffer which can be mapped to
+  // access equally by both processor and the device.
+  //
+  Status = Dev->VirtIo->AllocateSharedPages (
+                          Dev->VirtIo,
+                          EFI_SIZE_TO_PAGES (sizeof *HostStatus),
+                          &HostStatusBuffer
+                          );
+  if (EFI_ERROR (Status)) {
+    return EFI_DEVICE_ERROR;
+  }
+
+  //
+  // Map virtio-blk request header
+  //
+  Status = VirtioMapAllBytesInSharedBuffer (
+             Dev->VirtIo,
+             VirtioOperationBusMasterRead,
+             (VOID *) &Request,
+             sizeof Request,
+             &RequestDeviceAddress,
+             &RequestMapping
+             );
+  if (EFI_ERROR (Status)) {
+    Status = EFI_DEVICE_ERROR;
+    goto FreeHostStatusBuffer;
+  }
+
+  //
+  // Map data buffer
+  //
+  if (BufferSize > 0) {
+    if (RequestIsWrite) {
+      Status = VirtioMapAllBytesInSharedBuffer (
+                 Dev->VirtIo,
+                 VirtioOperationBusMasterRead,
+                 (VOID *) Buffer,
+                 BufferSize,
+                 &BufferDeviceAddress,
+                 &BufferMapping
+                 );
+    } else {
+      Status = VirtioMapAllBytesInSharedBuffer (
+                 Dev->VirtIo,
+                 VirtioOperationBusMasterWrite,
+                 (VOID *) Buffer,
+                 BufferSize,
+                 &BufferDeviceAddress,
+                 &BufferMapping
+                 );
+    }
+
+    if (EFI_ERROR (Status)) {
+      Status = EFI_DEVICE_ERROR;
+      goto UnmapRequestBuffer;
+    }
+  }
+
+  //
+  // Map the Status Buffer with VirtioOperationBusMasterCommonBuffer so that
+  // both processor and device can access it.
+  //
+  Status = VirtioMapAllBytesInSharedBuffer (
+             Dev->VirtIo,
+             VirtioOperationBusMasterCommonBuffer,
+             HostStatusBuffer,
+             sizeof *HostStatus,
+             &HostStatusDeviceAddress,
+             &StatusMapping
+             );
+  if (EFI_ERROR (Status)) {
+    Status = EFI_DEVICE_ERROR;
+    goto UnmapDataBuffer;
+  }
+
+  HostStatus = HostStatusBuffer;
+
+  //
   // preset a host status for ourselves that we do not accept as success
   //
-  HostStatus = VIRTIO_BLK_S_IOERR;
+  *HostStatus = VIRTIO_BLK_S_IOERR;
 
   //
   // ensured by VirtioBlkInit() -- this predicate, in combination with the
@@ -291,8 +379,13 @@ SynchronousRequest (
   //
   // virtio-blk header in first desc
   //
-  VirtioAppendDesc (&Dev->Ring, (UINTN) &Request, sizeof Request,
-    VRING_DESC_F_NEXT, &Indices);
+  VirtioAppendDesc (
+    &Dev->Ring,
+    RequestDeviceAddress,
+    sizeof Request,
+    VRING_DESC_F_NEXT,
+    &Indices
+    );
 
   //
   // data buffer for read/write in second desc
@@ -311,27 +404,62 @@ SynchronousRequest (
     //
     // VRING_DESC_F_WRITE is interpreted from the host's point of view.
     //
-    VirtioAppendDesc (&Dev->Ring, (UINTN) Buffer, (UINT32) BufferSize,
+    VirtioAppendDesc (
+      &Dev->Ring,
+      BufferDeviceAddress,
+      (UINT32) BufferSize,
       VRING_DESC_F_NEXT | (RequestIsWrite ? 0 : VRING_DESC_F_WRITE),
-      &Indices);
+      &Indices
+      );
   }
 
   //
   // host status in last (second or third) desc
   //
-  VirtioAppendDesc (&Dev->Ring, (UINTN) &HostStatus, sizeof HostStatus,
-    VRING_DESC_F_WRITE, &Indices);
+  VirtioAppendDesc (
+    &Dev->Ring,
+    HostStatusDeviceAddress,
+    sizeof *HostStatus,
+    VRING_DESC_F_WRITE,
+    &Indices
+    );
 
   //
   // virtio-blk's only virtqueue is #0, called "requestq" (see Appendix D).
   //
-  if (VirtioFlush (Dev->VirtIo, 0, &Dev->Ring, &Indices,
-        NULL) == EFI_SUCCESS &&
-      HostStatus == VIRTIO_BLK_S_OK) {
-    return EFI_SUCCESS;
+  Status = VirtioFlush (Dev->VirtIo, 0, &Dev->Ring, &Indices, NULL);
+
+  //
+  // Unmap the HostStatus buffer before accessing it
+  //
+  Ret = Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, StatusMapping);
+  if (EFI_ERROR (Ret)) {
+    Status = EFI_DEVICE_ERROR;
+  }
+
+  if (!EFI_ERROR (Status) &&
+      *HostStatus == VIRTIO_BLK_S_OK) {
+    Status = EFI_SUCCESS;
+  } else {
+    Status = EFI_DEVICE_ERROR;
   }
 
-  return EFI_DEVICE_ERROR;
+UnmapDataBuffer:
+  if (BufferSize > 0) {
+    Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, BufferMapping);
+  }
+
+UnmapRequestBuffer:
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, RequestMapping);
+
+FreeHostStatusBuffer:
+  Dev->VirtIo->FreeSharedPages (
+                 Dev->VirtIo,
+                 EFI_SIZE_TO_PAGES (sizeof *HostStatus),
+                 HostStatusBuffer
+                 );
+
+  return Status;
 }
 
 
@@ -580,7 +708,8 @@ VirtioBlkDriverBindingSupported (
                            virtio-blk attributes the host provides.
 
   @return                  Error codes from VirtioRingInit() or
-                           VIRTIO_CFG_READ() / VIRTIO_CFG_WRITE().
+                           VIRTIO_CFG_READ() / VIRTIO_CFG_WRITE or
+                           VirtioRingMap().
 
 **/
 
@@ -601,6 +730,7 @@ VirtioBlkInit (
   UINT8      AlignmentOffset;
   UINT32     OptIoSize;
   UINT16     QueueSize;
+  UINT64     RingBaseShift;
 
   PhysicalBlockExp = 0;
   AlignmentOffset = 0;
@@ -729,25 +859,42 @@ VirtioBlkInit (
   }
 
   //
+  // If anything fails from here on, we must release the ring resources
+  //
+  Status = VirtioRingMap (
+             Dev->VirtIo,
+             &Dev->Ring,
+             &RingBaseShift,
+             &Dev->RingMap
+             );
+  if (EFI_ERROR (Status)) {
+    goto ReleaseQueue;
+  }
+
+  //
   // Additional steps for MMIO: align the queue appropriately, and set the
-  // size. If anything fails from here on, we must release the ring resources.
+  // size. If anything fails from here on, we must unmap the ring resources.
   //
   Status = Dev->VirtIo->SetQueueNum (Dev->VirtIo, QueueSize);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
   Status = Dev->VirtIo->SetQueueAlign (Dev->VirtIo, EFI_PAGE_SIZE);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
   //
   // step 4c -- Report GPFN (guest-physical frame number) of queue.
   //
-  Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
+  Status = Dev->VirtIo->SetQueueAddress (
+                          Dev->VirtIo,
+                          &Dev->Ring,
+                          RingBaseShift
+                          );
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
 
@@ -758,7 +905,7 @@ VirtioBlkInit (
     Features &= ~(UINT64)VIRTIO_F_VERSION_1;
     Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features);
     if (EFI_ERROR (Status)) {
-      goto ReleaseQueue;
+      goto UnmapQueue;
     }
   }
 
@@ -768,7 +915,7 @@ VirtioBlkInit (
   NextDevStat |= VSTAT_DRIVER_OK;
   Status = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
   //
@@ -811,6 +958,9 @@ VirtioBlkInit (
   }
   return EFI_SUCCESS;
 
+UnmapQueue:
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
+
 ReleaseQueue:
   VirtioRingUninit (Dev->VirtIo, &Dev->Ring);
 
@@ -849,6 +999,7 @@ VirtioBlkUninit (
   //
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
 
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
   VirtioRingUninit (Dev->VirtIo, &Dev->Ring);
 
   SetMem (&Dev->BlockIo,      sizeof Dev->BlockIo,      0x00);
@@ -885,6 +1036,12 @@ VirtioBlkExitBoot (
   //
   Dev = Context;
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
+
+  //
+  // Unmap the ring buffer so that hypervisor will not be able to get
+  // readable data after device is reset.
+  //
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
 }
 
 /**
-- 
2.7.4