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>

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

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

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

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

On 8/25/2026 12:08 PM, Wesley Cheng wrote:
> 
> 
> 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.
> 

Hi Michal,

Thanks for this suggestion.  I think it actually makes the overall design a 
lot better.  So now that the sideband driver has its own segment pool (per 
sideband instance), we expect that any page allocations done from this pool 
is technically owned by the audio DSP.  This allows us to still utilize 4k 
ring segments, while mapping the entire 16k page, so it helps 
conserve/optimize the memory allocations.  I will do a bit more testing and 
review before submitting a new revision w/ these changes.

BTW, I tried my best to see if I could re-use existing ring/segment alloc 
apis w/o modifying the arguments, but up to a certain point it was 
unavoidable.  However, I think code re-use is better than having a, more or 
less the same, sideband API variant.

Thanks
Wesley Cheng

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

On Wed, 26 Aug 2026 00:50:53 -0700, Wesley Cheng wrote:
> Thanks for this suggestion.  I think it actually makes the overall
> design a lot better.  So now that the sideband driver has its own
> segment pool (per sideband instance), we expect that any page
> allocations done from this pool is technically owned by the audio
> DSP.  This allows us to still utilize 4k ring segments, while mapping
> the entire 16k page, so it helps conserve/optimize the memory
> allocations.  I will do a bit more testing and review before
> submitting a new revision w/ these changes.

The part about memory being "owned by the audio DSP" made me wonder
if it would be helpful to let offload drivers allocate their own memory
and then just dma_map() it for the xHC. No new rings would be allocated
for offloaded endpoints when they are enabled, we would point Endpoint
Context of the xHC to the sideband ring and leave ep->ring as NULL.

Offload drivers would have full control over memory allocation - size,
number of segments (it seems that qc-usb-audio only uses one out of two
allocated by xhci-hcd), alignment, anything else.

It would become impossible to offload an endpoint which is already
enabled, but is this an issue for anyone?

NULL ep->ring will cause oopses/panics when somebody submits URBs to
offloaded endpoints, but I think it wouldn't be a problem otherwise.

Regards,
Michal

On 8/26/26 13:25, Michal Pecio wrote:
> On Wed, 26 Aug 2026 00:50:53 -0700, Wesley Cheng wrote:
>> Thanks for this suggestion.  I think it actually makes the overall
>> design a lot better.  So now that the sideband driver has its own
>> segment pool (per sideband instance), we expect that any page
>> allocations done from this pool is technically owned by the audio
>> DSP.  This allows us to still utilize 4k ring segments, while mapping
>> the entire 16k page, so it helps conserve/optimize the memory
>> allocations.  I will do a bit more testing and review before
>> submitting a new revision w/ these changes.
> 
> The part about memory being "owned by the audio DSP" made me wonder
> if it would be helpful to let offload drivers allocate their own memory
> and then just dma_map() it for the xHC. No new rings would be allocated
> for offloaded endpoints when they are enabled, we would point Endpoint
> Context of the xHC to the sideband ring and leave ep->ring as NULL.
> 
> Offload drivers would have full control over memory allocation - size,
> number of segments (it seems that qc-usb-audio only uses one out of two
> allocated by xhci-hcd), alignment, anything else.
> 
> It would become impossible to offload an endpoint which is already
> enabled, but is this an issue for anyone?
> 
> NULL ep->ring will cause oopses/panics when somebody submits URBs to
> offloaded endpoints, but I think it wouldn't be a problem otherwise.
> 
I have similar thoughts.

One idea would be to basically let sideband allocate the entire ring and set
ep->new_ring early. This would tell xhci_endpoint_init() that a ring exists and
a new one should not be allocated.

xhci ring allocation would need some refactoring to create helpers for sideband
to allocate and initialize all the other parts of the ring.

This is something that VTIO (xhci spec section 4.25) would also need.
There an endpoint can be handed over to a secondary DMA ID (second, new PCI BDF),
that the normal xhci driver can be excluded from  with iommu.

VTIO use case is something like trusted VM accessing a secure usb storage device,
preventing regular OS running the xhci driver in another VM from touching it.

The secure VM needs to allocate and map the ring to this secondary PCI BDF

Thanks
Mathias

On Wed, 26 Aug 2026 14:44:42 +0300, Mathias Nyman wrote:
> On 8/26/26 13:25, Michal Pecio wrote:
> > The part about memory being "owned by the audio DSP" made me wonder
> > if it would be helpful to let offload drivers allocate their own
> > memory and then just dma_map() it for the xHC. No new rings would
> > be allocated for offloaded endpoints when they are enabled, we
> > would point Endpoint Context of the xHC to the sideband ring and
> > leave ep->ring as NULL.
> > 
> > Offload drivers would have full control over memory allocation -
> > size, number of segments (it seems that qc-usb-audio only uses one
> > out of two allocated by xhci-hcd), alignment, anything else.
> > 
> > It would become impossible to offload an endpoint which is already
> > enabled, but is this an issue for anyone?
> > 
> > NULL ep->ring will cause oopses/panics when somebody submits URBs to
> > offloaded endpoints, but I think it wouldn't be a problem otherwise.
> >   
> I have similar thoughts.
> 
> One idea would be to basically let sideband allocate the entire ring
> and set ep->new_ring early. This would tell xhci_endpoint_init() that
> a ring exists and a new one should not be allocated.

Actually, my suggestion was more radical: make the ring NULL for
offloaded endpoints. Driver core would only be concerned with copying
some dequeue pointer to the EP Context, which pointer would only have
meaning to the sideband client, the xHC, and maybe xhci-sideband.

This would be a new special case in xhci_endpoint_init(), but little
other core changes, I think. And yes, it makes sense that sideband
support shouldn't exclude using normal URBs, but they can't both work
at the same time. We can restore normal operation when SB is gone.

> xhci ring allocation would need some refactoring to create helpers
> for sideband to allocate and initialize all the other parts of the
> ring.

Helpers can be exported if clients need them. But it seems existing QC
driver has different idea about segment count (I think it uses one) and
hence it probably also writes its own link TRB and doesn't need ours.

Even segment size - does it need to be equal in QC DSP and xhci-hcd?
Today it is, but one or the other side might want to change it later.

It also seems that QC DSP expects the rings to appear at particular
IOVAs in particular order, so QC driver maps them one by one through
IOMMU. Alternatively, it could map one big block, divide it into rings
as the DSP desires and pass pointers to sideband_create_endpoint().
Looks like less work for the driver.

> This is something that VTIO (xhci spec section 4.25) would also need.
> There an endpoint can be handed over to a secondary DMA ID (second,
> new PCI BDF), that the normal xhci driver can be excluded from  with
> iommu.
> 
> VTIO use case is something like trusted VM accessing a secure usb
> storage device, preventing regular OS running the xhci driver in
> another VM from touching it.
> 
> The secure VM needs to allocate and map the ring to this secondary
> PCI BDF

If I understand correctly, that's something like Qubes OS archicture,
where untrusted VMs run drivers to contain any failures inside. Then it
seems we wouldn't want the xhci-hcd VM to have access to transfer rings
to prevent tampering with protected devices.

Hence, no allocation, no initialization. Only opaque pointers, again.

Regards,
Michal

On 8/26/2026 4:44 AM, Mathias Nyman wrote:
> On 8/26/26 13:25, Michal Pecio wrote:
>> On Wed, 26 Aug 2026 00:50:53 -0700, Wesley Cheng wrote:
>>> Thanks for this suggestion.  I think it actually makes the overall
>>> design a lot better.  So now that the sideband driver has its own
>>> segment pool (per sideband instance), we expect that any page
>>> allocations done from this pool is technically owned by the audio
>>> DSP.  This allows us to still utilize 4k ring segments, while mapping
>>> the entire 16k page, so it helps conserve/optimize the memory
>>> allocations.  I will do a bit more testing and review before
>>> submitting a new revision w/ these changes.
>>
>> The part about memory being "owned by the audio DSP" made me wonder
>> if it would be helpful to let offload drivers allocate their own memory
>> and then just dma_map() it for the xHC. No new rings would be allocated
>> for offloaded endpoints when they are enabled, we would point Endpoint
>> Context of the xHC to the sideband ring and leave ep->ring as NULL.
>>
>> Offload drivers would have full control over memory allocation - size,
>> number of segments (it seems that qc-usb-audio only uses one out of two
>> allocated by xhci-hcd), alignment, anything else.
>>
>> It would become impossible to offload an endpoint which is already
>> enabled, but is this an issue for anyone?
>>
>> NULL ep->ring will cause oopses/panics when somebody submits URBs to
>> offloaded endpoints, but I think it wouldn't be a problem otherwise.
>>
> I have similar thoughts.
> 
> One idea would be to basically let sideband allocate the entire ring and set
> ep->new_ring early. This would tell xhci_endpoint_init() that a ring exists 
> and
> a new one should not be allocated.
> 
> xhci ring allocation would need some refactoring to create helpers for 
> sideband
> to allocate and initialize all the other parts of the ring.
> 
> This is something that VTIO (xhci spec section 4.25) would also need.
> There an endpoint can be handed over to a secondary DMA ID (second, new PCI 
> BDF),
> that the normal xhci driver can be excluded from  with iommu.
> 
> VTIO use case is something like trusted VM accessing a secure usb storage 
> device,
> preventing regular OS running the xhci driver in another VM from touching it.
> 
> The secure VM needs to allocate and map the ring to this secondary PCI BDF
> 

Interesting, so in both you're comments, it looks like when USB endpoints 
are offloaded, you want that to be fully isolated from the xHCI layer 
running on the Linux machine/proc.  During the initial USB audio offload 
series submission, I think there was a point where we had a discussion 
where we decided to support both the Linux USB sound path alongside the 
offload path.  This is because applications that are unaware of the offload 
path can still utilize the USB sound PCM devices.

In that situation, we're needing to map the region for both domains, and 
proper ring structures in xHCI, which is the current design.

However, with the current changes I have, it might address some of these 
points.  I'll just give a quick highlight of them:

1. Currently, during xhci_sideband_register() I'm creating a sideband 
segment pool and saving that reference. (if we wanted to adjust this to 
your design, I think we can have the DMA segment pool allocations be 
handled by the offload client driver and passed into xhci-sideband)  I 
think keeping the dma pool design just fits better with the overall xHCI 
ring helpers, and all you need is the device structure associated w/ the 
SID you're trying to map to in the offload driver.

2. xhci_sideband_add_endpoint() will populate the sideband entry for an USB 
endpoint.  This is used as the trigger for which pool to fetch new_ring 
from in xhci_endpoint_init().  This pool will get propagated down to the 
normal xHCI ring segment allocator, and will fetch an entry from the dma 
pool.

I guess the only thing missing is a way to avoid the current xHCI APIs to 
avoid operating on rings that have been offloaded, but as stated earlier, 
at least in the USB audio offload use case, we'd still want the Linux 
environment to be able to operate on the ring.  I'm just trying to see if 
we can come up with a way to accommodate the VTIO situation as well.

Thanks
Wesley Cheng