Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/netbsd-9]: src/sys Pull up following revision(s) (requested by riastradh...
details: https://anonhg.NetBSD.org/src/rev/d6fd589b0bf7
branches: netbsd-9
changeset: 745398:d6fd589b0bf7
user: martin <martin%NetBSD.org@localhost>
date: Sun Mar 01 12:35:16 2020 +0000
description:
Pull up following revision(s) (requested by riastradh in ticket #744):
sys/dev/usb/uhci.c: revision 1.292
sys/dev/usb/uhci.c: revision 1.293
sys/dev/usb/usbdi.h: revision 1.99
sys/dev/usb/motg.c: revision 1.26
sys/dev/usb/motg.c: revision 1.27
sys/dev/usb/motg.c: revision 1.28
sys/dev/usb/motg.c: revision 1.29
sys/external/bsd/dwc2/dwc2.c: revision 1.70
sys/external/bsd/dwc2/dwc2.c: revision 1.71
sys/dev/usb/usb.c: revision 1.181
sys/arch/mips/adm5120/dev/ahci.c: revision 1.20
sys/dev/usb/usb.c: revision 1.182
sys/dev/usb/xhci.c: revision 1.116
sys/dev/usb/xhci.c: revision 1.117
sys/dev/usb/xhci.c: revision 1.118
sys/dev/usb/uhci.c: revision 1.289
sys/dev/usb/usbdivar.h: revision 1.121
sys/dev/usb/usbdi.c: revision 1.190
sys/dev/usb/usbdivar.h: revision 1.122
sys/dev/usb/usbdi.c: revision 1.191
sys/dev/usb/usbdi.c: revision 1.192
sys/external/bsd/dwc2/dwc2var.h: revision 1.7
sys/dev/usb/motg.c: revision 1.30
sys/dev/usb/motg.c: revision 1.31
sys/dev/usb/motg.c: revision 1.32
sys/dev/usb/motg.c: revision 1.33
sys/external/bsd/dwc2/dwc2.c: revision 1.67
sys/external/bsd/dwc2/dwc2.c: revision 1.68
sys/dev/usb/ehci.c: revision 1.270
sys/external/bsd/dwc2/dwc2.c: revision 1.69
sys/dev/usb/usbdi.h: revision 1.100
sys/dev/usb/ehci.c: revision 1.271
sys/arch/mips/adm5120/dev/ahci.c: revision 1.18
sys/dev/usb/usbdi.h: revision 1.101
sys/dev/usb/ehci.c: revision 1.272
sys/dev/ic/sl811hs.c: revision 1.103
sys/arch/mips/adm5120/dev/ahci.c: revision 1.19
sys/dev/usb/ehci.c: revision 1.273
sys/dev/usb/ohci.c: revision 1.293
sys/dev/usb/uhci.c: revision 1.290
sys/dev/usb/ohci.c: revision 1.294
sys/dev/usb/uhci.c: revision 1.291
sys/dev/usb/ohci.c: revision 1.295
Teach usb_rem_task to return whether removed from queue or not.
New function usb_task_pending for diagnostic assertions.
Usable only for negative diagnostic assertions:
KASSERT(!usb_task_pending(dev, task))
If you can think of a better name for this than !usb_task_pending,
I'm all ears.
-
Nothing guarantees xfer's timeout has completed.
Wait for it when we free the xfer.
-
New xfer state variables ux_timeout_set and ux_timeout_reset.
These are needed because:
- The host controller interrupt cannot wait for the callout or task
to finish running.
- Nothing in the USBD API as is waits for the callout or task to
finish running.
- Callers expect to be able to resubmit USB xfers from xfer callbacks
without waiting for anything to finish running.
The variable ux_timeout_set can be used by a host controller to
decide on submission whether to schedule the callout or to ask an
already-scheduled callout or already-queued task to reschedule the
callout, by setting the variable ux_timeout_reset to true.
When the callout or task runs and sees that ux_timeout_reset is true,
rather than queue the task or abort the xfer, it can instead just
schedule the callout anew.
-
Fix steady state of timeouts in ehci.
This is complicated because:
1. There are three ways that an xfer can be completed:
(a) hardware interrupt completes xfer
(b) software decision aborts xfer with USBD_CANCELLED
(c) timeout aborts xfer with USBD_TIMEOUT
2. The timeout abort can't be done in callout because ehci_sync_hc,
called unconditionally by ehci_abort_xfer to wait until the device
has finished using any references to the xfer, may sleep. So we
have to schedule a callout that, when run, will schedule a usb_task.
3. The hardware completion interrupt can't sleep waiting for a callout
or task to finish -- can't use callout_halt or usb_rem_task_wait.
So the callout and usb_task must be able to run _after_ the hardware
completion interrupt, and recognize that they're late to the party.
(Note, though, that usbd_free_xfer does wait for the callout and
task to complete, so there's no danger they may use themselves after
free.)
4. The xfer may resubmitted -- and the timeout may be rescheduled --
immediately after the hardware completion interrupt, _while_ the
callout and/or usb_task may still be scheduled. Specifically, we
may have the following sequence of events:
(a) hardware completion interrupt
(b) callout or usb_task fires
(c) driver resubmits xfer
(d) callout or usb_task acquires lock and looks around dazed and
bewildered at the firehose of events like reading the news in 2019
The mechanism for sorting this out is that we have two bits of state:
- xfer->ux_timeout_set informs the driver, when submitting an xfer and
setting up its timeout, whether either the callout or usb_task is
already scheduled or not.
- xfer->ux_timeout_reset informs the callout or usb_task whether it
should reschedule the callout, because the xfer got resubmitted, or
not.
-
Factor out HCI-independent xfer completion logic.
New API for HCI drivers to synchronize hardware completion
interrupts, synchronous aborts, and asynchronous timeouts:
- When submitting an xfer to hardware, call
usbd_xfer_schedule_timeout(xfer).
- On HCI completion interrupt for xfer completion:
if (!usbd_xfer_trycomplete(xfer))
return; /* timed out or aborted, ignore it */
- In upm_abort methods, call usbd_xfer_abort(xfer).
For HCI drivers that use this API (not needed in drivers that don't,
or for xfers like root intr xfers that don't use it):
- New ubm_abortx method serves role of former *hci_abort_xfer, but
without any logic for wrangling timeouts/callouts/tasks -- caller
in usbd_xfer_abort has already handled them.
- New ubm_dying method, returns true if the device is in the process
of detaching, used by the timeout logic.
Converted and tested:
- ehci
- ohci
Converted and compile-tested:
- ahci (XXX did this ever work?)
- dwc2
- motg (XXX missing usbd_xfer_schedule_timeout in motg_*_start?)
- uhci
- xhci
Not changed:
- slhci (sys/dev/ic/sl811hs.c) -- doesn't use a separate per-xfer
callout for timeouts (XXX but maybe should?)
- ugenhc (sys/rump/dev/lib/libugenhc/ugenhc.c) -- doesn't manage its
own transfer timeouts
-
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.
-
Initialize xfer->ux_status in uhci_root_intr_start.
Otherwise, it will be USBD_NOT_STARTED, so usbd_ar_pipe will skip
calling upm_abort.
Candidate fix for PR kern/54963, same problem as reported at:
href="https://mail-index.NetBSD.org/current-users/2020/02/13/msg037740.html
-
Set ux_isdone in uhci_poll_hub for DIAGNOSTIC.
-
Fix mistakes in previous sloppy change with root intr xfers.
- Make sure ux_status is set to USBD_IN_PROGRESS when started.
Otherwise, if it is still in flight when we abort the pipe,
usbd_ar_pipe will skip calling upm_abort.
- Initialize ux_status under the lock; in principle a completion
interrupt (or a delay) could race with the initialization.
- KASSERT that the xfer is in progress when we're about to complete
it.
Candidate fix for PR kern/54963 for other HCI drivers than uhci.
ok nick
ok phone
(This is the change that nick evidently MEANT to ok when he ok'd the
previous one!)
-
Fix build
-
Fix non-DIAGNOSTIC builds.
-
Fix wrong KASSERT in motg abort.
This has been wrong since last summer when we did the transition to
xfer->ux_status = USBD_CANCELLED earlier.
XXX pullup-9
-
Fix mistakes in timeout/abort/completion changes in motg(4).
- Call usbd_xfer_schedule_timeout so we actually do time out.
- Don't call usbd_xfer_trycomplete until all the data have been
transferred -- it commits to completion, not timeout.
- Use xfer->ux_status != USBD_IN_PROGRESS to test whether, after a
partial write, an xfer has been interrupted or timed out and need
not be continued.
- Remove wrong assertion.
-
Fix mistake in use of usbd_xfer_schedule_timeout in motg.
This code path is used both for xfers that are new, and xfers that
are being done piece by piece and are partway done. For the latter
case, skip usbd_xfer_schedule_timeout so we schedule it only once per
xfer.
-
Simplify some branches and kassert some redundant assignments.
diffstat:
sys/arch/mips/adm5120/dev/ahci.c | 125 +++++++++--
sys/dev/ic/sl811hs.c | 8 +-
sys/dev/usb/ehci.c | 181 +++++------------
sys/dev/usb/motg.c | 146 ++++++++-----
sys/dev/usb/ohci.c | 186 +++++------------
sys/dev/usb/uhci.c | 270 ++++++++++++-------------
sys/dev/usb/usb.c | 31 ++-
sys/dev/usb/usbdi.c | 404 ++++++++++++++++++++++++++++++++++++++-
sys/dev/usb/usbdi.h | 10 +-
sys/dev/usb/usbdivar.h | 17 +-
sys/dev/usb/xhci.c | 213 ++++++-------------
sys/external/bsd/dwc2/dwc2.c | 285 ++++++++++++---------------
sys/external/bsd/dwc2/dwc2var.h | 3 +-
13 files changed, 1084 insertions(+), 795 deletions(-)
diffs (truncated from 3397 to 300 lines):
diff -r dc2893c94a73 -r d6fd589b0bf7 sys/arch/mips/adm5120/dev/ahci.c
--- a/sys/arch/mips/adm5120/dev/ahci.c Sun Mar 01 11:53:09 2020 +0000
+++ b/sys/arch/mips/adm5120/dev/ahci.c Sun Mar 01 12:35:16 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ahci.c,v 1.17.4.1 2020/02/25 18:50:29 martin Exp $ */
+/* $NetBSD: ahci.c,v 1.17.4.2 2020/03/01 12:35:16 martin 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.17.4.1 2020/02/25 18:50:29 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.17.4.2 2020/03/01 12:35:16 martin Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -98,6 +98,7 @@
static struct usbd_xfer *
ahci_allocx(struct usbd_bus *, unsigned int);
static void ahci_freex(struct usbd_bus *, struct usbd_xfer *);
+static void ahci_abortx(struct usbd_xfer *);
static void ahci_get_lock(struct usbd_bus *, kmutex_t **);
static int ahci_roothub_ctrl(struct usbd_bus *, usb_device_request_t *,
@@ -136,7 +137,6 @@
static int ahci_transaction(struct ahci_softc *,
struct usbd_pipe *, uint8_t, int, u_char *, uint8_t);
static void ahci_noop(struct usbd_pipe *);
-static void ahci_abort_xfer(struct usbd_xfer *, usbd_status);
static void ahci_device_clear_toggle(struct usbd_pipe *);
extern int usbdebug;
@@ -169,6 +169,7 @@
.ubm_dopoll = ahci_poll,
.ubm_allocx = ahci_allocx,
.ubm_freex = ahci_freex,
+ .ubm_abortx = ahci_abortx,
.ubm_getlock = ahci_get_lock,
.ubm_rhctrl = ahci_roothub_ctrl,
};
@@ -282,6 +283,7 @@
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 */);
@@ -421,13 +423,40 @@
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;
+ KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
+
+ /*
+ * 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);
@@ -443,15 +472,23 @@
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 *
@@ -718,33 +755,73 @@
DPRINTF(D_TRACE, ("SLRIstart "));
+ mutex_enter(&sc->sc_lock);
+ 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;
+ xfer->ux_status = USBD_IN_PROGRESS;
+ mutex_exit(&sc->sc_lock);
+
return USBD_IN_PROGRESS;
}
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
@@ -922,7 +999,7 @@
ahci_device_ctrl_abort(struct usbd_xfer *xfer)
{
DPRINTF(D_TRACE, ("Cab "));
- ahci_abort_xfer(xfer, USBD_CANCELLED);
+ usbd_xfer_abort(xfer);
}
static void
@@ -1034,7 +1111,7 @@
} else {
printf("%s: sx == NULL!\n", __func__);
}
- ahci_abort_xfer(xfer, USBD_CANCELLED);
+ usbd_xfer_abort(xfer);
}
static void
@@ -1250,7 +1327,7 @@
ahci_device_bulk_abort(struct usbd_xfer *xfer)
{
DPRINTF(D_TRACE, ("Bab "));
- ahci_abort_xfer(xfer, USBD_CANCELLED);
+ usbd_xfer_abort(xfer);
}
static void
@@ -1380,11 +1457,15 @@
#endif
}
-void
-ahci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
+static void
+ahci_abortx(struct usbd_xfer *xfer)
{
- xfer->ux_status = status;
- usb_transfer_complete(xfer);
+ /*
+ * XXX This is totally busted; there's no way it can possibly
+ * work! All transfers are busy-waited, it seems, so there is
+ * no opportunity to abort.
+ */
+ KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
}
void
diff -r dc2893c94a73 -r d6fd589b0bf7 sys/dev/ic/sl811hs.c
--- a/sys/dev/ic/sl811hs.c Sun Mar 01 11:53:09 2020 +0000
+++ b/sys/dev/ic/sl811hs.c Sun Mar 01 12:35:16 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: sl811hs.c,v 1.101 2019/02/17 04:17:52 rin Exp $ */
+/* $NetBSD: sl811hs.c,v 1.101.4.1 2020/03/01 12:35:16 martin Exp $ */
/*
* Not (c) 2007 Matthew Orgass
@@ -68,7 +68,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.101 2019/02/17 04:17:52 rin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.101.4.1 2020/03/01 12:35:16 martin Exp $");
#ifdef _KERNEL_OPT
#include "opt_slhci.h"
@@ -1030,7 +1030,9 @@
KASSERT(spipe->ptype == PT_ROOT_INTR);
mutex_enter(&sc->sc_intr_lock);
+ KASSERT(t->rootintr == NULL);
t->rootintr = xfer;
+ xfer->ux_status = USBD_IN_PROGRESS;
mutex_exit(&sc->sc_intr_lock);
return USBD_IN_PROGRESS;
@@ -2389,6 +2391,8 @@
if (t->rootintr != NULL) {
u_char *p;
+ KASSERT(t->rootintr->ux_status ==
+ USBD_IN_PROGRESS);
p = t->rootintr->ux_buf;
p[0] = 2;
t->rootintr->ux_actlen = 1;
diff -r dc2893c94a73 -r d6fd589b0bf7 sys/dev/usb/ehci.c
--- a/sys/dev/usb/ehci.c Sun Mar 01 11:53:09 2020 +0000
+++ b/sys/dev/usb/ehci.c Sun Mar 01 12:35:16 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ehci.c,v 1.267.2.2 2020/02/25 18:50:29 martin Exp $ */
+/* $NetBSD: ehci.c,v 1.267.2.3 2020/03/01 12:35:16 martin 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.267.2.2 2020/02/25 18:50:29 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.267.2.3 2020/03/01 12:35:16 martin Exp $");
Home |
Main Index |
Thread Index |
Old Index