Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Thu, 30 Nov 2023 22:26:20 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     Jiaxun Yang <jiaxun.yang@flygoat.com>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        Mike Rapoport <rppt@kernel.org>,
        Matthew Wilcox <willy@infradead.org>,
        Tiezhu Yang <yangtiezhu@loongson.cn>,
        Huacai Chen <chenhuacai@kernel.org>,
        Yinglu Yang <yangyinglu@loongson.cn>,
        Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
        Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>,
        Aleksandar Rikalo <arikalo@gmail.com>,
        Dragan Mladjenovic <dragan.mladjenovic@syrmia.com>,
        Chao-ying Fu <cfu@wavecomp.com>, Marc Zyngier <maz@kernel.org>,
        "linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/7] mips: dmi: Fix early remap on MIPS32
Message-ID: <2b6pgqupd7uv5la5h52zczdfkpbtnn3xbz2oqjhqpyiqv6ew35@t2vb7vteespn>
References: <ZV9Fq1ihUm1Rn6yO@alpha.franken.de>
 <d6d7e27a-b1a1-48af-be6c-aa9097c48992@app.fastmail.com>
 <ZV94rifAIF2p9Nej@alpha.franken.de>
 <245d3985-9085-4be0-8c74-d95d06334584@app.fastmail.com>
 <3iksuovvsln3cw3xpmjd7f7xixfvwaneu4ok56fnookvyolpco@wrxxew3thgnq>
 <dfda70b6-3291-462f-bc87-06dcc87bd068@app.fastmail.com>
 <ysij22pivneyg7tk3bv3hti3tsgbzglb6pin3my7r3bokzxjj6@jrjmu45gbupr>
 <c1c0a409-902e-4609-ae84-8939226b4fa0@app.fastmail.com>
 <fvbe4625dgh57c3njx7fhd6vlnfxynzipfz43ieu2txflc2q4r@xzvrrmmktxsb>
 <00c225bf-ed99-4721-9d6a-1e58cdffc79f@app.fastmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <00c225bf-ed99-4721-9d6a-1e58cdffc79f@app.fastmail.com>
Precedence: bulk

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