Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Fix mistakes in previous sloppy change with root intr xf...



details:   https://anonhg.NetBSD.org/src/rev/eeb2fad06acc
branches:  trunk
changeset: 848939:eeb2fad06acc
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Feb 15 01:21:56 2020 +0000

description:
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!)

diffstat:

 sys/arch/mips/adm5120/dev/ahci.c |  10 +++++++---
 sys/dev/ic/sl811hs.c             |   8 ++++++--
 sys/dev/usb/ehci.c               |   9 +++++----
 sys/dev/usb/motg.c               |  37 +++++++++++++++++++++++++++++++++----
 sys/dev/usb/ohci.c               |  10 ++++++----
 sys/dev/usb/uhci.c               |   9 +++++----
 sys/dev/usb/vhci.c               |   8 ++++++--
 sys/dev/usb/xhci.c               |   6 ++++--
 sys/external/bsd/dwc2/dwc2.c     |  10 ++++++----
 9 files changed, 78 insertions(+), 29 deletions(-)

diffs (truncated from 412 to 300 lines):

diff -r 391305af3793 -r eeb2fad06acc sys/arch/mips/adm5120/dev/ahci.c
--- a/sys/arch/mips/adm5120/dev/ahci.c  Fri Feb 14 22:04:12 2020 +0000
+++ b/sys/arch/mips/adm5120/dev/ahci.c  Sat Feb 15 01:21:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ahci.c,v 1.19 2020/02/12 16:02:01 riastradh Exp $      */
+/*     $NetBSD: ahci.c,v 1.20 2020/02/15 01:21:56 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.19 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.20 2020/02/15 01:21:56 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -438,6 +438,7 @@
        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
@@ -754,11 +755,14 @@
 
        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_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;
 }
 
diff -r 391305af3793 -r eeb2fad06acc sys/dev/ic/sl811hs.c
--- a/sys/dev/ic/sl811hs.c      Fri Feb 14 22:04:12 2020 +0000
+++ b/sys/dev/ic/sl811hs.c      Sat Feb 15 01:21:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sl811hs.c,v 1.102 2019/12/27 09:41:50 msaitoh Exp $    */
+/*     $NetBSD: sl811hs.c,v 1.103 2020/02/15 01:21:56 riastradh Exp $  */
 
 /*
  * Not (c) 2007 Matthew Orgass
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.102 2019/12/27 09:41:50 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.103 2020/02/15 01:21:56 riastradh 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 391305af3793 -r eeb2fad06acc sys/dev/usb/ehci.c
--- a/sys/dev/usb/ehci.c        Fri Feb 14 22:04:12 2020 +0000
+++ b/sys/dev/usb/ehci.c        Sat Feb 15 01:21:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ehci.c,v 1.272 2020/02/12 16:02:01 riastradh Exp $ */
+/*     $NetBSD: ehci.c,v 1.273 2020/02/15 01:21:56 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.272 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.273 2020/02/15 01:21:56 riastradh Exp $");
 
 #include "ohci.h"
 #include "uhci.h"
@@ -785,6 +785,7 @@
                /* Just ignore the change. */
                goto done;
        }
+       KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
        p = xfer->ux_buf;
        m = uimin(sc->sc_noport, xfer->ux_length * 8 - 1);
@@ -2724,11 +2725,11 @@
                mutex_enter(&sc->sc_lock);
        KASSERT(sc->sc_intrxfer == NULL);
        sc->sc_intrxfer = xfer;
+       xfer->ux_status = USBD_IN_PROGRESS;
        if (!polling)
                mutex_exit(&sc->sc_lock);
 
-       xfer->ux_status = USBD_IN_PROGRESS;
-       return xfer->ux_status;
+       return USBD_IN_PROGRESS;
 }
 
 /* Abort a root interrupt request. */
diff -r 391305af3793 -r eeb2fad06acc sys/dev/usb/motg.c
--- a/sys/dev/usb/motg.c        Fri Feb 14 22:04:12 2020 +0000
+++ b/sys/dev/usb/motg.c        Sat Feb 15 01:21:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: motg.c,v 1.26 2020/02/12 16:01:00 riastradh Exp $      */
+/*     $NetBSD: motg.c,v 1.27 2020/02/15 01:21:56 riastradh Exp $      */
 
 /*
  * Copyright (c) 1998, 2004, 2011, 2012, 2014 The NetBSD Foundation, Inc.
@@ -40,7 +40,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.26 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.27 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -991,8 +991,16 @@
        KASSERT(mutex_owned(&sc->sc_lock));
        KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
 
-       sc->sc_intr_xfer = NULL;
+       /* If xfer has already completed, nothing to do here.  */
+       if (sc->sc_intr_xfer == NULL)
+               return;
 
+       /*
+        * Otherwise, sc->sc_intr_xfer had better be this transfer.
+        * Cancel it.
+        */
+       KASSERT(sc->sc_intr_xfer == xfer);
+       KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
        xfer->ux_status = USBD_CANCELLED;
        usb_transfer_complete(xfer);
 }
@@ -1032,7 +1040,14 @@
        if (sc->sc_dying)
                return USBD_IOERROR;
 
+       if (!polling)
+               mutex_enter(&sc->sc_lock);
+       KASSERT(sc->sc_intr_xfer == NULL);
        sc->sc_intr_xfer = xfer;
+       xfer->ux_status = USBD_IN_PROGRESS;
+       if (!polling)
+               mutex_exit(&sc->sc_lock);
+
        return USBD_IN_PROGRESS;
 }
 
@@ -1045,12 +1060,25 @@
 
        KASSERT(mutex_owned(&sc->sc_lock));
 
-       sc->sc_intr_xfer = NULL;
+       /*
+        * Caller must guarantee the xfer has completed first, by
+        * closing the pipe only after normal completion or an abort.
+        */
+       KASSERT(sc->sc_intr_xfer == NULL);
 }
 
 void
 motg_root_intr_done(struct usbd_xfer *xfer)
 {
+       struct motg_softc *sc = MOTG_PIPE2SC(pipe);
+       MOTGHIST_FUNC(); MOTGHIST_CALLED();
+
+       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;
 }
 
 void
@@ -1101,6 +1129,7 @@
 
        if (xfer == NULL)
                return; /* the interrupt pipe is not open */
+       KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
        pipe = xfer->ux_pipe;
        if (pipe->up_dev == NULL || pipe->up_dev->ud_bus == NULL)
diff -r 391305af3793 -r eeb2fad06acc sys/dev/usb/ohci.c
--- a/sys/dev/usb/ohci.c        Fri Feb 14 22:04:12 2020 +0000
+++ b/sys/dev/usb/ohci.c        Sat Feb 15 01:21:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ohci.c,v 1.294 2020/02/12 16:02:01 riastradh Exp $     */
+/*     $NetBSD: ohci.c,v 1.295 2020/02/15 01:21:56 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.294 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.295 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1698,6 +1698,8 @@
                /* Just ignore the change. */
                return;
        }
+       KASSERT(xfer == sc->sc_intrxfer);
+       KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
        p = xfer->ux_buf;
        m = uimin(sc->sc_noport, xfer->ux_length * 8 - 1);
@@ -2516,11 +2518,11 @@
                mutex_enter(&sc->sc_lock);
        KASSERT(sc->sc_intrxfer == NULL);
        sc->sc_intrxfer = xfer;
+       xfer->ux_status = USBD_IN_PROGRESS;
        if (!polling)
                mutex_exit(&sc->sc_lock);
 
-       xfer->ux_status = USBD_IN_PROGRESS;
-       return xfer->ux_status;
+       return USBD_IN_PROGRESS;
 }
 
 /* Abort a root interrupt request. */
diff -r 391305af3793 -r eeb2fad06acc sys/dev/usb/uhci.c
--- a/sys/dev/usb/uhci.c        Fri Feb 14 22:04:12 2020 +0000
+++ b/sys/dev/usb/uhci.c        Sat Feb 15 01:21:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uhci.c,v 1.292 2020/02/14 16:47:28 riastradh Exp $     */
+/*     $NetBSD: uhci.c,v 1.293 2020/02/15 01:21:56 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.292 2020/02/14 16:47:28 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.293 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1013,6 +1013,7 @@
        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
@@ -3905,12 +3906,12 @@
        sc->sc_ival = mstohz(ival);
        callout_schedule(&sc->sc_poll_handle, sc->sc_ival);
        sc->sc_intr_xfer = xfer;
+       xfer->ux_status = USBD_IN_PROGRESS;
 
        if (!polling)
                mutex_exit(&sc->sc_lock);
 
-       xfer->ux_status = USBD_IN_PROGRESS;
-       return xfer->ux_status;
+       return USBD_IN_PROGRESS;
 }
 
 /* Close the root interrupt pipe. */
diff -r 391305af3793 -r eeb2fad06acc sys/dev/usb/vhci.c
--- a/sys/dev/usb/vhci.c        Fri Feb 14 22:04:12 2020 +0000
+++ b/sys/dev/usb/vhci.c        Sat Feb 15 01:21:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vhci.c,v 1.5 2020/02/12 16:02:01 riastradh Exp $ */
+/*     $NetBSD: vhci.c,v 1.6 2020/02/15 01:21:56 riastradh Exp $ */
 
 /*
  * Copyright (c) 2019 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>



Home | Main Index | Thread Index | Old Index