Re: [PATCH 10/14] usb: xhci: Don't set the skip flag on non-isoc endpoints

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

On 10/9/26 13:16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] usb: xhci: invalid completion of URB with -EINPROGRESS on COMP_MISSED_SERVICE_ERROR
> --
> 
> commit dfab6709cb637a348bd004519bdfe39df717cdc4
> Author: Michal Pecio <michal.pecio@gmail.com>
> 
> usb: xhci: Don't set the skip flag on non-isoc endpoints
> 
> These events are unique to isochronous endpoints, ignore them otherwise.
> Update debug messages to reflect new policies. We could also log invalid
> events as errors, but it seems nobody has ever had problems with that,
> so don't bother.
> 
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 8b915a1d5b256..2dd11732bb87c 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -2778,16 +2778,18 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>>   		 * Set skip flag of the ep_ring; Complete the missed tds as
>>   		 * short transfer when process the ep_ring next time.
>>   		 */
>> -		ep->skip = true;
>> +		if (ep_ring->type == TYPE_ISOC)
>> +			ep->skip = true;
>>   		xhci_dbg(xhci,
>> -			 "Miss service interval error for slot %u ep %u, set skip flag%s\n",
>> -			 slot_id, ep_index, ep_trb_dma ? ", skip now" : "");
>> +			 "Missed Service Error for slot %u ep %u, skip %d, try now %d\n",
>> +			 slot_id, ep_index, ep->skip, !!ep_trb_dma);
>>   		break;
> 
> [Severity: High]
> In handle_tx_event() in drivers/usb/host/xhci-ring.c, for non-isochronous
> endpoints, does using break instead of return 0 here cause the event to
> improperly fall through into the rest of the handler?
> 
> If the event's DMA matches the current TD, it appears it is passed to
> process_bulk_intr_td() or process_ctrl_td(). Since those functions do not
> handle this completion code, does the URB's status remain initialized to
> -EINPROGRESS, completing the URB with an illegal status code?

Not an issue,

MISSED_SERVICE_ERROR is completion code only applies to isoch endpoints

see xhci spec section 4.10.3.2 Missed Service Error
"This error only applies to Isochronous endpoints."

> 
> Additionally, if the event DMA does not match the current TD, does the code
> fall back to the debug_finding_td error path, fatally shutting down the host
> controller with -ESHUTDOWN?

Not an issue

This completion code is exclusive to isoch endpoints, meaning td->skip
flag will always be set on this error, thus never taking the debug_finding_td
error path.

Keeping this patch, will send as part of v2


-Mathias