tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: USB lockup (probably solved)



Looks like I'm making progress after all.

> The change [nick] referred to was
> 
> Revision 1.254.2.76 / (download) - annotate - [select for diffs], Mon
> May 30 06:46:50 2016 UTC (4 years, 5 months ago) by skrll
> Branch: nick-nhusb
[...]
> 
> Restructure the abort code for TD based transfers (ctrl, bulk, intr).
> 
[...]

I (hopefully) adapted that to -8 and it seems to work!

I attach my adaption of nick's work plus some additional debugging code 
I added while analyzing the issue.

So what next? File a PR?
--- ohcivar.h.orig	2020-11-30 15:31:45.755906264 +0100
+++ ohcivar.h	2020-12-01 12:12:58.463657450 +0100
@@ -1,4 +1,4 @@
-/*	$NetBSD: ohcivar.h,v 1.58.10.1 2018/08/25 11:29:52 martin Exp $	*/
+/*	$NetBSD: ohcivar.h,v 1.55.6.15 2016/05/30 06:46:50 skrll Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -50,6 +50,7 @@
 	ohci_td_t td;
 	struct ohci_soft_td *nexttd;	/* mirrors nexttd in TD */
 	struct ohci_soft_td *dnext;	/* next in done list */
+	struct ohci_soft_td **held;	/* where the ref to this std is held */
 	ohci_physaddr_t physaddr;
 	usb_dma_t dma;
 	int offs;
@@ -71,6 +72,7 @@
 	ohci_itd_t itd;
 	struct ohci_soft_itd *nextitd;	/* mirrors nexttd in ITD */
 	struct ohci_soft_itd *dnext;	/* next in done list */
+	struct ohci_soft_itd **held;	/* where the ref to this sitd is held */
 	ohci_physaddr_t physaddr;
 	usb_dma_t dma;
 	int offs;
@@ -114,6 +116,8 @@
 	LIST_HEAD(, ohci_soft_td)  sc_hash_tds[OHCI_HASH_SIZE];
 	LIST_HEAD(, ohci_soft_itd) sc_hash_itds[OHCI_HASH_SIZE];
 
+	TAILQ_HEAD(, ohci_xfer)	sc_abortingxfers;
+
 	int sc_noport;
 
 	int sc_endian;
@@ -128,6 +128,8 @@
        int sc_flags;
 #define OHCIF_SUPERIO          0x0001
 
+       kcondvar_t sc_softwake_cv;
+
        ohci_soft_ed_t *sc_freeeds;
        ohci_soft_td_t *sc_freetds;
        ohci_soft_itd_t *sc_freeitds;
@@ -148,6 +152,8 @@
 
 struct ohci_xfer {
 	struct usbd_xfer xfer;
+	uint32_t ox_abintrs;
+	TAILQ_ENTRY(ohci_xfer) ox_abnext;
 	/* ctrl */
 	ohci_soft_td_t *ox_setup;
 	ohci_soft_td_t *ox_stat;
--- ohci.c	2020-11-23 18:30:07.000000000 +0100
+++ /tmp/ohci.c	2020-11-30 18:02:27.000000000 +0100
@@ -1,4 +1,4 @@
-/*	$NetBSD: ohci.c,v 1.273.6.6 2020/02/25 18:52:44 martin Exp $	*/
+/*	$NetBSD: ohci.c,v 1.254.2.76 2016/05/30 06:46:50 skrll Exp $	*/
 
 /*
  * Copyright (c) 1998, 2004, 2005, 2012 The NetBSD Foundation, Inc.
@@ -41,7 +41,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.273.6.6 2020/02/25 18:52:44 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.254.2.76 2016/05/30 06:46:50 skrll Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -384,6 +384,8 @@ ohci_detach(struct ohci_softc *sc, int f
 
 	softint_disestablish(sc->sc_rhsc_si);
 
+	cv_destroy(&sc->sc_softwake_cv);
+
 	mutex_destroy(&sc->sc_lock);
 	mutex_destroy(&sc->sc_intr_lock);
 
@@ -492,6 +494,7 @@ ohci_alloc_std(ohci_softc_t *sc)
 	memset(&std->td, 0, sizeof(ohci_td_t));
 	std->nexttd = NULL;
 	std->xfer = NULL;
+	std->held = NULL;
 
 	return std;
 }
@@ -539,14 +542,17 @@ ohci_alloc_std_chain(ohci_softc_t *sc, s
 
 	DPRINTFN(8, "xfer %#jx nstd %jd", (uintptr_t)xfer, nstd, 0, 0);
 
-	for (size_t j = 0; j < ox->ox_nstd;) {
+	for (size_t j = 0; j < ox->ox_nstd; j++) {
 		ohci_soft_td_t *cur = ohci_alloc_std(sc);
 		if (cur == NULL)
 			goto nomem;
 
-		ox->ox_stds[j++] = cur;
+		ox->ox_stds[j] = cur;
+		cur->held = &ox->ox_stds[j];
 		cur->xfer = xfer;
 		cur->flags = 0;
+		DPRINTFN(10, "xfer=%#jx new std=%#jx held at %#jx", ox, cur,
+		    cur->held, 0);
 	}
 
 	return 0;
@@ -788,6 +794,7 @@ ohci_init(ohci_softc_t *sc)
 
 	mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SOFTUSB);
 	mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_USB);
+	cv_init(&sc->sc_softwake_cv, "ohciab");
 
 	sc->sc_rhsc_si = softint_establish(SOFTINT_USB | SOFTINT_MPSAFE,
 	    ohci_rhsc_softint, sc);
@@ -797,6 +804,8 @@ ohci_init(ohci_softc_t *sc)
 	for (i = 0; i < OHCI_HASH_SIZE; i++)
 		LIST_INIT(&sc->sc_hash_itds[i]);
 
+	TAILQ_INIT(&sc->sc_abortingxfers);
+
 	sc->sc_xferpool = pool_cache_init(sizeof(struct ohci_xfer), 0, 0, 0,
 	    "ohcixfer", NULL, IPL_USB, NULL, NULL, NULL);
 
@@ -1334,12 +1343,26 @@ ohci_intr1(ohci_softc_t *sc)
 		 */
 		softint_schedule(sc->sc_rhsc_si);
 	}
+	if (eintrs & OHCI_SF) {
+		struct ohci_xfer *ox, *tmp;
+		TAILQ_FOREACH_SAFE(ox, &sc->sc_abortingxfers, ox_abnext, tmp) {
+			DPRINTFN(10, "SF %#jx xfer %#jx", (uintptr_t)sc, (uintptr_t)ox, 0, 0);
+			ox->ox_abintrs &= ~OHCI_SF;
+			KASSERT(ox->ox_abintrs == 0);
+			TAILQ_REMOVE(&sc->sc_abortingxfers, ox, ox_abnext);
+		}
+		cv_broadcast(&sc->sc_softwake_cv);
+
+		KASSERT(TAILQ_EMPTY(&sc->sc_abortingxfers));
+		DPRINTFN(10, "end SOF %#jx", (uintptr_t)sc, 0, 0, 0);
+		/* Don't remove OHIC_SF from eintrs so it is blocked below */
+	}
 
 	if (eintrs != 0) {
 		/* Block unprocessed interrupts. */
 		OWRITE4(sc, OHCI_INTERRUPT_DISABLE, eintrs);
 		sc->sc_eintrs &= ~eintrs;
-		DPRINTF("sc %#jx blocking intrs 0x%jx", (uintptr_t)sc,
+		DPRINTF("sc %#jx blocking/removing intrs 0x%jx", (uintptr_t)sc,
 		    eintrs, 0, 0);
 	}
 
@@ -1391,12 +1414,22 @@ ohci_softintr(void *v)
 	struct ohci_pipe *opipe;
 	int len, cc;
 	int i, j, actlen, iframes, uedir;
-	ohci_physaddr_t done;
+	ohci_physaddr_t done = 0;
 
 	KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock));
 
 	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 
+	/*
+	 * Only read hccadone if WDH is set - we might get here from places
+	 * other than an interrupt
+	 */
+	if (!(OREAD4(sc, OHCI_INTERRUPT_STATUS) & OHCI_WDH)) {
+		DPRINTFN(10, "no WDH %#jx", (uintptr_t)sc, 0, 0, 0);
+		return;
+	}
+
+	DPRINTFN(10, "WDH %#jx", (uintptr_t)sc, 0, 0, 0);
 	usb_syncmem(&sc->sc_hccadma, offsetof(struct ohci_hcca, hcca_done_head),
 	    sizeof(sc->sc_hcca->hcca_done_head),
 	    BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
@@ -1451,9 +1484,8 @@ ohci_softintr(void *v)
 	for (std = sdone; std; std = stdnext) {
 		xfer = std->xfer;
 		stdnext = std->dnext;
-		DPRINTFN(10, "std=%#jx xfer=%#jx hcpriv=%#jx", (uintptr_t)std,
-		    (uintptr_t)xfer, (uintptr_t)(xfer ? xfer->ux_hcpriv : 0),
-		    0);
+		DPRINTFN(10, "std=%#jx xfer=%#jx hcpriv=%#jx dnext=%#jx", (uintptr_t)std,
+		    (uintptr_t)xfer, (uintptr_t)(xfer ? xfer->ux_hcpriv : 0), (uintptr_t)stdnext);
 		if (xfer == NULL) {
 			/*
 			 * xfer == NULL: There seems to be no xfer associated
@@ -1463,25 +1495,25 @@ ohci_softintr(void *v)
 			 */
 			continue;
 		}
-
-		/*
-		 * If software has completed it, either by cancellation
-		 * or timeout, drop it on the floor.
-		 */
-		if (xfer->ux_status != USBD_IN_PROGRESS) {
-			KASSERT(xfer->ux_status == USBD_CANCELLED ||
-			    xfer->ux_status == USBD_TIMEOUT);
+		if (std->held == NULL) {
+			DPRINTFN(10, "std=%#jx held is null", (uintptr_t)std, 0, 0, 0);
+			ohci_hash_rem_td(sc, std);
+			ohci_free_std_locked(sc, std);
 			continue;
 		}
-
 		/*
-		 * Cancel the timeout and the task, which have not yet
-		 * run.  If they have already fired, at worst they are
-		 * waiting for the lock.  They will see that the xfer
-		 * is no longer in progress and give up.
+		 * Make sure the timeout handler didn't run or ran to the end
+		 * and set the transfer status.
 		 */
-		callout_stop(&xfer->ux_callout);
-		usb_rem_task(xfer->ux_pipe->up_dev, &xfer->ux_aborttask);
+		callout_halt(&xfer->ux_callout, &sc->sc_lock);
+
+		if (xfer->ux_status == USBD_CANCELLED ||
+		    xfer->ux_status == USBD_TIMEOUT) {
+			DPRINTF("cancel/timeout %#jx", (uintptr_t)xfer, 0, 0, 0);
+
+			/* Handled by abort routine. */
+			continue;
+		}
 
 		len = std->len;
 		if (std->td.td_cbp != 0)
@@ -1552,26 +1584,12 @@ ohci_softintr(void *v)
 		    0);
 		if (xfer == NULL)
 			continue;
-
-		/*
-		 * If software has completed it, either by cancellation
-		 * or timeout, drop it on the floor.
-		 */
-		if (xfer->ux_status != USBD_IN_PROGRESS) {
-			KASSERT(xfer->ux_status == USBD_CANCELLED ||
-			    xfer->ux_status == USBD_TIMEOUT);
+		if (xfer->ux_status == USBD_CANCELLED ||
+		    xfer->ux_status == USBD_TIMEOUT) {
+			DPRINTF("cancel/timeout %#jx", (uintptr_t)xfer, 0, 0, 0);
+			/* Handled by abort routine. */
 			continue;
 		}
-
-		/*
-		 * Cancel the timeout and the task, which have not yet
-		 * run.  If they have already fired, at worst they are
-		 * waiting for the lock.  They will see that the xfer
-		 * is no longer in progress and give up.
-		 */
-		callout_stop(&xfer->ux_callout);
-		usb_rem_task(xfer->ux_pipe->up_dev, &xfer->ux_aborttask);
-
 		KASSERT(!sitd->isdone);
 #ifdef DIAGNOSTIC
 		sitd->isdone = true;
@@ -1913,17 +1931,33 @@ ohci_hash_find_itd(ohci_softc_t *sc, ohc
 void
 ohci_timeout(void *addr)
 {
-	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 	struct usbd_xfer *xfer = addr;
 	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	struct usbd_device *dev = xfer->ux_pipe->up_dev;
+	bool timeout = false;
 
+	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 	DPRINTF("xfer=%#jx", (uintptr_t)xfer, 0, 0, 0);
 
 	mutex_enter(&sc->sc_lock);
-	if (!sc->sc_dying && xfer->ux_status == USBD_IN_PROGRESS)
-		usb_add_task(dev, &xfer->ux_aborttask, USB_TASKQ_HC);
+	if (sc->sc_dying) {
+		ohci_abort_xfer(xfer, USBD_TIMEOUT);
+		mutex_exit(&sc->sc_lock);
+		return;
+	}
+
+	if (xfer->ux_status != USBD_CANCELLED) {
+		xfer->ux_status = USBD_TIMEOUT;
+		timeout = true;
+	}
 	mutex_exit(&sc->sc_lock);
+ 
+	if (timeout) {
+		/* Execute the abort in a process context. */
+		usb_init_task(&xfer->ux_aborttask, ohci_timeout_task, addr,
+		    USB_TASKQ_MPSAFE);
+		usb_add_task(xfer->ux_pipe->up_dev, &xfer->ux_aborttask,
+		    USB_TASKQ_HC);
+	}
 }
 
 void
@@ -2095,6 +2129,7 @@ ohci_open(struct usbd_pipe *pipe)
 				goto bad;
 
 			opipe->tail.itd = sitd;
+			sitd->held = &opipe->tail.itd;
 			tdphys = sitd->physaddr;
 			fmt = OHCI_ED_FORMAT_ISO;
 			if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN)
@@ -2107,6 +2142,7 @@ ohci_open(struct usbd_pipe *pipe)
 				goto bad;
 
 			opipe->tail.td = std;
+			std->held = &opipe->tail.td;
 			tdphys = std->physaddr;
 			fmt = OHCI_ED_FORMAT_GEN | OHCI_ED_DIR_TD;
 		}
@@ -2236,67 +2272,65 @@ ohci_close_pipe(struct usbd_pipe *pipe, 
 void
 ohci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
 {
-	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 	struct ohci_pipe *opipe = OHCI_PIPE2OPIPE(xfer->ux_pipe);
 	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
 	ohci_soft_ed_t *sed = opipe->sed;
 	ohci_soft_td_t *p, *n;
 	ohci_physaddr_t headp;
 	int hit;
+	int wake;
 
-	KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT),
-	    "invalid status for abort: %d", (int)status);
-
+	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 	DPRINTF("xfer=%#jx pipe=%#jx sed=%#jx", (uintptr_t)xfer,
 	    (uintptr_t)opipe, (uintptr_t)sed, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
 	ASSERT_SLEEPABLE();
 
-	if (status == USBD_CANCELLED) {
-		/*
-		 * We are synchronously aborting.  Try to stop the
-		 * callout and task, but if we can't, wait for them to
-		 * complete.
-		 */
+	if (sc->sc_dying) {
+		/* If we're dying, just do the software part. */
+		KASSERT(xfer->ux_status == status);
 		callout_halt(&xfer->ux_callout, &sc->sc_lock);
-		usb_rem_task_wait(xfer->ux_pipe->up_dev, &xfer->ux_aborttask,
-		    USB_TASKQ_HC, &sc->sc_lock);
-	} else {
-		/* Otherwise, we are timing out.  */
-		KASSERT(status == USBD_TIMEOUT);
+		usb_transfer_complete(xfer);
+		return;
 	}
 
 	/*
-	 * The xfer cannot have been cancelled already.  It is the
-	 * responsibility of the caller of usbd_abort_pipe not to try
-	 * to abort a pipe multiple times, whether concurrently or
-	 * sequentially.
+	 * If an abort is already in progress then just wait for it to
+	 * complete and return.
 	 */
-	KASSERT(xfer->ux_status != USBD_CANCELLED);
-
-	/* Only the timeout, which runs only once, can time it out.  */
-	KASSERT(xfer->ux_status != USBD_TIMEOUT);
-
-	/* If anyone else beat us, we're done.  */
-	if (xfer->ux_status != USBD_IN_PROGRESS)
-		return;
-
-	/* We beat everyone else.  Claim the status.  */
-	xfer->ux_status = status;
+	if (xfer->ux_hcflags & UXFER_ABORTING) {
+		DPRINTFN(2, "already aborting", 0, 0, 0, 0);
+#ifdef DIAGNOSTIC
+		if (status == USBD_TIMEOUT)
+			printf("%s: TIMEOUT while aborting\n", __func__);
+#endif
+		/* Override the status which might be USBD_TIMEOUT. */
+		xfer->ux_status = status;
+		DPRINTFN(2, "waiting for abort to finish", 0, 0, 0, 0);
+		xfer->ux_hcflags |= UXFER_ABORTWAIT;
+		while (xfer->ux_hcflags & UXFER_ABORTING)
+			cv_wait(&xfer->ux_hccv, &sc->sc_lock);
+		goto done;
+	}
+	xfer->ux_hcflags |= UXFER_ABORTING;
 
 	/*
-	 * If we're dying, skip the hardware action and just notify the
-	 * software that we're done.
+	 * Step 1: When cancelling a transfer make sure the timeout handler
+	 * didn't run or ran to the end and saw the USBD_CANCELLED status.
+	 * Otherwise we must have got here via a timeout.
+	 *
+	 * If we timed out then
 	 */
-	if (sc->sc_dying) {
-		DPRINTFN(4, "xfer %#jx dying %ju", (uintptr_t)xfer,
-		    xfer->ux_status, 0, 0);
-		goto dying;
+	if (status == USBD_CANCELLED) {
+		xfer->ux_status = status;
+		callout_halt(&xfer->ux_callout, &sc->sc_lock);
+	} else {
+		KASSERT(xfer->ux_status == USBD_TIMEOUT);
 	}
 
 	/*
-	 * HC Step 1: Unless the endpoint is already halted, we set the endpoint
+	 * Step 2: Unless the endpoint is already halted, we set the endpoint
 	 * descriptor sKip bit and wait for hardware to complete processing.
 	 *
 	 * This includes ensuring that any TDs of the transfer that got onto
@@ -2307,20 +2341,39 @@ ohci_abort_xfer(struct usbd_xfer *xfer, 
 	usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags),
 	    sizeof(sed->ed.ed_flags),
 	    BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
-	sed->ed.ed_flags |= HTOO32(OHCI_ED_SKIP); /* force hardware skip */
-	usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags),
-	    sizeof(sed->ed.ed_flags),
-	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
+	if (!(sed->ed.ed_flags & OHCI_HALTED)) {
+		/* force hardware skip */
+		DPRINTFN(1, "pausing ed=%#jx", (uintptr_t)sed, 0, 0, 0);
+		sed->ed.ed_flags |= HTOO32(OHCI_ED_SKIP);
+		usb_syncmem(&sed->dma,
+		    sed->offs + offsetof(ohci_ed_t, ed_flags),
+		    sizeof(sed->ed.ed_flags),
+		    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 
-	/*
-	 * HC Step 2: Wait until we know hardware has finished any possible
-	 * use of the xfer.
-	 */
-	/* Hardware finishes in 1ms */
-	usb_delay_ms_locked(opipe->pipe.up_dev->ud_bus, 20, &sc->sc_lock);
+		DPRINTFN(10, "WDH %#jx xfer %#jx", (uintptr_t)sc, (uintptr_t)xfer,
+		    0, 0);
+		struct ohci_xfer *ox = OHCI_XFER2OXFER(xfer);
+		ox->ox_abintrs = OHCI_SF;
+		TAILQ_INSERT_TAIL(&sc->sc_abortingxfers, ox, ox_abnext);
+
+		OWRITE4(sc, OHCI_INTERRUPT_STATUS, OHCI_SF);
+		sc->sc_eintrs |= OHCI_SF;
+		OWRITE4(sc, OHCI_INTERRUPT_ENABLE, OHCI_SF);
+		/*
+		 * Step 2: Wait until we know hardware has finished any possible
+		 * use of the xfer.
+		 */
+		while (ox->ox_abintrs != 0) {
+			DPRINTFN(10, "WDH %#jx xfer %#jx intrs %#x", (uintptr_t)sc,
+			    (uintptr_t)xfer, ox->ox_abintrs, 0);
+			cv_wait(&sc->sc_softwake_cv, &sc->sc_lock);
+		}
+	} else {
+  		DPRINTFN(1, "halted ed=%#jx", (uintptr_t)sed, 0, 0, 0);
+	}
 
 	/*
-	 * HC Step 3: Remove any vestiges of the xfer from the hardware.
+	 * Step 3: Remove any vestiges of the xfer from the hardware.
 	 * The complication here is that the hardware may have executed
 	 * beyond the xfer we're trying to abort.  So as we're scanning
 	 * the TDs of this xfer we check if the hardware points to
@@ -2339,12 +2392,36 @@ ohci_abort_xfer(struct usbd_xfer *xfer, 
 	}
 	DPRINTF("--- dump end ---", 0, 0, 0, 0);
 #endif
+
+#define OHCI_CC_ACCESSED_P(x)		(((x) & OHCI_CC_NOT_ACCESSED_MASK) != OHCI_CC_NOT_ACCESSED)
 	headp = O32TOH(sed->ed.ed_headp) & OHCI_HEADMASK;
 	hit = 0;
 	for (; p->xfer == xfer; p = n) {
 		hit |= headp == p->physaddr;
 		n = p->nexttd;
-		ohci_hash_rem_td(sc, p);
+
+		int cc = OHCI_TD_GET_CC(O32TOH(p->td.td_flags));
+		if (!OHCI_CC_ACCESSED_P(cc)) {
+			ohci_hash_rem_td(sc, p);
+			continue;
+		}
+		DPRINTFN(10, "std=%#jx has been touched by HC", (uintptr_t)p,
+		    0, 0, 0);
+
+		mutex_exit(&sc->sc_lock);
+		ohci_soft_td_t *std = ohci_alloc_std(sc);
+		if (std == NULL) {
+			/* XXX What to do??? */
+			panic("hmm");
+		}
+		mutex_enter(&sc->sc_lock);
+
+		DPRINTFN(10, "new std=%#jx now held at %#jx", (uintptr_t)std,
+		    (uintptr_t)p->held, 0, 0);
+		*(p->held) = std;
+		std->held = p->held;
+		std->xfer = xfer;
+		p->held = NULL;
 	}
 	/* Zap headp register if hardware pointed inside the xfer. */
 	if (hit) {
@@ -2360,7 +2437,7 @@ ohci_abort_xfer(struct usbd_xfer *xfer, 
 	}
 
 	/*
-	 * HC Step 4: Turn on hardware again.
+	 * Step 4: Turn on hardware again.
 	 */
 	usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags),
 	    sizeof(sed->ed.ed_flags),
@@ -2371,12 +2448,15 @@ ohci_abort_xfer(struct usbd_xfer *xfer, 
 	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 
 	/*
-	 * Final step: Notify completion to waiting xfers.
+	 * Step 5: Execute callback.
 	 */
-dying:
+	wake = xfer->ux_hcflags & UXFER_ABORTWAIT;
+	xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT);
 	usb_transfer_complete(xfer);
-	DPRINTFN(14, "end", 0, 0, 0, 0);
+	if (wake)
+		cv_broadcast(&xfer->ux_hccv);
 
+done:
 	KASSERT(mutex_owned(&sc->sc_lock));
 }
 
@@ -2607,17 +2687,14 @@ Static usbd_status
 ohci_root_intr_start(struct usbd_xfer *xfer)
 {
 	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	const bool polling = sc->sc_bus.ub_usepolling;
 
 	if (sc->sc_dying)
 		return USBD_IOERROR;
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
+	mutex_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intrxfer == NULL);
 	sc->sc_intrxfer = xfer;
-	if (!polling)
-		mutex_exit(&sc->sc_lock);
+	mutex_exit(&sc->sc_lock);
 
 	return USBD_IN_PROGRESS;
 }
@@ -2677,6 +2754,13 @@ ohci_device_ctrl_init(struct usbd_xfer *
 	ox->ox_setup = setup;
 	ox->ox_stat = stat;
 	ox->ox_nstd = 0;
+	setup->held = &ox->ox_setup;
+	stat->held = &ox->ox_stat;
+
+	DPRINTFN(10, "xfer=%#jx setup=%#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)setup, (uintptr_t)setup->held, 0);
+	DPRINTFN(10, "xfer=%#jx stat= %#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)stat, (uintptr_t)stat->held, 0);
 
 	/* Set up data transaction */
 	if (len != 0) {
@@ -2753,7 +2837,6 @@ ohci_device_ctrl_start(struct usbd_xfer 
 	ohci_soft_ed_t *sed;
 	int isread;
 	int len;
-	const bool polling = sc->sc_bus.ub_usepolling;
 
 	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 
@@ -2772,8 +2855,7 @@ ohci_device_ctrl_start(struct usbd_xfer 
 	    UGETW(req->wIndex));
 
 	/* Need to take lock here for pipe->tail.td */
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
+	mutex_enter(&sc->sc_lock);
 
 	/*
 	 * Use the pipe "tail" TD as our first and loan our first TD to the
@@ -2782,13 +2864,21 @@ ohci_device_ctrl_start(struct usbd_xfer 
 	setup = opipe->tail.td;
 	opipe->tail.td = ox->ox_setup;
 	ox->ox_setup = setup;
+	setup->held = &ox->ox_setup;
+
+	DPRINTFN(10, "xfer=%#jx new setup=%#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)setup, (uintptr_t)setup->held, 0);
 
 	stat = ox->ox_stat;
 
 	/* point at sentinel */
 	tail = opipe->tail.td;
+	tail->held = &opipe->tail.td;
 	sed = opipe->sed;
 
+	DPRINTFN(10, "xfer=%#jx new tail=%#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)tail, (uintptr_t)tail->held, 0);
+
 	KASSERTMSG(OHCI_ED_GET_FA(O32TOH(sed->ed.ed_flags)) == dev->ud_addr,
 	    "address ED %d pipe %d\n",
 	    OHCI_ED_GET_FA(O32TOH(sed->ed.ed_flags)), dev->ud_addr);
@@ -2889,7 +2979,7 @@ ohci_device_ctrl_start(struct usbd_xfer 
 	    sizeof(sed->ed.ed_tailp),
 	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 	OWRITE4(sc, OHCI_COMMAND_STATUS, OHCI_CLF);
-	if (xfer->ux_timeout && !polling) {
+	if (xfer->ux_timeout && !sc->sc_bus.ub_usepolling) {
 		callout_reset(&xfer->ux_callout, mstohz(xfer->ux_timeout),
 			    ohci_timeout, xfer);
 	}
@@ -2897,8 +2987,7 @@ ohci_device_ctrl_start(struct usbd_xfer 
 	DPRINTF("done", 0, 0, 0, 0);
 
 	xfer->ux_status = USBD_IN_PROGRESS;
-	if (!polling)
-		mutex_exit(&sc->sc_lock);
+	mutex_exit(&sc->sc_lock);
 
 	return USBD_IN_PROGRESS;
 }
@@ -3027,7 +3116,6 @@ ohci_device_bulk_start(struct usbd_xfer 
 	ohci_soft_td_t *data, *tail, *tdp;
 	ohci_soft_ed_t *sed;
 	int len, isread, endpt;
-	const bool polling = sc->sc_bus.ub_usepolling;
 
 	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 
@@ -3045,8 +3133,7 @@ ohci_device_bulk_start(struct usbd_xfer 
 	    len, isread, xfer->ux_flags);
 	DPRINTFN(4, "endpt=%jd", endpt, 0, 0, 0);
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
+	mutex_enter(&sc->sc_lock);
 
 	/*
 	 * Use the pipe "tail" TD as our first and loan our first TD to the
@@ -3055,13 +3142,19 @@ ohci_device_bulk_start(struct usbd_xfer 
 	data = opipe->tail.td;
 	opipe->tail.td = ox->ox_stds[0];
 	ox->ox_stds[0] = data;
+	data->held = &ox->ox_stds[0];
 	ohci_reset_std_chain(sc, xfer, len, isread, data, &last);
+	DPRINTFN(10, "xfer=%#jx new data=%#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)data, (uintptr_t)data->held, 0);
 
 	/* point at sentinel */
 	tail = opipe->tail.td;
 	memset(&tail->td, 0, sizeof(tail->td));
+	tail->held = &opipe->tail.td;
 	tail->nexttd = NULL;
 	tail->xfer = NULL;
+	DPRINTFN(10, "xfer=%#jx new tail=%#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)tail, (uintptr_t)tail->held, 0);
 	usb_syncmem(&tail->dma, tail->offs, sizeof(tail->td),
 	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 	xfer->ux_hcpriv = data;
@@ -3112,8 +3205,7 @@ ohci_device_bulk_start(struct usbd_xfer 
 	}
 
 	xfer->ux_status = USBD_IN_PROGRESS;
-	if (!polling)
-		mutex_exit(&sc->sc_lock);
+	mutex_exit(&sc->sc_lock);
 
 	return USBD_IN_PROGRESS;
 }
@@ -3232,7 +3324,6 @@ ohci_device_intr_start(struct usbd_xfer 
 	ohci_soft_ed_t *sed = opipe->sed;
 	ohci_soft_td_t *data, *last, *tail;
 	int len, isread, endpt;
-	const bool polling = sc->sc_bus.ub_usepolling;
 
 	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 
@@ -3248,8 +3339,7 @@ ohci_device_intr_start(struct usbd_xfer 
 	endpt = xfer->ux_pipe->up_endpoint->ue_edesc->bEndpointAddress;
 	isread = UE_GET_DIR(endpt) == UE_DIR_IN;
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
+	mutex_enter(&sc->sc_lock);
 
 	/*
 	 * Use the pipe "tail" TD as our first and loan our first TD to the
@@ -3258,13 +3348,19 @@ ohci_device_intr_start(struct usbd_xfer 
 	data = opipe->tail.td;
 	opipe->tail.td = ox->ox_stds[0];
 	ox->ox_stds[0] = data;
+	data->held = &ox->ox_stds[0];
 	ohci_reset_std_chain(sc, xfer, len, isread, data, &last);
+	DPRINTFN(10, "xfer=%#jx new data=%#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)data, (uintptr_t)data->held, 0);
 
 	/* point at sentinel */
 	tail = opipe->tail.td;
 	memset(&tail->td, 0, sizeof(tail->td));
+	tail->held = &opipe->tail.td;
 	tail->nexttd = NULL;
 	tail->xfer = NULL;
+	DPRINTFN(10, "xfer=%#jx new tail=%#jx held at %#jx", (uintptr_t)ox,
+	    (uintptr_t)tail, (uintptr_t)tail->held, 0);
 	usb_syncmem(&tail->dma, tail->offs, sizeof(tail->td),
 	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 	xfer->ux_hcpriv = data;
@@ -3301,8 +3397,7 @@ ohci_device_intr_start(struct usbd_xfer 
 	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 
 	xfer->ux_status = USBD_IN_PROGRESS;
-	if (!polling)
-		mutex_exit(&sc->sc_lock);
+	mutex_exit(&sc->sc_lock);
 
 	return USBD_IN_PROGRESS;
 }
@@ -3467,8 +3562,10 @@ ohci_device_isoc_init(struct usbd_xfer *
 			goto fail;
 		}
 		ox->ox_sitds[i] = sitd;
+		sitd->held = &ox->ox_sitds[i];
 		sitd->xfer = xfer;
 		sitd->flags = 0;
+//		DPRINTFN(10, "xfer=%#jx new tail=%#jx held at %#jx", (uintptr_t)ox, (uintptr_t)tail, (uintptr_t)tail->held, 0);
 	}
 
 	return 0;
@@ -3562,6 +3659,7 @@ ohci_device_isoc_enter(struct usbd_xfer 
 	sitd = opipe->tail.itd;
 	opipe->tail.itd = ox->ox_sitds[0];
 	ox->ox_sitds[0] = sitd;
+	sitd->held = &ox->ox_sitds[0];
 
 	buf = DMAADDR(&xfer->ux_dmabuf, 0);
 	bp0 = OHCI_PAGE(buf);
@@ -3612,6 +3710,7 @@ ohci_device_isoc_enter(struct usbd_xfer 
 	/* point at sentinel */
 	tail = opipe->tail.itd;
 	memset(&tail->itd, 0, sizeof(tail->itd));
+	tail->held = &opipe->tail.itd;
 	tail->nextitd = NULL;
 	tail->xfer = NULL;
 	usb_syncmem(&tail->dma, tail->offs, sizeof(tail->itd),
Index: ohcivar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ohcivar.h,v
retrieving revision 1.58.10.1
diff -u -p -r1.58.10.1 ohcivar.h
--- ohcivar.h	25 Aug 2018 11:29:52 -0000	1.58.10.1
+++ ohcivar.h	30 Nov 2020 14:35:44 -0000
@@ -59,6 +59,9 @@ typedef struct ohci_soft_td {
 	uint16_t flags;
 #define OHCI_CALL_DONE	0x0001
 #define OHCI_ADD_LEN	0x0002
+#ifdef	OHCI_DEBUG
+	int beenthere;			/* loop detection */
+#endif
 } ohci_soft_td_t;
 #define OHCI_STD_SIZE ((sizeof(struct ohci_soft_td) + OHCI_TD_ALIGN - 1) / OHCI_TD_ALIGN * OHCI_TD_ALIGN)
 #define OHCI_STD_CHUNK 128
@@ -75,6 +78,9 @@ typedef struct ohci_soft_itd {
 	struct usbd_xfer *xfer;
 	uint16_t flags;
 	bool isdone;	/* used only when DIAGNOSTIC is defined */
+#ifdef	OHCI_DEBUG
+	int beenthere;			/* loop detection */
+#endif
 } ohci_soft_itd_t;
 #define OHCI_SITD_SIZE ((sizeof(struct ohci_soft_itd) + OHCI_ITD_ALIGN - 1) / OHCI_ITD_ALIGN * OHCI_ITD_ALIGN)
 #define OHCI_SITD_CHUNK 64
Index: ohci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ohci.c,v
retrieving revision 1.273.6.6
diff -u -p -r1.273.6.6 ohci.c
--- ohci.c	25 Feb 2020 18:52:44 -0000	1.273.6.6
+++ ohci.c	30 Nov 2020 14:35:44 -0000
@@ -230,6 +230,8 @@ Static void		ohci_dump_td(ohci_softc_t *
 Static void		ohci_dump_ed(ohci_softc_t *, ohci_soft_ed_t *);
 Static void		ohci_dump_itd(ohci_softc_t *, ohci_soft_itd_t *);
 Static void		ohci_dump_itds(ohci_softc_t *, ohci_soft_itd_t *);
+
+static int ohci_beenthere = 0;	/* td list loop detection */
 #endif
 
 #define OBARR(sc) bus_space_barrier((sc)->iot, (sc)->ioh, 0, (sc)->sc_size, \
@@ -693,6 +695,13 @@ ohci_reset_std_chain(ohci_softc_t *sc, s
 		DPRINTFN(2, "add 0 xfer", 0, 0, 0, 0);
 	}
 
+#ifdef OHCI_DEBUG
+	DPRINTFN(10, "--- dump start ---", 0, 0, 0, 0);
+	if (ohcidebug >= 10)
+		ohci_dump_td(sc, sp);
+	DPRINTFN(10, "--- dump end ---", 0, 0, 0, 0);
+#endif
+
 	/* Last TD gets usb_syncmem'ed by caller */
 	*ep = cur;
 }
@@ -1410,9 +1419,26 @@ ohci_softintr(void *v)
 	OWRITE4(sc, OHCI_INTERRUPT_ENABLE, OHCI_WDH);
 
 	/* Reverse the done list. */
+#ifdef OHCI_DEBUG
+	ohci_beenthere++;
+#endif
 	for (sdone = NULL, sidone = NULL; done != 0; ) {
+		DPRINTFN(10, "done=%#jx", (uintptr_t)done, 0, 0, 0);
 		std = ohci_hash_find_td(sc, done);
 		if (std != NULL) {
+#ifdef OHCI_DEBUG
+			if (ohcidebug >= 10) 
+				ohci_dump_td(sc, std);
+			if (std->beenthere == ohci_beenthere) {
+				DPRINTFN(1, "circular sdone: %#jx->%#jx", (uintptr_t)done, (uintptr_t)std->physaddr, 0, 0);
+				device_printf(sc->sc_dev, "circular sdone: %#jx->%#jx\n", (uintptr_t)done, (uintptr_t)std->physaddr);
+#if 0
+				panic("circular sdone");
+#endif
+				break;
+			}
+			std->beenthere = ohci_beenthere;
+#endif
 			usb_syncmem(&std->dma, std->offs, sizeof(std->td),
 			    BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
 			std->dnext = sdone;
@@ -1423,6 +1449,21 @@ ohci_softintr(void *v)
 		}
 		sitd = ohci_hash_find_itd(sc, done);
 		if (sitd != NULL) {
+#ifdef OHCI_DEBUG
+/* XXX no ohci_dump_itd() yet
+			if (ohcidebug >= 10) 
+				ohci_dump_itd(sc, sitd);
+*/
+			if (sitd->beenthere == ohci_beenthere) {
+				DPRINTFN(1, "circular sidone: %#jx->%#jx", (uintptr_t)done, (uintptr_t)sitd->physaddr, 0, 0);
+				device_printf(sc->sc_dev, "circular sidone: %#jx->%#jx\n", (uintptr_t)done, (uintptr_t)sitd->physaddr);
+#if 0
+				panic("circular sidone");
+#endif
+				break;
+			}
+			sitd->beenthere = ohci_beenthere;
+#endif
 			usb_syncmem(&sitd->dma, sitd->offs, sizeof(sitd->itd),
 			    BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
 			sitd->dnext = sidone;
@@ -1445,6 +1486,7 @@ ohci_softintr(void *v)
 		for (std = sdone; std; std = std->dnext)
 			ohci_dump_td(sc, std);
 	}
+/* XXX dump sidone list */
 #endif
 	DPRINTFN(10, "--- TD dump end ---", 0, 0, 0, 0);
 
@@ -1838,6 +1880,15 @@ ohci_hash_add_td(ohci_softc_t *sc, ohci_
 
 	KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock));
 
+#ifdef OHCI_DEBUG
+	for (ohci_soft_td_t *std2 = LIST_FIRST(&sc->sc_hash_tds[h]);
+	     std2 != NULL;
+	     std2 = LIST_NEXT(std2, hnext)) {
+		if (std2->physaddr == std->physaddr)
+			panic("OHCI: duplicate physaddr");
+	}
+#endif
+
 	LIST_INSERT_HEAD(&sc->sc_hash_tds[h], std, hnext);
 }
 
@@ -1945,7 +1996,11 @@ ohci_timeout_task(void *addr)
 void
 ohci_dump_tds(ohci_softc_t *sc, ohci_soft_td_t *std)
 {
+	ohci_beenthere++;
 	for (; std; std = std->nexttd) {
+		if (std->beenthere == ohci_beenthere)
+			panic("circular tds");
+		std->beenthere = ohci_beenthere;
 		ohci_dump_td(sc, std);
 		KASSERTMSG(std->nexttd == NULL || std != std->nexttd,
 		    "std %p next %p", std, std->nexttd);
@@ -1973,6 +2028,7 @@ ohci_dump_td(ohci_softc_t *sc, ohci_soft
 	       (u_long)O32TOH(std->td.td_cbp),
 	       (u_long)O32TOH(std->td.td_nexttd),
 	       (u_long)O32TOH(std->td.td_be), 0);
+	DPRINTF("    nexttd=%#jx", (uintptr_t)std->nexttd, 0, 0, 0);
 }
 
 void


Home | Main Index | Thread Index | Old Index