Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Christoffer Dall]

On Sun, Feb 04, 2018 at 09:57:49PM +0100, Arnd Bergmann wrote:
> On Sun, Feb 4, 2018 at 7:45 PM, Christoffer Dall
>  wrote:
> > Hi Arnd,
> >
> > On Fri, Feb 02, 2018 at 04:07:34PM +0100, Arnd Bergmann wrote:
> >> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
> >> statement to allow compilation of a multi-CPU kernel for ARMv6
> >> and older ARMv7-A that don't normally support access to the banked
> >> registers.
> >>
> >> This is considered to be a programming error by the gcc developers
> >> and will no longer work in gcc-8, where we now get a build error:
> >>
> >> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SP_usr'
> >> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,ELR_hyp'
> >> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SP_svc'
> >> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,LR_svc'
> >> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SPSR_svc'
> >> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SP_abt'
> >>
> >> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
> >> we know the functions won't ever be called on pre-ARMv7VE machines.
> >> Unfortunately, older compiler versions (4.8 and earlier) do not understand
> >> that flag, so we still need to keep the asm around.
> >
> > Does "not understand" mean "ignores" or do we get an error?
> 
> We get an error, which is why I used the $(call cc-option) Makefile
> helper to check if the compiler supports it.
> 

Right.

> >> Backporting to stable kernels (4.6+) is needed to allow those to be built
> >> with future compilers as well.
> >
> > This builds on the toolchains I have on my machine, so:
> >
> > Acked-by: Christoffer Dall 
> >
> > Are you applying this via a tree with other fixes or would you like me
> > to carry it in the kvmarm tree?
> 
> Please pick it up in your tree.
> 
Will do.

Thanks,
-Christoffer

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Christoffer Dall]

On Sun, Feb 04, 2018 at 09:57:49PM +0100, Arnd Bergmann wrote:
> On Sun, Feb 4, 2018 at 7:45 PM, Christoffer Dall
>  wrote:
> > Hi Arnd,
> >
> > On Fri, Feb 02, 2018 at 04:07:34PM +0100, Arnd Bergmann wrote:
> >> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
> >> statement to allow compilation of a multi-CPU kernel for ARMv6
> >> and older ARMv7-A that don't normally support access to the banked
> >> registers.
> >>
> >> This is considered to be a programming error by the gcc developers
> >> and will no longer work in gcc-8, where we now get a build error:
> >>
> >> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SP_usr'
> >> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,ELR_hyp'
> >> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SP_svc'
> >> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,LR_svc'
> >> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SPSR_svc'
> >> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
> >> architecture. -- `mrs r3,SP_abt'
> >>
> >> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
> >> we know the functions won't ever be called on pre-ARMv7VE machines.
> >> Unfortunately, older compiler versions (4.8 and earlier) do not understand
> >> that flag, so we still need to keep the asm around.
> >
> > Does "not understand" mean "ignores" or do we get an error?
> 
> We get an error, which is why I used the $(call cc-option) Makefile
> helper to check if the compiler supports it.
> 

Right.

> >> Backporting to stable kernels (4.6+) is needed to allow those to be built
> >> with future compilers as well.
> >
> > This builds on the toolchains I have on my machine, so:
> >
> > Acked-by: Christoffer Dall 
> >
> > Are you applying this via a tree with other fixes or would you like me
> > to carry it in the kvmarm tree?
> 
> Please pick it up in your tree.
> 
Will do.

Thanks,
-Christoffer

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Arnd Bergmann]

On Sun, Feb 4, 2018 at 7:45 PM, Christoffer Dall
 wrote:
> Hi Arnd,
>
> On Fri, Feb 02, 2018 at 04:07:34PM +0100, Arnd Bergmann wrote:
>> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
>> statement to allow compilation of a multi-CPU kernel for ARMv6
>> and older ARMv7-A that don't normally support access to the banked
>> registers.
>>
>> This is considered to be a programming error by the gcc developers
>> and will no longer work in gcc-8, where we now get a build error:
>>
>> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SP_usr'
>> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,ELR_hyp'
>> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SP_svc'
>> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,LR_svc'
>> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SPSR_svc'
>> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SP_abt'
>>
>> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
>> we know the functions won't ever be called on pre-ARMv7VE machines.
>> Unfortunately, older compiler versions (4.8 and earlier) do not understand
>> that flag, so we still need to keep the asm around.
>
> Does "not understand" mean "ignores" or do we get an error?

We get an error, which is why I used the $(call cc-option) Makefile
helper to check if the compiler supports it.

>> Backporting to stable kernels (4.6+) is needed to allow those to be built
>> with future compilers as well.
>
> This builds on the toolchains I have on my machine, so:
>
> Acked-by: Christoffer Dall 
>
> Are you applying this via a tree with other fixes or would you like me
> to carry it in the kvmarm tree?

Please pick it up in your tree.

Thanks,

   Arnd

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Arnd Bergmann]

On Sun, Feb 4, 2018 at 7:45 PM, Christoffer Dall
 wrote:
> Hi Arnd,
>
> On Fri, Feb 02, 2018 at 04:07:34PM +0100, Arnd Bergmann wrote:
>> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
>> statement to allow compilation of a multi-CPU kernel for ARMv6
>> and older ARMv7-A that don't normally support access to the banked
>> registers.
>>
>> This is considered to be a programming error by the gcc developers
>> and will no longer work in gcc-8, where we now get a build error:
>>
>> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SP_usr'
>> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,ELR_hyp'
>> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SP_svc'
>> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,LR_svc'
>> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SPSR_svc'
>> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
>> architecture. -- `mrs r3,SP_abt'
>>
>> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
>> we know the functions won't ever be called on pre-ARMv7VE machines.
>> Unfortunately, older compiler versions (4.8 and earlier) do not understand
>> that flag, so we still need to keep the asm around.
>
> Does "not understand" mean "ignores" or do we get an error?

We get an error, which is why I used the $(call cc-option) Makefile
helper to check if the compiler supports it.

>> Backporting to stable kernels (4.6+) is needed to allow those to be built
>> with future compilers as well.
>
> This builds on the toolchains I have on my machine, so:
>
> Acked-by: Christoffer Dall 
>
> Are you applying this via a tree with other fixes or would you like me
> to carry it in the kvmarm tree?

Please pick it up in your tree.

Thanks,

   Arnd

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Christoffer Dall]

Hi Arnd,

On Fri, Feb 02, 2018 at 04:07:34PM +0100, Arnd Bergmann wrote:
> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
> statement to allow compilation of a multi-CPU kernel for ARMv6
> and older ARMv7-A that don't normally support access to the banked
> registers.
> 
> This is considered to be a programming error by the gcc developers
> and will no longer work in gcc-8, where we now get a build error:
> 
> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SP_usr'
> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,ELR_hyp'
> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SP_svc'
> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,LR_svc'
> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SPSR_svc'
> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SP_abt'
> 
> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
> we know the functions won't ever be called on pre-ARMv7VE machines.
> Unfortunately, older compiler versions (4.8 and earlier) do not understand
> that flag, so we still need to keep the asm around.

Does "not understand" mean "ignores" or do we get an error?

> 
> Backporting to stable kernels (4.6+) is needed to allow those to be built
> with future compilers as well.

This builds on the toolchains I have on my machine, so:

Acked-by: Christoffer Dall 

Are you applying this via a tree with other fixes or would you like me
to carry it in the kvmarm tree?

Thanks,
-Christoffer

> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84129
> Fixes: 33280b4cd1dc ("ARM: KVM: Add banked registers save/restore")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arnd Bergmann 
> ---
>  arch/arm/kvm/hyp/Makefile| 5 +
>  arch/arm/kvm/hyp/banked-sr.c | 4 
>  2 files changed, 9 insertions(+)
> 
> diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
> index 5638ce0c9524..63d6b404d88e 100644
> --- a/arch/arm/kvm/hyp/Makefile
> +++ b/arch/arm/kvm/hyp/Makefile
> @@ -7,6 +7,8 @@ ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
>  
>  KVM=../../../../virt/kvm
>  
> +CFLAGS_ARMV7VE  :=$(call cc-option, -march=armv7ve)
> +
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
> @@ -15,7 +17,10 @@ obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
>  obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += vfp.o
>  obj-$(CONFIG_KVM_ARM_HOST) += banked-sr.o
> +CFLAGS_banked-sr.o  += $(CFLAGS_ARMV7VE)
> +
>  obj-$(CONFIG_KVM_ARM_HOST) += entry.o
>  obj-$(CONFIG_KVM_ARM_HOST) += hyp-entry.o
>  obj-$(CONFIG_KVM_ARM_HOST) += switch.o
> +CFLAGS_switch.o += $(CFLAGS_ARMV7VE)
>  obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
> diff --git a/arch/arm/kvm/hyp/banked-sr.c b/arch/arm/kvm/hyp/banked-sr.c
> index 111bda8cdebd..be4b8b0a40ad 100644
> --- a/arch/arm/kvm/hyp/banked-sr.c
> +++ b/arch/arm/kvm/hyp/banked-sr.c
> @@ -20,6 +20,10 @@
>  
>  #include 
>  
> +/*
> + * gcc before 4.9 doesn't understand -march=armv7ve, so we have to
> + * trick the assembler.
> + */
>  __asm__(".arch_extension virt");
>  
>  void __hyp_text __banked_save_state(struct kvm_cpu_context *ctxt)
> -- 
> 2.9.0
> 

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Christoffer Dall]

Hi Arnd,

On Fri, Feb 02, 2018 at 04:07:34PM +0100, Arnd Bergmann wrote:
> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
> statement to allow compilation of a multi-CPU kernel for ARMv6
> and older ARMv7-A that don't normally support access to the banked
> registers.
> 
> This is considered to be a programming error by the gcc developers
> and will no longer work in gcc-8, where we now get a build error:
> 
> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SP_usr'
> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,ELR_hyp'
> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SP_svc'
> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,LR_svc'
> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SPSR_svc'
> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
> architecture. -- `mrs r3,SP_abt'
> 
> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
> we know the functions won't ever be called on pre-ARMv7VE machines.
> Unfortunately, older compiler versions (4.8 and earlier) do not understand
> that flag, so we still need to keep the asm around.

Does "not understand" mean "ignores" or do we get an error?

> 
> Backporting to stable kernels (4.6+) is needed to allow those to be built
> with future compilers as well.

This builds on the toolchains I have on my machine, so:

Acked-by: Christoffer Dall 

Are you applying this via a tree with other fixes or would you like me
to carry it in the kvmarm tree?

Thanks,
-Christoffer

> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84129
> Fixes: 33280b4cd1dc ("ARM: KVM: Add banked registers save/restore")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arnd Bergmann 
> ---
>  arch/arm/kvm/hyp/Makefile| 5 +
>  arch/arm/kvm/hyp/banked-sr.c | 4 
>  2 files changed, 9 insertions(+)
> 
> diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
> index 5638ce0c9524..63d6b404d88e 100644
> --- a/arch/arm/kvm/hyp/Makefile
> +++ b/arch/arm/kvm/hyp/Makefile
> @@ -7,6 +7,8 @@ ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
>  
>  KVM=../../../../virt/kvm
>  
> +CFLAGS_ARMV7VE  :=$(call cc-option, -march=armv7ve)
> +
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
> @@ -15,7 +17,10 @@ obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
>  obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += vfp.o
>  obj-$(CONFIG_KVM_ARM_HOST) += banked-sr.o
> +CFLAGS_banked-sr.o  += $(CFLAGS_ARMV7VE)
> +
>  obj-$(CONFIG_KVM_ARM_HOST) += entry.o
>  obj-$(CONFIG_KVM_ARM_HOST) += hyp-entry.o
>  obj-$(CONFIG_KVM_ARM_HOST) += switch.o
> +CFLAGS_switch.o += $(CFLAGS_ARMV7VE)
>  obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
> diff --git a/arch/arm/kvm/hyp/banked-sr.c b/arch/arm/kvm/hyp/banked-sr.c
> index 111bda8cdebd..be4b8b0a40ad 100644
> --- a/arch/arm/kvm/hyp/banked-sr.c
> +++ b/arch/arm/kvm/hyp/banked-sr.c
> @@ -20,6 +20,10 @@
>  
>  #include 
>  
> +/*
> + * gcc before 4.9 doesn't understand -march=armv7ve, so we have to
> + * trick the assembler.
> + */
>  __asm__(".arch_extension virt");
>  
>  void __hyp_text __banked_save_state(struct kvm_cpu_context *ctxt)
> -- 
> 2.9.0
> 

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Robin Murphy]

On 02/02/18 16:29, Arnd Bergmann wrote:

On Fri, Feb 2, 2018 at 5:23 PM, Robin Murphy  wrote:

On 02/02/18 15:55, Robin Murphy wrote:


On 02/02/18 15:07, Arnd Bergmann wrote:


In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
statement to allow compilation of a multi-CPU kernel for ARMv6
and older ARMv7-A that don't normally support access to the banked
registers.

This is considered to be a programming error by the gcc developers
and will no longer work in gcc-8, where we now get a build error:

/tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this
architecture. -- `mrs r3,SP_usr'
/tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this
architecture. -- `mrs r3,ELR_hyp'
/tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this
architecture. -- `mrs r3,SP_svc'
/tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this
architecture. -- `mrs r3,LR_svc'
/tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this
architecture. -- `mrs r3,SPSR_svc'
/tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this
architecture. -- `mrs r3,SP_abt'

Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
we know the functions won't ever be called on pre-ARMv7VE machines.
Unfortunately, older compiler versions (4.8 and earlier) do not
understand
that flag, so we still need to keep the asm around.

Backporting to stable kernels (4.6+) is needed to allow those to be built
with future compilers as well.



Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files
already) viable as a possibly cleaner alternative, or is GCC itself now
policing the contents of inline asms?



In fact, looking at the binutils history, any version capable of assembling
this file should understand that (modulo my typo), so hopefully it ought to
be feasible to replace these global asms with assembler flags entirely.


No, this only works for .S files, not .c, since gcc starts the output with
an explicit .arch setting that overrides the command line. I think this
was done intentionally to prevent such a hack from working, and have
more reliable checks on the validity of the assembler instruction in
inline asm statements (which we try to circumvent here).


Ah, I see, that is unfortunate. Thanks for clarifying.

Robin.

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Robin Murphy]

On 02/02/18 16:29, Arnd Bergmann wrote:

On Fri, Feb 2, 2018 at 5:23 PM, Robin Murphy  wrote:

On 02/02/18 15:55, Robin Murphy wrote:


On 02/02/18 15:07, Arnd Bergmann wrote:


In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
statement to allow compilation of a multi-CPU kernel for ARMv6
and older ARMv7-A that don't normally support access to the banked
registers.

This is considered to be a programming error by the gcc developers
and will no longer work in gcc-8, where we now get a build error:

/tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this
architecture. -- `mrs r3,SP_usr'
/tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this
architecture. -- `mrs r3,ELR_hyp'
/tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this
architecture. -- `mrs r3,SP_svc'
/tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this
architecture. -- `mrs r3,LR_svc'
/tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this
architecture. -- `mrs r3,SPSR_svc'
/tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this
architecture. -- `mrs r3,SP_abt'

Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
we know the functions won't ever be called on pre-ARMv7VE machines.
Unfortunately, older compiler versions (4.8 and earlier) do not
understand
that flag, so we still need to keep the asm around.

Backporting to stable kernels (4.6+) is needed to allow those to be built
with future compilers as well.



Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files
already) viable as a possibly cleaner alternative, or is GCC itself now
policing the contents of inline asms?



In fact, looking at the binutils history, any version capable of assembling
this file should understand that (modulo my typo), so hopefully it ought to
be feasible to replace these global asms with assembler flags entirely.


No, this only works for .S files, not .c, since gcc starts the output with
an explicit .arch setting that overrides the command line. I think this
was done intentionally to prevent such a hack from working, and have
more reliable checks on the validity of the assembler instruction in
inline asm statements (which we try to circumvent here).


Ah, I see, that is unfortunate. Thanks for clarifying.

Robin.

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Arnd Bergmann]

On Fri, Feb 2, 2018 at 5:23 PM, Robin Murphy  wrote:
> On 02/02/18 15:55, Robin Murphy wrote:
>>
>> On 02/02/18 15:07, Arnd Bergmann wrote:
>>>
>>> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
>>> statement to allow compilation of a multi-CPU kernel for ARMv6
>>> and older ARMv7-A that don't normally support access to the banked
>>> registers.
>>>
>>> This is considered to be a programming error by the gcc developers
>>> and will no longer work in gcc-8, where we now get a build error:
>>>
>>> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SP_usr'
>>> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,ELR_hyp'
>>> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SP_svc'
>>> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,LR_svc'
>>> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SPSR_svc'
>>> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SP_abt'
>>>
>>> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
>>> we know the functions won't ever be called on pre-ARMv7VE machines.
>>> Unfortunately, older compiler versions (4.8 and earlier) do not
>>> understand
>>> that flag, so we still need to keep the asm around.
>>>
>>> Backporting to stable kernels (4.6+) is needed to allow those to be built
>>> with future compilers as well.
>>
>>
>> Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files
>> already) viable as a possibly cleaner alternative, or is GCC itself now
>> policing the contents of inline asms?
>
>
> In fact, looking at the binutils history, any version capable of assembling
> this file should understand that (modulo my typo), so hopefully it ought to
> be feasible to replace these global asms with assembler flags entirely.

No, this only works for .S files, not .c, since gcc starts the output with
an explicit .arch setting that overrides the command line. I think this
was done intentionally to prevent such a hack from working, and have
more reliable checks on the validity of the assembler instruction in
inline asm statements (which we try to circumvent here).

  Arnd

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Arnd Bergmann]

On Fri, Feb 2, 2018 at 5:23 PM, Robin Murphy  wrote:
> On 02/02/18 15:55, Robin Murphy wrote:
>>
>> On 02/02/18 15:07, Arnd Bergmann wrote:
>>>
>>> In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
>>> statement to allow compilation of a multi-CPU kernel for ARMv6
>>> and older ARMv7-A that don't normally support access to the banked
>>> registers.
>>>
>>> This is considered to be a programming error by the gcc developers
>>> and will no longer work in gcc-8, where we now get a build error:
>>>
>>> /tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SP_usr'
>>> /tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,ELR_hyp'
>>> /tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SP_svc'
>>> /tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,LR_svc'
>>> /tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SPSR_svc'
>>> /tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this
>>> architecture. -- `mrs r3,SP_abt'
>>>
>>> Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
>>> we know the functions won't ever be called on pre-ARMv7VE machines.
>>> Unfortunately, older compiler versions (4.8 and earlier) do not
>>> understand
>>> that flag, so we still need to keep the asm around.
>>>
>>> Backporting to stable kernels (4.6+) is needed to allow those to be built
>>> with future compilers as well.
>>
>>
>> Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files
>> already) viable as a possibly cleaner alternative, or is GCC itself now
>> policing the contents of inline asms?
>
>
> In fact, looking at the binutils history, any version capable of assembling
> this file should understand that (modulo my typo), so hopefully it ought to
> be feasible to replace these global asms with assembler flags entirely.

No, this only works for .S files, not .c, since gcc starts the output with
an explicit .arch setting that overrides the command line. I think this
was done intentionally to prevent such a hack from working, and have
more reliable checks on the validity of the assembler instruction in
inline asm statements (which we try to circumvent here).

  Arnd

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Robin Murphy]

On 02/02/18 15:55, Robin Murphy wrote:

On 02/02/18 15:07, Arnd Bergmann wrote:

In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
statement to allow compilation of a multi-CPU kernel for ARMv6
and older ARMv7-A that don't normally support access to the banked
registers.

This is considered to be a programming error by the gcc developers
and will no longer work in gcc-8, where we now get a build error:

/tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SP_usr'
/tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with 
this architecture. -- `mrs r3,ELR_hyp'
/tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SP_svc'
/tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with 
this architecture. -- `mrs r3,LR_svc'
/tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SPSR_svc'
/tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SP_abt'


Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
we know the functions won't ever be called on pre-ARMv7VE machines.
Unfortunately, older compiler versions (4.8 and earlier) do not 
understand

that flag, so we still need to keep the asm around.

Backporting to stable kernels (4.6+) is needed to allow those to be built
with future compilers as well.


Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files 
already) viable as a possibly cleaner alternative, or is GCC itself now 
policing the contents of inline asms?


In fact, looking at the binutils history, any version capable of 
assembling this file should understand that (modulo my typo), so 
hopefully it ought to be feasible to replace these global asms with 
assembler flags entirely.


Robin.


Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84129
Fixes: 33280b4cd1dc ("ARM: KVM: Add banked registers save/restore")
Cc: sta...@vger.kernel.org
Signed-off-by: Arnd Bergmann 
---
  arch/arm/kvm/hyp/Makefile    | 5 +
  arch/arm/kvm/hyp/banked-sr.c | 4 
  2 files changed, 9 insertions(+)

diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
index 5638ce0c9524..63d6b404d88e 100644
--- a/arch/arm/kvm/hyp/Makefile
+++ b/arch/arm/kvm/hyp/Makefile
@@ -7,6 +7,8 @@ ccflags-y += -fno-stack-protector 
-DDISABLE_BRANCH_PROFILING

  KVM=../../../../virt/kvm
+CFLAGS_ARMV7VE   :=$(call cc-option, -march=armv7ve)
+
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
@@ -15,7 +17,10 @@ obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
  obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += vfp.o
  obj-$(CONFIG_KVM_ARM_HOST) += banked-sr.o
+CFLAGS_banked-sr.o   += $(CFLAGS_ARMV7VE)
+
  obj-$(CONFIG_KVM_ARM_HOST) += entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += hyp-entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += switch.o
+CFLAGS_switch.o   += $(CFLAGS_ARMV7VE)
  obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
diff --git a/arch/arm/kvm/hyp/banked-sr.c b/arch/arm/kvm/hyp/banked-sr.c
index 111bda8cdebd..be4b8b0a40ad 100644
--- a/arch/arm/kvm/hyp/banked-sr.c
+++ b/arch/arm/kvm/hyp/banked-sr.c
@@ -20,6 +20,10 @@
  #include 
+/*
+ * gcc before 4.9 doesn't understand -march=armv7ve, so we have to
+ * trick the assembler.
+ */
  __asm__(".arch_extension virt");


Would it be worth wrapping this in a preprocessor check for compilers 
that won't understand the command-line flag? I believe LLVM tends to 
choke on these global asm statements entirely, so minimising exposure 
might be a good thing to do in general.


Robin.


  void __hyp_text __banked_save_state(struct kvm_cpu_context *ctxt)



___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Robin Murphy]

On 02/02/18 15:55, Robin Murphy wrote:

On 02/02/18 15:07, Arnd Bergmann wrote:

In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
statement to allow compilation of a multi-CPU kernel for ARMv6
and older ARMv7-A that don't normally support access to the banked
registers.

This is considered to be a programming error by the gcc developers
and will no longer work in gcc-8, where we now get a build error:

/tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SP_usr'
/tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with 
this architecture. -- `mrs r3,ELR_hyp'
/tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SP_svc'
/tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with 
this architecture. -- `mrs r3,LR_svc'
/tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SPSR_svc'
/tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with 
this architecture. -- `mrs r3,SP_abt'


Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
we know the functions won't ever be called on pre-ARMv7VE machines.
Unfortunately, older compiler versions (4.8 and earlier) do not 
understand

that flag, so we still need to keep the asm around.

Backporting to stable kernels (4.6+) is needed to allow those to be built
with future compilers as well.


Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files 
already) viable as a possibly cleaner alternative, or is GCC itself now 
policing the contents of inline asms?


In fact, looking at the binutils history, any version capable of 
assembling this file should understand that (modulo my typo), so 
hopefully it ought to be feasible to replace these global asms with 
assembler flags entirely.


Robin.


Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84129
Fixes: 33280b4cd1dc ("ARM: KVM: Add banked registers save/restore")
Cc: sta...@vger.kernel.org
Signed-off-by: Arnd Bergmann 
---
  arch/arm/kvm/hyp/Makefile    | 5 +
  arch/arm/kvm/hyp/banked-sr.c | 4 
  2 files changed, 9 insertions(+)

diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
index 5638ce0c9524..63d6b404d88e 100644
--- a/arch/arm/kvm/hyp/Makefile
+++ b/arch/arm/kvm/hyp/Makefile
@@ -7,6 +7,8 @@ ccflags-y += -fno-stack-protector 
-DDISABLE_BRANCH_PROFILING

  KVM=../../../../virt/kvm
+CFLAGS_ARMV7VE   :=$(call cc-option, -march=armv7ve)
+
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
@@ -15,7 +17,10 @@ obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
  obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += vfp.o
  obj-$(CONFIG_KVM_ARM_HOST) += banked-sr.o
+CFLAGS_banked-sr.o   += $(CFLAGS_ARMV7VE)
+
  obj-$(CONFIG_KVM_ARM_HOST) += entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += hyp-entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += switch.o
+CFLAGS_switch.o   += $(CFLAGS_ARMV7VE)
  obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
diff --git a/arch/arm/kvm/hyp/banked-sr.c b/arch/arm/kvm/hyp/banked-sr.c
index 111bda8cdebd..be4b8b0a40ad 100644
--- a/arch/arm/kvm/hyp/banked-sr.c
+++ b/arch/arm/kvm/hyp/banked-sr.c
@@ -20,6 +20,10 @@
  #include 
+/*
+ * gcc before 4.9 doesn't understand -march=armv7ve, so we have to
+ * trick the assembler.
+ */
  __asm__(".arch_extension virt");


Would it be worth wrapping this in a preprocessor check for compilers 
that won't understand the command-line flag? I believe LLVM tends to 
choke on these global asm statements entirely, so minimising exposure 
might be a good thing to do in general.


Robin.


  void __hyp_text __banked_save_state(struct kvm_cpu_context *ctxt)



___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Robin Murphy]

On 02/02/18 15:07, Arnd Bergmann wrote:

In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
statement to allow compilation of a multi-CPU kernel for ARMv6
and older ARMv7-A that don't normally support access to the banked
registers.

This is considered to be a programming error by the gcc developers
and will no longer work in gcc-8, where we now get a build error:

/tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SP_usr'
/tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
architecture. -- `mrs r3,ELR_hyp'
/tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SP_svc'
/tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
architecture. -- `mrs r3,LR_svc'
/tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SPSR_svc'
/tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SP_abt'

Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
we know the functions won't ever be called on pre-ARMv7VE machines.
Unfortunately, older compiler versions (4.8 and earlier) do not understand
that flag, so we still need to keep the asm around.

Backporting to stable kernels (4.6+) is needed to allow those to be built
with future compilers as well.


Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files 
already) viable as a possibly cleaner alternative, or is GCC itself now 
policing the contents of inline asms?



Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84129
Fixes: 33280b4cd1dc ("ARM: KVM: Add banked registers save/restore")
Cc: sta...@vger.kernel.org
Signed-off-by: Arnd Bergmann 
---
  arch/arm/kvm/hyp/Makefile| 5 +
  arch/arm/kvm/hyp/banked-sr.c | 4 
  2 files changed, 9 insertions(+)

diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
index 5638ce0c9524..63d6b404d88e 100644
--- a/arch/arm/kvm/hyp/Makefile
+++ b/arch/arm/kvm/hyp/Makefile
@@ -7,6 +7,8 @@ ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
  
  KVM=../../../../virt/kvm
  
+CFLAGS_ARMV7VE		   :=$(call cc-option, -march=armv7ve)

+
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
@@ -15,7 +17,10 @@ obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
  obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += vfp.o
  obj-$(CONFIG_KVM_ARM_HOST) += banked-sr.o
+CFLAGS_banked-sr.o+= $(CFLAGS_ARMV7VE)
+
  obj-$(CONFIG_KVM_ARM_HOST) += entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += hyp-entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += switch.o
+CFLAGS_switch.o   += $(CFLAGS_ARMV7VE)
  obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
diff --git a/arch/arm/kvm/hyp/banked-sr.c b/arch/arm/kvm/hyp/banked-sr.c
index 111bda8cdebd..be4b8b0a40ad 100644
--- a/arch/arm/kvm/hyp/banked-sr.c
+++ b/arch/arm/kvm/hyp/banked-sr.c
@@ -20,6 +20,10 @@
  
  #include 
  
+/*

+ * gcc before 4.9 doesn't understand -march=armv7ve, so we have to
+ * trick the assembler.
+ */
  __asm__(".arch_extension virt");


Would it be worth wrapping this in a preprocessor check for compilers 
that won't understand the command-line flag? I believe LLVM tends to 
choke on these global asm statements entirely, so minimising exposure 
might be a good thing to do in general.


Robin.

  
  void __hyp_text __banked_save_state(struct kvm_cpu_context *ctxt)

Re: [PATCH 1/2] ARM: kvm: fix building with gcc-8

[Robin Murphy]

On 02/02/18 15:07, Arnd Bergmann wrote:

In banked-sr.c, we use a top-level '__asm__(".arch_extension virt")'
statement to allow compilation of a multi-CPU kernel for ARMv6
and older ARMv7-A that don't normally support access to the banked
registers.

This is considered to be a programming error by the gcc developers
and will no longer work in gcc-8, where we now get a build error:

/tmp/cc4Qy7GR.s:34: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SP_usr'
/tmp/cc4Qy7GR.s:41: Error: Banked registers are not available with this 
architecture. -- `mrs r3,ELR_hyp'
/tmp/cc4Qy7GR.s:55: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SP_svc'
/tmp/cc4Qy7GR.s:62: Error: Banked registers are not available with this 
architecture. -- `mrs r3,LR_svc'
/tmp/cc4Qy7GR.s:69: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SPSR_svc'
/tmp/cc4Qy7GR.s:76: Error: Banked registers are not available with this 
architecture. -- `mrs r3,SP_abt'

Passign the '-march-armv7ve' flag to gcc works, and is ok here, because
we know the functions won't ever be called on pre-ARMv7VE machines.
Unfortunately, older compiler versions (4.8 and earlier) do not understand
that flag, so we still need to keep the asm around.

Backporting to stable kernels (4.6+) is needed to allow those to be built
with future compilers as well.


Is "-Wa,arch=armv7-a+virt" (as we appear to do for a couple of files 
already) viable as a possibly cleaner alternative, or is GCC itself now 
policing the contents of inline asms?



Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84129
Fixes: 33280b4cd1dc ("ARM: KVM: Add banked registers save/restore")
Cc: sta...@vger.kernel.org
Signed-off-by: Arnd Bergmann 
---
  arch/arm/kvm/hyp/Makefile| 5 +
  arch/arm/kvm/hyp/banked-sr.c | 4 
  2 files changed, 9 insertions(+)

diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
index 5638ce0c9524..63d6b404d88e 100644
--- a/arch/arm/kvm/hyp/Makefile
+++ b/arch/arm/kvm/hyp/Makefile
@@ -7,6 +7,8 @@ ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
  
  KVM=../../../../virt/kvm
  
+CFLAGS_ARMV7VE		   :=$(call cc-option, -march=armv7ve)

+
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
@@ -15,7 +17,10 @@ obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
  obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
  obj-$(CONFIG_KVM_ARM_HOST) += vfp.o
  obj-$(CONFIG_KVM_ARM_HOST) += banked-sr.o
+CFLAGS_banked-sr.o+= $(CFLAGS_ARMV7VE)
+
  obj-$(CONFIG_KVM_ARM_HOST) += entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += hyp-entry.o
  obj-$(CONFIG_KVM_ARM_HOST) += switch.o
+CFLAGS_switch.o   += $(CFLAGS_ARMV7VE)
  obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
diff --git a/arch/arm/kvm/hyp/banked-sr.c b/arch/arm/kvm/hyp/banked-sr.c
index 111bda8cdebd..be4b8b0a40ad 100644
--- a/arch/arm/kvm/hyp/banked-sr.c
+++ b/arch/arm/kvm/hyp/banked-sr.c
@@ -20,6 +20,10 @@
  
  #include 
  
+/*

+ * gcc before 4.9 doesn't understand -march=armv7ve, so we have to
+ * trick the assembler.
+ */
  __asm__(".arch_extension virt");


Would it be worth wrapping this in a preprocessor check for compilers 
that won't understand the command-line flag? I believe LLVM tends to 
choke on these global asm statements entirely, so minimising exposure 
might be a good thing to do in general.


Robin.

  
  void __hyp_text __banked_save_state(struct kvm_cpu_context *ctxt)