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

[Wesley Cheng <wesley.cheng@oss.qualcomm.com>]

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