[PATCH v3 2/4] usb: xhci: sideband: allocate sideband ring segments from a dedicated pool

From: Wesley Cheng

Date: Fri Sep 04 2026 - 03:05:11 EST


Ring segments are normally allocated from a shared DMA pool sized and
aligned to TRB_SEGMENT_SIZE (4096 bytes). On kernels built with a
larger PAGE_SIZE (e.g. 16K or 64K page arches), a segment can end up
at a non-page-aligned offset within its enclosing CPU page, and
multiple segments can share the same physical page.

A sideband client that maps a ring buffer directly via the IOMMU
(which operates at page granularity) needs to know exactly which
page(s) back the ring, and only pages that are actually intended to
be exposed to that client should ever be mapped this way.

Allow each xhci_sideband endpoint to pass its own segment_pool, allocated
separately from the core xhci->segment_pool, so every segment backing
a sideband-tagged endpoint always comes from a page that is meant to
be visible by the entity handling the offloaded endpoints. Normal
(non-offloaded) endpoints are unaffected, as they keep allocating from
xhci->segment_pool.

The offload client owns the pool's full lifetime, and since
that lifetime is no longer tied to the sideband instance itself,
xhci_sideband_unregister() must free any ring still backed by a
client-supplied pool before returning, rather than leaving it for xhci
to free later when the client and its pool may already be gone.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Wesley Cheng <wesley.cheng@xxxxxxxxxxxxxxxx>
---
drivers/usb/host/xhci-mem.c | 63 +++++++++++++++++++++++++--------------
drivers/usb/host/xhci-sideband.c | 40 ++++++++++++++++++++++---
drivers/usb/host/xhci.h | 16 +++++-----
include/linux/usb/xhci-sideband.h | 20 +++++++++++--
sound/usb/qcom/qc_audio_offload.c | 21 +++++++++++--
5 files changed, 121 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 7a21ac81f9c8..a041a35fcd4f 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -28,6 +28,7 @@
* "All components of all Command and Transfer TRBs shall be initialized to '0'"
*/
static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
+ struct dma_pool *pool,
unsigned int max_packet,
unsigned int num,
gfp_t flags)
@@ -40,7 +41,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
if (!seg)
return NULL;

- seg->trbs = dma_pool_zalloc(xhci->segment_pool, flags, &dma);
+ seg->trbs = dma_pool_zalloc(pool, flags, &dma);
if (!seg->trbs) {
kfree(seg);
return NULL;
@@ -50,7 +51,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
seg->bounce_buf = kzalloc_node(max_packet, flags,
dev_to_node(dev));
if (!seg->bounce_buf) {
- dma_pool_free(xhci->segment_pool, seg->trbs, dma);
+ dma_pool_free(pool, seg->trbs, dma);
kfree(seg);
return NULL;
}
@@ -62,10 +63,11 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
return seg;
}

-static void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
+static void xhci_segment_free(struct xhci_hcd *xhci, struct dma_pool *pool,
+ struct xhci_segment *seg)
{
if (seg->trbs) {
- dma_pool_free(xhci->segment_pool, seg->trbs, seg->dma);
+ dma_pool_free(pool, seg->trbs, seg->dma);
seg->trbs = NULL;
}
kfree(seg->bounce_buf);
@@ -81,7 +83,7 @@ static void xhci_ring_segments_free(struct xhci_hcd *xhci, struct xhci_ring *rin

while (seg) {
next = seg->next;
- xhci_segment_free(xhci, seg);
+ xhci_segment_free(xhci, ring->segment_pool, seg);
seg = next;
}
}
@@ -334,7 +336,7 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
struct xhci_segment *prev;
unsigned int num = 0;

- prev = xhci_segment_alloc(xhci, ring->bounce_buf_len, num, flags);
+ prev = xhci_segment_alloc(xhci, ring->segment_pool, ring->bounce_buf_len, num, flags);
if (!prev)
return -ENOMEM;
num++;
@@ -343,7 +345,8 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
while (num < ring->num_segs) {
struct xhci_segment *next;

- next = xhci_segment_alloc(xhci, ring->bounce_buf_len, num, flags);
+ next = xhci_segment_alloc(xhci, ring->segment_pool, ring->bounce_buf_len,
+ num, flags);
if (!next)
goto free_segments;

@@ -362,15 +365,10 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
return -ENOMEM;
}

-/*
- * Create a new ring with zero or more segments.
- *
- * Link each segment together into a ring.
- * Set the end flag and the cycle toggle bit on the last segment.
- * See section 4.9.1 and figures 15 and 16.
- */
-struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
- enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
+static struct xhci_ring *
+xhci_ring_alloc_from_pool(struct xhci_hcd *xhci, unsigned int num_segs,
+ enum xhci_ring_type type, unsigned int max_packet,
+ struct dma_pool *pool, gfp_t flags)
{
struct xhci_ring *ring;
int ret;
@@ -382,6 +380,7 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,

ring->num_segs = num_segs;
ring->bounce_buf_len = max_packet;
+ ring->segment_pool = pool;
INIT_LIST_HEAD(&ring->td_list);
ring->type = type;
if (num_segs == 0)
@@ -398,6 +397,20 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
return NULL;
}

+/*
+ * Create a new ring with zero or more segments.
+ *
+ * Link each segment together into a ring.
+ * Set the end flag and the cycle toggle bit on the last segment.
+ * See section 4.9.1 and figures 15 and 16.
+ */
+struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
+ enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
+{
+ return xhci_ring_alloc_from_pool(xhci, num_segs, type, max_packet,
+ xhci->segment_pool, flags);
+}
+
void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
struct xhci_virt_device *virt_dev,
unsigned int ep_index)
@@ -422,6 +435,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
new_ring.num_segs = num_new_segs;
new_ring.bounce_buf_len = ring->bounce_buf_len;
new_ring.type = ring->type;
+ new_ring.segment_pool = ring->segment_pool;
ret = xhci_alloc_segments_for_ring(xhci, &new_ring, flags);
if (ret)
return -ENOMEM;
@@ -1424,6 +1438,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
unsigned int mult;
unsigned int avg_trb_len;
unsigned int err_count = 0;
+ struct dma_pool *pool;

ep_index = xhci_get_endpoint_index(&ep->desc);
ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->in_ctx, ep_index);
@@ -1487,8 +1502,10 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
avg_trb_len = 8;

/* Set up the endpoint ring */
+ pool = virt_dev->eps[ep_index].priv_seg_pool ?
+ virt_dev->eps[ep_index].priv_seg_pool : xhci->segment_pool;
virt_dev->eps[ep_index].new_ring =
- xhci_ring_alloc(xhci, 2, ring_type, max_packet, mem_flags);
+ xhci_ring_alloc_from_pool(xhci, 2, ring_type, max_packet, pool, mem_flags);
if (!virt_dev->eps[ep_index].new_ring)
return -ENOMEM;

@@ -2291,7 +2308,8 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
}

static struct xhci_interrupter *
-xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs, gfp_t flags)
+xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs,
+ struct dma_pool *pool, gfp_t flags)
{
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
struct xhci_interrupter *ir;
@@ -2308,7 +2326,7 @@ xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs, gfp_t flags)
if (!ir)
return NULL;

- ir->event_ring = xhci_ring_alloc(xhci, segs, TYPE_EVENT, 0, flags);
+ ir->event_ring = xhci_ring_alloc_from_pool(xhci, segs, TYPE_EVENT, 0, pool, flags);
if (!ir->event_ring) {
xhci_warn(xhci, "Failed to allocate interrupter event ring\n");
kfree(ir);
@@ -2356,7 +2374,8 @@ void xhci_add_interrupter(struct xhci_hcd *xhci, unsigned int intr_num)

struct xhci_interrupter *
xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
- u32 imod_interval, unsigned int intr_num)
+ struct dma_pool *pool, u32 imod_interval,
+ unsigned int intr_num)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_interrupter *ir;
@@ -2367,7 +2386,7 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
intr_num >= xhci->max_interrupters)
return NULL;

- ir = xhci_alloc_interrupter(xhci, segs, GFP_KERNEL);
+ ir = xhci_alloc_interrupter(xhci, segs, pool, GFP_KERNEL);
if (!ir)
return NULL;

@@ -2498,7 +2517,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
if (!xhci->interrupters)
goto fail;

- xhci->interrupters[0] = xhci_alloc_interrupter(xhci, 0, flags);
+ xhci->interrupters[0] = xhci_alloc_interrupter(xhci, 0, xhci->segment_pool, flags);
if (!xhci->interrupters[0])
goto fail;

diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index beb637407e47..1bb6e5034b58 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -9,6 +9,7 @@
*/

#include <linux/usb/xhci-sideband.h>
+#include <linux/dmapool.h>

#include "xhci.h"

@@ -67,6 +68,7 @@ __xhci_sideband_remove_endpoint(struct xhci_sideband *sb, struct xhci_virt_ep *e
xhci_stop_endpoint_sync(sb->xhci, ep, 0, GFP_KERNEL);

ep->sideband = NULL;
+ ep->priv_seg_pool = NULL;
sb->eps[ep->ep_index] = NULL;
}

@@ -113,6 +115,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_notify_ep_ring_free);
* xhci_sideband_add_endpoint - add endpoint to sideband access list
* @sb: sideband instance for this usb device
* @host_ep: usb host endpoint
+ * @pool: dma pool to allocate this endpoint's ring segments from, or NULL
+ * to leave the endpoint's current pool selection untouched
*
* Adds an endpoint to the list of sideband accessed endpoints for this usb
* device.
@@ -123,7 +127,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_notify_ep_ring_free);
*/
int
xhci_sideband_add_endpoint(struct xhci_sideband *sb,
- struct usb_host_endpoint *host_ep)
+ struct usb_host_endpoint *host_ep,
+ struct dma_pool *pool)
{
struct xhci_virt_ep *ep;
unsigned int ep_index;
@@ -153,6 +158,9 @@ xhci_sideband_add_endpoint(struct xhci_sideband *sb,
ep->sideband = sb;
sb->eps[ep_index] = ep;

+ if (pool)
+ ep->priv_seg_pool = pool;
+
return 0;
}
EXPORT_SYMBOL_GPL(xhci_sideband_add_endpoint);
@@ -288,6 +296,7 @@ EXPORT_SYMBOL_GPL(xhci_sideband_check);
* xhci_sideband_create_interrupter - creates a new interrupter for this sideband
* @sb: sideband instance for this usb device
* @num_seg: number of event ring segments to allocate
+ * @pool: dma pool to allocate the interrupter's event ring segments from
* @ip_autoclear: IP autoclearing support such as MSI implemented
*
* Sets up a xhci interrupter that can be used for this sideband accessed usb
@@ -301,7 +310,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_check);
*/
int
xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
- bool ip_autoclear, u32 imod_interval, int intr_num)
+ struct dma_pool *pool, bool ip_autoclear,
+ u32 imod_interval, int intr_num)
{
if (!sb || !sb->xhci)
return -ENODEV;
@@ -315,8 +325,8 @@ xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
return -EBUSY;

sb->ir = xhci_create_secondary_interrupter(xhci_to_hcd(sb->xhci),
- num_seg, imod_interval,
- intr_num);
+ num_seg, pool,
+ imod_interval, intr_num);
if (!sb->ir)
return -ENOMEM;

@@ -370,6 +380,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_interrupter_id);
/**
* xhci_sideband_register - register a sideband for a usb device
* @intf: usb interface associated with the sideband device
+ * @type: xHCI sideband type
+ * @notify_client: callback for xHCI sideband sequences
*
* Allows for clients to utilize XHCI interrupters and fetch transfer and event
* ring parameters for executing data transfers.
@@ -436,6 +448,15 @@ EXPORT_SYMBOL_GPL(xhci_sideband_register);
* After this the endpoint and interrupter event buffers should no longer
* be accessed via sideband. The xhci driver can now take over handling
* the buffers.
+ * Any transfer ring allocated from a client supplied dma pool is freed here
+ * as well, as the client is not expected to keep that pool alive any longer
+ * than this call. This includes rings of endpoints already removed with
+ * xhci_sideband_remove_endpoint(), which xhci would otherwise only free once
+ * the device is reconfigured or torn down, i.e. after the client is gone.
+ *
+ * The caller must ensure the usb device is no longer streaming through the
+ * normal, non-sideband path when calling this, as the freed rings are still
+ * referenced by the endpoint contexts until xhci reconfigures the device.
*/
void
xhci_sideband_unregister(struct xhci_sideband *sb)
@@ -458,6 +479,17 @@ xhci_sideband_unregister(struct xhci_sideband *sb)
if (sb->eps[i])
__xhci_sideband_remove_endpoint(sb, sb->eps[i]);

+ spin_lock_irq(&xhci->lock);
+ for (i = 0; i < EP_CTX_PER_DEV; i++) {
+ struct xhci_ring *ring = vdev->eps[i].ring;
+
+ if (ring && ring->segment_pool != xhci->segment_pool) {
+ xhci_ring_free(xhci, ring);
+ vdev->eps[i].ring = NULL;
+ }
+ }
+ spin_unlock_irq(&xhci->lock);
+
__xhci_sideband_remove_interrupter(sb);

sb->vdev = NULL;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c7bfa7f028d3..1353d6fa2776 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -19,6 +19,7 @@
#include <linux/usb/hcd.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/io-64-nonatomic-hi-lo.h>
+#include <linux/usb/xhci-sideband.h>

/* Code sharing between pci-quirks and xhci hcd */
#include "xhci-ext-caps.h"
@@ -709,6 +710,8 @@ struct xhci_virt_ep {
bool use_extended_tbc;
/* set if this endpoint is controlled via sideband access*/
struct xhci_sideband *sideband;
+ /* dma pool to allocate this endpoint's ring segments from, if set */
+ struct dma_pool *priv_seg_pool;
};

enum xhci_overhead_type {
@@ -738,8 +741,6 @@ struct xhci_interval_bw_table {
unsigned int ss_bw_out;
};

-#define EP_CTX_PER_DEV 31
-
struct xhci_virt_device {
int slot_id;
struct usb_device *udev;
@@ -1253,14 +1254,11 @@ static inline const char *xhci_trb_type_string(u8 type)
#define NEC_FW_MAJOR(p) (((p) >> 8) & 0xff)

/*
- * TRBS_PER_SEGMENT must be a multiple of 4,
- * since the command ring is 64-byte aligned.
- * It must also be greater than 16.
+ * TRBS_PER_SEGMENT and TRB_SEGMENT_SIZE are defined in
+ * <linux/usb/xhci-sideband.h>, shared with sideband client drivers.
*/
-#define TRBS_PER_SEGMENT 256
/* Allow two commands + a link TRB, along with any reserved command TRBs */
#define MAX_RSVD_CMD_TRBS (TRBS_PER_SEGMENT - 3)
-#define TRB_SEGMENT_SIZE (TRBS_PER_SEGMENT*16)
#define TRB_SEGMENT_SHIFT (ilog2(TRB_SEGMENT_SIZE))
/* TRB buffer pointers can't cross 64KB boundaries */
#define TRB_MAX_BUFF_SHIFT 16
@@ -1380,6 +1378,7 @@ struct xhci_ring {
enum xhci_ring_type type;
u32 old_trb_comp_code;
struct radix_tree_root *trb_address_map;
+ struct dma_pool *segment_pool;
};

struct xhci_erst_entry {
@@ -1865,7 +1864,8 @@ void xhci_free_port_bw_ctx(struct xhci_hcd *xhci,
struct xhci_container_ctx *ctx);
struct xhci_interrupter *
xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
- u32 imod_interval, unsigned int intr_num);
+ struct dma_pool *pool, u32 imod_interval,
+ unsigned int intr_num);
void xhci_remove_secondary_interrupter(struct usb_hcd
*hcd, struct xhci_interrupter *ir);
void xhci_skip_sec_intr_events(struct xhci_hcd *xhci,
diff --git a/include/linux/usb/xhci-sideband.h b/include/linux/usb/xhci-sideband.h
index 005257085dcb..6d318e6a3bf6 100644
--- a/include/linux/usb/xhci-sideband.h
+++ b/include/linux/usb/xhci-sideband.h
@@ -13,7 +13,19 @@
#include <linux/usb.h>
#include <linux/usb/hcd.h>

-#define EP_CTX_PER_DEV 31 /* FIXME defined twice, from xhci.h */
+/*
+ * Constants shared with the xHCI host driver (drivers/usb/host/xhci.h),
+ * which includes this header for its canonical definitions.
+ */
+#define EP_CTX_PER_DEV 31
+
+/*
+ * TRBS_PER_SEGMENT must be a multiple of 4,
+ * since the command ring is 64-byte aligned.
+ * It must also be greater than 16.
+ */
+#define TRBS_PER_SEGMENT 256
+#define TRB_SEGMENT_SIZE (TRBS_PER_SEGMENT * 16)

struct xhci_sideband;

@@ -72,7 +84,8 @@ void
xhci_sideband_unregister(struct xhci_sideband *sb);
int
xhci_sideband_add_endpoint(struct xhci_sideband *sb,
- struct usb_host_endpoint *host_ep);
+ struct usb_host_endpoint *host_ep,
+ struct dma_pool *pool);
int
xhci_sideband_remove_endpoint(struct xhci_sideband *sb,
struct usb_host_endpoint *host_ep);
@@ -94,7 +107,8 @@ static inline bool xhci_sideband_check(struct usb_hcd *hcd)

int
xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
- bool ip_autoclear, u32 imod_interval, int intr_num);
+ struct dma_pool *pool, bool ip_autoclear,
+ u32 imod_interval, int intr_num);
void
xhci_sideband_remove_interrupter(struct xhci_sideband *sb);
int
diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index e4bfd43a2488..1b8877b8ee62 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -7,6 +7,7 @@
#include <linux/ctype.h>
#include <linux/dma-mapping.h>
#include <linux/dma-map-ops.h>
+#include <linux/dmapool.h>
#include <linux/init.h>
#include <linux/iommu.h>
#include <linux/module.h>
@@ -1786,6 +1787,7 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
struct usb_interface_descriptor *altsd;
struct usb_host_interface *alts;
struct snd_soc_usb_device *sdev;
+ struct dma_pool *segment_pool;
struct xhci_sideband *sb;

/*
@@ -1804,10 +1806,19 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
if (!sdev)
return;

- sb = xhci_sideband_register(intf, XHCI_SIDEBAND_VENDOR,
+ segment_pool = dma_pool_create("xHCI sideband ring segments",
+ interface_to_usbdev(intf)->bus->sysdev,
+ TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE,
+ TRB_SEGMENT_SIZE);
+ if (!segment_pool)
+ goto free_sdev;
+
+ sb = xhci_sideband_register(intf, XHCI_SIDEBAND_VENDOR, segment_pool,
uaudio_sideband_notifier);
- if (!sb)
+ if (!sb) {
+ dma_pool_destroy(segment_pool);
goto free_sdev;
+ }
} else {
sb = uadev[chip->card->number].sb;
sdev = uadev[chip->card->number].sdev;
@@ -1844,7 +1855,9 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
return;

unreg_xhci:
+ segment_pool = sb->segment_pool;
xhci_sideband_unregister(sb);
+ dma_pool_destroy(segment_pool);
uadev[chip->card->number].sb = NULL;
free_sdev:
kfree(sdev);
@@ -1905,8 +1918,12 @@ static void qc_usb_audio_offload_disconnect(struct snd_usb_audio *chip)
* This is to accommodate for devices w/ multiple UAC functions.
*/
if (chip->num_interfaces == 1) {
+ struct dma_pool *segment_pool = dev->sb->segment_pool;
+
snd_soc_usb_disconnect(uaudio_qdev->auxdev->dev.parent, dev->sdev);
xhci_sideband_unregister(dev->sb);
+ dma_pool_destroy(segment_pool);
+ dev->sb = NULL;
dev->chip = NULL;
kfree(dev->sdev->ppcm_idx);
kfree(dev->sdev);

--
2.34.1