* [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs @ 2012-11-02 2:32 David Gibson 2012-11-02 7:31 ` Gerd Hoffmann 0 siblings, 1 reply; 9+ messages in thread From: David Gibson @ 2012-11-02 2:32 UTC (permalink / raw) To: kraxel; +Cc: aliguori, qemu-devel, David Gibson The xhci device does correct endian switches on the results of some DMAs but not all. In particular, there are many DMAs of what are essentially arrays of 32-bit integers which never get byteswapped. This causes them to be interpreted incorrectly on big-endian hosts, since (as per the xhci spec) these arrays are always little-endian in guest memory. This patch adds some helper functions to fix these bugs. This may not be all the endian bugs in the xhci code, but it's certainly some of them and the Linux guest xhci driver certainly gets further with these fixes. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/usb/hcd-xhci.c | 76 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 27 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 7b65741..f1af6d2 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -607,6 +607,29 @@ static inline dma_addr_t xhci_mask64(uint64_t addr) } } +static inline void xhci_dma_read_u32s(XHCIState *xhci, dma_addr_t addr, + uint32_t *buf, size_t len) +{ + int i; + + pci_dma_read(&xhci->pci_dev, addr, buf, len); + + for (i = 0; i < (len / sizeof(uint32_t)); i++) { + buf[i] = le32_to_cpu(buf[i]); + } +} + +static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr, + uint32_t *buf, size_t len) +{ + int i; + + for (i = 0; i < (len / sizeof(uint32_t)); i++) { + buf[i] = cpu_to_le32(buf[i]); + } + pci_dma_write(&xhci->pci_dev, addr, buf, len); +} + static XHCIPort *xhci_lookup_port(XHCIState *xhci, struct USBPort *uport) { int index; @@ -1018,14 +1041,14 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx, { uint32_t ctx[5]; - pci_dma_read(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx)); + xhci_dma_read_u32s(xhci, epctx->pctx, ctx, sizeof(ctx)); ctx[0] &= ~EP_STATE_MASK; ctx[0] |= state; ctx[2] = epctx->ring.dequeue | epctx->ring.ccs; ctx[3] = (epctx->ring.dequeue >> 16) >> 16; DPRINTF("xhci: set epctx: " DMA_ADDR_FMT " state=%d dequeue=%08x%08x\n", epctx->pctx, state, ctx[3], ctx[2]); - pci_dma_write(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx)); + xhci_dma_write_u32s(xhci, epctx->pctx, ctx, sizeof(ctx)); epctx->state = state; } @@ -1857,14 +1880,14 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, 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)); + poctx = ldq_le_pci_dma(&xhci->pci_dev, dcbaap + 8*slotid); ictx = xhci_mask64(pictx); - octx = xhci_mask64(le64_to_cpu(poctx)); + octx = xhci_mask64(poctx); DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx); DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx); - pci_dma_read(&xhci->pci_dev, ictx, ictl_ctx, sizeof(ictl_ctx)); + xhci_dma_read_u32s(xhci, ictx, ictl_ctx, sizeof(ictl_ctx)); if (ictl_ctx[0] != 0x0 || ictl_ctx[1] != 0x3) { fprintf(stderr, "xhci: invalid input context control %08x %08x\n", @@ -1872,8 +1895,8 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, return CC_TRB_ERROR; } - pci_dma_read(&xhci->pci_dev, ictx+32, slot_ctx, sizeof(slot_ctx)); - pci_dma_read(&xhci->pci_dev, ictx+64, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_read_u32s(xhci, ictx+32, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, ictx+64, ep0_ctx, sizeof(ep0_ctx)); DPRINTF("xhci: input slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); @@ -1923,8 +1946,8 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output ep0 context: %08x %08x %08x %08x %08x\n", ep0_ctx[0], ep0_ctx[1], ep0_ctx[2], ep0_ctx[3], ep0_ctx[4]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); - pci_dma_write(&xhci->pci_dev, octx+32, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx+32, ep0_ctx, sizeof(ep0_ctx)); return res; } @@ -1957,17 +1980,17 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, } } - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); slot_ctx[3] &= ~(SLOT_STATE_MASK << SLOT_STATE_SHIFT); slot_ctx[3] |= SLOT_ADDRESSED << SLOT_STATE_SHIFT; DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); return CC_SUCCESS; } - pci_dma_read(&xhci->pci_dev, ictx, ictl_ctx, sizeof(ictl_ctx)); + xhci_dma_read_u32s(xhci, ictx, ictl_ctx, sizeof(ictl_ctx)); if ((ictl_ctx[0] & 0x3) != 0x0 || (ictl_ctx[1] & 0x3) != 0x1) { fprintf(stderr, "xhci: invalid input context control %08x %08x\n", @@ -1975,8 +1998,8 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, return CC_TRB_ERROR; } - pci_dma_read(&xhci->pci_dev, ictx+32, islot_ctx, sizeof(islot_ctx)); - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, ictx+32, islot_ctx, sizeof(islot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); if (SLOT_STATE(slot_ctx[3]) < SLOT_ADDRESSED) { fprintf(stderr, "xhci: invalid slot state %08x\n", slot_ctx[3]); @@ -1988,8 +2011,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, xhci_disable_ep(xhci, slotid, i); } if (ictl_ctx[1] & (1<<i)) { - pci_dma_read(&xhci->pci_dev, ictx+32+(32*i), ep_ctx, - sizeof(ep_ctx)); + xhci_dma_read_u32s(xhci, ictx+32+(32*i), ep_ctx, sizeof(ep_ctx)); DPRINTF("xhci: input ep%d.%d context: %08x %08x %08x %08x %08x\n", i/2, i%2, ep_ctx[0], ep_ctx[1], ep_ctx[2], ep_ctx[3], ep_ctx[4]); @@ -2001,7 +2023,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output ep%d.%d context: %08x %08x %08x %08x %08x\n", i/2, i%2, ep_ctx[0], ep_ctx[1], ep_ctx[2], ep_ctx[3], ep_ctx[4]); - pci_dma_write(&xhci->pci_dev, octx+(32*i), ep_ctx, sizeof(ep_ctx)); + xhci_dma_write_u32s(xhci, octx+(32*i), ep_ctx, sizeof(ep_ctx)); } } @@ -2013,7 +2035,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); return CC_SUCCESS; } @@ -2038,7 +2060,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx); DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx); - pci_dma_read(&xhci->pci_dev, ictx, ictl_ctx, sizeof(ictl_ctx)); + xhci_dma_read_u32s(xhci, ictx, ictl_ctx, sizeof(ictl_ctx)); if (ictl_ctx[0] != 0x0 || ictl_ctx[1] & ~0x3) { fprintf(stderr, "xhci: invalid input context control %08x %08x\n", @@ -2047,12 +2069,12 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, } if (ictl_ctx[1] & 0x1) { - pci_dma_read(&xhci->pci_dev, ictx+32, islot_ctx, sizeof(islot_ctx)); + xhci_dma_read_u32s(xhci, ictx+32, islot_ctx, sizeof(islot_ctx)); DPRINTF("xhci: input slot context: %08x %08x %08x %08x\n", islot_ctx[0], islot_ctx[1], islot_ctx[2], islot_ctx[3]); - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); slot_ctx[1] &= ~0xFFFF; /* max exit latency */ slot_ctx[1] |= islot_ctx[1] & 0xFFFF; @@ -2062,17 +2084,17 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); } if (ictl_ctx[1] & 0x2) { - pci_dma_read(&xhci->pci_dev, ictx+64, iep0_ctx, sizeof(iep0_ctx)); + xhci_dma_read_u32s(xhci, ictx+64, iep0_ctx, sizeof(iep0_ctx)); DPRINTF("xhci: input ep0 context: %08x %08x %08x %08x %08x\n", iep0_ctx[0], iep0_ctx[1], iep0_ctx[2], iep0_ctx[3], iep0_ctx[4]); - pci_dma_read(&xhci->pci_dev, octx+32, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_read_u32s(xhci, octx+32, ep0_ctx, sizeof(ep0_ctx)); ep0_ctx[1] &= ~0xFFFF0000; /* max packet size*/ ep0_ctx[1] |= iep0_ctx[1] & 0xFFFF0000; @@ -2080,7 +2102,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output ep0 context: %08x %08x %08x %08x %08x\n", ep0_ctx[0], ep0_ctx[1], ep0_ctx[2], ep0_ctx[3], ep0_ctx[4]); - pci_dma_write(&xhci->pci_dev, octx+32, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_write_u32s(xhci, octx+32, ep0_ctx, sizeof(ep0_ctx)); } return CC_SUCCESS; @@ -2105,12 +2127,12 @@ static TRBCCode xhci_reset_slot(XHCIState *xhci, unsigned int slotid) } } - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); slot_ctx[3] &= ~(SLOT_STATE_MASK << SLOT_STATE_SHIFT); slot_ctx[3] |= SLOT_DEFAULT << SLOT_STATE_SHIFT; DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); return CC_SUCCESS; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs 2012-11-02 2:32 [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs David Gibson @ 2012-11-02 7:31 ` Gerd Hoffmann 2012-11-02 8:43 ` David Gibson 2012-11-05 4:20 ` David Gibson 0 siblings, 2 replies; 9+ messages in thread From: Gerd Hoffmann @ 2012-11-02 7:31 UTC (permalink / raw) To: David Gibson; +Cc: aliguori, qemu-devel Hi, > +static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr, > + uint32_t *buf, size_t len) > +{ > + int i; > + > + for (i = 0; i < (len / sizeof(uint32_t)); i++) { > + buf[i] = cpu_to_le32(buf[i]); > + } > + pci_dma_write(&xhci->pci_dev, addr, buf, len); > +} I think we should use a temporary buffer here, otherwise you leave the values byteswapped in buf which likely has unwanted side effects. cheers, Gerd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs 2012-11-02 7:31 ` Gerd Hoffmann @ 2012-11-02 8:43 ` David Gibson 2012-11-05 4:20 ` David Gibson 1 sibling, 0 replies; 9+ messages in thread From: David Gibson @ 2012-11-02 8:43 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: aliguori, qemu-devel On Fri, Nov 02, 2012 at 08:31:23AM +0100, Gerd Hoffmann wrote: > Hi, > > > +static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr, > > + uint32_t *buf, size_t len) > > +{ > > + int i; > > + > > + for (i = 0; i < (len / sizeof(uint32_t)); i++) { > > + buf[i] = cpu_to_le32(buf[i]); > > + } > > + pci_dma_write(&xhci->pci_dev, addr, buf, len); > > +} > > I think we should use a temporary buffer here, otherwise you leave the > values byteswapped in buf which likely has unwanted side effects. Yeah, I wondered about that. I did check that the side-effect doesn't matter in any of the existing callers, so I left it that way to avoid the extra copies. But you're right, it's a pretty subtle constraint that could easily be broken by future changes. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs 2012-11-02 7:31 ` Gerd Hoffmann 2012-11-02 8:43 ` David Gibson @ 2012-11-05 4:20 ` David Gibson 2012-11-05 10:20 ` Gerd Hoffmann 1 sibling, 1 reply; 9+ messages in thread From: David Gibson @ 2012-11-05 4:20 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: aliguori, qemu-devel On Fri, Nov 02, 2012 at 08:31:23AM +0100, Gerd Hoffmann wrote: > Hi, > > > +static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr, > > + uint32_t *buf, size_t len) > > +{ > > + int i; > > + > > + for (i = 0; i < (len / sizeof(uint32_t)); i++) { > > + buf[i] = cpu_to_le32(buf[i]); > > + } > > + pci_dma_write(&xhci->pci_dev, addr, buf, len); > > +} > > I think we should use a temporary buffer here, otherwise you leave the > values byteswapped in buf which likely has unwanted side effects. Here's an updated version that uses a temporary buffer. >From 588a8f874c8d5a658ef95e35164e182a915091db Mon Sep 17 00:00:00 2001 From: David Gibson <david@gibson.dropbear.id.au> Date: Mon, 5 Nov 2012 14:29:01 +1100 Subject: [PATCH] xhci: Fix some DMA host endian bugs The xhci device does correct endian switches on the results of some DMAs but not all. In particular, there are many DMAs of what are essentially arrays of 32-bit integers which never get byteswapped. This causes them to be interpreted incorrectly on big-endian hosts, since (as per the xhci spec) these arrays are always little-endian in guest memory. This patch adds some helper functions to fix these bugs. This may not be all the endian bugs in the xhci code, but it's certainly some of them and the Linux guest xhci driver certainly gets further with these fixes. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- v2: * Change xhci_dma_write_u32s to use a temporary buffer rather than altering the given data in place. --- hw/usb/hcd-xhci.c | 81 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 27 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 7b65741..1465685 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -607,6 +607,34 @@ static inline dma_addr_t xhci_mask64(uint64_t addr) } } +static inline void xhci_dma_read_u32s(XHCIState *xhci, dma_addr_t addr, + uint32_t *buf, size_t len) +{ + int i; + + assert((len % sizeof(uint32_t)) == 0); + + pci_dma_read(&xhci->pci_dev, addr, buf, len); + + for (i = 0; i < (len / sizeof(uint32_t)); i++) { + buf[i] = le32_to_cpu(buf[i]); + } +} + +static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr, + uint32_t *buf, size_t len) +{ + int i; + uint32_t tmp[len / sizeof(uint32_t)]; + + assert((len % sizeof(uint32_t)) == 0); + + for (i = 0; i < (len / sizeof(uint32_t)); i++) { + tmp[i] = cpu_to_le32(buf[i]); + } + pci_dma_write(&xhci->pci_dev, addr, tmp, len); +} + static XHCIPort *xhci_lookup_port(XHCIState *xhci, struct USBPort *uport) { int index; @@ -1018,14 +1046,14 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx, { uint32_t ctx[5]; - pci_dma_read(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx)); + xhci_dma_read_u32s(xhci, epctx->pctx, ctx, sizeof(ctx)); ctx[0] &= ~EP_STATE_MASK; ctx[0] |= state; ctx[2] = epctx->ring.dequeue | epctx->ring.ccs; ctx[3] = (epctx->ring.dequeue >> 16) >> 16; DPRINTF("xhci: set epctx: " DMA_ADDR_FMT " state=%d dequeue=%08x%08x\n", epctx->pctx, state, ctx[3], ctx[2]); - pci_dma_write(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx)); + xhci_dma_write_u32s(xhci, epctx->pctx, ctx, sizeof(ctx)); epctx->state = state; } @@ -1857,14 +1885,14 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, 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)); + poctx = ldq_le_pci_dma(&xhci->pci_dev, dcbaap + 8*slotid); ictx = xhci_mask64(pictx); - octx = xhci_mask64(le64_to_cpu(poctx)); + octx = xhci_mask64(poctx); DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx); DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx); - pci_dma_read(&xhci->pci_dev, ictx, ictl_ctx, sizeof(ictl_ctx)); + xhci_dma_read_u32s(xhci, ictx, ictl_ctx, sizeof(ictl_ctx)); if (ictl_ctx[0] != 0x0 || ictl_ctx[1] != 0x3) { fprintf(stderr, "xhci: invalid input context control %08x %08x\n", @@ -1872,8 +1900,8 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, return CC_TRB_ERROR; } - pci_dma_read(&xhci->pci_dev, ictx+32, slot_ctx, sizeof(slot_ctx)); - pci_dma_read(&xhci->pci_dev, ictx+64, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_read_u32s(xhci, ictx+32, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, ictx+64, ep0_ctx, sizeof(ep0_ctx)); DPRINTF("xhci: input slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); @@ -1923,8 +1951,8 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output ep0 context: %08x %08x %08x %08x %08x\n", ep0_ctx[0], ep0_ctx[1], ep0_ctx[2], ep0_ctx[3], ep0_ctx[4]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); - pci_dma_write(&xhci->pci_dev, octx+32, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx+32, ep0_ctx, sizeof(ep0_ctx)); return res; } @@ -1957,17 +1985,17 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, } } - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); slot_ctx[3] &= ~(SLOT_STATE_MASK << SLOT_STATE_SHIFT); slot_ctx[3] |= SLOT_ADDRESSED << SLOT_STATE_SHIFT; DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); return CC_SUCCESS; } - pci_dma_read(&xhci->pci_dev, ictx, ictl_ctx, sizeof(ictl_ctx)); + xhci_dma_read_u32s(xhci, ictx, ictl_ctx, sizeof(ictl_ctx)); if ((ictl_ctx[0] & 0x3) != 0x0 || (ictl_ctx[1] & 0x3) != 0x1) { fprintf(stderr, "xhci: invalid input context control %08x %08x\n", @@ -1975,8 +2003,8 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, return CC_TRB_ERROR; } - pci_dma_read(&xhci->pci_dev, ictx+32, islot_ctx, sizeof(islot_ctx)); - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, ictx+32, islot_ctx, sizeof(islot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); if (SLOT_STATE(slot_ctx[3]) < SLOT_ADDRESSED) { fprintf(stderr, "xhci: invalid slot state %08x\n", slot_ctx[3]); @@ -1988,8 +2016,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, xhci_disable_ep(xhci, slotid, i); } if (ictl_ctx[1] & (1<<i)) { - pci_dma_read(&xhci->pci_dev, ictx+32+(32*i), ep_ctx, - sizeof(ep_ctx)); + xhci_dma_read_u32s(xhci, ictx+32+(32*i), ep_ctx, sizeof(ep_ctx)); DPRINTF("xhci: input ep%d.%d context: %08x %08x %08x %08x %08x\n", i/2, i%2, ep_ctx[0], ep_ctx[1], ep_ctx[2], ep_ctx[3], ep_ctx[4]); @@ -2001,7 +2028,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output ep%d.%d context: %08x %08x %08x %08x %08x\n", i/2, i%2, ep_ctx[0], ep_ctx[1], ep_ctx[2], ep_ctx[3], ep_ctx[4]); - pci_dma_write(&xhci->pci_dev, octx+(32*i), ep_ctx, sizeof(ep_ctx)); + xhci_dma_write_u32s(xhci, octx+(32*i), ep_ctx, sizeof(ep_ctx)); } } @@ -2013,7 +2040,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); return CC_SUCCESS; } @@ -2038,7 +2065,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx); DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx); - pci_dma_read(&xhci->pci_dev, ictx, ictl_ctx, sizeof(ictl_ctx)); + xhci_dma_read_u32s(xhci, ictx, ictl_ctx, sizeof(ictl_ctx)); if (ictl_ctx[0] != 0x0 || ictl_ctx[1] & ~0x3) { fprintf(stderr, "xhci: invalid input context control %08x %08x\n", @@ -2047,12 +2074,12 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, } if (ictl_ctx[1] & 0x1) { - pci_dma_read(&xhci->pci_dev, ictx+32, islot_ctx, sizeof(islot_ctx)); + xhci_dma_read_u32s(xhci, ictx+32, islot_ctx, sizeof(islot_ctx)); DPRINTF("xhci: input slot context: %08x %08x %08x %08x\n", islot_ctx[0], islot_ctx[1], islot_ctx[2], islot_ctx[3]); - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); slot_ctx[1] &= ~0xFFFF; /* max exit latency */ slot_ctx[1] |= islot_ctx[1] & 0xFFFF; @@ -2062,17 +2089,17 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); } if (ictl_ctx[1] & 0x2) { - pci_dma_read(&xhci->pci_dev, ictx+64, iep0_ctx, sizeof(iep0_ctx)); + xhci_dma_read_u32s(xhci, ictx+64, iep0_ctx, sizeof(iep0_ctx)); DPRINTF("xhci: input ep0 context: %08x %08x %08x %08x %08x\n", iep0_ctx[0], iep0_ctx[1], iep0_ctx[2], iep0_ctx[3], iep0_ctx[4]); - pci_dma_read(&xhci->pci_dev, octx+32, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_read_u32s(xhci, octx+32, ep0_ctx, sizeof(ep0_ctx)); ep0_ctx[1] &= ~0xFFFF0000; /* max packet size*/ ep0_ctx[1] |= iep0_ctx[1] & 0xFFFF0000; @@ -2080,7 +2107,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid, DPRINTF("xhci: output ep0 context: %08x %08x %08x %08x %08x\n", ep0_ctx[0], ep0_ctx[1], ep0_ctx[2], ep0_ctx[3], ep0_ctx[4]); - pci_dma_write(&xhci->pci_dev, octx+32, ep0_ctx, sizeof(ep0_ctx)); + xhci_dma_write_u32s(xhci, octx+32, ep0_ctx, sizeof(ep0_ctx)); } return CC_SUCCESS; @@ -2105,12 +2132,12 @@ static TRBCCode xhci_reset_slot(XHCIState *xhci, unsigned int slotid) } } - pci_dma_read(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_read_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); slot_ctx[3] &= ~(SLOT_STATE_MASK << SLOT_STATE_SHIFT); slot_ctx[3] |= SLOT_DEFAULT << SLOT_STATE_SHIFT; DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n", slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]); - pci_dma_write(&xhci->pci_dev, octx, slot_ctx, sizeof(slot_ctx)); + xhci_dma_write_u32s(xhci, octx, slot_ctx, sizeof(slot_ctx)); return CC_SUCCESS; } -- 1.7.10.4 -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs 2012-11-05 4:20 ` David Gibson @ 2012-11-05 10:20 ` Gerd Hoffmann 2012-11-08 15:08 ` [Qemu-devel] xhci and ehci qemu emulation performance Stefano Panella 0 siblings, 1 reply; 9+ messages in thread From: Gerd Hoffmann @ 2012-11-05 10:20 UTC (permalink / raw) To: David Gibson; +Cc: aliguori, qemu-devel On 11/05/12 05:20, David Gibson wrote: > Here's an updated version that uses a temporary buffer. > > From 588a8f874c8d5a658ef95e35164e182a915091db Mon Sep 17 00:00:00 2001 > From: David Gibson <david@gibson.dropbear.id.au> > Date: Mon, 5 Nov 2012 14:29:01 +1100 > Subject: [PATCH] xhci: Fix some DMA host endian bugs > > The xhci device does correct endian switches on the results of some DMAs > but not all. In particular, there are many DMAs of what are essentially > arrays of 32-bit integers which never get byteswapped. This causes them > to be interpreted incorrectly on big-endian hosts, since (as per the xhci > spec) these arrays are always little-endian in guest memory. > > This patch adds some helper functions to fix these bugs. This may not be > all the endian bugs in the xhci code, but it's certainly some of them and > the Linux guest xhci driver certainly gets further with these fixes. Patch added to usb patch queue. thanks, Gerd ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] xhci and ehci qemu emulation performance 2012-11-05 10:20 ` Gerd Hoffmann @ 2012-11-08 15:08 ` Stefano Panella 2012-11-08 15:20 ` Gerd Hoffmann 0 siblings, 1 reply; 9+ messages in thread From: Stefano Panella @ 2012-11-08 15:08 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel@nongnu.org, David Gibson Hi, I am very new to the qemu list, Thanks for your time in advance if you can answer some of my questions. I see you have done a big work on xhci emulation support for qemu! I would like to ask: - what is it the status of xhci support for SS usb3 devices? - Is it available any performance measurement on the emulation compared to native (for example attaching an usb3 SSD drive etc.)? Thanks again, Stefano ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] xhci and ehci qemu emulation performance 2012-11-08 15:08 ` [Qemu-devel] xhci and ehci qemu emulation performance Stefano Panella @ 2012-11-08 15:20 ` Gerd Hoffmann 2012-11-08 15:32 ` Stefano Panella 0 siblings, 1 reply; 9+ messages in thread From: Gerd Hoffmann @ 2012-11-08 15:20 UTC (permalink / raw) To: Stefano Panella; +Cc: qemu-devel@nongnu.org, David Gibson On 11/08/12 16:08, Stefano Panella wrote: > Hi, > > I am very new to the qemu list, > > Thanks for your time in advance if you can answer some of my questions. > > I see you have done a big work on xhci emulation support for qemu! > > I would like to ask: > - what is it the status of xhci support for SS usb3 devices? no streams support yet, otherwise it should work in theory. There might be bugs though, and xhci hasn't seen that much testing yet. > - Is it available any performance measurement on the emulation compared > to native (for example attaching an usb3 SSD drive etc.)? I don't have benchmark numbers. cheers, Gerd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] xhci and ehci qemu emulation performance 2012-11-08 15:20 ` Gerd Hoffmann @ 2012-11-08 15:32 ` Stefano Panella 2012-11-08 18:26 ` Gerd Hoffmann 0 siblings, 1 reply; 9+ messages in thread From: Stefano Panella @ 2012-11-08 15:32 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel@nongnu.org, David Gibson On 11/08/2012 03:20 PM, Gerd Hoffmann wrote: > On 11/08/12 16:08, Stefano Panella wrote: >> Hi, >> >> I am very new to the qemu list, >> >> Thanks for your time in advance if you can answer some of my questions. >> >> I see you have done a big work on xhci emulation support for qemu! >> >> I would like to ask: >> - what is it the status of xhci support for SS usb3 devices? > no streams support yet, otherwise it should work in theory. > There might be bugs though, and xhci hasn't seen that much testing yet. I would like to try it under Xen, do you think that would be feasible? Where can I find some documentation on how to use it? > >> - Is it available any performance measurement on the emulation compared >> to native (for example attaching an usb3 SSD drive etc.)? > I don't have benchmark numbers. > > cheers, > Gerd > Thanks + Regards, Stefano ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] xhci and ehci qemu emulation performance 2012-11-08 15:32 ` Stefano Panella @ 2012-11-08 18:26 ` Gerd Hoffmann 0 siblings, 0 replies; 9+ messages in thread From: Gerd Hoffmann @ 2012-11-08 18:26 UTC (permalink / raw) To: Stefano Panella; +Cc: qemu-devel@nongnu.org, David Gibson Hi, >> no streams support yet, otherwise it should work in theory. >> There might be bugs though, and xhci hasn't seen that much testing yet. > I would like to try it under Xen, do you think that would be feasible? > Where can I find some documentation on how to use it? >From the qemu side I see no reason why it should not work on Xen. There are some txt files on usb in docs/ in the src tree. Could be the xen management tools need some tweaks through to make them pass the required command line options to qemu, I don't know how this works these days in xen. cheers, Gerd ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-11-08 18:26 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-02 2:32 [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs David Gibson 2012-11-02 7:31 ` Gerd Hoffmann 2012-11-02 8:43 ` David Gibson 2012-11-05 4:20 ` David Gibson 2012-11-05 10:20 ` Gerd Hoffmann 2012-11-08 15:08 ` [Qemu-devel] xhci and ehci qemu emulation performance Stefano Panella 2012-11-08 15:20 ` Gerd Hoffmann 2012-11-08 15:32 ` Stefano Panella 2012-11-08 18:26 ` Gerd Hoffmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).