On 8.9.2025 14.01, Michal Pecio wrote:
Hi Mathias,
I wanted to finish and send v2 of "Simplify TD cancellation and drop
some states" over the weekend, but I encountered an annoying roadblock
and I need your input.
Basically, there is a bug: URB_ZERO_PACKET is queued as two TDs, and
when the first TD halts, the driver simply advances to the second one.
I found that extending the event handler to cover this case requires
multiple changes:
1. obviously, all TDs must be cancelled, not just the current one
2. they may be given back in weird order (waiting for Set Deq), so
we need to store the status on urb_priv and override td->status
3. xhci_invalidate_cancelled_tds() would need to recognize multiple
halted TDs on the same URB as valid
This is doable, and I have already implemented most of it in that
series, but there is an alternative solution: simply stop worrying
about halted multi-TD URBs, because this is the only case and it
can be converted to pretend that it's just one TD per URB as usual.
If you are OK with this patch, cancellation logic will be simpler,
because this time there really are no remaining cases of multi-TD
URBs except isochronous. This is clear in xhci_urb_enqueue():
Adding the zero-length TRB to the original TD when we need to send a
zero-length packet would simplify things, and I would otherwise fully
support this, but the xHCI spec is pretty clear that it requires a
dedicated TD for zero-length transactions.
See xhci spec section 4.9.1:
"To generate a “zero-length” USB transaction, software shall explicitly
define a TD with a single Transfer TRB, and its TRB Transfer Length
field shall equal ‘0’. Note that this TD may include non-Transfer TRBs,
e.g. an Event Data or Link TRB."
Thanks
Mathias
