linux-kernel - Re: [PATCH 09/10] arch: introduce memremap()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Jul 2015 21:04:22 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	"Luis R. Rodriguez" <mcgrof@...e.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>, linux-arch@...r.kernel.org,
	Tony Luck <tony.luck@...el.com>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Arnd Bergmann <arnd@...db.de>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Andy Shevchenko <andy.shevchenko@...il.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	"Kani, Toshimitsu" <toshi.kani@...com>,
	Ross Zwisler <ross.zwisler@...ux.intel.com>,
	Christoph Hellwig <hch@....de>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 09/10] arch: introduce memremap()

On Tue, Jul 21, 2015 at 4:58 PM, Luis R. Rodriguez <mcgrof@...e.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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/