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

Brijesh Singh posted 21 patches 8 years, 5 months ago
There is a newer version of this series
[edk2] [PATCH v2 18/23] OvmfPkg/VirtioScsiDxe: Use DeviceAddresses in vring descriptors
Posted by Brijesh Singh 8 years, 5 months ago
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 18/23] OvmfPkg/VirtioScsiDxe: Use DeviceAddresses in vring descriptors
Posted by Laszlo Ersek 8 years, 5 months ago
After all, I've taken a look here as well. I'm not going to point out
all the earlier remarks (please do address them here anyway, because
they certainly apply), I'll just say what I feel is specific to this patch:

On 08/14/17 13:36, Brijesh Singh wrote:
> 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;

(1) Please don't drop the volatile qualification. If you want to turn it
into a pointer, that might be OK, but the target of the pointer should
remain volatile.

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

(2) Hmm, this ZeroMem() now nulls the pointer. Probably not what you
intended.

> @@ -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.
> +  //

Good point!

(3) In that case however, please do the same for "HostStatus" in the
VirtioBlkDxe driver as well. We also pre-set that value for ourselves.

> +  Status = Dev->VirtIo->AllocateSharedPages (
> +                          Dev->VirtIo,
> +                          EFI_SIZE_TO_PAGES (sizeof *Response),
> +                          (VOID *) &Response
> +                          );

(4) In general I abhor this kind of pointer type punning (it is
undefined behavior in standard C), but it is so pervasive in edk2, when
looking up protocol interfaces, that I guess we can live with it.

However, the type that we cast &Response to should be (VOID **), not
(VOID *). The compiler didn't complain to you because (VOID *) silently
converts to and from other object pointer types -- assuming same const /
volatile qualification.

The cleanest solution for this would be, IMO:

  volatile VIRTIO_SCSI_RESP *Response;
  VOID                      *ResponseBuffer;

  Status = Dev->VirtIo->AllocateSharedPages (
                          Dev->VirtIo,
                          EFI_SIZE_TO_PAGES (sizeof *Response),
                          &ResponseBuffer
                          );
  ...
  Response = ResponseBuffer;

... I think this is all I wanted to say about this patch for now, on top
of my earlier comments for virtio-rng and virtio-blk, which also apply here.

Thanks,
Laszlo

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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel