[Qemu-devel] [PATCH 1/2] SCSI cleanup

[Laurent Vivier]

This patch reworks the interface between SCSI controller and SCSI device.
It allows to connect something else than scsi-disk.c to the SCSI controller.

It is needed to use SCSI passthrough patch.

Laurent
---
 hw/esp.c        |   16 +++++++-------
 hw/lsi53c895a.c |   41 ++++++++++++++++++++++++-------------
 hw/scsi-disk.c  |   62 ++++++++++++++++++++++++++++++++++----------------------
 hw/scsi-disk.h  |   27 ++++++++++++------------
 hw/usb-msd.c    |   16 +++++++-------
 5 files changed, 95 insertions(+), 67 deletions(-)

Index: qemu/hw/scsi-disk.c
===================================================================
--- qemu.orig/hw/scsi-disk.c    2007-12-07 09:49:32.000000000 +0100
+++ qemu/hw/scsi-disk.c 2007-12-07 09:49:40.000000000 +0100
@@ -37,7 +37,7 @@ do { fprintf(stderr, "scsi-disk: " fmt ,
 #define SCSI_DMA_BUF_SIZE    65536
 
 typedef struct SCSIRequest {
-    SCSIDevice *dev;
+    SCSIDeviceState *dev;
     uint32_t tag;
     /* ??? We should probably keep track of whether the data trasfer is
        a read or a write.  Currently we rely on the host getting it right.  */
@@ -51,7 +51,7 @@ typedef struct SCSIRequest {
     struct SCSIRequest *next;
 } SCSIRequest;
 
-struct SCSIDevice
+struct SCSIDeviceState
 {
     BlockDriverState *bdrv;
     SCSIRequest *requests;
@@ -69,7 +69,7 @@ struct SCSIDevice
 /* Global pool of SCSIRequest structures.  */
 static SCSIRequest *free_requests = NULL;
 
-static SCSIRequest *scsi_new_request(SCSIDevice *s, uint32_t tag)
+static SCSIRequest *scsi_new_request(SCSIDeviceState *s, uint32_t tag)
 {
     SCSIRequest *r;
 
@@ -94,7 +94,7 @@ static SCSIRequest *scsi_new_request(SCS
 static void scsi_remove_request(SCSIRequest *r)
 {
     SCSIRequest *last;
-    SCSIDevice *s = r->dev;
+    SCSIDeviceState *s = r->dev;
 
     if (s->requests == r) {
         s->requests = r->next;
@@ -112,7 +112,7 @@ static void scsi_remove_request(SCSIRequ
     free_requests = r;
 }
 
-static SCSIRequest *scsi_find_request(SCSIDevice *s, uint32_t tag)
+static SCSIRequest *scsi_find_request(SCSIDeviceState *s, uint32_t tag)
 {
     SCSIRequest *r;
 
@@ -126,7 +126,7 @@ static SCSIRequest *scsi_find_request(SC
 /* Helper function for command completion.  */
 static void scsi_command_complete(SCSIRequest *r, int sense)
 {
-    SCSIDevice *s = r->dev;
+    SCSIDeviceState *s = r->dev;
     uint32_t tag;
     DPRINTF("Command complete tag=0x%x sense=%d\n", r->tag, sense);
     s->sense = sense;
@@ -136,8 +136,9 @@ static void scsi_command_complete(SCSIRe
 }
 
 /* Cancel a pending data transfer.  */
-void scsi_cancel_io(SCSIDevice *s, uint32_t tag)
+static void scsi_cancel_io(SCSIDevice *d, uint32_t tag)
 {
+    SCSIDeviceState *s = d->state;
     SCSIRequest *r;
     DPRINTF("Cancel tag=0x%x\n", tag);
     r = scsi_find_request(s, tag);
@@ -152,7 +153,7 @@ void scsi_cancel_io(SCSIDevice *s, uint3
 static void scsi_read_complete(void * opaque, int ret)
 {
     SCSIRequest *r = (SCSIRequest *)opaque;
-    SCSIDevice *s = r->dev;
+    SCSIDeviceState *s = r->dev;
 
     if (ret) {
         DPRINTF("IO error\n");
@@ -165,8 +166,9 @@ static void scsi_read_complete(void * op
 }
 
 /* Read more data from scsi device into buffer.  */
-void scsi_read_data(SCSIDevice *s, uint32_t tag)
+static void scsi_read_data(SCSIDevice *d, uint32_t tag)
 {
+    SCSIDeviceState *s = d->state;
     SCSIRequest *r;
     uint32_t n;
 
@@ -205,7 +207,7 @@ void scsi_read_data(SCSIDevice *s, uint3
 static void scsi_write_complete(void * opaque, int ret)
 {
     SCSIRequest *r = (SCSIRequest *)opaque;
-    SCSIDevice *s = r->dev;
+    SCSIDeviceState *s = r->dev;
     uint32_t len;
 
     if (ret) {
@@ -229,8 +231,9 @@ static void scsi_write_complete(void * o
 
 /* Write data to a scsi device.  Returns nonzero on failure.
    The transfer may complete asynchronously.  */
-int scsi_write_data(SCSIDevice *s, uint32_t tag)
+static int scsi_write_data(SCSIDevice *d, uint32_t tag)
 {
+    SCSIDeviceState *s = d->state;
     SCSIRequest *r;
     uint32_t n;
 
@@ -260,8 +263,9 @@ int scsi_write_data(SCSIDevice *s, uint3
 }
 
 /* Return a pointer to the data buffer.  */
-uint8_t *scsi_get_buf(SCSIDevice *s, uint32_t tag)
+static uint8_t *scsi_get_buf(SCSIDevice *d, uint32_t tag)
 {
+    SCSIDeviceState *s = d->state;
     SCSIRequest *r;
 
     r = scsi_find_request(s, tag);
@@ -277,8 +281,10 @@ uint8_t *scsi_get_buf(SCSIDevice *s, uin
    (eg. disk reads), negative for transfers to the device (eg. disk writes),
    and zero if the command does not transfer any data.  */
 
-int32_t scsi_send_command(SCSIDevice *s, uint32_t tag, uint8_t *buf, int lun)
+static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
+                                 uint8_t *buf, int lun)
 {
+    SCSIDeviceState *s = d->state;
     int64_t nb_sectors;
     uint32_t lba;
     uint32_t len;
@@ -292,7 +298,7 @@ int32_t scsi_send_command(SCSIDevice *s,
     r = scsi_find_request(s, tag);
     if (r) {
         BADF("Tag 0x%x already in use\n", tag);
-        scsi_cancel_io(s, tag);
+        scsi_cancel_io(d, tag);
     }
     /* ??? Tags are not unique for different luns.  We only implement a
        single lun, so this should not matter.  */
@@ -577,19 +583,19 @@ int32_t scsi_send_command(SCSIDevice *s,
     }
 }
 
-void scsi_disk_destroy(SCSIDevice *s)
+static void scsi_destroy(SCSIDevice *d)
 {
-    qemu_free(s);
+    qemu_free(d->state);
+    qemu_free(d);
 }
 
-SCSIDevice *scsi_disk_init(BlockDriverState *bdrv,
-                           int tcq,
-                           scsi_completionfn completion,
-                           void *opaque)
+SCSIDevice *scsi_disk_init(BlockDriverState *bdrv, int tcq,
+                           scsi_completionfn completion, void *opaque)
 {
-    SCSIDevice *s;
+    SCSIDevice *d;
+    SCSIDeviceState *s;
 
-    s = (SCSIDevice *)qemu_mallocz(sizeof(SCSIDevice));
+    s = (SCSIDeviceState *)qemu_mallocz(sizeof(SCSIDeviceState));
     s->bdrv = bdrv;
     s->tcq = tcq;
     s->completion = completion;
@@ -600,6 +606,14 @@ SCSIDevice *scsi_disk_init(BlockDriverSt
         s->cluster_size = 1;
     }
 
-    return s;
-}
+    d = (SCSIDevice *)qemu_mallocz(sizeof(SCSIDevice));
+    d->state = s;
+    d->destroy = scsi_destroy;
+    d->send_command = scsi_send_command;
+    d->read_data = scsi_read_data;
+    d->write_data = scsi_write_data;
+    d->cancel_io = scsi_cancel_io;
+    d->get_buf = scsi_get_buf;
 
+    return d;
+}
Index: qemu/hw/esp.c
===================================================================
--- qemu.orig/hw/esp.c  2007-12-07 09:48:46.000000000 +0100
+++ qemu/hw/esp.c       2007-12-07 09:49:40.000000000 +0100
@@ -165,7 +165,7 @@ static int get_cmd(ESPState *s, uint8_t 
 
     if (s->current_dev) {
         /* Started a new command before the old one finished.  Cancel it.  */
-        scsi_cancel_io(s->current_dev, 0);
+        s->current_dev->cancel_io(s->current_dev, 0);
         s->async_len = 0;
     }
 
@@ -188,7 +188,7 @@ static void do_cmd(ESPState *s, uint8_t 
 
     DPRINTF("do_cmd: busid 0x%x\n", buf[0]);
     lun = buf[0] & 7;
-    datalen = scsi_send_command(s->current_dev, 0, &buf[1], lun);
+    datalen = s->current_dev->send_command(s->current_dev, 0, &buf[1], lun);
     s->ti_size = datalen;
     if (datalen != 0) {
         s->rregs[ESP_RSTAT] = STAT_IN | STAT_TC;
@@ -196,10 +196,10 @@ static void do_cmd(ESPState *s, uint8_t 
         s->dma_counter = 0;
         if (datalen > 0) {
             s->rregs[ESP_RSTAT] |= STAT_DI;
-            scsi_read_data(s->current_dev, 0);
+            s->current_dev->read_data(s->current_dev, 0);
         } else {
             s->rregs[ESP_RSTAT] |= STAT_DO;
-            scsi_write_data(s->current_dev, 0);
+            s->current_dev->write_data(s->current_dev, 0);
         }
     }
     s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
@@ -298,9 +298,9 @@ static void esp_do_dma(ESPState *s)
     if (s->async_len == 0) {
         if (to_device) {
             // ti_size is negative
-            scsi_write_data(s->current_dev, 0);
+            s->current_dev->write_data(s->current_dev, 0);
         } else {
-            scsi_read_data(s->current_dev, 0);
+            s->current_dev->read_data(s->current_dev, 0);
             /* If there is still data to be read from the device then
                complete the DMA operation immeriately.  Otherwise defer
                until the scsi layer has completed.  */
@@ -335,7 +335,7 @@ static void esp_command_complete(void *o
     } else {
         DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
         s->async_len = arg;
-        s->async_buf = scsi_get_buf(s->current_dev, 0);
+        s->async_buf = s->current_dev->get_buf(s->current_dev, 0);
         if (s->dma_left) {
             esp_do_dma(s);
         } else if (s->dma_counter != 0 && s->ti_size <= 0) {
@@ -611,7 +611,7 @@ void esp_scsi_attach(void *opaque, Block
     }
     if (s->scsi_dev[id]) {
         DPRINTF("Destroying device %d\n", id);
-        scsi_disk_destroy(s->scsi_dev[id]);
+        s->scsi_dev[id]->destroy(s->scsi_dev[id]);
     }
     DPRINTF("Attaching block device %d\n", id);
     /* Command queueing is not implemented.  */
Index: qemu/hw/lsi53c895a.c
===================================================================
--- qemu.orig/hw/lsi53c895a.c   2007-12-07 09:48:46.000000000 +0100
+++ qemu/hw/lsi53c895a.c        2007-12-07 09:49:40.000000000 +0100
@@ -187,6 +187,7 @@ typedef struct {
     /* The tag is a combination of the device ID and the SCSI tag.  */
     uint32_t current_tag;
     uint32_t current_dma_len;
+    int command_complete;
     uint8_t *dma_buf;
     lsi_queue *queue;
     int queue_len;
@@ -465,7 +466,8 @@ static void lsi_do_dma(LSIState *s, int 
     s->dbc -= count;
 
     if (s->dma_buf == NULL) {
-        s->dma_buf = scsi_get_buf(s->current_dev, s->current_tag);
+        s->dma_buf = s->current_dev->get_buf(s->current_dev,
+                                             s->current_tag);
     }
 
     /* ??? Set SFBR to first data byte.  */
@@ -479,10 +481,10 @@ static void lsi_do_dma(LSIState *s, int 
         s->dma_buf = NULL;
         if (out) {
             /* Write the data.  */
-            scsi_write_data(s->current_dev, s->current_tag);
+            s->current_dev->write_data(s->current_dev, s->current_tag);
         } else {
             /* Request any remaining data.  */
-            scsi_read_data(s->current_dev, s->current_tag);
+            s->current_dev->read_data(s->current_dev, s->current_tag);
         }
     } else {
         s->dma_buf += count;
@@ -596,6 +598,7 @@ static void lsi_command_complete(void *o
     if (reason == SCSI_REASON_DONE) {
         DPRINTF("Command complete sense=%d\n", (int)arg);
         s->sense = arg;
+        s->command_complete = 2;
         if (s->waiting && s->dbc != 0) {
             /* Raise phase mismatch for short transfers.  */
             lsi_bad_phase(s, out, PHASE_ST);
@@ -612,6 +615,7 @@ static void lsi_command_complete(void *o
     }
     DPRINTF("Data ready tag=0x%x len=%d\n", tag, arg);
     s->current_dma_len = arg;
+    s->command_complete = 1;
     if (!s->waiting)
         return;
     if (s->waiting == 1 || s->dbc == 0) {
@@ -631,21 +635,30 @@ static void lsi_do_command(LSIState *s)
         s->dbc = 16;
     cpu_physical_memory_read(s->dnad, buf, s->dbc);
     s->sfbr = buf[0];
-    n = scsi_send_command(s->current_dev, s->current_tag, buf, s->current_lun);
+    s->command_complete = 0;
+    n = s->current_dev->send_command(s->current_dev, s->current_tag, buf,
+                                     s->current_lun);
     if (n > 0) {
         lsi_set_phase(s, PHASE_DI);
-        scsi_read_data(s->current_dev, s->current_tag);
+        s->current_dev->read_data(s->current_dev, s->current_tag);
     } else if (n < 0) {
         lsi_set_phase(s, PHASE_DO);
-        scsi_write_data(s->current_dev, s->current_tag);
+        s->current_dev->write_data(s->current_dev, s->current_tag);
     }
-    if (n && s->current_dma_len == 0) {
-        /* Command did not complete immediately so disconnect.  */
-        lsi_add_msg_byte(s, 2); /* SAVE DATA POINTER */
-        lsi_add_msg_byte(s, 4); /* DISCONNECT */
-        lsi_set_phase(s, PHASE_MI);
-        s->msg_action = 1;
-        lsi_queue_command(s);
+
+    if (!s->command_complete) {
+        if (n) {
+            /* Command did not complete immediately so disconnect.  */
+            lsi_add_msg_byte(s, 2); /* SAVE DATA POINTER */
+            lsi_add_msg_byte(s, 4); /* DISCONNECT */
+            /* wait data */
+            lsi_set_phase(s, PHASE_MI);
+            s->msg_action = 1;
+            lsi_queue_command(s);
+        } else {
+            /* wait command complete */
+            lsi_set_phase(s, PHASE_DI);
+        }
     }
 }
 
@@ -1822,7 +1835,7 @@ void lsi_scsi_attach(void *opaque, Block
     }
     if (s->scsi_dev[id]) {
         DPRINTF("Destroying device %d\n", id);
-        scsi_disk_destroy(s->scsi_dev[id]);
+        s->scsi_dev[id]->destroy(s->scsi_dev[id]);
     }
     DPRINTF("Attaching block device %d\n", id);
     s->scsi_dev[id] = scsi_disk_init(bd, 1, lsi_command_complete, s);
Index: qemu/hw/usb-msd.c
===================================================================
--- qemu.orig/hw/usb-msd.c      2007-12-07 09:48:46.000000000 +0100
+++ qemu/hw/usb-msd.c   2007-12-07 09:49:40.000000000 +0100
@@ -149,9 +149,9 @@ static void usb_msd_copy_data(MSDState *
     s->data_len -= len;
     if (s->scsi_len == 0) {
         if (s->mode == USB_MSDM_DATAIN) {
-            scsi_read_data(s->scsi_dev, s->tag);
+            s->scsi_dev->read_data(s->scsi_dev, s->tag);
         } else if (s->mode == USB_MSDM_DATAOUT) {
-            scsi_write_data(s->scsi_dev, s->tag);
+            s->scsi_dev->write_data(s->scsi_dev, s->tag);
         }
     }
 }
@@ -204,7 +204,7 @@ static void usb_msd_command_complete(voi
         return;
     }
     s->scsi_len = arg;
-    s->scsi_buf = scsi_get_buf(s->scsi_dev, tag);
+    s->scsi_buf = s->scsi_dev->get_buf(s->scsi_dev, tag);
     if (p) {
         usb_msd_copy_data(s);
         if (s->usb_len == 0) {
@@ -342,7 +342,7 @@ static int usb_msd_handle_control(USBDev
 static void usb_msd_cancel_io(USBPacket *p, void *opaque)
 {
     MSDState *s = opaque;
-    scsi_cancel_io(s->scsi_dev, s->tag);
+    s->scsi_dev->cancel_io(s->scsi_dev, s->tag);
     s->packet = NULL;
     s->scsi_len = 0;
 }
@@ -390,14 +390,14 @@ static int usb_msd_handle_data(USBDevice
             DPRINTF("Command tag 0x%x flags %08x len %d data %d\n",
                     s->tag, cbw.flags, cbw.cmd_len, s->data_len);
             s->residue = 0;
-            scsi_send_command(s->scsi_dev, s->tag, cbw.cmd, 0);
+            s->scsi_dev->send_command(s->scsi_dev, s->tag, cbw.cmd, 0);
             /* ??? Should check that USB and SCSI data transfer
                directions match.  */
             if (s->residue == 0) {
                 if (s->mode == USB_MSDM_DATAIN) {
-                    scsi_read_data(s->scsi_dev, s->tag);
+                    s->scsi_dev->read_data(s->scsi_dev, s->tag);
                 } else if (s->mode == USB_MSDM_DATAOUT) {
-                    scsi_write_data(s->scsi_dev, s->tag);
+                    s->scsi_dev->write_data(s->scsi_dev, s->tag);
                 }
             }
             ret = len;
@@ -508,7 +508,7 @@ static void usb_msd_handle_destroy(USBDe
 {
     MSDState *s = (MSDState *)dev;
 
-    scsi_disk_destroy(s->scsi_dev);
+    s->scsi_dev->destroy(s->scsi_dev);
     bdrv_delete(s->bs);
     qemu_free(s);
 }
Index: qemu/hw/scsi-disk.h
===================================================================
--- qemu.orig/hw/scsi-disk.h    2007-12-07 09:48:46.000000000 +0100
+++ qemu/hw/scsi-disk.h 2007-12-07 09:49:40.000000000 +0100
@@ -7,24 +7,25 @@ enum scsi_reason {
     SCSI_REASON_DATA  /* Transfer complete, more data required.  */
 };
 
+typedef struct SCSIDeviceState SCSIDeviceState;
 typedef struct SCSIDevice SCSIDevice;
 typedef void (*scsi_completionfn)(void *opaque, int reason, uint32_t tag,
                                   uint32_t arg);
 
-SCSIDevice *scsi_disk_init(BlockDriverState *bdrv,
-                           int tcq,
-                           scsi_completionfn completion,
-                           void *opaque);
-void scsi_disk_destroy(SCSIDevice *s);
+struct SCSIDevice
+{
+    SCSIDeviceState *state;
+    void (*destroy)(SCSIDevice *s);
+    int32_t (*send_command)(SCSIDevice *s, uint32_t tag, uint8_t *buf,
+                            int lun);
+    void (*read_data)(SCSIDevice *s, uint32_t tag);
+    int (*write_data)(SCSIDevice *s, uint32_t tag);
+    void (*cancel_io)(SCSIDevice *s, uint32_t tag);
+    uint8_t *(*get_buf)(SCSIDevice *s, uint32_t tag);
+};
 
-int32_t scsi_send_command(SCSIDevice *s, uint32_t tag, uint8_t *buf, int lun);
-/* SCSI data transfers are asynchrnonous.  However, unlike the block IO
-   layer the completion routine may be called directly by
-   scsi_{read,write}_data.  */
-void scsi_read_data(SCSIDevice *s, uint32_t tag);
-int scsi_write_data(SCSIDevice *s, uint32_t tag);
-void scsi_cancel_io(SCSIDevice *s, uint32_t tag);
-uint8_t *scsi_get_buf(SCSIDevice *s, uint32_t tag);
+SCSIDevice *scsi_disk_init(BlockDriverState *bdrv, int tcq,
+                           scsi_completionfn completion, void *opaque);
 
 /* cdrom.c */
 int cdrom_read_toc(int nb_sectors, uint8_t *buf, int msf, int start_track);