[PATCH] Arm64: fix build with gcc 10

[PATCH] Arm64: fix build with gcc 10

[Jan Beulich]

With gcc10 inlining is (no longer?) the default for certain atomics.

Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/arm/arch.mk
+++ b/xen/arch/arm/arch.mk
@@ -12,6 +12,7 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-
 
 CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
 CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
+$(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics)
 
 ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),)
     $(error You must use 'make menuconfig' to enable/disable early printk now)

Re: [PATCH] Arm64: fix build with gcc 10

[Julien Grall]

Hi Jan,

I would suggest: "xen/arm64: Force GCC to always inline generic atomics 
helpers".

On 08/09/2020 13:53, Jan Beulich wrote:
> With gcc10 inlining is (no longer?) the default for certain atomics.

How about the following:

"Recent versions of GCC (at least GCC10) will not inline generic atomics 
helpers. Instead it will expect the software to either link with 
libatomic.so or implement the helpers.

To keep the previous behavior, force GCC to always inline the generic 
atomics helpers.

Long term we probably want to avoid relying on GCC atomics helpers as 
this doesn't allow us to switch between LSE and LL/SC atomic.
"

> 
> Suggested-by: Julien Grall <jgrall@amazon.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/arm/arch.mk
> +++ b/xen/arch/arm/arch.mk
> @@ -12,6 +12,7 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-
>   
>   CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
>   CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
> +$(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics)
>   
>   ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),)
>       $(error You must use 'make menuconfig' to enable/disable early printk now)
> 

-- 
Julien Grall

Re: [PATCH] Arm64: fix build with gcc 10

[Bertrand Marquis]

> On 8 Sep 2020, at 13:53, Jan Beulich <jbeulich@suse.com> wrote:
> 
> With gcc10 inlining is (no longer?) the default for certain atomics.
> 
> Suggested-by: Julien Grall <jgrall@amazon.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> --- a/xen/arch/arm/arch.mk
> +++ b/xen/arch/arm/arch.mk
> @@ -12,6 +12,7 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-
> 
> CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
> CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
> +$(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics)

Why not adding this before with the other “call cc-option” ?

Also it might be a good idea to have a sentence in the commit message with the
error happening when this is not added:
undefined reference to `__aarch64_ldadd4_acq_rel’

Because some might need to backport this to other Xen releases if they switch to
a new compiler (although i could not reproduce that with Xen 4.14).

I tested the patch with Yocto master which is using gcc 10.2 and this works.

Bertrand

> 
> ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),)
>     $(error You must use 'make menuconfig' to enable/disable early printk now)
> 

Re: [PATCH] Arm64: fix build with gcc 10

[Jan Beulich]

On 08.09.2020 15:05, Bertrand Marquis wrote:
>> On 8 Sep 2020, at 13:53, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> With gcc10 inlining is (no longer?) the default for certain atomics.
>>
>> Suggested-by: Julien Grall <jgrall@amazon.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> --- a/xen/arch/arm/arch.mk
>> +++ b/xen/arch/arm/arch.mk
>> @@ -12,6 +12,7 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-
>>
>> CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
>> CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
>> +$(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics)
> 
> Why not adding this before with the other “call cc-option” ?

I elected to group it with the other Arm64 specific ones.

> Also it might be a good idea to have a sentence in the commit message with the
> error happening when this is not added:
> undefined reference to `__aarch64_ldadd4_acq_rel’
> 
> Because some might need to backport this to other Xen releases if they switch to
> a new compiler (although i could not reproduce that with Xen 4.14).

I guess I'll go with Julien's suggested description (with some
minor adjustments perhaps).

Jan

Re: [PATCH] Arm64: fix build with gcc 10

[Jan Beulich]

On 08.09.2020 15:03, Julien Grall wrote:
> I would suggest: "xen/arm64: Force GCC to always inline generic atomics 
> helpers".
> 
> On 08/09/2020 13:53, Jan Beulich wrote:
>> With gcc10 inlining is (no longer?) the default for certain atomics.
> 
> How about the following:
> 
> "Recent versions of GCC (at least GCC10) will not inline generic atomics 
> helpers. Instead it will expect the software to either link with 
> libatomic.so or implement the helpers.
> 
> To keep the previous behavior, force GCC to always inline the generic 
> atomics helpers.
> 
> Long term we probably want to avoid relying on GCC atomics helpers as 
> this doesn't allow us to switch between LSE and LL/SC atomic.
> "

Fine with me; I've edited it further very slightly.

Jan

Re: [PATCH] Arm64: fix build with gcc 10

[Bertrand Marquis]

> On 8 Sep 2020, at 15:08, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 08.09.2020 15:05, Bertrand Marquis wrote:
>>> On 8 Sep 2020, at 13:53, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> With gcc10 inlining is (no longer?) the default for certain atomics.
>>> 
>>> Suggested-by: Julien Grall <jgrall@amazon.com>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> --- a/xen/arch/arm/arch.mk
>>> +++ b/xen/arch/arm/arch.mk
>>> @@ -12,6 +12,7 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-
>>> 
>>> CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
>>> CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
>>> +$(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics)
>> 
>> Why not adding this before with the other “call cc-option” ?
> 
> I elected to group it with the other Arm64 specific ones.

ok

> 
>> Also it might be a good idea to have a sentence in the commit message with the
>> error happening when this is not added:
>> undefined reference to `__aarch64_ldadd4_acq_rel’
>> 
>> Because some might need to backport this to other Xen releases if they switch to
>> a new compiler (although i could not reproduce that with Xen 4.14).
> 
> I guess I'll go with Julien's suggested description (with some
> minor adjustments perhaps).

Ok but please mention the error so that it appear somewhere :-)

Bertrand

> 
> Jan