[Xen-devel] [RFC 0/7] Build XEN with ARM Compiler 6.6.3

[Xen-devel] [RFC 0/7] Build XEN with ARM Compiler 6.6.3

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Dear All,

Here are patches to make XEN (hypervisor only) being able to be compiled for
ARMv8 with ARM Compiler 6.6.3 Long Term Maintenance. ARM compiler of that
version is safety certified, that is the reason for the choice. The chosen
compiler has a number of bugs and deviations from GNU Compiler, which required
correspondent workarounds. These patches are published for those who are
interested in XEN being built by this specific compiler. Also, these patches are
WIP and there is a lot of room for improvement.

Here is the list of ARM Compiler 6.6.3 bugs and deviations affected XEN build:
 - ARM Linker scatter file is pretty primitive, it has no feature to define
   symbols
 - ARM Linker defined symbols are not counted as referred if only mentioned
   in a steering file for rename or resolve
 - C style shift operators are missed among supported scatter file expressions,
   so some needed values are hardcoded in scatter file
 - ARM Compiler tools are not able to rename sections
 - armclang compiles data only C files with SoftVFP attributes
 - static data symbols, moved to an init section, becomes global
 - armclang fails to process .rept assembler directive with an expression as a
   parameter.

The latest issue is addressed in a very primitive way: these patches are ported
on top of commit f11fda966365db591d280ac1522993409e20fd8c, prior to commit
introduced `.rept` directive usage.:

Andrii Anisov (5):
  arm64:armds: ARM Compiler 6.6 does not accept `rx` registers naming
    for AArch64
  arm/gic: Drop pointless assertions
  WIP:arm64:armds: Build XEN with ARM Compiler 6.6
  arm: Introduce dummy empty functions for data only C files
  arm/gic-v3: add GIC version suffix to iomem range variables

Artem Mygaiev (1):
  WIP: Compilation with ARM DS-6 compiler

Julien Grall (1):
  xen: clang: Support correctly cross-compile

 Config.mk                                  |   8 +-
 config/StdGNU.mk                           |  20 ++-
 config/arm32.mk                            |  10 +-
 config/arm64.mk                            |   6 +-
 xen/Rules.mk                               |   8 +
 xen/arch/arm/Makefile                      |  24 +++
 xen/arch/arm/Rules.mk                      |   8 +
 xen/arch/arm/gic-v3.c                      |  68 ++++----
 xen/arch/arm/gic.c                         |   6 -
 xen/arch/arm/platforms/brcm-raspberry-pi.c |   2 +
 xen/arch/arm/platforms/thunderx.c          |   2 +
 xen/arch/arm/xen.scat.S                    | 266 +++++++++++++++++++++++++++++
 xen/arch/arm/xen.steer                     |   5 +
 xen/include/asm-arm/armds.h                |  91 ++++++++++
 xen/include/asm-arm/smccc.h                |  60 +++++++
 xen/xsm/flask/gen-policy.py                |   4 +
 16 files changed, 539 insertions(+), 49 deletions(-)
 create mode 100644 xen/arch/arm/xen.scat.S
 create mode 100644 xen/arch/arm/xen.steer
 create mode 100644 xen/include/asm-arm/armds.h

-- 
2.7.4


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

[Xen-devel] [RFC 1/7] xen: clang: Support correctly cross-compile

[Andrii Anisov]

From: Julien Grall <julien.grall@arm.com>

Clang uses "-target" option for cross-compilation.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 config/StdGNU.mk | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 039274e..48c50b5 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -1,8 +1,13 @@
 AS         = $(CROSS_COMPILE)as
 LD         = $(CROSS_COMPILE)ld
 ifeq ($(clang),y)
-CC         = $(CROSS_COMPILE)clang
-CXX        = $(CROSS_COMPILE)clang++
+ifneq ($(CROSS_COMPILE),)
+CC         = clang -target $(CROSS_COMPILE:-=)
+CXX        = clang++ -target $(CROSS_COMPILE:-=)
+else
+CC         = clang
+CXX        = clang++
+endif
 LD_LTO     = $(CROSS_COMPILE)llvm-ld
 else
 CC         = $(CROSS_COMPILE)gcc
-- 
2.7.4


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

Re: [Xen-devel] [RFC 1/7] xen: clang: Support correctly cross-compile

[Wei Liu]

On Wed, Nov 06, 2019 at 11:19:07AM +0200, Andrii Anisov wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> Clang uses "-target" option for cross-compilation.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  config/StdGNU.mk | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 039274e..48c50b5 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -1,8 +1,13 @@
>  AS         = $(CROSS_COMPILE)as
>  LD         = $(CROSS_COMPILE)ld
>  ifeq ($(clang),y)
> -CC         = $(CROSS_COMPILE)clang
> -CXX        = $(CROSS_COMPILE)clang++
> +ifneq ($(CROSS_COMPILE),)
> +CC         = clang -target $(CROSS_COMPILE:-=)
> +CXX        = clang++ -target $(CROSS_COMPILE:-=)
> +else
> +CC         = clang
> +CXX        = clang++
> +endif
>  LD_LTO     = $(CROSS_COMPILE)llvm-ld

Do you not need to fix llvm-ld too? I _think_ the relevant option is
-march.

Wei.

>  else
>  CC         = $(CROSS_COMPILE)gcc
> -- 
> 2.7.4
> 

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

Re: [Xen-devel] [RFC 1/7] xen: clang: Support correctly cross-compile

[Jan Beulich]

On 06.11.2019 10:19, Andrii Anisov wrote:
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -1,8 +1,13 @@
>  AS         = $(CROSS_COMPILE)as
>  LD         = $(CROSS_COMPILE)ld
>  ifeq ($(clang),y)
> -CC         = $(CROSS_COMPILE)clang
> -CXX        = $(CROSS_COMPILE)clang++
> +ifneq ($(CROSS_COMPILE),)
> +CC         = clang -target $(CROSS_COMPILE:-=)
> +CXX        = clang++ -target $(CROSS_COMPILE:-=)

And what guarantees that (with the hyphens dropped) the prefix
$(CROSS_COMPILE) originally represents to a valid value for
clang's -target? Please don't forget that people may use non-
standard $(CROSS_COMPILE) setting as well (to e.g. match their
local setup of how to invoke cross compilers).

Jan

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

Re: [Xen-devel] [RFC 1/7] xen: clang: Support correctly cross-compile

[Julien Grall]

Hi,

On 06/11/2019 09:19, Andrii Anisov wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> Clang uses "-target" option for cross-compilation.

While I appreaciate you want to carry this work, there were a lenghty 
discussion when I sent the patch (see [1]). This should have been 
addressed before resending it (even part of an RFC).

But, AFAICT, you don't use clang=y for this series. So why did you 
include it in this series?

Cheers,

[1] <20190327184531.30986-2-julien.grall@arm.com>
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>   config/StdGNU.mk | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 039274e..48c50b5 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -1,8 +1,13 @@
>   AS         = $(CROSS_COMPILE)as
>   LD         = $(CROSS_COMPILE)ld
>   ifeq ($(clang),y)
> -CC         = $(CROSS_COMPILE)clang
> -CXX        = $(CROSS_COMPILE)clang++
> +ifneq ($(CROSS_COMPILE),)
> +CC         = clang -target $(CROSS_COMPILE:-=)
> +CXX        = clang++ -target $(CROSS_COMPILE:-=)
> +else
> +CC         = clang
> +CXX        = clang++
> +endif
>   LD_LTO     = $(CROSS_COMPILE)llvm-ld
>   else
>   CC         = $(CROSS_COMPILE)gcc
> 

-- 
Julien Grall

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

[Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Andrii Anisov]

From: Artem Mygaiev <joculator@gmail.com>

Still have linker issues
---
 Config.mk             |  8 +++++++-
 config/StdGNU.mk      | 11 ++++++++++-
 config/arm32.mk       | 10 ++++++----
 config/arm64.mk       |  6 +++++-
 xen/Rules.mk          |  2 ++
 xen/arch/arm/Rules.mk |  8 ++++++++
 6 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/Config.mk b/Config.mk
index d8f90d7..01487a7 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,13 @@ 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
 
+ifneq ($(armds),y)
+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 48c50b5..3bf3462 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 --verbose --no_scanlib
+LD         = armlink --verbose --no_scanlib
+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 f95228e..5afed07 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 aa45772..46b203d 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 3090ea7..41a1c26 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -79,9 +79,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)
 
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 3d9a0ed..6f2b239 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -11,12 +11,20 @@ CFLAGS += -I$(BASEDIR)/include
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
 
+ifneq ($(armds),y)
 # Prevent floating-point variables from creeping into Xen.
 CFLAGS-$(CONFIG_ARM_32) += -msoft-float
 CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
 
 CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
 CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
+else
+CFLAGS-$(CONFIG_ARM_32) += -msoft-float
+CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
+
+CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
+CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
+endif
 
 ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
 CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
-- 
2.7.4


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

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Jan Beulich]

On 06.11.2019 10:19, Andrii Anisov wrote:
> From: Artem Mygaiev <joculator@gmail.com>
> 
> Still have linker issues

This may be acceptable for an RFC series, but doesn't justify there
not being any other description. Just to give an example,I'd like to
understand why ...

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

... this would be necessary.

Jan

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

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Artem Mygaiev]

Hello Jan

On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 06.11.2019 10:19, Andrii Anisov wrote:
> > From: Artem Mygaiev <joculator@gmail.com>
> >
> > Still have linker issues
>
> This may be acceptable for an RFC series, but doesn't justify there
> not being any other description. Just to give an example,I'd like to
> understand why ...
>
> > --- 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))
>
> ... this would be necessary.

I am very sorry, this patch does not have a proper description indeed.

For this particular change - arm clang does not undestand
-Wno-unused-but-set-variable
option at all, that is why it is under !$(armds)

>
> Jan

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

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Jan Beulich]

On 06.11.2019 23:08, Artem Mygaiev wrote:
> On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 06.11.2019 10:19, Andrii Anisov wrote:
>>> --- 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))
>>
>> ... this would be necessary.
> 
> I am very sorry, this patch does not have a proper description indeed.
> 
> For this particular change - arm clang does not undestand
> -Wno-unused-but-set-variable
> option at all, that is why it is under !$(armds)

But avoiding to add options which the compiler doesn't understand
is the purpose of using cc-option-add, rather than blindly adding
them to CFLAGS. What am I missing here?

Jan

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

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Artem Mygaiev]

Hi Jan,

Sorry for delayed reply

On Thu, 2019-11-07 at 08:31 +0100, Jan Beulich wrote:
> On 06.11.2019 23:08, Artem Mygaiev wrote:
> > On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <
> > jbeulich@suse.com
> > > wrote:
> > > On 06.11.2019 10:19, Andrii Anisov wrote:
> > > > --- 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))
> > > 
> > > ... this would be necessary.
> > 
> > I am very sorry, this patch does not have a proper description
> > indeed.
> > 
> > For this particular change - arm clang does not undestand
> > -Wno-unused-but-set-variable
> > option at all, that is why it is under !$(armds)
> 
> But avoiding to add options which the compiler doesn't understand
> is the purpose of using cc-option-add, rather than blindly
> adding
> them to CFLAGS. What am I missing here?

You are right, the script shall check the compiler option and avoid
including it to CFLAGS. But armclang require '--target=...' to be
specified in order to operate properly, and the proper fix shall be
something like this (instead of 'ifneq' hack above):

diff --git a/Config.mk b/Config.mk
index 01487a7..abe8e44 100644
--- a/Config.mk
+++ b/Config.mk
@@ -107,7 +107,7 @@ cc-option = $(shell if test -z "`echo 'void*p=1;' | \
 # Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
 cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
 define cc-option-add-closure
-    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
+    ifneq ($$(call cc-option,$$($(2) $(1)),$(3),n),n)
         $(1) += $(3)
     endif
 endef

so that CFLAGS that are already defined and include '--target=...'
option from config/arm*.mk are passed to compiler to make it happy. I
am not sure if this breaks anything else so decided to go with ugly
'ifneq' hack and check how this can be solved later on.

> Jan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> 
> https://urldefense.com/v3/__https://lists.xenproject.org/mailman/listinfo/xen-devel__;!K6dmGCEab4ueJg!mfAKUfGDnRPgNHksMlffaLrptu7demkLHApa3STsHRSKyoHnusYbCLzhLjD8K_vpFw$
>  
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 2537 bytes --]

On Thu, 14 Nov 2019, 02:15 Artem Mygaiev, <Artem_Mygaiev@epam.com> wrote:

> Hi Jan,
>
> Sorry for delayed reply
>
> On Thu, 2019-11-07 at 08:31 +0100, Jan Beulich wrote:
> > On 06.11.2019 23:08, Artem Mygaiev wrote:
> > > On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <
> > > jbeulich@suse.com
> > > > wrote:
> > > > On 06.11.2019 10:19, Andrii Anisov wrote:
> > > > > --- 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))
> > > >
> > > > ... this would be necessary.
> > >
> > > I am very sorry, this patch does not have a proper description
> > > indeed.
> > >
> > > For this particular change - arm clang does not undestand
> > > -Wno-unused-but-set-variable
> > > option at all, that is why it is under !$(armds)
> >
> > But avoiding to add options which the compiler doesn't understand
> > is the purpose of using cc-option-add, rather than blindly
> > adding
> > them to CFLAGS. What am I missing here?
>
> You are right, the script shall check the compiler option and avoid
> including it to CFLAGS. But armclang require '--target=...' to be
> specified in order to operate properly, and the proper fix shall be
> something like this (instead of 'ifneq' hack above):
>
> diff --git a/Config.mk b/Config.mk
> index 01487a7..abe8e44 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -107,7 +107,7 @@ cc-option = $(shell if test -z "`echo 'void*p=1;' | \
>  # Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
>  cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
>  define cc-option-add-closure
> -    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
> +    ifneq ($$(call cc-option,$$($(2) $(1)),$(3),n),n)
>          $(1) += $(3)
>      endif
>  endef
>
> so that CFLAGS that are already defined and include '--target=...'
> option from config/arm*.mk are passed to compiler to make it happy. I
> am not sure if this breaks anything else so decided to go with ugly
> 'ifneq' hack and check how this can be solved later on.
>

Why not including --target in CC variable as this was suggested for clang?

Cheers,

-- 
Julien Grall

[-- Attachment #1.2: Type: text/html, Size: 3577 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Artem Mygaiev]

Hello Julien

On Thu, 2019-11-14 at 08:19 +0900, Julien Grall wrote:
> 
> 
> On Thu, 14 Nov 2019, 02:15 Artem Mygaiev, <
> Artem_Mygaiev@epam.com> wrote:
> > Hi Jan,
> > 
> > Sorry for delayed reply
> > 
> > On Thu, 2019-11-07 at 08:31 +0100, Jan Beulich wrote:
> > > On 06.11.2019 23:08, Artem Mygaiev wrote:
> > > > On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <
> > > > jbeulich@suse.com
> > > > > wrote:
> > > > > On 06.11.2019 10:19, Andrii Anisov wrote:
> > > > > > --- 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))
> > > > > 
> > > > > ... this would be necessary.
> > > > 
> > > > I am very sorry, this patch does not have a proper description
> > > > indeed.
> > > > 
> > > > For this particular change - arm clang does not undestand
> > > > -Wno-unused-but-set-variable
> > > > option at all, that is why it is under !$(armds)
> > > 
> > > But avoiding to add options which the compiler doesn't understand
> > > is the purpose of using cc-option-add, rather than blindly
> > > adding
> > > them to CFLAGS. What am I missing here?
> > 
> > You are right, the script shall check the compiler option and avoid
> > including it to CFLAGS. But armclang require '--target=...' to be
> > specified in order to operate properly, and the proper fix shall be
> > something like this (instead of 'ifneq' hack above):
> > 
> > diff --git a/Config.mk b/Config.mk
> > index 01487a7..abe8e44 100644
> > --- a/Config.mk
> > +++ b/Config.mk
> > @@ -107,7 +107,7 @@ cc-option = $(shell if test -z "`echo
> > 'void*p=1;' | \
> >  # Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
> >  cc-option-add = $(eval $(call cc-option-add-
> > closure,$(1),$(2),$(3)))
> >  define cc-option-add-closure
> > -    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
> > +    ifneq ($$(call cc-option,$$($(2) $(1)),$(3),n),n)
> >          $(1) += $(3)
> >      endif
> >  endef
> > 
> > so that CFLAGS that are already defined and include '--target=...'
> > option from config/arm*.mk are passed to compiler to make it happy.
> > I
> > am not sure if this breaks anything else so decided to go with ugly
> > 'ifneq' hack and check how this can be solved later on.
> 
> 
> Why not including --target in CC variable as this was suggested for
> clang?

In case of armclang --target is not the same as CROSS_COMPILE, we would
need to introduce an extra variable instead of CFLAGS and then pass it
to the compiler in similar way -target passed to clang:

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 3bf3462..4bcfc58 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -3,8 +3,8 @@ AR         = $(CROSS_COMPILE)ar
 LD         = $(CROSS_COMPILE)ld
 ifeq ($(clang),y)
 ifeq ($(armds),y)
-CC         = armclang
-CXX        = armclang
+CC         = armclang --target=$(ARMDS_TARGET)
+CXX        = armclang --target=$(ARMDS_TARGET)
 LD_LTO     = armlink --verbose --no_scanlib
 LD         = armlink --verbose --no_scanlib
 AS         = armasm
diff --git a/config/arm32.mk b/config/arm32.mk
index 5afed07..b4c8fb1 100644
--- a/config/arm32.mk
+++ b/config/arm32.mk
@@ -4,10 +4,12 @@ CONFIG_ARM_$(XEN_OS) := y
 
 CONFIG_XEN_INSTALL_SUFFIX :=
 
+ARMDS_TARGET := arm-arm-none-eabi
+
 # Explicitly specifiy 32-bit ARM ISA since toolchain default can be
-mthumb:
 ifeq ($(armds),y)
 # VE needed
-CFLAGS += --target=arm-arm-none-eabi -march=armv7-a
+CFLAGS += -march=armv7-a
 else
 CFLAGS += -marm # -march= -mcpu=
 # Use only if calling $(LD) directly.
diff --git a/config/arm64.mk b/config/arm64.mk
index 46b203d..57a7335 100644
--- a/config/arm64.mk
+++ b/config/arm64.mk
@@ -4,9 +4,11 @@ CONFIG_ARM_$(XEN_OS) := y
 
 CONFIG_XEN_INSTALL_SUFFIX :=
 
+ARMDS_TARGET := aarch64-arm-none-eabi
+
 ifeq ($(armds),y)
 # VE needed
-CFLAGS += --target=aarch64-arm-none-eabi -march=armv8.1-a+nofp+nosimd
+CFLAGS += -march=armv8.1-a+nofp+nosimd
 else
 CFLAGS += #-marm -march= -mcpu= etc
 # Use only if calling $(LD) directly.

But personally, I really do not want to add more build variables and
flags (would also drop the 'armds' if I find a way how). Instead, I'd
prefer the idea of re-using known CFLAGS during the cc-option tests,
but, as I wrote above, wasn't sure if this is a right/safe thing to do,
so while working on it I just quickly hacked out the option causing
issues limiting amount of changes.

> 
> Cheers,
> 
> -- 
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Julien Grall]

On 14/11/2019 14:12, Artem Mygaiev wrote:
> Hello Julien

Hi,

> 
> On Thu, 2019-11-14 at 08:19 +0900, Julien Grall wrote:
>>
>>
>> On Thu, 14 Nov 2019, 02:15 Artem Mygaiev, <
>> Artem_Mygaiev@epam.com> wrote:
>>> Hi Jan,
>>>
>>> Sorry for delayed reply
>>>
>>> On Thu, 2019-11-07 at 08:31 +0100, Jan Beulich wrote:
>>>> On 06.11.2019 23:08, Artem Mygaiev wrote:
>>>>> On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <
>>>>> jbeulich@suse.com
>>>>>> wrote:
>>>>>> On 06.11.2019 10:19, Andrii Anisov wrote:
>>>>>>> --- 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))
>>>>>>
>>>>>> ... this would be necessary.
>>>>>
>>>>> I am very sorry, this patch does not have a proper description
>>>>> indeed.
>>>>>
>>>>> For this particular change - arm clang does not undestand
>>>>> -Wno-unused-but-set-variable
>>>>> option at all, that is why it is under !$(armds)
>>>>
>>>> But avoiding to add options which the compiler doesn't understand
>>>> is the purpose of using cc-option-add, rather than blindly
>>>> adding
>>>> them to CFLAGS. What am I missing here?
>>>
>>> You are right, the script shall check the compiler option and avoid
>>> including it to CFLAGS. But armclang require '--target=...' to be
>>> specified in order to operate properly, and the proper fix shall be
>>> something like this (instead of 'ifneq' hack above):
>>>
>>> diff --git a/Config.mk b/Config.mk
>>> index 01487a7..abe8e44 100644
>>> --- a/Config.mk
>>> +++ b/Config.mk
>>> @@ -107,7 +107,7 @@ cc-option = $(shell if test -z "`echo
>>> 'void*p=1;' | \
>>>   # Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
>>>   cc-option-add = $(eval $(call cc-option-add-
>>> closure,$(1),$(2),$(3)))
>>>   define cc-option-add-closure
>>> -    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
>>> +    ifneq ($$(call cc-option,$$($(2) $(1)),$(3),n),n)
>>>           $(1) += $(3)
>>>       endif
>>>   endef
>>>
>>> so that CFLAGS that are already defined and include '--target=...'
>>> option from config/arm*.mk are passed to compiler to make it happy.
>>> I
>>> am not sure if this breaks anything else so decided to go with ugly
>>> 'ifneq' hack and check how this can be solved later on.
>>
>>
>> Why not including --target in CC variable as this was suggested for
>> clang?
> 
> In case of armclang --target is not the same as CROSS_COMPILE, we would
> need to introduce an extra variable instead of CFLAGS and then pass it
> to the compiler in similar way -target passed to clang:

IHMO, --target (armds) and -target (clang) are exactly the same. You 
specify the targeted architecture to build. So I think we need a similar 
approach in the both case. Although, in clang there are a default one 
when not specified.

I agree that using CROSS_COMPILE is a bit of a stretch (even on clang). 
There was actually a lenghty discussion (see [1]) about the meaning of 
CROSS_COMPILE. Maybe we want to introduce a new variable (e.g. TARGET) 
that can be used to pass the triplet.

> 
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 3bf3462..4bcfc58 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -3,8 +3,8 @@ AR         = $(CROSS_COMPILE)ar
>   LD         = $(CROSS_COMPILE)ld
>   ifeq ($(clang),y)
>   ifeq ($(armds),y)
> -CC         = armclang
> -CXX        = armclang
> +CC         = armclang --target=$(ARMDS_TARGET)
> +CXX        = armclang --target=$(ARMDS_TARGET)
>   LD_LTO     = armlink --verbose --no_scanlib
>   LD         = armlink --verbose --no_scanlib
>   AS         = armasm
> diff --git a/config/arm32.mk b/config/arm32.mk
> index 5afed07..b4c8fb1 100644
> --- a/config/arm32.mk
> +++ b/config/arm32.mk
> @@ -4,10 +4,12 @@ CONFIG_ARM_$(XEN_OS) := y
>   
>   CONFIG_XEN_INSTALL_SUFFIX :=
>   
> +ARMDS_TARGET := arm-arm-none-eabi
> +
>   # Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> -mthumb:
>   ifeq ($(armds),y)
>   # VE needed
> -CFLAGS += --target=arm-arm-none-eabi -march=armv7-a
> +CFLAGS += -march=armv7-a
>   else
>   CFLAGS += -marm # -march= -mcpu=
>   # Use only if calling $(LD) directly.
> diff --git a/config/arm64.mk b/config/arm64.mk
> index 46b203d..57a7335 100644
> --- a/config/arm64.mk
> +++ b/config/arm64.mk
> @@ -4,9 +4,11 @@ CONFIG_ARM_$(XEN_OS) := y
>   
>   CONFIG_XEN_INSTALL_SUFFIX :=
>   
> +ARMDS_TARGET := aarch64-arm-none-eabi
> +
>   ifeq ($(armds),y)
>   # VE needed
> -CFLAGS += --target=aarch64-arm-none-eabi -march=armv8.1-a+nofp+nosimd
> +CFLAGS += -march=armv8.1-a+nofp+nosimd
>   else
>   CFLAGS += #-marm -march= -mcpu= etc
>   # Use only if calling $(LD) directly.
> 
> But personally, I really do not want to add more build variables and
> flags (would also drop the 'armds' if I find a way how). Instead, I'd
> prefer the idea of re-using known CFLAGS during the cc-option tests,
> but, as I wrote above, wasn't sure if this is a right/safe thing to do,
> so while working on it I just quickly hacked out the option causing
> issues limiting amount of changes.

The question here is whether the target is always fixed for arm64/arm32. 
Are the two triplets used the only existing for armds?

Cheers,

[1]  <20190327184531.30986-2-julien.grall@arm.com>

-- 
Julien Grall

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

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Artem Mygaiev]

Hi Julien

On Mon, 2019-11-18 at 06:18 +0000, Julien Grall wrote:
> 
> On 14/11/2019 14:12, Artem Mygaiev wrote:
> > Hello Julien
> 
> Hi,
> 
> > On Thu, 2019-11-14 at 08:19 +0900, Julien Grall wrote:
> > > 
> > > On Thu, 14 Nov 2019, 02:15 Artem Mygaiev, <
> > > Artem_Mygaiev@epam.com
> > > > wrote:
> > > > Hi Jan,
> > > > 
> > > > Sorry for delayed reply
> > > > 
> > > > On Thu, 2019-11-07 at 08:31 +0100, Jan Beulich wrote:
> > > > > On 06.11.2019 23:08, Artem Mygaiev wrote:
> > > > > > On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <
> > > > > > jbeulich@suse.com
> > > > > > 
> > > > > > > wrote:
> > > > > > > On 06.11.2019 10:19, Andrii Anisov wrote:
> > > > > > > > --- 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))
> > > > > > > 
> > > > > > > ... this would be necessary.
> > > > > > 
> > > > > > I am very sorry, this patch does not have a proper
> > > > > > description
> > > > > > indeed.
> > > > > > 
> > > > > > For this particular change - arm clang does not undestand
> > > > > > -Wno-unused-but-set-variable
> > > > > > option at all, that is why it is under !$(armds)
> > > > > 
> > > > > But avoiding to add options which the compiler doesn't
> > > > > understand
> > > > > is the purpose of using cc-option-add, rather than blindly
> > > > > adding
> > > > > them to CFLAGS. What am I missing here?
> > > > 
> > > > You are right, the script shall check the compiler option and
> > > > avoid
> > > > including it to CFLAGS. But armclang require '--target=...' to
> > > > be
> > > > specified in order to operate properly, and the proper fix
> > > > shall be
> > > > something like this (instead of 'ifneq' hack above):
> > > > 
> > > > diff --git a/Config.mk b/Config.mk
> > > > index 01487a7..abe8e44 100644
> > > > --- a/Config.mk
> > > > +++ b/Config.mk
> > > > @@ -107,7 +107,7 @@ cc-option = $(shell if test -z "`echo
> > > > 'void*p=1;' | \
> > > >   # Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
> > > >   cc-option-add = $(eval $(call cc-option-add-
> > > > closure,$(1),$(2),$(3)))
> > > >   define cc-option-add-closure
> > > > -    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
> > > > +    ifneq ($$(call cc-option,$$($(2) $(1)),$(3),n),n)
> > > >           $(1) += $(3)
> > > >       endif
> > > >   endef
> > > > 
> > > > so that CFLAGS that are already defined and include '
> > > > --target=...'
> > > > option from config/arm*.mk are passed to compiler to make it
> > > > happy.
> > > > I
> > > > am not sure if this breaks anything else so decided to go with
> > > > ugly
> > > > 'ifneq' hack and check how this can be solved later on.
> > > 
> > > 
> > > Why not including --target in CC variable as this was suggested
> > > for
> > > clang?
> > 
> > In case of armclang --target is not the same as CROSS_COMPILE, we
> > would
> > need to introduce an extra variable instead of CFLAGS and then pass
> > it
> > to the compiler in similar way -target passed to clang:
> 
> IHMO, --target (armds) and -target (clang) are exactly the same. You 
> specify the targeted architecture to build. So I think we need a
> similar 
> approach in the both case. Although, in clang there are a default
> one 
> when not specified.

Unfortunately this is not the case - we need to specify 2 different
targets: 1st is for GNU tools which are still used when building with
armclang, for debug symbols dumping etc. (there is no replacement in
Arm toolstack for them, and this is not on safety critical path so we
are fine) and 2nd is for armclang itself because it has its own
triplets definition which is not consistent with those used by GNU
tools.

> 
> I agree that using CROSS_COMPILE is a bit of a stretch (even on
> clang). 
> There was actually a lenghty discussion (see [1]) about the meaning
> of 
> CROSS_COMPILE. Maybe we want to introduce a new variable (e.g.
> TARGET) 
> that can be used to pass the triplet.
> 
> > diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> > index 3bf3462..4bcfc58 100644
> > --- a/config/StdGNU.mk
> > +++ b/config/StdGNU.mk
> > @@ -3,8 +3,8 @@ AR         = $(CROSS_COMPILE)ar
> >   LD         = $(CROSS_COMPILE)ld
> >   ifeq ($(clang),y)
> >   ifeq ($(armds),y)
> > -CC         = armclang
> > -CXX        = armclang
> > +CC         = armclang --target=$(ARMDS_TARGET)
> > +CXX        = armclang --target=$(ARMDS_TARGET)
> >   LD_LTO     = armlink --verbose --no_scanlib
> >   LD         = armlink --verbose --no_scanlib
> >   AS         = armasm
> > diff --git a/config/arm32.mk b/config/arm32.mk
> > index 5afed07..b4c8fb1 100644
> > --- a/config/arm32.mk
> > +++ b/config/arm32.mk
> > @@ -4,10 +4,12 @@ CONFIG_ARM_$(XEN_OS) := y
> >   
> >   CONFIG_XEN_INSTALL_SUFFIX :=
> >   
> > +ARMDS_TARGET := arm-arm-none-eabi
> > +
> >   # Explicitly specifiy 32-bit ARM ISA since toolchain default can
> > be
> > -mthumb:
> >   ifeq ($(armds),y)
> >   # VE needed
> > -CFLAGS += --target=arm-arm-none-eabi -march=armv7-a
> > +CFLAGS += -march=armv7-a
> >   else
> >   CFLAGS += -marm # -march= -mcpu=
> >   # Use only if calling $(LD) directly.
> > diff --git a/config/arm64.mk b/config/arm64.mk
> > index 46b203d..57a7335 100644
> > --- a/config/arm64.mk
> > +++ b/config/arm64.mk
> > @@ -4,9 +4,11 @@ CONFIG_ARM_$(XEN_OS) := y
> >   
> >   CONFIG_XEN_INSTALL_SUFFIX :=
> >   
> > +ARMDS_TARGET := aarch64-arm-none-eabi
> > +
> >   ifeq ($(armds),y)
> >   # VE needed
> > -CFLAGS += --target=aarch64-arm-none-eabi -march=armv8.1-
> > a+nofp+nosimd
> > +CFLAGS += -march=armv8.1-a+nofp+nosimd
> >   else
> >   CFLAGS += #-marm -march= -mcpu= etc
> >   # Use only if calling $(LD) directly.
> > 
> > But personally, I really do not want to add more build variables
> > and
> > flags (would also drop the 'armds' if I find a way how). Instead,
> > I'd
> > prefer the idea of re-using known CFLAGS during the cc-option
> > tests,
> > but, as I wrote above, wasn't sure if this is a right/safe thing to
> > do,
> > so while working on it I just quickly hacked out the option causing
> > issues limiting amount of changes.
> 
> The question here is whether the target is always fixed for
> arm64/arm32. 
> Are the two triplets used the only existing for armds?

They are fixed and defined in Arm compiler doc, only 2 options exist
for Aarch32 and Aarch64 architectures: --target=aarch64-arm-none-eabi
or --target=arm-arm-none-eabi. See e.g. [2]

[2] 
https://developer.arm.com/docs/100748/0612/using-common-compiler-options/common-arm-compiler-toolchain-options

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

Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Julien Grall]

Hi Artem,

On 19/11/2019 15:16, Artem Mygaiev wrote:
> 
> Unfortunately this is not the case - we need to specify 2 different
> targets: 1st is for GNU tools which are still used when building with
> armclang, for debug symbols dumping etc. (there is no replacement in
> Arm toolstack for them, and this is not on safety critical path so we
> are fine) and 2nd is for armclang itself because it has its own
> triplets definition which is not consistent with those used by GNU
> tools.

Well, in theory they may be different for clang. It just happens they 
are both similar at the moment.

There were also some concern to use CROSS_COMPILE as --target (see [1]), 
so I think we want to introduce a new option that would happen to be 
equal to CROSS_COMPILE for clang.

Cheers,

-- 
Julien Grall

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

[Xen-devel] [RFC 3/7] arm64:armds: ARM Compiler 6.6 does not accept `rx` registers naming for AArch64

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

So get the code duplication with `x`-es.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/include/asm-arm/smccc.h | 60 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 126399d..3fa1144 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -120,6 +120,8 @@ struct arm_smccc_res {
 #define __constraint_read_6 __constraint_read_5, "r" (r6)
 #define __constraint_read_7 __constraint_read_6, "r" (r7)
 
+#ifdef CONFIG_ARM_32
+
 #define __declare_arg_0(a0, res)                        \
     struct arm_smccc_res    *___res = res;              \
     register unsigned long  r0 asm("r0") = (uint32_t)a0;\
@@ -174,6 +176,64 @@ struct arm_smccc_res {
     __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
     register typeof(a7) r7 asm("r7") = __a7
 
+#else /* ARM_64 */
+
+#define __declare_arg_0(a0, res)                        \
+    struct arm_smccc_res    *___res = res;              \
+    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
+    register unsigned long  r1 asm("x1");               \
+    register unsigned long  r2 asm("x2");               \
+    register unsigned long  r3 asm("x3")
+
+#define __declare_arg_1(a0, a1, res)                    \
+    typeof(a1) __a1 = a1;                               \
+    struct arm_smccc_res    *___res = res;              \
+    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
+    register unsigned long  r1 asm("x1") = __a1;        \
+    register unsigned long  r2 asm("x2");               \
+    register unsigned long  r3 asm("x3")
+
+#define __declare_arg_2(a0, a1, a2, res)                \
+    typeof(a1) __a1 = a1;                               \
+    typeof(a2) __a2 = a2;                               \
+    struct arm_smccc_res    *___res = res;                              \
+    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
+    register unsigned long  r1 asm("x1") = __a1;        \
+    register unsigned long  r2 asm("x2") = __a2;        \
+    register unsigned long  r3 asm("x3")
+
+#define __declare_arg_3(a0, a1, a2, a3, res)            \
+    typeof(a1) __a1 = a1;                               \
+    typeof(a2) __a2 = a2;                               \
+    typeof(a3) __a3 = a3;                               \
+    struct arm_smccc_res    *___res = res;              \
+    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
+    register unsigned long  r1 asm("x1") = __a1;        \
+    register unsigned long  r2 asm("x2") = __a2;        \
+    register unsigned long  r3 asm("x3") = __a3
+
+#define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
+    typeof(a4) __a4 = a4;                               \
+    __declare_arg_3(a0, a1, a2, a3, res);               \
+    register unsigned long r4 asm("x4") = __a4
+
+#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
+    typeof(a5) __a5 = a5;                               \
+    __declare_arg_4(a0, a1, a2, a3, a4, res);           \
+    register typeof(a5) r5 asm("x5") = __a5
+
+#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
+    typeof(a6) __a6 = a6;                                   \
+    __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
+    register typeof(a6) r6 asm("x6") = __a6
+
+#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
+    typeof(a7) __a7 = a7;                                       \
+    __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
+    register typeof(a7) r7 asm("x7") = __a7
+
+#endif
+
 #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
 #define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
 
-- 
2.7.4


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

Re: [Xen-devel] [RFC 3/7] arm64:armds: ARM Compiler 6.6 does not accept `rx` registers naming for AArch64

[Jan Beulich]

On 06.11.2019 10:19, Andrii Anisov wrote:
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -120,6 +120,8 @@ struct arm_smccc_res {
>  #define __constraint_read_6 __constraint_read_5, "r" (r6)
>  #define __constraint_read_7 __constraint_read_6, "r" (r7)
>  
> +#ifdef CONFIG_ARM_32
> +
>  #define __declare_arg_0(a0, res)                        \
>      struct arm_smccc_res    *___res = res;              \
>      register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> @@ -174,6 +176,64 @@ struct arm_smccc_res {
>      __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
>      register typeof(a7) r7 asm("r7") = __a7
>  
> +#else /* ARM_64 */
> +
> +#define __declare_arg_0(a0, res)                        \
> +    struct arm_smccc_res    *___res = res;              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1");               \
> +    register unsigned long  r2 asm("x2");               \
> +    register unsigned long  r3 asm("x3")
> +
> +#define __declare_arg_1(a0, a1, res)                    \
> +    typeof(a1) __a1 = a1;                               \
> +    struct arm_smccc_res    *___res = res;              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1") = __a1;        \
> +    register unsigned long  r2 asm("x2");               \
> +    register unsigned long  r3 asm("x3")
> +
> +#define __declare_arg_2(a0, a1, a2, res)                \
> +    typeof(a1) __a1 = a1;                               \
> +    typeof(a2) __a2 = a2;                               \
> +    struct arm_smccc_res    *___res = res;                              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1") = __a1;        \
> +    register unsigned long  r2 asm("x2") = __a2;        \
> +    register unsigned long  r3 asm("x3")
> +
> +#define __declare_arg_3(a0, a1, a2, a3, res)            \
> +    typeof(a1) __a1 = a1;                               \
> +    typeof(a2) __a2 = a2;                               \
> +    typeof(a3) __a3 = a3;                               \
> +    struct arm_smccc_res    *___res = res;              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1") = __a1;        \
> +    register unsigned long  r2 asm("x2") = __a2;        \
> +    register unsigned long  r3 asm("x3") = __a3
> +
> +#define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
> +    typeof(a4) __a4 = a4;                               \
> +    __declare_arg_3(a0, a1, a2, a3, res);               \
> +    register unsigned long r4 asm("x4") = __a4
> +
> +#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
> +    typeof(a5) __a5 = a5;                               \
> +    __declare_arg_4(a0, a1, a2, a3, a4, res);           \
> +    register typeof(a5) r5 asm("x5") = __a5
> +
> +#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
> +    typeof(a6) __a6 = a6;                                   \
> +    __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
> +    register typeof(a6) r6 asm("x6") = __a6
> +
> +#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
> +    typeof(a7) __a7 = a7;                                       \
> +    __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
> +    register typeof(a7) r7 asm("x7") = __a7
> +
> +#endif

I'm not an Arm maintainer, so my opinion may not mean much, but
this is way too much code duplication for my taste. Isn't all you
need an abstraction of the "r0" etc vs "x0" etc strings? Or even
better, can't use to the "x0" etc form with the other compilers
(seeing that these are their architectural names when taking the
full with registers)?

Jan

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

Re: [Xen-devel] [RFC 3/7] arm64:armds: ARM Compiler 6.6 does not accept `rx` registers naming for AArch64

[Stefano Stabellini]

On Wed, 6 Nov 2019, Jan Beulich wrote:
> On 06.11.2019 10:19, Andrii Anisov wrote:
> > --- a/xen/include/asm-arm/smccc.h
> > +++ b/xen/include/asm-arm/smccc.h
> > @@ -120,6 +120,8 @@ struct arm_smccc_res {
> >  #define __constraint_read_6 __constraint_read_5, "r" (r6)
> >  #define __constraint_read_7 __constraint_read_6, "r" (r7)
> >  
> > +#ifdef CONFIG_ARM_32
> > +
> >  #define __declare_arg_0(a0, res)                        \
> >      struct arm_smccc_res    *___res = res;              \
> >      register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> > @@ -174,6 +176,64 @@ struct arm_smccc_res {
> >      __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
> >      register typeof(a7) r7 asm("r7") = __a7
> >  
> > +#else /* ARM_64 */
> > +
> > +#define __declare_arg_0(a0, res)                        \
> > +    struct arm_smccc_res    *___res = res;              \
> > +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> > +    register unsigned long  r1 asm("x1");               \
> > +    register unsigned long  r2 asm("x2");               \
> > +    register unsigned long  r3 asm("x3")
> > +
> > +#define __declare_arg_1(a0, a1, res)                    \
> > +    typeof(a1) __a1 = a1;                               \
> > +    struct arm_smccc_res    *___res = res;              \
> > +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> > +    register unsigned long  r1 asm("x1") = __a1;        \
> > +    register unsigned long  r2 asm("x2");               \
> > +    register unsigned long  r3 asm("x3")
> > +
> > +#define __declare_arg_2(a0, a1, a2, res)                \
> > +    typeof(a1) __a1 = a1;                               \
> > +    typeof(a2) __a2 = a2;                               \
> > +    struct arm_smccc_res    *___res = res;                              \
> > +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> > +    register unsigned long  r1 asm("x1") = __a1;        \
> > +    register unsigned long  r2 asm("x2") = __a2;        \
> > +    register unsigned long  r3 asm("x3")
> > +
> > +#define __declare_arg_3(a0, a1, a2, a3, res)            \
> > +    typeof(a1) __a1 = a1;                               \
> > +    typeof(a2) __a2 = a2;                               \
> > +    typeof(a3) __a3 = a3;                               \
> > +    struct arm_smccc_res    *___res = res;              \
> > +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> > +    register unsigned long  r1 asm("x1") = __a1;        \
> > +    register unsigned long  r2 asm("x2") = __a2;        \
> > +    register unsigned long  r3 asm("x3") = __a3
> > +
> > +#define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
> > +    typeof(a4) __a4 = a4;                               \
> > +    __declare_arg_3(a0, a1, a2, a3, res);               \
> > +    register unsigned long r4 asm("x4") = __a4
> > +
> > +#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
> > +    typeof(a5) __a5 = a5;                               \
> > +    __declare_arg_4(a0, a1, a2, a3, a4, res);           \
> > +    register typeof(a5) r5 asm("x5") = __a5
> > +
> > +#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
> > +    typeof(a6) __a6 = a6;                                   \
> > +    __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
> > +    register typeof(a6) r6 asm("x6") = __a6
> > +
> > +#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
> > +    typeof(a7) __a7 = a7;                                       \
> > +    __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
> > +    register typeof(a7) r7 asm("x7") = __a7
> > +
> > +#endif
> 
> I'm not an Arm maintainer, so my opinion may not mean much, but
> this is way too much code duplication for my taste. Isn't all you
> need an abstraction of the "r0" etc vs "x0" etc strings? Or even
> better, can't use to the "x0" etc form with the other compilers
> (seeing that these are their architectural names when taking the
> full with registers)?

Yes, please :-)

If there is no way to get the ARM C compiler to accept "r0" on aarch64
then a #define to abstract x0/r0 would be OK.

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

Re: [Xen-devel] [RFC 3/7] arm64:armds: ARM Compiler 6.6 does not accept `rx` registers naming for AArch64

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 4322 bytes --]

Hi,

Aside what Stefano and Jan already said. Please, reword the commit title.
It should reflect what the commit does not describe the error (this should
part of the message).

On Wed, 6 Nov 2019, 18:19 Andrii Anisov, <andrii.anisov@gmail.com> wrote:

> From: Andrii Anisov <andrii_anisov@epam.com>
>
> So get the code duplication with `x`-es.
>

Please provide a link to the documentation so this can be cross-checked.

Cheers,


> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> ---
>  xen/include/asm-arm/smccc.h | 60
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 126399d..3fa1144 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -120,6 +120,8 @@ struct arm_smccc_res {
>  #define __constraint_read_6 __constraint_read_5, "r" (r6)
>  #define __constraint_read_7 __constraint_read_6, "r" (r7)
>
> +#ifdef CONFIG_ARM_32
> +
>  #define __declare_arg_0(a0, res)                        \
>      struct arm_smccc_res    *___res = res;              \
>      register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> @@ -174,6 +176,64 @@ struct arm_smccc_res {
>      __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
>      register typeof(a7) r7 asm("r7") = __a7
>
> +#else /* ARM_64 */
> +
> +#define __declare_arg_0(a0, res)                        \
> +    struct arm_smccc_res    *___res = res;              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1");               \
> +    register unsigned long  r2 asm("x2");               \
> +    register unsigned long  r3 asm("x3")
> +
> +#define __declare_arg_1(a0, a1, res)                    \
> +    typeof(a1) __a1 = a1;                               \
> +    struct arm_smccc_res    *___res = res;              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1") = __a1;        \
> +    register unsigned long  r2 asm("x2");               \
> +    register unsigned long  r3 asm("x3")
> +
> +#define __declare_arg_2(a0, a1, a2, res)                \
> +    typeof(a1) __a1 = a1;                               \
> +    typeof(a2) __a2 = a2;                               \
> +    struct arm_smccc_res    *___res = res;                              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1") = __a1;        \
> +    register unsigned long  r2 asm("x2") = __a2;        \
> +    register unsigned long  r3 asm("x3")
> +
> +#define __declare_arg_3(a0, a1, a2, a3, res)            \
> +    typeof(a1) __a1 = a1;                               \
> +    typeof(a2) __a2 = a2;                               \
> +    typeof(a3) __a3 = a3;                               \
> +    struct arm_smccc_res    *___res = res;              \
> +    register unsigned long  r0 asm("x0") = (uint32_t)a0;\
> +    register unsigned long  r1 asm("x1") = __a1;        \
> +    register unsigned long  r2 asm("x2") = __a2;        \
> +    register unsigned long  r3 asm("x3") = __a3
> +
> +#define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
> +    typeof(a4) __a4 = a4;                               \
> +    __declare_arg_3(a0, a1, a2, a3, res);               \
> +    register unsigned long r4 asm("x4") = __a4
> +
> +#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
> +    typeof(a5) __a5 = a5;                               \
> +    __declare_arg_4(a0, a1, a2, a3, a4, res);           \
> +    register typeof(a5) r5 asm("x5") = __a5
> +
> +#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
> +    typeof(a6) __a6 = a6;                                   \
> +    __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
> +    register typeof(a6) r6 asm("x6") = __a6
> +
> +#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
> +    typeof(a7) __a7 = a7;                                       \
> +    __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
> +    register typeof(a7) r7 asm("x7") = __a7
> +
> +#endif
> +
>  #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
>  #define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
>
> --
> 2.7.4
>
>

[-- Attachment #1.2: Type: text/html, Size: 6107 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

[Xen-devel] [RFC 4/7] arm/gic: Drop pointless assertions

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Also armclang complains about the condition always true,
because `sgi` is of type enum with all its values under 16.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/arch/arm/gic.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 113655a..58c6141 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -294,8 +294,6 @@ void __init gic_init(void)
 
 void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
 {
-    ASSERT(sgi < 16); /* There are only 16 SGIs */
-
     gic_hw_ops->send_SGI(sgi, SGI_TARGET_LIST, cpumask);
 }
 
@@ -306,15 +304,11 @@ void send_SGI_one(unsigned int cpu, enum gic_sgi sgi)
 
 void send_SGI_self(enum gic_sgi sgi)
 {
-    ASSERT(sgi < 16); /* There are only 16 SGIs */
-
     gic_hw_ops->send_SGI(sgi, SGI_TARGET_SELF, NULL);
 }
 
 void send_SGI_allbutself(enum gic_sgi sgi)
 {
-   ASSERT(sgi < 16); /* There are only 16 SGIs */
-
    gic_hw_ops->send_SGI(sgi, SGI_TARGET_OTHERS, NULL);
 }
 
-- 
2.7.4


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

Re: [Xen-devel] [RFC 4/7] arm/gic: Drop pointless assertions

[Stefano Stabellini]

On Wed, 6 Nov 2019, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> Also armclang complains about the condition always true,
> because `sgi` is of type enum with all its values under 16.
> 
> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>

Although I am not completely opposed to this, given the choice I would
prefer to keep the ASSERTs.

Given that I would imagine that the ARM C Compiler will also complain
about many other ASSERTs, I wonder if it wouldn't be better to just
disable *all* ASSERTs when building with armcc by changing the
implementation of the ASSERT MACRO.


> ---
>  xen/arch/arm/gic.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 113655a..58c6141 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -294,8 +294,6 @@ void __init gic_init(void)
>  
>  void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
>  {
> -    ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
>      gic_hw_ops->send_SGI(sgi, SGI_TARGET_LIST, cpumask);
>  }
>  
> @@ -306,15 +304,11 @@ void send_SGI_one(unsigned int cpu, enum gic_sgi sgi)
>  
>  void send_SGI_self(enum gic_sgi sgi)
>  {
> -    ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
>      gic_hw_ops->send_SGI(sgi, SGI_TARGET_SELF, NULL);
>  }
>  
>  void send_SGI_allbutself(enum gic_sgi sgi)
>  {
> -   ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
>     gic_hw_ops->send_SGI(sgi, SGI_TARGET_OTHERS, NULL);
>  }
>  
> -- 
> 2.7.4
> 

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

Re: [Xen-devel] [RFC 4/7] arm/gic: Drop pointless assertions

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 2136 bytes --]

On Tue, 12 Nov 2019, 05:52 Stefano Stabellini, <sstabellini@kernel.org>
wrote:

> On Wed, 6 Nov 2019, Andrii Anisov wrote:
> > From: Andrii Anisov <andrii_anisov@epam.com>
> >
> > Also armclang complains about the condition always true,
> > because `sgi` is of type enum with all its values under 16.
> >
> > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
>
> Although I am not completely opposed to this, given the choice I would
> prefer to keep the ASSERTs.
>

Why? What would that prevent? It is an enum, so unless you do an horrible
hack on the other side, this should always be valid.

But then, why would this be an issue here and not in the tens other place
where enum is used?



> Given that I would imagine that the ARM C Compiler will also complain
> about many other ASSERTs, I wonder if it wouldn't be better to just
> disable *all* ASSERTs when building with armcc by changing the
> implementation of the ASSERT MACRO.


ARM C compiler is valid here and I would not be surprised this will come up
in Clang and GCC in the future.

If you are worry that the enum is going to grow more than 16 items, then
you should use a BUILD_BUG_ON.




>
> > ---
> >  xen/arch/arm/gic.c | 6 ------
> >  1 file changed, 6 deletions(-)
> >
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index 113655a..58c6141 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -294,8 +294,6 @@ void __init gic_init(void)
> >
> >  void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
> >  {
> > -    ASSERT(sgi < 16); /* There are only 16 SGIs */
> > -
> >      gic_hw_ops->send_SGI(sgi, SGI_TARGET_LIST, cpumask);
> >  }
> >
> > @@ -306,15 +304,11 @@ void send_SGI_one(unsigned int cpu, enum gic_sgi
> sgi)
> >
> >  void send_SGI_self(enum gic_sgi sgi)
> >  {
> > -    ASSERT(sgi < 16); /* There are only 16 SGIs */
> > -
> >      gic_hw_ops->send_SGI(sgi, SGI_TARGET_SELF, NULL);
> >  }
> >
> >  void send_SGI_allbutself(enum gic_sgi sgi)
> >  {
> > -   ASSERT(sgi < 16); /* There are only 16 SGIs */
> > -
> >     gic_hw_ops->send_SGI(sgi, SGI_TARGET_OTHERS, NULL);
> >  }
> >
> > --
> > 2.7.4
> >
>

[-- Attachment #1.2: Type: text/html, Size: 3596 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Xen-devel] [RFC 4/7] arm/gic: Drop pointless assertions

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2566 bytes --]

On Wed, 13 Nov 2019, Julien Grall wrote:
> On Tue, 12 Nov 2019, 05:52 Stefano Stabellini, <sstabellini@kernel.org> wrote:
>       On Wed, 6 Nov 2019, Andrii Anisov wrote:
>       > From: Andrii Anisov <andrii_anisov@epam.com>
>       >
>       > Also armclang complains about the condition always true,
>       > because `sgi` is of type enum with all its values under 16.
>       >
>       > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> 
>       Although I am not completely opposed to this, given the choice I would
>       prefer to keep the ASSERTs.
> 
> 
> Why? What would that prevent? It is an enum, so unless you do an horrible hack on the other side, this should always be valid.
> 
> But then, why would this be an issue here and not in the tens other place where enum is used?
> 
> 
> 
>       Given that I would imagine that the ARM C Compiler will also complain
>       about many other ASSERTs, I wonder if it wouldn't be better to just
>       disable *all* ASSERTs when building with armcc by changing the
>       implementation of the ASSERT MACRO.
> 
> 
> ARM C compiler is valid here and I would not be surprised this will come up in Clang and GCC in the future.
> 
> If you are worry that the enum is going to grow more than 16 items, then you should use a BUILD_BUG_ON.

That would be better actually

 
>       > ---
>       >  xen/arch/arm/gic.c | 6 ------
>       >  1 file changed, 6 deletions(-)
>       >
>       > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>       > index 113655a..58c6141 100644
>       > --- a/xen/arch/arm/gic.c
>       > +++ b/xen/arch/arm/gic.c
>       > @@ -294,8 +294,6 @@ void __init gic_init(void)
>       > 
>       >  void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
>       >  {
>       > -    ASSERT(sgi < 16); /* There are only 16 SGIs */
>       > -
>       >      gic_hw_ops->send_SGI(sgi, SGI_TARGET_LIST, cpumask);
>       >  }
>       > 
>       > @@ -306,15 +304,11 @@ void send_SGI_one(unsigned int cpu, enum gic_sgi sgi)
>       > 
>       >  void send_SGI_self(enum gic_sgi sgi)
>       >  {
>       > -    ASSERT(sgi < 16); /* There are only 16 SGIs */
>       > -
>       >      gic_hw_ops->send_SGI(sgi, SGI_TARGET_SELF, NULL);
>       >  }
>       > 
>       >  void send_SGI_allbutself(enum gic_sgi sgi)
>       >  {
>       > -   ASSERT(sgi < 16); /* There are only 16 SGIs */
>       > -
>       >     gic_hw_ops->send_SGI(sgi, SGI_TARGET_OTHERS, NULL);
>       >  }
>       > 
>       > --
>       > 2.7.4
>       >
> 
> 
> 

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

[Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Here several ARM Compiler 6.6 issues are solved or provided a work-around:

 - Scatter file is pretty primitive, it has no feature to define symbols
 - ARM linker defined symbols are not counted as referred if only mentioned
   in a steering file for rename or resolve, so a header file is used to
   redefine GNU linker script symbols into armlink defined symbols.

 - _srodata type clashes by type with __start_bug_frames so can not be both
   redefined as Load$$_rodata_bug_frames_0$$Base. Use resolve feature of armlink
   steering file.

 - C style shift operators are missed among supported scatter file expressions,
   so some needed values are hardcoded in scatter file.

 - Rename correspondent ARM Linker defined symbols to those needed by `symbols` tool
   using steering file feature.

 - ARM Compiler 6.6 tools are not able to rename sections, so we still need
   GNU toolchain's objcopy for this.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/Rules.mk                |   6 +
 xen/arch/arm/Makefile       |  24 ++++
 xen/arch/arm/xen.scat.S     | 266 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/xen.steer      |   5 +
 xen/include/asm-arm/armds.h |  91 +++++++++++++++
 5 files changed, 392 insertions(+)
 create mode 100644 xen/arch/arm/xen.scat.S
 create mode 100644 xen/arch/arm/xen.steer
 create mode 100644 xen/include/asm-arm/armds.h

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 41a1c26..67bedce 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -60,6 +60,12 @@ CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 $(call cc-option-add,CFLAGS,CC,-Wvla)
 CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
+
+ifeq ($(armds),y)
+CFLAGS += -nostdlibinc -nostdlib -Wno-unused-command-line-argument
+CFLAGS += -include $(BASEDIR)/include/asm/armds.h
+endif
+
 CFLAGS-$(CONFIG_DEBUG_INFO) += -g
 CFLAGS += '-D__OBJECT_FILE__="$@"'
 
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 70f532e..a5a3479 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -83,11 +83,16 @@ else
 all_symbols =
 endif
 
+ifeq ($(armds),y)
+$(TARGET): $(TARGET)-syms
+	fromelf --bin $< --output $@
+else
 $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
 ifeq ($(CONFIG_ARM_64),y)
 	ln -sf $(notdir $@)  ../../$(notdir $@).efi
 endif
+endif
 
 ifeq ($(CONFIG_LTO),y)
 # Gather all LTO objects together
@@ -102,6 +107,19 @@ prelink.o: $(ALL_OBJS)
 	$(LD) $(LDFLAGS) -r -o $@ $^
 endif
 
+ifeq ($(armds),y)
+$(TARGET)-syms: prelink.o xen.scat
+	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
+	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
+		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S
+	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
+	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
+	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
+		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S
+	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
+	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib --symdefs="$(@D)/$(@F).map" $(LDFLAGS) prelink.o $(build_id_linker) $(@D)/.$(@F).1.o -o $@
+	rm -f $(@D)/.$(@F).[0-9]*
+else
 $(TARGET)-syms: prelink.o xen.lds
 	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
@@ -119,14 +137,20 @@ $(TARGET)-syms: prelink.o xen.lds
 		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort \
 		>$(@D)/$(@F).map
 	rm -f $(@D)/.$(@F).[0-9]*
+endif
 
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
 	$(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $<
 
+ifeq ($(armds),y)
+xen.scat: xen.scat.S
+	$(CC) -P -E --target=aarch64-arm-none-eabi -o $@ $<
+else
 xen.lds: xen.lds.S
 	$(CC) -P -E -Ui386 $(AFLAGS) -o $@ $<
 	sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new
 	mv -f .xen.lds.d.new .xen.lds.d
+endif
 
 dtb.o: $(CONFIG_DTB_FILE)
 
diff --git a/xen/arch/arm/xen.scat.S b/xen/arch/arm/xen.scat.S
new file mode 100644
index 0000000..3bb405f
--- /dev/null
+++ b/xen/arch/arm/xen.scat.S
@@ -0,0 +1,266 @@
+#if 0 
+/*
+ * armlink does not understand shifts in scat file expressions
+ * so hardcode needed values
+ */
+#include <xen/cache.h>
+#include <asm/page.h>
+#include <asm/percpu.h>
+#undef ENTRY
+#undef ALIGN
+#else
+ #define PAGE_SIZE 4096
+ #define POINTER_ALIGN 8
+ #define SMP_CACHE_BYTES 128
+ #define STACK_SIZE 32768
+ #define XEN_VIRT_START 0x00200000
+#endif
+
+LOAD XEN_VIRT_START
+{
+;_start
+;_stext
+  _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090
+  {
+    *(.text*)
+    *(.text.cold)
+    *(.text.unlikely)
+    *(.fixup)
+    *(.gnu.warning)
+  }
+;_etext
+
+;_srodata
+;__start_bug_frames
+  _rodata_bug_frames_0 AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
+  {
+    *(.bug_frames.0)
+  }
+;__stop_bug_frames_0
+
+  _rodata_bug_frames_1 +0 FIXED ZEROPAD
+  {
+    *(.bug_frames.1)
+  }
+;__stop_bug_frames_1
+
+  _rodata_bug_frames_2 +0 FIXED ZEROPAD
+  {
+    *(.bug_frames.2)
+  }
+;__stop_bug_frames_2
+
+  _rodata_data +0
+  {
+    *(.rodata)
+    *(.rodata.*)
+    *(.data.rel.ro)
+    *(.data.rel.ro.*)
+  }
+
+#ifdef CONFIG_LOCK_PROFILE
+;__lock_profile_start
+  _rodata_lockprofile_data AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
+  {
+    *(.lockprofile.data)
+  }
+;__lock_profile_end
+#endif
+
+;__param_start
+  _rodata_data_param AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
+  {
+    *(.data.param)
+  }
+;__param_end
+
+;__proc_info_start
+  _rodata_proc_info +0 FIXED ZEROPAD
+  {
+    *(.proc.info)
+  }
+;__proc_info_end
+
+#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM)
+;__start_vpci_array
+  _rodata_data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD
+  {
+    *(SORT(.data.vpci.*))
+  }
+;__end_vpci_array
+#endif
+
+#if defined(BUILD_ID)
+;__note_gnu_build_id_start
+  _note_gnu_build_id +0 FIXED ZEROPAD
+  {
+    *(.note.gnu.build-id)
+  }
+;__note_gnu_build_id_end
+#endif
+
+;_erodata
+
+  _data AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
+  {
+    *(.data.page_aligned.*)
+    *(.data.*)
+  }
+
+;__start_schedulers_array
+  _data_schedulers AlignExpr(+0, 8) FIXED ZEROPAD
+  {
+    *(.data.schedulers)
+  }
+;__end_schedulers_array
+
+  _data_rel +0 FIXED ZEROPAD
+  {
+    *(.data.rel)
+    *(.data.rel.*)
+;#CONSTRUCTORS ????
+  }
+
+;__start___ex_table  
+  _data_ex_table AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD
+  {
+    *(.ex_table)
+  }
+;__stop___ex_table
+
+;__start___pre_ex_table
+  _data_ex_table_pre +0 FIXED ZEROPAD
+  {
+    *(.ex_table.pre)
+  }
+;__stop___pre_ex_table
+
+  _data_read_mostly +0 FIXED ZEROPAD
+  {
+    *(.data.read_mostly)
+  }
+
+;_splatform
+  _arch_info AlignExpr(+0, 8) FIXED ZEROPAD
+  {
+     *(.arch.info)
+  }
+;_eplatform
+
+;_sdevice
+  _dev_info AlignExpr(+0, 8) FIXED ZEROPAD
+  {
+    *(.dev.info)
+  }
+;_edevice
+
+;_asdevice
+  _adev_info AlignExpr(+0, 8) FIXED ZEROPAD
+  {
+    *(.adev.info)
+  }
+;_aedevice
+
+;__init_begin
+;_sinittext
+  _init_text AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
+  {
+    *(.init.text)
+  }
+;_einittext
+
+  _init_rodata AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
+  {
+    *(.init.rodata)
+    *(.init.rodata.rel)
+    *(.init.rodata.str*)
+  }
+
+;__setup_start
+  _init_setup AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
+  {
+    *(.init.setup)
+  }
+;__setup_end
+
+;__initcall_start
+  _initcallpresmp_init +0 FIXED ZEROPAD
+  {
+    *(.initcallpresmp.init)
+  }
+;__presmp_initcall_end
+
+  _initcall1_init +0 FIXED ZEROPAD
+  {
+    *(.initcall1.init)
+  }
+;__initcall_end
+
+;__alt_instructions
+  _altinstructions AlignExpr(+0, 4) FIXED ZEROPAD
+  {
+    *(.altinstructions)
+  }
+;__alt_instructions_end
+
+  _altinstr_replacement AlignExpr(+0, 4) FIXED ZEROPAD
+  {
+    *(.altinstr_replacement)
+  }
+
+  _init_data +0 FIXED ZEROPAD
+  {
+    *(.init.data)
+    *(.init.data.rel)
+    *(.init.data.rel.*)
+  }
+
+;__ctors_start
+  _ctors AlignExpr(+0, 8) FIXED ZEROPAD
+  {
+    *(.ctors)
+    *(.init_array)
+  }
+
+  _init_array_sorted AlignExpr(+0, 8) SORTTYPE Lexical FIXED ZEROPAD
+  {
+    *(.init_array.*)
+  }
+;__ctors_end
+
+#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM)
+  _data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD
+  {
+    *(.data.vpci.*)
+  }
+#endif
+;__init_end_efi
+
+;__init_end
+;__bss_start
+  _bss AlignExpr(+0, STACK_SIZE) FIXED ZEROPAD
+  {
+    *(.bss.stack_aligned*)
+    *(.bss.page_aligned*, OVERALIGN PAGE_SIZE)
+    *(.bss*)
+  }
+
+;__per_cpu_start
+  _bss_percpu AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD
+  {
+    *(.bss.percpu)
+    *(.bss.percpu.read_mostly, OVERALIGN SMP_CACHE_BYTES)
+  }
+;__per_cpu_data_end
+;__bss_end
+;_end
+
+#ifdef CONFIG_DTB_FILE
+;_sdtb
+  _dtb FIXED ZEROPAD
+ {
+    *(.dtb)
+ }
+#endif
+
+}
diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer
new file mode 100644
index 0000000..646e912
--- /dev/null
+++ b/xen/arch/arm/xen.steer
@@ -0,0 +1,5 @@
+RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base
+RENAME Load$$_text$$Base AS _stext
+RENAME Load$$_text$$Limit AS _etext
+RENAME Load$$_init_text$$Base AS _sinittext
+RENAME Load$$_init_text$$Limit AS _einittext
diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h
new file mode 100644
index 0000000..5ee2e5d
--- /dev/null
+++ b/xen/include/asm-arm/armds.h
@@ -0,0 +1,91 @@
+#define _start                      Load$$_text$$Base
+#define _stext                      Load$$_text$$Base
+
+#define _etext                      Load$$_text$$Limit
+
+//#define _srodata                    Load$$_rodata_bug_frames_0$$Base
+#define __start_bug_frames          Load$$_rodata_bug_frames_0$$Base
+
+#define __stop_bug_frames_0         Load$$_rodata_bug_frames_0$$Limit
+#define __stop_bug_frames_1         Load$$_rodata_bug_frames_1$$Limit
+#define __stop_bug_frames_2         Load$$_rodata_bug_frames_2$$Limit
+
+#ifdef CONFIG_LOCK_PROFILE
+#define __lock_profile_start        Load$$_rodata_lockprofile_data$$Base
+#define __lock_profile_end          Load$$_rodata_lockprofile_data$$Limit
+#endif
+
+#define __param_start               Load$$_rodata_data_param$$Base
+#define __param_end                 Load$$_rodata_data_param$$Limit
+
+#define __proc_info_start           Load$$_rodata_proc_info$$Base
+#define __proc_info_end             Load$$_rodata_proc_info$$Limit
+
+#define _erodata                    Load$$_rodata_proc_info$$Limit
+
+#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM)
+#define __start_vpci_array          Load$$_rodata_data_vpci$$Base
+#define __end_vpci_array            Load$$_rodata_data_vpci$$Limit
+
+#undef _erodata
+#define _erodata                    Load$$_rodata_data_vpci$$Limit
+#endif
+
+#if defined(BUILD_ID)
+#define __note_gnu_build_id_start   Load$$_note_gnu_build_id$$Base
+#define __note_gnu_build_id_end     Load$$_note_gnu_build_id$$Limit
+
+#undef _erodata
+#define _erodata                    Load$$_note_gnu_build_id$$Limit
+#endif
+
+#define __start_schedulers_array    Load$$_data_schedulers$$Base
+#define __end_schedulers_array      Load$$_data_schedulers$$Limit
+
+/* Does not exist for ARM
+#define __start___ex_table          Load$$_data_ex_table$$Base
+#define __stop___ex_table           Load$$_data_ex_table$$Limit
+*/
+
+#define __start___pre_ex_table      Load$$_data_ex_table_pre$$Base
+#define __stop___pre_ex_table       Load$$_data_ex_table_pre$$Limit
+
+#define _splatform                  Load$$_arch_info$$Base
+#define _eplatform                  Load$$_arch_info$$Limit
+
+#define _sdevice                    Load$$_dev_info$$Base
+#define _edevice                    Load$$_dev_info$$Limit
+
+#define _asdevice                   Load$$_adev_info$$Base
+#define _aedevice                   Load$$_adev_info$$Limit
+
+#define __init_begin                Load$$_init_text$$Base
+#define _sinittext                  Load$$_init_text$$Base
+#define _einittext                  Load$$_init_text$$Limit
+
+#define __setup_start               Load$$_init_setup$$Base
+#define __setup_end                 Load$$_init_setup$$Limit
+
+#define __initcall_start            Load$$_initcallpresmp_init$$Base
+#define __presmp_initcall_end       Load$$_initcallpresmp_init$$Limit
+#define __initcall_end              Load$$_initcall1_init$$Limit
+
+#define __alt_instructions          Load$$_altinstructions$$Base
+#define __alt_instructions_end      Load$$_altinstructions$$Limit
+
+#define __ctors_start               Load$$_ctors$$Base
+#define __ctors_end                 Load$$_init_array_sorted$$Limit
+#define __init_end_efi              Load$$_init_array_sorted$$Limit
+
+#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM)
+#undef __init_end_efi
+#define __init_end_efi              Load$$_data_vpci$$Limit
+#endif
+
+#define __init_end                  Load$$_bss$$Base
+#define __bss_start                 Load$$_bss$$Base
+
+#define __per_cpu_start             Load$$_bss_percpu$$Base
+#define __per_cpu_data_end          Load$$_bss_percpu$$Limit
+#define __bss_end                   Load$$_bss_percpu$$Limit
+#define _end                        Load$$_bss_percpu$$Limit
-- 
2.7.4


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

Re: [Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Stefano Stabellini]

On Wed, 6 Nov 2019, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> Here several ARM Compiler 6.6 issues are solved or provided a work-around:
> 
>  - Scatter file is pretty primitive, it has no feature to define symbols
>  - ARM linker defined symbols are not counted as referred if only mentioned
>    in a steering file for rename or resolve, so a header file is used to
>    redefine GNU linker script symbols into armlink defined symbols.
> 
>  - _srodata type clashes by type with __start_bug_frames so can not be both
>    redefined as Load$$_rodata_bug_frames_0$$Base. Use resolve feature of armlink
>    steering file.

Why _srodata and __start_bug_frames cannot both be defined as
Load$$_rodata_bug_frames_0$$Base when actually in the case of:

> +#define __per_cpu_data_end          Load$$_bss_percpu$$Limit
> +#define __bss_end                   Load$$_bss_percpu$$Limit
> +#define _end                        Load$$_bss_percpu$$Limit

They are all defined as "Load$$_bss_percpu$$Limit"? And:

> +#define __init_end                  Load$$_bss$$Base
> +#define __bss_start                 Load$$_bss$$Base

They are both defined as "Load$$_bss$$Base"? What's special about
"Load$$_rodata_bug_frames_0$$Base"?


>  - C style shift operators are missed among supported scatter file expressions,
>    so some needed values are hardcoded in scatter file.
> 
>  - Rename correspondent ARM Linker defined symbols to those needed by `symbols` tool
>    using steering file feature.
> 
>  - ARM Compiler 6.6 tools are not able to rename sections, so we still need
>    GNU toolchain's objcopy for this.
> 
> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> ---
>  xen/Rules.mk                |   6 +
>  xen/arch/arm/Makefile       |  24 ++++
>  xen/arch/arm/xen.scat.S     | 266 ++++++++++++++++++++++++++++++++++++++++++++

I would strongly suggest to rename this file with something not
potentially related to scat


>  xen/arch/arm/xen.steer      |   5 +
>  xen/include/asm-arm/armds.h |  91 +++++++++++++++
>  5 files changed, 392 insertions(+)
>  create mode 100644 xen/arch/arm/xen.scat.S
>  create mode 100644 xen/arch/arm/xen.steer
>  create mode 100644 xen/include/asm-arm/armds.h
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 41a1c26..67bedce 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -60,6 +60,12 @@ CFLAGS += -nostdinc -fno-builtin -fno-common
>  CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
>  $(call cc-option-add,CFLAGS,CC,-Wvla)
>  CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
> +
> +ifeq ($(armds),y)
> +CFLAGS += -nostdlibinc -nostdlib -Wno-unused-command-line-argument
> +CFLAGS += -include $(BASEDIR)/include/asm/armds.h
> +endif
> +
>  CFLAGS-$(CONFIG_DEBUG_INFO) += -g
>  CFLAGS += '-D__OBJECT_FILE__="$@"'
>  
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 70f532e..a5a3479 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -83,11 +83,16 @@ else
>  all_symbols =
>  endif
>  
> +ifeq ($(armds),y)
> +$(TARGET): $(TARGET)-syms
> +	fromelf --bin $< --output $@
> +else
>  $(TARGET): $(TARGET)-syms
>  	$(OBJCOPY) -O binary -S $< $@
>  ifeq ($(CONFIG_ARM_64),y)
>  	ln -sf $(notdir $@)  ../../$(notdir $@).efi
>  endif
> +endif
>  
>  ifeq ($(CONFIG_LTO),y)
>  # Gather all LTO objects together
> @@ -102,6 +107,19 @@ prelink.o: $(ALL_OBJS)
>  	$(LD) $(LDFLAGS) -r -o $@ $^
>  endif
>  
> +ifeq ($(armds),y)
> +$(TARGET)-syms: prelink.o xen.scat
> +	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> +	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
> +		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S
> +	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
> +	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
> +	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
> +		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S
> +	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
> +	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib --symdefs="$(@D)/$(@F).map" $(LDFLAGS) prelink.o $(build_id_linker) $(@D)/.$(@F).1.o -o $@
> +	rm -f $(@D)/.$(@F).[0-9]*
> +else
>  $(TARGET)-syms: prelink.o xen.lds
>  	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
>  	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> @@ -119,14 +137,20 @@ $(TARGET)-syms: prelink.o xen.lds
>  		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort \
>  		>$(@D)/$(@F).map
>  	rm -f $(@D)/.$(@F).[0-9]*
> +endif
>  
>  asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
>  	$(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $<
>  
> +ifeq ($(armds),y)
> +xen.scat: xen.scat.S
> +	$(CC) -P -E --target=aarch64-arm-none-eabi -o $@ $<
> +else
>  xen.lds: xen.lds.S
>  	$(CC) -P -E -Ui386 $(AFLAGS) -o $@ $<
>  	sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new
>  	mv -f .xen.lds.d.new .xen.lds.d
> +endif
>  
>  dtb.o: $(CONFIG_DTB_FILE)
>  
> diff --git a/xen/arch/arm/xen.scat.S b/xen/arch/arm/xen.scat.S
> new file mode 100644
> index 0000000..3bb405f
> --- /dev/null
> +++ b/xen/arch/arm/xen.scat.S
> @@ -0,0 +1,266 @@
> +#if 0 

#if 0 must be a mistake?

Also, is this basically the ARMCC version of a linker script? Is
xen.lds.S still used with ARMCC, or only xen.scat.S is used?


> +/*
> + * armlink does not understand shifts in scat file expressions
> + * so hardcode needed values
> + */
> +#include <xen/cache.h>
> +#include <asm/page.h>
> +#include <asm/percpu.h>
> +#undef ENTRY
> +#undef ALIGN
> +#else
> + #define PAGE_SIZE 4096
> + #define POINTER_ALIGN 8
> + #define SMP_CACHE_BYTES 128
> + #define STACK_SIZE 32768
> + #define XEN_VIRT_START 0x00200000
> +#endif
> +
> +LOAD XEN_VIRT_START
> +{
> +;_start
> +;_stext
> +  _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090
> +  {
> +    *(.text*)
> +    *(.text.cold)
> +    *(.text.unlikely)
> +    *(.fixup)
> +    *(.gnu.warning)
> +  }
> +;_etext
> +
> +;_srodata
> +;__start_bug_frames
> +  _rodata_bug_frames_0 AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
> +  {
> +    *(.bug_frames.0)
> +  }
> +;__stop_bug_frames_0
> +
> +  _rodata_bug_frames_1 +0 FIXED ZEROPAD
> +  {
> +    *(.bug_frames.1)
> +  }
> +;__stop_bug_frames_1
> +
> +  _rodata_bug_frames_2 +0 FIXED ZEROPAD
> +  {
> +    *(.bug_frames.2)
> +  }
> +;__stop_bug_frames_2
> +
> +  _rodata_data +0
> +  {
> +    *(.rodata)
> +    *(.rodata.*)
> +    *(.data.rel.ro)
> +    *(.data.rel.ro.*)
> +  }
> +
> +#ifdef CONFIG_LOCK_PROFILE
> +;__lock_profile_start
> +  _rodata_lockprofile_data AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
> +  {
> +    *(.lockprofile.data)
> +  }
> +;__lock_profile_end
> +#endif

Should be below


> +;__param_start
> +  _rodata_data_param AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
> +  {
> +    *(.data.param)
> +  }
> +;__param_end
> +
> +;__proc_info_start
> +  _rodata_proc_info +0 FIXED ZEROPAD
> +  {
> +    *(.proc.info)
> +  }
> +;__proc_info_end
> +
> +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM)
> +;__start_vpci_array
> +  _rodata_data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD
> +  {
> +    *(SORT(.data.vpci.*))
> +  }
> +;__end_vpci_array
> +#endif
> +
> +#if defined(BUILD_ID)
> +;__note_gnu_build_id_start
> +  _note_gnu_build_id +0 FIXED ZEROPAD
> +  {
> +    *(.note.gnu.build-id)
> +  }
> +;__note_gnu_build_id_end
> +#endif
> +
> +;_erodata
> +
> +  _data AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
> +  {
> +    *(.data.page_aligned.*)
> +    *(.data.*)
> +  }
> +
> +;__start_schedulers_array
> +  _data_schedulers AlignExpr(+0, 8) FIXED ZEROPAD
> +  {
> +    *(.data.schedulers)
> +  }
> +;__end_schedulers_array
> +
> +  _data_rel +0 FIXED ZEROPAD
> +  {
> +    *(.data.rel)
> +    *(.data.rel.*)
> +;#CONSTRUCTORS ????

Honestly I am not sure what this is either


> +  }
> +
> +;__start___ex_table  
> +  _data_ex_table AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD
> +  {
> +    *(.ex_table)
> +  }
> +;__stop___ex_table
> +
> +;__start___pre_ex_table
> +  _data_ex_table_pre +0 FIXED ZEROPAD
> +  {
> +    *(.ex_table.pre)
> +  }
> +;__stop___pre_ex_table
> +
> +  _data_read_mostly +0 FIXED ZEROPAD
> +  {
> +    *(.data.read_mostly)
> +  }
> +
> +;_splatform
> +  _arch_info AlignExpr(+0, 8) FIXED ZEROPAD
> +  {
> +     *(.arch.info)
> +  }
> +;_eplatform
> +
> +;_sdevice
> +  _dev_info AlignExpr(+0, 8) FIXED ZEROPAD
> +  {
> +    *(.dev.info)
> +  }
> +;_edevice
> +
> +;_asdevice
> +  _adev_info AlignExpr(+0, 8) FIXED ZEROPAD
> +  {
> +    *(.adev.info)
> +  }
> +;_aedevice

_steemediator/_eteemediator


> +;__init_begin
> +;_sinittext
> +  _init_text AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
> +  {
> +    *(.init.text)
> +  }
> +;_einittext
> +
> +  _init_rodata AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
> +  {
> +    *(.init.rodata)
> +    *(.init.rodata.rel)
> +    *(.init.rodata.str*)
> +  }
> +
> +;__setup_start
> +  _init_setup AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
> +  {
> +    *(.init.setup)
> +  }
> +;__setup_end
> +
> +;__initcall_start
> +  _initcallpresmp_init +0 FIXED ZEROPAD
> +  {
> +    *(.initcallpresmp.init)
> +  }
> +;__presmp_initcall_end
> +
> +  _initcall1_init +0 FIXED ZEROPAD
> +  {
> +    *(.initcall1.init)
> +  }
> +;__initcall_end
> +
> +;__alt_instructions
> +  _altinstructions AlignExpr(+0, 4) FIXED ZEROPAD
> +  {
> +    *(.altinstructions)
> +  }
> +;__alt_instructions_end
> +
> +  _altinstr_replacement AlignExpr(+0, 4) FIXED ZEROPAD
> +  {
> +    *(.altinstr_replacement)
> +  }

__lock_profile_start should be here


> +
> +  _init_data +0 FIXED ZEROPAD
> +  {
> +    *(.init.data)
> +    *(.init.data.rel)
> +    *(.init.data.rel.*)
> +  }
> +
> +;__ctors_start
> +  _ctors AlignExpr(+0, 8) FIXED ZEROPAD
> +  {
> +    *(.ctors)
> +    *(.init_array)
> +  }
> +
> +  _init_array_sorted AlignExpr(+0, 8) SORTTYPE Lexical FIXED ZEROPAD
> +  {
> +    *(.init_array.*)
> +  }
> +;__ctors_end
> +
> +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM)
> +  _data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD
> +  {
> +    *(.data.vpci.*)
> +  }
> +#endif

__start_vpci_array/__end_vpci_array?


> +;__init_end_efi
> +
> +;__init_end
> +;__bss_start
> +  _bss AlignExpr(+0, STACK_SIZE) FIXED ZEROPAD
> +  {
> +    *(.bss.stack_aligned*)
> +    *(.bss.page_aligned*, OVERALIGN PAGE_SIZE)
> +    *(.bss*)
> +  }
> +
> +;__per_cpu_start

this should be page aligned too?

> +  _bss_percpu AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD
> +  {
> +    *(.bss.percpu)
> +    *(.bss.percpu.read_mostly, OVERALIGN SMP_CACHE_BYTES)
> +  }
> +;__per_cpu_data_end
> +;__bss_end

__bss_end should be page aligned?

> +;_end
> +
> +#ifdef CONFIG_DTB_FILE
> +;_sdtb
> +  _dtb FIXED ZEROPAD
> + {
> +    *(.dtb)
> + }
> +#endif
> +
> +}
> diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer
> new file mode 100644
> index 0000000..646e912
> --- /dev/null
> +++ b/xen/arch/arm/xen.steer
> @@ -0,0 +1,5 @@
> +RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base
> +RENAME Load$$_text$$Base AS _stext
> +RENAME Load$$_text$$Limit AS _etext
> +RENAME Load$$_init_text$$Base AS _sinittext
> +RENAME Load$$_init_text$$Limit AS _einittext

I don't get why some if the "symbols" get renamed using RENAME here, and
some other we are using a #define in xen/include/asm-arm/armds.h. Can't
we rename them all here?


> diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h
> new file mode 100644
> index 0000000..5ee2e5d
> --- /dev/null
> +++ b/xen/include/asm-arm/armds.h

Missing guards. Also, probably you want to make sure this is only #ifdef
ARMCC.

Is this meant to be used when building C files, asm files, or both?

I would avoid this header file if we can get away with just xen.steer.


> @@ -0,0 +1,91 @@
> +#define _start                      Load$$_text$$Base
> +#define _stext                      Load$$_text$$Base
> +
> +#define _etext                      Load$$_text$$Limit
> +
> +//#define _srodata                    Load$$_rodata_bug_frames_0$$Base
> +#define __start_bug_frames          Load$$_rodata_bug_frames_0$$Base
> +
> +#define __stop_bug_frames_0         Load$$_rodata_bug_frames_0$$Limit
> +#define __stop_bug_frames_1         Load$$_rodata_bug_frames_1$$Limit
> +#define __stop_bug_frames_2         Load$$_rodata_bug_frames_2$$Limit
> +
> +#ifdef CONFIG_LOCK_PROFILE
> +#define __lock_profile_start        Load$$_rodata_lockprofile_data$$Base
> +#define __lock_profile_end          Load$$_rodata_lockprofile_data$$Limit
> +#endif
> +
> +#define __param_start               Load$$_rodata_data_param$$Base
> +#define __param_end                 Load$$_rodata_data_param$$Limit
> +
> +#define __proc_info_start           Load$$_rodata_proc_info$$Base
> +#define __proc_info_end             Load$$_rodata_proc_info$$Limit
> +
> +#define _erodata                    Load$$_rodata_proc_info$$Limit
> +
> +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM)
> +#define __start_vpci_array          Load$$_rodata_data_vpci$$Base
> +#define __end_vpci_array            Load$$_rodata_data_vpci$$Limit
> +
> +#undef _erodata
> +#define _erodata                    Load$$_rodata_data_vpci$$Limit
> +#endif
> +
> +#if defined(BUILD_ID)
> +#define __note_gnu_build_id_start   Load$$_note_gnu_build_id$$Base
> +#define __note_gnu_build_id_end     Load$$_note_gnu_build_id$$Limit
> +
> +#undef _erodata
> +#define _erodata                    Load$$_note_gnu_build_id$$Limit
> +#endif
> +
> +#define __start_schedulers_array    Load$$_data_schedulers$$Base
> +#define __end_schedulers_array      Load$$_data_schedulers$$Limit
> +
> +/* Does not exist for ARM
> +#define __start___ex_table          Load$$_data_ex_table$$Base
> +#define __stop___ex_table           Load$$_data_ex_table$$Limit
> +*/
> +
> +#define __start___pre_ex_table      Load$$_data_ex_table_pre$$Base
> +#define __stop___pre_ex_table       Load$$_data_ex_table_pre$$Limit
> +
> +#define _splatform                  Load$$_arch_info$$Base
> +#define _eplatform                  Load$$_arch_info$$Limit
> +
> +#define _sdevice                    Load$$_dev_info$$Base
> +#define _edevice                    Load$$_dev_info$$Limit
> +
> +#define _asdevice                   Load$$_adev_info$$Base
> +#define _aedevice                   Load$$_adev_info$$Limit
> +
> +#define __init_begin                Load$$_init_text$$Base
> +#define _sinittext                  Load$$_init_text$$Base
> +#define _einittext                  Load$$_init_text$$Limit
> +
> +#define __setup_start               Load$$_init_setup$$Base
> +#define __setup_end                 Load$$_init_setup$$Limit
> +
> +#define __initcall_start            Load$$_initcallpresmp_init$$Base
> +#define __presmp_initcall_end       Load$$_initcallpresmp_init$$Limit
> +#define __initcall_end              Load$$_initcall1_init$$Limit
> +
> +#define __alt_instructions          Load$$_altinstructions$$Base
> +#define __alt_instructions_end      Load$$_altinstructions$$Limit
> +
> +#define __ctors_start               Load$$_ctors$$Base
> +#define __ctors_end                 Load$$_init_array_sorted$$Limit
> +#define __init_end_efi              Load$$_init_array_sorted$$Limit
> +
> +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM)
> +#undef __init_end_efi
> +#define __init_end_efi              Load$$_data_vpci$$Limit
> +#endif
> +
> +#define __init_end                  Load$$_bss$$Base
> +#define __bss_start                 Load$$_bss$$Base
> +
> +#define __per_cpu_start             Load$$_bss_percpu$$Base
> +#define __per_cpu_data_end          Load$$_bss_percpu$$Limit
> +#define __bss_end                   Load$$_bss_percpu$$Limit
> +#define _end                        Load$$_bss_percpu$$Limit

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

Re: [Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 3086 bytes --]

On Tue, 12 Nov 2019, 06:27 Stefano Stabellini, <sstabellini@kernel.org>
wrote:

> On Wed, 6 Nov 2019, Andrii Anisov wrote:
> > From: Andrii Anisov <andrii_anisov@epam.com>
> >
> > Here several ARM Compiler 6.6 issues are solved or provided a
> work-around:
> >
> >  - Scatter file is pretty primitive, it has no feature to define symbols
> >  - ARM linker defined symbols are not counted as referred if only
> mentioned
> >    in a steering file for rename or resolve, so a header file is used to
> >    redefine GNU linker script symbols into armlink defined symbols.
> >
> >  - _srodata type clashes by type with __start_bug_frames so can not be
> both
> >    redefined as Load$$_rodata_bug_frames_0$$Base. Use resolve feature of
> armlink
> >    steering file.
>
> Why _srodata and __start_bug_frames cannot both be defined as
> Load$$_rodata_bug_frames_0$$Base when actually in the case of:
>
> > +#define __per_cpu_data_end          Load$$_bss_percpu$$Limit
> > +#define __bss_end                   Load$$_bss_percpu$$Limit
> > +#define _end                        Load$$_bss_percpu$$Limit
>
> They are all defined as "Load$$_bss_percpu$$Limit"? And:
>
> > +#define __init_end                  Load$$_bss$$Base
> > +#define __bss_start                 Load$$_bss$$Base
>
> They are both defined as "Load$$_bss$$Base"? What's special about
> "Load$$_rodata_bug_frames_0$$Base"?
>
>
> >  - C style shift operators are missed among supported scatter file
> expressions,
> >    so some needed values are hardcoded in scatter file.
> >
> >  - Rename correspondent ARM Linker defined symbols to those needed by
> `symbols` tool
> >    using steering file feature.
> >
> >  - ARM Compiler 6.6 tools are not able to rename sections, so we still
> need
> >    GNU toolchain's objcopy for this.
> >
> > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> > ---
> >  xen/Rules.mk                |   6 +
> >  xen/arch/arm/Makefile       |  24 ++++
> >  xen/arch/arm/xen.scat.S     | 266
> ++++++++++++++++++++++++++++++++++++++++++++
>
> I would strongly suggest to rename this file with something not
> potentially related to scat
>

To be honest, I don't think this file should even exist. This looks like a
copy of xen.lds.S with a different syntax. Furthermore, the comments from
Stefano shows that is going to be hard to maintain/check everything has
been written correctly.

So how about trying to abstract xen.lds.S?


>
> > +/*
> > + * armlink does not understand shifts in scat file expressions
> > + * so hardcode needed values
> > + */
>

Please give a pointer to the doc of the armlink in the commit message. So
we can easily cross-check what's happening.

In this case, I don't particularly like the re-definition of the defines
outside of their header. This is going to make more difficult if we have to
update them in the future.

I can see a few ways to do it:

 - Avoid using shifts in the definitions
 - Find a way to evaluate the value (maybe similar to asn-offset) before
using them.

My preference would be the latter but I could be convinced for the former.

Cheers,

[-- Attachment #1.2: Type: text/html, Size: 4512 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Andrii Anisov]

On 13.11.19 07:50, Julien Grall wrote:
> To be honest, I don't think this file should even exist. This looks like a copy of xen.lds.S with a different syntax.

And lacking features like symbols definition, current address setup, etc.

> Furthermore, the comments from Stefano shows that is going to be hard to maintain/check everything has been written correctly.

It will be terribly hard.

> So how about trying to abstract xen.lds.S?

I failed to find the common ground for them.
You are very welcomed to suggest that piece of code.


>      > +/*
>      > + * armlink does not understand shifts in scat file expressions
>      > + * so hardcode needed values
>      > + */
> 
> 
> Please give a pointer to the doc of the armlink in the commit message. So we can easily cross-check what's happening.

The best cross-check would be running the compiler. Yet, this particular thing is somehow documented [1].

> In this case, I don't particularly like the re-definition of the defines outside of their header. This is going to make more difficult if we have to update them in the future.
> 
> I can see a few ways to do it:
> 
>   - Avoid using shifts in the definitions
>   - Find a way to evaluate the value (maybe similar to asn-offset) before using them.
>
> My preference would be the latter but I could be convinced for the former.

The first option is not realistic. I suggested ARM to consider shifts support as an improvement for their compiler.
I'd be very happy to adopt the second option. Do you have any code examples or hints how to evaluate expressions on the pre-processing stage?

[1] https://developer.arm.com/docs/100070/0612/scatter-file-syntax/expression-evaluation-in-scatter-files/expression-rules-in-scatter-files

-- 
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Julien Grall]

Hi Andrii,

On 14/11/2019 11:31, Andrii Anisov wrote:
> 
> 
> On 13.11.19 07:50, Julien Grall wrote:
>> To be honest, I don't think this file should even exist. This looks 
>> like a copy of xen.lds.S with a different syntax.
> 
> And lacking features like symbols definition, current address setup, etc.

That's fine, you can introduce macro that will just be a NOP for 
armlinker and implemented for ld.

> 
>> Furthermore, the comments from Stefano shows that is going to be hard 
>> to maintain/check everything has been written correctly.
> 
> It will be terribly hard.
> 
>> So how about trying to abstract xen.lds.S?
> 
> I failed to find the common ground for them.
> You are very welcomed to suggest that piece of code.

I don't have time to work on the full feature, but I can provide a few 
ideas so you can implement it.

If we look at the .text section. For the GNU linker script, it is:

   _start = .;
   .text : {
         _stext = .;            /* Text section */
        *(.text)
        *(.text.cold)
        *(.text.unlikely)
        *(.fixup)
        *(.gnu.warning)
        _etext = .;             /* End of text section */
   } :text = 0x9090

For armds, you implement as

;_start
;_stext
   _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090
   {
     *(.text*)
     *(.text.cold)
     *(.text.unlikely)
     *(.fixup)
     *(.gnu.warning)
   }
;_etext

You could imagine the following abstraction:

SYMBOL(_start)
SECTION(_text)
   SYMBOL(_stext)
   *(.text)
   *(.text.cold)
   *(.text.unlikely)
   *(.fixup)
   *(.gnu.warning)
   SYMBOL(_etext)
ESECTION(text)

For GNU linker scripts, the macros would be implemented as

#define SYMBOL(sym) sym := .;
#define SECTION(sect) sect : {
#define ESECTION(phdr) } :phdr = 0x9090


For the Arm scatter file, the macros would be implemented as

/* Symbols are not declared in the scatter file */
#define SYMBOL(sym) ;sym
#define SECTION(sect) sect AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090 {
#define ESECTION(phdr) }

A few caveats:
    - I am not entirely sure why we specific the pading value only for 
.text and not the other. I also don't understand the 0x9090 value. 
Stefano? It may be possible to remove this completely.
    - The alignment could be passed as a parameter for the macro
    - The linker script may need some reshuffle in order to make it generic.

On a side note, I noticed that you are using *(.text*), rather than 
*(.text). Could you explain why?

> 
> 
>>      > +/*
>>      > + * armlink does not understand shifts in scat file expressions
>>      > + * so hardcode needed values
>>      > + */
>>
>>
>> Please give a pointer to the doc of the armlink in the commit message. 
>> So we can easily cross-check what's happening.
> 
> The best cross-check would be running the compiler. Yet, this particular 
> thing is somehow documented [1].
> 
>> In this case, I don't particularly like the re-definition of the 
>> defines outside of their header. This is going to make more difficult 
>> if we have to update them in the future.
>>
>> I can see a few ways to do it:
>>
>>   - Avoid using shifts in the definitions
>>   - Find a way to evaluate the value (maybe similar to asn-offset) 
>> before using them.
>>
>> My preference would be the latter but I could be convinced for the 
>> former.
> 
> The first option is not realistic. I suggested ARM to consider shifts 
> support as an improvement for their compiler.
> I'd be very happy to adopt the second option. Do you have any code 
> examples or hints how to evaluate expressions on the pre-processing stage?
I wasn't thinking to do it a pre-processing state but rather generating 
an header that will have the value already computed.

Have a look at how we generate defininition for assembly (see 
arch/arm32/asm-offsets.c).

Looking at the link you provided, I noticed that * is supported by the 
scatter file. So it might be possible to introduce a macro LSHIFT(a, b) 
that will be implemented as a << b or (a * a * a ...) depending on the 
users. The major difficulty here would be to find a way to generate a * 
a * a... nicely during pre-processing.

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Andrii Anisov]

On 11.11.19 23:26, Stefano Stabellini wrote:

> Why _srodata and __start_bug_frames cannot both be defined as
> Load$$_rodata_bug_frames_0$$Base when actually in the case of:
> 
>> +#define __per_cpu_data_end          Load$$_bss_percpu$$Limit
>> +#define __bss_end                   Load$$_bss_percpu$$Limit
>> +#define _end                        Load$$_bss_percpu$$Limit
> 
> They are all defined as "Load$$_bss_percpu$$Limit"? And:
> 
>> +#define __init_end                  Load$$_bss$$Base
>> +#define __bss_start                 Load$$_bss$$Base
> 
> They are both defined as "Load$$_bss$$Base"? What's special about
> "Load$$_rodata_bug_frames_0$$Base"?


Because in xen/include/asm-arm/bug.h:
   extern const struct bug_frame __start_bug_frames[]

and in xen/include/xen/kernel.h
   extern const char _srodata[];

After preprocessing we, effectively, have symbol declared with conflicting types:
   extern const struct bug_frame Load$$_rodata_bug_frames_0$$Base[];
   extern const char Load$$_rodata_bug_frames_0$$Base[];

Eventually other symbols do not have such conflicts.

> 
> 
>>   - C style shift operators are missed among supported scatter file expressions,
>>     so some needed values are hardcoded in scatter file.
>>
>>   - Rename correspondent ARM Linker defined symbols to those needed by `symbols` tool
>>     using steering file feature.
>>
>>   - ARM Compiler 6.6 tools are not able to rename sections, so we still need
>>     GNU toolchain's objcopy for this.
>>
>> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
>> ---
>>   xen/Rules.mk                |   6 +
>>   xen/arch/arm/Makefile       |  24 ++++
>>   xen/arch/arm/xen.scat.S     | 266 ++++++++++++++++++++++++++++++++++++++++++++
> 
> I would strongly suggest to rename this file with something not
> potentially related to scat

I just followed examples from ARM documentation, e.g. [1]. I suppose they know something about their product.
Yet, the suggestion is reasonable.

> 
> 
>>   xen/arch/arm/xen.steer      |   5 +
>>   xen/include/asm-arm/armds.h |  91 +++++++++++++++
>>   5 files changed, 392 insertions(+)
>>   create mode 100644 xen/arch/arm/xen.scat.S
>>   create mode 100644 xen/arch/arm/xen.steer
>>   create mode 100644 xen/include/asm-arm/armds.h
>>
>> diff --git a/xen/Rules.mk b/xen/Rules.mk
>> index 41a1c26..67bedce 100644
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -60,6 +60,12 @@ CFLAGS += -nostdinc -fno-builtin -fno-common
>>   CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
>>   $(call cc-option-add,CFLAGS,CC,-Wvla)
>>   CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
>> +
>> +ifeq ($(armds),y)
>> +CFLAGS += -nostdlibinc -nostdlib -Wno-unused-command-line-argument
>> +CFLAGS += -include $(BASEDIR)/include/asm/armds.h
>> +endif
>> +
>>   CFLAGS-$(CONFIG_DEBUG_INFO) += -g
>>   CFLAGS += '-D__OBJECT_FILE__="$@"'
>>   
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index 70f532e..a5a3479 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -83,11 +83,16 @@ else
>>   all_symbols =
>>   endif
>>   
>> +ifeq ($(armds),y)
>> +$(TARGET): $(TARGET)-syms
>> +	fromelf --bin $< --output $@
>> +else
>>   $(TARGET): $(TARGET)-syms
>>   	$(OBJCOPY) -O binary -S $< $@
>>   ifeq ($(CONFIG_ARM_64),y)
>>   	ln -sf $(notdir $@)  ../../$(notdir $@).efi
>>   endif
>> +endif
>>   
>>   ifeq ($(CONFIG_LTO),y)
>>   # Gather all LTO objects together
>> @@ -102,6 +107,19 @@ prelink.o: $(ALL_OBJS)
>>   	$(LD) $(LDFLAGS) -r -o $@ $^
>>   endif
>>   
>> +ifeq ($(armds),y)
>> +$(TARGET)-syms: prelink.o xen.scat
>> +	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
>> +	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
>> +		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S
>> +	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
>> +	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
>> +	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
>> +		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S
>> +	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
>> +	armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib --symdefs="$(@D)/$(@F).map" $(LDFLAGS) prelink.o $(build_id_linker) $(@D)/.$(@F).1.o -o $@
>> +	rm -f $(@D)/.$(@F).[0-9]*
>> +else
>>   $(TARGET)-syms: prelink.o xen.lds
>>   	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
>>   	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
>> @@ -119,14 +137,20 @@ $(TARGET)-syms: prelink.o xen.lds
>>   		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort \
>>   		>$(@D)/$(@F).map
>>   	rm -f $(@D)/.$(@F).[0-9]*
>> +endif
>>   
>>   asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
>>   	$(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $<
>>   
>> +ifeq ($(armds),y)
>> +xen.scat: xen.scat.S
>> +	$(CC) -P -E --target=aarch64-arm-none-eabi -o $@ $<
>> +else
>>   xen.lds: xen.lds.S
>>   	$(CC) -P -E -Ui386 $(AFLAGS) -o $@ $<
>>   	sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new
>>   	mv -f .xen.lds.d.new .xen.lds.d
>> +endif
>>   
>>   dtb.o: $(CONFIG_DTB_FILE)
>>   
>> diff --git a/xen/arch/arm/xen.scat.S b/xen/arch/arm/xen.scat.S
>> new file mode 100644
>> index 0000000..3bb405f
>> --- /dev/null
>> +++ b/xen/arch/arm/xen.scat.S
>> @@ -0,0 +1,266 @@
>> +#if 0
> 
> #if 0 must be a mistake?

It is here because of RFC. Because I do not like the hardcodes below, can't find the better way, but eager for suggestions.

> 
> Also, is this basically the ARMCC version of a linker script? Is
> xen.lds.S still used with ARMCC, or only xen.scat.S is used?

It is the ARMCC version of a linker script. xen.lds.S is not used.

> 
> 
>> +/*
>> + * armlink does not understand shifts in scat file expressions
>> + * so hardcode needed values
>> + */
>> +#include <xen/cache.h>
>> +#include <asm/page.h>
>> +#include <asm/percpu.h>
>> +#undef ENTRY
>> +#undef ALIGN
>> +#else
>> + #define PAGE_SIZE 4096
>> + #define POINTER_ALIGN 8
>> + #define SMP_CACHE_BYTES 128
>> + #define STACK_SIZE 32768
>> + #define XEN_VIRT_START 0x00200000
>> +#endif
>> +
>> +LOAD XEN_VIRT_START
>> +{
>> +;_start
>> +;_stext
>> +  _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090
>> +  {
>> +    *(.text*)
>> +    *(.text.cold)
>> +    *(.text.unlikely)
>> +    *(.fixup)
>> +    *(.gnu.warning)
>> +  }
>> +;_etext
>> +
>> +;_srodata
>> +;__start_bug_frames
>> +  _rodata_bug_frames_0 AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
>> +  {
>> +    *(.bug_frames.0)
>> +  }
>> +;__stop_bug_frames_0
>> +
>> +  _rodata_bug_frames_1 +0 FIXED ZEROPAD
>> +  {
>> +    *(.bug_frames.1)
>> +  }
>> +;__stop_bug_frames_1
>> +
>> +  _rodata_bug_frames_2 +0 FIXED ZEROPAD
>> +  {
>> +    *(.bug_frames.2)
>> +  }
>> +;__stop_bug_frames_2
>> +
>> +  _rodata_data +0
>> +  {
>> +    *(.rodata)
>> +    *(.rodata.*)
>> +    *(.data.rel.ro)
>> +    *(.data.rel.ro.*)
>> +  }
>> +
>> +#ifdef CONFIG_LOCK_PROFILE
>> +;__lock_profile_start
>> +  _rodata_lockprofile_data AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
>> +  {
>> +    *(.lockprofile.data)
>> +  }
>> +;__lock_profile_end
>> +#endif
> 
> Should be below

OK.

> 
> 
>> +;__param_start
>> +  _rodata_data_param AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
>> +  {
>> +    *(.data.param)
>> +  }
>> +;__param_end
>> +
>> +;__proc_info_start
>> +  _rodata_proc_info +0 FIXED ZEROPAD
>> +  {
>> +    *(.proc.info)
>> +  }
>> +;__proc_info_end
>> +
>> +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM)
>> +;__start_vpci_array
>> +  _rodata_data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD
>> +  {
>> +    *(SORT(.data.vpci.*))
>> +  }
>> +;__end_vpci_array
>> +#endif
>> +
>> +#if defined(BUILD_ID)
>> +;__note_gnu_build_id_start
>> +  _note_gnu_build_id +0 FIXED ZEROPAD
>> +  {
>> +    *(.note.gnu.build-id)
>> +  }
>> +;__note_gnu_build_id_end
>> +#endif
>> +
>> +;_erodata
>> +
>> +  _data AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
>> +  {
>> +    *(.data.page_aligned.*)
>> +    *(.data.*)
>> +  }
>> +
>> +;__start_schedulers_array
>> +  _data_schedulers AlignExpr(+0, 8) FIXED ZEROPAD
>> +  {
>> +    *(.data.schedulers)
>> +  }
>> +;__end_schedulers_array
>> +
>> +  _data_rel +0 FIXED ZEROPAD
>> +  {
>> +    *(.data.rel)
>> +    *(.data.rel.*)
>> +;#CONSTRUCTORS ????
> 
> Honestly I am not sure what this is either

Ah, it was a hint for myself to find place for constructors.

> 
> 
>> +  }
>> +
>> +;__start___ex_table
>> +  _data_ex_table AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD
>> +  {
>> +    *(.ex_table)
>> +  }
>> +;__stop___ex_table
>> +
>> +;__start___pre_ex_table
>> +  _data_ex_table_pre +0 FIXED ZEROPAD
>> +  {
>> +    *(.ex_table.pre)
>> +  }
>> +;__stop___pre_ex_table
>> +
>> +  _data_read_mostly +0 FIXED ZEROPAD
>> +  {
>> +    *(.data.read_mostly)
>> +  }
>> +
>> +;_splatform
>> +  _arch_info AlignExpr(+0, 8) FIXED ZEROPAD
>> +  {
>> +     *(.arch.info)
>> +  }
>> +;_eplatform
>> +
>> +;_sdevice
>> +  _dev_info AlignExpr(+0, 8) FIXED ZEROPAD
>> +  {
>> +    *(.dev.info)
>> +  }
>> +;_edevice
>> +
>> +;_asdevice
>> +  _adev_info AlignExpr(+0, 8) FIXED ZEROPAD
>> +  {
>> +    *(.adev.info)
>> +  }
>> +;_aedevice
> 
> _steemediator/_eteemediator

I suppose I did this .scat before _steemediator/_eteemediator were introduced to .lds.
OK.

> 
>> +;__init_begin
>> +;_sinittext
>> +  _init_text AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
>> +  {
>> +    *(.init.text)
>> +  }
>> +;_einittext
>> +
>> +  _init_rodata AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD
>> +  {
>> +    *(.init.rodata)
>> +    *(.init.rodata.rel)
>> +    *(.init.rodata.str*)
>> +  }
>> +
>> +;__setup_start
>> +  _init_setup AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD
>> +  {
>> +    *(.init.setup)
>> +  }
>> +;__setup_end
>> +
>> +;__initcall_start
>> +  _initcallpresmp_init +0 FIXED ZEROPAD
>> +  {
>> +    *(.initcallpresmp.init)
>> +  }
>> +;__presmp_initcall_end
>> +
>> +  _initcall1_init +0 FIXED ZEROPAD
>> +  {
>> +    *(.initcall1.init)
>> +  }
>> +;__initcall_end
>> +
>> +;__alt_instructions
>> +  _altinstructions AlignExpr(+0, 4) FIXED ZEROPAD
>> +  {
>> +    *(.altinstructions)
>> +  }
>> +;__alt_instructions_end
>> +
>> +  _altinstr_replacement AlignExpr(+0, 4) FIXED ZEROPAD
>> +  {
>> +    *(.altinstr_replacement)
>> +  }
> 
> __lock_profile_start should be here

OK.

> 
> 
>> +
>> +  _init_data +0 FIXED ZEROPAD
>> +  {
>> +    *(.init.data)
>> +    *(.init.data.rel)
>> +    *(.init.data.rel.*)
>> +  }
>> +
>> +;__ctors_start
>> +  _ctors AlignExpr(+0, 8) FIXED ZEROPAD
>> +  {
>> +    *(.ctors)
>> +    *(.init_array)
>> +  }
>> +
>> +  _init_array_sorted AlignExpr(+0, 8) SORTTYPE Lexical FIXED ZEROPAD
>> +  {
>> +    *(.init_array.*)
>> +  }
>> +;__ctors_end
>> +
>> +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM)
>> +  _data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD
>> +  {
>> +    *(.data.vpci.*)
>> +  }
>> +#endif
> 
> __start_vpci_array/__end_vpci_array?

OK.

> 
>> +;__init_end_efi
>> +
>> +;__init_end
>> +;__bss_start
>> +  _bss AlignExpr(+0, STACK_SIZE) FIXED ZEROPAD
>> +  {
>> +    *(.bss.stack_aligned*)
>> +    *(.bss.page_aligned*, OVERALIGN PAGE_SIZE)
>> +    *(.bss*)
>> +  }
>> +
>> +;__per_cpu_start
> 
> this should be page aligned too?



> 
>> +  _bss_percpu AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD
>> +  {
>> +    *(.bss.percpu)
>> +    *(.bss.percpu.read_mostly, OVERALIGN SMP_CACHE_BYTES)
>> +  }
>> +;__per_cpu_data_end
>> +;__bss_end
> 
> __bss_end should be page aligned?

Well... Maybe, but I'm not sure how to do that.

> 
>> +;_end
>> +
>> +#ifdef CONFIG_DTB_FILE
>> +;_sdtb
>> +  _dtb FIXED ZEROPAD
>> + {
>> +    *(.dtb)
>> + }
>> +#endif
>> +
>> +}
>> diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer
>> new file mode 100644
>> index 0000000..646e912
>> --- /dev/null
>> +++ b/xen/arch/arm/xen.steer
>> @@ -0,0 +1,5 @@
>> +RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base
>> +RENAME Load$$_text$$Base AS _stext
>> +RENAME Load$$_text$$Limit AS _etext
>> +RENAME Load$$_init_text$$Base AS _sinittext
>> +RENAME Load$$_init_text$$Limit AS _einittext
> 
> I don't get why some if the "symbols" get renamed using RENAME here, and
> some other we are using a #define in xen/include/asm-arm/armds.h. Can't
> we rename them all here?

Well, the situation here is really complicated. I described above a reason why _srodata is resolved here.
Other symbols are renamed here because the tool "xen/tools/symbols", used at the latest linking stages, needs `_stext`, `_etext`, and the rest to be present in the elf.

> 
>> diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h
>> new file mode 100644
>> index 0000000..5ee2e5d
>> --- /dev/null
>> +++ b/xen/include/asm-arm/armds.h
> 
> Missing guards. Also, probably you want to make sure this is only #ifdef
> ARMCC.

OK.

> 
> Is this meant to be used when building C files, asm files, or both?

Well, I have to check.

> 
> I would avoid this header file if we can get away with just xen.steer.

We can't go with xen.steer only. One of the armlink issues is "ARM linker defined symbols are not counted as referred if only mentioned in a steering file for rename or resolve". Also, linker-defined symbols are only generated when the code references them [2].
I tried resolving existing symbols (e.g. _start) to armlink defined symbols with .steer only, and got errors that armlink can't find those linker-defined symbols.
I tried a specific C file with references to all needed linker-defined symbols, then resolving all .lds-style symbols to armlink defined symbols with the steering file. But it did not work, I don't remember exactly the issue. Maybe C file with externs only (without using them in the effective code) did not result in an object file referring those linker-defined symbols.

> 
> 
>> @@ -0,0 +1,91 @@
>> +#define _start                      Load$$_text$$Base
>> +#define _stext                      Load$$_text$$Base
>> +
>> +#define _etext                      Load$$_text$$Limit
>> +
>> +//#define _srodata                    Load$$_rodata_bug_frames_0$$Base
>> +#define __start_bug_frames          Load$$_rodata_bug_frames_0$$Base
>> +
>> +#define __stop_bug_frames_0         Load$$_rodata_bug_frames_0$$Limit
>> +#define __stop_bug_frames_1         Load$$_rodata_bug_frames_1$$Limit
>> +#define __stop_bug_frames_2         Load$$_rodata_bug_frames_2$$Limit
>> +
>> +#ifdef CONFIG_LOCK_PROFILE
>> +#define __lock_profile_start        Load$$_rodata_lockprofile_data$$Base
>> +#define __lock_profile_end          Load$$_rodata_lockprofile_data$$Limit
>> +#endif
>> +
>> +#define __param_start               Load$$_rodata_data_param$$Base
>> +#define __param_end                 Load$$_rodata_data_param$$Limit
>> +
>> +#define __proc_info_start           Load$$_rodata_proc_info$$Base
>> +#define __proc_info_end             Load$$_rodata_proc_info$$Limit
>> +
>> +#define _erodata                    Load$$_rodata_proc_info$$Limit
>> +
>> +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM)
>> +#define __start_vpci_array          Load$$_rodata_data_vpci$$Base
>> +#define __end_vpci_array            Load$$_rodata_data_vpci$$Limit
>> +
>> +#undef _erodata
>> +#define _erodata                    Load$$_rodata_data_vpci$$Limit
>> +#endif
>> +
>> +#if defined(BUILD_ID)
>> +#define __note_gnu_build_id_start   Load$$_note_gnu_build_id$$Base
>> +#define __note_gnu_build_id_end     Load$$_note_gnu_build_id$$Limit
>> +
>> +#undef _erodata
>> +#define _erodata                    Load$$_note_gnu_build_id$$Limit
>> +#endif
>> +
>> +#define __start_schedulers_array    Load$$_data_schedulers$$Base
>> +#define __end_schedulers_array      Load$$_data_schedulers$$Limit
>> +
>> +/* Does not exist for ARM
>> +#define __start___ex_table          Load$$_data_ex_table$$Base
>> +#define __stop___ex_table           Load$$_data_ex_table$$Limit
>> +*/
>> +
>> +#define __start___pre_ex_table      Load$$_data_ex_table_pre$$Base
>> +#define __stop___pre_ex_table       Load$$_data_ex_table_pre$$Limit
>> +
>> +#define _splatform                  Load$$_arch_info$$Base
>> +#define _eplatform                  Load$$_arch_info$$Limit
>> +
>> +#define _sdevice                    Load$$_dev_info$$Base
>> +#define _edevice                    Load$$_dev_info$$Limit
>> +
>> +#define _asdevice                   Load$$_adev_info$$Base
>> +#define _aedevice                   Load$$_adev_info$$Limit
>> +
>> +#define __init_begin                Load$$_init_text$$Base
>> +#define _sinittext                  Load$$_init_text$$Base
>> +#define _einittext                  Load$$_init_text$$Limit
>> +
>> +#define __setup_start               Load$$_init_setup$$Base
>> +#define __setup_end                 Load$$_init_setup$$Limit
>> +
>> +#define __initcall_start            Load$$_initcallpresmp_init$$Base
>> +#define __presmp_initcall_end       Load$$_initcallpresmp_init$$Limit
>> +#define __initcall_end              Load$$_initcall1_init$$Limit
>> +
>> +#define __alt_instructions          Load$$_altinstructions$$Base
>> +#define __alt_instructions_end      Load$$_altinstructions$$Limit
>> +
>> +#define __ctors_start               Load$$_ctors$$Base
>> +#define __ctors_end                 Load$$_init_array_sorted$$Limit
>> +#define __init_end_efi              Load$$_init_array_sorted$$Limit
>> +
>> +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM)
>> +#undef __init_end_efi
>> +#define __init_end_efi              Load$$_data_vpci$$Limit
>> +#endif
>> +
>> +#define __init_end                  Load$$_bss$$Base
>> +#define __bss_start                 Load$$_bss$$Base
>> +
>> +#define __per_cpu_start             Load$$_bss_percpu$$Base
>> +#define __per_cpu_data_end          Load$$_bss_percpu$$Limit
>> +#define __bss_end                   Load$$_bss_percpu$$Limit
>> +#define _end                        Load$$_bss_percpu$$Limit


[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0803j/rbm1505486312921.html
[2] http://infocenter.arm.com/help/topic/com.arm.doc.dui0803j/nbd1509536435303.html

-- 
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Stefano Stabellini]

On Thu, 14 Nov 2019, Andrii Anisov wrote:
> On 11.11.19 23:26, Stefano Stabellini wrote:
> 
> > Why _srodata and __start_bug_frames cannot both be defined as
> > Load$$_rodata_bug_frames_0$$Base when actually in the case of:
> > 
> > > +#define __per_cpu_data_end          Load$$_bss_percpu$$Limit
> > > +#define __bss_end                   Load$$_bss_percpu$$Limit
> > > +#define _end                        Load$$_bss_percpu$$Limit
> > 
> > They are all defined as "Load$$_bss_percpu$$Limit"? And:
> > 
> > > +#define __init_end                  Load$$_bss$$Base
> > > +#define __bss_start                 Load$$_bss$$Base
> > 
> > They are both defined as "Load$$_bss$$Base"? What's special about
> > "Load$$_rodata_bug_frames_0$$Base"?
> 
> 
> Because in xen/include/asm-arm/bug.h:
>   extern const struct bug_frame __start_bug_frames[]
> 
> and in xen/include/xen/kernel.h
>   extern const char _srodata[];
> 
> After preprocessing we, effectively, have symbol declared with conflicting
> types:
>   extern const struct bug_frame Load$$_rodata_bug_frames_0$$Base[];
>   extern const char Load$$_rodata_bug_frames_0$$Base[];
> 
> Eventually other symbols do not have such conflicts.

That is something to add to the commit message


> > >   - C style shift operators are missed among supported scatter file
> > > expressions,
> > >     so some needed values are hardcoded in scatter file.
> > > 
> > >   - Rename correspondent ARM Linker defined symbols to those needed by
> > > `symbols` tool
> > >     using steering file feature.
> > > 
> > >   - ARM Compiler 6.6 tools are not able to rename sections, so we still
> > > need
> > >     GNU toolchain's objcopy for this.
> > > 
> > > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> > > ---
> > >   xen/Rules.mk                |   6 +
> > >   xen/arch/arm/Makefile       |  24 ++++
> > >   xen/arch/arm/xen.scat.S     | 266
> > > ++++++++++++++++++++++++++++++++++++++++++++
> > 
> > I would strongly suggest to rename this file with something not
> > potentially related to scat
> 
> I just followed examples from ARM documentation, e.g. [1]. I suppose they know
> something about their product.
> Yet, the suggestion is reasonable.

LOL!!
 
 
> > > diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer
> > > new file mode 100644
> > > index 0000000..646e912
> > > --- /dev/null
> > > +++ b/xen/arch/arm/xen.steer
> > > @@ -0,0 +1,5 @@
> > > +RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base
> > > +RENAME Load$$_text$$Base AS _stext
> > > +RENAME Load$$_text$$Limit AS _etext
> > > +RENAME Load$$_init_text$$Base AS _sinittext
> > > +RENAME Load$$_init_text$$Limit AS _einittext
> > 
> > I don't get why some if the "symbols" get renamed using RENAME here, and
> > some other we are using a #define in xen/include/asm-arm/armds.h. Can't
> > we rename them all here?
> 
> Well, the situation here is really complicated. I described above a reason why
> _srodata is resolved here.
> Other symbols are renamed here because the tool "xen/tools/symbols", used at
> the latest linking stages, needs `_stext`, `_etext`, and the rest to be
> present in the elf.
> 
> > 
> > > diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h
> > > new file mode 100644
> > > index 0000000..5ee2e5d
> > > --- /dev/null
> > > +++ b/xen/include/asm-arm/armds.h
> > 
> > Missing guards. Also, probably you want to make sure this is only #ifdef
> > ARMCC.
> 
> OK.
> 
> > 
> > Is this meant to be used when building C files, asm files, or both?
> 
> Well, I have to check.
> 
> > 
> > I would avoid this header file if we can get away with just xen.steer.
> 
> We can't go with xen.steer only. One of the armlink issues is "ARM linker
> defined symbols are not counted as referred if only mentioned in a steering
> file for rename or resolve". Also, linker-defined symbols are only generated
> when the code references them [2].
> I tried resolving existing symbols (e.g. _start) to armlink defined symbols
> with .steer only, and got errors that armlink can't find those linker-defined
> symbols.
> I tried a specific C file with references to all needed linker-defined
> symbols, then resolving all .lds-style symbols to armlink defined symbols with
> the steering file. But it did not work, I don't remember exactly the issue.
> Maybe C file with externs only (without using them in the effective code) did
> not result in an object file referring those linker-defined symbols.

I don't know what the right solution is, but it would be nice to have
some consistency. Otherwise the next time a new symbol is added we won't
know whether it needs to be added to xen.steer or armds.h. We need to
have a clear rule to follow so that we can easily figure out why each
symbol is in xen.steer and/or armds.h.

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

[Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

ARM Compiler 6 has a proven bug: it compiles data only C files with
SoftVFP attributes. This leads to a failed linkage afterwards with
an error:

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

The known workaround is introducing some code into the affected file,
e.g. an empty (non-static) function is enough.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/arch/arm/platforms/brcm-raspberry-pi.c | 2 ++
 xen/arch/arm/platforms/thunderx.c          | 2 ++
 xen/xsm/flask/gen-policy.py                | 4 ++++
 3 files changed, 8 insertions(+)

diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c b/xen/arch/arm/platforms/brcm-raspberry-pi.c
index b697fa2..7ab1810 100644
--- a/xen/arch/arm/platforms/brcm-raspberry-pi.c
+++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c
@@ -40,6 +40,8 @@ static const struct dt_device_match rpi4_blacklist_dev[] __initconst =
     { /* sentinel */ },
 };
 
+void brcm_raspberry_pi_dummy_func(void) {}
+
 PLATFORM_START(rpi4, "Raspberry Pi 4")
     .compatible     = rpi4_dt_compat,
     .blacklist_dev  = rpi4_blacklist_dev,
diff --git a/xen/arch/arm/platforms/thunderx.c b/xen/arch/arm/platforms/thunderx.c
index 9b32a29..8015323 100644
--- a/xen/arch/arm/platforms/thunderx.c
+++ b/xen/arch/arm/platforms/thunderx.c
@@ -33,6 +33,8 @@ static const struct dt_device_match thunderx_blacklist_dev[] __initconst =
     { /* sentinel */ },
 };
 
+void thunderx_dummy_func(void) {}
+
 PLATFORM_START(thunderx, "THUNDERX")
     .compatible = thunderx_dt_compat,
     .blacklist_dev = thunderx_blacklist_dev,
diff --git a/xen/xsm/flask/gen-policy.py b/xen/xsm/flask/gen-policy.py
index c7501e4..73bf7d2 100644
--- a/xen/xsm/flask/gen-policy.py
+++ b/xen/xsm/flask/gen-policy.py
@@ -21,3 +21,7 @@ sys.stdout.write("""
 };
 const unsigned int __initconst xsm_flask_init_policy_size = %d;
 """ % policy_size)
+
+sys.stdout.write("""
+void policy_dummy_func(void) {}
+""")
-- 
2.7.4


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

Re: [Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Stefano Stabellini]

On Wed, 6 Nov 2019, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> ARM Compiler 6 has a proven bug: it compiles data only C files with
> SoftVFP attributes. This leads to a failed linkage afterwards with
> an error:
> 
> Error: L6242E: Cannot link object built_in.o as its attributes are incompatible with the image attributes.
> ... A64 clashes with SoftVFP.
> 
> The known workaround is introducing some code into the affected file,
> e.g. an empty (non-static) function is enough.

Oh man, this is truly horrible.

If we really have to do this please:

- use the same dummy function name in all files
- the function should be static
- hiding the function within a #ifdef ARMCC block
- potentially hide the whole horrible hack behind a #define so that it
  would become at the call site:

 +ARMCC_DUMMY_FUNC_HACK()



> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
>
> ---
>  xen/arch/arm/platforms/brcm-raspberry-pi.c | 2 ++
>  xen/arch/arm/platforms/thunderx.c          | 2 ++
>  xen/xsm/flask/gen-policy.py                | 4 ++++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> index b697fa2..7ab1810 100644
> --- a/xen/arch/arm/platforms/brcm-raspberry-pi.c
> +++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> @@ -40,6 +40,8 @@ static const struct dt_device_match rpi4_blacklist_dev[] __initconst =
>      { /* sentinel */ },
>  };
>  
> +void brcm_raspberry_pi_dummy_func(void) {}
> +
>  PLATFORM_START(rpi4, "Raspberry Pi 4")
>      .compatible     = rpi4_dt_compat,
>      .blacklist_dev  = rpi4_blacklist_dev,
> diff --git a/xen/arch/arm/platforms/thunderx.c b/xen/arch/arm/platforms/thunderx.c
> index 9b32a29..8015323 100644
> --- a/xen/arch/arm/platforms/thunderx.c
> +++ b/xen/arch/arm/platforms/thunderx.c
> @@ -33,6 +33,8 @@ static const struct dt_device_match thunderx_blacklist_dev[] __initconst =
>      { /* sentinel */ },
>  };
>  
> +void thunderx_dummy_func(void) {}
> +
>  PLATFORM_START(thunderx, "THUNDERX")
>      .compatible = thunderx_dt_compat,
>      .blacklist_dev = thunderx_blacklist_dev,
> diff --git a/xen/xsm/flask/gen-policy.py b/xen/xsm/flask/gen-policy.py
> index c7501e4..73bf7d2 100644
> --- a/xen/xsm/flask/gen-policy.py
> +++ b/xen/xsm/flask/gen-policy.py
> @@ -21,3 +21,7 @@ sys.stdout.write("""
>  };
>  const unsigned int __initconst xsm_flask_init_policy_size = %d;
>  """ % policy_size)
> +
> +sys.stdout.write("""
> +void policy_dummy_func(void) {}
> +""")
> -- 
> 2.7.4
> 

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

Re: [Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Andrii Anisov]

Hello Stefano,

On 11.11.19 22:57, Stefano Stabellini wrote:
> Oh man, this is truly horrible.

I feel your pain.

> 
> If we really have to do this please:
> 
> - use the same dummy function name in all files

No, because

> - the function should be static

those functions will not handle the case if are static.

ARM commented the issue in the correspondent support case:

"A known workaround is to edit the source file and add an unused empty function, as you’ve already found. By default, an unused empty function should be removed from the final image by the unused section elimination feature of the linker, so it shouldn’t have a code size impact.

A command-line option workaround isn’t available. We’ll now set this case to a “Defect/Enhancement” state while the issue remains unfixed, and will keep you updated accordingly."


> - hiding the function within a #ifdef ARMCC block
> - potentially hide the whole horrible hack behind a #define so that it
>    would become at the call site:
> 
>   +ARMCC_DUMMY_FUNC_HACK()

This is quite reasonable.

-- 
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 1545 bytes --]

On Tue, 12 Nov 2019, 05:57 Stefano Stabellini, <sstabellini@kernel.org>
wrote:

> On Wed, 6 Nov 2019, Andrii Anisov wrote:
> > From: Andrii Anisov <andrii_anisov@epam.com>
> >
> > ARM Compiler 6 has a proven bug: it compiles data only C files with
> > SoftVFP attributes. This leads to a failed linkage afterwards with
> > an error:
> >
> > Error: L6242E: Cannot link object built_in.o as its attributes are
> incompatible with the image attributes.
> > ... A64 clashes with SoftVFP.
> >
> > The known workaround is introducing some code into the affected file,
> > e.g. an empty (non-static) function is enough.
>
> Oh man, this is truly horrible.
>
> If we really have to do this please:
>
> - use the same dummy function name in all files
> - the function should be static
> - hiding the function within a #ifdef ARMCC block
> - potentially hide the whole horrible hack behind a #define so that it
>   would become at the call site:
>
>  +ARMCC_DUMMY_FUNC_HACK()


The risk here is we may introduce new file in the future possibly in common
code with similar issues. So I would prefer if we can find an automatic way
to do this. Some ideas:
    - Add the function at compile time (via makefile). This would be done
for all the files but that's should not be a major issues.
    - Force disable softfvp either via command line, new line in the code
or rewriting the attribute of the object.

But before spending time trying to workaround a buggy compiler. What's the
plan with it? Is it going to be used in production or just a demo?

Cheers,

[-- Attachment #1.2: Type: text/html, Size: 2258 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Artem Mygaiev]

Hello Julien

On Thu, 2019-11-14 at 08:03 +0900, Julien Grall wrote:
> 
> 
> On Tue, 12 Nov 2019, 05:57 Stefano Stabellini, <
> sstabellini@kernel.org> wrote:
> > On Wed, 6 Nov 2019, Andrii Anisov wrote:
> > > From: Andrii Anisov <andrii_anisov@epam.com>
> > > 
> > > ARM Compiler 6 has a proven bug: it compiles data only C files
> > with
> > > SoftVFP attributes. This leads to a failed linkage afterwards
> > with
> > > an error:
> > > 
> > > Error: L6242E: Cannot link object built_in.o as its attributes
> > are incompatible with the image attributes.
> > > ... A64 clashes with SoftVFP.
> > > 
> > > The known workaround is introducing some code into the affected
> > file,
> > > e.g. an empty (non-static) function is enough.
> > 
> > Oh man, this is truly horrible.
> > 
> > If we really have to do this please:
> > 
> > - use the same dummy function name in all files
> > - the function should be static
> > - hiding the function within a #ifdef ARMCC block
> > - potentially hide the whole horrible hack behind a #define so that
> > it
> >   would become at the call site:
> > 
> >  +ARMCC_DUMMY_FUNC_HACK()
> 
> 
> The risk here is we may introduce new file in the future possibly in
> common code with similar issues. So I would prefer if we can find an
> automatic way to do this. Some ideas:
>     - Add the function at compile time (via makefile). This would be
> done for all the files but that's should not be a major issues.
>     - Force disable softfvp either via command line, new line in the
> code or rewriting the attribute of the object.
> 
> But before spending time trying to workaround a buggy compiler.
> What's the plan with it? Is it going to be used in production or just
> a demo?

This is not intended for a production program at the moment, and it
obviously require lot of further work. I would not try to upstream ugly
workarounds for issues like the one above, it would be much better to
somehow persuade Arm tools team to properly fix them.

This RFC series has following goals:
1) prove that we can use safety-certified tools for Xen and avoid
possible arguments on compiler/linker certification path
2) research possible issues when using non-standard compiler/linker and
try to see if it is easy to adjust Xen to use them

In the end, it would be great to make Xen build system flexible enough
to use with non-standard compilers without overcomplicating it or changing it significantly, causing too much disruption to community.

> Cheers,
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> 
> https://urldefense.com/v3/__https://lists.xenproject.org/mailman/listinfo/xen-devel__;!K6dmGCEab4ueJg!kCpXu2prtUxCHZV8aCvxYk9E82KnsHuNftyCeG745Ei3vhO2VP_SYXDnItHeZZCwdw$
>  
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Stefano Stabellini]

+ fusa-sig

On Thu, 14 Nov 2019, Artem Mygaiev wrote:
> Hello Julien
> 
> On Thu, 2019-11-14 at 08:03 +0900, Julien Grall wrote:
> > 
> > 
> > On Tue, 12 Nov 2019, 05:57 Stefano Stabellini, <
> > sstabellini@kernel.org> wrote:
> > > On Wed, 6 Nov 2019, Andrii Anisov wrote:
> > > > From: Andrii Anisov <andrii_anisov@epam.com>
> > > > 
> > > > ARM Compiler 6 has a proven bug: it compiles data only C files
> > > with
> > > > SoftVFP attributes. This leads to a failed linkage afterwards
> > > with
> > > > an error:
> > > > 
> > > > Error: L6242E: Cannot link object built_in.o as its attributes
> > > are incompatible with the image attributes.
> > > > ... A64 clashes with SoftVFP.
> > > > 
> > > > The known workaround is introducing some code into the affected
> > > file,
> > > > e.g. an empty (non-static) function is enough.
> > > 
> > > Oh man, this is truly horrible.
> > > 
> > > If we really have to do this please:
> > > 
> > > - use the same dummy function name in all files
> > > - the function should be static
> > > - hiding the function within a #ifdef ARMCC block
> > > - potentially hide the whole horrible hack behind a #define so that
> > > it
> > >   would become at the call site:
> > > 
> > >  +ARMCC_DUMMY_FUNC_HACK()
> > 
> > 
> > The risk here is we may introduce new file in the future possibly in
> > common code with similar issues. So I would prefer if we can find an
> > automatic way to do this. Some ideas:
> >     - Add the function at compile time (via makefile). This would be
> > done for all the files but that's should not be a major issues.
> >     - Force disable softfvp either via command line, new line in the
> > code or rewriting the attribute of the object.
> > 
> > But before spending time trying to workaround a buggy compiler.
> > What's the plan with it? Is it going to be used in production or just
> > a demo?
> 
> This is not intended for a production program at the moment, and it
> obviously require lot of further work. I would not try to upstream ugly
> workarounds for issues like the one above, it would be much better to
> somehow persuade Arm tools team to properly fix them.
> 
> This RFC series has following goals:
> 1) prove that we can use safety-certified tools for Xen and avoid
> possible arguments on compiler/linker certification path
> 2) research possible issues when using non-standard compiler/linker and
> try to see if it is easy to adjust Xen to use them
> 
> In the end, it would be great to make Xen build system flexible enough
> to use with non-standard compilers without overcomplicating it or changing it significantly, causing too much disruption to community.

I am aligned with you on the goals.

On this specific issue, it would be great if Arm fixed their compiler.
Maybe we could discussed this problem with the Arm folks during one of
the next FuSa calls (list in CC).

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

Re: [Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 2385 bytes --]

Hi,

On Wed, 6 Nov 2019, 18:20 Andrii Anisov, <andrii.anisov@gmail.com> wrote:

> From: Andrii Anisov <andrii_anisov@epam.com>
>
> ARM Compiler 6 has a proven bug: it compiles data only C files with
> SoftVFP attributes. This leads to a failed linkage afterwards with
> an error:
>

And there are no way to force disabling the softfvp attributes?


> Error: L6242E: Cannot link object built_in.o as its attributes are
> incompatible with the image attributes.
> ... A64 clashes with SoftVFP.
>
> The known workaround is introducing some code into the affected file,
> e.g. an empty (non-static) function is enough.
>

Was this reported to Arm? If so, what was there answer?

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> ---
>  xen/arch/arm/platforms/brcm-raspberry-pi.c | 2 ++
>  xen/arch/arm/platforms/thunderx.c          | 2 ++
>  xen/xsm/flask/gen-policy.py                | 4 ++++
>  3 files changed, 8 insertions(+)
>
> diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c
> b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> index b697fa2..7ab1810 100644
> --- a/xen/arch/arm/platforms/brcm-raspberry-pi.c
> +++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> @@ -40,6 +40,8 @@ static const struct dt_device_match rpi4_blacklist_dev[]
> __initconst =
>      { /* sentinel */ },
>  };
>
> +void brcm_raspberry_pi_dummy_func(void) {}
> +
>  PLATFORM_START(rpi4, "Raspberry Pi 4")
>      .compatible     = rpi4_dt_compat,
>      .blacklist_dev  = rpi4_blacklist_dev,
> diff --git a/xen/arch/arm/platforms/thunderx.c
> b/xen/arch/arm/platforms/thunderx.c
> index 9b32a29..8015323 100644
> --- a/xen/arch/arm/platforms/thunderx.c
> +++ b/xen/arch/arm/platforms/thunderx.c
> @@ -33,6 +33,8 @@ static const struct dt_device_match
> thunderx_blacklist_dev[] __initconst =
>      { /* sentinel */ },
>  };
>
> +void thunderx_dummy_func(void) {}
> +
>  PLATFORM_START(thunderx, "THUNDERX")
>      .compatible = thunderx_dt_compat,
>      .blacklist_dev = thunderx_blacklist_dev,
> diff --git a/xen/xsm/flask/gen-policy.py b/xen/xsm/flask/gen-policy.py
> index c7501e4..73bf7d2 100644
> --- a/xen/xsm/flask/gen-policy.py
> +++ b/xen/xsm/flask/gen-policy.py
> @@ -21,3 +21,7 @@ sys.stdout.write("""
>  };
>  const unsigned int __initconst xsm_flask_init_policy_size = %d;
>  """ % policy_size)
> +
> +sys.stdout.write("""
> +void policy_dummy_func(void) {}
> +""")
> --
> 2.7.4
>
>

[-- Attachment #1.2: Type: text/html, Size: 3607 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Xen-devel] [RFC 6/7] arm: Introduce dummy empty functions for data only C files

[Andrii Anisov]

Dear Julien,

On 13.11.19 07:51, Julien Grall wrote:
> Was this reported to Arm? 
All mentioned ARM Compiler issues were reported to ARM. Unfortunately, ARM hesitated to discus them in community, yet asked to open support cases, e.g. [1].
All issues and their WA's were acknowledged by ARM in correspondent support cases. Except C-style shifts issue what is not a bug but the documented feature [2], and ARM's answer about it was really uncertain.
Only after getting answer for all cases I finalized the patches.


> If so, what was there answer?

I already cited the answer for this particular issue while answering Stefano.

[1] https://community.arm.com/developer/tools-software/tools/f/arm-compilers-forum/44287/arm-compiler-6-compiles-data-only-c-file-with-softvfp-attribute
[2] https://developer.arm.com/docs/100070/0612/scatter-file-syntax/expression-evaluation-in-scatter-files/expression-rules-in-scatter-files

-- 
Sincerely,
Andrii Anisov.

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

[Xen-devel] [RFC 7/7] arm/gic-v3: add GIC version suffix to iomem range variables

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

ARM Compiler 6.6 has a proven bug: static data symbols, moved to an init
section, becomes global. Thus these symbols clash with ones defined in
gic-v2.c. The straight forward way to resolve the issue is to add the GIC
version suffix, at least for one of the conflicting side.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/arch/arm/gic-v3.c | 68 +++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 0f6cbf6..f57597a 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1328,14 +1328,14 @@ static const hw_irq_controller gicv3_guest_irq_type = {
     .set_affinity = gicv3_irq_set_affinity,
 };
 
-static paddr_t __initdata dbase = INVALID_PADDR;
-static paddr_t __initdata vbase = INVALID_PADDR, vsize = 0;
-static paddr_t __initdata cbase = INVALID_PADDR, csize = 0;
+static paddr_t __initdata dbase_v3 = INVALID_PADDR;
+static paddr_t __initdata vbase_v3 = INVALID_PADDR, vsize_v3 = 0;
+static paddr_t __initdata cbase_v3 = INVALID_PADDR, csize_v3 = 0;
 
 /* If the GICv3 supports GICv2, initialize it */
 static void __init gicv3_init_v2(void)
 {
-    if ( cbase == INVALID_PADDR || vbase == INVALID_PADDR )
+    if ( cbase_v3 == INVALID_PADDR || vbase_v3 == INVALID_PADDR )
         return;
 
     /*
@@ -1343,26 +1343,26 @@ static void __init gicv3_init_v2(void)
      * So only support GICv2 on GICv3 when the virtual CPU interface is
      * at least GUEST_GICC_SIZE.
      */
-    if ( vsize < GUEST_GICC_SIZE )
+    if ( vsize_v3 < GUEST_GICC_SIZE )
     {
         printk(XENLOG_WARNING
                "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
                "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
-               vsize, GUEST_GICC_SIZE);
+               vsize_v3, GUEST_GICC_SIZE);
         return;
     }
 
     printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase %#"PRIpaddr"\n",
-           cbase, vbase);
+           cbase_v3, vbase_v3);
 
-    vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
+    vgic_v2_setup_hw(dbase_v3, cbase_v3, csize_v3, vbase_v3, 0);
 }
 
 static void __init gicv3_ioremap_distributor(paddr_t dist_paddr)
 {
     if ( dist_paddr & ~PAGE_MASK )
         panic("GICv3:  Found unaligned distributor address %"PRIpaddr"\n",
-              dbase);
+              dbase_v3);
 
     gicv3.map_dbase = ioremap_nocache(dist_paddr, SZ_64K);
     if ( !gicv3.map_dbase )
@@ -1375,11 +1375,11 @@ static void __init gicv3_dt_init(void)
     int res, i;
     const struct dt_device_node *node = gicv3_info.node;
 
-    res = dt_device_get_address(node, 0, &dbase, NULL);
+    res = dt_device_get_address(node, 0, &dbase_v3, NULL);
     if ( res )
         panic("GICv3: Cannot find a valid distributor address\n");
 
-    gicv3_ioremap_distributor(dbase);
+    gicv3_ioremap_distributor(dbase_v3);
 
     if ( !dt_property_read_u32(node, "#redistributor-regions",
                 &gicv3.rdist_count) )
@@ -1416,10 +1416,10 @@ static void __init gicv3_dt_init(void)
      * provided.
      */
     res = dt_device_get_address(node, 1 + gicv3.rdist_count,
-                                &cbase, &csize);
+                                &cbase_v3, &csize_v3);
     if ( !res )
         dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
-                              &vbase, &vsize);
+                              &vbase_v3, &vsize_v3);
 }
 
 static int gicv3_iomem_deny_access(const struct domain *d)
@@ -1427,7 +1427,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
     int rc, i;
     unsigned long mfn, nr;
 
-    mfn = dbase >> PAGE_SHIFT;
+    mfn = dbase_v3 >> PAGE_SHIFT;
     nr = PFN_UP(SZ_64K);
     rc = iomem_deny_access(d, mfn, mfn + nr);
     if ( rc )
@@ -1446,19 +1446,19 @@ static int gicv3_iomem_deny_access(const struct domain *d)
             return rc;
     }
 
-    if ( cbase != INVALID_PADDR )
+    if ( cbase_v3 != INVALID_PADDR )
     {
-        mfn = cbase >> PAGE_SHIFT;
-        nr = PFN_UP(csize);
+        mfn = cbase_v3 >> PAGE_SHIFT;
+        nr = PFN_UP(csize_v3);
         rc = iomem_deny_access(d, mfn, mfn + nr);
         if ( rc )
             return rc;
     }
 
-    if ( vbase != INVALID_PADDR )
+    if ( vbase_v3 != INVALID_PADDR )
     {
-        mfn = vbase >> PAGE_SHIFT;
-        nr = PFN_UP(csize);
+        mfn = vbase_v3 >> PAGE_SHIFT;
+        nr = PFN_UP(csize_v3);
         return iomem_deny_access(d, mfn, mfn + nr);
     }
 
@@ -1564,8 +1564,8 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
     /* Read from APIC table and fill up the GIC variables */
     if ( !cpu_base_assigned )
     {
-        cbase = processor->base_address;
-        vbase = processor->gicv_base_address;
+        cbase_v3 = processor->base_address;
+        vbase_v3 = processor->gicv_base_address;
         gicv3_info.maintenance_irq = processor->vgic_interrupt;
 
         if ( processor->flags & ACPI_MADT_VGIC_IRQ_MODE )
@@ -1577,8 +1577,8 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
     }
     else
     {
-        if ( cbase != processor->base_address
-             || vbase != processor->gicv_base_address
+        if ( cbase_v3 != processor->base_address
+             || vbase_v3 != processor->gicv_base_address
              || gicv3_info.maintenance_irq != processor->vgic_interrupt )
         {
             printk("GICv3: GICC entries are not same in MADT table\n");
@@ -1599,7 +1599,7 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
     if ( BAD_MADT_ENTRY(dist, end) )
         return -EINVAL;
 
-    dbase = dist->base_address;
+    dbase_v3 = dist->base_address;
 
     return 0;
 }
@@ -1674,7 +1674,7 @@ static void __init gicv3_acpi_init(void)
     if ( count <= 0 )
         panic("GICv3: No valid GICD entries exists\n");
 
-    gicv3_ioremap_distributor(dbase);
+    gicv3_ioremap_distributor(dbase_v3);
 
     /* Get number of redistributor */
     count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
@@ -1722,15 +1722,15 @@ static void __init gicv3_acpi_init(void)
      * Also set the size of the GICC and GICV when there base address
      * is not invalid as those values are not present in ACPI.
      */
-    if ( !cbase )
-        cbase = INVALID_PADDR;
+    if ( !cbase_v3 )
+        cbase_v3 = INVALID_PADDR;
     else
-        csize = SZ_8K;
+        csize_v3 = SZ_8K;
 
-    if ( !vbase )
-        vbase = INVALID_PADDR;
+    if ( !vbase_v3 )
+        vbase_v3 = INVALID_PADDR;
     else
-        vsize = GUEST_GICC_SIZE;
+        vsize_v3 = GUEST_GICC_SIZE;
 
 }
 #else
@@ -1789,7 +1789,7 @@ static int __init gicv3_init(void)
            "      gic_maintenance_irq=%u\n"
            "      gic_rdist_stride=%#x\n"
            "      gic_rdist_regions=%d\n",
-           dbase, gicv3_info.maintenance_irq,
+           dbase_v3, gicv3_info.maintenance_irq,
            gicv3.rdist_stride, gicv3.rdist_count);
     printk("      redistributor regions:\n");
     for ( i = 0; i < gicv3.rdist_count; i++ )
@@ -1803,7 +1803,7 @@ static int __init gicv3_init(void)
     reg = readl_relaxed(GICD + GICD_TYPER);
     intid_bits = GICD_TYPE_ID_BITS(reg);
 
-    vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions, intid_bits);
+    vgic_v3_setup_hw(dbase_v3, gicv3.rdist_count, gicv3.rdist_regions, intid_bits);
     gicv3_init_v2();
 
     spin_lock_init(&gicv3.lock);
-- 
2.7.4


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

Re: [Xen-devel] [RFC 7/7] arm/gic-v3: add GIC version suffix to iomem range variables

[Stefano Stabellini]

On Wed, 6 Nov 2019, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> ARM Compiler 6.6 has a proven bug: static data symbols, moved to an init
> section, becomes global. Thus these symbols clash with ones defined in
> gic-v2.c. The straight forward way to resolve the issue is to add the GIC
> version suffix, at least for one of the conflicting side.
> 
> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>

The patch is acceptable but this seems a very serious compiler bug.
This, together with the other bug described in the previous patch, makes
me think the ARMCC is not quite ready for showtime. Do you know if there
are any later version of the compiler that don't have these problems?

I would hate to introduce these workarounds, especially the one in patch
#6, if they won't be required anymore with the next ARMCC version.



> ---
>  xen/arch/arm/gic-v3.c | 68 +++++++++++++++++++++++++--------------------------
>  1 file changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 0f6cbf6..f57597a 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1328,14 +1328,14 @@ static const hw_irq_controller gicv3_guest_irq_type = {
>      .set_affinity = gicv3_irq_set_affinity,
>  };
>  
> -static paddr_t __initdata dbase = INVALID_PADDR;
> -static paddr_t __initdata vbase = INVALID_PADDR, vsize = 0;
> -static paddr_t __initdata cbase = INVALID_PADDR, csize = 0;
> +static paddr_t __initdata dbase_v3 = INVALID_PADDR;
> +static paddr_t __initdata vbase_v3 = INVALID_PADDR, vsize_v3 = 0;
> +static paddr_t __initdata cbase_v3 = INVALID_PADDR, csize_v3 = 0;
>  
>  /* If the GICv3 supports GICv2, initialize it */
>  static void __init gicv3_init_v2(void)
>  {
> -    if ( cbase == INVALID_PADDR || vbase == INVALID_PADDR )
> +    if ( cbase_v3 == INVALID_PADDR || vbase_v3 == INVALID_PADDR )
>          return;
>  
>      /*
> @@ -1343,26 +1343,26 @@ static void __init gicv3_init_v2(void)
>       * So only support GICv2 on GICv3 when the virtual CPU interface is
>       * at least GUEST_GICC_SIZE.
>       */
> -    if ( vsize < GUEST_GICC_SIZE )
> +    if ( vsize_v3 < GUEST_GICC_SIZE )
>      {
>          printk(XENLOG_WARNING
>                 "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
>                 "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
> -               vsize, GUEST_GICC_SIZE);
> +               vsize_v3, GUEST_GICC_SIZE);
>          return;
>      }
>  
>      printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase %#"PRIpaddr"\n",
> -           cbase, vbase);
> +           cbase_v3, vbase_v3);
>  
> -    vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
> +    vgic_v2_setup_hw(dbase_v3, cbase_v3, csize_v3, vbase_v3, 0);
>  }
>  
>  static void __init gicv3_ioremap_distributor(paddr_t dist_paddr)
>  {
>      if ( dist_paddr & ~PAGE_MASK )
>          panic("GICv3:  Found unaligned distributor address %"PRIpaddr"\n",
> -              dbase);
> +              dbase_v3);
>  
>      gicv3.map_dbase = ioremap_nocache(dist_paddr, SZ_64K);
>      if ( !gicv3.map_dbase )
> @@ -1375,11 +1375,11 @@ static void __init gicv3_dt_init(void)
>      int res, i;
>      const struct dt_device_node *node = gicv3_info.node;
>  
> -    res = dt_device_get_address(node, 0, &dbase, NULL);
> +    res = dt_device_get_address(node, 0, &dbase_v3, NULL);
>      if ( res )
>          panic("GICv3: Cannot find a valid distributor address\n");
>  
> -    gicv3_ioremap_distributor(dbase);
> +    gicv3_ioremap_distributor(dbase_v3);
>  
>      if ( !dt_property_read_u32(node, "#redistributor-regions",
>                  &gicv3.rdist_count) )
> @@ -1416,10 +1416,10 @@ static void __init gicv3_dt_init(void)
>       * provided.
>       */
>      res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> -                                &cbase, &csize);
> +                                &cbase_v3, &csize_v3);
>      if ( !res )
>          dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> -                              &vbase, &vsize);
> +                              &vbase_v3, &vsize_v3);
>  }
>  
>  static int gicv3_iomem_deny_access(const struct domain *d)
> @@ -1427,7 +1427,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
>      int rc, i;
>      unsigned long mfn, nr;
>  
> -    mfn = dbase >> PAGE_SHIFT;
> +    mfn = dbase_v3 >> PAGE_SHIFT;
>      nr = PFN_UP(SZ_64K);
>      rc = iomem_deny_access(d, mfn, mfn + nr);
>      if ( rc )
> @@ -1446,19 +1446,19 @@ static int gicv3_iomem_deny_access(const struct domain *d)
>              return rc;
>      }
>  
> -    if ( cbase != INVALID_PADDR )
> +    if ( cbase_v3 != INVALID_PADDR )
>      {
> -        mfn = cbase >> PAGE_SHIFT;
> -        nr = PFN_UP(csize);
> +        mfn = cbase_v3 >> PAGE_SHIFT;
> +        nr = PFN_UP(csize_v3);
>          rc = iomem_deny_access(d, mfn, mfn + nr);
>          if ( rc )
>              return rc;
>      }
>  
> -    if ( vbase != INVALID_PADDR )
> +    if ( vbase_v3 != INVALID_PADDR )
>      {
> -        mfn = vbase >> PAGE_SHIFT;
> -        nr = PFN_UP(csize);
> +        mfn = vbase_v3 >> PAGE_SHIFT;
> +        nr = PFN_UP(csize_v3);
>          return iomem_deny_access(d, mfn, mfn + nr);
>      }
>  
> @@ -1564,8 +1564,8 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>      /* Read from APIC table and fill up the GIC variables */
>      if ( !cpu_base_assigned )
>      {
> -        cbase = processor->base_address;
> -        vbase = processor->gicv_base_address;
> +        cbase_v3 = processor->base_address;
> +        vbase_v3 = processor->gicv_base_address;
>          gicv3_info.maintenance_irq = processor->vgic_interrupt;
>  
>          if ( processor->flags & ACPI_MADT_VGIC_IRQ_MODE )
> @@ -1577,8 +1577,8 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>      }
>      else
>      {
> -        if ( cbase != processor->base_address
> -             || vbase != processor->gicv_base_address
> +        if ( cbase_v3 != processor->base_address
> +             || vbase_v3 != processor->gicv_base_address
>               || gicv3_info.maintenance_irq != processor->vgic_interrupt )
>          {
>              printk("GICv3: GICC entries are not same in MADT table\n");
> @@ -1599,7 +1599,7 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
>      if ( BAD_MADT_ENTRY(dist, end) )
>          return -EINVAL;
>  
> -    dbase = dist->base_address;
> +    dbase_v3 = dist->base_address;
>  
>      return 0;
>  }
> @@ -1674,7 +1674,7 @@ static void __init gicv3_acpi_init(void)
>      if ( count <= 0 )
>          panic("GICv3: No valid GICD entries exists\n");
>  
> -    gicv3_ioremap_distributor(dbase);
> +    gicv3_ioremap_distributor(dbase_v3);
>  
>      /* Get number of redistributor */
>      count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
> @@ -1722,15 +1722,15 @@ static void __init gicv3_acpi_init(void)
>       * Also set the size of the GICC and GICV when there base address
>       * is not invalid as those values are not present in ACPI.
>       */
> -    if ( !cbase )
> -        cbase = INVALID_PADDR;
> +    if ( !cbase_v3 )
> +        cbase_v3 = INVALID_PADDR;
>      else
> -        csize = SZ_8K;
> +        csize_v3 = SZ_8K;
>  
> -    if ( !vbase )
> -        vbase = INVALID_PADDR;
> +    if ( !vbase_v3 )
> +        vbase_v3 = INVALID_PADDR;
>      else
> -        vsize = GUEST_GICC_SIZE;
> +        vsize_v3 = GUEST_GICC_SIZE;
>  
>  }
>  #else
> @@ -1789,7 +1789,7 @@ static int __init gicv3_init(void)
>             "      gic_maintenance_irq=%u\n"
>             "      gic_rdist_stride=%#x\n"
>             "      gic_rdist_regions=%d\n",
> -           dbase, gicv3_info.maintenance_irq,
> +           dbase_v3, gicv3_info.maintenance_irq,
>             gicv3.rdist_stride, gicv3.rdist_count);
>      printk("      redistributor regions:\n");
>      for ( i = 0; i < gicv3.rdist_count; i++ )
> @@ -1803,7 +1803,7 @@ static int __init gicv3_init(void)
>      reg = readl_relaxed(GICD + GICD_TYPER);
>      intid_bits = GICD_TYPE_ID_BITS(reg);
>  
> -    vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions, intid_bits);
> +    vgic_v3_setup_hw(dbase_v3, gicv3.rdist_count, gicv3.rdist_regions, intid_bits);
>      gicv3_init_v2();
>  
>      spin_lock_init(&gicv3.lock);
> -- 
> 2.7.4
> 

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

Re: [Xen-devel] [RFC 7/7] arm/gic-v3: add GIC version suffix to iomem range variables

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 1098 bytes --]

On Tue, 12 Nov 2019, 05:59 Stefano Stabellini, <sstabellini@kernel.org>
wrote:

> On Wed, 6 Nov 2019, Andrii Anisov wrote:
> > From: Andrii Anisov <andrii_anisov@epam.com>
> >
> > ARM Compiler 6.6 has a proven bug: static data symbols, moved to an init
> > section, becomes global. Thus these symbols clash with ones defined in
> > gic-v2.c. The straight forward way to resolve the issue is to add the GIC
> > version suffix, at least for one of the conflicting side.
> >
> > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
>
> The patch is acceptable but this seems a very serious compiler bug.
>

I am a bit worried this is not going to prevent introducing any similar bug
in the future. I think, we have a way to enforce uniq symbols (see
CONFIG_UNIQUE_SYMBOLS). Would it work for you here?


This, together with the other bug described in the previous patch, makes
> me think the ARMCC is not quite ready for showtime. Do you know if there
> are any later version of the compiler that don't have these problems?
>

Related to this as this been reported to Arm?

Cheers,

-- 
Julien Grall

[-- Attachment #1.2: Type: text/html, Size: 2038 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Xen-devel] [RFC 7/7] arm/gic-v3: add GIC version suffix to iomem range variables

[Andrii Anisov]

Hello Stefano,

On 11.11.19 22:59, Stefano Stabellini wrote:
> this seems a very serious compiler bug.

Yep.

> This, together with the other bug described in the previous patch, makes
> me think the ARMCC is not quite ready for showtime.

Yet, this particular ARM Compiler version is safety certified and LTS.

> Do you know if there
> are any later version of the compiler that don't have these problems?

I don't know, ARM did not say something special about it. As I know, the reason to take this compiler version was that it is the "latest and greatest" safety certified

> I would hate to introduce these workarounds

I hated finding and publishing these workarounds, but here we are.

The main question here is if XEN needs a tag "Support safety certified compiler" by the cost of accepting such workarounds.
Then discuss how to reduce their stench.

-- 
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [RFC 7/7] arm/gic-v3: add GIC version suffix to iomem range variables

[Stefano Stabellini]

On Thu, 14 Nov 2019, Andrii Anisov wrote:
> Hello Stefano,
> 
> On 11.11.19 22:59, Stefano Stabellini wrote:
> > this seems a very serious compiler bug.
> 
> Yep.
> 
> > This, together with the other bug described in the previous patch, makes
> > me think the ARMCC is not quite ready for showtime.
> 
> Yet, this particular ARM Compiler version is safety certified and LTS.
> 
> > Do you know if there
> > are any later version of the compiler that don't have these problems?
> 
> I don't know, ARM did not say something special about it. As I know, the
> reason to take this compiler version was that it is the "latest and greatest"
> safety certified
> 
> > I would hate to introduce these workarounds
> 
> I hated finding and publishing these workarounds, but here we are.
> 
> The main question here is if XEN needs a tag "Support safety certified
> compiler" by the cost of accepting such workarounds.
> Then discuss how to reduce their stench.

Before we get to that point, maybe we can raise the issue with Arm using
our combined channels. I'll raise it internally at Xilinx, and we could
also discuss it during one of the next FuSa calls (list in CC).

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

Re: [Xen-devel] [RFC 7/7] arm/gic-v3: add GIC version suffix to iomem range variables

[Andrii Anisov]

[-- Attachment #1.1: Type: text/plain, Size: 4863 bytes --]

Hello Stefano,

I suppose, in the discussion with ARM, it might be useful to come with existing support case numbers.
Here they are:

Case ID. sort descending<https://support.developer.arm.com/my-cases/?_ga=2.64479850.476004835.1574326833-426220411.1564048157#> Status. sort descending<https://support.developer.arm.com/my-cases/?_ga=2.64479850.476004835.1574326833-426220411.1564048157#>  Product. sort descending<https://support.developer.arm.com/my-cases/?_ga=2.64479850.476004835.1574326833-426220411.1564048157#> Summary. sort descending<https://support.developer.arm.com/my-cases/?_ga=2.64479850.476004835.1574326833-426220411.1564048157#> Created On. sort descending<https://support.developer.arm.com/my-cases/?_ga=2.64479850.476004835.1574326833-426220411.1564048157#>      Updated on . sort descending<https://support.developer.arm.com/my-cases/?_ga=2.64479850.476004835.1574326833-426220411.1564048157#>     Actions
CAS-138402-Y0Y9C3<https://support.developer.arm.com/case-details/?id=9c2bd9a0-8af1-e911-b862-28187887f93a>      Defect/Enhancement      ARM COMPILER 6  C style shift operators are missed among supported scatter file expressions     Oct-18-2019     Nov-19-2019
CAS-137352-T7F4V1<https://support.developer.arm.com/case-details/?id=bacb8de4-92ea-e911-b862-28187887f93a>      More Information Needed DS-5 ARM COMPILER-6 ULTIMATE FL ARM Linker defined symbols are not counted as referred from a steering file.    Oct-09-2019     Nov-12-2019
CAS-138292-L5S0V0<https://support.developer.arm.com/case-details/?id=c69f223a-ebf0-e911-b862-28187887f93a>      More Information Needed ARM COMPILER 6  Static data symbols, moved to init section, becomes global.     Oct-17-2019     Oct-24-2019
CAS-137357-Z7W3B8<https://support.developer.arm.com/case-details/?id=cd97ea01-97ea-e911-b862-28187887f93a>      Defect/Enhancement      DS-5 ARM COMPILER-6 ULTIMATE FL ARM Compiler 6 compiles data only C file with SoftVFP attribute.        Oct-09-2019     Oct-18-2019
CAS-137359-V7G6W6<https://support.developer.arm.com/case-details/?id=c921919d-97ea-e911-b862-28187887f93a>      Closed  DS-5 ARM COMPILER-6 ULTIMATE FL How to rename sections using ARM Compiler 6 tools?      Oct-09-2019     Oct-11-2019



ANDRII ANISOV

Lead Systems Engineer



Office: +380 44 390 5457<tel:+380%2044%20390%205457> x 66766<tel:66766>   Cell: +380 50 573 8852<tel:+380%2050%20573%208852>   Email: andrii_anisov@epam.com<mailto:andrii_anisov@epam.com>

Kyiv, Ukraine (GMT+3)   epam.com<http://www.epam.com>



CONFIDENTIALITY CAUTION AND DISCLAIMER
This message is intended only for the use of the individual(s) or entity(ies) to which it is addressed and contains information that is legally privileged and confidential. If you are not the intended recipient, or the person responsible for delivering the message to the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. All unintended recipients are obliged to delete this message and destroy any printed copies.

________________________________
From: Stefano Stabellini <sstabellini@kernel.org>
Sent: Thursday, November 21, 2019 12:23 AM
To: Andrii Anisov <andrii.anisov@gmail.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>; Andrii Anisov <Andrii_Anisov@epam.com>; Julien Grall <julien@xen.org>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; fusa-sig@lists.xenproject.org <fusa-sig@lists.xenproject.org>; lars.kurth.xen@gmail.com <lars.kurth.xen@gmail.com>
Subject: Re: [RFC 7/7] arm/gic-v3: add GIC version suffix to iomem range variables

On Thu, 14 Nov 2019, Andrii Anisov wrote:
> Hello Stefano,
>
> On 11.11.19 22:59, Stefano Stabellini wrote:
> > this seems a very serious compiler bug.
>
> Yep.
>
> > This, together with the other bug described in the previous patch, makes
> > me think the ARMCC is not quite ready for showtime.
>
> Yet, this particular ARM Compiler version is safety certified and LTS.
>
> > Do you know if there
> > are any later version of the compiler that don't have these problems?
>
> I don't know, ARM did not say something special about it. As I know, the
> reason to take this compiler version was that it is the "latest and greatest"
> safety certified
>
> > I would hate to introduce these workarounds
>
> I hated finding and publishing these workarounds, but here we are.
>
> The main question here is if XEN needs a tag "Support safety certified
> compiler" by the cost of accepting such workarounds.
> Then discuss how to reduce their stench.

Before we get to that point, maybe we can raise the issue with Arm using
our combined channels. I'll raise it internally at Xilinx, and we could
also discuss it during one of the next FuSa calls (list in CC).

[-- Attachment #1.2: Type: text/html, Size: 21355 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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