Re: [PATCH V3 4/4] dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA

["Neeli, Srinivas" <srneeli@amd.com>]

Hi,

On 7/9/2026 3:41 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Modifying `chan->free_seg_list` without holding `chan->lock` causes linked list corruption.
> --
>
> --- Patch [4]: [PATCH V3 4/4] dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA ---
> commit 7c1e9fcfa16f54b84c1fd0328dc790510cde97be
> Author: Suraj Gupta <suraj.gupta2@amd.com>
>
> dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA
>      
> This commit extends metadata handling for AXI DMA and MCDMA to expose
> status and sideband information. The metadata pointer now starts at the
> descriptor status word instead of the APP fields, and probe logic is
> updated to attach metadata_ops when an AXI Stream interface is present.
>
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index 9b8b6f1acd4ea..03c45369086b8 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
> [ ... ]
>
>> @@ -2623,6 +2653,9 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>>   		segment->hw.control |= XILINX_MCDMA_BD_EOP;
>>   	}
>>   
>> +	if (chan->xdev->has_axistream_connected)
>> +		desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;
>> +
>>   	return &desc->async_tx;
>>   
>>   error:
>>   	xilinx_dma_free_tx_descriptor(chan, desc);
> [Severity: High]
> This isn't a bug introduced by this patch, but does this error path corrupt
> the channel's free segment list?
>
> When xilinx_mcdma_prep_slave_sg() encounters an error, it calls
> xilinx_dma_free_tx_descriptor() without holding chan->lock. This eventually
> cascades to xilinx_mcdma_free_tx_segment() and xilinx_dma_free_tx_segment(),
> which add the segment back to chan->free_seg_list:
>
> xilinx_mcdma_free_tx_segment()
>      list_add_tail(&segment->node, &chan->free_seg_list);
>
> If another CPU core concurrently calls xilinx_aximcdma_alloc_tx_segment(),
> which acquires chan->lock to remove items from this same list, could this
> unprotected modification lead to linked list corruption?
>
> This unprotected error path also appears to be present in
> xilinx_dma_tx_submit() when chan->cyclic is true.
The locking issue with free_seg_list modification is a valid 
pre-existing concern, not introduced by this patch.

   I'll address this in a separate patch series along with the other 
pre-existing issues identified in this review.

   Thanks,
   Neeli Srinivas