| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251119120601.77c46152.michal.pecio@gmail.com>
Date: Wed, 19 Nov 2025 12:06:01 +0100
From: Michal Pecio <michal.pecio@...il.com>
To: Mathias Nyman <mathias.nyman@...el.com>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH 5/5] usb: xhci: Handle short transfers as "done TDs"
Short transfers (on any endpoint type) are similar to isochronous
errors mid TD - the TD can be given back, but we need to keep track
of xHC's progress through its TRBs and handle some more events. And
here too there are buggy HCs which don't generate expected events.
Reuse the "done TD" mechanism to also handle short transfers on all
endpoints other than control, which have some special requirements.
Done TD is more accurate than the previous code, because it matches
events with TDs by TRB pointers, not by completion codes alone.
Remove the SPURIOUS_SUCCESS quirk, as we can now detect missing end
TRB events automatically and reliably. Ensure that no xhci_dbg()
shows on every short transfer, which can be 8000 times per second.
The patch was tested on isochronous and bulk endpoints at FS/HS/SS
with several host controllers from various vendors. Behavior after
Short Packet mid TD varies:
1. NEC uPD720200 and Etron EJ168 ignore IOC on the last TRB.
2. Renesas uPD720202 and AMD Carrizo APU generate Success with
residual equal to last TRB size.
3. Fresco Logic FL1100 generates Short Packet, residual as above.
4. Various ASMedia HCs and AMD Promontory generate Short Packet
again with repeated residual, even if the last TRB is shorter.
5. VIA VL805 generates Success with zero residual.
No case of a Success with non-zero residual was observed, but commit
1530bbc6272d ("xhci: Add new short TX quirk for Fresco Logic host.")
says that some Fresco Logic hardware (FL1000?) does that.
Signed-off-by: Michal Pecio <michal.pecio@...il.com>
---
drivers/usb/host/xhci-mtk.c | 5 ----
drivers/usb/host/xhci-pci.c | 5 ----
drivers/usb/host/xhci-ring.c | 51 +++++++++---------------------------
drivers/usb/host/xhci.c | 7 -----
drivers/usb/host/xhci.h | 3 +--
5 files changed, 13 insertions(+), 58 deletions(-)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 208558cf822d..d85f84e39291 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -454,11 +454,6 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
xhci->quirks |= XHCI_MTK_HOST;
- /*
- * MTK host controller gives a spurious successful event after a
- * short transfer. Ignore it.
- */
- xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
if (mtk->lpm_support)
xhci->quirks |= XHCI_LPM_SUPPORT;
if (mtk->u2_lpm_disable)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index d292adc65e5a..bcfe45d23f17 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -454,11 +454,6 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
pdev->device == PCI_DEVICE_ID_ASMEDIA_1042_XHCI) {
- /*
- * try to tame the ASMedia 1042 controller which reports 0.96
- * but appears to behave more like 1.0
- */
- xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
xhci->quirks |= XHCI_BROKEN_STREAMS;
}
if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2b889caff0f5..a21d403305f1 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2450,16 +2450,16 @@ static void process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
/* handle completion code */
switch (trb_comp_code) {
case COMP_SUCCESS:
- if (remaining) {
- frame->status = short_framestatus;
- sum_trbs_for_length = true;
+ /* check transfer completeness, some HCs may lie */
+ if (ep_trb == td->end_trb && !remaining) {
+ frame->status = 0;
break;
}
- frame->status = 0;
- break;
+ fallthrough;
case COMP_SHORT_PACKET:
frame->status = short_framestatus;
sum_trbs_for_length = true;
+ more_events = ep_trb != td->end_trb; /* xHCI 4.10.1.1.2 */
break;
case COMP_BANDWIDTH_OVERRUN_ERROR:
frame->status = -ECOMM;
@@ -2547,6 +2547,8 @@ static void process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
struct xhci_slot_ctx *slot_ctx;
u32 trb_comp_code;
u32 remaining, requested, ep_trb_len;
+ /* Expect more events for this TD after short transfers before the last TRB */
+ bool more_events = false;
slot_ctx = xhci_get_slot_ctx(xhci, ep->vdev->out_ctx);
trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
@@ -2568,6 +2570,7 @@ static void process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
break;
case COMP_SHORT_PACKET:
td->status = 0;
+ more_events = ep_trb != td->end_trb; /* xHCI 4.10.1.1.2 */
break;
case COMP_STOPPED_SHORT_PACKET:
td->urb->actual_length = remaining;
@@ -2604,11 +2607,11 @@ static void process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
td->urb->actual_length = 0;
}
- finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb, false);
+ finish_td(xhci, ep, ep_ring, td, trb_comp_code, ep_seg, ep_trb, more_events);
}
/*
- * Process events for an already finished TD. See xHCI 4.9.1.
+ * Process events for an already finished TD. See xHCI 4.9.1 and 4.10.1.1.2.
*/
static void process_done_td(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
struct xhci_segment *ep_seg, union xhci_trb *ep_trb,
@@ -2661,17 +2664,6 @@ static int handle_transferless_tx_event(struct xhci_hcd *xhci, struct xhci_virt_
return 0;
}
-static bool xhci_spurious_success_tx_event(struct xhci_hcd *xhci,
- struct xhci_ring *ring)
-{
- switch (ring->old_trb_comp_code) {
- case COMP_SHORT_PACKET:
- return xhci->quirks & XHCI_SPURIOUS_SUCCESS;
- default:
- return false;
- }
-}
-
/*
* If this function returns an error condition, it means it got a Transfer
* event with a corrupted Slot ID, Endpoint ID, or TRB DMA address.
@@ -2733,11 +2725,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
* transfer type
*/
case COMP_SUCCESS:
- if (EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
- trb_comp_code = COMP_SHORT_PACKET;
- xhci_dbg(xhci, "Successful completion on short TX for slot %u ep %u with last td comp code %d\n",
- slot_id, ep_index, ep_ring->old_trb_comp_code);
- }
break;
case COMP_SHORT_PACKET:
break;
@@ -2857,7 +2844,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
/*
* 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.
+ * the xHC finished traversing all its TRBs, because it completed short or with an error.
*/
if (ep_ring->done_end_trb) {
if (trb_in_range(xhci, ep_ring->dequeue, ep_ring->done_end_trb, ep_seg, ep_trb)) {
@@ -2880,8 +2867,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
*/
if (trb_comp_code != COMP_STOPPED &&
trb_comp_code != COMP_STOPPED_LENGTH_INVALID &&
- !ring_xrun_event &&
- !xhci_spurious_success_tx_event(xhci, ep_ring)) {
+ !ring_xrun_event) {
xhci_warn(xhci, "Event TRB for slot %u ep %u with no TDs queued\n",
slot_id, ep_index);
}
@@ -2942,17 +2928,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
return 0;
}
- /*
- * Some hosts give a spurious success event after a short
- * transfer or error on last TRB. Ignore it.
- */
- if (xhci_spurious_success_tx_event(xhci, ep_ring)) {
- xhci_dbg(xhci, "Spurious event dma %pad, comp_code %u after %u\n",
- &ep_trb_dma, trb_comp_code, ep_ring->old_trb_comp_code);
- ep_ring->old_trb_comp_code = 0;
- return 0;
- }
-
/* HC is busted, give up! */
goto debug_finding_td;
}
@@ -2972,8 +2947,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
*/
} while (ep->skip);
- ep_ring->old_trb_comp_code = trb_comp_code;
-
/* Get out if a TD was queued at enqueue after the xrun occurred */
if (ring_xrun_event)
return 0;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6da583c7778b..4e32cbe1c6b8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5473,13 +5473,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
if (get_quirks)
get_quirks(dev, xhci);
- /* In xhci controllers which follow xhci 1.0 spec gives a spurious
- * success event after a short transfer. This quirk will ignore such
- * spurious event.
- */
- if (xhci->hci_version > 0x96)
- xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
-
if (xhci->hci_version == 0x95 && link_quirk) {
xhci_dbg(xhci, "QUIRK: Not clearing Link TRB chain bits");
xhci->quirks |= XHCI_LINK_TRB_QUIRK;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index a1dd9ce5f8aa..e72eda6cee62 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1380,7 +1380,6 @@ struct xhci_ring {
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;
};
@@ -1587,7 +1586,7 @@ struct xhci_hcd {
#define XHCI_RESET_EP_QUIRK BIT_ULL(1) /* Deprecated */
#define XHCI_NEC_HOST BIT_ULL(2)
#define XHCI_AMD_PLL_FIX BIT_ULL(3)
-#define XHCI_SPURIOUS_SUCCESS BIT_ULL(4)
+#define XHCI_SPURIOUS_SUCCESS BIT_ULL(4) /* Deprecated */
/*
* Certain Intel host controllers have a limit to the number of endpoint
* contexts they can handle. Ideally, they would signal that they can't handle
--
2.48.1
Powered by blists - more mailing lists