From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933292AbcERUZs (ORCPT ); Wed, 18 May 2016 16:25:48 -0400 Received: from mout.kundenserver.de ([212.227.17.13]:56105 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933202AbcERUZp (ORCPT ); Wed, 18 May 2016 16:25:45 -0400 From: Arnd Bergmann To: Russell King - ARM Linux Cc: Dave Gerlach , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, Russ Dill , Greg Kroah-Hartman , Shawn Guo , Tony Lindgren , Alexandre Belloni , Nishanth Menon Subject: Re: [RFC PATCH 1/3] asm-generic: io: Add exec versions of ioremap Date: Wed, 18 May 2016 22:25:03 +0200 Message-ID: <3861017.ZJOlWYvEFT@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20160518175102.GX5783@n2100.arm.linux.org.uk> References: <1462830111-28172-1-git-send-email-d-gerlach@ti.com> <573C7844.5030107@ti.com> <20160518175102.GX5783@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:CufHS6XzKdPQRaeyCl8ziw3HygJnGI3wBGYuJ0SLs2KdpFAbXDi GukckqNPNMIPbne1064j6Rs1J/Es2evf22ffjNFWMCsM54pbq3R3TuihLycQyXFo6earFfC 23xBKD48vsGfHi9uth+UOab7HB4QskAaLJ0l9pxPAdibGLz4/Me6JijCxNLm3BiTL8zrPLF OQeb6ueqZdADdkSWgOwcw== X-UI-Out-Filterresults: notjunk:1;V01:K0:4jPe/4fpmZE=:dLM4Z/rjhq+tKRQDKLUEuy pxpJrefI6whVH9Exo4qMPLUmcr6bsjgX14u/Jz5tex3/Zkjru46qNNFAv/4CFXEbaH8cKQth0 fPQi4FdR6qskz7Q4xelAp+9Z/PklTpAquIWWCfTEWYC2b+MiWZBBAxkuY5vKRacn3FncJONwg Zghjbm8ciXqXAEhIJ02SMpb16wB763JkBfQMbT1IaXGwsCNwWmhmGx7QkwH7TnRGEGVa6CxAn vw4iTuSPG64pXdp4cII1T0i04e0a269x0gddhnz0LGiTOzQVBgT3oEzHIzD8En6tsd4wRZ44k V3HNZ0YhEDOcD7Ei1X5MRC7kJ4rYcddChZCgsgED0ctfN7EAryZ15Yf2uICJ91ZhVL3r6fs9M gvU9+chmTDrZNMeVhILN2lvLnvwwAwD+D4g0LUbogQvPpwyP/QkxYM1eyzTtSdouGbCvu5IhW Pw6olM8wXyGWdEy5M6ivVtnWLZC34fIRwlTnZ+iuUW2+LiUedqACGOhWmjsFdQapc88j7jVk/ osCU98378konHt6noBf+zgeYy8L5Dazewce+O3VkNGTMzfdtyacUS9b9NQ9vZFAlM4dcSvxvB vvEB5/WSlk1A4w7FzkxPTAl76PFhLo8sfBG2GRFLUBDALoptyA9OrejecUbXpDfhuSlAr6XsI bQPnF4wzmvrF/j7G98nEckEt8phvZipgaOlu1neFv4KRhcoeThRXq/sNCWsvKk//t8gu9smOK 5expirk04n7wmq3H/VwXOo+3ujdOYV5AB900oQ== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 18 May 2016 18:51:02 Russell King - ARM Linux wrote: > On Wed, May 18, 2016 at 09:12:20AM -0500, Dave Gerlach wrote: > > Ok thank you for the pointer. I agree, the memremap API looks like a better > > fit for this. I think it likely makes the most sense to still add these > > ioremap_exec and ioremap_exec_nocache and then call them through the > > memremap API based on new flags. This will fit into the current use model > > for memremap as it currently uses all of the other ioremap calls internally, > > and doing it how I just described will let this code evolve along with > > memremap. > > I would _really_ prefer not to do that. Why? Because IO memory does > not have the required properties to be executable. IO memory is normally > memory which has side effects - and by side effects, I mean reading it > can provoke hardware to perform some action. You don't want to be > executing from such memory. > > So, in my mind, ioremap_exec makes absolutely no sense, and having it > gives people a new interface to abuse - and abuse they will. Agreed, calling it ioremap when it is really memremap makes no sense. I also see another problem in the asm-generic portion: +#ifndef ARCH_HAS_IOREMAP_EXEC +#define ioremap_exec ioremap +#define ioremap_exec_nocache ioremap_nocache +#endif The ARM version of ioremap_exec() that gets added in this patch is cached (like memremap()), but then the asm-generic version is not? This is even more confusing, it should at least do roughly the same thing across architectures. There should also be some documentation about what the expected behavior is, e.g.: - is memremap_exec() by default cached or not? (I assume it would be like memremap()) - If we have an interface that does explicit uncached executable mapping, what about architectures on which this is not possible? Should they fall back to cached or non-executable, or cause a link error? Arnd