Clang build of arm64 kernel fails

Clang build of arm64 kernel fails

[Andrey Konovalov]

Hi Marc!

I've tried to pull in new upstream commits and the kernel build
started failing for me with the following errors (see below).

It seems that the reason is your commit "arm64: Add
ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support". It seems that Clang
doesn't like 32 bits registers being used in 64 bits build.

Would you mind sending a fix?

Thanks!

arch/arm64/kernel/cpu_errata.c:156:2: error: unknown register name 'r0' in asm
        arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
        ^
./include/linux/arm-smccc.h:276:32: note: expanded from macro
'arm_smccc_1_1_smc'
#define arm_smccc_1_1_smc(...)  __arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__)
                                ^
./include/linux/arm-smccc.h:255:3: note: expanded from macro '__arm_smccc_1_1'
                __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
                ^
./include/linux/arm-smccc.h:240:37: note: expanded from macro '__declare_args'
#define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
                                    ^
./include/linux/arm-smccc.h:239:37: note: expanded from macro '___declare_args'
#define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
                                    ^
<scratch space>:136:1: note: expanded from here
__declare_arg_0
^
./include/linux/arm-smccc.h:197:32: note: expanded from macro '__declare_arg_0'
        register u32           r0 asm("r0") = a0;

Re: Clang build of arm64 kernel fails

[Robin Murphy]

Hi Andrey,

On 28/02/18 19:32, Andrey Konovalov wrote:
> Hi Marc!
> 
> I've tried to pull in new upstream commits and the kernel build
> started failing for me with the following errors (see below).
> 
> It seems that the reason is your commit "arm64: Add
> ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support". It seems that Clang
> doesn't like 32 bits registers being used in 64 bits build.

I'd say this is really a bug in Clang. Architecturally, the register in 
AArch64 state is still named "r0"; "x0"/"w0" are assembler operands 
which additionally encode the size of the corresponding *access* to r0.

I note that GCC's documentation on register variables[1] does just say 
"the name of the register", which implies this code is not incorrect. 
Given that Clang already likes to infer the operand size from the 
argument type in actual inline asms, it seems funny that its register 
allocator should care in this non-instruction context.

> Would you mind sending a fix?

That said, I guess it's a bug we might have to work around anyway. Oh well.

Robin.

[1] 
https://gcc.gnu.org/onlinedocs/gcc/Global-Register-Variables.html#Global-Register-Variables

> 
> Thanks!
> 
> arch/arm64/kernel/cpu_errata.c:156:2: error: unknown register name 'r0' in asm
>          arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
>          ^
> ./include/linux/arm-smccc.h:276:32: note: expanded from macro
> 'arm_smccc_1_1_smc'
> #define arm_smccc_1_1_smc(...)  __arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__)
>                                  ^
> ./include/linux/arm-smccc.h:255:3: note: expanded from macro '__arm_smccc_1_1'
>                  __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
>                  ^
> ./include/linux/arm-smccc.h:240:37: note: expanded from macro '__declare_args'
> #define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
>                                      ^
> ./include/linux/arm-smccc.h:239:37: note: expanded from macro '___declare_args'
> #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
>                                      ^
> <scratch space>:136:1: note: expanded from here
> __declare_arg_0
> ^
> ./include/linux/arm-smccc.h:197:32: note: expanded from macro '__declare_arg_0'
>          register u32           r0 asm("r0") = a0;
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: Clang build of arm64 kernel fails

[Dave Martin]

On Thu, Mar 01, 2018 at 09:45:24AM +0000, Robin Murphy wrote:
> Hi Andrey,
> 
> On 28/02/18 19:32, Andrey Konovalov wrote:
> >Hi Marc!
> >
> >I've tried to pull in new upstream commits and the kernel build
> >started failing for me with the following errors (see below).
> >
> >It seems that the reason is your commit "arm64: Add
> >ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support". It seems that Clang
> >doesn't like 32 bits registers being used in 64 bits build.
> 
> I'd say this is really a bug in Clang. Architecturally, the register in
> AArch64 state is still named "r0"; "x0"/"w0" are assembler operands which
> additionally encode the size of the corresponding *access* to r0.
> 
> I note that GCC's documentation on register variables[1] does just say "the
> name of the register", which implies this code is not incorrect. Given that
> Clang already likes to infer the operand size from the argument type in
> actual inline asms, it seems funny that its register allocator should care
> in this non-instruction context.

+1

rN is perfectly reasonable here and has always been supported by GCC for
AArch64 AFAIK.

(IMHO rN is preferable, because this separates the register allocation
specification from how that register is used to encode the data type --
GCC has no choice about the latter, but using "w"/"x" gives the
misleading impression of control over this.)

> >Would you mind sending a fix?
> 
> That said, I guess it's a bug we might have to work around anyway. Oh well.

It would be preferable to see evidence of the llvm community committing
to fix this before we consider merging a bodge into Linux for it.

Although this one issue is easy-ish to work around, we're on a slippery
slope towards allowing the LLVM and GCC inline assemblers to diverge if
we don't push back on worthless incompatibilities.

Cheers
---Dave

Re: Clang build of arm64 kernel fails

[Marc Zyngier]

Hi Andrey,

On 28/02/18 19:32, Andrey Konovalov wrote:
> Hi Marc!
> 
> I've tried to pull in new upstream commits and the kernel build
> started failing for me with the following errors (see below).
> 
> It seems that the reason is your commit "arm64: Add
> ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support". It seems that Clang
> doesn't like 32 bits registers being used in 64 bits build.

These are not AArch32 registers. They are AArch64 registers that the
compiler specialises to an x (64bit wide) or w (32bit wide) register
depending on the type that is used to define the corresponding variable.
The fact that they are named just as AArch32 is an added compatibility
bonus.

For example:

void foo(void)
{
	register unsigned long reg0 asm("r0") = 0;
	register unsigned int  reg1 asm("r1") = 1;

	asm volatile("hvc #0" : : "r" (reg0), "r" (reg1));
}

results in:

Disassembly of section .text:

0000000000000000 <foo>:
   0:	d2800000 	mov	x0, #0x0                   	// #0
   4:	52800021 	mov	w1, #0x1                   	// #1
   8:	d4000002 	hvc	#0x0
   c:	d65f03c0 	ret

> Would you mind sending a fix?

I can look into it. Is there an equivalent Clang construct that wouldn't
result in a sea of #ifdefs?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: Clang build of arm64 kernel fails

[Andrey Konovalov]

On Thu, Mar 1, 2018 at 11:47 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Hi Andrey,
>
> On 28/02/18 19:32, Andrey Konovalov wrote:
>> Hi Marc!
>>
>> I've tried to pull in new upstream commits and the kernel build
>> started failing for me with the following errors (see below).
>>
>> It seems that the reason is your commit "arm64: Add
>> ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support". It seems that Clang
>> doesn't like 32 bits registers being used in 64 bits build.
>
> These are not AArch32 registers. They are AArch64 registers that the
> compiler specialises to an x (64bit wide) or w (32bit wide) register
> depending on the type that is used to define the corresponding variable.
> The fact that they are named just as AArch32 is an added compatibility
> bonus.
>
> For example:
>
> void foo(void)
> {
>         register unsigned long reg0 asm("r0") = 0;
>         register unsigned int  reg1 asm("r1") = 1;
>
>         asm volatile("hvc #0" : : "r" (reg0), "r" (reg1));
> }
>
> results in:
>
> Disassembly of section .text:
>
> 0000000000000000 <foo>:
>    0:   d2800000        mov     x0, #0x0                        // #0
>    4:   52800021        mov     w1, #0x1                        // #1
>    8:   d4000002        hvc     #0x0
>    c:   d65f03c0        ret

OK, I see, thanks for the explanation! I assumed there was something
wrong with the patch, since before it was applied everything was
compiling fine.

>
>> Would you mind sending a fix?
>
> I can look into it. Is there an equivalent Clang construct that wouldn't
> result in a sea of #ifdefs?

I'm having trouble to find any LLVM documentation related to this.

Kostya and Evgeniy, whom from the LLVM side we can ask about this?

>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny...

Re: Clang build of arm64 kernel fails

[Matthias Kaehlcke]

El Thu, Mar 01, 2018 at 10:31:02AM +0000 Dave Martin ha dit:

> On Thu, Mar 01, 2018 at 09:45:24AM +0000, Robin Murphy wrote:
> > Hi Andrey,
> > 
> > On 28/02/18 19:32, Andrey Konovalov wrote:
> > >Hi Marc!
> > >
> > >I've tried to pull in new upstream commits and the kernel build
> > >started failing for me with the following errors (see below).
> > >
> > >It seems that the reason is your commit "arm64: Add
> > >ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support". It seems that Clang
> > >doesn't like 32 bits registers being used in 64 bits build.
> > 
> > I'd say this is really a bug in Clang. Architecturally, the register in
> > AArch64 state is still named "r0"; "x0"/"w0" are assembler operands which
> > additionally encode the size of the corresponding *access* to r0.
> > 
> > I note that GCC's documentation on register variables[1] does just say "the
> > name of the register", which implies this code is not incorrect. Given that
> > Clang already likes to infer the operand size from the argument type in
> > actual inline asms, it seems funny that its register allocator should care
> > in this non-instruction context.
> 
> +1
> 
> rN is perfectly reasonable here and has always been supported by GCC for
> AArch64 AFAIK.
> 
> (IMHO rN is preferable, because this separates the register allocation
> specification from how that register is used to encode the data type --
> GCC has no choice about the latter, but using "w"/"x" gives the
> misleading impression of control over this.)
> 
> > >Would you mind sending a fix?
> > 
> > That said, I guess it's a bug we might have to work around anyway. Oh well.
> 
> It would be preferable to see evidence of the llvm community committing
> to fix this before we consider merging a bodge into Linux for it.
> 
> Although this one issue is easy-ish to work around, we're on a slippery
> slope towards allowing the LLVM and GCC inline assemblers to diverge if
> we don't push back on worthless incompatibilities.

For the record, a LLVM bug was opened to add support for the rN names
in constraints: https://bugs.llvm.org/show_bug.cgi?id=36862 . I know
Manoj intends to submit a fix once the scope has been clarified.

I plan to send a kernel fix soon, unless someone beats me to it.

Matthias