On Sun, Dec 24, 2017 at 9:55 AM, christophe leroy
[off-list ref] wrote:

Le 23/12/2017 à 16:57, Guenter Roeck a écrit :

quoted

On 12/23/2017 05:48 AM, Greg KH wrote:

quoted

On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:

quoted

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