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
next 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