This code path is used both for xfers that are new, and xfers that
are being done piece by piece and are partway done. For the latter
case, skip usbd_xfer_schedule_timeout so we schedule it only once per
xfer.
- Call usbd_xfer_schedule_timeout so we actually do time out.
- Don't call usbd_xfer_trycomplete until all the data have been
transferred -- it commits to completion, not timeout.
- Use xfer->ux_status != USBD_IN_PROGRESS to test whether, after a
partial write, an xfer has been interrupted or timed out and need
not be continued.
- Remove wrong assertion.
- 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!)
New API for HCI drivers to synchronize hardware completion
interrupts, synchronous aborts, and asynchronous timeouts:
- When submitting an xfer to hardware, call
usbd_xfer_schedule_timeout(xfer).
- On HCI completion interrupt for xfer completion:
if (!usbd_xfer_trycomplete(xfer))
return; /* timed out or aborted, ignore it */
- In upm_abort methods, call usbd_xfer_abort(xfer).
For HCI drivers that use this API (not needed in drivers that don't,
or for xfers like root intr xfers that don't use it):
- New ubm_abortx method serves role of former *hci_abort_xfer, but
without any logic for wrangling timeouts/callouts/tasks -- caller
in usbd_xfer_abort has already handled them.
- New ubm_dying method, returns true if the device is in the process
of detaching, used by the timeout logic.
Converted and tested:
- ehci
- ohci
Converted and compile-tested:
- ahci (XXX did this ever work?)
- dwc2
- motg (XXX missing usbd_xfer_schedule_timeout in motg_*_start?)
- uhci
- xhci
Not changed:
- slhci (sys/dev/ic/sl811hs.c) -- doesn't use a separate per-xfer
callout for timeouts (XXX but maybe should?)
- ugenhc (sys/rump/dev/lib/libugenhc/ugenhc.c) -- doesn't manage its
own transfer timeouts
- vhci -- times transfers out only on detach; could be adapted easily
if we wanted to use the xfer->ux_callout
are detached.
This is because xfers of USBD_NOT_STARTED can be removed from queue
in an invisible way to host controller drivers.
Discussed on tech-kern.
These functions are defined on unsigned int. The generic name
min/max should not silently truncate to 32 bits on 64-bit systems.
This is purely a name change -- no functional change intended.
HOWEVER! Some subsystems have
#define min(a, b) ((a) < (b) ? (a) : (b))
#define max(a, b) ((a) > (b) ? (a) : (b))
even though our standard name for that is MIN/MAX. Although these
may invite multiple evaluation bugs, these do _not_ cause integer
truncation.
To avoid `fixing' these cases, I first changed the name in libkern,
and then compile-tested every file where min/max occurred in order to
confirm that it failed -- and thus confirm that nothing shadowed
min/max -- before changing it.
I have left a handful of bootloaders that are too annoying to
compile-test, and some dead code:
cobalt ews4800mips hp300 hppa ia64 luna68k vax
acorn32/if_ie.c (not included in any kernels)
macppc/if_gm.c (superseded by gem(4))
It should be easy to fix the fallout once identified -- this way of
doing things fails safe, and the goal here, after all, is to _avoid_
silent integer truncations, not introduce them.
Maybe one day we can reintroduce min/max as type-generic things that
never silently truncate. But we should avoid doing that for a while,
so that existing code has a chance to be detected by the compiler for
conversion to uimin/uimax without changing the semantics until we can
properly audit it all. (Who knows, maybe in some cases integer
truncation is actually intended!)
ideas from Taylor and Nick, and myself. special thanks to both who
inspired much of the code here, if not wrote it directly.
among other problems, this assert should no longer trigger:
panic: kernel diagnostic assertion "xfer->ux_state == XFER_ONQU" failed: file "/current/src/sys/dev/usb/usbdi.c", line 914
using usbhist i was able to track down my instance of it being related
to userland close() beginning, dropping the sc_lock, and then the usb
softintr completes the transfer normally, and when it is done, the
abort path attempts to re-complete the transfer, and the above assert
is tripped.
changes from nhusb were commited with these logs:
--
Move the struct usb_task to struct usbd_xfer for everyone to use.
--
Set device transfer status to USBD_IN_PROGRESS if start methods succeeds
--
Actually set the transfer status on transfers in ohci_abort_xfer and
the controller is dying
--
Don't supply the lock to callout_halt when polling as it won't be held
--
Improve transfer abort
--
Mark device transfers as USBD_IN_PROGRESS appropriately and improve
abort handling
--
#ifdef DIAGNOSTIC -> KASSERT and add another KASSERT
--
Mark device transfers as USBD_IN_PROGRESS appropriately and improve
abort handling
--
additional changes include:
- initialise the usb abort task in the HCI allocx routine, so that it
can be safely usb_rem_task()'d.
- rework the handling of softintr vs cancellation vs timeout abort based
upon a scheme from Taylor:
when completing a transfer normally:
- if the status is not in progress, it must be cancelled or timed out,
and we should not process this xfer.
- set the status as normal.
- unconditionallly callout_stop() and usb_rem_task(). they're safe and
either aren't running, or will run and do nothing.
- finally call usb_transfer_complete().
when aborting a transfer:
- status should be cancelled or timed out.
- if cancelling, callout_halt and usb_rem_task_wait() to make sure the
timer is either done or cancelled.
- at this point, the ux_status must not be cancelled or timed out, and
if it is not in progress we're done.
- set the status.
- if the controller is dying, just return.
- perform HCI-specific tasks to abort this xfer.
- finally call usb_transfer_complete().
for the timeout and timeout task:
- if the HCI is not dying, and the ux_status is in progress, then
trigger the usb abort task.
- remove UXFER_ABORTWAIT and UXFER_ABORTING.
tested on:
- multiple PC systems with several types of devices: ugen/UPS, ucom,
umass with disk, ssd and cdrom backends, kbd, ms, using uhci, ehci
and xhci.
- erlite3: sd@umass on dwc2.
- sunblade2000: kbd/ms and umass disk on ohci.
untested:
- motg, slhci and ahci. motg has some portion of the new scheme
applied, but slhci and ahci require more study.
future work includes pushing a lot of the common abort handling into
usbdi.c and leaving upm_abort() for HC specific tasks, but this change
is pullup-able to netbsd-7 and netbsd-8 as it does not change any
external API, as well as removing over 100 lines of code while adding
over 30 new asserts.
XXX: pullup-7, pullup-8.
device and string descriptors.
Firstly: Few vendors have identical PCI-SIG vendor IDs and USB-IF vendor
IDs. As such, using the PCI vendor ID as a USB vendor ID may trample
on whomever is allocated that USB vendor ID.
Secondly: The vendor of the host controller hardware implementation has
little to nothing to do with our usbroothub implementation. Thus we
should not potentially associate any problems therewith to such third
party.
This change will result in root hubs being identified by USB Vendor ID
0x0000. Root hub vendor string will now be "NetBSD" (or, specifically:
ostype). Product ID (0x0000) and product strings remain unchanged.
in PR kern/52639, as well as some general cleaning-up...
(As proposed on tech-kern@ with additional changes and enhancements.)
Details of changes:
* All history arguments are now stored as uintmax_t values[1], both in
the kernel and in the structures used for exporting the history data
to userland via sysctl(9). This avoids problems on some architectures
where passing a 64-bit (or larger) value to printf(3) can cause it to
process the value as multiple arguments. (This can be particularly
problematic when printf()'s format string is not a literal, since in
that case the compiler cannot know how large each argument should be.)
* Update the data structures used for exporting kernel history data to
include a version number as well as the length of history arguments.
* All [2] existing users of kernhist(9) have had their format strings
updated. Each format specifier now includes an explicit length
modifier 'j' to refer to numeric values of the size of uintmax_t.
* All [2] existing users of kernhist(9) have had their format strings
updated to replace uses of "%p" with "%#jx", and the pointer
arguments are now cast to (uintptr_t) before being subsequently cast
to (uintmax_t). This is needed to avoid compiler warnings about
casting "pointer to integer of a different size."
* All [2] existing users of kernhist(9) have had instances of "%s" or
"%c" format strings replaced with numeric formats; several instances
of mis-match between format string and argument list have been fixed.
* vmstat(1) has been modified to handle the new size of arguments in the
history data as exported by sysctl(9).
* vmstat(1) now provides a warning message if the history requested with
the -u option does not exist (previously, this condition was silently
ignored, with only a single blank line being printed).
* vmstat(1) now checks the version and argument length included in the
data exported via sysctl(9) and exits if they do not match the values
with which vmstat was built.
* The kernhist(9) man-page has been updated to note the additional
requirements imposed on the format strings, along with several other
minor changes and enhancements.
[1] It would have been possible to use an explicit length (for example,
uint64_t) for the history arguments. But that would require another
"rototill" of all the users in the future when we add support for an
architecture that supports a larger size. Also, the printf(3) format
specifiers for explicitly-sized values, such as "%"PRIu64, are much
more verbose (and less aesthetically appealing, IMHO) than simply
using "%ju".
[2] I've tried very hard to find "all [the] existing users of kernhist(9)"
but it is possible that I've missed some of them. I would be glad to
update any stragglers that anyone identifies.
- API / infrastructure changes to support memory management changes.
- Memory management improvements and bug fixes.
- HCDs should now be MP safe
- conversion to KERNHIST based debug
- FS/LS isoc support on ehci(4).
- conversion to kmem(9)
- Some USB 3 support - mostly from Takahiro HAYASHI (t-hash).
- interrupt transfers now get proper DMA operations
- general bug fixes
- kern/48308
- uhub status notification improvements
- umass(4) probe fix (applied to HEAD already)
- ohci(4) short transfer fix
were getting out of hand), move the AllWinner reg definitions to
arch/arm/allwinner/awin_otgreg.h and include that instead of
dev/usb/motgreg.h when MOTG_ALLWINNER is defined.
as found in several SoCs.
Only host mode is supported, good enough for umass (USB keys), kubd and ums,
and USB2 hubs.
It fails to properly talk to USB/serial adapters at this time.
