From f584fc7044a2a65293fd1f568a065e2b2e6dce09 Mon Sep 17 00:00:00 2001 From: mpi Date: Sun, 18 Jan 2015 11:54:02 +0000 Subject: [PATCH] Since we are no longer resetting rings when a Babble or Stall condition is detected, simply keep track of the faulty xfer instead of completing all the pending ones. Fix a race condition where we could end up aborting a freshly enqueued xfer when two different threads are submitting control transfers (i.e. usbdevs(8) and a kernel driver). --- sys/dev/usb/xhci.c | 70 +++++++++++++++++++------------------------ sys/dev/usb/xhcivar.h | 4 +-- 2 files changed, 32 insertions(+), 42 deletions(-) diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c index 3652c9c0306..6ad74c60afa 100644 --- a/sys/dev/usb/xhci.c +++ b/sys/dev/usb/xhci.c @@ -1,7 +1,7 @@ -/* $OpenBSD: xhci.c,v 1.54 2015/01/17 18:37:12 mpi Exp $ */ +/* $OpenBSD: xhci.c,v 1.55 2015/01/18 11:54:02 mpi Exp $ */ /* - * Copyright (c) 2014 Martin Pieuchot + * Copyright (c) 2014-2015 Martin Pieuchot * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -68,6 +68,7 @@ struct xhci_pipe { * interrupt routine, better way? */ struct usbd_xfer *pending_xfers[XHCI_MAX_XFER]; + struct usbd_xfer *aborted_xfer; int halted; size_t free_trbs; }; @@ -738,11 +739,6 @@ xhci_event_xfer(struct xhci_softc *sc, uint64_t paddr, uint32_t status, xfer = xp->pending_xfers[trb_idx]; if (xfer == NULL) { -#if 1 - DPRINTF(("%s: dev %d dci=%d paddr=0x%016llx idx=%d remain=%u" - " code=%u\n", DEVNAME(sc), slot, dci, (long long)paddr, - trb_idx, remain, code)); -#endif printf("%s: NULL xfer pointer\n", DEVNAME(sc)); return; } @@ -778,25 +774,25 @@ xhci_event_xfer(struct xhci_softc *sc, uint64_t paddr, uint32_t status, xfer->status = USBD_IOERROR; break; case XHCI_CODE_STALL: - /* We need to report this condition for umass(4). */ - xfer->status = USBD_STALLED; - - /* FALLTHROUGH */ case XHCI_CODE_BABBLE: + /* Prevent any timeout to kick in. */ + timeout_del(&xfer->timeout_handle); + + /* We need to report this condition for umass(4). */ + if (code == XHCI_CODE_STALL) + xfer->status = USBD_STALLED; + else + xfer->status = USBD_IOERROR; /* * Since the stack might try to start a new transfer as * soon as a pending one finishes, make sure the endpoint * is fully reset before calling usb_transfer_complete(). */ xp->halted = 1; + xp->aborted_xfer = xfer; xhci_cmd_reset_ep_async(sc, slot, dci); return; default: -#if 1 - DPRINTF(("%s: dev %d dci=%d paddr=0x%016llx idx=%d remain=%u" - " code=%u\n", DEVNAME(sc), slot, dci, (long long)paddr, - trb_idx, remain, code)); -#endif DPRINTF(("%s: unhandled code %d\n", DEVNAME(sc), code)); xfer->status = USBD_IOERROR; xp->halted = 1; @@ -810,11 +806,10 @@ void xhci_event_command(struct xhci_softc *sc, uint64_t paddr) { struct xhci_trb *trb; - struct usbd_xfer *xfer; struct xhci_pipe *xp; uint32_t flags; uint8_t dci, slot; - int i, trb_idx; + int trb_idx; trb_idx = (paddr - sc->sc_cmd_ring.dma.paddr) / sizeof(*trb); if (trb_idx < 0 || trb_idx >= sc->sc_cmd_ring.ntrb) { @@ -847,14 +842,8 @@ xhci_event_command(struct xhci_softc *sc, uint64_t paddr) xp->halted = 0; - /* Complete all pending transfers. */ - for (i = 0; i < XHCI_MAX_XFER; i++) { - xfer = xp->pending_xfers[i]; - if (xfer != NULL && xfer->done == 0) { - if (xfer->status != USBD_STALLED) - xfer->status = USBD_IOERROR; - xhci_xfer_done(xfer); - } + if (xp->aborted_xfer != NULL) { + xhci_xfer_done(xp->aborted_xfer); } break; default: @@ -900,13 +889,18 @@ xhci_xfer_done(struct usbd_xfer *xfer) struct xhci_xfer *xx = (struct xhci_xfer *)xfer; int ntrb, i; + splsoftassert(IPL_SOFTUSB); + #ifdef XHCI_DEBUG if (xx->index < 0 || xp->pending_xfers[xx->index] == NULL) { - printf("%s: xfer=%p done (index=%d, ntrb=%zd)\n", __func__, + printf("%s: xfer=%p done (idx=%d, ntrb=%zd)\n", __func__, xfer, xx->index, xx->ntrb); } #endif + if (xp->aborted_xfer == xfer) + xp->aborted_xfer = NULL; + for (ntrb = 0, i = xx->index; ntrb < xx->ntrb; ntrb++, i--) { xp->pending_xfers[i] = NULL; if (i == 0) @@ -1263,7 +1257,6 @@ xhci_pipe_close(struct usbd_pipe *pipe) /* Root Hub */ if (pipe->device->depth == 0) return; - if (!xp->halted || xhci_cmd_stop_ep(sc, xp->slot, xp->dci)) DPRINTF(("%s: error stopping ep (%d)\n", DEVNAME(sc), xp->dci)); @@ -1517,7 +1510,7 @@ xhci_ring_produce(struct xhci_softc *sc, struct xhci_ring *ring) } struct xhci_trb * -xhci_xfer_get_trb(struct xhci_softc *sc, struct usbd_xfer* xfer, +xhci_xfer_get_trb(struct xhci_softc *sc, struct usbd_xfer *xfer, uint8_t *togglep, int last) { struct xhci_pipe *xp = (struct xhci_pipe *)xfer->pipe; @@ -1582,19 +1575,16 @@ xhci_command_submit(struct xhci_softc *sc, struct xhci_trb *trb0, int timeout) memcpy(trb0, &sc->sc_result_trb, sizeof(struct xhci_trb)); - if (XHCI_TRB_GET_CODE(letoh32(trb0->trb_status)) != XHCI_CODE_SUCCESS) { - printf("%s: event error code=%d\n", DEVNAME(sc), - XHCI_TRB_GET_CODE(letoh32(trb0->trb_status))); - error = EIO; - } + if (XHCI_TRB_GET_CODE(letoh32(trb0->trb_status)) == XHCI_CODE_SUCCESS) + return (0); #ifdef XHCI_DEBUG - if (error) { - printf("result = %d ", XHCI_TRB_TYPE(letoh32(trb0->trb_flags))); - xhci_dump_trb(trb0); - } + printf("%s: event error code=%d, result=%d \n", DEVNAME(sc), + XHCI_TRB_GET_CODE(letoh32(trb0->trb_status)), + XHCI_TRB_TYPE(letoh32(trb0->trb_flags))); + xhci_dump_trb(trb0); #endif - return (error); + return (EIO); } int @@ -1895,7 +1885,7 @@ xhci_abort_xfer(struct usbd_xfer *xfer, usbd_status status) { splsoftassert(IPL_SOFTUSB); - DPRINTF(("%s: xfer=%p status=%s err=%s actlen=%d len=%d index=%d\n", + DPRINTF(("%s: xfer=%p status=%s err=%s actlen=%d len=%d idx=%d\n", __func__, xfer, usbd_errstr(xfer->status), usbd_errstr(status), xfer->actlen, xfer->length, ((struct xhci_xfer *)xfer)->index)); diff --git a/sys/dev/usb/xhcivar.h b/sys/dev/usb/xhcivar.h index fad436ead25..e2a1f56fda3 100644 --- a/sys/dev/usb/xhcivar.h +++ b/sys/dev/usb/xhcivar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: xhcivar.h,v 1.6 2014/12/15 17:10:44 mpi Exp $ */ +/* $OpenBSD: xhcivar.h,v 1.7 2015/01/18 11:54:02 mpi Exp $ */ /* * Copyright (c) 2014 Martin Pieuchot @@ -35,7 +35,7 @@ struct usbd_dma_info { struct xhci_xfer { struct usbd_xfer xfer; - int index; /* Index of the first TRB */ + int index; /* Index of the last TRB */ size_t ntrb; /* Number of associated TRBs */ }; -- 2.20.1