Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933273AbcCHMQg (ORCPT ); Tue, 8 Mar 2016 07:16:36 -0500 Received: from mail-wm0-f48.google.com ([74.125.82.48]:38901 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932301AbcCHMQI (ORCPT ); Tue, 8 Mar 2016 07:16:08 -0500 Date: Tue, 8 Mar 2016 13:16:01 +0100 From: Ingo Molnar To: Toshi Kani Cc: "Luis R. Rodriguez" , Toshi Kani , Paul McKenney , Dave Airlie , Benjamin Herrenschmidt , "linux-kernel@vger.kernel.org" , linux-arch@vger.kernel.org, X86 ML , Daniel Vetter , Thomas Gleixner , "H. Peter Anvin" , Peter Zijlstra , Borislav Petkov , Linus Torvalds , Andrew Morton , Andy Lutomirski , Brian Gerst Subject: Re: Overlapping ioremap() calls, set_memory_*() semantics Message-ID: <20160308121601.GA6573@gmail.com> References: <20160304094424.GA16228@gmail.com> <1457115514.15454.216.camel@hpe.com> <20160305114012.GA7259@gmail.com> <1457370228.15454.311.camel@hpe.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1457370228.15454.311.camel@hpe.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2925 Lines: 65 * Toshi Kani 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