Re: [PATCH] dmaengine: xilinx_dma: Fix hardware buffer descriptor reuse order

["Gupta, Suraj" <suraj.gupta2@amd.com>]

On 8/18/2026 5:41 PM, Alex Bereza wrote:
> On Tue Aug 18, 2026 at 12:58 PM CEST, Radhey Shyam Pandey wrote:
>>
>> + Suraj
>>
>> On 8/17/2026 2:53 PM, Alex Bereza wrote:
>>> xilinx_dma_alloc_chan_resources() builds a static ring of hardware
>>> buffer descriptors once and the driver uses this ring throughout the
>>> lifetime of a channel. This requires the allocation order of hardware
>>> buffer descriptors from chan->free_seg_list to stay in sync with the
>>> hardware buffer descriptor ring built at channel allocation time by
>>> returning oldest descriptors to chan->free_seg_list first.
>>>
>>> When chan->pending_list is not empty e.g. during
>>> xilinx_dma_terminate_all() the chan->free_seg_list and the order of the
>>> static hardware buffer descriptor ring get out of sync. Descriptors age
>>> in this order: pending -> active -> done. So freeing pending_list first
>>> returns the newest buffer descriptors to the chan->free_seg_list first
>>> and thus breaks the order required by the static hardware buffer
>>> descriptor ring. Then when the channel is reused, after a wrap around of
>>> the free_seg_list the DMA will find a hardware buffer descriptor with a
>>> length field that is still zeroed and stop with something like this:
>>>
>>>     xilinx-vdma 86000000.dma: Channel 000000003a21d7b8 has errors 10, cdr 6de4c000 tdr 6de4c000
>>>
>>
>> Just to understand - can you share steps on how are seeing this issue ?
>> During terminate_all hw is in reset so how is free ordering matter?
>> Want to ensure that change of order in not masking a real corruption.
> 
> To reproduce, pending_list must not be empty when
> xilinx_dma_terminate_all() is called. For example by calling
> dmaengine_submit without calling dma_async_issue_pending and then
> terminating.
> 
> Since pending_list was freed first, the newest descriptors were appended
> to the tail of the free_seg_list. Then when you continue to use the
> channel and allocate new descriptors from free_seg_list you will
> eventually cause the hardware to walk into a buffer descriptor with
> length 0. This happens after a lap of free_seg_list, when you get the
> descriptor freed from the pending_list previously. The hardware buffer
> descriptor ring reads a different descriptor than what you "allocated"
> from free_seg_list and fails, since the read descriptor's length field
> is still 0. This is because the hardware ring order is different from
> the order in free_seg_list, caused by the wrong freeing order.
> 
> The reset does not help to prevent this, because the hardware advances
> along the hardware buffer descriptor ring via the next pointer. But the
> driver hands out the descriptors in free_seg_list order, which no longer
> matches hardware buffer descriptor ring order. The reset only re-syncs
> the starting point, but not the order of the two.
> 

Looks fine to me.

Reviewed-by: Suraj Gupta <suraj.gupta2@amd.com>

Thanks,
Suraj
>>
>>> After this no more descriptors are completed and a consumer potentially
>>> blocks and waits forever. The only way to get out of this error state is
>>> to rebuild the static hardware buffer descriptor ring and the
>>> free_seg_list by releasing and re-acquiring the channel.
>>>
>>> Fix the order in which hardware buffer descriptors are returned to
>>> free_seg_list to ensure the mentioned requirement holds.
>>>
>>> Fixes: 23059408b6a3 ("dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario")
>>> Signed-off-by: Alex Bereza <alex@bereza.email>
>>> ---
>>>    drivers/dma/xilinx/xilinx_dma.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>>> index bef2b031dba1..0817b74f7450 100644
>>> --- a/drivers/dma/xilinx/xilinx_dma.c
>>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>>> @@ -920,9 +920,9 @@ static void xilinx_dma_free_descriptors(struct xilinx_dma_chan *chan)
>>>
>>>    	spin_lock_irqsave(&chan->lock, flags);
>>>
>>> -	xilinx_dma_free_desc_list(chan, &chan->pending_list);
>>>    	xilinx_dma_free_desc_list(chan, &chan->done_list);
>>>    	xilinx_dma_free_desc_list(chan, &chan->active_list);
>>> +	xilinx_dma_free_desc_list(chan, &chan->pending_list);
>>>
>>>    	spin_unlock_irqrestore(&chan->lock, flags);
>>>    }
>>>
>>> ---
>>> base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
>>> change-id: 20260817-fix-hw-buf-desc-reuse-b730e9e02185
>>>
>>> Best regards,
>>> --
>>> Alex Bereza <alex@bereza.email>
>>>
>>>