| Age | Commit message (Collapse) | Author |
|---|
|
Commit 9ebf30007858 ("xhci: Fix halted endpoint at stop endpoint command
completion") in 5.12 changes how cancelled URBs are given back.
To cancel a URB xhci driver needs to stop the endpoint first.
To clear a halted endpoint xhci driver needs to reset the endpoint.
In rare cases when an endpoint halt (error) races with a endpoint stop we
need to clear the reset before removing, and giving back the cancelled URB.
The above change in 5.12 takes care of this, but it also relies on the
reset endpoint completion handler to give back the cancelled URBs.
There are cases when driver refuses to queue reset endpoint commands,
for example when a link suddenly goes to an inactive error state.
In this case the cancelled URB is never given back.
Fix this by giving back the URB in the stop endpoint if queuing a reset
endpoint command fails.
Fixes: 9ebf30007858 ("xhci: Fix halted endpoint at stop endpoint command completion")
CC: <stable@vger.kernel.org> # 5.12
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210512080816.866037-3-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The same values are parsed several times from transfer and event
TRBs by different functions in the same call path, all while processing
one transfer event.
As the TRBs are in DMA memory and can be accessed by the xHC host we want
to avoid this to prevent double-fetch issues.
To resolve this pass the already parsed values to the different functions
in the path of parsing a transfer event
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210406070208.3406266-5-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Force-threaded interrupt handlers used to run with interrupts enabled,
something which could lead to deadlocks in case a threaded handler
shared a lock with code running in hard interrupt context (e.g. timer
callbacks) and did not explicitly disable interrupts.
Since commit 81e2073c175b ("genirq: Disable interrupts for force
threaded handlers") interrupt handlers always run with interrupts
disabled on non-RT so that drivers no longer need to do handle forced
threading ("threadirqs").
Drop the now obsolete workaround added by commit 63aea0dbab90 ("USB:
xhci: fix lock-inversion problem").
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20210322111140.32056-1-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
We want the USB fixes in here as well.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
On some systems rt2800usb and mt7601u devices are unable to operate since
commit f8f80be501aa ("xhci: Use soft retry to recover faster from
transaction errors")
Seems that some xHCI controllers can not perform Soft Retry correctly,
affecting those devices.
To avoid the problem add xhci->quirks flag that restore pre soft retry
xhci behaviour for affected xHCI controllers. Currently those are
AMD_PROMONTORYA_4 and AMD_PROMONTORYA_2, since it was confirmed
by the users: on those xHCI hosts issue happen and is gone after
disabling Soft Retry.
[minor commit message rewording for checkpatch -Mathias]
Fixes: f8f80be501aa ("xhci: Use soft retry to recover faster from transaction errors")
Cc: <stable@vger.kernel.org> # 4.20+
Reported-by: Bernhard <bernhard.gebetsberger@gmx.at>
Tested-by: Bernhard <bernhard.gebetsberger@gmx.at>
Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202541
Link: https://lore.kernel.org/r/20210311115353.2137560-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Currently xhci-hcd.ko building depends on USB_XHCI_MTK, this
is not flexible for some cases. For example:
USB_XHCI_HCD is y, and USB_XHCI_MTK is m, then we can't
implement extended functions if only update xhci-mtk.ko
This patch is used to remove the dependence.
Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Link: https://lore.kernel.org/r/0b62e21ddfacc1c2874726dd27ccab80c993f303.1615170625.git.chunfeng.yun@mediatek.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
We need the USB fixes in here as well.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
xhci driver may in some special cases need to copy small amounts
of payload data to a bounce buffer in order to meet the boundary
and alignment restrictions set by the xHCI specification.
In the majority of these cases the data is in a sg list, and
driver incorrectly assumed data is always in urb->sg when using
the bounce buffer.
If data instead is contiguous, and in urb->transfer_buffer, we may still
need to bounce buffer a small part if data starts very close (less than
packet size) to a 64k boundary.
Check if sg list is used before copying data to/from it.
Fixes: f9c589e142d0 ("xhci: TD-fragment, align the unsplittable case with a bounce buffer")
Cc: stable@vger.kernel.org
Reported-by: Andreas Hartmann <andihartmann@01019freenet.de>
Tested-by: Andreas Hartmann <andihartmann@01019freenet.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210203113702.436762-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
If we receive a transfer event indicating that an endpoint should be
halted, but current endpoint state doesn't match it, then the halt might
be just resolved by the stop endpoint completion handler that detects the
halted endpoint due to a context state error.
In this case the TD we halted on is already moved to the cancelled TD list,
and should not be successfully completed and given back anymore.
Let the stop endpoint completion handler reset the endpoint, and then let
the reset endpoint handler give back the cancelled TD among all other
ones on the cancelled TD list
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-28-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
A halted endpoint can be detected both when transfer events complete, and
in stop endpoint command completion. Both these handlers will start
clearing up the halted endpoint and queue a reset endpoint command.
It's possible to get both events for the same halted endpoint if right
after a URB cancel queues a stop endpoint command the endpoint stalls.
Use the EP_HALTED flag to prevent resetting the endpoint twice.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-27-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
xhci_find_new_dequeue_state() and xhci_queue_new_dequeue_state() are no
longer used afer introducing the move_dequeue_past_td() function.
also remove struct xhci_dequeue_state as its no longer used.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-26-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Replace xhci_find_new_dequeue_state() and xhci_queue_new_dequeue_state()
functions with one combined function.
These function were always called after each other, and had a lot of extra
code just to pass the newly found dequeue state from the first function
to the other.
The new function also returns error in case there is a failure to
queue the new dequeue state. This way the caller can decide on
recovery measures to handle it.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-25-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Handle race where a stop endpoint command fails with "context state error"
as hardware hasn't actually started the ring yet after a previous urb
cancellation completed and restarted the endpoint.
Flushing the doorbell write that restart the endpoint reduced these cases,
but didn't completely resolve them.
Check if the ring is running in the stop endpoint completion handler, and
issue a new stop endpoint command in this case.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-24-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
xhci 4.6.9: "A busy endpoint may asynchronously transition from the
Running to the Halted or Error state due to error conditions detected
while processing TRBs. A possible race condition may occur if software,
thinking an endpoint is in the running state, issues a Stop Endpoint
Command, however at the same time the xHC asynchronously transitions
the endpoint to the Halted or Error state. In this case, a Context
State Error may be generated for the command completion. Software
may verify that this case occurred by inspecting the EP State for
Halted or Error when a Stop Endpoint Command results in a Context
State Error."
Halted endpoints were not detected or handled at all in the stop endpoint
completion handler. A set TR Deq ptr command was bluntly queued instead
of resetting the endpoint first. The set TR Deq command would fail with
a context state error.
Fix this case by resetting the halted endpoint first to get it to a
stopped state instead of the halted (error) state.
Handle cancelled TDs once endpoint reset completes,
invalidating cancelled TDs on ring either by turning them to no-op,
or in case ring stopped on cancelled TD then move hardware dequeue pointer
past it, which will clear the cancelled TD from hw cache, and make sure
HW does not process it
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-23-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Don't queue both a reset endpoint command and a
set TR deq command at once when handling a halted endpoint.
split this into two steps.
Initially only queue a reset endpoint command, and then if needed queue a
set TR deq command in the reset endpoint handler.
Note: This removes the RESET_EP_QUIRK handling which was added in
commit ac9d8fe7c6a8 ("USB: xhci: Add quirk for Fresco Logic xHCI hardware.")
This quirk was added in 2009 for prototype xHCI hardware meant for
evaluation purposes only, and should not reach consumers.
This hardware could not handle two commands queued at once, and had
bad data in the output context after a reset endpoint command.
After this patch two command are no longer queued at once, so that
part is solved in this rewrite, but the workaround for bad data in the
output context solved by issuing an extra configure endpoint command is
bluntly removed.
Adding this workaround to the new rewrite just adds complexity, and I
think it's time to let this quirk go.
Print a debug message instead.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-22-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Halted endpoints can be discoverd both when handling transfer events and
command completion events. Move code that handles halted endpoints before
both of those event handlers.
Rename the function to xhci_handle_halted_ep() to better describe
what it does. Try to reserve "cleanup" word in function names for last
stage cleanup activities.
No functional changes
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-21-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Refactor handler for stop endpoint command completion. Yank out the part
that invalidates cancelled TDs and turn it into a separate function.
Invalidating cancelled TDs should be done while the ring is stopped,
but not exclusively in the stop endpoint command completeion handler.
We will need to invalidate TDs after resetting endpoints as well.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-20-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
In cases where the TD can't be given back in current handler we want
to be able to store it until its time to return the TD.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-19-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
use the existing xhci_td_cleanup() to give back cancelled TDs when a
ring is stopped.
A minor change to make sure we don't try to remove an already removed
td from the list is needed as cancelled TDs are already removed from the
td_list immediatelty when it's cancelled.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-18-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
No funtional changes
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-17-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Create a separate helper function to issue reset endpont commands
to clear halted endpoints.
This is useful for cases where a halted endpoint is discovered while
completing another command, and the endpoint halt needs to be cleared
with a endpoint reset first.
No functional changes
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-16-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Stop endpoint command fails with "context state error" if the endpoint is
already stopped.
This case was observed when a previous URB cancel had just completed and
rang the doorbell to restart the ring, when a new URB cancel queued a stop
endpoint command.
>From xHC hardware pov the endpoint had not yet started, so the stop
endpoint command failed with context state error.
Right after this the doorbell ring took effect and ring was restarted.
Interrupt handler saw a stop endpoint command completion event with
"context state error" and discovered that the ring was back up in
running state.
flushing the write reduces these cases in stress testing, but does not
completely remove the issue.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-15-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
xhci driver relies on link TRBs existing in the correct places in TRB
ring buffers shared with the host controller.
The controller should not modify these link TRBs, but in theory a faulty
xHC could do it.
Add some basic sanity checks to avoid infinite loops in interrupt handler,
or accessing unallocated memory outside a ring segment due to missing or
misplaced link TRBs.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-14-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Instead of re-reading, masking and endianness correcting the same trb
several times to get the trb type from an event, just do it once and
store it in a local variable.
Also pass the trb_type directly to the vendor specific event handler,
avoiding one more similar read.
In addition to the security benefit this also cleans up the code
and helps readability.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-13-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
When finishing a TD we walk the endpoint dequeue trb pointer
until it matches the last TRB of the TD.
TDs can contain over 100 TRBs, meaning we call a function 100 times,
do a few comparisons and increase a couple values for each of these calls,
all in interrupt context.
This can all be avoided by adding a pointer to the last TRB segment, and
a number of TRBs in the TD. So instead of walking through each TRB just
set the new dequeue segment, pointer, and number of free TRBs directly.
Getting rid of the while loop also reduces the risk of getting stuck in a
infinite loop in the interrupt handler. Loop relied on valid matching
dequeue and last_trb values to break.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-12-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Check that the slot_id that we dug out from command completion event
TRB, is valid before using it to identify the slot associated with the
command that generated the event.
Signed-off-by: Lalithambika Krishna Kumar <lalithambika.krishnakumar@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-11-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
xhci driver links together segments in a ring buffer by turning the last
TRB of a segment into a link TRB, pointing to the beginning of
the next segment.
If the first TRB of every segment for some unknown reason is a link TRB
pointing to the next segment, then prepare_ring() loops indefinitely.
This isn't something the xhci driver would do.
xHC hardware has access to these rings, it sholdn't be writing link
TRBs either, but with broken xHC hardware this could in theory be
possible.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-10-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The one case that used this function can use the
xhci_triad_to_transfer_ring() helper instead.
Avoid having several functions doing basically the same thing.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-9-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Two existing ring helpers, xhci_triad_to_transfer_ring() and
xhci_stream_id_to_ring() have partially similar functionality.
Both have some limitation, especieally with boundary checking.
Add a new xhci_virt_ep_to_ring() helper with proper boundary checking
that can replace parts of one helper, and later will completely
replace the other helper.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-8-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Check that the xhci_virt_dev structure that we dug out based
on a slot_id value from a command completion is valid before
dereferencing it.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-7-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
In several event handlers we need to find the right endpoint
structure from slot_id and ep_index in the event.
Add a helper for this, check that slot_id and ep_index are valid.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-6-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
several command completion handlers are passed the event trb
as a paramtere even if it't not used.
Remove it.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-5-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Instead of passing slot id and endpoint index to
cleanup_halted_endpoint() pass the endpoint structure pointer
as it's already known.
Avoids again digging out the endpoint structure based on
slot id and endpoint index, and passing them along the
call chain for this purpose only.
Add slot_id to the virt_dev structure so that it
can easily be found from a virt_dev, or its child, the
virt_ep endpoint structure.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-4-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
isochronous endpoints do not support streams, meaning that
there is only one ring per endpoint.
Avoid double-fetching the transfer event DMA to get the
ring. Also makes passing the event to skip_isoc_td() uncecessary.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-3-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
When handling transfer events the event is passed along the handling
callpath and parsed again in several occasions.
The event contains slot_id and endpoint index, from which the driver
endpoint structure can be found. There wasn't however a way to get the
endpoint index or parent usb device from this endpoint structure.
A lot of extra event parsing, and thus some DMA doublefetch cases,
and excess variables and code can be avoided by adding endpoint index
and parent usb virt device pointer to the endpoint structure.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210129130044.206855-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Once the command ring doorbell is rung the xHC controller will parse all
command TRBs on the command ring that have the cycle bit set properly.
If the driver just started writing the next command TRB to the ring when
hardware finished the previous TRB, then HW might fetch an incomplete TRB
as long as its cycle bit set correctly.
A command TRB is 16 bytes (128 bits) long.
Driver writes the command TRB in four 32 bit chunks, with the chunk
containing the cycle bit last. This does however not guarantee that
chunks actually get written in that order.
This was detected in stress testing when canceling URBs with several
connected USB devices.
Two consecutive "Set TR Dequeue pointer" commands got queued right
after each other, and the second one was only partially written when
the controller parsed it, causing the dequeue pointer to be set
to bogus values. This was seen as error messages:
"Mismatch between completed Set TR Deq Ptr command & xHCI internal state"
Solution is to add a write memory barrier before writing the cycle bit.
Cc: <stable@vger.kernel.org>
Tested-by: Ross Zwisler <zwisler@google.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210115161907.2875631-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The Synopsys xHC has an internal TRB cache of size TRB_CACHE_SIZE for
each endpoint. The default value for TRB_CACHE_SIZE is 16 for SS and 8
for HS. The controller loads and updates the TRB cache from the transfer
ring in system memory whenever the driver issues a start transfer or
update transfer command.
For chained TRBs, the Synopsys xHC requires that the total amount of
bytes for all TRBs loaded in the TRB cache be greater than or equal to 1
MPS. Or the chain ends within the TRB cache (with a last TRB).
If this requirement is not met, the controller will not be able to send
or receive a packet and it will hang causing a driver timeout and error.
This can be a problem if a class driver queues SG requests with many
small-buffer entries. The XHCI driver will create a chained TRB for each
entry which may trigger this issue.
This patch adds logic to the XHCI driver to detect and prevent this from
happening.
For every (TRB_CACHE_SIZE - 2), we check the total buffer size of
the SG list and if the last window of (TRB_CACHE_SIZE - 2) SG list length
and we don't make up at least 1 MPS, we create a temporary buffer to
consolidate full SG list into the buffer.
We check at (TRB_CACHE_SIZE - 2) window because it is possible that there
would be a link and/or event data TRB that take up to 2 of the cache
entries.
We discovered this issue with devices on other platforms but have not
yet come across any device that triggers this on Linux. But it could be
a real problem now or in the future. All it takes is N number of small
chained TRBs. And other instances of the Synopsys IP may have smaller
values for the TRB_CACHE_SIZE which would exacerbate the problem.
Signed-off-by: Tejas Joglekar <joglekar@synopsys.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20201208092912.1773650-3-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The "fallthrough" pseudo-keyword was added as a portable way to denote
intentional fallthrough. Clang will still warn on cases where there is a
fallthrough to an immediate break. Add explicit breaks for those cases.
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Link: https://lore.kernel.org/r/20201111014716.260633-1-ndesaulniers@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
controllers with XHCI_AVOID_BEI quirk cause too frequent interrupts
and affect power management.
To avoid interrupting on every isochronous interval the BEI (Block
Event Interrupt) flag is set for all except the last Isoch TRB in a URB.
This lead to event ring filling up in case several isoc URB were
queued and cancelled rapidly, which some controllers didn't
handle well, and thus the XHCI_AVOID_BEI quirk was introduced.
see commit 227a4fd801c8 ("usb: xhci: apply XHCI_AVOID_BEI quirk to all
Intel xHCI controllers")
With the XHCI_AVOID_BEI quirk each Isoch TRB will trigger an interrupt.
This can cause up to 8000 interrupts per second for isochronous transfers
with HD USB3 cameras, affecting power saving.
The event ring fits 256 events, instead of interrupting on every
isochronous TRB if XHCI_AVOID_BEI is set we make sure at least every
8th Isochronous TRB asserts an interrupt, clearing the event ring.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20200918131752.16488-9-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Replace the existing /* fall through */ comments and its variants with
the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
|
|
On platforms with IOMMU enabled, multiple SGs can be coalesced into one
by the IOMMU driver. In that case the SG list processing as part of the
completion of a urb on a bulk endpoint can result into a NULL pointer
dereference with the below stack dump.
<6> Unable to handle kernel NULL pointer dereference at virtual address 0000000c
<6> pgd = c0004000
<6> [0000000c] *pgd=00000000
<6> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
<2> PC is at xhci_queue_bulk_tx+0x454/0x80c
<2> LR is at xhci_queue_bulk_tx+0x44c/0x80c
<2> pc : [<c08907c4>] lr : [<c08907bc>] psr: 000000d3
<2> sp : ca337c80 ip : 00000000 fp : ffffffff
<2> r10: 00000000 r9 : 50037000 r8 : 00004000
<2> r7 : 00000000 r6 : 00004000 r5 : 00000000 r4 : 00000000
<2> r3 : 00000000 r2 : 00000082 r1 : c2c1a200 r0 : 00000000
<2> Flags: nzcv IRQs off FIQs off Mode SVC_32 ISA ARM Segment none
<2> Control: 10c0383d Table: b412c06a DAC: 00000051
<6> Process usb-storage (pid: 5961, stack limit = 0xca336210)
<snip>
<2> [<c08907c4>] (xhci_queue_bulk_tx)
<2> [<c0881b3c>] (xhci_urb_enqueue)
<2> [<c0831068>] (usb_hcd_submit_urb)
<2> [<c08350b4>] (usb_sg_wait)
<2> [<c089f384>] (usb_stor_bulk_transfer_sglist)
<2> [<c089f2c0>] (usb_stor_bulk_srb)
<2> [<c089fe38>] (usb_stor_Bulk_transport)
<2> [<c089f468>] (usb_stor_invoke_transport)
<2> [<c08a11b4>] (usb_stor_control_thread)
<2> [<c014a534>] (kthread)
The above NULL pointer dereference is the result of block_len and the
sent_len set to zero after the first SG of the list when IOMMU driver
is enabled. Because of this the loop of processing the SGs has run
more than num_sgs which resulted in a sg_next on the last SG of the
list which has SG_END set.
Fix this by check for the sg before any attributes of the sg are
accessed.
[modified reason for null pointer dereference in commit message subject -Mathias]
Fixes: f9c589e142d04 ("xhci: TD-fragment, align the unsplittable case with a bounce buffer")
Cc: stable@vger.kernel.org
Signed-off-by: Sriharsha Allenki <sallenki@codeaurora.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20200514110432.25564-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The default control endpoint ep0 can return a STALL indicating the
device does not support the control transfer requests. This is called
a protocol stall and does not halt the endpoint.
xHC behaves a bit different. Its internal endpoint state will always
be halted on any stall, even if the device side of the endpiont is not
halted. So we do need to issue the reset endpoint command to clear the
xHC host intenal endpoint halt state, but should not request the HS hub
to clear the TT buffer unless device side of endpoint is halted.
Clearing the hub TT buffer at protocol stall caused ep0 to become
unresponsive for some FS/LS devices behind HS hubs, and class drivers
failed to set the interface due to timeout:
usb 1-2.1: 1:1: usb_set_interface failed (-110)
Fixes: ef513be0a905 ("usb: xhci: Add Clear_TT_Buffer")
Cc: <stable@vger.kernel.org> # v5.3
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20200421140822.28233-4-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
If a class driver cancels its only URB then the endpoint ring buffer will
appear empty to the xhci driver. xHC hardware may still process cached
TRBs, and complete with a STALL, halting the endpoint.
This halted endpoint was not handled correctly by xhci driver as events on
empty rings were all assumed to be spurious events.
xhci driver refused to restart the ring with EP_HALTED flag set, so class
driver was never informed the endpoint halted even if it queued new URBs.
The host side of the endpoint needs to be reset, and dequeue pointer should
be moved in order to clear the cached TRBs and resetart the endpoint.
Small adjustments in finding the new dequeue pointer are needed to support
the case of stall on an empty ring and unknown current TD.
Cc: <stable@vger.kernel.org>
cc: Jeremy Compostella <jeremy.compostella@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20200421140822.28233-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Like U3 case, xHCI spec doesn't specify the upper bound of U0 transition
time. The 20ms is not enough for some devices.
Intead of polling PLS or PLC, we can facilitate the port change event to
know that the link transits to U0 is completed.
While at it, also separate U0 and U3 case to make the code cleaner.
[variable rename to u3exit, and skip completion for usb2 ports -Mathias ]
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20200312144517.1593-8-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Additional debugging to show xHC USBSTS register when stop endpoint
command watchdog triggers and host is assumed dead.
useful to know the current status before the controller is stopped by
the xhci driver and everything is released and freed.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20200312144517.1593-4-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Don't show the same error message for transaction errors and split
transaction errors. It's very confusing while debugging.
Transaction errors are often due to electrical interference.
Split transaction errors are about xHC not being able to
schedule start and complete split transactions needed to address
low- and full-speed devices behind high-speed hubs.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20200312144517.1593-3-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
xhci driver claims it needs XHCI_TRUST_TX_LENGTH quirk for both
Broadcom/Cavium and a Renesas xHC controllers.
The quirk was inteded for handling false "success" complete event for
transfers that had data left untransferred.
These transfers should complete with "short packet" events instead.
In these two new cases the false "success" completion is reported
after a "short packet" if the TD consists of several TRBs.
xHCI specs 4.10.1.1.2 say remaining TRBs should report "short packet"
as well after the first short packet in a TD, but this issue seems so
common it doesn't make sense to add the quirk for all vendors.
Turn these events into short packets automatically instead.
This gets rid of the "The WARN Successful completion on short TX for
slot 1 ep 1: needs XHCI_TRUST_TX_LENGTH quirk" warning in many cases.
Cc: <stable@vger.kernel.org>
Reported-by: Eli Billauer <eli.billauer@gmail.com>
Reported-by: Ard Biesheuvel <ardb@kernel.org>
Tested-by: Eli Billauer <eli.billauer@gmail.com>
Tested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20191211142007.8847-6-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
A race in xhci USB3 remote wake handling may force device back to suspend
after it initiated resume siganaling, causing a missed resume event or warm
reset of device.
When a USB3 link completes resume signaling and goes to enabled (UO)
state a interrupt is issued and the interrupt handler will clear the
bus_state->port_remote_wakeup resume flag, allowing bus suspend.
If the USB3 roothub thread just finished reading port status before
the interrupt, finding ports still in suspended (U3) state, but hasn't
yet started suspending the hub, then the xhci interrupt handler will clear
the flag that prevented roothub suspend and allow bus to suspend, forcing
all port links back to suspended (U3) state.
Example case:
usb_runtime_suspend() # because all ports still show suspended U3
usb_suspend_both()
hub_suspend(); # successful as hub->wakeup_bits not set yet
==> INTERRUPT
xhci_irq()
handle_port_status()
clear bus_state->port_remote_wakeup
usb_wakeup_notification()
sets hub->wakeup_bits;
kick_hub_wq()
<== END INTERRUPT
hcd_bus_suspend()
xhci_bus_suspend() # success as port_remote_wakeup bits cleared
Fix this by increasing roothub usage count during port resume to prevent
roothub autosuspend, and by making sure bus_state->port_remote_wakeup
flag is only cleared after resume completion is visible, i.e.
after xhci roothub returned U0 or other non-U3 link state link on a
get port status request.
Issue rootcaused by Chiasheng Lee
Cc: <stable@vger.kernel.org>
Cc: Lee, Hou-hsun <hou-hsun.lee@intel.com>
Reported-by: Lee, Chiasheng <chiasheng.lee@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20191211142007.8847-3-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Patch "USB: HCD: support giveback of URB in tasklet context"[1]
introduced giveback of urb in tasklet context. [1] This patch was
applied to ehci but not xhci. [2] This patch significantly reduces
the hard irq time of xhci. Especially for uvc driver, the hard irq
including the uvc completion function runs quite long but applying
this patch reduces the hard irq time of xhci.
I have tested four SS devices to check if performance degradation
occurs when urb completion functions run in the tasklet context.
As a result of the test, all devices works well and shows very
similar performance with the upstream kernel. Moreover, usb ethernet
adapter show better performance than the upstream kernel about 5% for
RX and 2% for TX. Four SS devices is as follows.
SS devices for test
1. WD My Passport 2TB (external hard drive)
2. Sandisk Ultra Flair USB 3.0 32GB
3. Logitech Brio webcam
4. Iptime 1gigabit ethernet adapter (Mediatek RTL8153)
Test description
1. Mass storage (hard drive) performance test
- run below command 10 times and compute the average performance
dd if=/dev/sdN iflag=direct of=/dev/null bs=1G count=1
2. Mass storage (flash memory) performance test
- run below command 10 times and compute the average performance
dd if=/dev/sdN iflag=direct of=/dev/null bs=1G count=1
3. Webcam streaming performance test
- run simple capture program and get the average frame rate per second
- capture 1500 frames
- program link
https://github.com/asfaca/Webcam-performance-analyzing-tool
- video resolution : 4096 X 2160 (4K) at 30 or 24 fps
- device (Logitech Brio) spec url for the highest resolution and fps
https://support.logitech.com/en_gb/product/brio-stream/specs
4. USB Ethernet adapter performance test
- directly connect two linux machines with ethernet cable
- run pktgen of linux kernel and send 1500 bytes packets
- run vnstat to measure the network bandwidth for 180 seconds
Test machine
- CPU : Intel i5-7600 @ 3.5GHz
Test results
1. Mass storage (hard drive) performance test
WD My Passport 2TB (external hard drive)
--------------------------------------------------------------------
xhci without tasklet | xhci with tasklet
--------------------------------------------------------------------
103.667MB/s | 103.692MB/s
--------------------------------------------------------------------
2. Mass storage (flash memory) performance test
Sandisk Ultra Flair USB 3.0 32GB
--------------------------------------------------------------------
xhci without tasklet | xhci with tasklet
--------------------------------------------------------------------
129.727MB/s | 130.2MB/s
--------------------------------------------------------------------
3. Webcam streaming performance test
Logitech Brio webcam
--------------------------------------------------------------------
xhci without tasklet | xhci with tasklet
--------------------------------------------------------------------
26.4451 fps | 26.3949 fps
--------------------------------------------------------------------
4. USB Ethernet adapter performance test
Iptime 1gigabit ethernet adapter (Mediatek RTL8153)
--------------------------------------------------------------------
xhci without tasklet | xhci with tasklet
--------------------------------------------------------------------
RX 933.86 Mbit/s | 983.86 Mbit/s
--------------------------------------------------------------------
TX 830.18 Mbit/s | 882.75 Mbit/s
--------------------------------------------------------------------
[1], https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=94dfd7edfd5c9b605caf7b562de7a813d216e011
[2], https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=428aac8a81058e2303677a8fbf26670229e51d3a
Signed-off-by: Suwan Kim <suwan.kim027@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/1573836603-10871-4-git-send-email-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Trace when a register in the doorbell array is written,
both for host controller command doorbell and device doorbells,
including for which endpoint and stream
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/1573836603-10871-3-git-send-email-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
