Re: [PATCH] ARM: refine compiler target architecture

[Andre Przywara <andre.przywara@linaro.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 <andre.przywara@linaro.org>
>> ---
>>
>> 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.

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.
>