linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler
@ 2020-08-05 18:19 Sami Tolvanen
  2020-08-05 19:15 ` Nick Desaulniers
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Sami Tolvanen @ 2020-08-05 18:19 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Zhenyu Ye
  Cc: Mark Rutland, Marc Zyngier, Nick Desaulniers, Nathan Chancellor,
	Kees Cook, linux-arm-kernel, linux-kernel, clang-built-linux,
	Sami Tolvanen

Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
LLVM's integrated assembler, because -Wa,-march is only passed to
external assemblers and therefore, the new instructions are not enabled
when IAS is used.

As binutils doesn't support .arch_extension tlb-rmi, this change adds
.arch armv8.4-a to __TLBI_0 and __TLBI_1 to fix the issue with both LLVM
IAS and binutils.

Fixes: 7c78f67e9bd9 ("arm64: enable tlbi range instructions")
Link: https://github.com/ClangBuiltLinux/linux/issues/1106
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 arch/arm64/include/asm/tlbflush.h | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index d493174415db..66c2aab5e9cb 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -16,6 +16,16 @@
 #include <asm/cputype.h>
 #include <asm/mmu.h>
 
+/*
+ * Enable ARMv8.4-TLBI instructions with ARM64_TLB_RANGE. Note that binutils
+ * doesn't support .arch_extension tlb-rmi, so use .arch armv8.4-a instead.
+ */
+#ifdef CONFIG_ARM64_TLB_RANGE
+#define __TLBI_PREAMBLE	".arch armv8.4-a\n"
+#else
+#define __TLBI_PREAMBLE
+#endif
+
 /*
  * Raw TLBI operations.
  *
@@ -28,14 +38,16 @@
  * not. The macros handles invoking the asm with or without the
  * register argument as appropriate.
  */
-#define __TLBI_0(op, arg) asm ("tlbi " #op "\n"				       \
+#define __TLBI_0(op, arg) asm (__TLBI_PREAMBLE				       \
+			       "tlbi " #op "\n"				       \
 		   ALTERNATIVE("nop\n			nop",		       \
 			       "dsb ish\n		tlbi " #op,	       \
 			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
 			       CONFIG_ARM64_WORKAROUND_REPEAT_TLBI)	       \
 			    : : )
 
-#define __TLBI_1(op, arg) asm ("tlbi " #op ", %0\n"			       \
+#define __TLBI_1(op, arg) asm (__TLBI_PREAMBLE				       \
+			       "tlbi " #op ", %0\n"			       \
 		   ALTERNATIVE("nop\n			nop",		       \
 			       "dsb ish\n		tlbi " #op ", %0",     \
 			       ARM64_WORKAROUND_REPEAT_TLBI,		       \

base-commit: 4834ce9d8e074bb7ae197632e0708219b9f389b5
-- 
2.28.0.163.g6104cc2f0b6-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler
  2020-08-05 18:19 [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler Sami Tolvanen
@ 2020-08-05 19:15 ` Nick Desaulniers
  2020-08-06 11:30   ` Catalin Marinas
  2020-08-06  7:17 ` Zhenyu Ye
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Nick Desaulniers @ 2020-08-05 19:15 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: Catalin Marinas, Will Deacon, Zhenyu Ye, Mark Rutland,
	Marc Zyngier, Nathan Chancellor, Kees Cook, Linux ARM, LKML,
	clang-built-linux

On Wed, Aug 5, 2020 at 11:19 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
> LLVM's integrated assembler, because -Wa,-march is only passed to
> external assemblers and therefore, the new instructions are not enabled
> when IAS is used.
>
> As binutils doesn't support .arch_extension tlb-rmi, this change adds
> .arch armv8.4-a to __TLBI_0 and __TLBI_1 to fix the issue with both LLVM
> IAS and binutils.
>
> Fixes: 7c78f67e9bd9 ("arm64: enable tlbi range instructions")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1106
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

I've filed https://sourceware.org/bugzilla/show_bug.cgi?id=26339 to
discuss more with ARM binutils devs about some of the compat issues
around these assembler directives.

> ---
>  arch/arm64/include/asm/tlbflush.h | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index d493174415db..66c2aab5e9cb 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -16,6 +16,16 @@
>  #include <asm/cputype.h>
>  #include <asm/mmu.h>
>
> +/*
> + * Enable ARMv8.4-TLBI instructions with ARM64_TLB_RANGE. Note that binutils
> + * doesn't support .arch_extension tlb-rmi, so use .arch armv8.4-a instead.
> + */
> +#ifdef CONFIG_ARM64_TLB_RANGE
> +#define __TLBI_PREAMBLE        ".arch armv8.4-a\n"
> +#else
> +#define __TLBI_PREAMBLE
> +#endif
> +
>  /*
>   * Raw TLBI operations.
>   *
> @@ -28,14 +38,16 @@
>   * not. The macros handles invoking the asm with or without the
>   * register argument as appropriate.
>   */
> -#define __TLBI_0(op, arg) asm ("tlbi " #op "\n"                                       \
> +#define __TLBI_0(op, arg) asm (__TLBI_PREAMBLE                                \
> +                              "tlbi " #op "\n"                                \
>                    ALTERNATIVE("nop\n                   nop",                  \
>                                "dsb ish\n               tlbi " #op,            \
>                                ARM64_WORKAROUND_REPEAT_TLBI,                   \
>                                CONFIG_ARM64_WORKAROUND_REPEAT_TLBI)            \
>                             : : )
>
> -#define __TLBI_1(op, arg) asm ("tlbi " #op ", %0\n"                           \
> +#define __TLBI_1(op, arg) asm (__TLBI_PREAMBLE                                \
> +                              "tlbi " #op ", %0\n"                            \
>                    ALTERNATIVE("nop\n                   nop",                  \
>                                "dsb ish\n               tlbi " #op ", %0",     \
>                                ARM64_WORKAROUND_REPEAT_TLBI,                   \
>
> base-commit: 4834ce9d8e074bb7ae197632e0708219b9f389b5
> --
> 2.28.0.163.g6104cc2f0b6-goog
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler
  2020-08-05 18:19 [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler Sami Tolvanen
  2020-08-05 19:15 ` Nick Desaulniers
@ 2020-08-06  7:17 ` Zhenyu Ye
  2020-08-06 11:33   ` Catalin Marinas
  2020-08-06 12:01 ` Catalin Marinas
  2020-08-27 20:36 ` [PATCH v2] arm64: use a common .arch preamble for inline assembly Sami Tolvanen
  3 siblings, 1 reply; 10+ messages in thread
From: Zhenyu Ye @ 2020-08-06  7:17 UTC (permalink / raw)
  To: Sami Tolvanen, Catalin Marinas, Will Deacon
  Cc: Mark Rutland, Marc Zyngier, Nick Desaulniers, Nathan Chancellor,
	Kees Cook, linux-arm-kernel, linux-kernel, clang-built-linux

Hi,

On 2020/8/6 2:19, Sami Tolvanen wrote:
> Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
> LLVM's integrated assembler, because -Wa,-march is only passed to
> external assemblers and therefore, the new instructions are not enabled
> when IAS is used.
> 

I have looked through the discussion on Github issues. The best way to
solve this problem is try to pass the "-Wa,-march" parameter to clang
even when IAS is enabled, which may need the cooperation of compilation
tool chains :(

Currently, I think we can solve the problem by passing
the '-march=armv8.4-a' when using the integrated assembler, just like:

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 55bc8546d9c7..e5ce184e98c2 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -91,8 +91,12 @@ KBUILD_CFLAGS += $(branch-prot-flags-y)

 ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
 # make sure to pass the newest target architecture to -march.
+ifneq ($(LLVM),)
+KBUILD_CFLAGS  += -march=armv8.4-a
+else
 KBUILD_CFLAGS  += -Wa,-march=armv8.4-a
 endif
+endif

 ifeq ($(CONFIG_SHADOW_CALL_STACK), y)


No need to worry about that this might generate instructions that are not
supported on older hardware, because the 'TLB range' feature is only
enabled when the hardware support ARMv8.4.

> As binutils doesn't support .arch_extension tlb-rmi, this change adds
> .arch armv8.4-a to __TLBI_0 and __TLBI_1 to fix the issue with both LLVM
> IAS and binutils.
> 
> Fixes: 7c78f67e9bd9 ("arm64: enable tlbi range instructions")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1106
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

Thanks,
Zhenyu


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler
  2020-08-05 19:15 ` Nick Desaulniers
@ 2020-08-06 11:30   ` Catalin Marinas
  0 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2020-08-06 11:30 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Sami Tolvanen, Will Deacon, Zhenyu Ye, Mark Rutland,
	Marc Zyngier, Nathan Chancellor, Kees Cook, Linux ARM, LKML,
	clang-built-linux

On Wed, Aug 05, 2020 at 12:15:54PM -0700, Nick Desaulniers wrote:
> On Wed, Aug 5, 2020 at 11:19 AM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> > Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
> > LLVM's integrated assembler, because -Wa,-march is only passed to
> > external assemblers and therefore, the new instructions are not enabled
> > when IAS is used.
> >
> > As binutils doesn't support .arch_extension tlb-rmi, this change adds
> > .arch armv8.4-a to __TLBI_0 and __TLBI_1 to fix the issue with both LLVM
> > IAS and binutils.
> >
> > Fixes: 7c78f67e9bd9 ("arm64: enable tlbi range instructions")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1106
> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> 
> I've filed https://sourceware.org/bugzilla/show_bug.cgi?id=26339 to
> discuss more with ARM binutils devs about some of the compat issues
> around these assembler directives.

Until we get some alignment between binutils and the LLVM's integrated
assembler, the latter will be officially unsupported by the kernel. It's
just insane to maintain different options for architecture extensions,
e.g. memtag vs mte, armv8.4-a vs tlb-rmi. Even worse, I think you can't
add some .arch_extension in binutils without bumping the .arch version.
So maybe ".arch_extension tlb-rmi" works for the integrated assembler
but, if such option is added to binutils, it would require ".arch
armv8.4-a" as well.

So, please sort it out guys, collaborate between yourselves when
inventing architecture mnemonics so that you are aligned.

We make take the occasional patch to fix the integrated assembler if
it's not intrusive but at some point we may say it's just not supported
and reject the fix. We have a hard line on the compiler not generating
newer than ARMv8.0 instructions (unless they are in the NOP/HINT space),
so we limit newer instructions to (inline) asm. That's why -march
doesn't work, it needs to be -Wa,-march.

-- 
Catalin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler
  2020-08-06  7:17 ` Zhenyu Ye
@ 2020-08-06 11:33   ` Catalin Marinas
  0 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2020-08-06 11:33 UTC (permalink / raw)
  To: Zhenyu Ye
  Cc: Sami Tolvanen, Will Deacon, Mark Rutland, Marc Zyngier,
	Nick Desaulniers, Nathan Chancellor, Kees Cook, linux-arm-kernel,
	linux-kernel, clang-built-linux

On Thu, Aug 06, 2020 at 03:17:40PM +0800, Zhenyu Ye wrote:
> On 2020/8/6 2:19, Sami Tolvanen wrote:
> > Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
> > LLVM's integrated assembler, because -Wa,-march is only passed to
> > external assemblers and therefore, the new instructions are not enabled
> > when IAS is used.
> 
> I have looked through the discussion on Github issues. The best way to
> solve this problem is try to pass the "-Wa,-march" parameter to clang
> even when IAS is enabled, which may need the cooperation of compilation
> tool chains :(
> 
> Currently, I think we can solve the problem by passing
> the '-march=armv8.4-a' when using the integrated assembler, just like:
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 55bc8546d9c7..e5ce184e98c2 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -91,8 +91,12 @@ KBUILD_CFLAGS += $(branch-prot-flags-y)
> 
>  ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
>  # make sure to pass the newest target architecture to -march.
> +ifneq ($(LLVM),)
> +KBUILD_CFLAGS  += -march=armv8.4-a
> +else
>  KBUILD_CFLAGS  += -Wa,-march=armv8.4-a
>  endif
> +endif

No, see my other reply. This only works for .S files. For inline
assembly, passing -march to .c files will make the compiler generate
ARMv8.4 instructions and break the kernel single image that's supposed
to run on ARMv8.0 hardware (even if you don't have any range TLBI
instructions).

-- 
Catalin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler
  2020-08-05 18:19 [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler Sami Tolvanen
  2020-08-05 19:15 ` Nick Desaulniers
  2020-08-06  7:17 ` Zhenyu Ye
@ 2020-08-06 12:01 ` Catalin Marinas
  2020-08-06 19:17   ` Sami Tolvanen
  2020-08-27 20:36 ` [PATCH v2] arm64: use a common .arch preamble for inline assembly Sami Tolvanen
  3 siblings, 1 reply; 10+ messages in thread
From: Catalin Marinas @ 2020-08-06 12:01 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: Will Deacon, Zhenyu Ye, Mark Rutland, Marc Zyngier,
	Nick Desaulniers, Nathan Chancellor, Kees Cook, linux-arm-kernel,
	linux-kernel, clang-built-linux

On Wed, Aug 05, 2020 at 11:19:20AM -0700, Sami Tolvanen wrote:
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index d493174415db..66c2aab5e9cb 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -16,6 +16,16 @@
>  #include <asm/cputype.h>
>  #include <asm/mmu.h>
>  
> +/*
> + * Enable ARMv8.4-TLBI instructions with ARM64_TLB_RANGE. Note that binutils
> + * doesn't support .arch_extension tlb-rmi, so use .arch armv8.4-a instead.
> + */
> +#ifdef CONFIG_ARM64_TLB_RANGE
> +#define __TLBI_PREAMBLE	".arch armv8.4-a\n"
> +#else
> +#define __TLBI_PREAMBLE
> +#endif
> +
>  /*
>   * Raw TLBI operations.
>   *
> @@ -28,14 +38,16 @@
>   * not. The macros handles invoking the asm with or without the
>   * register argument as appropriate.
>   */
> -#define __TLBI_0(op, arg) asm ("tlbi " #op "\n"				       \
> +#define __TLBI_0(op, arg) asm (__TLBI_PREAMBLE				       \
> +			       "tlbi " #op "\n"				       \
>  		   ALTERNATIVE("nop\n			nop",		       \
>  			       "dsb ish\n		tlbi " #op,	       \
>  			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
>  			       CONFIG_ARM64_WORKAROUND_REPEAT_TLBI)	       \
>  			    : : )
>  
> -#define __TLBI_1(op, arg) asm ("tlbi " #op ", %0\n"			       \
> +#define __TLBI_1(op, arg) asm (__TLBI_PREAMBLE				       \
> +			       "tlbi " #op ", %0\n"			       \
>  		   ALTERNATIVE("nop\n			nop",		       \
>  			       "dsb ish\n		tlbi " #op ", %0",     \
>  			       ARM64_WORKAROUND_REPEAT_TLBI,		       \

A potential problem here is that for gas (not sure about the integrated
assembler), .arch overrides any other .arch. So if we end up with two
preambles included in the same generated .S files in the future, it will
lead to some random behaviour.

Does the LLVM integrated assembler have the same behaviour on .arch
overriding a prior .arch?

Maybe a better solution is for all inline asm on arm64 to have a
standard preamble which is the maximum supported architecture version.
We can add individual .arch_extension as those are not overriding.

-- 
Catalin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler
  2020-08-06 12:01 ` Catalin Marinas
@ 2020-08-06 19:17   ` Sami Tolvanen
  0 siblings, 0 replies; 10+ messages in thread
From: Sami Tolvanen @ 2020-08-06 19:17 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Will Deacon, Zhenyu Ye, Mark Rutland, Marc Zyngier,
	Nick Desaulniers, Nathan Chancellor, Kees Cook, linux-arm-kernel,
	linux-kernel, clang-built-linux

On Thu, Aug 06, 2020 at 01:01:09PM +0100, Catalin Marinas wrote:
> On Wed, Aug 05, 2020 at 11:19:20AM -0700, Sami Tolvanen wrote:
> > diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> > index d493174415db..66c2aab5e9cb 100644
> > --- a/arch/arm64/include/asm/tlbflush.h
> > +++ b/arch/arm64/include/asm/tlbflush.h
> > @@ -16,6 +16,16 @@
> >  #include <asm/cputype.h>
> >  #include <asm/mmu.h>
> >  
> > +/*
> > + * Enable ARMv8.4-TLBI instructions with ARM64_TLB_RANGE. Note that binutils
> > + * doesn't support .arch_extension tlb-rmi, so use .arch armv8.4-a instead.
> > + */
> > +#ifdef CONFIG_ARM64_TLB_RANGE
> > +#define __TLBI_PREAMBLE	".arch armv8.4-a\n"
> > +#else
> > +#define __TLBI_PREAMBLE
> > +#endif
> > +
> >  /*
> >   * Raw TLBI operations.
> >   *
> > @@ -28,14 +38,16 @@
> >   * not. The macros handles invoking the asm with or without the
> >   * register argument as appropriate.
> >   */
> > -#define __TLBI_0(op, arg) asm ("tlbi " #op "\n"				       \
> > +#define __TLBI_0(op, arg) asm (__TLBI_PREAMBLE				       \
> > +			       "tlbi " #op "\n"				       \
> >  		   ALTERNATIVE("nop\n			nop",		       \
> >  			       "dsb ish\n		tlbi " #op,	       \
> >  			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
> >  			       CONFIG_ARM64_WORKAROUND_REPEAT_TLBI)	       \
> >  			    : : )
> >  
> > -#define __TLBI_1(op, arg) asm ("tlbi " #op ", %0\n"			       \
> > +#define __TLBI_1(op, arg) asm (__TLBI_PREAMBLE				       \
> > +			       "tlbi " #op ", %0\n"			       \
> >  		   ALTERNATIVE("nop\n			nop",		       \
> >  			       "dsb ish\n		tlbi " #op ", %0",     \
> >  			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
> 
> A potential problem here is that for gas (not sure about the integrated
> assembler), .arch overrides any other .arch. So if we end up with two
> preambles included in the same generated .S files in the future, it will
> lead to some random behaviour.
> 
> Does the LLVM integrated assembler have the same behaviour on .arch
> overriding a prior .arch?

I would assume so, but each inline assembly block is independent in
LLVM, so unless there are .arch changes within the block, that shouldn't
be an issue for the integrated assembler.

> Maybe a better solution is for all inline asm on arm64 to have a
> standard preamble which is the maximum supported architecture version.
> We can add individual .arch_extension as those are not overriding.

Sure, that works. How would you feel about something like this, so we can
keep the preamble in sync with future -Wa,-march changes? I'm not sure if
asm/compiler.h is the correct place for the definition though.

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 55bc8546d9c7..0dd07059beaa 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -82,8 +82,8 @@ endif
 # compiler to generate them and consequently to break the single image contract
 # we pass it only to the assembler. This option is utilized only in case of non
 # integrated assemblers.
-ifneq ($(CONFIG_AS_HAS_ARMV8_4), y)
-branch-prot-flags-$(CONFIG_AS_HAS_PAC) += -Wa,-march=armv8.3-a
+ifeq ($(CONFIG_AS_HAS_PAC), y)
+asm-arch := armv8.3-a
 endif
 endif
 
@@ -91,7 +91,12 @@ KBUILD_CFLAGS += $(branch-prot-flags-y)
 
 ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
 # make sure to pass the newest target architecture to -march.
-KBUILD_CFLAGS	+= -Wa,-march=armv8.4-a
+asm-arch := armv8.4-a
+endif
+
+ifdef asm-arch
+KBUILD_CFLAGS	+= -Wa,-march=$(asm-arch) \
+		   -DARM64_ASM_ARCH='"$(asm-arch)"'
 endif
 
 ifeq ($(CONFIG_SHADOW_CALL_STACK), y)
diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h
index 51a7ce87cdfe..6fb2e6bcc392 100644
--- a/arch/arm64/include/asm/compiler.h
+++ b/arch/arm64/include/asm/compiler.h
@@ -2,6 +2,12 @@
 #ifndef __ASM_COMPILER_H
 #define __ASM_COMPILER_H
 
+#ifdef ARM64_ASM_ARCH
+#define ARM64_ASM_PREAMBLE ".arch " ARM64_ASM_ARCH "\n"
+#else
+#define ARM64_ASM_PREAMBLE
+#endif
+
 /*
  * The EL0/EL1 pointer bits used by a pointer authentication code.
  * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply.
diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index d493174415db..cc3f5a33ff9c 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -28,14 +28,16 @@
  * not. The macros handles invoking the asm with or without the
  * register argument as appropriate.
  */
-#define __TLBI_0(op, arg) asm ("tlbi " #op "\n"				       \
+#define __TLBI_0(op, arg) asm (ARM64_ASM_PREAMBLE			       \
+			       "tlbi " #op "\n"				       \
 		   ALTERNATIVE("nop\n			nop",		       \
 			       "dsb ish\n		tlbi " #op,	       \
 			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
 			       CONFIG_ARM64_WORKAROUND_REPEAT_TLBI)	       \
 			    : : )
 
-#define __TLBI_1(op, arg) asm ("tlbi " #op ", %0\n"			       \
+#define __TLBI_1(op, arg) asm (ARM64_ASM_PREAMBLE			       \
+			       "tlbi " #op ", %0\n"			       \
 		   ALTERNATIVE("nop\n			nop",		       \
 			       "dsb ish\n		tlbi " #op ", %0",     \
 			       ARM64_WORKAROUND_REPEAT_TLBI,		       \

Sami

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] arm64: use a common .arch preamble for inline assembly
  2020-08-05 18:19 [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler Sami Tolvanen
                   ` (2 preceding siblings ...)
  2020-08-06 12:01 ` Catalin Marinas
@ 2020-08-27 20:36 ` Sami Tolvanen
  2020-08-27 20:57   ` Nathan Chancellor
  2020-08-28 10:41   ` Catalin Marinas
  3 siblings, 2 replies; 10+ messages in thread
From: Sami Tolvanen @ 2020-08-27 20:36 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Zhenyu Ye
  Cc: Mark Rutland, Marc Zyngier, Nick Desaulniers, Nathan Chancellor,
	Kees Cook, linux-arm-kernel, linux-kernel, clang-built-linux,
	Sami Tolvanen

Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
LLVM's integrated assembler, because -Wa,-march is only passed to
external assemblers and therefore, the new instructions are not enabled
when IAS is used.

This change adds a common architecture version preamble, which can be
used in inline assembly blocks that contain instructions that require
a newer architecture version, and uses it to fix __TLBI_0 and __TLBI_1
with ARM64_TLB_RANGE.

Fixes: 7c78f67e9bd9 ("arm64: enable tlbi range instructions")
Link: https://github.com/ClangBuiltLinux/linux/issues/1106
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
Changes in v2:
- Switched to a standard preamble for the architecture version.

---
 arch/arm64/Makefile               | 11 ++++++++---
 arch/arm64/include/asm/compiler.h |  6 ++++++
 arch/arm64/include/asm/tlbflush.h |  6 ++++--
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index b45f0124cc16..20ab5c9375a5 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -82,8 +82,8 @@ endif
 # compiler to generate them and consequently to break the single image contract
 # we pass it only to the assembler. This option is utilized only in case of non
 # integrated assemblers.
-ifneq ($(CONFIG_AS_HAS_ARMV8_4), y)
-branch-prot-flags-$(CONFIG_AS_HAS_PAC) += -Wa,-march=armv8.3-a
+ifeq ($(CONFIG_AS_HAS_PAC), y)
+asm-arch := armv8.3-a
 endif
 endif
 
@@ -91,7 +91,12 @@ KBUILD_CFLAGS += $(branch-prot-flags-y)
 
 ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
 # make sure to pass the newest target architecture to -march.
-KBUILD_CFLAGS	+= -Wa,-march=armv8.4-a
+asm-arch := armv8.4-a
+endif
+
+ifdef asm-arch
+KBUILD_CFLAGS	+= -Wa,-march=$(asm-arch) \
+		   -DARM64_ASM_ARCH='"$(asm-arch)"'
 endif
 
 ifeq ($(CONFIG_SHADOW_CALL_STACK), y)
diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h
index 51a7ce87cdfe..6fb2e6bcc392 100644
--- a/arch/arm64/include/asm/compiler.h
+++ b/arch/arm64/include/asm/compiler.h
@@ -2,6 +2,12 @@
 #ifndef __ASM_COMPILER_H
 #define __ASM_COMPILER_H
 
+#ifdef ARM64_ASM_ARCH
+#define ARM64_ASM_PREAMBLE ".arch " ARM64_ASM_ARCH "\n"
+#else
+#define ARM64_ASM_PREAMBLE
+#endif
+
 /*
  * The EL0/EL1 pointer bits used by a pointer authentication code.
  * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply.
diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index d493174415db..cc3f5a33ff9c 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -28,14 +28,16 @@
  * not. The macros handles invoking the asm with or without the
  * register argument as appropriate.
  */
-#define __TLBI_0(op, arg) asm ("tlbi " #op "\n"				       \
+#define __TLBI_0(op, arg) asm (ARM64_ASM_PREAMBLE			       \
+			       "tlbi " #op "\n"				       \
 		   ALTERNATIVE("nop\n			nop",		       \
 			       "dsb ish\n		tlbi " #op,	       \
 			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
 			       CONFIG_ARM64_WORKAROUND_REPEAT_TLBI)	       \
 			    : : )
 
-#define __TLBI_1(op, arg) asm ("tlbi " #op ", %0\n"			       \
+#define __TLBI_1(op, arg) asm (ARM64_ASM_PREAMBLE			       \
+			       "tlbi " #op ", %0\n"			       \
 		   ALTERNATIVE("nop\n			nop",		       \
 			       "dsb ish\n		tlbi " #op ", %0",     \
 			       ARM64_WORKAROUND_REPEAT_TLBI,		       \

base-commit: 15bc20c6af4ceee97a1f90b43c0e386643c071b4
-- 
2.28.0.297.g1956fa8f8d-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: use a common .arch preamble for inline assembly
  2020-08-27 20:36 ` [PATCH v2] arm64: use a common .arch preamble for inline assembly Sami Tolvanen
@ 2020-08-27 20:57   ` Nathan Chancellor
  2020-08-28 10:41   ` Catalin Marinas
  1 sibling, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2020-08-27 20:57 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: Catalin Marinas, Will Deacon, Zhenyu Ye, Mark Rutland,
	Marc Zyngier, Nick Desaulniers, Kees Cook, linux-arm-kernel,
	linux-kernel, clang-built-linux

On Thu, Aug 27, 2020 at 01:36:08PM -0700, Sami Tolvanen wrote:
> Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
> LLVM's integrated assembler, because -Wa,-march is only passed to
> external assemblers and therefore, the new instructions are not enabled
> when IAS is used.
> 
> This change adds a common architecture version preamble, which can be
> used in inline assembly blocks that contain instructions that require
> a newer architecture version, and uses it to fix __TLBI_0 and __TLBI_1
> with ARM64_TLB_RANGE.
> 
> Fixes: 7c78f67e9bd9 ("arm64: enable tlbi range instructions")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1106
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

I have verified that this fixes the build with LLVM_IAS=1. Additionally,
I have booted a kernel with this patch on my Raspberry Pi and saw no
adverse affects through a compilation workload.

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
> Changes in v2:
> - Switched to a standard preamble for the architecture version.
> 
> ---
>  arch/arm64/Makefile               | 11 ++++++++---
>  arch/arm64/include/asm/compiler.h |  6 ++++++
>  arch/arm64/include/asm/tlbflush.h |  6 ++++--
>  3 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index b45f0124cc16..20ab5c9375a5 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -82,8 +82,8 @@ endif
>  # compiler to generate them and consequently to break the single image contract
>  # we pass it only to the assembler. This option is utilized only in case of non
>  # integrated assemblers.
> -ifneq ($(CONFIG_AS_HAS_ARMV8_4), y)
> -branch-prot-flags-$(CONFIG_AS_HAS_PAC) += -Wa,-march=armv8.3-a
> +ifeq ($(CONFIG_AS_HAS_PAC), y)
> +asm-arch := armv8.3-a
>  endif
>  endif
>  
> @@ -91,7 +91,12 @@ KBUILD_CFLAGS += $(branch-prot-flags-y)
>  
>  ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
>  # make sure to pass the newest target architecture to -march.
> -KBUILD_CFLAGS	+= -Wa,-march=armv8.4-a
> +asm-arch := armv8.4-a
> +endif
> +
> +ifdef asm-arch
> +KBUILD_CFLAGS	+= -Wa,-march=$(asm-arch) \
> +		   -DARM64_ASM_ARCH='"$(asm-arch)"'
>  endif
>  
>  ifeq ($(CONFIG_SHADOW_CALL_STACK), y)
> diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h
> index 51a7ce87cdfe..6fb2e6bcc392 100644
> --- a/arch/arm64/include/asm/compiler.h
> +++ b/arch/arm64/include/asm/compiler.h
> @@ -2,6 +2,12 @@
>  #ifndef __ASM_COMPILER_H
>  #define __ASM_COMPILER_H
>  
> +#ifdef ARM64_ASM_ARCH
> +#define ARM64_ASM_PREAMBLE ".arch " ARM64_ASM_ARCH "\n"
> +#else
> +#define ARM64_ASM_PREAMBLE
> +#endif
> +
>  /*
>   * The EL0/EL1 pointer bits used by a pointer authentication code.
>   * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply.
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index d493174415db..cc3f5a33ff9c 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -28,14 +28,16 @@
>   * not. The macros handles invoking the asm with or without the
>   * register argument as appropriate.
>   */
> -#define __TLBI_0(op, arg) asm ("tlbi " #op "\n"				       \
> +#define __TLBI_0(op, arg) asm (ARM64_ASM_PREAMBLE			       \
> +			       "tlbi " #op "\n"				       \
>  		   ALTERNATIVE("nop\n			nop",		       \
>  			       "dsb ish\n		tlbi " #op,	       \
>  			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
>  			       CONFIG_ARM64_WORKAROUND_REPEAT_TLBI)	       \
>  			    : : )
>  
> -#define __TLBI_1(op, arg) asm ("tlbi " #op ", %0\n"			       \
> +#define __TLBI_1(op, arg) asm (ARM64_ASM_PREAMBLE			       \
> +			       "tlbi " #op ", %0\n"			       \
>  		   ALTERNATIVE("nop\n			nop",		       \
>  			       "dsb ish\n		tlbi " #op ", %0",     \
>  			       ARM64_WORKAROUND_REPEAT_TLBI,		       \
> 
> base-commit: 15bc20c6af4ceee97a1f90b43c0e386643c071b4
> -- 
> 2.28.0.297.g1956fa8f8d-goog
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: use a common .arch preamble for inline assembly
  2020-08-27 20:36 ` [PATCH v2] arm64: use a common .arch preamble for inline assembly Sami Tolvanen
  2020-08-27 20:57   ` Nathan Chancellor
@ 2020-08-28 10:41   ` Catalin Marinas
  1 sibling, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2020-08-28 10:41 UTC (permalink / raw)
  To: Sami Tolvanen, Will Deacon, Zhenyu Ye
  Cc: Marc Zyngier, linux-arm-kernel, linux-kernel, Nick Desaulniers,
	clang-built-linux, Mark Rutland, Nathan Chancellor, Kees Cook

On Thu, 27 Aug 2020 13:36:08 -0700, Sami Tolvanen wrote:
> Commit 7c78f67e9bd9 ("arm64: enable tlbi range instructions") breaks
> LLVM's integrated assembler, because -Wa,-march is only passed to
> external assemblers and therefore, the new instructions are not enabled
> when IAS is used.
> 
> This change adds a common architecture version preamble, which can be
> used in inline assembly blocks that contain instructions that require
> a newer architecture version, and uses it to fix __TLBI_0 and __TLBI_1
> with ARM64_TLB_RANGE.

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: use a common .arch preamble for inline assembly
      https://git.kernel.org/arm64/c/1764c3edc668

-- 
Catalin


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-08-28 10:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-05 18:19 [PATCH] arm64: tlb: fix ARM64_TLB_RANGE with LLVM's integrated assembler Sami Tolvanen
2020-08-05 19:15 ` Nick Desaulniers
2020-08-06 11:30   ` Catalin Marinas
2020-08-06  7:17 ` Zhenyu Ye
2020-08-06 11:33   ` Catalin Marinas
2020-08-06 12:01 ` Catalin Marinas
2020-08-06 19:17   ` Sami Tolvanen
2020-08-27 20:36 ` [PATCH v2] arm64: use a common .arch preamble for inline assembly Sami Tolvanen
2020-08-27 20:57   ` Nathan Chancellor
2020-08-28 10:41   ` Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).