public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: "Mathias Nyman" <mathias.nyman@linux.intel.com>,
	"Michał Pecio" <michal.pecio@gmail.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Sasha Levin" <sashal@kernel.org>
Subject: [PATCH 6.1 40/71] xhci: process isoc TD properly when there was a transaction error mid TD.
Date: Wed, 13 Mar 2024 12:39:26 -0400	[thread overview]
Message-ID: <20240313163957.615276-41-sashal@kernel.org> (raw)
In-Reply-To: <20240313163957.615276-1-sashal@kernel.org>

From: Mathias Nyman <mathias.nyman@linux.intel.com>

[ Upstream commit 5372c65e1311a16351ef03dd096ff576e6477674 ]

The last TRB of a isoc TD might not trigger an event if there was
an error event for a TRB mid TD. This is seen on a NEC Corporation
uPD720200 USB 3.0 Host

After an error mid a multi-TRB TD the xHC should according to xhci 4.9.1
generate events for passed TRBs with IOC flag set if it proceeds to the
next TD. This event is either a copy of the original error, or a
"success" transfer event.

If that event is missing then the driver and xHC host get out of sync as
the driver is still expecting a transfer event for that first TD, while
xHC host is already sending events for the next TD in the list.
This leads to
"Transfer event TRB DMA ptr not part of current TD" messages.

As a solution we tag the isoc TDs that get error events mid TD.
If an event doesn't match the first TD, then check if the tag is
set, and event points to the next TD.
In that case give back the fist TD and process the next TD normally

Make sure TD status and transferred length stay valid in both cases
with and without final TD completion event.

Reported-by: Michał Pecio <michal.pecio@gmail.com>
Closes: https://lore.kernel.org/linux-usb/20240112235205.1259f60c@foxbook/
Tested-by: Michał Pecio <michal.pecio@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20240125152737.2983959-4-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/usb/host/xhci-ring.c | 74 +++++++++++++++++++++++++++++-------
 drivers/usb/host/xhci.h      |  1 +
 2 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1239e06dfe411..e4441a71368e5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2363,6 +2363,9 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct 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 = short_framestatus;
 			if (xhci->quirks & XHCI_TRUST_TX_LENGTH)
@@ -2388,8 +2391,9 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 		break;
 	case COMP_USB_TRANSACTION_ERROR:
 		frame->status = -EPROTO;
+		sum_trbs_for_length = true;
 		if (ep_trb != td->last_trb)
-			return 0;
+			td->error_mid_td = true;
 		break;
 	case COMP_STOPPED:
 		sum_trbs_for_length = true;
@@ -2409,6 +2413,9 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 		break;
 	}
 
+	if (td->urb_length_set)
+		goto finish_td;
+
 	if (sum_trbs_for_length)
 		frame->actual_length = sum_trb_lengths(xhci, ep->ring, ep_trb) +
 			ep_trb_len - remaining;
@@ -2417,6 +2424,14 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 
 	td->urb->actual_length += frame->actual_length;
 
+finish_td:
+	/* Don't give back TD yet if we encountered an error mid TD */
+	if (td->error_mid_td && ep_trb != td->last_trb) {
+		xhci_dbg(xhci, "Error mid isoc TD, wait for final completion event\n");
+		td->urb_length_set = true;
+		return 0;
+	}
+
 	return finish_td(xhci, ep, ep_ring, td, trb_comp_code);
 }
 
@@ -2801,17 +2816,51 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 		}
 
 		if (!ep_seg) {
-			if (!ep->skip ||
-			    !usb_endpoint_xfer_isoc(&td->urb->ep->desc)) {
-				/* Some host controllers give a spurious
-				 * successful event after a short transfer.
-				 * Ignore it.
-				 */
-				if ((xhci->quirks & XHCI_SPURIOUS_SUCCESS) &&
-						ep_ring->last_td_was_short) {
-					ep_ring->last_td_was_short = false;
-					goto cleanup;
+
+			if (ep->skip && usb_endpoint_xfer_isoc(&td->urb->ep->desc)) {
+				skip_isoc_td(xhci, td, ep, status);
+				goto cleanup;
+			}
+
+			/*
+			 * Some hosts give a spurious success event after a short
+			 * transfer. Ignore it.
+			 */
+			if ((xhci->quirks & XHCI_SPURIOUS_SUCCESS) &&
+			    ep_ring->last_td_was_short) {
+				ep_ring->last_td_was_short = false;
+				goto cleanup;
+			}
+
+			/*
+			 * 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.
+			 * So this event might be for the next TD.
+			 */
+			if (td->error_mid_td &&
+			    !list_is_last(&td->td_list, &ep_ring->td_list)) {
+				struct xhci_td *td_next = list_next_entry(td, td_list);
+
+				ep_seg = trb_in_td(xhci, td_next->start_seg, td_next->first_trb,
+						   td_next->last_trb, ep_trb_dma, false);
+				if (ep_seg) {
+					/* give back previous TD, start handling new */
+					xhci_dbg(xhci, "Missing TD completion event after mid TD error\n");
+					ep_ring->dequeue = td->last_trb;
+					ep_ring->deq_seg = td->last_trb_seg;
+					inc_deq(xhci, ep_ring);
+					xhci_td_cleanup(xhci, td, ep_ring, td->status);
+					td = td_next;
 				}
+			}
+
+			if (!ep_seg) {
 				/* HC is busted, give up! */
 				xhci_err(xhci,
 					"ERROR Transfer event TRB DMA ptr not "
@@ -2823,9 +2872,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 					  ep_trb_dma, true);
 				return -ESHUTDOWN;
 			}
-
-			skip_isoc_td(xhci, td, ep, status);
-			goto cleanup;
 		}
 		if (trb_comp_code == COMP_SHORT_PACKET)
 			ep_ring->last_td_was_short = true;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 1354310cb37b1..fc25a5b09710c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1570,6 +1570,7 @@ 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;
 	unsigned int		num_trbs;
 };
 
-- 
2.43.0


  parent reply	other threads:[~2024-03-13 16:40 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-13 16:38 [PATCH 6.1 00/71] 6.1.82-rc1 review Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 01/71] ceph: switch to corrected encoding of max_xattr_size in mdsmap Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 02/71] net: lan78xx: fix runtime PM count underflow on link stop Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 03/71] ixgbe: {dis, en}able irqs in ixgbe_txrx_ring_{dis, en}able Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 04/71] i40e: disable NAPI right after disabling irqs when handling xsk_pool Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 05/71] ice: reorder disabling IRQ and NAPI in ice_qp_dis Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 06/71] tracing/net_sched: Fix tracepoints that save qdisc_dev() as a string Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 07/71] geneve: make sure to pull inner header in geneve_rx() Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 08/71] net: sparx5: Fix use after free inside sparx5_del_mact_entry Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 09/71] ice: virtchnl: stop pretending to support RSS over AQ or registers Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 10/71] net: ice: Fix potential NULL pointer dereference in ice_bridge_setlink() Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 11/71] igc: avoid returning frame twice in XDP_REDIRECT Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 12/71] net/ipv6: avoid possible UAF in ip6_route_mpath_notify() Sasha Levin
2024-03-13 16:38 ` [PATCH 6.1 13/71] cpumap: Zero-initialise xdp_rxq_info struct before running XDP program Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 14/71] net: dsa: microchip: fix register write order in ksz8_ind_write8() Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 15/71] net/rds: fix WARNING in rds_conn_connect_if_down Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 16/71] netfilter: nft_ct: fix l3num expectations with inet pseudo family Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 17/71] netfilter: nf_conntrack_h323: Add protection for bmp length out of range Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 18/71] erofs: apply proper VMA alignment for memory mapped files on THP Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 19/71] netrom: Fix a data-race around sysctl_netrom_default_path_quality Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 20/71] netrom: Fix a data-race around sysctl_netrom_obsolescence_count_initialiser Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 21/71] netrom: Fix data-races around sysctl_netrom_network_ttl_initialiser Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 22/71] netrom: Fix a data-race around sysctl_netrom_transport_timeout Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 23/71] netrom: Fix a data-race around sysctl_netrom_transport_maximum_tries Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 24/71] netrom: Fix a data-race around sysctl_netrom_transport_acknowledge_delay Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 25/71] netrom: Fix a data-race around sysctl_netrom_transport_busy_delay Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 26/71] netrom: Fix a data-race around sysctl_netrom_transport_requested_window_size Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 27/71] netrom: Fix a data-race around sysctl_netrom_transport_no_activity_timeout Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 28/71] netrom: Fix a data-race around sysctl_netrom_routing_control Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 29/71] netrom: Fix a data-race around sysctl_netrom_link_fails_count Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 30/71] netrom: Fix data-races around sysctl_net_busy_read Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 31/71] KVM: s390: add stat counter for shadow gmap events Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 32/71] KVM: s390: vsie: fix race during shadow creation Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 33/71] ASoC: codecs: wcd938x: fix headphones volume controls Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 34/71] drm/amd/display: Fix uninitialized variable usage in core_link_ 'read_dpcd() & write_dpcd()' functions Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 35/71] nfp: flower: add goto_chain_index for ct entry Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 36/71] nfp: flower: add hardware offload check for post " Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 37/71] readahead: avoid multiple marked readahead pages Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 38/71] selftests/mm: switch to bash from sh Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 39/71] selftests: mm: fix map_hugetlb failure on 64K page size systems Sasha Levin
2024-03-13 16:39 ` Sasha Levin [this message]
2024-03-13 16:39 ` [PATCH 6.1 41/71] xhci: handle isoc Babble and Buffer Overrun events properly Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 42/71] drm/amdgpu: Reset IH OVERFLOW_CLEAR bit Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 43/71] x86/mmio: Disable KVM mitigation when X86_FEATURE_CLEAR_CPU_BUF is set Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 44/71] Documentation/hw-vuln: Add documentation for RFDS Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 45/71] x86/rfds: Mitigate Register File Data Sampling (RFDS) Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 46/71] KVM/x86: Export RFDS_NO and RFDS_CLEAR to guests Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 47/71] selftests: mptcp: decrease BW in simult flows Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 48/71] blk-iocost: disable writeback throttling Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 49/71] elevator: remove redundant code in elv_unregister_queue() Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 50/71] blk-wbt: remove unnecessary check in wbt_enable_default() Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 51/71] elevator: add new field flags in struct elevator_queue Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 52/71] blk-wbt: don't enable throttling if default elevator is bfq Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 53/71] blk-wbt: pass a gendisk to wbt_{enable,disable}_default Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 54/71] blk-wbt: pass a gendisk to wbt_init Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 55/71] blk-rq-qos: move rq_qos_add and rq_qos_del out of line Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 56/71] blk-rq-qos: make rq_qos_add and rq_qos_del more useful Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 57/71] blk-rq-qos: constify rq_qos_ops Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 58/71] blk-rq-qos: store a gendisk instead of request_queue in struct rq_qos Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 59/71] blk-wbt: Fix detection of dirty-throttled tasks Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 60/71] drm/amd/display: Wrong colorimetry workaround Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 61/71] drm/amd/display: Fix MST Null Ptr for RV Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 62/71] getrusage: add the "signal_struct *sig" local variable Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 63/71] getrusage: move thread_group_cputime_adjusted() outside of lock_task_sighand() Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 64/71] getrusage: use __for_each_thread() Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 65/71] getrusage: use sig->stats_lock rather than lock_task_sighand() Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 66/71] fs/proc: do_task_stat: use __for_each_thread() Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 67/71] fs/proc: do_task_stat: use sig->stats_lock to gather the threads/children stats Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 68/71] exit: wait_task_zombie: kill the no longer necessary spin_lock_irq(siglock) Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 69/71] blk-wbt: fix that wbt can't be disabled by default Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 70/71] blk-iocost: Pass gendisk to ioc_refresh_params Sasha Levin
2024-03-13 16:39 ` [PATCH 6.1 71/71] Linux 6.1.82-rc1 Sasha Levin
2024-03-13 20:04 ` [PATCH 6.1 00/71] 6.1.82-rc1 review Pavel Machek
2024-03-13 20:13 ` Mateusz Jończyk
2024-03-13 21:27   ` Mateusz Jończyk
2024-03-14 21:12     ` Mateusz Jończyk
2024-03-14 22:04       ` Jens Axboe
2024-03-14 22:35         ` Sasha Levin
2024-03-14 22:40           ` Jens Axboe
2024-03-15 12:14             ` Sasha Levin
2024-03-15 14:42               ` Sasha Levin
2024-03-15 14:49                 ` Jens Axboe
2024-03-15 19:31                   ` Ron Economos
2024-03-14 14:43 ` Naresh Kamboju
2024-03-14 20:45 ` Florian Fainelli
2024-03-15 10:37 ` Shreeya Patel
2024-03-15 15:34 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240313163957.615276-41-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=michal.pecio@gmail.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox