[PATCH v3 0/4] Add larger page size support for USB audio offload path

[PATCH v3 0/4] Add larger page size support for USB audio offload path

[Wesley Cheng]

On some environments, 16kB pages can be enabled from the Linux subsystem,
which manages the IOMMU mappings for the audio DSP within the system.  In
the current design, the following assumptions break when 16k pages are
utilized:
  1. xHCI ring size is equal to PAGE_SIZE
  2. Ring addresses start at the beginning of a page

When the USB offload driver maps the rings (w/ the audio DSP SID), it is
set with a 16k granular, which is a problem, as several xHCI rings could
exist on the same page.  This is because the rings are currently allocated
from the segment_pool.  Hence, potentially mapping non USB audio related
rings into the region accessible by the audio DSP.

To mitigate this, this series introduces a separate segment_pool
associated to a sideband ring.  Before the USB audio offload path
is enabled, the USB audio data streams/endpoint are not active.  Only when
the class driver issues a usb_set_interface() call (done from
snd_usb_endpoint_prepare()), will the xHCI allocate the transfer ring
resources.  Which pool is selected is all based on if the sideband path
is being enabled, and if so, memory can be allocated from that pool,
which expects to be owned in conjunction with the audio DSP.  This
concept allows to keep the same model existing in xHCI, where multiple
4k segments can reside on the same page, which reduces potentially over
allocating based on the page size.

Likewise this mechanism also allows for the offload client driver to
determine which SID is associated to the segment_pool if it decides to
map outside of the Linux subsystem.  The new ring allocation flow for
sideband/offload clients will be as follows:

qc_usb_audio_offload_probe()
  ├─ segment_pool = dma_pool_create(...)
  ▼
xhci_sideband_register(intf, XHCI_SIDEBAND_VENDOR, segment_pool, notify_client)
  │  sb->segment_pool = segment_pool
  ▼
uadev[card_num].sb = sb

handle_uaudio_stream_req()
  ▼
enable_audio_stream(subs, ..., pcm_card_num)
  ├─ data_ep = uaudio_find_host_endpoint(subs, subs->data_endpoint)
  ├─ xhci_sideband_add_endpoint(sb, data_ep) ← ep->sideband = sb; sb->eps[ep_index] = ep
  ├─ snd_usb_endpoint_prepare(chip, sync_endpoint) ─┐
  ├─ snd_usb_endpoint_prepare(chip, data_endpoint)  ├─→ xhci_check_bandwidth()
  │                                                 ▼
  │            xhci_endpoint_init(xhci, virt_dev, ep, ...)
  │            pool = sideband ? sideband->segment_pool : xhci->segment_pool
  │            new_ring = xhci_ring_alloc_from_pool(..., pool, ...)
  │            ▼
  │            xhci_ring_alloc_from_pool(..., pool, flags)
  │            ring->segment_pool = pool
  │            ▼
  │            xhci_alloc_segments_for_ring(xhci, ring, flags)
  │            xhci_segment_alloc(xhci, ring->segment_pool, max_packet, num, flags)
  │            ▼
  │            xhci_segment_alloc(xhci, pool, max_packet, num, flags)
  │            seg->trbs = dma_pool_zalloc(pool, flags, &dma)
  ▼
xhci_sideband_get_endpoint_buffer(sb, data_ep) → xhci_ring_to_sgtable()

qc_usb_audio_offload_disconnect() / unreg_xhci:
  ├─ segment_pool = sb->segment_pool
  ├─ xhci_sideband_unregister(sb)
  ▼
  dma_pool_destroy(segment_pool)

Similar logic is added for the secondary interrupter path as well.  The USB
offload class driver calls xhci_sideband_create_interrupter(), which will
be responsible for allocating the secondary event ring.

There is one point that was seen during disconnecting the USB audio
device while audio offload was active, where the offload client driver
was pre-maturely freeing the DMA pool before xHCI core was able to free
the DMA memory allocated for the rings.  This is the reason for needing
to explicitly call the ring free during the sideband unregister path.

This was confirmed to work on the SM8350 MTP platform, with the 
CONFIG_ARM64_16K_PAGES config enabled, alongside tinyaudio binaries:

tinymix -D 0 set 513 1  (Enables USB_RX multimedia#1 path)
tinyplay -D 0 -d 0....  (Routes PCM data to ASoC platform sound card) 

Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com>
---
Changes in v3:
- Moved sideband segment pool assignment to when a sideband endpoint
  is added.
- Address an issue where during USB device disconnect (while offload
  active), where the offload client driver is freeing the DMA pool
  before xHCI core is able to free the dma allocations.
- Update xhci_ring_to_sgtable to correctly allocate and fill the sg
  table which capture rings within page offsets.
- Link to v2: https://patch.msgid.link/20260828-16k_offload_v1_b4-v2-0-8a46369ebbb6@oss.qualcomm.com

Changes in v2:
- Moved from using alignment_req to having the offload driver maintain
  its own segment pool.
- Fixed OOB condition seen in xhci_ring_to_sgtable() and will capture
  page offsets properly.
- Addressed inter-patch build failures.
- Link to v1: https://patch.msgid.link/20260824-16k_offload_v1_b4-v1-0-49a6be60ca30@oss.qualcomm.com

To: Mathias Nyman <mathias.nyman@intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
Cc: linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-sound@vger.kernel.org

---
Wesley Cheng (4):
      xhci: sideband: fix ring sg table for sub-page TRB segments
      usb: xhci: sideband: allocate sideband ring segments from a dedicated pool
      ALSA: usb-audio: qcom: tag sideband endpoints before ring allocation
      ALSA: usb-audio: qcom: fix xfer ring IOMMU unmap on 16K+ page kernels

 drivers/usb/host/xhci-mem.c       |  63 ++++++++++------
 drivers/usb/host/xhci-sideband.c  |  97 ++++++++++++++-----------
 drivers/usb/host/xhci.h           |  16 ++---
 include/linux/usb/xhci-sideband.h |  20 +++++-
 sound/usb/qcom/qc_audio_offload.c | 147 +++++++++++++++++++++++++++++++-------
 5 files changed, 243 insertions(+), 100 deletions(-)
---
base-commit: e1e6e541c5c9cf548e9fdc35fc26808c82074440
change-id: 20260824-16k_offload_v1_b4-3d1460405774

Best regards,
--  
Wesley Cheng <wesley.cheng@oss.qualcomm.com>

[PATCH v3 1/4] xhci: sideband: fix ring sg table for sub-page TRB segments

[Wesley Cheng]

xhci_ring_to_sgtable() populated its sg_table via dma_get_sgtable()
per segment and sg_alloc_table_from_pages(), both of which only
operate at whole PAGE_SIZE granularity. Since TRB_SEGMENT_SIZE (4096)
can be smaller than PAGE_SIZE, multiple ring segments can share the
same physical page on larger-PAGE_SIZE kernels (16K/64K), which these
helpers cannot correctly represent.

Build the sg_table directly instead: allocate one sg entry per ring
segment with sg_alloc_table(), and fill each entry explicitly with
sg_set_page() using the segment's own page (resolved via
is_vmalloc_addr()/vmalloc_to_page() or virt_to_page()),
TRB_SEGMENT_SIZE as the length, and offset_in_page() for the exact
intra-page offset. This guarantees each segment gets its own sg
entry regardless of page sharing.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com>
---
 drivers/usb/host/xhci-sideband.c | 57 +++++++++++++---------------------------
 1 file changed, 18 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index a5deeee4d5dc..beb637407e47 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -9,57 +9,42 @@
  */
 
 #include <linux/usb/xhci-sideband.h>
-#include <linux/dma-direct.h>
 
 #include "xhci.h"
 
 /* sideband internal helpers */
 static struct sg_table *
-xhci_ring_to_sgtable(struct xhci_sideband *sb, struct xhci_ring *ring)
+xhci_ring_to_sgtable(struct xhci_ring *ring)
 {
 	struct xhci_segment *seg;
 	struct sg_table	*sgt;
-	unsigned int n_pages;
-	struct page **pages;
-	struct device *dev;
-	size_t sz;
+	struct page *page;
 	int i;
 
-	dev = xhci_to_hcd(sb->xhci)->self.sysdev;
-	sz = ring->num_segs * TRB_SEGMENT_SIZE;
-	n_pages = PAGE_ALIGN(sz) >> PAGE_SHIFT;
-	pages = kvmalloc_objs(struct page *, n_pages);
-	if (!pages)
+	seg = ring->first_seg;
+	if (!seg)
 		return NULL;
 
 	sgt = kzalloc_obj(*sgt);
-	if (!sgt) {
-		kvfree(pages);
+	if (!sgt)
+		return NULL;
+
+	if (sg_alloc_table(sgt, ring->num_segs, GFP_KERNEL)) {
+		kfree(sgt);
 		return NULL;
 	}
 
-	seg = ring->first_seg;
-	if (!seg)
-		goto err;
-	/*
-	 * Rings can potentially have multiple segments, create an array that
-	 * carries page references to allocated segments.  Utilize the
-	 * sg_alloc_table_from_pages() to create the sg table, and to ensure
-	 * that page links are created.
-	 */
 	for (i = 0; i < ring->num_segs; i++) {
-		dma_get_sgtable(dev, sgt, seg->trbs, seg->dma,
-				TRB_SEGMENT_SIZE);
-		pages[i] = sg_page(sgt->sgl);
-		sg_free_table(sgt);
+		if (is_vmalloc_addr(seg->trbs))
+			page = vmalloc_to_page(seg->trbs);
+		else
+			page = virt_to_page(seg->trbs);
+
+		sg_set_page(&sgt->sgl[i], page, TRB_SEGMENT_SIZE,
+			    offset_in_page(seg->trbs));
 		seg = seg->next;
 	}
 
-	if (sg_alloc_table_from_pages(sgt, pages, n_pages, 0, sz, GFP_KERNEL))
-		goto err;
-
-	kvfree(pages);
-
 	/*
 	 * Save first segment dma address to sg dma_address field for the sideband
 	 * client to have access to the IOVA of the ring.
@@ -67,12 +52,6 @@ xhci_ring_to_sgtable(struct xhci_sideband *sb, struct xhci_ring *ring)
 	sg_dma_address(sgt->sgl) = ring->first_seg->dma;
 
 	return sgt;
-
-err:
-	kvfree(pages);
-	kfree(sgt);
-
-	return NULL;
 }
 
 /* Caller must hold sb->mutex */
@@ -254,7 +233,7 @@ xhci_sideband_get_endpoint_buffer(struct xhci_sideband *sb,
 	if (!ep || !ep->ring || !ep->sideband || ep->sideband != sb)
 		return NULL;
 
-	return xhci_ring_to_sgtable(sb, ep->ring);
+	return xhci_ring_to_sgtable(ep->ring);
 }
 EXPORT_SYMBOL_GPL(xhci_sideband_get_endpoint_buffer);
 
@@ -276,7 +255,7 @@ xhci_sideband_get_event_buffer(struct xhci_sideband *sb)
 	if (!sb || !sb->ir)
 		return NULL;
 
-	return xhci_ring_to_sgtable(sb, sb->ir->event_ring);
+	return xhci_ring_to_sgtable(sb->ir->event_ring);
 }
 EXPORT_SYMBOL_GPL(xhci_sideband_get_event_buffer);
 

-- 
2.34.1

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

[Wesley Cheng]

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@oss.qualcomm.com>
---
 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

[PATCH v3 3/4] ALSA: usb-audio: qcom: tag sideband endpoints before ring allocation

[Wesley Cheng]

xhci_endpoint_init() picks the ring's segment pool based on whether the
endpoint has already been tagged via xhci_sideband_add_endpoint():
sideband-tagged endpoints get their ring allocated from the offload
client's own segment_pool instead of the shared xhci->segment_pool, so the
buffer reported to the ADSP over QMI is guaranteed to come from a page
meant to be ADSP-visible.

xhci_sideband_add_endpoint() must therefore run before the endpoint's
transfer ring is first allocated (i.e. before snd_usb_endpoint_prepare()
triggers xhci_endpoint_init()) for that pool selection to apply to the
first allocation. Move the xhci_sideband_add_endpoint() calls out of
uaudio_endpoint_setup() and into enable_audio_stream(), before
snd_usb_endpoint_prepare() is called for the data and sync endpoints,
and unwind them on the new error paths.

At that point in the setup sequence dev->ep_in[]/ep_out[] are not yet
populated, since the endpoint's altsetting has not been activated, so
usb_pipe_endpoint() cannot be used to find the usb_host_endpoint. Add
uaudio_find_host_endpoint(), which resolves it directly from the
interface's altsetting descriptor table instead.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com>
---
 sound/usb/qcom/qc_audio_offload.c | 111 +++++++++++++++++++++++++++++++-------
 1 file changed, 93 insertions(+), 18 deletions(-)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index 1b8877b8ee62..f09dae8334d0 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -132,6 +132,7 @@ struct uaudio_dev {
 
 	/* xhci sideband */
 	struct xhci_sideband *sb;
+	struct dma_pool *segment_pool;
 
 	/* SoC USB device */
 	struct snd_soc_usb_device *sdev;
@@ -942,6 +943,45 @@ static void uaudio_dev_release(struct kref *kref)
 	wake_up(&dev->disconnect_wq);
 }
 
+/**
+ * uaudio_find_host_endpoint() - look up usb_host_endpoint for a snd_usb_endpoint
+ * @subs: usb substream owning the target snd_usb_endpoint
+ * @endpoint: sync or data snd_usb_endpoint to resolve
+ *
+ * usb_pipe_endpoint() resolves via dev->ep_in[]/ep_out[], which are only
+ * populated once usb_set_interface() has activated the endpoint's altsetting
+ * (i.e. after snd_usb_endpoint_prepare() has run for it). Looking that up
+ * beforehand returns NULL.
+ *
+ * Instead, look the endpoint up directly in the interface's altsetting
+ * descriptor table, which is populated once at enumeration time and stays
+ * valid regardless of which altsetting is currently active.
+ *
+ * Return: matching usb_host_endpoint, or NULL if not found.
+ */
+static struct usb_host_endpoint *
+uaudio_find_host_endpoint(struct snd_usb_substream *subs,
+			  struct snd_usb_endpoint *endpoint)
+{
+	struct usb_host_interface *alt;
+	struct usb_interface *iface;
+	int i;
+
+	iface = usb_ifnum_to_if(subs->dev, endpoint->iface);
+	if (!iface)
+		return NULL;
+
+	alt = usb_altnum_to_altsetting(iface, endpoint->altsetting);
+	if (!alt)
+		return NULL;
+
+	for (i = 0; i < alt->desc.bNumEndpoints; i++)
+		if (alt->endpoint[i].desc.bEndpointAddress == endpoint->ep_num)
+			return &alt->endpoint[i];
+
+	return NULL;
+}
+
 /**
  * enable_audio_stream() - enable usb snd endpoints
  * @subs: usb substream
@@ -959,8 +999,9 @@ static void uaudio_dev_release(struct kref *kref)
 static int enable_audio_stream(struct snd_usb_substream *subs,
 			       snd_pcm_format_t pcm_format,
 			       unsigned int channels, unsigned int cur_rate,
-			       int datainterval)
+			       int datainterval, unsigned int card_num)
 {
+	struct usb_host_endpoint *data_ep = NULL, *sync_ep = NULL;
 	struct snd_pcm_hw_params params;
 	struct snd_usb_audio *chip;
 	struct snd_interval *i;
@@ -998,17 +1039,49 @@ static int enable_audio_stream(struct snd_usb_substream *subs,
 			goto detach_ep;
 		}
 
+		data_ep = uaudio_find_host_endpoint(subs, subs->data_endpoint);
+		if (!data_ep) {
+			dev_err(&subs->dev->dev, "data ep # %d not found\n",
+				subs->data_endpoint->ep_num);
+			ret = -ENODEV;
+			goto detach_ep;
+		}
+
+		ret = xhci_sideband_add_endpoint(uadev[card_num].sb, data_ep,
+						  uadev[card_num].segment_pool);
+		if (ret < 0) {
+			dev_err(&subs->dev->dev,
+				"failed to add data ep to sec intr: %d\n", ret);
+			goto detach_ep;
+		}
+
 		if (subs->sync_endpoint) {
+			sync_ep = uaudio_find_host_endpoint(subs, subs->sync_endpoint);
+			if (!sync_ep) {
+				dev_err(&subs->dev->dev, "sync ep # %d not found\n",
+					subs->sync_endpoint->ep_num);
+				ret = -ENODEV;
+				goto remove_data_ep;
+			}
+
+			ret = xhci_sideband_add_endpoint(uadev[card_num].sb, sync_ep,
+							  uadev[card_num].segment_pool);
+			if (ret < 0) {
+				dev_err(&subs->dev->dev,
+					"failed to add sync ep to sec intr: %d\n", ret);
+				goto remove_data_ep;
+			}
+
 			ret = snd_usb_endpoint_prepare(chip, subs->sync_endpoint);
 			if (ret < 0)
-				goto detach_ep;
+				goto remove_sync_ep;
 		}
 
 		ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint);
 		if (ret < 0)
-			goto detach_ep;
+			goto remove_sync_ep;
 
-		dev_dbg(uaudio_qdev->data->dev,
+		dev_dbg(&subs->dev->dev,
 			"selected %s iface:%d altsetting:%d datainterval:%dus\n",
 			subs->direction ? "capture" : "playback",
 			subs->cur_audiofmt->iface, subs->cur_audiofmt->altsetting,
@@ -1020,6 +1093,11 @@ static int enable_audio_stream(struct snd_usb_substream *subs,
 
 	return 0;
 
+remove_sync_ep:
+	if (sync_ep)
+		xhci_sideband_remove_endpoint(uadev[card_num].sb, sync_ep);
+remove_data_ep:
+	xhci_sideband_remove_endpoint(uadev[card_num].sb, data_ep);
 detach_ep:
 	snd_usb_hw_free(subs);
 
@@ -1141,14 +1219,6 @@ uaudio_endpoint_setup(struct snd_usb_substream *subs,
 
 	memcpy(ep_desc, &ep->desc, sizeof(ep->desc));
 
-	ret = xhci_sideband_add_endpoint(uadev[card_num].sb, ep);
-	if (ret < 0) {
-		dev_err(&subs->dev->dev,
-			"failed to add data ep to sec intr: %d\n", ret);
-		ret = -ENODEV;
-		goto exit;
-	}
-
 	sgt = xhci_sideband_get_endpoint_buffer(uadev[card_num].sb, ep);
 	if (!sgt) {
 		dev_err(&subs->dev->dev,
@@ -1212,8 +1282,9 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 		goto exit;
 
 	/* event ring */
-	ret = xhci_sideband_create_interrupter(uadev[card_num].sb, 1, false,
-					       0, uaudio_qdev->data->intr_num);
+	ret = xhci_sideband_create_interrupter(uadev[card_num].sb, 1,
+					       uadev[card_num].segment_pool,
+					       false, 0, uaudio_qdev->data->intr_num);
 	if (ret < 0) {
 		dev_err(&subs->dev->dev, "failed to fetch interrupter\n");
 		goto put_offload;
@@ -1638,7 +1709,7 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 		ret = enable_audio_stream(subs,
 					  map_pcm_format(req_msg->audio_format),
 					  req_msg->number_of_ch, req_msg->bit_rate,
-					  datainterval);
+					  datainterval, pcm_card_num);
 
 		if (!ret)
 			ret = prepare_qmi_response(subs, req_msg, &resp,
@@ -1813,12 +1884,14 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
 		if (!segment_pool)
 			goto free_sdev;
 
-		sb = xhci_sideband_register(intf, XHCI_SIDEBAND_VENDOR, segment_pool,
+		sb = xhci_sideband_register(intf, XHCI_SIDEBAND_VENDOR,
 					    uaudio_sideband_notifier);
 		if (!sb) {
 			dma_pool_destroy(segment_pool);
 			goto free_sdev;
 		}
+
+		uadev[chip->card->number].segment_pool = segment_pool;
 	} else {
 		sb = uadev[chip->card->number].sb;
 		sdev = uadev[chip->card->number].sdev;
@@ -1855,10 +1928,11 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
 	return;
 
 unreg_xhci:
-	segment_pool = sb->segment_pool;
+	segment_pool = uadev[chip->card->number].segment_pool;
 	xhci_sideband_unregister(sb);
 	dma_pool_destroy(segment_pool);
 	uadev[chip->card->number].sb = NULL;
+	uadev[chip->card->number].segment_pool = NULL;
 free_sdev:
 	kfree(sdev);
 	uadev[chip->card->number].sdev = NULL;
@@ -1918,12 +1992,13 @@ 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;
+		struct dma_pool *segment_pool = dev->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->segment_pool = NULL;
 		dev->chip = NULL;
 		kfree(dev->sdev->ppcm_idx);
 		kfree(dev->sdev);

-- 
2.34.1

[PATCH v3 4/4] ALSA: usb-audio: qcom: fix xfer ring IOMMU unmap on 16K+ page kernels

[Wesley Cheng]

TRB_SEGMENT_SIZE is hardcoded to 4096 bytes, but on kernels built with
a larger PAGE_SIZE (e.g. 16K or 64K page arches) the IOMMU still maps
and unmaps in units of PAGE_SIZE. A ring segment's physical page can
therefore start at a non-page-aligned offset relative to the segment
itself, and the DMA address handed back for the ring
(sg_dma_address()) carries that same intra-page offset.

Add that offset back onto the mapped iova before sending it to the
ADSP over QMI, so the reported address resolves to the start of the
segment rather than the start of its containing page, and report the
true TRB_SEGMENT_SIZE instead of PAGE_SIZE as the ring size.

This broke the reverse direction: recovering the raw, page-aligned
iova for iommu_unmap() by masking off the low PAGE_SIZE bits of the
QMI-reported iova only works if that iova happens to already be
page-aligned before the offset was added, which is not guaranteed.
Add RING_IOVA_BASE(), which instead subtracts the exact offset that
was added at setup time, and use it for both the cached
data/sync_xfer_ring_va and the drop_sync_ep/drop_data_ep unmap error
paths.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com>
---
 sound/usb/qcom/qc_audio_offload.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index f09dae8334d0..515f4271a58c 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -58,6 +58,8 @@
 #define PREPEND_SID_TO_IOVA(iova, sid) ((u64)(((u64)(iova)) | \
 					(((u64)sid) << 32)))
 #define IOVA_MASK(iova) (((u64)(iova)) & 0xFFFFFFFF)
+/* recover the raw xfer ring iova by subtracting the intra-page offset added at setup */
+#define RING_IOVA_BASE(mem) (IOVA_MASK((mem).iova) - ((mem).dma & ~PAGE_MASK))
 #define IOVA_BASE 0x1000
 #define IOVA_XFER_RING_BASE (IOVA_BASE + PAGE_SIZE * (SNDRV_CARDS + 1))
 #define IOVA_XFER_BUF_BASE (IOVA_XFER_RING_BASE + PAGE_SIZE * SNDRV_CARDS * 32)
@@ -1241,8 +1243,10 @@ uaudio_endpoint_setup(struct snd_usb_substream *subs,
 		goto clear_pa;
 	}
 
-	mem_info->iova = PREPEND_SID_TO_IOVA(iova, uaudio_qdev->data->sid);
-	mem_info->size = PAGE_SIZE;
+	/* add intra-page offset so DSP IOVA resolves to the correct 4K slot */
+	mem_info->iova = PREPEND_SID_TO_IOVA(iova + (mem_info->dma & ~PAGE_MASK),
+					     uaudio_qdev->data->sid);
+	mem_info->size = TRB_SEGMENT_SIZE;
 
 	return 0;
 
@@ -1311,8 +1315,10 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 		goto clear_pa;
 	}
 
-	mem_info->iova = PREPEND_SID_TO_IOVA(iova, uaudio_qdev->data->sid);
-	mem_info->size = PAGE_SIZE;
+	/* add intra-page offset so DSP IOVA resolves to the correct 4K slot */
+	mem_info->iova = PREPEND_SID_TO_IOVA(iova + (mem_info->dma & ~PAGE_MASK),
+					     uaudio_qdev->data->sid);
+	mem_info->size = TRB_SEGMENT_SIZE;
 
 	return 0;
 
@@ -1551,10 +1557,10 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,
 
 	/* cache intf specific info to use it for unmap and free xfer buf */
 	uadev[card_num].info[info_idx].data_xfer_ring_va =
-					IOVA_MASK(resp->xhci_mem_info.tr_data.iova);
+					RING_IOVA_BASE(resp->xhci_mem_info.tr_data);
 	uadev[card_num].info[info_idx].data_xfer_ring_size = PAGE_SIZE;
 	uadev[card_num].info[info_idx].sync_xfer_ring_va =
-					IOVA_MASK(resp->xhci_mem_info.tr_sync.iova);
+					RING_IOVA_BASE(resp->xhci_mem_info.tr_sync);
 	uadev[card_num].info[info_idx].sync_xfer_ring_size = PAGE_SIZE;
 	uadev[card_num].info[info_idx].xfer_buf_iova =
 					IOVA_MASK(resp->xhci_mem_info.xfer_buff.iova);
@@ -1589,13 +1595,14 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,
 drop_sync_ep:
 	if (subs->sync_endpoint) {
 		uaudio_iommu_unmap(MEM_XFER_RING,
-				   IOVA_MASK(resp->xhci_mem_info.tr_sync.iova),
+				   RING_IOVA_BASE(resp->xhci_mem_info.tr_sync),
 				   PAGE_SIZE, PAGE_SIZE);
 		xhci_sideband_remove_endpoint(uadev[card_num].sb,
 			usb_pipe_endpoint(subs->dev, subs->sync_endpoint->pipe));
 	}
 drop_data_ep:
-	uaudio_iommu_unmap(MEM_XFER_RING, IOVA_MASK(resp->xhci_mem_info.tr_data.iova),
+	uaudio_iommu_unmap(MEM_XFER_RING,
+			   RING_IOVA_BASE(resp->xhci_mem_info.tr_data),
 			   PAGE_SIZE, PAGE_SIZE);
 	xhci_sideband_remove_endpoint(uadev[card_num].sb,
 			usb_pipe_endpoint(subs->dev, subs->data_endpoint->pipe));

-- 
2.34.1

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

[Mathias Nyman]

On 9/4/26 09:57, Wesley Cheng wrote:
> 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@oss.qualcomm.com>
> ---
>   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);


Passing a custom dma pool to xhci_create_secondary_interrupters() shuld be optional.
Callers shoudn't need to be aware of the xhci->segment_pool, and pass it in the default case.

Instead use the default xhci->segment_pool if caller passes NULL for pool.


>   	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;

I would split the patch here.

Do all xhci "core" code above in one patch, and the sideband changes in a separate patch.
Only sideband change needed in first patch would be passing NULL for pool when
creating the secondary interrupter:

  	sb->ir = xhci_create_secondary_interrupter(xhci_to_hcd(sb->xhci),
-						   num_seg, imod_interval,
-						   intr_num);
+						   num_seg, NULL,
+						   imod_interval, intr_num);

Thanks
Mathias

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

[Wesley Cheng]

On 9/9/2026 3:08 AM, Mathias Nyman wrote:
> On 9/4/26 09:57, Wesley Cheng wrote:
>> 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@oss.qualcomm.com>
>> ---
>>   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);
> 
> 
> Passing a custom dma pool to xhci_create_secondary_interrupters() shuld be 
> optional.
> Callers shoudn't need to be aware of the xhci->segment_pool, and pass it in 
> the default case.
> 

Makes sense, will fix this.

> Instead use the default xhci->segment_pool if caller passes NULL for pool.
> 
> 
>>       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;
> 
> I would split the patch here.
> 
> Do all xhci "core" code above in one patch, and the sideband changes in a 
> separate patch.
> Only sideband change needed in first patch would be passing NULL for pool when
> creating the secondary interrupter:
> 
>       sb->ir = xhci_create_secondary_interrupter(xhci_to_hcd(sb->xhci),
> -                           num_seg, imod_interval,
> -                           intr_num);
> +                           num_seg, NULL,
> +                           imod_interval, intr_num);
> 
Sounds good. Will split the patch accordingly.

Thanks
Wesley Cheng