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

["Alex Bereza" <alex@bereza.email>]

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.

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