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).

diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index 3652c9c..6ad74c6 100644 (file)
--- 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 fad436e..e2a1f56 100644 (file)
--- 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 */
 };