Re: [PATCH] riscv: introduce the ioremap_prot() function

From: Arnd Bergmann
Date: Thu Mar 20 2025 - 05:22:05 EST


On Thu, Mar 20, 2025, at 09:44, Yunhui Cui wrote:
> It's advisable to avoid mapping memory with the non-cache attribute.
> This is because issues may arise when the same physical address is
> mapped as both cacheable and non-cacheable simultaneously, such as
> in the case of hardware prefetching.
>
> Signed-off-by: Yunhui Cui <cuiyunhui@xxxxxxxxxxxxx>

Makes sense to me. Ideally we'd have the check in generic_ioremap_prot(),
but I believe this would break on (at least) x86 because of
legacy callers of ioremap() on memory.

> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> index a0e51840b9db..736c5557bd06 100644
> --- a/arch/riscv/include/asm/io.h
> +++ b/arch/riscv/include/asm/io.h
> @@ -133,6 +133,8 @@ __io_writes_outs(outs, u64, q, __io_pbr(), __io_paw())
> #define outsq(addr, buffer, count) __outsq(PCI_IOBASE + (addr), buffer, count)
> #endif
>
> +#define ioremap_prot ioremap_prot
> +
> #include <asm-generic/io.h>
>
> #ifdef CONFIG_MMU

This feels slightly wrong to me, the "#define foo foo" method
is normally used to override a declaration or inline function with
another one, but this one only overrides the implementation, not
the declaration.

I see the same is done on arc, arm64, parisc, powerpc, s390,
sh and xtensa, so we can keep this one as well, but it would be
nice to change all of these to a less surprising approach.

Maybe we should just remove these macros from asm/io.h and
the trivial wrapper from mm/ioremap.c, and instead change the
other architectures that have GENERIC_IOREMAP to use

#define ioremap_prot generic_ioremap_prot

It seems this would be only csky, hexagon, (some) loongarch
and openrisc.

Arnd