[Spice-commits] 19 commits - VERSION block/qed.c block/stream.c console.c hw/qdev-monitor.c hw/usb hw/usb.h target-xtensa/xtensa-semi.c tests/qemu-iotests

Gerd Hoffmann kraxel at kemper.freedesktop.org
Tue Sep 4 02:56:44 PDT 2012


 VERSION                     |    2 
 block/qed.c                 |   11 +++
 block/stream.c              |    6 ++
 console.c                   |    2 
 hw/qdev-monitor.c           |    2 
 hw/usb.h                    |    4 +
 hw/usb/core.c               |   38 ++++++++++---
 hw/usb/dev-uas.c            |    3 -
 hw/usb/hcd-ehci.c           |  122 ++++++++++++++++++++++++++++----------------
 hw/usb/hcd-musb.c           |    3 -
 hw/usb/hcd-ohci.c           |    4 -
 hw/usb/hcd-uhci.c           |   20 ++++++-
 hw/usb/hcd-xhci.c           |    2 
 target-xtensa/xtensa-semi.c |    2 
 tests/qemu-iotests/030      |   33 +++++++++++
 tests/qemu-iotests/030.out  |    4 -
 16 files changed, 194 insertions(+), 64 deletions(-)

New commits:
commit e7eee62a90c671d22d50964b7de05e3f4fd96f5f
Author: Max Filippov <jcmvbkbc at gmail.com>
Date:   Wed Aug 22 22:03:35 2012 +0400

    target-xtensa: return ENOSYS for unimplemented simcalls
    
    This prevents guest from proceeding with uninitialised garbage returned
    from unimplemented simcalls.
    
    Signed-off-by: Max Filippov <jcmvbkbc at gmail.com>
    Signed-off-by: Blue Swirl <blauwirbel at gmail.com>

diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c
index 1c8a19e..6d001c2 100644
--- a/target-xtensa/xtensa-semi.c
+++ b/target-xtensa/xtensa-semi.c
@@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env)
 
     default:
         qemu_log("%s(%d): not implemented\n", __func__, regs[2]);
+        regs[2] = -1;
+        regs[3] = ENOSYS;
         break;
     }
 }
commit 0232cd355d37e1ef938c3636a0e934da87f3bcc8
Author: Anthony Liguori <aliguori at us.ibm.com>
Date:   Fri Aug 31 10:50:46 2012 -0500

    Update version to 1.2.0-rc3
    
    Signed-off-by: Anthony Liguori <aliguori at us.ibm.com>

diff --git a/VERSION b/VERSION
index 99f6a00..9a03ea6 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.1.92
+1.1.93
commit 8bd6b06d7b718b3e595aab279699ef3651ce2e48
Author: Stefan Weil <sw at weilnetz.de>
Date:   Fri Aug 17 15:50:44 2012 +0200

    console: Fix warning from clang (and potential crash)
    
    ccc-analyzer reports this warning:
    
    console.c:1090:29: warning: Dereference of null pointer
            if (active_console->cursor_timer) {
                                ^
    
    Function console_select allows active_console to be NULL,
    but would crash when accessing cursor_timer. Fix this.
    
    Reviewed-by: Jan Kiszka <jan.kiszka at siemens.com>
    Signed-off-by: Stefan Weil <sw at weilnetz.de>
    Signed-off-by: Anthony Liguori <aliguori at us.ibm.com>

diff --git a/console.c b/console.c
index 4525cc7..f5e8814 100644
--- a/console.c
+++ b/console.c
@@ -1087,7 +1087,7 @@ void console_select(unsigned int index)
     if (s) {
         DisplayState *ds = s->ds;
 
-        if (active_console->cursor_timer) {
+        if (active_console && active_console->cursor_timer) {
             qemu_del_timer(active_console->cursor_timer);
         }
         active_console = s;
commit 23aec6005af30e29180496b434edcc51660ce94e
Merge: cdedd9d... 347e40f...
Author: Anthony Liguori <aliguori at us.ibm.com>
Date:   Fri Aug 31 10:04:54 2012 -0500

    Merge remote-tracking branch 'kraxel/usb.61' into staging
    
    * kraxel/usb.61:
      uas: move transfer kickoff
      ehci: Fix interrupt endpoints no longer working
      ehci: handle TD deactivation of inflight packets
      ehci: add ehci_cancel_queue()
      ehci: simplify ehci_state_executing
      ehci: Remove unnecessary ehci_flush_qh call
      ehci: Schedule async-bh when IAAD bit gets set
      ehci: Fix NULL ptr deref when unplugging an USB dev with an iso stream active
      usb: unique packet ids
      usb: Halt ep queue en cancel pending packets on a packet error
      fix info qtree indention

commit cdedd9d867f2e955e022f07808b10a4a5d383841
Merge: b834b50... 774a885...
Author: Anthony Liguori <aliguori at us.ibm.com>
Date:   Fri Aug 31 10:04:18 2012 -0500

    Merge remote-tracking branch 'kwolf/for-anthony' into staging
    
    * kwolf/for-anthony:
      qemu-iotests: add backing file smaller than image test case
      stream: complete early if end of backing file is reached
      qed: refuse unaligned zero writes with a backing file

commit 347e40ffe61b7cc8d4565be476c20acd00611669
Author: Gerd Hoffmann <kraxel at redhat.com>
Date:   Fri Aug 31 14:34:19 2012 +0200

    uas: move transfer kickoff
    
    Kick next scsi transfer from request release callback instead of command
    completion callback, otherwise we might get stuck in case scsi_req_unref()
    doesn't release the request instantly due to someone else holding a
    reference too.
    
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index b13eeba..5a0057a 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -424,6 +424,7 @@ static void usb_uas_scsi_free_request(SCSIBus *bus, void *priv)
     }
     QTAILQ_REMOVE(&uas->requests, req, next);
     g_free(req);
+    usb_uas_start_next_transfer(uas);
 }
 
 static UASRequest *usb_uas_find_request(UASDevice *uas, uint16_t tag)
@@ -456,7 +457,6 @@ static void usb_uas_scsi_command_complete(SCSIRequest *r,
                                           uint32_t status, size_t resid)
 {
     UASRequest *req = r->hba_private;
-    UASDevice *uas = req->uas;
 
     trace_usb_uas_scsi_complete(req->uas->dev.addr, req->tag, status, resid);
     req->complete = true;
@@ -465,7 +465,6 @@ static void usb_uas_scsi_command_complete(SCSIRequest *r,
     }
     usb_uas_queue_sense(req, status);
     scsi_req_unref(req->req);
-    usb_uas_start_next_transfer(uas);
 }
 
 static void usb_uas_scsi_request_cancelled(SCSIRequest *r)
commit adf478342b11cf9f540baf1f387b669210d3bea1
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Aug 30 11:20:51 2012 +0200

    ehci: Fix interrupt endpoints no longer working
    
    One of the recent changes (likely the addition of queuing support) has broken
    interrupt endpoints, this patch fixes this.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index eca1431..017342b 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1987,10 +1987,19 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
         ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
         again = 1;
     } else if (p != NULL) {
-        if (p->async == EHCI_ASYNC_INFLIGHT) {
+        switch (p->async) {
+        case EHCI_ASYNC_NONE:
+            /* Previously nacked packet (likely interrupt ep) */
+           ehci_set_state(q->ehci, q->async, EST_EXECUTE);
+           break;
+        case EHCI_ASYNC_INFLIGHT:
+            /* Unfinyshed async handled packet, go horizontal */
             ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
-        } else {
+            break;
+        case EHCI_ASYNC_FINISHED:
+            /* Should never happen, as this case is caught by fetchqh */
             ehci_set_state(q->ehci, q->async, EST_EXECUTING);
+            break;
         }
         again = 1;
     } else {
commit 287fd3f1dd0b2abbd69e58b402e5364b334e95bd
Author: Gerd Hoffmann <kraxel at redhat.com>
Date:   Tue Aug 21 14:03:09 2012 +0200

    ehci: handle TD deactivation of inflight packets
    
    Check the TDs of inflight packets, cancel
    packets in case the guest clears the active bit.
    
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 30e5e8f..eca1431 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1816,11 +1816,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
         q->dev = ehci_find_device(q->ehci, devaddr);
     }
 
-    if (p && p->async == EHCI_ASYNC_INFLIGHT) {
-        /* I/O still in progress -- skip queue */
-        ehci_set_state(ehci, async, EST_HORIZONTALQH);
-        goto out;
-    }
     if (p && p->async == EHCI_ASYNC_FINISHED) {
         /* I/O finished -- continue processing queue */
         trace_usb_ehci_packet_action(p->queue, p, "complete");
@@ -1969,28 +1964,41 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
     ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &qtd);
 
     p = QTAILQ_FIRST(&q->packets);
-    if (p != NULL && p->qtdaddr != q->qtdaddr) {
-        /* should not happen (guest bug) */
-        ehci_cancel_queue(q);
-        p = NULL;
-    }
     if (p != NULL) {
-        ehci_qh_do_overlay(q);
+        if (p->qtdaddr != q->qtdaddr ||
+            (!NLPTR_TBIT(p->qtd.next) && (p->qtd.next != qtd.next)) ||
+            (!NLPTR_TBIT(p->qtd.altnext) && (p->qtd.altnext != qtd.altnext)) ||
+            p->qtd.bufptr[0] != qtd.bufptr[0]) {
+            /* guest bug: guest updated active QH or qTD underneath us */
+            ehci_cancel_queue(q);
+            p = NULL;
+        } else {
+            p->qtd = qtd;
+            ehci_qh_do_overlay(q);
+        }
+    }
+
+    if (!(qtd.token & QTD_TOKEN_ACTIVE)) {
+        if (p != NULL) {
+            /* transfer canceled by guest (clear active) */
+            ehci_cancel_queue(q);
+            p = NULL;
+        }
+        ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
+        again = 1;
+    } else if (p != NULL) {
         if (p->async == EHCI_ASYNC_INFLIGHT) {
             ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
         } else {
             ehci_set_state(q->ehci, q->async, EST_EXECUTING);
         }
         again = 1;
-    } else if (qtd.token & QTD_TOKEN_ACTIVE) {
+    } else {
         p = ehci_alloc_packet(q);
         p->qtdaddr = q->qtdaddr;
         p->qtd = qtd;
         ehci_set_state(q->ehci, q->async, EST_EXECUTE);
         again = 1;
-    } else {
-        ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
-        again = 1;
     }
 
     return again;
commit c7cdca3b853eed0dd521c43098b6d07bcce24fd1
Author: Gerd Hoffmann <kraxel at redhat.com>
Date:   Tue Aug 21 13:58:40 2012 +0200

    ehci: add ehci_cancel_queue()
    
    Factor out function to cancel all packets of a queue.
    No behavior change.
    
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index b6169ce..30e5e8f 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -766,15 +766,27 @@ static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, uint32_t addr, int async)
     return q;
 }
 
+static void ehci_cancel_queue(EHCIQueue *q)
+{
+    EHCIPacket *p;
+
+    p = QTAILQ_FIRST(&q->packets);
+    if (p == NULL) {
+        return;
+    }
+
+    trace_usb_ehci_queue_action(q, "cancel");
+    do {
+        ehci_free_packet(p);
+    } while ((p = QTAILQ_FIRST(&q->packets)) != NULL);
+}
+
 static void ehci_free_queue(EHCIQueue *q)
 {
     EHCIQueueHead *head = q->async ? &q->ehci->aqueues : &q->ehci->pqueues;
-    EHCIPacket *p;
 
     trace_usb_ehci_queue_action(q, "free");
-    while ((p = QTAILQ_FIRST(&q->packets)) != NULL) {
-        ehci_free_packet(p);
-    }
+    ehci_cancel_queue(q);
     QTAILQ_REMOVE(head, q, next);
     g_free(q);
 }
@@ -1796,9 +1808,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
     if (q->dev != NULL && q->dev->addr != devaddr) {
         if (!QTAILQ_EMPTY(&q->packets)) {
             /* should not happen (guest bug) */
-            while ((p = QTAILQ_FIRST(&q->packets)) != NULL) {
-                ehci_free_packet(p);
-            }
+            ehci_cancel_queue(q);
         }
         q->dev = NULL;
     }
@@ -1959,10 +1969,10 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
     ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &qtd);
 
     p = QTAILQ_FIRST(&q->packets);
-    while (p != NULL && p->qtdaddr != q->qtdaddr) {
+    if (p != NULL && p->qtdaddr != q->qtdaddr) {
         /* should not happen (guest bug) */
-        ehci_free_packet(p);
-        p = QTAILQ_FIRST(&q->packets);
+        ehci_cancel_queue(q);
+        p = NULL;
     }
     if (p != NULL) {
         ehci_qh_do_overlay(q);
commit 574ef17191f5ec5a3cc4782c1f59dc5eb8279654
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Aug 17 11:39:17 2012 +0200

    ehci: simplify ehci_state_executing
    
    ehci_state_executing does not need to check for p->usb_status == USB_RET_ASYNC
    or USB_RET_PROCERR, since ehci_execute_complete already does a similar check
    and will trigger an assert if either value is encountered.
    
    USB_RET_ASYNC should never be the packet status when execute_complete runs
    for obvious reasons, and USB_RET_PROCERR is only used by ehci_state_execute /
    ehci_execute not by ehci_state_executing / ehci_execute_complete.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 923a949..b6169ce 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2084,19 +2084,11 @@ out:
 static int ehci_state_executing(EHCIQueue *q)
 {
     EHCIPacket *p = QTAILQ_FIRST(&q->packets);
-    int again = 0;
 
     assert(p != NULL);
     assert(p->qtdaddr == q->qtdaddr);
 
     ehci_execute_complete(q);
-    if (p->usb_status == USB_RET_ASYNC) {
-        goto out;
-    }
-    if (p->usb_status == USB_RET_PROCERR) {
-        again = -1;
-        goto out;
-    }
 
     // 4.10.3
     if (!q->async) {
@@ -2114,11 +2106,8 @@ static int ehci_state_executing(EHCIQueue *q)
         ehci_set_state(q->ehci, q->async, EST_WRITEBACK);
     }
 
-    again = 1;
-
-out:
     ehci_flush_qh(q);
-    return again;
+    return 1;
 }
 
 
commit 53dd6f7032ec4898ca8f95356df795a92cd27e09
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Aug 16 15:47:29 2012 +0200

    ehci: Remove unnecessary ehci_flush_qh call
    
    ehci_qh_do_overlay() already calls ehci_flush_qh() before it returns, calling
    it twice is useless.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 3e10977..923a949 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1966,7 +1966,6 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
     }
     if (p != NULL) {
         ehci_qh_do_overlay(q);
-        ehci_flush_qh(q);
         if (p->async == EHCI_ASYNC_INFLIGHT) {
             ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
         } else {
commit a1c3e4b839f8e7ec7f1792b8a11c63ca845aa021
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Aug 30 09:55:19 2012 +0200

    ehci: Schedule async-bh when IAAD bit gets set
    
    After the "ehci: Print a warning when a queue unexpectedly contains packets
    on cancel" commit. Under certain reproducable conditions I was getting the
    following message: "EHCI: Warning queue not empty on queue reset".
    
    After aprox. 8 hours of debugging I've finally found the cause. The Linux EHCI
    driver has an IAAD watchdog, to work around certain EHCI hardware sometimes
    not acknowledging the doorbell at all. This watchdog has a timeout of 10 ms,
    which is less then the time between 2 runs through the async schedule when
    async_stepdown is at its highest value.
    
    Thus the watchdog can trigger, after which Linux clears the IAAD bit and
    re-uses the QH. IOW we were not properly detecting the unlink of the qh, due
    to us missing (ignoring for more then 10 ms) the IAAD command, which triggered
    the warning.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index a3aea6d..3e10977 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1194,6 +1194,15 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
             val &= ~USBCMD_FLS;
         }
 
+        if (val & USBCMD_IAAD) {
+            /*
+             * Process IAAD immediately, otherwise the Linux IAAD watchdog may
+             * trigger and re-use a qh without us seeing the unlink.
+             */
+            s->async_stepdown = 0;
+            qemu_bh_schedule(s->async_bh);
+        }
+
         if (((USBCMD_RUNSTOP | USBCMD_PSE | USBCMD_ASE) & val) !=
             ((USBCMD_RUNSTOP | USBCMD_PSE | USBCMD_ASE) & s->usbcmd)) {
             if (s->pstate == EST_INACTIVE) {
commit 7ce86aa1aafaa65e7d3e572873bdf37bdb896f49
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Tue Aug 28 11:50:26 2012 +0200

    ehci: Fix NULL ptr deref when unplugging an USB dev with an iso stream active
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index f43d690..a3aea6d 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1598,7 +1598,7 @@ static int ehci_process_itd(EHCIState *ehci,
 
             dev = ehci_find_device(ehci, devaddr);
             ep = usb_ep_get(dev, pid, endp);
-            if (ep->type == USB_ENDPOINT_XFER_ISOC) {
+            if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) {
                 usb_packet_setup(&ehci->ipacket, pid, ep, addr);
                 usb_packet_map(&ehci->ipacket, &ehci->isgl);
                 ret = usb_handle_packet(dev, &ehci->ipacket);
commit e983395d30d1d5bfa0ed3ae9c028c130f7c498cc
Author: Gerd Hoffmann <kraxel at redhat.com>
Date:   Thu Aug 23 13:30:13 2012 +0200

    usb: unique packet ids
    
    This patch adds IDs to usb packets.  Those IDs are (a) supposed to be
    unique for the lifecycle of a packet (from packet setup until the packet
    is either completed or canceled) and (b) stable across migration.
    
    uhci, ohci, ehci and xhci use the guest physical address of the transfer
    descriptor for this.
    
    musb needs a different approach because there is no transfer descriptor.
    But musb also doesn't support pipelining, so we have never more than one
    packet per endpoint in flight.  So we go create an ID based on endpoint
    and device address.
    
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/usb.h b/hw/usb.h
index e574477..b8fceec 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -332,6 +332,7 @@ typedef enum USBPacketState {
 struct USBPacket {
     /* Data fields for use by the driver.  */
     int pid;
+    uint64_t id;
     USBEndpoint *ep;
     QEMUIOVector iov;
     uint64_t parameter; /* control transfers */
@@ -344,7 +345,7 @@ struct USBPacket {
 void usb_packet_init(USBPacket *p);
 void usb_packet_set_state(USBPacket *p, USBPacketState state);
 void usb_packet_check_state(USBPacket *p, USBPacketState expected);
-void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep);
+void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id);
 void usb_packet_addbuf(USBPacket *p, void *ptr, size_t len);
 int usb_packet_map(USBPacket *p, QEMUSGList *sgl);
 void usb_packet_unmap(USBPacket *p, QEMUSGList *sgl);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 28b840e..2da38e7 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -520,10 +520,11 @@ void usb_packet_set_state(USBPacket *p, USBPacketState state)
     p->state = state;
 }
 
-void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep)
+void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id)
 {
     assert(!usb_packet_is_inflight(p));
     assert(p->iov.iov != NULL);
+    p->id = id;
     p->pid = pid;
     p->ep = ep;
     p->result = 0;
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 8504a6a..f43d690 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1530,7 +1530,7 @@ static int ehci_execute(EHCIPacket *p, const char *action)
     endp = get_field(p->queue->qh.epchar, QH_EPCHAR_EP);
     ep = usb_ep_get(p->queue->dev, p->pid, endp);
 
-    usb_packet_setup(&p->packet, p->pid, ep);
+    usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr);
     usb_packet_map(&p->packet, &p->sgl);
 
     trace_usb_ehci_packet_action(p->queue, p, action);
@@ -1552,7 +1552,8 @@ static int ehci_execute(EHCIPacket *p, const char *action)
  */
 
 static int ehci_process_itd(EHCIState *ehci,
-                            EHCIitd *itd)
+                            EHCIitd *itd,
+                            uint32_t addr)
 {
     USBDevice *dev;
     USBEndpoint *ep;
@@ -1598,7 +1599,7 @@ static int ehci_process_itd(EHCIState *ehci,
             dev = ehci_find_device(ehci, devaddr);
             ep = usb_ep_get(dev, pid, endp);
             if (ep->type == USB_ENDPOINT_XFER_ISOC) {
-                usb_packet_setup(&ehci->ipacket, pid, ep);
+                usb_packet_setup(&ehci->ipacket, pid, ep, addr);
                 usb_packet_map(&ehci->ipacket, &ehci->isgl);
                 ret = usb_handle_packet(dev, &ehci->ipacket);
                 assert(ret != USB_RET_ASYNC);
@@ -1862,7 +1863,7 @@ static int ehci_state_fetchitd(EHCIState *ehci, int async)
                sizeof(EHCIitd) >> 2);
     ehci_trace_itd(ehci, entry, &itd);
 
-    if (ehci_process_itd(ehci, &itd) != 0) {
+    if (ehci_process_itd(ehci, &itd, entry) != 0) {
         return -1;
     }
 
diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c
index fa9385e..0bb5c7b 100644
--- a/hw/usb/hcd-musb.c
+++ b/hw/usb/hcd-musb.c
@@ -626,7 +626,8 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep,
     /* A wild guess on the FADDR semantics... */
     dev = usb_find_device(&s->port, ep->faddr[idx]);
     uep = usb_ep_get(dev, pid, ep->type[idx] & 0xf);
-    usb_packet_setup(&ep->packey[dir].p, pid, uep);
+    usb_packet_setup(&ep->packey[dir].p, pid, uep,
+                     (dev->addr << 16) | (uep->nr << 8) | pid);
     usb_packet_addbuf(&ep->packey[dir].p, ep->buf[idx], len);
     ep->packey[dir].ep = ep;
     ep->packey[dir].dir = dir;
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 844e7ed..c36184a 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -812,7 +812,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
     } else {
         dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
         ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
-        usb_packet_setup(&ohci->usb_packet, pid, ep);
+        usb_packet_setup(&ohci->usb_packet, pid, ep, addr);
         usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len);
         ret = usb_handle_packet(dev, &ohci->usb_packet);
         if (ret == USB_RET_ASYNC) {
@@ -1011,7 +1011,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
         }
         dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
         ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
-        usb_packet_setup(&ohci->usb_packet, pid, ep);
+        usb_packet_setup(&ohci->usb_packet, pid, ep, addr);
         usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, pktlen);
         ret = usb_handle_packet(dev, &ohci->usb_packet);
 #ifdef DEBUG_PACKET
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 8987734..b0db921 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -859,14 +859,14 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
      * for initial isochronous requests
      */
     async->queue->valid = 32;
-    async->isoc  = td->ctrl & TD_CTRL_IOS;
+    async->isoc = td->ctrl & TD_CTRL_IOS;
 
     max_len = ((td->token >> 21) + 1) & 0x7ff;
     pid = td->token & 0xff;
 
     dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
     ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
-    usb_packet_setup(&async->packet, pid, ep);
+    usb_packet_setup(&async->packet, pid, ep, addr);
     qemu_sglist_add(&async->sgl, td->buffer, max_len);
     usb_packet_map(&async->packet, &async->sgl);
 
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 6c2ff02..3eb27fa 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1392,7 +1392,7 @@ static int xhci_setup_packet(XHCITransfer *xfer, USBDevice *dev)
 
     dir = xfer->in_xfer ? USB_TOKEN_IN : USB_TOKEN_OUT;
     ep = usb_ep_get(dev, dir, xfer->epid >> 1);
-    usb_packet_setup(&xfer->packet, dir, ep);
+    usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr);
     usb_packet_addbuf(&xfer->packet, xfer->data, xfer->data_length);
     DPRINTF("xhci: setup packet pid 0x%x addr %d ep %d\n",
             xfer->packet.pid, dev->addr, ep->nr);
commit 0132b4b6595423c92f54d7e0b172b5d73aaa8375
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Aug 17 15:24:49 2012 +0200

    usb: Halt ep queue en cancel pending packets on a packet error
    
    For controllers which queue up more then 1 packet at a time, we must halt the
    ep queue, and inside the controller code cancel all pending packets on an
    error.
    
    There are multiple reasons for this:
    1) Guests expect the controllers to halt ep queues on error, so that they
    get the opportunity to cancel transfers which the scheduled after the failing
    one, before processing continues
    
    2) Not cancelling queued up packets after a failed transfer also messes up
    the controller state machine, in the case of EHCI causing the following
    assert to trigger: "assert(p->qtdaddr == q->qtdaddr)" at hcd-ehci.c:2075
    
    3) For bulk endpoints with pipelining enabled (redirection to a real USB
    device), we must cancel all the transfers after this a failed one so that:
    a) If they've completed already, they are not processed further causing more
       stalls to be reported, originating from the same failed transfer
    b) If still in flight, they are cancelled before the guest does
       a clear stall, otherwise the guest and device can loose sync!
    
    Note this patch only touches the ehci and uhci controller changes, since AFAIK
    no other controllers actually queue up multiple transfer. If I'm wrong on this
    other controllers need to be updated too!
    
    Also note that this patch was heavily tested with the ehci code, where I had
    a reproducer for a device causing a transfer to fail. The uhci code is not
    tested with actually failing transfers and could do with a thorough review!
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/usb.h b/hw/usb.h
index 432ccae..e574477 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -179,6 +179,7 @@ struct USBEndpoint {
     uint8_t ifnum;
     int max_packet_size;
     bool pipeline;
+    bool halted;
     USBDevice *dev;
     QTAILQ_HEAD(, USBPacket) queue;
 };
diff --git a/hw/usb/core.c b/hw/usb/core.c
index c7e5bc0..28b840e 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -382,12 +382,23 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
     usb_packet_check_state(p, USB_PACKET_SETUP);
     assert(p->ep != NULL);
 
+    /* Submitting a new packet clears halt */
+    if (p->ep->halted) {
+        assert(QTAILQ_EMPTY(&p->ep->queue));
+        p->ep->halted = false;
+    }
+
     if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) {
         ret = usb_process_one(p);
         if (ret == USB_RET_ASYNC) {
             usb_packet_set_state(p, USB_PACKET_ASYNC);
             QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue);
         } else {
+            /*
+             * When pipelining is enabled usb-devices must always return async,
+             * otherwise packets can complete out of order!
+             */
+            assert(!p->ep->pipeline);
             p->result = ret;
             usb_packet_set_state(p, USB_PACKET_COMPLETE);
         }
@@ -399,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
     return ret;
 }
 
+static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
+{
+    USBEndpoint *ep = p->ep;
+
+    assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK);
+
+    if (p->result < 0) {
+        ep->halted = true;
+    }
+    usb_packet_set_state(p, USB_PACKET_COMPLETE);
+    QTAILQ_REMOVE(&ep->queue, p, queue);
+    dev->port->ops->complete(dev->port, p);
+}
+
 /* Notify the controller that an async packet is complete.  This should only
    be called for packets previously deferred by returning USB_RET_ASYNC from
    handle_packet. */
@@ -409,11 +434,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
 
     usb_packet_check_state(p, USB_PACKET_ASYNC);
     assert(QTAILQ_FIRST(&ep->queue) == p);
-    usb_packet_set_state(p, USB_PACKET_COMPLETE);
-    QTAILQ_REMOVE(&ep->queue, p, queue);
-    dev->port->ops->complete(dev->port, p);
+    __usb_packet_complete(dev, p);
 
-    while (!QTAILQ_EMPTY(&ep->queue)) {
+    while (!ep->halted && !QTAILQ_EMPTY(&ep->queue)) {
         p = QTAILQ_FIRST(&ep->queue);
         if (p->state == USB_PACKET_ASYNC) {
             break;
@@ -425,9 +448,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
             break;
         }
         p->result = ret;
-        usb_packet_set_state(p, USB_PACKET_COMPLETE);
-        QTAILQ_REMOVE(&ep->queue, p, queue);
-        dev->port->ops->complete(dev->port, p);
+        __usb_packet_complete(ep->dev, p);
     }
 }
 
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 8b94b17..8504a6a 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2138,6 +2138,19 @@ static int ehci_state_writeback(EHCIQueue *q)
      * bit is clear.
      */
     if (q->qh.token & QTD_TOKEN_HALT) {
+        /*
+         * We should not do any further processing on a halted queue!
+         * This is esp. important for bulk endpoints with pipelining enabled
+         * (redirection to a real USB device), where we must cancel all the
+         * transfers after this one so that:
+         * 1) If they've completed already, they are not processed further
+         *    causing more stalls, originating from the same failed transfer
+         * 2) If still in flight, they are cancelled before the guest does
+         *    a clear stall, otherwise the guest and device can loose sync!
+         */
+        while ((p = QTAILQ_FIRST(&q->packets)) != NULL) {
+            ehci_free_packet(p);
+        }
         ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
         again = 1;
     } else {
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 1ace2a4..8987734 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -748,6 +748,22 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_
     return TD_RESULT_COMPLETE;
 
 out:
+    /*
+     * We should not do any further processing on a queue with errors!
+     * This is esp. important for bulk endpoints with pipelining enabled
+     * (redirection to a real USB device), where we must cancel all the
+     * transfers after this one so that:
+     * 1) If they've completed already, they are not processed further
+     *    causing more stalls, originating from the same failed transfer
+     * 2) If still in flight, they are cancelled before the guest does
+     *    a clear stall, otherwise the guest and device can loose sync!
+     */
+    while (!QTAILQ_EMPTY(&async->queue->asyncs)) {
+        UHCIAsync *as = QTAILQ_FIRST(&async->queue->asyncs);
+        uhci_async_unlink(as);
+        uhci_async_cancel(as);
+    }
+
     switch(ret) {
     case USB_RET_STALL:
         td->ctrl |= TD_CTRL_STALL;
commit da9fbe76a0639c529f9678aabcc052dfe4cd9cc4
Author: Gerd Hoffmann <kraxel at redhat.com>
Date:   Wed Jul 11 12:21:23 2012 +0200

    fix info qtree indention
    
    Without the patch bus properties are are not in line with the other
    properties:
    
    [ ... ]
      dev: fw_cfg, id ""
        ctl_iobase = 0x510
        data_iobase = 0x511
          irq 0
          mmio ffffffffffffffff/0000000000000002
          mmio ffffffffffffffff/0000000000000001
    [ ... ]
    
    With the patch applied everything is lined up properly:
    
    [ ... ]
      dev: fw_cfg, id ""
        ctl_iobase = 0x510
        data_iobase = 0x511
        irq 0
        mmio ffffffffffffffff/0000000000000002
        mmio ffffffffffffffff/0000000000000001
    [ ... ]
    
    Needed to make the autotest qtree parser happy.
    
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 018b386..33b7f79 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -543,7 +543,7 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
         qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
         class = object_class_get_parent(class);
     } while (class != object_class_by_name(TYPE_DEVICE));
-    bus_print_dev(dev->parent_bus, mon, dev, indent + 2);
+    bus_print_dev(dev->parent_bus, mon, dev, indent);
     QLIST_FOREACH(child, &dev->child_bus, sibling) {
         qbus_print(mon, child, indent);
     }
commit 774a8850d708aeb6dd6de493c28b374098c1a4c3
Author: Stefan Hajnoczi <stefanha at linux.vnet.ibm.com>
Date:   Tue Aug 28 15:26:49 2012 +0100

    qemu-iotests: add backing file smaller than image test case
    
    This new test case checks that streaming completes successfully when the
    backing file is smaller than the image file.
    
    Signed-off-by: Stefan Hajnoczi <stefanha at linux.vnet.ibm.com>
    Reviewed-by: Paolo Bonzini <pbonzini at redhat.com>
    Signed-off-by: Kevin Wolf <kwolf at redhat.com>

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index f71ab8d..55b16f8 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -125,6 +125,39 @@ class TestSingleDrive(ImageStreamingTestCase):
         result = self.vm.qmp('block-stream', device='nonexistent')
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
+
+class TestSmallerBackingFile(ImageStreamingTestCase):
+    backing_len = 1 * 1024 * 1024 # MB
+    image_len = 2 * backing_len
+
+    def setUp(self):
+        self.create_image(backing_img, self.backing_len)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img, str(self.image_len))
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    # If this hangs, then you are missing a fix to complete streaming when the
+    # end of the backing file is reached.
+    def test_stream(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/offset', self.image_len)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+
+        self.assert_no_active_streams()
+        self.vm.shutdown()
+
+
 class TestStreamStop(ImageStreamingTestCase):
     image_len = 8 * 1024 * 1024 * 1024 # GB
 
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index 3f8a935..2f7d390 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -1,5 +1,5 @@
-......
+.......
 ----------------------------------------------------------------------
-Ran 6 tests
+Ran 7 tests
 
 OK
commit 571cd9dcc7f2fee59e47913365ced7781f33c2d3
Author: Stefan Hajnoczi <stefanha at linux.vnet.ibm.com>
Date:   Tue Aug 28 15:26:48 2012 +0100

    stream: complete early if end of backing file is reached
    
    It is possible to create an image that is larger than its backing file.
    Reading beyond the end of the backing file produces zeroes if no writes
    have been made to those sectors in the image file.
    
    This patch finishes streaming early when the end of the backing file is
    reached.  Without this patch the block job hangs and continually tries
    to stream the first sectors beyond the end of the backing file.
    
    To reproduce the hung block job bug:
    
      $ qemu-img create -f qcow2 backing.qcow2 128M
      $ qemu-img create -f qcow2 -o backing_file=backing.qcow2 image.qcow2 6G
      $ qemu -drive if=virtio,cache=none,file=image.qcow2
      (qemu) block_stream virtio0
      (qemu) info block-jobs
    
    The qemu-iotests 030 streaming test still passes.
    
    Signed-off-by: Stefan Hajnoczi <stefanha at linux.vnet.ibm.com>
    Reviewed-by: Paolo Bonzini <pbonzini at redhat.com>
    Signed-off-by: Kevin Wolf <kwolf at redhat.com>

diff --git a/block/stream.c b/block/stream.c
index 37c4652..c4f87dd 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -122,6 +122,12 @@ wait:
              * known-unallocated area [sector_num, sector_num+n).  */
             ret = bdrv_co_is_allocated_above(bs->backing_hd, base,
                                              sector_num, n, &n);
+
+            /* Finish early if end of backing file has been reached */
+            if (ret == 0 && n == 0) {
+                n = end - sector_num;
+            }
+
             copy = (ret == 1);
         }
         trace_stream_one_iteration(s, sector_num, n, ret);
commit ef72f76e58107bd4096018c3db2912d28249308e
Author: Stefan Hajnoczi <stefanha at linux.vnet.ibm.com>
Date:   Tue Aug 28 14:04:27 2012 +0100

    qed: refuse unaligned zero writes with a backing file
    
    Zero writes have cluster granularity in QED.  Therefore they can only be
    used to zero entire clusters.
    
    If the zero write request leaves sectors untouched, zeroing the entire
    cluster would obscure the backing file.  Instead return -ENOTSUP, which
    is handled by block.c:bdrv_co_do_write_zeroes() and falls back to a
    regular write.
    
    The qemu-iotests 034 test cases covers this scenario.
    
    Signed-off-by: Stefan Hajnoczi <stefanha at linux.vnet.ibm.com>
    Reviewed-by: Paolo Bonzini <pbonzini at redhat.com>
    Signed-off-by: Kevin Wolf <kwolf at redhat.com>

diff --git a/block/qed.c b/block/qed.c
index a02dbfd..21cb239 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1363,10 +1363,21 @@ static int coroutine_fn bdrv_qed_co_write_zeroes(BlockDriverState *bs,
                                                  int nb_sectors)
 {
     BlockDriverAIOCB *blockacb;
+    BDRVQEDState *s = bs->opaque;
     QEDWriteZeroesCB cb = { .done = false };
     QEMUIOVector qiov;
     struct iovec iov;
 
+    /* Refuse if there are untouched backing file sectors */
+    if (bs->backing_hd) {
+        if (qed_offset_into_cluster(s, sector_num * BDRV_SECTOR_SIZE) != 0) {
+            return -ENOTSUP;
+        }
+        if (qed_offset_into_cluster(s, nb_sectors * BDRV_SECTOR_SIZE) != 0) {
+            return -ENOTSUP;
+        }
+    }
+
     /* Zero writes start without an I/O buffer.  If a buffer becomes necessary
      * then it will be allocated during request processing.
      */


More information about the Spice-commits mailing list