[src/trunk]: src/sys/dev/usb new comment explains this hack:

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

details:   https://anonhg.NetBSD.org/src/rev/a05c2938d469
branches:  trunk
changeset: 998924:a05c2938d469
user:      mrg <mrg%NetBSD.org@localhost>
date:      Wed May 08 06:31:02 2019 +0000

description:
new comment explains this hack:

 * XXXMRG: Stall task can run after slot is disabled when yanked.
 * This hack notices that the xs has been memset() in
 * xhci_disable_slot() and returns.  Both xhci_reset_endpoint()
 * and xhci_set_dequeue() rely upon a valid ring setup for correct
 * operation, and the latter will fault, as would
 * usb_transfer_complete() if it got that far.

this truly fixes yanking active ucom devices (uchcom, uplcom and
umcs all tested.)  prior efforts largely worked by allowing the
stall task to run first, as a side effect to their actual work.

ok @skrll.

XXX: pullup.

diffstat:

 sys/dev/usb/xhci.c |  19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diffs (47 lines):

diff -r 7cd237458cd8 -r a05c2938d469 sys/dev/usb/xhci.c
--- a/sys/dev/usb/xhci.c        Wed May 08 06:10:30 2019 +0000
+++ b/sys/dev/usb/xhci.c        Wed May 08 06:31:02 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xhci.c,v 1.106 2019/02/17 04:17:52 rin Exp $   */
+/*     $NetBSD: xhci.c,v 1.107 2019/05/08 06:31:02 mrg Exp $   */
 
 /*
  * Copyright (c) 2013 Jonathan A. Kollasch
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.106 2019/02/17 04:17:52 rin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.107 2019/05/08 06:31:02 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1839,6 +1839,19 @@
        DPRINTFN(4, "xfer %#jx slot %ju dci %ju", (uintptr_t)xfer, xs->xs_idx,
            dci, 0);
 
+       /*
+        * XXXMRG: Stall task can run after slot is disabled when yanked.
+        * This hack notices that the xs has been memset() in
+        * xhci_disable_slot() and returns.  Both xhci_reset_endpoint()
+        * and xhci_set_dequeue() rely upon a valid ring setup for correct
+        * operation, and the latter will fault, as would
+        * usb_transfer_complete() if it got that far.
+        */
+       if (xs->xs_idx == 0) {
+               DPRINTFN(4, "ends xs_idx is 0", 0, 0, 0, 0);
+               return;
+       }
+
        xhci_reset_endpoint(xfer->ux_pipe);
        xhci_set_dequeue(xfer->ux_pipe);
 
@@ -2085,7 +2098,7 @@
                KASSERTMSG((xfer->ux_status == USBD_CANCELLED ||
                            xfer->ux_status == USBD_TIMEOUT),
                           "xfer %p status %x", xfer, xfer->ux_status);
-               return;;
+               return;
        }
 
        /* Otherwise, set the status.  */