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

[Luis R. Rodriguez]

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:
> > On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux 
> > wrote:
>   :
> > > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/