[Qemu-devel] [PATCH 00/10] usb: uhci & usb-storage updates

[Qemu-devel] [PATCH 00/10] usb: uhci & usb-storage updates

[Gerd Hoffmann]

  Hi,

This patch series brings some bandwidth management tweaks for uhci, and
also a little uhci migration fix.

It also brings live migration support for usb-storage, including an
update for scsi-disk live migration support.  Paolo, feel free to
cherry-pick the scsi patch, I'll go drop it for the pull request then.

Obviously post-freeze material.

please review,
  Gerd

The following changes since commit aeb29b6459cb9496b38c820f3faff64cf2369d0d:

  audio: Always call fini on exit (2012-05-24 19:35:27 +0400)

are available in the git repository at:
  git://git.kraxel.org/qemu usb.51

Gerd Hoffmann (10):
      uhci: fix bandwidth management
      uhci: use bottom half
      uhci: make bandwidth tunable
      uhci: fix trace format strings
      uhci: zap uhci_pre_save
      scsi: prepare migration code for usb-storage support
      usb-storage: remove MSDState->residue
      usb-storage: add usb_msd_packet_complete()
      usb-storage: add scsi_off, remove scsi_buf
      usb-storage: migration support

 hw/scsi-bus.c        |    8 +++---
 hw/scsi-disk.c       |   13 ++++++++-
 hw/usb/dev-storage.c |   69 ++++++++++++++++++++++++++++++++-----------------
 hw/usb/hcd-uhci.c    |   46 +++++++++++++++++++-------------
 trace-events         |   10 +++---
 5 files changed, 92 insertions(+), 54 deletions(-)

[Qemu-devel] [PATCH 01/10] uhci: fix bandwidth management

[Gerd Hoffmann]

uhci_process_frame() can be invoked multiple times per frame, so
accounting usb bandwith in a local variable doesn't fly, use a variable
in UHCIState instead.  Also check the limit more frequently.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |   22 ++++++++++++----------
 trace-events      |    2 +-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 9e211a0..48ad35c 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -131,6 +131,7 @@ struct UHCIState {
     uint8_t status2; /* bit 0 and 1 are used to generate UHCI_STS_USBINT */
     int64_t expire_time;
     QEMUTimer *frame_timer;
+    uint32_t frame_bytes;
     UHCIPort ports[NB_PORTS];
 
     /* Interrupts that should be raised at the end of the current frame.  */
@@ -985,7 +986,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td)
 static void uhci_process_frame(UHCIState *s)
 {
     uint32_t frame_addr, link, old_td_ctrl, val, int_mask;
-    uint32_t curr_qh, td_count = 0, bytes_count = 0;
+    uint32_t curr_qh, td_count = 0;
     int cnt, ret;
     UHCI_TD td;
     UHCI_QH qh;
@@ -1002,6 +1003,12 @@ static void uhci_process_frame(UHCIState *s)
     qhdb_reset(&qhdb);
 
     for (cnt = FRAME_MAX_LOOPS; is_valid(link) && cnt; cnt--) {
+        if (s->frame_bytes >= 1280) {
+            /* We've reached the usb 1.1 bandwidth, which is
+               1280 bytes/frame, stop processing */
+            trace_usb_uhci_frame_stop_bandwidth();
+            break;
+        }
         if (is_qh(link)) {
             /* QH */
             trace_usb_uhci_qh_load(link & ~0xf);
@@ -1011,18 +1018,12 @@ static void uhci_process_frame(UHCIState *s)
                  * We're going in circles. Which is not a bug because
                  * HCD is allowed to do that as part of the BW management.
                  *
-                 * Stop processing here if
-                 *  (a) no transaction has been done since we've been
-                 *      here last time, or
-                 *  (b) we've reached the usb 1.1 bandwidth, which is
-                 *      1280 bytes/frame.
+                 * Stop processing here if no transaction has been done
+                 * since we've been here last time.
                  */
                 if (td_count == 0) {
                     trace_usb_uhci_frame_loop_stop_idle();
                     break;
-                } else if (bytes_count >= 1280) {
-                    trace_usb_uhci_frame_loop_stop_bandwidth();
-                    break;
                 } else {
                     trace_usb_uhci_frame_loop_continue();
                     td_count = 0;
@@ -1085,7 +1086,7 @@ static void uhci_process_frame(UHCIState *s)
             trace_usb_uhci_td_complete(curr_qh & ~0xf, link & ~0xf);
             link = td.link;
             td_count++;
-            bytes_count += (td.ctrl & 0x7ff) + 1;
+            s->frame_bytes += (td.ctrl & 0x7ff) + 1;
 
             if (curr_qh) {
                 /* update QH element link */
@@ -1118,6 +1119,7 @@ static void uhci_frame_timer(void *opaque)
 
     /* prepare the timer for the next frame */
     s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ);
+    s->frame_bytes = 0;
 
     if (!(s->cmd & UHCI_CMD_RS)) {
         /* Full stop */
diff --git a/trace-events b/trace-events
index 87cb96c..f848bee 100644
--- a/trace-events
+++ b/trace-events
@@ -263,8 +263,8 @@ usb_uhci_reset(void) "=== RESET ==="
 usb_uhci_schedule_start(void) ""
 usb_uhci_schedule_stop(void) ""
 usb_uhci_frame_start(uint32_t num) "nr %d"
+usb_uhci_frame_stop_bandwidth(void) ""
 usb_uhci_frame_loop_stop_idle(void) ""
-usb_uhci_frame_loop_stop_bandwidth(void) ""
 usb_uhci_frame_loop_continue(void) ""
 usb_uhci_mmio_readw(uint32_t addr, uint32_t val) "addr %04x, ret 0x04%x"
 usb_uhci_mmio_writew(uint32_t addr, uint32_t val) "addr %04x, val 0x04%x"
-- 
1.7.1

[Qemu-devel] [PATCH 02/10] uhci: use bottom half

[Gerd Hoffmann]

Schedule bottom half on completion of async packets instead of calling
uhci_process_frame directly.  This way we run uhci_process_frame only
once in case multiple packets finish in a row.  Also check whenever
there is bandwidth left before scheduling uhci_process_frame.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 48ad35c..91bcc7e 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -131,6 +131,7 @@ struct UHCIState {
     uint8_t status2; /* bit 0 and 1 are used to generate UHCI_STS_USBINT */
     int64_t expire_time;
     QEMUTimer *frame_timer;
+    QEMUBH *bh;
     uint32_t frame_bytes;
     UHCIPort ports[NB_PORTS];
 
@@ -370,6 +371,7 @@ static void uhci_reset(void *opaque)
     }
 
     uhci_async_cancel_all(s);
+    qemu_bh_cancel(s->bh);
     uhci_update_irq(s);
 }
 
@@ -906,7 +908,9 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet)
         uhci_async_free(async);
     } else {
         async->done = 1;
-        uhci_process_frame(s);
+        if (s->frame_bytes < 1280) {
+            qemu_bh_schedule(s->bh);
+        }
     }
 }
 
@@ -1113,6 +1117,12 @@ out:
     s->pending_int_mask |= int_mask;
 }
 
+static void uhci_bh(void *opaque)
+{
+    UHCIState *s = opaque;
+    uhci_process_frame(s);
+}
+
 static void uhci_frame_timer(void *opaque)
 {
     UHCIState *s = opaque;
@@ -1120,6 +1130,7 @@ static void uhci_frame_timer(void *opaque)
     /* prepare the timer for the next frame */
     s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ);
     s->frame_bytes = 0;
+    qemu_bh_cancel(s->bh);
 
     if (!(s->cmd & UHCI_CMD_RS)) {
         /* Full stop */
@@ -1206,6 +1217,7 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
                               USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL);
         }
     }
+    s->bh = qemu_bh_new(uhci_bh, s);
     s->frame_timer = qemu_new_timer_ns(vm_clock, uhci_frame_timer, s);
     s->num_ports_vmstate = NB_PORTS;
     QTAILQ_INIT(&s->queues);
-- 
1.7.1

[Qemu-devel] [PATCH 03/10] uhci: make bandwidth tunable

[Gerd Hoffmann]

Add a property for the uhci bandwidth.  Can be used to make uhci
emulation run faster than real hardware.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 91bcc7e..2e7c8f9 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -133,6 +133,7 @@ struct UHCIState {
     QEMUTimer *frame_timer;
     QEMUBH *bh;
     uint32_t frame_bytes;
+    uint32_t frame_bandwidth;
     UHCIPort ports[NB_PORTS];
 
     /* Interrupts that should be raised at the end of the current frame.  */
@@ -908,7 +909,7 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet)
         uhci_async_free(async);
     } else {
         async->done = 1;
-        if (s->frame_bytes < 1280) {
+        if (s->frame_bytes < s->frame_bandwidth) {
             qemu_bh_schedule(s->bh);
         }
     }
@@ -1007,7 +1008,7 @@ static void uhci_process_frame(UHCIState *s)
     qhdb_reset(&qhdb);
 
     for (cnt = FRAME_MAX_LOOPS; is_valid(link) && cnt; cnt--) {
-        if (s->frame_bytes >= 1280) {
+        if (s->frame_bytes >= s->frame_bandwidth) {
             /* We've reached the usb 1.1 bandwidth, which is
                1280 bytes/frame, stop processing */
             trace_usb_uhci_frame_stop_bandwidth();
@@ -1258,6 +1259,7 @@ static int usb_uhci_exit(PCIDevice *dev)
 static Property uhci_properties[] = {
     DEFINE_PROP_STRING("masterbus", UHCIState, masterbus),
     DEFINE_PROP_UINT32("firstport", UHCIState, firstport, 0),
+    DEFINE_PROP_UINT32("bandwidth", UHCIState, frame_bandwidth, 1280),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.1

[Qemu-devel] [PATCH 04/10] uhci: fix trace format strings

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 trace-events |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/trace-events b/trace-events
index f848bee..73583f3 100644
--- a/trace-events
+++ b/trace-events
@@ -266,10 +266,10 @@ usb_uhci_frame_start(uint32_t num) "nr %d"
 usb_uhci_frame_stop_bandwidth(void) ""
 usb_uhci_frame_loop_stop_idle(void) ""
 usb_uhci_frame_loop_continue(void) ""
-usb_uhci_mmio_readw(uint32_t addr, uint32_t val) "addr %04x, ret 0x04%x"
-usb_uhci_mmio_writew(uint32_t addr, uint32_t val) "addr %04x, val 0x04%x"
-usb_uhci_mmio_readl(uint32_t addr, uint32_t val) "addr %04x, ret 0x08%x"
-usb_uhci_mmio_writel(uint32_t addr, uint32_t val) "addr %04x, val 0x08%x"
+usb_uhci_mmio_readw(uint32_t addr, uint32_t val) "addr 0x%04x, ret 0x%04x"
+usb_uhci_mmio_writew(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%04x"
+usb_uhci_mmio_readl(uint32_t addr, uint32_t val) "addr 0x%04x, ret 0x%08x"
+usb_uhci_mmio_writel(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%08x"
 usb_uhci_queue_add(uint32_t token) "token 0x%x"
 usb_uhci_queue_del(uint32_t token) "token 0x%x"
 usb_uhci_packet_add(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x"
-- 
1.7.1

[Qemu-devel] [PATCH 05/10] uhci: zap uhci_pre_save

[Gerd Hoffmann]

Cancel transactions before saving vmstate is pretty pointless and just
causes disruptions.  We need to cancel them before *loading* vmstate,
but in that case uhci_reset() handles it already and no special action
is needed.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 2e7c8f9..3ea388c 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -376,13 +376,6 @@ static void uhci_reset(void *opaque)
     uhci_update_irq(s);
 }
 
-static void uhci_pre_save(void *opaque)
-{
-    UHCIState *s = opaque;
-
-    uhci_async_cancel_all(s);
-}
-
 static const VMStateDescription vmstate_uhci_port = {
     .name = "uhci port",
     .version_id = 1,
@@ -399,7 +392,6 @@ static const VMStateDescription vmstate_uhci = {
     .version_id = 2,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
-    .pre_save = uhci_pre_save,
     .fields      = (VMStateField []) {
         VMSTATE_PCI_DEVICE(dev, UHCIState),
         VMSTATE_UINT8_EQUAL(num_ports_vmstate, UHCIState),
-- 
1.7.1

[Qemu-devel] [PATCH 06/10] scsi: prepare migration code for usb-storage support

[Gerd Hoffmann]

usb-storage can't handle requests in one go as the data transfer can be
splitted into lots of usb packets.  Because of that there can be
normal in-flight requests at savevm time and we need to handle that.
With other scsi hba's this happens only in case i/o is stopped due to
errors and there are pending requests which need to be restarted
(req->retry = true).

So, first we need to save req->retry and then handle the req->retry =
false case.  Write requests are handled fine already.  For read requests
we have to save the buffer as we will not restart the request (and thus
not refill the buffer) on the target host.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/scsi-bus.c  |    8 ++++----
 hw/scsi-disk.c |   13 +++++++++++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 8ab9bcd..92b8176 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1507,10 +1507,9 @@ static void put_scsi_requests(QEMUFile *f, void *pv, size_t size)
     QTAILQ_FOREACH(req, &s->requests, next) {
         assert(!req->io_canceled);
         assert(req->status == -1);
-        assert(req->retry);
         assert(req->enqueued);
 
-        qemu_put_sbyte(f, 1);
+        qemu_put_sbyte(f, req->retry ? 1 : 2);
         qemu_put_buffer(f, req->cmd.buf, sizeof(req->cmd.buf));
         qemu_put_be32s(f, &req->tag);
         qemu_put_be32s(f, &req->lun);
@@ -1528,8 +1527,9 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
 {
     SCSIDevice *s = pv;
     SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, s->qdev.parent_bus);
+    int8_t sbyte;
 
-    while (qemu_get_sbyte(f)) {
+    while ((sbyte = qemu_get_sbyte(f)) > 0) {
         uint8_t buf[SCSI_CMD_BUF_SIZE];
         uint32_t tag;
         uint32_t lun;
@@ -1539,6 +1539,7 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
         qemu_get_be32s(f, &tag);
         qemu_get_be32s(f, &lun);
         req = scsi_req_new(s, tag, lun, buf, NULL);
+        req->retry = (sbyte == 1);
         if (bus->info->load_request) {
             req->hba_private = bus->info->load_request(f, req);
         }
@@ -1547,7 +1548,6 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
         }
 
         /* Just restart it later.  */
-        req->retry = true;
         scsi_req_enqueue_internal(req);
 
         /* At this point, the request will be kept alive by the reference
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 045c764..4fd39ff 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -132,8 +132,13 @@ static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req)
     qemu_put_be64s(f, &r->sector);
     qemu_put_be32s(f, &r->sector_count);
     qemu_put_be32s(f, &r->buflen);
-    if (r->buflen && r->req.cmd.mode == SCSI_XFER_TO_DEV) {
-        qemu_put_buffer(f, r->iov.iov_base, r->iov.iov_len);
+    if (r->buflen) {
+        if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
+            qemu_put_buffer(f, r->iov.iov_base, r->iov.iov_len);
+        } else if (!req->retry) {
+            qemu_put_be64s(f, &r->iov.iov_len);
+            qemu_put_buffer(f, r->iov.iov_base, r->iov.iov_len);
+        }
     }
 }
 
@@ -148,6 +153,10 @@ static void scsi_disk_load_request(QEMUFile *f, SCSIRequest *req)
         scsi_init_iovec(r, r->buflen);
         if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
             qemu_get_buffer(f, r->iov.iov_base, r->iov.iov_len);
+        } else if (!r->req.retry) {
+            qemu_get_be64s(f, &r->iov.iov_len);
+            assert(r->iov.iov_len <= r->buflen);
+            qemu_get_buffer(f, r->iov.iov_base, r->iov.iov_len);
         }
     }
 
-- 
1.7.1

[Qemu-devel] [PATCH 07/10] usb-storage: remove MSDState->residue

[Gerd Hoffmann]

We have the field twice, once in MSDState directly and one in the status
word struct.  Drop one.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-storage.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae22fb1..86f6851 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -51,7 +51,6 @@ typedef struct {
     uint32_t scsi_len;
     uint8_t *scsi_buf;
     uint32_t data_len;
-    uint32_t residue;
     struct usb_msd_csw csw;
     SCSIRequest *req;
     SCSIBus bus;
@@ -229,11 +228,10 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status, size_t r
     USBPacket *p = s->packet;
 
     DPRINTF("Command complete %d tag 0x%x\n", status, req->tag);
-    s->residue = s->data_len;
 
     s->csw.sig = cpu_to_le32(0x53425355);
     s->csw.tag = cpu_to_le32(req->tag);
-    s->csw.residue = cpu_to_le32(s->residue);
+    s->csw.residue = cpu_to_le32(s->data_len);
     s->csw.status = status != 0;
 
     if (s->packet) {
@@ -378,7 +376,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
             }
             DPRINTF("Command tag 0x%x flags %08x len %d data %d\n",
                     tag, cbw.flags, cbw.cmd_len, s->data_len);
-            s->residue = 0;
+            assert(le32_to_cpu(s->csw.residue) == 0);
             s->scsi_len = 0;
             s->req = scsi_req_new(s->scsi_dev, tag, 0, cbw.cmd, NULL);
             scsi_req_enqueue(s->req);
@@ -397,7 +395,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
             if (s->scsi_len) {
                 usb_msd_copy_data(s, p);
             }
-            if (s->residue) {
+            if (le32_to_cpu(s->csw.residue)) {
                 int len = p->iov.size - p->result;
                 if (len) {
                     usb_packet_skip(p, len);
@@ -458,7 +456,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
             if (s->scsi_len) {
                 usb_msd_copy_data(s, p);
             }
-            if (s->residue) {
+            if (le32_to_cpu(s->csw.residue)) {
                 int len = p->iov.size - p->result;
                 if (len) {
                     usb_packet_skip(p, len);
-- 
1.7.1

[Qemu-devel] [PATCH 08/10] usb-storage: add usb_msd_packet_complete()

[Gerd Hoffmann]

Factor out packet completion to a separate function which
cares to get the MSDState->packet update right.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-storage.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 86f6851..1477599 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -200,6 +200,18 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
     memset(&s->csw, 0, sizeof(s->csw));
 }
 
+static void usb_msd_packet_complete(MSDState *s)
+{
+    USBPacket *p = s->packet;
+
+    /* Set s->packet to NULL before calling usb_packet_complete
+       because another request may be issued before
+       usb_packet_complete returns.  */
+    DPRINTF("Packet complete %p\n", p);
+    s->packet = NULL;
+    usb_packet_complete(&s->dev, p);
+}
+
 static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
@@ -212,12 +224,7 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
         usb_msd_copy_data(s, p);
         p = s->packet;
         if (p && p->result == p->iov.size) {
-            /* Set s->packet to NULL before calling usb_packet_complete
-               because another request may be issued before
-               usb_packet_complete returns.  */
-            DPRINTF("Packet complete %p\n", p);
-            s->packet = NULL;
-            usb_packet_complete(&s->dev, p);
+            usb_msd_packet_complete(s);
         }
     }
 }
@@ -250,8 +257,7 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status, size_t r
                 s->mode = USB_MSDM_CSW;
             }
         }
-        s->packet = NULL;
-        usb_packet_complete(&s->dev, p);
+        usb_msd_packet_complete(s);
     } else if (s->data_len == 0) {
         s->mode = USB_MSDM_CSW;
     }
@@ -281,10 +287,8 @@ static void usb_msd_handle_reset(USBDevice *dev)
     assert(s->req == NULL);
 
     if (s->packet) {
-        USBPacket *p = s->packet;
-        s->packet = NULL;
-        p->result = USB_RET_STALL;
-        usb_packet_complete(dev, p);
+        s->packet->result = USB_RET_STALL;
+        usb_msd_packet_complete(s);
     }
 
     s->mode = USB_MSDM_CBW;
-- 
1.7.1

[Qemu-devel] [PATCH 09/10] usb-storage: add scsi_off, remove scsi_buf

[Gerd Hoffmann]

Repace the running buffer pointer (scsi_buf) with a buffer offset
field (scsi_off).  The later is alot easier to live-migrate.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-storage.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 1477599..1975d26 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -48,8 +48,8 @@ struct usb_msd_csw {
 typedef struct {
     USBDevice dev;
     enum USBMSDMode mode;
+    uint32_t scsi_off;
     uint32_t scsi_len;
-    uint8_t *scsi_buf;
     uint32_t data_len;
     struct usb_msd_csw csw;
     SCSIRequest *req;
@@ -178,9 +178,9 @@ static void usb_msd_copy_data(MSDState *s, USBPacket *p)
     len = p->iov.size - p->result;
     if (len > s->scsi_len)
         len = s->scsi_len;
-    usb_packet_copy(p, s->scsi_buf, len);
+    usb_packet_copy(p, scsi_req_get_buf(s->req) + s->scsi_off, len);
     s->scsi_len -= len;
-    s->scsi_buf += len;
+    s->scsi_off += len;
     s->data_len -= len;
     if (s->scsi_len == 0 || s->data_len == 0) {
         scsi_req_continue(s->req);
@@ -219,7 +219,7 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
 
     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
     s->scsi_len = len;
-    s->scsi_buf = scsi_req_get_buf(req);
+    s->scsi_off = 0;
     if (p) {
         usb_msd_copy_data(s, p);
         p = s->packet;
-- 
1.7.1

[Qemu-devel] [PATCH 10/10] usb-storage: migration support

[Gerd Hoffmann]

With all scsi migration support bits in place the
final step is pretty simple ;)

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-storage.c |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 1975d26..7646a77 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -506,6 +506,17 @@ static void usb_msd_password_cb(void *opaque, int err)
         qdev_unplug(&s->dev.qdev, NULL);
 }
 
+static void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req)
+{
+    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
+
+    /* nothing to load, just store req in our state struct */
+    assert(s->req == NULL);
+    scsi_req_ref(req);
+    s->req = req;
+    return NULL;
+}
+
 static const struct SCSIBusInfo usb_msd_scsi_info = {
     .tcq = false,
     .max_target = 0,
@@ -513,7 +524,8 @@ static const struct SCSIBusInfo usb_msd_scsi_info = {
 
     .transfer_data = usb_msd_transfer_data,
     .complete = usb_msd_command_complete,
-    .cancel = usb_msd_request_cancelled
+    .cancel = usb_msd_request_cancelled,
+    .load_request = usb_msd_load_request,
 };
 
 static int usb_msd_initfn(USBDevice *dev)
@@ -633,11 +645,18 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
 
 static const VMStateDescription vmstate_usb_msd = {
     .name = "usb-storage",
-    .unmigratable = 1, /* FIXME: handle transactions which are in flight */
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField []) {
         VMSTATE_USB_DEVICE(dev, MSDState),
+        VMSTATE_UINT32(mode, MSDState),
+        VMSTATE_UINT32(scsi_len, MSDState),
+        VMSTATE_UINT32(scsi_off, MSDState),
+        VMSTATE_UINT32(data_len, MSDState),
+        VMSTATE_UINT32(csw.sig, MSDState),
+        VMSTATE_UINT32(csw.tag, MSDState),
+        VMSTATE_UINT32(csw.residue, MSDState),
+        VMSTATE_UINT8(csw.status, MSDState),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
1.7.1

Re: [Qemu-devel] [PATCH 06/10] scsi: prepare migration code for usb-storage support

[Paolo Bonzini]

Il 25/05/2012 11:44, Gerd Hoffmann ha scritto:
> usb-storage can't handle requests in one go as the data transfer can be
> splitted into lots of usb packets.  Because of that there can be
> normal in-flight requests at savevm time and we need to handle that.
> With other scsi hba's this happens only in case i/o is stopped due to
> errors and there are pending requests which need to be restarted
> (req->retry = true).
> 
> So, first we need to save req->retry and then handle the req->retry =
> false case.  Write requests are handled fine already.  For read requests
> we have to save the buffer as we will not restart the request (and thus
> not refill the buffer) on the target host.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/scsi-bus.c  |    8 ++++----
>  hw/scsi-disk.c |   13 +++++++++++--
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 8ab9bcd..92b8176 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -1507,10 +1507,9 @@ static void put_scsi_requests(QEMUFile *f, void *pv, size_t size)
>      QTAILQ_FOREACH(req, &s->requests, next) {
>          assert(!req->io_canceled);
>          assert(req->status == -1);
> -        assert(req->retry);
>          assert(req->enqueued);
>  
> -        qemu_put_sbyte(f, 1);
> +        qemu_put_sbyte(f, req->retry ? 1 : 2);
>          qemu_put_buffer(f, req->cmd.buf, sizeof(req->cmd.buf));
>          qemu_put_be32s(f, &req->tag);
>          qemu_put_be32s(f, &req->lun);
> @@ -1528,8 +1527,9 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
>  {
>      SCSIDevice *s = pv;
>      SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, s->qdev.parent_bus);
> +    int8_t sbyte;
>  
> -    while (qemu_get_sbyte(f)) {
> +    while ((sbyte = qemu_get_sbyte(f)) > 0) {
>          uint8_t buf[SCSI_CMD_BUF_SIZE];
>          uint32_t tag;
>          uint32_t lun;
> @@ -1539,6 +1539,7 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
>          qemu_get_be32s(f, &tag);
>          qemu_get_be32s(f, &lun);
>          req = scsi_req_new(s, tag, lun, buf, NULL);
> +        req->retry = (sbyte == 1);
>          if (bus->info->load_request) {
>              req->hba_private = bus->info->load_request(f, req);
>          }
> @@ -1547,7 +1548,6 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
>          }
>  
>          /* Just restart it later.  */
> -        req->retry = true;
>          scsi_req_enqueue_internal(req);
>  
>          /* At this point, the request will be kept alive by the reference
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 045c764..4fd39ff 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -132,8 +132,13 @@ static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req)
>      qemu_put_be64s(f, &r->sector);
>      qemu_put_be32s(f, &r->sector_count);
>      qemu_put_be32s(f, &r->buflen);
> -    if (r->buflen && r->req.cmd.mode == SCSI_XFER_TO_DEV) {
> -        qemu_put_buffer(f, r->iov.iov_base, r->iov.iov_len);
> +    if (r->buflen) {
> +        if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
> +            qemu_put_buffer(f, r->iov.iov_base, r->iov.iov_len);
> +        } else if (!req->retry) {
> +            qemu_put_be64s(f, &r->iov.iov_len);
> +            qemu_put_buffer(f, r->iov.iov_base, r->iov.iov_len);
> +        }
>      }
>  }
>  
> @@ -148,6 +153,10 @@ static void scsi_disk_load_request(QEMUFile *f, SCSIRequest *req)
>          scsi_init_iovec(r, r->buflen);
>          if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
>              qemu_get_buffer(f, r->iov.iov_base, r->iov.iov_len);
> +        } else if (!r->req.retry) {
> +            qemu_get_be64s(f, &r->iov.iov_len);
> +            assert(r->iov.iov_len <= r->buflen);
> +            qemu_get_buffer(f, r->iov.iov_base, r->iov.iov_len);
>          }
>      }
>  

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Feel free to push this to Anthony yourself
.

Paolo

Re: [Qemu-devel] [PATCH 09/10] usb-storage: add scsi_off, remove scsi_buf

[Paolo Bonzini]

Il 25/05/2012 11:44, Gerd Hoffmann ha scritto:
> Repace the running buffer pointer (scsi_buf) with a buffer offset
> field (scsi_off).  The later is alot easier to live-migrate.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb/dev-storage.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 1477599..1975d26 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -48,8 +48,8 @@ struct usb_msd_csw {
>  typedef struct {
>      USBDevice dev;
>      enum USBMSDMode mode;
> +    uint32_t scsi_off;
>      uint32_t scsi_len;
> -    uint8_t *scsi_buf;
>      uint32_t data_len;
>      struct usb_msd_csw csw;
>      SCSIRequest *req;
> @@ -178,9 +178,9 @@ static void usb_msd_copy_data(MSDState *s, USBPacket *p)
>      len = p->iov.size - p->result;
>      if (len > s->scsi_len)
>          len = s->scsi_len;
> -    usb_packet_copy(p, s->scsi_buf, len);
> +    usb_packet_copy(p, scsi_req_get_buf(s->req) + s->scsi_off, len);
>      s->scsi_len -= len;
> -    s->scsi_buf += len;
> +    s->scsi_off += len;
>      s->data_len -= len;
>      if (s->scsi_len == 0 || s->data_len == 0) {
>          scsi_req_continue(s->req);
> @@ -219,7 +219,7 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
>  
>      assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
>      s->scsi_len = len;
> -    s->scsi_buf = scsi_req_get_buf(req);
> +    s->scsi_off = 0;
>      if (p) {
>          usb_msd_copy_data(s, p);
>          p = s->packet;

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 10/10] usb-storage: migration support

[Paolo Bonzini]

Il 25/05/2012 11:44, Gerd Hoffmann ha scritto:
> With all scsi migration support bits in place the
> final step is pretty simple ;)
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb/dev-storage.c |   23 +++++++++++++++++++++--
>  1 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 1975d26..7646a77 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -506,6 +506,17 @@ static void usb_msd_password_cb(void *opaque, int err)
>          qdev_unplug(&s->dev.qdev, NULL);
>  }
>  
> +static void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req)
> +{
> +    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> +
> +    /* nothing to load, just store req in our state struct */
> +    assert(s->req == NULL);
> +    scsi_req_ref(req);
> +    s->req = req;
> +    return NULL;
> +}
> +
>  static const struct SCSIBusInfo usb_msd_scsi_info = {
>      .tcq = false,
>      .max_target = 0,
> @@ -513,7 +524,8 @@ static const struct SCSIBusInfo usb_msd_scsi_info = {
>  
>      .transfer_data = usb_msd_transfer_data,
>      .complete = usb_msd_command_complete,
> -    .cancel = usb_msd_request_cancelled
> +    .cancel = usb_msd_request_cancelled,
> +    .load_request = usb_msd_load_request,
>  };
>  
>  static int usb_msd_initfn(USBDevice *dev)
> @@ -633,11 +645,18 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
>  
>  static const VMStateDescription vmstate_usb_msd = {
>      .name = "usb-storage",
> -    .unmigratable = 1, /* FIXME: handle transactions which are in flight */
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField []) {
>          VMSTATE_USB_DEVICE(dev, MSDState),
> +        VMSTATE_UINT32(mode, MSDState),
> +        VMSTATE_UINT32(scsi_len, MSDState),
> +        VMSTATE_UINT32(scsi_off, MSDState),
> +        VMSTATE_UINT32(data_len, MSDState),
> +        VMSTATE_UINT32(csw.sig, MSDState),
> +        VMSTATE_UINT32(csw.tag, MSDState),
> +        VMSTATE_UINT32(csw.residue, MSDState),
> +        VMSTATE_UINT8(csw.status, MSDState),
>          VMSTATE_END_OF_LIST()
>      }
>  };

Can usb-storage have multiple requests in flight (in principle, i.e.
according to the USB protocol)?  If so, it may be better if you save
scsi_len/scsi_off in save_request/load_request.  The migration format
will be forward-compatible.

Otherwise looks good!

Paolo

Re: [Qemu-devel] [PATCH 10/10] usb-storage: migration support

[Gerd Hoffmann]

  Hi,

> Can usb-storage have multiple requests in flight (in principle, i.e.
> according to the USB protocol)?

No.  Well, not with the protocol emulated here.

There is a new one for usb storage devices, designed with USB 3.0 in
mind and using streams (usb 3.0 feature) to handle multiple requests in
parallel.  But I guess when ever implementing that it would be a new
device, not a usb-storage extension.

cheers,
  Gerd