| Message ID | 20200218164906.35685-1-vincenzo.frascino@arm.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | dd1f6308b28edf0452dd5dc7877992903ec61e69 |
| Headers | show |
| Series | arm64: lse: Fix LSE atomics with LLVM | expand |
[+Sami] On Tue, Feb 18, 2020 at 04:49:06PM +0000, Vincenzo Frascino wrote: > The introduction of the commit e0d5896bd356cd broke the compilation of > the kernel when the selected compiler is clang and it is used in > combination with "-no-integrated-as". Curious, but have you tested this change with the integrated assembler as well? > This happens because __LSE_PREAMBLE is defined as ".arch armv8-a+lse" > and this overrides the version of the architecture passed via -march > command line to the gas compiler. > > The issue was noticed during the development of pauth on arm64 and an > error example is reported below: > > $ aarch64-none-linux-gnu-as -EL -I ./arch/arm64/include > -I ./arch/arm64/include/generated > -I ./include -I ./include > -I ./arch/arm64/include/uapi > -I ./arch/arm64/include/generated/uapi > -I ./include/uapi -I ./include/generated/uapi > -I ./init -I ./init > -march=armv8.3-a -o init/do_mounts.o > /tmp/do_mounts-d7992a.s > /tmp/do_mounts-d7992a.s: Assembler messages: > /tmp/do_mounts-d7992a.s:1959: Error: selected processor does not support `autiasp' > /tmp/do_mounts-d7992a.s:2021: Error: selected processor does not support `paciasp' > /tmp/do_mounts-d7992a.s:2157: Error: selected processor does not support `autiasp' > /tmp/do_mounts-d7992a.s:2175: Error: selected processor does not support `paciasp' > /tmp/do_mounts-d7992a.s:2494: Error: selected processor does not support `autiasp' > > Fix the issue replacing ".arch armv8-a+lse" with ".arch_extension lse" that does > not override the command line parameter. > > Fixes: e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated assembler") > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Reported-by: Amit Kachhap <Amit.Kachhap@arm.com> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > --- > arch/arm64/include/asm/lse.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h > index d429f7701c36..5d10051c3e62 100644 > --- a/arch/arm64/include/asm/lse.h > +++ b/arch/arm64/include/asm/lse.h > @@ -6,7 +6,7 @@ > > #ifdef CONFIG_ARM64_LSE_ATOMICS > > -#define __LSE_PREAMBLE ".arch armv8-a+lse\n" > +#define __LSE_PREAMBLE ".arch_extension lse\n" I'm ok with this, but Sami assumedly changed this for a reason in e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated assembler"). Sami? Will
Hi Will, On 18/02/2020 16:54, Will Deacon wrote: > [+Sami] > Thanks for this, I forgot to add Sami in Cc. > On Tue, Feb 18, 2020 at 04:49:06PM +0000, Vincenzo Frascino wrote: >> The introduction of the commit e0d5896bd356cd broke the compilation of >> the kernel when the selected compiler is clang and it is used in >> combination with "-no-integrated-as". > > Curious, but have you tested this change with the integrated assembler as > well? > The integrated assembler as far as I am aware cannot assemble the kernel for reasons independent from lse (AS=clang generates a lot of errors). Not sure how Sami is testing it. I would be happy to learn it myself. The default option for clang in the kernel Makefile is "-no-integrated-as" though hence e0d5896bd356cd introduces a regression. >> This happens because __LSE_PREAMBLE is defined as ".arch armv8-a+lse" >> and this overrides the version of the architecture passed via -march >> command line to the gas compiler. >> >> The issue was noticed during the development of pauth on arm64 and an >> error example is reported below: >> >> $ aarch64-none-linux-gnu-as -EL -I ./arch/arm64/include >> -I ./arch/arm64/include/generated >> -I ./include -I ./include >> -I ./arch/arm64/include/uapi >> -I ./arch/arm64/include/generated/uapi >> -I ./include/uapi -I ./include/generated/uapi >> -I ./init -I ./init >> -march=armv8.3-a -o init/do_mounts.o >> /tmp/do_mounts-d7992a.s >> /tmp/do_mounts-d7992a.s: Assembler messages: >> /tmp/do_mounts-d7992a.s:1959: Error: selected processor does not support `autiasp' >> /tmp/do_mounts-d7992a.s:2021: Error: selected processor does not support `paciasp' >> /tmp/do_mounts-d7992a.s:2157: Error: selected processor does not support `autiasp' >> /tmp/do_mounts-d7992a.s:2175: Error: selected processor does not support `paciasp' >> /tmp/do_mounts-d7992a.s:2494: Error: selected processor does not support `autiasp' >> >> Fix the issue replacing ".arch armv8-a+lse" with ".arch_extension lse" that does >> not override the command line parameter. >> >> Fixes: e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated assembler") >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Will Deacon <will@kernel.org> >> Reported-by: Amit Kachhap <Amit.Kachhap@arm.com> >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> >> --- >> arch/arm64/include/asm/lse.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h >> index d429f7701c36..5d10051c3e62 100644 >> --- a/arch/arm64/include/asm/lse.h >> +++ b/arch/arm64/include/asm/lse.h >> @@ -6,7 +6,7 @@ >> >> #ifdef CONFIG_ARM64_LSE_ATOMICS >> >> -#define __LSE_PREAMBLE ".arch armv8-a+lse\n" >> +#define __LSE_PREAMBLE ".arch_extension lse\n" > > I'm ok with this, but Sami assumedly changed this for a reason in > e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated > assembler"). > > Sami? > > Will >
On Tue, Feb 18, 2020 at 8:54 AM Will Deacon <will@kernel.org> wrote: > > -#define __LSE_PREAMBLE ".arch armv8-a+lse\n" > > +#define __LSE_PREAMBLE ".arch_extension lse\n" > > I'm ok with this, but Sami assumedly changed this for a reason in > e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated > assembler"). Correct, I changed this because clang's integrated assembler wasn't happy with .arch_extension lse at the time. However, it looks like current versions of clang don't have this problem, so this change looks good to me. Tested-by: Sami Tolvanen <samitolvanen@google.com> Sami
On Tue, Feb 18, 2020 at 9:43 AM Vincenzo Frascino <vincenzo.frascino@arm.com> wrote: > The integrated assembler as far as I am aware cannot assemble the kernel for > reasons independent from lse (AS=clang generates a lot of errors). Not sure how > Sami is testing it. I would be happy to learn it myself. We use LTO in Android kernels, which flips on Clang's integrated assembler for inline assembly only. Sami
On Tue, Feb 18, 2020 at 10:02:24AM -0800, Sami Tolvanen wrote: > On Tue, Feb 18, 2020 at 8:54 AM Will Deacon <will@kernel.org> wrote: > > > -#define __LSE_PREAMBLE ".arch armv8-a+lse\n" > > > +#define __LSE_PREAMBLE ".arch_extension lse\n" > > > > I'm ok with this, but Sami assumedly changed this for a reason in > > e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated > > assembler"). > > Correct, I changed this because clang's integrated assembler wasn't > happy with .arch_extension lse at the time. However, it looks like > current versions of clang don't have this problem, so this change > looks good to me. > > Tested-by: Sami Tolvanen <samitolvanen@google.com> Cheers, I'll queue this with your tag. Will
Hi Will and Sami, On 18/02/2020 18:05, Will Deacon wrote: > On Tue, Feb 18, 2020 at 10:02:24AM -0800, Sami Tolvanen wrote: >> On Tue, Feb 18, 2020 at 8:54 AM Will Deacon <will@kernel.org> wrote: >>>> -#define __LSE_PREAMBLE ".arch armv8-a+lse\n" >>>> +#define __LSE_PREAMBLE ".arch_extension lse\n" >>> >>> I'm ok with this, but Sami assumedly changed this for a reason in >>> e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated >>> assembler"). >> >> Correct, I changed this because clang's integrated assembler wasn't >> happy with .arch_extension lse at the time. However, it looks like >> current versions of clang don't have this problem, so this change >> looks good to me. >> >> Tested-by: Sami Tolvanen <samitolvanen@google.com> > > Cheers, I'll queue this with your tag. > > Will > Thank you for the quick turn around :)
diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h index d429f7701c36..5d10051c3e62 100644 --- a/arch/arm64/include/asm/lse.h +++ b/arch/arm64/include/asm/lse.h @@ -6,7 +6,7 @@ #ifdef CONFIG_ARM64_LSE_ATOMICS -#define __LSE_PREAMBLE ".arch armv8-a+lse\n" +#define __LSE_PREAMBLE ".arch_extension lse\n" #include <linux/compiler_types.h> #include <linux/export.h>
The introduction of the commit e0d5896bd356cd broke the compilation of the kernel when the selected compiler is clang and it is used in combination with "-no-integrated-as". This happens because __LSE_PREAMBLE is defined as ".arch armv8-a+lse" and this overrides the version of the architecture passed via -march command line to the gas compiler. The issue was noticed during the development of pauth on arm64 and an error example is reported below: $ aarch64-none-linux-gnu-as -EL -I ./arch/arm64/include -I ./arch/arm64/include/generated -I ./include -I ./include -I ./arch/arm64/include/uapi -I ./arch/arm64/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi -I ./init -I ./init -march=armv8.3-a -o init/do_mounts.o /tmp/do_mounts-d7992a.s /tmp/do_mounts-d7992a.s: Assembler messages: /tmp/do_mounts-d7992a.s:1959: Error: selected processor does not support `autiasp' /tmp/do_mounts-d7992a.s:2021: Error: selected processor does not support `paciasp' /tmp/do_mounts-d7992a.s:2157: Error: selected processor does not support `autiasp' /tmp/do_mounts-d7992a.s:2175: Error: selected processor does not support `paciasp' /tmp/do_mounts-d7992a.s:2494: Error: selected processor does not support `autiasp' Fix the issue replacing ".arch armv8-a+lse" with ".arch_extension lse" that does not override the command line parameter. Fixes: e0d5896bd356cd ("arm64: lse: fix LSE atomics with LLVM's integrated assembler") Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Reported-by: Amit Kachhap <Amit.Kachhap@arm.com> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> --- arch/arm64/include/asm/lse.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)