[PATCH] dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA

[PATCH] dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA

[Alex Bereza]

Using the DMA in cyclic mode modifies the hardware buffer descriptor
chain in xilinx_dma_prep_dma_cyclic so that the last descriptor used by
the cyclic transfer points back to the first descriptor, but it never
restores the original descriptor ring. This breaks using non-cyclic mode
after cyclic mode with an error like:

  xilinx-vdma 86000000.dma: Channel 00000000354d5c8d has errors 100, cdr 6de40000 tdr 6de40400

The only way to get out of this error state is to rebuild the hardware
buffer descriptor ring by releasing and re-acquiring the channel.

Fix using non-cyclic mode after cyclic mode by always restoring the
original buffer descriptor ring in the same manner as it is set up by
xilinx_dma_alloc_chan_resources().

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 | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index bef2b031dba1..6d00aa97c8fc 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -756,15 +756,25 @@ xilinx_aximcdma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 	return segment;
 }
 
-static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
+static void xilinx_dma_clean_hw_desc(struct xilinx_dma_chan *chan,
+				     struct xilinx_axidma_tx_segment *segment)
 {
-	u32 next_desc = hw->next_desc;
-	u32 next_desc_msb = hw->next_desc_msb;
+	dma_addr_t next;
+	u32 i;
 
-	memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
+	/*
+	 * Restore the buffer descriptor's next descriptor pointer to the value
+	 * set up in xilinx_dma_alloc_chan_resources(). Otherwise using the DMA
+	 * in cyclic mode leaves the next descriptor pointer altered and
+	 * prevents subsequent non-cyclic transfers.
+	 */
+	i = (segment->phys - chan->seg_p) / sizeof(*chan->seg_v);
+	next = chan->seg_p +
+	       sizeof(*chan->seg_v) * ((i + 1) % XILINX_DMA_NUM_DESCS);
 
-	hw->next_desc = next_desc;
-	hw->next_desc_msb = next_desc_msb;
+	memset(&segment->hw, 0, sizeof(segment->hw));
+	segment->hw.next_desc = lower_32_bits(next);
+	segment->hw.next_desc_msb = upper_32_bits(next);
 }
 
 static void xilinx_mcdma_clean_hw_desc(struct xilinx_aximcdma_desc_hw *hw)
@@ -786,7 +796,7 @@ static void xilinx_mcdma_clean_hw_desc(struct xilinx_aximcdma_desc_hw *hw)
 static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
 				struct xilinx_axidma_tx_segment *segment)
 {
-	xilinx_dma_clean_hw_desc(&segment->hw);
+	xilinx_dma_clean_hw_desc(chan, segment);
 
 	list_add_tail(&segment->node, &chan->free_seg_list);
 }

---
base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
change-id: 20260817-fix-hw-buf-desc-after-cyclic-mode-cf0a7834ab50

Best regards,
--  
Alex Bereza <alex@bereza.email>

Re: [PATCH] dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Fragile 64-bit division on `dma_addr_t` using `sizeof`.

Pre-existing issues:
- [Critical] Hardware descriptor ring desynchronization due to relying on `free_seg_list` ordering mapping exactly to physical continuity.
- [High] Unlocked modification of `chan->free_seg_list` in error paths and submission rejection paths causes list corruption.
--

--- Patch [1]: [PATCH] dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA ---
commit df3c4c6392659223fe0433932cd3afd45fdb012e
Author: Alex Bereza <alex@bereza.email>

dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA

Using the DMA in cyclic mode modifies the hardware buffer descriptor chain
so that the last descriptor points back to the first. This patch ensures
non-cyclic mode works after cyclic mode by restoring the original buffer
descriptor ring setup during segment cleanup.

> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index bef2b031dba19..6d00aa97c8fc9 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -756,15 +756,25 @@ xilinx_aximcdma_alloc_tx_segment(struct xilinx_dma_chan *chan)

[ ... ]

> -static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
> +static void xilinx_dma_clean_hw_desc(struct xilinx_dma_chan *chan,
> +				     struct xilinx_axidma_tx_segment *segment)
>  {
> -	u32 next_desc = hw->next_desc;
> -	u32 next_desc_msb = hw->next_desc_msb;
> +	dma_addr_t next;
> +	u32 i;
>  
> -	memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
> +	/*
> +	 * Restore the buffer descriptor's next descriptor pointer to the value
> +	 * set up in xilinx_dma_alloc_chan_resources(). Otherwise using the DMA
> +	 * in cyclic mode leaves the next descriptor pointer altered and
> +	 * prevents subsequent non-cyclic transfers.
> +	 */
> +	i = (segment->phys - chan->seg_p) / sizeof(*chan->seg_v);

[Severity: Low]
Is this calculation relying on compiler optimization to avoid an unresolved
__udivdi3 build failure on 32-bit platforms?

Because segment->phys and chan->seg_p are dma_addr_t, which can be 64-bit
on 32-bit platforms with LPAE, this creates a 64-bit division. It currently
works because sizeof(*chan->seg_v) happens to be exactly 128 bytes due to
__aligned(64), allowing the compiler to optimize it into a bitwise right
shift.

If a field is added that pushes the alignment boundary to a non-power-of-2,
would this break 32-bit builds? Could we use native pointer arithmetic like
i = segment - chan->seg_v instead?

> +	next = chan->seg_p +
> +	       sizeof(*chan->seg_v) * ((i + 1) % XILINX_DMA_NUM_DESCS);
>  
> -	hw->next_desc = next_desc;
> -	hw->next_desc_msb = next_desc_msb;
> +	memset(&segment->hw, 0, sizeof(segment->hw));
> +	segment->hw.next_desc = lower_32_bits(next);
> +	segment->hw.next_desc_msb = upper_32_bits(next);
>  }
>  
>  static void xilinx_mcdma_clean_hw_desc(struct xilinx_aximcdma_desc_hw *hw)

[ ... ]

> @@ -786,7 +796,7 @@ static void xilinx_mcdma_clean_hw_desc(struct xilinx_aximcdma_desc_hw *hw)
>  static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
>  				struct xilinx_axidma_tx_segment *segment)
>  {
> -	xilinx_dma_clean_hw_desc(&segment->hw);
> +	xilinx_dma_clean_hw_desc(chan, segment);
>  
>  	list_add_tail(&segment->node, &chan->free_seg_list);

[Severity: High]
This is a pre-existing issue, but could this unlocked modification of
chan->free_seg_list lead to list corruption?

In error paths like xilinx_dma_prep_slave_sg() on allocation failure, or
when xilinx_dma_tx_submit() rejects a submission,
xilinx_dma_free_tx_descriptor() is called locklessly. This invokes
xilinx_dma_free_tx_segment() which calls list_add_tail() on
chan->free_seg_list here.

If the interrupt tasklet concurrently calls xilinx_axidma_alloc_tx_segment()
and modifies the list while holding chan->lock, couldn't this cause
linked-list corruption and kernel panics?

[Severity: Critical]
This is a pre-existing issue, but will relying on free_seg_list ordering
cause hardware descriptor ring desynchronization?

In xilinx_dma_prep_slave_sg(), segments are allocated and queued:

drivers/dma/xilinx/xilinx_dma.c:xilinx_dma_prep_slave_sg() {
    ...
    segment = xilinx_axidma_alloc_tx_segment(chan);
    ...
    hw = &segment->hw;
    ...
    list_add_tail(&segment->node, &desc->segments);
    ...
}

This never dynamically updates hw->next_desc to link the transaction's
segments together. If the free_seg_list becomes out of order (for example,
due to xilinx_dma_tx_submit() rejecting and freeing segments), the segments
popped will not be physically contiguous.

Since hw->next_desc isn't updated, won't the hardware blindly follow the
statically-initialized next_desc pointers into unrelated physical segments,
potentially corrupting other active transactions?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-fix-hw-buf-desc-after-cyclic-mode-v1-1-1fe47e701d6c@bereza.email?part=1

Re: [PATCH] dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA

[Frank Li]

On Mon, Aug 17, 2026 at 03:36:52PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Low] Fragile 64-bit division on `dma_addr_t` using `sizeof`.
>
> Pre-existing issues:
> - [Critical] Hardware descriptor ring desynchronization due to relying on `free_seg_list` ordering mapping exactly to physical continuity.
> - [High] Unlocked modification of `chan->free_seg_list` in error paths and submission rejection paths causes list corruption.
> --
>
> --- Patch [1]: [PATCH] dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA ---
> commit df3c4c6392659223fe0433932cd3afd45fdb012e
> Author: Alex Bereza <alex@bereza.email>
>
> dmaengine: xilinx_dma: Fix hardware buffer descriptor chain after cyclic DMA
>
> Using the DMA in cyclic mode modifies the hardware buffer descriptor chain
> so that the last descriptor points back to the first. This patch ensures
> non-cyclic mode works after cyclic mode by restoring the original buffer
> descriptor ring setup during segment cleanup.
>
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > index bef2b031dba19..6d00aa97c8fc9 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -756,15 +756,25 @@ xilinx_aximcdma_alloc_tx_segment(struct xilinx_dma_chan *chan)
>
> [ ... ]
>
> > -static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
> > +static void xilinx_dma_clean_hw_desc(struct xilinx_dma_chan *chan,
> > +				     struct xilinx_axidma_tx_segment *segment)
> >  {
> > -	u32 next_desc = hw->next_desc;
> > -	u32 next_desc_msb = hw->next_desc_msb;
> > +	dma_addr_t next;
> > +	u32 i;
> >
> > -	memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
> > +	/*
> > +	 * Restore the buffer descriptor's next descriptor pointer to the value
> > +	 * set up in xilinx_dma_alloc_chan_resources(). Otherwise using the DMA
> > +	 * in cyclic mode leaves the next descriptor pointer altered and
> > +	 * prevents subsequent non-cyclic transfers.
> > +	 */
> > +	i = (segment->phys - chan->seg_p) / sizeof(*chan->seg_v);
>
> [Severity: Low]
> Is this calculation relying on compiler optimization to avoid an unresolved
> __udivdi3 build failure on 32-bit platforms?
>
> Because segment->phys and chan->seg_p are dma_addr_t, which can be 64-bit
> on 32-bit platforms with LPAE, this creates a 64-bit division. It currently
> works because sizeof(*chan->seg_v) happens to be exactly 128 bytes due to
> __aligned(64), allowing the compiler to optimize it into a bitwise right
> shift.
>
> If a field is added that pushes the alignment boundary to a non-power-of-2,
> would this break 32-bit builds? Could we use native pointer arithmetic like
> i = segment - chan->seg_v instead?

look good by this way.

Frank

>
> > +	next = chan->seg_p +
> > +	       sizeof(*chan->seg_v) * ((i + 1) % XILINX_DMA_NUM_DESCS);
> >
> > -	hw->next_desc = next_desc;
> > -	hw->next_desc_msb = next_desc_msb;
> > +	memset(&segment->hw, 0, sizeof(segment->hw));
> > +	segment->hw.next_desc = lower_32_bits(next);
> > +	segment->hw.next_desc_msb = upper_32_bits(next);
> >  }
> >
> >  static void xilinx_mcdma_clean_hw_desc(struct xilinx_aximcdma_desc_hw *hw)
>
> [ ... ]
>
> > @@ -786,7 +796,7 @@ static void xilinx_mcdma_clean_hw_desc(struct xilinx_aximcdma_desc_hw *hw)
> >  static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
> >  				struct xilinx_axidma_tx_segment *segment)
> >  {
> > -	xilinx_dma_clean_hw_desc(&segment->hw);
> > +	xilinx_dma_clean_hw_desc(chan, segment);
> >
> >  	list_add_tail(&segment->node, &chan->free_seg_list);
>
> [Severity: High]
> This is a pre-existing issue, but could this unlocked modification of
> chan->free_seg_list lead to list corruption?
>
> In error paths like xilinx_dma_prep_slave_sg() on allocation failure, or
> when xilinx_dma_tx_submit() rejects a submission,
> xilinx_dma_free_tx_descriptor() is called locklessly. This invokes
> xilinx_dma_free_tx_segment() which calls list_add_tail() on
> chan->free_seg_list here.
>
> If the interrupt tasklet concurrently calls xilinx_axidma_alloc_tx_segment()
> and modifies the list while holding chan->lock, couldn't this cause
> linked-list corruption and kernel panics?
>
> [Severity: Critical]
> This is a pre-existing issue, but will relying on free_seg_list ordering
> cause hardware descriptor ring desynchronization?
>
> In xilinx_dma_prep_slave_sg(), segments are allocated and queued:
>
> drivers/dma/xilinx/xilinx_dma.c:xilinx_dma_prep_slave_sg() {
>     ...
>     segment = xilinx_axidma_alloc_tx_segment(chan);
>     ...
>     hw = &segment->hw;
>     ...
>     list_add_tail(&segment->node, &desc->segments);
>     ...
> }
>
> This never dynamically updates hw->next_desc to link the transaction's
> segments together. If the free_seg_list becomes out of order (for example,
> due to xilinx_dma_tx_submit() rejecting and freeing segments), the segments
> popped will not be physically contiguous.
>
> Since hw->next_desc isn't updated, won't the hardware blindly follow the
> statically-initialized next_desc pointers into unrelated physical segments,
> potentially corrupting other active transactions?
>
> >  }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260817-fix-hw-buf-desc-after-cyclic-mode-v1-1-1fe47e701d6c@bereza.email?part=1