All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@intel.com>
To: Michal Pecio <michal.pecio@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/5] xHCI: Decouple updating Dequeue from giveback
Date: Wed, 19 Nov 2025 14:35:37 +0200	[thread overview]
Message-ID: <88841c24-ee36-421d-bbbe-74d07b0fd594@intel.com> (raw)
In-Reply-To: <20251119120208.6a025eb0.michal.pecio@gmail.com>

On 11/19/25 13:02, Michal Pecio wrote:
> Hi,
> 
> This introduces unified mechanism for handling all short transfers
> and errors mid TD plus their later events, accurately and in-spec.
> I have been working on this code on and off for the last few months,
> it's dead simple conceptually and tested a lot on various hardware.
> 
> When a TD completes early, store its end_trb on ep_ring and give it
> back. Use end_trb to recognize future events for the TD. Remove the
> SPURIOUS_SUCCESS quirk and unreliable comp code guesswork.
> 
> Isochronous TDs with errors are given back instantly, which reduces
> latency and eliminates the need for error_mid_td flag and some hacks
> like handling Missed Service only to give back error_mid_td.
> 
> Dequeue will be moved in accordance with received events, never to
> the next TD right away. Previous code would do that on Short Packet,
> allowing overwriting of remaining TRBs if it happens across segment
> boundary. Rare enough that no one complained, but obviously wrong.
> 
> We will need a trb_in_range(), and I used this as an opportunity to
> smuggle some long-discussed changes and improve trb_in_td() usage.
> After converting from dma_addr_t to trb* once in handle_tx_event(),
> pass ep_trb instead ep_trb_dma to trb_in_td().
> 
> This requires a rewrite of trb_in_td(). New version is easier and
> shorter. While I'm aware it could be shorter still by using segment
> numbers, it has two advantages which I thought are neat:
> 
> * It can detect when "suspect" TRB is on a different ring than TD.
>    This means it has a loop, but common cases never enter it.
> 
>    (And yes, I've seen this warning once, but I suspect corruption -
>     DMA UAF was involved. Things improved with pdev->untrusted = 1).
> 
> * Needs only one segment pointer (suspect). Call sites are tidier
>    and I don't need to store last TD's end_seg together with end_trb.
> 
> Alternatively, segment numbers can also be used, but I really wanted
> to see if the code could be less noisy.


Looks like we both picked this up again.

I have queued up a couple patches that does basically the same trb_in_range()
changes as your patches 2/5 and 3/5, but uses segment indexes.

I'm sending all for-usb-next patches forward today for 6.19, including those
two patches. Just need to do one small test.

I'll take a closer look at the rest of the series (patches 1/5, 4/5, and 5/5)
They all seam like a good idea, but will need to be rebased on current
for-usb-next

Links to the two patches I mentioned:
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-next&id=7cca2b801bae19bfda2e54d7187f78e8af3f3700
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-next&id=b78fafee824b964c1db718662ef9854f9d7154dc

Link to my for-usb-next branch:
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=for-usb-next

Thanks
Mathias

      parent reply	other threads:[~2025-11-19 12:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-19 11:02 [PATCH 0/5] xHCI: Decouple updating Dequeue from giveback Michal Pecio
2025-11-19 11:02 ` [PATCH 1/5] usb: xhci: Track transfer ring dequeue progress properly Michal Pecio
2025-11-19 11:03 ` [PATCH 2/5] usb: xhci: Find transfer TRB early in handle_tx_event() Michal Pecio
2025-11-19 11:04 ` [PATCH 3/5] usb: xhci: Refactor and generalize trb_in_td() Michal Pecio
2025-11-19 11:05 ` [PATCH 4/5] usb: xhci: Reduce error mid TD latency with a new "done TD" mechanism Michal Pecio
2025-11-19 11:06 ` [PATCH 5/5] usb: xhci: Handle short transfers as "done TDs" Michal Pecio
2025-11-19 12:35 ` Mathias Nyman [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=88841c24-ee36-421d-bbbe-74d07b0fd594@intel.com \
    --to=mathias.nyman@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michal.pecio@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.