From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: linux-usb@vger.kernel.org
Subject: [PATCH v2] xhci: fix matching completion events with TDs
Date: Fri, 26 Jan 2024 10:59:59 +0100 [thread overview]
Message-ID: <20240126105959.28d7848b@foxbook> (raw)
In-Reply-To: <20240123112230.4d9e1ef0@foxbook>
A trb_in_td() call is used to determine if a completion event matches
any TRB of the currently executing TD. This function is told to start
searching right after the last finished TD, which is not at all where
the currently expected TD is guaranteed to begin, because some TDs in
between may have been cancelled.
Not only is a pointless work performed, but a bug resulting in the HC
executing cancelled TDs was seen to trick the driver into associating
events from a TD just cancelled with an unrelated future TD.
Since the ring is being traversed for the specific purpose of finding
a match with the current TD, always start from its first TRB. This is
the most reliable bit of information that we posses.
Tracking of HC's work progress is not affected, except for cases when
a misattributed event would have moved dequeue past a pending TD.
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
v2: improved commit message based on new findings
I am now fairly convinced that this is indeed a good idea. Otherwise,
certain event abnormalities develop into several further failures:
- completion of TDs not yet completed can be reported to the core
- ... which may conceivably lead even to DMA-after-free
- ring->dequeue is progressed past a TD not yet released by hardware
- diagnostics are printed only on a later, actually correct event
drivers/usb/host/xhci-ring.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9673354d70d5..d9be5023abe6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2809,7 +2809,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
td_num--;
/* Is this a TRB in the currently executing TD? */
- ep_seg = trb_in_td(xhci, ep_ring->deq_seg, ep_ring->dequeue,
+ ep_seg = trb_in_td(xhci, td->start_seg, td->first_trb,
td->last_trb, ep_trb_dma, false);
/*
@@ -2877,9 +2877,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
"part of current TD ep_index %d "
"comp_code %u\n", ep_index,
trb_comp_code);
- trb_in_td(xhci, ep_ring->deq_seg,
- ep_ring->dequeue, td->last_trb,
- ep_trb_dma, true);
+ trb_in_td(xhci, td->start_seg, td->first_trb,
+ td->last_trb, ep_trb_dma, true);
return -ESHUTDOWN;
}
}
--
2.43.0
prev parent reply other threads:[~2024-01-26 10:00 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-23 10:22 [RFC PATCH] xhci: fix matching completion events with TDs Michał Pecio
2024-01-26 9:59 ` Michal Pecio [this message]
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=20240126105959.28d7848b@foxbook \
--to=michal.pecio@gmail.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@linux.intel.com \
/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;
as well as URLs for NNTP newsgroup(s).