[PATCH v2 18/23] OvmfPkg/VirtioScsiDxe: Use DeviceAddresses in vring descriptors

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

The VirtioScsiPassThru(), programs the vring descriptor using the host
addresses pointed-by virtio-scsi request, response and memory that is
referenced inside the request and response header.

The patch uses newly introduced VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer()
function to map system memory to device address and programs the vring
descriptors 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/VirtioScsiDxe/VirtioScsi.h |   1 +
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 148 +++++++++++++++++---
 2 files changed, 133 insertions(+), 16 deletions(-)

diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
index 6d00567e8cb8..bb1c5c70ef74 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
@@ -60,6 +60,7 @@ typedef struct {
   VRING                           Ring;           // VirtioRingInit      2
   EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;       // VirtioScsiInit      1
   EFI_EXT_SCSI_PASS_THRU_MODE     PassThruMode;   // VirtioScsiInit      1
+  VOID                            *RingBufMapping;// VirtioScsiInit      1
 } VSCSI_DEV;
 
 #define VIRTIO_SCSI_FROM_PASS_THRU(PassThruPointer) \
diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
index a983b3df7b9c..65e9bda0827a 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
@@ -409,8 +409,13 @@ VirtioScsiPassThru (
   UINT16                    TargetValue;
   EFI_STATUS                Status;
   volatile VIRTIO_SCSI_REQ  Request;
-  volatile VIRTIO_SCSI_RESP Response;
+  VIRTIO_SCSI_RESP          *Response;
   DESC_INDICES              Indices;
+  VOID                      *RequestMapping;
+  VOID                      *ResponseMapping;
+  VOID                      *InDataMapping;
+  VOID                      *OutDataMapping;
+  EFI_PHYSICAL_ADDRESS      DeviceAddress;
 
   ZeroMem ((VOID*) &Request, sizeof (Request));
   ZeroMem ((VOID*) &Response, sizeof (Response));
@@ -418,9 +423,41 @@ VirtioScsiPassThru (
   Dev = VIRTIO_SCSI_FROM_PASS_THRU (This);
   CopyMem (&TargetValue, Target, sizeof TargetValue);
 
+  Response = NULL;
+  ResponseMapping = NULL;
+  RequestMapping = NULL;
+  InDataMapping = NULL;
+  OutDataMapping = NULL;
+
+  //
+  // Response header is bi-direction (we preset with host status and expect the
+  // device to update it). Allocate a response buffer which can be mapped to
+  // access equally by both processor and device.
+  //
+  Status = Dev->VirtIo->AllocateSharedPages (
+                          Dev->VirtIo,
+                          EFI_SIZE_TO_PAGES (sizeof *Response),
+                          (VOID *) &Response
+                          );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = VirtioMapAllBytesInSharedBuffer (
+             Dev->VirtIo,
+             VirtioOperationBusMasterCommonBuffer,
+             (VOID *) Response,
+             sizeof (*Response),
+             &DeviceAddress,
+             &ResponseMapping
+             );
+  if (EFI_ERROR (Status)) {
+    goto Free_Response_Buffer;
+  }
+
   Status = PopulateRequest (Dev, TargetValue, Lun, Packet, &Request);
   if (EFI_ERROR (Status)) {
-    return Status;
+    goto Unmap_Response_Buffer;
   }
 
   VirtioPrepare (&Dev->Ring, &Indices);
@@ -428,7 +465,7 @@ VirtioScsiPassThru (
   //
   // preset a host status for ourselves that we do not accept as success
   //
-  Response.Response = VIRTIO_SCSI_S_FAILURE;
+  Response->Response = VIRTIO_SCSI_S_FAILURE;
 
   //
   // ensured by VirtioScsiInit() -- this predicate, in combination with the
@@ -437,23 +474,51 @@ VirtioScsiPassThru (
   ASSERT (Dev->Ring.QueueSize >= 4);
 
   //
+  // Map the scsi-blk request header HostAddress to DeviceAddress
+  //
+  Status = VirtioMapAllBytesInSharedBuffer (
+             Dev->VirtIo,
+             VirtioOperationBusMasterRead,
+             (VOID *) &Request,
+             sizeof Request,
+             &DeviceAddress,
+             &RequestMapping);
+  if (EFI_ERROR (Status)) {
+    goto Unmap_Response_Buffer;
+  }
+
+  //
   // enqueue Request
   //
-  VirtioAppendDesc (&Dev->Ring, (UINTN) &Request, sizeof Request,
+  VirtioAppendDesc (&Dev->Ring, (UINTN) DeviceAddress, sizeof Request,
     VRING_DESC_F_NEXT, &Indices);
 
   //
   // enqueue "dataout" if any
   //
   if (Packet->OutTransferLength > 0) {
-    VirtioAppendDesc (&Dev->Ring, (UINTN) Packet->OutDataBuffer,
+    //
+    // Map the buffer address to a device address
+    //
+    Status = VirtioMapAllBytesInSharedBuffer (
+               Dev->VirtIo,
+               VirtioOperationBusMasterRead,
+               Packet->OutDataBuffer,
+               Packet->OutTransferLength,
+               &DeviceAddress,
+               &OutDataMapping
+               );
+    if (EFI_ERROR (Status)) {
+      goto Unmap_Request_Buffer;
+    }
+    VirtioAppendDesc (&Dev->Ring, (UINTN) DeviceAddress,
       Packet->OutTransferLength, VRING_DESC_F_NEXT, &Indices);
   }
 
   //
   // enqueue Response, to be written by the host
   //
-  VirtioAppendDesc (&Dev->Ring, (UINTN) &Response, sizeof Response,
+  VirtioAppendDesc (&Dev->Ring, (UINTN) Response, sizeof *Response,
     VRING_DESC_F_WRITE | (Packet->InTransferLength > 0 ?
                           VRING_DESC_F_NEXT : 0),
     &Indices);
@@ -462,7 +527,22 @@ VirtioScsiPassThru (
   // enqueue "datain" if any, to be written by the host
   //
   if (Packet->InTransferLength > 0) {
-    VirtioAppendDesc (&Dev->Ring, (UINTN) Packet->InDataBuffer,
+    //
+    // Map the buffer address to a device address
+    //
+    Status = VirtioMapAllBytesInSharedBuffer (
+               Dev->VirtIo,
+               VirtioOperationBusMasterWrite,
+               Packet->InDataBuffer,
+               Packet->InTransferLength,
+               &DeviceAddress,
+               &InDataMapping
+               );
+    if (EFI_ERROR (Status)) {
+      goto Unmap_Out_Buffer;
+    }
+
+    VirtioAppendDesc (&Dev->Ring, (UINTN) DeviceAddress,
       Packet->InTransferLength, VRING_DESC_F_WRITE, &Indices);
   }
 
@@ -480,7 +560,29 @@ VirtioScsiPassThru (
     return EFI_DEVICE_ERROR;
   }
 
-  return ParseResponse (Packet, &Response);
+  Status = ParseResponse (Packet, Response);
+
+  if (Packet->InTransferLength > 0) {
+    Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, InDataMapping);
+  }
+
+Unmap_Out_Buffer:
+  if (Packet->OutTransferLength > 0) {
+    Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, OutDataMapping);
+  }
+
+Unmap_Request_Buffer:
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, RequestMapping);
+Unmap_Response_Buffer:
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, ResponseMapping);
+Free_Response_Buffer:
+  Dev->VirtIo->FreeSharedPages (
+                 Dev->VirtIo,
+                 EFI_SIZE_TO_PAGES (sizeof *Response),
+                 (VOID *) Response
+                 );
+
+  return Status;
 }
 
 
@@ -707,7 +809,7 @@ VirtioScsiInit (
 {
   UINT8      NextDevStat;
   EFI_STATUS Status;
-
+  UINT64     RingBaseShift;
   UINT64     Features;
   UINT16     MaxChannel; // for validation only
   UINT32     NumQueues;  // for validation only
@@ -838,18 +940,24 @@ VirtioScsiInit (
     goto Failed;
   }
 
+  Status = VirtioRingMap (Dev->VirtIo, &Dev->Ring, &RingBaseShift,
+             &Dev->RingBufMapping);
+  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.
   //
   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;
   }
 
   //
@@ -857,7 +965,7 @@ VirtioScsiInit (
   //
   Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
   //
@@ -867,7 +975,7 @@ VirtioScsiInit (
     Features &= ~(UINT64)VIRTIO_F_VERSION_1;
     Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features);
     if (EFI_ERROR (Status)) {
-      goto ReleaseQueue;
+      goto UnmapQueue;
     }
   }
 
@@ -877,11 +985,11 @@ VirtioScsiInit (
   //
   Status = VIRTIO_CFG_WRITE (Dev, CdbSize, VIRTIO_SCSI_CDB_SIZE);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
   Status = VIRTIO_CFG_WRITE (Dev, SenseSize, VIRTIO_SCSI_SENSE_SIZE);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
   //
@@ -890,7 +998,7 @@ VirtioScsiInit (
   NextDevStat |= VSTAT_DRIVER_OK;
   Status = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
   //
@@ -926,6 +1034,8 @@ VirtioScsiInit (
 
   return EFI_SUCCESS;
 
+UnmapQueue:
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingBufMapping);
 ReleaseQueue:
   VirtioRingUninit (Dev->VirtIo, &Dev->Ring);
 
@@ -995,6 +1105,12 @@ VirtioScsiExitBoot (
   //
   Dev = Context;
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
+
+  //
+  // If Ring buffer mapping exist then unmap it so that hypervisor can
+  // not get readable data after device reset.
+  //
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingBufMapping);
 }
 
 
-- 
2.7.4