[PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases

On Tue, Jul 07, 2015 at 05:10:58PM -0600, Toshi Kani wrote:

On Tue, 2015-07-07 at 18:07 +0200, Luis R. Rodriguez wrote:

quoted

On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux 
wrote:

  :

quoted

quoted

On ARM, we (probably) have a lot of cases where ioremap() is used 
multiple
times for the same physical address space, so we shouldn't rule out 
having
multiple mappings of the same type.

Why is that done? Don't worry if you are not sure why but only 
speculate of the
practice's existence (sloppy drivers or lazy driver developers). FWIW 
for x86
IIRC I ended up concluding that overlapping ioremap() calls with the 
same type
would work but not if they differ in type.  Although I haven't 
written a
grammer rule to hunt down overlapping ioremap() I suspected its use 
was likely
odd and likely should be reconsidered. Would this be true for ARM too 
? Or are
you saying this should be a feature ? I don't expect an answer now 
but I'm
saying we *should* all together decide on this, and if you're 
inclined to
believe that this should ideally be avoided I'd like to hear that. If 
you feel
strongly though this should be a feature I would like to know why.

There are multiple mapping interfaces, and overlapping can happen among
them as well.  For instance, remap_pfn_range() (and 
io_remap_pfn_range(), which is the same as remap_pfn_range() on x86)
creates a mapping to user space. The same physical ranges may be
mapped to kernel and user spaces.  /dev/mem is one example that may
create a user space mapping to a physical address that is already
mapped with ioremap() by other module.

Thanks for the feedback. The restriction seems to be differing cache types
requirements, other than this, are there any other concerns ? For instance are
we completley happy with aliasing so long as cache types match everywhere?  I'd
expect no architecture would want cache types to differ when aliasing, what
should differ then I think would just be how to verify this and it doesn't seem
we may be doing this for all architectures.

Even for userspace we seem to be covered -- we enable userspace mmap() calls to
get their mapped space with a cache type, on the kernel we'd say use
pgprot_writecombine() on the vma->vm_page_prot prior to the
io_remap_pfn_range() -- that maps to remap_pfn_range() on x86 and as you note
that checks cache type via reserve_memtype() -- but only on x86...

Other than this differing cache type concern are we OK with aliasing in
userspace all the time ?

If we want to restrict aliasing either for the kernel or userspace mapping
we might be able to do it, I just want to know if we want to or not care
at all.

pmem and DAX also create mappings to the same NVDIMM ranges.  DAX calls
vm_insert_mixed(), which is particularly a problematic since
vm_insert_mixed() does not verify aliasing.  ioremap() and remap_pfn_range()
call reserve_memtype() to verify aliasing on x86.  reserve_memtype() is
x86-specific and there is no arch-generic wrapper for such check.

As clarified by Matthew Wilcox via commit d92576f1167cacf7844 ("dax: does not
work correctly with virtual aliasing caches") caches are virtually mapped for
some architectures, it seems it should be possible to fix this for DAX somehow
though.

I think DAX could get a cache type from pmem to keep them in sync, though.

pmem is x86 specific right now, are other folks going to expose something
similar ? Otherwise we seem to only be addressing these deep concerns for
x86 so far.

 Luis