From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>
Cc: linux-usb@vger.kernel.org
Subject: [PATCH v4 0/3] xhci: Fix Stop Endpoint problems
Date: Mon, 4 Nov 2024 10:32:00 +0100 [thread overview]
Message-ID: <20241104103200.533fe1fb@foxbook> (raw)
v4 of the attempt to sort out Stop Endpoint retries and related issues.
General design:
1/3 guarantee no infinite retries under any failure conditions, ever.
2/3 fix bugs and make Set TR Dequeue handler call invalidate/giveback.
3/3 eliminate cases of pointless Stop EP commands, depends on 2/3.
Changes since v3:
2/3 no longer tries to add code cleanups (and bugs, as it turned out)
to a bugfix patch, and became much shorter as a result.
3/3 dropped a desperate attempt at detecting a halt/unlink race, which
I was unable to reproduce because it seems impossible - EP_HALTED
or SET_DEQ_PENDING remains set until after the halted URB is given
back to USB core, so we would never queue a pointless Stop command.
Also some commit message and comment cleanups in all three.
And I documented another bug which 2/3 appears to (partly) fix.
The state of other chips:
VIA VL805 and Etron EJ168A don't seem to have the "latent restart" bug
so they don't care about this code, unless it causes pointless retries
on an already stopped endpoint for 100ms, if it's enabled for them.
ASM1042 and ASM3142 have the bug, although not as common as on NEC. It
seems to be particularly triggered by multiple endpoints cancelling at
the same time, so for example, both loops below simultaneously:
while true; do timeout .3 yavta -c /dev/video0; done
while true; do timeout .1 cat /dev/ttyUSB0 >/dev/null; done
On ASM3142 this triggers the bug about once per minute, and prints:
WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state.
The "mitigation" patch seems to be helping, because reverting it adds:
ERROR Transfer event TRB DMA ptr not part of current TD ep_index 6 comp_code 13
Looking for event-dma 0000000003715ec0 trb-start 0000000003715ed0 trb-end 0000000003715ed0 seg-start 0000000003715000 seg-end 0000000003715ff0
Enabling the NEC workaround for this chip appears to fix the problem
entirely and so far it only took one retry each time:
Stop ep completion ctx error, ctx_state 3
I have left this running overninght and nothing unexpected was logged
in dmesg. IOW, no undesirable side effects known so far. But I haven't
done much tests with bad cables yet, and ASMedia chips appear to be
quite buggy and prone to strange internal race conditions. Examples:
- the ugly ifconfig up/down lockup issue on 3142 (not tried 1042 yet)
- a pair of "stopped" events out of order preceding the above lockup
- on 1042 Set Deq on streams seems to be no-op (streams are disabled)
- Soft Retry preceding a Set Deq seems to undo the Set Deq's effect
- Stop Endpoint *success* and then a spontaneous restart (1042 only)
- spontaneous restart causes a halt, but transfer event is missing
- TRB 15 (Stop EP) on the event ring (seen only once on 1042)
ASM1042 still not tested for compatilibity with the workaround at all.
Michal Pecio (3):
usb: xhci: Limit Stop Endpoint retries
usb: xhci: Fix TD invalidation under pending Set TR Dequeue
usb: xhci: Avoid queuing redundant Stop Endpoint commands
drivers/usb/host/xhci-ring.c | 59 ++++++++++++++++++++++++++++++------
drivers/usb/host/xhci.c | 21 ++++++++++---
drivers/usb/host/xhci.h | 2 ++
3 files changed, 69 insertions(+), 13 deletions(-)
next reply other threads:[~2024-11-04 9:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-04 9:32 Michal Pecio [this message]
2024-11-04 9:33 ` [PATCH v4 1/3] usb: xhci: Limit Stop Endpoint retries Michal Pecio
2024-11-04 9:33 ` [PATCH v4 2/3] usb: xhci: Fix TD invalidation under pending Set TR Dequeue Michal Pecio
2024-11-04 9:34 ` [PATCH v4 3/3] usb: xhci: Avoid queuing redundant Stop Endpoint commands Michal Pecio
2024-11-05 11:05 ` [PATCH v4 0/3] xhci: Fix Stop Endpoint problems Mathias Nyman
2024-11-05 12:18 ` Michał Pecio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241104103200.533fe1fb@foxbook \
--to=michal.pecio@gmail.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).