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

1 files changed, 30 insertions, 40 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));