Re: [PATCH 0/2] xhci: Fix the NEC stop bug workaround

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 29.10.2024 11.16, Mathias Nyman wrote:
> On 29.10.2024 10.28, Michał Pecio wrote:
>>
>> By the way, I think this race is already possible today, without my
>> patches. There is no testing for SET_DEQ_PENDING in xhci_urb_dequeue()
>> and no testing in handle_cmd_stop_ep(). If this happens in the middle
>> of a Set TR Deq chain on a streams endpoint, nothing seems to stop the
>> Stop EP handler from attempting invalidation under SET_DEQ_PENDING.
>>
>> Maybe invalidate_cancelled_tds() should bail out if SET_DEQ_PENDING
>> and later Set Deq completion handler should unconditionally call the
>> invalidate/giveback combo before it exits.
>>
> 
> I think you are on to something.
> If we add invalidate/givaback to Set TR deq completion handler, allowing
> it to possible queue new Set TR Deq commands, then we can bail out in
> xhci_urb_dequeue() if SET_DEQ_PENDING is set.
> 
> xhci_urb_dequeue() would not queue a extra stop endpoint command, only
> set td->cancel_status to TD_DIRTY dirty, and Set TR Deq handler would
> not ring the doorbell unnecessary.
> 
> Sounds like a plan to ne.

I wrote a testseries for this.

1st patch avoids stopping endpoint in urb cancel if Set TR Deq is pending
2nd patch handles Set TR Deq command ctx error due to running ep.
3rd patch tracks doorbell ring with a flag. It's for now only used to prevent
     infinite stop ep retries. Flag is not properly cleared in other cases.

Series can be found in my tree in a fix_stop_ep_race branch:

https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=fix_stop_ep_race
git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git fix_stop_ep_race branch

Do these help in your NEC host case?

I'll see if I can set up some system to trigger this myself

Thanks
Mathias