From 05f43d44e4bc26611ce25fd7d726e483f73363ce Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 10 Oct 2016 12:46:22 +0200 Subject: [PATCH 01/11] xhci: limit the number of link trbs we are willing to process Needed to avoid we run in circles forever in case the guest builds an endless loop with link trbs. Reported-by: Li Qiang Tested-by: P J P Signed-off-by: Gerd Hoffmann Message-id: 1476096382-7981-1-git-send-email-kraxel@redhat.com --- hw/usb/hcd-xhci.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 726435c462..ee4fa484d6 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -54,6 +54,8 @@ * to the specs when it gets them */ #define ER_FULL_HACK +#define TRB_LINK_LIMIT 4 + #define LEN_CAP 0x40 #define LEN_OPER (0x400 + 0x10 * MAXPORTS) #define LEN_RUNTIME ((MAXINTRS + 1) * 0x20) @@ -1000,6 +1002,7 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb, dma_addr_t *addr) { PCIDevice *pci_dev = PCI_DEVICE(xhci); + uint32_t link_cnt = 0; while (1) { TRBType type; @@ -1026,6 +1029,9 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb, ring->dequeue += TRB_SIZE; return type; } else { + if (++link_cnt > TRB_LINK_LIMIT) { + return 0; + } ring->dequeue = xhci_mask64(trb->parameter); if (trb->control & TRB_LK_TC) { ring->ccs = !ring->ccs; @@ -1043,6 +1049,7 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring) bool ccs = ring->ccs; /* hack to bundle together the two/three TDs that make a setup transfer */ bool control_td_set = 0; + uint32_t link_cnt = 0; while (1) { TRBType type; @@ -1058,6 +1065,9 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring) type = TRB_TYPE(trb); if (type == TR_LINK) { + if (++link_cnt > TRB_LINK_LIMIT) { + return -length; + } dequeue = xhci_mask64(trb.parameter); if (trb.control & TRB_LK_TC) { ccs = !ccs; From 1fe163feeb31cbd20e2ace071f34141892c8e06b Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Sep 2016 10:32:46 +0200 Subject: [PATCH 02/11] xhci: decouple EV_QUEUE from TD_QUEUE EV_QUEUE must not change because an array of that size is part of live migration data. Hard-code current value there, so we can touch TD_QUEUE without breaking live migration. Signed-off-by: Gerd Hoffmann Message-id: 1474965172-30321-3-git-send-email-kraxel@redhat.com --- 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 ee4fa484d6..d9ac1b4be3 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -49,7 +49,7 @@ #define TD_QUEUE 24 /* Very pessimistic, let's hope it's enough for all cases */ -#define EV_QUEUE (((3*TD_QUEUE)+16)*MAXSLOTS) +#define EV_QUEUE (((3 * 24) + 16) * MAXSLOTS) /* Do not deliver ER Full events. NEC's driver does some things not bound * to the specs when it gets them */ #define ER_FULL_HACK From 7512b13dd7f77c3e93a5b856eddf78378bddcc7f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Sep 2016 10:32:47 +0200 Subject: [PATCH 03/11] xhci: drop unused comp_xfer field Signed-off-by: Gerd Hoffmann Message-id: 1474965172-30321-4-git-send-email-kraxel@redhat.com --- hw/usb/hcd-xhci.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index d9ac1b4be3..3a035c8177 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -386,7 +386,6 @@ struct XHCIEPContext { XHCIRing ring; unsigned int next_xfer; - unsigned int comp_xfer; XHCITransfer transfers[TD_QUEUE]; XHCITransfer *retry; EPType type; From 94b037f2a451b3dc855f9f2c346e5049a361bd55 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Sep 2016 10:32:48 +0200 Subject: [PATCH 04/11] xhci: use linked list for transfers xhci has a fixed number of 24 (TD_QUEUE) XHCITransfer structs per endpoint, which turns out to be a problem for usb3 devices with 32 (or more) bulk streams. xhci re-checks the trb rings on every finished transfer to make sure it'll pick up any pending work. But that scheme breaks in case the first transfer of a ring can't be started because we ran out of XHCITransfer structs already. So remove static XHCITransfer array from XHCIEPContext. Use a linked list instead, and allocate/free XHCITransfer as needed. Add helper functions to allocate & initialize and to cleanup & release XHCITransfer structs. That also simplifies trb management, we never have to realloc XHCITransfer->trbs because we don't reuse XHCITransfer structs any more. New dynamic limit for in-flight xhci transfers per endpoint is number-of-streams + 16. Signed-off-by: Gerd Hoffmann Message-id: 1474965172-30321-5-git-send-email-kraxel@redhat.com --- hw/usb/hcd-xhci.c | 122 ++++++++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 54 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 3a035c8177..e14c2a8164 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -21,6 +21,7 @@ #include "qemu/osdep.h" #include "hw/hw.h" #include "qemu/timer.h" +#include "qemu/queue.h" #include "hw/usb.h" #include "hw/pci/pci.h" #include "hw/pci/msi.h" @@ -46,8 +47,6 @@ #define MAXSLOTS 64 #define MAXINTRS 16 -#define TD_QUEUE 24 - /* Very pessimistic, let's hope it's enough for all cases */ #define EV_QUEUE (((3 * 24) + 16) * MAXSLOTS) /* Do not deliver ER Full events. NEC's driver does some things not bound @@ -346,6 +345,7 @@ typedef struct XHCIPort { typedef struct XHCITransfer { XHCIState *xhci; + XHCIEPContext *epctx; USBPacket packet; QEMUSGList sgl; bool running_async; @@ -361,7 +361,6 @@ typedef struct XHCITransfer { bool timed_xfer; unsigned int trb_count; - unsigned int trb_alloced; XHCITRB *trbs; TRBCCode status; @@ -371,6 +370,8 @@ typedef struct XHCITransfer { unsigned int cur_pkt; uint64_t mfindex_kick; + + QTAILQ_ENTRY(XHCITransfer) next; } XHCITransfer; struct XHCIStreamContext { @@ -385,8 +386,8 @@ struct XHCIEPContext { unsigned int epid; XHCIRing ring; - unsigned int next_xfer; - XHCITransfer transfers[TD_QUEUE]; + uint32_t xfer_count; + QTAILQ_HEAD(, XHCITransfer) transfers; XHCITransfer *retry; EPType type; dma_addr_t pctx; @@ -1370,19 +1371,13 @@ static XHCIEPContext *xhci_alloc_epctx(XHCIState *xhci, unsigned int epid) { XHCIEPContext *epctx; - int i; epctx = g_new0(XHCIEPContext, 1); epctx->xhci = xhci; epctx->slotid = slotid; epctx->epid = epid; - for (i = 0; i < ARRAY_SIZE(epctx->transfers); i++) { - epctx->transfers[i].xhci = xhci; - epctx->transfers[i].slotid = slotid; - epctx->transfers[i].epid = epid; - usb_packet_init(&epctx->transfers[i].packet); - } + QTAILQ_INIT(&epctx->transfers); epctx->kick_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_ep_kick_timer, epctx); return epctx; @@ -1443,6 +1438,41 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid, return CC_SUCCESS; } +static XHCITransfer *xhci_ep_alloc_xfer(XHCIEPContext *epctx, + uint32_t length) +{ + uint32_t limit = epctx->nr_pstreams + 16; + XHCITransfer *xfer; + + if (epctx->xfer_count >= limit) { + return NULL; + } + + xfer = g_new0(XHCITransfer, 1); + xfer->xhci = epctx->xhci; + xfer->epctx = epctx; + xfer->slotid = epctx->slotid; + xfer->epid = epctx->epid; + xfer->trbs = g_new(XHCITRB, length); + xfer->trb_count = length; + usb_packet_init(&xfer->packet); + + QTAILQ_INSERT_TAIL(&epctx->transfers, xfer, next); + epctx->xfer_count++; + + return xfer; +} + +static void xhci_ep_free_xfer(XHCITransfer *xfer) +{ + QTAILQ_REMOVE(&xfer->epctx->transfers, xfer, next); + xfer->epctx->xfer_count--; + + usb_packet_cleanup(&xfer->packet); + g_free(xfer->trbs); + g_free(xfer); +} + static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report) { int killed = 0; @@ -1469,7 +1499,7 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report) g_free(t->trbs); t->trbs = NULL; - t->trb_count = t->trb_alloced = 0; + t->trb_count = 0; return killed; } @@ -1479,7 +1509,8 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, { XHCISlot *slot; XHCIEPContext *epctx; - int i, xferi, killed = 0; + XHCITransfer *xfer; + int killed = 0; USBEndpoint *ep = NULL; assert(slotid >= 1 && slotid <= xhci->numslots); assert(epid >= 1 && epid <= 31); @@ -1494,14 +1525,16 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, epctx = slot->eps[epid-1]; - xferi = epctx->next_xfer; - for (i = 0; i < TD_QUEUE; i++) { - killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi], report); + for (;;) { + xfer = QTAILQ_FIRST(&epctx->transfers); + if (xfer == NULL) { + break; + } + killed += xhci_ep_nuke_one_xfer(xfer, report); if (killed) { report = 0; /* Only report once */ } - epctx->transfers[xferi].packet.ep = NULL; - xferi = (xferi + 1) % TD_QUEUE; + xhci_ep_free_xfer(xfer); } ep = xhci_epid_to_usbep(xhci, slotid, epid); @@ -1516,7 +1549,6 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, { XHCISlot *slot; XHCIEPContext *epctx; - int i; trace_usb_xhci_ep_disable(slotid, epid); assert(slotid >= 1 && slotid <= xhci->numslots); @@ -1537,10 +1569,6 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, xhci_free_streams(epctx); } - for (i = 0; i < ARRAY_SIZE(epctx->transfers); i++) { - usb_packet_cleanup(&epctx->transfers[i].packet); - } - /* only touch guest RAM if we're not resetting the HC */ if (xhci->dcbaap_low || xhci->dcbaap_high) { xhci_set_ep_state(xhci, epctx, NULL, EP_DISABLED); @@ -2104,6 +2132,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, { XHCIStreamContext *stctx; XHCIEPContext *epctx; + XHCITransfer *xfer; XHCIRing *ring; USBEndpoint *ep = NULL; uint64_t mfindex; @@ -2168,6 +2197,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, xhci_complete_packet(xfer); } assert(!xfer->running_retry); + xhci_ep_free_xfer(epctx->retry); epctx->retry = NULL; } @@ -2193,27 +2223,14 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, assert(ring->dequeue != 0); while (1) { - XHCITransfer *xfer = &epctx->transfers[epctx->next_xfer]; - if (xfer->running_async || xfer->running_retry) { - break; - } length = xhci_ring_chain_length(xhci, ring); - if (length < 0) { - break; - } else if (length == 0) { + if (length <= 0) { break; } - if (xfer->trbs && xfer->trb_alloced < length) { - xfer->trb_count = 0; - xfer->trb_alloced = 0; - g_free(xfer->trbs); - xfer->trbs = NULL; + xfer = xhci_ep_alloc_xfer(epctx, length); + if (xfer == NULL) { + break; } - if (!xfer->trbs) { - xfer->trbs = g_new(XHCITRB, length); - xfer->trb_alloced = length; - } - xfer->trb_count = length; for (i = 0; i < length; i++) { TRBType type; @@ -2223,25 +2240,19 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, xfer->streamid = streamid; if (epid == 1) { - if (xhci_fire_ctl_transfer(xhci, xfer) >= 0) { - epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; - } else { - DPRINTF("xhci: error firing CTL transfer\n"); - } + xhci_fire_ctl_transfer(xhci, xfer); } else { - if (xhci_fire_transfer(xhci, xfer, epctx) >= 0) { - epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; - } else { - if (!xfer->timed_xfer) { - DPRINTF("xhci: error firing data transfer\n"); - } - } + xhci_fire_transfer(xhci, xfer, epctx); + } + if (xfer->complete) { + xhci_ep_free_xfer(xfer); + xfer = NULL; } if (epctx->state == EP_HALTED) { break; } - if (xfer->running_retry) { + if (xfer != NULL && xfer->running_retry) { DPRINTF("xhci: xfer nacked, stopping schedule\n"); epctx->retry = xfer; break; @@ -3480,6 +3491,9 @@ static void xhci_complete(USBPort *port, USBPacket *packet) } xhci_complete_packet(xfer); xhci_kick_ep(xfer->xhci, xfer->slotid, xfer->epid, xfer->streamid); + if (xfer->complete) { + xhci_ep_free_xfer(xfer); + } } static void xhci_child_detach(USBPort *uport, USBDevice *child) From 5612564ea9cf5b9636438a1b58ae9a2ab6ca16ae Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Sep 2016 10:32:49 +0200 Subject: [PATCH 05/11] xhci: drop XHCITransfer->xhci Use XHCITransfer->epctx->xhci instead. Signed-off-by: Gerd Hoffmann Message-id: 1474965172-30321-6-git-send-email-kraxel@redhat.com --- hw/usb/hcd-xhci.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index e14c2a8164..20f46c4496 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -344,7 +344,6 @@ typedef struct XHCIPort { } XHCIPort; typedef struct XHCITransfer { - XHCIState *xhci; XHCIEPContext *epctx; USBPacket packet; QEMUSGList sgl; @@ -1449,7 +1448,6 @@ static XHCITransfer *xhci_ep_alloc_xfer(XHCIEPContext *epctx, } xfer = g_new0(XHCITransfer, 1); - xfer->xhci = epctx->xhci; xfer->epctx = epctx; xfer->slotid = epctx->slotid; xfer->epid = epctx->epid; @@ -1488,10 +1486,9 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report) killed = 1; } if (t->running_retry) { - XHCIEPContext *epctx = t->xhci->slots[t->slotid-1].eps[t->epid-1]; - if (epctx) { - epctx->retry = NULL; - timer_del(epctx->kick_timer); + if (t->epctx) { + t->epctx->retry = NULL; + timer_del(t->epctx->kick_timer); } t->running_retry = 0; killed = 1; @@ -1721,7 +1718,7 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid, static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer) { - XHCIState *xhci = xfer->xhci; + XHCIState *xhci = xfer->epctx->xhci; int i; xfer->int_req = false; @@ -1780,7 +1777,7 @@ static void xhci_xfer_report(XHCITransfer *xfer) bool reported = 0; bool shortpkt = 0; XHCIEvent event = {ER_TRANSFER, CC_SUCCESS}; - XHCIState *xhci = xfer->xhci; + XHCIState *xhci = xfer->epctx->xhci; int i; left = xfer->packet.actual_length; @@ -1854,9 +1851,8 @@ static void xhci_xfer_report(XHCITransfer *xfer) static void xhci_stall_ep(XHCITransfer *xfer) { - XHCIState *xhci = xfer->xhci; - XHCISlot *slot = &xhci->slots[xfer->slotid-1]; - XHCIEPContext *epctx = slot->eps[xfer->epid-1]; + XHCIEPContext *epctx = xfer->epctx; + XHCIState *xhci = epctx->xhci; uint32_t err; XHCIStreamContext *sctx; @@ -1880,7 +1876,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, static int xhci_setup_packet(XHCITransfer *xfer) { - XHCIState *xhci = xfer->xhci; + XHCIState *xhci = xfer->epctx->xhci; USBEndpoint *ep; int dir; @@ -3490,7 +3486,7 @@ static void xhci_complete(USBPort *port, USBPacket *packet) return; } xhci_complete_packet(xfer); - xhci_kick_ep(xfer->xhci, xfer->slotid, xfer->epid, xfer->streamid); + xhci_kick_ep(xfer->epctx->xhci, xfer->slotid, xfer->epid, xfer->streamid); if (xfer->complete) { xhci_ep_free_xfer(xfer); } From 3a533ee8fda6457ddc85d3c5dfeff037a808fcb3 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Sep 2016 10:32:50 +0200 Subject: [PATCH 06/11] xhci: add & use xhci_kick_epctx() xhci_kick_epctx is a xhci_kick_ep variant which takes an XHCIEPContext as input instead of slotid and epid. So in case we have a XHCIEPContext at hand at the callsite we can just pass it directly. Signed-off-by: Gerd Hoffmann Message-id: 1474965172-30321-7-git-send-email-kraxel@redhat.com --- hw/usb/hcd-xhci.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 20f46c4496..c82fe091d1 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -509,6 +509,7 @@ enum xhci_flags { static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid, unsigned int streamid); +static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid); static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid); static void xhci_xfer_report(XHCITransfer *xfer); @@ -1362,7 +1363,7 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx, static void xhci_ep_kick_timer(void *opaque) { XHCIEPContext *epctx = opaque; - xhci_kick_ep(epctx->xhci, epctx->slotid, epctx->epid, 0); + xhci_kick_epctx(epctx, 0); } static XHCIEPContext *xhci_alloc_epctx(XHCIState *xhci, @@ -2008,7 +2009,7 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) xhci_complete_packet(xfer); if (!xfer->running_async && !xfer->running_retry) { - xhci_kick_ep(xhci, xfer->slotid, xfer->epid, 0); + xhci_kick_epctx(xfer->epctx, 0); } return 0; } @@ -2112,7 +2113,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx xhci_complete_packet(xfer); if (!xfer->running_async && !xfer->running_retry) { - xhci_kick_ep(xhci, xfer->slotid, xfer->epid, xfer->streamid); + xhci_kick_epctx(xfer->epctx, xfer->streamid); } return 0; } @@ -2126,16 +2127,8 @@ static int xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid, unsigned int streamid) { - XHCIStreamContext *stctx; XHCIEPContext *epctx; - XHCITransfer *xfer; - XHCIRing *ring; - USBEndpoint *ep = NULL; - uint64_t mfindex; - int length; - int i; - trace_usb_xhci_ep_kick(slotid, epid, streamid); assert(slotid >= 1 && slotid <= xhci->numslots); assert(epid >= 1 && epid <= 31); @@ -2150,11 +2143,27 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, return; } + xhci_kick_epctx(epctx, streamid); +} + +static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid) +{ + XHCIState *xhci = epctx->xhci; + XHCIStreamContext *stctx; + XHCITransfer *xfer; + XHCIRing *ring; + USBEndpoint *ep = NULL; + uint64_t mfindex; + int length; + int i; + + trace_usb_xhci_ep_kick(epctx->slotid, epctx->epid, streamid); + /* If the device has been detached, but the guest has not noticed this yet the 2 above checks will succeed, but we must NOT continue */ - if (!xhci->slots[slotid - 1].uport || - !xhci->slots[slotid - 1].uport->dev || - !xhci->slots[slotid - 1].uport->dev->attached) { + if (!xhci->slots[epctx->slotid - 1].uport || + !xhci->slots[epctx->slotid - 1].uport->dev || + !xhci->slots[epctx->slotid - 1].uport->dev->attached) { return; } @@ -2235,7 +2244,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, } xfer->streamid = streamid; - if (epid == 1) { + if (epctx->epid == 1) { xhci_fire_ctl_transfer(xhci, xfer); } else { xhci_fire_transfer(xhci, xfer, epctx); @@ -2255,7 +2264,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, } } - ep = xhci_epid_to_usbep(xhci, slotid, epid); + ep = xhci_epid_to_usbep(xhci, epctx->slotid, epctx->epid); if (ep) { usb_device_flush_ep_queue(ep->dev, ep); } @@ -3486,7 +3495,7 @@ static void xhci_complete(USBPort *port, USBPacket *packet) return; } xhci_complete_packet(xfer); - xhci_kick_ep(xfer->epctx->xhci, xfer->slotid, xfer->epid, xfer->streamid); + xhci_kick_epctx(xfer->epctx, xfer->streamid); if (xfer->complete) { xhci_ep_free_xfer(xfer); } From d6fcb2936f9bacf3ad696e957e229ee2aadc0d0d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Sep 2016 10:32:51 +0200 Subject: [PATCH 07/11] xhci: drop XHCITransfer->{slotid,epid} We can use XHCITransfer->epctx->{slotid,epid} instead. Signed-off-by: Gerd Hoffmann Message-id: 1474965172-30321-8-git-send-email-kraxel@redhat.com --- hw/usb/hcd-xhci.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index c82fe091d1..80840d3e20 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -352,8 +352,6 @@ typedef struct XHCITransfer { bool complete; bool int_req; unsigned int iso_pkts; - unsigned int slotid; - unsigned int epid; unsigned int streamid; bool in_xfer; bool iso_xfer; @@ -1450,8 +1448,6 @@ static XHCITransfer *xhci_ep_alloc_xfer(XHCIEPContext *epctx, xfer = g_new0(XHCITransfer, 1); xfer->epctx = epctx; - xfer->slotid = epctx->slotid; - xfer->epid = epctx->epid; xfer->trbs = g_new(XHCITRB, length); xfer->trb_count = length; usb_packet_init(&xfer->packet); @@ -1816,8 +1812,8 @@ static void xhci_xfer_report(XHCITransfer *xfer) if (!reported && ((trb->control & TRB_TR_IOC) || (shortpkt && (trb->control & TRB_TR_ISP)) || (xfer->status != CC_SUCCESS && left == 0))) { - event.slotid = xfer->slotid; - event.epid = xfer->epid; + event.slotid = xfer->epctx->slotid; + event.epid = xfer->epctx->epid; event.length = (trb->status & 0x1ffff) - chunk; event.flags = 0; event.ptr = trb->addr; @@ -1886,7 +1882,7 @@ static int xhci_setup_packet(XHCITransfer *xfer) if (xfer->packet.ep) { ep = xfer->packet.ep; } else { - ep = xhci_epid_to_usbep(xhci, xfer->slotid, xfer->epid); + ep = xhci_epid_to_usbep(xhci, xfer->epctx->slotid, xfer->epctx->epid); if (!ep) { DPRINTF("xhci: slot %d has no device\n", xfer->slotid); @@ -1966,7 +1962,8 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) trb_setup = &xfer->trbs[0]; trb_status = &xfer->trbs[xfer->trb_count-1]; - trace_usb_xhci_xfer_start(xfer, xfer->slotid, xfer->epid, xfer->streamid); + trace_usb_xhci_xfer_start(xfer, xfer->epctx->slotid, + xfer->epctx->epid, xfer->streamid); /* at most one Event Data TRB allowed after STATUS */ if (TRB_TYPE(*trb_status) == TR_EVDATA && xfer->trb_count > 2) { @@ -2120,7 +2117,8 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx static int xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx) { - trace_usb_xhci_xfer_start(xfer, xfer->slotid, xfer->epid, xfer->streamid); + trace_usb_xhci_xfer_start(xfer, xfer->epctx->slotid, + xfer->epctx->epid, xfer->streamid); return xhci_submit(xhci, xfer, epctx); } From 070eeef9e0821fbaeda7002de4ee61b5b4015fa6 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Sep 2016 10:32:52 +0200 Subject: [PATCH 08/11] xhci: make xhci_epid_to_usbep accept XHCIEPContext All callsites have a XHCIEPContext pointer anyway, so we can just pass it directly instead of fiddeling with slotid and epid. Signed-off-by: Gerd Hoffmann Message-id: 1474965172-30321-9-git-send-email-kraxel@redhat.com --- hw/usb/hcd-xhci.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 80840d3e20..4acf0c6dd8 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -513,8 +513,7 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, static void xhci_xfer_report(XHCITransfer *xfer); static void xhci_event(XHCIState *xhci, XHCIEvent *event, int v); static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v); -static USBEndpoint *xhci_epid_to_usbep(XHCIState *xhci, - unsigned int slotid, unsigned int epid); +static USBEndpoint *xhci_epid_to_usbep(XHCIEPContext *epctx); static const char *TRBType_names[] = { [TRB_RESERVED] = "TRB_RESERVED", @@ -1200,7 +1199,7 @@ static int xhci_epmask_to_eps_with_streams(XHCIState *xhci, } epctx = slot->eps[i - 1]; - ep = xhci_epid_to_usbep(xhci, slotid, i); + ep = xhci_epid_to_usbep(epctx); if (!epctx || !epctx->nr_pstreams || !ep) { continue; } @@ -1531,7 +1530,7 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, xhci_ep_free_xfer(xfer); } - ep = xhci_epid_to_usbep(xhci, slotid, epid); + ep = xhci_epid_to_usbep(epctx); if (ep) { usb_device_ep_stopped(ep->dev, ep); } @@ -1873,7 +1872,6 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, static int xhci_setup_packet(XHCITransfer *xfer) { - XHCIState *xhci = xfer->epctx->xhci; USBEndpoint *ep; int dir; @@ -1882,7 +1880,7 @@ static int xhci_setup_packet(XHCITransfer *xfer) if (xfer->packet.ep) { ep = xfer->packet.ep; } else { - ep = xhci_epid_to_usbep(xhci, xfer->epctx->slotid, xfer->epctx->epid); + ep = xhci_epid_to_usbep(xfer->epctx); if (!ep) { DPRINTF("xhci: slot %d has no device\n", xfer->slotid); @@ -2262,7 +2260,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid) } } - ep = xhci_epid_to_usbep(xhci, epctx->slotid, epctx->epid); + ep = xhci_epid_to_usbep(epctx); if (ep) { usb_device_flush_ep_queue(ep->dev, ep); } @@ -3527,17 +3525,20 @@ static int xhci_find_epid(USBEndpoint *ep) } } -static USBEndpoint *xhci_epid_to_usbep(XHCIState *xhci, - unsigned int slotid, unsigned int epid) +static USBEndpoint *xhci_epid_to_usbep(XHCIEPContext *epctx) { - assert(slotid >= 1 && slotid <= xhci->numslots); + USBPort *uport; + uint32_t token; - if (!xhci->slots[slotid - 1].uport) { + if (!epctx) { return NULL; } - - return usb_ep_get(xhci->slots[slotid - 1].uport->dev, - (epid & 1) ? USB_TOKEN_IN : USB_TOKEN_OUT, epid >> 1); + uport = epctx->xhci->slots[epctx->slotid - 1].uport; + token = (epctx->epid & 1) ? USB_TOKEN_IN : USB_TOKEN_OUT; + if (!uport) { + return NULL; + } + return usb_ep_get(uport->dev, token, epctx->epid >> 1); } static void xhci_wakeup_endpoint(USBBus *bus, USBEndpoint *ep, From 0136464d10f1fd9393a8125f2c552ef24f3e592c Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 5 Oct 2016 11:33:18 +0200 Subject: [PATCH 09/11] usb: fix serial generator snprintf return value is *not* the number of chars written into the buffer, but the number of chars needed. So in case the buffer is too small you can go alloc a bigger one and try again. But that also means you can't simply use the return value for the next snprintf call without checking beforehand that things did actually fit. Problem is that usb_desc_create_serial didn't perform that check, so a loooong path string (can happen with deep pci-bridge nesting) results in the third snprintf call smashing the stack. Fix this by throwing out all the snpintf calls and use g_strdup_printf instead. https://bugzilla.redhat.com/show_bug.cgi?id=1381630 Reported-by: Thomas Huth Signed-off-by: Gerd Hoffmann Reviewed-by: Thomas Huth Reviewed-by: Eric Blake Message-id: 1475659998-22045-1-git-send-email-kraxel@redhat.com --- hw/usb/desc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/usb/desc.c b/hw/usb/desc.c index 5e0e1d157e..7828e52c6f 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -556,9 +556,7 @@ void usb_desc_create_serial(USBDevice *dev) DeviceState *hcd = dev->qdev.parent_bus->parent; const USBDesc *desc = usb_device_get_usb_desc(dev); int index = desc->id.iSerialNumber; - char serial[64]; - char *path; - int dst; + char *path, *serial; if (dev->serial) { /* 'serial' usb bus property has priority if present */ @@ -567,14 +565,16 @@ void usb_desc_create_serial(USBDevice *dev) } assert(index != 0 && desc->str[index] != NULL); - dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]); path = qdev_get_dev_path(hcd); if (path) { - dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path); + serial = g_strdup_printf("%s-%s-%s", desc->str[index], + path, dev->port->path); + } else { + serial = g_strdup_printf("%s-%s", desc->str[index], dev->port->path); } - dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path); usb_desc_set_string(dev, index, serial); g_free(path); + g_free(serial); } const char *usb_desc_get_string(USBDevice *dev, uint8_t index) From 6998b6c7c77e22d386fb8792365e668351d22f91 Mon Sep 17 00:00:00 2001 From: Vijay Kumar B Date: Wed, 28 Sep 2016 16:39:18 +0530 Subject: [PATCH 10/11] usb: Fix incorrect default DMA offset. The default DMA offset is set to 3. When the property is not set by the consumer, the default causes DMA access to be shifted by 3 bytes. In PXA, this results in incorrect DMA access, leading to error notification in the USB controller driver. A better default would be 0, so that there is no offset, when the consumer does not specify one. Signed-off-by: Vijay Kumar B. Reviewed-by: Deepak S. Message-id: 1475060958-7760-1-git-send-email-vijaykumar@zilogic.com Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index fa5703832c..c82a92fff7 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -2139,7 +2139,7 @@ static const TypeInfo ohci_pci_info = { static Property ohci_sysbus_properties[] = { DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3), - DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 3), + DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 0), DEFINE_PROP_END_OF_LIST(), }; From d5c42857d6b0c35028897df8dfc3749eba6f6de3 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 10 Oct 2016 12:45:13 +0200 Subject: [PATCH 11/11] usb-redir: allocate buffers before waking up the host adapter Needed to make sure usb redirection is prepared to actually handle the callback from the usb host adapter. Without this interrupt endpoints don't work on xhci. Note: On ehci the usb_wakeup() call only schedules a BH for the actual work, which hides this bug because the allocation happens before ehci calls back even without this patch. Signed-off-by: Hans de Goede Message-id: 1476096313-7730-1-git-send-email-kraxel@redhat.com Signed-off-by: Gerd Hoffmann --- hw/usb/redirect.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 444672a000..d4ca026f00 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -2036,18 +2036,22 @@ static void usbredir_interrupt_packet(void *priv, uint64_t id, } if (ep & USB_DIR_IN) { + bool q_was_empty; + if (dev->endpoint[EP2I(ep)].interrupt_started == 0) { DPRINTF("received int packet while not started ep %02X\n", ep); free(data); return; } - if (QTAILQ_EMPTY(&dev->endpoint[EP2I(ep)].bufpq)) { - usb_wakeup(usb_ep_get(&dev->dev, USB_TOKEN_IN, ep & 0x0f), 0); - } + q_was_empty = QTAILQ_EMPTY(&dev->endpoint[EP2I(ep)].bufpq); /* bufp_alloc also adds the packet to the ep queue */ bufp_alloc(dev, data, data_len, interrupt_packet->status, ep, data); + + if (q_was_empty) { + usb_wakeup(usb_ep_get(&dev->dev, USB_TOKEN_IN, ep & 0x0f), 0); + } } else { /* * We report output interrupt packets as completed directly upon