DMA Engine development
 help / color / mirror / Atom feed
* [PATCH v4 0/4] dmaengine: dma-axi-dmac: Some memory related fixes
@ 2026-04-24 17:40 Nuno Sá via B4 Relay
  2026-04-24 17:40 ` [PATCH v4 1/4] dmaengine: Fix possible use after free Nuno Sá via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Nuno Sá via B4 Relay @ 2026-04-24 17:40 UTC (permalink / raw)
  To: dmaengine, linux-kernel; +Cc: Lars-Peter Clausen, Vinod Koul, Frank Li

Here it goes v3. Given there are some discussions about adding devlinks
in damengine, I dropped the patch implementing the .device_release() for
now.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
Changes in v4:
- Patch 1:
  * Commit reword as per Frank suggestion
- Patch 2:
  * Commit reword as per Frank suggestion
- Patch 3:
  * Commit reword as per Frank suggestion
- Patch 4:
  * Commit subjectreword as per Frank suggestion
  * Added Fixes tag
- Link to v3: https://patch.msgid.link/20260408-dma-dmac-handle-vunmap-v3-0-2456ad292154@analog.com

---
Nuno Sá (4):
      dmaengine: Fix possible use after free
      dmaengine: dma-axi-dmac: Properly free struct axi_dmac_desc
      dmaengine: dma-axi-dmac: Drop struct clk from main struct
      dmaengine: dma-axi-dmac: use DMA pool to manange DMA descriptor

 drivers/dma/dma-axi-dmac.c | 77 +++++++++++++++++++++++++++-------------------
 drivers/dma/dmaengine.c    |  3 +-
 2 files changed, 47 insertions(+), 33 deletions(-)
---
base-commit: b7560798466a07d9c3fb011698e92c335ab28baf
change-id: 20260325-dma-dmac-handle-vunmap-84a06df7d133
--

Thanks!
- Nuno Sá



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4 1/4] dmaengine: Fix possible use after free
  2026-04-24 17:40 [PATCH v4 0/4] dmaengine: dma-axi-dmac: Some memory related fixes Nuno Sá via B4 Relay
@ 2026-04-24 17:40 ` Nuno Sá via B4 Relay
  2026-05-07 18:33   ` Frank Li
  2026-04-24 17:40 ` [PATCH v4 2/4] dmaengine: dma-axi-dmac: Properly free struct axi_dmac_desc Nuno Sá via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Nuno Sá via B4 Relay @ 2026-04-24 17:40 UTC (permalink / raw)
  To: dmaengine, linux-kernel; +Cc: Lars-Peter Clausen, Vinod Koul, Frank Li

From: Nuno Sá <nuno.sa@analog.com>

In dma_release_channel(), check chan->device->privatecnt after call
dma_chan_put(). However, dma_chan_put() call dma_device_put() which could
release the last reference of the device if the DMA provider is already
gone and hence free it.

Fixes it by moving dma_chan_put() after the check.

Fixes: 0f571515c332 ("dmaengine: Add privatecnt to revert DMA_PRIVATE property")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/dma/dmaengine.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 405bd2fbb4a3..9049171df857 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -905,11 +905,12 @@ void dma_release_channel(struct dma_chan *chan)
 	mutex_lock(&dma_list_mutex);
 	WARN_ONCE(chan->client_count != 1,
 		  "chan reference count %d != 1\n", chan->client_count);
-	dma_chan_put(chan);
 	/* drop PRIVATE cap enabled by __dma_request_channel() */
 	if (--chan->device->privatecnt == 0)
 		dma_cap_clear(DMA_PRIVATE, chan->device->cap_mask);
 
+	dma_chan_put(chan);
+
 	if (chan->slave) {
 		sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME);
 		sysfs_remove_link(&chan->slave->kobj, chan->name);

-- 
2.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 2/4] dmaengine: dma-axi-dmac: Properly free struct axi_dmac_desc
  2026-04-24 17:40 [PATCH v4 0/4] dmaengine: dma-axi-dmac: Some memory related fixes Nuno Sá via B4 Relay
  2026-04-24 17:40 ` [PATCH v4 1/4] dmaengine: Fix possible use after free Nuno Sá via B4 Relay
@ 2026-04-24 17:40 ` Nuno Sá via B4 Relay
  2026-05-07 18:33   ` Frank Li
  2026-04-24 17:40 ` [PATCH v4 3/4] dmaengine: dma-axi-dmac: Drop struct clk from main struct Nuno Sá via B4 Relay
  2026-04-24 17:40 ` [PATCH v4 4/4] dmaengine: dma-axi-dmac: use DMA pool to manange DMA descriptor Nuno Sá via B4 Relay
  3 siblings, 1 reply; 9+ messages in thread
From: Nuno Sá via B4 Relay @ 2026-04-24 17:40 UTC (permalink / raw)
  To: dmaengine, linux-kernel; +Cc: Lars-Peter Clausen, Vinod Koul, Frank Li

From: Nuno Sá <nuno.sa@analog.com>

Use axi_dmac_free_desc() to free fully the descriptor at fail path when
call axi_dmac_alloc_desc() in axi_dmac_prep_peripheral_dma_vec().

Fixes: 74609e568670 ("dmaengine: dma-axi-dmac: Implement device_prep_peripheral_dma_vec")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/dma/dma-axi-dmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index 45c2c8e4bc45..127c3cf80a0e 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -769,7 +769,7 @@ axi_dmac_prep_peripheral_dma_vec(struct dma_chan *c, const struct dma_vec *vecs,
 	for (i = 0; i < nb; i++) {
 		if (!axi_dmac_check_addr(chan, vecs[i].addr) ||
 		    !axi_dmac_check_len(chan, vecs[i].len)) {
-			kfree(desc);
+			axi_dmac_free_desc(desc);
 			return NULL;
 		}
 

-- 
2.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 3/4] dmaengine: dma-axi-dmac: Drop struct clk from main struct
  2026-04-24 17:40 [PATCH v4 0/4] dmaengine: dma-axi-dmac: Some memory related fixes Nuno Sá via B4 Relay
  2026-04-24 17:40 ` [PATCH v4 1/4] dmaengine: Fix possible use after free Nuno Sá via B4 Relay
  2026-04-24 17:40 ` [PATCH v4 2/4] dmaengine: dma-axi-dmac: Properly free struct axi_dmac_desc Nuno Sá via B4 Relay
@ 2026-04-24 17:40 ` Nuno Sá via B4 Relay
  2026-05-07 18:34   ` Frank Li
  2026-04-24 17:40 ` [PATCH v4 4/4] dmaengine: dma-axi-dmac: use DMA pool to manange DMA descriptor Nuno Sá via B4 Relay
  3 siblings, 1 reply; 9+ messages in thread
From: Nuno Sá via B4 Relay @ 2026-04-24 17:40 UTC (permalink / raw)
  To: dmaengine, linux-kernel; +Cc: Lars-Peter Clausen, Vinod Koul, Frank Li

From: Nuno Sá <nuno.sa@analog.com>

There's no reason to keep struct clk in struct axi_dmac. Hence, use a
local clk variable in .probe() and drop it from struct axi_dmac.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/dma/dma-axi-dmac.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index 127c3cf80a0e..41898d594be7 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -170,8 +170,6 @@ struct axi_dmac {
 	void __iomem *base;
 	int irq;
 
-	struct clk *clk;
-
 	struct dma_device dma_dev;
 	struct axi_dmac_chan chan;
 };
@@ -1198,6 +1196,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
 {
 	struct dma_device *dma_dev;
 	struct axi_dmac *dmac;
+	struct clk *clk;
 	struct regmap *regmap;
 	unsigned int version;
 	u32 irq_mask = 0;
@@ -1217,9 +1216,9 @@ static int axi_dmac_probe(struct platform_device *pdev)
 	if (IS_ERR(dmac->base))
 		return PTR_ERR(dmac->base);
 
-	dmac->clk = devm_clk_get_enabled(&pdev->dev, NULL);
-	if (IS_ERR(dmac->clk))
-		return PTR_ERR(dmac->clk);
+	clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
 
 	version = axi_dmac_read(dmac, ADI_AXI_REG_VERSION);
 

-- 
2.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 4/4] dmaengine: dma-axi-dmac: use DMA pool to manange DMA descriptor
  2026-04-24 17:40 [PATCH v4 0/4] dmaengine: dma-axi-dmac: Some memory related fixes Nuno Sá via B4 Relay
                   ` (2 preceding siblings ...)
  2026-04-24 17:40 ` [PATCH v4 3/4] dmaengine: dma-axi-dmac: Drop struct clk from main struct Nuno Sá via B4 Relay
@ 2026-04-24 17:40 ` Nuno Sá via B4 Relay
  2026-05-07 18:37   ` Frank Li
  3 siblings, 1 reply; 9+ messages in thread
From: Nuno Sá via B4 Relay @ 2026-04-24 17:40 UTC (permalink / raw)
  To: dmaengine, linux-kernel; +Cc: Lars-Peter Clausen, Vinod Koul, Frank Li

From: Nuno Sá <nuno.sa@analog.com>

For architectures like Microblaze or arm64 (where this IP is used),
DMA_DIRECT_REMAP is set which means that dma_alloc_coherent() might
remap (and hence vmalloc()) some memory. This became visible in a design
where dma_direct_use_pool() is not possible.

With the above, when calling dma_free_coherent(), vunmap() would be
called from softirq context and thus leading to a BUG().

To fix it, use a dma pool that is allocated in
.device_alloc_chan_resources() and allocate blocks from it. The key
point is that now dma_pool_free() is used in axi_dmac_free_desc() to
free the blocks and that just frees the blocks from the pool in the
sense they can be used again. In other words, no actual call to
dma_free_coherent() happens. That only happens when destroying the pool
in axi_dmac_free_chan_resources() which does not happen in any interrupt
context.

Fixes: 3f8fd25936ee ("dmaengine: axi-dmac: Allocate hardware descriptors")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/dma/dma-axi-dmac.c | 66 ++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index 41898d594be7..d47ff27e1408 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -13,6 +13,7 @@
 #include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
+#include <linux/dmapool.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -147,6 +148,7 @@ struct axi_dmac_chan {
 	struct virt_dma_chan vchan;
 
 	struct axi_dmac_desc *next_desc;
+	void *pool;
 	struct list_head active_descs;
 	enum dma_transfer_direction direction;
 
@@ -648,11 +650,17 @@ static void axi_dmac_issue_pending(struct dma_chan *c)
 	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 }
 
+static void axi_dmac_free_desc(struct axi_dmac_desc *desc)
+{
+	for (unsigned int i = 0; i < desc->num_sgs; i++)
+		dma_pool_free(desc->chan->pool, desc->sg[i].hw, desc->sg[i].hw_phys);
+
+	kfree(desc);
+}
+
 static struct axi_dmac_desc *
 axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
 {
-	struct axi_dmac *dmac = chan_to_axi_dmac(chan);
-	struct device *dev = dmac->dma_dev.dev;
 	struct axi_dmac_hw_desc *hws;
 	struct axi_dmac_desc *desc;
 	dma_addr_t hw_phys;
@@ -664,22 +672,22 @@ axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
 	desc->num_sgs = num_sgs;
 	desc->chan = chan;
 
-	hws = dma_alloc_coherent(dev, PAGE_ALIGN(num_sgs * sizeof(*hws)),
-				&hw_phys, GFP_ATOMIC);
-	if (!hws) {
-		kfree(desc);
-		return NULL;
-	}
-
 	for (i = 0; i < num_sgs; i++) {
-		desc->sg[i].hw = &hws[i];
-		desc->sg[i].hw_phys = hw_phys + i * sizeof(*hws);
+		hws = dma_pool_zalloc(chan->pool, GFP_NOWAIT, &hw_phys);
+		if (!hws) {
+			desc->num_sgs = i;
+			axi_dmac_free_desc(desc);
+			return NULL;
+		}
 
-		hws[i].id = AXI_DMAC_SG_UNUSED;
-		hws[i].flags = 0;
+		desc->sg[i].hw = hws;
+		desc->sg[i].hw_phys = hw_phys;
+
+		hws->id = AXI_DMAC_SG_UNUSED;
 
 		/* Link hardware descriptors */
-		hws[i].next_sg_addr = hw_phys + (i + 1) * sizeof(*hws);
+		if (i)
+			desc->sg[i - 1].hw->next_sg_addr = hw_phys;
 	}
 
 	/* The last hardware descriptor will trigger an interrupt */
@@ -688,18 +696,6 @@ axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
 	return desc;
 }
 
-static void axi_dmac_free_desc(struct axi_dmac_desc *desc)
-{
-	struct axi_dmac *dmac = chan_to_axi_dmac(desc->chan);
-	struct device *dev = dmac->dma_dev.dev;
-	struct axi_dmac_hw_desc *hw = desc->sg[0].hw;
-	dma_addr_t hw_phys = desc->sg[0].hw_phys;
-
-	dma_free_coherent(dev, PAGE_ALIGN(desc->num_sgs * sizeof(*hw)),
-			  hw, hw_phys);
-	kfree(desc);
-}
-
 static struct axi_dmac_sg *axi_dmac_fill_linear_sg(struct axi_dmac_chan *chan,
 	enum dma_transfer_direction direction, dma_addr_t addr,
 	unsigned int num_periods, unsigned int period_len,
@@ -933,9 +929,26 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_interleaved(
 	return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
 }
 
+static int axi_dmac_alloc_chan_resources(struct dma_chan *c)
+{
+	struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
+	struct device *dev = c->device->dev;
+
+	chan->pool = dma_pool_create(dev_name(dev), dev,
+				     sizeof(struct axi_dmac_hw_desc),
+				     __alignof__(struct axi_dmac_hw_desc), 0);
+	if (!chan->pool)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static void axi_dmac_free_chan_resources(struct dma_chan *c)
 {
+	struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
+
 	vchan_free_chan_resources(to_virt_chan(c));
+	dma_pool_destroy(chan->pool);
 }
 
 static void axi_dmac_desc_free(struct virt_dma_desc *vdesc)
@@ -1238,6 +1251,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
 	dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
 	dma_cap_set(DMA_CYCLIC, dma_dev->cap_mask);
 	dma_cap_set(DMA_INTERLEAVE, dma_dev->cap_mask);
+	dma_dev->device_alloc_chan_resources = axi_dmac_alloc_chan_resources;
 	dma_dev->device_free_chan_resources = axi_dmac_free_chan_resources;
 	dma_dev->device_tx_status = dma_cookie_status;
 	dma_dev->device_issue_pending = axi_dmac_issue_pending;

-- 
2.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/4] dmaengine: Fix possible use after free
  2026-04-24 17:40 ` [PATCH v4 1/4] dmaengine: Fix possible use after free Nuno Sá via B4 Relay
@ 2026-05-07 18:33   ` Frank Li
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Li @ 2026-05-07 18:33 UTC (permalink / raw)
  To: Nuno Sá
  Cc: dmaengine, linux-kernel, Lars-Peter Clausen, Vinod Koul, Frank Li

On Fri, Apr 24, 2026 at 06:40:14PM +0100, Nuno Sá wrote:
> In dma_release_channel(), check chan->device->privatecnt after call
> dma_chan_put(). However, dma_chan_put() call dma_device_put() which could
> release the last reference of the device if the DMA provider is already
> gone and hence free it.
>
> Fixes it by moving dma_chan_put() after the check.
>
> Fixes: 0f571515c332 ("dmaengine: Add privatecnt to revert DMA_PRIVATE property")
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/dma/dmaengine.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 405bd2fbb4a3..9049171df857 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -905,11 +905,12 @@ void dma_release_channel(struct dma_chan *chan)
>  	mutex_lock(&dma_list_mutex);
>  	WARN_ONCE(chan->client_count != 1,
>  		  "chan reference count %d != 1\n", chan->client_count);
> -	dma_chan_put(chan);
>  	/* drop PRIVATE cap enabled by __dma_request_channel() */
>  	if (--chan->device->privatecnt == 0)
>  		dma_cap_clear(DMA_PRIVATE, chan->device->cap_mask);
>
> +	dma_chan_put(chan);
> +
>  	if (chan->slave) {
>  		sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME);
>  		sysfs_remove_link(&chan->slave->kobj, chan->name);
>
> --
> 2.54.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 2/4] dmaengine: dma-axi-dmac: Properly free struct axi_dmac_desc
  2026-04-24 17:40 ` [PATCH v4 2/4] dmaengine: dma-axi-dmac: Properly free struct axi_dmac_desc Nuno Sá via B4 Relay
@ 2026-05-07 18:33   ` Frank Li
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Li @ 2026-05-07 18:33 UTC (permalink / raw)
  To: Nuno Sá
  Cc: dmaengine, linux-kernel, Lars-Peter Clausen, Vinod Koul, Frank Li

On Fri, Apr 24, 2026 at 06:40:15PM +0100, Nuno Sá wrote:
> Use axi_dmac_free_desc() to free fully the descriptor at fail path when
> call axi_dmac_alloc_desc() in axi_dmac_prep_peripheral_dma_vec().
>
> Fixes: 74609e568670 ("dmaengine: dma-axi-dmac: Implement device_prep_peripheral_dma_vec")
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/dma/dma-axi-dmac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index 45c2c8e4bc45..127c3cf80a0e 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
> @@ -769,7 +769,7 @@ axi_dmac_prep_peripheral_dma_vec(struct dma_chan *c, const struct dma_vec *vecs,
>  	for (i = 0; i < nb; i++) {
>  		if (!axi_dmac_check_addr(chan, vecs[i].addr) ||
>  		    !axi_dmac_check_len(chan, vecs[i].len)) {
> -			kfree(desc);
> +			axi_dmac_free_desc(desc);
>  			return NULL;
>  		}
>
>
> --
> 2.54.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 3/4] dmaengine: dma-axi-dmac: Drop struct clk from main struct
  2026-04-24 17:40 ` [PATCH v4 3/4] dmaengine: dma-axi-dmac: Drop struct clk from main struct Nuno Sá via B4 Relay
@ 2026-05-07 18:34   ` Frank Li
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Li @ 2026-05-07 18:34 UTC (permalink / raw)
  To: Nuno Sá
  Cc: dmaengine, linux-kernel, Lars-Peter Clausen, Vinod Koul, Frank Li

On Fri, Apr 24, 2026 at 06:40:16PM +0100, Nuno Sá wrote:
> There's no reason to keep struct clk in struct axi_dmac. Hence, use a
> local clk variable in .probe() and drop it from struct axi_dmac.
>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/dma-axi-dmac.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index 127c3cf80a0e..41898d594be7 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
> @@ -170,8 +170,6 @@ struct axi_dmac {
>  	void __iomem *base;
>  	int irq;
>
> -	struct clk *clk;
> -
>  	struct dma_device dma_dev;
>  	struct axi_dmac_chan chan;
>  };
> @@ -1198,6 +1196,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  {
>  	struct dma_device *dma_dev;
>  	struct axi_dmac *dmac;
> +	struct clk *clk;
>  	struct regmap *regmap;
>  	unsigned int version;
>  	u32 irq_mask = 0;
> @@ -1217,9 +1216,9 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  	if (IS_ERR(dmac->base))
>  		return PTR_ERR(dmac->base);
>
> -	dmac->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> -	if (IS_ERR(dmac->clk))
> -		return PTR_ERR(dmac->clk);
> +	clk = devm_clk_get_enabled(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
>
>  	version = axi_dmac_read(dmac, ADI_AXI_REG_VERSION);
>
>
> --
> 2.54.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 4/4] dmaengine: dma-axi-dmac: use DMA pool to manange DMA descriptor
  2026-04-24 17:40 ` [PATCH v4 4/4] dmaengine: dma-axi-dmac: use DMA pool to manange DMA descriptor Nuno Sá via B4 Relay
@ 2026-05-07 18:37   ` Frank Li
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Li @ 2026-05-07 18:37 UTC (permalink / raw)
  To: Nuno Sá
  Cc: dmaengine, linux-kernel, Lars-Peter Clausen, Vinod Koul, Frank Li

On Fri, Apr 24, 2026 at 06:40:17PM +0100, Nuno Sá wrote:
> For architectures like Microblaze or arm64 (where this IP is used),
> DMA_DIRECT_REMAP is set which means that dma_alloc_coherent() might
> remap (and hence vmalloc()) some memory. This became visible in a design
> where dma_direct_use_pool() is not possible.
>
> With the above, when calling dma_free_coherent(), vunmap() would be
> called from softirq context and thus leading to a BUG().
>
> To fix it, use a dma pool that is allocated in
> .device_alloc_chan_resources() and allocate blocks from it. The key
> point is that now dma_pool_free() is used in axi_dmac_free_desc() to
> free the blocks and that just frees the blocks from the pool in the
> sense they can be used again. In other words, no actual call to
> dma_free_coherent() happens. That only happens when destroying the pool
> in axi_dmac_free_chan_resources() which does not happen in any interrupt
> context.
>
> Fixes: 3f8fd25936ee ("dmaengine: axi-dmac: Allocate hardware descriptors")
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/dma/dma-axi-dmac.c | 66 ++++++++++++++++++++++++++++------------------
>  1 file changed, 40 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index 41898d594be7..d47ff27e1408 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
> @@ -13,6 +13,7 @@
>  #include <linux/device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
> +#include <linux/dmapool.h>
>  #include <linux/err.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -147,6 +148,7 @@ struct axi_dmac_chan {
>  	struct virt_dma_chan vchan;
>
>  	struct axi_dmac_desc *next_desc;
> +	void *pool;
>  	struct list_head active_descs;
>  	enum dma_transfer_direction direction;
>
> @@ -648,11 +650,17 @@ static void axi_dmac_issue_pending(struct dma_chan *c)
>  	spin_unlock_irqrestore(&chan->vchan.lock, flags);
>  }
>
> +static void axi_dmac_free_desc(struct axi_dmac_desc *desc)
> +{
> +	for (unsigned int i = 0; i < desc->num_sgs; i++)
> +		dma_pool_free(desc->chan->pool, desc->sg[i].hw, desc->sg[i].hw_phys);
> +
> +	kfree(desc);
> +}
> +
>  static struct axi_dmac_desc *
>  axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
>  {
> -	struct axi_dmac *dmac = chan_to_axi_dmac(chan);
> -	struct device *dev = dmac->dma_dev.dev;
>  	struct axi_dmac_hw_desc *hws;
>  	struct axi_dmac_desc *desc;
>  	dma_addr_t hw_phys;
> @@ -664,22 +672,22 @@ axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
>  	desc->num_sgs = num_sgs;
>  	desc->chan = chan;
>
> -	hws = dma_alloc_coherent(dev, PAGE_ALIGN(num_sgs * sizeof(*hws)),
> -				&hw_phys, GFP_ATOMIC);
> -	if (!hws) {
> -		kfree(desc);
> -		return NULL;
> -	}
> -
>  	for (i = 0; i < num_sgs; i++) {
> -		desc->sg[i].hw = &hws[i];
> -		desc->sg[i].hw_phys = hw_phys + i * sizeof(*hws);
> +		hws = dma_pool_zalloc(chan->pool, GFP_NOWAIT, &hw_phys);
> +		if (!hws) {
> +			desc->num_sgs = i;
> +			axi_dmac_free_desc(desc);
> +			return NULL;
> +		}
>
> -		hws[i].id = AXI_DMAC_SG_UNUSED;
> -		hws[i].flags = 0;
> +		desc->sg[i].hw = hws;
> +		desc->sg[i].hw_phys = hw_phys;
> +
> +		hws->id = AXI_DMAC_SG_UNUSED;
>
>  		/* Link hardware descriptors */
> -		hws[i].next_sg_addr = hw_phys + (i + 1) * sizeof(*hws);
> +		if (i)
> +			desc->sg[i - 1].hw->next_sg_addr = hw_phys;
>  	}
>
>  	/* The last hardware descriptor will trigger an interrupt */
> @@ -688,18 +696,6 @@ axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
>  	return desc;
>  }
>
> -static void axi_dmac_free_desc(struct axi_dmac_desc *desc)
> -{
> -	struct axi_dmac *dmac = chan_to_axi_dmac(desc->chan);
> -	struct device *dev = dmac->dma_dev.dev;
> -	struct axi_dmac_hw_desc *hw = desc->sg[0].hw;
> -	dma_addr_t hw_phys = desc->sg[0].hw_phys;
> -
> -	dma_free_coherent(dev, PAGE_ALIGN(desc->num_sgs * sizeof(*hw)),
> -			  hw, hw_phys);
> -	kfree(desc);
> -}
> -
>  static struct axi_dmac_sg *axi_dmac_fill_linear_sg(struct axi_dmac_chan *chan,
>  	enum dma_transfer_direction direction, dma_addr_t addr,
>  	unsigned int num_periods, unsigned int period_len,
> @@ -933,9 +929,26 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_interleaved(
>  	return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
>  }
>
> +static int axi_dmac_alloc_chan_resources(struct dma_chan *c)
> +{
> +	struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
> +	struct device *dev = c->device->dev;
> +
> +	chan->pool = dma_pool_create(dev_name(dev), dev,
> +				     sizeof(struct axi_dmac_hw_desc),
> +				     __alignof__(struct axi_dmac_hw_desc), 0);
> +	if (!chan->pool)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
>  static void axi_dmac_free_chan_resources(struct dma_chan *c)
>  {
> +	struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
> +
>  	vchan_free_chan_resources(to_virt_chan(c));
> +	dma_pool_destroy(chan->pool);
>  }
>
>  static void axi_dmac_desc_free(struct virt_dma_desc *vdesc)
> @@ -1238,6 +1251,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  	dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
>  	dma_cap_set(DMA_CYCLIC, dma_dev->cap_mask);
>  	dma_cap_set(DMA_INTERLEAVE, dma_dev->cap_mask);
> +	dma_dev->device_alloc_chan_resources = axi_dmac_alloc_chan_resources;
>  	dma_dev->device_free_chan_resources = axi_dmac_free_chan_resources;
>  	dma_dev->device_tx_status = dma_cookie_status;
>  	dma_dev->device_issue_pending = axi_dmac_issue_pending;
>
> --
> 2.54.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-05-07 18:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-24 17:40 [PATCH v4 0/4] dmaengine: dma-axi-dmac: Some memory related fixes Nuno Sá via B4 Relay
2026-04-24 17:40 ` [PATCH v4 1/4] dmaengine: Fix possible use after free Nuno Sá via B4 Relay
2026-05-07 18:33   ` Frank Li
2026-04-24 17:40 ` [PATCH v4 2/4] dmaengine: dma-axi-dmac: Properly free struct axi_dmac_desc Nuno Sá via B4 Relay
2026-05-07 18:33   ` Frank Li
2026-04-24 17:40 ` [PATCH v4 3/4] dmaengine: dma-axi-dmac: Drop struct clk from main struct Nuno Sá via B4 Relay
2026-05-07 18:34   ` Frank Li
2026-04-24 17:40 ` [PATCH v4 4/4] dmaengine: dma-axi-dmac: use DMA pool to manange DMA descriptor Nuno Sá via B4 Relay
2026-05-07 18:37   ` Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox