On 09/03/17(Thu) 15:04, Mark Kettenis wrote:
> > Date: Thu, 9 Mar 2017 14:32:29 +0100
> > From: Martin Pieuchot <m...@openbsd.org>
> >
> > Diff below move per HC driver polling code to the stack. I know this
> > code contains a use-after-free and this will be addressed in a later
> > diff. For the moment merge 4 copies into 1 such that I don't have to
> > fix all of them.
> >
> > As a side effect, this fix xhci(4) polling mode. That means you can
> > now use ddb(4) with an xhci(4)-attached keyboard.
> >
> > ok?
>
> Looks like a good idea to me. Hwoever...
> [...]
> > + if (polling) {
> > + int timo;
> > +
> > + for (timo = xfer->timeout; timo >= 0; timo--) {
> > + usb_delay_ms(bus, 1);
>
> The xxx_waitintr() implementations have a sc->sc_bus.dying check here.
> I'd expect that that check would still be necessary...
That check is needed when coming back from a sleep. In polling mode
usb_delay_ms() calls delay(9) so we know that all the resources are
still there.
But it might be better to leave that for another diff. Revised diff
below.
Index: usbdi.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.87
diff -u -p -r1.87 usbdi.c
--- usbdi.c 6 Mar 2017 12:13:58 -0000 1.87
+++ usbdi.c 9 Mar 2017 14:58:44 -0000
@@ -279,6 +279,8 @@ usbd_status
usbd_transfer(struct usbd_xfer *xfer)
{
struct usbd_pipe *pipe = xfer->pipe;
+ struct usbd_bus *bus = pipe->device->bus;
+ int polling = bus->use_polling;
usbd_status err;
int flags, s;
@@ -298,8 +300,6 @@ usbd_transfer(struct usbd_xfer *xfer)
/* If there is no buffer, allocate one. */
if ((xfer->rqflags & URQ_DEV_DMABUF) == 0) {
- struct usbd_bus *bus = pipe->device->bus;
-
#ifdef DIAGNOSTIC
if (xfer->rqflags & URQ_AUTO_DMABUF)
printf("usbd_transfer: has old buffer!\n");
@@ -325,8 +325,6 @@ usbd_transfer(struct usbd_xfer *xfer)
if (err != USBD_IN_PROGRESS && err) {
/* The transfer has not been queued, so free buffer. */
if (xfer->rqflags & URQ_AUTO_DMABUF) {
- struct usbd_bus *bus = pipe->device->bus;
-
usb_freemem(bus, &xfer->dmabuf);
xfer->rqflags &= ~URQ_AUTO_DMABUF;
}
@@ -338,19 +336,40 @@ usbd_transfer(struct usbd_xfer *xfer)
/* Sync transfer, wait for completion. */
if (err != USBD_IN_PROGRESS)
return (err);
+
s = splusb();
- while (!xfer->done) {
- if (pipe->device->bus->use_polling)
- panic("usbd_transfer: not done");
- flags = PRIBIO | (xfer->flags & USBD_CATCH ? PCATCH : 0);
-
- err = tsleep(xfer, flags, "usbsyn", 0);
- if (err && !xfer->done) {
- usbd_abort_pipe(pipe);
- if (err == EINTR)
- xfer->status = USBD_INTERRUPTED;
- else
- xfer->status = USBD_TIMEOUT;
+ if (polling) {
+ int timo;
+
+ for (timo = xfer->timeout; timo >= 0; timo--) {
+ usb_delay_ms(bus, 1);
+ if (bus->dying) {
+ xfer->status = USBD_IOERROR;
+ usb_transfer_complete(xfer);
+ break;
+ }
+
+ usbd_dopoll(pipe->device);
+ if (xfer->done)
+ break;
+ }
+
+ if (timo < 0) {
+ xfer->status = USBD_TIMEOUT;
+ usb_transfer_complete(xfer);
+ }
+ } else {
+ while (!xfer->done) {
+ flags = PRIBIO|(xfer->flags & USBD_CATCH ? PCATCH : 0);
+
+ err = tsleep(xfer, flags, "usbsyn", 0);
+ if (err && !xfer->done) {
+ usbd_abort_pipe(pipe);
+ if (err == EINTR)
+ xfer->status = USBD_INTERRUPTED;
+ else
+ xfer->status = USBD_TIMEOUT;
+ }
}
}
splx(s);
Index: ehci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.195
diff -u -p -r1.195 ehci.c
--- ehci.c 8 Nov 2016 10:31:30 -0000 1.195
+++ ehci.c 9 Mar 2017 13:19:42 -0000
@@ -117,7 +117,6 @@ int ehci_setaddr(struct usbd_device *,
void ehci_poll(struct usbd_bus *);
void ehci_softintr(void *);
int ehci_intr1(struct ehci_softc *);
-void ehci_waitintr(struct ehci_softc *, struct usbd_xfer *);
void ehci_check_intr(struct ehci_softc *, struct usbd_xfer *);
void ehci_check_qh_intr(struct ehci_softc *, struct usbd_xfer *);
void ehci_check_itd_intr(struct ehci_softc *, struct usbd_xfer *);
@@ -918,37 +917,6 @@ ehci_idone(struct usbd_xfer *xfer)
DPRINTFN(/*12*/2, ("ehci_idone: ex=%p done\n", ex));
}
-/*
- * Wait here until controller claims to have an interrupt.
- * Then call ehci_intr and return. Use timeout to avoid waiting
- * too long.
- */
-void
-ehci_waitintr(struct ehci_softc *sc, struct usbd_xfer *xfer)
-{
- int timo;
- u_int32_t intrs;
-
- xfer->status = USBD_IN_PROGRESS;
- for (timo = xfer->timeout; timo >= 0; timo--) {
- usb_delay_ms(&sc->sc_bus, 1);
- if (sc->sc_bus.dying)
- break;
- intrs = EHCI_STS_INTRS(EOREAD4(sc, EHCI_USBSTS)) &
- sc->sc_eintrs;
- if (intrs) {
- ehci_intr1(sc);
- if (xfer->status != USBD_IN_PROGRESS)
- return;
- }
- }
-
- /* Timeout */
- xfer->status = USBD_TIMEOUT;
- usb_transfer_complete(xfer);
- /* XXX should free TD */
-}
-
void
ehci_poll(struct usbd_bus *bus)
{
@@ -2973,9 +2941,6 @@ ehci_device_ctrl_start(struct usbd_xfer
xfer->status = USBD_IN_PROGRESS;
splx(s);
- if (sc->sc_bus.use_polling)
- ehci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
bad3:
@@ -3073,9 +3038,6 @@ ehci_device_bulk_start(struct usbd_xfer
xfer->status = USBD_IN_PROGRESS;
splx(s);
- if (sc->sc_bus.use_polling)
- ehci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
}
@@ -3188,9 +3150,6 @@ ehci_device_intr_start(struct usbd_xfer
TAILQ_INSERT_TAIL(&sc->sc_intrhead, ex, inext);
xfer->status = USBD_IN_PROGRESS;
splx(s);
-
- if (sc->sc_bus.use_polling)
- ehci_waitintr(sc, xfer);
return (USBD_IN_PROGRESS);
}
Index: ohci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ohci.c,v
retrieving revision 1.147
diff -u -p -r1.147 ohci.c
--- ohci.c 15 Sep 2016 02:00:17 -0000 1.147
+++ ohci.c 9 Mar 2017 13:22:37 -0000
@@ -90,7 +90,6 @@ usbd_status ohci_open(struct usbd_pipe *
int ohci_setaddr(struct usbd_device *, int);
void ohci_poll(struct usbd_bus *);
void ohci_softintr(void *);
-void ohci_waitintr(struct ohci_softc *, struct usbd_xfer *);
void ohci_add_done(struct ohci_softc *, ohci_physaddr_t);
void ohci_rhsc(struct ohci_softc *, struct usbd_xfer *);
@@ -1506,42 +1505,6 @@ ohci_root_ctrl_done(struct usbd_xfer *xf
{
}
-/*
- * Wait here until controller claims to have an interrupt.
- * Then call ohci_intr and return. Use timeout to avoid waiting
- * too long.
- */
-void
-ohci_waitintr(struct ohci_softc *sc, struct usbd_xfer *xfer)
-{
- int timo;
- u_int32_t intrs;
-
- xfer->status = USBD_IN_PROGRESS;
- for (timo = xfer->timeout; timo >= 0; timo--) {
- usb_delay_ms(&sc->sc_bus, 1);
- if (sc->sc_bus.dying)
- break;
- intrs = OREAD4(sc, OHCI_INTERRUPT_STATUS) & sc->sc_eintrs;
- DPRINTFN(15,("ohci_waitintr: 0x%04x\n", intrs));
-#ifdef OHCI_DEBUG
- if (ohcidebug > 15)
- ohci_dumpregs(sc);
-#endif
- if (intrs) {
- ohci_intr1(sc);
- if (xfer->status != USBD_IN_PROGRESS)
- return;
- }
- }
-
- /* Timeout */
- DPRINTF(("ohci_waitintr: timeout\n"));
- xfer->status = USBD_TIMEOUT;
- usb_transfer_complete(xfer);
- /* XXX should free TD */
-}
-
void
ohci_poll(struct usbd_bus *bus)
{
@@ -2686,9 +2649,6 @@ ohci_device_ctrl_start(struct usbd_xfer
if (err)
return (err);
- if (sc->sc_bus.use_polling)
- ohci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
}
@@ -2826,9 +2786,6 @@ ohci_device_bulk_start(struct usbd_xfer
splx(s);
- if (sc->sc_bus.use_polling)
- ohci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
}
@@ -2944,9 +2901,6 @@ ohci_device_intr_start(struct usbd_xfer
#endif
splx(s);
- if (sc->sc_bus.use_polling)
- ohci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
}
@@ -3219,13 +3173,6 @@ ohci_device_isoc_start(struct usbd_xfer
if (xfer->status != USBD_IN_PROGRESS)
printf("ohci_device_isoc_start: not in progress %p\n", xfer);
#endif
-
- /* XXX anything to do? */
-
- if (sc->sc_bus.use_polling) {
- DPRINTF(("Starting ohci isoc xfer with polling. Bad idea?\n"));
- ohci_waitintr(sc, xfer);
- }
return (USBD_IN_PROGRESS);
}
Index: uhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhci.c,v
retrieving revision 1.140
diff -u -p -r1.140 uhci.c
--- uhci.c 2 Feb 2017 22:31:05 -0000 1.140
+++ uhci.c 9 Mar 2017 13:22:56 -0000
@@ -119,7 +119,6 @@ usbd_status uhci_alloc_std_chain(struct
struct usbd_xfer *, struct uhci_soft_td **,
struct uhci_soft_td **);
void uhci_poll_hub(void *);
-void uhci_waitintr(struct uhci_softc *, struct usbd_xfer *);
void uhci_check_intr(struct uhci_softc *, struct usbd_xfer *);
void uhci_idone(struct usbd_xfer *);
@@ -1337,38 +1336,6 @@ uhci_timeout_task(void *addr)
splx(s);
}
-/*
- * Wait here until controller claims to have an interrupt.
- * Then call uhci_intr and return. Use timeout to avoid waiting
- * too long.
- * Only used during boot when interrupts are not enabled yet.
- */
-void
-uhci_waitintr(struct uhci_softc *sc, struct usbd_xfer *xfer)
-{
- int timo;
- u_int32_t intrs;
-
- xfer->status = USBD_IN_PROGRESS;
- for (timo = xfer->timeout; timo >= 0; timo--) {
- usb_delay_ms(&sc->sc_bus, 1);
- if (sc->sc_bus.dying)
- break;
- intrs = UREAD2(sc, UHCI_STS) & UHCI_STS_ALLINTRS;
- DPRINTFN(15,("uhci_waitintr: 0x%04x\n", intrs));
- if (intrs) {
- uhci_intr1(sc);
- if (xfer->status != USBD_IN_PROGRESS)
- return;
- }
- }
-
- /* Timeout */
- DPRINTF(("uhci_waitintr: timeout\n"));
- xfer->status = USBD_TIMEOUT;
- usb_transfer_complete(xfer);
-}
-
void
uhci_poll(struct usbd_bus *bus)
{
@@ -1707,9 +1674,6 @@ uhci_device_bulk_start(struct usbd_xfer
}
#endif
- if (sc->sc_bus.use_polling)
- uhci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
}
@@ -1841,9 +1805,6 @@ uhci_device_ctrl_start(struct usbd_xfer
if (err)
return (err);
- if (sc->sc_bus.use_polling)
- uhci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
}
@@ -1932,9 +1893,6 @@ uhci_device_intr_start(struct usbd_xfer
}
#endif
- if (sc->sc_bus.use_polling)
- uhci_waitintr(sc, xfer);
-
return (USBD_IN_PROGRESS);
}
@@ -2253,11 +2211,6 @@ uhci_device_isoc_start(struct usbd_xfer
uhci_add_intr_list(sc, ux);
splx(s);
-
- if (sc->sc_bus.use_polling) {
- DPRINTF(("Starting uhci isoc xfer with polling. Bad idea?\n"));
- uhci_waitintr(sc, xfer);
- }
return (USBD_IN_PROGRESS);
}
Index: xhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.70
diff -u -p -r1.70 xhci.c
--- xhci.c 8 Nov 2016 10:31:30 -0000 1.70
+++ xhci.c 9 Mar 2017 13:20:11 -0000
@@ -75,7 +75,6 @@ struct xhci_pipe {
int xhci_reset(struct xhci_softc *);
int xhci_intr1(struct xhci_softc *);
-void xhci_waitintr(struct xhci_softc *, struct usbd_xfer *);
void xhci_event_dequeue(struct xhci_softc *);
void xhci_event_xfer(struct xhci_softc *, uint64_t, uint32_t, uint32_t);
void xhci_event_command(struct xhci_softc *, uint64_t);
@@ -634,26 +633,6 @@ xhci_poll(struct usbd_bus *bus)
}
void
-xhci_waitintr(struct xhci_softc *sc, struct usbd_xfer *xfer)
-{
- int timo;
-
- for (timo = xfer->timeout; timo >= 0; timo--) {
- usb_delay_ms(&sc->sc_bus, 1);
- if (sc->sc_bus.dying)
- break;
-
- if (xfer->status != USBD_IN_PROGRESS)
- return;
-
- xhci_intr1(sc);
- }
-
- xfer->status = USBD_TIMEOUT;
- usb_transfer_complete(xfer);
-}
-
-void
xhci_softintr(void *v)
{
struct xhci_softc *sc = v;
@@ -2525,10 +2504,7 @@ xhci_device_ctrl_start(struct usbd_xfer
XDWRITE4(sc, XHCI_DOORBELL(xp->slot), xp->dci);
xfer->status = USBD_IN_PROGRESS;
-
- if (sc->sc_bus.use_polling)
- xhci_waitintr(sc, xfer);
- else if (xfer->timeout) {
+ if (xfer->timeout && !sc->sc_bus.use_polling) {
timeout_del(&xfer->timeout_handle);
timeout_set(&xfer->timeout_handle, xhci_timeout, xfer);
timeout_add_msec(&xfer->timeout_handle, xfer->timeout);
@@ -2645,10 +2621,7 @@ xhci_device_generic_start(struct usbd_xf
XDWRITE4(sc, XHCI_DOORBELL(xp->slot), xp->dci);
xfer->status = USBD_IN_PROGRESS;
-
- if (sc->sc_bus.use_polling)
- xhci_waitintr(sc, xfer);
- else if (xfer->timeout) {
+ if (xfer->timeout && !sc->sc_bus.use_polling) {
timeout_del(&xfer->timeout_handle);
timeout_set(&xfer->timeout_handle, xhci_timeout, xfer);
timeout_add_msec(&xfer->timeout_handle, xfer->timeout);
Index: dwc2/dwc2.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2.c,v
retrieving revision 1.41
diff -u -p -r1.41 dwc2.c
--- dwc2/dwc2.c 16 Feb 2017 14:09:00 -0000 1.41
+++ dwc2/dwc2.c 9 Mar 2017 13:26:13 -0000
@@ -94,7 +94,6 @@ STATIC usbd_status dwc2_open(struct usbd
STATIC int dwc2_setaddr(struct usbd_device *, int);
STATIC void dwc2_poll(struct usbd_bus *);
STATIC void dwc2_softintr(void *);
-STATIC void dwc2_waitintr(struct dwc2_softc *, struct usbd_xfer *);
#if 0
STATIC usbd_status dwc2_allocm(struct usbd_bus *, struct usb_dma *,
uint32_t);
@@ -406,38 +405,6 @@ dwc2_softintr(void *v)
}
STATIC void
-dwc2_waitintr(struct dwc2_softc *sc, struct usbd_xfer *xfer)
-{
- struct dwc2_hsotg *hsotg = sc->sc_hsotg;
- uint32_t intrs;
- int timo;
-
- xfer->status = USBD_IN_PROGRESS;
- for (timo = xfer->timeout; timo >= 0; timo--) {
- usb_delay_ms(&sc->sc_bus, 1);
- if (sc->sc_dying)
- break;
- intrs = dwc2_read_core_intr(hsotg);
-
- DPRINTFN(15, "0x%08x\n", intrs);
-
- if (intrs) {
- mtx_enter(&hsotg->lock);
- dwc2_interrupt(sc);
- mtx_leave(&hsotg->lock);
- if (xfer->status != USBD_IN_PROGRESS)
- return;
- }
- }
-
- /* Timeout */
- DPRINTF("timeout\n");
-
- xfer->status = USBD_TIMEOUT;
- usb_transfer_complete(xfer);
-}
-
-STATIC void
dwc2_timeout(void *addr)
{
struct usbd_xfer *xfer = addr;
@@ -987,7 +954,6 @@ dwc2_device_ctrl_transfer(struct usbd_xf
STATIC usbd_status
dwc2_device_ctrl_start(struct usbd_xfer *xfer)
{
- struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
usbd_status err;
DPRINTF("\n");
@@ -995,9 +961,6 @@ dwc2_device_ctrl_start(struct usbd_xfer
xfer->status = USBD_IN_PROGRESS;
err = dwc2_device_start(xfer);
- if (sc->sc_bus.use_polling)
- dwc2_waitintr(sc, xfer);
-
return err;
}
@@ -1044,7 +1007,6 @@ dwc2_device_bulk_transfer(struct usbd_xf
STATIC usbd_status
dwc2_device_bulk_start(struct usbd_xfer *xfer)
{
- struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
usbd_status err;
DPRINTF("xfer=%p\n", xfer);
@@ -1052,9 +1014,6 @@ dwc2_device_bulk_start(struct usbd_xfer
xfer->status = USBD_IN_PROGRESS;
err = dwc2_device_start(xfer);
- if (sc->sc_bus.use_polling)
- dwc2_waitintr(sc, xfer);
-
return err;
}
@@ -1103,15 +1062,10 @@ dwc2_device_intr_transfer(struct usbd_xf
STATIC usbd_status
dwc2_device_intr_start(struct usbd_xfer *xfer)
{
- struct dwc2_pipe *dpipe = DWC2_XFER2DPIPE(xfer)
- struct dwc2_softc *sc = DWC2_DPIPE2SC(dpipe);
usbd_status err;
xfer->status = USBD_IN_PROGRESS;
err = dwc2_device_start(xfer);
-
- if (sc->sc_bus.use_polling)
- dwc2_waitintr(sc, xfer);
return err;
}
