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

From: Artem Mygaiev <Artem_Mygaiev@epam.com>
To: "sstabellini@kernel.org" <sstabellini@kernel.org>,
	"julien.grall@arm.com" <julien.grall@arm.com>
Cc: "tamas@tklengyel.com" <tamas@tklengyel.com>,
	"wei.liu2@citrix.com" <wei.liu2@citrix.com>,
	"rcojocaru@bitdefender.com" <rcojocaru@bitdefender.com>,
	"konrad.wilk@oracle.com" <konrad.wilk@oracle.com>,
	"George.Dunlap@eu.citrix.com" <George.Dunlap@eu.citrix.com>,
	"andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>,
	"ian.jackson@eu.citrix.com" <ian.jackson@eu.citrix.com>,
	"tim@xen.org" <tim@xen.org>,
	Oleksandr Tyshchenko <Oleksandr_Tyshchenko@epam.com>,
	"jbeulich@suse.com" <jbeulich@suse.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrii Anisov <Andrii_Anisov@epam.com>
Subject: Re: [PATCH 00/12] xen/arm: Add support to build with clang
Date: Thu, 18 Apr 2019 11:15:02 +0000	[thread overview]
Message-ID: <18f9d557324a69437cd062aeced21e6b4759a968.camel@epam.com> (raw)
In-Reply-To: <91a841b7-424a-79a3-b551-15587bf92ed3@arm.com>

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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel