From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH] ARM: refine compiler target architecture Date: Wed, 18 Dec 2013 08:48:10 +0100 Message-ID: <52B1533A.30803@linaro.org> References: <1387272871-2341-1-git-send-email-andre.przywara@linaro.org> <1387275661.27441.34.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VtBrz-0002hi-Eb for xen-devel@lists.xenproject.org; Wed, 18 Dec 2013 07:48:39 +0000 Received: by mail-bk0-f53.google.com with SMTP id na10so67632bkb.12 for ; Tue, 17 Dec 2013 23:48:37 -0800 (PST) In-Reply-To: <1387275661.27441.34.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, julien.grall@linaro.org, patches@linaro.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 12/17/2013 11:21 AM, Ian Campbell wrote: > On Tue, 2013-12-17 at 10:34 +0100, Andre Przywara wrote: >> Currently we compile the tools just with -marm. This breaks >> compilation when the toolchain default is less than ARMv7, because >> we require the "dmb" instruction in xenstored. >> One possible (and working) fix would be to just adjust that for the >> tools, but in fact the same rationale is true for the hypervisor. >> So lets mandate ARMv7-A as the minimum for both Xen and the tools. >> >> Unfortunately for some reasons -mcpu=cortex-a15 does not go together >> with this -march, so lets be more generic and explicitly specify the >> architecture extensions we actually need for the hypervisor. >> >> This fixes native tool compilation on my Slackware system. >> >> Signed-off-by: Andre Przywara >> --- >> >> Not sure if this is safe enough for 4.4. On normal build environemnts >> this shouldn't change anything, but one never knows. >> In any case I want to drop it here for reference. I will try to do some >> tests to prove that it's harmless enough. > > My major concern here is that even if the tools build on an older > toolchain we require EABI layouts for shared structs (xenstore rings , > hypercall arguments etc) and a OABI, which differs in this regard. But actually the ABI is orthogonal to the -march setting, right? AFAIK -march just controls which instruction the compiler emits and which instructions the assembler allows. xenstored for instance uses xen_*mb(), which for ARM translates into "dmb" (which is >= v7), so we have to tell the assembler about it. The latest ARM cross compiler I apt-got for my amd64 Debian 7.1 for instance gives me: $ arm-linux-gnueabi-gcc -v Target: arm-linux-gnueabi ... --with-arch=armv4t ... This is pretty conservative, but the compiler is fine and with -march=armv7-a -mfloat-abi=hard it even generates modern code. Are there really people out there still using OABI? Do we care? Both compilers I tested (Slackware and Debian cross) are purely EABI, but have less than armv7 as their default. Regards, Andre. > > That said at least the second hunk looks like a good cleanup, but one > for post 4.4 IMHO. > >> diff --git a/config/arm32.mk b/config/arm32.mk >> index aa79d22..c5eb30e 100644 >> --- a/config/arm32.mk >> +++ b/config/arm32.mk >> @@ -6,8 +6,8 @@ CONFIG_XEN_INSTALL_SUFFIX := >> >> # -march= -mcpu= >> >> -# Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb: >> -CFLAGS += -marm >> +# Explicitly specifiy 32-bit ARMv7-A ISA since toolchain default can be less: >> +CFLAGS += -marm -march=armv7-a > > The original rationale here seems a bit odd (or at least out dated) I > don't think there is any reason why thumb(2) userspace shouldn't be OK. > > You carried over the misspelling of specify. > >> >> HAS_PL011 := y >> HAS_EXYNOS4210 := y >> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk >> index aaa203e..891df25 100644 >> --- a/xen/arch/arm/Rules.mk >> +++ b/xen/arch/arm/Rules.mk >> @@ -20,7 +20,8 @@ arm := y >> ifeq ($(TARGET_SUBARCH),arm32) >> # Prevent floating-point variables from creeping into Xen. >> CFLAGS += -msoft-float >> -CFLAGS += -mcpu=cortex-a15 >> +# allow assembly of virtualization extension instructions and smc for PSCI >> +CFLAGS += -Wa,-march=armv7-a+sec+virt > > This looks good in principal and seems to be present at least as far > back as binutils 2.21, which is the oldest I had lying around. > > I wonder why we have this odd split between config/arm*.mk and > xen/arch/arm/Rules.mk.. Probably my fault somewhere in the mists of > time... > > Ian. >