public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Zheng Xiang <zhengxiang9@huawei.com>
To: <edk2-devel@lists.01.org>
Cc: Zheng Xiang <zhengxiang9@huawei.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	Shannon Zhao <zhaoshenglong@huawei.com>
Subject: [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit
Date: Wed, 13 Dec 2017 11:16:32 +0800	[thread overview]
Message-ID: <20171213031632.11856-1-zhengxiang9@huawei.com> (raw)

VirtioScsiInit() allocate only one request queue which causes the
address of the other vrings to be 0. In AARCH64 virtual machines,
the address of system memory starts at 0x40000000 and the address
of rom starts at 0. This causes an error when QEMU translates the
guest physical memory of vhost-scsi vrings to host virtual memory.

So this patch fixes this issue by allocating all the required Vrings.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zheng Xiang <zhengxiang9@huawei.com>
---
When using qemu with vhost-scsi on ARM64, it will fail with below error:
qemu-kvm: Error start vhost dev
qemu-kvm: unable to start vhost-scsi: Cannot allocate memory
---
 OvmfPkg/Include/IndustryStandard/VirtioScsi.h |   1 +
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c            | 192 ++++++++++++++++----------
 OvmfPkg/VirtioScsiDxe/VirtioScsi.h            |   7 +-
 3 files changed, 122 insertions(+), 78 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/VirtioScsi.h b/OvmfPkg/Include/IndustryStandard/VirtioScsi.h
index 0c9b520..9f4e6be 100644
--- a/OvmfPkg/Include/IndustryStandard/VirtioScsi.h
+++ b/OvmfPkg/Include/IndustryStandard/VirtioScsi.h
@@ -81,6 +81,7 @@ typedef struct {
 // selector of first virtio queue usable for request transfer
 //
 #define VIRTIO_SCSI_REQUEST_QUEUE 2
+#define VIRTIO_SCSI_QUEUE_MAX     1024
 
 //
 // host response codes
diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
index 1a68f06..b9e72c8 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
@@ -590,19 +590,19 @@ VirtioScsiPassThru (
     goto FreeResponseBuffer;
   }
 
-  VirtioPrepare (&Dev->Ring, &Indices);
+  VirtioPrepare (&Dev->Ring[VIRTIO_SCSI_REQUEST_QUEUE], &Indices);
 
   //
   // ensured by VirtioScsiInit() -- this predicate, in combination with the
   // lock-step progress, ensures we don't have to track free descriptors.
   //
-  ASSERT (Dev->Ring.QueueSize >= 4);
+  ASSERT (Dev->Ring[VIRTIO_SCSI_REQUEST_QUEUE].QueueSize >= 4);
 
   //
   // enqueue Request
   //
   VirtioAppendDesc (
-    &Dev->Ring,
+    &Dev->Ring[VIRTIO_SCSI_REQUEST_QUEUE],
     RequestDeviceAddress,
     sizeof Request,
     VRING_DESC_F_NEXT,
@@ -614,7 +614,7 @@ VirtioScsiPassThru (
   //
   if (Packet->OutTransferLength > 0) {
     VirtioAppendDesc (
-      &Dev->Ring,
+      &Dev->Ring[VIRTIO_SCSI_REQUEST_QUEUE],
       OutDataDeviceAddress,
       Packet->OutTransferLength,
       VRING_DESC_F_NEXT,
@@ -626,7 +626,7 @@ VirtioScsiPassThru (
   // enqueue Response, to be written by the host
   //
   VirtioAppendDesc (
-    &Dev->Ring,
+    &Dev->Ring[VIRTIO_SCSI_REQUEST_QUEUE],
     ResponseDeviceAddress,
     sizeof *Response,
     VRING_DESC_F_WRITE | (Packet->InTransferLength > 0 ? VRING_DESC_F_NEXT : 0),
@@ -638,7 +638,7 @@ VirtioScsiPassThru (
   //
   if (Packet->InTransferLength > 0) {
     VirtioAppendDesc (
-      &Dev->Ring,
+      &Dev->Ring[VIRTIO_SCSI_REQUEST_QUEUE],
       InDataDeviceAddress,
       Packet->InTransferLength,
       VRING_DESC_F_WRITE,
@@ -650,7 +650,8 @@ VirtioScsiPassThru (
   // EFI_NOT_READY would save us the effort, but it would also suggest that the
   // caller retry.
   //
-  if (VirtioFlush (Dev->VirtIo, VIRTIO_SCSI_REQUEST_QUEUE, &Dev->Ring,
+  if (VirtioFlush (Dev->VirtIo, VIRTIO_SCSI_REQUEST_QUEUE,
+        &Dev->Ring[VIRTIO_SCSI_REQUEST_QUEUE],
         &Indices, NULL) != EFI_SUCCESS) {
     Status = ReportHostAdapterError (Packet);
     goto UnmapResponseBuffer;
@@ -912,6 +913,88 @@ VirtioScsiGetNextTarget (
   return EFI_NOT_FOUND;
 }
 
+STATIC
+EFI_STATUS
+EFIAPI
+VirtioScsiInitRing (
+ IN OUT VSCSI_DEV  *Dev,
+ IN     UINT16     Selector,
+ OUT    VRING      *Ring,
+ OUT    VOID       **Mapping
+ )
+{
+  EFI_STATUS Status;
+  UINT16     QueueSize;
+  UINT64     RingBaseShift;
+  VOID       *MapInfo;
+
+  //
+  // step 4b -- allocate request virtqueue
+  //
+  Status = Dev->VirtIo->SetQueueSel (Dev->VirtIo, Selector);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  Status = Dev->VirtIo->GetQueueNumMax (Dev->VirtIo, &QueueSize);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  //
+  // VirtioScsiPassThru() uses at most four descriptors
+  //
+  if (QueueSize < 4) {
+    Status = EFI_UNSUPPORTED;
+    return Status;
+  }
+
+  Status = VirtioRingInit (Dev->VirtIo, QueueSize, Ring);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // If anything fails from here on, we must release the ring resources
+  //
+  Status = VirtioRingMap (Dev->VirtIo, Ring, &RingBaseShift, &MapInfo);
+  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 unmap the ring resources.
+  //
+  Status = Dev->VirtIo->SetQueueNum (Dev->VirtIo, QueueSize);
+  if (EFI_ERROR (Status)) {
+    goto UnmapQueue;
+  }
+
+  Status = Dev->VirtIo->SetQueueAlign (Dev->VirtIo, EFI_PAGE_SIZE);
+  if (EFI_ERROR (Status)) {
+    goto UnmapQueue;
+  }
+
+  //
+  // step 4c -- Report GPFN (guest-physical frame number) of queue.
+  //
+  Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring, RingBaseShift);
+  if (EFI_ERROR (Status)) {
+    goto UnmapQueue;
+  }
+
+  *Mapping = MapInfo;
+
+  return EFI_SUCCESS;
+
+UnmapQueue:
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, MapInfo);
+
+ReleaseQueue:
+  VirtioRingUninit (Dev->VirtIo, Ring);
+
+  return Status;
+}
+
 
 STATIC
 EFI_STATUS
@@ -922,11 +1005,10 @@ VirtioScsiInit (
 {
   UINT8      NextDevStat;
   EFI_STATUS Status;
-  UINT64     RingBaseShift;
   UINT64     Features;
   UINT16     MaxChannel; // for validation only
-  UINT32     NumQueues;  // for validation only
-  UINT16     QueueSize;
+  UINT16     Count;
+  UINT16     Index;
 
   //
   // Execute virtio-0.9.5, 2.2.1 Device Initialization Sequence.
@@ -978,11 +1060,11 @@ VirtioScsiInit (
     goto Failed;
   }
 
-  Status = VIRTIO_CFG_READ (Dev, NumQueues, &NumQueues);
+  Status = VIRTIO_CFG_READ (Dev, NumQueues, &Dev->NumQueues);
   if (EFI_ERROR (Status)) {
     goto Failed;
   }
-  if (NumQueues < 1) {
+  if (Dev->NumQueues < 1 || Dev->NumQueues > VIRTIO_SCSI_QUEUE_MAX - 2) {
     Status = EFI_UNSUPPORTED;
     goto Failed;
   }
@@ -1031,66 +1113,18 @@ VirtioScsiInit (
   }
 
   //
-  // step 4b -- allocate request virtqueue
-  //
-  Status = Dev->VirtIo->SetQueueSel (Dev->VirtIo, VIRTIO_SCSI_REQUEST_QUEUE);
-  if (EFI_ERROR (Status)) {
-    goto Failed;
-  }
-  Status = Dev->VirtIo->GetQueueNumMax (Dev->VirtIo, &QueueSize);
-  if (EFI_ERROR (Status)) {
-    goto Failed;
-  }
-  //
-  // VirtioScsiPassThru() uses at most four descriptors
-  //
-  if (QueueSize < 4) {
-    Status = EFI_UNSUPPORTED;
-    goto Failed;
-  }
-
-  Status = VirtioRingInit (Dev->VirtIo, QueueSize, &Dev->Ring);
-  if (EFI_ERROR (Status)) {
-    goto Failed;
-  }
-
-  //
-  // If anything fails from here on, we must release the ring resources
+  // step 4b, 4c -- allocate and report virtqueues
   //
-  Status = VirtioRingMap (
-             Dev->VirtIo,
-             &Dev->Ring,
-             &RingBaseShift,
-             &Dev->RingMap
+  for (Count = 0; Count < Dev->NumQueues + 2; Count++) {
+    Status = VirtioScsiInitRing (
+             Dev,
+             Count,
+             &Dev->Ring[Count],
+             &Dev->RingMap[Count]
              );
-  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 unmap the ring resources.
-  //
-  Status = Dev->VirtIo->SetQueueNum (Dev->VirtIo, QueueSize);
-  if (EFI_ERROR (Status)) {
-    goto UnmapQueue;
-  }
-
-  Status = Dev->VirtIo->SetQueueAlign (Dev->VirtIo, EFI_PAGE_SIZE);
-  if (EFI_ERROR (Status)) {
-    goto UnmapQueue;
-  }
-
-  //
-  // step 4c -- Report GPFN (guest-physical frame number) of queue.
-  //
-  Status = Dev->VirtIo->SetQueueAddress (
-                          Dev->VirtIo,
-                          &Dev->Ring,
-                          RingBaseShift
-                          );
-  if (EFI_ERROR (Status)) {
-    goto UnmapQueue;
+    if (EFI_ERROR (Status)) {
+      goto UnmapQueue;
+    }
   }
 
   //
@@ -1160,10 +1194,10 @@ VirtioScsiInit (
   return EFI_SUCCESS;
 
 UnmapQueue:
-  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
-
-ReleaseQueue:
-  VirtioRingUninit (Dev->VirtIo, &Dev->Ring);
+  for (Index = Count - 1; Index >= 0; Index--) {
+    Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap[Index]);
+    VirtioRingUninit (Dev->VirtIo, &Dev->Ring[Index]);
+  }
 
 Failed:
   //
@@ -1177,6 +1211,7 @@ Failed:
   Dev->MaxTarget      = 0;
   Dev->MaxLun         = 0;
   Dev->MaxSectors     = 0;
+  Dev->NumQueues      = 0;
 
   return Status; // reached only via Failed above
 }
@@ -1189,6 +1224,7 @@ VirtioScsiUninit (
   IN OUT VSCSI_DEV *Dev
   )
 {
+  INT16    Index;
   //
   // Reset the virtual device -- see virtio-0.9.5, 2.2.2.1 Device Status. When
   // VIRTIO_CFG_WRITE() returns, the host will have learned to stay away from
@@ -1201,8 +1237,12 @@ VirtioScsiUninit (
   Dev->MaxLun         = 0;
   Dev->MaxSectors     = 0;
 
-  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
-  VirtioRingUninit (Dev->VirtIo, &Dev->Ring);
+  for (Index = 0; Index < Dev->NumQueues + 2; Index++) {
+    Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap[Index]);
+    VirtioRingUninit (Dev->VirtIo, &Dev->Ring[Index]);
+  }
+
+  Dev->NumQueues      = 0;
 
   SetMem (&Dev->PassThru,     sizeof Dev->PassThru,     0x00);
   SetMem (&Dev->PassThruMode, sizeof Dev->PassThruMode, 0x00);
diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
index 05a6bf5..40ee6ba 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
@@ -57,10 +57,13 @@ typedef struct {
   UINT16                          MaxTarget;      // VirtioScsiInit      1
   UINT32                          MaxLun;         // VirtioScsiInit      1
   UINT32                          MaxSectors;     // VirtioScsiInit      1
-  VRING                           Ring;           // VirtioRingInit      2
+  UINT32                          NumQueues;      // VirtioScsiInit      1
+  VRING                           Ring[VIRTIO_SCSI_QUEUE_MAX];
+                                                  // VirtioScsiInitRing  2
   EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;       // VirtioScsiInit      1
   EFI_EXT_SCSI_PASS_THRU_MODE     PassThruMode;   // VirtioScsiInit      1
-  VOID                            *RingMap;       // VirtioRingMap       2
+  VOID                            *RingMap[VIRTIO_SCSI_QUEUE_MAX];
+                                                  // VirtioRingMap       3
 } VSCSI_DEV;
 
 #define VIRTIO_SCSI_FROM_PASS_THRU(PassThruPointer) \
-- 
1.8.3.1




             reply	other threads:[~2017-12-13  3:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13  3:16 Zheng Xiang [this message]
2017-12-13  8:35 ` [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit Laszlo Ersek
2017-12-13  9:29   ` Paolo Bonzini
2017-12-13 11:04     ` Laszlo Ersek
2017-12-13 11:16     ` Laszlo Ersek
2017-12-14  6:55       ` zhengxiang (A)
2017-12-14  9:06         ` Paolo Bonzini
2017-12-14 13:25           ` zhengxiang (A)
2018-01-11 13:23             ` Maxime Coquelin
2018-01-11 14:46               ` Maxime Coquelin
2018-01-12  1:35                 ` zhengxiang (A)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171213031632.11856-1-zhengxiang9@huawei.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox