mbox series

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

Message ID 20190327184531.30986-1-julien.grall@arm.com (mailing list archive)
Headers show
Series xen/arm: Add support to build with clang | expand

Message

Julien Grall March 27, 2019, 6:45 p.m. UTC
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.

Cheers,

Julien Grall (12):
  xen: clang: Support correctly cross-compile
  xen/arm: fix get_cpu_info() when built with clang
  xen/arm: zynqmp: Fix header guard for xilinx-zynqmp-eemi.h
  xen/arm: memaccess: Initialize correctly *access in
    __p2m_get_mem_access
  xen/arm64: bitops: Match the register size with the value size in flsl
  xen/arm64: sysreg: Implement the 32-bit helpers using the 64-bit
    helpers
  xen/arm: cpuerrata: Match register size with value size in
    check_workaround_*
  xen/arm: cpufeature: Match register size with value size in
    cpus_have_const_cap
  xen/arm: guest_walk: Avoid theoritical unitialized value in
    get_top_bit
  xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
  xen/arm: traps: Mark check_stack_alignment_constraints as unused
  xen/arm64: __cmpxchg and __cmpxchg_mb should always be inline

 config/StdGNU.mk                                   |  9 +++++++--
 xen/arch/arm/guest_walk.c                          |  2 +-
 xen/arch/arm/mem_access.c                          |  2 +-
 xen/arch/arm/mm.c                                  |  3 ++-
 xen/arch/arm/traps.c                               |  3 ++-
 xen/include/asm-arm/arm64/bitops.h                 |  3 ++-
 xen/include/asm-arm/arm64/cmpxchg.h                | 10 ++++++----
 xen/include/asm-arm/arm64/sysregs.h                | 11 +++--------
 xen/include/asm-arm/cpuerrata.h                    |  2 +-
 xen/include/asm-arm/cpufeature.h                   |  2 +-
 xen/include/asm-arm/current.h                      | 10 +++++++++-
 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h |  2 +-
 12 files changed, 36 insertions(+), 23 deletions(-)

Comments

Artem Mygaiev March 28, 2019, 11:27 a.m. UTC | #1
Hi Julien,

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.

> 
> Cheers,
> 
> Julien Grall (12):
>   xen: clang: Support correctly cross-compile
>   xen/arm: fix get_cpu_info() when built with clang
>   xen/arm: zynqmp: Fix header guard for xilinx-zynqmp-eemi.h
>   xen/arm: memaccess: Initialize correctly *access in
>     __p2m_get_mem_access
>   xen/arm64: bitops: Match the register size with the value size in flsl
>   xen/arm64: sysreg: Implement the 32-bit helpers using the 64-bit
>     helpers
>   xen/arm: cpuerrata: Match register size with value size in
>     check_workaround_*
>   xen/arm: cpufeature: Match register size with value size in
>     cpus_have_const_cap
>   xen/arm: guest_walk: Avoid theoritical unitialized value in
>     get_top_bit
>   xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
>   xen/arm: traps: Mark check_stack_alignment_constraints as unused
>   xen/arm64: __cmpxchg and __cmpxchg_mb should always be inline
> 
>  config/StdGNU.mk                                   |  9 +++++++--
>  xen/arch/arm/guest_walk.c                          |  2 +-
>  xen/arch/arm/mem_access.c                          |  2 +-
>  xen/arch/arm/mm.c                                  |  3 ++-
>  xen/arch/arm/traps.c                               |  3 ++-
>  xen/include/asm-arm/arm64/bitops.h                 |  3 ++-
>  xen/include/asm-arm/arm64/cmpxchg.h                | 10 ++++++----
>  xen/include/asm-arm/arm64/sysregs.h                | 11 +++--------
>  xen/include/asm-arm/cpuerrata.h                    |  2 +-
>  xen/include/asm-arm/cpufeature.h                   |  2 +-
>  xen/include/asm-arm/current.h                      | 10 +++++++++-
>  xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h |  2 +-
>  12 files changed, 36 insertions(+), 23 deletions(-)
> 

[1] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.subset.swdev.comp6/index.html

[2] 
https://store.developer.arm.com/store/embedded-iot-software-tools/arm-compiler-6-functional-safety?_ga=2.198817711.1237223028.1553771353-424456504.1550666847
Julien Grall March 29, 2019, 10:13 a.m. UTC | #2
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.

Cheers,
Stefano Stabellini April 16, 2019, 10:43 p.m. UTC | #3
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?
Julien Grall April 17, 2019, 9:42 a.m. UTC | #4
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.

Cheers,
Artem Mygaiev April 18, 2019, 9:15 a.m. UTC | #5
Hello Julien, Stefano

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.

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.

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


[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)
+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
+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
+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
+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)
Julien Grall April 18, 2019, 10:43 a.m. UTC | #6
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 April 18, 2019, 11:13 a.m. UTC | #7
Hi,

On 27/03/2019 18:45, Julien Grall wrote:

Hi,

I have applied the follow patches to my branch next-4.13. I will merge it to 
unstable on the commit moratorium is lifted.

>    xen/arm: zynqmp: Fix header guard for xilinx-zynqmp-eemi.h
>    xen/arm64: sysreg: Implement the 32-bit helpers using the 64-bit
>      helpers
>    xen/arm: guest_walk: Avoid theoritical unitialized value in
>      get_top_bit
>    xen/arm64: __cmpxchg and __cmpxchg_mb should always be inline

I will respin the rest.

Cheers,
Artem Mygaiev April 18, 2019, 11:15 a.m. UTC | #8
Hi Julien

On Thu, 2019-04-18 at 11:43 +0100, Julien Grall wrote:
> 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?
> 

Yes, armlink has a "Safety-certified" version of it, while ld doesn't,
unfortunately :(

Though I must mention that I do not have access to safety-certified
version of arm compiler suite, it is not public. Thus checking only
against the 30-day trial version of ARM DS 2019-0 which is shipped with
compiler 6.12

> > Also armlink sometimes fails with Internal fault:
> > [0xe81a5a:6120001]
> 
> Do you have more output?

Just opened a ticket in Arm community forums, see details there
including full build log (dont want to spam ml)
https://community.arm.com/developer/tools-software/tools/f/arm-compilers-forum/12989/linker-internal-fault-0xe81a5a-6120001

> 
> > 
> > [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?

error: unknown warning option '-Wno-unused-but-set-variable'; did you
mean '-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-option]

> 
> > +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?

Yes. ropi/rwpi are according to Arm Compiler 6.12 reference guide, see
[100067_0612_00_en 1-54] and [100067_0612_00_en 1-56]

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

You're right, I forgot to mention - have not yet built for Arm32 so
this is not checked. But, according to manual:

For targets in AArch32 state (--target=arm-arm-none-eabi), there is no
default. You must specify either -march (to target an architecture) or
-mcpu (to target a processor).

[100067_0612_00_en page 1-82]

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

If I don't specify -march this way, it will built:
 1) without some registers, e.g. TTLB0_EL2 support (build fails)
 2) with vfp and simd enabled (linking fails)

According to Arm compiler manual:
---
For targets in AArch64 state (--target=aarch64-arm-none-eabi), unless
you target a particular processor using -mcpu or a particular
architecture using -march, the compiler defaults to -march=armv8-a,
generating generic code for Armv8‑A in AArch64 state.
[100067_0612_00_en page 1-82]
---
You can prevent the use of floating-point instructions or floating-
point registers for targets in AArch64 state with the
-mcpu=name+nofp+nosimd option. Subsequent use of floating-point data
types in this mode is unsupported.
[100067_0612_00_en page 1-93]
---

> 

> > +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,

[100067_0612_00_en] https://developer.arm.com/docs/100067/latest
Julien Grall April 18, 2019, 6:33 p.m. UTC | #9
(+ Roger)

On 18/04/2019 12:15, Artem Mygaiev wrote:
> Hi Julien
> 
> On Thu, 2019-04-18 at 11:43 +0100, Julien Grall wrote:
>> 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?
>>
> 
> Yes, armlink has a "Safety-certified" version of it, while ld doesn't,
> unfortunately :(

I am not sure if anyone tried to build Xen other than with ld so far. I have 
CCed Roger who might have a clue whether there are other blocker.

> 
> Though I must mention that I do not have access to safety-certified
> version of arm compiler suite, it is not public. Thus checking only
> against the 30-day trial version of ARM DS 2019-0 which is shipped with
> compiler 6.12

Any step towards armclang is good :). We can look at the safety-certified 
version afterwards.

I don't seem to be able to link with this patch on 6.12:

Loading object built_in.o.
Error: L6242E: Cannot link object built_in.o as its attributes are incompatible 
with the image attributes.
    ... A64 clashes with SoftVFP.
Error: L6242E: Cannot link object built_in.o as its attributes are incompatible 
with the image attributes.
    ... A64 clashes with SoftVFP.
Error: L6242E: Cannot link object built_in.o as its attributes are incompatible 
with the image attributes.
    ... A64 clashes with SoftVFP.
Error: L6242E: Cannot link object built_in.o as its attributes are incompatible 
with the image attributes.
    ... A64 clashes with SoftVFP.

I am using the following command line to build:

42sh> make CROSS_COMPILE=aarch64-linux-gnu- XEN_TARGET_ARCH=arm64 clang=y 
armds=y  -C ~/works/xen/xen  XEN_CONFIG_EXPERT=y

> 
>>> Also armlink sometimes fails with Internal fault:
>>> [0xe81a5a:6120001]
>>
>> Do you have more output?
> 
> Just opened a ticket in Arm community forums, see details there
> including full build log (dont want to spam ml)
> https://community.arm.com/developer/tools-software/tools/f/arm-compilers-forum/12989/linker-internal-fault-0xe81a5a-6120001

There are nothing interesting in except a flood of "armclang: warning: Your 
license for feature ds_suite_eval will expire in 29 days".

Let me know how it goes and I can help to resolve it.

> 
>>
>>>
>>> [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?
> 
> error: unknown warning option '-Wno-unused-but-set-variable'; did you
> mean '-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-option]

But cc-option-add should only add the flag if it is supported by the compiler. 
So it is not supported, how come this option is actually used to compile?

> 
>>
>>> +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?
> 
> Yes. ropi/rwpi are according to Arm Compiler 6.12 reference guide, see
> [100067_0612_00_en 1-54] and [100067_0612_00_en 1-56]

Hmmm, but the docs says:

"This option is not supported in AArch64 state". So is that a really good 
replacement?

[...]

>> You would still need --target=.... but that's should depend on
>> $CROSS_COMPILE
>> (or any other name we decide).
> 
> You're right, I forgot to mention - have not yet built for Arm32 so
> this is not checked. But, according to manual:
> 
> For targets in AArch32 state (--target=arm-arm-none-eabi), there is no
> default. You must specify either -march (to target an architecture) or
> -mcpu (to target a processor)

We actually pass -mcpu=cortex-a15 in arch/arm/Rules.mk (I admit that cortex-a15 
is probably not the most suitable). If some options should applied to the tools 
as well, then we need to migrated them to config/arm32.mk.

> 
> [100067_0612_00_en page 1-82]
> 
>>
>>> +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.
>>
> 
> If I don't specify -march this way, it will built:
>   1) without some registers, e.g. TTLB0_EL2 support (build fails)

I guess you mean TTBR0_EL2 here.

Xen is fully Armv8.0 compliant. So if you can't compile Xen with -march=armv8-a 
then this needs to be investigated and be reported.

Looking at the error thrown:

arm64/head.S:399:13: error: expected writable system register or pstate
         msr TTBR0_EL2, x4

This is likely a compiler bug.

>   2) with vfp and simd enabled (linking fails)
> 
> According to Arm compiler manual:
> ---
> For targets in AArch64 state (--target=aarch64-arm-none-eabi), unless
> you target a particular processor using -mcpu or a particular
> architecture using -march, the compiler defaults to -march=armv8-a,
> generating generic code for Armv8‑A in AArch64 state.
> [100067_0612_00_en page 1-82]
> ---
> You can prevent the use of floating-point instructions or floating-
> point registers for targets in AArch64 state with the
> -mcpu=name+nofp+nosimd option. Subsequent use of floating-point data
> types in this mode is unsupported.
> [100067_0612_00_en page 1-93]
> ---

I assume that -mgeneral-regs-only does not work in your case?

Also, it just occurred to me that nofp+nosimd should only be done for the 
hypervisor. For the tools, we can properly build with SIMD. So this will have to 
be moved in xen/arch/arm/Rules.mk.

Cheers,
Artem Mygaiev April 23, 2019, 1:39 p.m. UTC | #10
Hello Julien, Roger

On Thu, 2019-04-18 at 19:33 +0100, Julien Grall wrote:
> (+ Roger)
> 
> On 18/04/2019 12:15, Artem Mygaiev wrote:
> > Hi Julien
> > 
> > On Thu, 2019-04-18 at 11:43 +0100, Julien Grall wrote:
> > > 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?
> > > 
> > 
> > Yes, armlink has a "Safety-certified" version of it, while ld
> > doesn't,
> > unfortunately :(
> 
> I am not sure if anyone tried to build Xen other than with ld so far.
> I have 
> CCed Roger who might have a clue whether there are other blocker.
> 

Looking forward to hearing from Roger.

> > Though I must mention that I do not have access to safety-certified
> > version of arm compiler suite, it is not public. Thus checking only
> > against the 30-day trial version of ARM DS 2019-0 which is shipped
> > with
> > compiler 6.12
> 
> Any step towards armclang is good :). We can look at the safety-
> certified 
> version afterwards.
> 
> I don't seem to be able to link with this patch on 6.12:
> 
> Loading object built_in.o.
> Error: L6242E: Cannot link object built_in.o as its attributes are
> incompatible 
> with the image attributes.
>     ... A64 clashes with SoftVFP.
> Error: L6242E: Cannot link object built_in.o as its attributes are
> incompatible 
> with the image attributes.
>     ... A64 clashes with SoftVFP.
> Error: L6242E: Cannot link object built_in.o as its attributes are
> incompatible 
> with the image attributes.
>     ... A64 clashes with SoftVFP.
> Error: L6242E: Cannot link object built_in.o as its attributes are
> incompatible 
> with the image attributes.
>     ... A64 clashes with SoftVFP.
> 
> I am using the following command line to build:
> 
> 42sh> make CROSS_COMPILE=aarch64-linux-gnu- XEN_TARGET_ARCH=arm64
> clang=y 
> armds=y  -C ~/works/xen/xen  XEN_CONFIG_EXPERT=y
> 

I am still struggling with linker internal fault, not sure what changed
in my environment so it started to fail.

> > > > Also armlink sometimes fails with Internal fault:
> > > > [0xe81a5a:6120001]
> > > 
> > > Do you have more output?
> > 
> > Just opened a ticket in Arm community forums, see details there
> > including full build log (dont want to spam ml)
> > https://community.arm.com/developer/tools-software/tools/f/arm-compilers-forum/12989/linker-internal-fault-0xe81a5a-6120001
> > 
> 
> There are nothing interesting in except a flood of "armclang:
> warning: Your 
> license for feature ds_suite_eval will expire in 29 days".
> 
> Let me know how it goes and I can help to resolve it.
> 

This is because of the "free" evaluation license I have, probably can
be safely ignored. So far no reaction on my initial post in Arm
Community.

Playing with linker options I found that the error is caused by the use
of "partial linking model" which is enabled by --partial that
correspond to -r in Xen Makefiles, which is not documented but has
similar effect.

> > > > [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?
> > 
> > error: unknown warning option '-Wno-unused-but-set-variable'; did
> > you
> > mean '-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-
> > option]
> 
> But cc-option-add should only add the flag if it is supported by the
> compiler. 
> So it is not supported, how come this option is actually used to
> compile?
> 

Because the idea of supported option detection rely on the fact that
compiler will throw an error or warning when makefile try and use the
option being tested, but armclang does not even try to check that and
failing with
  armclang: fatal error: no target architecture given; use --
target=arm-arm-none-eabi or --target=aarch64-arm-none-eabi
and thus cc-option-add does not detect unsupported options

> > > > +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?
> > 
> > Yes. ropi/rwpi are according to Arm Compiler 6.12 reference guide,
> > see
> > [100067_0612_00_en 1-54] and [100067_0612_00_en 1-56]
> 
> Hmmm, but the docs says:
> 
> "This option is not supported in AArch64 state". So is that a really
> good 
> replacement?
> 

You're right, reading further in the document this does not seem to be
a valid replacement. Moreover, it seems that we dont need this option
with armclang - by default it does not generate position-independent
code

> [...]
> 
> > > You would still need --target=.... but that's should depend on
> > > $CROSS_COMPILE
> > > (or any other name we decide).
> > 
> > You're right, I forgot to mention - have not yet built for Arm32 so
> > this is not checked. But, according to manual:
> > 
> > For targets in AArch32 state (--target=arm-arm-none-eabi), there is
> > no
> > default. You must specify either -march (to target an architecture)
> > or
> > -mcpu (to target a processor)
> 
> We actually pass -mcpu=cortex-a15 in arch/arm/Rules.mk (I admit that
> cortex-a15 
> is probably not the most suitable). If some options should applied to
> the tools 
> as well, then we need to migrated them to config/arm32.mk.

Yes, I do not think we shall specify the particular CPU, better to have
lowest supported architecture defined.

> 
> > [100067_0612_00_en page 1-82]
> > 
> > > > +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.
> > > 
> > 
> > If I don't specify -march this way, it will built:
> >   1) without some registers, e.g. TTLB0_EL2 support (build fails)
> 
> I guess you mean TTBR0_EL2 here.
> 
> Xen is fully Armv8.0 compliant. So if you can't compile Xen with
> -march=armv8-a 
> then this needs to be investigated and be reported.
> 
> Looking at the error thrown:
> 
> arm64/head.S:399:13: error: expected writable system register or
> pstate
>          msr TTBR0_EL2, x4
> 
> This is likely a compiler bug.
> 

Actually I do not know what is the proper way to report such issues and
get them fixed. My post on ARM forum regarding linker failue has no
comments/reactions, which means it may not be the best way for
reporting. Would it be possible for you to check if we can get some
attention from ARM Compiler folks?

> >   2) with vfp and simd enabled (linking fails)
> > 
> > According to Arm compiler manual:
> > ---
> > For targets in AArch64 state (--target=aarch64-arm-none-eabi),
> > unless
> > you target a particular processor using -mcpu or a particular
> > architecture using -march, the compiler defaults to -march=armv8-a,
> > generating generic code for Armv8‑A in AArch64 state.
> > [100067_0612_00_en page 1-82]
> > ---
> > You can prevent the use of floating-point instructions or floating-
> > point registers for targets in AArch64 state with the
> > -mcpu=name+nofp+nosimd option. Subsequent use of floating-point
> > data
> > types in this mode is unsupported.
> > [100067_0612_00_en page 1-93]
> > ---
> 
> I assume that -mgeneral-regs-only does not work in your case?

Nope :\

> 
> Also, it just occurred to me that nofp+nosimd should only be done for
> the 
> hypervisor. For the tools, we can properly build with SIMD. So this
> will have to 
> be moved in xen/arch/arm/Rules.mk.

Noted.

> 
> Cheers,
> 
>
Roger Pau Monné April 24, 2019, 11:01 a.m. UTC | #11
On Thu, Apr 18, 2019 at 07:33:05PM +0100, Julien Grall wrote:
> (+ Roger)
> 
> On 18/04/2019 12:15, Artem Mygaiev wrote:
> > Hi Julien
> > 
> > On Thu, 2019-04-18 at 11:43 +0100, Julien Grall wrote:
> > > 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?
> > > 
> > 
> > Yes, armlink has a "Safety-certified" version of it, while ld doesn't,
> > unfortunately :(
> 
> I am not sure if anyone tried to build Xen other than with ld so far. I have
> CCed Roger who might have a clue whether there are other blocker.

On x86 you can build Xen and the toolstack with a full llvm based
toolchain (clang + lld). In fact that's how the Xen packages for
FreeBSD are built, IIRC lld and clang 6 and greater should work fine
with Xen 4.12 and upwards.

I had to do some fixes to lld and clang in order to understand some of
the assembly tricks that Xen does, but it was quite minor.

Roger.
Julien Grall April 24, 2019, 9:07 p.m. UTC | #12
Hi Artem,

On 4/23/19 2:39 PM, Artem Mygaiev wrote:
> Hello Julien, Roger
> 
> On Thu, 2019-04-18 at 19:33 +0100, Julien Grall wrote:
>> (+ Roger)
>>
>> On 18/04/2019 12:15, Artem Mygaiev wrote:
>>> Hi Julien
>>>
>>> On Thu, 2019-04-18 at 11:43 +0100, Julien Grall wrote:
>>>> 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?
>>>>
>>>
>>> Yes, armlink has a "Safety-certified" version of it, while ld
>>> doesn't,
>>> unfortunately :(
>>
>> I am not sure if anyone tried to build Xen other than with ld so far.
>> I have
>> CCed Roger who might have a clue whether there are other blocker.
>>
> 
> Looking forward to hearing from Roger.
> 
>>> Though I must mention that I do not have access to safety-certified
>>> version of arm compiler suite, it is not public. Thus checking only
>>> against the 30-day trial version of ARM DS 2019-0 which is shipped
>>> with
>>> compiler 6.12
>>
>> Any step towards armclang is good :). We can look at the safety-
>> certified
>> version afterwards.
>>
>> I don't seem to be able to link with this patch on 6.12:
>>
>> Loading object built_in.o.
>> Error: L6242E: Cannot link object built_in.o as its attributes are
>> incompatible
>> with the image attributes.
>>      ... A64 clashes with SoftVFP.
>> Error: L6242E: Cannot link object built_in.o as its attributes are
>> incompatible
>> with the image attributes.
>>      ... A64 clashes with SoftVFP.
>> Error: L6242E: Cannot link object built_in.o as its attributes are
>> incompatible
>> with the image attributes.
>>      ... A64 clashes with SoftVFP.
>> Error: L6242E: Cannot link object built_in.o as its attributes are
>> incompatible
>> with the image attributes.
>>      ... A64 clashes with SoftVFP.
>>
>> I am using the following command line to build:
>>
>> 42sh> make CROSS_COMPILE=aarch64-linux-gnu- XEN_TARGET_ARCH=arm64
>> clang=y
>> armds=y  -C ~/works/xen/xen  XEN_CONFIG_EXPERT=y
>>
> 
> I am still struggling with linker internal fault, not sure what changed
> in my environment so it started to fail.
> 
>>>>> Also armlink sometimes fails with Internal fault:
>>>>> [0xe81a5a:6120001]
>>>>
>>>> Do you have more output?
>>>
>>> Just opened a ticket in Arm community forums, see details there
>>> including full build log (dont want to spam ml)
>>> https://community.arm.com/developer/tools-software/tools/f/arm-compilers-forum/12989/linker-internal-fault-0xe81a5a-6120001
>>>
>>
>> There are nothing interesting in except a flood of "armclang:
>> warning: Your
>> license for feature ds_suite_eval will expire in 29 days".
>>
>> Let me know how it goes and I can help to resolve it.
>>
> 
> This is because of the "free" evaluation license I have, probably can
> be safely ignored.

I know :). I was pointed out how verbose it was compare to the lack of 
information regarding the internal fault.

> So far no reaction on my initial post in Arm Community.
> 
> Playing with linker options I found that the error is caused by the use
> of "partial linking model" which is enabled by --partial that
> correspond to -r in Xen Makefiles, which is not documented but has
> similar effect.

Do you mean the "Internal Fault"?

> 
>>>>> [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?
>>>
>>> error: unknown warning option '-Wno-unused-but-set-variable'; did
>>> you
>>> mean '-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-
>>> option]
>>
>> But cc-option-add should only add the flag if it is supported by the
>> compiler.
>> So it is not supported, how come this option is actually used to
>> compile?
>>
> 
> Because the idea of supported option detection rely on the fact that
> compiler will throw an error or warning when makefile try and use the
> option being tested, but armclang does not even try to check that and
> failing with
>    armclang: fatal error: no target architecture given; use --
> target=arm-arm-none-eabi or --target=aarch64-arm-none-eabi
> and thus cc-option-add does not detect unsupported options

I think defining CC as below should do the trick:

CC := armclang --target=aarch64-arm-none-eabi

> 
>>>>> +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?
>>>
>>> Yes. ropi/rwpi are according to Arm Compiler 6.12 reference guide,
>>> see
>>> [100067_0612_00_en 1-54] and [100067_0612_00_en 1-56]
>>
>> Hmmm, but the docs says:
>>
>> "This option is not supported in AArch64 state". So is that a really
>> good
>> replacement?
>>
> 
> You're right, reading further in the document this does not seem to be
> a valid replacement. Moreover, it seems that we dont need this option
> with armclang - by default it does not generate position-independent
> code

As this always been the case? I know that for GCC it depends on the on 
distro/version of the compiler.

>> [...]
>>
>>>> You would still need --target=.... but that's should depend on
>>>> $CROSS_COMPILE
>>>> (or any other name we decide).
>>>
>>> You're right, I forgot to mention - have not yet built for Arm32 so
>>> this is not checked. But, according to manual:
>>>
>>> For targets in AArch32 state (--target=arm-arm-none-eabi), there is
>>> no
>>> default. You must specify either -march (to target an architecture)
>>> or
>>> -mcpu (to target a processor)
>>
>> We actually pass -mcpu=cortex-a15 in arch/arm/Rules.mk (I admit that
>> cortex-a15
>> is probably not the most suitable). If some options should applied to
>> the tools
>> as well, then we need to migrated them to config/arm32.mk.
> 
> Yes, I do not think we shall specify the particular CPU, better to have
> lowest supported architecture defined.

I vaguely remembered to try to replace cortex-a15 with something else in 
the past. IIRC, the problem was armv7-a does not necessarily include 
virt extension.

Trying again today with -march=armv7-a instead of -mcpu=cortex-a15, I 
can see build error when using Linaro GCC 6.1-2016.80:

{standard input}: Assembler messages:
{standard input}:285: Error: selected processor does not support `smc 
#0' in ARM mode
{standard input}:429: Error: selected processor does not support `smc 
#0' in ARM mode
{standard input}:462: Error: selected processor does not support `smc 
#0' in ARM mode
{standard input}:539: Error: selected processor does not support `smc 
#0' in ARM mode
/home/julieng/works/xen/xen/Rules.mk:196: recipe for target 
'cpuerrata.o' failed
make[3]: *** [cpuerrata.o] Error 1

I think this one is solvable by specifying the secure extension. But it 
might be possible there are other issues with older compiler (i.e 4.6 & co).

>>
>>> [100067_0612_00_en page 1-82]
>>>
>>>>> +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.
>>>>
>>>
>>> If I don't specify -march this way, it will built:
>>>    1) without some registers, e.g. TTLB0_EL2 support (build fails)
>>
>> I guess you mean TTBR0_EL2 here.
>>
>> Xen is fully Armv8.0 compliant. So if you can't compile Xen with
>> -march=armv8-a
>> then this needs to be investigated and be reported.
>>
>> Looking at the error thrown:
>>
>> arm64/head.S:399:13: error: expected writable system register or
>> pstate
>>           msr TTBR0_EL2, x4
>>
>> This is likely a compiler bug.
>>
> 
> Actually I do not know what is the proper way to report such issues and
> get them fixed. My post on ARM forum regarding linker failue has no
> comments/reactions, which means it may not be the best way for
> reporting. Would it be possible for you to check if we can get some
> attention from ARM Compiler folks?

I will have a chat and let you know.

> 
>>>    2) with vfp and simd enabled (linking fails)
>>>
>>> According to Arm compiler manual:
>>> ---
>>> For targets in AArch64 state (--target=aarch64-arm-none-eabi),
>>> unless
>>> you target a particular processor using -mcpu or a particular
>>> architecture using -march, the compiler defaults to -march=armv8-a,
>>> generating generic code for Armv8‑A in AArch64 state.
>>> [100067_0612_00_en page 1-82]
>>> ---
>>> You can prevent the use of floating-point instructions or floating-
>>> point registers for targets in AArch64 state with the
>>> -mcpu=name+nofp+nosimd option. Subsequent use of floating-point
>>> data
>>> types in this mode is unsupported.
>>> [100067_0612_00_en page 1-93]
>>> ---
>>
>> I assume that -mgeneral-regs-only does not work in your case?
> 
> Nope :\

It would be interesting to know whether -march=armv8-a... works on all 
GCC version we support.

Cheers,
Artem Mygaiev April 25, 2019, 3:31 a.m. UTC | #13
Hello Julien

On Wed, 2019-04-24 at 22:07 +0100, Julien Grall wrote:
> Hi Artem,
> 
> On 4/23/19 2:39 PM, Artem Mygaiev wrote:
> > Hello Julien, Roger
> > 
> > On Thu, 2019-04-18 at 19:33 +0100, Julien Grall wrote:
> > > (+ Roger)
> > > 
> > > On 18/04/2019 12:15, Artem Mygaiev wrote:
> > > > Hi Julien
> > > > 
> > > > On Thu, 2019-04-18 at 11:43 +0100, Julien Grall wrote:
> > > > > 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?
> > > > > 
> > > > 
> > > > Yes, armlink has a "Safety-certified" version of it, while ld
> > > > doesn't,
> > > > unfortunately :(
> > > 
> > > I am not sure if anyone tried to build Xen other than with ld so
> > > far.
> > > I have
> > > CCed Roger who might have a clue whether there are other blocker.
> > > 
> > 
> > Looking forward to hearing from Roger.
> > 
> > > > Though I must mention that I do not have access to safety-
> > > > certified
> > > > version of arm compiler suite, it is not public. Thus checking
> > > > only
> > > > against the 30-day trial version of ARM DS 2019-0 which is
> > > > shipped
> > > > with
> > > > compiler 6.12
> > > 
> > > Any step towards armclang is good :). We can look at the safety-
> > > certified
> > > version afterwards.
> > > 
> > > I don't seem to be able to link with this patch on 6.12:
> > > 
> > > Loading object built_in.o.
> > > Error: L6242E: Cannot link object built_in.o as its attributes
> > > are
> > > incompatible
> > > with the image attributes.
> > >      ... A64 clashes with SoftVFP.
> > > Error: L6242E: Cannot link object built_in.o as its attributes
> > > are
> > > incompatible
> > > with the image attributes.
> > >      ... A64 clashes with SoftVFP.
> > > Error: L6242E: Cannot link object built_in.o as its attributes
> > > are
> > > incompatible
> > > with the image attributes.
> > >      ... A64 clashes with SoftVFP.
> > > Error: L6242E: Cannot link object built_in.o as its attributes
> > > are
> > > incompatible
> > > with the image attributes.
> > >      ... A64 clashes with SoftVFP.
> > > 
> > > I am using the following command line to build:
> > > 
> > > 42sh> make CROSS_COMPILE=aarch64-linux-gnu- XEN_TARGET_ARCH=arm64
> > > clang=y
> > > armds=y  -C ~/works/xen/xen  XEN_CONFIG_EXPERT=y
> > > 
> > 
> > I am still struggling with linker internal fault, not sure what
> > changed
> > in my environment so it started to fail.
> > 
> > > > > > Also armlink sometimes fails with Internal fault:
> > > > > > [0xe81a5a:6120001]
> > > > > 
> > > > > Do you have more output?
> > > > 
> > > > Just opened a ticket in Arm community forums, see details there
> > > > including full build log (dont want to spam ml)
> > > > https://community.arm.com/developer/tools-software/tools/f/arm-compilers-forum/12989/linker-internal-fault-0xe81a5a-6120001
> > > > 
> > > > 
> > > 
> > > There are nothing interesting in except a flood of "armclang:
> > > warning: Your
> > > license for feature ds_suite_eval will expire in 29 days".
> > > 
> > > Let me know how it goes and I can help to resolve it.
> > > 
> > 
> > This is because of the "free" evaluation license I have, probably
> > can
> > be safely ignored.
> 
> I know :). I was pointed out how verbose it was compare to the lack
> of 
> information regarding the internal fault.
> 
> > So far no reaction on my initial post in Arm Community.
> > 
> > Playing with linker options I found that the error is caused by the
> > use
> > of "partial linking model" which is enabled by --partial that
> > correspond to -r in Xen Makefiles, which is not documented but has
> > similar effect.
> 
> Do you mean the "Internal Fault"?
> 

Yes.

But I kinda sorted this issue out. That was a dumb mistake - while
working I have moved to a different code tree that I used to check gcov
so it had CONFIG_COVERAGE enabled. I got rid of linker internal fault
after disabling it (though still not clear why this led to linker
failing with -r)

Now I am getting the same linker error you mentioned. Will continue
working on this after holidays, suspect that armlink missing proper
configuration and probably section placement.

> > > > > > [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-OW 
> > > > > > aft
> > > > > > 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?
> > > > 
> > > > error: unknown warning option '-Wno-unused-but-set-variable';
> > > > did
> > > > you
> > > > mean '-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-
> > > > option]
> > > 
> > > But cc-option-add should only add the flag if it is supported by
> > > the
> > > compiler.
> > > So it is not supported, how come this option is actually used to
> > > compile?
> > > 
> > 
> > Because the idea of supported option detection rely on the fact
> > that
> > compiler will throw an error or warning when makefile try and use
> > the
> > option being tested, but armclang does not even try to check that
> > and
> > failing with
> >    armclang: fatal error: no target architecture given; use --
> > target=arm-arm-none-eabi or --target=aarch64-arm-none-eabi
> > and thus cc-option-add does not detect unsupported options
> 
> I think defining CC as below should do the trick:
> 
> CC := armclang --target=aarch64-arm-none-eabi
> 

Agree

> > > > > > +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?
> > > > 
> > > > Yes. ropi/rwpi are according to Arm Compiler 6.12 reference
> > > > guide,
> > > > see
> > > > [100067_0612_00_en 1-54] and [100067_0612_00_en 1-56]
> > > 
> > > Hmmm, but the docs says:
> > > 
> > > "This option is not supported in AArch64 state". So is that a
> > > really
> > > good
> > > replacement?
> > > 
> > 
> > You're right, reading further in the document this does not seem to
> > be
> > a valid replacement. Moreover, it seems that we dont need this
> > option
> > with armclang - by default it does not generate position-
> > independent
> > code
> 
> As this always been the case? I know that for GCC it depends on the
> on 
> distro/version of the compiler.

I will check how armclang implements this later on.

> 
> > > [...]
> > > 
> > > > > You would still need --target=.... but that's should depend
> > > > > on
> > > > > $CROSS_COMPILE
> > > > > (or any other name we decide).
> > > > 
> > > > You're right, I forgot to mention - have not yet built for
> > > > Arm32 so
> > > > this is not checked. But, according to manual:
> > > > 
> > > > For targets in AArch32 state (--target=arm-arm-none-eabi),
> > > > there is
> > > > no
> > > > default. You must specify either -march (to target an
> > > > architecture)
> > > > or
> > > > -mcpu (to target a processor)
> > > 
> > > We actually pass -mcpu=cortex-a15 in arch/arm/Rules.mk (I admit
> > > that
> > > cortex-a15
> > > is probably not the most suitable). If some options should
> > > applied to
> > > the tools
> > > as well, then we need to migrated them to config/arm32.mk.
> > 
> > Yes, I do not think we shall specify the particular CPU, better to
> > have
> > lowest supported architecture defined.
> 
> I vaguely remembered to try to replace cortex-a15 with something else
> in 
> the past. IIRC, the problem was armv7-a does not necessarily include 
> virt extension.
> 
> Trying again today with -march=armv7-a instead of -mcpu=cortex-a15,
> I 
> can see build error when using Linaro GCC 6.1-2016.80:
> 
> {standard input}: Assembler messages:
> {standard input}:285: Error: selected processor does not support
> `smc 
> #0' in ARM mode
> {standard input}:429: Error: selected processor does not support
> `smc 
> #0' in ARM mode
> {standard input}:462: Error: selected processor does not support
> `smc 
> #0' in ARM mode
> {standard input}:539: Error: selected processor does not support
> `smc 
> #0' in ARM mode
> /home/julieng/works/xen/xen/Rules.mk:196: recipe for target 
> 'cpuerrata.o' failed
> make[3]: *** [cpuerrata.o] Error 1
> 
> I think this one is solvable by specifying the secure extension. But
> it 
> might be possible there are other issues with older compiler (i.e 4.6
> & co).
> 

Could be. We might need do do automatic checks with old compilers

> > > > [100067_0612_00_en page 1-82]
> > > > 
> > > > > > +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.
> > > > > 
> > > > 
> > > > If I don't specify -march this way, it will built:
> > > >    1) without some registers, e.g. TTLB0_EL2 support (build
> > > > fails)
> > > 
> > > I guess you mean TTBR0_EL2 here.
> > > 
> > > Xen is fully Armv8.0 compliant. So if you can't compile Xen with
> > > -march=armv8-a
> > > then this needs to be investigated and be reported.
> > > 
> > > Looking at the error thrown:
> > > 
> > > arm64/head.S:399:13: error: expected writable system register or
> > > pstate
> > >           msr TTBR0_EL2, x4
> > > 
> > > This is likely a compiler bug.
> > > 
> > 
> > Actually I do not know what is the proper way to report such issues
> > and
> > get them fixed. My post on ARM forum regarding linker failue has no
> > comments/reactions, which means it may not be the best way for
> > reporting. Would it be possible for you to check if we can get some
> > attention from ARM Compiler folks?
> 
> I will have a chat and let you know.
> 
> > > >    2) with vfp and simd enabled (linking fails)
> > > > 
> > > > According to Arm compiler manual:
> > > > ---
> > > > For targets in AArch64 state (--target=aarch64-arm-none-eabi),
> > > > unless
> > > > you target a particular processor using -mcpu or a particular
> > > > architecture using -march, the compiler defaults to
> > > > -march=armv8-a,
> > > > generating generic code for Armv8‑A in AArch64 state.
> > > > [100067_0612_00_en page 1-82]
> > > > ---
> > > > You can prevent the use of floating-point instructions or
> > > > floating-
> > > > point registers for targets in AArch64 state with the
> > > > -mcpu=name+nofp+nosimd option. Subsequent use of floating-point
> > > > data
> > > > types in this mode is unsupported.
> > > > [100067_0612_00_en page 1-93]
> > > > ---
> > > 
> > > I assume that -mgeneral-regs-only does not work in your case?
> > 
> > Nope :\
> 
> It would be interesting to know whether -march=armv8-a... works on
> all 
> GCC version we support.
> 
> Cheers,
> 
>