[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 6/7] scsi: Update sense handling
From: |
Nicholas A. Bellinger |
Subject: |
[Qemu-devel] Re: [PATCH 6/7] scsi: Update sense handling |
Date: |
Wed, 16 Jun 2010 02:00:06 -0700 |
On Tue, 2010-06-15 at 17:16 +0200, Hannes Reinecke wrote:
> Each driver uses it's own means of storing the sense data, so the
> sense code should be stored within the driver-specific structure.
> And we should not allow a direct access to it but rather implement
> an accessor 'scsi_req_sense' to copy the sense data into a
> device-supplied buffer.
>
> Signed-off-by: Hannes Reinecke <address@hidden>
> ---
> hw/megasas.c | 52 +++++++++++++++++++++++++++++++++++++---------------
> hw/scsi-bus.c | 9 +++++++--
> hw/scsi-disk.c | 24 ++++++++++++++++++++----
> hw/scsi-generic.c | 18 +++++++++++++++++-
> hw/scsi.h | 4 +++-
> 5 files changed, 84 insertions(+), 23 deletions(-)
>
Makes sense to me, commited.
--nab
> diff --git a/hw/megasas.c b/hw/megasas.c
> index bc35566..a75872b 100644
> --- a/hw/megasas.c
> +++ b/hw/megasas.c
> @@ -180,23 +180,46 @@ static void megasas_unmap_sgl(struct megasas_cmd_t *cmd)
> /*
> * passthrough sense and io sense are at the same offset
> */
> -static void megasas_build_sense(struct megasas_cmd_t *cmd, SCSISense sense)
> +static int megasas_build_sense(struct megasas_cmd_t *cmd, uint8_t *sense_ptr,
> + uint8_t sense_len)
> {
> - uint8_t *sense_ptr;
> - uint8_t sense_len;
> target_phys_addr_t pa, pa_hi = 0, pa_lo;
> uint16_t flags = le16_to_cpu(cmd->frame->header.flags);
> int is_sense64 = (flags & MFI_FRAME_SENSE64) ? 1 : 0;
>
> - sense_ptr = qemu_mallocz(cmd->frame->header.sense_len);
> - sense_len = scsi_build_sense(SENSE_CODE(INVALID_OPCODE), sense_ptr,
> - cmd->frame->header.sense_len, 0);
> + if (sense_len > cmd->frame->header.sense_len)
> + sense_len = cmd->frame->header.sense_len;
> +
> pa_lo = le32_to_cpu(cmd->frame->pass.sense_addr_lo);
> if (is_sense64)
> pa_hi = le32_to_cpu(cmd->frame->pass.sense_addr_hi);
> pa = ((uint64_t) pa_hi << 32) | pa_lo;
> cpu_physical_memory_write(pa, sense_ptr, sense_len);
> - megasas_frame_set_sense_len(cmd->pa, sense_len);
> + cmd->frame->header.sense_len = sense_len;
> + return sense_len;
> +}
> +
> +static void megasas_write_sense(struct megasas_cmd_t *cmd, SCSISense sense)
> +{
> + uint8_t *sense_ptr;
> + uint8_t sense_len;
> +
> + sense_ptr = qemu_mallocz(cmd->frame->header.sense_len);
> + sense_len = scsi_build_sense(sense, sense_ptr,
> + cmd->frame->header.sense_len, 0);
> + megasas_build_sense(cmd, sense_ptr, sense_len);
> + qemu_free(sense_ptr);
> +}
> +
> +static void megasas_copy_sense(struct megasas_cmd_t *cmd)
> +{
> + uint8_t *sense_ptr;
> + uint8_t sense_len;
> +
> + sense_ptr = qemu_mallocz(cmd->frame->header.sense_len);
> + sense_len = scsi_req_sense(cmd->req, sense_ptr,
> + cmd->frame->header.sense_len);
> + megasas_build_sense(cmd, sense_ptr, sense_len);
> qemu_free(sense_ptr);
> }
>
> @@ -1124,8 +1147,8 @@ static int megasas_handle_scsi(MPTState *s, struct
> megasas_cmd_t *cmd, int is_lo
> mfi_frame_desc[cmd->frame->header.frame_cmd],
> cmd->frame->header.target_id, cmd->frame->header.lun_id,
> cmd->frame->header.cdb_len);
> - megasas_build_sense(cmd, SENSE_CODE(INVALID_OPCODE));
> - megasas_frame_set_scsi_status(cmd->pa, CHECK_CONDITION);
> + megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
> + cmd->frame->header.scsi_status = CHECK_CONDITION;
> s->event_count++;
> return MFI_STAT_SCSI_DONE_WITH_ERROR;
> }
> @@ -1176,8 +1199,8 @@ static int megasas_handle_io(MPTState *s, struct
> megasas_cmd_t *cmd)
> mfi_frame_desc[cmd->frame->header.frame_cmd],
> cmd->frame->header.target_id, cmd->frame->header.lun_id,
> cmd->frame->header.cdb_len);
> - megasas_build_sense(cmd, SENSE_CODE(INVALID_OPCODE));
> - megasas_frame_set_scsi_status(cmd->pa, CHECK_CONDITION);
> + megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
> + cmd->frame->header.scsi_status = CHECK_CONDITION;
> s->event_count++;
> return MFI_STAT_SCSI_DONE_WITH_ERROR;
> }
> @@ -1234,14 +1257,13 @@ static void megasas_command_complete(SCSIRequest *req)
> }
> } else {
> DPRINTF_IO("%s req %p cmd %p lun %p finished with status %x len
> %u\n",
> - mfi_frame_desc[cmd->frame->header.frame_cmd], req, cmd,
> cmd->sdev,
> - req->status, (unsigned)req->xferlen);
> + mfi_frame_desc[cmd->frame->header.frame_cmd], req, cmd,
> + cmd->sdev, req->status, (unsigned)req->xferlen);
> if (req->status != GOOD) {
> cmd_status = MFI_STAT_SCSI_DONE_WITH_ERROR;
> }
> if (req->status == CHECK_CONDITION) {
> - megasas_build_sense(cmd, cmd->sdev->sense);
> - scsi_dev_clear_sense(cmd->sdev);
> + megasas_copy_sense(cmd);
> }
>
> megasas_unmap_sgl(cmd);
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index c0e6dd3..b7c5178 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -200,9 +200,9 @@ int scsi_build_sense(SCSISense sense, uint8_t *buf, int
> len, int fixed)
> return len;
> }
>
> -void scsi_dev_clear_sense(SCSIDevice *dev)
> +int scsi_sense_valid(SCSISense sense)
> {
> - memset(&dev->sense, 0, sizeof(dev->sense));
> + return (sense.key != NO_SENSE);
> }
>
> SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> uint32_t lun)
> @@ -685,3 +685,8 @@ void scsi_req_put(SCSIRequest *req)
> {
> return req->dev->info->request_put(req);
> }
> +
> +int scsi_req_sense(SCSIRequest *req, uint8_t *buf, int len)
> +{
> + return req->dev->info->request_sense(req, buf, len);
> +}
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index fb3c189..a186874 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -67,6 +67,7 @@ struct SCSIDiskState
> uint32_t max_inflight;
> uint32_t max_xfer;
> uint32_t counts[32];
> + struct SCSISense sense;
> };
>
> /* fwd decls */
> @@ -91,8 +92,10 @@ static void scsi_remove_request(SCSIDiskReq *r)
>
> static void scsi_req_set_status(SCSIRequest *req, int status, SCSISense
> sense)
> {
> + SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
> +
> req->status = status;
> - req->dev->sense = sense;
> + s->sense = sense;
> }
>
> static void scsi_dma_restart_bh(void *opaque)
> @@ -534,9 +537,9 @@ static int scsi_disk_emulate_command(SCSIRequest *req,
> uint8_t *outbuf)
> case REQUEST_SENSE:
> if (req->cmd.xfer < 4)
> goto illegal_request;
> - buflen = scsi_build_sense(req->dev->sense, outbuf, req->cmd.xfer,
> - req->cmd.buf[1] & 0x1);
> - scsi_dev_clear_sense(req->dev);
> + buflen = scsi_build_sense(s->sense, outbuf, req->cmd.xfer,
> + req->cmd.buf[1] & 0x1);
> + memset(&s->sense, 0, sizeof(s->sense));
> break;
> case INQUIRY:
> buflen = scsi_disk_emulate_inquiry(req, outbuf);
> @@ -940,6 +943,18 @@ static void scsi_disk_req_put(SCSIRequest *req)
> scsi_remove_request(r);
> }
>
> +static int scsi_disk_req_sense(SCSIRequest *req, uint8_t *buf, int len)
> +{
> + SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
> + int n = 0;
> +
> + if (scsi_sense_valid(s->sense)) {
> + n = scsi_build_sense(s->sense, buf, len, 0);
> + memset(&s->sense, 0, sizeof(s->sense));
> + }
> + return n;
> +}
> +
> static SCSIDeviceInfo scsi_disk_info = {
> .qdev.name = "scsi-disk",
> .qdev.desc = "virtual scsi disk or cdrom",
> @@ -952,6 +967,7 @@ static SCSIDeviceInfo scsi_disk_info = {
> .request_buf = scsi_disk_req_buf,
> .request_sgl = scsi_disk_req_sgl,
> .request_put = scsi_disk_req_put,
> + .request_sense = scsi_disk_req_sense,
>
> .qdev.props = (Property[]) {
> DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> index fe58975..1776048 100644
> --- a/hw/scsi-generic.c
> +++ b/hw/scsi-generic.c
> @@ -98,7 +98,7 @@ static void scsi_command_complete(void *opaque, int ret)
> if (ret != 0) {
> if (ret == -EDOM) {
> /* sg driver uses EDOM to signal queue busy */
> - fprintf(stderr, "%s: sg queue busy\n", __FUNCTION__);
> + DPRINTF("%s: sg queue busy\n", __FUNCTION__);
> r->req.status = TASK_SET_FULL;
> } else {
> scsi_req_print(&r->req);
> @@ -523,6 +523,21 @@ static void scsi_generic_req_put(SCSIRequest *req)
> scsi_remove_request(r);
> }
>
> +static int scsi_generic_req_sense(SCSIRequest *req, uint8_t *buf, int len)
> +{
> + SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
> + SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r->req.dev);
> +
> + if (buf == NULL || !len || !s->senselen) {
> + return 0;
> + }
> + if (len > s->senselen) {
> + len = s->senselen;
> + }
> + memcpy(buf, s->sensebuf, len);
> + return len;
> +}
> +
> static SCSIDeviceInfo scsi_generic_info = {
> .qdev.name = "scsi-generic",
> .qdev.desc = "pass through generic scsi device (/dev/sg*)",
> @@ -535,6 +550,7 @@ static SCSIDeviceInfo scsi_generic_info = {
> .request_buf = scsi_generic_req_buf,
> .request_sgl = scsi_generic_req_sgl,
> .request_put = scsi_generic_req_put,
> + .request_sense = scsi_generic_req_sense,
>
> .qdev.props = (Property[]) {
> DEFINE_BLOCK_PROPERTIES(SCSIGenericState, qdev.conf),
> diff --git a/hw/scsi.h b/hw/scsi.h
> index 645fa45..1d5ce93 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -60,7 +60,6 @@ struct SCSIDevice
> QTAILQ_HEAD(, SCSIRequest) requests;
> int blocksize;
> int type;
> - struct SCSISense sense;
> };
>
> /* cdrom.c */
> @@ -81,6 +80,7 @@ struct SCSIDeviceInfo {
> int (*request_buf)(SCSIRequest *req, uint8_t *buffer);
> int (*request_sgl)(SCSIRequest *req, struct QEMUSGList *sg);
> void (*request_put)(SCSIRequest *req);
> + int (*request_sense)(SCSIRequest *req, uint8_t *buf, int len);
> };
>
> typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
> @@ -142,6 +142,8 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d,
> uint32_t tag, uint32_t l
> SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag);
> void scsi_req_free(SCSIRequest *req);
> void scsi_req_complete(SCSIRequest *req);
> +int scsi_req_sense(SCSIRequest *req, uint8_t *buf, int len);
> +int scsi_sense_valid(SCSISense sense);
>
> /* for debugging */
> const char *scsi_status_name(uint8_t status);