Re: [PATCH v4 3/5] usb: xhci: sideband: allocate sideband ring segments from a dedicated pool

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

On 10/20/2026 4:13 PM, Wesley Cheng wrote:
> 
> 
> On 10/14/2026 5:44 PM, Wesley Cheng wrote:
>>
>>
>> On 10/14/2026 7:26 AM, Mathias Nyman wrote:
>>> On 10/13/26 12:11, Michal Pecio wrote:
>>>> On Mon, 12 Oct 2026 18:01:50 -0700, Wesley Cheng wrote:
>>>>> On 10/5/2026 4:03 AM, Mathias Nyman wrote:
>>>>>> xhci is the one allocating the ring when adding an endpoint, and
>>>>>> telling xHC hardware to write to that ring. It seems more
>>>>>> approriate that xhci should be the one freeing the ring after it
>>>>>> told xHC hardware to drop the endpoint, and there no longer is a
>>>>>> risk on hardware accessing it.
>>>>>
>>>>> Hi Mathias,
>>>>>
>>>>> Sorry for the late response.  Was out last week.
>>>>>
>>>>> Specifically in the disconnect path (while audio offload is active),
>>>>> when we utilize the usb_set_interface() path, it'll return early due
>>>>> to the iface->unregistering check.  This would lead to the
>>>>> drop_endpoint not to be called, and rely on the xhci_free_dev() to
>>>>> handle the transfer ring cleanup.  This wouldn't work in the offload
>>>>> situation, because qc_audio_offload is the only entity managing the
>>>>> DMA pool, and that would be freed during the .disconnect callback,
>>>>> which happens BEFORE xhci_free_dev().
>>>
>>> You are right, audio class driver .disconnect is called, driver unbound
>>> and interface device gone before xhci_free_dev() is called.
>>>
>>> If sideband is unregistered as part of .disconnect callback it also means
>>> ring/pool would be destroyed before xHC has dropped the endpoint/disabled
>>> the slot. Would be better to free the ring after we know xHC controller
>>> won't access it.
>>>
>>>>>
>>>>> Would you be more inclined to prefer a solution where we can defer
>>>>> the DMA pool free'ing to the xHCI core during xhci_free_dev, or maybe
>>>>> adding another sideband notifier (sb->notify_client) enum to tell the
>>>>> client driver that it can release the pool?
>>>
>>> sb->notify_client callback has the risk that this sideband entity is
>>> unregistered and gone by the time we want to call it
>>>
>>> Deferring freeing the DMA poolby some kind of reference counting again adds
>>> complexity, and maybe not a suitable solution for this issue.
>>> (taking a reference at sideband register and every dma_pool_zalloc(), 
>>> putting
>>> when freeing or sideband unregister, finally freeing pool when reaching 
>>> zero.)
>>>
>>> I'm trying to figure out a simpler solution.
>>>
>>> How about skipping dma pools completely for endpoints in these cases?
>>> (page size over 4k with offloaded endpoints)
>>>
>>> Instead of sideband client creating a custom dma pool it could just set a
>>> 'no dma pool' flag for the endpoint. xhci driver would allocate the
>>> ring segments using dma_alloc_coherent() instead of dma_pool_zalloc() for
>>> these, possibly even allocate a full page for each segment?
>>> Freeing would be done similarly.
>>>
>>> Would this be a viable simple solution?
>>>
> 
> Hi Mathias,
> 
> There is one difficulty I ran into, and that's w/ dma_free_coherent() not 
> being able to be done in the current xhci_ring_free() context, as it is 
> under atomic context.  I could move the dma memory ownership from xhci core 
> to xhci sideband, which could probably get around this.
> 
> xHCI sideband will allocate the DMA backed memory then populate the 
> new_ring reference.  Then when xhci_add_endpoint() --> xhci_endpoint_init() 
> is run it'll check if new_ring is present, if not, follow the non offload 
> path and allocate from xHCI segment pool.  Then on the sideband unregister 
> path, we can handle the freeing of this DMA memory as well.  This kind of 
> moves the memory operations from xHCI to xHCI sideband.  We would need to 
> have a set of sideband specific ring alloc/free though.
> 
Hi Mathias,

I'm going to go ahead and start working on this approach.  Feel free to 
comment on if you have any major concerns from your end.  From what I've 
looked at so far, I think it is a feasible solution, and the only major 
shift is that xHCI sideband will own some of the memory allocations that 
will populate xHCI core references.

Thanks
Wesley Cheng