* [PATCH v7 1/9] dmaengine: Add API to combine configuration and preparation (sg and single)
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 15:32 ` [PATCH v7 2/9] dmaengine: Add safe API to combine configuration and preparation Frank.Li
` (7 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li
From: Frank Li <Frank.Li@nxp.com>
Previously, configuration and preparation required two separate calls. This
works well when configuration is done only once during initialization.
However, in cases where the burst length or source/destination address must
be adjusted for each transfer, calling two functions is verbose and
requires additional locking to ensure both steps complete atomically.
Add a new API dmaengine_prep_config_single() and dmaengine_prep_config_sg()
and callback device_prep_config_sg() that combines configuration and
preparation into a single operation. If the configuration argument is
passed as NULL, fall back to the existing implementation.
Tested-by: Niklas Cassel <cassel@kernel.org>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v4
- drop context in device_prep_config_sg()
change in v3
- remove Deprecated for callback device_prep_slave_sg().
- Move condition check before sg init.
- split function at return type.
- move safe version to next patch
change in v2
- add () for function
- use short name device_prep_sg(), remove "slave" and "config". the 'slave'
is reduntant. after remove slave, the function name is difference existed
one, so remove _config suffix.
---
Documentation/driver-api/dmaengine/client.rst | 9 ++++
include/linux/dmaengine.h | 63 +++++++++++++++++++++++----
2 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index d491e385d61a9..5ee5d4a3596dd 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -80,6 +80,10 @@ The details of these operations are:
- slave_sg: DMA a list of scatter gather buffers from/to a peripheral
+ - config_sg: Similar with slave_sg, just pass down dma_slave_config
+ struct to avoid calling dmaengine_slave_config() every time adjusting the
+ burst length or the FIFO address is needed.
+
- peripheral_dma_vec: DMA an array of scatter gather buffers from/to a
peripheral. Similar to slave_sg, but uses an array of dma_vec
structures instead of a scatterlist.
@@ -106,6 +110,11 @@ The details of these operations are:
unsigned int sg_len, enum dma_data_direction direction,
unsigned long flags);
+ struct dma_async_tx_descriptor *dmaengine_prep_config_sg(
+ struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sg_len, enum dma_transfer_direction dir,
+ unsigned long flags, struct dma_slave_config *config);
+
struct dma_async_tx_descriptor *dmaengine_prep_peripheral_dma_vec(
struct dma_chan *chan, const struct dma_vec *vecs,
size_t nents, enum dma_data_direction direction,
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index b3d251c9734e9..defa377d2ef54 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -835,6 +835,7 @@ struct dma_filter {
* where the address and size of each segment is located in one entry of
* the dma_vec array.
* @device_prep_slave_sg: prepares a slave dma operation
+ * @device_prep_config_sg: prepares a slave DMA operation with dma_slave_config
* @device_prep_dma_cyclic: prepare a cyclic dma operation suitable for audio.
* The function takes a buffer of size buf_len. The callback function will
* be called after period_len bytes have been transferred.
@@ -934,6 +935,10 @@ struct dma_device {
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_transfer_direction direction,
unsigned long flags, void *context);
+ struct dma_async_tx_descriptor *(*device_prep_config_sg)(
+ struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sg_len, enum dma_transfer_direction direction,
+ unsigned long flags, struct dma_slave_config *config);
struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
size_t period_len, enum dma_transfer_direction direction,
@@ -974,22 +979,44 @@ static inline bool is_slave_direction(enum dma_transfer_direction direction)
(direction == DMA_DEV_TO_DEV);
}
-static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
- struct dma_chan *chan, dma_addr_t buf, size_t len,
- enum dma_transfer_direction dir, unsigned long flags)
+static inline struct dma_async_tx_descriptor *
+dmaengine_prep_config_single(struct dma_chan *chan, dma_addr_t buf, size_t len,
+ enum dma_transfer_direction dir,
+ unsigned long flags,
+ struct dma_slave_config *config)
{
struct scatterlist sg;
+
+ if (!chan || !chan->device)
+ return NULL;
+
sg_init_table(&sg, 1);
sg_dma_address(&sg) = buf;
sg_dma_len(&sg) = len;
- if (!chan || !chan->device || !chan->device->device_prep_slave_sg)
+ if (chan->device->device_prep_config_sg)
+ return chan->device->device_prep_config_sg(chan, &sg, 1, dir,
+ flags, config);
+
+ if (config)
+ if (dmaengine_slave_config(chan, config))
+ return NULL;
+
+ if (!chan->device->device_prep_slave_sg)
return NULL;
return chan->device->device_prep_slave_sg(chan, &sg, 1,
dir, flags, NULL);
}
+static inline struct dma_async_tx_descriptor *
+dmaengine_prep_slave_single(struct dma_chan *chan, dma_addr_t buf, size_t len,
+ enum dma_transfer_direction dir,
+ unsigned long flags)
+{
+ return dmaengine_prep_config_single(chan, buf, len, dir, flags, NULL);
+}
+
/**
* dmaengine_prep_peripheral_dma_vec() - Prepare a DMA scatter-gather descriptor
* @chan: The channel to be used for this descriptor
@@ -1010,17 +1037,37 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_peripheral_dma_vec(
dir, flags);
}
-static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg(
- struct dma_chan *chan, struct scatterlist *sgl, unsigned int sg_len,
- enum dma_transfer_direction dir, unsigned long flags)
+static inline struct dma_async_tx_descriptor *
+dmaengine_prep_config_sg(struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sg_len, enum dma_transfer_direction dir,
+ unsigned long flags, struct dma_slave_config *config)
{
- if (!chan || !chan->device || !chan->device->device_prep_slave_sg)
+ if (!chan || !chan->device)
+ return NULL;
+
+ if (chan->device->device_prep_config_sg)
+ return chan->device->device_prep_config_sg(chan, sgl, sg_len,
+ dir, flags, config);
+
+ if (config)
+ if (dmaengine_slave_config(chan, config))
+ return NULL;
+
+ if (!chan->device->device_prep_slave_sg)
return NULL;
return chan->device->device_prep_slave_sg(chan, sgl, sg_len,
dir, flags, NULL);
}
+static inline struct dma_async_tx_descriptor *
+dmaengine_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sg_len, enum dma_transfer_direction dir,
+ unsigned long flags)
+{
+ return dmaengine_prep_config_sg(chan, sgl, sg_len, dir, flags, NULL);
+}
+
#ifdef CONFIG_RAPIDIO_DMA_ENGINE
struct rio_dma_ext;
static inline struct dma_async_tx_descriptor *dmaengine_prep_rio_sg(
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v7 2/9] dmaengine: Add safe API to combine configuration and preparation
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
2026-05-21 15:32 ` [PATCH v7 1/9] dmaengine: Add API to combine configuration and preparation (sg and single) Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 17:10 ` sashiko-bot
2026-05-21 15:32 ` [PATCH v7 3/9] PCI: endpoint: pci-epf-test: Use dmaenigne_prep_config_single() to simplify code Frank.Li
` (6 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li
From: Frank Li <Frank.Li@nxp.com>
Introduce dmaengine_prep_config_single_safe() and
dmaengine_prep_config_sg_safe() to provide a reentrant-safe way to
combine slave configuration and transfer preparation.
Drivers may implement the new device_prep_config_sg() callback to perform
both steps atomically. If the callback is not provided, the helpers fall
back to calling dmaengine_slave_config() followed by
dmaengine_prep_slave_sg() under per-channel spinlock protection.
Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v6
- replace mutex with spinlock in commit message
- use spinlock_saveirq according to AI review results
"The documentation in struct dma_chan notes that *_prep() may be called
from a completion callback. Since completion callbacks often execute in
softirq or hardirq contexts, if a thread calls this function from
process context, local interrupts remain enabled.
If a DMA interrupt fires on the same CPU while the lock is held, the
completion callback could attempt to call this function again to queue
the next transfer, leading it to wait on the already-held chan->lock.
Does this fallback path need to use spin_lock_irqsave() and
spin_unlock_irqrestore() to safely disable interrupts?
"
chagne in v5
- remove reduntant lock commments.
- use kernel doc to descritp API
chagne in v4
- use spinlock() to protect config() and prep()
change in v3
- new patch
---
drivers/dma/dmaengine.c | 2 ++
include/linux/dmaengine.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 405bd2fbb4a3b..ba29e60160c1a 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -1099,6 +1099,8 @@ static int __dma_async_device_channel_register(struct dma_device *device,
chan->dev->device.parent = device->dev;
chan->dev->chan = chan;
chan->dev->dev_id = device->dev_id;
+ spin_lock_init(&chan->lock);
+
if (!name)
dev_set_name(&chan->dev->device, "dma%dchan%d", device->dev_id, chan->chan_id);
else
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index defa377d2ef54..6fe46c0c94527 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -322,6 +322,8 @@ struct dma_router {
* @slave: ptr to the device using this channel
* @cookie: last cookie value returned to client
* @completed_cookie: last completed cookie for this channel
+ * @lock: protect between config and prepare transfer when driver have not
+ * implemented callback device_prep_config_sg().
* @chan_id: channel ID for sysfs
* @dev: class device for sysfs
* @name: backlink name for sysfs
@@ -341,6 +343,12 @@ struct dma_chan {
dma_cookie_t cookie;
dma_cookie_t completed_cookie;
+ /*
+ * protect between config and prepare transfer because *_prep() may be
+ * called from complete callback, which is in GFP_NOSLEEP context.
+ */
+ spinlock_t lock;
+
/* sysfs */
int chan_id;
struct dma_chan_dev *dev;
@@ -1068,6 +1076,84 @@ dmaengine_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
return dmaengine_prep_config_sg(chan, sgl, sg_len, dir, flags, NULL);
}
+/**
+ * dmaengine_prep_config_sg_safe - prepare a scatter-gather DMA transfer
+ * with atomic slave configuration update
+ * @chan: DMA channel
+ * @sgl: scatterlist for the transfer
+ * @sg_len: number of entries in @sgl
+ * @dir: DMA transfer direction
+ * @flags: transfer preparation flags
+ * @config: DMA slave configuration for this transfer
+ *
+ * Prepare a DMA scatter-gather transfer together with a corresponding slave
+ * configuration update in a re-entrant and race-safe manner.
+ *
+ * DMA engine drivers may implement the optional
+ * device_prep_config_sg() callback to perform both the slave configuration
+ * and descriptor preparation atomically. In this case, the operation is
+ * fully handled by the DMA engine driver.
+ *
+ * If the DMA engine driver does not implement device_prep_config_sg(), falls
+ * back to calling dmaengine_slave_config() followed by dmaengine_prep_slave_sg().
+ * The fallback path is protected by a per-channel spinlock to ensure that
+ * concurrent callers cannot interleave configuration and descriptor preparation
+ * on the same DMA channel.
+ *
+ * Return: Pointer to a prepared DMA async transaction descriptor on success,
+ * or %NULL if the transfer could not be prepared.
+ */
+static inline struct dma_async_tx_descriptor *
+dmaengine_prep_config_sg_safe(struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sg_len,
+ enum dma_transfer_direction dir,
+ unsigned long flags,
+ struct dma_slave_config *config)
+{
+ struct dma_async_tx_descriptor *tx;
+ unsigned long spinlock_flags;
+
+ if (!chan || !chan->device)
+ return NULL;
+
+ if (!chan->device->device_prep_config_sg)
+ spin_lock_irqsave(&chan->lock, spinlock_flags);
+
+ tx = dmaengine_prep_config_sg(chan, sgl, sg_len, dir, flags, config);
+
+ if (!chan->device->device_prep_config_sg)
+ spin_unlock_irqrestore(&chan->lock, spinlock_flags);
+
+ return tx;
+}
+
+/**
+ * dmaengine_prep_config_single_safe - prepare a single-buffer DMA transfer
+ * with atomic slave configuration update
+ * @chan: DMA channel
+ * @buf: DMA buffer address
+ * @len: length of the transfer in bytes
+ * @dir: DMA transfer direction
+ * @flags: transfer preparation flags
+ * @config: DMA slave configuration for this transfer
+ *
+ * Detail see dmaengine_prep_config_sg_safe().
+ */
+static inline struct dma_async_tx_descriptor *
+dmaengine_prep_config_single_safe(struct dma_chan *chan, dma_addr_t buf,
+ size_t len, enum dma_transfer_direction dir,
+ unsigned long flags,
+ struct dma_slave_config *config)
+{
+ struct scatterlist sg;
+
+ sg_init_table(&sg, 1);
+ sg_dma_address(&sg) = buf;
+ sg_dma_len(&sg) = len;
+
+ return dmaengine_prep_config_sg_safe(chan, &sg, 1, dir, flags, config);
+}
+
#ifdef CONFIG_RAPIDIO_DMA_ENGINE
struct rio_dma_ext;
static inline struct dma_async_tx_descriptor *dmaengine_prep_rio_sg(
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 2/9] dmaengine: Add safe API to combine configuration and preparation
2026-05-21 15:32 ` [PATCH v7 2/9] dmaengine: Add safe API to combine configuration and preparation Frank.Li
@ 2026-05-21 17:10 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-21 17:10 UTC (permalink / raw)
To: Frank.Li; +Cc: vkoul, imx, Frank.Li, linux-pci, dmaengine
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Calling `device_config` or `device_prep_slave_sg` while holding a spinlock with interrupts disabled (`spin_lock_irqsave`) will cause lockups and `WARN_ON_ONCE` in drivers that use bottom-half locks.
- [Medium] `spinlock_flags` may be used uninitialized and the lock may become unbalanced due to re-evaluation of `chan->device->device_prep_config_sg` across an opaque function call.
Pre-existing issues:
- [Critical] The function `dmaengine_prep_config_single_safe` passes a stack-allocated `scatterlist` to the DMA engine driver, which causes a stack use-after-free if the driver defers processing the scatterlist.
--
commit d2363736f5c7b002b0f30c81cef1791d2f12abd2
Author: Frank Li <Frank.Li@nxp.com>
Subject: dmaengine: Add safe API to combine configuration and preparation
Introduce new safe APIs to atomically combine slave configuration and transfer
preparation. The new functions fall back to calling slave_config and
prep_slave_sg under a channel spinlock if the device doesn't support the
atomic callback.
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index defa377d2ef54..6fe46c0c94527 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
[ ... ]
> +static inline struct dma_async_tx_descriptor *
> +dmaengine_prep_config_sg_safe(struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sg_len,
> + enum dma_transfer_direction dir,
> + unsigned long flags,
> + struct dma_slave_config *config)
> +{
> + struct dma_async_tx_descriptor *tx;
> + unsigned long spinlock_flags;
> +
> + if (!chan || !chan->device)
> + return NULL;
> +
> + if (!chan->device->device_prep_config_sg)
> + spin_lock_irqsave(&chan->lock, spinlock_flags);
> +
> + tx = dmaengine_prep_config_sg(chan, sgl, sg_len, dir, flags, config);
[Severity: High]
Will this cause lockups or warnings in drivers that use bottom-half locks?
Since spin_lock_irqsave() disables hard interrupts, calling into
dmaengine_prep_config_sg() invokes the underlying DMA driver's
device_prep_slave_sg() or device_config() callbacks. Some drivers use
bottom-half locks in these implementations:
dmaengine_prep_config_sg_safe()
spin_lock_irqsave()
dmaengine_prep_config_sg()
chan->device->device_prep_slave_sg()
td_prep_slave_sg()
td_desc_get()
spin_lock_bh()
When the driver eventually calls spin_unlock_bh(), it invokes
local_bh_enable(), which triggers a WARN_ON_ONCE(irqs_disabled()) because
hard interrupts were disabled by the outer spin_lock_irqsave().
> +
> + if (!chan->device->device_prep_config_sg)
> + spin_unlock_irqrestore(&chan->lock, spinlock_flags);
[Severity: Medium]
Could this lead to an uninitialized variable usage or an unbalanced lock?
The condition checking chan->device->device_prep_config_sg is evaluated
twice, before and after calling dmaengine_prep_config_sg(). Because that
function invokes opaque function pointers passing chan, the compiler must
assume memory reachable through chan might be modified.
This forces the compiler to consider a control flow path where the condition
is initially false (skipping spin_lock_irqsave and leaving spinlock_flags
uninitialized) but evaluates to true after the opaque call, leading to
spin_unlock_irqrestore() with uninitialized flags and an unbalanced lock.
Even if this doesn't happen dynamically, it can emit a -Wmaybe-uninitialized
warning that breaks builds with -Werror. Caching the condition in a local
boolean variable would prevent this.
> +
> + return tx;
> +}
> +
> +/**
> + * dmaengine_prep_config_single_safe - prepare a single-buffer DMA transfer
> + * with atomic slave configuration update
[ ... ]
> +static inline struct dma_async_tx_descriptor *
> +dmaengine_prep_config_single_safe(struct dma_chan *chan, dma_addr_t buf,
> + size_t len, enum dma_transfer_direction dir,
> + unsigned long flags,
> + struct dma_slave_config *config)
> +{
> + struct scatterlist sg;
> +
> + sg_init_table(&sg, 1);
> + sg_dma_address(&sg) = buf;
> + sg_dma_len(&sg) = len;
> +
> + return dmaengine_prep_config_sg_safe(chan, &sg, 1, dir, flags, config);
[Severity: Critical]
This is a pre-existing issue, but does passing a stack-allocated scatterlist
to the driver cause a stack use-after-free if the driver defers processing?
The scatterlist sg is allocated on the stack and passed down to the
underlying DMA engine's device_prep_slave_sg() callback. While some drivers
consume the scatterlist synchronously, others save the pointer to process
it asynchronously later:
uniphier_mdmac_prep_slave_sg()
md->sgl = sgl;
uniphier_mdmac_handle()
dereferences md->sgl
Because dmaengine_prep_config_single_safe() returns immediately, the stack
frame is destroyed. When the DMA driver later accesses the saved pointer
from an IRQ context, it accesses freed stack memory.
This pattern already exists in dmaengine_prep_slave_single(), but
replicating it here propagates the regression to the new API.
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com?part=2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 3/9] PCI: endpoint: pci-epf-test: Use dmaenigne_prep_config_single() to simplify code
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
2026-05-21 15:32 ` [PATCH v7 1/9] dmaengine: Add API to combine configuration and preparation (sg and single) Frank.Li
2026-05-21 15:32 ` [PATCH v7 2/9] dmaengine: Add safe API to combine configuration and preparation Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 17:31 ` sashiko-bot
2026-05-21 15:32 ` [PATCH v7 4/9] dmaengine: dw-edma: Use new .device_prep_config_sg() callback Frank.Li
` (5 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li,
Damien Le Moal
From: Frank Li <Frank.Li@nxp.com>
Use dmaenigne_prep_config_single() to simplify code.
No functional change.
Tested-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3
- add Damien Le Moal review tag
---
drivers/pci/endpoint/functions/pci-epf-test.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 591d301fa89d8..0f5cf2d795108 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -182,12 +182,8 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
else
sconf.src_addr = dma_remote;
- if (dmaengine_slave_config(chan, &sconf)) {
- dev_err(dev, "DMA slave config fail\n");
- return -EIO;
- }
- tx = dmaengine_prep_slave_single(chan, dma_local, len, dir,
- flags);
+ tx = dmaengine_prep_config_single(chan, dma_local, len,
+ dir, flags, &sconf);
} else {
tx = dmaengine_prep_dma_memcpy(chan, dma_dst, dma_src, len,
flags);
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 3/9] PCI: endpoint: pci-epf-test: Use dmaenigne_prep_config_single() to simplify code
2026-05-21 15:32 ` [PATCH v7 3/9] PCI: endpoint: pci-epf-test: Use dmaenigne_prep_config_single() to simplify code Frank.Li
@ 2026-05-21 17:31 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-21 17:31 UTC (permalink / raw)
To: Frank.Li; +Cc: linux-pci, Frank.Li, dmaengine, imx, vkoul
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unvalidated `size` from PCIe test registers leads to arbitrarily large `kzalloc` allocations, potentially triggering kernel warnings or panics.
--
commit a2e73aaa3ad55ca4f65ceeb2cf567d9f9ac1f014
Author: Frank Li <Frank.Li@nxp.com>
PCI: endpoint: pci-epf-test: Use dmaenigne_prep_config_single() to simplify code
This commit simplifies the DMA configuration code by replacing
dmaengine_slave_config() and dmaengine_prep_slave_single() with a single
call to dmaengine_prep_config_single().
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -182,12 +182,8 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
> else
> sconf.src_addr = dma_remote;
>
> - if (dmaengine_slave_config(chan, &sconf)) {
> - dev_err(dev, "DMA slave config fail\n");
> - return -EIO;
> - }
> - tx = dmaengine_prep_slave_single(chan, dma_local, len, dir,
> - flags);
> + tx = dmaengine_prep_config_single(chan, dma_local, len,
> + dir, flags, &sconf);
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem with how the
len parameter is handled by the callers of this transfer code.
The functions pci_epf_test_copy(), pci_epf_test_read(), and
pci_epf_test_write() read a 32-bit size value directly from the memory-mapped
reg->size register, which is controlled by an external PCIe host.
This raw size is then passed directly to kzalloc() without any bounds checking
or validation.
Could this allow a malicious or misconfigured PCIe host to pass an arbitrarily
large size to kzalloc(), triggering a page allocator warning and stack trace
that might remotely crash the endpoint kernel if it is configured with
panic_on_warn?
> } else {
> tx = dmaengine_prep_dma_memcpy(chan, dma_dst, dma_src, len,
> flags);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 4/9] dmaengine: dw-edma: Use new .device_prep_config_sg() callback
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
` (2 preceding siblings ...)
2026-05-21 15:32 ` [PATCH v7 3/9] PCI: endpoint: pci-epf-test: Use dmaenigne_prep_config_single() to simplify code Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 17:56 ` sashiko-bot
2026-05-21 15:32 ` [PATCH v7 5/9] dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer() Frank.Li
` (4 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li,
Damien Le Moal
From: Frank Li <Frank.Li@nxp.com>
Use the new .device_prep_config_sg() callback to combine configuration and
descriptor preparation.
No functional changes.
Tested-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v6
- check dw_edma_device_config() return value; find by sashiko AI.
change in v4
- drop context in callback.
change in v3
- add Damien Le Moal review tag
---
drivers/dma/dw-edma/dw-edma-core.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c2feb3adc79fa..92572dd8131e6 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -577,10 +577,11 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
}
static struct dma_async_tx_descriptor *
-dw_edma_device_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
- unsigned int len,
- enum dma_transfer_direction direction,
- unsigned long flags, void *context)
+dw_edma_device_prep_config_sg(struct dma_chan *dchan, struct scatterlist *sgl,
+ unsigned int len,
+ enum dma_transfer_direction direction,
+ unsigned long flags,
+ struct dma_slave_config *config)
{
struct dw_edma_transfer xfer;
@@ -591,6 +592,9 @@ dw_edma_device_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
xfer.flags = flags;
xfer.type = EDMA_XFER_SCATTER_GATHER;
+ if (config && dw_edma_device_config(dchan, config))
+ return NULL;
+
return dw_edma_device_transfer(&xfer);
}
@@ -970,7 +974,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
dma->device_terminate_all = dw_edma_device_terminate_all;
dma->device_issue_pending = dw_edma_device_issue_pending;
dma->device_tx_status = dw_edma_device_tx_status;
- dma->device_prep_slave_sg = dw_edma_device_prep_slave_sg;
+ dma->device_prep_config_sg = dw_edma_device_prep_config_sg;
dma->device_prep_dma_cyclic = dw_edma_device_prep_dma_cyclic;
dma->device_prep_interleaved_dma = dw_edma_device_prep_interleaved_dma;
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 4/9] dmaengine: dw-edma: Use new .device_prep_config_sg() callback
2026-05-21 15:32 ` [PATCH v7 4/9] dmaengine: dw-edma: Use new .device_prep_config_sg() callback Frank.Li
@ 2026-05-21 17:56 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-21 17:56 UTC (permalink / raw)
To: Frank.Li; +Cc: dmaengine, vkoul, imx, Frank.Li, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `dw_edma_device_prep_config_sg` callback modifies channel configuration locklessly, breaking the atomicity guarantee expected by the DMA engine framework.
--
commit be13dc9c4b2aec5db6e6e0dc4003902282d50c88
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: dw-edma: Use new .device_prep_config_sg() callback
This commit implements the new device_prep_config_sg() callback for the
dw-edma driver. It aims to combine configuration and descriptor preparation
into a single step.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2feb3adc79fa..92572dd8131e6 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -577,10 +577,11 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
> }
>
> static struct dma_async_tx_descriptor *
> -dw_edma_device_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> - unsigned int len,
> - enum dma_transfer_direction direction,
> - unsigned long flags, void *context)
> +dw_edma_device_prep_config_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> + unsigned int len,
> + enum dma_transfer_direction direction,
> + unsigned long flags,
> + struct dma_slave_config *config)
> {
> struct dw_edma_transfer xfer;
>
> @@ -591,6 +592,9 @@ dw_edma_device_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> xfer.flags = flags;
> xfer.type = EDMA_XFER_SCATTER_GATHER;
>
> + if (config && dw_edma_device_config(dchan, config))
> + return NULL;
[Severity: High]
Does this lockless configuration introduce a race condition?
The dmaengine_prep_config_sg_safe API is documented to be re-entrant and
skips acquiring the per-channel spinlock if the driver implements the
device_prep_config_sg callback, relying entirely on the driver to ensure
atomic configuration and descriptor preparation.
However, dw_edma_device_prep_config_sg() directly calls
dw_edma_device_config(), which mutates shared channel state (such as
chan->config, chan->non_ll, and chan->configured) without taking any
internal locks.
If multiple threads concurrently prepare transfers on the same channel,
these updates will race. This could lead to corrupted channel states where
dw_edma_device_transfer() may read interleaved or invalid parameters, such
as an incorrect non_ll flag.
Note that even if a subsequent patch in the series partially mitigates this
by passing the config object to resolve source and destination address races,
it still calls dw_edma_device_config() which locklessly modifies chan->non_ll
and chan->config, leaving the underlying race on the shared channel state
unresolved.
> +
> return dw_edma_device_transfer(&xfer);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com?part=4
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 5/9] dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer()
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
` (3 preceding siblings ...)
2026-05-21 15:32 ` [PATCH v7 4/9] dmaengine: dw-edma: Use new .device_prep_config_sg() callback Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 18:10 ` sashiko-bot
2026-05-21 15:32 ` [PATCH v7 6/9] nvmet: pci-epf: Remove unnecessary dmaengine_terminate_sync() on each DMA transfer Frank.Li
` (3 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li
From: Frank Li <Frank.Li@nxp.com>
Pass dma_slave_config to dw_edma_device_transfer() to support atomic
configuration and descriptor preparation when a non-NULL config is
provided to device_prep_config_sg().
Tested-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3
- rewrite dw_edma_device_slave_config() according to Damien's suggestion.
---
drivers/dma/dw-edma/dw-edma-core.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 92572dd8131e6..ba37bc983dcd2 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -267,6 +267,20 @@ static int dw_edma_device_config(struct dma_chan *dchan,
return 0;
}
+static struct dma_slave_config *
+dw_edma_device_get_config(struct dma_chan *dchan,
+ struct dma_slave_config *config)
+{
+ struct dw_edma_chan *chan;
+
+ if (config)
+ return config;
+
+ chan = dchan2dw_edma_chan(dchan);
+
+ return &chan->config;
+}
+
static int dw_edma_device_pause(struct dma_chan *dchan)
{
struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
@@ -385,7 +399,8 @@ dw_edma_device_tx_status(struct dma_chan *dchan, dma_cookie_t cookie,
}
static struct dma_async_tx_descriptor *
-dw_edma_device_transfer(struct dw_edma_transfer *xfer)
+dw_edma_device_transfer(struct dw_edma_transfer *xfer,
+ struct dma_slave_config *config)
{
struct dw_edma_chan *chan = dchan2dw_edma_chan(xfer->dchan);
enum dma_transfer_direction dir = xfer->direction;
@@ -472,8 +487,8 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
src_addr = xfer->xfer.il->src_start;
dst_addr = xfer->xfer.il->dst_start;
} else {
- src_addr = chan->config.src_addr;
- dst_addr = chan->config.dst_addr;
+ src_addr = config->src_addr;
+ dst_addr = config->dst_addr;
}
if (dir == DMA_DEV_TO_MEM)
@@ -595,7 +610,7 @@ dw_edma_device_prep_config_sg(struct dma_chan *dchan, struct scatterlist *sgl,
if (config && dw_edma_device_config(dchan, config))
return NULL;
- return dw_edma_device_transfer(&xfer);
+ return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, config));
}
static struct dma_async_tx_descriptor *
@@ -614,7 +629,7 @@ dw_edma_device_prep_dma_cyclic(struct dma_chan *dchan, dma_addr_t paddr,
xfer.flags = flags;
xfer.type = EDMA_XFER_CYCLIC;
- return dw_edma_device_transfer(&xfer);
+ return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, NULL));
}
static struct dma_async_tx_descriptor *
@@ -630,7 +645,7 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
xfer.flags = flags;
xfer.type = EDMA_XFER_INTERLEAVED;
- return dw_edma_device_transfer(&xfer);
+ return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, NULL));
}
static void dw_hdma_set_callback_result(struct virt_dma_desc *vd,
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 5/9] dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer()
2026-05-21 15:32 ` [PATCH v7 5/9] dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer() Frank.Li
@ 2026-05-21 18:10 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-21 18:10 UTC (permalink / raw)
To: Frank.Li; +Cc: vkoul, imx, linux-pci, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The patch claims to support atomic configuration but continues to locklessly mutate and read global channel state, causing data races and broken atomicity.
--
commit e550cfd0855adb0abdce8c07ba7dc3b15ac63f86
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer()
This commit updates dw_edma_device_transfer() to take a dma_slave_config
argument. It intends to support atomic configuration and descriptor
preparation when a non-NULL config is provided to device_prep_config_sg().
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -595,7 +610,7 @@ dw_edma_device_prep_config_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> if (config && dw_edma_device_config(dchan, config))
> return NULL;
>
> - return dw_edma_device_transfer(&xfer);
> + return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, config));
> }
[Severity: High]
This is a pre-existing issue, but does this actually achieve the intended
atomic configuration and descriptor preparation mentioned in the commit
message?
Looking at dw_edma_device_prep_config_sg(), it still explicitly calls
dw_edma_device_config() before delegating to dw_edma_device_transfer().
The dw_edma_device_config() function locklessly mutates global channel state:
static int dw_edma_device_config(...)
{
...
if (cfg_non_ll || non_ll)
chan->non_ll = true;
...
memcpy(&chan->config, config, sizeof(*config));
chan->configured = true;
...
}
If multiple threads use this API concurrently for the same channel, could they
race on this memcpy() and global state updates?
Additionally, dw_edma_device_transfer() continues to read the globally mutated
chan->non_ll state to determine burst limits:
static struct dma_async_tx_descriptor *
dw_edma_device_transfer(...)
{
...
bursts_max = chan->non_ll ? 1 : chan->ll_max;
...
}
Because chan->non_ll is evaluated globally rather than from the locally passed
config, could a concurrent preparation silently alter the hardware burst
behavior of an ostensibly atomic and isolated descriptor preparation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com?part=5
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 6/9] nvmet: pci-epf: Remove unnecessary dmaengine_terminate_sync() on each DMA transfer
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
` (4 preceding siblings ...)
2026-05-21 15:32 ` [PATCH v7 5/9] dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer() Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 18:31 ` sashiko-bot
2026-05-21 15:32 ` [PATCH v7 7/9] nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API Frank.Li
` (2 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li,
Damien Le Moal
From: Frank Li <Frank.Li@nxp.com>
dmaengine_terminate_sync() cancels all pending requests. Calling it for
every DMA transfer is unnecessary and counterproductive. This function is
generally intended for cleanup paths such as module removal, device close,
or unbind operations.
Remove the redundant calls for success path and keep it only at error path.
Tested-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
This one also fix stress test failure after remove mutex and use new API
dmaengine_prep_slave_sg_config().
---
drivers/nvme/target/pci-epf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index 4e9db96ebfecd..2afe8f4d0e461 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -420,10 +420,9 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
if (dma_sync_wait(chan, cookie) != DMA_COMPLETE) {
dev_err(dev, "DMA transfer failed\n");
ret = -EIO;
+ dmaengine_terminate_sync(chan);
}
- dmaengine_terminate_sync(chan);
-
unmap:
dma_unmap_single(dma_dev, dma_addr, seg->length, dir);
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 6/9] nvmet: pci-epf: Remove unnecessary dmaengine_terminate_sync() on each DMA transfer
2026-05-21 15:32 ` [PATCH v7 6/9] nvmet: pci-epf: Remove unnecessary dmaengine_terminate_sync() on each DMA transfer Frank.Li
@ 2026-05-21 18:31 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-21 18:31 UTC (permalink / raw)
To: Frank.Li; +Cc: imx, vkoul, linux-pci, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Directly mapping embedded, non-cacheline-aligned struct fields (`cmd`) for DMA causes cacheline sharing corruption on non-cache-coherent architectures.
--
commit bb3da580c11e6aae50b53789fc87a1f21f670249
Author: Frank Li <Frank.Li@nxp.com>
nvmet: pci-epf: Remove unnecessary dmaengine_terminate_sync() on each DMA transfer
This commit removes unnecessary dmaengine_terminate_sync() calls from the
success path of DMA transfers, leaving them only in the error path.
> diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
> index 4e9db96ebfecd..2afe8f4d0e461 100644
> --- a/drivers/nvme/target/pci-epf.c
> +++ b/drivers/nvme/target/pci-epf.c
> @@ -420,10 +420,9 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
[Severity: High]
This is a pre-existing issue, but does mapping this buffer for DMA cause
cacheline sharing corruption on non-coherent architectures?
In nvmet_pci_epf_process_sq(), a pointer to the embedded cmd field of
struct nvmet_pci_epf_iod is passed to nvmet_pci_epf_transfer() with
DMA_FROM_DEVICE. This eventually reaches nvmet_pci_epf_dma_transfer(),
where the buffer is mapped via dma_map_single().
Because cmd is not marked with ____cacheline_aligned in
struct nvmet_pci_epf_iod, it shares cachelines with actively modified
adjacent fields like req and cqe. When dma_map_single() invalidates
the cachelines spanning cmd, any pending CPU writes to adjacent fields
are discarded.
Could this lead to the CPU later reading stale or uninitialized memory when
accessing iod->req or other adjacent fields?
> if (dma_sync_wait(chan, cookie) != DMA_COMPLETE) {
> dev_err(dev, "DMA transfer failed\n");
> ret = -EIO;
> + dmaengine_terminate_sync(chan);
> }
>
> - dmaengine_terminate_sync(chan);
> -
> unmap:
> dma_unmap_single(dma_dev, dma_addr, seg->length, dir);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com?part=6
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 7/9] nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
` (5 preceding siblings ...)
2026-05-21 15:32 ` [PATCH v7 6/9] nvmet: pci-epf: Remove unnecessary dmaengine_terminate_sync() on each DMA transfer Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 19:05 ` sashiko-bot
2026-05-21 15:32 ` [PATCH v7 8/9] PCI: epf-mhi: Use dmaengine_prep_config_single() to simplify code Frank.Li
2026-05-21 15:32 ` [PATCH v7 9/9] crypto: atmel: Use dmaengine_prep_config_sg() API Frank.Li
8 siblings, 1 reply; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li
From: Frank Li <Frank.Li@nxp.com>
Use the new dmaengine_prep_config_single_safe() API to combine the
configuration and descriptor preparation into a single call.
Since dmaengine_prep_config_single_safe() performs the configuration and
preparation atomically and the mutex can be removed.
Tested-by: Niklas Cassel <cassel@kernel.org>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v7
- remove dma_(rx|tx)_lock totally (sashika AI)
change in v6
- remove local unused variable lock (sashika AI)
---
drivers/nvme/target/pci-epf.c | 30 ++++--------------------------
1 file changed, 4 insertions(+), 26 deletions(-)
diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index 2afe8f4d0e461..b1ba2d0bea6d9 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -210,9 +210,7 @@ struct nvmet_pci_epf {
bool dma_enabled;
struct dma_chan *dma_tx_chan;
- struct mutex dma_tx_lock;
struct dma_chan *dma_rx_chan;
- struct mutex dma_rx_lock;
struct mutex mmio_lock;
@@ -295,9 +293,6 @@ static void nvmet_pci_epf_init_dma(struct nvmet_pci_epf *nvme_epf)
struct dma_chan *chan;
dma_cap_mask_t mask;
- mutex_init(&nvme_epf->dma_rx_lock);
- mutex_init(&nvme_epf->dma_tx_lock);
-
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
@@ -336,8 +331,6 @@ static void nvmet_pci_epf_init_dma(struct nvmet_pci_epf *nvme_epf)
nvme_epf->dma_rx_chan = NULL;
out_dma_no_rx:
- mutex_destroy(&nvme_epf->dma_rx_lock);
- mutex_destroy(&nvme_epf->dma_tx_lock);
nvme_epf->dma_enabled = false;
dev_info(&epf->dev, "DMA not supported, falling back to MMIO\n");
@@ -352,8 +345,6 @@ static void nvmet_pci_epf_deinit_dma(struct nvmet_pci_epf *nvme_epf)
nvme_epf->dma_tx_chan = NULL;
dma_release_channel(nvme_epf->dma_rx_chan);
nvme_epf->dma_rx_chan = NULL;
- mutex_destroy(&nvme_epf->dma_rx_lock);
- mutex_destroy(&nvme_epf->dma_tx_lock);
nvme_epf->dma_enabled = false;
}
@@ -368,18 +359,15 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
struct dma_chan *chan;
dma_cookie_t cookie;
dma_addr_t dma_addr;
- struct mutex *lock;
int ret;
switch (dir) {
case DMA_FROM_DEVICE:
- lock = &nvme_epf->dma_rx_lock;
chan = nvme_epf->dma_rx_chan;
sconf.direction = DMA_DEV_TO_MEM;
sconf.src_addr = seg->pci_addr;
break;
case DMA_TO_DEVICE:
- lock = &nvme_epf->dma_tx_lock;
chan = nvme_epf->dma_tx_chan;
sconf.direction = DMA_MEM_TO_DEV;
sconf.dst_addr = seg->pci_addr;
@@ -388,22 +376,15 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
return -EINVAL;
}
- mutex_lock(lock);
-
dma_dev = dmaengine_get_dma_device(chan);
dma_addr = dma_map_single(dma_dev, seg->buf, seg->length, dir);
ret = dma_mapping_error(dma_dev, dma_addr);
if (ret)
- goto unlock;
-
- ret = dmaengine_slave_config(chan, &sconf);
- if (ret) {
- dev_err(dev, "Failed to configure DMA channel\n");
- goto unmap;
- }
+ return ret;
- desc = dmaengine_prep_slave_single(chan, dma_addr, seg->length,
- sconf.direction, DMA_CTRL_ACK);
+ desc = dmaengine_prep_config_single_safe(chan, dma_addr, seg->length,
+ sconf.direction,
+ DMA_CTRL_ACK, &sconf);
if (!desc) {
dev_err(dev, "Failed to prepare DMA\n");
ret = -EIO;
@@ -426,9 +407,6 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
unmap:
dma_unmap_single(dma_dev, dma_addr, seg->length, dir);
-unlock:
- mutex_unlock(lock);
-
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 7/9] nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API
2026-05-21 15:32 ` [PATCH v7 7/9] nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API Frank.Li
@ 2026-05-21 19:05 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-21 19:05 UTC (permalink / raw)
To: Frank.Li; +Cc: imx, linux-pci, vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the DMA channel mutexes allows concurrent synchronous transfers on a shared DMA channel, leading to destructive error handling (cascading aborts), severe CPU starvation, spurious timeouts, and descriptor pool exhaustion.
--
commit 57a71fe2904773875ab4a20b8b7b4de2296d0683
Author: Frank Li <Frank.Li@nxp.com>
nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API
This commit replaces the separate DMA channel configuration and descriptor
preparation steps with the new dmaengine_prep_config_single_safe() API.
It also removes the dma_tx_lock and dma_rx_lock mutexes, assuming they are
no longer needed because the new API call is atomic.
> diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
> index 2afe8f4d0e461..b1ba2d0bea6d9 100644
> --- a/drivers/nvme/target/pci-epf.c
> +++ b/drivers/nvme/target/pci-epf.c
[ ... ]
> @@ -388,22 +376,15 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
> return -EINVAL;
> }
>
> - mutex_lock(lock);
> -
[Severity: High]
Does removing this mutex expose the entire synchronous transfer lifecycle to
concurrent access?
While dmaengine_prep_config_single_safe() is atomic, the removed mutex
also protected the subsequent dmaengine_submit() and dma_sync_wait() calls
in this function. Since NVMe commands are processed concurrently by unbound
workqueues, multiple worker threads might now share the same global DMA
channel without synchronization.
If multiple threads concurrently spin in dma_sync_wait() on the same channel,
could this cause severe CPU starvation due to its tight busy-wait loop?
Additionally, dma_sync_wait() has a fixed 5-second timeout from the moment
it is called. With concurrent queuing, a newly submitted transfer's wait time
would include the execution of all preceding queued transfers on the channel.
Under high load, might this exceed 5 seconds and cause spurious timeouts?
If a timeout does occur, the error path calls dmaengine_terminate_sync(chan).
Will this forcefully abort all pending and active transfers on the channel,
destroying the in-flight DMA operations of all other concurrent NVMe commands?
Finally, unbounded concurrent submissions could potentially exhaust the DMA
engine's hardware descriptor pool under high I/O load, causing
dmaengine_prep_config_single_safe() to return NULL and fail valid commands.
> dma_dev = dmaengine_get_dma_device(chan);
> dma_addr = dma_map_single(dma_dev, seg->buf, seg->length, dir);
> ret = dma_mapping_error(dma_dev, dma_addr);
> if (ret)
> - goto unlock;
> -
> - ret = dmaengine_slave_config(chan, &sconf);
> - if (ret) {
> - dev_err(dev, "Failed to configure DMA channel\n");
> - goto unmap;
> - }
> + return ret;
>
> - desc = dmaengine_prep_slave_single(chan, dma_addr, seg->length,
> - sconf.direction, DMA_CTRL_ACK);
> + desc = dmaengine_prep_config_single_safe(chan, dma_addr, seg->length,
> + sconf.direction,
> + DMA_CTRL_ACK, &sconf);
[ ... ]
> @@ -426,9 +407,6 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
> unmap:
> dma_unmap_single(dma_dev, dma_addr, seg->length, dir);
>
> -unlock:
> - mutex_unlock(lock);
> -
> return ret;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com?part=7
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 8/9] PCI: epf-mhi: Use dmaengine_prep_config_single() to simplify code
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
` (6 preceding siblings ...)
2026-05-21 15:32 ` [PATCH v7 7/9] nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
2026-05-21 15:32 ` [PATCH v7 9/9] crypto: atmel: Use dmaengine_prep_config_sg() API Frank.Li
8 siblings, 0 replies; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li
From: Frank Li <Frank.Li@nxp.com>
Use dmaengine_prep_config_single() to simplify
pci_epf_mhi_edma_read[_sync]() and pci_epf_mhi_edma_write[_sync]().
No functional change.
Tested-by: Niklas Cassel <cassel@kernel.org>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Keep mutex lock because sync with other function.
---
drivers/pci/endpoint/functions/pci-epf-mhi.c | 52 +++++++++-------------------
1 file changed, 16 insertions(+), 36 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index 7f5326925ed54..c3e3b58fb86cd 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -328,12 +328,6 @@ static int pci_epf_mhi_edma_read(struct mhi_ep_cntrl *mhi_cntrl,
config.direction = DMA_DEV_TO_MEM;
config.src_addr = buf_info->host_addr;
- ret = dmaengine_slave_config(chan, &config);
- if (ret) {
- dev_err(dev, "Failed to configure DMA channel\n");
- goto err_unlock;
- }
-
dst_addr = dma_map_single(dma_dev, buf_info->dev_addr, buf_info->size,
DMA_FROM_DEVICE);
ret = dma_mapping_error(dma_dev, dst_addr);
@@ -342,9 +336,10 @@ static int pci_epf_mhi_edma_read(struct mhi_ep_cntrl *mhi_cntrl,
goto err_unlock;
}
- desc = dmaengine_prep_slave_single(chan, dst_addr, buf_info->size,
- DMA_DEV_TO_MEM,
- DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
+ desc = dmaengine_prep_config_single(chan, dst_addr, buf_info->size,
+ DMA_DEV_TO_MEM,
+ DMA_CTRL_ACK | DMA_PREP_INTERRUPT,
+ &config);
if (!desc) {
dev_err(dev, "Failed to prepare DMA\n");
ret = -EIO;
@@ -401,12 +396,6 @@ static int pci_epf_mhi_edma_write(struct mhi_ep_cntrl *mhi_cntrl,
config.direction = DMA_MEM_TO_DEV;
config.dst_addr = buf_info->host_addr;
- ret = dmaengine_slave_config(chan, &config);
- if (ret) {
- dev_err(dev, "Failed to configure DMA channel\n");
- goto err_unlock;
- }
-
src_addr = dma_map_single(dma_dev, buf_info->dev_addr, buf_info->size,
DMA_TO_DEVICE);
ret = dma_mapping_error(dma_dev, src_addr);
@@ -415,9 +404,10 @@ static int pci_epf_mhi_edma_write(struct mhi_ep_cntrl *mhi_cntrl,
goto err_unlock;
}
- desc = dmaengine_prep_slave_single(chan, src_addr, buf_info->size,
- DMA_MEM_TO_DEV,
- DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
+ desc = dmaengine_prep_config_single(chan, src_addr, buf_info->size,
+ DMA_MEM_TO_DEV,
+ DMA_CTRL_ACK | DMA_PREP_INTERRUPT,
+ &config);
if (!desc) {
dev_err(dev, "Failed to prepare DMA\n");
ret = -EIO;
@@ -506,12 +496,6 @@ static int pci_epf_mhi_edma_read_async(struct mhi_ep_cntrl *mhi_cntrl,
config.direction = DMA_DEV_TO_MEM;
config.src_addr = buf_info->host_addr;
- ret = dmaengine_slave_config(chan, &config);
- if (ret) {
- dev_err(dev, "Failed to configure DMA channel\n");
- goto err_unlock;
- }
-
dst_addr = dma_map_single(dma_dev, buf_info->dev_addr, buf_info->size,
DMA_FROM_DEVICE);
ret = dma_mapping_error(dma_dev, dst_addr);
@@ -520,9 +504,10 @@ static int pci_epf_mhi_edma_read_async(struct mhi_ep_cntrl *mhi_cntrl,
goto err_unlock;
}
- desc = dmaengine_prep_slave_single(chan, dst_addr, buf_info->size,
- DMA_DEV_TO_MEM,
- DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
+ desc = dmaengine_prep_config_single(chan, dst_addr, buf_info->size,
+ DMA_DEV_TO_MEM,
+ DMA_CTRL_ACK | DMA_PREP_INTERRUPT,
+ &config);
if (!desc) {
dev_err(dev, "Failed to prepare DMA\n");
ret = -EIO;
@@ -585,12 +570,6 @@ static int pci_epf_mhi_edma_write_async(struct mhi_ep_cntrl *mhi_cntrl,
config.direction = DMA_MEM_TO_DEV;
config.dst_addr = buf_info->host_addr;
- ret = dmaengine_slave_config(chan, &config);
- if (ret) {
- dev_err(dev, "Failed to configure DMA channel\n");
- goto err_unlock;
- }
-
src_addr = dma_map_single(dma_dev, buf_info->dev_addr, buf_info->size,
DMA_TO_DEVICE);
ret = dma_mapping_error(dma_dev, src_addr);
@@ -599,9 +578,10 @@ static int pci_epf_mhi_edma_write_async(struct mhi_ep_cntrl *mhi_cntrl,
goto err_unlock;
}
- desc = dmaengine_prep_slave_single(chan, src_addr, buf_info->size,
- DMA_MEM_TO_DEV,
- DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
+ desc = dmaengine_prep_config_single(chan, src_addr, buf_info->size,
+ DMA_MEM_TO_DEV,
+ DMA_CTRL_ACK | DMA_PREP_INTERRUPT,
+ &config);
if (!desc) {
dev_err(dev, "Failed to prepare DMA\n");
ret = -EIO;
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v7 9/9] crypto: atmel: Use dmaengine_prep_config_sg() API
2026-05-21 15:32 [PATCH v7 0/9] dmaengine: Add new API to combine configuration and descriptor preparation Frank.Li
` (7 preceding siblings ...)
2026-05-21 15:32 ` [PATCH v7 8/9] PCI: epf-mhi: Use dmaengine_prep_config_single() to simplify code Frank.Li
@ 2026-05-21 15:32 ` Frank.Li
8 siblings, 0 replies; 16+ messages in thread
From: Frank.Li @ 2026-05-21 15:32 UTC (permalink / raw)
To: Vinod Koul, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Herbert Xu, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Koichiro Den,
Niklas Cassel
Cc: dmaengine, linux-kernel, linux-pci, linux-nvme, mhi,
linux-arm-msm, linux-crypto, linux-arm-kernel, imx, Frank Li
From: Frank Li <Frank.Li@nxp.com>
Using new API dmaengine_prep_config_sg() to simple code.
dmaengine_prep_config_sg() does not distinguish between configuration
failures and descriptor preparation failures, as both are reported through
a NULL return value. Converting both cases to -ENOMEM is therefore
acceptable and consistent with the helper's abstraction.
In practice, most users only care whether the operation succeeds or fails,
and do not depend on the exact errno value returned from this path.
Tested-by: Niklas Cassel <cassel@kernel.org>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v6
- add commit message about error propagation (sashaki AI)
---
drivers/crypto/atmel-aes.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index b393689400b4c..d890b5a277b9c 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -795,7 +795,6 @@ static int atmel_aes_dma_transfer_start(struct atmel_aes_dev *dd,
struct dma_slave_config config;
dma_async_tx_callback callback;
struct atmel_aes_dma *dma;
- int err;
memset(&config, 0, sizeof(config));
config.src_addr_width = addr_width;
@@ -820,12 +819,9 @@ static int atmel_aes_dma_transfer_start(struct atmel_aes_dev *dd,
return -EINVAL;
}
- err = dmaengine_slave_config(dma->chan, &config);
- if (err)
- return err;
-
- desc = dmaengine_prep_slave_sg(dma->chan, dma->sg, dma->sg_len, dir,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ desc = dmaengine_prep_config_sg(dma->chan, dma->sg, dma->sg_len, dir,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK,
+ &config);
if (!desc)
return -ENOMEM;
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread