Re: [PATCH 1/7] mips: dmi: Fix early remap on MIPS32

From: Arnd Bergmann
Date: Tue Nov 28 2023 - 16:59:50 EST


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.

>> 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.

Arnd