From nobody Tue Dec 2 02:32:13 2025 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 75248350A2C for ; Wed, 19 Nov 2025 11:05:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763550331; cv=none; b=hyppB2yTaROsr73qs/pb7jVaUVkpXgFnp4ybpDowgUzvWSHlO3YScZMmQ2s4K4PutZE9mjAjUCZWAnyNOO6YVFIIXm+Wf7ciUPXvHn45CgkX0teO2AJQK1AZjMuTxGvoC4hlRA9ILfluQ6q9DkSJbyGKb3MfA5ZTs+Qkcb9WKAc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763550331; c=relaxed/simple; bh=ZZlMYEL4UcoCzWKCh7+jEM7X2xcMYZBm/2G+suCYUWA=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=cnYifAMCzRU+jc5ydxkI/MtqOPBV3fuIChiUvFi2u8IL4dTi62TJFKJIQj5h4T+A5wm0R9A//jFws78+vs2T7/bwrzJwxEQmZtxex4gWY4X7DkIwgVsdqBOsWTsqVUen4qtkyIELagPjsimBgxUWSnmgIdQgBgmTtlktntMhES4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=lv+oDTbg; arc=none smtp.client-ip=209.85.218.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="lv+oDTbg" Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-b73161849e1so1212232666b.2 for ; Wed, 19 Nov 2025 03:05:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763550324; x=1764155124; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=a2r3ybB/cy39+Bu51zs1AfwKD0YYeIolsm0blPv0hSA=; b=lv+oDTbgFdwuK37WwCmQ8t+i87fMV97ZseEEsSWR6d3i+BKB6cK1WtUoh8n7g9YZXc ko0YSNYxVUg/DCwRIOonVMPwPJv3nbqJqT0W2DicQGMtpyWBg2JTtcdXMXenptC7v8MR cBf/5JSRUEMDcBensGmeP78l77TFRDC6lJX2553rcZFID5lgSu5kNVki6AIkoo7Yyerp Mhk11na11IubK1a1XQQbN82CxFZpD/iSG8NnBxbiayDZUgapd5LXDzsh/bCmok7GLDA3 oI3my9TQ8zptlU04+OOwwSVEJybp1LkRLEe8UhV1v5aCbbJBquP0fuViSXmnYetiq3L/ UkYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763550324; x=1764155124; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=a2r3ybB/cy39+Bu51zs1AfwKD0YYeIolsm0blPv0hSA=; b=fBfpQ0SqAwnv+QgAfkcDSxbiDsIdTCt5F33/siJdlDS09uZUOIU4oWzDV8xF7sGJ2E fuC2s1XKjryM/NibHXoa3kXsaeszix+ZjpvpRL+PlPlZwuFkOOLseIHsn1fvKcUw9uTE wjgexFtTMGOtgohkfkFv0FaMHgA6/rHYxAZl/5VDDipgSxSJu2CI8ohiMe0PFbTSUHBZ tXFykOWZ+nIi8ZuVI7695EDPGsrvf3EIJXwS+iQ0QtxPVldU7noZaxnEiuu0bfOi5yY+ 6v3fmJ1de7Yq0Pqw1nh7t5qgoyYf66+xQfgz37NW3TO20XXASjcM9WbQ8JfsKg2exKLZ snwg== X-Forwarded-Encrypted: i=1; AJvYcCWuejn7wshJzzVu+enu/LHulF8YyblCWvJjtf8B/g4MOZanXjTchamraTXXsay7yU1aG2bNCAczoVL9fiE=@vger.kernel.org X-Gm-Message-State: AOJu0YzTPfe8HFx/SAhPfrP2VyGSHZJO/6I5n58ok4Axf7R9wtIbHV/u cp9KzNEDMifBBSDI31AEm0j+WiFuAyvJT1n8mcEi9aYYdQNlMuJoqOS2 X-Gm-Gg: ASbGncudyHQFrb7OPo5yToK3lohfzNC/HUptpjfDFN1nb4ZvqogLuJMkm8qIos9fACh 989CwrMB9oaQE7sxbEYPcYHVcJe4kWZW9ahTodT3yhNrIM/JIGQSQu4i0D6ut8Uj8GFZXp953r7 Yv8tiv2ECy1IyyJ1754hqncLr2SL5m/0gNLPGRb7C6I2o2dlpz/ipTic66seuMbaEmMipnOwE57 aF+a4UPw1hHXloyfnfxHO//UDG6ePRgdxdzwlZ1LC5lxo6qnjQuGxHgKbd4kQF/0kGxS5Wktel+ 83HvNKAzwg/z0vSquoGdNbVQLZUtvRQRVTMlfxmPUN+WIVH/RWYrE+xk/YEgahRBBgTL5sxU/5y EgloXGd9Um0IMwPsIbmFmHI1qNEdXqf9qGB/CvwyMhPCzxxfNoZkoPwqZKqVUU//KYD0VOXc4Vb uAqMj28ifbIOj97npnX3XCUfY= X-Google-Smtp-Source: AGHT+IEw5t/eIx5bVX1xsnAa+zaDwgtTnfVs0G0iRtj0eD3+nJI5zrqG9M+9zwSEwsT23oCUk2bCQw== X-Received: by 2002:a17:907:7fa5:b0:b73:75ea:febf with SMTP id a640c23a62f3a-b7637a128afmr200845966b.55.1763550323812; Wed, 19 Nov 2025 03:05:23 -0800 (PST) Received: from foxbook (bhd138.neoplus.adsl.tpnet.pl. [83.28.93.138]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-b734fad45afsm1599924766b.24.2025.11.19.03.05.23 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 19 Nov 2025 03:05:23 -0800 (PST) Date: Wed, 19 Nov 2025 12:05:20 +0100 From: Michal Pecio To: Mathias Nyman , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 4/5] usb: xhci: Reduce error mid TD latency with a new "done TD" mechanism Message-ID: <20251119120520.7827bfa7.michal.pecio@gmail.com> In-Reply-To: <20251119120208.6a025eb0.michal.pecio@gmail.com> References: <20251119120208.6a025eb0.michal.pecio@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" No data transfer will occur after an error mid TD, so the TD can be given back to class driver without waiting for the final completion event. This reduces latency, particularly on buggy HCs from vendors like NEC or ASMedia which ignore the IOC flag after errors, making the TD wait until next ESIT to be given back. Since we can no longer expect the 'xhci_td' to stay around after it is given back, copy minimum metadata to 'ep_ring' which will enable recognition of subsequent events for the done TD. Existing solution with trb_in_td() proved very effective, so use a similar mechanism. Also take over handling out-of-spec Etron events, because we can do it more accurately than the previous implementation. We no longer need to handle Missed Service Errors with NULL 'ep_trb' for the sake of TDs stuck in error mid TD state, so drop that. Signed-off-by: Michal Pecio --- drivers/usb/host/xhci-ring.c | 116 +++++++++++++++++++---------------- drivers/usb/host/xhci.h | 2 +- 2 files changed, 64 insertions(+), 54 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index a2257e1dc396..2b889caff0f5 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1542,6 +1542,8 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *= xhci, int slot_id, */ ep_ring->deq_seg =3D ep->queued_deq_seg; ep_ring->dequeue =3D ep->queued_deq_ptr; + /* If we were waiting for a done TD, we aren't anymore */ + ep_ring->done_end_trb =3D NULL; } else { xhci_warn(xhci, "Mismatch between completed Set TR Deq Ptr command & xH= CI internal state.\n"); xhci_warn(xhci, "ep deq seg =3D %p, deq ptr =3D %p\n", @@ -2260,7 +2262,7 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, u= nsigned int trb_comp_code) =20 static void finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, struct xhci_ring *ep_ring, struct xhci_td *td, u32 trb_comp_code, - struct xhci_segment *ep_seg, union xhci_trb *ep_trb) + struct xhci_segment *ep_seg, union xhci_trb *ep_trb, bool more_eve= nts) { struct xhci_ep_ctx *ep_ctx; =20 @@ -2300,6 +2302,9 @@ static void finish_td(struct xhci_hcd *xhci, struct x= hci_virt_ep *ep, /* update ring dequeue state */ ep_ring->deq_seg =3D ep_seg; ep_ring->dequeue =3D ep_trb; + /* td is done and going away, but it may still get more events */ + if (more_events) + ep_ring->done_end_trb =3D td->end_trb; =20 xhci_td_cleanup(xhci, td, ep_ring, td->status); } @@ -2408,7 +2413,7 @@ static void process_ctrl_td(struct xhci_hcd *xhci, st= ruct xhci_virt_ep *ep, td->urb->actual_length =3D requested; =20 finish_td: - finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb); + finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb, false); } =20 /* @@ -2426,6 +2431,11 @@ static void process_isoc_td(struct xhci_hcd *xhci, s= truct xhci_virt_ep *ep, bool sum_trbs_for_length =3D false; u32 remaining, requested, ep_trb_len; int short_framestatus; + /* Expect more events for this TD after certain events before the last TR= B */ + bool more_events =3D false; + /* On Etron SuperSpeed endpoints some errors on any TRB trigger more even= ts */ + bool etron_ss_bug =3D xhci->quirks & XHCI_ETRON_HOST && + td->urb->dev->speed =3D=3D USB_SPEED_SUPER; =20 trb_comp_code =3D GET_COMP_CODE(le32_to_cpu(event->transfer_len)); urb_priv =3D td->urb->hcpriv; @@ -2440,9 +2450,6 @@ static void process_isoc_td(struct xhci_hcd *xhci, st= ruct xhci_virt_ep *ep, /* handle completion code */ switch (trb_comp_code) { case COMP_SUCCESS: - /* Don't overwrite status if TD had an error, see xHCI 4.9.1 */ - if (td->error_mid_td) - break; if (remaining) { frame->status =3D short_framestatus; sum_trbs_for_length =3D true; @@ -2462,14 +2469,12 @@ static void process_isoc_td(struct xhci_hcd *xhci, = struct xhci_virt_ep *ep, fallthrough; case COMP_ISOCH_BUFFER_OVERRUN: frame->status =3D -EOVERFLOW; - if (ep_trb !=3D td->end_trb) - td->error_mid_td =3D true; + more_events =3D ep_trb !=3D td->end_trb || etron_ss_bug; /* xHCI 4.9.1 */ break; case COMP_MISSED_SERVICE_ERROR: frame->status =3D -EXDEV; sum_trbs_for_length =3D true; - if (ep_trb !=3D td->end_trb) - td->error_mid_td =3D true; + more_events =3D ep_trb !=3D td->end_trb; /* xHCI 4.11.2.5.2, no Etron bu= g */ break; case COMP_INCOMPATIBLE_DEVICE_ERROR: case COMP_STALL_ERROR: @@ -2478,8 +2483,7 @@ static void process_isoc_td(struct xhci_hcd *xhci, st= ruct xhci_virt_ep *ep, case COMP_USB_TRANSACTION_ERROR: frame->status =3D -EPROTO; sum_trbs_for_length =3D true; - if (ep_trb !=3D td->end_trb) - td->error_mid_td =3D true; + more_events =3D ep_trb !=3D td->end_trb || etron_ss_bug; /* xHCI 4.9.1 */ break; case COMP_STOPPED: sum_trbs_for_length =3D true; @@ -2501,9 +2505,6 @@ static void process_isoc_td(struct xhci_hcd *xhci, st= ruct xhci_virt_ep *ep, break; } =20 - if (td->urb_length_set) - goto finish_td; - if (sum_trbs_for_length) frame->actual_length =3D sum_trb_lengths(td, ep_trb) + ep_trb_len - remaining; @@ -2512,14 +2513,7 @@ static void process_isoc_td(struct xhci_hcd *xhci, s= truct xhci_virt_ep *ep, =20 td->urb->actual_length +=3D frame->actual_length; =20 -finish_td: - /* Don't give back TD yet if we encountered an error mid TD */ - if (td->error_mid_td && ep_trb !=3D td->end_trb) { - xhci_dbg(xhci, "Error mid isoc TD, wait for final completion event\n"); - td->urb_length_set =3D true; - return; - } - finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb); + finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb, more_even= ts); } =20 static void skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td, @@ -2610,7 +2604,34 @@ static void process_bulk_intr_td(struct xhci_hcd *xh= ci, struct xhci_virt_ep *ep, td->urb->actual_length =3D 0; } =20 - finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb); + finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb, false); +} + +/* + * Process events for an already finished TD. See xHCI 4.9.1. + */ +static void process_done_td(struct xhci_hcd *xhci, struct xhci_ring *ep_ri= ng, + struct xhci_segment *ep_seg, union xhci_trb *ep_trb, + u32 trb_comp_code) +{ + switch (trb_comp_code) { + case COMP_STOPPED: + case COMP_STOPPED_LENGTH_INVALID: + case COMP_STOPPED_SHORT_PACKET: + /* stopped events don't terminate the TD */ + break; + default: + /* clear done TD if this is the final event */ + if (ep_trb =3D=3D ep_ring->done_end_trb) + ep_ring->done_end_trb =3D NULL; + else /* unexpected */ + xhci_dbg(xhci, "Done TD '%s' before end_trb\n", + xhci_trb_comp_code_string(trb_comp_code)); + } + + /* release TRBs completed by the xHC */ + ep_ring->deq_seg =3D ep_seg; + ep_ring->dequeue =3D ep_trb; } =20 /* Transfer events which don't point to a transfer TRB, see xhci 4.17.4 */ @@ -2646,11 +2667,6 @@ static bool xhci_spurious_success_tx_event(struct xh= ci_hcd *xhci, switch (ring->old_trb_comp_code) { case COMP_SHORT_PACKET: return xhci->quirks & XHCI_SPURIOUS_SUCCESS; - case COMP_USB_TRANSACTION_ERROR: - case COMP_BABBLE_DETECTED_ERROR: - case COMP_ISOCH_BUFFER_OVERRUN: - return xhci->quirks & XHCI_ETRON_HOST && - ring->type =3D=3D TYPE_ISOC; default: return false; } @@ -2800,15 +2816,17 @@ static int handle_tx_event(struct xhci_hcd *xhci, break; case COMP_MISSED_SERVICE_ERROR: /* - * When encounter missed service error, one or more isoc tds - * may be missed by xHC. - * Set skip flag of the ep_ring; Complete the missed tds as - * short transfer when process the ep_ring next time. + * One or more isoc TDs were missed by the xHC and ep_trb points to the + * last missed TD, or it may be NULL. Flag the endpoint and run skipping + * now if we know the missed TDs or leave them to be skipped later. + * See xHCI 4.10.3.2, note differences between spec rev 1.0 and later. */ ep->skip =3D 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" : ""); + slot_id, ep_index, ep_trb ? ", skip now" : ""); + if (!ep_trb) + return 0; break; case COMP_NO_PING_RESPONSE_ERROR: ep->skip =3D true; @@ -2838,29 +2856,21 @@ static int handle_tx_event(struct xhci_hcd *xhci, } =20 /* - * xhci 4.10.2 states isoc endpoints should continue - * processing the next TD if there was an error mid TD. - * So host like NEC don't generate an event for the last - * isoc TRB even if the IOC flag is set. - * xhci 4.9.1 states that if there are errors in mult-TRB - * TDs xHC should generate an error for that TRB, and if xHC - * proceeds to the next TD it should genete an event for - * any TRB with IOC flag on the way. Other host follow this. - * - * We wait for the final IOC event, but if we get an event - * anywhere outside this TD, just give it back already. + * Check if we are expecting more events for a "done" TD, which has been = given back before + * the xHC finished traversing all its TRBs, because it completed with an= error. */ - td =3D list_first_entry_or_null(&ep_ring->td_list, struct xhci_td, td_lis= t); - - if (td && td->error_mid_td && !trb_in_td(xhci, td, ep_seg, ep_trb)) { - xhci_dbg(xhci, "Missing TD completion event after mid TD error\n"); - xhci_dequeue_td(xhci, td, ep_ring, td->status); + if (ep_ring->done_end_trb) { + if (trb_in_range(xhci, ep_ring->dequeue, ep_ring->done_end_trb, ep_seg, = ep_trb)) { + process_done_td(xhci, ep_ring, ep_seg, ep_trb, trb_comp_code); + return 0; + } + /* + * Some HCs don't generate these events and silently move forward. We ge= t an event + * for the next TD, or maybe Missed Service or Ring Underrun. Handle it = normally. + */ + ep_ring->done_end_trb =3D NULL; } =20 - /* If the TRB pointer is NULL, missed TDs will be skipped on the next eve= nt */ - if (trb_comp_code =3D=3D COMP_MISSED_SERVICE_ERROR && !ep_trb_dma) - return 0; - if (list_empty(&ep_ring->td_list)) { /* * Don't print wanings if ring is empty due to a stopped endpoint genera= ting an diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 485ea7fc0433..a1dd9ce5f8aa 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1314,7 +1314,6 @@ struct xhci_td { struct xhci_segment *bounce_seg; /* actual_length of the URB has already been set */ bool urb_length_set; - bool error_mid_td; }; =20 /* @@ -1380,6 +1379,7 @@ struct xhci_ring { unsigned int num_trbs_free; /* used only by xhci DbC */ unsigned int bounce_buf_len; enum xhci_ring_type type; + union xhci_trb *done_end_trb; u32 old_trb_comp_code; struct radix_tree_root *trb_address_map; }; --=20 2.48.1