[Qemu-devel] [PATCH v3 21/21] scsi: split command_complete callback in two

[Paolo Bonzini <pbonzini@redhat.com>]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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 051b0fa..5a33c67 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->sense = arg;
-        s->rregs[ESP_RSTAT] = STAT_ST;
+    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->sense = 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 3f618fa..43de6f8 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;
@@ -2236,6 +2243,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 7daa112..57cfc87 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -633,7 +633,7 @@ void scsi_req_kick(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)
@@ -669,7 +669,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 7eed475..93e9d35 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 8a47de0..b195e06 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_kick(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_kick(req->sreq);
         return;
     }
 
@@ -559,6 +530,45 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
     scsi_req_kick(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 14e42e5..dc80014 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