[src/trunk]: src/sys/dev deal with partial attach failures in usb_attach vs u...

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

details:   https://anonhg.NetBSD.org/src/rev/5ec17039cbc5
branches:  trunk
changeset: 433511:5ec17039cbc5
user:      mrg <mrg%NetBSD.org@localhost>
date:      Tue Sep 18 05:24:10 2018 +0000

description:
deal with partial attach failures in usb_attach vs usb_detach aka PR 53598.

- make sure xhci's sc->sc_ios is NULL if failure happens.
- rearrange usb_attach() / usb_doattach() to make it simpler to clean up.
- move usb_async_intr softint into usb_once_init().  previously, each USB
  controller would start a new one, and leave the old one leaked.
- handle controller interrupts without a bus attached

diffstat:

 sys/dev/pci/xhci_pci.c |   6 ++++--
 sys/dev/usb/usb.c      |  28 ++++++++++++++++++++--------
 2 files changed, 24 insertions(+), 10 deletions(-)

diffs (132 lines):

diff -r 8b519c00a614 -r 5ec17039cbc5 sys/dev/pci/xhci_pci.c
--- a/sys/dev/pci/xhci_pci.c    Tue Sep 18 02:58:10 2018 +0000
+++ b/sys/dev/pci/xhci_pci.c    Tue Sep 18 05:24:10 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xhci_pci.c,v 1.13 2018/06/29 17:48:24 msaitoh Exp $    */
+/*     $NetBSD: xhci_pci.c,v 1.14 2018/09/18 05:24:10 mrg Exp $        */
 /*     OpenBSD: xhci_pci.c,v 1.4 2014/07/12 17:38:51 yuo Exp   */
 
 /*
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xhci_pci.c,v 1.13 2018/06/29 17:48:24 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xhci_pci.c,v 1.14 2018/09/18 05:24:10 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_xhci_pci.h"
@@ -143,6 +143,7 @@
        printf("%s: csr: %08x\n", __func__, csr);
 #endif
        if ((csr & PCI_COMMAND_MEM_ENABLE) == 0) {
+               sc->sc_ios = 0;
                aprint_error_dev(self, "memory access is disabled\n");
                return;
        }
@@ -160,6 +161,7 @@
                }
                break;
        default:
+               sc->sc_ios = 0;
                aprint_error_dev(self, "BAR not 64 or 32-bit MMIO\n");
                return;
        }
diff -r 8b519c00a614 -r 5ec17039cbc5 sys/dev/usb/usb.c
--- a/sys/dev/usb/usb.c Tue Sep 18 02:58:10 2018 +0000
+++ b/sys/dev/usb/usb.c Tue Sep 18 05:24:10 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usb.c,v 1.172 2018/09/16 20:21:56 mrg Exp $    */
+/*     $NetBSD: usb.c,v 1.173 2018/09/18 05:24:10 mrg 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.172 2018/09/16 20:21:56 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.173 2018/09/18 05:24:10 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -138,6 +138,7 @@
        struct lwp      *sc_event_thread;
 
        char            sc_dying;
+       bool            sc_pmf_registered;
 };
 
 struct usb_taskq {
@@ -189,6 +190,7 @@
 Static struct selinfo usb_selevent;
 Static kmutex_t usb_event_lock;
 Static kcondvar_t usb_event_cv;
+/* XXX this is gross and broken */
 Static proc_t *usb_async_proc;  /* process that wants USB SIGIO */
 Static void *usb_async_sih;
 Static int usb_dev_open = 0;
@@ -239,6 +241,9 @@
        sc->sc_bus = aux;
        usbrev = sc->sc_bus->ub_revision;
 
+       cv_init(&sc->sc_bus->ub_needsexplore_cv, "usbevt");
+       sc->sc_pmf_registered = false;
+
        aprint_naive("\n");
        aprint_normal(": USB revision %s", usbrev_str[usbrev]);
        switch (usbrev) {
@@ -307,6 +312,11 @@
                 * end up using them in usb_doattach().
                 */
        }
+
+       KASSERT(usb_async_sih == NULL);
+       usb_async_sih = softint_establish(SOFTINT_CLOCK | SOFTINT_MPSAFE,
+          usb_async_intr, NULL);
+
        return 0;
 }
 
@@ -342,8 +352,6 @@
                panic("usb_doattach");
        }
 
-       cv_init(&sc->sc_bus->ub_needsexplore_cv, "usbevt");
-
        ue = usb_alloc_event();
        ue->u.ue_ctrlr.ue_bus = device_unit(self);
        usb_add_event(USB_EVENT_CTRLR_ATTACH, ue);
@@ -383,9 +391,8 @@
 
        if (!pmf_device_register(self, NULL, NULL))
                aprint_error_dev(self, "couldn't establish power handler\n");
-
-       usb_async_sih = softint_establish(SOFTINT_CLOCK | SOFTINT_MPSAFE,
-          usb_async_intr, NULL);
+       else
+               sc->sc_pmf_registered = true;
 
        return;
 }
@@ -1179,6 +1186,10 @@
 
        DPRINTFN(10, "polling=%jd", bus->ub_usepolling, 0, 0, 0);
 
+       /* In case the bus never finished setting up. */
+       if (__predict_false(bus->ub_soft == NULL))
+               return;
+
        if (bus->ub_usepolling) {
                bus->ub_methods->ubm_softint(bus);
        } else {
@@ -1231,7 +1242,8 @@
            (rc = usb_disconnect_port(&sc->sc_port, self, flags)) != 0)
                return rc;
 
-       pmf_device_deregister(self);
+       if (sc->sc_pmf_registered)
+               pmf_device_deregister(self);
        /* Kill off event thread. */
        sc->sc_dying = 1;
        while (sc->sc_event_thread != NULL) {