NetBSD-Bugs archive

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

kern/55835: circular done list in ohci_softint()



>Number:         55835
>Category:       kern
>Synopsis:       circular done list in ohci_softint()
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Dec 01 18:20:00 +0000 2020
>Originator:     Edgar FuÃ?
>Release:        NetBSD 8.2_STABLE
>Organization:
Mathematisches Institut der Universität Bonn
>Environment:
	
	
System: NetBSD trave.math.uni-bonn.de 8.2_STABLE NetBSD 8.2_STABLE (MI-Server) #13: Tue Dec 1 12:39:36 CET 2020 support%trave.math.uni-bonn.de@localhost:/var/work/obj-8/sys/arch/amd64/compile/miserv-DEBUG amd64
Architecture: x86_64
Machine: amd64
>Description:
	Running a libusb test program with Masterguard A700-19 UPSs connected via USB stalls the system the second time run. The kernel loops endlessly in the "Reverse the done list" part in ohci_softint() because the td_nexttd linking (not the nexttd linking) is circular. Most probably, a previously aborted TD has been recycled by the driver, but the controller is still in command of it and puts it onto the done list. Then, so to speak, two incarnations of the same TD are on the done list, leading to the circular list.
>How-To-Repeat:
	Run this:

	#include <err.h>
	#include <stdio.h>
	#include <usb.h>

	struct usb_bus *bus;
	struct usb_device *dev;
	usb_dev_handle *udev;

	int main(int argc, char *argv[]) {
		puts("init"); fflush(stdout);
		usb_init();
		puts("find_busses"); fflush(stdout);
		usb_find_busses();
		puts("find_devices"); fflush(stdout);
		usb_find_devices();

		puts("scan"); fflush(stdout);
		for (bus = usb_busses; bus; bus = bus->next) {
			puts(bus->dirname); fflush(stdout);
			for (dev = bus->devices; dev; dev = dev->next) {
				printf("%d: %s\n", dev->devnum, dev->filename); fflush(stdout);
				udev = usb_open(dev);
				if (!udev) {
					warnx("usb_open: %s", usb_strerror());
					continue;
				}
				printf("%0x %0x %0x\n", dev->descriptor.idVendor, dev->descriptor.idProduct, dev->descriptor.bcdDevice);
	#if 0
				if (usb_claim_interface(udev, 0) < 0) {
					errx(1, "usb_claim: %s", usb_strerror());
				}
	#endif
				usb_close(udev);
			}
		}
		return 0;
	}

	twice with a Masterguard A700-19 connected via USB. Unfortunately, these devices are out of production for a century.

	Use this:

	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

	diff to verify the list has gone circular and rule out that a physaddr has been allocated twice plus break out of the endless loop (or panic).
>Fix:
	Backported (to -8) from the nusb branch, ohci.c 1.254.2.76 (thanks, nick@!):

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

>Unformatted:
 	
 	


Home | Main Index | Thread Index | Old Index