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

[PATCH 0/2] 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 the alignment_req parameter.
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.  By setting the alignment_req
beforehand, when allocating the ring segment, it can fulfill the audio DSP
alignment requirements by allocating DMA-able memory on the fly (based on
what is being requested) versus fetching it from the segment pool.
Likewise, keep track of if memory was dynamically allocated to handle the
free path properly.  The function call flow will now look like the
following:

handle_uaudio_stream_req()
  │
  ▼
enable_audio_stream(subs, ..., pcm_card_num)
  │
  ├─ xhci_sideband_add_endpoint(sb, data_ep, PAGE_SIZE)
  │    │  alignment_req == PAGE_SIZE
  │    ▼
  │  sb->alignment_req = alignment_req
  │
  ├─ snd_usb_endpoint_prepare(chip, data_endpoint)
  │     → xhci_check_bandwidth() → xhci_endpoint_init())
  │    ▼
  xhci_endpoint_init(..., ep_index, ...)
  │    if (sideband && sideband->alignment_req)
  │        new_ring = xhci_ring_alloc(xhci, 2, ring_type, max_packet,
  │                                    sideband->alignment_req, mem_flags)
  │    ▼
  xhci_ring_alloc(..., alignment_req, ...)
  │    ring->alignment_req = alignment_req
  │    ▼
  xhci_alloc_segments_for_ring(xhci, ring, flags)
  │    xhci_segment_alloc(xhci, ..., ring->alignment_req, flags)
  │    ▼
  xhci_segment_alloc(..., alignment_req, flags)
       if (alignment_req > TRB_SEGMENT_SIZE)
           seg->trbs = dma_alloc_coherent(dev, alignment_req, &dma, flags)
       else
           seg->trbs = dma_pool_zalloc(xhci->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.  The same
alignment_req parameter is passed, and during xHCI event ring creation, the
same set of APIs are utilized, so the runtime memory allocation is already
handled.

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>
---
Wesley Cheng (2):
      xhci: sideband: support page-aligned ring segment allocation
      ALSA: usb-audio: qcom: request page-aligned xHCI ring buffers

 drivers/usb/host/xhci-mem.c       | 70 ++++++++++++++++++++--------
 drivers/usb/host/xhci-sideband.c  |  9 ++--
 drivers/usb/host/xhci.h           | 11 ++++-
 include/linux/usb/xhci-sideband.h |  7 ++-
 sound/usb/qcom/qc_audio_offload.c | 96 +++++++++++++++++++++++++++++++++------
 5 files changed, 152 insertions(+), 41 deletions(-)
---
base-commit: e1e6e541c5c9cf548e9fdc35fc26808c82074440
change-id: 20260824-16k_offload_v1_b4-3d1460405774

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

[PATCH 1/2] xhci: sideband: support page-aligned ring segment allocation

[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.

A sideband client that maps a ring buffer directly via the IOMMU
(which operates at page granularity) needs the ring to start at a
page boundary and occupy whole pages, otherwise the IOVA it is given
does not correspond to the actual start of the ring.

Add an alignment_req parameter to the ring/segment allocation path
(xhci_ring_alloc(), xhci_segment_alloc(), xhci_endpoint_init(),
xhci_alloc_interrupter(), xhci_create_secondary_interrupter()) and to
the sideband API (xhci_sideband_add_endpoint(),
xhci_sideband_create_interrupter()) so a sideband client can request
a specific alignment for its endpoint ring and interrupter event
ring. When the requested alignment exceeds TRB_SEGMENT_SIZE, allocate
the segment directly with dma_alloc_coherent() at that size instead of
from the shared pool. Store the alignment requirement on struct
xhci_ring so later segments added via ring expansion keep the same
alignment for the life of the ring.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com>
---
 drivers/usb/host/xhci-mem.c       | 70 ++++++++++++++++++++++++++++-----------
 drivers/usb/host/xhci-sideband.c  |  9 +++--
 drivers/usb/host/xhci.h           | 11 ++++--
 include/linux/usb/xhci-sideband.h |  7 ++--
 4 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 7a21ac81f9c8..5f09b36c127b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -15,6 +15,7 @@
 #include <linux/dmapool.h>
 #include <linux/dma-mapping.h>
 #include <linux/bitfield.h>
+#include <linux/usb/xhci-sideband.h>
 
 #include "xhci.h"
 #include "xhci-trace.h"
@@ -30,6 +31,7 @@
 static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
 					       unsigned int max_packet,
 					       unsigned int num,
+					       unsigned int alignment_req,
 					       gfp_t flags)
 {
 	struct xhci_segment *seg;
@@ -40,7 +42,14 @@ 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);
+	if (alignment_req > TRB_SEGMENT_SIZE) {
+		seg->trbs = dma_alloc_coherent(dev, alignment_req, &dma, flags);
+		if (seg->trbs)
+			seg->alloc_size = alignment_req;
+	} else {
+		seg->trbs = dma_pool_zalloc(xhci->segment_pool, flags, &dma);
+	}
+
 	if (!seg->trbs) {
 		kfree(seg);
 		return NULL;
@@ -50,7 +59,10 @@ 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);
+			if (seg->alloc_size)
+				dma_free_coherent(dev, seg->alloc_size, seg->trbs, dma);
+			else
+				dma_pool_free(xhci->segment_pool, seg->trbs, dma);
 			kfree(seg);
 			return NULL;
 		}
@@ -65,7 +77,11 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
 static void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
 {
 	if (seg->trbs) {
-		dma_pool_free(xhci->segment_pool, seg->trbs, seg->dma);
+		if (seg->alloc_size)
+			dma_free_coherent(xhci_to_hcd(xhci)->self.sysdev,
+					  seg->alloc_size, seg->trbs, seg->dma);
+		else
+			dma_pool_free(xhci->segment_pool, seg->trbs, seg->dma);
 		seg->trbs = NULL;
 	}
 	kfree(seg->bounce_buf);
@@ -334,7 +350,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->bounce_buf_len, num, ring->alignment_req, flags);
 	if (!prev)
 		return -ENOMEM;
 	num++;
@@ -343,7 +359,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->bounce_buf_len, num,
+					  ring->alignment_req, flags);
 		if (!next)
 			goto free_segments;
 
@@ -370,7 +387,8 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
  * 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)
+				  enum xhci_ring_type type, unsigned int max_packet,
+				  unsigned int alignment_req, gfp_t flags)
 {
 	struct xhci_ring	*ring;
 	int ret;
@@ -382,6 +400,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->alignment_req = alignment_req;
 	INIT_LIST_HEAD(&ring->td_list);
 	ring->type = type;
 	if (num_segs == 0)
@@ -421,6 +440,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.alignment_req = ring->alignment_req;
 	new_ring.type = ring->type;
 	ret = xhci_alloc_segments_for_ring(xhci, &new_ring, flags);
 	if (ret)
@@ -663,7 +683,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 
 	for (cur_stream = 1; cur_stream < num_streams; cur_stream++) {
 		stream_info->stream_rings[cur_stream] =
-			xhci_ring_alloc(xhci, 2, TYPE_STREAM, max_packet, mem_flags);
+			xhci_ring_alloc(xhci, 2, TYPE_STREAM, max_packet, 0, mem_flags);
 		cur_ring = stream_info->stream_rings[cur_stream];
 		if (!cur_ring)
 			goto cleanup_rings;
@@ -1007,7 +1027,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
 	}
 
 	/* Allocate endpoint 0 ring */
-	dev->eps[0].ring = xhci_ring_alloc(xhci, 2, TYPE_CTRL, 0, flags);
+	dev->eps[0].ring = xhci_ring_alloc(xhci, 2, TYPE_CTRL, 0, 0, flags);
 	if (!dev->eps[0].ring)
 		goto fail;
 
@@ -1486,11 +1506,20 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 	if (usb_endpoint_xfer_control(&ep->desc) && xhci->hci_version >= 0x100)
 		avg_trb_len = 8;
 
-	/* Set up the endpoint ring */
-	virt_dev->eps[ep_index].new_ring =
-		xhci_ring_alloc(xhci, 2, ring_type, max_packet, mem_flags);
-	if (!virt_dev->eps[ep_index].new_ring)
-		return -ENOMEM;
+	if (virt_dev->eps[ep_index].sideband && virt_dev->eps[ep_index].sideband->alignment_req) {
+		virt_dev->eps[ep_index].new_ring =
+			xhci_ring_alloc(xhci, 2, ring_type, max_packet,
+					virt_dev->eps[ep_index].sideband->alignment_req,
+					mem_flags);
+		if (!virt_dev->eps[ep_index].new_ring)
+			return -ENOMEM;
+	} else {
+		/* Set up the endpoint ring */
+		virt_dev->eps[ep_index].new_ring =
+			xhci_ring_alloc(xhci, 2, ring_type, max_packet, 0, mem_flags);
+		if (!virt_dev->eps[ep_index].new_ring)
+			return -ENOMEM;
+	}
 
 	virt_dev->eps[ep_index].skip = false;
 	virt_dev->eps[ep_index].next_uframe = -1;
@@ -2291,7 +2320,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,
+		       unsigned int alignment_req, gfp_t flags)
 {
 	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
 	struct xhci_interrupter *ir;
@@ -2307,8 +2337,7 @@ xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs, gfp_t flags)
 	ir = kzalloc_node(sizeof(*ir), flags, dev_to_node(dev));
 	if (!ir)
 		return NULL;
-
-	ir->event_ring = xhci_ring_alloc(xhci, segs, TYPE_EVENT, 0, flags);
+	ir->event_ring = xhci_ring_alloc(xhci, segs, TYPE_EVENT, 0, alignment_req, flags);
 	if (!ir->event_ring) {
 		xhci_warn(xhci, "Failed to allocate interrupter event ring\n");
 		kfree(ir);
@@ -2356,7 +2385,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)
+				  u32 imod_interval, unsigned int intr_num,
+				  unsigned int alignment_req)
 {
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 	struct xhci_interrupter *ir;
@@ -2367,7 +2397,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, alignment_req, GFP_KERNEL);
 	if (!ir)
 		return NULL;
 
@@ -2485,7 +2515,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 		goto fail;
 
 	/* Set up the command ring to have one segments for now. */
-	xhci->cmd_ring = xhci_ring_alloc(xhci, 1, TYPE_COMMAND, 0, flags);
+	xhci->cmd_ring = xhci_ring_alloc(xhci, 1, TYPE_COMMAND, 0, 0, flags);
 	if (!xhci->cmd_ring)
 		goto fail;
 
@@ -2498,7 +2528,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, 0, flags);
 	if (!xhci->interrupters[0])
 		goto fail;
 
diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index a5deeee4d5dc..471219d7d86d 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -144,7 +144,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,
+			   unsigned int alignment_req)
 {
 	struct xhci_virt_ep *ep;
 	unsigned int ep_index;
@@ -171,6 +172,7 @@ xhci_sideband_add_endpoint(struct xhci_sideband *sb,
 	if (sb->eps[ep_index] || ep->sideband)
 		return -EBUSY;
 
+	sb->alignment_req = alignment_req;
 	ep->sideband = sb;
 	sb->eps[ep_index] = ep;
 
@@ -322,7 +324,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)
+				 bool ip_autoclear, u32 imod_interval, int intr_num,
+				 unsigned int alignment_req)
 {
 	if (!sb || !sb->xhci)
 		return -ENODEV;
@@ -337,7 +340,7 @@ xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
 
 	sb->ir = xhci_create_secondary_interrupter(xhci_to_hcd(sb->xhci),
 						   num_seg, imod_interval,
-						   intr_num);
+						   intr_num, alignment_req);
 	if (!sb->ir)
 		return -ENOMEM;
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c7bfa7f028d3..e356405c6351 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1290,6 +1290,10 @@ struct xhci_segment {
 	void			*bounce_buf;
 	unsigned int		bounce_offs;
 	unsigned int		bounce_len;
+	/* nonzero if trbs was allocated via dma_alloc_coherent() at this size,
+	 * instead of from xhci->segment_pool
+	 */
+	unsigned int		alloc_size;
 };
 
 enum xhci_cancelled_td_status {
@@ -1377,6 +1381,7 @@ struct xhci_ring {
 	unsigned int		stream_id;
 	unsigned int		num_segs;
 	unsigned int		bounce_buf_len;
+	unsigned int		alignment_req;
 	enum xhci_ring_type	type;
 	u32			old_trb_comp_code;
 	struct radix_tree_root	*trb_address_map;
@@ -1823,7 +1828,8 @@ int xhci_endpoint_init(struct xhci_hcd *xhci, struct xhci_virt_device *virt_dev,
 		struct usb_device *udev, struct usb_host_endpoint *ep,
 		gfp_t mem_flags);
 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);
+		enum xhci_ring_type type, unsigned int max_packet,
+		unsigned int alignment_req, gfp_t flags);
 void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring);
 int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
 		unsigned int num_trbs, gfp_t flags);
@@ -1865,7 +1871,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);
+				  u32 imod_interval, unsigned int intr_num,
+				  unsigned int alignment_req);
 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..8ab4e1cb0fc6 100644
--- a/include/linux/usb/xhci-sideband.h
+++ b/include/linux/usb/xhci-sideband.h
@@ -62,6 +62,7 @@ struct xhci_sideband {
 	struct usb_interface		*intf;
 	int (*notify_client)(struct usb_interface *intf,
 			     struct xhci_sideband_event *evt);
+	unsigned int			alignment_req;
 };
 
 struct xhci_sideband *
@@ -72,7 +73,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,
+			   unsigned int alignment_req);
 int
 xhci_sideband_remove_endpoint(struct xhci_sideband *sb,
 			      struct usb_host_endpoint *host_ep);
@@ -94,7 +96,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);
+				 bool ip_autoclear, u32 imod_interval, int intr_num,
+				 unsigned int alignment_req);
 void
 xhci_sideband_remove_interrupter(struct xhci_sideband *sb);
 int

-- 
2.34.1

[PATCH 2/2] ALSA: usb-audio: qcom: request page-aligned xHCI ring buffers

[Wesley Cheng]

Now that xhci sideband supports requesting a specific ring alignment,
ask for PAGE_SIZE alignment when adding the data/sync endpoints to the
sideband and when creating the interrupter's event ring, so the
buffers reported to the ADSP over QMI always start at a page boundary
and span a full page.

xhci_sideband_add_endpoint() must run before the endpoint's transfer
ring is first allocated (i.e. before snd_usb_endpoint_prepare()
triggers xhci_endpoint_init()) for the alignment request to apply to
that 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 | 96 +++++++++++++++++++++++++++++++++------
 1 file changed, 82 insertions(+), 14 deletions(-)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index e4bfd43a2488..87e016104c3d 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -941,6 +941,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
@@ -958,8 +997,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;
@@ -997,17 +1037,47 @@ 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, PAGE_SIZE);
+		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, PAGE_SIZE);
+			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,
@@ -1019,6 +1089,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);
 
@@ -1140,14 +1215,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,7 +1279,8 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 
 	/* event ring */
 	ret = xhci_sideband_create_interrupter(uadev[card_num].sb, 1, false,
-					       0, uaudio_qdev->data->intr_num);
+					       0, uaudio_qdev->data->intr_num,
+					       PAGE_SIZE);
 	if (ret < 0) {
 		dev_err(&subs->dev->dev, "failed to fetch interrupter\n");
 		goto put_offload;
@@ -1637,7 +1705,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,

-- 
2.34.1

Re: [PATCH 0/2] Add larger page size support for USB audio offload path

[Michal Pecio]

Hi,

On Mon, 24 Aug 2026 19:06:54 -0700, Wesley Cheng wrote:
> 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

FYI it's worse than you think - xhci_ring_to_sgtable() returns wrong
data and uses some allocation out of bounds on these systems. Quickly
scanning through the patch I haven't noticed any changes there.

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

If that's a security or reliability concern, perhaps each sideband
instance should create its own DMA pool, as opposed to allocating every
ring segment on a separate page?

I suppose each 'xhci_ring' could keep a pointer to its segment pool and
things would work for everyone, with very few changes.

Regards,
Michal

Re: [PATCH 0/2] Add larger page size support for USB audio offload path

[Takashi Iwai]

On Tue, 25 Aug 2026 04:06:54 +0200,
Wesley Cheng wrote:
> 
> 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 the alignment_req parameter.
> 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.  By setting the alignment_req
> beforehand, when allocating the ring segment, it can fulfill the audio DSP
> alignment requirements by allocating DMA-able memory on the fly (based on
> what is being requested) versus fetching it from the segment pool.
> Likewise, keep track of if memory was dynamically allocated to handle the
> free path properly.  The function call flow will now look like the
> following:
> 
> handle_uaudio_stream_req()
>   │
>   ▼
> enable_audio_stream(subs, ..., pcm_card_num)
>   │
>   ├─ xhci_sideband_add_endpoint(sb, data_ep, PAGE_SIZE)
>   │    │  alignment_req == PAGE_SIZE
>   │    ▼
>   │  sb->alignment_req = alignment_req
>   │
>   ├─ snd_usb_endpoint_prepare(chip, data_endpoint)
>   │     → xhci_check_bandwidth() → xhci_endpoint_init())
>   │    ▼
>   xhci_endpoint_init(..., ep_index, ...)
>   │    if (sideband && sideband->alignment_req)
>   │        new_ring = xhci_ring_alloc(xhci, 2, ring_type, max_packet,
>   │                                    sideband->alignment_req, mem_flags)
>   │    ▼
>   xhci_ring_alloc(..., alignment_req, ...)
>   │    ring->alignment_req = alignment_req
>   │    ▼
>   xhci_alloc_segments_for_ring(xhci, ring, flags)
>   │    xhci_segment_alloc(xhci, ..., ring->alignment_req, flags)
>   │    ▼
>   xhci_segment_alloc(..., alignment_req, flags)
>        if (alignment_req > TRB_SEGMENT_SIZE)
>            seg->trbs = dma_alloc_coherent(dev, alignment_req, &dma, flags)
>        else
>            seg->trbs = dma_pool_zalloc(xhci->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.  The same
> alignment_req parameter is passed, and during xHCI event ring creation, the
> same set of APIs are utilized, so the runtime memory allocation is already
> handled.
> 
> 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>
> ---
> Wesley Cheng (2):
>       xhci: sideband: support page-aligned ring segment allocation
>       ALSA: usb-audio: qcom: request page-aligned xHCI ring buffers

I guess your first patch alone breaks the build, and this is bad for
bisection.  When you change the API, the callers should be addressed
in the same commit altogether in order to keep the stuff working
during the transition.


thanks,

Takashi

Re: [PATCH 0/2] Add larger page size support for USB audio offload path

[Wesley Cheng]

On 8/25/2026 12:43 AM, Michal Pecio wrote:
> Hi,
> 
> On Mon, 24 Aug 2026 19:06:54 -0700, Wesley Cheng wrote:
>> 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
> 
> FYI it's worse than you think - xhci_ring_to_sgtable() returns wrong
> data and uses some allocation out of bounds on these systems. Quickly
> scanning through the patch I haven't noticed any changes there.
> 

Hi Michal,

Thanks for the review.

I had a tidbit that I tested that addressed an OOB condition, but as it 
currently stands, that API should be working properly, if TRB segment size 
== page size.  Hence, why I left it out as a change.

The OOB condition I saw was that when 16k pages were used (w/o this 
series), since specified rings can exist at a page offset, that offset 
information is never populated, so we might be mapping the incorrect range.

Regardless, I'll introduce that change in the next revision, since that's 
information that shouldn't be left out.

>> 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.
> 
> If that's a security or reliability concern, perhaps each sideband
> instance should create its own DMA pool, as opposed to allocating every
> ring segment on a separate page?
> 

This is an interesting suggestion.  Let me take a look at it more and get 
back to you.

Thanks
Wesley Cheng

> I suppose each 'xhci_ring' could keep a pointer to its segment pool and
> things would work for everyone, with very few changes.
> 
> Regards,
> Michal

Re: [PATCH 0/2] Add larger page size support for USB audio offload path

[Wesley Cheng]

On 8/25/2026 4:09 AM, Takashi Iwai wrote:
> On Tue, 25 Aug 2026 04:06:54 +0200,
> Wesley Cheng wrote:
>>
>> 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 the alignment_req parameter.
>> 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.  By setting the alignment_req
>> beforehand, when allocating the ring segment, it can fulfill the audio DSP
>> alignment requirements by allocating DMA-able memory on the fly (based on
>> what is being requested) versus fetching it from the segment pool.
>> Likewise, keep track of if memory was dynamically allocated to handle the
>> free path properly.  The function call flow will now look like the
>> following:
>>
>> handle_uaudio_stream_req()
>>    │
>>    ▼
>> enable_audio_stream(subs, ..., pcm_card_num)
>>    │
>>    ├─ xhci_sideband_add_endpoint(sb, data_ep, PAGE_SIZE)
>>    │    │  alignment_req == PAGE_SIZE
>>    │    ▼
>>    │  sb->alignment_req = alignment_req
>>    │
>>    ├─ snd_usb_endpoint_prepare(chip, data_endpoint)
>>    │     → xhci_check_bandwidth() → xhci_endpoint_init())
>>    │    ▼
>>    xhci_endpoint_init(..., ep_index, ...)
>>    │    if (sideband && sideband->alignment_req)
>>    │        new_ring = xhci_ring_alloc(xhci, 2, ring_type, max_packet,
>>    │                                    sideband->alignment_req, mem_flags)
>>    │    ▼
>>    xhci_ring_alloc(..., alignment_req, ...)
>>    │    ring->alignment_req = alignment_req
>>    │    ▼
>>    xhci_alloc_segments_for_ring(xhci, ring, flags)
>>    │    xhci_segment_alloc(xhci, ..., ring->alignment_req, flags)
>>    │    ▼
>>    xhci_segment_alloc(..., alignment_req, flags)
>>         if (alignment_req > TRB_SEGMENT_SIZE)
>>             seg->trbs = dma_alloc_coherent(dev, alignment_req, &dma, flags)
>>         else
>>             seg->trbs = dma_pool_zalloc(xhci->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.  The same
>> alignment_req parameter is passed, and during xHCI event ring creation, the
>> same set of APIs are utilized, so the runtime memory allocation is already
>> handled.
>>
>> 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>
>> ---
>> Wesley Cheng (2):
>>        xhci: sideband: support page-aligned ring segment allocation
>>        ALSA: usb-audio: qcom: request page-aligned xHCI ring buffers
> 
> I guess your first patch alone breaks the build, and this is bad for
> bisection.  When you change the API, the callers should be addressed
> in the same commit altogether in order to keep the stuff working
> during the transition.
> 

Hi Takashi,

Understood, I will figure out how to adjust these patches so that 
incremental builds don't break on the next revision.

Thanks
Wesley Cheng

> 
> thanks,
> 
> Takashi