Message-ID: <2b6pgqupd7uv5la5h52zczdfkpbtnn3xbz2oqjhqpyiqv6ew35@t2vb7vteespn>
Date:   Thu, 30 Nov 2023 22:26:20 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Jiaxun Yang <jiaxun.yang@...goat.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Mike Rapoport <rppt@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Huacai Chen <chenhuacai@...nel.org>,
        Yinglu Yang <yangyinglu@...ngson.cn>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Aleksandar Rikalo <aleksandar.rikalo@...mia.com>,
        Aleksandar Rikalo <arikalo@...il.com>,
        Dragan Mladjenovic <dragan.mladjenovic@...mia.com>,
        Chao-ying Fu <cfu@...ecomp.com>, Marc Zyngier <maz@...nel.org>,
        "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] mips: dmi: Fix early remap on MIPS32

On Tue, Nov 28, 2023 at 10:59:10PM +0100, Arnd Bergmann wrote:
> On Tue, Nov 28, 2023, at 14:52, Serge Semin wrote:
> > On Tue, Nov 28, 2023 at 01:41:51PM +0100, Arnd Bergmann wrote:
> >> On Mon, Nov 27, 2023, at 17:23, Serge Semin wrote:
> >> > On Fri, Nov 24, 2023 at 10:03:49PM +0000, Jiaxun Yang wrote:
> >> but ioremap_cache() is generally underspecified because the
> >> resulting pointer is neither safe to dereference nor to pass into
> >> readl()/writel()/memcpy_fromio() on all architectures.
> >
> > I don't know about ARM64 (which for instance has it utilized to access
> > the DMI region), but at least in case of MIPS32 (a fortiori MIPS64
> > seeing the ioremap_cache() method actually returns a pointer to the
> > uncached region) I don't see a reason why it wouldn't be safe in both
> > cases described by you. All IO and memory regions are accessed by the
> > generic load and store instructions. The only difference is that the
> > MMIO-space accessors normally implies additional barriers, which just
> > slow down the execution, but shouldn't cause any other problem. Could
> > you clarify why do you think otherwise?
> 
> On arch/powerpc, CONFIG_PPC_INDIRECT_MMIO makes all ioremap()
> type functions return a token that can be passed into the readl/writel
> family but that is not a pointer you can dereference.
> 
> On s390, the mechanism is different, but similarly __iomem
> tokens are not pointers at all.

Ah, you meant that it was not generically safe. Then your were correct
for sure. I was talking about the MIPS arch, which doesn't
differentiate normal and IO memory pointers: all of them are accessed
by the same instructions. So ioremap_prot() returns just a normal
pointer there, which can be safely de-referenced.

> 
> >> There was an effort to convert the remaining ioremap_cache() calls
> >> into memremap() a few years ago, not sure if that's still being worked
> >> on but it would be the right thing to do.
> >
> > I see. Thanks for the pointing out to that. I guess it could be done
> > for MIPS too (at least on our MIPS32 platform DMI is just a memory
> > region pre-initialized by the bootloader), but the conversion would
> > require much efforts. Alas currently I can't afford to get it
> > implemented in the framework of this patchset. (I saved your note in
> > my MIPS TODO list though. Let's hope eventually I'll be able to get
> > back to this topic.)
> 

> I just noticed that the only architectures that actually provide
> ioremap_cache() are x86, arm, arm64, mips, loongarch, powerpc, sh
> and xtensa. The ones that have ACPI support still definitely
> need it, most of the other ones can probably be fixed without
> too much trouble.

Ok. Thanks. I'll have a look at that on my free time.

-Serge(y)

> 
>      Arnd

Powered by blists - more mailing lists