Re: [PATCH v5] dmaengine: dw-axi-dmac: report paused state and residue in tx_status

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dmaengine: dw-axi-dmac: stale hardware pointer race on buffer reuse
- [Medium] dmaengine: dw-axi-dmac: incorrect residue for overlapping SG regions
--

commit 51268a3a8963bcf17968d0065986bd47e9b6a2a8
Author: Rui Wang <wr574332525@163.com>
dmaengine: dw-axi-dmac: report paused state and residue in tx_status

The driver implements device_pause/device_resume, but device_tx_status
keeps reporting DMA_IN_PROGRESS for a paused channel. This commit updates
the reporting to handle paused state and calculates actual byte residue.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index eebed2474210d..f0cb3c7956629 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c

[ ... ]

> @@ -352,33 +352,110 @@ static void vchan_desc_put(struct virt_dma_desc *vdesc)
>  	axi_desc_put(vd_to_axi_desc(vdesc));
>  }
>  
> +/*
> + * Read a 64-bit channel address register while the transfer may be
> + * running. The two 32-bit halves cannot be read atomically on all

[ ... ]

> +static u32 axi_chan_get_xferred(struct axi_dma_chan *chan,
> +				struct axi_dma_desc *desc)
> +{
> +	struct axi_dma_hw_desc *hw_desc;
> +	bool dst = chan->direction == DMA_DEV_TO_MEM;
> +	u64 pos, start;
> +	u32 xferred = 0;
> +	int i;
> +
> +	pos = axi_chan_readq(chan, dst ? CH_DAR : CH_SAR);
> +
> +	for (i = 0; i < desc->nr_hw_descs; i++) {
> +		hw_desc = &desc->hw_desc[i];
> +		start = le64_to_cpu(dst ? hw_desc->lli->dar : hw_desc->lli->sar);
> +
> +		/* Current position is inside this block: partial progress */
> +		if (pos >= start && pos <= start + hw_desc->len)
> +			return xferred + (u32)(pos - start);

[Severity: Medium]
Will this logic correctly handle scatter-gather lists that contain
overlapping or repeating physical memory addresses?

If a DMA client submits an SG list using a common memory region for padding
across multiple elements, pos could match an earlier block while the DMA
engine is actually processing a subsequent block reusing the same physical
address.

Could this cause the residue to jump backwards and report an artificially
high remaining byte count to clients that rely on a monotonically
decreasing residue?

> +
> +		xferred += hw_desc->len;
> +	}
> +
> +	/* Position doesn't match any block, be conservative */
> +	return 0;
> +}

[ ... ]

> @@ -468,6 +545,9 @@ static void axi_chan_block_xfer_start(struct axi_dma_chan *chan,
>  	}
>  	axi_chan_config_write(chan, &config);
>  
> +	/* The hardware pointer registers now belong to this descriptor */
> +	chan->desc = first;
> +
>  	write_chan_llp(chan, first->hw_desc[0].llp | lms);
>  
>  	irq_mask = DWAXIDMAC_IRQ_DMA_TRF | DWAXIDMAC_IRQ_ALL_ERR;
>  	axi_chan_irq_sig_set(chan, irq_mask);
>  
>  	/* Generate 'suspend' status but don't generate interrupt */
>  	irq_mask |= DWAXIDMAC_IRQ_SUSPENDED;
>  	axi_chan_irq_set(chan, irq_mask);
>  
>  	axi_chan_enable(chan);
>  }

[Severity: High]
Is there a race condition between setting the descriptor as active and the
hardware actually updating its pointer registers?

If a client reuses a previously completed buffer for a new transfer,
axi_chan_block_xfer_start() assigns chan->desc = first and enables the
channel.

If dma_chan_tx_status() is called to check the status before the DMA engine
fetches the first LLI, it might evaluate a stale hardware pointer.

Since the buffer is identical to the previous run, could the stale pointer
match the end of the new transfer's memory range, causing
axi_chan_get_xferred() to erroneously return the full block length and
report a 0 residue for a newly started transfer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924025739.844-1-wr574332525@163.com?part=1