Message-ID: <20160308121601.GA6573@gmail.com>
Date:	Tue, 8 Mar 2016 13:16:01 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Toshi Kani <toshi.kani@....com>
Cc:	"Luis R. Rodriguez" <mcgrof@...nel.org>,
	Toshi Kani <toshi.kani@...com>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Dave Airlie <airlied@...hat.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-arch@...r.kernel.org, X86 ML <x86@...nel.org>,
	Daniel Vetter <daniel.vetter@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Borislav Petkov <bp@...en8.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Lutomirski <luto@...nel.org>,
	Brian Gerst <brgerst@...il.com>
Subject: Re: Overlapping ioremap() calls, set_memory_*() semantics


* Toshi Kani <toshi.kani@....com> wrote:

> > So where is the problem? The memtype implementation and hence most
> > ioremap() users are supposed to be safe. set_memory_*() APIs are supposed
> > to be safe as well, as they too go via the memtype API.
> 
> Let me try to summarize...
> 
> The original issue Luis brought up was that drivers written to work with
> MTRR may create a single ioremap range covering multiple cache attributes
> since MTRR can overwrite cache attribute of a certain range.  Converting
> such drivers with PAT-based ioremap interfaces, i.e. ioremap_wc() and
> ioremap_nocache(), requires a separate ioremap map for each cache
> attribute, which can be challenging as it may result in overlapping ioremap
> ranges (in his term) with different cache attributes.
> 
> So, Luis asked about 'sematics of overlapping ioremap()' calls.  Hence, I
> responded that aliasing mapping itself is supported, but alias with
> different cache attribute is not.  We have checks in place to detect such
> condition.  Overlapping ioremap calls with a different cache attribute
> either fails or gets redirected to the existing cache attribute on x86.

Ok, fair enough!

So to go back to the original suggestion from Luis, I've quoted it, but with a 
s/overlapping/aliased substitution:

> I had suggested long ago then that one possible resolution was for us to add an 
> API that *enables* aliased ioremap() calls, and only use it on select locations 
> in the kernel. This means we only have to convert a few users to that call to 
> white list such semantics, and by default we'd disable aliased calls. To kick 
> things off -- is this strategy agreeable for all other architectures?

I'd say that since the overwhelming majority of ioremap() calls are not aliased, 
ever, thus making it 'harder' to accidentally alias is probably a good idea.

The memtype infrastructure of phyisical memory ranges in that case acts as a 
security measure, to avoid unintended (not just physically incompatible) aliasing. 
I suspect it would be helpful during driver development as well.

What extra API are you thinking about to enable aliasing in the few cases where 
it's justified?

the other problem listed:

> As another problem case, set_memor_*() will not fail on MMIO even though 
> set_memor_*() is designed only for RAM.

So what does this mean exactly? Having WB caching on MMIO area is not good, but 
UC, WC and WB sure is still sensible in some cases, right?

> [...] If the above strategy on avoiding aliasing is agreeable, could the next 
> step, or an orthogonal step be to error out on set_memory_*() on IO memory?

Well, do we have drivers that nevertheless change caching attributes on MMIO 
areas?

Basically if ioremap_uc() and ioremap_wc() is allowed on MMIO areas, then I see no 
reason in principle why it should be invalid to change the area from UC to WC 
after it has been ioremap()ed.

Thanks,

	Ingo

Powered by blists - more mailing lists