From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Luis R. Rodriguez" Subject: Re: RFC: default ioremap_*() variant defintions Date: Tue, 7 Jul 2015 10:19:28 +0200 Message-ID: <20150707081928.GO7021@wotan.suse.de> References: <20150703181709.GD7021@wotan.suse.de> <20150703190924.GZ7557@n2100.arm.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20150703190924.GZ7557@n2100.arm.linux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" To: Russell King - ARM Linux Cc: linux-arch@vger.kernel.org, Toshi Kani , Arnd Bergmann , "Luis R. Rodriguez" , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , x86@kernel.org, Borislav Petkov , linuxppc-dev@lists.ozlabs.org, Ingo Molnar , linux-arm-kernel@lists.infradead.org List-Id: linux-arch.vger.kernel.org T24gRnJpLCBKdWwgMDMsIDIwMTUgYXQgMDg6MDk6MjRQTSArMDEwMCwgUnVzc2VsbCBLaW5nIC0g QVJNIExpbnV4IHdyb3RlOgo+IE9uIEZyaSwgSnVsIDAzLCAyMDE1IGF0IDA4OjE3OjA5UE0gKzAy MDAsIEx1aXMgUi4gUm9kcmlndWV6IHdyb3RlOgo+ID4gVGhlIDAtZGF5IGJ1aWxkIGJvdCBkZXRl Y3RlZCBhIGJ1aWxkIGlzc3VlIG9uIGEgcGF0Y2ggbm90IHVwc3RyZWFtIHlldCB0aGF0Cj4gPiBt YWtlcyBhIGRyaXZlciB1c2UgaW9yZW1wYV91YygpLCB0aGlzIGNhbGwgaXMgbm93IHVwc3RyZWFt IGJ1dCB3ZSBoYXZlIG5vCj4gPiBkcml2ZXJzIHlldCB1c2luZyBpdCwgdGhlIHBhdGNoIGluIHF1 ZXN0aW9uIG1ha2VzIHRoZSBhdHlmYiBmcmFtZWJ1ZmZlcgo+ID4gZHJpdmVyIHVzZSBpdC4gVGhl IGJ1aWxkIGlzc3VlIHdhcyB0aGUgbGFjayBvZiB0aGUgaW9yZW1hcF91YygpIGNhbGwgYmVpbmcK PiA+IGltcGxlbWVudGVkIG9uIHNvbWUgbm9uLXg4NiBhcmNoaXRlY3R1cmVzLgo+IAo+IFlvdSBo YXZlIHRvIGJlIGNhcmVmdWwgaGVyZS4gIFdlJ3ZlIGJlZW4gdGhyb3VnaCB0aGlzIHdpdGggaW9y ZW1hcF93dCgpCj4gd2hpY2ggaXMgaW5jb3JyZWN0IGZvciBBUk0gYXMgdGhpbmdzIHN0YW5kIGF0 IHRoZSBtb21lbnQgKEkgaGF2ZSBwYXRjaGVzCj4gd2hpY2ggYWRkcyBkb2N1bWVudGF0aW9uIG9u IHRoaXMgaXNzdWUsIGFuZCBmaXhlcyBpb3JlbWFwX3d0KCkuKQoKVGhhbmtzLCBpdCBzZWVtcyB0 aGF0IGdvaW5nIGluIGZvciB2NC4yIGlzIHRoYXQgcmlnaHQ/Cgo+IFRoZSBxdWVzdGlvbiBpcyB3 aGV0aGVyIHRoZSBhbGxvY2F0ZWQgbWVtb3J5IG1heSBoYXZlIHVuYWxpZ25lZCBhY2Nlc3Nlcwo+ IHBlcmZvcm1lZCBvbiBpdCAtIGVpdGhlciBpbnRlbnRpb25hbGx5LCBvciB1bmludGVudGlvbmFs bHkgKGVnLCBieSBHQ0MKPiBpbmxpbmluZyBtZW1jcHkoKS4pCj4gCj4gSWYgdGhlIGFuc3dlciB0 byB0aGF0IHF1ZXN0aW9uIGlzIHllcyBvciBwb3NzaWJseSB5ZXMsIG5laXRoZXIgaW9yZW1hcCgp Cj4gbm9yIGlvcmVtYXBfbm9jYWNoZSgpICh3aGljaCBpcywgdW5mb3J0dW5hdGVseSwgdGhlIHNh bWUgYXMgaW9yZW1hcCgpIGR1ZQo+IHRvIHZhcmlvdXMgZG9jdW1lbnRhdGlvbiB0ZWxsaW5nIHBl b3BsZSB0byB1c2UgaXQgZm9yIGRldmljZXMpIGNhbiBiZSB1c2VkCj4gZm9yIHRoZSBtYXBwaW5n IG9uIEFSTS4KClRoZXJlIGlzIGEgZGlmZmVyZW5jZSBiZXR3ZWVuIHVuYWxpZ25lZCBhY2Nlc3Nl cyBiZWluZyBhbiBpc3N1ZSBmb3IgYW4KYXJjaGl0ZWN0dXJlIChub3QgSEFWRV9FRkZJQ0lFTlRf VU5BTElHTkVEX0FDQ0VTUykgb24gYSBtYXBwaW5nIGFyZWEgVnMgaGF2aW5nCmEgcmVxdWlyZW1l bnQgZm9yIGFsbCBkZXZpY2UgZHJpdmVycyB0byBub3QgaGF2ZSB1bmFsaWduZWQgYWNjZXNzZXMg Zm9yIG1hcHBlZAptZW1vcnksIGl0IHNlZW1zIHdlIHdhbnQgdGhlIGxhdGVyIGFueXdheS4uLiBo b3dldmVyIHdlIG9idmlvdXNseSBkb24ndCBoYXZlIGEKc2NyYXBlciB0byBnbyBhbmQgdmV0IGFs bCBkcml2ZXJzIC8gY2hlY2sgYSBkcml2ZXIgaXQgc2VlbXMuIEhybSwgSSB3b25kZXIKaWYgd2Ug Y2FuIHVzZSBncmFtbWFyIGNoZWNrcyBmb3IgdGhpcy4gQW55d2F5LCBPSyBnb3QgaXQhCgo+IFNv IHdlIGNhbid0IGdvIGFyb3VuZCBhZGRpbmcgbmV3IGlvcmVtYXAoKSB2YXJpYW50cyBhbmQgaG9w aW5nIHRoYXQKPiBpb3JlbWFwX25vY2FjaGUoKSBpcyBhIHNhZmUgZGVmYXVsdCBldmVyeXdoZXJl LiAgSXQgaXNuJ3QuCgpTdXJlLCBPSy4KCj4gPiBJICp0aG91Z2h0KiBJIGhhZCBhZGRlZCBib2ls ZXIgcGxhdGUgY29kZSB0byBtYXAKPiA+IHRoZSBpb3JlbWFwX3VjKCkgY2FsbCB0byBpb3JlbWFw X25vY2FjaGUoKSBmb3IgYXJjaHMgdGhhdCBkbyBub3QgYWxyZWFkeSBkZWZpbmUKPiA+IHRoZWly IG93biBpb3JlbXBhX3VjKCkgY2FsbCwgYnV0IHVwb24gZnVydGhlciBpbnZlc3RpZ2F0aW9uIGl0 IHNlZW1zIHRoYXQgd2FzCj4gPiBub3QgdGhlIGNhc2UgYnV0IGZvdW5kIHRoYXQgdGhpcyBtYXkg YmUgYSBiaXQgZGlmZmVyZW50IGlzc3VlIGFsdG9nZXRoZXIuCj4gPiAKPiA+IFRoZSB3YXkgaW5j bHVkZS9hc20tZ2VuZXJpYy9pby5oIHdvcmtzIGZvciBpb3JlbWFwKCkgY2FsbHMgYW5kIGl0cyB2 YXJpYW50cyBpczoKPiA+IAo+ID4gI2lmbmRlZiBDT05GSUdfTU1VICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAKPiA+ICNpZm5kZWYg aW9yZW1hcCAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgCj4gPiAjZGVmaW5lIGlvcmVtYXAgaW9yZW1hcCAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIAo+ID4gc3RhdGljIGlu bGluZSB2b2lkIF9faW9tZW0gKmlvcmVtYXAocGh5c19hZGRyX3Qgb2Zmc2V0LCBzaXplX3Qgc2l6 ZSkgICAgICAgICAgICAKPiA+IHsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgCj4gPiAgICAgICAgIHJl dHVybiAodm9pZCBfX2lvbWVtICopKHVuc2lnbmVkIGxvbmcpb2Zmc2V0OyAgICAgICAgICAgICAg ICAgICAgICAgICAgIAo+ID4gfSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAKPiA+ICNlbmRpZiAKPiA+ IC4uLgo+ID4gI2RlZmluZSBpb3VubWFwIGlvdW5tYXAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAKPiA+ICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgCj4gPiBzdGF0aWMgaW5saW5lIHZvaWQgaW91bm1hcCh2b2lkIF9faW9tZW0gKmFkZHIpICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIAo+ID4geyAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAKPiA+IH0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgCj4gPiAjZW5kaWYgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg IAo+ID4gI2VuZGlmIC8qIENPTkZJR19NTVUgKi8gIAo+IAo+ID4gVGhhdCdzIHRoZSBnaXN0IG9m IGl0LCBidXQgdGhlIGNhdGNoIGhlcmUgaXMgdGhlIGlvcmVtYXBfKigpIHZhcmlhbnRzIGFuZCB3 aGVyZQo+ID4gdGhleSBhcmUgZGVmaW5lZC4gVGhlIGZpcnN0IHZhcmlhbnQgaXMgaW9yZW1hcF9u b2NhY2hlKCkgYW5kIHRoZW4gYWxsIG90aGVyCj4gPiB2YXJpYW50cyBieSBkZWZhdWx0IG1hcCB0 byB0aGlzIG9uZS4gV2UndmUgYmVlbiBzdHVmZmluZyB0aGUgdmFyaWFudCBkZWZpbml0aW9ucwo+ ID4gd2l0aGluIHRoZSAjaWZuZGVmIENPTkZJR19NTVUgYW5kIEkgZG9uJ3QgdGhpbmsgd2Ugc2hv dWxkIGJlIGFzIG90aGVyd2lzZSBlYWNoCj4gPiBhbmQgZXZlcnlvbmUncyBhcmNocyB3aWxsIGhh dmUgdG8gYWRkIHRoZWlyIG93biB2YXJpYW50IGRlZmF1bHQgbWFwIHRvIHRoZQo+ID4gZGVmYXVs dCBpb3JlbWFwX25vY2FjaGUoKSBvciB3aGF0ZXZlci4gVGhhdCdzIGV4YWNsdHkgd2hhdCB3ZSBo YXZlIHRvIGRheSwgYW5kCj4gPiBmcm9tIHdoYXQgSSBnYXRoZXIgdGhlIHB1cnBvc2Ugb2YgdGhl IHZhcmlhbnQgYm9pbGVyIHBsYXRlIGlzIGxvc3QuIEkgdGhpbmsKPiA+IHdlIHNob3VsZCBrZWVw IHRoZSBpb3JlbWFwKCkgYW5kIGlvcmV1bm1hcCgpIHR1Y2tlZCB1bmRlciB0aGUgQ09ORklHX01N VQo+ID4gYnV0IG5vdCB0aGUgdmFyaWFudHMuIEZvciBpbnN0YW5jZSB0byBhZGRyZXNzIHRoZSBi dWlsZCBpc3N1ZSBmb3IgaW9yZW1hcF91YygpCj4gPiB3ZSBlaXRoZXIgZGVmaW5lIGlvcmVtYXBf dWMoKSBmb3IgYWxsIGFyY2hzIG9yIGRvIHNvbWV0aGluZyBsaWtlIHRoaXM6Cj4gCj4gU3VyZWx5 LCBpZiBhcmNoaXRlY3R1cmVzIGNhbiBzdXBwb3J0IHRoZSBkaWZmZXJlbnQgdmFyaWFudHMsIHRo ZW4gdGhleQo+IHNob3VsZCBpbXBsZW1lbnQgdGhlbT8KClN1cmUsIHRoZSBhc20tZ2VuZXJpYyBm aWxlIHNlZW1lZCB0byBhbGx1ZGUgdG8gdGhhdCBzb21lIGlvcmVtYXBzIGFyZSBzYWZlCnRvIGhh dmUgZGVmYXVsdHMgZm9yLCBpZiB3ZSB3YW50IHRvIGFkZHJlc3MgdGhpcyBhcyBhIHByb2JsZW0g aXQgbXVzdApiZSBhZGRyZXNzZWQgdGhlcmUsIHRoaXMgaXMgd2hhdCBteSBwb3N0aW5nIGFpbXMg dG8gYWRkcmVzcy4gSXQgc2VlbXMKdGhlIG90aGVyIHRocmVhZHMgd2l0aCBEYW4gYWxzbyBhZGRy ZXNzIHNvbWUgb2YgdGhpcyBzbyB3ZSdyZSBhbHJlYWR5Cm9uIHRyYWNrLgoKPiBUaGUgb25seSBj YXNlIHdoZXJlIGl0IG1ha2VzIHNlbnNlIHRvIHByb3ZpZGUgYm9pbGVycGxhdGUgZm9yIHRoZXNl IGlzCj4gd2hlbiBhcmNoaXRlY3R1cmVzIGRvbid0IHN1cHBvcnQgdGhlIG1hcHBpbmcgdHlwZSAt IGFuZCB0aGVuIHdlIG5lZWQgaGVscAo+IGZyb20gYXJjaGl0ZWN0dXJlIHBlb3BsZSB0byBkZWNp ZGUgd2hhdCBpcyB0aGUgYXBwcm9wcmlhdGUgbWFwcGluZyB0eXBlCj4gKGFuZCB0aGVyZWZvcmUg d2hpY2ggaW9yZW1hcCgpIHZhcmlhbnQpIHRvIGJlIHVzZWQgYXMgYSBmYWxsYmFjay4KClNvdW5k cyBsaWtlIGEgZm9ybWFsIHByb2Nlc3MgYmVpbmcgYnJld2VkIC8gZG9jdW1lbnRlZC4KCiAgTHVp cwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpMaW51eHBw Yy1kZXYgbWFpbGluZyBsaXN0CkxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnCmh0dHBzOi8v bGlzdHMub3psYWJzLm9yZy9saXN0aW5mby9saW51eHBwYy1kZXY= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cantor2.suse.de ([195.135.220.15]:41952 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751714AbbGGITa (ORCPT ); Tue, 7 Jul 2015 04:19:30 -0400 Date: Tue, 7 Jul 2015 10:19:28 +0200 From: "Luis R. Rodriguez" Subject: Re: RFC: default ioremap_*() variant defintions Message-ID: <20150707081928.GO7021@wotan.suse.de> References: <20150703181709.GD7021@wotan.suse.de> <20150703190924.GZ7557@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150703190924.GZ7557@n2100.arm.linux.org.uk> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Russell King - ARM Linux Cc: linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, x86@kernel.org, Toshi Kani , Arnd Bergmann , "Luis R. Rodriguez" , Benjamin Herrenschmidt , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Borislav Petkov , Ingo Molnar Message-ID: <20150707081928.0frUxA50yBMoVsc-uu33ZcTqdsJLkONIsfNevPn74Oo@z> On Fri, Jul 03, 2015 at 08:09:24PM +0100, Russell King - ARM Linux wrote: > On Fri, Jul 03, 2015 at 08:17:09PM +0200, Luis R. Rodriguez wrote: > > The 0-day build bot detected a build issue on a patch not upstream yet that > > makes a driver use iorempa_uc(), this call is now upstream but we have no > > drivers yet using it, the patch in question makes the atyfb framebuffer > > driver use it. The build issue was the lack of the ioremap_uc() call being > > implemented on some non-x86 architectures. > > You have to be careful here. We've been through this with ioremap_wt() > which is incorrect for ARM as things stand at the moment (I have patches > which adds documentation on this issue, and fixes ioremap_wt().) Thanks, it seems that going in for v4.2 is that right? > The question is whether the allocated memory may have unaligned accesses > performed on it - either intentionally, or unintentionally (eg, by GCC > inlining memcpy().) > > If the answer to that question is yes or possibly yes, neither ioremap() > nor ioremap_nocache() (which is, unfortunately, the same as ioremap() due > to various documentation telling people to use it for devices) can be used > for the mapping on ARM. There is a difference between unaligned accesses being an issue for an architecture (not HAVE_EFFICIENT_UNALIGNED_ACCESS) on a mapping area Vs having a requirement for all device drivers to not have unaligned accesses for mapped memory, it seems we want the later anyway... however we obviously don't have a scraper to go and vet all drivers / check a driver it seems. Hrm, I wonder if we can use grammar checks for this. Anyway, OK got it! > So we can't go around adding new ioremap() variants and hoping that > ioremap_nocache() is a safe default everywhere. It isn't. Sure, OK. > > I *thought* I had added boiler plate code to map > > the ioremap_uc() call to ioremap_nocache() for archs that do not already define > > their own iorempa_uc() call, but upon further investigation it seems that was > > not the case but found that this may be a bit different issue altogether. > > > > The way include/asm-generic/io.h works for ioremap() calls and its variants is: > > > > #ifndef CONFIG_MMU > > #ifndef ioremap > > #define ioremap ioremap > > static inline void __iomem *ioremap(phys_addr_t offset, size_t size) > > { > > return (void __iomem *)(unsigned long)offset; > > } > > #endif > > ... > > #define iounmap iounmap > > > > static inline void iounmap(void __iomem *addr) > > { > > } > > #endif > > #endif /* CONFIG_MMU */ > > > That's the gist of it, but the catch here is the ioremap_*() variants and where > > they are defined. The first variant is ioremap_nocache() and then all other > > variants by default map to this one. We've been stuffing the variant definitions > > within the #ifndef CONFIG_MMU and I don't think we should be as otherwise each > > and everyone's archs will have to add their own variant default map to the > > default ioremap_nocache() or whatever. That's exaclty what we have to day, and > > from what I gather the purpose of the variant boiler plate is lost. I think > > we should keep the ioremap() and ioreunmap() tucked under the CONFIG_MMU > > but not the variants. For instance to address the build issue for ioremap_uc() > > we either define ioremap_uc() for all archs or do something like this: > > Surely, if architectures can support the different variants, then they > should implement them? Sure, the asm-generic file seemed to allude to that some ioremaps are safe to have defaults for, if we want to address this as a problem it must be addressed there, this is what my posting aims to address. It seems the other threads with Dan also address some of this so we're already on track. > The only case where it makes sense to provide boilerplate for these is > when architectures don't support the mapping type - and then we need help > from architecture people to decide what is the appropriate mapping type > (and therefore which ioremap() variant) to be used as a fallback. Sounds like a formal process being brewed / documented. Luis