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
