| 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: <2akk754wgro2fgs6wradkqtuzu3tfq5klidvy4qhosnk6lbgqs@7c7thld5kqn3>
Date: Thu, 5 Feb 2026 15:48:30 +0900
From: Koichiro Den <den@...inux.co.jp>
To: Frank Li <Frank.li@....com>
Cc: vkoul@...nel.org, mani@...nel.org, jingoohan1@...il.com,
lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com,
dmaengine@...r.kernel.org, linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 03/11] dmaengine: dw-edma: Add per-channel interrupt
routing control
On Wed, Feb 04, 2026 at 12:42:41PM -0500, Frank Li wrote:
> On Wed, Feb 04, 2026 at 11:54:31PM +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 dw_edma_peripheral_config that holds per-channel interrupt
> > routing mode. 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 peripheral_config is explicitly
> > used and set, no functional changes.
> >
> > For now, HDMA is not supported for the peripheral_config. Until the
> > support is implemented and validated, explicitly reject it for HDMA to
> > avoid silently misconfiguring interrupt routing.
> >
> > Signed-off-by: Koichiro Den <den@...inux.co.jp>
> > ---
> > drivers/dma/dw-edma/dw-edma-core.c | 24 +++++++++++++++++++++++
> > drivers/dma/dw-edma/dw-edma-core.h | 13 +++++++++++++
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 26 +++++++++++++++++--------
> > include/linux/dma/edma.h | 28 +++++++++++++++++++++++++++
> > 4 files changed, 83 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 38832d9447fd..b4cb02d545bd 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -224,6 +224,29 @@ static int dw_edma_device_config(struct dma_chan *dchan,
> > struct dma_slave_config *config)
> > {
> > struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > + const struct dw_edma_peripheral_config *pcfg;
> > +
> > + /* peripheral_config is optional, default keeps legacy behaviour. */
> > + chan->irq_mode = DW_EDMA_CH_IRQ_DEFAULT;
> > +
> > + if (config->peripheral_config) {
> > + if (chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE)
> > + return -EOPNOTSUPP;
> > +
> > + if (config->peripheral_size < sizeof(*pcfg))
> > + return -EINVAL;
>
> It is good to check here.
>
> > +
> > + pcfg = config->peripheral_config;
>
> save whole peripheral_config in case need more special peripheral
> configuration in future.
Ok, while I initially thought a deep copy (snapshot) was unnecessary for
now, I agree it makes future extensions easier. I'll do so.
>
> > + switch (pcfg->irq_mode) {
> > + case DW_EDMA_CH_IRQ_DEFAULT:
> > + case DW_EDMA_CH_IRQ_LOCAL:
> > + case DW_EDMA_CH_IRQ_REMOTE:
> > + chan->irq_mode = pcfg->irq_mode;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > + }
>
> use helper function to get irq_mode. [...]
Ok, my current plan is to keep chan->irq_mode as sticky per-channel state,
and factor out the parsing/validation of irq_mode (from
config->peripheral_config) into a small helper used by
dw_edma_device_config() and the new prep_config path.
Does this match what you meant by "helper function"?
> [...] I posted combine config and prep by
> one call.
>
> https://lore.kernel.org/dmaengine/20260105-dma_prep_config-v3-0-a8480362fd42@nxp.com/
>
> So we use such helper to get irq node after above patch merge. It is not
> big deal, I can change it later. If provide helper funtions, it will be
> slice better.
>
> >
> > memcpy(&chan->config, config, sizeof(*config));
> > chan->configured = true;
> > @@ -750,6 +773,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);
> ...
> >
> > +/*
> > + * 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
>
> keep consistent after "," for each enum
Ok, will add ", local interrupt unmasked" for it.
Thanks for the review,
Koichiro
>
> Frank
>
> > + * @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_peripheral_config - dw-edma specific slave configuration
> > + * @irq_mode: per-channel interrupt routing control.
> > + *
> > + * Pass this structure via dma_slave_config.peripheral_config and
> > + * dma_slave_config.peripheral_size.
> > + */
> > +struct dw_edma_peripheral_config {
> > + enum dw_edma_ch_irq_mode irq_mode;
> > +};
> > +
> > /**
> > * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> > * @dev: struct device of the eDMA controller
> > --
> > 2.51.0
> >
Powered by blists - more mailing lists