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

On Thu, 2015-07-09 at 03:40 +0200, Luis R. Rodriguez wrote:

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

quoted

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.

Yes, we allow to create multiple mappings to a same physical page as
long as their cache type is the same.  There are multiple use-cases
that depend on this ability.

quoted

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 simply described this DAX case as an example of how two modules might
request different cache types.  Yes, we should be able to fix this
case.

quoted

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.

pmem is a generic driver and is not x86-specific. 

Thanks,
-Toshi