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

[Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>]

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