From ad2d30f79d3b0812f02c741be2189796b788d6d7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 18 Apr 2011 16:01:56 +0200 Subject: [PATCH] scsi: reference-count requests With the next patch, a device may hold SCSIRequest for an indefinite time. Split a rather big patch, and protect against access errors, by reference counting them. There is some ugliness in scsi_send_command implementation due to the need to unref the request when it fails. This will go away with the next patches, which move the unref'ing to the devices. Signed-off-by: Paolo Bonzini Cc: Christoph Hellwig --- hw/scsi-bus.c | 29 ++++++++++++++++++++++------- hw/scsi-disk.c | 23 +++++++++++++++-------- hw/scsi-generic.c | 24 ++++++++++++++++-------- hw/scsi.h | 5 +++++ 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 1850a87da..e7fd90362 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -136,6 +136,8 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l SCSIRequest *req; req = qemu_mallocz(size); + /* Two references: one is passed back to the HBA, one is in d->requests. */ + req->refcount = 2; req->bus = scsi_bus_from_device(d); req->dev = d; req->tag = tag; @@ -159,21 +161,16 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag) return NULL; } -static void scsi_req_dequeue(SCSIRequest *req) +void scsi_req_dequeue(SCSIRequest *req) { trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag); if (req->enqueued) { QTAILQ_REMOVE(&req->dev->requests, req, next); req->enqueued = false; + scsi_req_unref(req); } } -void scsi_req_free(SCSIRequest *req) -{ - scsi_req_dequeue(req); - qemu_free(req); -} - static int scsi_req_length(SCSIRequest *req, uint8_t *cmd) { switch (cmd[0] >> 5) { @@ -495,6 +492,22 @@ static const char *scsi_command_name(uint8_t cmd) return names[cmd]; } +SCSIRequest *scsi_req_ref(SCSIRequest *req) +{ + req->refcount++; + return req; +} + +void scsi_req_unref(SCSIRequest *req) +{ + if (--req->refcount == 0) { + if (req->dev->info->free_req) { + req->dev->info->free_req(req); + } + qemu_free(req); + } +} + /* Called by the devices when data is ready for the HBA. The HBA should start a DMA operation to read or fill the device's data buffer. Once it completes, calling one of req->dev->info->read_data or @@ -537,10 +550,12 @@ void scsi_req_print(SCSIRequest *req) void scsi_req_complete(SCSIRequest *req) { assert(req->status != -1); + scsi_req_ref(req); scsi_req_dequeue(req); req->bus->ops->complete(req->bus, SCSI_REASON_DONE, req->tag, req->status); + scsi_req_unref(req); } static char *scsibus_get_fw_dev_path(DeviceState *dev) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 741cf396b..87d7b9305 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -98,10 +98,11 @@ static SCSIDiskReq *scsi_new_request(SCSIDiskState *s, uint32_t tag, return r; } -static void scsi_remove_request(SCSIDiskReq *r) +static void scsi_free_request(SCSIRequest *req) { + SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req); + qemu_vfree(r->iov.iov_base); - scsi_req_free(&r->req); } static SCSIDiskReq *scsi_find_request(SCSIDiskState *s, uint32_t tag) @@ -134,7 +135,6 @@ static void scsi_command_complete(SCSIDiskReq *r, int status, int sense) r->req.tag, status, sense); scsi_req_set_status(r, status, sense); scsi_req_complete(&r->req); - scsi_remove_request(r); } /* Cancel a pending data transfer. */ @@ -148,7 +148,7 @@ static void scsi_cancel_io(SCSIDevice *d, uint32_t tag) if (r->req.aiocb) bdrv_aio_cancel(r->req.aiocb); r->req.aiocb = NULL; - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -1033,7 +1033,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, uint8_t *buf, int lun) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d); - uint32_t len; + int32_t len; int is_write; uint8_t command; uint8_t *outbuf; @@ -1095,6 +1095,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, case REZERO_UNIT: rc = scsi_disk_emulate_command(r, outbuf); if (rc < 0) { + scsi_req_unref(&r->req); return 0; } @@ -1181,9 +1182,11 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]); fail: scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST); + scsi_req_unref(&r->req); return 0; illegal_lba: scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR); + scsi_req_unref(&r->req); return 0; } if (r->sector_count == 0 && r->iov.iov_len == 0) { @@ -1191,12 +1194,13 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, } len = r->sector_count * 512 + r->iov.iov_len; if (is_write) { - return -len; + len = -len; } else { if (!r->sector_count) r->sector_count = -1; - return len; } + scsi_req_unref(&r->req); + return len; } static void scsi_disk_purge_requests(SCSIDiskState *s) @@ -1208,7 +1212,7 @@ static void scsi_disk_purge_requests(SCSIDiskState *s) if (r->req.aiocb) { bdrv_aio_cancel(r->req.aiocb); } - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -1321,6 +1325,7 @@ static SCSIDeviceInfo scsi_disk_info[] = { .qdev.reset = scsi_disk_reset, .init = scsi_hd_initfn, .destroy = scsi_destroy, + .free_req = scsi_free_request, .send_command = scsi_send_command, .read_data = scsi_read_data, .write_data = scsi_write_data, @@ -1339,6 +1344,7 @@ static SCSIDeviceInfo scsi_disk_info[] = { .qdev.reset = scsi_disk_reset, .init = scsi_cd_initfn, .destroy = scsi_destroy, + .free_req = scsi_free_request, .send_command = scsi_send_command, .read_data = scsi_read_data, .write_data = scsi_write_data, @@ -1356,6 +1362,7 @@ static SCSIDeviceInfo scsi_disk_info[] = { .qdev.reset = scsi_disk_reset, .init = scsi_disk_initfn, .destroy = scsi_destroy, + .free_req = scsi_free_request, .send_command = scsi_send_command, .read_data = scsi_read_data, .write_data = scsi_write_data, diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c index bd099835f..06e9dfea8 100644 --- a/hw/scsi-generic.c +++ b/hw/scsi-generic.c @@ -74,10 +74,11 @@ static SCSIGenericReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lu return DO_UPCAST(SCSIGenericReq, req, req); } -static void scsi_remove_request(SCSIGenericReq *r) +static void scsi_free_request(SCSIRequest *req) { + SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req); + qemu_free(r->buf); - scsi_req_free(&r->req); } static SCSIGenericReq *scsi_find_request(SCSIGenericState *s, uint32_t tag) @@ -113,7 +114,6 @@ static void scsi_command_complete(void *opaque, int ret) r, r->req.tag, r->req.status); scsi_req_complete(&r->req); - scsi_remove_request(r); } /* Cancel a pending data transfer. */ @@ -128,7 +128,7 @@ static void scsi_cancel_io(SCSIDevice *d, uint32_t tag) if (r->req.aiocb) bdrv_aio_cancel(r->req.aiocb); r->req.aiocb = NULL; - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -323,6 +323,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, SCSIGenericReq *r; SCSIBus *bus; int ret; + int32_t len; if (cmd[0] != REQUEST_SENSE && (lun != s->lun || (cmd[1] >> 5) != s->lun)) { @@ -351,7 +352,8 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, if (-1 == scsi_req_parse(&r->req, cmd)) { BADF("Unsupported command length, command %x\n", cmd[0]); - scsi_remove_request(r); + scsi_req_dequeue(&r->req); + scsi_req_unref(&r->req); return 0; } scsi_req_fixup(&r->req); @@ -377,8 +379,10 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, ret = execute_command(s->bs, r, SG_DXFER_NONE, scsi_command_complete); if (ret == -1) { scsi_command_complete(r, -EINVAL); + scsi_req_unref(&r->req); return 0; } + scsi_req_unref(&r->req); return 0; } @@ -393,10 +397,13 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, r->len = r->req.cmd.xfer; if (r->req.cmd.mode == SCSI_XFER_TO_DEV) { r->len = 0; - return -r->req.cmd.xfer; + len = -r->req.cmd.xfer; + } else { + len = r->req.cmd.xfer; } - return r->req.cmd.xfer; + scsi_req_unref(&r->req); + return len; } static int get_blocksize(BlockDriverState *bdrv) @@ -469,7 +476,7 @@ static void scsi_generic_purge_requests(SCSIGenericState *s) if (r->req.aiocb) { bdrv_aio_cancel(r->req.aiocb); } - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -561,6 +568,7 @@ static SCSIDeviceInfo scsi_generic_info = { .qdev.reset = scsi_generic_reset, .init = scsi_generic_initfn, .destroy = scsi_destroy, + .free_req = scsi_free_request, .send_command = scsi_send_command, .read_data = scsi_read_data, .write_data = scsi_write_data, diff --git a/hw/scsi.h b/hw/scsi.h index d4ecc9bec..a1d0e7451 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -29,6 +29,7 @@ enum SCSIXferMode { typedef struct SCSIRequest { SCSIBus *bus; SCSIDevice *dev; + uint32_t refcount; uint32_t tag; uint32_t lun; uint32_t status; @@ -65,6 +66,7 @@ struct SCSIDeviceInfo { DeviceInfo qdev; scsi_qdev_initfn init; void (*destroy)(SCSIDevice *s); + void (*free_req)(SCSIRequest *req); int32_t (*send_command)(SCSIDevice *s, uint32_t tag, uint8_t *buf, int lun); void (*read_data)(SCSIDevice *s, uint32_t tag); @@ -103,6 +105,9 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus); SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun); SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag); void scsi_req_free(SCSIRequest *req); +void scsi_req_dequeue(SCSIRequest *req); +SCSIRequest *scsi_req_ref(SCSIRequest *req); +void scsi_req_unref(SCSIRequest *req); int scsi_req_parse(SCSIRequest *req, uint8_t *buf); void scsi_req_print(SCSIRequest *req); -- 2.39.2