Re: [PATCH AUTOSEL 6.14 08/15] usb: xhci: Don't trust the EP Context cycle bit when moving HW dequeue

[Sasha Levin <sashal@kernel.org>]

On Mon, May 12, 2025 at 11:16:28PM +0200, Michał Pecio wrote:
>On Mon, 12 May 2025 14:03:43 -0400, Sasha Levin wrote:
>> From: Michal Pecio <michal.pecio@gmail.com>
>>
>> [ Upstream commit 6328bdc988d23201c700e1e7e04eb05a1149ac1e ]
>>
>> VIA VL805 doesn't bother updating the EP Context cycle bit when the
>> endpoint halts. This is seen by patching xhci_move_dequeue_past_td()
>> to print the cycle bits of the EP Context and the TRB at hw_dequeue
>> and then disconnecting a flash drive while reading it. Actual cycle
>> state is random as expected, but the EP Context bit is always 1.
>>
>> This means that the cycle state produced by this function is wrong
>> half the time, and then the endpoint stops working.
>>
>> Work around it by looking at the cycle bit of TD's end_trb instead
>> of believing the Endpoint or Stream Context. Specifically:
>>
>> - rename cycle_found to hw_dequeue_found to avoid confusion
>> - initialize new_cycle from td->end_trb instead of hw_dequeue
>> - switch new_cycle toggling to happen after end_trb is found
>>
>> Now a workload which regularly stalls the device works normally for
>> a few hours and clearly demonstrates the HW bug - the EP Context bit
>> is not updated in a new cycle until Set TR Dequeue overwrites it:
>>
>> [  +0,000298] sd 10:0:0:0: [sdc] Attached SCSI disk
>> [  +0,011758] cycle bits: TRB 1 EP Ctx 1
>> [  +5,947138] cycle bits: TRB 1 EP Ctx 1
>> [  +0,065731] cycle bits: TRB 0 EP Ctx 1
>> [  +0,064022] cycle bits: TRB 0 EP Ctx 0
>> [  +0,063297] cycle bits: TRB 0 EP Ctx 0
>> [  +0,069823] cycle bits: TRB 0 EP Ctx 0
>> [  +0,063390] cycle bits: TRB 1 EP Ctx 0
>> [  +0,063064] cycle bits: TRB 1 EP Ctx 1
>> [  +0,062293] cycle bits: TRB 1 EP Ctx 1
>> [  +0,066087] cycle bits: TRB 0 EP Ctx 1
>> [  +0,063636] cycle bits: TRB 0 EP Ctx 0
>> [  +0,066360] cycle bits: TRB 0 EP Ctx 0
>>
>> Also tested on the buggy ASM1042 which moves EP Context dequeue to
>> the next TRB after errors, one problem case addressed by the rework
>> that implemented this loop. In this case hw_dequeue can be enqueue,
>> so simply picking the cycle bit of TRB at hw_dequeue wouldn't work.
>>
>> Commit 5255660b208a ("xhci: add quirk for host controllers that
>> don't update endpoint DCS") tried to solve the stale cycle problem,
>> but it was more complex and got reverted due to a reported issue.
>>
>> Cc: Jonathan Bell <jonathan@raspberrypi.org>
>> Cc: Oliver Neukum <oneukum@suse.com>
>> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Link: https://lore.kernel.org/r/20250505125630.561699-2-mathias.nyman@linux.intel.com
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
>Hi,
>
>This wasn't tagged for stable because the function may potentially
>still be affected by some unforeseen HW bugs, and previous attempt
>at fixing the issue ran into trouble and nobody truly knows why.
>
>The problem is very old and not critically severe, so I think this
>can wait till 6.15. People don't like minor release regressions.

I'll drop it, thanks!

-- 
Thanks,
Sasha