Re: [Xen-devel] [PATCH] ARM: refine compiler target architecture

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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 <andre.przywara@xxxxxxxxxx>
> ---
> 
> 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 <v7 toolchain will often be using the
OABI, which differs in this regard.

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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel