Re: [PATCH v3] usb: xhci: route endpoints to secondary interrupters

[Michal Pecio <michal.pecio@gmail.com>]

On Wed, 17 Dec 2025 17:46:08 +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
> 
> Some xHCI hosts expose multiple MSI/MSI-X vectors, but the driver
> currently routes all transfer completions through interrupter 0.
> This can lead to unnecessary contention on the primary event ring
> and IRQ vector.
> 
> If the host reports more than one interrupter, allocate secondary
> interrupters [1..max_interrupters-1] and cap the number of created
> interrupters by num_online_cpus(). Each secondary interrupter has
> its own event ring and can be requested as a separate IRQ vector.
> 
> An interrupter is bound to each endpoint once at endpoint enable
> time. EP0 is always kept on interrupter 0, while all other endpoints
> are assigned in a round-robin fashion over the enabled secondary
> interrupters. Multiple endpoints may therefore share the same
> interrupter.
> 
> Interrupt routing is performed by programming TRB_INTR_TARGET()
> from the endpoint's bound interrupter number when queueing TRBs.
> As a result, transfer completions are delivered to the event ring
> (and IRQ vector) of the selected interrupter instead of always
> landing on interrupter 0.
> 
> All interrupters share a common IRQ handler. STS_EINT is only
> checked and cleared for interrupter 0, as it is only meaningful
> for the primary interrupter. Secondary MSI/MSI-X vectors may fire
> independently and simply process their associated event rings.
> 
> When requesting IRQs, the usb_hcd pointer is used as the dev_id
> for both primary and secondary vectors. Although each secondary
> interrupter has its own event ring, using per-interrupter dev_id
> objects complicates teardown ordering in xhci_cleanup_msix().
> In particular, IRQs must be freed before the corresponding
> interrupter structures are removed, and the existing cleanup
> sequence imposes constraints on dev_id lifetime. Using a common
> dev_id avoids dev_id mismatches during free_irq() and keeps the
> IRQ teardown consistent with the current xHCI removal flow.
> 
> Testing on an MSI-X capable host controller shows that interrupts
> are effectively distributed across secondary vectors. For example,
> after sustained USB transfers:
> 
>   /proc/interrupts (filtered):
>     32:          0          0          0          0          0          0 \
>           0        522  IR-PCI-MSIX-0000:03:00.3  0-edge  xhci_hcd
>     33:      12297          0          0          0          0          0 \
>           0          0  IR-PCI-MSIX-0000:03:00.3  1-edge  xhci_hcd
>     34:          0      17082          0          0          0          0 \
>           0          0  IR-PCI-MSIX-0000:03:00.3  2-edge  xhci_hcd
>     35:          0          0      27567          0          0          0 \
>           0          0  IR-PCI-MSIX-0000:03:00.3  3-edge  xhci_hcd
>     36:          0          0          0      33234          0          0 \
>           0          0  IR-PCI-MSIX-0000:03:00.3  4-edge  xhci_hcd
>     37:          0          0          0          0    1519411          0 \
>           0          0  IR-PCI-MSIX-0000:03:00.3  5-edge  xhci_hcd
> 
> This demonstrates that transfer completions are no longer handled
> exclusively by interrupter 0, but are instead distributed across
> multiple MSI-X vectors.

What's the practical outcome and does it justify all this complexity?

The patch isn't even complete, because it opens a can of worms which
has never been dealt with before: completion events could be handled
in different order than they actually happened.

Most transfer events will go to secondary rings, yes. But some events
don't refer to any TRB and there is no TRB_INTR_TARGET applicable, so
they go to the slot's default interrupter. Command completion events
can only be delivered to the primary interrupter.

All event handling code relies on the assumption that transfer events
that happened earlier on the same endpoint have already been handled,
which won't be true if they are waiting in a different event ring.

It doesn't sound like fun to go through *all* of that code and come up
with workarounds. I guess it would be necessary to scan the other ring
for relevant events before doing anything. Figuring out ordering may
be further complicated by not all events having TRB pointers.


Also, I think there is no debugfs support for secondary event rings?

Regards,
Michal