shithub: riscv

Download patch

ref: 808480f76bedbbe4215da11de31f7403de668b2a
parent: 1556afae40c09034f2e1e53570c61593912c4fd4
author: cinap_lenrek <[email protected]>
date: Sun Aug 25 18:03:02 EDT 2013 

usbehci, usbuhci: paranoia

double the td abort delay and make sure the tsleep() isnt
shortened by a pending note. in that case, tsleep() would
raise error(Eintr); immidiately and would not sleep the
requested amount potentially cauing us to release active
dma memory too early! so we wrap the tsleep() call in a
while(waserror()) so we will at least wait the Abortdelay
amount if error is raised.

also, only try to idle the still active td's.

do not copy data in epio() when there was an error, theres
no reason to touch user buffer in that case.

for uhci, we also check that theres not more data in the
buffers than requested to avoid overflowing user buffer
in epio(). this should not happen but we'r paranoid.

for ehci, we also halt the queue head first in aborttds().
mark the queue heads as Qfree after unlinking and remove
some silly nil checks that are impossible.


--- a/sys/src/9/pc/usbuhci.c
+++ b/sys/src/9/pc/usbuhci.c
@@ -30,7 +30,7 @@
 {
 	Resetdelay	= 100,		/* delay after a controller reset (ms) */
 	Enabledelay	= 100,		/* waiting for a port to enable */
-	Abortdelay	= 5,		/* delay after cancelling Tds (ms) */
+	Abortdelay	= 10,		/* delay after cancelling Tds (ms) */
 	Incr		= 64,		/* for Td and Qh pools */
 
 	Tdatomic	= 8,		/* max nb. of Tds per bulk I/O op. */
@@ -704,12 +704,8 @@
 qhfree(Ctlr *ctlr, Qh *qh)
 {
 	Td *td;
-	Td *ltd;
 	Qh *q;
 
-	if(qh == nil)
-		return;
-
 	ilock(ctlr);
 	for(q = ctlr->qhs; q != nil; q = q->next)
 		if(q->next == qh)
@@ -718,14 +714,15 @@
 		panic("qhfree: nil q");
 	q->next = qh->next;
 	q->link = qh->link;
+	qh->state = Qfree;	/* paranoia */
 	iunlock(ctlr);
 
-	for(td = qh->tds; td != nil; td = ltd){
-		ltd = td->next;
+	while((td = qh->tds) != nil){
+		qh->tds = td->next;
 		tdfree(td);
 	}
+
 	lock(&qhpool);
-	qh->state = Qfree;	/* paranoia */
 	qh->next = qhpool.free;
 	qh->tag = nil;
 	qh->io = nil;
@@ -961,7 +958,7 @@
 	OUTS(Status, sts & Sall);
 	cmd = INS(Cmd);
 	if(cmd & Crun == 0){
-		print("uhci %#ux: not running: uhci bug?\n", ctlr->port);
+		iprint("uhci %#ux: not running: uhci bug?\n", ctlr->port);
 		/* BUG: should abort everything in this case */
 	}
 	if(debug > 1){
@@ -968,7 +965,7 @@
 		frptr = INL(Flbaseadd);
 		frno = INL(Frnum);
 		frno = TRUNC(frno, Nframes);
-		print("cmd %#ux sts %#ux frptr %#ux frno %d\n",
+		iprint("cmd %#ux sts %#ux frptr %#ux frno %d\n",
 			cmd, sts, frptr, frno);
 	}
 	ctlr->ntdintr++;
@@ -1220,12 +1217,14 @@
 {
 	Td *td;
 
-	qh->state = Qdone;
 	qh->elink = QHterm;
+	coherence();
 	for(td = qh->tds; td != nil; td = td->next){
-		if(td->csw & Tdactive)
+		if(td->csw & Tdactive){
 			td->ndata = 0;
-		td->csw &= ~(Tdactive|Tdioc);
+			td->csw &= ~(Tdactive|Tdioc);
+			coherence();
+		}
 	}
 }
 
@@ -1263,13 +1262,15 @@
 	else if(qh->state != Qdone && qh->state != Qclose)
 		panic("epio: queue not done and not closed");
 	if(timedout){
-		aborttds(io->qh);
-		io->err = "request timed out";
+		aborttds(qh);
+		qh->state = Qdone;
+		if(io->err == nil)
+			io->err = "request timed out";
 		iunlock(ctlr);
-		if(!waserror()){
-			tsleep(&up->sleep, return0, 0, Abortdelay);
-			poperror();
-		}
+		while(waserror())
+			;
+		tsleep(&up->sleep, return0, 0, Abortdelay);
+		poperror();
 		ilock(ctlr);
 	}
 	if(qh->state != Qclose)
@@ -1297,7 +1298,6 @@
 	ulong load;
 	char *err;
 
-	qh = io->qh;
 	ctlr = ep->hp->aux;
 	io->debug = ep->debug;
 	tmout = ep->tmout;
@@ -1317,7 +1317,8 @@
 	}
 	io->err = nil;
 	ilock(ctlr);
-	if(qh->state == Qclose){	/* Tds released by cancelio */
+	qh = io->qh;
+	if(qh == nil || qh->state == Qclose){	/* Tds released by cancelio */
 		iunlock(ctlr);
 		error(io->err ? io->err : Eio);
 	}
@@ -1365,6 +1366,8 @@
 	if(debug > 1 || ep->debug > 1)
 		dumptd(td0, "epio: got tds: ");
 
+	err = io->err;
+
 	tot = 0;
 	c = a;
 	saved = 0;
@@ -1380,16 +1383,22 @@
 			if(saved++ == 0)
 				io->toggle = td->token & Tddata1;
 		}else{
-			tot += td->ndata;
-			if(c != nil && tdtok(td) == Tdtokin && td->ndata > 0){
-				memmove(c, td->data, td->ndata);
-				c += td->ndata;
+			n = td->ndata;
+			if(err == nil && n < 0)
+				err = Eio;
+			if(err == nil && n > 0 && tot < count){
+				if((tot + n) > count)
+					n = count - tot;
+				if(c != nil && tdtok(td) == Tdtokin){
+					memmove(c, td->data, n);
+					c += n;
+				}
+				tot += n;
 			}
 		}
 		ntd = td->next;
 		tdfree(td);
 	}
-	err = io->err;
 	if(mustlock){
 		qunlock(io);
 		poperror();
@@ -1398,8 +1407,6 @@
 		io, ntds, tot, err);
 	if(err != nil)
 		error(err);
-	if(tot < 0)
-		error(Eio);
 	return tot;
 }
 
@@ -1817,7 +1824,7 @@
 
 	ilock(ctlr);
 	qh = io->qh;
-	if(io == nil || io->qh == nil || io->qh->state == Qclose){
+	if(qh == nil || qh->state == Qclose){
 		iunlock(ctlr);
 		return;
 	}
@@ -1826,18 +1833,20 @@
 	aborttds(qh);
 	qh->state = Qclose;
 	iunlock(ctlr);
-	if(!waserror()){
-		tsleep(&up->sleep, return0, 0, Abortdelay);
-		poperror();
-	}
 
+	while(waserror())
+		;
+	tsleep(&up->sleep, return0, 0, Abortdelay);
+	poperror();
+
 	wakeup(io);
 	qlock(io);
 	/* wait for epio if running */
+	if(io->qh == qh)
+		io->qh = nil;
 	qunlock(io);
 
 	qhfree(ctlr, qh);
-	io->qh = nil;
 }
 
 static void
--- a/sys/src/9/port/usbehci.c
+++ b/sys/src/9/port/usbehci.c
@@ -53,7 +53,7 @@
 	Qfree,
 
 	Enabledelay	= 100,		/* waiting for a port to enable */
-	Abortdelay	= 5,		/* delay after cancelling Tds (ms) */
+	Abortdelay	= 10,		/* delay after cancelling Tds (ms) */
 
 	Incr		= 64,		/* for pools of Tds, Qhs, etc. */
 	Align		= 128,		/* in bytes for all those descriptors */
@@ -739,11 +739,12 @@
 	qlock(&ctlr->portlck);
 	ctlr->opio->cmd |= Ciasync;	/* ask for intr. on async advance */
 	coherence();
-	for(i = 0; i < 3 && qhadvanced(ctlr) == 0; i++)
-		if(!waserror()){
-			tsleep(ctlr, qhadvanced, ctlr, Abortdelay);
-			poperror();
-		}
+	for(i = 0; i < 3 && qhadvanced(ctlr) == 0; i++){
+		while(waserror())
+			;
+		tsleep(ctlr, qhadvanced, ctlr, Abortdelay);
+		poperror();
+	}
 	dprint("ehci: qhcoherency: doorbell %d\n", qhadvanced(ctlr));
 	if(i == 3)
 		print("ehci: async advance doorbell did not ring\n");
@@ -754,11 +755,9 @@
 static void
 qhfree(Ctlr *ctlr, Qh *qh)
 {
-	Td *td, *ltd;
+	Td *td;
 	Qh *q;
 
-	if(qh == nil)
-		return;
 	ilock(ctlr);
 	if(qh->sched < 0){
 		for(q = ctlr->qhs; q != nil; q = q->next)
@@ -771,12 +770,13 @@
 		coherence();
 	}else
 		unschedq(ctlr, qh);
+	qh->state = Qfree;	/* paranoia */
 	iunlock(ctlr);
 
 	qhcoherency(ctlr);
 
-	for(td = qh->tds; td != nil; td = ltd){
-		ltd = td->next;
+	while((td = qh->tds) != nil){
+		qh->tds = td->next;
 		tdfree(td);
 	}
 
@@ -1512,9 +1512,8 @@
 }
 
 static int
-ehciintr(Hci *hp)
+ctlrinterrupt(Ctlr *ctlr)
 {
-	Ctlr *ctlr;
 	Eopio *opio;
 	Isoio *iso;
 	ulong sts;
@@ -1521,27 +1520,22 @@
 	Qh *qh;
 	int i, some;
 
-	ctlr = hp->aux;
 	opio = ctlr->opio;
-
 	/*
 	 * Will we know in USB 3.0 who the interrupt was for?.
 	 * Do they still teach indexing in CS?
 	 * This is Intel's doing.
 	 */
-	ilock(ctlr);
-	ctlr->nintr++;
 	sts = opio->sts & Sintrs;
-	if(sts == 0){		/* not ours; shared intr. */
-		iunlock(ctlr);
+	if(sts == 0)		/* not ours; shared intr. */
 		return 0;
-	}
 	opio->sts = sts;
 	coherence();
+	ctlr->nintr++;
 	if((sts & Sherr) != 0)
-		print("ehci: port %#p fatal host system error\n", ctlr->capio);
+		iprint("ehci: port %#p fatal host system error\n", ctlr->capio);
 	if((sts & Shalted) != 0)
-		print("ehci: port %#p: halted\n", ctlr->capio);
+		iprint("ehci: port %#p: halted\n", ctlr->capio);
 	if((sts & Sasync) != 0){
 		dprint("ehci: doorbell\n");
 		wakeup(ctlr);
@@ -1555,12 +1549,12 @@
 	if((sts & (Serrintr|Sintr)) != 0){
 		ctlr->ntdintr++;
 		if(ehcidebug > 1){
-			print("ehci port %#p frames %#p nintr %d ntdintr %d",
+			iprint("ehci port %#p frames %#p nintr %d ntdintr %d",
 				ctlr->capio, ctlr->frames,
 				ctlr->nintr, ctlr->ntdintr);
-			print(" nqhintr %d nisointr %d\n",
+			iprint(" nqhintr %d nisointr %d\n",
 				ctlr->nqhintr, ctlr->nisointr);
-			print("\tcmd %#lux sts %#lux intr %#lux frno %uld",
+			iprint("\tcmd %#lux sts %#lux intr %#lux frno %uld",
 				opio->cmd, opio->sts, opio->intr, opio->frno);
 		}
 
@@ -1588,8 +1582,20 @@
 			qh = qh->next;
 		}while(qh != ctlr->qhs && i++ < 100);
 		if(i > 100)
-			print("echi: interrupt: qh loop?\n");
+			iprint("echi: interrupt: qh loop?\n");
 	}
+	return some;
+}
+
+static int
+ehciintr(Hci *hp)
+{
+	Ctlr *ctlr;
+	int some;
+
+	ctlr = hp->aux;
+	ilock(ctlr);
+	some = ctlrinterrupt(ctlr);
 	iunlock(ctlr);
 	return some;
 }
@@ -1694,7 +1700,7 @@
 	for(i = 0; *portscp & Psreset && i < 10; i++)
 		delay(10);
 	if (*portscp & Psreset)
-		iprint("ehci %#p: port %d didn't reset within %d ms; sts %#lux\n",
+		if(0) iprint("ehci %#p: port %d didn't reset within %d ms; sts %#lux\n",
 			ctlr->capio, port, i * 10, *portscp);
 	*portscp &= ~Psreset;		/* force appearance of reset done */
 	coherence();
@@ -2180,16 +2186,16 @@
 {
 	Td *td;
 
-	qh->state = Qdone;
-	coherence();
 	if(qh->sched >= 0 && (qh->eps0 & Qhspeedmask) != Qhhigh)
 		qh->eps0 |= Qhint;	/* inactivate on next pass */
+	qh->csw = (qh->csw & ~Tdactive) | Tdhalt;
 	coherence();
 	for(td = qh->tds; td != nil; td = td->next){
-		if(td->csw & Tdactive)
+		if(td->csw & Tdactive){
 			td->ndata = 0;
-		td->csw |= Tdhalt;
-		coherence();
+			td->csw |= Tdhalt;
+			coherence();
+		}
 	}
 }
 
@@ -2288,9 +2294,7 @@
 	ilock(ctlr);
 	/* Are we missing interrupts? */
 	if(qh->state == Qrun){
-		iunlock(ctlr);
-		ehciintr(hp);
-		ilock(ctlr);
+		ctlrinterrupt(ctlr);
 		if(qh->state == Qdone){
 			dqprint("ehci %#p: polling required\n", ctlr->capio);
 			ctlr->poll.must = 1;
@@ -2304,18 +2308,19 @@
 	}else if(qh->state != Qdone && qh->state != Qclose)
 		panic("ehci: epio: queue state %d", qh->state);
 	if(timedout){
-		aborttds(io->qh);
-		io->err = "request timed out";
+		aborttds(qh);
+		qh->state = Qdone;
+		if(io->err == nil)
+			io->err = "request timed out";
 		iunlock(ctlr);
-		if(!waserror()){
-			tsleep(&up->sleep, return0, 0, Abortdelay);
-			poperror();
-		}
+		while(waserror())
+			;
+		tsleep(&up->sleep, return0, 0, Abortdelay);
+		poperror();
 		ilock(ctlr);
 	}
 	if(qh->state != Qclose)
 		qh->state = Qidle;
-	coherence();
 	qhlinktd(qh, nil);
 	ctlr->load -= load;
 	ctlr->nreqs--;
@@ -2340,7 +2345,6 @@
 	Qh* qh;
 	Td *td, *ltd, *td0, *ntd;
 
-	qh = io->qh;
 	ctlr = ep->hp->aux;
 	io->debug = ep->debug;
 	tmout = ep->tmout;
@@ -2360,7 +2364,8 @@
 	}
 	io->err = nil;
 	ilock(ctlr);
-	if(qh->state == Qclose){	/* Tds released by cancelio */
+	qh = io->qh;
+	if(qh == nil || qh->state == Qclose){	/* Tds released by cancelio */
 		iunlock(ctlr);
 		error(io->err ? io->err : Eio);
 	}
@@ -2417,6 +2422,7 @@
 		dumptd(td0, "epio: got: ");
 		qhdump(qh);
 	}
+	err = io->err;
 
 	tot = 0;
 	c = a;
@@ -2438,7 +2444,7 @@
 				io->toggle = td->csw & Tddata1;
 				coherence();
 			}
-			if((n = td->ndata) > 0 && tot < count){
+			if(err == nil && (n = td->ndata) > 0 && tot < count){
 				if((tot + n) > count)
 					n = count - tot;
 				if(c != nil && (td->csw & Tdtok) == Tdtokin){
@@ -2451,7 +2457,6 @@
 		ntd = td->next;
 		tdfree(td);
 	}
-	err = io->err;
 	if(mustlock){
 		qunlock(io);
 		poperror();
@@ -2955,7 +2960,7 @@
 
 	ilock(ctlr);
 	qh = io->qh;
-	if(io == nil || io->qh == nil || io->qh->state == Qclose){
+	if(qh == nil || qh->state == Qclose){
 		iunlock(ctlr);
 		return;
 	}
@@ -2964,17 +2969,18 @@
 	aborttds(qh);
 	qh->state = Qclose;
 	iunlock(ctlr);
-	if(!waserror()){
-		tsleep(&up->sleep, return0, 0, Abortdelay);
-		poperror();
-	}
+	while(waserror())
+		;
+	tsleep(&up->sleep, return0, 0, Abortdelay);
+	poperror();
 	wakeup(io);
 	qlock(io);
 	/* wait for epio if running */
+	if(io->qh == qh)
+		io->qh = nil;
 	qunlock(io);
 
 	qhfree(ctlr, qh);
-	io->qh = nil;
 }
 
 static void