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

[Wesley Cheng]

On 8/31/2026 6:04 AM, Mathias Nyman wrote:
> On 8/29/26 00:37, 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 a separate segment_pool
> > associated to each sideband instance.  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.
>
> The custom dma pool looks like a good solution.
>
> I think we should tune this a bit and pass the custom pool pointer to
> xhci_sideband_add_endpoint() and xhci_sideband_create_interrupter() instead
> of xhci_sideband_register()
>
> xhci.h:
> struct xhci_virt_ep {
>      ...
>      struct dma_pool    *priv_seg_pool;
> }
>
> xhci-mem.c:
> xhci_endpoint_init()
> {
>      struct dma_pool *pool;
>      struct xhci_virt_ep *ep;
>      ...
>
>      ep = &virt_dev->eps[ep_index]
>
>      /* use ep->priv_seg_pool if set by sideband or .add_endpoint wrapper */
>      if (ep->priv_seg_pool)
>          pool = ep->priv_seg_pool;
>      else
>          pool = xhci->segment_pool;
>
>      xhci_ring_alloc_from_pool(..., pool);}
>
> xhci-sideband.c:
>
> xhci_sideband_add_endpoint(..., struct dma_pool *pool)
> {
>      ...
>      if (pool)
>          ep->priv_seg_pool = pool;
> }
>
> This allows finer granularity in selecting dma pools for endpoints.
>
> It also keeps the xhci "core" sideband agnostic, avoids including xhci- 
> sideband.h
> in xhci-mem.c
>
> It also helps possible vtio support so it can set its own ep->priv_seg_pool 
> in a
> possible .add_endpint wrapper.

Hi Mathias,

Makes sense.  I made the switch to adding it into the add_endpoint path and 
its cleaner.  I'll wait for a little bit for more reviews before submitting 
the next revision.

Thanks
Wesley Cheng