Module Name: src
Committed By: riastradh
Date: Wed Feb 12 16:02:01 UTC 2020
Modified Files:
src/sys/arch/mips/adm5120/dev: ahci.c
src/sys/dev/usb: ehci.c ohci.c uhci.c vhci.c xhci.c
src/sys/external/bsd/dwc2: dwc2.c
Log Message:
Fix steady state of root intr xfers.
Why?
- Avoid completing a root intr xfer multiple times in races.
- Avoid potential use-after-free in poll_hub callouts (uhci, ahci).
How?
- Use sc->sc_intr_xfer or equivalent to store only a pending xfer
that has not yet completed -- whether successfully, by timeout, or
by synchronous abort. When any of those happens, set it to null
under the lock, so the xfer is completed only once.
- For hci drivers that use a callout to poll the root hub (uhci, ahci):
. Pass the softc pointer, not the xfer, to the callout, so the
callout is not even tempted to use xfer after free -- if the
callout fires, but the xfer is synchronously aborted before the
callout can do anything, the xfer might be freed by the time the
callout starts to examine it.
. Teach the callout to do nothing if it is callout_pending after it
has fired. This way:
1. completion or synchronous abort can just callout_stop
2. start can just callout_schedule
If the callout had already fired before (1), and doesn't acquire
the bus lock until after (2), it may be tempted to abort the new
root intr xfer just after submission, which would be wrong -- so
instead we just have the callout do nothing if it notices it has
been rescheduled, since it will fire again after the appropriate
time has elapsed.
To generate a diff of this commit:
cvs rdiff -u -r1.18 -r1.19 src/sys/arch/mips/adm5120/dev/ahci.c
cvs rdiff -u -r1.271 -r1.272 src/sys/dev/usb/ehci.c
cvs rdiff -u -r1.293 -r1.294 src/sys/dev/usb/ohci.c
cvs rdiff -u -r1.289 -r1.290 src/sys/dev/usb/uhci.c
cvs rdiff -u -r1.4 -r1.5 src/sys/dev/usb/vhci.c
cvs rdiff -u -r1.116 -r1.117 src/sys/dev/usb/xhci.c
cvs rdiff -u -r1.67 -r1.68 src/sys/external/bsd/dwc2/dwc2.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/arch/mips/adm5120/dev/ahci.c
diff -u src/sys/arch/mips/adm5120/dev/ahci.c:1.18 src/sys/arch/mips/adm5120/dev/ahci.c:1.19
--- src/sys/arch/mips/adm5120/dev/ahci.c:1.18 Wed Feb 12 16:01:00 2020
+++ src/sys/arch/mips/adm5120/dev/ahci.c Wed Feb 12 16:02:01 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ahci.c,v 1.18 2020/02/12 16:01:00 riastradh Exp $ */
+/* $NetBSD: ahci.c,v 1.19 2020/02/12 16:02:01 riastradh Exp $ */
/*-
* Copyright (c) 2007 Ruslan Ermilov and Vsevolod Lobko.
@@ -64,7 +64,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.18 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.19 2020/02/12 16:02:01 riastradh Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -283,6 +283,7 @@ ahci_attach(device_t parent, device_t se
SIMPLEQ_INIT(&sc->sc_free_xfers);
callout_init(&sc->sc_poll_handle, 0);
+ callout_setfunc(&sc->sc_poll_handle, ahci_poll_hub, sc);
mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SOFTUSB);
mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_SCHED /* XXXNH */);
@@ -422,13 +423,39 @@ ahci_poll(struct usbd_bus *bus)
void
ahci_poll_hub(void *arg)
{
- struct usbd_xfer *xfer = arg;
- struct ahci_softc *sc = AHCI_XFER2SC(xfer);
+ struct ahci_softc *sc = arg;
+ struct usbd_xfer *xfer;
u_char *p;
static int p0_state=0;
static int p1_state=0;
- callout_reset(&sc->sc_poll_handle, sc->sc_interval, ahci_poll_hub, xfer);
+ mutex_enter(&sc->sc_lock);
+
+ /*
+ * If the intr xfer has completed or been synchronously
+ * aborted, we have nothing to do.
+ */
+ xfer = sc->sc_intr_xfer;
+ if (xfer == NULL)
+ goto out;
+
+ /*
+ * If the intr xfer for which we were scheduled is done, and
+ * another intr xfer has been submitted, let that one be dealt
+ * with when the callout fires again.
+ *
+ * The call to callout_pending is racy, but the the transition
+ * from pending to invoking happens atomically. The
+ * callout_ack ensures callout_invoking does not return true
+ * due to this invocation of the callout; the lock ensures the
+ * next invocation of the callout cannot callout_ack (unless it
+ * had already run to completion and nulled sc->sc_intr_xfer,
+ * in which case would have bailed out already).
+ */
+ callout_ack(&sc->sc_poll_handle);
+ if (callout_pending(&sc->sc_poll_handle) ||
+ callout_invoking(&sc->sc_poll_handle))
+ goto out;
/* USB spec 11.13.3 (p.260) */
p = KERNADDR(&xfer->ux_dmabuf, 0);
@@ -444,15 +471,23 @@ ahci_poll_hub(void *arg)
p1_state=(REG_READ(ADMHCD_REG_PORTSTATUS1) & ADMHCD_CCS);
};
- /* no change, return NAK */
- if (p[0] == 0)
- return;
+ /* no change, return NAK and try again later */
+ if (p[0] == 0) {
+ callout_schedule(&sc->sc_poll_handle, sc->sc_interval);
+ goto out;
+ }
+ /*
+ * Interrupt completed, and the xfer has not been completed or
+ * synchronously aborted. Complete the xfer now.
+ *
+ * XXX Set ux_isdone if DIAGNOSTIC?
+ */
xfer->ux_actlen = 1;
xfer->ux_status = USBD_NORMAL_COMPLETION;
- mutex_enter(&sc->sc_lock);
usb_transfer_complete(xfer);
- mutex_exit(&sc->sc_lock);
+
+out: mutex_exit(&sc->sc_lock);
}
struct usbd_xfer *
@@ -719,8 +754,10 @@ ahci_root_intr_start(struct usbd_xfer *x
DPRINTF(D_TRACE, ("SLRIstart "));
+ KASSERT(sc->sc_intr_xfer == NULL);
+
sc->sc_interval = MS_TO_TICKS(xfer->ux_pipe->up_endpoint->ue_edesc->bInterval);
- callout_reset(&sc->sc_poll_handle, sc->sc_interval, ahci_poll_hub, xfer);
+ callout_schedule(&sc->sc_poll_handle, sc->sc_interval);
sc->sc_intr_xfer = xfer;
return USBD_IN_PROGRESS;
}
@@ -728,24 +765,59 @@ ahci_root_intr_start(struct usbd_xfer *x
static void
ahci_root_intr_abort(struct usbd_xfer *xfer)
{
+ struct ahci_softc *sc = AHCI_XFER2SC(xfer);
+
DPRINTF(D_TRACE, ("SLRIabort "));
+
+ KASSERT(mutex_owned(&sc->sc_lock));
+ KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
+
+ /*
+ * Try to stop the callout before it starts. If we got in too
+ * late, too bad; but if the callout had yet to run and time
+ * out the xfer, cancel it ourselves.
+ */
+ callout_stop(&sc->sc_poll_handle);
+ if (sc->sc_intr_xfer == NULL)
+ return;
+
+ KASSERT(sc->sc_intr_xfer == xfer);
+ xfer->ux_status = USBD_CANCELLED;
+ usb_transfer_complete(xfer);
}
static void
ahci_root_intr_close(struct usbd_pipe *pipe)
{
- struct ahci_softc *sc = AHCI_PIPE2SC(pipe);
+ struct ahci_softc *sc __diagused = AHCI_PIPE2SC(pipe);
DPRINTF(D_TRACE, ("SLRIclose "));
- callout_stop(&sc->sc_poll_handle);
- sc->sc_intr_xfer = NULL;
+ KASSERT(mutex_owned(&sc->sc_lock));
+
+ /*
+ * The caller must arrange to have aborted the pipe already, so
+ * there can be no intr xfer in progress. The callout may
+ * still be pending from a prior intr xfer -- if it has already
+ * fired, it will see there is nothing to do, and do nothing.
+ */
+ KASSERT(sc->sc_intr_xfer == NULL);
+ KASSERT(!callout_pending(&sc->sc_poll_handle));
}
static void
ahci_root_intr_done(struct usbd_xfer *xfer)
{
+ struct ahci_softc *sc = AHCI_XFER2SC(xfer);
+
//DPRINTF(D_XFER, ("RIdn "));
+
+ KASSERT(mutex_owned(&sc->sc_lock));
+
+ /* Claim the xfer so it doesn't get completed again. */
+ KASSERT(sc->sc_intr_xfer == xfer);
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
+ sc->sc_intr_xfer = NULL;
}
static usbd_status
Index: src/sys/dev/usb/ehci.c
diff -u src/sys/dev/usb/ehci.c:1.271 src/sys/dev/usb/ehci.c:1.272
--- src/sys/dev/usb/ehci.c:1.271 Wed Feb 12 16:01:00 2020
+++ src/sys/dev/usb/ehci.c Wed Feb 12 16:02:01 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ehci.c,v 1.271 2020/02/12 16:01:00 riastradh Exp $ */
+/* $NetBSD: ehci.c,v 1.272 2020/02/12 16:02:01 riastradh Exp $ */
/*
* Copyright (c) 2004-2012 The NetBSD Foundation, Inc.
@@ -53,7 +53,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.271 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.272 2020/02/12 16:02:01 riastradh Exp $");
#include "ohci.h"
#include "uhci.h"
@@ -2722,11 +2722,13 @@ ehci_root_intr_start(struct usbd_xfer *x
if (!polling)
mutex_enter(&sc->sc_lock);
+ KASSERT(sc->sc_intrxfer == NULL);
sc->sc_intrxfer = xfer;
if (!polling)
mutex_exit(&sc->sc_lock);
- return USBD_IN_PROGRESS;
+ xfer->ux_status = USBD_IN_PROGRESS;
+ return xfer->ux_status;
}
/* Abort a root interrupt request. */
@@ -2738,8 +2740,16 @@ ehci_root_intr_abort(struct usbd_xfer *x
KASSERT(mutex_owned(&sc->sc_lock));
KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
- sc->sc_intrxfer = NULL;
+ /* If xfer has already completed, nothing to do here. */
+ if (sc->sc_intrxfer == NULL)
+ return;
+ /*
+ * Otherwise, sc->sc_intrxfer had better be this transfer.
+ * Cancel it.
+ */
+ KASSERT(sc->sc_intrxfer == xfer);
+ KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
xfer->ux_status = USBD_CANCELLED;
usb_transfer_complete(xfer);
}
@@ -2748,18 +2758,30 @@ ehci_root_intr_abort(struct usbd_xfer *x
Static void
ehci_root_intr_close(struct usbd_pipe *pipe)
{
- ehci_softc_t *sc = EHCI_PIPE2SC(pipe);
+ ehci_softc_t *sc __diagused = EHCI_PIPE2SC(pipe);
EHCIHIST_FUNC(); EHCIHIST_CALLED();
KASSERT(mutex_owned(&sc->sc_lock));
- sc->sc_intrxfer = NULL;
+ /*
+ * Caller must guarantee the xfer has completed first, by
+ * closing the pipe only after normal completion or an abort.
+ */
+ KASSERT(sc->sc_intrxfer == NULL);
}
Static void
ehci_root_intr_done(struct usbd_xfer *xfer)
{
+ struct ehci_softc *sc = EHCI_XFER2SC(xfer);
+
+ KASSERT(mutex_owned(&sc->sc_lock));
+
+ /* Claim the xfer so it doesn't get completed again. */
+ KASSERT(sc->sc_intrxfer == xfer);
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
+ sc->sc_intrxfer = NULL;
}
/************************/
Index: src/sys/dev/usb/ohci.c
diff -u src/sys/dev/usb/ohci.c:1.293 src/sys/dev/usb/ohci.c:1.294
--- src/sys/dev/usb/ohci.c:1.293 Wed Feb 12 16:01:00 2020
+++ src/sys/dev/usb/ohci.c Wed Feb 12 16:02:01 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ohci.c,v 1.293 2020/02/12 16:01:00 riastradh Exp $ */
+/* $NetBSD: ohci.c,v 1.294 2020/02/12 16:02:01 riastradh 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.293 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.294 2020/02/12 16:02:01 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -1708,6 +1708,7 @@ ohci_rhsc(ohci_softc_t *sc, struct usbd_
p[i/8] |= 1 << (i%8);
}
DPRINTF("change=0x%02jx", *p, 0, 0, 0);
+ KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
xfer->ux_actlen = xfer->ux_length;
xfer->ux_status = USBD_NORMAL_COMPLETION;
@@ -1721,7 +1722,9 @@ ohci_root_intr_done(struct usbd_xfer *xf
KASSERT(mutex_owned(&sc->sc_lock));
+ /* Claim the xfer so it doesn't get completed again. */
KASSERT(sc->sc_intrxfer == xfer);
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
sc->sc_intrxfer = NULL;
}
@@ -2516,18 +2519,29 @@ ohci_root_intr_start(struct usbd_xfer *x
if (!polling)
mutex_exit(&sc->sc_lock);
- return USBD_IN_PROGRESS;
+ xfer->ux_status = USBD_IN_PROGRESS;
+ return xfer->ux_status;
}
/* Abort a root interrupt request. */
Static void
ohci_root_intr_abort(struct usbd_xfer *xfer)
{
- ohci_softc_t *sc __diagused = OHCI_XFER2SC(xfer);
+ ohci_softc_t *sc = OHCI_XFER2SC(xfer);
KASSERT(mutex_owned(&sc->sc_lock));
KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
+ /* If xfer has already completed, nothing to do here. */
+ if (sc->sc_intrxfer == NULL)
+ return;
+
+ /*
+ * Otherwise, sc->sc_intrxfer had better be this transfer.
+ * Cancel it.
+ */
+ KASSERT(sc->sc_intrxfer == xfer);
+ KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
xfer->ux_status = USBD_CANCELLED;
usb_transfer_complete(xfer);
}
@@ -2536,13 +2550,17 @@ ohci_root_intr_abort(struct usbd_xfer *x
Static void
ohci_root_intr_close(struct usbd_pipe *pipe)
{
- ohci_softc_t *sc = OHCI_PIPE2SC(pipe);
+ ohci_softc_t *sc __diagused = OHCI_PIPE2SC(pipe);
KASSERT(mutex_owned(&sc->sc_lock));
OHCIHIST_FUNC(); OHCIHIST_CALLED();
- sc->sc_intrxfer = NULL;
+ /*
+ * Caller must guarantee the xfer has completed first, by
+ * closing the pipe only after normal completion or an abort.
+ */
+ KASSERT(sc->sc_intrxfer == NULL);
}
/************************/
Index: src/sys/dev/usb/uhci.c
diff -u src/sys/dev/usb/uhci.c:1.289 src/sys/dev/usb/uhci.c:1.290
--- src/sys/dev/usb/uhci.c:1.289 Wed Feb 12 16:01:00 2020
+++ src/sys/dev/usb/uhci.c Wed Feb 12 16:02:01 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: uhci.c,v 1.289 2020/02/12 16:01:00 riastradh Exp $ */
+/* $NetBSD: uhci.c,v 1.290 2020/02/12 16:02:01 riastradh Exp $ */
/*
* Copyright (c) 1998, 2004, 2011, 2012 The NetBSD Foundation, Inc.
@@ -42,7 +42,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.289 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.290 2020/02/12 16:02:01 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -574,6 +574,7 @@ uhci_init(uhci_softc_t *sc)
"uhcixfer", NULL, IPL_USB, NULL, NULL, NULL);
callout_init(&sc->sc_poll_handle, CALLOUT_MPSAFE);
+ callout_setfunc(&sc->sc_poll_handle, uhci_poll_hub, sc);
/* Set up the bus struct. */
sc->sc_bus.ub_methods = &uhci_bus_methods;
@@ -739,8 +740,7 @@ uhci_resume(device_t dv, const pmf_qual_
usb_delay_ms_locked(&sc->sc_bus, USB_RESUME_RECOVERY, &sc->sc_intr_lock);
sc->sc_bus.ub_usepolling--;
if (sc->sc_intr_xfer != NULL)
- callout_reset(&sc->sc_poll_handle, sc->sc_ival, uhci_poll_hub,
- sc->sc_intr_xfer);
+ callout_schedule(&sc->sc_poll_handle, sc->sc_ival);
#ifdef UHCI_DEBUG
if (uhcidebug >= 2)
uhci_dumpregs(sc);
@@ -766,9 +766,9 @@ uhci_suspend(device_t dv, const pmf_qual
if (uhcidebug >= 2)
uhci_dumpregs(sc);
#endif
- if (sc->sc_intr_xfer != NULL)
- callout_stop(&sc->sc_poll_handle);
sc->sc_suspend = PWR_SUSPEND;
+ if (sc->sc_intr_xfer != NULL)
+ callout_halt(&sc->sc_poll_handle, &sc->sc_intr_lock);
sc->sc_bus.ub_usepolling++;
uhci_run(sc, 0, 1); /* stop the controller */
@@ -998,38 +998,85 @@ void iidump(void) { uhci_dump_iis(thesc)
void
uhci_poll_hub(void *addr)
{
- struct usbd_xfer *xfer = addr;
- struct usbd_pipe *pipe = xfer->ux_pipe;
- uhci_softc_t *sc;
+ struct uhci_softc *sc = addr;
+ struct usbd_xfer *xfer;
u_char *p;
UHCIHIST_FUNC(); UHCIHIST_CALLED();
- if (__predict_false(pipe->up_dev == NULL || pipe->up_dev->ud_bus == NULL))
- return; /* device has detached */
- sc = UHCI_PIPE2SC(pipe);
- callout_reset(&sc->sc_poll_handle, sc->sc_ival, uhci_poll_hub, xfer);
+ mutex_enter(&sc->sc_lock);
+
+ /*
+ * If the intr xfer has completed or been synchronously
+ * aborted, we have nothing to do.
+ */
+ xfer = sc->sc_intr_xfer;
+ if (xfer == NULL)
+ goto out;
+
+ /*
+ * If the intr xfer for which we were scheduled is done, and
+ * another intr xfer has been submitted, let that one be dealt
+ * with when the callout fires again.
+ *
+ * The call to callout_pending is racy, but the the transition
+ * from pending to invoking happens atomically. The
+ * callout_ack ensures callout_invoking does not return true
+ * due to this invocation of the callout; the lock ensures the
+ * next invocation of the callout cannot callout_ack (unless it
+ * had already run to completion and nulled sc->sc_intr_xfer,
+ * in which case would have bailed out already).
+ */
+ callout_ack(&sc->sc_poll_handle);
+ if (callout_pending(&sc->sc_poll_handle) ||
+ callout_invoking(&sc->sc_poll_handle))
+ goto out;
+ /*
+ * Check flags for the two interrupt ports, and set them in the
+ * buffer if an interrupt arrived; otherwise arrange .
+ */
p = xfer->ux_buf;
p[0] = 0;
if (UREAD2(sc, UHCI_PORTSC1) & (UHCI_PORTSC_CSC|UHCI_PORTSC_OCIC))
p[0] |= 1<<1;
if (UREAD2(sc, UHCI_PORTSC2) & (UHCI_PORTSC_CSC|UHCI_PORTSC_OCIC))
p[0] |= 1<<2;
- if (p[0] == 0)
- /* No change, try again in a while */
- return;
+ if (p[0] == 0) {
+ /*
+ * No change -- try again in a while, unless we're
+ * suspending, in which case we'll try again after
+ * resume.
+ */
+ if (sc->sc_suspend != PWR_SUSPEND)
+ callout_schedule(&sc->sc_poll_handle, sc->sc_ival);
+ goto out;
+ }
+ /*
+ * Interrupt completed, and the xfer has not been completed or
+ * synchronously aborted. Complete the xfer now.
+ *
+ * XXX Set ux_isdone if DIAGNOSTIC?
+ */
xfer->ux_actlen = 1;
xfer->ux_status = USBD_NORMAL_COMPLETION;
- mutex_enter(&sc->sc_lock);
usb_transfer_complete(xfer);
- mutex_exit(&sc->sc_lock);
+
+out: mutex_exit(&sc->sc_lock);
}
void
uhci_root_intr_done(struct usbd_xfer *xfer)
{
+ struct uhci_softc *sc = UHCI_XFER2SC(xfer);
+
+ KASSERT(mutex_owned(&sc->sc_lock));
+
+ /* Claim the xfer so it doesn't get completed again. */
+ KASSERT(sc->sc_intr_xfer == xfer);
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
+ sc->sc_intr_xfer = NULL;
}
/*
@@ -3793,9 +3840,16 @@ uhci_root_intr_abort(struct usbd_xfer *x
KASSERT(mutex_owned(&sc->sc_lock));
KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
+ /*
+ * Try to stop the callout before it starts. If we got in too
+ * late, too bad; but if the callout had yet to run and time
+ * out the xfer, cancel it ourselves.
+ */
callout_stop(&sc->sc_poll_handle);
- sc->sc_intr_xfer = NULL;
+ if (sc->sc_intr_xfer == NULL)
+ return;
+ KASSERT(sc->sc_intr_xfer == xfer);
xfer->ux_status = USBD_CANCELLED;
#ifdef DIAGNOSTIC
UHCI_XFER2UXFER(xfer)->ux_isdone = true;
@@ -3830,6 +3884,7 @@ uhci_root_intr_start(struct usbd_xfer *x
struct usbd_pipe *pipe = xfer->ux_pipe;
uhci_softc_t *sc = UHCI_PIPE2SC(pipe);
unsigned int ival;
+ const bool polling = sc->sc_bus.ub_usepolling;
UHCIHIST_FUNC(); UHCIHIST_CALLED();
DPRINTF("xfer=%#jx len=%jd flags=%jd", (uintptr_t)xfer, xfer->ux_length,
@@ -3838,11 +3893,20 @@ uhci_root_intr_start(struct usbd_xfer *x
if (sc->sc_dying)
return USBD_IOERROR;
+ if (!polling)
+ mutex_enter(&sc->sc_lock);
+
+ KASSERT(sc->sc_intr_xfer == NULL);
+
/* XXX temporary variable needed to avoid gcc3 warning */
ival = xfer->ux_pipe->up_endpoint->ue_edesc->bInterval;
sc->sc_ival = mstohz(ival);
- callout_reset(&sc->sc_poll_handle, sc->sc_ival, uhci_poll_hub, xfer);
+ callout_schedule(&sc->sc_poll_handle, sc->sc_ival);
sc->sc_intr_xfer = xfer;
+
+ if (!polling)
+ mutex_exit(&sc->sc_lock);
+
return USBD_IN_PROGRESS;
}
@@ -3850,11 +3914,17 @@ uhci_root_intr_start(struct usbd_xfer *x
void
uhci_root_intr_close(struct usbd_pipe *pipe)
{
- uhci_softc_t *sc = UHCI_PIPE2SC(pipe);
+ uhci_softc_t *sc __diagused = UHCI_PIPE2SC(pipe);
UHCIHIST_FUNC(); UHCIHIST_CALLED();
KASSERT(mutex_owned(&sc->sc_lock));
- callout_stop(&sc->sc_poll_handle);
- sc->sc_intr_xfer = NULL;
+ /*
+ * The caller must arrange to have aborted the pipe already, so
+ * there can be no intr xfer in progress. The callout may
+ * still be pending from a prior intr xfer -- if it has already
+ * fired, it will see there is nothing to do, and do nothing.
+ */
+ KASSERT(sc->sc_intr_xfer == NULL);
+ KASSERT(!callout_pending(&sc->sc_poll_handle));
}
Index: src/sys/dev/usb/vhci.c
diff -u src/sys/dev/usb/vhci.c:1.4 src/sys/dev/usb/vhci.c:1.5
--- src/sys/dev/usb/vhci.c:1.4 Sun Nov 17 11:28:48 2019
+++ src/sys/dev/usb/vhci.c Wed Feb 12 16:02:01 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: vhci.c,v 1.4 2019/11/17 11:28:48 maxv Exp $ */
+/* $NetBSD: vhci.c,v 1.5 2020/02/12 16:02:01 riastradh Exp $ */
/*
* Copyright (c) 2019 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vhci.c,v 1.4 2019/11/17 11:28:48 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vhci.c,v 1.5 2020/02/12 16:02:01 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -627,6 +627,7 @@ vhci_root_intr_start(struct usbd_xfer *x
if (!polling)
mutex_enter(&sc->sc_lock);
+ KASSERT(sc->sc_intrxfer == NULL);
sc->sc_intrxfer = xfer;
if (!polling)
mutex_exit(&sc->sc_lock);
@@ -644,8 +645,15 @@ vhci_root_intr_abort(struct usbd_xfer *x
KASSERT(mutex_owned(&sc->sc_lock));
KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
- sc->sc_intrxfer = NULL;
+ /* If xfer has already completed, nothing to do here. */
+ if (sc->sc_intrxfer == NULL)
+ return;
+ /*
+ * Otherwise, sc->sc_intrxfer had better be this transfer.
+ * Cancel it.
+ */
+ KASSERT(sc->sc_intrxfer == xfer);
xfer->ux_status = USBD_CANCELLED;
usb_transfer_complete(xfer);
}
@@ -653,13 +661,17 @@ vhci_root_intr_abort(struct usbd_xfer *x
static void
vhci_root_intr_close(struct usbd_pipe *pipe)
{
- vhci_softc_t *sc = pipe->up_dev->ud_bus->ub_hcpriv;
+ vhci_softc_t *sc __diagused = pipe->up_dev->ud_bus->ub_hcpriv;
DPRINTF("%s: called\n", __func__);
KASSERT(mutex_owned(&sc->sc_lock));
- sc->sc_intrxfer = NULL;
+ /*
+ * Caller must guarantee the xfer has completed first, by
+ * closing the pipe only after normal completion or an abort.
+ */
+ KASSERT(sc->sc_intrxfer == NULL);
}
static void
@@ -671,6 +683,14 @@ vhci_root_intr_cleartoggle(struct usbd_p
static void
vhci_root_intr_done(struct usbd_xfer *xfer)
{
+ vhci_softc_t *sc = xfer->ux_bus->ub_hcpriv;
+
+ KASSERT(mutex_owned(&sc->sc_lock));
+
+ /* Claim the xfer so it doesn't get completed again. */
+ KASSERT(sc->sc_intrxfer == xfer);
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
+ sc->sc_intrxfer = NULL;
}
/* -------------------------------------------------------------------------- */
Index: src/sys/dev/usb/xhci.c
diff -u src/sys/dev/usb/xhci.c:1.116 src/sys/dev/usb/xhci.c:1.117
--- src/sys/dev/usb/xhci.c:1.116 Wed Feb 12 16:01:00 2020
+++ src/sys/dev/usb/xhci.c Wed Feb 12 16:02:01 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: xhci.c,v 1.116 2020/02/12 16:01:00 riastradh Exp $ */
+/* $NetBSD: xhci.c,v 1.117 2020/02/12 16:02:01 riastradh Exp $ */
/*
* Copyright (c) 2013 Jonathan A. Kollasch
@@ -34,7 +34,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.116 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.117 2020/02/12 16:02:01 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -3715,6 +3715,7 @@ xhci_root_intr_start(struct usbd_xfer *x
if (!polling)
mutex_enter(&sc->sc_lock);
+ KASSERT(sc->sc_intrxfer[bn] == NULL);
sc->sc_intrxfer[bn] = xfer;
if (!polling)
mutex_exit(&sc->sc_lock);
@@ -3725,13 +3726,23 @@ xhci_root_intr_start(struct usbd_xfer *x
static void
xhci_root_intr_abort(struct usbd_xfer *xfer)
{
- struct xhci_softc * const sc __diagused = XHCI_XFER2SC(xfer);
+ struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
+ const size_t bn = XHCI_XFER2BUS(xfer) == &sc->sc_bus ? 0 : 1;
XHCIHIST_FUNC(); XHCIHIST_CALLED();
KASSERT(mutex_owned(&sc->sc_lock));
KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
+ /* If xfer has already completed, nothing to do here. */
+ if (sc->sc_intrxfer[bn] == NULL)
+ return;
+
+ /*
+ * Otherwise, sc->sc_intrxfer[bn] had better be this transfer.
+ * Cancel it.
+ */
+ KASSERT(sc->sc_intrxfer[bn] == xfer);
xfer->ux_status = USBD_CANCELLED;
usb_transfer_complete(xfer);
}
@@ -3739,22 +3750,35 @@ xhci_root_intr_abort(struct usbd_xfer *x
static void
xhci_root_intr_close(struct usbd_pipe *pipe)
{
- struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
- const struct usbd_xfer *xfer = pipe->up_intrxfer;
- const size_t bn = XHCI_XFER2BUS(xfer) == &sc->sc_bus ? 0 : 1;
+ struct xhci_softc * const sc __diagused = XHCI_PIPE2SC(pipe);
+ const struct usbd_xfer *xfer __diagused = pipe->up_intrxfer;
+ const size_t bn __diagused = XHCI_XFER2BUS(xfer) == &sc->sc_bus ? 0 : 1;
XHCIHIST_FUNC(); XHCIHIST_CALLED();
KASSERT(mutex_owned(&sc->sc_lock));
- sc->sc_intrxfer[bn] = NULL;
+ /*
+ * Caller must guarantee the xfer has completed first, by
+ * closing the pipe only after normal completion or an abort.
+ */
+ KASSERT(sc->sc_intrxfer[bn] == NULL);
}
static void
xhci_root_intr_done(struct usbd_xfer *xfer)
{
+ struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
+ const size_t bn = XHCI_XFER2BUS(xfer) == &sc->sc_bus ? 0 : 1;
+
XHCIHIST_FUNC(); XHCIHIST_CALLED();
+ KASSERT(mutex_owned(&sc->sc_lock));
+
+ /* Claim the xfer so it doesn't get completed again. */
+ KASSERT(sc->sc_intrxfer[bn] == xfer);
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
+ sc->sc_intrxfer[bn] = NULL;
}
/* -------------- */
Index: src/sys/external/bsd/dwc2/dwc2.c
diff -u src/sys/external/bsd/dwc2/dwc2.c:1.67 src/sys/external/bsd/dwc2/dwc2.c:1.68
--- src/sys/external/bsd/dwc2/dwc2.c:1.67 Wed Feb 12 16:01:00 2020
+++ src/sys/external/bsd/dwc2/dwc2.c Wed Feb 12 16:02:01 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: dwc2.c,v 1.67 2020/02/12 16:01:00 riastradh Exp $ */
+/* $NetBSD: dwc2.c,v 1.68 2020/02/12 16:02:01 riastradh Exp $ */
/*-
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.67 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.68 2020/02/12 16:02:01 riastradh Exp $");
#include "opt_usb.h"
@@ -649,7 +649,8 @@ dwc2_root_intr_start(struct usbd_xfer *x
if (!polling)
mutex_exit(&sc->sc_lock);
- return USBD_IN_PROGRESS;
+ xfer->ux_status = USBD_IN_PROGRESS;
+ return xfer->ux_status;
}
/* Abort a root interrupt request. */
@@ -663,6 +664,16 @@ dwc2_root_intr_abort(struct usbd_xfer *x
KASSERT(mutex_owned(&sc->sc_lock));
KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
+ /* If xfer has already completed, nothing to do here. */
+ if (sc->sc_intrxfer == NULL)
+ return;
+
+ /*
+ * Otherwise, sc->sc_intrxfer had better be this transfer.
+ * Cancel it.
+ */
+ KASSERT(sc->sc_intrxfer == xfer);
+ KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
xfer->ux_status = USBD_CANCELLED;
usb_transfer_complete(xfer);
}
@@ -676,7 +687,11 @@ dwc2_root_intr_close(struct usbd_pipe *p
KASSERT(mutex_owned(&sc->sc_lock));
- sc->sc_intrxfer = NULL;
+ /*
+ * Caller must guarantee the xfer has completed first, by
+ * closing the pipe only after normal completion or an abort.
+ */
+ KASSERT(sc->sc_intrxfer == NULL);
}
Static void
@@ -684,9 +699,12 @@ dwc2_root_intr_done(struct usbd_xfer *xf
{
struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
- KASSERT(sc->sc_intrxfer != NULL);
- sc->sc_intrxfer = NULL;
DPRINTF("\n");
+
+ /* Claim the xfer so it doesn't get completed again. */
+ KASSERT(sc->sc_intrxfer == xfer);
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
+ sc->sc_intrxfer = NULL;
}
/***********************************************************************/
