* [Qemu-devel] [PATCH 1/7] util: add return value to qemu_iovec_concat_iov
2014-06-12 12:09 [Qemu-devel] [PATCH 0/7] virtio-scsi: do not rely on iov boundaries Paolo Bonzini
@ 2014-06-12 12:09 ` Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 2/7] virtio-scsi: start preparing for any_layout Paolo Bonzini
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst
This will be necessary later to recognize the case where a
request has both dataout and datain.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu-common.h | 6 +++---
util/iov.c | 10 ++++++----
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 66ceceb..ae76197 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -315,9 +315,9 @@ void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov);
void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
void qemu_iovec_concat(QEMUIOVector *dst,
QEMUIOVector *src, size_t soffset, size_t sbytes);
-void qemu_iovec_concat_iov(QEMUIOVector *dst,
- struct iovec *src_iov, unsigned int src_cnt,
- size_t soffset, size_t sbytes);
+size_t qemu_iovec_concat_iov(QEMUIOVector *dst,
+ struct iovec *src_iov, unsigned int src_cnt,
+ size_t soffset, size_t sbytes);
bool qemu_iovec_is_zero(QEMUIOVector *qiov);
void qemu_iovec_destroy(QEMUIOVector *qiov);
void qemu_iovec_reset(QEMUIOVector *qiov);
diff --git a/util/iov.c b/util/iov.c
index 49f8838..2b4f46d 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -295,15 +295,15 @@ void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len)
* of src".
* Only vector pointers are processed, not the actual data buffers.
*/
-void qemu_iovec_concat_iov(QEMUIOVector *dst,
- struct iovec *src_iov, unsigned int src_cnt,
- size_t soffset, size_t sbytes)
+size_t qemu_iovec_concat_iov(QEMUIOVector *dst,
+ struct iovec *src_iov, unsigned int src_cnt,
+ size_t soffset, size_t sbytes)
{
int i;
size_t done;
if (!sbytes) {
- return;
+ return 0;
}
assert(dst->nalloc != -1);
for (i = 0, done = 0; done < sbytes && i < src_cnt; i++) {
@@ -317,6 +317,8 @@ void qemu_iovec_concat_iov(QEMUIOVector *dst,
}
}
assert(soffset == 0); /* offset beyond end of src */
+
+ return done;
}
/*
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 2/7] virtio-scsi: start preparing for any_layout
2014-06-12 12:09 [Qemu-devel] [PATCH 0/7] virtio-scsi: do not rely on iov boundaries Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 1/7] util: add return value to qemu_iovec_concat_iov Paolo Bonzini
@ 2014-06-12 12:09 ` Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 3/7] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields Paolo Bonzini
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst
- Introduce virtio_scsi_init_req and virtio_scsi_free_req
- rename qemu_sgl_init_external to qemu_sgl_concat
- move virtio_scsi_parse_req from virtio_scsi_pop_req to callers
and add header length checks to virtio_scsi_parse_req.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/virtio-scsi.c | 121 ++++++++++++++++++++++++++++++--------------------
1 file changed, 72 insertions(+), 49 deletions(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index b39880a..e548ad3 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -15,6 +15,7 @@
#include "hw/virtio/virtio-scsi.h"
#include "qemu/error-report.h"
+#include "qemu/iov.h"
#include <hw/scsi/scsi.h>
#include <block/scsi.h>
#include <hw/virtio/virtio-bus.h>
@@ -56,18 +57,35 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
}
+static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
+{
+ VirtIOSCSIReq *req;
+ req = g_malloc(sizeof(*req));
+
+ req->vq = vq;
+ req->dev = s;
+ req->sreq = NULL;
+ qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
+ return req;
+}
+
+static void virtio_scsi_free_req(VirtIOSCSIReq *req)
+{
+ qemu_sglist_destroy(&req->qsgl);
+ g_free(req);
+}
+
static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
{
VirtIOSCSI *s = req->dev;
VirtQueue *vq = req->vq;
VirtIODevice *vdev = VIRTIO_DEVICE(s);
virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
- qemu_sglist_destroy(&req->qsgl);
if (req->sreq) {
req->sreq->hba_private = NULL;
scsi_req_unref(req->sreq);
}
- g_free(req);
+ virtio_scsi_free_req(req);
virtio_notify(vdev, vq);
}
@@ -77,50 +95,55 @@ static void virtio_scsi_bad_req(void)
exit(1);
}
-static void qemu_sgl_init_external(VirtIOSCSIReq *req, struct iovec *sg,
+static void qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *sg,
hwaddr *addr, int num)
{
QEMUSGList *qsgl = &req->qsgl;
- qemu_sglist_init(qsgl, DEVICE(req->dev), num, &address_space_memory);
while (num--) {
qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len);
}
}
-static void virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq,
- VirtIOSCSIReq *req)
+static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
+ unsigned req_size, unsigned resp_size)
{
- assert(req->elem.in_num);
- req->vq = vq;
- req->dev = s;
- req->sreq = NULL;
+ if (req->elem.in_num == 0) {
+ return -EINVAL;
+ }
+
+ if (req->elem.out_sg[0].iov_len < req_size) {
+ return -EINVAL;
+ }
if (req->elem.out_num) {
req->req.buf = req->elem.out_sg[0].iov_base;
}
+
+ if (req->elem.in_sg[0].iov_len < resp_size) {
+ return -EINVAL;
+ }
req->resp.buf = req->elem.in_sg[0].iov_base;
if (req->elem.out_num > 1) {
- qemu_sgl_init_external(req, &req->elem.out_sg[1],
- &req->elem.out_addr[1],
- req->elem.out_num - 1);
+ qemu_sgl_concat(req, &req->elem.out_sg[1],
+ &req->elem.out_addr[1],
+ req->elem.out_num - 1);
} else {
- qemu_sgl_init_external(req, &req->elem.in_sg[1],
- &req->elem.in_addr[1],
- req->elem.in_num - 1);
+ qemu_sgl_concat(req, &req->elem.in_sg[1],
+ &req->elem.in_addr[1],
+ req->elem.in_num - 1);
}
+
+ return 0;
}
static VirtIOSCSIReq *virtio_scsi_pop_req(VirtIOSCSI *s, VirtQueue *vq)
{
- VirtIOSCSIReq *req;
- req = g_malloc(sizeof(*req));
+ VirtIOSCSIReq *req = virtio_scsi_init_req(s, vq);
if (!virtqueue_pop(vq, &req->elem)) {
- g_free(req);
+ virtio_scsi_free_req(req);
return NULL;
}
-
- virtio_scsi_parse_req(s, vq, req);
return req;
}
@@ -143,9 +166,9 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
VirtIOSCSIReq *req;
uint32_t n;
- req = g_malloc(sizeof(*req));
qemu_get_be32s(f, &n);
assert(n < vs->conf.num_queues);
+ req = virtio_scsi_init_req(s, vs->cmd_vqs[n]);
qemu_get_buffer(f, (unsigned char *)&req->elem, sizeof(req->elem));
/* TODO: add a way for SCSIBusInfo's load_request to fail,
* and fail migration instead of asserting here.
@@ -156,7 +179,12 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
#endif
assert(req->elem.in_num <= ARRAY_SIZE(req->elem.in_sg));
assert(req->elem.out_num <= ARRAY_SIZE(req->elem.out_sg));
- virtio_scsi_parse_req(s, vs->cmd_vqs[n], req);
+
+ if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
+ sizeof(VirtIOSCSICmdResp) + vs->sense_size) < 0) {
+ error_report("invalid SCSI request migration data");
+ exit(1);
+ }
scsi_req_ref(sreq);
req->sreq = sreq;
@@ -281,29 +309,29 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
VirtIOSCSIReq *req;
while ((req = virtio_scsi_pop_req(s, vq))) {
- int out_size, in_size;
- if (req->elem.out_num < 1 || req->elem.in_num < 1) {
+ int type;
+
+ if (iov_to_buf(&req->elem.out_sg[0], req->elem.out_num, 0,
+ &type, sizeof(type)) < sizeof(type)) {
virtio_scsi_bad_req();
- continue;
- }
- out_size = req->elem.out_sg[0].iov_len;
- in_size = req->elem.in_sg[0].iov_len;
- if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
- if (out_size < sizeof(VirtIOSCSICtrlTMFReq) ||
- in_size < sizeof(VirtIOSCSICtrlTMFResp)) {
+ } else if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
+ if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
+ sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
virtio_scsi_bad_req();
+ } else {
+ virtio_scsi_do_tmf(s, req);
}
- virtio_scsi_do_tmf(s, req);
} else if (req->req.tmf->type == VIRTIO_SCSI_T_AN_QUERY ||
req->req.tmf->type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
- if (out_size < sizeof(VirtIOSCSICtrlANReq) ||
- in_size < sizeof(VirtIOSCSICtrlANResp)) {
+ if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
+ sizeof(VirtIOSCSICtrlANResp)) < 0) {
virtio_scsi_bad_req();
+ } else {
+ req->resp.an->event_actual = 0;
+ req->resp.an->response = VIRTIO_SCSI_S_OK;
}
- req->resp.an->event_actual = 0;
- req->resp.an->response = VIRTIO_SCSI_S_OK;
}
virtio_scsi_complete_req(req);
}
@@ -373,23 +401,18 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
while ((req = virtio_scsi_pop_req(s, vq))) {
SCSIDevice *d;
- int out_size, in_size;
- if (req->elem.out_num < 1 || req->elem.in_num < 1) {
- virtio_scsi_bad_req();
- }
-
- out_size = req->elem.out_sg[0].iov_len;
- in_size = req->elem.in_sg[0].iov_len;
- if (out_size < sizeof(VirtIOSCSICmdReq) + vs->cdb_size ||
- in_size < sizeof(VirtIOSCSICmdResp) + vs->sense_size) {
- virtio_scsi_bad_req();
- }
-
+ int rc;
if (req->elem.out_num > 1 && req->elem.in_num > 1) {
virtio_scsi_fail_cmd_req(req);
continue;
}
+ rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
+ sizeof(VirtIOSCSICmdResp) + vs->sense_size);
+ if (rc < 0) {
+ virtio_scsi_bad_req();
+ }
+
d = virtio_scsi_device_find(s, req->req.cmd->lun);
if (!d) {
req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET;
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 3/7] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields
2014-06-12 12:09 [Qemu-devel] [PATCH 0/7] virtio-scsi: do not rely on iov boundaries Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 1/7] util: add return value to qemu_iovec_concat_iov Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 2/7] virtio-scsi: start preparing for any_layout Paolo Bonzini
@ 2014-06-12 12:09 ` Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 4/7] virtio-scsi: add extra argument and return type to qemu_sgl_concat Paolo Bonzini
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/virtio-scsi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index e548ad3..c32c99b 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -207,6 +207,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
/* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */
req->resp.tmf->response = VIRTIO_SCSI_S_OK;
+ tswap32s(&req->req.tmf->subtype);
switch (req->req.tmf->subtype) {
case VIRTIO_SCSI_T_TMF_ABORT_TASK:
case VIRTIO_SCSI_T_TMF_QUERY_TASK:
@@ -314,8 +315,11 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
if (iov_to_buf(&req->elem.out_sg[0], req->elem.out_num, 0,
&type, sizeof(type)) < sizeof(type)) {
virtio_scsi_bad_req();
+ continue;
+ }
- } else if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
+ tswap32s(&req->req.tmf->type);
+ if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
virtio_scsi_bad_req();
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 4/7] virtio-scsi: add extra argument and return type to qemu_sgl_concat
2014-06-12 12:09 [Qemu-devel] [PATCH 0/7] virtio-scsi: do not rely on iov boundaries Paolo Bonzini
` (2 preceding siblings ...)
2014-06-12 12:09 ` [Qemu-devel] [PATCH 3/7] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields Paolo Bonzini
@ 2014-06-12 12:09 ` Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 5/7] virtio-scsi: prepare sense data handling for any_layout Paolo Bonzini
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst
Will be used for anylayout support.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/virtio-scsi.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index c32c99b..7337dbf 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -95,14 +95,27 @@ static void virtio_scsi_bad_req(void)
exit(1);
}
-static void qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *sg,
- hwaddr *addr, int num)
+static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov,
+ hwaddr *addr, int num, size_t skip)
{
QEMUSGList *qsgl = &req->qsgl;
-
- while (num--) {
- qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len);
+ size_t copied = 0;
+
+ while (num) {
+ if (skip >= iov->iov_len) {
+ skip -= iov->iov_len;
+ } else {
+ qemu_sglist_add(qsgl, *addr + skip, iov->iov_len - skip);
+ copied += iov->iov_len - skip;
+ skip = 0;
+ }
+ iov++;
+ addr++;
+ num--;
}
+
+ assert(skip == 0);
+ return copied;
}
static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
@@ -127,11 +140,11 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
if (req->elem.out_num > 1) {
qemu_sgl_concat(req, &req->elem.out_sg[1],
&req->elem.out_addr[1],
- req->elem.out_num - 1);
+ req->elem.out_num - 1, 0);
} else {
qemu_sgl_concat(req, &req->elem.in_sg[1],
&req->elem.in_addr[1],
- req->elem.in_num - 1);
+ req->elem.in_num - 1, 0);
}
return 0;
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 5/7] virtio-scsi: prepare sense data handling for any_layout
2014-06-12 12:09 [Qemu-devel] [PATCH 0/7] virtio-scsi: do not rely on iov boundaries Paolo Bonzini
` (3 preceding siblings ...)
2014-06-12 12:09 ` [Qemu-devel] [PATCH 4/7] virtio-scsi: add extra argument and return type to qemu_sgl_concat Paolo Bonzini
@ 2014-06-12 12:09 ` Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 6/7] virtio-scsi: introduce virtio_scsi_complete_cmd_req Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 7/7] virtio-scsi: add support for the any_layout feature Paolo Bonzini
6 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst
Retrieve sense and copy it to guest memory, to prepare for when we will use
qemu_iovec_from_buf.
Swap response and request, since we'll use the tail of VirtIOSCSIReq
for the CDB.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/virtio-scsi.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 7337dbf..60f77cd 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -26,12 +26,7 @@ typedef struct VirtIOSCSIReq {
VirtQueueElement elem;
QEMUSGList qsgl;
SCSIRequest *sreq;
- union {
- char *buf;
- VirtIOSCSICmdReq *cmd;
- VirtIOSCSICtrlTMFReq *tmf;
- VirtIOSCSICtrlANReq *an;
- } req;
+ size_t resp_size;
union {
char *buf;
VirtIOSCSICmdResp *cmd;
@@ -39,6 +34,12 @@ typedef struct VirtIOSCSIReq {
VirtIOSCSICtrlANResp *an;
VirtIOSCSIEvent *event;
} resp;
+ union {
+ char *buf;
+ VirtIOSCSICmdReq *cmd;
+ VirtIOSCSICtrlTMFReq *tmf;
+ VirtIOSCSICtrlANReq *an;
+ } req;
} VirtIOSCSIReq;
static inline int virtio_scsi_get_lun(uint8_t *lun)
@@ -136,6 +137,7 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
return -EINVAL;
}
req->resp.buf = req->elem.in_sg[0].iov_base;
+ req->resp_size = resp_size;
if (req->elem.out_num > 1) {
qemu_sgl_concat(req, &req->elem.out_sg[1],
@@ -358,8 +360,7 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
size_t resid)
{
VirtIOSCSIReq *req = r->hba_private;
- VirtIOSCSI *s = req->dev;
- VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+ uint8_t sense[SCSI_SENSE_BUF_SIZE];
uint32_t sense_len;
if (r->io_canceled) {
@@ -372,8 +373,9 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
req->resp.cmd->resid = tswap32(resid);
} else {
req->resp.cmd->resid = 0;
- sense_len = scsi_req_get_sense(r, req->resp.cmd->sense,
- vs->sense_size);
+ sense_len = scsi_req_get_sense(r, sense, sizeof(sense));
+ sense_len = MIN(sense_len, req->resp_size - sizeof(req->resp.cmd));
+ memcpy(req->resp.cmd->sense, sense, sense_len);
req->resp.cmd->sense_len = tswap32(sense_len);
}
virtio_scsi_complete_req(req);
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 6/7] virtio-scsi: introduce virtio_scsi_complete_cmd_req
2014-06-12 12:09 [Qemu-devel] [PATCH 0/7] virtio-scsi: do not rely on iov boundaries Paolo Bonzini
` (4 preceding siblings ...)
2014-06-12 12:09 ` [Qemu-devel] [PATCH 5/7] virtio-scsi: prepare sense data handling for any_layout Paolo Bonzini
@ 2014-06-12 12:09 ` Paolo Bonzini
2014-06-12 12:09 ` [Qemu-devel] [PATCH 7/7] virtio-scsi: add support for the any_layout feature Paolo Bonzini
6 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst
This is also related to sense handling, and will be used
by anylayout.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/virtio-scsi.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 60f77cd..107e9cb 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -356,6 +356,11 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
}
}
+static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
+{
+ virtio_scsi_complete_req(req);
+}
+
static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
size_t resid)
{
@@ -378,7 +383,7 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
memcpy(req->resp.cmd->sense, sense, sense_len);
req->resp.cmd->sense_len = tswap32(sense_len);
}
- virtio_scsi_complete_req(req);
+ virtio_scsi_complete_cmd_req(req);
}
static QEMUSGList *virtio_scsi_get_sg_list(SCSIRequest *r)
@@ -435,7 +440,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
d = virtio_scsi_device_find(s, req->req.cmd->lun);
if (!d) {
req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET;
- virtio_scsi_complete_req(req);
+ virtio_scsi_complete_cmd_req(req);
continue;
}
req->sreq = scsi_req_new(d, req->req.cmd->tag,
@@ -449,7 +454,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
if (req->sreq->cmd.mode != req_mode ||
req->sreq->cmd.xfer > req->qsgl.size) {
req->resp.cmd->response = VIRTIO_SCSI_S_OVERRUN;
- virtio_scsi_complete_req(req);
+ virtio_scsi_complete_cmd_req(req);
continue;
}
}
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 7/7] virtio-scsi: add support for the any_layout feature
2014-06-12 12:09 [Qemu-devel] [PATCH 0/7] virtio-scsi: do not rely on iov boundaries Paolo Bonzini
` (5 preceding siblings ...)
2014-06-12 12:09 ` [Qemu-devel] [PATCH 6/7] virtio-scsi: introduce virtio_scsi_complete_cmd_req Paolo Bonzini
@ 2014-06-12 12:09 ` Paolo Bonzini
2014-06-12 12:33 ` Michael S. Tsirkin
6 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 12:09 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, mst
Store the request and response headers by value, and let
virtio_scsi_parse_req check that there is only one of datain
and dataout.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/virtio-scsi.c | 161 ++++++++++++++++++++++------------------
include/hw/i386/pc.h | 4 +
include/hw/virtio/virtio-scsi.h | 4 +-
3 files changed, 93 insertions(+), 76 deletions(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 107e9cb..391717d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -27,21 +27,26 @@ typedef struct VirtIOSCSIReq {
QEMUSGList qsgl;
SCSIRequest *sreq;
size_t resp_size;
+ QEMUIOVector resp_iov;
union {
- char *buf;
- VirtIOSCSICmdResp *cmd;
- VirtIOSCSICtrlTMFResp *tmf;
- VirtIOSCSICtrlANResp *an;
- VirtIOSCSIEvent *event;
+ VirtIOSCSICmdResp cmd;
+ VirtIOSCSICtrlTMFResp tmf;
+ VirtIOSCSICtrlANResp an;
+ VirtIOSCSIEvent event;
} resp;
union {
- char *buf;
- VirtIOSCSICmdReq *cmd;
- VirtIOSCSICtrlTMFReq *tmf;
- VirtIOSCSICtrlANReq *an;
+ struct {
+ VirtIOSCSICmdReq cmd;
+ uint8_t cdb[];
+ } QEMU_PACKED;
+ VirtIOSCSICtrlTMFReq tmf;
+ VirtIOSCSICtrlANReq an;
} req;
} VirtIOSCSIReq;
+QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) !=
+ offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq));
+
static inline int virtio_scsi_get_lun(uint8_t *lun)
{
return ((lun[2] << 8) | lun[3]) & 0x3FFF;
@@ -61,17 +66,21 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
{
VirtIOSCSIReq *req;
- req = g_malloc(sizeof(*req));
+ VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+
+ req = g_malloc(sizeof(*req) + vs->cdb_size);
req->vq = vq;
req->dev = s;
req->sreq = NULL;
qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
+ qemu_iovec_init(&req->resp_iov, 1);
return req;
}
static void virtio_scsi_free_req(VirtIOSCSIReq *req)
{
+ qemu_iovec_destroy(&req->resp_iov);
qemu_sglist_destroy(&req->qsgl);
g_free(req);
}
@@ -81,7 +90,9 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
VirtIOSCSI *s = req->dev;
VirtQueue *vq = req->vq;
VirtIODevice *vdev = VIRTIO_DEVICE(s);
- virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
+
+ qemu_iovec_from_buf(&req->resp_iov, 0, &req->resp, req->resp_size);
+ virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
if (req->sreq) {
req->sreq->hba_private = NULL;
scsi_req_unref(req->sreq);
@@ -122,31 +133,29 @@ static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov,
static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
unsigned req_size, unsigned resp_size)
{
- if (req->elem.in_num == 0) {
- return -EINVAL;
- }
+ size_t in_size, out_size;
- if (req->elem.out_sg[0].iov_len < req_size) {
+ if (iov_to_buf(&req->elem.out_sg[0], req->elem.out_num, 0,
+ &req->req, req_size) < req_size) {
return -EINVAL;
}
- if (req->elem.out_num) {
- req->req.buf = req->elem.out_sg[0].iov_base;
- }
- if (req->elem.in_sg[0].iov_len < resp_size) {
+ if (qemu_iovec_concat_iov(&req->resp_iov,
+ &req->elem.in_sg[0], req->elem.in_num, 0,
+ resp_size) < resp_size) {
return -EINVAL;
}
- req->resp.buf = req->elem.in_sg[0].iov_base;
req->resp_size = resp_size;
- if (req->elem.out_num > 1) {
- qemu_sgl_concat(req, &req->elem.out_sg[1],
- &req->elem.out_addr[1],
- req->elem.out_num - 1, 0);
- } else {
- qemu_sgl_concat(req, &req->elem.in_sg[1],
- &req->elem.in_addr[1],
- req->elem.in_num - 1, 0);
+ out_size = qemu_sgl_concat(req, &req->elem.out_sg[0],
+ &req->elem.out_addr[0], req->elem.out_num,
+ req_size);
+ in_size = qemu_sgl_concat(req, &req->elem.in_sg[0],
+ &req->elem.in_addr[0], req->elem.in_num,
+ resp_size);
+
+ if (out_size && in_size) {
+ return -ENOTSUP;
}
return 0;
@@ -214,27 +223,27 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
{
- SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf->lun);
+ SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
SCSIRequest *r, *next;
BusChild *kid;
int target;
/* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */
- req->resp.tmf->response = VIRTIO_SCSI_S_OK;
+ req->resp.tmf.response = VIRTIO_SCSI_S_OK;
- tswap32s(&req->req.tmf->subtype);
- switch (req->req.tmf->subtype) {
+ tswap32s(&req->req.tmf.subtype);
+ switch (req->req.tmf.subtype) {
case VIRTIO_SCSI_T_TMF_ABORT_TASK:
case VIRTIO_SCSI_T_TMF_QUERY_TASK:
if (!d) {
goto fail;
}
- if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
+ if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
goto incorrect_lun;
}
QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
VirtIOSCSIReq *cmd_req = r->hba_private;
- if (cmd_req && cmd_req->req.cmd->tag == req->req.tmf->tag) {
+ if (cmd_req && cmd_req->req.cmd.tag == req->req.tmf.tag) {
break;
}
}
@@ -244,11 +253,11 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
* check for it in the loop above.
*/
assert(r->hba_private);
- if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) {
+ if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) {
/* "If the specified command is present in the task set, then
* return a service response set to FUNCTION SUCCEEDED".
*/
- req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
+ req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
} else {
scsi_req_cancel(r);
}
@@ -259,7 +268,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
if (!d) {
goto fail;
}
- if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
+ if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
goto incorrect_lun;
}
s->resetting++;
@@ -273,16 +282,16 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
if (!d) {
goto fail;
}
- if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
+ if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
goto incorrect_lun;
}
QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
if (r->hba_private) {
- if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
+ if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
/* "If there is any command present in the task set, then
* return a service response set to FUNCTION SUCCEEDED".
*/
- req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
+ req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
break;
} else {
scsi_req_cancel(r);
@@ -292,7 +301,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
break;
case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
- target = req->req.tmf->lun[1];
+ target = req->req.tmf.lun[1];
s->resetting++;
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
d = DO_UPCAST(SCSIDevice, qdev, kid->child);
@@ -305,18 +314,18 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
case VIRTIO_SCSI_T_TMF_CLEAR_ACA:
default:
- req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_REJECTED;
+ req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_REJECTED;
break;
}
return;
incorrect_lun:
- req->resp.tmf->response = VIRTIO_SCSI_S_INCORRECT_LUN;
+ req->resp.tmf.response = VIRTIO_SCSI_S_INCORRECT_LUN;
return;
fail:
- req->resp.tmf->response = VIRTIO_SCSI_S_BAD_TARGET;
+ req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET;
}
static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
@@ -333,8 +342,8 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
continue;
}
- tswap32s(&req->req.tmf->type);
- if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
+ tswap32s(&req->req.tmf.type);
+ if (req->req.tmf.type == VIRTIO_SCSI_T_TMF) {
if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
virtio_scsi_bad_req();
@@ -342,14 +351,14 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
virtio_scsi_do_tmf(s, req);
}
- } else if (req->req.tmf->type == VIRTIO_SCSI_T_AN_QUERY ||
- req->req.tmf->type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
+ } else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY ||
+ req->req.tmf.type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
sizeof(VirtIOSCSICtrlANResp)) < 0) {
virtio_scsi_bad_req();
} else {
- req->resp.an->event_actual = 0;
- req->resp.an->response = VIRTIO_SCSI_S_OK;
+ req->resp.an.event_actual = 0;
+ req->resp.an.response = VIRTIO_SCSI_S_OK;
}
}
virtio_scsi_complete_req(req);
@@ -358,6 +367,10 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
{
+ /* Sense data is not in req->resp and is copied separately
+ * in virtio_scsi_command_complete.
+ */
+ req->resp_size = sizeof(VirtIOSCSICmdResp);
virtio_scsi_complete_req(req);
}
@@ -372,16 +385,17 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
return;
}
- req->resp.cmd->response = VIRTIO_SCSI_S_OK;
- req->resp.cmd->status = status;
- if (req->resp.cmd->status == GOOD) {
- req->resp.cmd->resid = tswap32(resid);
+ req->resp.cmd.response = VIRTIO_SCSI_S_OK;
+ req->resp.cmd.status = status;
+ if (req->resp.cmd.status == GOOD) {
+ req->resp.cmd.resid = tswap32(resid);
} else {
- req->resp.cmd->resid = 0;
+ req->resp.cmd.resid = 0;
sense_len = scsi_req_get_sense(r, sense, sizeof(sense));
- sense_len = MIN(sense_len, req->resp_size - sizeof(req->resp.cmd));
- memcpy(req->resp.cmd->sense, sense, sense_len);
- req->resp.cmd->sense_len = tswap32(sense_len);
+ sense_len = MIN(sense_len, req->resp_iov.size - sizeof(req->resp.cmd));
+ qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd),
+ &req->resp, sense_len);
+ req->resp.cmd.sense_len = tswap32(sense_len);
}
virtio_scsi_complete_cmd_req(req);
}
@@ -401,16 +415,16 @@ static void virtio_scsi_request_cancelled(SCSIRequest *r)
return;
}
if (req->dev->resetting) {
- req->resp.cmd->response = VIRTIO_SCSI_S_RESET;
+ req->resp.cmd.response = VIRTIO_SCSI_S_RESET;
} else {
- req->resp.cmd->response = VIRTIO_SCSI_S_ABORTED;
+ req->resp.cmd.response = VIRTIO_SCSI_S_ABORTED;
}
virtio_scsi_complete_req(req);
}
static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req)
{
- req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE;
+ req->resp.cmd.response = VIRTIO_SCSI_S_FAILURE;
virtio_scsi_complete_req(req);
}
@@ -426,26 +440,27 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
while ((req = virtio_scsi_pop_req(s, vq))) {
SCSIDevice *d;
int rc;
- if (req->elem.out_num > 1 && req->elem.in_num > 1) {
- virtio_scsi_fail_cmd_req(req);
- continue;
- }
rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
sizeof(VirtIOSCSICmdResp) + vs->sense_size);
if (rc < 0) {
- virtio_scsi_bad_req();
+ if (rc == -ENOTSUP) {
+ virtio_scsi_fail_cmd_req(req);
+ } else {
+ virtio_scsi_bad_req();
+ }
+ continue;
}
- d = virtio_scsi_device_find(s, req->req.cmd->lun);
+ d = virtio_scsi_device_find(s, req->req.cmd.lun);
if (!d) {
- req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET;
+ req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
virtio_scsi_complete_cmd_req(req);
continue;
}
- req->sreq = scsi_req_new(d, req->req.cmd->tag,
- virtio_scsi_get_lun(req->req.cmd->lun),
- req->req.cmd->cdb, req);
+ req->sreq = scsi_req_new(d, req->req.cmd.tag,
+ virtio_scsi_get_lun(req->req.cmd.lun),
+ req->req.cdb, req);
if (req->sreq->cmd.mode != SCSI_XFER_NONE) {
int req_mode =
@@ -453,7 +468,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
if (req->sreq->cmd.mode != req_mode ||
req->sreq->cmd.xfer > req->qsgl.size) {
- req->resp.cmd->response = VIRTIO_SCSI_S_OVERRUN;
+ req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
virtio_scsi_complete_cmd_req(req);
continue;
}
@@ -574,7 +589,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
virtio_scsi_bad_req();
}
- evt = req->resp.event;
+ evt = &req->resp.event;
memset(evt, 0, sizeof(VirtIOSCSIEvent));
evt->event = event;
evt->reason = reason;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fa9d997..ca7a0bd 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -268,6 +268,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_0 \
{\
+ .driver = "virtio-scsi-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },{\
.driver = "apic",\
.property = "version",\
.value = stringify(0x11),\
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 42b1024..de0615b 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -84,14 +84,13 @@
#define VIRTIO_SCSI_EVT_RESET_RESCAN 1
#define VIRTIO_SCSI_EVT_RESET_REMOVED 2
-/* SCSI command request, followed by data-out */
+/* SCSI command request, followed by CDB and data-out */
typedef struct {
uint8_t lun[8]; /* Logical Unit Number */
uint64_t tag; /* Command identifier */
uint8_t task_attr; /* Task attribute */
uint8_t prio;
uint8_t crn;
- uint8_t cdb[];
} QEMU_PACKED VirtIOSCSICmdReq;
/* Response, followed by sense data and data-in */
@@ -101,7 +100,6 @@ typedef struct {
uint16_t status_qualifier; /* Status qualifier */
uint8_t status; /* Command completion status */
uint8_t response; /* Response values */
- uint8_t sense[];
} QEMU_PACKED VirtIOSCSICmdResp;
/* Task Management Request */
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 7/7] virtio-scsi: add support for the any_layout feature
2014-06-12 12:09 ` [Qemu-devel] [PATCH 7/7] virtio-scsi: add support for the any_layout feature Paolo Bonzini
@ 2014-06-12 12:33 ` Michael S. Tsirkin
2014-06-12 13:52 ` Paolo Bonzini
0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2014-06-12 12:33 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: famz, qemu-devel
On Thu, Jun 12, 2014 at 02:09:08PM +0200, Paolo Bonzini wrote:
> Store the request and response headers by value, and let
> virtio_scsi_parse_req check that there is only one of datain
> and dataout.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/scsi/virtio-scsi.c | 161 ++++++++++++++++++++++------------------
> include/hw/i386/pc.h | 4 +
> include/hw/virtio/virtio-scsi.h | 4 +-
> 3 files changed, 93 insertions(+), 76 deletions(-)
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 107e9cb..391717d 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -27,21 +27,26 @@ typedef struct VirtIOSCSIReq {
> QEMUSGList qsgl;
> SCSIRequest *sreq;
> size_t resp_size;
> + QEMUIOVector resp_iov;
> union {
> - char *buf;
> - VirtIOSCSICmdResp *cmd;
> - VirtIOSCSICtrlTMFResp *tmf;
> - VirtIOSCSICtrlANResp *an;
> - VirtIOSCSIEvent *event;
> + VirtIOSCSICmdResp cmd;
> + VirtIOSCSICtrlTMFResp tmf;
> + VirtIOSCSICtrlANResp an;
> + VirtIOSCSIEvent event;
> } resp;
> union {
> - char *buf;
> - VirtIOSCSICmdReq *cmd;
> - VirtIOSCSICtrlTMFReq *tmf;
> - VirtIOSCSICtrlANReq *an;
> + struct {
> + VirtIOSCSICmdReq cmd;
> + uint8_t cdb[];
> + } QEMU_PACKED;
> + VirtIOSCSICtrlTMFReq tmf;
> + VirtIOSCSICtrlANReq an;
> } req;
> } VirtIOSCSIReq;
>
> +QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) !=
> + offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq));
> +
> static inline int virtio_scsi_get_lun(uint8_t *lun)
> {
> return ((lun[2] << 8) | lun[3]) & 0x3FFF;
> @@ -61,17 +66,21 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
> static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
> {
> VirtIOSCSIReq *req;
> - req = g_malloc(sizeof(*req));
> + VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> +
> + req = g_malloc(sizeof(*req) + vs->cdb_size);
>
> req->vq = vq;
> req->dev = s;
> req->sreq = NULL;
> qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
> + qemu_iovec_init(&req->resp_iov, 1);
> return req;
> }
>
> static void virtio_scsi_free_req(VirtIOSCSIReq *req)
> {
> + qemu_iovec_destroy(&req->resp_iov);
> qemu_sglist_destroy(&req->qsgl);
> g_free(req);
> }
> @@ -81,7 +90,9 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
> VirtIOSCSI *s = req->dev;
> VirtQueue *vq = req->vq;
> VirtIODevice *vdev = VIRTIO_DEVICE(s);
> - virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
> +
> + qemu_iovec_from_buf(&req->resp_iov, 0, &req->resp, req->resp_size);
> + virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
> if (req->sreq) {
> req->sreq->hba_private = NULL;
> scsi_req_unref(req->sreq);
> @@ -122,31 +133,29 @@ static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov,
> static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
> unsigned req_size, unsigned resp_size)
> {
> - if (req->elem.in_num == 0) {
> - return -EINVAL;
> - }
> + size_t in_size, out_size;
>
> - if (req->elem.out_sg[0].iov_len < req_size) {
> + if (iov_to_buf(&req->elem.out_sg[0], req->elem.out_num, 0,
> + &req->req, req_size) < req_size) {
> return -EINVAL;
> }
I'd like to suggest converting &req->elem.out_sg[0] to
plain req->elem.out_sg. We can then easily greap for
in_sg[X] out_sg[X] and take that as a sign that
any_layout rules are violated.
> - if (req->elem.out_num) {
> - req->req.buf = req->elem.out_sg[0].iov_base;
> - }
>
> - if (req->elem.in_sg[0].iov_len < resp_size) {
> + if (qemu_iovec_concat_iov(&req->resp_iov,
> + &req->elem.in_sg[0], req->elem.in_num, 0,
> + resp_size) < resp_size) {
> return -EINVAL;
> }
> - req->resp.buf = req->elem.in_sg[0].iov_base;
> req->resp_size = resp_size;
>
> - if (req->elem.out_num > 1) {
> - qemu_sgl_concat(req, &req->elem.out_sg[1],
> - &req->elem.out_addr[1],
> - req->elem.out_num - 1, 0);
> - } else {
> - qemu_sgl_concat(req, &req->elem.in_sg[1],
> - &req->elem.in_addr[1],
> - req->elem.in_num - 1, 0);
> + out_size = qemu_sgl_concat(req, &req->elem.out_sg[0],
> + &req->elem.out_addr[0], req->elem.out_num,
> + req_size);
> + in_size = qemu_sgl_concat(req, &req->elem.in_sg[0],
> + &req->elem.in_addr[0], req->elem.in_num,
> + resp_size);
> +
> + if (out_size && in_size) {
> + return -ENOTSUP;
> }
>
> return 0;
> @@ -214,27 +223,27 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
>
> static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> {
> - SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf->lun);
> + SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
> SCSIRequest *r, *next;
> BusChild *kid;
> int target;
>
> /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */
> - req->resp.tmf->response = VIRTIO_SCSI_S_OK;
> + req->resp.tmf.response = VIRTIO_SCSI_S_OK;
>
> - tswap32s(&req->req.tmf->subtype);
> - switch (req->req.tmf->subtype) {
> + tswap32s(&req->req.tmf.subtype);
> + switch (req->req.tmf.subtype) {
> case VIRTIO_SCSI_T_TMF_ABORT_TASK:
> case VIRTIO_SCSI_T_TMF_QUERY_TASK:
> if (!d) {
> goto fail;
> }
> - if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
> + if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
> goto incorrect_lun;
> }
> QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
> VirtIOSCSIReq *cmd_req = r->hba_private;
> - if (cmd_req && cmd_req->req.cmd->tag == req->req.tmf->tag) {
> + if (cmd_req && cmd_req->req.cmd.tag == req->req.tmf.tag) {
> break;
> }
> }
> @@ -244,11 +253,11 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> * check for it in the loop above.
> */
> assert(r->hba_private);
> - if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) {
> + if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) {
> /* "If the specified command is present in the task set, then
> * return a service response set to FUNCTION SUCCEEDED".
> */
> - req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
> + req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
> } else {
> scsi_req_cancel(r);
> }
> @@ -259,7 +268,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> if (!d) {
> goto fail;
> }
> - if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
> + if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
> goto incorrect_lun;
> }
> s->resetting++;
> @@ -273,16 +282,16 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> if (!d) {
> goto fail;
> }
> - if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
> + if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
> goto incorrect_lun;
> }
> QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
> if (r->hba_private) {
> - if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
> + if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
> /* "If there is any command present in the task set, then
> * return a service response set to FUNCTION SUCCEEDED".
> */
> - req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
> + req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
> break;
> } else {
> scsi_req_cancel(r);
> @@ -292,7 +301,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> break;
>
> case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
> - target = req->req.tmf->lun[1];
> + target = req->req.tmf.lun[1];
> s->resetting++;
> QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
> d = DO_UPCAST(SCSIDevice, qdev, kid->child);
> @@ -305,18 +314,18 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
>
> case VIRTIO_SCSI_T_TMF_CLEAR_ACA:
> default:
> - req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_REJECTED;
> + req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_REJECTED;
> break;
> }
>
> return;
>
> incorrect_lun:
> - req->resp.tmf->response = VIRTIO_SCSI_S_INCORRECT_LUN;
> + req->resp.tmf.response = VIRTIO_SCSI_S_INCORRECT_LUN;
> return;
>
> fail:
> - req->resp.tmf->response = VIRTIO_SCSI_S_BAD_TARGET;
> + req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET;
> }
>
> static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> @@ -333,8 +342,8 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> continue;
> }
>
> - tswap32s(&req->req.tmf->type);
> - if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
> + tswap32s(&req->req.tmf.type);
> + if (req->req.tmf.type == VIRTIO_SCSI_T_TMF) {
> if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
> sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
> virtio_scsi_bad_req();
> @@ -342,14 +351,14 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> virtio_scsi_do_tmf(s, req);
> }
>
> - } else if (req->req.tmf->type == VIRTIO_SCSI_T_AN_QUERY ||
> - req->req.tmf->type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
> + } else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY ||
> + req->req.tmf.type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
> if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
> sizeof(VirtIOSCSICtrlANResp)) < 0) {
> virtio_scsi_bad_req();
> } else {
> - req->resp.an->event_actual = 0;
> - req->resp.an->response = VIRTIO_SCSI_S_OK;
> + req->resp.an.event_actual = 0;
> + req->resp.an.response = VIRTIO_SCSI_S_OK;
> }
> }
> virtio_scsi_complete_req(req);
> @@ -358,6 +367,10 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>
> static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
> {
> + /* Sense data is not in req->resp and is copied separately
> + * in virtio_scsi_command_complete.
> + */
> + req->resp_size = sizeof(VirtIOSCSICmdResp);
> virtio_scsi_complete_req(req);
> }
>
> @@ -372,16 +385,17 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
> return;
> }
>
> - req->resp.cmd->response = VIRTIO_SCSI_S_OK;
> - req->resp.cmd->status = status;
> - if (req->resp.cmd->status == GOOD) {
> - req->resp.cmd->resid = tswap32(resid);
> + req->resp.cmd.response = VIRTIO_SCSI_S_OK;
> + req->resp.cmd.status = status;
> + if (req->resp.cmd.status == GOOD) {
> + req->resp.cmd.resid = tswap32(resid);
> } else {
> - req->resp.cmd->resid = 0;
> + req->resp.cmd.resid = 0;
> sense_len = scsi_req_get_sense(r, sense, sizeof(sense));
> - sense_len = MIN(sense_len, req->resp_size - sizeof(req->resp.cmd));
> - memcpy(req->resp.cmd->sense, sense, sense_len);
> - req->resp.cmd->sense_len = tswap32(sense_len);
> + sense_len = MIN(sense_len, req->resp_iov.size - sizeof(req->resp.cmd));
> + qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd),
> + &req->resp, sense_len);
> + req->resp.cmd.sense_len = tswap32(sense_len);
> }
> virtio_scsi_complete_cmd_req(req);
> }
> @@ -401,16 +415,16 @@ static void virtio_scsi_request_cancelled(SCSIRequest *r)
> return;
> }
> if (req->dev->resetting) {
> - req->resp.cmd->response = VIRTIO_SCSI_S_RESET;
> + req->resp.cmd.response = VIRTIO_SCSI_S_RESET;
> } else {
> - req->resp.cmd->response = VIRTIO_SCSI_S_ABORTED;
> + req->resp.cmd.response = VIRTIO_SCSI_S_ABORTED;
> }
> virtio_scsi_complete_req(req);
> }
>
> static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req)
> {
> - req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE;
> + req->resp.cmd.response = VIRTIO_SCSI_S_FAILURE;
> virtio_scsi_complete_req(req);
> }
>
> @@ -426,26 +440,27 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
> while ((req = virtio_scsi_pop_req(s, vq))) {
> SCSIDevice *d;
> int rc;
> - if (req->elem.out_num > 1 && req->elem.in_num > 1) {
> - virtio_scsi_fail_cmd_req(req);
> - continue;
> - }
>
> rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
> sizeof(VirtIOSCSICmdResp) + vs->sense_size);
> if (rc < 0) {
> - virtio_scsi_bad_req();
> + if (rc == -ENOTSUP) {
> + virtio_scsi_fail_cmd_req(req);
> + } else {
> + virtio_scsi_bad_req();
> + }
> + continue;
> }
>
> - d = virtio_scsi_device_find(s, req->req.cmd->lun);
> + d = virtio_scsi_device_find(s, req->req.cmd.lun);
> if (!d) {
> - req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET;
> + req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
> virtio_scsi_complete_cmd_req(req);
> continue;
> }
> - req->sreq = scsi_req_new(d, req->req.cmd->tag,
> - virtio_scsi_get_lun(req->req.cmd->lun),
> - req->req.cmd->cdb, req);
> + req->sreq = scsi_req_new(d, req->req.cmd.tag,
> + virtio_scsi_get_lun(req->req.cmd.lun),
> + req->req.cdb, req);
>
> if (req->sreq->cmd.mode != SCSI_XFER_NONE) {
> int req_mode =
> @@ -453,7 +468,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
>
> if (req->sreq->cmd.mode != req_mode ||
> req->sreq->cmd.xfer > req->qsgl.size) {
> - req->resp.cmd->response = VIRTIO_SCSI_S_OVERRUN;
> + req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
> virtio_scsi_complete_cmd_req(req);
> continue;
> }
> @@ -574,7 +589,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
> virtio_scsi_bad_req();
> }
>
> - evt = req->resp.event;
> + evt = &req->resp.event;
> memset(evt, 0, sizeof(VirtIOSCSIEvent));
> evt->event = event;
> evt->reason = reason;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index fa9d997..ca7a0bd 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -268,6 +268,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
> #define PC_COMPAT_2_0 \
> {\
> + .driver = "virtio-scsi-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> .driver = "apic",\
> .property = "version",\
> .value = stringify(0x11),\
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 42b1024..de0615b 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -84,14 +84,13 @@
> #define VIRTIO_SCSI_EVT_RESET_RESCAN 1
> #define VIRTIO_SCSI_EVT_RESET_REMOVED 2
>
> -/* SCSI command request, followed by data-out */
> +/* SCSI command request, followed by CDB and data-out */
> typedef struct {
> uint8_t lun[8]; /* Logical Unit Number */
> uint64_t tag; /* Command identifier */
> uint8_t task_attr; /* Task attribute */
> uint8_t prio;
> uint8_t crn;
> - uint8_t cdb[];
> } QEMU_PACKED VirtIOSCSICmdReq;
>
> /* Response, followed by sense data and data-in */
> @@ -101,7 +100,6 @@ typedef struct {
> uint16_t status_qualifier; /* Status qualifier */
> uint8_t status; /* Command completion status */
> uint8_t response; /* Response values */
> - uint8_t sense[];
> } QEMU_PACKED VirtIOSCSICmdResp;
>
> /* Task Management Request */
> --
> 1.9.3
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 7/7] virtio-scsi: add support for the any_layout feature
2014-06-12 12:33 ` Michael S. Tsirkin
@ 2014-06-12 13:52 ` Paolo Bonzini
0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-06-12 13:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: famz, qemu-devel
Il 12/06/2014 14:33, Michael S. Tsirkin ha scritto:
> On Thu, Jun 12, 2014 at 02:09:08PM +0200, Paolo Bonzini wrote:
>> Store the request and response headers by value, and let
>> virtio_scsi_parse_req check that there is only one of datain
>> and dataout.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> hw/scsi/virtio-scsi.c | 161 ++++++++++++++++++++++------------------
>> include/hw/i386/pc.h | 4 +
>> include/hw/virtio/virtio-scsi.h | 4 +-
>> 3 files changed, 93 insertions(+), 76 deletions(-)
>>
>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index 107e9cb..391717d 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -27,21 +27,26 @@ typedef struct VirtIOSCSIReq {
>> QEMUSGList qsgl;
>> SCSIRequest *sreq;
>> size_t resp_size;
>> + QEMUIOVector resp_iov;
>> union {
>> - char *buf;
>> - VirtIOSCSICmdResp *cmd;
>> - VirtIOSCSICtrlTMFResp *tmf;
>> - VirtIOSCSICtrlANResp *an;
>> - VirtIOSCSIEvent *event;
>> + VirtIOSCSICmdResp cmd;
>> + VirtIOSCSICtrlTMFResp tmf;
>> + VirtIOSCSICtrlANResp an;
>> + VirtIOSCSIEvent event;
>> } resp;
>> union {
>> - char *buf;
>> - VirtIOSCSICmdReq *cmd;
>> - VirtIOSCSICtrlTMFReq *tmf;
>> - VirtIOSCSICtrlANReq *an;
>> + struct {
>> + VirtIOSCSICmdReq cmd;
>> + uint8_t cdb[];
>> + } QEMU_PACKED;
>> + VirtIOSCSICtrlTMFReq tmf;
>> + VirtIOSCSICtrlANReq an;
>> } req;
>> } VirtIOSCSIReq;
>>
>> +QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) !=
>> + offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq));
>> +
>> static inline int virtio_scsi_get_lun(uint8_t *lun)
>> {
>> return ((lun[2] << 8) | lun[3]) & 0x3FFF;
>> @@ -61,17 +66,21 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
>> static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
>> {
>> VirtIOSCSIReq *req;
>> - req = g_malloc(sizeof(*req));
>> + VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
>> +
>> + req = g_malloc(sizeof(*req) + vs->cdb_size);
>>
>> req->vq = vq;
>> req->dev = s;
>> req->sreq = NULL;
>> qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
>> + qemu_iovec_init(&req->resp_iov, 1);
>> return req;
>> }
>>
>> static void virtio_scsi_free_req(VirtIOSCSIReq *req)
>> {
>> + qemu_iovec_destroy(&req->resp_iov);
>> qemu_sglist_destroy(&req->qsgl);
>> g_free(req);
>> }
>> @@ -81,7 +90,9 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
>> VirtIOSCSI *s = req->dev;
>> VirtQueue *vq = req->vq;
>> VirtIODevice *vdev = VIRTIO_DEVICE(s);
>> - virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
>> +
>> + qemu_iovec_from_buf(&req->resp_iov, 0, &req->resp, req->resp_size);
>> + virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
>> if (req->sreq) {
>> req->sreq->hba_private = NULL;
>> scsi_req_unref(req->sreq);
>> @@ -122,31 +133,29 @@ static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov,
>> static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
>> unsigned req_size, unsigned resp_size)
>> {
>> - if (req->elem.in_num == 0) {
>> - return -EINVAL;
>> - }
>> + size_t in_size, out_size;
>>
>> - if (req->elem.out_sg[0].iov_len < req_size) {
>> + if (iov_to_buf(&req->elem.out_sg[0], req->elem.out_num, 0,
>> + &req->req, req_size) < req_size) {
>> return -EINVAL;
>> }
>
> I'd like to suggest converting &req->elem.out_sg[0] to
> plain req->elem.out_sg. We can then easily greap for
> in_sg[X] out_sg[X] and take that as a sign that
> any_layout rules are violated.
Very good idea, and it found a place where I was using iov_len instead
of the iov_size() function.
I also noticed that I'm using in_num > 1 to check for read vs. write
commands. All fixed like this:
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index ddfe5c7..73626fa 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -27,6 +27,7 @@ typedef struct VirtIOSCSIReq {
QEMUSGList qsgl;
SCSIRequest *sreq;
size_t resp_size;
+ enum SCSIXferMode mode;
QEMUIOVector resp_iov;
union {
VirtIOSCSICmdResp cmd;
VirtIOSCSICtrlTMFResp tmf;
@@ -158,6 +162,12 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
return -ENOTSUP;
}
+ if (out_size) {
+ req->mode = SCSI_XFER_TO_DEV;
+ } else if (in_size) {
+ req->mode = SCSI_XFER_FROM_DEV;
+ }
+
return 0;
}
@@ -213,10 +223,7 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
scsi_req_ref(sreq);
req->sreq = sreq;
if (req->sreq->cmd.mode != SCSI_XFER_NONE) {
- int req_mode =
- (req->elem.in_num > 1 ? SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV);
-
- assert(req->sreq->cmd.mode == req_mode);
+ assert(req->sreq->cmd.mode == req->mode);
}
return req;
}
@@ -463,15 +470,11 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
req->req.cdb, req);
if (req->sreq->cmd.mode != SCSI_XFER_NONE) {
- int req_mode =
- (req->elem.in_num > 1 ? SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV);
-
- if (req->sreq->cmd.mode != req_mode ||
+ && (req->sreq->cmd.mode != req->mode ||
req->sreq->cmd.xfer > req->qsgl.size) {
- req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
- virtio_scsi_complete_cmd_req(req);
- continue;
- }
+ req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
+ virtio_scsi_complete_cmd_req(req);
+ continue;
}
n = scsi_req_enqueue(req->sreq);
@@ -575,7 +578,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
return;
}
- if (req->elem.out_num || req->elem.in_num != 1) {
+ if (req->elem.out_num) {
virtio_scsi_bad_req();
}
@@ -584,7 +587,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
s->events_dropped = false;
}
- in_size = req->elem.in_sg[0].iov_len;
+ in_size = iov_size(req->elem.in_sg, req->elem.in_num);
if (in_size < sizeof(VirtIOSCSIEvent)) {
virtio_scsi_bad_req();
}
^ permalink raw reply related [flat|nested] 10+ messages in thread