[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two |
Date: |
Mon, 23 May 2011 22:38:25 +0300 |
On Mon, May 23, 2011 at 7:09 PM, Paolo Bonzini <address@hidden> wrote:
> Signed-off-by: Paolo Bonzini <address@hidden>
> Cc: Christoph Hellwig <address@hidden>
> ---
> hw/esp.c | 60 +++++++++++++++++---------------
> hw/lsi53c895a.c | 48 +++++++++++++++-----------
> hw/scsi-bus.c | 4 +-
> hw/scsi.h | 9 +----
> hw/spapr_vscsi.c | 101
> ++++++++++++++++++++++++++++++------------------------
> hw/usb-msd.c | 69 +++++++++++++++++++++----------------
> 6 files changed, 159 insertions(+), 132 deletions(-)
>
> diff --git a/hw/esp.c b/hw/esp.c
> index 0b80529..32ed327 100644
> --- a/hw/esp.c
> +++ b/hw/esp.c
> @@ -395,38 +395,41 @@ static void esp_do_dma(ESPState *s)
> esp_dma_done(s);
> }
>
> -static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +static void esp_command_complete(SCSIRequest *req, uint32_t arg)
> {
> ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
>
> - if (reason == SCSI_REASON_DONE) {
> - DPRINTF("SCSI Command complete\n");
> - if (s->ti_size != 0)
> - DPRINTF("SCSI command completed unexpectedly\n");
> - s->ti_size = 0;
> - s->dma_left = 0;
> - s->async_len = 0;
> - if (arg)
> - DPRINTF("Command failed\n");
> - s->status = arg;
> - s->rregs[ESP_RSTAT] = STAT_ST;
> + DPRINTF("SCSI Command complete\n");
> + if (s->ti_size != 0)
While touching the code, please add braces.
> + DPRINTF("SCSI command completed unexpectedly\n");
> + s->ti_size = 0;
> + s->dma_left = 0;
> + s->async_len = 0;
> + if (arg)
> + DPRINTF("Command failed\n");
> + s->status = arg;
> + s->rregs[ESP_RSTAT] = STAT_ST;
> + esp_dma_done(s);
> + if (s->current_req) {
> + scsi_req_unref(s->current_req);
> + s->current_req = NULL;
> + s->current_dev = NULL;
> + }
> +}
> +
> +static void esp_transfer_data(SCSIRequest *req, uint32_t arg)
> +{
> + ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
> +
> + DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
> + s->async_len = arg;
> + s->async_buf = scsi_req_get_buf(req);
> + if (s->dma_left) {
> + esp_do_dma(s);
> + } else if (s->dma_counter != 0 && s->ti_size <= 0) {
> + /* If this was the last part of a DMA transfer then the
> + completion interrupt is deferred to here. */
> esp_dma_done(s);
> - if (s->current_req) {
> - scsi_req_unref(s->current_req);
> - s->current_req = NULL;
> - s->current_dev = NULL;
> - }
> - } else {
> - DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
> - s->async_len = arg;
> - s->async_buf = scsi_req_get_buf(req);
> - if (s->dma_left) {
> - esp_do_dma(s);
> - } else if (s->dma_counter != 0 && s->ti_size <= 0) {
> - /* If this was the last part of a DMA transfer then the
> - completion interrupt is deferred to here. */
> - esp_dma_done(s);
> - }
> }
> }
>
> @@ -725,6 +728,7 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
> }
>
> static struct SCSIBusOps esp_scsi_ops = {
> + .transfer_data = esp_transfer_data,
> .complete = esp_command_complete,
> .cancel = esp_request_cancelled
> };
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index a6c7c5b..62fe191 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -711,40 +711,47 @@ static int lsi_queue_tag(LSIState *s, uint32_t tag,
> uint32_t arg)
> return 1;
> }
> }
> - /* Callback to indicate that the SCSI layer has completed a transfer. */
> -static void lsi_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +
> + /* Callback to indicate that the SCSI layer has completed a command. */
> +static void lsi_command_complete(SCSIRequest *req, uint32_t arg)
> {
> LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> int out;
>
> out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
> - if (reason == SCSI_REASON_DONE) {
> - DPRINTF("Command complete status=%d\n", (int)arg);
> - s->status = arg;
> - s->command_complete = 2;
> - if (s->waiting && s->dbc != 0) {
> - /* Raise phase mismatch for short transfers. */
> - lsi_bad_phase(s, out, PHASE_ST);
> - } else {
> - lsi_set_phase(s, PHASE_ST);
> - }
> + DPRINTF("Command complete status=%d\n", (int)arg);
> + s->status = arg;
> + s->command_complete = 2;
> + if (s->waiting && s->dbc != 0) {
> + /* Raise phase mismatch for short transfers. */
> + lsi_bad_phase(s, out, PHASE_ST);
> + } else {
> + lsi_set_phase(s, PHASE_ST);
> + }
>
> - if (req == s->current->req) {
> - scsi_req_unref(s->current->req);
> - qemu_free(s->current);
> - s->current = NULL;
> - }
> - lsi_resume_script(s);
> - return;
> + if (req == s->current->req) {
> + scsi_req_unref(s->current->req);
> + qemu_free(s->current);
> + s->current = NULL;
> }
> + lsi_resume_script(s);
> +}
> +
> + /* Callback to indicate that the SCSI layer has completed a transfer. */
> +static void lsi_transfer_data(SCSIRequest *req, uint32_t arg)
> +{
> + LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> + int out;
>
> if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
> (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
> if (lsi_queue_tag(s, req->tag, arg)) {
> return;
> - }
> + }
> }
>
> + out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
> +
> /* host adapter (re)connected */
> DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, arg);
> s->current->dma_len = arg;
> @@ -2239,6 +2246,7 @@ static int lsi_scsi_uninit(PCIDevice *d)
> }
>
> static struct SCSIBusOps lsi_scsi_ops = {
> + .transfer_data = lsi_transfer_data,
> .complete = lsi_command_complete,
> .cancel = lsi_request_cancelled
> };
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index d4fd61d..086a998 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -628,7 +628,7 @@ void scsi_req_continue(SCSIRequest *req)
> void scsi_req_data(SCSIRequest *req, int len)
> {
> trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
> - req->bus->ops->complete(req, SCSI_REASON_DATA, len);
> + req->bus->ops->transfer_data(req, len);
> }
>
> void scsi_req_print(SCSIRequest *req)
> @@ -664,7 +664,7 @@ void scsi_req_complete(SCSIRequest *req)
> assert(req->status != -1);
> scsi_req_ref(req);
> scsi_req_dequeue(req);
> - req->bus->ops->complete(req, SCSI_REASON_DONE, req->status);
> + req->bus->ops->complete(req, req->status);
> scsi_req_unref(req);
> }
>
> diff --git a/hw/scsi.h b/hw/scsi.h
> index fa9c9fa..70f9e89 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -9,12 +9,6 @@
>
> #define SCSI_CMD_BUF_SIZE 16
>
> -/* scsi-disk.c */
> -enum scsi_reason {
> - SCSI_REASON_DONE, /* Command complete. */
> - SCSI_REASON_DATA /* Transfer complete, more data required. */
> -};
> -
> typedef struct SCSIBus SCSIBus;
> typedef struct SCSIBusOps SCSIBusOps;
> typedef struct SCSIDevice SCSIDevice;
> @@ -84,7 +78,8 @@ struct SCSIDeviceInfo {
> };
>
> struct SCSIBusOps {
> - void (*complete)(SCSIRequest *req, int reason, uint32_t arg);
> + void (*transfer_data)(SCSIRequest *req, uint32_t arg);
> + void (*complete)(SCSIRequest *req, uint32_t arg);
> void (*cancel)(SCSIRequest *req);
> };
>
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index f0e08e3..d84e10d 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -480,63 +480,34 @@ static void vscsi_send_request_sense(VSCSIState *s,
> vscsi_req *req)
> }
>
> /* Callback to indicate that the SCSI layer has completed a transfer. */
> -static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t
> arg)
> +static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
> {
> VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
> vscsi_req *req = vscsi_find_req(s, sreq);
> uint8_t *buf;
> - int32_t res_in = 0, res_out = 0;
> int len, rc = 0;
>
> - dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
> - reason, sreq->tag, arg, req);
> + dprintf("VSCSI: SCSI xfer complete tag=0x%x arg=0x%x, req=%p\n",
> + sreq->tag, arg, req);
> if (req == NULL) {
> fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n",
> sreq->tag);
> return;
> }
>
> if (req->sensing) {
> - if (reason == SCSI_REASON_DONE) {
> - dprintf("VSCSI: Sense done !\n");
> - vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
> - vscsi_put_req(s, req);
> - } else {
> - uint8_t *buf = scsi_req_get_buf(sreq);
> -
> - len = MIN(arg, 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 (reason == SCSI_REASON_DONE) {
> - dprintf("VSCSI: Command complete err=%d\n", arg);
> - if (arg == 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);
> - } else if (arg == CHECK_CONDITION) {
> - vscsi_send_request_sense(s, req);
> - return;
> - } else {
> - vscsi_send_rsp(s, req, arg, 0, 0);
> - }
> - vscsi_put_req(s, req);
> + uint8_t *buf = scsi_req_get_buf(sreq);
> +
> + len = MIN(arg, 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;
> }
>
> @@ -559,6 +530,45 @@ static void vscsi_command_complete(SCSIRequest *sreq,
> int reason, uint32_t arg)
> scsi_req_continue(sreq);
> }
>
> +/* Callback to indicate that the SCSI layer has completed a transfer. */
> +static void vscsi_command_complete(SCSIRequest *sreq, uint32_t arg)
> +{
> + VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
> + vscsi_req *req = vscsi_find_req(s, sreq);
> + int32_t res_in = 0, res_out = 0;
> +
> + dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
> + reason, sreq->tag, arg, req);
> + if (req == NULL) {
> + fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n",
> sreq->tag);
> + return;
> + }
> +
> + if (!req->sensing && arg == CHECK_CONDITION) {
> + vscsi_send_request_sense(s, req);
> + return;
> + }
> +
> + if (req->sensing) {
> + dprintf("VSCSI: Sense done !\n");
> + arg = CHECK_CONDITION;
> + } else {
> + dprintf("VSCSI: Command complete err=%d\n", arg);
> + if (arg == 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_put_req(s, req);
> +}
> +
> static void vscsi_request_cancelled(SCSIRequest *sreq)
> {
> VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
> @@ -916,6 +926,7 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev,
> uint8_t *crq_data)
> }
>
> static struct SCSIBusOps vscsi_scsi_ops = {
> + .transfer_data = vscsi_transfer_data,
> .complete = vscsi_command_complete,
> .cancel = vscsi_request_cancelled
> };
> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
> index 27a37fa..c808815 100644
> --- a/hw/usb-msd.c
> +++ b/hw/usb-msd.c
> @@ -208,7 +208,7 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
> memcpy(p->data, &csw, len);
> }
>
> -static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t
> arg)
> +static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
> {
> MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> USBPacket *p = s->packet;
> @@ -216,35 +216,7 @@ static void usb_msd_command_complete(SCSIRequest *req,
> int reason, uint32_t arg)
> if (req->tag != s->tag) {
> fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
> }
> - if (reason == SCSI_REASON_DONE) {
> - DPRINTF("Command complete %d\n", arg);
> - s->residue = s->data_len;
> - s->result = arg != 0;
> - if (s->packet) {
> - if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
> - /* A deferred packet with no write data remaining must be
> - the status read packet. */
> - usb_msd_send_status(s, p);
> - s->mode = USB_MSDM_CBW;
> - } else {
> - if (s->data_len) {
> - s->data_len -= s->usb_len;
> - if (s->mode == USB_MSDM_DATAIN)
> - memset(s->usb_buf, 0, s->usb_len);
> - s->usb_len = 0;
> - }
> - if (s->data_len == 0)
> - s->mode = USB_MSDM_CSW;
> - }
> - s->packet = NULL;
> - usb_packet_complete(&s->dev, p);
> - } else if (s->data_len == 0) {
> - s->mode = USB_MSDM_CSW;
> - }
> - scsi_req_unref(req);
> - s->req = NULL;
> - return;
> - }
> +
> assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode ==
> SCSI_XFER_TO_DEV));
> s->scsi_len = arg;
> s->scsi_buf = scsi_req_get_buf(req);
> @@ -261,6 +233,42 @@ static void usb_msd_command_complete(SCSIRequest *req,
> int reason, uint32_t arg)
> }
> }
>
> +static void usb_msd_command_complete(SCSIRequest *req, uint32_t arg)
> +{
> + MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> + USBPacket *p = s->packet;
> +
> + if (req->tag != s->tag) {
> + fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
> + }
> + DPRINTF("Command complete %d\n", arg);
> + s->residue = s->data_len;
> + s->result = arg != 0;
> + if (s->packet) {
> + if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
> + /* A deferred packet with no write data remaining must be
> + the status read packet. */
> + usb_msd_send_status(s, p);
> + s->mode = USB_MSDM_CBW;
> + } else {
> + if (s->data_len) {
> + s->data_len -= s->usb_len;
> + if (s->mode == USB_MSDM_DATAIN)
> + memset(s->usb_buf, 0, s->usb_len);
> + s->usb_len = 0;
> + }
> + if (s->data_len == 0)
> + s->mode = USB_MSDM_CSW;
> + }
> + s->packet = NULL;
> + usb_packet_complete(&s->dev, p);
> + } else if (s->data_len == 0) {
> + s->mode = USB_MSDM_CSW;
> + }
> + scsi_req_unref(req);
> + s->req = NULL;
> +}
> +
> static void usb_msd_request_cancelled(SCSIRequest *req)
> {
> MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> @@ -494,6 +502,7 @@ static void usb_msd_password_cb(void *opaque, int err)
> }
>
> static struct SCSIBusOps usb_msd_scsi_ops = {
> + .transfer_data = usb_msd_transfer_data,
> .complete = usb_msd_command_complete,
> .cancel = usb_msd_request_cancelled
> };
> --
> 1.7.4.4
>
>
>
>
- [Qemu-devel] [PATCH v4 14/24] scsi: introduce scsi_req_new, (continued)
- [Qemu-devel] [PATCH v4 14/24] scsi: introduce scsi_req_new, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 13/24] scsi: do not call send_command directly, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 15/24] scsi: introduce scsi_req_continue, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 16/24] scsi: introduce scsi_req_get_buf, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 17/24] scsi: Implement 'get_sense' callback, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 18/24] scsi-disk: add data direction checking, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 19/24] scsi: make write_data return void, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 20/24] scsi-generic: Handle queue full, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 21/24] esp: rename sense to status, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two, Paolo Bonzini, 2011/05/23
- Re: [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two,
Blue Swirl <=
- [Qemu-devel] [PATCH v4 23/24] scsi: rename arguments to the new callbacks, Paolo Bonzini, 2011/05/23
- [Qemu-devel] [PATCH v4 24/24] scsi: ignore LUN field in the CDB, Paolo Bonzini, 2011/05/23