Re: [PATCH 5.10 311/491] dmaengine: xilinx: xilinx_dma: Fix unmasked residue subtraction

Marek Vasut
2026-04-16T18:20:15Z

On 4/16/26 7:58 PM, Ben Hutchings wrote:
> On Mon, 2026-04-13 at 17:59 +0200, Greg Kroah-Hartman wrote:
>> 5.10-stable review patch. If anyone has any objections, please let me know.
>>
>> ------------------
>>
>> From: Marek Vasut <marex@nabladev.com>
>>
>> [ Upstream commit c7d812e33f3e8ca0fa9eeabf71d1c7bc3acedc09 ]
>>
>> The segment .control and .status fields both contain top bits which are
>> not part of the buffer size, the buffer size is located only in the bottom
>> max_buffer_len bits. To avoid interference from those top bits, mask out
>> the size using max_buffer_len first, and only then subtract the values.
>
> This change is harmless, but the problem it claims to fix does not
> exist.

The current code subtracts two independently read values which both
contain status/control MSbits and the actual value LSbits. Depending on
the MSbits being identical in both separately read values is unsafe, so
the change in this patch masks out the MSbits first and then does the
subtraction on the actual value LSbits only, which is safe.

Why do you think the original unsafe behavior can not trigger a failure?

> Ben.
>
>> Fixes: a575d0b4e663 ("dmaengine: xilinx_dma: Introduce xilinx_dma_get_residue")
>> Signed-off-by: Marek Vasut <marex@nabladev.com>
>> Link: https://patch.msgid.link/20260316222530.163815-1-marex@nabladev.com
>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>> drivers/dma/xilinx/xilinx_dma.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index ca80a1dee8489..a89a150be3284 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -964,16 +964,16 @@ static u32 xilinx_dma_get_residue(struct xilinx_dma_chan *chan,
>> struct xilinx_cdma_tx_segment,
>> node);
>> cdma_hw = &cdma_seg->hw;
>> - residue += (cdma_hw->control - cdma_hw->status) &
>> - chan->xdev->max_buffer_len;
>> + residue += (cdma_hw->control & chan->xdev->max_buffer_len) -
>> + (cdma_hw->status & chan->xdev->max_buffer_len);
>> } else if (chan->xdev->dma_config->dmatype ==
>> XDMA_TYPE_AXIDMA) {
>> axidma_seg = list_entry(entry,
>> struct xilinx_axidma_tx_segment,
>> node);
>> axidma_hw = &axidma_seg->hw;
>> - residue += (axidma_hw->control - axidma_hw->status) &
>> - chan->xdev->max_buffer_len;
>> + residue += (axidma_hw->control & chan->xdev->max_buffer_len) -
>> + (axidma_hw->status & chan->xdev->max_buffer_len);
>> } else {
>> aximcdma_seg =
>> list_entry(entry,
>> @@ -981,8 +981,8 @@ static u32 xilinx_dma_get_residue(struct xilinx_dma_chan *chan,
>> node);
>> aximcdma_hw = &aximcdma_seg->hw;
>> residue +=
>> - (aximcdma_hw->control - aximcdma_hw->status) &
>> - chan->xdev->max_buffer_len;
>> + (aximcdma_hw->control & chan->xdev->max_buffer_len) -
>> + (aximcdma_hw->status & chan->xdev->max_buffer_len);
>> }
>> }
>> …