From 0454b611bb129ada92d16cfb9bec5fd6e1df5ab0 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:08 +0100 Subject: usb-redir: Set ep type and interface Since we don't use usb_desc.c we need to do this ourselves. This fixes iso transfers no longer working for USB 2 devices due to the ep->type check in ehci.c Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- usb-redir.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/usb-redir.c b/usb-redir.c index 755492f..a87de6e 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -1122,6 +1122,7 @@ static void usbredir_device_disconnect(void *priv) for (i = 0; i < MAX_ENDPOINTS; i++) { QTAILQ_INIT(&dev->endpoint[i].bufpq); } + usb_ep_init(&dev->dev); dev->interface_info.interface_count = 0; } @@ -1148,6 +1149,7 @@ static void usbredir_ep_info(void *priv, struct usb_redir_ep_info_header *ep_info) { USBRedirDevice *dev = priv; + struct USBEndpoint *usb_ep; int i; for (i = 0; i < MAX_ENDPOINTS; i++) { @@ -1172,7 +1174,13 @@ static void usbredir_ep_info(void *priv, default: ERROR("Received invalid endpoint type\n"); usbredir_device_disconnect(dev); + return; } + usb_ep = usb_ep_get(&dev->dev, + (i & 0x10) ? USB_TOKEN_IN : USB_TOKEN_OUT, + i & 0x0f); + usb_ep->type = dev->endpoint[i].type; + usb_ep->ifnum = dev->endpoint[i].interface; } } -- cgit v1.1 From 2a5ff735dc1074171a0cbb1dc228d6d6e907f571 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:09 +0100 Subject: usb-ehci: Never follow table entries with the T-bit set Before this patch the T-bit was not checked in 2 places, while it should be. Once we properly check the T-bit everywhere we no longer need the weird entry < 0x1000 and entry > 0x1000 checks, so this patch removes them. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index afc8ccf..d41b80e 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1568,8 +1568,7 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async) int again = 0; uint32_t entry = ehci_get_fetch_addr(ehci, async); - if (entry < 0x1000) { - DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry); + if (NLPTR_TBIT(entry)) { ehci_set_state(ehci, async, EST_ACTIVE); goto out; } @@ -1677,7 +1676,8 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) if (q->qh.token & QTD_TOKEN_HALT) { ehci_set_state(ehci, async, EST_HORIZONTALQH); - } else if ((q->qh.token & QTD_TOKEN_ACTIVE) && (q->qh.current_qtd > 0x1000)) { + } else if ((q->qh.token & QTD_TOKEN_ACTIVE) && + (NLPTR_TBIT(q->qh.current_qtd) == 0)) { q->qtdaddr = q->qh.current_qtd; ehci_set_state(ehci, async, EST_FETCHQTD); @@ -1756,7 +1756,6 @@ static int ehci_state_advqueue(EHCIQueue *q, int async) * want data and alt-next qTD is valid */ if (((q->qh.token & QTD_TOKEN_TBYTES_MASK) != 0) && - (q->qh.altnext_qtd > 0x1000) && (NLPTR_TBIT(q->qh.altnext_qtd) == 0)) { q->qtdaddr = q->qh.altnext_qtd; ehci_set_state(q->ehci, async, EST_FETCHQTD); @@ -1764,8 +1763,7 @@ static int ehci_state_advqueue(EHCIQueue *q, int async) /* * next qTD is valid */ - } else if ((q->qh.next_qtd > 0x1000) && - (NLPTR_TBIT(q->qh.next_qtd) == 0)) { + } else if (NLPTR_TBIT(q->qh.next_qtd) == 0) { q->qtdaddr = q->qh.next_qtd; ehci_set_state(q->ehci, async, EST_FETCHQTD); -- cgit v1.1 From df5d5c5c9eeaedb7705b4df6f5ee8083f39cf7b5 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:10 +0100 Subject: usb-ehci: split our qh queue into async and periodic queues qhs can be part of both the async and the periodic schedule, as is shown in later patches in this series it is useful to keep track of the qhs on a per schedule basis. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 62 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index d41b80e..840022d 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -347,7 +347,6 @@ enum async_state { struct EHCIQueue { EHCIState *ehci; QTAILQ_ENTRY(EHCIQueue) next; - bool async_schedule; uint32_t seen; uint64_t ts; @@ -367,6 +366,8 @@ struct EHCIQueue { int usb_status; }; +typedef QTAILQ_HEAD(EHCIQueueHead, EHCIQueue) EHCIQueueHead; + struct EHCIState { PCIDevice dev; USBBus bus; @@ -410,7 +411,8 @@ struct EHCIState { USBPort ports[NB_PORTS]; USBPort *companion_ports[NB_PORTS]; uint32_t usbsts_pending; - QTAILQ_HEAD(, EHCIQueue) queues; + EHCIQueueHead aqueues; + EHCIQueueHead pqueues; uint32_t a_fetch_addr; // which address to look at next uint32_t p_fetch_addr; // which address to look at next @@ -660,31 +662,34 @@ static void ehci_trace_sitd(EHCIState *s, target_phys_addr_t addr, static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, int async) { + EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues; EHCIQueue *q; q = g_malloc0(sizeof(*q)); q->ehci = ehci; - q->async_schedule = async; - QTAILQ_INSERT_HEAD(&ehci->queues, q, next); + QTAILQ_INSERT_HEAD(head, q, next); trace_usb_ehci_queue_action(q, "alloc"); return q; } -static void ehci_free_queue(EHCIQueue *q) +static void ehci_free_queue(EHCIQueue *q, int async) { + EHCIQueueHead *head = async ? &q->ehci->aqueues : &q->ehci->pqueues; trace_usb_ehci_queue_action(q, "free"); if (q->async == EHCI_ASYNC_INFLIGHT) { usb_cancel_packet(&q->packet); } - QTAILQ_REMOVE(&q->ehci->queues, q, next); + QTAILQ_REMOVE(head, q, next); g_free(q); } -static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr) +static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr, + int async) { + EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues; EHCIQueue *q; - QTAILQ_FOREACH(q, &ehci->queues, next) { + QTAILQ_FOREACH(q, head, next) { if (addr == q->qhaddr) { return q; } @@ -692,11 +697,12 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr) return NULL; } -static void ehci_queues_rip_unused(EHCIState *ehci) +static void ehci_queues_rip_unused(EHCIState *ehci, int async) { + EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues; EHCIQueue *q, *tmp; - QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) { + QTAILQ_FOREACH_SAFE(q, head, next, tmp) { if (q->seen) { q->seen = 0; q->ts = ehci->last_run_ns; @@ -706,29 +712,31 @@ static void ehci_queues_rip_unused(EHCIState *ehci) /* allow 0.25 sec idle */ continue; } - ehci_free_queue(q); + ehci_free_queue(q, async); } } -static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev) +static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev, int async) { + EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues; EHCIQueue *q, *tmp; - QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) { + QTAILQ_FOREACH_SAFE(q, head, next, tmp) { if (!usb_packet_is_inflight(&q->packet) || q->packet.ep->dev != dev) { continue; } - ehci_free_queue(q); + ehci_free_queue(q, async); } } -static void ehci_queues_rip_all(EHCIState *ehci) +static void ehci_queues_rip_all(EHCIState *ehci, int async) { + EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues; EHCIQueue *q, *tmp; - QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) { - ehci_free_queue(q); + QTAILQ_FOREACH_SAFE(q, head, next, tmp) { + ehci_free_queue(q, async); } } @@ -773,7 +781,8 @@ static void ehci_detach(USBPort *port) return; } - ehci_queues_rip_device(s, port->dev); + ehci_queues_rip_device(s, port->dev, 0); + ehci_queues_rip_device(s, port->dev, 1); *portsc &= ~(PORTSC_CONNECT|PORTSC_PED); *portsc |= PORTSC_CSC; @@ -793,7 +802,8 @@ static void ehci_child_detach(USBPort *port, USBDevice *child) return; } - ehci_queues_rip_device(s, child); + ehci_queues_rip_device(s, child, 0); + ehci_queues_rip_device(s, child, 1); } static void ehci_wakeup(USBPort *port) @@ -911,7 +921,8 @@ static void ehci_reset(void *opaque) usb_device_reset(devs[i]); } } - ehci_queues_rip_all(s); + ehci_queues_rip_all(s, 0); + ehci_queues_rip_all(s, 1); qemu_del_timer(s->frame_timer); } @@ -1526,7 +1537,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci, int async) ehci_set_usbsts(ehci, USBSTS_REC); } - ehci_queues_rip_unused(ehci); + ehci_queues_rip_unused(ehci, async); /* Find the head of the list (4.9.1.1) */ for(i = 0; i < MAX_QH; i++) { @@ -1613,7 +1624,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) int reload; entry = ehci_get_fetch_addr(ehci, async); - q = ehci_find_queue_by_qh(ehci, entry); + q = ehci_find_queue_by_qh(ehci, entry, async); if (NULL == q) { q = ehci_alloc_queue(ehci, async); } @@ -2064,7 +2075,7 @@ static void ehci_advance_state(EHCIState *ehci, static void ehci_advance_async_state(EHCIState *ehci) { - int async = 1; + const int async = 1; switch(ehci_get_state(ehci, async)) { case EST_INACTIVE: @@ -2121,7 +2132,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci) { uint32_t entry; uint32_t list; - int async = 0; + const int async = 0; // 4.6 @@ -2354,7 +2365,8 @@ static int usb_ehci_initfn(PCIDevice *dev) } s->frame_timer = qemu_new_timer_ns(vm_clock, ehci_frame_timer, s); - QTAILQ_INIT(&s->queues); + QTAILQ_INIT(&s->aqueues); + QTAILQ_INIT(&s->pqueues); qemu_register_reset(ehci_reset, s); -- cgit v1.1 From 7bce354e148b0ae0ceae7582574810adc6fab4e4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:11 +0100 Subject: usb-ehci: always call ehci_queues_rip_unused for period queues Before this patch USB 2 devices with interrupt endpoints were not working properly. The problem is that to avoid loops we stop processing as soon as we encounter a queue-head (qh) we've already seen since qhs can be linked in a circular fashion, this is tracked by the seen flag in our qh struct. The resetting of the seen flag is done from ehci_queues_rip_unused which before this patch was only called when executing the statemachine for the async schedule. But packets for interrupt endpoints are part of the periodic schedule! So what would happen is that when there were no ctrl or bulk packets for a USB 2 device with an interrupt endpoint, the async schedule would become non active, then ehci_queues_rip_unused would no longer get called and when processing the qhs for the interrupt endpoints from the periodic schedule their seen bit would still be 1 and they would be skipped. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 840022d..d384fcc 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -2167,6 +2167,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci) ehci_set_fetch_addr(ehci, async,entry); ehci_set_state(ehci, async, EST_FETCHENTRY); ehci_advance_state(ehci, async); + ehci_queues_rip_unused(ehci, async); break; default: -- cgit v1.1 From 4be23939ab0d7019c7e59a37485b416fbbf0f073 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:12 +0100 Subject: usb-ehci: Drop cached qhs when the doorbell gets rung The purpose of the IAAD bit / the doorbell is to make the ehci controller forget about cached qhs, this is mainly used when cancelling transactions, the qh is unlinked from the async schedule and then the doorbell gets rung, once the doorbell is acked by the controller the hcd knows that the qh is no longer in use and that it can do something else with the memory, such as re-use it for a new qh! But we keep our struct representing this qh around for circa 250 ms. This allows for a (mightily large) race window where the following could happen: -hcd submits a qh at address 0xdeadbeef -our ehci code sees the qh, sends a request to a usb-device, gets a result of USB_RET_ASYNC, sets the async_state of the qh to EHCI_ASYNC_INFLIGHT -hcd unlinks the qh at address 0xdeadbeef -hcd rings the doorbell, wait for us to ack it -hcd re-uses the qh at address 0xdeadbeef -our ehci code sees the qh, looks in the async_queue, sees there already is a qh at address 0xdeadbeef there with async_state of EHCI_ASYNC_INFLIGHT, does nothing -the *original* (which the hcd thinks it has cancelled) transaction finishes -our ehci code sees the qh on yet another pass through the async list, looks in the async_queue, sees there already is a qh at address 0xdeadbeef there with async_state of EHCI_ASYNC_COMPLETED, and finished the transaction with the results of the *original* transaction. Not good (tm), this patch fixes this race by removing all qhs which have not been seen during the last cycle through the async list immidiately when the doorbell is rung. Note this patch does not fix any actually observed problem, but upon reading of the EHCI spec it became apparent to me that the above race could happen and the usb-ehci behavior from before this patch is not good. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index d384fcc..b349003 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -697,7 +697,7 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr, return NULL; } -static void ehci_queues_rip_unused(EHCIState *ehci, int async) +static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush) { EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues; EHCIQueue *q, *tmp; @@ -708,7 +708,7 @@ static void ehci_queues_rip_unused(EHCIState *ehci, int async) q->ts = ehci->last_run_ns; continue; } - if (ehci->last_run_ns < q->ts + 250000000) { + if (!flush && ehci->last_run_ns < q->ts + 250000000) { /* allow 0.25 sec idle */ continue; } @@ -1537,7 +1537,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci, int async) ehci_set_usbsts(ehci, USBSTS_REC); } - ehci_queues_rip_unused(ehci, async); + ehci_queues_rip_unused(ehci, async, 0); /* Find the head of the list (4.9.1.1) */ for(i = 0; i < MAX_QH; i++) { @@ -2093,18 +2093,7 @@ static void ehci_advance_async_state(EHCIState *ehci) break; } - /* If the doorbell is set, the guest wants to make a change to the - * schedule. The host controller needs to release cached data. - * (section 4.8.2) - */ - if (ehci->usbcmd & USBCMD_IAAD) { - DPRINTF("ASYNC: doorbell request acknowledged\n"); - ehci->usbcmd &= ~USBCMD_IAAD; - ehci_set_interrupt(ehci, USBSTS_IAA); - break; - } - - /* make sure guest has acknowledged */ + /* make sure guest has acknowledged the doorbell interrupt */ /* TO-DO: is this really needed? */ if (ehci->usbsts & USBSTS_IAA) { DPRINTF("IAA status bit still set.\n"); @@ -2118,6 +2107,18 @@ static void ehci_advance_async_state(EHCIState *ehci) ehci_set_state(ehci, async, EST_WAITLISTHEAD); ehci_advance_state(ehci, async); + + /* If the doorbell is set, the guest wants to make a change to the + * schedule. The host controller needs to release cached data. + * (section 4.8.2) + */ + if (ehci->usbcmd & USBCMD_IAAD) { + /* Remove all unseen qhs from the async qhs queue */ + ehci_queues_rip_unused(ehci, async, 1); + DPRINTF("ASYNC: doorbell request acknowledged\n"); + ehci->usbcmd &= ~USBCMD_IAAD; + ehci_set_interrupt(ehci, USBSTS_IAA); + } break; default: @@ -2167,7 +2168,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci) ehci_set_fetch_addr(ehci, async,entry); ehci_set_state(ehci, async, EST_FETCHENTRY); ehci_advance_state(ehci, async); - ehci_queues_rip_unused(ehci, async); + ehci_queues_rip_unused(ehci, async, 0); break; default: -- cgit v1.1 From e850c2b45306b59d179c4df93d91edc1c3c45106 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:13 +0100 Subject: usb-ehci: Rip the queues when the async or period schedule is halted Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index b349003..d386b84 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1076,7 +1076,8 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val) if (!(val & USBCMD_RUNSTOP) && (s->usbcmd & USBCMD_RUNSTOP)) { qemu_del_timer(s->frame_timer); - // TODO - should finish out some stuff before setting halt + ehci_queues_rip_all(s, 0); + ehci_queues_rip_all(s, 1); ehci_set_usbsts(s, USBSTS_HALT); } @@ -2088,6 +2089,7 @@ static void ehci_advance_async_state(EHCIState *ehci) case EST_ACTIVE: if ( !(ehci->usbcmd & USBCMD_ASE)) { + ehci_queues_rip_all(ehci, async); ehci_clear_usbsts(ehci, USBSTS_ASS); ehci_set_state(ehci, async, EST_INACTIVE); break; @@ -2148,6 +2150,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci) case EST_ACTIVE: if ( !(ehci->frindex & 7) && !(ehci->usbcmd & USBCMD_PSE)) { + ehci_queues_rip_all(ehci, async); ehci_clear_usbsts(ehci, USBSTS_PSS); ehci_set_state(ehci, async, EST_INACTIVE); break; -- cgit v1.1 From 2763cbc751c494dd2f58f902ad80a8048f9cfd7b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:14 +0100 Subject: usb-ehci: Any packet completion except for NAK should set the interrupt As clearly stated in the 2.3.2 of the EHCI spec, any time USBERRINT get sets then if the td has its IOC bit set USBINT should be set as well. This means that for any status except for USB_RET_NAK we should set USBINT if the IOC bit is set. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index d386b84..507e4a8 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1360,7 +1360,7 @@ err: q->qh.token ^= QTD_TOKEN_DTOGGLE; q->qh.token &= ~QTD_TOKEN_ACTIVE; - if ((q->usb_status >= 0) && (q->qh.token & QTD_TOKEN_IOC)) { + if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) { ehci_record_interrupt(q->ehci, USBSTS_INT); } } -- cgit v1.1 From dd54cfe0bcf30d1f3a34e9c397f619d423fe0c5b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:15 +0100 Subject: usb-ehci: Fix cerr tracking cerr should only be decremented on errors which cause XactErr to be set, and when that happens the failing transaction should be retried until cerr reaches 0 and only then should USBSTS_ERRINT be set (and inactive cleared and USBSTS_INT set if requested). Since we don't have any hardware level errors (and in case of redirection the real hardware has already retried), re-trying makes no sense, so immediately set cerr to 0 on errors which set XactErr. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 507e4a8..2685adc 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1291,7 +1291,7 @@ static void ehci_async_complete_packet(USBPort *port, USBPacket *packet) static void ehci_execute_complete(EHCIQueue *q) { - int c_err, reload; + int reload; assert(q->async != EHCI_ASYNC_INFLIGHT); q->async = EHCI_ASYNC_NONE; @@ -1300,15 +1300,10 @@ static void ehci_execute_complete(EHCIQueue *q) q->qhaddr, q->qh.next, q->qtdaddr, q->usb_status); if (q->usb_status < 0) { -err: - /* TO-DO: put this is in a function that can be invoked below as well */ - c_err = get_field(q->qh.token, QTD_TOKEN_CERR); - c_err--; - set_field(&q->qh.token, c_err, QTD_TOKEN_CERR); - switch(q->usb_status) { case USB_RET_NODEV: q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR); + set_field(&q->qh.token, 0, QTD_TOKEN_CERR); ehci_record_interrupt(q->ehci, USBSTS_ERRINT); break; case USB_RET_STALL: @@ -1336,15 +1331,13 @@ err: assert(0); break; } + } else if ((q->usb_status > q->tbytes) && (q->pid == USB_TOKEN_IN)) { + q->usb_status = USB_RET_BABBLE; + q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE); + ehci_record_interrupt(q->ehci, USBSTS_ERRINT); } else { - // DPRINTF("Short packet condition\n"); // TODO check 4.12 for splits - if ((q->usb_status > q->tbytes) && (q->pid == USB_TOKEN_IN)) { - q->usb_status = USB_RET_BABBLE; - goto err; - } - if (q->tbytes && q->pid == USB_TOKEN_IN) { q->tbytes -= q->usb_status; } else { -- cgit v1.1 From aa73fcdcc1bf4157121716e2eb3230130b0d1232 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:16 +0100 Subject: usb-ehci: Remove dead nakcnt code This patch removes 2 bits of dead nakcnt code: 1) usb_ehci_execute calls ehci_qh_do_overlay which does: nakcnt = reload; and then has a block of code which is conditional on: if (reload && !nakcnt) { which ofcourse is never true now as nakcnt == reload. 2) ehci_state_fetchqh does: nakcnt = reload; but before nakcnt is ever used ehci_state_fetchqh is always followed by a ehci_qh_do_overlay call which also does: nakcnt = reload; So doing this from ehci_state_fetchqh is redundant. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 2685adc..07bcd1f 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1615,7 +1615,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) { uint32_t entry; EHCIQueue *q; - int reload; entry = ehci_get_fetch_addr(ehci, async); q = ehci_find_queue_by_qh(ehci, entry, async); @@ -1673,11 +1672,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) } #endif - reload = get_field(q->qh.epchar, QH_EPCHAR_RL); - if (reload) { - set_field(&q->qh.altnext_qtd, reload, QH_ALTNEXT_NAKCNT); - } - if (q->qh.token & QTD_TOKEN_HALT) { ehci_set_state(ehci, async, EST_HORIZONTALQH); @@ -1837,25 +1831,11 @@ static void ehci_flush_qh(EHCIQueue *q) static int ehci_state_execute(EHCIQueue *q, int async) { int again = 0; - int reload, nakcnt; - int smask; if (ehci_qh_do_overlay(q) != 0) { return -1; } - smask = get_field(q->qh.epcap, QH_EPCAP_SMASK); - - if (!smask) { - reload = get_field(q->qh.epchar, QH_EPCHAR_RL); - nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT); - if (reload && !nakcnt) { - ehci_set_state(q->ehci, async, EST_HORIZONTALQH); - again = 1; - goto out; - } - } - // TODO verify enough time remains in the uframe as in 4.4.1.1 // TODO write back ptr to async list when done or out of time // TODO Windows does not seem to ever set the MULT field -- cgit v1.1 From 553a6a59f6931bf3a034945e0c1585f4b05d6000 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:17 +0100 Subject: usb-ehci: Fix and simplify nakcnt handling The nakcnt code in ehci_execute_complete() marked transactions as finished when a packet completed with a result of USB_RET_NAK, but USB_RET_NAK means that the device cannot receive / send data at that time and that the transaction should be retried later, which is also what the usb-uhci and usb-ohci code does. Note that there already was some special code in place to handle this for interrupt endpoints in the form of doing a return from ehci_execute_complete() when reload == 0, but that for bulk transactions this was not handled correctly (where as for example the usb-ccid device does return USB_RET_NAK for bulk packets). Besides that the code in ehci_execute_complete() decrement nakcnt by 1 on a packet result of USB_RET_NAK, but -since the transaction got marked as finished, nakcnt would never be decremented again -there is no code checking for nakcnt becoming 0 -there is no use in re-trying the transaction within the same usb frame / usb-ehci frame-timer call, since the status of emulated devices won't change as long as the usb-ehci frame-timer is running So we should simply set the nakcnt to 0 when we get a USB_RET_NAK, thus claiming that we've tried reload times (or as many times as possible if reload is 0). Besides the code in ehci_execute_complete() handling USB_RET_NAK there was also code handling it in ehci_state_executing(), which calls ehci_execute_complete(), and then does its own handling on top of the handling in ehci_execute_complete(), this code would decrement nakcnt *again* (if not already 0), or restore the reload value (which was never changed) on success. Since the double decrement was wrong to begin with, and is no longer needed now that we set nakcnt directly to 0 on USB_RET_NAK, and the restore of reload is not needed either, this patch simply removes all nakcnt handling from ehci_state_executing(). Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 07bcd1f..9197298 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1291,8 +1291,6 @@ static void ehci_async_complete_packet(USBPort *port, USBPacket *packet) static void ehci_execute_complete(EHCIQueue *q) { - int reload; - assert(q->async != EHCI_ASYNC_INFLIGHT); q->async = EHCI_ASYNC_NONE; @@ -1311,16 +1309,8 @@ static void ehci_execute_complete(EHCIQueue *q) ehci_record_interrupt(q->ehci, USBSTS_ERRINT); break; case USB_RET_NAK: - /* 4.10.3 */ - reload = get_field(q->qh.epchar, QH_EPCHAR_RL); - if ((q->pid == USB_TOKEN_IN) && reload) { - int nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT); - nakcnt--; - set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT); - } else if (!reload) { - return; - } - break; + set_field(&q->qh.altnext_qtd, 0, QH_ALTNEXT_NAKCNT); + return; /* We're not done yet with this transaction */ case USB_RET_BABBLE: q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE); ehci_record_interrupt(q->ehci, USBSTS_ERRINT); @@ -1353,7 +1343,7 @@ static void ehci_execute_complete(EHCIQueue *q) q->qh.token ^= QTD_TOKEN_DTOGGLE; q->qh.token &= ~QTD_TOKEN_ACTIVE; - if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) { + if (q->qh.token & QTD_TOKEN_IOC) { ehci_record_interrupt(q->ehci, USBSTS_INT); } } @@ -1877,7 +1867,6 @@ out: static int ehci_state_executing(EHCIQueue *q, int async) { int again = 0; - int reload, nakcnt; ehci_execute_complete(q); if (q->usb_status == USB_RET_ASYNC) { @@ -1897,21 +1886,8 @@ static int ehci_state_executing(EHCIQueue *q, int async) // counter decrements to 0 } - reload = get_field(q->qh.epchar, QH_EPCHAR_RL); - if (reload) { - nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT); - if (q->usb_status == USB_RET_NAK) { - if (nakcnt) { - nakcnt--; - } - } else { - nakcnt = reload; - } - set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT); - } - /* 4.10.5 */ - if ((q->usb_status == USB_RET_NAK) || (q->qh.token & QTD_TOKEN_ACTIVE)) { + if (q->usb_status == USB_RET_NAK) { ehci_set_state(q->ehci, async, EST_HORIZONTALQH); } else { ehci_set_state(q->ehci, async, EST_WRITEBACK); -- cgit v1.1 From 5eafd438c9e3b7b698c4a4ec755f52d8dfb870ae Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:18 +0100 Subject: usb-ehci: Cleanup itd error handling All error statuses except for NAK are handled in a switch case, move the handling of NAK into the same switch case. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 9197298..825fcc0 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1466,20 +1466,7 @@ static int ehci_process_itd(EHCIState *ehci, } qemu_sglist_destroy(&ehci->isgl); - if (ret == USB_RET_NAK) { - /* no data for us, so do a zero-length transfer */ - ret = 0; - } - - if (ret >= 0) { - if (!dir) { - /* OUT */ - set_field(&itd->transact[i], len - ret, ITD_XACT_LENGTH); - } else { - /* IN */ - set_field(&itd->transact[i], ret, ITD_XACT_LENGTH); - } - } else { + if (ret < 0) { switch (ret) { default: fprintf(stderr, "Unexpected iso usb result: %d\n", ret); @@ -1495,6 +1482,19 @@ static int ehci_process_itd(EHCIState *ehci, itd->transact[i] |= ITD_XACT_BABBLE; ehci_record_interrupt(ehci, USBSTS_ERRINT); break; + case USB_RET_NAK: + /* no data for us, so do a zero-length transfer */ + ret = 0; + break; + } + } + if (ret >= 0) { + if (!dir) { + /* OUT */ + set_field(&itd->transact[i], len - ret, ITD_XACT_LENGTH); + } else { + /* IN */ + set_field(&itd->transact[i], ret, ITD_XACT_LENGTH); } } if (itd->transact[i] & ITD_XACT_IOC) { -- cgit v1.1 From 4d819a9bde7f52e7421d4128dc69c296c9fd017a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:19 +0100 Subject: usb: return BABBLE rather then NAK when we receive too much data Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- usb-linux.c | 8 +++++++- usb-redir.c | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/usb-linux.c b/usb-linux.c index 47994f3..38df9e6 100644 --- a/usb-linux.c +++ b/usb-linux.c @@ -364,6 +364,10 @@ static void async_complete(void *opaque) p->result = USB_RET_STALL; break; + case -EOVERFLOW: + p->result = USB_RET_BABBLE; + break; + default: p->result = USB_RET_NAK; break; @@ -722,6 +726,8 @@ static int urb_status_to_usb_ret(int status) switch (status) { case -EPIPE: return USB_RET_STALL; + case -EOVERFLOW: + return USB_RET_BABBLE; default: return USB_RET_NAK; } @@ -759,7 +765,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p, int in) } else if (aurb[i].urb.iso_frame_desc[j].actual_length > p->iov.size) { printf("husb: received iso data is larger then packet\n"); - len = USB_RET_NAK; + len = USB_RET_BABBLE; /* All good copy data over */ } else { len = aurb[i].urb.iso_frame_desc[j].actual_length; diff --git a/usb-redir.c b/usb-redir.c index a87de6e..c52311a 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -447,7 +447,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, USBPacket *p, ERROR("received iso data is larger then packet ep %02X (%d > %d)\n", ep, len, (int)p->iov.size); bufp_free(dev, isop, ep); - return USB_RET_NAK; + return USB_RET_BABBLE; } usb_packet_copy(p, isop->data, len); bufp_free(dev, isop, ep); @@ -566,7 +566,7 @@ static int usbredir_handle_interrupt_data(USBRedirDevice *dev, if (len > p->iov.size) { ERROR("received int data is larger then packet ep %02X\n", ep); bufp_free(dev, intp, ep); - return USB_RET_NAK; + return USB_RET_BABBLE; } usb_packet_copy(p, intp->data, len); bufp_free(dev, intp, ep); -- cgit v1.1 From d61000a8b1d99c5155440b727ea32f12a4988120 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 2 Mar 2012 21:27:20 +0100 Subject: usb: add USB_RET_IOERROR We already have USB_RET_NAK, but that means that a device does not want to send/receive right now. But with host / network redirection we can actually have a transaction fail due to some io error, rather then ie the device just not having any data atm. This patch adds a new error code named USB_RET_IOERROR for this, and uses it were appropriate. Notes: -Currently all usb-controllers handle this the same as NODEV, but that may change in the future, OHCI could indicate a CRC error instead for example. -This patch does not touch hw/usb-musb.c, that is because the code in there handles STALL and NAK specially and has a if status < 0 generic catch all for all other errors Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 2 ++ hw/usb-ohci.c | 2 ++ hw/usb-uhci.c | 1 + hw/usb.h | 11 ++++++----- usb-linux.c | 4 ++-- usb-redir.c | 9 ++++++--- 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 825fcc0..df742f7 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1299,6 +1299,7 @@ static void ehci_execute_complete(EHCIQueue *q) if (q->usb_status < 0) { switch(q->usb_status) { + case USB_RET_IOERROR: case USB_RET_NODEV: q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR); set_field(&q->qh.token, 0, QTD_TOKEN_CERR); @@ -1471,6 +1472,7 @@ static int ehci_process_itd(EHCIState *ehci, default: fprintf(stderr, "Unexpected iso usb result: %d\n", ret); /* Fall through */ + case USB_RET_IOERROR: case USB_RET_NODEV: /* 3.3.2: XACTERR is only allowed on IN transactions */ if (dir) { diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c index 7aa19fe..20aaa74 100644 --- a/hw/usb-ohci.c +++ b/hw/usb-ohci.c @@ -837,6 +837,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, OHCI_CC_DATAUNDERRUN); } else { switch (ret) { + case USB_RET_IOERROR: case USB_RET_NODEV: OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_CC, OHCI_CC_DEVICENOTRESPONDING); @@ -1052,6 +1053,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed) OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DATAUNDERRUN); } else { switch (ret) { + case USB_RET_IOERROR: case USB_RET_NODEV: OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DEVICENOTRESPONDING); case USB_RET_NAK: diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c index 70e3881..2c6ed38 100644 --- a/hw/usb-uhci.c +++ b/hw/usb-uhci.c @@ -765,6 +765,7 @@ out: break; return 1; + case USB_RET_IOERROR: case USB_RET_NODEV: default: break; diff --git a/hw/usb.h b/hw/usb.h index 8e83697..1a30ebb 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -39,11 +39,12 @@ #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_ASYNC (-5) +#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_SPEED_LOW 0 #define USB_SPEED_FULL 1 diff --git a/usb-linux.c b/usb-linux.c index 38df9e6..050ea7a 100644 --- a/usb-linux.c +++ b/usb-linux.c @@ -369,7 +369,7 @@ static void async_complete(void *opaque) break; default: - p->result = USB_RET_NAK; + p->result = USB_RET_IOERROR; break; } @@ -729,7 +729,7 @@ static int urb_status_to_usb_ret(int status) case -EOVERFLOW: return USB_RET_BABBLE; default: - return USB_RET_NAK; + return USB_RET_IOERROR; } } diff --git a/usb-redir.c b/usb-redir.c index c52311a..8e9f175 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -431,7 +431,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, USBPacket *p, /* Check iso_error for stream errors, otherwise its an underrun */ status = dev->endpoint[EP2I(ep)].iso_error; dev->endpoint[EP2I(ep)].iso_error = 0; - return status ? USB_RET_NAK : 0; + return status ? USB_RET_IOERROR : 0; } DPRINTF2("iso-token-in ep %02X status %d len %d queue-size: %d\n", ep, isop->status, isop->len, dev->endpoint[EP2I(ep)].bufpq_size); @@ -439,7 +439,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, USBPacket *p, status = isop->status; if (status != usb_redir_success) { bufp_free(dev, isop, ep); - return USB_RET_NAK; + return USB_RET_IOERROR; } len = isop->len; @@ -1018,11 +1018,14 @@ static int usbredir_handle_status(USBRedirDevice *dev, return USB_RET_STALL; case usb_redir_cancelled: WARNING("returning cancelled packet to HC?\n"); + return USB_RET_NAK; case usb_redir_inval: + WARNING("got invalid param error from usb-host?\n"); + return USB_RET_NAK; case usb_redir_ioerror: case usb_redir_timeout: default: - return USB_RET_NAK; + return USB_RET_IOERROR; } } -- cgit v1.1 From 7c308b7e38812b49e3e9eccdb74b01989cdf7b78 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 27 Feb 2012 16:41:57 +0100 Subject: uhci_fill_queue: zap debug printf --- hw/usb-uhci.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c index 2c6ed38..304b84b 100644 --- a/hw/usb-uhci.c +++ b/hw/usb-uhci.c @@ -951,7 +951,6 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) UHCI_TD ptd; int ret; - fprintf(stderr, "%s: -- %x\n", __func__, token); while (is_valid(plink)) { pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd)); le32_to_cpus(&ptd.link); -- cgit v1.1 From eb9d4673e3594baaa857a4c033fc7c21f4ea904b Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 28 Feb 2012 15:36:06 +0100 Subject: usb: queue can have async packets This can happen today in case the ->complete() callback queues up the next packet. Also we'll support pipelining soon, which allows to have multiple packets per queue in flight (aka ASYNC) state. Signed-off-by: Gerd Hoffmann --- hw/usb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/usb.c b/hw/usb.c index 57fc5e3..fc41d62 100644 --- a/hw/usb.c +++ b/hw/usb.c @@ -356,6 +356,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) while (!QTAILQ_EMPTY(&ep->queue)) { p = QTAILQ_FIRST(&ep->queue); + if (p->state == USB_PACKET_ASYNC) { + break; + } assert(p->state == USB_PACKET_QUEUED); ret = usb_process_one(p); if (ret == USB_RET_ASYNC) { -- cgit v1.1 From 7936e0f0d2fcd05bf5a78985a53b6b2276115749 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 1 Mar 2012 14:39:28 +0100 Subject: usb: add pipelining option to usb endpoints With this patch applied USB drivers can enable pipelining per endpoint. With pipelining enabled the usb core will continue submitting packets even when there are still async transfers in flight instead of passing them on one by one. Signed-off-by: Gerd Hoffmann --- hw/usb.c | 11 ++++++++++- hw/usb.h | 2 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/hw/usb.c b/hw/usb.c index fc41d62..800d912 100644 --- a/hw/usb.c +++ b/hw/usb.c @@ -323,7 +323,7 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) assert(p->state == USB_PACKET_SETUP); assert(p->ep != NULL); - if (QTAILQ_EMPTY(&p->ep->queue)) { + if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) { ret = usb_process_one(p); if (ret == USB_RET_ASYNC) { usb_packet_set_state(p, USB_PACKET_ASYNC); @@ -468,6 +468,7 @@ void usb_ep_init(USBDevice *dev) dev->ep_ctl.type = USB_ENDPOINT_XFER_CONTROL; dev->ep_ctl.ifnum = 0; dev->ep_ctl.dev = dev; + dev->ep_ctl.pipeline = false; QTAILQ_INIT(&dev->ep_ctl.queue); for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) { dev->ep_in[ep].nr = ep + 1; @@ -480,6 +481,8 @@ void usb_ep_init(USBDevice *dev) dev->ep_out[ep].ifnum = 0; dev->ep_in[ep].dev = dev; dev->ep_out[ep].dev = dev; + dev->ep_in[ep].pipeline = false; + dev->ep_out[ep].pipeline = false; QTAILQ_INIT(&dev->ep_in[ep].queue); QTAILQ_INIT(&dev->ep_out[ep].queue); } @@ -593,3 +596,9 @@ int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep) struct USBEndpoint *uep = usb_ep_get(dev, pid, ep); return uep->max_packet_size; } + +void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled) +{ + struct USBEndpoint *uep = usb_ep_get(dev, pid, ep); + uep->pipeline = enabled; +} diff --git a/hw/usb.h b/hw/usb.h index 1a30ebb..f6df0ad 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -177,6 +177,7 @@ struct USBEndpoint { uint8_t type; uint8_t ifnum; int max_packet_size; + bool pipeline; USBDevice *dev; QTAILQ_HEAD(, USBPacket) queue; }; @@ -364,6 +365,7 @@ void usb_ep_set_ifnum(USBDevice *dev, int pid, int ep, uint8_t ifnum); void usb_ep_set_max_packet_size(USBDevice *dev, int pid, int ep, uint16_t raw); int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep); +void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled); void usb_attach(USBPort *port); void usb_detach(USBPort *port); -- cgit v1.1 From 9424d4e7c67d9d0cbc50fc8a1d00db31772c37c5 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 1 Mar 2012 14:42:34 +0100 Subject: usb-host: enable pipelineing for bulk endpoints. We really don't want to wait for packets finish before submitting the next, we want keep the data flow running. Signed-off-by: Gerd Hoffmann --- usb-linux.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/usb-linux.c b/usb-linux.c index 050ea7a..90919c2 100644 --- a/usb-linux.c +++ b/usb-linux.c @@ -1192,6 +1192,9 @@ static int usb_linux_update_endp_table(USBHostDevice *s) USB_ENDPOINT_XFER_INVALID); usb_ep_set_type(&s->dev, pid, ep, type); usb_ep_set_ifnum(&s->dev, pid, ep, interface); + if (type == USB_ENDPOINT_XFER_BULK) { + usb_ep_set_pipeline(&s->dev, pid, ep, true); + } epd = get_endp(s, pid, ep); epd->halted = 0; -- cgit v1.1 From 1b4b29a11483a050855838014413c91e9c1f8c19 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 2 Mar 2012 13:22:29 +0100 Subject: usb: add shortcut for control transfers Add a more direct code path to submit control transfers. Instead of feeding three usb packets (setup, data, ack) to usb_handle_packet and have the do_token_* functions in usb.c poke the control transfer parameters out of it just submit a single packet carrying the actual data with the control xfer parameters filled into USBPacket->parameters. Signed-off-by: Gerd Hoffmann --- hw/usb.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/usb.h | 1 + 2 files changed, 60 insertions(+) diff --git a/hw/usb.c b/hw/usb.c index 800d912..1ec2e90 100644 --- a/hw/usb.c +++ b/hw/usb.c @@ -95,6 +95,7 @@ void usb_wakeup(USBEndpoint *ep) #define SETUP_STATE_SETUP 1 #define SETUP_STATE_DATA 2 #define SETUP_STATE_ACK 3 +#define SETUP_STATE_PARAM 4 static int do_token_setup(USBDevice *s, USBPacket *p) { @@ -226,6 +227,50 @@ static int do_token_out(USBDevice *s, USBPacket *p) } } +static int do_parameter(USBDevice *s, USBPacket *p) +{ + int request, value, index; + int i, ret = 0; + + for (i = 0; i < 8; i++) { + s->setup_buf[i] = p->parameter >> (i*8); + } + + s->setup_state = SETUP_STATE_PARAM; + s->setup_len = (s->setup_buf[7] << 8) | s->setup_buf[6]; + s->setup_index = 0; + + request = (s->setup_buf[0] << 8) | s->setup_buf[1]; + value = (s->setup_buf[3] << 8) | s->setup_buf[2]; + index = (s->setup_buf[5] << 8) | s->setup_buf[4]; + + if (s->setup_len > sizeof(s->data_buf)) { + fprintf(stderr, + "usb_generic_handle_packet: ctrl buffer too small (%d > %zu)\n", + s->setup_len, sizeof(s->data_buf)); + return USB_RET_STALL; + } + + if (p->pid == USB_TOKEN_OUT) { + usb_packet_copy(p, s->data_buf, s->setup_len); + } + + ret = usb_device_handle_control(s, p, request, value, index, + s->setup_len, s->data_buf); + if (ret < 0) { + return ret; + } + + if (ret < s->setup_len) { + s->setup_len = ret; + } + if (p->pid == USB_TOKEN_IN) { + usb_packet_copy(p, s->data_buf, s->setup_len); + } + + return ret; +} + /* ctrl complete function for devices which use usb_generic_handle_packet and may return USB_RET_ASYNC from their handle_control callback. Device code which does this *must* call this function instead of the normal @@ -250,6 +295,16 @@ void usb_generic_async_ctrl_complete(USBDevice *s, USBPacket *p) p->result = 0; break; + case SETUP_STATE_PARAM: + if (p->result < s->setup_len) { + s->setup_len = p->result; + } + if (p->pid == USB_TOKEN_IN) { + p->result = 0; + usb_packet_copy(p, s->data_buf, s->setup_len); + } + break; + default: break; } @@ -292,6 +347,9 @@ static int usb_process_one(USBPacket *p) if (p->ep->nr == 0) { /* control pipe */ + if (p->parameter) { + return do_parameter(dev, p); + } switch (p->pid) { case USB_TOKEN_SETUP: return do_token_setup(dev, p); @@ -416,6 +474,7 @@ void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep) p->pid = pid; p->ep = ep; p->result = 0; + p->parameter = 0; qemu_iovec_reset(&p->iov); usb_packet_set_state(p, USB_PACKET_SETUP); } diff --git a/hw/usb.h b/hw/usb.h index f6df0ad..d60d03d 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -327,6 +327,7 @@ struct USBPacket { int pid; USBEndpoint *ep; QEMUIOVector iov; + uint64_t parameter; /* control transfers */ int result; /* transfer length or USB_RET_* status code */ /* Internal use by the USB layer. */ USBPacketState state; -- cgit v1.1 From 2850ca9ed1023ce30ecd0582a66ded7a180e7616 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 1 Mar 2012 15:51:51 +0100 Subject: xhci: fix control xfers Use the new, direct control transfer submission method instead of bypassing the usb core by calling usb_device_handle_control directly. The later fails for async control transfers. This patch gets xhci + usb-host combo going. --- hw/usb-xhci.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/hw/usb-xhci.c b/hw/usb-xhci.c index fc5b542..8305489 100644 --- a/hw/usb-xhci.c +++ b/hw/usb-xhci.c @@ -1470,8 +1470,8 @@ static USBDevice *xhci_find_device(XHCIPort *port, uint8_t addr) static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) { XHCITRB *trb_setup, *trb_status; - uint8_t bmRequestType, bRequest; - uint16_t wValue, wLength, wIndex; + uint8_t bmRequestType; + uint16_t wLength; XHCIPort *port; USBDevice *dev; int ret; @@ -1508,9 +1508,6 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) } bmRequestType = trb_setup->parameter; - bRequest = trb_setup->parameter >> 8; - wValue = trb_setup->parameter >> 16; - wIndex = trb_setup->parameter >> 32; wLength = trb_setup->parameter >> 48; if (xfer->data && xfer->data_alloced < wLength) { @@ -1537,12 +1534,12 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) xfer->iso_xfer = false; xhci_setup_packet(xfer, dev); + xfer->packet.parameter = trb_setup->parameter; if (!xfer->in_xfer) { xhci_xfer_data(xfer, xfer->data, wLength, 0, 1, 0); } - ret = usb_device_handle_control(dev, &xfer->packet, - (bmRequestType << 8) | bRequest, - wValue, wIndex, wLength, xfer->data); + + ret = usb_handle_packet(dev, &xfer->packet); xhci_complete_packet(xfer, ret); if (!xfer->running_async && !xfer->running_retry) { -- cgit v1.1 From cf21a4aef712075d313da4ea924a3376574c16e5 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 1 Mar 2012 15:14:12 +0100 Subject: xhci: fix port status Don't signal port status change if the usb device isn't in attached state. Happens with usb-host devices with the pass-through device being plugged out at the host. Signed-off-by: Gerd Hoffmann --- hw/usb-xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb-xhci.c b/hw/usb-xhci.c index 8305489..e8f1b6e 100644 --- a/hw/usb-xhci.c +++ b/hw/usb-xhci.c @@ -2279,7 +2279,7 @@ static void xhci_update_port(XHCIState *xhci, XHCIPort *port, int is_detach) int nr = port->port.index + 1; port->portsc = PORTSC_PP; - if (port->port.dev && !is_detach) { + if (port->port.dev && port->port.dev->attached && !is_detach) { port->portsc |= PORTSC_CCS; switch (port->port.dev->speed) { case USB_SPEED_LOW: -- cgit v1.1