On Sun, Dec 24, 2017 at 9:55 AM, christophe leroy christophe.leroy@c-s.fr wrote:
Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
On 12/23/2017 05:48 AM, Greg KH wrote:
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
Hi all,
When I tried to use devm_ioremap function and review related code, I found devm_ioremap and devm_ioremap_nocache is almost the same with each other, except one use ioremap while the other use ioremap_nocache.
For all arches? Really? Look at MIPS, and x86, they have different functions.
Both mips and x86 end up mapping the same function, but other arches don't. mn10300 is one where ioremap and ioremap_nocache are definitely different.
alpha: identical arc: identical arm: identical arm64: identical cris: different <== frv: identical hexagone: identical ia64: different <== m32r: identical m68k: identical metag: identical microblaze: identical mips: identical mn10300: different <== nios: identical openrisc: different <== parisc: identical riscv: identical s390: identical sh: identical sparc: identical tile: identical um: rely on asm/generic unicore32: identical x86: identical asm/generic (no mmu): identical
So 4 among all arches seems to have ioremap() and ioremap_nocache() being different.
Could we have a define set by the 4 arches on which ioremap() and ioremap_nocache() are different, something like HAVE_DIFFERENT_IOREMAP_NOCACHE ?
I wonder if those are actually correct or not. What I found looking at those architectures:
- openrisc only has one driver using ioremap (drivers/net/ethernet/ethoc.c) and that calls ioremap_nocache(). Presumably the authors went with the implementation for ioremap that made sense (using default attributes) rather than the one that actually works (using uncached).
- On ia64, ioremap() checks the attributes for the physical address based on firmware tables and then picks either cached or uncached mappings. ioremap_nocache() does the same but returns NULL instead of a cached mapping for anything that is not an MMIO address. Presumably it would just work to always call ioremap().
- mn10300 appears to be wrong, broken by David Howells in commit 83c2dc15ce82 ("MN10300: Handle cacheable PCI regions in pci_iomap()") for any driver calling ioremap() by to get uncached memory, if I understand the comment for commit 34f1bdee1910 ("mn10300: switch to GENERIC_PCI_IOMAP") correctly: it seems that PCI addresses include the 'uncached' bit by default to get the right behavior, but dropping that bit breaks it.
- cris seems similar to mn10300 in hardware, using an phys address bit for uncached access. There are two callers in arch code that appear to rely on the cachable output of ioremap() arch/cris/arch-v32/kernel/signal.c: __ioremap_prot(virt_to_phys(data), PAGE_SIZE, PAGE_SIGNAL_TRAMPOLINE); arch/cris/arch-v32/mm/intmem.c: intmem_virtual = ioremap(MEM_INTMEM_START + RESERVED_SIZE, It's unclear whether ioremap_nocache() actually has any users on cris, or whether it was only added for compile-time testing, and calling plain ioremap() would always work too (assuming we pass the phys address with the uncached-bit set).
Arnd