linux-kernel - Re: [External] Re: [PATCH] riscv: introduce the ioremap

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEEQ3wk95SMxbKQqD08ACaErG9bvUNQ9QOK6KsZSc5xGHRj+pQ@mail.gmail.com>
Date: Mon, 24 Mar 2025 09:30:22 +0800
From: yunhui cui <cuiyunhui@...edance.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>, 
	Anshuman Khandual <anshuman.khandual@....com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Ingo Molnar <mingo@...nel.org>, Catalin Marinas <catalin.marinas@....com>, 
	Ryan Roberts <ryan.roberts@....com>, 
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>, Nam Cao <namcao@...utronix.de>, 
	Björn Töpel <bjorn@...osinc.com>, 
	Stuart Menefy <stuart.menefy@...asip.com>, Xu Lu <luxu.kernel@...edance.com>, 
	Vincenzo Frascino <vincenzo.frascino@....com>, Samuel Holland <samuel.holland@...ive.com>, 
	Christophe Leroy <christophe.leroy@...roup.eu>, Dawei Li <dawei.li@...ngroup.cn>, 
	Mike Rapoport <rppt@...nel.org>, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH] riscv: introduce the ioremap_prot() function

Hi Arnd,


On Thu, Mar 20, 2025 at 5:22 PM Arnd Bergmann <arnd@...db.de> wrote:
>
> 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@...edance.com>
>
> 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

It seems that we can't simply use #define ioremap_prot
generic_ioremap_prot because some architectures have certain special
behaviors before calling generic_ioremap_prot(). For example, there's
the ioremap_prot_hook() logic on ARM64 and the CONFIG_EISA logic on
PA-RISC, among others.

Regarding the check of whether the address is a memory address, I
think we can directly incorporate pfn_valid() into
generic_ioremap_prot. This probably won't affect architectures that
directly use generic_ioremap_prot(), such as C-SKY, Hexagon, and
LoongArch.

So, my next plan is to add pfn_valid() to generic_ioremap_prot().

What's your opinion?

Thanks，
Yunhui