[src/trunk]: src/sys/dev/usb usb: Fix roothub ctrl xfer aborts.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/97cd7553346d
branches:  trunk
changeset: 363463:97cd7553346d
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun Mar 13 11:28:52 2022 +0000

description:
usb: Fix roothub ctrl xfer aborts.

No mechanism for actually aborting, but at least this now waits for
the xfer to have completed instead of blithely barging ahead whether
it's done or not.

diffstat:

 sys/dev/usb/usb.c        |   6 ++++--
 sys/dev/usb/usbdivar.h   |   4 +++-
 sys/dev/usb/usbroothub.c |  36 +++++++++++++++++++++++++-----------
 3 files changed, 32 insertions(+), 14 deletions(-)

diffs (129 lines):

diff -r 1c8285880501 -r 97cd7553346d sys/dev/usb/usb.c
--- a/sys/dev/usb/usb.c Sun Mar 13 11:28:42 2022 +0000
+++ b/sys/dev/usb/usb.c Sun Mar 13 11:28:52 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usb.c,v 1.199 2022/03/06 09:03:42 riastradh Exp $      */
+/*     $NetBSD: usb.c,v 1.200 2022/03/13 11:28:52 riastradh Exp $      */
 
 /*
  * Copyright (c) 1998, 2002, 2008, 2012 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.199 2022/03/06 09:03:42 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.200 2022/03/13 11:28:52 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -296,6 +296,7 @@
        usbrev = sc->sc_bus->ub_revision;
 
        cv_init(&sc->sc_bus->ub_needsexplore_cv, "usbevt");
+       cv_init(&sc->sc_bus->ub_rhxfercv, "usbrhxfer");
        sc->sc_pmf_registered = false;
 
        aprint_naive("\n");
@@ -1430,6 +1431,7 @@
        usb_add_event(USB_EVENT_CTRLR_DETACH, ue);
 
        cv_destroy(&sc->sc_bus->ub_needsexplore_cv);
+       cv_destroy(&sc->sc_bus->ub_rhxfercv);
 
        return 0;
 }
diff -r 1c8285880501 -r 97cd7553346d sys/dev/usb/usbdivar.h
--- a/sys/dev/usb/usbdivar.h    Sun Mar 13 11:28:42 2022 +0000
+++ b/sys/dev/usb/usbdivar.h    Sun Mar 13 11:28:52 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdivar.h,v 1.136 2022/03/13 11:28:42 riastradh Exp $ */
+/*     $NetBSD: usbdivar.h,v 1.137 2022/03/13 11:28:52 riastradh Exp $ */
 
 /*
  * Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -181,6 +181,8 @@
        /* Filled by usb driver */
        kmutex_t               *ub_lock;
        struct usbd_device     *ub_roothub;
+       struct usbd_xfer       *ub_rhxfer;      /* roothub xfer in progress */
+       kcondvar_t              ub_rhxfercv;
        uint8_t                 ub_rhaddr;      /* roothub address */
        uint8_t                 ub_rhconf;      /* roothub configuration */
        struct usbd_device     *ub_devices[USB_TOTAL_DEVICES];
diff -r 1c8285880501 -r 97cd7553346d sys/dev/usb/usbroothub.c
--- a/sys/dev/usb/usbroothub.c  Sun Mar 13 11:28:42 2022 +0000
+++ b/sys/dev/usb/usbroothub.c  Sun Mar 13 11:28:52 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: usbroothub.c,v 1.14 2022/03/09 22:17:41 riastradh Exp $ */
+/* $NetBSD: usbroothub.c,v 1.15 2022/03/13 11:28:52 riastradh Exp $ */
 
 /*-
  * Copyright (c) 1998, 2004, 2011, 2012 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbroothub.c,v 1.14 2022/03/09 22:17:41 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbroothub.c,v 1.15 2022/03/13 11:28:52 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>         /* for ostype */
@@ -368,6 +368,9 @@
         */
        KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock));
 
+       /* Roothub xfers are serialized through the pipe.  */
+       KASSERTMSG(bus->ub_rhxfer == NULL, "rhxfer=%p", bus->ub_rhxfer);
+
        KASSERT(xfer->ux_rqflags & URQ_REQUEST);
        req = &xfer->ux_request;
 
@@ -549,19 +552,19 @@
                break;
        }
 
-       /*
-        * XXX This needs some mechanism for concurrent
-        * roothub_ctrl_abort to wait for ubm_rhctrl to finish.  We
-        * can't use the bus lock because many ubm_rhctrl methods do
-        * usb_delay_ms and many bus locks are taken in softint
-        * context, leading to deadlock in the softclock needed to wake
-        * up usb_delay_ms.
-        */
+       KASSERTMSG(bus->ub_rhxfer == NULL, "rhxfer=%p", bus->ub_rhxfer);
+       bus->ub_rhxfer = xfer;
        if (!bus->ub_usepolling)
                mutex_exit(bus->ub_lock);
+
        actlen = bus->ub_methods->ubm_rhctrl(bus, req, buf, buflen);
+
        if (!bus->ub_usepolling)
                mutex_enter(bus->ub_lock);
+       KASSERTMSG(bus->ub_rhxfer == xfer, "rhxfer=%p", bus->ub_rhxfer);
+       bus->ub_rhxfer = NULL;
+       cv_signal(&bus->ub_rhxfercv);
+
        if (actlen < 0)
                goto fail;
 
@@ -582,8 +585,19 @@
 Static void
 roothub_ctrl_abort(struct usbd_xfer *xfer)
 {
+       struct usbd_bus *bus = xfer->ux_bus;
 
-       /* Nothing to do, all transfers are synchronous. */
+       KASSERT(mutex_owned(bus->ub_lock));
+       KASSERTMSG(bus->ub_rhxfer == xfer, "rhxfer=%p", bus->ub_rhxfer);
+
+       /*
+        * No mechanism to abort the xfer (would have to coordinate
+        * with the bus's ubm_rhctrl to be useful, and usually at most
+        * there's some short bounded delays of a few tens of
+        * milliseconds), so just wait for it to complete.
+        */
+       while (bus->ub_rhxfer == xfer)
+               cv_wait(&bus->ub_rhxfercv, bus->ub_lock);
 }
 
 /* Close the root pipe. */