[PATCH 09/10] arch: introduce memremap()

[Dan Williams]

On Tue, Jul 21, 2015 at 4:58 PM, Luis R. Rodriguez <mcgrof at suse.com> wrote:
> On Sun, Jul 19, 2015 at 08:18:23PM -0400, Dan Williams wrote:
>> diff --git a/include/linux/io.h b/include/linux/io.h
>> index 080a4fbf2ba4..2983b6e63970 100644
>> --- a/include/linux/io.h
>> +++ b/include/linux/io.h
>> @@ -192,4 +192,15 @@ static inline int arch_phys_wc_index(int handle)
>>  #endif
>>  #endif
>>
>> +enum {
>> +     MEMREMAP_WB = 1 << 0,
>> +     MEMREMAP_WT = 1 << 1,
>> +     MEMREMAP_WC = 1 << 2,
>> +     MEMREMAP_STRICT = 1 << 3,
>> +     MEMREMAP_CACHE = MEMREMAP_WB,
>> +};
>
> A few things:
>
> 1) You'll need MEMREMAP_UC now as well now.

Why?  I don't think it fits.  If there are any I/O areas (non-memory)
in the range then it simply is not "memory" and should not be using
memremap().  In other words it seems like you really do need to heed
the __iomem annotation in the return value from ioremap_uc().

> 2) as you are doing all this sweep over architectures on this please
> also address the lack of ioremap_*() variant implemention to return
> NULL, ie not supported, because we've decided for now that so long
> as the semantics are not well defined we can't expect architectures
> to get it right unless they are doing the work themselves, and the
> old strategy of just #defin'ing a variant to iorempa_nocache() which
> folks tended to just can lead to issues. In your case since you are
> jumping to the flags implementation this might be knocking two birds
> with one stone.

I'm not going to do a general sweep for this as the expectation that
ioremap silently falls back if a mapping type is not supported is
buried all over the place.  That said, new usages and conversions to
memremap() can be strict about this. For now, I'm only handling
ioremap_cache() and ioremap_wt() conversions.

> 3) For the case that architectures have no MMU we currently do a direct
> mapping such as what you try to describe to do with memremap(). I wonder
> if its worth it to then enable that code to just map to memremap(). That
> throws off your usage of CONFIG_ARCH_HAS_MEMREMAP if we want to repurpose
> that implementation for no the MMU case, unless of course you just have a
> __memremap() which does the default basic direct mapping implementation.

Yes, in the next rev of this series I am having it fall back to direct
mappings where it makes sense.

> 4) Since we are all blaming semantics on our woes I'd like to ask for
> some effort on semantics to be well defined. Semantics here sholud cover
> some form of Documentation but also sparse annotation checks and perhaps
> Coccinelle grammar rules for how things should be done. This should not only
> cover general use but also if there are calls which depend on a cache
> type to have been set. If we used sparse annotations it  may meen a
> different return type for each cache type.  Not sure if we want this.
> If we went with grammar rules I'm looking to for instance have in place
> rules on scripts/coccinelle which would allow developers to use

memremap() explicitly does not want get into arch semantics debates.
The pointer returned from memremap() is a "void *" that can be used as
normal memory.  If it is a normal pointer I don't see the role for
sparse annotation.

>
> make coccicheck M=foo/
>
> to find issues. I can perhaps help with this, but we'd need to do a good
> sweep here to not only cover good territory but also get folks to agree
> on things.
>
> 5) This may be related to 4), not sure. There are aligment requirements we
> should probably iron out for architectures. How will we annotate these
> requirements or allow architectures to be pedantic over these requirements?

What requirements beyond PAGE_SIZE alignment would we need to worry about?