| 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
| ||
|
Date: Fri, 25 Mar 2022 13:58:40 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc: Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
Vinod Koul <vkoul@...nel.org>,
Jingoo Han <jingoohan1@...il.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Frank Li <Frank.Li@....com>,
Serge Semin <fancer.lancer@...il.com>,
Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Rob Herring <robh@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
linux-pci@...r.kernel.org, dmaengine@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 20/25] dmaengine: dw-edma: Use non-atomic io-64 methods
On Thu, Mar 24, 2022 at 04:48:31AM +0300, Serge Semin wrote:
> Instead of splitting the 64-bits IOs up into two 32-bits ones it's
> possible to use an available set of the non-atomic readq/writeq methods
> implemented exactly for such cases. They are defined in the dedicated
> header files io-64-nonatomic-lo-hi.h/io-64-nonatomic-hi-lo.h. So in case
> if the 64-bits readq/writeq methods are unavailable on some platforms at
> consideration, the corresponding drivers can have any of these headers
> included and stop locally re-implementing the 64-bits IO accessors taking
> into account the non-atomic nature of the included methods. Let's do that
> in the DW eDMA driver too. Note by doing so we can discard the
> CONFIG_64BIT config ifdefs from the code. Also note that if a platform
> doesn't support 64-bit DBI IOs then the corresponding accessors will just
> directly call the lo_hi_readq()/lo_hi_writeq() methods.
>
> Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> ---
> drivers/dma/dw-edma/dw-edma-v0-core.c | 71 +++++++++------------------
> 1 file changed, 24 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 6b303d5a6b2a..ebb860e19c75 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -8,6 +8,8 @@
>
> #include <linux/bitfield.h>
>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> #include "dw-edma-v0-regs.h"
> @@ -53,8 +55,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> SET_32(dw, rd_##name, value); \
> } while (0)
>
> -#ifdef CONFIG_64BIT
> -
> #define SET_64(dw, name, value) \
> writeq(value, &(__dw_regs(dw)->name))
>
> @@ -80,8 +80,6 @@ static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> SET_64(dw, rd_##name, value); \
> } while (0)
>
> -#endif /* CONFIG_64BIT */
> -
> #define SET_COMPAT(dw, name, value) \
> writel(value, &(__dw_regs(dw)->type.unroll.name))
>
> @@ -164,14 +162,13 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> #define SET_LL_32(ll, value) \
> writel(value, ll)
>
> -#ifdef CONFIG_64BIT
> -
> static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> u64 value, void __iomem *addr)
> {
> + unsigned long flags;
> +
> if (dw->chip->mf == EDMA_MF_EDMA_LEGACY) {
> u32 viewport_sel;
> - unsigned long flags;
>
> raw_spin_lock_irqsave(&dw->lock, flags);
>
> @@ -181,22 +178,25 @@ static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
>
> writel(viewport_sel,
> &(__dw_regs(dw)->type.legacy.viewport_sel));
> + }
> +
> + if (dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI)
> + lo_hi_writeq(value, addr);
> + else
> writeq(value, addr);
>
> + if (dw->chip->mf == EDMA_MF_EDMA_LEGACY)
> raw_spin_unlock_irqrestore(&dw->lock, flags);
> - } else {
> - writeq(value, addr);
> - }
> }
>
> static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> const void __iomem *addr)
> {
> - u32 value;
> + unsigned long flags;
> + u64 value;
>
> if (dw->chip->mf == EDMA_MF_EDMA_LEGACY) {
> u32 viewport_sel;
> - unsigned long flags;
>
> raw_spin_lock_irqsave(&dw->lock, flags);
>
> @@ -206,12 +206,15 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
>
> writel(viewport_sel,
> &(__dw_regs(dw)->type.legacy.viewport_sel));
> + }
> +
> + if (dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI)
> + value = lo_hi_readq(addr);
> + else
> value = readq(addr);
>
> + if (dw->chip->mf == EDMA_MF_EDMA_LEGACY)
> raw_spin_unlock_irqrestore(&dw->lock, flags);
> - } else {
> - value = readq(addr);
> - }
>
> return value;
> }
> @@ -225,8 +228,6 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> #define SET_LL_64(ll, value) \
> writeq(value, ll)
>
> -#endif /* CONFIG_64BIT */
> -
> /* eDMA management callbacks */
> void dw_edma_v0_core_off(struct dw_edma *dw)
> {
> @@ -325,19 +326,10 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> /* Transfer size */
> SET_LL_32(&lli[i].transfer_size, child->sz);
> /* SAR */
> - #ifdef CONFIG_64BIT
> - SET_LL_64(&lli[i].sar.reg, child->sar);
> - #else /* CONFIG_64BIT */
> - SET_LL_32(&lli[i].sar.lsb, lower_32_bits(child->sar));
> - SET_LL_32(&lli[i].sar.msb, upper_32_bits(child->sar));
> - #endif /* CONFIG_64BIT */
> + SET_LL_64(&lli[i].sar.reg, child->sar);
This macro still uses writeq(), that's not available on 32bit platforms.
Am I missing anything?
Thanks,
Mani
> /* DAR */
> - #ifdef CONFIG_64BIT
> - SET_LL_64(&lli[i].dar.reg, child->dar);
> - #else /* CONFIG_64BIT */
> - SET_LL_32(&lli[i].dar.lsb, lower_32_bits(child->dar));
> - SET_LL_32(&lli[i].dar.msb, upper_32_bits(child->dar));
> - #endif /* CONFIG_64BIT */
> + SET_LL_64(&lli[i].dar.reg, child->dar);
> +
> i++;
> }
>
> @@ -349,12 +341,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> /* Channel control */
> SET_LL_32(&llp->control, control);
> /* Linked list */
> - #ifdef CONFIG_64BIT
> - SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
> - #else /* CONFIG_64BIT */
> - SET_LL_32(&llp->llp.lsb, lower_32_bits(chunk->ll_region.paddr));
> - SET_LL_32(&llp->llp.msb, upper_32_bits(chunk->ll_region.paddr));
> - #endif /* CONFIG_64BIT */
> + SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
> }
>
> void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> @@ -417,18 +404,8 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> SET_CH_32(dw, chan->dir, chan->id, ch_control1,
> (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
> /* Linked list */
> - if ((chan->dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
> - !IS_ENABLED(CONFIG_64BIT)) {
> - SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
> - lower_32_bits(chunk->ll_region.paddr));
> - SET_CH_32(dw, chan->dir, chan->id, llp.msb,
> - upper_32_bits(chunk->ll_region.paddr));
> - } else {
> - #ifdef CONFIG_64BIT
> - SET_CH_64(dw, chan->dir, chan->id, llp.reg,
> - chunk->ll_region.paddr);
> - #endif
> - }
> + SET_CH_64(dw, chan->dir, chan->id, llp.reg,
> + chunk->ll_region.paddr);
> }
> /* Doorbell */
> SET_RW_32(dw, chan->dir, doorbell,
> --
> 2.35.1
>
Powered by blists - more mailing lists