Thu Mar 3 06:09:33 2022 UTC ()
usbdi(9): New usbd_suspend_pipe, usbd_resume_pipe.

- New usbd_suspend_pipe to persistently stop transfers on a pipe and
  cancel pending ones or wait for their callbacks to finish.
  Idempotent.

- New usbd_resume_pipe to allow transfers again.  Idempotent, but no
  new xfers may be submitted before repeating this.

  This way it is safe to usbd_abort_pipe in two threads concurrently,
  e.g. if one thread is closing a device while another is revoking it
  -- but the threads have to agree on when it is done being aborted
  before starting to use it again.

- Existing usbd_abort_pipe now does suspend then resume.  No change
  in semantics so drivers that relied on being able to submit
  transfers again won't be broken any worse than the already are
  broken.

This allows drivers to avoid races such as:

	/* read */
	if (sc->sc_dying)
		return ENXIO;
	/* (*) */
	err = usbd_bulk_transfer(...);

	/* detach or or close */
	sc->sc_dying = true;
	usbd_abort_pipe(...);
	wait_for_io_to_drain(...);

The detach or close logic might happen at the same time as (*), with
no way to stop the bulk transfer before it starts, leading to
deadlock when detach/close waits for I/O operations like read to
drain.  Instead, the close routine can use usbd_suspend_pipe, and the
usbd_bulk_transfer is guaranteed to fail.

But some drivers such as ucom(4) don't close and reopen pipes after
aborting them -- they open on attach and close on detach, and just
abort when the /dev node is closed, expecting that xfers will
continue to work when next opened.  These drivers can instead use
usbd_suspend_pipe on close and usbd_resume_pipe on open.  Perhaps it
would be better to make them open pipes on open and close pipes on
close, but these functions make for a less intrusive transition.


(riastradh)
diff -r1.227 -r1.228 src/sys/dev/usb/usbdi.c
diff -r1.106 -r1.107 src/sys/dev/usb/usbdi.h

cvs diff -r1.227 -r1.228 src/sys/dev/usb/usbdi.c (expand / switch to unified diff)

--- src/sys/dev/usb/usbdi.c 2022/03/03 06:08:50 1.227
+++ src/sys/dev/usb/usbdi.c 2022/03/03 06:09:33 1.228
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: usbdi.c,v 1.227 2022/03/03 06:08:50 riastradh Exp $ */ 1/* $NetBSD: usbdi.c,v 1.228 2022/03/03 06:09:33 riastradh Exp $ */
2 2
3/* 3/*
4 * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc. 4 * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc.
5 * All rights reserved. 5 * All rights reserved.
6 * 6 *
7 * This code is derived from software contributed to The NetBSD Foundation 7 * This code is derived from software contributed to The NetBSD Foundation
8 * by Lennart Augustsson (lennart@augustsson.net) at 8 * by Lennart Augustsson (lennart@augustsson.net) at
9 * Carlstedt Research & Technology, Matthew R. Green (mrg@eterna.com.au), 9 * Carlstedt Research & Technology, Matthew R. Green (mrg@eterna.com.au),
10 * and Nick Hudson. 10 * and Nick Hudson.
11 * 11 *
12 * Redistribution and use in source and binary forms, with or without 12 * Redistribution and use in source and binary forms, with or without
13 * modification, are permitted provided that the following conditions 13 * modification, are permitted provided that the following conditions
14 * are met: 14 * are met:
@@ -22,27 +22,27 @@ @@ -22,27 +22,27 @@
22 * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED 22 * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
23 * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 23 * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
24 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS 24 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
25 * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR 25 * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
26 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF 26 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
27 * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS 27 * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
28 * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN 28 * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
29 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) 29 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
30 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE 30 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
31 * POSSIBILITY OF SUCH DAMAGE. 31 * POSSIBILITY OF SUCH DAMAGE.
32 */ 32 */
33 33
34#include <sys/cdefs.h> 34#include <sys/cdefs.h>
35__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.227 2022/03/03 06:08:50 riastradh Exp $"); 35__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.228 2022/03/03 06:09:33 riastradh Exp $");
36 36
37#ifdef _KERNEL_OPT 37#ifdef _KERNEL_OPT
38#include "opt_usb.h" 38#include "opt_usb.h"
39#include "opt_compat_netbsd.h" 39#include "opt_compat_netbsd.h"
40#include "usb_dma.h" 40#include "usb_dma.h"
41#endif 41#endif
42 42
43#include <sys/param.h> 43#include <sys/param.h>
44#include <sys/systm.h> 44#include <sys/systm.h>
45#include <sys/kernel.h> 45#include <sys/kernel.h>
46#include <sys/device.h> 46#include <sys/device.h>
47#include <sys/kmem.h> 47#include <sys/kmem.h>
48#include <sys/proc.h> 48#include <sys/proc.h>
@@ -766,33 +766,49 @@ usbd_interface2endpoint_descriptor(struc @@ -766,33 +766,49 @@ usbd_interface2endpoint_descriptor(struc
766 766
767/* Some drivers may wish to abort requests on the default pipe, * 767/* Some drivers may wish to abort requests on the default pipe, *
768 * but there is no mechanism for getting a handle on it. */ 768 * but there is no mechanism for getting a handle on it. */
769void 769void
770usbd_abort_default_pipe(struct usbd_device *device) 770usbd_abort_default_pipe(struct usbd_device *device)
771{ 771{
772 usbd_abort_pipe(device->ud_pipe0); 772 usbd_abort_pipe(device->ud_pipe0);
773} 773}
774 774
775void 775void
776usbd_abort_pipe(struct usbd_pipe *pipe) 776usbd_abort_pipe(struct usbd_pipe *pipe)
777{ 777{
778 778
779 KASSERT(pipe != NULL); 779 usbd_suspend_pipe(pipe);
 780 usbd_resume_pipe(pipe);
 781}
 782
 783void
 784usbd_suspend_pipe(struct usbd_pipe *pipe)
 785{
780 786
781 usbd_lock_pipe(pipe); 787 usbd_lock_pipe(pipe);
782 usbd_ar_pipe(pipe); 788 usbd_ar_pipe(pipe);
783 usbd_unlock_pipe(pipe); 789 usbd_unlock_pipe(pipe);
784} 790}
785 791
 792void
 793usbd_resume_pipe(struct usbd_pipe *pipe)
 794{
 795
 796 usbd_lock_pipe(pipe);
 797 KASSERT(SIMPLEQ_EMPTY(&pipe->up_queue));
 798 pipe->up_aborting = 0;
 799 usbd_unlock_pipe(pipe);
 800}
 801
786usbd_status 802usbd_status
787usbd_clear_endpoint_stall(struct usbd_pipe *pipe) 803usbd_clear_endpoint_stall(struct usbd_pipe *pipe)
788{ 804{
789 struct usbd_device *dev = pipe->up_dev; 805 struct usbd_device *dev = pipe->up_dev;
790 usbd_status err; 806 usbd_status err;
791 807
792 USBHIST_FUNC(); USBHIST_CALLED(usbdebug); 808 USBHIST_FUNC(); USBHIST_CALLED(usbdebug);
793 SDT_PROBE1(usb, device, pipe, clear__endpoint__stall, pipe); 809 SDT_PROBE1(usb, device, pipe, clear__endpoint__stall, pipe);
794 810
795 /* 811 /*
796 * Clearing en endpoint stall resets the endpoint toggle, so 812 * Clearing en endpoint stall resets the endpoint toggle, so
797 * do the same to the HC toggle. 813 * do the same to the HC toggle.
798 */ 814 */
@@ -993,27 +1009,26 @@ usbd_ar_pipe(struct usbd_pipe *pipe) @@ -993,27 +1009,26 @@ usbd_ar_pipe(struct usbd_pipe *pipe)
993 /* Make the HC abort it (and invoke the callback). */ 1009 /* Make the HC abort it (and invoke the callback). */
994 SDT_PROBE1(usb, device, xfer, abort, xfer); 1010 SDT_PROBE1(usb, device, xfer, abort, xfer);
995 pipe->up_methods->upm_abort(xfer); 1011 pipe->up_methods->upm_abort(xfer);
996 while (pipe->up_callingxfer == xfer) { 1012 while (pipe->up_callingxfer == xfer) {
997 USBHIST_LOG(usbdebug, "wait for callback" 1013 USBHIST_LOG(usbdebug, "wait for callback"
998 "pipe = %#jx xfer = %#jx", 1014 "pipe = %#jx xfer = %#jx",
999 (uintptr_t)pipe, (uintptr_t)xfer, 0, 0); 1015 (uintptr_t)pipe, (uintptr_t)xfer, 0, 0);
1000 cv_wait(&pipe->up_callingcv, 1016 cv_wait(&pipe->up_callingcv,
1001 pipe->up_dev->ud_bus->ub_lock); 1017 pipe->up_dev->ud_bus->ub_lock);
1002 } 1018 }
1003 /* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */ 1019 /* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
1004 } 1020 }
1005 } 1021 }
1006 pipe->up_aborting = 0; 
1007 SDT_PROBE1(usb, device, pipe, abort__done, pipe); 1022 SDT_PROBE1(usb, device, pipe, abort__done, pipe);
1008} 1023}
1009 1024
1010/* Called with USB lock held. */ 1025/* Called with USB lock held. */
1011void 1026void
1012usb_transfer_complete(struct usbd_xfer *xfer) 1027usb_transfer_complete(struct usbd_xfer *xfer)
1013{ 1028{
1014 struct usbd_pipe *pipe = xfer->ux_pipe; 1029 struct usbd_pipe *pipe = xfer->ux_pipe;
1015 struct usbd_bus *bus = pipe->up_dev->ud_bus; 1030 struct usbd_bus *bus = pipe->up_dev->ud_bus;
1016 int sync = xfer->ux_flags & USBD_SYNCHRONOUS; 1031 int sync = xfer->ux_flags & USBD_SYNCHRONOUS;
1017 int erred; 1032 int erred;
1018 int polling = bus->ub_usepolling; 1033 int polling = bus->ub_usepolling;
1019 int repeat = pipe->up_repeat; 1034 int repeat = pipe->up_repeat;

cvs diff -r1.106 -r1.107 src/sys/dev/usb/usbdi.h (expand / switch to unified diff)

--- src/sys/dev/usb/usbdi.h 2022/03/03 06:06:52 1.106
+++ src/sys/dev/usb/usbdi.h 2022/03/03 06:09:33 1.107
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: usbdi.h,v 1.106 2022/03/03 06:06:52 riastradh Exp $ */ 1/* $NetBSD: usbdi.h,v 1.107 2022/03/03 06:09:33 riastradh Exp $ */
2/* $FreeBSD: src/sys/dev/usb/usbdi.h,v 1.18 1999/11/17 22:33:49 n_hibma Exp $ */ 2/* $FreeBSD: src/sys/dev/usb/usbdi.h,v 1.18 1999/11/17 22:33:49 n_hibma Exp $ */
3 3
4/* 4/*
5 * Copyright (c) 1998 The NetBSD Foundation, Inc. 5 * Copyright (c) 1998 The NetBSD Foundation, Inc.
6 * All rights reserved. 6 * All rights reserved.
7 * 7 *
8 * This code is derived from software contributed to The NetBSD Foundation 8 * This code is derived from software contributed to The NetBSD Foundation
9 * by Lennart Augustsson (lennart@augustsson.net) at 9 * by Lennart Augustsson (lennart@augustsson.net) at
10 * Carlstedt Research & Technology. 10 * Carlstedt Research & Technology.
11 * 11 *
12 * Redistribution and use in source and binary forms, with or without 12 * Redistribution and use in source and binary forms, with or without
13 * modification, are permitted provided that the following conditions 13 * modification, are permitted provided that the following conditions
14 * are met: 14 * are met:
@@ -115,26 +115,29 @@ void usbd_setup_default_xfer(struct usbd @@ -115,26 +115,29 @@ void usbd_setup_default_xfer(struct usbd
115 115
116void usbd_setup_isoc_xfer(struct usbd_xfer *, void *, uint16_t *, 116void usbd_setup_isoc_xfer(struct usbd_xfer *, void *, uint16_t *,
117 uint32_t, uint16_t, usbd_callback); 117 uint32_t, uint16_t, usbd_callback);
118 118
119void usbd_get_xfer_status(struct usbd_xfer *, void **, 119void usbd_get_xfer_status(struct usbd_xfer *, void **,
120 void **, uint32_t *, usbd_status *); 120 void **, uint32_t *, usbd_status *);
121 121
122usb_endpoint_descriptor_t *usbd_interface2endpoint_descriptor 122usb_endpoint_descriptor_t *usbd_interface2endpoint_descriptor
123 (struct usbd_interface *, uint8_t); 123 (struct usbd_interface *, uint8_t);
124 124
125void usbd_abort_pipe(struct usbd_pipe *); 125void usbd_abort_pipe(struct usbd_pipe *);
126void usbd_abort_default_pipe(struct usbd_device *); 126void usbd_abort_default_pipe(struct usbd_device *);
127 127
 128void usbd_suspend_pipe(struct usbd_pipe *);
 129void usbd_resume_pipe(struct usbd_pipe *);
 130
128usbd_status usbd_clear_endpoint_stall(struct usbd_pipe *); 131usbd_status usbd_clear_endpoint_stall(struct usbd_pipe *);
129void usbd_clear_endpoint_stall_async(struct usbd_pipe *); 132void usbd_clear_endpoint_stall_async(struct usbd_pipe *);
130 133
131void usbd_clear_endpoint_toggle(struct usbd_pipe *); 134void usbd_clear_endpoint_toggle(struct usbd_pipe *);
132usbd_status usbd_endpoint_count(struct usbd_interface *, uint8_t *); 135usbd_status usbd_endpoint_count(struct usbd_interface *, uint8_t *);
133 136
134usbd_status usbd_interface_count(struct usbd_device *, uint8_t *); 137usbd_status usbd_interface_count(struct usbd_device *, uint8_t *);
135 138
136void usbd_interface2device_handle(struct usbd_interface *, struct usbd_device **); 139void usbd_interface2device_handle(struct usbd_interface *, struct usbd_device **);
137usbd_status usbd_device2interface_handle(struct usbd_device *, 140usbd_status usbd_device2interface_handle(struct usbd_device *,
138 uint8_t, struct usbd_interface **); 141 uint8_t, struct usbd_interface **);
139 142
140struct usbd_device *usbd_pipe2device_handle(struct usbd_pipe *); 143struct usbd_device *usbd_pipe2device_handle(struct usbd_pipe *);