From 00a0770de3ddb803a0f81f6aea40b0f945154a68 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:13:57 +0200 Subject: [PATCH 01/36] uhci: Properly unmap packets on cancel / invalid pid Packets with an invalid pid, or which were cancelled have usb_packet_map() called on them on init, but not usb_packet_unmap() before being freed. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index c2f08e3735..671c712112 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -236,6 +236,7 @@ static void uhci_async_cancel(UHCIAsync *async) trace_usb_uhci_packet_cancel(async->queue->token, async->td, async->done); if (!async->done) usb_cancel_packet(&async->packet); + usb_packet_unmap(&async->packet, &async->sgl); uhci_async_free(async); } @@ -887,6 +888,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, default: /* invalid pid : frame interrupted */ + usb_packet_unmap(&async->packet, &async->sgl); uhci_async_free(async); s->status |= UHCI_STS_HCPERR; uhci_update_irq(s); From 7c2eaca4efa46e02caaec4ca7ddf05a6e461da94 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:13:58 +0200 Subject: [PATCH 02/36] uhci: Move checks to continue queuing to uhci_fill_queue() Rather then having a special check to start queuing after the first packet, and then another check for the other packets in uhci_fill_queue(), simply check the previous packet beforehand in uhci_fill_queue() Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 671c712112..600d095573 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -991,7 +991,8 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) UHCI_TD ptd; int ret; - while (is_valid(plink)) { + ptd.ctrl = td->ctrl; + while (is_valid(plink) && !(ptd.ctrl & TD_CTRL_SPD)) { pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd)); le32_to_cpus(&ptd.link); le32_to_cpus(&ptd.ctrl); @@ -1010,9 +1011,6 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) } assert(ret == TD_RESULT_ASYNC_START); assert(int_mask == 0); - if (ptd.ctrl & TD_CTRL_SPD) { - break; - } plink = ptd.link; } } @@ -1110,9 +1108,7 @@ static void uhci_process_frame(UHCIState *s) case TD_RESULT_ASYNC_START: trace_usb_uhci_td_async(curr_qh & ~0xf, link & ~0xf); - if (is_valid(td.link) && !(td.ctrl & TD_CTRL_SPD)) { - uhci_fill_queue(s, &td); - } + uhci_fill_queue(s, &td); link = curr_qh ? qh.link : td.link; continue; From 549a3c3d9665bdc0246fb6594b6e36ce3afadd21 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:13:59 +0200 Subject: [PATCH 03/36] ehci: Get rid of packet tbytes field This field is used in some places to track the tbytes field of the token, but in other places the field is used directly, use it directly everywhere for consistency. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 6c65a734c8..82f4dbd15a 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -362,7 +362,6 @@ struct EHCIPacket { USBPacket packet; QEMUSGList sgl; int pid; - uint32_t tbytes; enum async_state async; int usb_status; }; @@ -1505,15 +1504,16 @@ static void ehci_execute_complete(EHCIQueue *q) } } else { // TODO check 4.12 for splits + uint32_t tbytes = get_field(q->qh.token, QTD_TOKEN_TBYTES); - if (p->tbytes && p->pid == USB_TOKEN_IN) { - p->tbytes -= p->usb_status; + if (tbytes && p->pid == USB_TOKEN_IN) { + tbytes -= p->usb_status; } else { - p->tbytes = 0; + tbytes = 0; } - DPRINTF("updating tbytes to %d\n", p->tbytes); - set_field(&q->qh.token, p->tbytes, QTD_TOKEN_TBYTES); + DPRINTF("updating tbytes to %d\n", tbytes); + set_field(&q->qh.token, tbytes, QTD_TOKEN_TBYTES); } ehci_finish_transfer(q, p->usb_status); usb_packet_unmap(&p->packet, &p->sgl); @@ -1544,8 +1544,7 @@ static int ehci_execute(EHCIPacket *p, const char *action) return USB_RET_PROCERR; } - p->tbytes = (p->qtd.token & QTD_TOKEN_TBYTES_MASK) >> QTD_TOKEN_TBYTES_SH; - if (p->tbytes > BUFF_SIZE) { + if (get_field(p->qtd.token, QTD_TOKEN_TBYTES) > BUFF_SIZE) { ehci_trace_guest_bug(p->queue->ehci, "guest requested more bytes than allowed"); return USB_RET_PROCERR; @@ -1582,10 +1581,9 @@ static int ehci_execute(EHCIPacket *p, const char *action) trace_usb_ehci_packet_action(p->queue, p, action); ret = usb_handle_packet(p->queue->dev, &p->packet); - DPRINTF("submit: qh %x next %x qtd %x pid %x len %zd " - "(total %d) endp %x ret %d\n", + DPRINTF("submit: qh %x next %x qtd %x pid %x len %zd endp %x ret %d\n", q->qhaddr, q->qh.next, q->qtdaddr, q->pid, - q->packet.iov.size, q->tbytes, endp, ret); + q->packet.iov.size, endp, ret); if (ret > BUFF_SIZE) { fprintf(stderr, "ret from usb_handle_packet > BUFF_SIZE\n"); From cf08a8a1f600b2ac25f72cf5736247f3e95cc43d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:00 +0200 Subject: [PATCH 04/36] ehci: Set int flag on a short input packet According to 4.15.1.2 an interrupt must be raised when a short packet is received. If we don't do this it may take a significant time for the guest to notice a short trasnfer has completed, since only the last td will have its IOC flag set, and a short transfer may complete in an earlier packet. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 82f4dbd15a..8b4e3c859e 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1508,6 +1508,10 @@ static void ehci_execute_complete(EHCIQueue *q) if (tbytes && p->pid == USB_TOKEN_IN) { tbytes -= p->usb_status; + if (tbytes) { + /* 4.15.1.2 must raise int on a short input packet */ + ehci_raise_irq(q->ehci, USBSTS_INT); + } } else { tbytes = 0; } From 0262f65aaae49d582e7d4e4b1b5c8cfe4cd19d6d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:01 +0200 Subject: [PATCH 05/36] ehci: Improve latency of interrupt delivery and async schedule scanning While doing various performance tests of reading from USB mass storage devices I noticed the following:: 1) When an async handled packet completes, we don't immediately report an interrupt to the guest, instead we wait for the frame-timer to run and report it from there 2) If 1) has been fixed and an async handled packet takes a while to complete, then async_stepdown will become a high value, which means that there will be a large latency before any new packets queued by the guest in response to the interrupt get seen 1) was done deliberately as part of commit f0ad01f92: http://www.kraxel.org/cgit/qemu/commit/?h=usb.57&id=f0ad01f92ca02eee7cadbfd225c5de753ebd5fce Since setting the interrupt immediately on async packet completion was causing issues with Linux guests, I believe this recently fixed Linux bug explains why this is happening: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=361aabf395e4a23cf554cf4ec0c0c6963b8beb01 Note that we can *not* count on this fix being present in all Linux guests! I was hoping that the recently added support for Interrupt Threshold Control would fix the issues with Linux guests, but adding a simple ehci_commit_irq() call to ehci_async_bh() still caused problems with Linux guests. The problem is, that when doing ehci_commit_irq() from ehci_async_bh(), the "old" frindex value is used to calculate usbsts_frindex, and when the frame-timer then runs possibly very shortly after ehci_async_bh(), it increases the frame-timer, and thus any interrupts raised from that frame-timer run, will also get reported to the guest immediately, rather then being delayed to the next frame-timer run. Luckily the solution for this is simple, this means that we need to increase frindex before calling ehci_commit_irq() from ehci_async_bh(), which in the end boils down to simple calling ehci_frame_timer() instead of ehci_async_bh() from the bh. This may seem like it causes a lot of extra work to be done, but this is not true. Any work done from the frame-timer processing the periodic schedule is work which then does not need to be done the next time the frame timer runs, also the frame-timer will re-arm itself at (possibly) a later time then it was armed for saving a vmexit at that time. As an additional advantage moving to simply calling the frame-timer also fixes 2) as the packet completion will set async_stepdown to 0, and the re-arming of the timer with an async_stepdown of 0 ensures that any newly queued up packets get seen in a reasonable amount of time. This improves the speed (MB/s) of a Linux guest reading from a USB mass storage device by a factor of 1.5 - 1.7 with input pipelining disabled, and by a factor of 1.8 with input pipelining enabled. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8b4e3c859e..f9ae05e71d 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1244,7 +1244,7 @@ static void ehci_opreg_write(void *ptr, hwaddr addr, s->usbcmd = val; /* Set usbcmd for ehci_update_halt() */ ehci_update_halt(s); s->async_stepdown = 0; - qemu_mod_timer(s->frame_timer, qemu_get_clock_ns(vm_clock)); + qemu_bh_schedule(s->async_bh); } break; @@ -2510,12 +2510,6 @@ static void ehci_frame_timer(void *opaque) } } -static void ehci_async_bh(void *opaque) -{ - EHCIState *ehci = opaque; - ehci_advance_async_state(ehci); -} - static const MemoryRegionOps ehci_mmio_caps_ops = { .read = ehci_caps_read, .valid.min_access_size = 1, @@ -2744,7 +2738,7 @@ static int usb_ehci_initfn(PCIDevice *dev) } s->frame_timer = qemu_new_timer_ns(vm_clock, ehci_frame_timer, s); - s->async_bh = qemu_bh_new(ehci_async_bh, s); + s->async_bh = qemu_bh_new(ehci_frame_timer, s); QTAILQ_INIT(&s->aqueues); QTAILQ_INIT(&s->pqueues); usb_packet_init(&s->ipacket); From 44272b0f88247e2d2960c0ef19b546c206a10080 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:02 +0200 Subject: [PATCH 06/36] ehci: Speed up the timer of raising int from the async schedule Often the guest will queue up new packets in response to a packet, in the async schedule with its IOC flag set, completing. By speeding up the frame-timer, we notice these new packets earlier. This increases the speed (MB/s) of a Linux guest reading from a USB mass storage device by a factor of 1.15 on top of the "Improve latency of interrupt delivery" speed-ups, both with and without input pipelining enabled. I've not tested the speed-up of this patch without the "Improve latency of interrupt delivery" patch. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index f9ae05e71d..d600f08f6b 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -443,6 +443,7 @@ struct EHCIState { uint64_t last_run_ns; uint32_t async_stepdown; + bool int_req_by_async; }; #define SET_LAST_RUN_CLOCK(s) \ @@ -1529,6 +1530,9 @@ static void ehci_execute_complete(EHCIQueue *q) if (q->qh.token & QTD_TOKEN_IOC) { ehci_raise_irq(q->ehci, USBSTS_INT); + if (q->async) { + q->ehci->int_req_by_async = true; + } } } @@ -2504,8 +2508,15 @@ static void ehci_frame_timer(void *opaque) } if (need_timer) { - expire_time = t_now + (get_ticks_per_sec() + /* If we've raised int, we speed up the timer, so that we quickly + * notice any new packets queued up in response */ + if (ehci->int_req_by_async && (ehci->usbsts & USBSTS_INT)) { + expire_time = t_now + get_ticks_per_sec() / (FRAME_TIMER_FREQ * 2); + ehci->int_req_by_async = false; + } else { + expire_time = t_now + (get_ticks_per_sec() * (ehci->async_stepdown+1) / FRAME_TIMER_FREQ); + } qemu_mod_timer(ehci->frame_timer, expire_time); } } From e3a36bce1d0123d003855f7731494e6d6f550fcc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:03 +0200 Subject: [PATCH 07/36] ehci: Detect going in circles when filling the queue For ctrl endpoints Windows (atleast Win7) creates circular td lists, so far these were not a problem because we would stop filling the queue if altnext was set. Since further patches in this patchset remove the altnext check this does become a problem and we need detection for going in circles. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index d600f08f6b..8bd87c76d5 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2073,7 +2073,7 @@ static int ehci_fill_queue(EHCIPacket *p) { EHCIQueue *q = p->queue; EHCIqtd qtd = p->qtd; - uint32_t qtdaddr; + uint32_t qtdaddr, start_addr = p->qtdaddr; for (;;) { if (NLPTR_TBIT(qtd.altnext) == 0) { @@ -2083,6 +2083,13 @@ static int ehci_fill_queue(EHCIPacket *p) break; } qtdaddr = qtd.next; + /* + * Detect circular td lists, Windows creates these, counting on the + * active bit going low after execution to make the queue stop. + */ + if (qtdaddr == start_addr) { + break; + } get_dwords(q->ehci, NLPTR_GET(qtdaddr), (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2); ehci_trace_qtd(q, NLPTR_GET(qtdaddr), &qtd); From b4ea86649915eca5551a5166f76e7a9d9032de50 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:04 +0200 Subject: [PATCH 08/36] ehci: Retry to fill the queue while waiting for td completion If the guest is using multiple transfers to try and keep the usb bus busy / used at maximum efficiency, currently we would see / do the following: 1) submit transfer 1 to the device 2) submit transfer 2 to the device 3) report transfer 1 completion to guest 4) report transfer 2 completion to guest 5) submit transfer 1 to the device 6) report transfer 1 completion to guest 7) submit transfer 2 to the device 8) report transfer 2 completion to guest etc. So after the initial submission we would effectively only have 1 transfer in flight, rather then 2. This is caused by us not checking the queue for addition of new transfers by the guest (ie the resubmission of a recently finished transfer), while waiting for a pending transfer to complete. This patch does add a check for this, changing the sequence to: 1) submit transfer 1 to the device 2) submit transfer 2 to the device 3) report transfer 1 completion to guest 4) submit transfer 1 to the device 5) report transfer 2 completion to guest 6) submit transfer 2 to the device etc. Thus keeping 2 transfers in flight (most of the time, and always 1), as intended by the guest. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8bd87c76d5..a02fe9642f 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -381,7 +381,7 @@ struct EHCIQueue { uint32_t qhaddr; /* address QH read from */ uint32_t qtdaddr; /* address QTD read from */ USBDevice *dev; - QTAILQ_HEAD(, EHCIPacket) packets; + QTAILQ_HEAD(pkts_head, EHCIPacket) packets; }; typedef QTAILQ_HEAD(EHCIQueueHead, EHCIQueue) EHCIQueueHead; @@ -488,6 +488,7 @@ static const char *ehci_mmio_names[] = { static int ehci_state_executing(EHCIQueue *q); static int ehci_state_writeback(EHCIQueue *q); +static int ehci_fill_queue(EHCIPacket *p); static const char *nr2str(const char **n, size_t len, uint32_t nr) { @@ -1994,7 +1995,7 @@ static int ehci_state_fetchqtd(EHCIQueue *q) { EHCIqtd qtd; EHCIPacket *p; - int again = 0; + int again = 1; get_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2); @@ -2022,7 +2023,6 @@ static int ehci_state_fetchqtd(EHCIQueue *q) p = NULL; } ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); - again = 1; } else if (p != NULL) { switch (p->async) { case EHCI_ASYNC_NONE: @@ -2031,6 +2031,9 @@ static int ehci_state_fetchqtd(EHCIQueue *q) ehci_set_state(q->ehci, q->async, EST_EXECUTE); break; case EHCI_ASYNC_INFLIGHT: + /* Check if the guest has added new tds to the queue */ + again = (ehci_fill_queue(QTAILQ_LAST(&q->packets, pkts_head)) == + USB_RET_PROCERR) ? -1 : 1; /* Unfinished async handled packet, go horizontal */ ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); break; @@ -2042,13 +2045,11 @@ static int ehci_state_fetchqtd(EHCIQueue *q) ehci_set_state(q->ehci, q->async, EST_EXECUTING); break; } - again = 1; } else { p = ehci_alloc_packet(q); p->qtdaddr = q->qtdaddr; p->qtd = qtd; ehci_set_state(q->ehci, q->async, EST_EXECUTE); - again = 1; } return again; From 3151f2096dd676dc42e81ef0d55ae80780fd8769 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:05 +0200 Subject: [PATCH 09/36] xhci: Add a xhci_ep_nuke_one_xfer helper function Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 49 +++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 37b3dbbcb6..47d5702d1e 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1082,6 +1082,35 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid, return CC_SUCCESS; } +static int xhci_ep_nuke_one_xfer(XHCITransfer *t) +{ + int killed = 0; + + if (t->running_async) { + usb_cancel_packet(&t->packet); + t->running_async = 0; + t->cancelled = 1; + DPRINTF("xhci: cancelling transfer, waiting for it to complete\n"); + killed = 1; + } + if (t->running_retry) { + XHCIEPContext *epctx = t->xhci->slots[t->slotid-1].eps[t->epid-1]; + if (epctx) { + epctx->retry = NULL; + qemu_del_timer(epctx->kick_timer); + } + t->running_retry = 0; + } + if (t->trbs) { + g_free(t->trbs); + } + + t->trbs = NULL; + t->trb_count = t->trb_alloced = 0; + + return killed; +} + static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, unsigned int epid) { @@ -1103,25 +1132,7 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, xferi = epctx->next_xfer; for (i = 0; i < TD_QUEUE; i++) { - XHCITransfer *t = &epctx->transfers[xferi]; - if (t->running_async) { - usb_cancel_packet(&t->packet); - t->running_async = 0; - t->cancelled = 1; - DPRINTF("xhci: cancelling transfer %d, waiting for it to complete...\n", i); - killed++; - } - if (t->running_retry) { - t->running_retry = 0; - epctx->retry = NULL; - qemu_del_timer(epctx->kick_timer); - } - if (t->trbs) { - g_free(t->trbs); - } - - t->trbs = NULL; - t->trb_count = t->trb_alloced = 0; + killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi]); xferi = (xferi + 1) % TD_QUEUE; } return killed; From d0ff81b871af3c6cf31c807dfdd480b9d1032780 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:06 +0200 Subject: [PATCH 10/36] usb: Rename __usb_packet_complete to usb_packet_complete_one And make it available for use outside of core.c Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb.h | 1 + hw/usb/core.c | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index 48c8926a76..01dd423629 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -370,6 +370,7 @@ USBDevice *usb_find_device(USBPort *port, uint8_t addr); int usb_handle_packet(USBDevice *dev, USBPacket *p); void usb_packet_complete(USBDevice *dev, USBPacket *p); +void usb_packet_complete_one(USBDevice *dev, USBPacket *p); void usb_cancel_packet(USBPacket * p); void usb_ep_init(USBDevice *dev); diff --git a/hw/usb/core.c b/hw/usb/core.c index b9f1f7a8be..e2e31ca439 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -412,10 +412,11 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) return ret; } -static void __usb_packet_complete(USBDevice *dev, USBPacket *p) +void usb_packet_complete_one(USBDevice *dev, USBPacket *p) { USBEndpoint *ep = p->ep; + assert(QTAILQ_FIRST(&ep->queue) == p); assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK); if (p->result < 0) { @@ -435,8 +436,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) int ret; usb_packet_check_state(p, USB_PACKET_ASYNC); - assert(QTAILQ_FIRST(&ep->queue) == p); - __usb_packet_complete(dev, p); + usb_packet_complete_one(dev, p); while (!ep->halted && !QTAILQ_EMPTY(&ep->queue)) { p = QTAILQ_FIRST(&ep->queue); @@ -450,7 +450,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) break; } p->result = ret; - __usb_packet_complete(ep->dev, p); + usb_packet_complete_one(ep->dev, p); } } From 36dfe324fd4b5efd9ef1a5b4c352bbb158952e24 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:07 +0200 Subject: [PATCH 11/36] usb: Add USB_RET_ADD_TO_QUEUE packet result code This can be used by usb-device code which wishes to process an entire endpoint queue at once, to do this the usb-device code returns USB_RET_ADD_TO_QUEUE from its handle_data class method and defines a flush_ep_queue class method to call when the hcd is done queuing up packets. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb.h | 21 +++++++++++++++------ hw/usb/bus.c | 8 ++++++++ hw/usb/core.c | 4 ++++ hw/usb/hcd-ehci.c | 4 ++++ hw/usb/hcd-musb.c | 1 + hw/usb/hcd-ohci.c | 2 ++ hw/usb/hcd-uhci.c | 16 +++++++++++----- hw/usb/hcd-xhci.c | 6 ++++++ 8 files changed, 51 insertions(+), 11 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index 01dd423629..435cd42a03 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -38,12 +38,13 @@ #define USB_TOKEN_IN 0x69 /* device -> host */ #define USB_TOKEN_OUT 0xe1 /* host -> device */ -#define USB_RET_NODEV (-1) -#define USB_RET_NAK (-2) -#define USB_RET_STALL (-3) -#define USB_RET_BABBLE (-4) -#define USB_RET_IOERROR (-5) -#define USB_RET_ASYNC (-6) +#define USB_RET_NODEV (-1) +#define USB_RET_NAK (-2) +#define USB_RET_STALL (-3) +#define USB_RET_BABBLE (-4) +#define USB_RET_IOERROR (-5) +#define USB_RET_ASYNC (-6) +#define USB_RET_ADD_TO_QUEUE (-7) #define USB_SPEED_LOW 0 #define USB_SPEED_FULL 1 @@ -293,6 +294,12 @@ typedef struct USBDeviceClass { void (*set_interface)(USBDevice *dev, int interface, int alt_old, int alt_new); + /* + * Called when the hcd is done queuing packets for an endpoint, only + * necessary for devices which can return USB_RET_ADD_TO_QUEUE. + */ + void (*flush_ep_queue)(USBDevice *dev, USBEndpoint *ep); + const char *product_desc; const USBDesc *usb_desc; } USBDeviceClass; @@ -507,6 +514,8 @@ int usb_device_handle_data(USBDevice *dev, USBPacket *p); void usb_device_set_interface(USBDevice *dev, int interface, int alt_old, int alt_new); +void usb_device_flush_ep_queue(USBDevice *dev, USBEndpoint *ep); + const char *usb_device_get_product_desc(USBDevice *dev); const USBDesc *usb_device_get_usb_desc(USBDevice *dev); diff --git a/hw/usb/bus.c b/hw/usb/bus.c index b649360dd3..8066291b34 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -181,6 +181,14 @@ void usb_device_set_interface(USBDevice *dev, int interface, } } +void usb_device_flush_ep_queue(USBDevice *dev, USBEndpoint *ep) +{ + USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev); + if (klass->flush_ep_queue) { + klass->flush_ep_queue(dev, ep); + } +} + static int usb_qdev_init(DeviceState *qdev) { USBDevice *dev = USB_DEVICE(qdev); diff --git a/hw/usb/core.c b/hw/usb/core.c index e2e31ca439..014e3ac090 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -393,6 +393,10 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) if (ret == USB_RET_ASYNC) { usb_packet_set_state(p, USB_PACKET_ASYNC); QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue); + } else if (ret == USB_RET_ADD_TO_QUEUE) { + usb_packet_set_state(p, USB_PACKET_QUEUED); + QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue); + ret = USB_RET_ASYNC; } else { /* * When pipelining is enabled usb-devices must always return async, diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index a02fe9642f..d11311e87f 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2072,6 +2072,7 @@ static int ehci_state_horizqh(EHCIQueue *q) static int ehci_fill_queue(EHCIPacket *p) { + USBEndpoint *ep = p->packet.ep; EHCIQueue *q = p->queue; EHCIqtd qtd = p->qtd; uint32_t qtdaddr, start_addr = p->qtdaddr; @@ -2107,6 +2108,9 @@ static int ehci_fill_queue(EHCIPacket *p) assert(p->usb_status == USB_RET_ASYNC); p->async = EHCI_ASYNC_INFLIGHT; } + if (p->usb_status != USB_RET_PROCERR) { + usb_device_flush_ep_queue(ep->dev, ep); + } return p->usb_status; } diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c index dc114fed2a..212dd12fc7 100644 --- a/hw/usb/hcd-musb.c +++ b/hw/usb/hcd-musb.c @@ -635,6 +635,7 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep, ret = usb_handle_packet(dev, &ep->packey[dir].p); if (ret == USB_RET_ASYNC) { + usb_device_flush_ep_queue(dev, uep); ep->status[dir] = len; return; } diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 0cc1e5de85..31dcfbb0c6 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -816,6 +816,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len); ret = usb_handle_packet(dev, &ohci->usb_packet); if (ret == USB_RET_ASYNC) { + usb_device_flush_ep_queue(dev, ep); return 1; } } @@ -1018,6 +1019,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed) DPRINTF("ret=%d\n", ret); #endif if (ret == USB_RET_ASYNC) { + usb_device_flush_ep_queue(dev, ep); ohci->async_td = addr; return 1; } diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 600d095573..46e544b910 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -818,7 +818,8 @@ out: } static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, - uint32_t *int_mask, bool queuing) + uint32_t *int_mask, bool queuing, + struct USBEndpoint **ep_ret) { UHCIAsync *async; int len = 0, max_len; @@ -870,6 +871,9 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, dev = uhci_find_device(s, (td->token >> 8) & 0x7f); ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf); + if (ep_ret) { + *ep_ret = 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); @@ -983,7 +987,7 @@ static int qhdb_insert(QhDb *db, uint32_t addr) return 0; } -static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) +static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep) { uint32_t int_mask = 0; uint32_t plink = td->link; @@ -1005,7 +1009,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) break; } trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token); - ret = uhci_handle_td(s, plink, &ptd, &int_mask, true); + ret = uhci_handle_td(s, plink, &ptd, &int_mask, true, NULL); if (ret == TD_RESULT_ASYNC_CONT) { break; } @@ -1013,12 +1017,14 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) assert(int_mask == 0); plink = ptd.link; } + usb_device_flush_ep_queue(ep->dev, ep); } 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; + struct USBEndpoint *curr_ep; int cnt, ret; UHCI_TD td; UHCI_QH qh; @@ -1089,7 +1095,7 @@ static void uhci_process_frame(UHCIState *s) trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token); old_td_ctrl = td.ctrl; - ret = uhci_handle_td(s, link, &td, &int_mask, false); + ret = uhci_handle_td(s, link, &td, &int_mask, false, &curr_ep); if (old_td_ctrl != td.ctrl) { /* update the status bits of the TD */ val = cpu_to_le32(td.ctrl); @@ -1108,7 +1114,7 @@ static void uhci_process_frame(UHCIState *s) case TD_RESULT_ASYNC_START: trace_usb_uhci_td_async(curr_qh & ~0xf, link & ~0xf); - uhci_fill_queue(s, &td); + uhci_fill_queue(s, &td, curr_ep); link = curr_qh ? qh.link : td.link; continue; diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 47d5702d1e..e8929a0785 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1652,6 +1652,7 @@ static int xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid) { XHCIEPContext *epctx; + USBEndpoint *ep = NULL; uint64_t mfindex; int length; int i; @@ -1745,12 +1746,14 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid if (epid == 1) { if (xhci_fire_ctl_transfer(xhci, xfer) >= 0) { epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; + ep = xfer->packet.ep; } else { fprintf(stderr, "xhci: error firing CTL transfer\n"); } } else { if (xhci_fire_transfer(xhci, xfer, epctx) >= 0) { epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; + ep = xfer->packet.ep; } else { if (!xfer->iso_xfer) { fprintf(stderr, "xhci: error firing data transfer\n"); @@ -1767,6 +1770,9 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid break; } } + if (ep) { + usb_device_flush_ep_queue(ep->dev, ep); + } } static TRBCCode xhci_enable_slot(XHCIState *xhci, unsigned int slotid) From 0cae7b1a004d6857e3bde3d511d7efa39d3cb48a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:08 +0200 Subject: [PATCH 12/36] usb: Move clearing of queue on halt to the core hcds which queue up more then one packet at once (uhci, ehci and xhci), must clear the queue after an error which has caused the queue to halt. Currently this is handled as a special case inside the hcd code, this patch instead adds an USB_RET_REMOVE_FROM_QUEUE packet result code, teaches the 3 hcds about this and moves the clearing of the queue on a halt into the USB core. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb.h | 1 + hw/usb/core.c | 8 +++++++- hw/usb/hcd-ehci.c | 22 ++++++++-------------- hw/usb/hcd-uhci.c | 22 ++++++---------------- hw/usb/hcd-xhci.c | 4 ++++ 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index 435cd42a03..ead03c9f6b 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -45,6 +45,7 @@ #define USB_RET_IOERROR (-5) #define USB_RET_ASYNC (-6) #define USB_RET_ADD_TO_QUEUE (-7) +#define USB_RET_REMOVE_FROM_QUEUE (-8) #define USB_SPEED_LOW 0 #define USB_SPEED_FULL 1 diff --git a/hw/usb/core.c b/hw/usb/core.c index 014e3ac090..5a97a0e412 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -442,8 +442,14 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) usb_packet_check_state(p, USB_PACKET_ASYNC); usb_packet_complete_one(dev, p); - while (!ep->halted && !QTAILQ_EMPTY(&ep->queue)) { + while (!QTAILQ_EMPTY(&ep->queue)) { p = QTAILQ_FIRST(&ep->queue); + if (ep->halted) { + /* Empty the queue on a halt */ + p->result = USB_RET_REMOVE_FROM_QUEUE; + dev->port->ops->complete(dev->port, p); + continue; + } if (p->state == USB_PACKET_ASYNC) { break; } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index d11311e87f..74a2587eba 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1457,8 +1457,15 @@ static void ehci_async_complete_packet(USBPort *port, USBPacket *packet) } p = container_of(packet, EHCIPacket, packet); - trace_usb_ehci_packet_action(p->queue, p, "wakeup"); assert(p->async == EHCI_ASYNC_INFLIGHT); + + if (packet->result == USB_RET_REMOVE_FROM_QUEUE) { + trace_usb_ehci_packet_action(p->queue, p, "remove"); + ehci_free_packet(p); + return; + } + + trace_usb_ehci_packet_action(p->queue, p, "wakeup"); p->async = EHCI_ASYNC_FINISHED; p->usb_status = packet->result; @@ -2216,19 +2223,6 @@ 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 46e544b910..00dc9d538f 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -744,22 +744,6 @@ 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; @@ -918,6 +902,12 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet) UHCIAsync *async = container_of(packet, UHCIAsync, packet); UHCIState *s = async->queue->uhci; + if (packet->result == USB_RET_REMOVE_FROM_QUEUE) { + uhci_async_unlink(async); + uhci_async_cancel(async); + return; + } + if (async->isoc) { UHCI_TD td; uint32_t link = async->td; diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index e8929a0785..1f437cc15f 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2839,6 +2839,10 @@ static void xhci_complete(USBPort *port, USBPacket *packet) { XHCITransfer *xfer = container_of(packet, XHCITransfer, packet); + if (packet->result == USB_RET_REMOVE_FROM_QUEUE) { + xhci_ep_nuke_one_xfer(xfer); + return; + } xhci_complete_packet(xfer, packet->result); xhci_kick_ep(xfer->xhci, xfer->slotid, xfer->epid); } From 6ba43f1f6b60159e731b1f37ffb53ab9ab59efa9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:09 +0200 Subject: [PATCH 13/36] usb: Move short-not-ok handling to the core After a short-not-ok packet ending short, we should not advance the queue. Move enforcing this to the core, rather then handling it in the hcd code. This may result in the queue now actually containing multiple input packets (which would not happen before), and this requires special handling in combination with pipelining, so disable pipleining for input endpoints (for now). Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb.h | 4 +++- hw/usb/core.c | 6 ++++-- hw/usb/hcd-ehci.c | 9 ++++----- hw/usb/hcd-musb.c | 2 +- hw/usb/hcd-ohci.c | 4 ++-- hw/usb/hcd-uhci.c | 7 ++++--- hw/usb/hcd-xhci.c | 2 +- hw/usb/host-linux.c | 3 ++- hw/usb/redirect.c | 18 ++++++++++++------ 9 files changed, 33 insertions(+), 22 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index ead03c9f6b..1fcf79c72f 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -351,6 +351,7 @@ struct USBPacket { USBEndpoint *ep; QEMUIOVector iov; uint64_t parameter; /* control transfers */ + bool short_not_ok; int result; /* transfer length or USB_RET_* status code */ /* Internal use by the USB layer. */ USBPacketState state; @@ -360,7 +361,8 @@ 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, uint64_t id); +void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id, + bool short_not_ok); 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 5a97a0e412..f4a5ad2333 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -423,7 +423,7 @@ void usb_packet_complete_one(USBDevice *dev, USBPacket *p) assert(QTAILQ_FIRST(&ep->queue) == p); assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK); - if (p->result < 0) { + if (p->result < 0 || (p->short_not_ok && (p->result < p->iov.size))) { ep->halted = true; } usb_packet_set_state(p, USB_PACKET_COMPLETE); @@ -532,7 +532,8 @@ void usb_packet_set_state(USBPacket *p, USBPacketState state) p->state = state; } -void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id) +void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id, + bool short_not_ok) { assert(!usb_packet_is_inflight(p)); assert(p->iov.iov != NULL); @@ -541,6 +542,7 @@ void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id) p->ep = ep; p->result = 0; p->parameter = 0; + p->short_not_ok = short_not_ok; qemu_iovec_reset(&p->iov); usb_packet_set_state(p, USB_PACKET_SETUP); } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 74a2587eba..8e8ec6b6c5 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1551,6 +1551,7 @@ static int ehci_execute(EHCIPacket *p, const char *action) USBEndpoint *ep; int ret; int endp; + bool spd; assert(p->async == EHCI_ASYNC_NONE || p->async == EHCI_ASYNC_INITIALIZED); @@ -1590,7 +1591,8 @@ static int ehci_execute(EHCIPacket *p, const char *action) return USB_RET_PROCERR; } - usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr); + spd = (p->pid == USB_TOKEN_IN && NLPTR_TBIT(p->qtd.altnext) == 0); + usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr, spd); usb_packet_map(&p->packet, &p->sgl); p->async = EHCI_ASYNC_INITIALIZED; } @@ -1660,7 +1662,7 @@ static int ehci_process_itd(EHCIState *ehci, dev = ehci_find_device(ehci, devaddr); ep = usb_ep_get(dev, pid, endp); if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) { - usb_packet_setup(&ehci->ipacket, pid, ep, addr); + usb_packet_setup(&ehci->ipacket, pid, ep, addr, false); usb_packet_map(&ehci->ipacket, &ehci->isgl); ret = usb_handle_packet(dev, &ehci->ipacket); assert(ret != USB_RET_ASYNC); @@ -2085,9 +2087,6 @@ static int ehci_fill_queue(EHCIPacket *p) uint32_t qtdaddr, start_addr = p->qtdaddr; for (;;) { - if (NLPTR_TBIT(qtd.altnext) == 0) { - break; - } if (NLPTR_TBIT(qtd.next) != 0) { break; } diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c index 212dd12fc7..c4309a4e6b 100644 --- a/hw/usb/hcd-musb.c +++ b/hw/usb/hcd-musb.c @@ -627,7 +627,7 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep, 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, - (dev->addr << 16) | (uep->nr << 8) | pid); + (dev->addr << 16) | (uep->nr << 8) | pid, false); 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 31dcfbb0c6..00e2e1acac 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, addr); + usb_packet_setup(&ohci->usb_packet, pid, ep, addr, false); usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len); ret = usb_handle_packet(dev, &ohci->usb_packet); if (ret == USB_RET_ASYNC) { @@ -1012,7 +1012,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, addr); + usb_packet_setup(&ohci->usb_packet, pid, ep, addr, !flag_r); 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 00dc9d538f..953897b64c 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -808,6 +808,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, UHCIAsync *async; int len = 0, max_len; uint8_t pid; + bool spd; USBDevice *dev; USBEndpoint *ep; @@ -852,13 +853,14 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, max_len = ((td->token >> 21) + 1) & 0x7ff; pid = td->token & 0xff; + spd = (pid == USB_TOKEN_IN && (td->ctrl & TD_CTRL_SPD) != 0); dev = uhci_find_device(s, (td->token >> 8) & 0x7f); ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf); if (ep_ret) { *ep_ret = ep; } - usb_packet_setup(&async->packet, pid, ep, addr); + usb_packet_setup(&async->packet, pid, ep, addr, spd); qemu_sglist_add(&async->sgl, td->buffer, max_len); usb_packet_map(&async->packet, &async->sgl); @@ -985,8 +987,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep) UHCI_TD ptd; int ret; - ptd.ctrl = td->ctrl; - while (is_valid(plink) && !(ptd.ctrl & TD_CTRL_SPD)) { + while (is_valid(plink)) { pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd)); le32_to_cpus(&ptd.link); le32_to_cpus(&ptd.ctrl); diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 1f437cc15f..ebfc5b892f 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1446,7 +1446,7 @@ static int xhci_setup_packet(XHCITransfer *xfer) ep = usb_ep_get(dev, dir, xfer->epid >> 1); } - usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr); + usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr, false); xhci_xfer_map(xfer); DPRINTF("xhci: setup packet pid 0x%x addr %d ep %d\n", xfer->packet.pid, dev->addr, ep->nr); diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 44f1a64b35..3a258b4bd4 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -1224,7 +1224,8 @@ static int usb_linux_update_endp_table(USBHostDevice *s) usb_ep_set_type(&s->dev, pid, ep, type); usb_ep_set_ifnum(&s->dev, pid, ep, interface); if ((s->options & (1 << USB_HOST_OPT_PIPELINE)) && - (type == USB_ENDPOINT_XFER_BULK)) { + (type == USB_ENDPOINT_XFER_BULK) && + (pid == USB_TOKEN_OUT)) { usb_ep_set_pipeline(&s->dev, pid, ep, true); } diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 2283565b0c..22f671b3b1 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -1270,6 +1270,16 @@ static void usbredir_interface_info(void *priv, } } +static void usbredir_set_pipeline(USBRedirDevice *dev, struct USBEndpoint *uep) +{ + if (uep->type != USB_ENDPOINT_XFER_BULK) { + return; + } + if (uep->pid == USB_TOKEN_OUT) { + uep->pipeline = true; + } +} + static void usbredir_ep_info(void *priv, struct usb_redir_ep_info_header *ep_info) { @@ -1311,9 +1321,7 @@ static void usbredir_ep_info(void *priv, dev->endpoint[i].max_packet_size = usb_ep->max_packet_size = ep_info->max_packet_size[i]; } - if (ep_info->type[i] == usb_redir_type_bulk) { - usb_ep->pipeline = true; - } + usbredir_set_pipeline(dev, usb_ep); } } @@ -1574,9 +1582,7 @@ static int usbredir_post_load(void *priv, int version_id) usb_ep->type = dev->endpoint[i].type; usb_ep->ifnum = dev->endpoint[i].interface; usb_ep->max_packet_size = dev->endpoint[i].max_packet_size; - if (dev->endpoint[i].type == usb_redir_type_bulk) { - usb_ep->pipeline = true; - } + usbredir_set_pipeline(dev, usb_ep); } return 0; } From a6fb2ddb1417fcc2d24f3231c84035fcbd90123a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:14:10 +0200 Subject: [PATCH 14/36] usb: Add an int_req flag to USBPacket Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb.h | 3 ++- hw/usb/core.c | 3 ++- hw/usb/hcd-ehci.c | 6 ++++-- hw/usb/hcd-musb.c | 2 +- hw/usb/hcd-ohci.c | 7 +++++-- hw/usb/hcd-uhci.c | 3 ++- hw/usb/hcd-xhci.c | 16 +++++++++++----- 7 files changed, 27 insertions(+), 13 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index 1fcf79c72f..3a6cc8444a 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -352,6 +352,7 @@ struct USBPacket { QEMUIOVector iov; uint64_t parameter; /* control transfers */ bool short_not_ok; + bool int_req; int result; /* transfer length or USB_RET_* status code */ /* Internal use by the USB layer. */ USBPacketState state; @@ -362,7 +363,7 @@ 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, uint64_t id, - bool short_not_ok); + bool short_not_ok, bool int_req); 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 f4a5ad2333..87a513fa7e 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -533,7 +533,7 @@ void usb_packet_set_state(USBPacket *p, USBPacketState state) } void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id, - bool short_not_ok) + bool short_not_ok, bool int_req) { assert(!usb_packet_is_inflight(p)); assert(p->iov.iov != NULL); @@ -543,6 +543,7 @@ void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id, p->result = 0; p->parameter = 0; p->short_not_ok = short_not_ok; + p->int_req = int_req; qemu_iovec_reset(&p->iov); usb_packet_set_state(p, USB_PACKET_SETUP); } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8e8ec6b6c5..8ac78adf95 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1592,7 +1592,8 @@ static int ehci_execute(EHCIPacket *p, const char *action) } spd = (p->pid == USB_TOKEN_IN && NLPTR_TBIT(p->qtd.altnext) == 0); - usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr, spd); + usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr, spd, + (p->qtd.token & QTD_TOKEN_IOC) != 0); usb_packet_map(&p->packet, &p->sgl); p->async = EHCI_ASYNC_INITIALIZED; } @@ -1662,7 +1663,8 @@ static int ehci_process_itd(EHCIState *ehci, dev = ehci_find_device(ehci, devaddr); ep = usb_ep_get(dev, pid, endp); if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) { - usb_packet_setup(&ehci->ipacket, pid, ep, addr, false); + usb_packet_setup(&ehci->ipacket, pid, ep, addr, false, + (itd->transact[i] & ITD_XACT_IOC) != 0); usb_packet_map(&ehci->ipacket, &ehci->isgl); ret = usb_handle_packet(dev, &ehci->ipacket); assert(ret != USB_RET_ASYNC); diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c index c4309a4e6b..4f5539020b 100644 --- a/hw/usb/hcd-musb.c +++ b/hw/usb/hcd-musb.c @@ -627,7 +627,7 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep, 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, - (dev->addr << 16) | (uep->nr << 8) | pid, false); + (dev->addr << 16) | (uep->nr << 8) | pid, false, true); 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 00e2e1acac..7571e9e44a 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -810,9 +810,11 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, if (completion) { ret = ohci->usb_packet.result; } else { + bool int_req = relative_frame_number == frame_count && + OHCI_BM(iso_td.flags, TD_DI) == 0; 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, addr, false); + usb_packet_setup(&ohci->usb_packet, pid, ep, addr, false, int_req); usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len); ret = usb_handle_packet(dev, &ohci->usb_packet); if (ret == USB_RET_ASYNC) { @@ -1012,7 +1014,8 @@ 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, addr, !flag_r); + usb_packet_setup(&ohci->usb_packet, pid, ep, addr, !flag_r, + OHCI_BM(td.flags, TD_DI) == 0); 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 953897b64c..4a1ea6b436 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -860,7 +860,8 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, if (ep_ret) { *ep_ret = ep; } - usb_packet_setup(&async->packet, pid, ep, addr, spd); + usb_packet_setup(&async->packet, pid, ep, addr, spd, + (td->ctrl & TD_CTRL_IOC) != 0); 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 ebfc5b892f..caa5f3e36a 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -322,6 +322,7 @@ typedef struct XHCITransfer { bool running_retry; bool cancelled; bool complete; + bool int_req; unsigned int iso_pkts; unsigned int slotid; unsigned int epid; @@ -1292,18 +1293,22 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid, return CC_SUCCESS; } -static int xhci_xfer_map(XHCITransfer *xfer) +static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer) { - int in_xfer = (xfer->packet.pid == USB_TOKEN_IN); XHCIState *xhci = xfer->xhci; int i; + xfer->int_req = false; pci_dma_sglist_init(&xfer->sgl, &xhci->pci_dev, xfer->trb_count); for (i = 0; i < xfer->trb_count; i++) { XHCITRB *trb = &xfer->trbs[i]; dma_addr_t addr; unsigned int chunk = 0; + if (trb->control & TRB_TR_IOC) { + xfer->int_req = true; + } + switch (TRB_TYPE(*trb)) { case TR_DATA: if ((!(trb->control & TRB_TR_DIR)) != (!in_xfer)) { @@ -1328,7 +1333,6 @@ static int xhci_xfer_map(XHCITransfer *xfer) } } - usb_packet_map(&xfer->packet, &xfer->sgl); return 0; err: @@ -1446,8 +1450,10 @@ static int xhci_setup_packet(XHCITransfer *xfer) ep = usb_ep_get(dev, dir, xfer->epid >> 1); } - usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr, false); - xhci_xfer_map(xfer); + xhci_xfer_create_sgl(xfer, dir == USB_TOKEN_IN); /* Also sets int_req */ + usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr, false, + xfer->int_req); + usb_packet_map(&xfer->packet, &xfer->sgl); DPRINTF("xhci: setup packet pid 0x%x addr %d ep %d\n", xfer->packet.pid, dev->addr, ep->nr); return 0; From aaac74343d761947e57a1b0cbaa7f2dd0c6a40ca Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:04 +0200 Subject: [PATCH 15/36] usb: Enforce iso endpoints never returing USB_RET_ASYNC ehci was already testing for this, and we depend in various places on no devices doing this, so lets move the check for this to the usb core. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/core.c | 1 + hw/usb/hcd-ehci.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/core.c b/hw/usb/core.c index 87a513fa7e..632a8efe47 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -391,6 +391,7 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) { ret = usb_process_one(p); if (ret == USB_RET_ASYNC) { + assert(p->ep->type != USB_ENDPOINT_XFER_ISOC); usb_packet_set_state(p, USB_PACKET_ASYNC); QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue); } else if (ret == USB_RET_ADD_TO_QUEUE) { diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8ac78adf95..f14f9d7018 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1667,7 +1667,6 @@ static int ehci_process_itd(EHCIState *ehci, (itd->transact[i] & ITD_XACT_IOC) != 0); usb_packet_map(&ehci->ipacket, &ehci->isgl); ret = usb_handle_packet(dev, &ehci->ipacket); - assert(ret != USB_RET_ASYNC); usb_packet_unmap(&ehci->ipacket, &ehci->isgl); } else { DPRINTF("ISOCH: attempt to addess non-iso endpoint\n"); From 5b352ed537e31a6b5f9e33e26429ba8b909af234 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:05 +0200 Subject: [PATCH 16/36] uhci: No need to handle async completion of isoc packets No devices ever return async for isoc endpoints and the core already enforces this. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 4a1ea6b436..129792b662 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -101,7 +101,6 @@ struct UHCIAsync { UHCIQueue *queue; QTAILQ_ENTRY(UHCIAsync) next; uint32_t td; - uint8_t isoc; uint8_t done; }; @@ -849,7 +848,6 @@ 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; max_len = ((td->token >> 21) + 1) & 0x7ff; pid = td->token & 0xff; @@ -911,30 +909,9 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet) return; } - if (async->isoc) { - UHCI_TD td; - uint32_t link = async->td; - uint32_t int_mask = 0, val; - - pci_dma_read(&s->dev, link & ~0xf, &td, sizeof(td)); - le32_to_cpus(&td.link); - le32_to_cpus(&td.ctrl); - le32_to_cpus(&td.token); - le32_to_cpus(&td.buffer); - - uhci_async_unlink(async); - uhci_complete_td(s, &td, async, &int_mask); - s->pending_int_mask |= int_mask; - - /* update the status bits of the TD */ - val = cpu_to_le32(td.ctrl); - pci_dma_write(&s->dev, (link & ~0xf) + 4, &val, sizeof(val)); - uhci_async_free(async); - } else { - async->done = 1; - if (s->frame_bytes < s->frame_bandwidth) { - qemu_bh_schedule(s->bh); - } + async->done = 1; + if (s->frame_bytes < s->frame_bandwidth) { + qemu_bh_schedule(s->bh); } } From 2f2ee2689ffe8a784a86bf10fb946772748632c7 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:06 +0200 Subject: [PATCH 17/36] uhci: cleanup: Add an unlink call to uhci_async_cancel() All callers of uhci_async_cancel() call uhci_async_unlink() first, so lets move the unlink call to uhci_async_cancel() Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 129792b662..82dd5c2358 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -232,6 +232,7 @@ static void uhci_async_unlink(UHCIAsync *async) static void uhci_async_cancel(UHCIAsync *async) { + uhci_async_unlink(async); trace_usb_uhci_packet_cancel(async->queue->token, async->td, async->done); if (!async->done) usb_cancel_packet(&async->packet); @@ -266,7 +267,6 @@ static void uhci_async_validate_end(UHCIState *s) } while (!QTAILQ_EMPTY(&queue->asyncs)) { async = QTAILQ_FIRST(&queue->asyncs); - uhci_async_unlink(async); uhci_async_cancel(async); } uhci_queue_free(queue); @@ -284,7 +284,6 @@ static void uhci_async_cancel_device(UHCIState *s, USBDevice *dev) curr->packet.ep->dev != dev) { continue; } - uhci_async_unlink(curr); uhci_async_cancel(curr); } } @@ -297,7 +296,6 @@ static void uhci_async_cancel_all(UHCIState *s) QTAILQ_FOREACH_SAFE(queue, &s->queues, next, nq) { QTAILQ_FOREACH_SAFE(curr, &queue->asyncs, next, n) { - uhci_async_unlink(curr); uhci_async_cancel(curr); } uhci_queue_free(queue); From a89e255b0cbe7c5b56348201e1c0f6d427c98fc8 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:07 +0200 Subject: [PATCH 18/36] uhci: Don't retry on error Since we are either dealing with emulated devices, where retrying is not going to help, or with redirected devices where the host OS will have already retried, don't bother retrying on failed transfers. Also move some common/indentical code out of all the error cases into the generic error path. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 62 +++++++++++++++-------------------------------- 1 file changed, 19 insertions(+), 43 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 82dd5c2358..eecd291f3d 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -696,10 +696,6 @@ static USBDevice *uhci_find_device(UHCIState *s, uint8_t addr) static void uhci_async_complete(USBPort *port, USBPacket *packet); static void uhci_process_frame(UHCIState *s); -/* return -1 if fatal error (frame must be stopped) - 0 if TD successful - 1 if TD unsuccessful or inactive -*/ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask) { int len = 0, max_len, err, ret; @@ -742,60 +738,40 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_ out: switch(ret) { + case USB_RET_NAK: + td->ctrl |= TD_CTRL_NAK; + return TD_RESULT_NEXT_QH; + case USB_RET_STALL: td->ctrl |= TD_CTRL_STALL; - td->ctrl &= ~TD_CTRL_ACTIVE; - s->status |= UHCI_STS_USBERR; - if (td->ctrl & TD_CTRL_IOC) { - *int_mask |= 0x01; - } - uhci_update_irq(s); trace_usb_uhci_packet_complete_stall(async->queue->token, async->td); - return TD_RESULT_NEXT_QH; + err = TD_RESULT_NEXT_QH; + break; case USB_RET_BABBLE: td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL; - td->ctrl &= ~TD_CTRL_ACTIVE; - s->status |= UHCI_STS_USBERR; - if (td->ctrl & TD_CTRL_IOC) { - *int_mask |= 0x01; - } - uhci_update_irq(s); /* frame interrupted */ trace_usb_uhci_packet_complete_babble(async->queue->token, async->td); - return TD_RESULT_STOP_FRAME; - - case USB_RET_NAK: - td->ctrl |= TD_CTRL_NAK; - if (pid == USB_TOKEN_SETUP) - break; - return TD_RESULT_NEXT_QH; + err = TD_RESULT_STOP_FRAME; + break; case USB_RET_IOERROR: case USB_RET_NODEV: default: - break; + td->ctrl |= TD_CTRL_TIMEOUT; + td->ctrl &= ~(3 << TD_CTRL_ERROR_SHIFT); + trace_usb_uhci_packet_complete_error(async->queue->token, async->td); + err = TD_RESULT_NEXT_QH; + break; } - /* Retry the TD if error count is not zero */ - - td->ctrl |= TD_CTRL_TIMEOUT; - err = (td->ctrl >> TD_CTRL_ERROR_SHIFT) & 3; - if (err != 0) { - err--; - if (err == 0) { - td->ctrl &= ~TD_CTRL_ACTIVE; - s->status |= UHCI_STS_USBERR; - if (td->ctrl & TD_CTRL_IOC) - *int_mask |= 0x01; - uhci_update_irq(s); - trace_usb_uhci_packet_complete_error(async->queue->token, - async->td); - } + td->ctrl &= ~TD_CTRL_ACTIVE; + s->status |= UHCI_STS_USBERR; + if (td->ctrl & TD_CTRL_IOC) { + *int_mask |= 0x01; } - td->ctrl = (td->ctrl & ~(3 << TD_CTRL_ERROR_SHIFT)) | - (err << TD_CTRL_ERROR_SHIFT); - return TD_RESULT_NEXT_QH; + uhci_update_irq(s); + return err; } static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, From 3c87c76d1a0e9ce1dde6f919e3b1b9bcabb69985 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:08 +0200 Subject: [PATCH 19/36] uhci: Drop unnecessary forward declaration of some static functions Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index eecd291f3d..7dfedef5b2 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -693,9 +693,6 @@ static USBDevice *uhci_find_device(UHCIState *s, uint8_t addr) return NULL; } -static void uhci_async_complete(USBPort *port, USBPacket *packet); -static void uhci_process_frame(UHCIState *s); - static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask) { int len = 0, max_len, err, ret; From 40507377261250fe19e441300788c77b748688cb Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:09 +0200 Subject: [PATCH 20/36] uhci: Move emptying of the queue's asyncs' queue to uhci_queue_free Cleanup: all callers of uhci_queue_free first unconditionally cancel all remaining asyncs in the queue, so lets move this to uhci_queue_free(). Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 7dfedef5b2..0335f3c490 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -160,6 +160,8 @@ typedef struct UHCI_QH { uint32_t el_link; } UHCI_QH; +static void uhci_async_cancel(UHCIAsync *async); + static inline int32_t uhci_queue_token(UHCI_TD *td) { /* covers ep, dev, pid -> identifies the endpoint */ @@ -189,6 +191,12 @@ static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td) static void uhci_queue_free(UHCIQueue *queue) { UHCIState *s = queue->uhci; + UHCIAsync *async; + + while (!QTAILQ_EMPTY(&queue->asyncs)) { + async = QTAILQ_FIRST(&queue->asyncs); + uhci_async_cancel(async); + } trace_usb_uhci_queue_del(queue->token); QTAILQ_REMOVE(&s->queues, queue, next); @@ -259,17 +267,11 @@ static void uhci_async_validate_begin(UHCIState *s) static void uhci_async_validate_end(UHCIState *s) { UHCIQueue *queue, *n; - UHCIAsync *async; QTAILQ_FOREACH_SAFE(queue, &s->queues, next, n) { - if (queue->valid > 0) { - continue; + if (!queue->valid) { + uhci_queue_free(queue); } - while (!QTAILQ_EMPTY(&queue->asyncs)) { - async = QTAILQ_FIRST(&queue->asyncs); - uhci_async_cancel(async); - } - uhci_queue_free(queue); } } @@ -292,12 +294,8 @@ static void uhci_async_cancel_device(UHCIState *s, USBDevice *dev) static void uhci_async_cancel_all(UHCIState *s) { UHCIQueue *queue, *nq; - UHCIAsync *curr, *n; QTAILQ_FOREACH_SAFE(queue, &s->queues, next, nq) { - QTAILQ_FOREACH_SAFE(curr, &queue->asyncs, next, n) { - uhci_async_cancel(curr); - } uhci_queue_free(queue); } } From 1f250cc77224f9b950c867e47d9f47e4cd46d7d4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:10 +0200 Subject: [PATCH 21/36] uhci: Rename UHCIAsync->td to UHCIAsync->td_addr We use the name td both to refer to a UHCI_TD read from guest memory as well as to refer to the guest address where a td is stored, switch over to always use td_addr in the second case for consistency. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 0335f3c490..7589a5b2d6 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -100,7 +100,7 @@ struct UHCIAsync { QEMUSGList sgl; UHCIQueue *queue; QTAILQ_ENTRY(UHCIAsync) next; - uint32_t td; + uint32_t td_addr; uint8_t done; }; @@ -203,22 +203,22 @@ static void uhci_queue_free(UHCIQueue *queue) g_free(queue); } -static UHCIAsync *uhci_async_alloc(UHCIQueue *queue, uint32_t addr) +static UHCIAsync *uhci_async_alloc(UHCIQueue *queue, uint32_t td_addr) { UHCIAsync *async = g_new0(UHCIAsync, 1); async->queue = queue; - async->td = addr; + async->td_addr = td_addr; usb_packet_init(&async->packet); pci_dma_sglist_init(&async->sgl, &queue->uhci->dev, 1); - trace_usb_uhci_packet_add(async->queue->token, async->td); + trace_usb_uhci_packet_add(async->queue->token, async->td_addr); return async; } static void uhci_async_free(UHCIAsync *async) { - trace_usb_uhci_packet_del(async->queue->token, async->td); + trace_usb_uhci_packet_del(async->queue->token, async->td_addr); usb_packet_cleanup(&async->packet); qemu_sglist_destroy(&async->sgl); g_free(async); @@ -228,20 +228,21 @@ static void uhci_async_link(UHCIAsync *async) { UHCIQueue *queue = async->queue; QTAILQ_INSERT_TAIL(&queue->asyncs, async, next); - trace_usb_uhci_packet_link_async(async->queue->token, async->td); + trace_usb_uhci_packet_link_async(async->queue->token, async->td_addr); } static void uhci_async_unlink(UHCIAsync *async) { UHCIQueue *queue = async->queue; QTAILQ_REMOVE(&queue->asyncs, async, next); - trace_usb_uhci_packet_unlink_async(async->queue->token, async->td); + trace_usb_uhci_packet_unlink_async(async->queue->token, async->td_addr); } static void uhci_async_cancel(UHCIAsync *async) { uhci_async_unlink(async); - trace_usb_uhci_packet_cancel(async->queue->token, async->td, async->done); + trace_usb_uhci_packet_cancel(async->queue->token, async->td_addr, + async->done); if (!async->done) usb_cancel_packet(&async->packet); usb_packet_unmap(&async->packet, &async->sgl); @@ -300,7 +301,8 @@ static void uhci_async_cancel_all(UHCIState *s) } } -static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t addr, UHCI_TD *td) +static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t td_addr, + UHCI_TD *td) { uint32_t token = uhci_queue_token(td); UHCIQueue *queue; @@ -316,7 +318,7 @@ static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t addr, UHCI_TD *td) } QTAILQ_FOREACH(async, &queue->asyncs, next) { - if (async->td == addr) { + if (async->td_addr == td_addr) { return async; } } @@ -722,13 +724,14 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_ *int_mask |= 0x02; /* short packet: do not update QH */ trace_usb_uhci_packet_complete_shortxfer(async->queue->token, - async->td); + async->td_addr); return TD_RESULT_NEXT_QH; } } /* success */ - trace_usb_uhci_packet_complete_success(async->queue->token, async->td); + trace_usb_uhci_packet_complete_success(async->queue->token, + async->td_addr); return TD_RESULT_COMPLETE; out: @@ -739,14 +742,16 @@ out: case USB_RET_STALL: td->ctrl |= TD_CTRL_STALL; - trace_usb_uhci_packet_complete_stall(async->queue->token, async->td); + trace_usb_uhci_packet_complete_stall(async->queue->token, + async->td_addr); err = TD_RESULT_NEXT_QH; break; case USB_RET_BABBLE: td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL; /* frame interrupted */ - trace_usb_uhci_packet_complete_babble(async->queue->token, async->td); + trace_usb_uhci_packet_complete_babble(async->queue->token, + async->td_addr); err = TD_RESULT_STOP_FRAME; break; @@ -755,7 +760,8 @@ out: default: td->ctrl |= TD_CTRL_TIMEOUT; td->ctrl &= ~(3 << TD_CTRL_ERROR_SHIFT); - trace_usb_uhci_packet_complete_error(async->queue->token, async->td); + trace_usb_uhci_packet_complete_error(async->queue->token, + async->td_addr); err = TD_RESULT_NEXT_QH; break; } @@ -769,7 +775,7 @@ out: return err; } -static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, +static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td, uint32_t *int_mask, bool queuing, struct USBEndpoint **ep_ret) { @@ -792,7 +798,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, return TD_RESULT_NEXT_QH; } - async = uhci_async_find_td(s, addr, td); + async = uhci_async_find_td(s, td_addr, td); if (async) { /* Already submitted */ async->queue->valid = 32; @@ -811,7 +817,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, } /* Allocate new packet */ - async = uhci_async_alloc(uhci_queue_get(s, td), addr); + async = uhci_async_alloc(uhci_queue_get(s, td), td_addr); /* valid needs to be large enough to handle 10 frame delay * for initial isochronous requests @@ -827,7 +833,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, if (ep_ret) { *ep_ret = ep; } - usb_packet_setup(&async->packet, pid, ep, addr, spd, + usb_packet_setup(&async->packet, pid, ep, td_addr, spd, (td->ctrl & TD_CTRL_IOC) != 0); qemu_sglist_add(&async->sgl, td->buffer, max_len); usb_packet_map(&async->packet, &async->sgl); From 963a68b54f8f4e5dcc7cc2c6ec8d047557ac5feb Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:11 +0200 Subject: [PATCH 22/36] uhci: Add uhci_read_td() helper function Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 7589a5b2d6..6d2db7ffdb 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -693,6 +693,15 @@ static USBDevice *uhci_find_device(UHCIState *s, uint8_t addr) return NULL; } +static void uhci_read_td(UHCIState *s, UHCI_TD *td, uint32_t link) +{ + pci_dma_read(&s->dev, link & ~0xf, td, sizeof(*td)); + le32_to_cpus(&td->link); + le32_to_cpus(&td->ctrl); + le32_to_cpus(&td->token); + le32_to_cpus(&td->buffer); +} + static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask) { int len = 0, max_len, err, ret; @@ -941,11 +950,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep) int ret; while (is_valid(plink)) { - pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd)); - le32_to_cpus(&ptd.link); - le32_to_cpus(&ptd.ctrl); - le32_to_cpus(&ptd.token); - le32_to_cpus(&ptd.buffer); + uhci_read_td(s, &ptd, plink); if (!(ptd.ctrl & TD_CTRL_ACTIVE)) { break; } @@ -1031,11 +1036,7 @@ static void uhci_process_frame(UHCIState *s) } /* TD */ - pci_dma_read(&s->dev, link & ~0xf, &td, sizeof(td)); - le32_to_cpus(&td.link); - le32_to_cpus(&td.ctrl); - le32_to_cpus(&td.token); - le32_to_cpus(&td.buffer); + uhci_read_td(s, &td, link); trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token); old_td_ctrl = td.ctrl; From a4f30cd766888e69e1d75cd87d251c04e829688e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:12 +0200 Subject: [PATCH 23/36] uhci: Make uhci_fill_queue() actually operate on an UHCIQueue And move its calling point to handle_td, this removes the ep_ret ugliness, and prepates the way for further cleanups in the follow-up patches in this patch-set. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 6d2db7ffdb..2bbc6fbc96 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -161,6 +161,7 @@ typedef struct UHCI_QH { } UHCI_QH; static void uhci_async_cancel(UHCIAsync *async); +static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep); static inline int32_t uhci_queue_token(UHCI_TD *td) { @@ -784,9 +785,8 @@ out: return err; } -static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td, - uint32_t *int_mask, bool queuing, - struct USBEndpoint **ep_ret) +static int uhci_handle_td(UHCIState *s, UHCIQueue *q, + UHCI_TD *td, uint32_t td_addr, uint32_t *int_mask) { UHCIAsync *async; int len = 0, max_len; @@ -794,6 +794,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td, bool spd; USBDevice *dev; USBEndpoint *ep; + bool queuing = (q != NULL); /* Is active ? */ if (!(td->ctrl & TD_CTRL_ACTIVE)) { @@ -826,7 +827,10 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td, } /* Allocate new packet */ - async = uhci_async_alloc(uhci_queue_get(s, td), td_addr); + if (q == NULL) { + q = uhci_queue_get(s, td); + } + async = uhci_async_alloc(q, td_addr); /* valid needs to be large enough to handle 10 frame delay * for initial isochronous requests @@ -839,9 +843,6 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td, dev = uhci_find_device(s, (td->token >> 8) & 0x7f); ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf); - if (ep_ret) { - *ep_ret = ep; - } usb_packet_setup(&async->packet, pid, ep, td_addr, spd, (td->ctrl & TD_CTRL_IOC) != 0); qemu_sglist_add(&async->sgl, td->buffer, max_len); @@ -870,6 +871,9 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td, if (len == USB_RET_ASYNC) { uhci_async_link(async); + if (!queuing) { + uhci_queue_fill(q, td, ep); + } return TD_RESULT_ASYNC_START; } @@ -941,24 +945,23 @@ static int qhdb_insert(QhDb *db, uint32_t addr) return 0; } -static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep) +static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep) { uint32_t int_mask = 0; uint32_t plink = td->link; - uint32_t token = uhci_queue_token(td); UHCI_TD ptd; int ret; while (is_valid(plink)) { - uhci_read_td(s, &ptd, plink); + uhci_read_td(q->uhci, &ptd, plink); if (!(ptd.ctrl & TD_CTRL_ACTIVE)) { break; } - if (uhci_queue_token(&ptd) != token) { + if (uhci_queue_token(&ptd) != q->token) { break; } trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token); - ret = uhci_handle_td(s, plink, &ptd, &int_mask, true, NULL); + ret = uhci_handle_td(q->uhci, q, &ptd, plink, &int_mask); if (ret == TD_RESULT_ASYNC_CONT) { break; } @@ -973,7 +976,6 @@ 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; - struct USBEndpoint *curr_ep; int cnt, ret; UHCI_TD td; UHCI_QH qh; @@ -1040,7 +1042,7 @@ static void uhci_process_frame(UHCIState *s) trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token); old_td_ctrl = td.ctrl; - ret = uhci_handle_td(s, link, &td, &int_mask, false, &curr_ep); + ret = uhci_handle_td(s, NULL, &td, link, &int_mask); if (old_td_ctrl != td.ctrl) { /* update the status bits of the TD */ val = cpu_to_le32(td.ctrl); @@ -1059,7 +1061,6 @@ static void uhci_process_frame(UHCIState *s) case TD_RESULT_ASYNC_START: trace_usb_uhci_td_async(curr_qh & ~0xf, link & ~0xf); - uhci_fill_queue(s, &td, curr_ep); link = curr_qh ? qh.link : td.link; continue; From 11d15e402beec122fb11104e36c35d1a46f650ff Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:13 +0200 Subject: [PATCH 24/36] uhci: Store ep in UHCIQueue This avoids the need to repeatedly lookup the device, and ep. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 2bbc6fbc96..8214a21a7a 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -107,6 +107,7 @@ struct UHCIAsync { struct UHCIQueue { uint32_t token; UHCIState *uhci; + USBEndpoint *ep; QTAILQ_ENTRY(UHCIQueue) next; QTAILQ_HEAD(, UHCIAsync) asyncs; int8_t valid; @@ -161,7 +162,7 @@ typedef struct UHCI_QH { } UHCI_QH; static void uhci_async_cancel(UHCIAsync *async); -static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep); +static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td); static inline int32_t uhci_queue_token(UHCI_TD *td) { @@ -169,7 +170,7 @@ static inline int32_t uhci_queue_token(UHCI_TD *td) return td->token & 0x7ffff; } -static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td) +static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td, USBEndpoint *ep) { uint32_t token = uhci_queue_token(td); UHCIQueue *queue; @@ -183,6 +184,7 @@ static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td) queue = g_new0(UHCIQueue, 1); queue->uhci = s; queue->token = token; + queue->ep = ep; QTAILQ_INIT(&queue->asyncs); QTAILQ_INSERT_HEAD(&s->queues, queue, next); trace_usb_uhci_queue_add(queue->token); @@ -790,11 +792,9 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, { UHCIAsync *async; int len = 0, max_len; - uint8_t pid; bool spd; - USBDevice *dev; - USBEndpoint *ep; bool queuing = (q != NULL); + uint8_t pid = td->token & 0xff; /* Is active ? */ if (!(td->ctrl & TD_CTRL_ACTIVE)) { @@ -828,7 +828,9 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, /* Allocate new packet */ if (q == NULL) { - q = uhci_queue_get(s, td); + USBDevice *dev = uhci_find_device(s, (td->token >> 8) & 0x7f); + USBEndpoint *ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf); + q = uhci_queue_get(s, td, ep); } async = uhci_async_alloc(q, td_addr); @@ -838,12 +840,8 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, async->queue->valid = 32; max_len = ((td->token >> 21) + 1) & 0x7ff; - pid = td->token & 0xff; spd = (pid == USB_TOKEN_IN && (td->ctrl & TD_CTRL_SPD) != 0); - - 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, td_addr, spd, + usb_packet_setup(&async->packet, pid, q->ep, td_addr, spd, (td->ctrl & TD_CTRL_IOC) != 0); qemu_sglist_add(&async->sgl, td->buffer, max_len); usb_packet_map(&async->packet, &async->sgl); @@ -851,13 +849,13 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, switch(pid) { case USB_TOKEN_OUT: case USB_TOKEN_SETUP: - len = usb_handle_packet(dev, &async->packet); + len = usb_handle_packet(q->ep->dev, &async->packet); if (len >= 0) len = max_len; break; case USB_TOKEN_IN: - len = usb_handle_packet(dev, &async->packet); + len = usb_handle_packet(q->ep->dev, &async->packet); break; default: @@ -872,7 +870,7 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, if (len == USB_RET_ASYNC) { uhci_async_link(async); if (!queuing) { - uhci_queue_fill(q, td, ep); + uhci_queue_fill(q, td); } return TD_RESULT_ASYNC_START; } @@ -945,7 +943,7 @@ static int qhdb_insert(QhDb *db, uint32_t addr) return 0; } -static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep) +static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td) { uint32_t int_mask = 0; uint32_t plink = td->link; @@ -969,7 +967,7 @@ static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep) assert(int_mask == 0); plink = ptd.link; } - usb_device_flush_ep_queue(ep->dev, ep); + usb_device_flush_ep_queue(q->ep->dev, q->ep); } static void uhci_process_frame(UHCIState *s) From 5ad23e873c858292dc58b9296261365312b8f683 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:14 +0200 Subject: [PATCH 25/36] uhci: Immediately free queues on device disconnect There is no need to just cancel any in-flight packets, and then wait for validate-end to clean things up, we can simply clean things up immediately on device removal. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 8214a21a7a..a8b74bdb66 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -281,16 +281,11 @@ static void uhci_async_validate_end(UHCIState *s) static void uhci_async_cancel_device(UHCIState *s, USBDevice *dev) { - UHCIQueue *queue; - UHCIAsync *curr, *n; + UHCIQueue *queue, *n; - QTAILQ_FOREACH(queue, &s->queues, next) { - QTAILQ_FOREACH_SAFE(curr, &queue->asyncs, next, n) { - if (!usb_packet_is_inflight(&curr->packet) || - curr->packet.ep->dev != dev) { - continue; - } - uhci_async_cancel(curr); + QTAILQ_FOREACH_SAFE(queue, &s->queues, next, n) { + if (queue->ep->dev == dev) { + uhci_queue_free(queue, "cancel-device"); } } } From 66a08cbe6ad1aebec8eecf58b3ba042e19dd1649 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:15 +0200 Subject: [PATCH 26/36] uhci: Verify queue has not been changed by guest According to the spec a guest can unlink a qh, and then as soon as frindex has changed by 1 since the unlink, assume it is idle and re-use it. However for various reasons, we cannot simply consider a qh as unlinked if we've not seen it for 1 frame. This means that it is possible for a guest to re-use / restart the queue while we still see its old state. This patch adds a safety check for this, and "early" retires queues when they were changed by the guest. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 62 ++++++++++++++++++++++++++++++++++------------- trace-events | 2 +- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index a8b74bdb66..0984bee4f3 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -105,6 +105,7 @@ struct UHCIAsync { }; struct UHCIQueue { + uint32_t qh_addr; uint32_t token; UHCIState *uhci; USBEndpoint *ep; @@ -170,20 +171,15 @@ static inline int32_t uhci_queue_token(UHCI_TD *td) return td->token & 0x7ffff; } -static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td, USBEndpoint *ep) +static UHCIQueue *uhci_queue_new(UHCIState *s, uint32_t qh_addr, UHCI_TD *td, + USBEndpoint *ep) { - uint32_t token = uhci_queue_token(td); UHCIQueue *queue; - QTAILQ_FOREACH(queue, &s->queues, next) { - if (queue->token == token) { - return queue; - } - } - queue = g_new0(UHCIQueue, 1); queue->uhci = s; - queue->token = token; + queue->qh_addr = qh_addr; + queue->token = uhci_queue_token(td); queue->ep = ep; QTAILQ_INIT(&queue->asyncs); QTAILQ_INSERT_HEAD(&s->queues, queue, next); @@ -191,7 +187,7 @@ static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td, USBEndpoint *ep) return queue; } -static void uhci_queue_free(UHCIQueue *queue) +static void uhci_queue_free(UHCIQueue *queue, const char *reason) { UHCIState *s = queue->uhci; UHCIAsync *async; @@ -201,11 +197,35 @@ static void uhci_queue_free(UHCIQueue *queue) uhci_async_cancel(async); } - trace_usb_uhci_queue_del(queue->token); + trace_usb_uhci_queue_del(queue->token, reason); QTAILQ_REMOVE(&s->queues, queue, next); g_free(queue); } +static UHCIQueue *uhci_queue_find(UHCIState *s, UHCI_TD *td) +{ + uint32_t token = uhci_queue_token(td); + UHCIQueue *queue; + + QTAILQ_FOREACH(queue, &s->queues, next) { + if (queue->token == token) { + return queue; + } + } + return NULL; +} + +static bool uhci_queue_verify(UHCIQueue *queue, uint32_t qh_addr, UHCI_TD *td, + uint32_t td_addr, bool queuing) +{ + UHCIAsync *first = QTAILQ_FIRST(&queue->asyncs); + + return queue->qh_addr == qh_addr && + queue->token == uhci_queue_token(td) && + (queuing || !(td->ctrl & TD_CTRL_ACTIVE) || first == NULL || + first->td_addr == td_addr); +} + static UHCIAsync *uhci_async_alloc(UHCIQueue *queue, uint32_t td_addr) { UHCIAsync *async = g_new0(UHCIAsync, 1); @@ -274,7 +294,7 @@ static void uhci_async_validate_end(UHCIState *s) QTAILQ_FOREACH_SAFE(queue, &s->queues, next, n) { if (!queue->valid) { - uhci_queue_free(queue); + uhci_queue_free(queue, "validate-end"); } } } @@ -295,7 +315,7 @@ static void uhci_async_cancel_all(UHCIState *s) UHCIQueue *queue, *nq; QTAILQ_FOREACH_SAFE(queue, &s->queues, next, nq) { - uhci_queue_free(queue); + uhci_queue_free(queue, "cancel-all"); } } @@ -782,7 +802,7 @@ out: return err; } -static int uhci_handle_td(UHCIState *s, UHCIQueue *q, +static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, UHCI_TD *td, uint32_t td_addr, uint32_t *int_mask) { UHCIAsync *async; @@ -791,6 +811,14 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, bool queuing = (q != NULL); uint8_t pid = td->token & 0xff; + if (q == NULL) { + q = uhci_queue_find(s, td); + if (q && !uhci_queue_verify(q, qh_addr, td, td_addr, queuing)) { + uhci_queue_free(q, "guest re-used qh"); + q = NULL; + } + } + /* Is active ? */ if (!(td->ctrl & TD_CTRL_ACTIVE)) { /* @@ -825,7 +853,7 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, if (q == NULL) { USBDevice *dev = uhci_find_device(s, (td->token >> 8) & 0x7f); USBEndpoint *ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf); - q = uhci_queue_get(s, td, ep); + q = uhci_queue_new(s, qh_addr, td, ep); } async = uhci_async_alloc(q, td_addr); @@ -954,7 +982,7 @@ static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td) break; } trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token); - ret = uhci_handle_td(q->uhci, q, &ptd, plink, &int_mask); + ret = uhci_handle_td(q->uhci, q, q->qh_addr, &ptd, plink, &int_mask); if (ret == TD_RESULT_ASYNC_CONT) { break; } @@ -1035,7 +1063,7 @@ static void uhci_process_frame(UHCIState *s) trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token); old_td_ctrl = td.ctrl; - ret = uhci_handle_td(s, NULL, &td, link, &int_mask); + ret = uhci_handle_td(s, NULL, curr_qh, &td, link, &int_mask); if (old_td_ctrl != td.ctrl) { /* update the status bits of the TD */ val = cpu_to_le32(td.ctrl); diff --git a/trace-events b/trace-events index e2d4580d4c..ed136a56a1 100644 --- a/trace-events +++ b/trace-events @@ -287,7 +287,7 @@ 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_queue_del(uint32_t token, const char *reason) "token 0x%x: %s" usb_uhci_packet_add(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x" usb_uhci_packet_link_async(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x" usb_uhci_packet_unlink_async(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x" From 8c75a899f85dda9f8305d25f0cc453a79115fd90 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:16 +0200 Subject: [PATCH 27/36] uhci: Detect guest td re-use A td can be reused by the guest in a different queue, before we notice the original queue has been unlinked. So search for tds by addr only, detect guest td reuse, and cancel the original queue, this is necessary to keep our packet ids unique. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 0984bee4f3..c4f2f98f17 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -319,28 +319,18 @@ static void uhci_async_cancel_all(UHCIState *s) } } -static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t td_addr, - UHCI_TD *td) +static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t td_addr) { - uint32_t token = uhci_queue_token(td); UHCIQueue *queue; UHCIAsync *async; QTAILQ_FOREACH(queue, &s->queues, next) { - if (queue->token == token) { - break; + QTAILQ_FOREACH(async, &queue->asyncs, next) { + if (async->td_addr == td_addr) { + return async; + } } } - if (queue == NULL) { - return NULL; - } - - QTAILQ_FOREACH(async, &queue->asyncs, next) { - if (async->td_addr == td_addr) { - return async; - } - } - return NULL; } @@ -805,11 +795,21 @@ out: static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, UHCI_TD *td, uint32_t td_addr, uint32_t *int_mask) { - UHCIAsync *async; int len = 0, max_len; bool spd; bool queuing = (q != NULL); uint8_t pid = td->token & 0xff; + UHCIAsync *async = uhci_async_find_td(s, td_addr); + + if (async) { + if (uhci_queue_verify(async->queue, qh_addr, td, td_addr, queuing)) { + assert(q == NULL || q == async->queue); + q = async->queue; + } else { + uhci_queue_free(async->queue, "guest re-used pending td"); + async = NULL; + } + } if (q == NULL) { q = uhci_queue_find(s, td); @@ -831,7 +831,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, return TD_RESULT_NEXT_QH; } - async = uhci_async_find_td(s, td_addr, td); if (async) { /* Already submitted */ async->queue->valid = 32; From 420ca987d5d14fabacdec86c568832d6b0056ca3 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:17 +0200 Subject: [PATCH 28/36] uhci: When the guest marks a pending td non-active, cancel the queue Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index c4f2f98f17..592ad8d3e7 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -821,6 +821,10 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, /* Is active ? */ if (!(td->ctrl & TD_CTRL_ACTIVE)) { + if (async) { + /* Guest marked a pending td non-active, cancel the queue */ + uhci_queue_free(async->queue, "pending td non-active"); + } /* * ehci11d spec page 22: "Even if the Active bit in the TD is already * cleared when the TD is fetched ... an IOC interrupt is generated" From 3905097ea8ea0e1b3dd1556f3ef74a123fe8622a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:18 +0200 Subject: [PATCH 29/36] uhci: Always mark a queue valid when we encounter it Before this patch we would not mark a queue valid when its head was a non-active td. This causes us to misbehave in the following scenario: 1) queue with multiple input transfers queued 2) We hit some latency issue, causing qemu to get behind processing frames 3) When qemu gets to run again, it notices the first transfer ends short, marking the head td non-active 4) It now processes 32+ frames in a row without giving the guest a chance to run since it is behind 5) valid is decreased to 0, causing the queue to get cancelled also cancelling already queued up further input transfers 6) guest gets to run, notices the inactive td, cleanups up further tds from the short transfer, and lets the queue continue at the first td of the next input transfer 7) we re-start the queue, issuing the second input transfer for the *second* time, and any data read by the first time we issued it has been lost Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 592ad8d3e7..beeb3fd7b0 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -183,6 +183,9 @@ static UHCIQueue *uhci_queue_new(UHCIState *s, uint32_t qh_addr, UHCI_TD *td, queue->ep = ep; QTAILQ_INIT(&queue->asyncs); QTAILQ_INSERT_HEAD(&s->queues, queue, next); + /* valid needs to be large enough to handle 10 frame delay + * for initial isochronous requests */ + queue->valid = 32; trace_usb_uhci_queue_add(queue->token); return queue; } @@ -819,6 +822,10 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, } } + if (q) { + q->valid = 32; + } + /* Is active ? */ if (!(td->ctrl & TD_CTRL_ACTIVE)) { if (async) { @@ -836,9 +843,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, } if (async) { - /* Already submitted */ - async->queue->valid = 32; - if (!async->done) return TD_RESULT_ASYNC_CONT; if (queuing) { @@ -860,11 +864,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, } async = uhci_async_alloc(q, td_addr); - /* valid needs to be large enough to handle 10 frame delay - * for initial isochronous requests - */ - async->queue->valid = 32; - max_len = ((td->token >> 21) + 1) & 0x7ff; spd = (pid == USB_TOKEN_IN && (td->ctrl & TD_CTRL_SPD) != 0); usb_packet_setup(&async->packet, pid, q->ep, td_addr, spd, From 8928c9c43df1a0eeda1ae2e9582beb34dce49330 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:19 +0200 Subject: [PATCH 30/36] uhci: Retry to fill the queue while waiting for td completion If the guest is using multiple transfers to try and keep the usb bus busy / used at maximum efficiency, currently we would see / do the following: 1) submit transfer 1 to the device 2) submit transfer 2 to the device 3) report transfer 1 completion to guest 4) report transfer 2 completion to guest 5) submit transfer 1 to the device 6) report transfer 1 completion to guest 7) submit transfer 2 to the device 8) report transfer 2 completion to guest etc. So after the initial submission we would effectively only have 1 transfer in flight, rather then 2. This is caused by us not checking the queue for addition of new transfers by the guest (ie the resubmission of a recently finished transfer), while waiting for a pending transfer to complete. This patch does add a check for this, changing the sequence to: 1) submit transfer 1 to the device 2) submit transfer 2 to the device 3) report transfer 1 completion to guest 4) submit transfer 1 to the device 5) report transfer 2 completion to guest 6) submit transfer 2 to the device etc. Thus keeping 2 transfers in flight (most of the time, and always 1), as intended by the guest. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index beeb3fd7b0..a9e06ef5aa 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -110,7 +110,7 @@ struct UHCIQueue { UHCIState *uhci; USBEndpoint *ep; QTAILQ_ENTRY(UHCIQueue) next; - QTAILQ_HEAD(, UHCIAsync) asyncs; + QTAILQ_HEAD(asyncs_head, UHCIAsync) asyncs; int8_t valid; }; @@ -843,15 +843,25 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr, } if (async) { - if (!async->done) - return TD_RESULT_ASYNC_CONT; if (queuing) { /* we are busy filling the queue, we are not prepared to consume completed packages then, just leave them in async state */ return TD_RESULT_ASYNC_CONT; } + if (!async->done) { + UHCI_TD last_td; + UHCIAsync *last = QTAILQ_LAST(&async->queue->asyncs, asyncs_head); + /* + * While we are waiting for the current td to complete, the guest + * may have added more tds to the queue. Note we re-read the td + * rather then caching it, as we want to see guest made changes! + */ + uhci_read_td(s, &last_td, last->td_addr); + uhci_queue_fill(async->queue, &last_td); + return TD_RESULT_ASYNC_CONT; + } uhci_async_unlink(async); goto done; } From 6fe30910abd7e0f4af69f8e522ccfe6b5125abbb Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 24 Oct 2012 18:31:20 +0200 Subject: [PATCH 31/36] uhci: Use only one queue for ctrl endpoints ctrl endpoints use different pids for different phases of a control transfer, this patch makes us use only one queue for a ctrl ep, rather then 3. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index a9e06ef5aa..b6b972fa92 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -167,8 +167,13 @@ static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td); static inline int32_t uhci_queue_token(UHCI_TD *td) { - /* covers ep, dev, pid -> identifies the endpoint */ - return td->token & 0x7ffff; + if ((td->token & (0xf << 15)) == 0) { + /* ctrl ep, cover ep and dev, not pid! */ + return td->token & 0x7ff00; + } else { + /* covers ep, dev, pid -> identifies the endpoint */ + return td->token & 0x7ffff; + } } static UHCIQueue *uhci_queue_new(UHCIState *s, uint32_t qh_addr, UHCI_TD *td, From 79a8af3509151dd1b650c996defdda13e02a7e38 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 24 Oct 2012 09:36:55 +0200 Subject: [PATCH 32/36] xhci: fix function name in error message Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index caa5f3e36a..8345fa34a2 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2676,7 +2676,7 @@ static void xhci_runtime_write(void *ptr, hwaddr reg, trace_usb_xhci_runtime_write(reg, val); if (reg < 0x20) { - fprintf(stderr, "xhci_oper_write: reg 0x%x unimplemented\n", (int)reg); + fprintf(stderr, "%s: reg 0x%x unimplemented\n", __func__, (int)reg); return; } From 3f973ee84ef51e448566c08ec5cfe746c9640269 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 24 Oct 2012 10:14:16 +0200 Subject: [PATCH 33/36] xhci: flush endpoint context unconditinally Not updating the endpoint context in case the state didn't change is wrong. Other context fields might have changed, for example the dequeue pointer in response to a CR_SET_TR_DEQUEUE command. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 8345fa34a2..d8d1226a5b 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1009,9 +1009,6 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx, uint32_t state) { uint32_t ctx[5]; - if (epctx->state == state) { - return; - } pci_dma_read(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx)); ctx[0] &= ~EP_STATE_MASK; From e099ad4b7e9ca7debdd624a0d465a33198a6844f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 24 Oct 2012 09:38:08 +0200 Subject: [PATCH 34/36] xhci: allow disabling interrupters For secondary interrupters this is explicitly allowed in the specs. For the primary interrupter behavior is undefined, lets be friendly and allow disabling too. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index d8d1226a5b..bd8d4a5b39 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -964,6 +964,12 @@ static void xhci_er_reset(XHCIState *xhci, int v) XHCIInterrupter *intr = &xhci->intr[v]; XHCIEvRingSeg seg; + if (intr->erstsz == 0) { + /* disabled */ + intr->er_start = 0; + intr->er_size = 0; + return; + } /* cache the (sole) event ring segment location */ if (intr->erstsz != 1) { fprintf(stderr, "xhci: invalid value for ERSTSZ: %d\n", intr->erstsz); From 91062ae00fd4a88a40c28288e8ac7fe50ee83adf Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 24 Oct 2012 11:49:30 +0200 Subject: [PATCH 35/36] xhci: make number of interrupters and slots configurable Add properties to tweak the numbers of available interrupters and slots. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 79 ++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index bd8d4a5b39..25b04cd11b 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -417,6 +417,8 @@ struct XHCIState { /* properties */ uint32_t numports_2; uint32_t numports_3; + uint32_t numintrs; + uint32_t numslots; uint32_t flags; /* Operational Registers */ @@ -816,8 +818,8 @@ static void xhci_event(XHCIState *xhci, XHCIEvent *event, int v) dma_addr_t erdp; unsigned int dp_idx; - if (v >= MAXINTRS) { - DPRINTF("intr nr out of range (%d >= %d)\n", v, MAXINTRS); + if (v >= xhci->numintrs) { + DPRINTF("intr nr out of range (%d >= %d)\n", v, xhci->numintrs); return; } intr = &xhci->intr[v]; @@ -1043,7 +1045,7 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid, int i; trace_usb_xhci_ep_enable(slotid, epid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); assert(epid >= 1 && epid <= 31); slot = &xhci->slots[slotid-1]; @@ -1121,7 +1123,7 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, XHCISlot *slot; XHCIEPContext *epctx; int i, xferi, killed = 0; - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); assert(epid >= 1 && epid <= 31); DPRINTF("xhci_ep_nuke_xfers(%d, %d)\n", slotid, epid); @@ -1149,7 +1151,7 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, XHCIEPContext *epctx; trace_usb_xhci_ep_disable(slotid, epid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); assert(epid >= 1 && epid <= 31); slot = &xhci->slots[slotid-1]; @@ -1179,7 +1181,7 @@ static TRBCCode xhci_stop_ep(XHCIState *xhci, unsigned int slotid, XHCIEPContext *epctx; trace_usb_xhci_ep_stop(slotid, epid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); if (epid < 1 || epid > 31) { fprintf(stderr, "xhci: bad ep %d\n", epid); @@ -1213,7 +1215,7 @@ static TRBCCode xhci_reset_ep(XHCIState *xhci, unsigned int slotid, USBDevice *dev; trace_usb_xhci_ep_reset(slotid, epid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); if (epid < 1 || epid > 31) { fprintf(stderr, "xhci: bad ep %d\n", epid); @@ -1263,7 +1265,7 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid, XHCIEPContext *epctx; dma_addr_t dequeue; - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); if (epid < 1 || epid > 31) { fprintf(stderr, "xhci: bad ep %d\n", epid); @@ -1667,7 +1669,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid int i; trace_usb_xhci_ep_kick(slotid, epid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); assert(epid >= 1 && epid <= 31); if (!xhci->slots[slotid-1].enabled) { @@ -1787,7 +1789,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid static TRBCCode xhci_enable_slot(XHCIState *xhci, unsigned int slotid) { trace_usb_xhci_slot_enable(slotid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); xhci->slots[slotid-1].enabled = 1; xhci->slots[slotid-1].uport = NULL; memset(xhci->slots[slotid-1].eps, 0, sizeof(XHCIEPContext*)*31); @@ -1800,7 +1802,7 @@ static TRBCCode xhci_disable_slot(XHCIState *xhci, unsigned int slotid) int i; trace_usb_xhci_slot_disable(slotid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); for (i = 1; i <= 31; i++) { if (xhci->slots[slotid-1].eps[i-1]) { @@ -1852,7 +1854,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, TRBCCode res; trace_usb_xhci_slot_address(slotid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high); pci_dma_read(&xhci->pci_dev, dcbaap + 8*slotid, &poctx, sizeof(poctx)); @@ -1891,7 +1893,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, return CC_USB_TRANSACTION_ERROR; } - for (i = 0; i < MAXSLOTS; i++) { + for (i = 0; i < xhci->numslots; i++) { if (xhci->slots[i].uport == uport) { fprintf(stderr, "xhci: port %s already assigned to slot %d\n", uport->path, i+1); @@ -1940,7 +1942,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, TRBCCode res; trace_usb_xhci_slot_configure(slotid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); ictx = xhci_mask64(pictx); octx = xhci->slots[slotid-1].ctx; @@ -2028,7 +2030,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, uint32_t slot_ctx[4]; trace_usb_xhci_slot_evaluate(slotid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); ictx = xhci_mask64(pictx); octx = xhci->slots[slotid-1].ctx; @@ -2091,7 +2093,7 @@ static TRBCCode xhci_reset_slot(XHCIState *xhci, unsigned int slotid) int i; trace_usb_xhci_slot_reset(slotid); - assert(slotid >= 1 && slotid <= MAXSLOTS); + assert(slotid >= 1 && slotid <= xhci->numslots); octx = xhci->slots[slotid-1].ctx; @@ -2117,7 +2119,7 @@ static unsigned int xhci_get_slot(XHCIState *xhci, XHCIEvent *event, XHCITRB *tr { unsigned int slotid; slotid = (trb->control >> TRB_CR_SLOTID_SHIFT) & TRB_CR_SLOTID_MASK; - if (slotid < 1 || slotid > MAXSLOTS) { + if (slotid < 1 || slotid > xhci->numslots) { fprintf(stderr, "xhci: bad slot id %d\n", slotid); event->ccode = CC_TRB_ERROR; return 0; @@ -2209,12 +2211,12 @@ static void xhci_process_commands(XHCIState *xhci) event.ptr = addr; switch (type) { case CR_ENABLE_SLOT: - for (i = 0; i < MAXSLOTS; i++) { + for (i = 0; i < xhci->numslots; i++) { if (!xhci->slots[i].enabled) { break; } } - if (i >= MAXSLOTS) { + if (i >= xhci->numslots) { fprintf(stderr, "xhci: no device slots available\n"); event.ccode = CC_NO_SLOTS_ERROR; } else { @@ -2361,7 +2363,7 @@ static void xhci_reset(DeviceState *dev) xhci->config = 0; xhci->devaddr = 2; - for (i = 0; i < MAXSLOTS; i++) { + for (i = 0; i < xhci->numslots; i++) { xhci_disable_slot(xhci, i+1); } @@ -2369,7 +2371,7 @@ static void xhci_reset(DeviceState *dev) xhci_update_port(xhci, xhci->ports + i, 0); } - for (i = 0; i < MAXINTRS; i++) { + for (i = 0; i < xhci->numintrs; i++) { xhci->intr[i].iman = 0; xhci->intr[i].imod = 0; xhci->intr[i].erstsz = 0; @@ -2401,7 +2403,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, unsigned size) break; case 0x04: /* HCSPARAMS 1 */ ret = ((xhci->numports_2+xhci->numports_3)<<24) - | (MAXINTRS<<8) | MAXSLOTS; + | (xhci->numintrs<<8) | xhci->numslots; break; case 0x08: /* HCSPARAMS 2 */ ret = 0x0000000f; @@ -2756,7 +2758,7 @@ static void xhci_doorbell_write(void *ptr, hwaddr reg, (uint32_t)val); } } else { - if (reg > MAXSLOTS) { + if (reg > xhci->numslots) { fprintf(stderr, "xhci: bad doorbell %d\n", (int)reg); } else if (val > 31) { fprintf(stderr, "xhci: bad doorbell %d write: 0x%x\n", @@ -2862,7 +2864,7 @@ static void xhci_child_detach(USBPort *uport, USBDevice *child) XHCIState *xhci = container_of(bus, XHCIState, bus); int i; - for (i = 0; i < MAXSLOTS; i++) { + for (i = 0; i < xhci->numslots; i++) { if (xhci->slots[i].uport == uport) { xhci->slots[i].uport = NULL; } @@ -2882,7 +2884,7 @@ static int xhci_find_slotid(XHCIState *xhci, USBDevice *dev) XHCISlot *slot; int slotid; - for (slotid = 1; slotid <= MAXSLOTS; slotid++) { + for (slotid = 1; slotid <= xhci->numslots; slotid++) { slot = &xhci->slots[slotid-1]; if (slot->devaddr == dev->addr) { return slotid; @@ -2978,6 +2980,19 @@ static int usb_xhci_initfn(struct PCIDevice *dev) usb_xhci_init(xhci, &dev->qdev); + if (xhci->numintrs > MAXINTRS) { + xhci->numintrs = MAXINTRS; + } + if (xhci->numintrs < 1) { + xhci->numintrs = 1; + } + if (xhci->numslots > MAXSLOTS) { + xhci->numslots = MAXSLOTS; + } + if (xhci->numslots < 1) { + xhci->numslots = 1; + } + xhci->mfwrap_timer = qemu_new_timer_ns(vm_clock, xhci_mfwrap_timer, xhci); xhci->irq = xhci->pci_dev.irq[0]; @@ -3014,10 +3029,10 @@ static int usb_xhci_initfn(struct PCIDevice *dev) assert(ret >= 0); if (xhci->flags & (1 << XHCI_FLAG_USE_MSI)) { - msi_init(&xhci->pci_dev, 0x70, MAXINTRS, true, false); + msi_init(&xhci->pci_dev, 0x70, xhci->numintrs, true, false); } if (xhci->flags & (1 << XHCI_FLAG_USE_MSI_X)) { - msix_init(&xhci->pci_dev, MAXINTRS, + msix_init(&xhci->pci_dev, xhci->numintrs, &xhci->mem, 0, OFF_MSIX_TABLE, &xhci->mem, 0, OFF_MSIX_PBA, 0x90); @@ -3032,10 +3047,12 @@ static const VMStateDescription vmstate_xhci = { }; static Property xhci_properties[] = { - DEFINE_PROP_BIT("msi", XHCIState, flags, XHCI_FLAG_USE_MSI, true), - DEFINE_PROP_BIT("msix", XHCIState, flags, XHCI_FLAG_USE_MSI_X, true), - DEFINE_PROP_UINT32("p2", XHCIState, numports_2, 4), - DEFINE_PROP_UINT32("p3", XHCIState, numports_3, 4), + DEFINE_PROP_BIT("msi", XHCIState, flags, XHCI_FLAG_USE_MSI, true), + DEFINE_PROP_BIT("msix", XHCIState, flags, XHCI_FLAG_USE_MSI_X, true), + DEFINE_PROP_UINT32("intrs", XHCIState, numintrs, MAXINTRS), + DEFINE_PROP_UINT32("slots", XHCIState, numslots, MAXSLOTS), + DEFINE_PROP_UINT32("p2", XHCIState, numports_2, 4), + DEFINE_PROP_UINT32("p3", XHCIState, numports_3, 4), DEFINE_PROP_END_OF_LIST(), }; From 0ebfb144e8ad3f2da436d630fdcc5aa9ab646341 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 24 Oct 2012 16:19:21 +0200 Subject: [PATCH 36/36] xhci: fix usb name in caps Used to be "UTB" not "USB". Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 25b04cd11b..7b65741dd5 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2430,7 +2430,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, unsigned size) ret = 0x02000402; /* USB 2.0 */ break; case 0x24: /* Supported Protocol:04 */ - ret = 0x20425455; /* "USB " */ + ret = 0x20425355; /* "USB " */ break; case 0x28: /* Supported Protocol:08 */ ret = 0x00000001 | (xhci->numports_2<<8); @@ -2442,7 +2442,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, unsigned size) ret = 0x03000002; /* USB 3.0 */ break; case 0x34: /* Supported Protocol:04 */ - ret = 0x20425455; /* "USB " */ + ret = 0x20425355; /* "USB " */ break; case 0x38: /* Supported Protocol:08 */ ret = 0x00000000 | (xhci->numports_2+1) | (xhci->numports_3<<8);