| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2bcksnyuxj33bjctjombrstfvjrcdtap6i3v6xhfxtqjmbdkwm@jcaoy2iuh5pr>
Date: Thu, 22 Jan 2026 11:44:53 +0900
From: Koichiro Den <den@...inux.co.jp>
To: Vinod Koul <vkoul@...nel.org>, mani@...nel.org
Cc: Frank Li <Frank.li@....com>, dave.jiang@...el.com, cassel@...nel.org,
kwilczynski@...nel.org, kishon@...nel.org, bhelgaas@...gle.com, geert+renesas@...der.be,
robh@...nel.org, jdmason@...zu.us, allenbh@...il.com, jingoohan1@...il.com,
lpieralisi@...nel.org, linux-pci@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org, devicetree@...r.kernel.org,
dmaengine@...r.kernel.org, iommu@...ts.linux.dev, ntb@...ts.linux.dev,
netdev@...r.kernel.org, linux-kselftest@...r.kernel.org, arnd@...db.de,
gregkh@...uxfoundation.org, joro@...tes.org, will@...nel.org, robin.murphy@....com,
magnus.damm@...il.com, krzk+dt@...nel.org, conor+dt@...nel.org, corbet@....net,
skhan@...uxfoundation.org, andriy.shevchenko@...ux.intel.com, jbrunet@...libre.com,
utkarsh02t@...il.com
Subject: Re: [RFC PATCH v4 02/38] dmaengine: dw-edma: Add per-channel
interrupt routing control
On Wed, Jan 21, 2026 at 09:32:37PM +0530, Vinod Koul wrote:
> On 19-01-26, 23:26, Koichiro Den wrote:
> > On Sun, Jan 18, 2026 at 12:03:19PM -0500, Frank Li wrote:
> > > On Sun, Jan 18, 2026 at 10:54:04PM +0900, Koichiro Den wrote:
> > > > DesignWare EP eDMA can generate interrupts both locally and remotely
> > > > (LIE/RIE). Remote eDMA users need to decide, per channel, whether
> > > > completions should be handled locally, remotely, or both. Unless
> > > > carefully configured, the endpoint and host would race to ack the
> > > > interrupt.
> > > >
> > > > Introduce a per-channel interrupt routing mode and export small APIs to
> > > > configure and query it. Update v0 programming so that RIE and local
> > > > done/abort interrupt masking follow the selected mode. The default mode
> > > > keeps the original behavior, so unless the new APIs are explicitly used,
> > > > no functional changes.
> > > >
> > > > Signed-off-by: Koichiro Den <den@...inux.co.jp>
> > > > ---
> > > > drivers/dma/dw-edma/dw-edma-core.c | 52 +++++++++++++++++++++++++++
> > > > drivers/dma/dw-edma/dw-edma-core.h | 2 ++
> > > > drivers/dma/dw-edma/dw-edma-v0-core.c | 26 +++++++++-----
> > > > include/linux/dma/edma.h | 44 +++++++++++++++++++++++
> > > > 4 files changed, 116 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > > > index b9d59c3c0cb4..059b3996d383 100644
> > > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > > @@ -768,6 +768,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> > > > chan->configured = false;
> > > > chan->request = EDMA_REQ_NONE;
> > > > chan->status = EDMA_ST_IDLE;
> > > > + chan->irq_mode = DW_EDMA_CH_IRQ_DEFAULT;
> > > >
> > > > if (chan->dir == EDMA_DIR_WRITE)
> > > > chan->ll_max = (chip->ll_region_wr[chan->id].sz / EDMA_LL_SZ);
> > > > @@ -1062,6 +1063,57 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> > > > }
> > > > EXPORT_SYMBOL_GPL(dw_edma_remove);
> > > >
> > > > +int dw_edma_chan_irq_config(struct dma_chan *dchan,
> > > > + enum dw_edma_ch_irq_mode mode)
> > > > +{
> > > > + struct dw_edma_chan *chan;
> > > > +
> > > > + switch (mode) {
> > > > + case DW_EDMA_CH_IRQ_DEFAULT:
> > > > + case DW_EDMA_CH_IRQ_LOCAL:
> > > > + case DW_EDMA_CH_IRQ_REMOTE:
> > > > + break;
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (!dchan || !dchan->device)
> > > > + return -ENODEV;
> > > > +
> > > > + chan = dchan2dw_edma_chan(dchan);
> > > > + if (!chan)
> > > > + return -ENODEV;
> > > > +
> > > > + chan->irq_mode = mode;
> > > > +
> > > > + dev_vdbg(chan->dw->chip->dev, "Channel: %s[%u] set irq_mode=%u\n",
> > > > + str_write_read(chan->dir == EDMA_DIR_WRITE),
> > > > + chan->id, mode);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dw_edma_chan_irq_config);
> > > > +
> > > > +bool dw_edma_chan_ignore_irq(struct dma_chan *dchan)
> > > > +{
> > > > + struct dw_edma_chan *chan;
> > > > + struct dw_edma *dw;
> > > > +
> > > > + if (!dchan || !dchan->device)
> > > > + return false;
> > > > +
> > > > + chan = dchan2dw_edma_chan(dchan);
> > > > + if (!chan)
> > > > + return false;
> > > > +
> > > > + dw = chan->dw;
> > > > + if (dw->chip->flags & DW_EDMA_CHIP_LOCAL)
> > > > + return chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE;
> > > > + else
> > > > + return chan->irq_mode == DW_EDMA_CH_IRQ_LOCAL;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dw_edma_chan_ignore_irq);
> > > > +
> > > > MODULE_LICENSE("GPL v2");
> > > > MODULE_DESCRIPTION("Synopsys DesignWare eDMA controller core driver");
> > > > MODULE_AUTHOR("Gustavo Pimentel <gustavo.pimentel@...opsys.com>");
> > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> > > > index 71894b9e0b15..8458d676551a 100644
> > > > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > > > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > > > @@ -81,6 +81,8 @@ struct dw_edma_chan {
> > > >
> > > > struct msi_msg msi;
> > > >
> > > > + enum dw_edma_ch_irq_mode irq_mode;
> > > > +
> > > > enum dw_edma_request request;
> > > > enum dw_edma_status status;
> > > > u8 configured;
> > > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > index 2850a9df80f5..80472148c335 100644
> > > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > @@ -256,8 +256,10 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
> > > > for_each_set_bit(pos, &val, total) {
> > > > chan = &dw->chan[pos + off];
> > > >
> > > > - dw_edma_v0_core_clear_done_int(chan);
> > > > - done(chan);
> > > > + if (!dw_edma_chan_ignore_irq(&chan->vc.chan)) {
> > > > + dw_edma_v0_core_clear_done_int(chan);
> > > > + done(chan);
> > > > + }
> > > >
> > > > ret = IRQ_HANDLED;
> > > > }
> > > > @@ -267,8 +269,10 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
> > > > for_each_set_bit(pos, &val, total) {
> > > > chan = &dw->chan[pos + off];
> > > >
> > > > - dw_edma_v0_core_clear_abort_int(chan);
> > > > - abort(chan);
> > > > + if (!dw_edma_chan_ignore_irq(&chan->vc.chan)) {
> > > > + dw_edma_v0_core_clear_abort_int(chan);
> > > > + abort(chan);
> > > > + }
> > > >
> > > > ret = IRQ_HANDLED;
> > > > }
> > > > @@ -331,7 +335,8 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > > > j--;
> > > > if (!j) {
> > > > control |= DW_EDMA_V0_LIE;
> > > > - if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> > > > + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) &&
> > > > + chan->irq_mode != DW_EDMA_CH_IRQ_LOCAL)
> > > > control |= DW_EDMA_V0_RIE;
> > > > }
> > > >
> > > > @@ -408,12 +413,17 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > > > break;
> > > > }
> > > > }
> > > > - /* Interrupt unmask - done, abort */
> > > > + /* Interrupt mask/unmask - done, abort */
> > > > raw_spin_lock_irqsave(&dw->lock, flags);
> > > >
> > > > tmp = GET_RW_32(dw, chan->dir, int_mask);
> > > > - tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
> > > > - tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> > > > + if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE) {
> > > > + tmp |= FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
> > > > + tmp |= FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> > > > + } else {
> > > > + tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
> > > > + tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> > > > + }
> > > > SET_RW_32(dw, chan->dir, int_mask, tmp);
> > > > /* Linked list error */
> > > > tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
> > > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > > > index ffad10ff2cd6..6f50165ac084 100644
> > > > --- a/include/linux/dma/edma.h
> > > > +++ b/include/linux/dma/edma.h
> > > > @@ -60,6 +60,23 @@ enum dw_edma_chip_flags {
> > > > DW_EDMA_CHIP_LOCAL = BIT(0),
> > > > };
> > > >
> > > > +/*
> > > > + * enum dw_edma_ch_irq_mode - per-channel interrupt routing control
> > > > + * @DW_EDMA_CH_IRQ_DEFAULT: LIE=1/RIE=1, local interrupt unmasked
> > > > + * @DW_EDMA_CH_IRQ_LOCAL: LIE=1/RIE=0
> > > > + * @DW_EDMA_CH_IRQ_REMOTE: LIE=1/RIE=1, local interrupt masked
> > > > + *
> > > > + * Some implementations require using LIE=1/RIE=1 with the local interrupt
> > > > + * masked to generate a remote-only interrupt (rather than LIE=0/RIE=1).
> > > > + * See the DesignWare endpoint databook 5.40, "Hint" below "Figure 8-22
> > > > + * Write Interrupt Generation".
> > > > + */
> > > > +enum dw_edma_ch_irq_mode {
> > > > + DW_EDMA_CH_IRQ_DEFAULT = 0,
> > > > + DW_EDMA_CH_IRQ_LOCAL,
> > > > + DW_EDMA_CH_IRQ_REMOTE,
> > > > +};
> > > > +
> > > > /**
> > > > * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> > > > * @dev: struct device of the eDMA controller
> > > > @@ -105,6 +122,22 @@ struct dw_edma_chip {
> > > > #if IS_REACHABLE(CONFIG_DW_EDMA)
> > > > int dw_edma_probe(struct dw_edma_chip *chip);
> > > > int dw_edma_remove(struct dw_edma_chip *chip);
> > > > +/**
> > > > + * dw_edma_chan_irq_config - configure per-channel interrupt routing
> > > > + * @chan: DMA channel obtained from dma_request_channel()
> > > > + * @mode: interrupt routing mode
> > > > + *
> > > > + * Returns 0 on success, -EINVAL for invalid @mode, or -ENODEV if @chan does
> > > > + * not belong to the DesignWare eDMA driver.
> > > > + */
> > > > +int dw_edma_chan_irq_config(struct dma_chan *chan,
> > > > + enum dw_edma_ch_irq_mode mode);
> > > > +
> > > > +/**
> > > > + * dw_edma_chan_ignore_irq - tell whether local IRQ handling should be ignored
> > > > + * @chan: DMA channel obtained from dma_request_channel()
> > > > + */
> > > > +bool dw_edma_chan_ignore_irq(struct dma_chan *chan);
> > > > #else
> > > > static inline int dw_edma_probe(struct dw_edma_chip *chip)
> > > > {
> > > > @@ -115,6 +148,17 @@ static inline int dw_edma_remove(struct dw_edma_chip *chip)
> > > > {
> > > > return 0;
> > > > }
> > > > +
> > > > +static inline int dw_edma_chan_irq_config(struct dma_chan *chan,
> > > > + enum dw_edma_ch_irq_mode mode)
> > > > +{
> > > > + return -ENODEV;
> > > > +}
> > > > +
> > > > +static inline bool dw_edma_chan_ignore_irq(struct dma_chan *chan)
> > > > +{
> > > > + return false;
> > > > +}
> > >
> > > I think it'd better go thought
> > >
> > > struct dma_slave_config {
> > > ...
> > > void *peripheral_config;
> > > size_t peripheral_size;
> > >
> > > };
> > >
> > > So DMA consumer can use standard DMAengine API, dmaengine_slave_config().
> >
> > Using .peripheral_config wasn't something I had initially considered, but I
> > agree that this is preferable in the sense that it avoids introducing the
> > additional exported APIs. I'm not entirely sure whether it's clean to use
> > it for non-peripheral settings in the strict sense, but there seem to be
> > precedents such as stm32_mdma_dma_config, so I guess it seems acceptable.
> > If I'm missing something, please correct me.
>
> Strictly speaking slave config should be used for peripheral transfers.
> For memcpy users (this seems more like that), I would argue slave config
> does not make much sense.
Thank you for the comment. Understood, so it seems outside the intended
semantics of .peripheral_config.
Now I see two possible directions:
1. Keep my original approach (i.e. add a dw-edma specific exported helper in
dw-edma-core, like dw_edma_chan_irq_config()).
2. Introduce a more generic mechanism than .peripheral_config/size (e.g.
.hw_config/size), and use that instead.
If you see a better approach, I'd be glad to hear it. Also, Mani's input on
whether or not (1) is acceptable in the overall picture would be helpful
(from a dw-edma-core maintainer perspective).
Regards,
Koichiro
>
> --
> ~Vinod
Powered by blists - more mailing lists