linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
@ 2022-03-09 15:57 Marc Zyngier
  2022-03-09 16:09 ` James Morse
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Marc Zyngier @ 2022-03-09 15:57 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: kernel-team, James Morse, Nick Desaulniers, Will Deacon, Catalin Marinas

Compiling the arm64 kernel with the BHB workarounds and Clang+LTO
results in a bunch of:

<instantiation>:4:2: error: invalid fixup for movz/movk instruction
 mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3

when compiling arch/arm64/kernel/entry.S, and makes no sense at all.

As it turns out, moving a single include line around makes the
problem disappear. Why, you'd ask? Well, I don't have the faintest
idea, and I'm running out of patience. So make of that what you want.

Cc: James Morse <james.morse@arm.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/arm-smccc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 220c8c60e021..0a341dd9ff61 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -5,7 +5,6 @@
 #ifndef __LINUX_ARM_SMCCC_H
 #define __LINUX_ARM_SMCCC_H
 
-#include <linux/init.h>
 #include <uapi/linux/const.h>
 
 /*
@@ -193,6 +192,7 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/init.h>
 #include <linux/linkage.h>
 #include <linux/types.h>
 
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
  2022-03-09 15:57 [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue Marc Zyngier
@ 2022-03-09 16:09 ` James Morse
  2022-03-09 17:11 ` Nathan Chancellor
  2022-03-09 17:19 ` Robin Murphy
  2 siblings, 0 replies; 8+ messages in thread
From: James Morse @ 2022-03-09 16:09 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel
  Cc: kernel-team, Nick Desaulniers, Will Deacon, Catalin Marinas

Hi Marc,

On 09/03/2022 15:57, Marc Zyngier wrote:
> Compiling the arm64 kernel with the BHB workarounds and Clang+LTO
> results in a bunch of:
> 
> <instantiation>:4:2: error: invalid fixup for movz/movk instruction
>  mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
> 
> when compiling arch/arm64/kernel/entry.S, and makes no sense at all.
> 
> As it turns out, moving a single include line around makes the
> problem disappear. Why, you'd ask? Well, I don't have the faintest
> idea, and I'm running out of patience. So make of that what you want.

Thanks for this - I've been banging my head against the keyboard too.
My attempt is below, maybe that helps the toolchain people pin it down.
Catalin doesn't like it as he sensibly wants to keep the sequences
together.

Acked-by: James Morse <james.morse@arm.com>
Tested-by: James Morse <james.morse@arm.com>


Thanks,

James


Build incantation: make LLVM=1 -s -j 56 LD=ld.lld-11
clang --version:
Debian clang version 12.0.0-++20200929085817+962a247aebb-1~exp1
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

The 'exp' in debian's clang version spooked me, but I can also reproduce this with
debian's clang-11:

make LLVM=1 -s -j 56 LD=ld.lld-11 CC=clang-11

clang-11 --version
Debian clang version 11.0.1-2
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Works, but equally inexplicable.
-----%<------
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 6ebdc0f834a7..fd83baf1c552 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -880,20 +880,6 @@ alternative_cb_end
 #endif /* CONFIG_MITIGATE_SPECTRE_BRANCH_HISTORY */
 	.endm
 -	/* Save/restores x0-x3 to the stack */
-	.macro __mitigate_spectre_bhb_fw
-#ifdef CONFIG_MITIGATE_SPECTRE_BRANCH_HISTORY
-	stp	x0, x1, [sp, #-16]!
-	stp	x2, x3, [sp, #-16]!
-	mov	w0, #ARM_SMCCC_ARCH_WORKAROUND_3
-alternative_cb	smccc_patch_fw_mitigation_conduit
-	nop					// Patched to SMC/HVC #0
-alternative_cb_end
-	ldp	x2, x3, [sp], #16
-	ldp	x0, x1, [sp], #16
-#endif /* CONFIG_MITIGATE_SPECTRE_BRANCH_HISTORY */
-	.endm
-
 	.macro mitigate_spectre_bhb_clear_insn
 #ifdef CONFIG_MITIGATE_SPECTRE_BRANCH_HISTORY
 alternative_cb	spectre_bhb_patch_clearbhb
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 4a3a653df07e..03fbb561dcb3 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -659,6 +659,20 @@ alternative_else_nop_endif
 #define BHB_MITIGATION_FW	2
 #define BHB_MITIGATION_INSN	3
 +	/* Save/restores x0-x3 to the stack */
+	.macro __mitigate_spectre_bhb_fw
+#ifdef CONFIG_MITIGATE_SPECTRE_BRANCH_HISTORY
+	stp	x0, x1, [sp, #-16]!
+	stp	x2, x3, [sp, #-16]!
+	mov	w0, #ARM_SMCCC_ARCH_WORKAROUND_3
+alternative_cb	smccc_patch_fw_mitigation_conduit
+	nop					// Patched to SMC/HVC #0
+alternative_cb_end
+	ldp	x2, x3, [sp], #16
+	ldp	x0, x1, [sp], #16
+#endif /* CONFIG_MITIGATE_SPECTRE_BRANCH_HISTORY */
+	.endm
+
 	.macro tramp_ventry, vector_start, regsize, kpti, bhb
 	.align	7
 1:
-----%<------

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
  2022-03-09 15:57 [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue Marc Zyngier
  2022-03-09 16:09 ` James Morse
@ 2022-03-09 17:11 ` Nathan Chancellor
  2022-03-09 17:50   ` Nathan Chancellor
  2022-03-09 17:19 ` Robin Murphy
  2 siblings, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2022-03-09 17:11 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, kernel-team, James Morse, Nick Desaulniers,
	Will Deacon, Catalin Marinas, llvm

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

On Wed, Mar 09, 2022 at 03:57:16PM +0000, Marc Zyngier wrote:
> Compiling the arm64 kernel with the BHB workarounds and Clang+LTO
> results in a bunch of:
> 
> <instantiation>:4:2: error: invalid fixup for movz/movk instruction
>  mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
> 
> when compiling arch/arm64/kernel/entry.S, and makes no sense at all.
> 
> As it turns out, moving a single include line around makes the
> problem disappear. Why, you'd ask? Well, I don't have the faintest
> idea, and I'm running out of patience. So make of that what you want.
> 
> Cc: James Morse <james.morse@arm.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  include/linux/arm-smccc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 220c8c60e021..0a341dd9ff61 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -5,7 +5,6 @@
>  #ifndef __LINUX_ARM_SMCCC_H
>  #define __LINUX_ARM_SMCCC_H
>  
> -#include <linux/init.h>
>  #include <uapi/linux/const.h>
>  
>  /*
> @@ -193,6 +192,7 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#include <linux/init.h>
>  #include <linux/linkage.h>
>  #include <linux/types.h>
>  
> -- 
> 2.34.1

I am still looking at this but I wanted to post a preliminary finding.
It seems the source of the error is that ARM_SMCCC_ARCH_WORKAROUND_3 is
not getting expanded by the preprocessor. If you preprocess
arch/arm64/kernel/entry.S without LTO then with LTO, it reveals the
attached diff, which shows just "#ARM_SMCCC_ARCH_WORKAROUND_3", rather
than "# (((1) << 31) | ((0) << 30) | (((0) & 0x3F) << 24) | ((0x3fff) & 0xFFFF))".

$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 mrproper defconfig arch/arm64/kernel/entry.s

I'll see if I can figure out what is going wrong in the include chain.

Cheers,
Nathan

[-- Attachment #2: entry.s.diff --]
[-- Type: text/plain, Size: 9597 bytes --]

diff --git a/entry.s.good b/entry.s.bad
index ab98e34..b331cc4 100644
--- a/entry.s.good
+++ b/entry.s.bad
@@ -35,71 +35,17 @@
 # 6 "./include/linux/compiler.h" 2
 # 266 "./include/linux/compiler.h"
 # 1 "./arch/arm64/include/asm/rwonce.h" 1
-# 71 "./arch/arm64/include/asm/rwonce.h"
-# 1 "./include/asm-generic/rwonce.h" 1
-# 72 "./arch/arm64/include/asm/rwonce.h" 2
-# 267 "./include/linux/compiler.h" 2
-# 6 "./include/linux/init.h" 2
-# 1 "./include/linux/types.h" 1
-
-
-
-
-
-# 1 "./include/uapi/linux/types.h" 1
-
-
-
-
-# 1 "./arch/arm64/include/generated/uapi/asm/types.h" 1
-# 1 "./include/uapi/asm-generic/types.h" 1
-
-
-
-
-
-
-# 1 "./include/asm-generic/int-ll64.h" 1
-# 11 "./include/asm-generic/int-ll64.h"
-# 1 "./include/uapi/asm-generic/int-ll64.h" 1
-# 12 "./include/uapi/asm-generic/int-ll64.h"
-# 1 "./arch/arm64/include/uapi/asm/bitsperlong.h" 1
-# 22 "./arch/arm64/include/uapi/asm/bitsperlong.h"
-# 1 "./include/asm-generic/bitsperlong.h" 1
-
-
-
-
-# 1 "./include/uapi/asm-generic/bitsperlong.h" 1
-# 6 "./include/asm-generic/bitsperlong.h" 2
-# 23 "./arch/arm64/include/uapi/asm/bitsperlong.h" 2
-# 13 "./include/uapi/asm-generic/int-ll64.h" 2
-# 12 "./include/asm-generic/int-ll64.h" 2
-# 8 "./include/uapi/asm-generic/types.h" 2
-# 2 "./arch/arm64/include/generated/uapi/asm/types.h" 2
-# 6 "./include/uapi/linux/types.h" 2
-# 7 "./include/linux/types.h" 2
-# 7 "./include/linux/init.h" 2
-# 9 "./include/linux/arm-smccc.h" 2
-# 1 "./include/uapi/linux/const.h" 1
-# 10 "./include/linux/arm-smccc.h" 2
-# 11 "arch/arm64/kernel/entry.S" 2
-
-# 1 "./include/linux/linkage.h" 1
-
-
-
-
-
-# 1 "./include/linux/stringify.h" 1
-# 7 "./include/linux/linkage.h" 2
-# 1 "./include/linux/export.h" 1
-# 8 "./include/linux/linkage.h" 2
-# 1 "./arch/arm64/include/asm/linkage.h" 1
+# 11 "./arch/arm64/include/asm/rwonce.h"
+# 1 "./arch/arm64/include/asm/alternative-macros.h" 1
 
 
 
 
+# 1 "./arch/arm64/include/generated/asm/cpucaps.h" 1
+# 6 "./arch/arm64/include/asm/alternative-macros.h" 2
+# 1 "./arch/arm64/include/asm/insn-def.h" 1
+# 7 "./arch/arm64/include/asm/alternative-macros.h" 2
+# 80 "./arch/arm64/include/asm/alternative-macros.h"
 # 1 "./arch/arm64/include/asm/assembler.h" 1
 # 15 "./arch/arm64/include/asm/assembler.h"
 # 1 "./include/asm-generic/export.h" 1
@@ -141,108 +87,6 @@ __kstrtab_\name:
 
 
 # 1 "./arch/arm64/include/asm/alternative-macros.h" 1
-
-
-
-
-# 1 "./arch/arm64/include/generated/asm/cpucaps.h" 1
-# 6 "./arch/arm64/include/asm/alternative-macros.h" 2
-# 1 "./arch/arm64/include/asm/insn-def.h" 1
-# 7 "./arch/arm64/include/asm/alternative-macros.h" 2
-# 80 "./arch/arm64/include/asm/alternative-macros.h"
-# 1 "./arch/arm64/include/asm/assembler.h" 1
-# 81 "./arch/arm64/include/asm/alternative-macros.h" 2
-
-.macro altinstruction_entry orig_offset alt_offset feature orig_len alt_len
- .word \orig_offset - .
- .word \alt_offset - .
- .hword \feature
- .byte \orig_len
- .byte \alt_len
-.endm
-
-.macro alternative_insn insn1, insn2, cap, enable = 1
- .if \enable
-661: \insn1
-662: .pushsection .altinstructions, "a"
- altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f
- .popsection
- .subsection 1
-663: \insn2
-664: .org . - (664b-663b) + (662b-661b)
- .org . - (662b-661b) + (664b-663b)
- .previous
- .endif
-.endm
-# 126 "./arch/arm64/include/asm/alternative-macros.h"
-.macro alternative_if_not cap
- .set .Lasm_alt_mode, 0
- .pushsection .altinstructions, "a"
- altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
- .popsection
-661:
-.endm
-
-.macro alternative_if cap
- .set .Lasm_alt_mode, 1
- .pushsection .altinstructions, "a"
- altinstruction_entry 663f, 661f, \cap, 664f-663f, 662f-661f
- .popsection
- .subsection 1
- .align 2
-661:
-.endm
-
-.macro alternative_cb cb
- .set .Lasm_alt_mode, 0
- .pushsection .altinstructions, "a"
- altinstruction_entry 661f, \cb, 71, 662f-661f, 0
- .popsection
-661:
-.endm
-
-
-
-
-.macro alternative_else
-662:
- .if .Lasm_alt_mode==0
- .subsection 1
- .else
- .previous
- .endif
-663:
-.endm
-
-
-
-
-.macro alternative_endif
-664:
- .org . - (664b-663b) + (662b-661b)
- .org . - (662b-661b) + (664b-663b)
- .if .Lasm_alt_mode==0
- .previous
- .endif
-.endm
-
-
-
-
-.macro alternative_cb_end
-662:
-.endm
-
-
-
-
-
-
-.macro alternative_else_nop_endif
-alternative_else
- nops (662b-661b) / 4
-alternative_endif
-.endm
 # 6 "./arch/arm64/include/asm/alternative.h" 2
 # 18 "./arch/arm64/include/asm/assembler.h" 2
 # 1 "./arch/arm64/include/asm/asm-bug.h" 1
@@ -305,12 +149,30 @@ alternative_endif
 
 
 # 1 "./include/vdso/const.h" 1
+
+
+
+
+# 1 "./include/uapi/linux/const.h" 1
+# 6 "./include/vdso/const.h" 2
 # 5 "./include/linux/const.h" 2
 # 6 "./include/linux/bits.h" 2
 # 1 "./include/vdso/bits.h" 1
 # 7 "./include/linux/bits.h" 2
-# 13 "./arch/arm64/include/asm/sysreg.h" 2
+# 1 "./arch/arm64/include/uapi/asm/bitsperlong.h" 1
+# 22 "./arch/arm64/include/uapi/asm/bitsperlong.h"
+# 1 "./include/asm-generic/bitsperlong.h" 1
+
+
 
+
+# 1 "./include/uapi/asm-generic/bitsperlong.h" 1
+# 6 "./include/asm-generic/bitsperlong.h" 2
+# 23 "./arch/arm64/include/uapi/asm/bitsperlong.h" 2
+# 8 "./include/linux/bits.h" 2
+# 13 "./arch/arm64/include/asm/sysreg.h" 2
+# 1 "./include/linux/stringify.h" 1
+# 14 "./arch/arm64/include/asm/sysreg.h" 2
 # 1 "./include/linux/kasan-tags.h" 1
 # 15 "./arch/arm64/include/asm/sysreg.h" 2
 
@@ -365,8 +227,35 @@ alternative_endif
 # 2 "./include/uapi/linux/errno.h" 2
 # 6 "./include/linux/errno.h" 2
 # 9 "./arch/arm64/include/asm/debug-monitors.h" 2
+# 1 "./include/linux/types.h" 1
+
+
+
 
 
+# 1 "./include/uapi/linux/types.h" 1
+
+
+
+
+# 1 "./arch/arm64/include/generated/uapi/asm/types.h" 1
+# 1 "./include/uapi/asm-generic/types.h" 1
+
+
+
+
+
+
+# 1 "./include/asm-generic/int-ll64.h" 1
+# 11 "./include/asm-generic/int-ll64.h"
+# 1 "./include/uapi/asm-generic/int-ll64.h" 1
+# 12 "./include/asm-generic/int-ll64.h" 2
+# 8 "./include/uapi/asm-generic/types.h" 2
+# 2 "./arch/arm64/include/generated/uapi/asm/types.h" 2
+# 6 "./include/uapi/linux/types.h" 2
+# 7 "./include/linux/types.h" 2
+# 10 "./arch/arm64/include/asm/debug-monitors.h" 2
+
 # 1 "./arch/arm64/include/asm/esr.h" 1
 # 10 "./arch/arm64/include/asm/esr.h"
 # 1 "./arch/arm64/include/asm/memory.h" 1
@@ -389,6 +278,12 @@ alternative_endif
 # 1 "./arch/arm64/include/asm/insn.h" 1
 # 10 "./arch/arm64/include/asm/insn.h"
 # 1 "./include/linux/build_bug.h" 1
+
+
+
+
+# 1 "./include/linux/compiler.h" 1
+# 6 "./include/linux/build_bug.h" 2
 # 11 "./arch/arm64/include/asm/insn.h" 2
 # 13 "./arch/arm64/include/asm/debug-monitors.h" 2
 # 1 "./arch/arm64/include/asm/ptrace.h" 1
@@ -409,6 +304,9 @@ alternative_endif
 # 26 "./arch/arm64/include/asm/assembler.h" 2
 
 # 1 "./arch/arm64/include/asm/thread_info.h" 1
+# 11 "./arch/arm64/include/asm/thread_info.h"
+# 1 "./include/linux/compiler.h" 1
+# 12 "./arch/arm64/include/asm/thread_info.h" 2
 # 28 "./arch/arm64/include/asm/assembler.h" 2
 
 
@@ -1089,7 +987,7 @@ alternative_cb_end
 
  stp x0, x1, [sp, #-16]!
  stp x2, x3, [sp, #-16]!
- mov w0, #(((1) << 31) | ((0) << 30) | (((0) & 0x3F) << 24) | ((0x3fff) & 0xFFFF))
+ mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
 alternative_cb smccc_patch_fw_mitigation_conduit
  nop
 alternative_cb_end
@@ -1097,6 +995,122 @@ alternative_cb_end
  ldp x0, x1, [sp], #16
 
  .endm
+# 81 "./arch/arm64/include/asm/alternative-macros.h" 2
+
+.macro altinstruction_entry orig_offset alt_offset feature orig_len alt_len
+ .word \orig_offset - .
+ .word \alt_offset - .
+ .hword \feature
+ .byte \orig_len
+ .byte \alt_len
+.endm
+
+.macro alternative_insn insn1, insn2, cap, enable = 1
+ .if \enable
+661: \insn1
+662: .pushsection .altinstructions, "a"
+ altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f
+ .popsection
+ .subsection 1
+663: \insn2
+664: .org . - (664b-663b) + (662b-661b)
+ .org . - (662b-661b) + (664b-663b)
+ .previous
+ .endif
+.endm
+# 126 "./arch/arm64/include/asm/alternative-macros.h"
+.macro alternative_if_not cap
+ .set .Lasm_alt_mode, 0
+ .pushsection .altinstructions, "a"
+ altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
+ .popsection
+661:
+.endm
+
+.macro alternative_if cap
+ .set .Lasm_alt_mode, 1
+ .pushsection .altinstructions, "a"
+ altinstruction_entry 663f, 661f, \cap, 664f-663f, 662f-661f
+ .popsection
+ .subsection 1
+ .align 2
+661:
+.endm
+
+.macro alternative_cb cb
+ .set .Lasm_alt_mode, 0
+ .pushsection .altinstructions, "a"
+ altinstruction_entry 661f, \cb, 71, 662f-661f, 0
+ .popsection
+661:
+.endm
+
+
+
+
+.macro alternative_else
+662:
+ .if .Lasm_alt_mode==0
+ .subsection 1
+ .else
+ .previous
+ .endif
+663:
+.endm
+
+
+
+
+.macro alternative_endif
+664:
+ .org . - (664b-663b) + (662b-661b)
+ .org . - (662b-661b) + (664b-663b)
+ .if .Lasm_alt_mode==0
+ .previous
+ .endif
+.endm
+
+
+
+
+.macro alternative_cb_end
+662:
+.endm
+
+
+
+
+
+
+.macro alternative_else_nop_endif
+alternative_else
+ nops (662b-661b) / 4
+alternative_endif
+.endm
+# 12 "./arch/arm64/include/asm/rwonce.h" 2
+# 71 "./arch/arm64/include/asm/rwonce.h"
+# 1 "./include/asm-generic/rwonce.h" 1
+# 72 "./arch/arm64/include/asm/rwonce.h" 2
+# 267 "./include/linux/compiler.h" 2
+# 6 "./include/linux/init.h" 2
+# 9 "./include/linux/arm-smccc.h" 2
+# 11 "arch/arm64/kernel/entry.S" 2
+
+# 1 "./include/linux/linkage.h" 1
+
+
+
+
+
+
+# 1 "./include/linux/export.h" 1
+# 8 "./include/linux/linkage.h" 2
+# 1 "./arch/arm64/include/asm/linkage.h" 1
+
+
+
+
+# 1 "./arch/arm64/include/asm/assembler.h" 1
 # 6 "./arch/arm64/include/asm/linkage.h" 2
 # 9 "./include/linux/linkage.h" 2
 # 13 "arch/arm64/kernel/entry.S" 2

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
  2022-03-09 15:57 [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue Marc Zyngier
  2022-03-09 16:09 ` James Morse
  2022-03-09 17:11 ` Nathan Chancellor
@ 2022-03-09 17:19 ` Robin Murphy
  2 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2022-03-09 17:19 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel
  Cc: kernel-team, James Morse, Nick Desaulniers, Will Deacon, Catalin Marinas

On 2022-03-09 15:57, Marc Zyngier wrote:
> Compiling the arm64 kernel with the BHB workarounds and Clang+LTO
> results in a bunch of:
> 
> <instantiation>:4:2: error: invalid fixup for movz/movk instruction
>   mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
> 
> when compiling arch/arm64/kernel/entry.S, and makes no sense at all.
> 
> As it turns out, moving a single include line around makes the
> problem disappear. Why, you'd ask? Well, I don't have the faintest
> idea, and I'm running out of patience. So make of that what you want.

Alternatively, this is a perfectly valid cleanup, reducing the scope of 
a header inclusion to the C prototype that actually needs it, thus 
reducing header bloat for assembly files that don't. Which happens to 
also have a pleasant side-effect in avoiding an inexplicable LTO nonsense :D

Even just in terms of that cleanup,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>


FWIW, my gut feeling is that LTO has somehow blurred the lines between 
different expansions of _AC() in ARM_SMCCC_ARCH_WORKAROUND_3 that it 
remembers from other compilation units that it's busy munging together, 
and confused itself.

Robin.

> Cc: James Morse <james.morse@arm.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>   include/linux/arm-smccc.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 220c8c60e021..0a341dd9ff61 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -5,7 +5,6 @@
>   #ifndef __LINUX_ARM_SMCCC_H
>   #define __LINUX_ARM_SMCCC_H
>   
> -#include <linux/init.h>
>   #include <uapi/linux/const.h>
>   
>   /*
> @@ -193,6 +192,7 @@
>   
>   #ifndef __ASSEMBLY__
>   
> +#include <linux/init.h>
>   #include <linux/linkage.h>
>   #include <linux/types.h>
>   

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
  2022-03-09 17:11 ` Nathan Chancellor
@ 2022-03-09 17:50   ` Nathan Chancellor
  2022-03-09 18:26     ` James Morse
  2022-03-09 18:45     ` Catalin Marinas
  0 siblings, 2 replies; 8+ messages in thread
From: Nathan Chancellor @ 2022-03-09 17:50 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, kernel-team, James Morse, Nick Desaulniers,
	Will Deacon, Catalin Marinas, llvm

On Wed, Mar 09, 2022 at 10:11:18AM -0700, Nathan Chancellor wrote:
> On Wed, Mar 09, 2022 at 03:57:16PM +0000, Marc Zyngier wrote:
> > Compiling the arm64 kernel with the BHB workarounds and Clang+LTO
> > results in a bunch of:
> > 
> > <instantiation>:4:2: error: invalid fixup for movz/movk instruction
> >  mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
> > 
> > when compiling arch/arm64/kernel/entry.S, and makes no sense at all.
> > 
> > As it turns out, moving a single include line around makes the
> > problem disappear. Why, you'd ask? Well, I don't have the faintest
> > idea, and I'm running out of patience. So make of that what you want.
> > 
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  include/linux/arm-smccc.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > index 220c8c60e021..0a341dd9ff61 100644
> > --- a/include/linux/arm-smccc.h
> > +++ b/include/linux/arm-smccc.h
> > @@ -5,7 +5,6 @@
> >  #ifndef __LINUX_ARM_SMCCC_H
> >  #define __LINUX_ARM_SMCCC_H
> >  
> > -#include <linux/init.h>
> >  #include <uapi/linux/const.h>
> >  
> >  /*
> > @@ -193,6 +192,7 @@
> >  
> >  #ifndef __ASSEMBLY__
> >  
> > +#include <linux/init.h>
> >  #include <linux/linkage.h>
> >  #include <linux/types.h>
> >  
> > -- 
> > 2.34.1
> 
> I am still looking at this but I wanted to post a preliminary finding.
> It seems the source of the error is that ARM_SMCCC_ARCH_WORKAROUND_3 is
> not getting expanded by the preprocessor. If you preprocess
> arch/arm64/kernel/entry.S without LTO then with LTO, it reveals the
> attached diff, which shows just "#ARM_SMCCC_ARCH_WORKAROUND_3", rather
> than "# (((1) << 31) | ((0) << 30) | (((0) & 0x3F) << 24) | ((0x3fff) & 0xFFFF))".

CONFIG_LTO=y causes the following include chain:

include/linux/arm-smccc.h
include/linux/init.h
include/linux/compiler.h
arch/arm64/include/asm/rwonce.h
arch/arm64/include/asm/alternative-macros.h
arch/arm64/include/asm/assembler.h

assembler.h has the use of ARM_SMCCC_ARCH_WORKAROUND_3 but the init.h
include in arm-smccc.h happens before the definition, so it does not get
expanded. This completely hacky patch proves that, as the error goes
away with it:

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 220c8c60e021..6cf6bcf00647 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -5,7 +5,6 @@
 #ifndef __LINUX_ARM_SMCCC_H
 #define __LINUX_ARM_SMCCC_H
 
-#include <linux/init.h>
 #include <uapi/linux/const.h>
 
 /*
@@ -97,6 +96,8 @@
 			   ARM_SMCCC_SMC_32,				\
 			   0, 0x3fff)
 
+#include <linux/init.h>
+
 #define ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID				\
 	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,				\
 			   ARM_SMCCC_SMC_32,				\

This diff seems like a somewhat proper solution, as __READ_ONCE() cannot
be used in assembly, but I am open to other suggestions.

diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
index 1bce62fa908a..56f7b1d4d54b 100644
--- a/arch/arm64/include/asm/rwonce.h
+++ b/arch/arm64/include/asm/rwonce.h
@@ -5,7 +5,7 @@
 #ifndef __ASM_RWONCE_H
 #define __ASM_RWONCE_H
 
-#ifdef CONFIG_LTO
+#if defined(CONFIG_LTO) && !defined(__ASSEMBLY__)
 
 #include <linux/compiler_types.h>
 #include <asm/alternative-macros.h>
@@ -66,7 +66,7 @@
 })
 
 #endif	/* !BUILD_VDSO */
-#endif	/* CONFIG_LTO */
+#endif	/* CONFIG_LTO && !__ASSEMBLY__ */
 
 #include <asm-generic/rwonce.h>
 

Cheers,
Nathan

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
  2022-03-09 17:50   ` Nathan Chancellor
@ 2022-03-09 18:26     ` James Morse
  2022-03-09 18:45     ` Catalin Marinas
  1 sibling, 0 replies; 8+ messages in thread
From: James Morse @ 2022-03-09 18:26 UTC (permalink / raw)
  To: Nathan Chancellor, Marc Zyngier
  Cc: linux-arm-kernel, kernel-team, Nick Desaulniers, Will Deacon,
	Catalin Marinas, llvm

Hi Nathan,

On 09/03/2022 17:50, Nathan Chancellor wrote:
> On Wed, Mar 09, 2022 at 10:11:18AM -0700, Nathan Chancellor wrote:
>> On Wed, Mar 09, 2022 at 03:57:16PM +0000, Marc Zyngier wrote:
>>> Compiling the arm64 kernel with the BHB workarounds and Clang+LTO
>>> results in a bunch of:
>>>
>>> <instantiation>:4:2: error: invalid fixup for movz/movk instruction
>>>  mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
>>>
>>> when compiling arch/arm64/kernel/entry.S, and makes no sense at all.
>>>
>>> As it turns out, moving a single include line around makes the
>>> problem disappear. Why, you'd ask? Well, I don't have the faintest
>>> idea, and I'm running out of patience. So make of that what you want.

[...]

> CONFIG_LTO=y causes the following include chain:
> 
> include/linux/arm-smccc.h
> include/linux/init.h
> include/linux/compiler.h

> arch/arm64/include/asm/rwonce.h

Gah, a header file that does something completely different under LTO!


> arch/arm64/include/asm/alternative-macros.h
> arch/arm64/include/asm/assembler.h
> 
> assembler.h has the use of ARM_SMCCC_ARCH_WORKAROUND_3 but the init.h
> include in arm-smccc.h happens before the definition, so it does not get
> expanded.

Thanks for getting to the bottom of this! The error message had me stumped.

...

> This diff seems like a somewhat proper solution, as __READ_ONCE() cannot
> be used in assembly, but I am open to other suggestions.
> 
> diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
> index 1bce62fa908a..56f7b1d4d54b 100644
> --- a/arch/arm64/include/asm/rwonce.h
> +++ b/arch/arm64/include/asm/rwonce.h
> @@ -5,7 +5,7 @@
>  #ifndef __ASM_RWONCE_H
>  #define __ASM_RWONCE_H
>  
> -#ifdef CONFIG_LTO
> +#if defined(CONFIG_LTO) && !defined(__ASSEMBLY__)
>  
>  #include <linux/compiler_types.h>
>  #include <asm/alternative-macros.h>
> @@ -66,7 +66,7 @@
>  })
>  
>  #endif	/* !BUILD_VDSO */
> -#endif	/* CONFIG_LTO */
> +#endif	/* CONFIG_LTO && !__ASSEMBLY__ */
>  
>  #include <asm-generic/rwonce.h>

This looks like the smallest/cleanest fix.

Acked-by: James Morse <james.morse@arm.com>


I guess ultimately we should split things like arm-smccc.h into multiple files...


Thanks,

James




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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
  2022-03-09 17:50   ` Nathan Chancellor
  2022-03-09 18:26     ` James Morse
@ 2022-03-09 18:45     ` Catalin Marinas
  2022-03-09 19:20       ` Nathan Chancellor
  1 sibling, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2022-03-09 18:45 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Marc Zyngier, linux-arm-kernel, kernel-team, James Morse,
	Nick Desaulniers, Will Deacon, llvm

On Wed, Mar 09, 2022 at 10:50:36AM -0700, Nathan Chancellor wrote:
> This diff seems like a somewhat proper solution, as __READ_ONCE() cannot
> be used in assembly, but I am open to other suggestions.
> 
> diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
> index 1bce62fa908a..56f7b1d4d54b 100644
> --- a/arch/arm64/include/asm/rwonce.h
> +++ b/arch/arm64/include/asm/rwonce.h
> @@ -5,7 +5,7 @@
>  #ifndef __ASM_RWONCE_H
>  #define __ASM_RWONCE_H
>  
> -#ifdef CONFIG_LTO
> +#if defined(CONFIG_LTO) && !defined(__ASSEMBLY__)
>  
>  #include <linux/compiler_types.h>
>  #include <asm/alternative-macros.h>
> @@ -66,7 +66,7 @@
>  })
>  
>  #endif	/* !BUILD_VDSO */
> -#endif	/* CONFIG_LTO */
> +#endif	/* CONFIG_LTO && !__ASSEMBLY__ */

Thanks Nathan. Would you please send a proper patch for this? I'd go
with your proposal as it seems more related to LTO than Marc's patch ;).

-- 
Catalin

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
  2022-03-09 18:45     ` Catalin Marinas
@ 2022-03-09 19:20       ` Nathan Chancellor
  0 siblings, 0 replies; 8+ messages in thread
From: Nathan Chancellor @ 2022-03-09 19:20 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Marc Zyngier, linux-arm-kernel, kernel-team, James Morse,
	Nick Desaulniers, Will Deacon, llvm

On Wed, Mar 09, 2022 at 06:45:13PM +0000, Catalin Marinas wrote:
> On Wed, Mar 09, 2022 at 10:50:36AM -0700, Nathan Chancellor wrote:
> > This diff seems like a somewhat proper solution, as __READ_ONCE() cannot
> > be used in assembly, but I am open to other suggestions.
> > 
> > diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
> > index 1bce62fa908a..56f7b1d4d54b 100644
> > --- a/arch/arm64/include/asm/rwonce.h
> > +++ b/arch/arm64/include/asm/rwonce.h
> > @@ -5,7 +5,7 @@
> >  #ifndef __ASM_RWONCE_H
> >  #define __ASM_RWONCE_H
> >  
> > -#ifdef CONFIG_LTO
> > +#if defined(CONFIG_LTO) && !defined(__ASSEMBLY__)
> >  
> >  #include <linux/compiler_types.h>
> >  #include <asm/alternative-macros.h>
> > @@ -66,7 +66,7 @@
> >  })
> >  
> >  #endif	/* !BUILD_VDSO */
> > -#endif	/* CONFIG_LTO */
> > +#endif	/* CONFIG_LTO && !__ASSEMBLY__ */
> 
> Thanks Nathan. Would you please send a proper patch for this? I'd go
> with your proposal as it seems more related to LTO than Marc's patch ;).

Done, thanks Catalin and James for your input!

https://lore.kernel.org/r/20220309191633.2307110-1-nathan@kernel.org/

Cheers,
Nathan

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-09 19:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 15:57 [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue Marc Zyngier
2022-03-09 16:09 ` James Morse
2022-03-09 17:11 ` Nathan Chancellor
2022-03-09 17:50   ` Nathan Chancellor
2022-03-09 18:26     ` James Morse
2022-03-09 18:45     ` Catalin Marinas
2022-03-09 19:20       ` Nathan Chancellor
2022-03-09 17:19 ` Robin Murphy

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).