Re: [PATCH 00/12] xen/arm: Add support to build with clang

[Julien Grall <julien.grall@arm.com>]

On 18/04/2019 10:15, Artem Mygaiev wrote:
> Hello Julien, Stefano

Hi Artem,

> On Wed, 2019-04-17 at 10:42 +0100, Julien Grall wrote:
>> Hi,
>>
>> On 16/04/2019 23:43, Stefano Stabellini wrote:
>>> On Fri, 29 Mar 2019, Julien Grall wrote:
>>>> On 28/03/2019 11:27, Artem Mygaiev wrote:
>>>>> Hi Julien,
>>>>
>>>> Hi Artem,
>>>>
>>>>> On Wed, 2019-03-27 at 18:45 +0000, Julien Grall wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> This series adds support to build Xen Arm with clang. This series was
>>>>>> tested
>>>>>> with clang 8.0.
>>>>>>
>>>>>> Note that I only did build for arm64. I still need to look at the arm32
>>>>>> build.
>>>>>>
>>>>>
>>>>> I wonder if you have time to try the series with Arm Compiler 6? I am
>>>>> asking because AFAIK it is based on clang/llvm [1] and there's a
>>>>> safety-compliant version of it certified by TUV [2]. I don't have a
>>>>> license yet so cannot try it myself but maybe you have access.
>>>> I gave a quick try to the Arm Compiler. I had to hack a bit config/StdGNU.mk
>>>> to pass armclang and the appropriate target option.
>>>>
>>>> I also had a linking issue at the end where __2snprintf was not found. It
>>>> seems the compiler replace snprintf with __2snprintf, I haven't figured out
>>>> why yet.
>>>
>>> But after these changes, does it work?
>>
>> I haven't tried to fix the linking issues. I only gave a quick try because Artem
>> asked. I have no plan at the moment to go further than that for now.
>>
>> Patches are welcomed to add support for armclang.
>>
> 
> I have implemented a bunch of HACKs [1] so can build Xen master with
> armclang 6.12. Not even "smoke"-tested, just trying to identify missing
> parameters and proper linker configuration.

Thank you for looking at it. Some comments below.

> 
> Not yet fixed section placement, lots of warnings from linker like:
> Warning: L6170W: Mapping symbol #40 '$x.20' in
> .altinstr_replacement(ns16550.o:42) identifies code, but is in a
> section not marked as executable.

Instruction in the sections .altinstr_replacement are never meant to be executed.

I guess this is coming from armlink? Any particular reason to use armlink and 
not ld as we do on clang?

> 
> Also armlink sometimes fails with Internal fault: [0xe81a5a:6120001]

Do you have more output?

> 
> 
> [1] Diff below just for reference with xen master + Julien's clang
> patch series applied
> ---
> 
> diff --git a/Config.mk b/Config.mk
> index 417039d7f6..0fc84293f9 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -221,7 +221,9 @@ CFLAGS += -Wall -Wstrict-prototypes
>   
>   $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
>   $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
> +ifneq ($(armds),y)
>   $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)

I didn't need this on Arm Compiler 6.11. Can you provide the list of error you 
get here?

> +endif
>   $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
>   
>   LDFLAGS += $(foreach i, $(EXTRA_LIB), -L$(i))
> @@ -234,9 +236,15 @@ endif
>   APPEND_LDFLAGS += $(foreach i, $(APPEND_LIB), -L$(i))
>   APPEND_CFLAGS += $(foreach i, $(APPEND_INCLUDES), -I$(i))
>   
> -EMBEDDED_EXTRA_CFLAGS := -nopie -fno-stack-protector -fno-stack-
> protector-all
> +EMBEDDED_EXTRA_CFLAGS := -fno-stack-protector -fno-stack-protector-all
>   EMBEDDED_EXTRA_CFLAGS += -fno-exceptions
>   
> +ifeq ($(armds),y)
> +EMBEDDED_EXTRA_CFLAGS += -fno-ropi -fno-rwpi

Why do you need this? Is it because armlink does not support -nopie?

> +else
> +EMBEDDED_EXTRA_CFLAGS += -nopie
> +endif
> +
>   XEN_EXTFILES_URL ?= http://xenbits.xen.org/xen-extfiles
>   # All the files at that location were downloaded from elsewhere on
>   # the internet.  The original download URL is preserved as a comment
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 48c50b5ad7..585d076d4f 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -1,6 +1,15 @@
>   AS         = $(CROSS_COMPILE)as
> +AR         = $(CROSS_COMPILE)ar
>   LD         = $(CROSS_COMPILE)ld
>   ifeq ($(clang),y)
> +ifeq ($(armds),y)
> +CC         = armclang
> +CXX        = armclang
> +LD_LTO     = armlink
> +LD         = armlink -v
> +AS         = armasm
> +AR         = armar
> +else
>   ifneq ($(CROSS_COMPILE),)
>   CC         = clang -target $(CROSS_COMPILE:-=)
>   CXX        = clang++ -target $(CROSS_COMPILE:-=)
> @@ -9,13 +18,13 @@ CC         = clang
>   CXX        = clang++
>   endif
>   LD_LTO     = $(CROSS_COMPILE)llvm-ld
> +endif
>   else
>   CC         = $(CROSS_COMPILE)gcc
>   CXX        = $(CROSS_COMPILE)g++
>   LD_LTO     = $(CROSS_COMPILE)ld
>   endif
>   CPP        = $(CC) -E
> -AR         = $(CROSS_COMPILE)ar
>   RANLIB     = $(CROSS_COMPILE)ranlib
>   NM         = $(CROSS_COMPILE)nm
>   STRIP      = $(CROSS_COMPILE)strip
> diff --git a/config/arm32.mk b/config/arm32.mk
> index f95228e3c0..5afed07357 100644
> --- a/config/arm32.mk
> +++ b/config/arm32.mk
> @@ -4,12 +4,14 @@ CONFIG_ARM_$(XEN_OS) := y
>   
>   CONFIG_XEN_INSTALL_SUFFIX :=
>   
> -# -march= -mcpu=
> -
>   # Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> -mthumb:
> -CFLAGS += -marm
> -
> +ifeq ($(armds),y)
> +# VE needed
> +CFLAGS += --target=arm-arm-none-eabi -march=armv7-a

-marm should do the right thing even on armclang.

You would still need --target=.... but that's should depend on $CROSS_COMPILE 
(or any other name we decide).

> +else
> +CFLAGS += -marm # -march= -mcpu=
>   # Use only if calling $(LD) directly.
>   LDFLAGS_DIRECT += -EL
> +endif
>   
>   IOEMU_CPU_ARCH ?= arm
> diff --git a/config/arm64.mk b/config/arm64.mk
> index aa45772b61..46b203d384 100644
> --- a/config/arm64.mk
> +++ b/config/arm64.mk
> @@ -4,10 +4,14 @@ CONFIG_ARM_$(XEN_OS) := y
>   
>   CONFIG_XEN_INSTALL_SUFFIX :=
>   
> +ifeq ($(armds),y)
> +# VE needed
> +CFLAGS += --target=aarch64-arm-none-eabi -march=armv8.1-a+nofp+nosimd

Same remark for --target.

Also, -march=armv8.1 looks wrong to me because this may generate code that will 
not work on armv8.0 platform.

> +else
>   CFLAGS += #-marm -march= -mcpu= etc
> -
>   # Use only if calling $(LD) directly.
>   LDFLAGS_DIRECT += -EL
> +endif
>   
>   IOEMU_CPU_ARCH ?= aarch64
>   
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index a151b3f625..72b34451d2 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -76,9 +76,11 @@ AFLAGS-y                += -D__ASSEMBLY__
>   # Older clang's built-in assembler doesn't understand .skip with
> labels:
>   # https://bugs.llvm.org/show_bug.cgi?id=27369
>   ifeq ($(clang),y)
> +ifneq ($(armds),y)
>   $(call as-option-add,CFLAGS,CC,".L0:\n.L1:\n.skip (.L1 - .L0)",,\
>                        -no-integrated-as)
>   endif
> +endif
>   
>   ALL_OBJS := $(ALL_OBJS-y)
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel