* [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 16:32 ` Christoph Hellwig
2011-08-04 13:35 ` Stefan Hajnoczi
2011-08-03 8:49 ` [Qemu-devel] [PATCH 02/16] vscsi: always use get_sense Paolo Bonzini
` (15 subsequent siblings)
16 siblings, 2 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
the request will be completed successfully instead of showing a failure.
It can even cause a segmentation fault.
An easy way to trigger it is "eject -f cd" during installation (during media
test if the installer does something like that).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-disk.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f42a5d1..814bf74 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -217,9 +217,6 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
vm_stop(VMSTOP_DISKFULL);
} else {
- if (type == SCSI_REQ_STATUS_RETRY_READ) {
- scsi_req_data(&r->req, 0);
- }
switch (error) {
case ENOMEM:
scsi_command_complete(r, CHECK_CONDITION,
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read
2011-08-03 8:49 ` [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read Paolo Bonzini
@ 2011-08-03 16:32 ` Christoph Hellwig
2011-08-04 13:35 ` Stefan Hajnoczi
1 sibling, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2011-08-03 16:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Wed, Aug 03, 2011 at 10:49:04AM +0200, Paolo Bonzini wrote:
> In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
> the request will be completed successfully instead of showing a failure.
> It can even cause a segmentation fault.
>
> An easy way to trigger it is "eject -f cd" during installation (during media
> test if the installer does something like that).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read
2011-08-03 8:49 ` [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read Paolo Bonzini
2011-08-03 16:32 ` Christoph Hellwig
@ 2011-08-04 13:35 ` Stefan Hajnoczi
2011-08-04 13:46 ` Kevin Wolf
1 sibling, 1 reply; 29+ messages in thread
From: Stefan Hajnoczi @ 2011-08-04 13:35 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Paolo Bonzini, qemu-devel
On Wed, Aug 3, 2011 at 9:49 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
> the request will be completed successfully instead of showing a failure.
> It can even cause a segmentation fault.
>
> An easy way to trigger it is "eject -f cd" during installation (during media
> test if the installer does something like that).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/scsi-disk.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index f42a5d1..814bf74 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -217,9 +217,6 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
> vm_stop(VMSTOP_DISKFULL);
> } else {
> - if (type == SCSI_REQ_STATUS_RETRY_READ) {
> - scsi_req_data(&r->req, 0);
> - }
> switch (error) {
> case ENOMEM:
> scsi_command_complete(r, CHECK_CONDITION,
Kevin, do you remember why you added this in 5dba48a8?
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read
2011-08-04 13:35 ` Stefan Hajnoczi
@ 2011-08-04 13:46 ` Kevin Wolf
0 siblings, 0 replies; 29+ messages in thread
From: Kevin Wolf @ 2011-08-04 13:46 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel
Am 04.08.2011 15:35, schrieb Stefan Hajnoczi:
> On Wed, Aug 3, 2011 at 9:49 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
>> the request will be completed successfully instead of showing a failure.
>> It can even cause a segmentation fault.
>>
>> An easy way to trigger it is "eject -f cd" during installation (during media
>> test if the installer does something like that).
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> hw/scsi-disk.c | 3 ---
>> 1 files changed, 0 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>> index f42a5d1..814bf74 100644
>> --- a/hw/scsi-disk.c
>> +++ b/hw/scsi-disk.c
>> @@ -217,9 +217,6 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
>> vm_stop(VMSTOP_DISKFULL);
>> } else {
>> - if (type == SCSI_REQ_STATUS_RETRY_READ) {
>> - scsi_req_data(&r->req, 0);
>> - }
>> switch (error) {
>> case ENOMEM:
>> scsi_command_complete(r, CHECK_CONDITION,
>
> Kevin, do you remember why you added this in 5dba48a8?
No, I don't remember anything specific, I just tried to leave the
rerror=report case unchanged. This looks like it's moved code from
scsi_read_complete.
Kevin
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 02/16] vscsi: always use get_sense
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 16:32 ` Christoph Hellwig
2011-08-03 8:49 ` [Qemu-devel] [PATCH 03/16] scsi: pass status when completing Paolo Bonzini
` (14 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
vscsi supports autosensing by providing sense data directly in the
response. When get_sense was added, the older state machine approach
that sent REQUEST SENSE commands separately was left in place. Remove
it, all existing SCSIDevices do support autosensing and the next patches
will make the support come for free from the SCSIBus.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/spapr_vscsi.c | 91 ++++++++++++-----------------------------------------
1 files changed, 21 insertions(+), 70 deletions(-)
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 646b1e3..c65308c 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -80,7 +80,6 @@ typedef struct vscsi_req {
int active;
long data_len;
int writing;
- int sensing;
int senselen;
uint8_t sense[SCSI_SENSE_BUF_SIZE];
@@ -436,40 +435,6 @@ static int vscsi_preprocess_desc(vscsi_req *req)
return 0;
}
-static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
-{
- uint8_t *cdb = req->iu.srp.cmd.cdb;
- int n;
-
- n = scsi_req_get_sense(req->sreq, req->sense, sizeof(req->sense));
- if (n) {
- req->senselen = n;
- vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
- vscsi_put_req(req);
- return;
- }
-
- dprintf("VSCSI: Got CHECK_CONDITION, requesting sense...\n");
- cdb[0] = 3;
- cdb[1] = 0;
- cdb[2] = 0;
- cdb[3] = 0;
- cdb[4] = 96;
- cdb[5] = 0;
- req->sensing = 1;
- n = scsi_req_enqueue(req->sreq, cdb);
- dprintf("VSCSI: Queued request sense tag 0x%x\n", req->qtag);
- if (n < 0) {
- fprintf(stderr, "VSCSI: REQUEST_SENSE wants write data !?!?!?\n");
- vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
- scsi_req_abort(req->sreq, CHECK_CONDITION);
- return;
- } else if (n == 0) {
- return;
- }
- scsi_req_continue(req->sreq);
-}
-
/* Callback to indicate that the SCSI layer has completed a transfer. */
static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
{
@@ -485,23 +450,6 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
return;
}
- if (req->sensing) {
- uint8_t *buf = scsi_req_get_buf(sreq);
-
- len = MIN(len, SCSI_SENSE_BUF_SIZE);
- dprintf("VSCSI: Sense data, %d bytes:\n", len);
- dprintf(" %02x %02x %02x %02x %02x %02x %02x %02x\n",
- buf[0], buf[1], buf[2], buf[3],
- buf[4], buf[5], buf[6], buf[7]);
- dprintf(" %02x %02x %02x %02x %02x %02x %02x %02x\n",
- buf[8], buf[9], buf[10], buf[11],
- buf[12], buf[13], buf[14], buf[15]);
- memcpy(req->sense, buf, len);
- req->senselen = len;
- scsi_req_continue(req->sreq);
- return;
- }
-
if (len) {
buf = scsi_req_get_buf(sreq);
rc = vscsi_srp_transfer_data(s, req, req->writing, buf, len);
@@ -532,28 +480,31 @@ static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status)
return;
}
- if (!req->sensing && status == CHECK_CONDITION) {
- vscsi_send_request_sense(s, req);
- return;
+ if (status == CHECK_CONDITION) {
+ req->senselen = scsi_req_get_sense(req->sreq, req->sense,
+ sizeof(req->sense));
+ status = 0;
+ dprintf("VSCSI: Sense data, %d bytes:\n", len);
+ dprintf(" %02x %02x %02x %02x %02x %02x %02x %02x\n",
+ req->sense[0], req->sense[1], req->sense[2], req->sense[3],
+ req->sense[4], req->sense[5], req->sense[6], req->sense[7]);
+ dprintf(" %02x %02x %02x %02x %02x %02x %02x %02x\n",
+ req->sense[8], req->sense[9], req->sense[10], req->sense[11],
+ req->sense[12], req->sense[13], req->sense[14], req->sense[15]);
}
- if (req->sensing) {
- dprintf("VSCSI: Sense done !\n");
- status = CHECK_CONDITION;
- } else {
- dprintf("VSCSI: Command complete err=%d\n", status);
- if (status == 0) {
- /* We handle overflows, not underflows for normal commands,
- * but hopefully nobody cares
- */
- if (req->writing) {
- res_out = req->data_len;
- } else {
- res_in = req->data_len;
- }
+ dprintf("VSCSI: Command complete err=%d\n", status);
+ if (status == 0) {
+ /* We handle overflows, not underflows for normal commands,
+ * but hopefully nobody cares
+ */
+ if (req->writing) {
+ res_out = req->data_len;
+ } else {
+ res_in = req->data_len;
}
}
- vscsi_send_rsp(s, req, 0, res_in, res_out);
+ vscsi_send_rsp(s, req, status, res_in, res_out);
vscsi_put_req(req);
}
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH 02/16] vscsi: always use get_sense
2011-08-03 8:49 ` [Qemu-devel] [PATCH 02/16] vscsi: always use get_sense Paolo Bonzini
@ 2011-08-03 16:32 ` Christoph Hellwig
0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2011-08-03 16:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Wed, Aug 03, 2011 at 10:49:05AM +0200, Paolo Bonzini wrote:
> vscsi supports autosensing by providing sense data directly in the
> response. When get_sense was added, the older state machine approach
> that sent REQUEST SENSE commands separately was left in place. Remove
> it, all existing SCSIDevices do support autosensing and the next patches
> will make the support come for free from the SCSIBus.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 03/16] scsi: pass status when completing
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 01/16] scsi-disk: no need to call scsi_req_data on a short read Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 02/16] vscsi: always use get_sense Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 16:34 ` Christoph Hellwig
2011-08-03 8:49 ` [Qemu-devel] [PATCH 04/16] scsi: move sense handling to generic code Paolo Bonzini
` (13 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
A small improvement in the SCSI request API. Pass the status
at the time the request is completed, so that we can assert that
no request is completed twice. This would have detected the
problem fixed in the previous patch.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 8 ++++----
hw/scsi-disk.c | 15 ++++-----------
hw/scsi-generic.c | 31 ++++++++++++++++---------------
hw/scsi.h | 2 +-
4 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 8b1a412..bff2f5b 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -672,9 +672,10 @@ void scsi_req_print(SCSIRequest *req)
}
}
-void scsi_req_complete(SCSIRequest *req)
+void scsi_req_complete(SCSIRequest *req, int status)
{
- assert(req->status != -1);
+ assert(req->status == -1);
+ req->status = status;
scsi_req_ref(req);
scsi_req_dequeue(req);
req->bus->ops->complete(req, req->status);
@@ -696,11 +697,10 @@ void scsi_req_cancel(SCSIRequest *req)
void scsi_req_abort(SCSIRequest *req, int status)
{
- req->status = status;
if (req->dev && req->dev->info->cancel_io) {
req->dev->info->cancel_io(req);
}
- scsi_req_complete(req);
+ scsi_req_complete(req, status);
}
void scsi_device_purge_requests(SCSIDevice *sdev)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 814bf74..8beaebf 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -105,21 +105,15 @@ static void scsi_disk_clear_sense(SCSIDiskState *s)
memset(&s->sense, 0, sizeof(s->sense));
}
-static void scsi_req_set_status(SCSIDiskReq *r, int status, SCSISense sense)
-{
- SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-
- r->req.status = status;
- s->sense = sense;
-}
-
/* Helper function for command completion. */
static void scsi_command_complete(SCSIDiskReq *r, int status, SCSISense sense)
{
+ SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
DPRINTF("Command complete tag=0x%x status=%d sense=%d/%d/%d\n",
r->req.tag, status, sense.key, sense.asc, sense.ascq);
- scsi_req_set_status(r, status, sense);
- scsi_req_complete(&r->req);
+ s->sense = sense;
+ scsi_req_complete(&r->req, status);
}
/* Cancel a pending data transfer. */
@@ -979,7 +973,6 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
return -1;
}
- scsi_req_set_status(r, GOOD, SENSE_CODE(NO_SENSE));
return buflen;
not_ready:
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 63361b3..f119f33 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -115,6 +115,7 @@ static void scsi_free_request(SCSIRequest *req)
/* Helper function for command completion. */
static void scsi_command_complete(void *opaque, int ret)
{
+ int status;
SCSIGenericReq *r = (SCSIGenericReq *)opaque;
SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r->req.dev);
@@ -126,36 +127,37 @@ static void scsi_command_complete(void *opaque, int ret)
if (ret != 0) {
switch (ret) {
case -EDOM:
- r->req.status = TASK_SET_FULL;
+ status = TASK_SET_FULL;
break;
case -EINVAL:
- r->req.status = CHECK_CONDITION;
+ status = CHECK_CONDITION;
scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
break;
case -ENOMEM:
- r->req.status = CHECK_CONDITION;
+ status = CHECK_CONDITION;
scsi_set_sense(s, SENSE_CODE(TARGET_FAILURE));
break;
default:
- r->req.status = CHECK_CONDITION;
+ status = CHECK_CONDITION;
scsi_set_sense(s, SENSE_CODE(IO_ERROR));
break;
}
} else {
if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) {
- r->req.status = BUSY;
+ status = BUSY;
BADF("Driver Timeout\n");
- } else if (r->io_header.status)
- r->req.status = r->io_header.status;
- else if (s->driver_status & SG_ERR_DRIVER_SENSE)
- r->req.status = CHECK_CONDITION;
- else
- r->req.status = GOOD;
+ } else if (r->io_header.status) {
+ status = r->io_header.status;
+ } else if (s->driver_status & SG_ERR_DRIVER_SENSE) {
+ status = CHECK_CONDITION;
+ } else {
+ status = GOOD;
+ }
}
DPRINTF("Command complete 0x%p tag=0x%x status=%d\n",
- r, r->req.tag, r->req.status);
+ r, r->req.tag, status);
- scsi_req_complete(&r->req);
+ scsi_req_complete(&r->req, status);
}
/* Cancel a pending data transfer. */
@@ -341,8 +343,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
if (cmd[0] != REQUEST_SENSE && req->lun != s->lun) {
DPRINTF("Unimplemented LUN %d\n", req->lun);
scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
- r->req.status = CHECK_CONDITION;
- scsi_req_complete(&r->req);
+ scsi_req_complete(&r->req, CHECK_CONDITION);
return 0;
}
diff --git a/hw/scsi.h b/hw/scsi.h
index 6b15bbc..18d3643 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -153,7 +153,7 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
void scsi_req_print(SCSIRequest *req);
void scsi_req_continue(SCSIRequest *req);
void scsi_req_data(SCSIRequest *req, int len);
-void scsi_req_complete(SCSIRequest *req);
+void scsi_req_complete(SCSIRequest *req, int status);
uint8_t *scsi_req_get_buf(SCSIRequest *req);
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
void scsi_req_abort(SCSIRequest *req, int status);
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 04/16] scsi: move sense handling to generic code
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (2 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 03/16] scsi: pass status when completing Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 16:34 ` Christoph Hellwig
2011-08-03 8:49 ` [Qemu-devel] [PATCH 05/16] scsi: introduce SCSIReqOps Paolo Bonzini
` (12 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
With this patch, sense data is stored in the generic data structures
for SCSI devices and requests. The SCSI layer takes care of storing
sense data in the SCSIDevice for the subsequent REQUEST SENSE command.
At the same time, get_sense is removed and scsi_req_get_sense can use
an entirely generic implementation.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 79 +++++++++++++++++++++++++++++++++++++++++++++-------
hw/scsi-disk.c | 69 +++++++++++++++-------------------------------
hw/scsi-generic.c | 76 +++++++++++---------------------------------------
hw/scsi.h | 10 +++++-
trace-events | 1 +
5 files changed, 117 insertions(+), 118 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index bff2f5b..067a615 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -7,6 +7,8 @@
#include "trace.h"
static char *scsibus_get_fw_dev_path(DeviceState *dev);
+static int scsi_build_sense(uint8_t *in_buf, int in_len,
+ uint8_t *buf, int len, bool fixed);
static struct BusInfo scsi_bus_info = {
.name = "SCSI",
@@ -144,6 +146,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
req->lun = lun;
req->hba_private = hba_private;
req->status = -1;
+ req->sense_len = 0;
trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
return req;
}
@@ -161,11 +164,28 @@ uint8_t *scsi_req_get_buf(SCSIRequest *req)
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len)
{
- if (req->dev->info->get_sense) {
- return req->dev->info->get_sense(req, buf, len);
- } else {
+ assert(len >= 14);
+ if (!req->sense_len) {
return 0;
}
+ return scsi_build_sense(req->sense, req->sense_len, buf, len, true);
+}
+
+int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed)
+{
+ return scsi_build_sense(dev->sense, dev->sense_len, buf, len, fixed);
+}
+
+void scsi_req_build_sense(SCSIRequest *req, SCSISense sense)
+{
+ trace_scsi_req_build_sense(req->dev->id, req->lun, req->tag,
+ sense.key, sense.asc, sense.ascq);
+ memset(req->sense, 0, 18);
+ req->sense[0] = 0xf0;
+ req->sense[2] = sense.key;
+ req->sense[12] = sense.asc;
+ req->sense[13] = sense.ascq;
+ req->sense_len = 18;
}
int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
@@ -486,14 +506,40 @@ const struct SCSISense sense_code_LUN_FAILURE = {
/*
* scsi_build_sense
*
- * Build a sense buffer
+ * Convert between fixed and descriptor sense buffers
*/
-int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed)
+int scsi_build_sense(uint8_t *in_buf, int in_len,
+ uint8_t *buf, int len, bool fixed)
{
+ bool fixed_in;
+ SCSISense sense;
if (!fixed && len < 8) {
return 0;
}
+ if (in_len == 0) {
+ sense.key = NO_SENSE;
+ sense.asc = 0;
+ sense.ascq = 0;
+ } else {
+ fixed_in = (in_buf[0] & 2) == 0;
+
+ if (fixed == fixed_in) {
+ memcpy(buf, in_buf, MIN(len, in_len));
+ return MIN(len, in_len);
+ }
+
+ if (fixed_in) {
+ sense.key = in_buf[2];
+ sense.asc = in_buf[12];
+ sense.ascq = in_buf[13];
+ } else {
+ sense.key = in_buf[1];
+ sense.asc = in_buf[2];
+ sense.ascq = in_buf[3];
+ }
+ }
+
memset(buf, 0, len);
if (fixed) {
/* Return fixed format sense buffer */
@@ -676,6 +722,17 @@ void scsi_req_complete(SCSIRequest *req, int status)
{
assert(req->status == -1);
req->status = status;
+
+ assert(req->sense_len < sizeof(req->sense));
+ if (status == GOOD) {
+ req->sense_len = 0;
+ }
+
+ if (req->sense_len) {
+ memcpy(req->dev->sense, req->sense, req->sense_len);
+ }
+ req->dev->sense_len = req->sense_len;
+
scsi_req_ref(req);
scsi_req_dequeue(req);
req->bus->ops->complete(req, req->status);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 8beaebf..4c1e58f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -73,7 +73,6 @@ struct SCSIDiskState
QEMUBH *bh;
char *version;
char *serial;
- SCSISense sense;
SCSIDriveKind drive_kind;
};
@@ -100,20 +99,13 @@ static void scsi_free_request(SCSIRequest *req)
qemu_vfree(r->iov.iov_base);
}
-static void scsi_disk_clear_sense(SCSIDiskState *s)
+/* Helper function for command completion with sense. */
+static void scsi_check_condition(SCSIDiskReq *r, SCSISense sense)
{
- memset(&s->sense, 0, sizeof(s->sense));
-}
-
-/* Helper function for command completion. */
-static void scsi_command_complete(SCSIDiskReq *r, int status, SCSISense sense)
-{
- SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-
DPRINTF("Command complete tag=0x%x status=%d sense=%d/%d/%d\n",
r->req.tag, status, sense.key, sense.asc, sense.ascq);
- s->sense = sense;
- scsi_req_complete(&r->req, status);
+ scsi_req_build_sense(&r->req, sense);
+ scsi_req_complete(&r->req, CHECK_CONDITION);
}
/* Cancel a pending data transfer. */
@@ -165,7 +157,8 @@ static void scsi_read_data(SCSIRequest *req)
}
DPRINTF("Read sector_count=%d\n", r->sector_count);
if (r->sector_count == 0) {
- scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+ /* This also clears the sense buffer for REQUEST SENSE. */
+ scsi_req_complete(&r->req, GOOD);
return;
}
@@ -213,16 +206,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
} else {
switch (error) {
case ENOMEM:
- scsi_command_complete(r, CHECK_CONDITION,
- SENSE_CODE(TARGET_FAILURE));
+ scsi_check_condition(r, SENSE_CODE(TARGET_FAILURE));
break;
case EINVAL:
- scsi_command_complete(r, CHECK_CONDITION,
- SENSE_CODE(INVALID_FIELD));
+ scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
break;
default:
- scsi_command_complete(r, CHECK_CONDITION,
- SENSE_CODE(IO_ERROR));
+ scsi_check_condition(r, SENSE_CODE(IO_ERROR));
break;
}
bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
@@ -248,7 +238,7 @@ static void scsi_write_complete(void * opaque, int ret)
r->sector += n;
r->sector_count -= n;
if (r->sector_count == 0) {
- scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+ scsi_req_complete(&r->req, GOOD);
} else {
len = r->sector_count * 512;
if (len > SCSI_DMA_BUF_SIZE) {
@@ -317,7 +307,7 @@ static void scsi_dma_restart_bh(void *opaque)
case SCSI_REQ_STATUS_RETRY_FLUSH:
ret = scsi_disk_emulate_command(r, r->iov.iov_base);
if (ret == 0) {
- scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+ scsi_req_complete(&r->req, GOOD);
}
}
}
@@ -345,14 +335,6 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)
return (uint8_t *)r->iov.iov_base;
}
-/* Copy sense information into the provided buffer */
-static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
-{
- SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
-
- return scsi_build_sense(s->sense, outbuf, len, len > 14);
-}
-
static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
@@ -831,9 +813,8 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
case REQUEST_SENSE:
if (req->cmd.xfer < 4)
goto illegal_request;
- buflen = scsi_build_sense(s->sense, outbuf, req->cmd.xfer,
- req->cmd.xfer > 13);
- scsi_disk_clear_sense(s);
+ buflen = scsi_device_get_sense(&s->qdev, outbuf, req->cmd.xfer,
+ (req->cmd.buf[1] & 1) == 0);
break;
case INQUIRY:
buflen = scsi_disk_emulate_inquiry(req, outbuf);
@@ -970,21 +951,21 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
}
break;
default:
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+ scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
return -1;
}
return buflen;
not_ready:
if (!bdrv_is_inserted(s->bs)) {
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(NO_MEDIUM));
+ scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
} else {
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LUN_NOT_READY));
+ scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY));
}
return -1;
illegal_request:
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
+ scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
return -1;
}
@@ -1008,7 +989,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
if (scsi_req_parse(&r->req, buf) != 0) {
BADF("Unsupported command length, command %x\n", command);
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+ scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
return 0;
}
#ifdef DEBUG_SCSI
@@ -1025,8 +1006,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
/* Only LUN 0 supported. */
DPRINTF("Unimplemented LUN %d\n", req->lun);
if (command != REQUEST_SENSE && command != INQUIRY) {
- scsi_command_complete(r, CHECK_CONDITION,
- SENSE_CODE(LUN_NOT_SUPPORTED));
+ scsi_check_condition(r, SENSE_CODE(LUN_NOT_SUPPORTED));
return 0;
}
}
@@ -1135,17 +1115,17 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
break;
default:
DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+ scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
return 0;
fail:
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
+ scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
return 0;
illegal_lba:
- scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LBA_OUT_OF_RANGE));
+ scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
return 0;
}
if (r->sector_count == 0 && r->iov.iov_len == 0) {
- scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+ scsi_req_complete(&r->req, GOOD);
}
len = r->sector_count * 512 + r->iov.iov_len;
if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
@@ -1275,7 +1255,6 @@ static SCSIDeviceInfo scsi_disk_info[] = {
.write_data = scsi_write_data,
.cancel_io = scsi_cancel_io,
.get_buf = scsi_get_buf,
- .get_sense = scsi_get_sense,
.qdev.props = (Property[]) {
DEFINE_SCSI_DISK_PROPERTIES(),
DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),
@@ -1296,7 +1275,6 @@ static SCSIDeviceInfo scsi_disk_info[] = {
.write_data = scsi_write_data,
.cancel_io = scsi_cancel_io,
.get_buf = scsi_get_buf,
- .get_sense = scsi_get_sense,
.qdev.props = (Property[]) {
DEFINE_SCSI_DISK_PROPERTIES(),
DEFINE_PROP_END_OF_LIST(),
@@ -1316,7 +1294,6 @@ static SCSIDeviceInfo scsi_disk_info[] = {
.write_data = scsi_write_data,
.cancel_io = scsi_cancel_io,
.get_buf = scsi_get_buf,
- .get_sense = scsi_get_sense,
.qdev.props = (Property[]) {
DEFINE_SCSI_DISK_PROPERTIES(),
DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index f119f33..fc7cf01 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -61,41 +61,8 @@ struct SCSIGenericState
SCSIDevice qdev;
BlockDriverState *bs;
int lun;
- int driver_status;
- uint8_t sensebuf[SCSI_SENSE_BUF_SIZE];
- uint8_t senselen;
};
-static void scsi_set_sense(SCSIGenericState *s, SCSISense sense)
-{
- s->senselen = scsi_build_sense(sense, s->sensebuf, SCSI_SENSE_BUF_SIZE, 0);
- s->driver_status = SG_ERR_DRIVER_SENSE;
-}
-
-static void scsi_clear_sense(SCSIGenericState *s)
-{
- memset(s->sensebuf, 0, SCSI_SENSE_BUF_SIZE);
- s->senselen = 0;
- s->driver_status = 0;
-}
-
-static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
-{
- SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
- int size = SCSI_SENSE_BUF_SIZE;
-
- if (!(s->driver_status & SG_ERR_DRIVER_SENSE)) {
- size = scsi_build_sense(SENSE_CODE(NO_SENSE), s->sensebuf,
- SCSI_SENSE_BUF_SIZE, 0);
- }
- if (size > len) {
- size = len;
- }
- memcpy(outbuf, s->sensebuf, size);
-
- return size;
-}
-
static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
void *hba_private)
{
@@ -117,12 +84,10 @@ static void scsi_command_complete(void *opaque, int ret)
{
int status;
SCSIGenericReq *r = (SCSIGenericReq *)opaque;
- SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r->req.dev);
r->req.aiocb = NULL;
- s->driver_status = r->io_header.driver_status;
- if (s->driver_status & SG_ERR_DRIVER_SENSE)
- s->senselen = r->io_header.sb_len_wr;
+ if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE)
+ r->req.sense_len = r->io_header.sb_len_wr;
if (ret != 0) {
switch (ret) {
@@ -131,24 +96,24 @@ static void scsi_command_complete(void *opaque, int ret)
break;
case -EINVAL:
status = CHECK_CONDITION;
- scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
+ scsi_req_build_sense(&r->req, SENSE_CODE(INVALID_FIELD));
break;
case -ENOMEM:
status = CHECK_CONDITION;
- scsi_set_sense(s, SENSE_CODE(TARGET_FAILURE));
+ scsi_req_build_sense(&r->req, SENSE_CODE(TARGET_FAILURE));
break;
default:
status = CHECK_CONDITION;
- scsi_set_sense(s, SENSE_CODE(IO_ERROR));
+ scsi_req_build_sense(&r->req, SENSE_CODE(IO_ERROR));
break;
}
} else {
- if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) {
+ if (r->io_header.driver_status & SG_ERR_DRIVER_TIMEOUT) {
status = BUSY;
BADF("Driver Timeout\n");
} else if (r->io_header.status) {
status = r->io_header.status;
- } else if (s->driver_status & SG_ERR_DRIVER_SENSE) {
+ } else if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
status = CHECK_CONDITION;
} else {
status = GOOD;
@@ -176,16 +141,14 @@ static int execute_command(BlockDriverState *bdrv,
SCSIGenericReq *r, int direction,
BlockDriverCompletionFunc *complete)
{
- SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r->req.dev);
-
r->io_header.interface_id = 'S';
r->io_header.dxfer_direction = direction;
r->io_header.dxferp = r->buf;
r->io_header.dxfer_len = r->buflen;
r->io_header.cmdp = r->req.cmd.buf;
r->io_header.cmd_len = r->req.cmd.len;
- r->io_header.mx_sb_len = sizeof(s->sensebuf);
- r->io_header.sbp = s->sensebuf;
+ r->io_header.mx_sb_len = sizeof(r->req.sense);
+ r->io_header.sbp = r->req.sense;
r->io_header.timeout = MAX_UINT;
r->io_header.usr_ptr = r;
r->io_header.flags |= SG_FLAG_DIRECT_IO;
@@ -234,21 +197,19 @@ static void scsi_read_data(SCSIRequest *req)
return;
}
- if (r->req.cmd.buf[0] == REQUEST_SENSE && s->driver_status & SG_ERR_DRIVER_SENSE)
- {
- s->senselen = MIN(r->len, s->senselen);
- memcpy(r->buf, s->sensebuf, s->senselen);
+ if (r->req.cmd.buf[0] == REQUEST_SENSE) {
r->io_header.driver_status = 0;
r->io_header.status = 0;
- r->io_header.dxfer_len = s->senselen;
+ r->io_header.dxfer_len =
+ scsi_device_get_sense(&s->qdev, r->buf, r->req.cmd.xfer,
+ (r->req.cmd.buf[1] & 1) == 0);
r->len = -1;
- DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, s->senselen);
+ DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, r->io_header.dxfer_len);
DPRINTF("Sense: %d %d %d %d %d %d %d %d\n",
r->buf[0], r->buf[1], r->buf[2], r->buf[3],
r->buf[4], r->buf[5], r->buf[6], r->buf[7]);
- scsi_req_data(&r->req, s->senselen);
- /* Clear sensebuf after REQUEST_SENSE */
- scsi_clear_sense(s);
+ scsi_req_data(&r->req, r->io_header.dxfer_len);
+ /* The sense buffer is cleared when we return GOOD */
return;
}
@@ -342,7 +303,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
if (cmd[0] != REQUEST_SENSE && req->lun != s->lun) {
DPRINTF("Unimplemented LUN %d\n", req->lun);
- scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
+ scsi_req_build_sense(&r->req, SENSE_CODE(LUN_NOT_SUPPORTED));
scsi_req_complete(&r->req, CHECK_CONDITION);
return 0;
}
@@ -533,8 +494,6 @@ static int scsi_generic_initfn(SCSIDevice *dev)
}
}
DPRINTF("block size %d\n", s->qdev.blocksize);
- s->driver_status = 0;
- memset(s->sensebuf, 0, sizeof(s->sensebuf));
bdrv_set_removable(s->bs, 0);
return 0;
}
@@ -553,7 +512,6 @@ static SCSIDeviceInfo scsi_generic_info = {
.write_data = scsi_write_data,
.cancel_io = scsi_cancel_io,
.get_buf = scsi_get_buf,
- .get_sense = scsi_get_sense,
.qdev.props = (Property[]) {
DEFINE_BLOCK_PROPERTIES(SCSIGenericState, qdev.conf),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/scsi.h b/hw/scsi.h
index 18d3643..c1cb987 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -27,6 +27,8 @@ typedef struct SCSISense {
uint8_t ascq;
} SCSISense;
+#define SCSI_SENSE_BUF_SIZE 96
+
struct SCSIRequest {
SCSIBus *bus;
SCSIDevice *dev;
@@ -42,6 +44,8 @@ struct SCSIRequest {
enum SCSIXferMode mode;
} cmd;
BlockDriverAIOCB *aiocb;
+ uint8_t sense[SCSI_SENSE_BUF_SIZE];
+ uint32_t sense_len;
bool enqueued;
void *hba_private;
QTAILQ_ENTRY(SCSIRequest) next;
@@ -53,6 +57,8 @@ struct SCSIDevice
uint32_t id;
BlockConf conf;
SCSIDeviceInfo *info;
+ uint8_t sense[SCSI_SENSE_BUF_SIZE];
+ uint32_t sense_len;
QTAILQ_HEAD(, SCSIRequest) requests;
int blocksize;
int type;
@@ -76,7 +82,6 @@ struct SCSIDeviceInfo {
void (*write_data)(SCSIRequest *req);
void (*cancel_io)(SCSIRequest *req);
uint8_t *(*get_buf)(SCSIRequest *req);
- int (*get_sense)(SCSIRequest *req, uint8_t *buf, int len);
};
struct SCSIBusOps {
@@ -137,7 +142,6 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
#define SENSE_CODE(x) sense_code_ ## x
-int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
int scsi_sense_valid(SCSISense sense);
SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
@@ -149,6 +153,7 @@ void scsi_req_free(SCSIRequest *req);
SCSIRequest *scsi_req_ref(SCSIRequest *req);
void scsi_req_unref(SCSIRequest *req);
+void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
void scsi_req_print(SCSIRequest *req);
void scsi_req_continue(SCSIRequest *req);
@@ -159,5 +164,6 @@ int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
void scsi_req_abort(SCSIRequest *req, int status);
void scsi_req_cancel(SCSIRequest *req);
void scsi_device_purge_requests(SCSIDevice *sdev);
+int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
#endif
diff --git a/trace-events b/trace-events
index 713f042..55023b0 100644
--- a/trace-events
+++ b/trace-events
@@ -248,6 +248,7 @@ disable scsi_req_continue(int target, int lun, int tag) "target %d lun %d tag %d
disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int xfer) "target %d lun %d tag %d command %d dir %d length %d"
disable scsi_req_parsed_lba(int target, int lun, int tag, int cmd, uint64_t lba) "target %d lun %d tag %d command %d lba %"PRIu64""
disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d lun %d tag %d command %d"
+disable scsi_req_build_sense(int target, int lun, int tag, int key, int asc, int ascq) "target %d lun %d tag %d key %#02x asc %#02x ascq %#02x"
# vl.c
disable vm_state_notify(int running, int reason) "running %d reason %d"
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 05/16] scsi: introduce SCSIReqOps
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (3 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 04/16] scsi: move sense handling to generic code Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 06/16] scsi: move request-related callbacks from SCSIDeviceInfo to SCSIReqOps Paolo Bonzini
` (11 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
This will let allow requests to be dispatched through different callbacks,
either common or per-device.
This patch adjusts the API, the next one will move members to SCSIReqOps.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 5 +++--
hw/scsi-disk.c | 30 +++++++++++++++++-------------
hw/scsi-generic.c | 22 +++++++++++++---------
hw/scsi.h | 8 +++++++-
4 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 067a615..4709b19 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -133,12 +133,12 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
return res;
}
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+SCSIRequest *scsi_req_alloc(SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag,
uint32_t lun, void *hba_private)
{
SCSIRequest *req;
- req = qemu_mallocz(size);
+ req = qemu_mallocz(reqops->size);
req->refcount = 1;
req->bus = scsi_bus_from_device(d);
req->dev = d;
@@ -147,6 +147,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
req->hba_private = hba_private;
req->status = -1;
req->sense_len = 0;
+ req->ops = reqops;
trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
return req;
}
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4c1e58f..7483638 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -79,19 +79,6 @@ struct SCSIDiskState
static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
-static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
- uint32_t lun, void *hba_private)
-{
- SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
- SCSIRequest *req;
- SCSIDiskReq *r;
-
- req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun, hba_private);
- r = DO_UPCAST(SCSIDiskReq, req, req);
- r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
- return req;
-}
-
static void scsi_free_request(SCSIRequest *req)
{
SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
@@ -1234,6 +1221,23 @@ static int scsi_disk_initfn(SCSIDevice *dev)
return scsi_initfn(dev, kind);
}
+static SCSIReqOps scsi_disk_reqops = {
+ .size = sizeof(SCSIDiskReq),
+};
+
+static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
+ uint32_t lun, void *hba_private)
+{
+ SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+ SCSIRequest *req;
+ SCSIDiskReq *r;
+
+ req = scsi_req_alloc(&scsi_disk_reqops, &s->qdev, tag, lun, hba_private);
+ r = DO_UPCAST(SCSIDiskReq, req, req);
+ r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
+ return req;
+}
+
#define DEFINE_SCSI_DISK_PROPERTIES() \
DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf), \
DEFINE_PROP_STRING("ver", SCSIDiskState, version), \
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index fc7cf01..87fb6ab 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -63,15 +63,6 @@ struct SCSIGenericState
int lun;
};
-static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
- void *hba_private)
-{
- SCSIRequest *req;
-
- req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun, hba_private);
- return req;
-}
-
static void scsi_free_request(SCSIRequest *req)
{
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
@@ -498,6 +489,19 @@ static int scsi_generic_initfn(SCSIDevice *dev)
return 0;
}
+static SCSIReqOps scsi_generic_req_ops = {
+ .size = sizeof(SCSIGenericReq),
+};
+
+static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
+ void *hba_private)
+{
+ SCSIRequest *req;
+
+ req = scsi_req_alloc(&scsi_generic_req_ops, d, tag, lun, hba_private);
+ return req;
+}
+
static SCSIDeviceInfo scsi_generic_info = {
.qdev.name = "scsi-generic",
.qdev.desc = "pass through generic scsi device (/dev/sg*)",
diff --git a/hw/scsi.h b/hw/scsi.h
index c1cb987..ee76c64 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -14,6 +14,7 @@ typedef struct SCSIBusOps SCSIBusOps;
typedef struct SCSIDevice SCSIDevice;
typedef struct SCSIDeviceInfo SCSIDeviceInfo;
typedef struct SCSIRequest SCSIRequest;
+typedef struct SCSIReqOps SCSIReqOps;
enum SCSIXferMode {
SCSI_XFER_NONE, /* TEST_UNIT_READY, ... */
@@ -32,6 +33,7 @@ typedef struct SCSISense {
struct SCSIRequest {
SCSIBus *bus;
SCSIDevice *dev;
+ SCSIReqOps *ops;
uint32_t refcount;
uint32_t tag;
uint32_t lun;
@@ -69,6 +71,10 @@ int cdrom_read_toc(int nb_sectors, uint8_t *buf, int msf, int start_track);
int cdrom_read_toc_raw(int nb_sectors, uint8_t *buf, int msf, int session_num);
/* scsi-bus.c */
+struct SCSIReqOps {
+ size_t size;
+};
+
typedef int (*scsi_qdev_initfn)(SCSIDevice *dev);
struct SCSIDeviceInfo {
DeviceInfo qdev;
@@ -144,7 +150,7 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
int scsi_sense_valid(SCSISense sense);
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+SCSIRequest *scsi_req_alloc(SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag,
uint32_t lun, void *hba_private);
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
void *hba_private);
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 06/16] scsi: move request-related callbacks from SCSIDeviceInfo to SCSIReqOps
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (4 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 05/16] scsi: introduce SCSIReqOps Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 07/16] scsi: pass cdb already to scsi_req_new Paolo Bonzini
` (10 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 20 ++++++++++----------
hw/scsi-disk.c | 24 ++++++------------------
hw/scsi-generic.c | 12 ++++++------
hw/scsi.h | 13 +++++++------
4 files changed, 29 insertions(+), 40 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 4709b19..be73f29 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -160,7 +160,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
uint8_t *scsi_req_get_buf(SCSIRequest *req)
{
- return req->dev->info->get_buf(req);
+ return req->ops->get_buf(req);
}
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len)
@@ -199,7 +199,7 @@ int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
scsi_req_ref(req);
- rc = req->dev->info->send_command(req, buf);
+ rc = req->ops->send_command(req, buf);
scsi_req_unref(req);
return rc;
}
@@ -663,8 +663,8 @@ SCSIRequest *scsi_req_ref(SCSIRequest *req)
void scsi_req_unref(SCSIRequest *req)
{
if (--req->refcount == 0) {
- if (req->dev->info->free_req) {
- req->dev->info->free_req(req);
+ if (req->ops->free_req) {
+ req->ops->free_req(req);
}
qemu_free(req);
}
@@ -676,9 +676,9 @@ void scsi_req_continue(SCSIRequest *req)
{
trace_scsi_req_continue(req->dev->id, req->lun, req->tag);
if (req->cmd.mode == SCSI_XFER_TO_DEV) {
- req->dev->info->write_data(req);
+ req->ops->write_data(req);
} else {
- req->dev->info->read_data(req);
+ req->ops->read_data(req);
}
}
@@ -742,8 +742,8 @@ void scsi_req_complete(SCSIRequest *req, int status)
void scsi_req_cancel(SCSIRequest *req)
{
- if (req->dev && req->dev->info->cancel_io) {
- req->dev->info->cancel_io(req);
+ if (req->ops->cancel_io) {
+ req->ops->cancel_io(req);
}
scsi_req_ref(req);
scsi_req_dequeue(req);
@@ -755,8 +755,8 @@ void scsi_req_cancel(SCSIRequest *req)
void scsi_req_abort(SCSIRequest *req, int status)
{
- if (req->dev && req->dev->info->cancel_io) {
- req->dev->info->cancel_io(req);
+ if (req->ops->cancel_io) {
+ req->ops->cancel_io(req);
}
scsi_req_complete(req, status);
}
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 7483638..ceb0e0a 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1223,6 +1223,12 @@ static int scsi_disk_initfn(SCSIDevice *dev)
static SCSIReqOps scsi_disk_reqops = {
.size = sizeof(SCSIDiskReq),
+ .free_req = scsi_free_request,
+ .send_command = scsi_send_command,
+ .read_data = scsi_read_data,
+ .write_data = scsi_write_data,
+ .cancel_io = scsi_cancel_io,
+ .get_buf = scsi_get_buf,
};
static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
@@ -1253,12 +1259,6 @@ static SCSIDeviceInfo scsi_disk_info[] = {
.init = scsi_hd_initfn,
.destroy = scsi_destroy,
.alloc_req = scsi_new_request,
- .free_req = scsi_free_request,
- .send_command = scsi_send_command,
- .read_data = scsi_read_data,
- .write_data = scsi_write_data,
- .cancel_io = scsi_cancel_io,
- .get_buf = scsi_get_buf,
.qdev.props = (Property[]) {
DEFINE_SCSI_DISK_PROPERTIES(),
DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),
@@ -1273,12 +1273,6 @@ static SCSIDeviceInfo scsi_disk_info[] = {
.init = scsi_cd_initfn,
.destroy = scsi_destroy,
.alloc_req = scsi_new_request,
- .free_req = scsi_free_request,
- .send_command = scsi_send_command,
- .read_data = scsi_read_data,
- .write_data = scsi_write_data,
- .cancel_io = scsi_cancel_io,
- .get_buf = scsi_get_buf,
.qdev.props = (Property[]) {
DEFINE_SCSI_DISK_PROPERTIES(),
DEFINE_PROP_END_OF_LIST(),
@@ -1292,12 +1286,6 @@ static SCSIDeviceInfo scsi_disk_info[] = {
.init = scsi_disk_initfn,
.destroy = scsi_destroy,
.alloc_req = scsi_new_request,
- .free_req = scsi_free_request,
- .send_command = scsi_send_command,
- .read_data = scsi_read_data,
- .write_data = scsi_write_data,
- .cancel_io = scsi_cancel_io,
- .get_buf = scsi_get_buf,
.qdev.props = (Property[]) {
DEFINE_SCSI_DISK_PROPERTIES(),
DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 87fb6ab..453a295 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -491,6 +491,12 @@ static int scsi_generic_initfn(SCSIDevice *dev)
static SCSIReqOps scsi_generic_req_ops = {
.size = sizeof(SCSIGenericReq),
+ .free_req = scsi_free_request,
+ .send_command = scsi_send_command,
+ .read_data = scsi_read_data,
+ .write_data = scsi_write_data,
+ .cancel_io = scsi_cancel_io,
+ .get_buf = scsi_get_buf,
};
static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
@@ -510,12 +516,6 @@ static SCSIDeviceInfo scsi_generic_info = {
.init = scsi_generic_initfn,
.destroy = scsi_destroy,
.alloc_req = scsi_new_request,
- .free_req = scsi_free_request,
- .send_command = scsi_send_command,
- .read_data = scsi_read_data,
- .write_data = scsi_write_data,
- .cancel_io = scsi_cancel_io,
- .get_buf = scsi_get_buf,
.qdev.props = (Property[]) {
DEFINE_BLOCK_PROPERTIES(SCSIGenericState, qdev.conf),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/scsi.h b/hw/scsi.h
index ee76c64..5c0e076 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -73,6 +73,12 @@ int cdrom_read_toc_raw(int nb_sectors, uint8_t *buf, int msf, int session_num);
/* scsi-bus.c */
struct SCSIReqOps {
size_t size;
+ void (*free_req)(SCSIRequest *req);
+ int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
+ void (*read_data)(SCSIRequest *req);
+ void (*write_data)(SCSIRequest *req);
+ void (*cancel_io)(SCSIRequest *req);
+ uint8_t *(*get_buf)(SCSIRequest *req);
};
typedef int (*scsi_qdev_initfn)(SCSIDevice *dev);
@@ -82,12 +88,7 @@ struct SCSIDeviceInfo {
void (*destroy)(SCSIDevice *s);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
void *hba_private);
- void (*free_req)(SCSIRequest *req);
- int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
- void (*read_data)(SCSIRequest *req);
- void (*write_data)(SCSIRequest *req);
- void (*cancel_io)(SCSIRequest *req);
- uint8_t *(*get_buf)(SCSIRequest *req);
+ SCSIReqOps reqops;
};
struct SCSIBusOps {
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 07/16] scsi: pass cdb already to scsi_req_new
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (5 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 06/16] scsi: move request-related callbacks from SCSIDeviceInfo to SCSIReqOps Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 08/16] scsi: introduce SCSICommand Paolo Bonzini
` (9 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Right now the CDB is not passed to the SCSIBus until scsi_req_enqueue.
Passing it to scsi_req_new will let scsi_req_new dispatch common requests
through different reqops.
Moving the memcpy to scsi_req_new is a hack that will go away as
soon as scsi_req_new will also take care of the parsing.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/esp.c | 4 ++--
hw/lsi53c895a.c | 4 ++--
hw/scsi-bus.c | 13 ++++++++-----
hw/scsi.h | 4 ++--
hw/spapr_vscsi.c | 4 ++--
hw/usb-msd.c | 4 ++--
6 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 9ddd637..be3a35d 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,8 +244,8 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
lun = busid & 7;
- s->current_req = scsi_req_new(s->current_dev, 0, lun, NULL);
- datalen = scsi_req_enqueue(s->current_req, buf);
+ s->current_req = scsi_req_new(s->current_dev, 0, lun, buf, NULL);
+ datalen = scsi_req_enqueue(s->current_req);
s->ti_size = datalen;
if (datalen != 0) {
s->rregs[ESP_RSTAT] = STAT_TC;
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index e9904c4..dac176a 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -782,10 +782,10 @@ static void lsi_do_command(LSIState *s)
assert(s->current == NULL);
s->current = qemu_mallocz(sizeof(lsi_request));
s->current->tag = s->select_tag;
- s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun,
+ s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun, buf,
s->current);
- n = scsi_req_enqueue(s->current->req, buf);
+ n = scsi_req_enqueue(s->current->req);
if (n) {
if (n > 0) {
lsi_set_phase(s, PHASE_DI);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index be73f29..9b81b9c 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -153,9 +153,12 @@ SCSIRequest *scsi_req_alloc(SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag,
}
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
- void *hba_private)
+ uint8_t *buf, void *hba_private)
{
- return d->info->alloc_req(d, tag, lun, hba_private);
+ SCSIRequest *req;
+ req = d->info->alloc_req(d, tag, lun, hba_private);
+ memcpy(req->cmd.buf, buf, 16);
+ return req;
}
uint8_t *scsi_req_get_buf(SCSIRequest *req)
@@ -189,7 +192,7 @@ void scsi_req_build_sense(SCSIRequest *req, SCSISense sense)
req->sense_len = 18;
}
-int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
+int32_t scsi_req_enqueue(SCSIRequest *req)
{
int32_t rc;
@@ -199,7 +202,7 @@ int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
scsi_req_ref(req);
- rc = req->ops->send_command(req, buf);
+ rc = req->ops->send_command(req, req->cmd.buf);
scsi_req_unref(req);
return rc;
}
@@ -433,7 +436,7 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
if (rc != 0)
return rc;
- memcpy(req->cmd.buf, buf, req->cmd.len);
+ assert(buf == req->cmd.buf);
scsi_req_xfer_mode(req);
req->cmd.lba = scsi_req_lba(req);
trace_scsi_req_parsed(req->dev->id, req->lun, req->tag, buf[0],
diff --git a/hw/scsi.h b/hw/scsi.h
index 5c0e076..7ed7550 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -154,8 +154,8 @@ int scsi_sense_valid(SCSISense sense);
SCSIRequest *scsi_req_alloc(SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag,
uint32_t lun, void *hba_private);
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
- void *hba_private);
-int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
+ uint8_t *buf, void *hba_private);
+int32_t scsi_req_enqueue(SCSIRequest *req);
void scsi_req_free(SCSIRequest *req);
SCSIRequest *scsi_req_ref(SCSIRequest *req);
void scsi_req_unref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index c65308c..d98d1fd 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -600,8 +600,8 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
}
req->lun = lun;
- req->sreq = scsi_req_new(sdev, req->qtag, lun, req);
- n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
+ req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, req);
+ n = scsi_req_enqueue(req->sreq);
dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
req->qtag, srp->cmd.cdb[0], id, lun, n);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index cdeac58..63305b8 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -380,8 +380,8 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
s->tag, cbw.flags, cbw.cmd_len, s->data_len);
s->residue = 0;
s->scsi_len = 0;
- s->req = scsi_req_new(s->scsi_dev, s->tag, 0, NULL);
- scsi_req_enqueue(s->req, cbw.cmd);
+ s->req = scsi_req_new(s->scsi_dev, s->tag, 0, cbw.cmd, NULL);
+ scsi_req_enqueue(s->req);
/* ??? Should check that USB and SCSI data transfer
directions match. */
if (s->mode != USB_MSDM_CSW && s->residue == 0) {
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 08/16] scsi: introduce SCSICommand
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (6 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 07/16] scsi: pass cdb already to scsi_req_new Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 09/16] scsi: push lun field to SCSIDevice Paolo Bonzini
` (8 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
This struct is currently unnamed. Give it a name and use it
explicitly to decouple (some parts of) CDB parsing from
SCSIRequest.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 95 +++++++++++++++++++++++++++++----------------------------
hw/scsi.h | 17 ++++++----
2 files changed, 58 insertions(+), 54 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 9b81b9c..579f6b4 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -217,35 +217,35 @@ static void scsi_req_dequeue(SCSIRequest *req)
}
}
-static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
+static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
{
- switch (cmd[0] >> 5) {
+ switch (buf[0] >> 5) {
case 0:
- req->cmd.xfer = cmd[4];
- req->cmd.len = 6;
+ cmd->xfer = buf[4];
+ cmd->len = 6;
/* length 0 means 256 blocks */
- if (req->cmd.xfer == 0)
- req->cmd.xfer = 256;
+ if (cmd->xfer == 0) {
+ cmd->xfer = 256;
+ }
break;
case 1:
case 2:
- req->cmd.xfer = cmd[8] | (cmd[7] << 8);
- req->cmd.len = 10;
+ cmd->xfer = buf[8] | (buf[7] << 8);
+ cmd->len = 10;
break;
case 4:
- req->cmd.xfer = cmd[13] | (cmd[12] << 8) | (cmd[11] << 16) | (cmd[10] << 24);
- req->cmd.len = 16;
+ cmd->xfer = buf[13] | (buf[12] << 8) | (buf[11] << 16) | (buf[10] << 24);
+ cmd->len = 16;
break;
case 5:
- req->cmd.xfer = cmd[9] | (cmd[8] << 8) | (cmd[7] << 16) | (cmd[6] << 24);
- req->cmd.len = 12;
+ cmd->xfer = buf[9] | (buf[8] << 8) | (buf[7] << 16) | (buf[6] << 24);
+ cmd->len = 12;
break;
default:
- trace_scsi_req_parse_bad(req->dev->id, req->lun, req->tag, cmd[0]);
return -1;
}
- switch(cmd[0]) {
+ switch (buf[0]) {
case TEST_UNIT_READY:
case REZERO_UNIT:
case START_STOP:
@@ -266,27 +266,27 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
case WRITE_LONG:
case MOVE_MEDIUM:
case UPDATE_BLOCK:
- req->cmd.xfer = 0;
+ cmd->xfer = 0;
break;
case MODE_SENSE:
break;
case WRITE_SAME:
- req->cmd.xfer = 1;
+ cmd->xfer = 1;
break;
case READ_CAPACITY:
- req->cmd.xfer = 8;
+ cmd->xfer = 8;
break;
case READ_BLOCK_LIMITS:
- req->cmd.xfer = 6;
+ cmd->xfer = 6;
break;
case READ_POSITION:
- req->cmd.xfer = 20;
+ cmd->xfer = 20;
break;
case SEND_VOLUME_TAG:
- req->cmd.xfer *= 40;
+ cmd->xfer *= 40;
break;
case MEDIUM_SCAN:
- req->cmd.xfer *= 8;
+ cmd->xfer *= 8;
break;
case WRITE_10:
case WRITE_VERIFY:
@@ -295,7 +295,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
case WRITE_VERIFY_12:
case WRITE_16:
case WRITE_VERIFY_16:
- req->cmd.xfer *= req->dev->blocksize;
+ cmd->xfer *= dev->blocksize;
break;
case READ_10:
case READ_6:
@@ -303,50 +303,51 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
case RECOVER_BUFFERED_DATA:
case READ_12:
case READ_16:
- req->cmd.xfer *= req->dev->blocksize;
+ cmd->xfer *= dev->blocksize;
break;
case INQUIRY:
- req->cmd.xfer = cmd[4] | (cmd[3] << 8);
+ cmd->xfer = buf[4] | (buf[3] << 8);
break;
case MAINTENANCE_OUT:
case MAINTENANCE_IN:
- if (req->dev->type == TYPE_ROM) {
+ if (dev->type == TYPE_ROM) {
/* GPCMD_REPORT_KEY and GPCMD_SEND_KEY from multi media commands */
- req->cmd.xfer = cmd[9] | (cmd[8] << 8);
+ cmd->xfer = buf[9] | (buf[8] << 8);
}
break;
}
return 0;
}
-static int scsi_req_stream_length(SCSIRequest *req, uint8_t *cmd)
+static int scsi_req_stream_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
{
- switch(cmd[0]) {
+ switch (buf[0]) {
/* stream commands */
case READ_6:
case READ_REVERSE:
case RECOVER_BUFFERED_DATA:
case WRITE_6:
- req->cmd.len = 6;
- req->cmd.xfer = cmd[4] | (cmd[3] << 8) | (cmd[2] << 16);
- if (cmd[1] & 0x01) /* fixed */
- req->cmd.xfer *= req->dev->blocksize;
+ cmd->len = 6;
+ cmd->xfer = buf[4] | (buf[3] << 8) | (buf[2] << 16);
+ if (buf[1] & 0x01) { /* fixed */
+ cmd->xfer *= dev->blocksize;
+ }
break;
case REWIND:
case START_STOP:
- req->cmd.len = 6;
- req->cmd.xfer = 0;
+ cmd->len = 6;
+ cmd->xfer = 0;
break;
/* generic commands */
default:
- return scsi_req_length(req, cmd);
+ return scsi_req_length(cmd, dev, buf);
}
return 0;
}
-static void scsi_req_xfer_mode(SCSIRequest *req)
+static void scsi_cmd_xfer_mode(SCSICommand *cmd)
{
- switch (req->cmd.buf[0]) {
+ switch (cmd->buf[0]) {
case WRITE_6:
case WRITE_10:
case WRITE_VERIFY:
@@ -380,21 +381,21 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
case WRITE_LONG_2:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
- req->cmd.mode = SCSI_XFER_TO_DEV;
+ cmd->mode = SCSI_XFER_TO_DEV;
break;
default:
- if (req->cmd.xfer)
- req->cmd.mode = SCSI_XFER_FROM_DEV;
+ if (cmd->xfer)
+ cmd->mode = SCSI_XFER_FROM_DEV;
else {
- req->cmd.mode = SCSI_XFER_NONE;
+ cmd->mode = SCSI_XFER_NONE;
}
break;
}
}
-static uint64_t scsi_req_lba(SCSIRequest *req)
+static uint64_t scsi_cmd_lba(SCSICommand *cmd)
{
- uint8_t *buf = req->cmd.buf;
+ uint8_t *buf = cmd->buf;
uint64_t lba;
switch (buf[0] >> 5) {
@@ -429,16 +430,16 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
int rc;
if (req->dev->type == TYPE_TAPE) {
- rc = scsi_req_stream_length(req, buf);
+ rc = scsi_req_stream_length(&req->cmd, req->dev, buf);
} else {
- rc = scsi_req_length(req, buf);
+ rc = scsi_req_length(&req->cmd, req->dev, buf);
}
if (rc != 0)
return rc;
assert(buf == req->cmd.buf);
- scsi_req_xfer_mode(req);
- req->cmd.lba = scsi_req_lba(req);
+ scsi_cmd_xfer_mode(&req->cmd);
+ req->cmd.lba = scsi_cmd_lba(&req->cmd);
trace_scsi_req_parsed(req->dev->id, req->lun, req->tag, buf[0],
req->cmd.mode, req->cmd.xfer);
if (req->cmd.lba != -1) {
diff --git a/hw/scsi.h b/hw/scsi.h
index 7ed7550..f29d65f 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -11,6 +11,7 @@
typedef struct SCSIBus SCSIBus;
typedef struct SCSIBusOps SCSIBusOps;
+typedef struct SCSICommand SCSICommand;
typedef struct SCSIDevice SCSIDevice;
typedef struct SCSIDeviceInfo SCSIDeviceInfo;
typedef struct SCSIRequest SCSIRequest;
@@ -30,6 +31,14 @@ typedef struct SCSISense {
#define SCSI_SENSE_BUF_SIZE 96
+struct SCSICommand {
+ uint8_t buf[SCSI_CMD_BUF_SIZE];
+ int len;
+ size_t xfer;
+ uint64_t lba;
+ enum SCSIXferMode mode;
+};
+
struct SCSIRequest {
SCSIBus *bus;
SCSIDevice *dev;
@@ -38,13 +47,7 @@ struct SCSIRequest {
uint32_t tag;
uint32_t lun;
uint32_t status;
- struct {
- uint8_t buf[SCSI_CMD_BUF_SIZE];
- int len;
- size_t xfer;
- uint64_t lba;
- enum SCSIXferMode mode;
- } cmd;
+ SCSICommand cmd;
BlockDriverAIOCB *aiocb;
uint8_t sense[SCSI_SENSE_BUF_SIZE];
uint32_t sense_len;
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 09/16] scsi: push lun field to SCSIDevice
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (7 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 08/16] scsi: introduce SCSICommand Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code Paolo Bonzini
` (7 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
This will let SCSIBus detect requests sent to an invalid LUN, and
handle them itself. However, there will be still support for only one
LUN per target
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 1 +
hw/scsi-generic.c | 5 +----
hw/scsi.h | 1 +
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 579f6b4..07d8704 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -16,6 +16,7 @@ static struct BusInfo scsi_bus_info = {
.get_fw_dev_path = scsibus_get_fw_dev_path,
.props = (Property[]) {
DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
+ DEFINE_PROP_UINT32("lun", SCSIDevice, lun, 0),
DEFINE_PROP_END_OF_LIST(),
},
};
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 453a295..699831a 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -60,7 +60,6 @@ struct SCSIGenericState
{
SCSIDevice qdev;
BlockDriverState *bs;
- int lun;
};
static void scsi_free_request(SCSIRequest *req)
@@ -292,7 +291,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
int ret;
- if (cmd[0] != REQUEST_SENSE && req->lun != s->lun) {
+ if (cmd[0] != REQUEST_SENSE && req->lun != s->qdev.lun) {
DPRINTF("Unimplemented LUN %d\n", req->lun);
scsi_req_build_sense(&r->req, SENSE_CODE(LUN_NOT_SUPPORTED));
scsi_req_complete(&r->req, CHECK_CONDITION);
@@ -466,8 +465,6 @@ static int scsi_generic_initfn(SCSIDevice *dev)
}
/* define device state */
- s->lun = scsiid.lun;
- DPRINTF("LUN %d\n", s->lun);
s->qdev.type = scsiid.scsi_type;
DPRINTF("device type %d\n", s->qdev.type);
if (s->qdev.type == TYPE_TAPE) {
diff --git a/hw/scsi.h b/hw/scsi.h
index f29d65f..5a1bbe2 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -65,6 +65,7 @@ struct SCSIDevice
uint8_t sense[SCSI_SENSE_BUF_SIZE];
uint32_t sense_len;
QTAILQ_HEAD(, SCSIRequest) requests;
+ uint32_t lun;
int blocksize;
int type;
};
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (8 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 09/16] scsi: push lun field to SCSIDevice Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-12 16:12 ` Peter Maydell
2011-08-12 16:55 ` Peter Maydell
2011-08-03 8:49 ` [Qemu-devel] [PATCH 11/16] scsi: move handling of REPORT LUNS and invalid LUNs " Paolo Bonzini
` (6 subsequent siblings)
16 siblings, 2 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Also introduce the first occurrence of "independent" SCSIReqOps,
to handle invalid commands in common code.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 53 ++++++++++++++++++++++++++++++++++++++---------------
hw/scsi-disk.c | 5 -----
hw/scsi-generic.c | 9 ---------
hw/scsi.h | 1 -
4 files changed, 38 insertions(+), 30 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 07d8704..01dbe02 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -7,6 +7,7 @@
#include "trace.h"
static char *scsibus_get_fw_dev_path(DeviceState *dev);
+static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
static int scsi_build_sense(uint8_t *in_buf, int in_len,
uint8_t *buf, int len, bool fixed);
@@ -134,6 +135,20 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
return res;
}
+/* SCSIReqOps implementation for invalid commands. */
+
+static int32_t scsi_invalid_command(SCSIRequest *req, uint8_t *buf)
+{
+ scsi_req_build_sense(req, SENSE_CODE(INVALID_OPCODE));
+ scsi_req_complete(req, CHECK_CONDITION);
+ return 0;
+}
+
+struct SCSIReqOps reqops_invalid_opcode = {
+ .size = sizeof(SCSIRequest),
+ .send_command = scsi_invalid_command
+};
+
SCSIRequest *scsi_req_alloc(SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag,
uint32_t lun, void *hba_private)
{
@@ -157,8 +172,22 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
uint8_t *buf, void *hba_private)
{
SCSIRequest *req;
- req = d->info->alloc_req(d, tag, lun, hba_private);
- memcpy(req->cmd.buf, buf, 16);
+ SCSICommand cmd;
+
+ if (scsi_req_parse(&cmd, d, buf) != 0) {
+ trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
+ req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
+ } else {
+ trace_scsi_req_parsed(d->id, lun, tag, buf[0],
+ cmd.mode, cmd.xfer);
+ if (req->cmd.lba != -1) {
+ trace_scsi_req_parsed_lba(d->id, lun, tag, buf[0],
+ cmd.lba);
+ }
+ req = d->info->alloc_req(d, tag, lun, hba_private);
+ }
+
+ req->cmd = cmd;
return req;
}
@@ -426,27 +455,21 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
return lba;
}
-int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
+int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
{
int rc;
- if (req->dev->type == TYPE_TAPE) {
- rc = scsi_req_stream_length(&req->cmd, req->dev, buf);
+ if (dev->type == TYPE_TAPE) {
+ rc = scsi_req_stream_length(cmd, dev, buf);
} else {
- rc = scsi_req_length(&req->cmd, req->dev, buf);
+ rc = scsi_req_length(cmd, dev, buf);
}
if (rc != 0)
return rc;
- assert(buf == req->cmd.buf);
- scsi_cmd_xfer_mode(&req->cmd);
- req->cmd.lba = scsi_cmd_lba(&req->cmd);
- trace_scsi_req_parsed(req->dev->id, req->lun, req->tag, buf[0],
- req->cmd.mode, req->cmd.xfer);
- if (req->cmd.lba != -1) {
- trace_scsi_req_parsed_lba(req->dev->id, req->lun, req->tag, buf[0],
- req->cmd.lba);
- }
+ memcpy(cmd->buf, buf, cmd->len);
+ scsi_cmd_xfer_mode(cmd);
+ cmd->lba = scsi_cmd_lba(cmd);
return 0;
}
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ceb0e0a..64aa141 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -974,11 +974,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
outbuf = (uint8_t *)r->iov.iov_base;
DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", req->lun, req->tag, buf[0]);
- if (scsi_req_parse(&r->req, buf) != 0) {
- BADF("Unsupported command length, command %x\n", command);
- scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
- return 0;
- }
#ifdef DEBUG_SCSI
{
int i;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 699831a..e91c32e 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -84,10 +84,6 @@ static void scsi_command_complete(void *opaque, int ret)
case -EDOM:
status = TASK_SET_FULL;
break;
- case -EINVAL:
- status = CHECK_CONDITION;
- scsi_req_build_sense(&r->req, SENSE_CODE(INVALID_FIELD));
- break;
case -ENOMEM:
status = CHECK_CONDITION;
scsi_req_build_sense(&r->req, SENSE_CODE(TARGET_FAILURE));
@@ -298,11 +294,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
return 0;
}
- if (-1 == scsi_req_parse(&r->req, cmd)) {
- BADF("Unsupported command length, command %x\n", cmd[0]);
- scsi_command_complete(r, -EINVAL);
- return 0;
- }
scsi_req_fixup(&r->req);
DPRINTF("Command: lun=%d tag=0x%x len %zd data=0x%02x", lun, tag,
diff --git a/hw/scsi.h b/hw/scsi.h
index 5a1bbe2..dec814a 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -165,7 +165,6 @@ SCSIRequest *scsi_req_ref(SCSIRequest *req);
void scsi_req_unref(SCSIRequest *req);
void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
-int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
void scsi_req_print(SCSIRequest *req);
void scsi_req_continue(SCSIRequest *req);
void scsi_req_data(SCSIRequest *req, int len);
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code
2011-08-03 8:49 ` [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code Paolo Bonzini
@ 2011-08-12 16:12 ` Peter Maydell
2011-08-12 17:05 ` Paolo Bonzini
2011-08-12 16:55 ` Peter Maydell
1 sibling, 1 reply; 29+ messages in thread
From: Peter Maydell @ 2011-08-12 16:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 3 August 2011 09:49, Paolo Bonzini <pbonzini@redhat.com> wrote:
> @@ -157,8 +172,22 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> uint8_t *buf, void *hba_private)
> {
> SCSIRequest *req;
> - req = d->info->alloc_req(d, tag, lun, hba_private);
> - memcpy(req->cmd.buf, buf, 16);
> + SCSICommand cmd;
> +
> + if (scsi_req_parse(&cmd, d, buf) != 0) {
> + trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
> + req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
> + } else {
> + trace_scsi_req_parsed(d->id, lun, tag, buf[0],
> + cmd.mode, cmd.xfer);
> + if (req->cmd.lba != -1) {
> + trace_scsi_req_parsed_lba(d->id, lun, tag, buf[0],
> + cmd.lba);
> + }
> + req = d->info->alloc_req(d, tag, lun, hba_private);
> + }
> +
> + req->cmd = cmd;
> return req;
> }
This patch makes current master fail to compile with optimisation on:
gcc complains:
hw/scsi-bus.c: In function ‘scsi_req_new’:
hw/scsi-bus.c:375: error: ‘req’ may be used uninitialized in this function
because in the 'else' clause we look at req->cmd.lba before we've
called alloc_req().
My guess is that the tracing should just be moved down to after the
allocation?
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code
2011-08-12 16:12 ` Peter Maydell
@ 2011-08-12 17:05 ` Paolo Bonzini
0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-12 17:05 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
On 08/12/2011 06:12 PM, Peter Maydell wrote:
> This patch makes current master fail to compile with optimisation on:
>
> gcc complains:
> hw/scsi-bus.c: In function ‘scsi_req_new’:
> hw/scsi-bus.c:375: error: ‘req’ may be used uninitialized in this function
>
> because in the 'else' clause we look at req->cmd.lba before we've
> called alloc_req().
>
> My guess is that the tracing should just be moved down to after the
> allocation?
You can also use cmd.lba instead of req->cmd.lba.
I guess the failure depends on the compiler version and tracing options.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code
2011-08-03 8:49 ` [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code Paolo Bonzini
2011-08-12 16:12 ` Peter Maydell
@ 2011-08-12 16:55 ` Peter Maydell
2011-08-12 17:11 ` Paolo Bonzini
1 sibling, 1 reply; 29+ messages in thread
From: Peter Maydell @ 2011-08-12 16:55 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 3 August 2011 09:49, Paolo Bonzini <pbonzini@redhat.com> wrote:
> @@ -157,8 +172,22 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> uint8_t *buf, void *hba_private)
> {
> SCSIRequest *req;
> - req = d->info->alloc_req(d, tag, lun, hba_private);
> - memcpy(req->cmd.buf, buf, 16);
> + SCSICommand cmd;
> +
> + if (scsi_req_parse(&cmd, d, buf) != 0) {
> + trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
> + req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
> + } else {
> + trace_scsi_req_parsed(d->id, lun, tag, buf[0],
> + cmd.mode, cmd.xfer);
> + if (req->cmd.lba != -1) {
> + trace_scsi_req_parsed_lba(d->id, lun, tag, buf[0],
> + cmd.lba);
> + }
> + req = d->info->alloc_req(d, tag, lun, hba_private);
> + }
> +
> + req->cmd = cmd;
> return req;
> }
Does it still make sense to set req->cmd to cmd (and to look at cmd
at all) in the case where scsi_req_parse() failed and might not have
actually initialised all of cmd? For instance the tracing code (added
to scsi_req_new() after this patch) looks at cmd.buf[] based on the
value of buf[0], which seems kind of fragile to me.
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code
2011-08-12 16:55 ` Peter Maydell
@ 2011-08-12 17:11 ` Paolo Bonzini
0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-12 17:11 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
On 08/12/2011 06:55 PM, Peter Maydell wrote:
> Does it still make sense to set req->cmd to cmd (and to look at cmd
> at all) in the case where scsi_req_parse() failed and might not have
> actually initialised all of cmd? For instance the tracing code (added
> to scsi_req_new() after this patch) looks at cmd.buf[] based on the
> value of buf[0], which seems kind of fragile to me.
At the point tracing is reached, we know that cmd.buf[] has been
initialized. But you're right that it is at least not tidy.
We know that the size of the cdb is 16 (it is always like that, and we
can make it a requirement). So we can copy it to cmd->buf before
knowing cmd->len, at the beginning of scsi_req_parse. We can also zero
unconditionally len/xfer/mode (plus set lba to -1) in case the parsing
fails.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 11/16] scsi: move handling of REPORT LUNS and invalid LUNs to common code
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (9 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 12/16] scsi: move handling of REQUEST SENSE " Paolo Bonzini
` (5 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
hw/scsi-defs.h | 3 +
hw/scsi-disk.c | 21 ------
hw/scsi-generic.c | 7 --
4 files changed, 177 insertions(+), 29 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 01dbe02..139f6a6 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -149,6 +149,172 @@ struct SCSIReqOps reqops_invalid_opcode = {
.send_command = scsi_invalid_command
};
+/* SCSIReqOps implementation for REPORT LUNS and for commands sent to
+ an invalid LUN. */
+
+typedef struct SCSITargetReq SCSITargetReq;
+
+struct SCSITargetReq {
+ SCSIRequest req;
+ int len;
+ uint8_t buf[64];
+};
+
+static void store_lun(uint8_t *outbuf, int lun)
+{
+ if (lun < 256) {
+ outbuf[1] = lun;
+ return;
+ }
+ outbuf[1] = (lun & 255);
+ outbuf[0] = (lun >> 8) | 0x40;
+}
+
+static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
+{
+ int len;
+ if (r->req.cmd.xfer < 16) {
+ return false;
+ }
+ if (r->req.cmd.buf[2] > 2) {
+ return false;
+ }
+ len = MIN(sizeof r->buf, r->req.cmd.xfer);
+ memset(r->buf, 0, len);
+ if (r->req.dev->lun != 0) {
+ r->buf[3] = 16;
+ r->len = 24;
+ store_lun(&r->buf[16], r->req.dev->lun);
+ } else {
+ r->buf[3] = 8;
+ r->len = 16;
+ }
+ return true;
+}
+
+static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
+{
+ assert(r->req.dev->lun != r->req.lun);
+ if (r->req.cmd.buf[1] & 0x2) {
+ /* Command support data - optional, not implemented */
+ return false;
+ }
+
+ if (r->req.cmd.buf[1] & 0x1) {
+ /* Vital product data */
+ uint8_t page_code = r->req.cmd.buf[2];
+ if (r->req.cmd.xfer < 4) {
+ return false;
+ }
+
+ r->buf[r->len++] = page_code ; /* this page */
+ r->buf[r->len++] = 0x00;
+
+ switch (page_code) {
+ case 0x00: /* Supported page codes, mandatory */
+ {
+ int pages;
+ pages = r->len++;
+ r->buf[r->len++] = 0x00; /* list of supported pages (this page) */
+ r->buf[pages] = r->len - pages - 1; /* number of pages */
+ break;
+ }
+ default:
+ return false;
+ }
+ /* done with EVPD */
+ assert(r->len < sizeof(r->buf));
+ r->len = MIN(r->req.cmd.xfer, r->len);
+ return true;
+ }
+
+ /* Standard INQUIRY data */
+ if (r->req.cmd.buf[2] != 0) {
+ return false;
+ }
+
+ /* PAGE CODE == 0 */
+ if (r->req.cmd.xfer < 5) {
+ return -1;
+ }
+
+ r->len = MIN(r->req.cmd.xfer, 36);
+ memset(r->buf, 0, r->len);
+ if (r->req.lun != 0) {
+ r->buf[0] = TYPE_NO_LUN;
+ } else {
+ r->buf[0] = TYPE_NOT_PRESENT | TYPE_INACTIVE;
+ r->buf[2] = 5; /* Version */
+ r->buf[3] = 2 | 0x10; /* HiSup, response data format */
+ r->buf[4] = r->len - 5; /* Additional Length = (Len - 1) - 4 */
+ r->buf[7] = 0x10 | (r->req.bus->tcq ? 0x02 : 0); /* Sync, TCQ. */
+ memcpy(&r->buf[8], "QEMU ", 8);
+ memcpy(&r->buf[16], "QEMU TARGET ", 16);
+ strncpy((char *) &r->buf[32], QEMU_VERSION, 4);
+ }
+ return true;
+}
+
+static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
+{
+ SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
+
+ switch (buf[0]) {
+ case REPORT_LUNS:
+ if (!scsi_target_emulate_report_luns(r)) {
+ goto illegal_request;
+ }
+ break;
+ case INQUIRY:
+ if (!scsi_target_emulate_inquiry(r)) {
+ goto illegal_request;
+ }
+ break;
+ default:
+ scsi_req_build_sense(req, SENSE_CODE(LUN_NOT_SUPPORTED));
+ scsi_req_complete(req, CHECK_CONDITION);
+ return 0;
+ illegal_request:
+ scsi_req_build_sense(req, SENSE_CODE(INVALID_FIELD));
+ scsi_req_complete(req, CHECK_CONDITION);
+ return 0;
+ }
+
+ if (!r->len) {
+ scsi_req_complete(req, GOOD);
+ }
+ return r->len;
+}
+
+static void scsi_target_read_data(SCSIRequest *req)
+{
+ SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
+ uint32_t n;
+
+ n = r->len;
+ if (n > 0) {
+ r->len = 0;
+ scsi_req_data(&r->req, n);
+ } else {
+ scsi_req_complete(&r->req, GOOD);
+ }
+}
+
+static uint8_t *scsi_target_get_buf(SCSIRequest *req)
+{
+ SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
+
+ return r->buf;
+}
+
+struct SCSIReqOps reqops_target_command = {
+ .size = sizeof(SCSITargetReq),
+ .send_command = scsi_target_send_command,
+ .read_data = scsi_target_read_data,
+ .get_buf = scsi_target_get_buf,
+};
+
+
SCSIRequest *scsi_req_alloc(SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag,
uint32_t lun, void *hba_private)
{
@@ -184,7 +350,14 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
trace_scsi_req_parsed_lba(d->id, lun, tag, buf[0],
cmd.lba);
}
- req = d->info->alloc_req(d, tag, lun, hba_private);
+
+ if ((lun != d->lun && buf[0] != REQUEST_SENSE) ||
+ buf[0] == REPORT_LUNS) {
+ req = scsi_req_alloc(&reqops_target_command, d, tag, lun,
+ hba_private);
+ } else {
+ req = d->info->alloc_req(d, tag, lun, hba_private);
+ }
}
req->cmd = cmd;
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 413cce0..79ab9d5 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -162,5 +162,8 @@
* - treated as TYPE_DISK */
#define TYPE_MEDIUM_CHANGER 0x08
#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
+#define TYPE_WLUN 0x1e /* Well known LUN */
+#define TYPE_NOT_PRESENT 0x1f
+#define TYPE_INACTIVE 0x20
#define TYPE_NO_LUN 0x7f
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 64aa141..a5fdb05 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -485,11 +485,6 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
memset(outbuf, 0, buflen);
- if (req->lun) {
- outbuf[0] = 0x7f; /* LUN not supported */
- return buflen;
- }
-
if (s->drive_kind == SCSI_CD) {
outbuf[0] = 5;
outbuf[1] = 0x80;
@@ -922,13 +917,6 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
}
DPRINTF("Unsupported Service Action In\n");
goto illegal_request;
- case REPORT_LUNS:
- if (req->cmd.xfer < 16)
- goto illegal_request;
- memset(outbuf, 0, 16);
- outbuf[3] = 8;
- buflen = 16;
- break;
case VERIFY:
break;
case REZERO_UNIT:
@@ -984,14 +972,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
}
#endif
- if (req->lun) {
- /* Only LUN 0 supported. */
- DPRINTF("Unimplemented LUN %d\n", req->lun);
- if (command != REQUEST_SENSE && command != INQUIRY) {
- scsi_check_condition(r, SENSE_CODE(LUN_NOT_SUPPORTED));
- return 0;
- }
- }
switch (command) {
case TEST_UNIT_READY:
case REQUEST_SENSE:
@@ -1009,7 +989,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
case READ_TOC:
case GET_CONFIGURATION:
case SERVICE_ACTION_IN:
- case REPORT_LUNS:
case VERIFY:
case REZERO_UNIT:
rc = scsi_disk_emulate_command(r, outbuf);
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e91c32e..5217aaf 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -287,13 +287,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
int ret;
- if (cmd[0] != REQUEST_SENSE && req->lun != s->qdev.lun) {
- DPRINTF("Unimplemented LUN %d\n", req->lun);
- scsi_req_build_sense(&r->req, SENSE_CODE(LUN_NOT_SUPPORTED));
- scsi_req_complete(&r->req, CHECK_CONDITION);
- return 0;
- }
-
scsi_req_fixup(&r->req);
DPRINTF("Command: lun=%d tag=0x%x len %zd data=0x%02x", lun, tag,
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 12/16] scsi: move handling of REQUEST SENSE to common code
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (10 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 11/16] scsi: move handling of REPORT LUNS and invalid LUNs " Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 13/16] scsi: add a bunch more common sense codes Paolo Bonzini
` (4 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 12 ++++++++++--
hw/scsi-disk.c | 9 ++-------
hw/scsi-generic.c | 16 ----------------
3 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 139f6a6..fbb9801 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -270,6 +270,13 @@ static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
goto illegal_request;
}
break;
+ case REQUEST_SENSE:
+ if (req->cmd.xfer < 4) {
+ goto illegal_request;
+ }
+ r->len = scsi_device_get_sense(r->req.dev, r->buf, req->cmd.xfer,
+ (req->cmd.buf[1] & 1) == 0);
+ break;
default:
scsi_req_build_sense(req, SENSE_CODE(LUN_NOT_SUPPORTED));
scsi_req_complete(req, CHECK_CONDITION);
@@ -351,8 +358,9 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
cmd.lba);
}
- if ((lun != d->lun && buf[0] != REQUEST_SENSE) ||
- buf[0] == REPORT_LUNS) {
+ if (lun != d->lun ||
+ buf[0] == REPORT_LUNS ||
+ buf[0] == REQUEST_SENSE) {
req = scsi_req_alloc(&reqops_target_command, d, tag, lun,
hba_private);
} else {
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a5fdb05..5f21d68 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -792,12 +792,6 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
if (!bdrv_is_inserted(s->bs))
goto not_ready;
break;
- case REQUEST_SENSE:
- if (req->cmd.xfer < 4)
- goto illegal_request;
- buflen = scsi_device_get_sense(&s->qdev, outbuf, req->cmd.xfer,
- (req->cmd.buf[1] & 1) == 0);
- break;
case INQUIRY:
buflen = scsi_disk_emulate_inquiry(req, outbuf);
if (buflen < 0)
@@ -974,7 +968,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
switch (command) {
case TEST_UNIT_READY:
- case REQUEST_SENSE:
case INQUIRY:
case MODE_SENSE:
case MODE_SENSE_10:
@@ -1074,6 +1067,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
}
break;
+ case REQUEST_SENSE:
+ abort();
default:
DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 5217aaf..c122ad3 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -183,22 +183,6 @@ static void scsi_read_data(SCSIRequest *req)
return;
}
- if (r->req.cmd.buf[0] == REQUEST_SENSE) {
- r->io_header.driver_status = 0;
- r->io_header.status = 0;
- r->io_header.dxfer_len =
- scsi_device_get_sense(&s->qdev, r->buf, r->req.cmd.xfer,
- (r->req.cmd.buf[1] & 1) == 0);
- r->len = -1;
- DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, r->io_header.dxfer_len);
- DPRINTF("Sense: %d %d %d %d %d %d %d %d\n",
- r->buf[0], r->buf[1], r->buf[2], r->buf[3],
- r->buf[4], r->buf[5], r->buf[6], r->buf[7]);
- scsi_req_data(&r->req, r->io_header.dxfer_len);
- /* The sense buffer is cleared when we return GOOD */
- return;
- }
-
ret = execute_command(s->bs, r, SG_DXFER_FROM_DEV, scsi_read_complete);
if (ret < 0) {
scsi_command_complete(r, ret);
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 13/16] scsi: add a bunch more common sense codes
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (11 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 12/16] scsi: move handling of REQUEST SENSE " Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 14/16] scsi: add support for unit attention conditions Paolo Bonzini
` (3 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 30 ++++++++++++++++++++++++++++++
hw/scsi.h | 12 ++++++++++++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index fbb9801..0e2e4cd 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -698,6 +698,16 @@ const struct SCSISense sense_code_LUN_NOT_SUPPORTED = {
.key = ILLEGAL_REQUEST, .asc = 0x25, .ascq = 0x00
};
+/* Illegal request, Saving parameters not supported */
+const struct SCSISense sense_code_SAVING_PARAMS_NOT_SUPPORTED = {
+ .key = ILLEGAL_REQUEST, .asc = 0x39, .ascq = 0x00
+};
+
+/* Illegal request, Incompatible medium installed */
+const struct SCSISense sense_code_INCOMPATIBLE_MEDIUM = {
+ .key = ILLEGAL_REQUEST, .asc = 0x30, .ascq = 0x00
+};
+
/* Command aborted, I/O process terminated */
const struct SCSISense sense_code_IO_ERROR = {
.key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
@@ -713,6 +723,26 @@ const struct SCSISense sense_code_LUN_FAILURE = {
.key = ABORTED_COMMAND, .asc = 0x3e, .ascq = 0x01
};
+/* Unit attention, Power on, reset or bus device reset occurred */
+const struct SCSISense sense_code_RESET = {
+ .key = UNIT_ATTENTION, .asc = 0x29, .ascq = 0x00
+};
+
+/* Unit attention, Medium may have changed */
+const struct SCSISense sense_code_MEDIUM_CHANGED = {
+ .key = UNIT_ATTENTION, .asc = 0x28, .ascq = 0x00
+};
+
+/* Unit attention, Reported LUNs data has changed */
+const struct SCSISense sense_code_REPORTED_LUNS_CHANGED = {
+ .key = UNIT_ATTENTION, .asc = 0x3f, .ascq = 0x0e
+};
+
+/* Unit attention, Device internal reset */
+const struct SCSISense sense_code_DEVICE_INTERNAL_RESET = {
+ .key = UNIT_ATTENTION, .asc = 0x29, .ascq = 0x04
+};
+
/*
* scsi_build_sense
*
diff --git a/hw/scsi.h b/hw/scsi.h
index dec814a..4d5b596 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -144,12 +144,24 @@ extern const struct SCSISense sense_code_LBA_OUT_OF_RANGE;
extern const struct SCSISense sense_code_INVALID_FIELD;
/* Illegal request, LUN not supported */
extern const struct SCSISense sense_code_LUN_NOT_SUPPORTED;
+/* Illegal request, Saving parameters not supported */
+extern const struct SCSISense sense_code_SAVING_PARAMS_NOT_SUPPORTED;
+/* Illegal request, Incompatible format */
+extern const struct SCSISense sense_code_INCOMPATIBLE_FORMAT;
/* Command aborted, I/O process terminated */
extern const struct SCSISense sense_code_IO_ERROR;
/* Command aborted, I_T Nexus loss occurred */
extern const struct SCSISense sense_code_I_T_NEXUS_LOSS;
/* Command aborted, Logical Unit failure */
extern const struct SCSISense sense_code_LUN_FAILURE;
+/* Unit attention, Power on, reset or bus device reset occurred */
+extern const struct SCSISense sense_code_RESET;
+/* Unit attention, Medium may have changed*/
+extern const struct SCSISense sense_code_MEDIUM_CHANGED;
+/* Unit attention, Reported LUNs data has changed */
+extern const struct SCSISense sense_code_REPORTED_LUNS_CHANGED;
+/* Unit attention, Device internal reset */
+extern const struct SCSISense sense_code_DEVICE_INTERNAL_RESET;
#define SENSE_CODE(x) sense_code_ ## x
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 14/16] scsi: add support for unit attention conditions
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (12 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 13/16] scsi: add a bunch more common sense codes Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 15/16] scsi: report unit attention on reset Paolo Bonzini
` (2 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Unit attention conditions override any sense data the device already
has. Their signaling and clearing is handled entirely by the SCSIBus
code, and they are completely transparent to the SCSIDevices.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
hw/scsi-defs.h | 1 +
hw/scsi.h | 2 +
3 files changed, 94 insertions(+), 2 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 0e2e4cd..a5bb517 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -149,6 +149,24 @@ struct SCSIReqOps reqops_invalid_opcode = {
.send_command = scsi_invalid_command
};
+/* SCSIReqOps implementation for unit attention conditions. */
+
+static int32_t scsi_unit_attention(SCSIRequest *req, uint8_t *buf)
+{
+ if (req->dev && req->dev->unit_attention.key == UNIT_ATTENTION) {
+ scsi_req_build_sense(req, req->dev->unit_attention);
+ } else if (req->bus->unit_attention.key == UNIT_ATTENTION) {
+ scsi_req_build_sense(req, req->bus->unit_attention);
+ }
+ scsi_req_complete(req, CHECK_CONDITION);
+ return 0;
+}
+
+struct SCSIReqOps reqops_unit_attention = {
+ .size = sizeof(SCSIRequest),
+ .send_command = scsi_unit_attention
+};
+
/* SCSIReqOps implementation for REPORT LUNS and for commands sent to
an invalid LUN. */
@@ -344,6 +362,7 @@ SCSIRequest *scsi_req_alloc(SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag,
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
uint8_t *buf, void *hba_private)
{
+ SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
SCSIRequest *req;
SCSICommand cmd;
@@ -358,7 +377,15 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
cmd.lba);
}
- if (lun != d->lun ||
+ if ((d->unit_attention.key == UNIT_ATTENTION ||
+ bus->unit_attention.key == UNIT_ATTENTION) &&
+ (buf[0] != INQUIRY &&
+ buf[0] != REPORT_LUNS &&
+ buf[0] != GET_CONFIGURATION &&
+ buf[0] != GET_EVENT_STATUS_NOTIFICATION)) {
+ req = scsi_req_alloc(&reqops_unit_attention, d, tag, lun,
+ hba_private);
+ } else if (lun != d->lun ||
buf[0] == REPORT_LUNS ||
buf[0] == REQUEST_SENSE) {
req = scsi_req_alloc(&reqops_target_command, d, tag, lun,
@@ -377,13 +404,68 @@ uint8_t *scsi_req_get_buf(SCSIRequest *req)
return req->ops->get_buf(req);
}
+static void scsi_clear_unit_attention(SCSIRequest *req)
+{
+ SCSISense *ua;
+ if (req->dev->unit_attention.key != UNIT_ATTENTION &&
+ req->bus->unit_attention.key != UNIT_ATTENTION) {
+ return;
+ }
+
+ /*
+ * If an INQUIRY command enters the enabled command state,
+ * the device server shall [not] clear any unit attention condition;
+ * See also MMC-6, paragraphs 6.5 and 6.6.2.
+ */
+ if (req->cmd.buf[0] == INQUIRY ||
+ req->cmd.buf[0] == GET_CONFIGURATION ||
+ req->cmd.buf[0] == GET_EVENT_STATUS_NOTIFICATION) {
+ return;
+ }
+
+ if (req->dev->unit_attention.key == UNIT_ATTENTION) {
+ ua = &req->dev->unit_attention;
+ } else {
+ ua = &req->bus->unit_attention;
+ }
+
+ /*
+ * If a REPORT LUNS command enters the enabled command state, [...]
+ * the device server shall clear any pending unit attention condition
+ * with an additional sense code of REPORTED LUNS DATA HAS CHANGED.
+ */
+ if (req->cmd.buf[0] == REPORT_LUNS &&
+ !(ua->asc == SENSE_CODE(REPORTED_LUNS_CHANGED).asc &&
+ ua->ascq == SENSE_CODE(REPORTED_LUNS_CHANGED).ascq)) {
+ return;
+ }
+
+ *ua = SENSE_CODE(NO_SENSE);
+}
+
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len)
{
+ int ret;
+
assert(len >= 14);
if (!req->sense_len) {
return 0;
}
- return scsi_build_sense(req->sense, req->sense_len, buf, len, true);
+
+ ret = scsi_build_sense(req->sense, req->sense_len, buf, len, true);
+
+ /*
+ * FIXME: clearing unit attention conditions upon autosense should be done
+ * only if the UA_INTLCK_CTRL field in the Control mode page is set to 00b
+ * (SAM-5, 5.14).
+ *
+ * We assume UA_INTLCK_CTRL to be 00b for HBAs that support autosense, and
+ * 10b for HBAs that do not support it (do not call scsi_req_get_sense).
+ * In the latter case, scsi_req_complete clears unit attention conditions
+ * after moving them to the device's sense buffer.
+ */
+ scsi_clear_unit_attention(req);
+ return ret;
}
int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed)
@@ -973,6 +1055,13 @@ void scsi_req_complete(SCSIRequest *req, int status)
}
req->dev->sense_len = req->sense_len;
+ /*
+ * Unit attention state is now stored in the device's sense buffer
+ * if the HBA didn't do autosense. Clear the pending unit attention
+ * flags.
+ */
+ scsi_clear_unit_attention(req);
+
scsi_req_ref(req);
scsi_req_dequeue(req);
req->bus->ops->complete(req, req->status);
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 79ab9d5..9e0b788 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -102,6 +102,7 @@
#define REWIND 0x01
#define REPORT_DENSITY_SUPPORT 0x44
#define GET_CONFIGURATION 0x46
+#define GET_EVENT_STATUS_NOTIFICATION 0x4a
#define READ_16 0x88
#define WRITE_16 0x8a
#define WRITE_VERIFY_16 0x8e
diff --git a/hw/scsi.h b/hw/scsi.h
index 4d5b596..09c3606 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -62,6 +62,7 @@ struct SCSIDevice
uint32_t id;
BlockConf conf;
SCSIDeviceInfo *info;
+ SCSISense unit_attention;
uint8_t sense[SCSI_SENSE_BUF_SIZE];
uint32_t sense_len;
QTAILQ_HEAD(, SCSIRequest) requests;
@@ -105,6 +106,7 @@ struct SCSIBus {
BusState qbus;
int busnr;
+ SCSISense unit_attention;
int tcq, ndev;
const SCSIBusOps *ops;
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 15/16] scsi: report unit attention on reset
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (13 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 14/16] scsi: add support for unit attention conditions Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-03 8:49 ` [Qemu-devel] [PATCH 16/16] scsi: add special traces for common commands Paolo Bonzini
2011-08-12 13:49 ` [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Anthony Liguori
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 3 ++-
hw/scsi-disk.c | 4 ++--
hw/scsi-generic.c | 4 ++--
hw/scsi.h | 2 +-
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index a5bb517..f6fb4f0 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1089,7 +1089,7 @@ void scsi_req_abort(SCSIRequest *req, int status)
scsi_req_complete(req, status);
}
-void scsi_device_purge_requests(SCSIDevice *sdev)
+void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense)
{
SCSIRequest *req;
@@ -1097,6 +1097,7 @@ void scsi_device_purge_requests(SCSIDevice *sdev)
req = QTAILQ_FIRST(&sdev->requests);
scsi_req_cancel(req);
}
+ sdev->unit_attention = sense;
}
static char *scsibus_get_fw_dev_path(DeviceState *dev)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5f21d68..655fcda 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1098,7 +1098,7 @@ static void scsi_disk_reset(DeviceState *dev)
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev.qdev, dev);
uint64_t nb_sectors;
- scsi_device_purge_requests(&s->qdev);
+ scsi_device_purge_requests(&s->qdev, SENSE_CODE(RESET));
bdrv_get_geometry(s->bs, &nb_sectors);
nb_sectors /= s->cluster_size;
@@ -1112,7 +1112,7 @@ static void scsi_destroy(SCSIDevice *dev)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
- scsi_device_purge_requests(&s->qdev);
+ scsi_device_purge_requests(&s->qdev, SENSE_CODE(NO_SENSE));
blockdev_mark_auto_del(s->qdev.conf.bs);
}
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index c122ad3..e8053da 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -381,14 +381,14 @@ static void scsi_generic_reset(DeviceState *dev)
{
SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev.qdev, dev);
- scsi_device_purge_requests(&s->qdev);
+ scsi_device_purge_requests(&s->qdev, SENSE_CODE(RESET));
}
static void scsi_destroy(SCSIDevice *d)
{
SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
- scsi_device_purge_requests(&s->qdev);
+ scsi_device_purge_requests(&s->qdev, SENSE_CODE(NO_SENSE));
blockdev_mark_auto_del(s->qdev.conf.bs);
}
diff --git a/hw/scsi.h b/hw/scsi.h
index 09c3606..98fd689 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -187,7 +187,7 @@ uint8_t *scsi_req_get_buf(SCSIRequest *req);
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
void scsi_req_abort(SCSIRequest *req, int status);
void scsi_req_cancel(SCSIRequest *req);
-void scsi_device_purge_requests(SCSIDevice *sdev);
+void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
#endif
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 16/16] scsi: add special traces for common commands
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (14 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 15/16] scsi: report unit attention on reset Paolo Bonzini
@ 2011-08-03 8:49 ` Paolo Bonzini
2011-08-12 13:49 ` [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Anthony Liguori
16 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2011-08-03 8:49 UTC (permalink / raw)
To: qemu-devel
Can be useful when debugging the device scan phase.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 17 +++++++++++++++++
trace-events | 4 ++++
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index f6fb4f0..143c7eb 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -396,6 +396,23 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
}
req->cmd = cmd;
+ switch (buf[0]) {
+ case INQUIRY:
+ trace_scsi_inquiry(d->id, lun, tag, cmd.buf[1], cmd.buf[2]);
+ break;
+ case TEST_UNIT_READY:
+ trace_scsi_test_unit_ready(d->id, lun, tag);
+ break;
+ case REPORT_LUNS:
+ trace_scsi_report_luns(d->id, lun, tag);
+ break;
+ case REQUEST_SENSE:
+ trace_scsi_request_sense(d->id, lun, tag);
+ break;
+ default:
+ break;
+ }
+
return req;
}
diff --git a/trace-events b/trace-events
index 55023b0..c565748 100644
--- a/trace-events
+++ b/trace-events
@@ -249,6 +249,10 @@ disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int xfe
disable scsi_req_parsed_lba(int target, int lun, int tag, int cmd, uint64_t lba) "target %d lun %d tag %d command %d lba %"PRIu64""
disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d lun %d tag %d command %d"
disable scsi_req_build_sense(int target, int lun, int tag, int key, int asc, int ascq) "target %d lun %d tag %d key %#02x asc %#02x ascq %#02x"
+disable scsi_report_luns(int target, int lun, int tag) "target %d lun %d tag %d"
+disable scsi_inquiry(int target, int lun, int tag, int cdb1, int cdb2) "target %d lun %d tag %d page %#02x/%#02x"
+disable scsi_test_unit_ready(int target, int lun, int tag) "target %d lun %d tag %d"
+disable scsi_request_sense(int target, int lun, int tag) "target %d lun %d tag %d"
# vl.c
disable vm_state_notify(int running, int reason) "running %d reason %d"
--
1.7.6
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul
2011-08-03 8:49 [Qemu-devel] [PATCH 00/16] SCSI sense and target request overhaul Paolo Bonzini
` (15 preceding siblings ...)
2011-08-03 8:49 ` [Qemu-devel] [PATCH 16/16] scsi: add special traces for common commands Paolo Bonzini
@ 2011-08-12 13:49 ` Anthony Liguori
2011-08-12 15:48 ` Paolo Bonzini
16 siblings, 1 reply; 29+ messages in thread
From: Anthony Liguori @ 2011-08-12 13:49 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 08/03/2011 03:49 AM, Paolo Bonzini wrote:
> This is a pretty important step in the modernization and improvement
> of the SCSI backends, and a prerequisite for pretty much everything
> that is on the table: migration, addressing, improved CD-ROM support,
> hotplug.
Applied all. Thanks.
It fuzzed a bit, it seems pretty trivial to me but wouldn't hurt to
double check. I tested SCSI quite a bit afterwards too.
Regards,
Anthony Liguori
>
> The series touches two main parts:
>
> 1) sense data. Autosense is made a first-class citizen of the
> subsystem. Sense data is communicated to the target primarily
> via autosense, and it is then the target that uses this information
> to reply to REQUEST SENSE commands. In addition, each LUN or
> target can be in a unit attention condition, which will be
> communicated via either autosense or REQUEST SENSE, or cleared
> automatically.
>
> 2) target requests. The common example of this is REPORT LUNS
> and commands sent to invalid LUNs. Handling of these commands
> previously had to be done for every SCSIDevice. This series splits
> request-related SCSIDevice callbacks out into a new SCSIReqOps
> structure, thus allowing the target (SCSIBus) to hijack those requests
> and reply to them without involving the SCSIDevice at all. The
> system is quite flexible, and is also used for invalid commands,
> REQUEST SENSE and commands sent while a unit attention
> condition is pending.
>
> Finally, other parts of the SCSIDevice code are moved to common
> code including general parsing of requests.
>
> The two parts are slightly intertwined. Sense data is handled by patches
> 2/3/4/12/13/14/15, and target requests are handled by patches 5 to 11.
> (patches 1 and 16 are somewhat unrelated). The reason for this is that
> the current device-specific handling of sense data conflicts heavily
> with the goal of handling some commands in a device-independent way.
> So, the series starts by moving sense handling to generic code, and
> comes back to generic handling of REQUEST SENSE and unit attention after
> implementing a few others device-independent requests.
>
> It conflicts somewhat with Markus's tray series, but not in a
> fatal way; the conflicts are trivial.
>
> Paolo Bonzini (16):
> scsi-disk: no need to call scsi_req_data on a short read
> vscsi: always use get_sense
> scsi: pass status when completing
> scsi: move sense handling to generic code
> scsi: introduce SCSIReqOps
> scsi: move request-related callbacks from SCSIDeviceInfo to SCSIReqOps
> scsi: pass cdb already to scsi_req_new
> scsi: introduce SCSICommand
> scsi: push lun field to SCSIDevice
> scsi: move request parsing to common code
> scsi: move handling of REPORT LUNS and invalid LUNs to common code
> scsi: move handling of REQUEST SENSE to common code
> scsi: add a bunch more common sense codes
> scsi: add support for unit attention conditions
> scsi: report unit attention on reset
> scsi: add special traces for common commands
>
> hw/esp.c | 4 +-
> hw/lsi53c895a.c | 4 +-
> hw/scsi-bus.c | 578 +++++++++++++++++++++++++++++++++++++++++++++--------
> hw/scsi-defs.h | 4 +
> hw/scsi-disk.c | 164 +++++-----------
> hw/scsi-generic.c | 156 ++++-----------
> hw/scsi.h | 72 +++++--
> hw/spapr_vscsi.c | 95 ++-------
> hw/usb-msd.c | 4 +-
> trace-events | 5 +
> 10 files changed, 668 insertions(+), 418 deletions(-)
>
^ permalink raw reply [flat|nested] 29+ messages in thread