Re: [PATCH v11 2/2] usb: xhci: enable secondary interrupters and route transfers per slot

["Neronin, Niklas" <niklas.neronin@linux.intel.com>]

On 27/01/2026 4.39, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
> 
> Some xHCI hosts expose multiple MSI/MSI-X vectors and support multiple
> interrupters with independent event rings, but Linux commonly routes all
> transfer completions through interrupter 0.
> 
> This is not about increasing USB link throughput, but about avoiding a
> driver-imposed single hot spot. On hosts that already provide multiple
> MSI/MSI-X vectors and independent event rings, routing all completions
> through interrupter 0 creates unnecessary contention (serialized event
> handling/locks and coupled moderation), increasing CPU cost and tail
> latency under many active devices/endpoints. Using secondary interrupters
> simply matches the hardware's design, similar in spirit to merged
> xHCI-sideband work: exploit available parallel paths rather than
> funneling all events through one.
> 
> Allocate a small capped set of secondary interrupters in xhci_mem_init()
> (MAX_SECONDARY_INTRNUM, default 4) and request up to the matching number
> of IRQ vectors (bounded by what PCI core provides).  Dispatch each vector
> to its interrupter via the per-vector irq_ctx.
> 
> Route transfers per USB device (slot) by assigning vdev->interrupter at
> device allocation time and programming the interrupter target (slot
> context + TRB_INTR_TARGET) from that selection, so completions land on the
> selected event ring.  Drain a slot's secondary event ring at selected
> command completion boundaries to reduce late-event artifacts when teardown
> happens on interrupter 0.
> 
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> 

...

> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index b3efd6d8fd9c..4ff3e29070b1 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -150,6 +150,8 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>  	struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
>  	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>  	int ret;
> +	unsigned int irqs_num;
> +	int i;
>  	struct xhci_irq_ctx *ctx;
> 
>  	/*
> @@ -173,7 +175,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
> 
>  	/* TODO: Check with MSI Soc for sysdev */
>  	xhci->nvecs = pci_alloc_irq_vectors(pdev, 1, xhci->nvecs,
> -					    PCI_IRQ_MSIX | PCI_IRQ_MSI);
> +					    PCI_IRQ_MSIX | PCI_IRQ_MSI | PCI_IRQ_AFFINITY);
>  	if (xhci->nvecs < 0) {
>  		xhci_dbg_trace(xhci, trace_xhci_dbg_init,
>  			       "failed to allocate IRQ vectors");
> @@ -183,14 +185,30 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>  	memset(xhci->irq_ctx, 0, sizeof(xhci->irq_ctx));
>  	xhci->irqs_enabled = 0;
> 
> -	ctx = &xhci->irq_ctx[0];
> -	ctx->hcd = hcd;
> -	ctx->intr_num = 0;
> -	ret = request_irq(pci_irq_vector(pdev, 0), xhci_msi_irq, 0, "xhci_hcd", ctx);
> -	if (ret)
> -		goto free_irq_vectors;
> +	/*
> +	 * Request up to 1 + secondary_irqs_alloc vectors (vector 0 + secondary),
> +	 * limited by what PCI core actually allocated.
> +	 */
> +	irqs_num = min_t(unsigned int,
> +		     (unsigned int)xhci->nvecs,
> +		     (unsigned int)(1 + xhci->secondary_irqs_alloc));
> +
> +	for (i = 0; i < irqs_num; i++) {
> +		ctx = &xhci->irq_ctx[i];
> +		ctx->hcd = hcd;
> +		ctx->intr_num = i;
> +		ret = request_irq(pci_irq_vector(pdev, i), xhci_msi_irq, 0,
> +				  "xhci_hcd", ctx);
> +		if (ret) {
> +			while (--i >= 0)
> +				free_irq(pci_irq_vector(pdev, i), &xhci->irq_ctx[i]);
> +			xhci->irqs_enabled = 0;
> +			memset(xhci->irq_ctx, 0, sizeof(xhci->irq_ctx));
> +			goto free_irq_vectors;

Won't this 'goto' end up freeing all IRQ vectors?

Now, if requesting an IRQ for a secondary interrupter fails,
the xhci driver will then fall back to using the legacy IRQ?

> +		}
> +		xhci->irqs_enabled++;
> +	}

Maybe it would be useful to add a debug message here, for example:
	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "enabled %u %s interrupters",
		       xhci->irqs_enabled, pdev->msix_enabled ? "MSI-X" : "MSI");

I'm not sure whether this information is already reported elsewhere.

Thanks,
Niklas