Message ID | 20100615151633.CC3F32B964@ochil.suse.de |
---|---|
State | New |
Headers | show |
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 <hare@suse.de> > --- > 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);
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);
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 <hare@suse.de> --- 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(-)