Prevent a race resulting in an infinite loop printing "ehci_idone" messages.

As soon as a transfer is being cancelled, remove it from the list of pending
xfers.  This way the soft interrupt routine won't try to process it before
ehci_abort() gives it back to the stack.

Problem analysed by and previous diff ok stsp@

1 files changed, 28 insertions, 20 deletions

diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c
index e0506ae2f05..7b096c14d2f 100644
--- a/sys/dev/usb/ehci.c
+++ b/sys/dev/usb/ehci.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ehci.c,v 1.176 2015/03/06 22:53:03 mpi Exp $ */
+/*	$OpenBSD: ehci.c,v 1.177 2015/03/17 14:45:07 mpi Exp $ */
 /*	$NetBSD: ehci.c,v 1.66 2004/06/30 03:11:56 mycroft Exp $	*/
 
 /*
@@ -206,11 +206,7 @@ void		ehci_dump_exfer(struct ehci_xfer *);
 #define ehci_add_intr_list(sc, ex) \
 	TAILQ_INSERT_TAIL(&(sc)->sc_intrhead, (ex), inext);
 #define ehci_del_intr_list(sc, ex) \
-	do { \
-		TAILQ_REMOVE(&sc->sc_intrhead, (ex), inext); \
-		(ex)->inext.tqe_prev = NULL; \
-	} while (0)
-#define ehci_active_intr_list(ex) ((ex)->inext.tqe_prev != NULL)
+	TAILQ_REMOVE(&sc->sc_intrhead, (ex), inext);
 
 struct usbd_bus_methods ehci_bus_methods = {
 	.open_pipe = ehci_open,
@@ -753,6 +749,7 @@ ehci_check_qh_intr(struct ehci_softc *sc, struct usbd_xfer *xfer)
 	}
  done:
 	DPRINTFN(12, ("ehci_check_intr: ex=%p done\n", ex));
+	ehci_del_intr_list(sc, ex);
 	timeout_del(&xfer->timeout_handle);
 	usb_rem_task(xfer->pipe->device, &xfer->abort_task);
 	ehci_idone(xfer);
@@ -803,6 +800,7 @@ ehci_check_itd_intr(struct ehci_softc *sc, struct usbd_xfer *xfer)
 	return;
 done:
 	DPRINTFN(12, ("ehci_check_itd_intr: ex=%p done\n", ex));
+	ehci_del_intr_list(sc, ex);
 	timeout_del(&xfer->timeout_handle);
 	usb_rem_task(xfer->pipe->device, &xfer->abort_task);
 	ehci_idone(xfer);
@@ -1056,6 +1054,15 @@ ehci_activate(struct device *self, int act)
 	switch (act) {
 	case DVACT_SUSPEND:
 		rv = config_activate_children(self, act);
+
+#ifdef DIAGNOSTIC
+		if (!TAILQ_EMPTY(&sc->sc_intrhead)) {
+			printf("%s: interrupt list not empty\n",
+			    sc->sc_bus.bdev.dv_xname);
+			return (-1);
+		}
+#endif
+
 		sc->sc_bus.use_polling++;
 
 		for (i = 1; i <= sc->sc_noport; i++) {
@@ -2718,9 +2725,10 @@ ehci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
 
 	DPRINTF(("ehci_abort_xfer: xfer=%p pipe=%p\n", xfer, epipe));
 
-	if (sc->sc_bus.dying) {
-		/* If we're dying, just do the software part. */
+	if (sc->sc_bus.dying || xfer->status == USBD_NOT_STARTED) {
 		s = splusb();
+		if (xfer->status != USBD_NOT_STARTED)
+			ehci_del_intr_list(sc, ex);
 		xfer->status = status;	/* make software ignore it */
 		timeout_del(&xfer->timeout_handle);
 		usb_rem_task(xfer->device, &xfer->abort_task);
@@ -2759,6 +2767,7 @@ ehci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
 	s = splusb();
 	ex->ehci_xfer_flags |= EHCI_XFER_ABORTING;
 	xfer->status = status;	/* make software ignore it */
+	ehci_del_intr_list(sc, ex);
 	timeout_del(&xfer->timeout_handle);
 	usb_rem_task(xfer->device, &xfer->abort_task);
 	splx(s);
@@ -2827,8 +2836,10 @@ ehci_abort_isoc_xfer(struct usbd_xfer *xfer, usbd_status status)
 
 	DPRINTF(("ehci_abort_isoc_xfer: xfer %p pipe %p\n", xfer, xfer->pipe));
 
-	if (sc->sc_bus.dying) {
+	if (sc->sc_bus.dying || xfer->status == USBD_NOT_STARTED) {
 		s = splusb();
+		if (xfer->status != USBD_NOT_STARTED)
+			ehci_del_intr_list(sc, ex);
 		xfer->status = status;
 		timeout_del(&xfer->timeout_handle);
 		usb_rem_task(xfer->device, &xfer->abort_task);
@@ -2852,9 +2863,10 @@ ehci_abort_isoc_xfer(struct usbd_xfer *xfer, usbd_status status)
 			tsleep(&ex->ehci_xfer_flags, PZERO, "ehciiaw", 0);
 		return;
 	}
-	ex->ehci_xfer_flags |= EHCI_XFER_ABORTING;
 
+	ex->ehci_xfer_flags |= EHCI_XFER_ABORTING;
 	xfer->status = status;
+	ehci_del_intr_list(sc, ex);
 	timeout_del(&xfer->timeout_handle);
 	usb_rem_task(xfer->device, &xfer->abort_task);
 
@@ -2998,8 +3010,7 @@ ehci_device_ctrl_done(struct usbd_xfer *xfer)
 	}
 #endif
 
-	if (xfer->status != USBD_NOMEM && ehci_active_intr_list(ex)) {
-		ehci_del_intr_list(sc, ex);	/* remove from active list */
+	if (xfer->status != USBD_NOMEM) {
 		ehci_free_sqtd_chain(sc, ex);
 	}
 
@@ -3283,8 +3294,7 @@ ehci_device_bulk_done(struct usbd_xfer *xfer)
 	DPRINTFN(10,("ehci_bulk_done: xfer=%p, actlen=%d\n",
 	    xfer, xfer->actlen));
 
-	if (xfer->status != USBD_NOMEM && ehci_active_intr_list(ex)) {
-		ehci_del_intr_list(sc, ex);	/* remove from active list */
+	if (xfer->status != USBD_NOMEM) {
 		ehci_free_sqtd_chain(sc, ex);
 		usb_syncmem(&xfer->dmabuf, 0, xfer->length,
 		    usbd_xfer_isread(xfer) ?
@@ -3480,11 +3490,10 @@ ehci_device_intr_done(struct usbd_xfer *xfer)
 			timeout_set(&xfer->timeout_handle, ehci_timeout, xfer);
 			timeout_add_msec(&xfer->timeout_handle, xfer->timeout);
 		}
-		splx(s);
-
+		ehci_add_intr_list(sc, ex);
 		xfer->status = USBD_IN_PROGRESS;
-	} else if (xfer->status != USBD_NOMEM && ehci_active_intr_list(ex)) {
-		ehci_del_intr_list(sc, ex); /* remove from active list */
+		splx(s);
+	} else if (xfer->status != USBD_NOMEM) {
 		ehci_free_sqtd_chain(sc, ex);
 		usb_syncmem(&xfer->dmabuf, 0, xfer->length,
 		    usbd_xfer_isread(xfer) ?
@@ -3789,8 +3798,7 @@ ehci_device_isoc_done(struct usbd_xfer *xfer)
 
 	s = splusb();
 	epipe->u.isoc.cur_xfers--;
-	if (xfer->status != USBD_NOMEM && ehci_active_intr_list(ex)) {
-		ehci_del_intr_list(sc, ex);
+	if (xfer->status != USBD_NOMEM) {
 		ehci_rem_free_itd_chain(sc, ex);
 	}
 	splx(s);