* [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-06 7:45 ` Fangrui Song
0 siblings, 0 replies; 26+ messages in thread
From: Fangrui Song @ 2024-02-06 7:45 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, linux-arm-kernel
Cc: Jisheng Zhang, Dave Martin, Ard Biesheuvel, Peter Smith, llvm,
linux-kernel, Fangrui Song
The generic constraint "i" seems to be copied from x86 or arm (and with
a redundant generic operand modifier "c"). It works with -fno-PIE but
not with -fPIE/-fPIC in GCC's aarch64 port.
The machine constraint "S", which denotes a symbol or label reference
with a constant offset, supports PIC and has been available in GCC since
2012 and in Clang since 7.0. However, Clang before 19 does not support
"S" on a symbol with a constant offset [1] (e.g.
`static_key_false(&nf_hooks_needed[pf][hook])` in
include/linux/netfilter.h), so we use "i" as a fallback.
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Fangrui Song <maskray@google.com>
Link: https://github.com/llvm/llvm-project/pull/80255 [1]
---
Changes from
arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
* Use "Si" as Ard suggested to support Clang<19
* Make branch a separate operand
Changes from v1:
* Use asmSymbolicName for readability
---
arch/arm64/include/asm/jump_label.h | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
index 48ddc0f45d22..b7716b215f91 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -15,6 +15,10 @@
#define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
+/*
+ * Prefer the constraint "S" to support PIC with GCC. Clang before 19 does not
+ * support "S" on a symbol with a constant offset, so we use "i" as a fallback.
+ */
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
{
@@ -23,9 +27,9 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
" .pushsection __jump_table, \"aw\" \n\t"
" .align 3 \n\t"
" .long 1b - ., %l[l_yes] - . \n\t"
- " .quad %c0 - . \n\t"
+ " .quad (%[key] - .) + %[bit0] \n\t"
" .popsection \n\t"
- : : "i"(&((char *)key)[branch]) : : l_yes);
+ : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
return false;
l_yes:
@@ -40,9 +44,9 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
" .pushsection __jump_table, \"aw\" \n\t"
" .align 3 \n\t"
" .long 1b - ., %l[l_yes] - . \n\t"
- " .quad %c0 - . \n\t"
+ " .quad (%[key] - .) + %[bit0] \n\t"
" .popsection \n\t"
- : : "i"(&((char *)key)[branch]) : : l_yes);
+ : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
return false;
l_yes:
--
2.43.0.594.gd9cf4e227d-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-06 7:45 ` Fangrui Song
@ 2024-02-09 11:11 ` Mark Rutland
-1 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2024-02-09 11:11 UTC (permalink / raw)
To: Fangrui Song
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Jisheng Zhang,
Dave Martin, Ard Biesheuvel, Peter Smith, llvm, linux-kernel
On Mon, Feb 05, 2024 at 11:45:52PM -0800, Fangrui Song wrote:
> The generic constraint "i" seems to be copied from x86 or arm (and with
> a redundant generic operand modifier "c"). It works with -fno-PIE but
> not with -fPIE/-fPIC in GCC's aarch64 port.
>
> The machine constraint "S", which denotes a symbol or label reference
> with a constant offset, supports PIC and has been available in GCC since
> 2012 and in Clang since 7.0. However, Clang before 19 does not support
> "S" on a symbol with a constant offset [1] (e.g.
> `static_key_false(&nf_hooks_needed[pf][hook])` in
> include/linux/netfilter.h), so we use "i" as a fallback.
>
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Link: https://github.com/llvm/llvm-project/pull/80255 [1]
This looks reasonable to me, and works with the toolchains I've tried, so:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> Changes from
> arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
>
> * Use "Si" as Ard suggested to support Clang<19
> * Make branch a separate operand
>
> Changes from v1:
>
> * Use asmSymbolicName for readability
> ---
> arch/arm64/include/asm/jump_label.h | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
> index 48ddc0f45d22..b7716b215f91 100644
> --- a/arch/arm64/include/asm/jump_label.h
> +++ b/arch/arm64/include/asm/jump_label.h
> @@ -15,6 +15,10 @@
>
> #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
>
> +/*
> + * Prefer the constraint "S" to support PIC with GCC. Clang before 19 does not
> + * support "S" on a symbol with a constant offset, so we use "i" as a fallback.
> + */
> static __always_inline bool arch_static_branch(struct static_key * const key,
> const bool branch)
> {
> @@ -23,9 +27,9 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
> @@ -40,9 +44,9 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
> --
> 2.43.0.594.gd9cf4e227d-goog
>
>
_______________________________________________
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] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-09 11:11 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2024-02-09 11:11 UTC (permalink / raw)
To: Fangrui Song
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Jisheng Zhang,
Dave Martin, Ard Biesheuvel, Peter Smith, llvm, linux-kernel
On Mon, Feb 05, 2024 at 11:45:52PM -0800, Fangrui Song wrote:
> The generic constraint "i" seems to be copied from x86 or arm (and with
> a redundant generic operand modifier "c"). It works with -fno-PIE but
> not with -fPIE/-fPIC in GCC's aarch64 port.
>
> The machine constraint "S", which denotes a symbol or label reference
> with a constant offset, supports PIC and has been available in GCC since
> 2012 and in Clang since 7.0. However, Clang before 19 does not support
> "S" on a symbol with a constant offset [1] (e.g.
> `static_key_false(&nf_hooks_needed[pf][hook])` in
> include/linux/netfilter.h), so we use "i" as a fallback.
>
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Link: https://github.com/llvm/llvm-project/pull/80255 [1]
This looks reasonable to me, and works with the toolchains I've tried, so:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> Changes from
> arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
>
> * Use "Si" as Ard suggested to support Clang<19
> * Make branch a separate operand
>
> Changes from v1:
>
> * Use asmSymbolicName for readability
> ---
> arch/arm64/include/asm/jump_label.h | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
> index 48ddc0f45d22..b7716b215f91 100644
> --- a/arch/arm64/include/asm/jump_label.h
> +++ b/arch/arm64/include/asm/jump_label.h
> @@ -15,6 +15,10 @@
>
> #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
>
> +/*
> + * Prefer the constraint "S" to support PIC with GCC. Clang before 19 does not
> + * support "S" on a symbol with a constant offset, so we use "i" as a fallback.
> + */
> static __always_inline bool arch_static_branch(struct static_key * const key,
> const bool branch)
> {
> @@ -23,9 +27,9 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
> @@ -40,9 +44,9 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
> --
> 2.43.0.594.gd9cf4e227d-goog
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-06 7:45 ` Fangrui Song
@ 2024-02-09 18:31 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2024-02-09 18:31 UTC (permalink / raw)
To: linux-arm-kernel, Catalin Marinas, Fangrui Song
Cc: kernel-team, Will Deacon, Ard Biesheuvel, Peter Smith,
Jisheng Zhang, linux-kernel, llvm, Dave Martin
On Mon, 5 Feb 2024 23:45:52 -0800, Fangrui Song wrote:
> The generic constraint "i" seems to be copied from x86 or arm (and with
> a redundant generic operand modifier "c"). It works with -fno-PIE but
> not with -fPIE/-fPIC in GCC's aarch64 port.
>
> The machine constraint "S", which denotes a symbol or label reference
> with a constant offset, supports PIC and has been available in GCC since
> 2012 and in Clang since 7.0. However, Clang before 19 does not support
> "S" on a symbol with a constant offset [1] (e.g.
> `static_key_false(&nf_hooks_needed[pf][hook])` in
> include/linux/netfilter.h), so we use "i" as a fallback.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] arm64: jump_label: use constraints "Si" instead of "i"
https://git.kernel.org/arm64/c/f9daab0ad01c
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-09 18:31 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2024-02-09 18:31 UTC (permalink / raw)
To: linux-arm-kernel, Catalin Marinas, Fangrui Song
Cc: kernel-team, Will Deacon, Ard Biesheuvel, Peter Smith,
Jisheng Zhang, linux-kernel, llvm, Dave Martin
On Mon, 5 Feb 2024 23:45:52 -0800, Fangrui Song wrote:
> The generic constraint "i" seems to be copied from x86 or arm (and with
> a redundant generic operand modifier "c"). It works with -fno-PIE but
> not with -fPIE/-fPIC in GCC's aarch64 port.
>
> The machine constraint "S", which denotes a symbol or label reference
> with a constant offset, supports PIC and has been available in GCC since
> 2012 and in Clang since 7.0. However, Clang before 19 does not support
> "S" on a symbol with a constant offset [1] (e.g.
> `static_key_false(&nf_hooks_needed[pf][hook])` in
> include/linux/netfilter.h), so we use "i" as a fallback.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] arm64: jump_label: use constraints "Si" instead of "i"
https://git.kernel.org/arm64/c/f9daab0ad01c
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
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] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-06 7:45 ` Fangrui Song
@ 2024-02-19 10:03 ` Geert Uytterhoeven
-1 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2024-02-19 10:03 UTC (permalink / raw)
To: Fangrui Song
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Jisheng Zhang,
Dave Martin, Ard Biesheuvel, Peter Smith, llvm, linux-kernel
Hi Fangrui,
On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> The generic constraint "i" seems to be copied from x86 or arm (and with
> a redundant generic operand modifier "c"). It works with -fno-PIE but
> not with -fPIE/-fPIC in GCC's aarch64 port.
Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> The machine constraint "S", which denotes a symbol or label reference
> with a constant offset, supports PIC and has been available in GCC since
> 2012 and in Clang since 7.0. However, Clang before 19 does not support
> "S" on a symbol with a constant offset [1] (e.g.
> `static_key_false(&nf_hooks_needed[pf][hook])` in
> include/linux/netfilter.h), so we use "i" as a fallback.
https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
i.e. after 2012 ...
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Link: https://github.com/llvm/llvm-project/pull/80255 [1]
>
> ---
> Changes from
> arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
>
> * Use "Si" as Ard suggested to support Clang<19
> * Make branch a separate operand
>
> Changes from v1:
>
> * Use asmSymbolicName for readability
But it still fails on gcc-5:
arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
invalid operand
asm goto(
^
http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> --- a/arch/arm64/include/asm/jump_label.h
> +++ b/arch/arm64/include/asm/jump_label.h
> @@ -15,6 +15,10 @@
>
> #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
>
> +/*
> + * Prefer the constraint "S" to support PIC with GCC. Clang before 19 does not
> + * support "S" on a symbol with a constant offset, so we use "i" as a fallback.
> + */
> static __always_inline bool arch_static_branch(struct static_key * const key,
> const bool branch)
> {
> @@ -23,9 +27,9 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
> @@ -40,9 +44,9 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
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] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-19 10:03 ` Geert Uytterhoeven
0 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2024-02-19 10:03 UTC (permalink / raw)
To: Fangrui Song
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Jisheng Zhang,
Dave Martin, Ard Biesheuvel, Peter Smith, llvm, linux-kernel
Hi Fangrui,
On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> The generic constraint "i" seems to be copied from x86 or arm (and with
> a redundant generic operand modifier "c"). It works with -fno-PIE but
> not with -fPIE/-fPIC in GCC's aarch64 port.
Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> The machine constraint "S", which denotes a symbol or label reference
> with a constant offset, supports PIC and has been available in GCC since
> 2012 and in Clang since 7.0. However, Clang before 19 does not support
> "S" on a symbol with a constant offset [1] (e.g.
> `static_key_false(&nf_hooks_needed[pf][hook])` in
> include/linux/netfilter.h), so we use "i" as a fallback.
https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
i.e. after 2012 ...
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Link: https://github.com/llvm/llvm-project/pull/80255 [1]
>
> ---
> Changes from
> arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
>
> * Use "Si" as Ard suggested to support Clang<19
> * Make branch a separate operand
>
> Changes from v1:
>
> * Use asmSymbolicName for readability
But it still fails on gcc-5:
arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
invalid operand
asm goto(
^
http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> --- a/arch/arm64/include/asm/jump_label.h
> +++ b/arch/arm64/include/asm/jump_label.h
> @@ -15,6 +15,10 @@
>
> #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
>
> +/*
> + * Prefer the constraint "S" to support PIC with GCC. Clang before 19 does not
> + * support "S" on a symbol with a constant offset, so we use "i" as a fallback.
> + */
> static __always_inline bool arch_static_branch(struct static_key * const key,
> const bool branch)
> {
> @@ -23,9 +27,9 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
> @@ -40,9 +44,9 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
> " .pushsection __jump_table, \"aw\" \n\t"
> " .align 3 \n\t"
> " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> + " .quad (%[key] - .) + %[bit0] \n\t"
> " .popsection \n\t"
> - : : "i"(&((char *)key)[branch]) : : l_yes);
> + : : [key]"Si"(key), [bit0]"i"(branch) : : l_yes);
>
> return false;
> l_yes:
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 10:03 ` Geert Uytterhoeven
@ 2024-02-19 10:56 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-19 10:56 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Fangrui,
>
> On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > The generic constraint "i" seems to be copied from x86 or arm (and with
> > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > not with -fPIE/-fPIC in GCC's aarch64 port.
>
> Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
>
> > The machine constraint "S", which denotes a symbol or label reference
> > with a constant offset, supports PIC and has been available in GCC since
> > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > "S" on a symbol with a constant offset [1] (e.g.
> > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > include/linux/netfilter.h), so we use "i" as a fallback.
>
> https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> i.e. after 2012 ...
>
> > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Fangrui Song <maskray@google.com>
> > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> >
> > ---
> > Changes from
> > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> >
> > * Use "Si" as Ard suggested to support Clang<19
> > * Make branch a separate operand
> >
> > Changes from v1:
> >
> > * Use asmSymbolicName for readability
>
> But it still fails on gcc-5:
>
> arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> invalid operand
> asm goto(
> ^
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
>
How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
https://godbolt.org/z/szzG3s59K
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-19 10:56 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-19 10:56 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Fangrui,
>
> On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > The generic constraint "i" seems to be copied from x86 or arm (and with
> > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > not with -fPIE/-fPIC in GCC's aarch64 port.
>
> Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
>
> > The machine constraint "S", which denotes a symbol or label reference
> > with a constant offset, supports PIC and has been available in GCC since
> > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > "S" on a symbol with a constant offset [1] (e.g.
> > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > include/linux/netfilter.h), so we use "i" as a fallback.
>
> https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> i.e. after 2012 ...
>
> > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Fangrui Song <maskray@google.com>
> > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> >
> > ---
> > Changes from
> > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> >
> > * Use "Si" as Ard suggested to support Clang<19
> > * Make branch a separate operand
> >
> > Changes from v1:
> >
> > * Use asmSymbolicName for readability
>
> But it still fails on gcc-5:
>
> arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> invalid operand
> asm goto(
> ^
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
>
How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
https://godbolt.org/z/szzG3s59K
_______________________________________________
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] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 10:56 ` Ard Biesheuvel
@ 2024-02-19 10:57 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-19 10:57 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Fangrui,
> >
> > On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > > The generic constraint "i" seems to be copied from x86 or arm (and with
> > > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > > not with -fPIE/-fPIC in GCC's aarch64 port.
> >
> > Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> > jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> >
> > > The machine constraint "S", which denotes a symbol or label reference
> > > with a constant offset, supports PIC and has been available in GCC since
> > > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > > "S" on a symbol with a constant offset [1] (e.g.
> > > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > > include/linux/netfilter.h), so we use "i" as a fallback.
> >
> > https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> > i.e. after 2012 ...
> >
> > > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> > >
> > > ---
> > > Changes from
> > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > >
> > > * Use "Si" as Ard suggested to support Clang<19
> > > * Make branch a separate operand
> > >
> > > Changes from v1:
> > >
> > > * Use asmSymbolicName for readability
> >
> > But it still fails on gcc-5:
> >
> > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > invalid operand
> > asm goto(
> > ^
> >
> > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> >
>
> How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
>
> https://godbolt.org/z/szzG3s59K
Wrong link
https://godbolt.org/z/GTnf3vPaT
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-19 10:57 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-19 10:57 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Fangrui,
> >
> > On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > > The generic constraint "i" seems to be copied from x86 or arm (and with
> > > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > > not with -fPIE/-fPIC in GCC's aarch64 port.
> >
> > Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> > jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> >
> > > The machine constraint "S", which denotes a symbol or label reference
> > > with a constant offset, supports PIC and has been available in GCC since
> > > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > > "S" on a symbol with a constant offset [1] (e.g.
> > > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > > include/linux/netfilter.h), so we use "i" as a fallback.
> >
> > https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> > i.e. after 2012 ...
> >
> > > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> > >
> > > ---
> > > Changes from
> > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > >
> > > * Use "Si" as Ard suggested to support Clang<19
> > > * Make branch a separate operand
> > >
> > > Changes from v1:
> > >
> > > * Use asmSymbolicName for readability
> >
> > But it still fails on gcc-5:
> >
> > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > invalid operand
> > asm goto(
> > ^
> >
> > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> >
>
> How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
>
> https://godbolt.org/z/szzG3s59K
Wrong link
https://godbolt.org/z/GTnf3vPaT
_______________________________________________
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] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 10:57 ` Ard Biesheuvel
@ 2024-02-19 14:42 ` Geert Uytterhoeven
-1 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2024-02-19 14:42 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
Hi Ard,
On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > > > The generic constraint "i" seems to be copied from x86 or arm (and with
> > > > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > > > not with -fPIE/-fPIC in GCC's aarch64 port.
> > >
> > > Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> > > jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> > >
> > > > The machine constraint "S", which denotes a symbol or label reference
> > > > with a constant offset, supports PIC and has been available in GCC since
> > > > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > > > "S" on a symbol with a constant offset [1] (e.g.
> > > > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > > > include/linux/netfilter.h), so we use "i" as a fallback.
> > >
> > > https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> > > i.e. after 2012 ...
> > >
> > > > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> > > >
> > > > ---
> > > > Changes from
> > > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > > >
> > > > * Use "Si" as Ard suggested to support Clang<19
> > > > * Make branch a separate operand
> > > >
> > > > Changes from v1:
> > > >
> > > > * Use asmSymbolicName for readability
> > >
> > > But it still fails on gcc-5:
> > >
> > > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > > invalid operand
> > > asm goto(
> > > ^
> > >
> > > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> > >
> >
> > How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
> https://godbolt.org/z/GTnf3vPaT
I could reproduce the issue on v6.8-rc5 using arm64 defconfig
and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
In file included from ./include/linux/jump_label.h:112:0,
from ./include/linux/static_key.h:1,
from ./include/linux/kasan-enabled.h:5,
from ./arch/arm64/include/asm/cache.h:41,
from ./include/linux/cache.h:6,
from ./include/linux/slab.h:15,
from ./include/linux/resource_ext.h:11,
from ./include/linux/acpi.h:13,
from arch/arm64/mm/fault.c:10:
arch/arm64/mm/fault.c: In function 'do_page_fault':
./arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
invalid operand
asm goto(
^
./arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
invalid operand
There are also a few warnings due to unrecognized options:
arch/arm64/mm/fault.c: At top level:
cc1: warning: unrecognized command line option '-Wno-shift-negative-value'
cc1: warning: unrecognized command line option '-Wno-stringop-overflow'
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
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] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-19 14:42 ` Geert Uytterhoeven
0 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2024-02-19 14:42 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
Hi Ard,
On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > > > The generic constraint "i" seems to be copied from x86 or arm (and with
> > > > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > > > not with -fPIE/-fPIC in GCC's aarch64 port.
> > >
> > > Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> > > jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> > >
> > > > The machine constraint "S", which denotes a symbol or label reference
> > > > with a constant offset, supports PIC and has been available in GCC since
> > > > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > > > "S" on a symbol with a constant offset [1] (e.g.
> > > > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > > > include/linux/netfilter.h), so we use "i" as a fallback.
> > >
> > > https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> > > i.e. after 2012 ...
> > >
> > > > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> > > >
> > > > ---
> > > > Changes from
> > > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > > >
> > > > * Use "Si" as Ard suggested to support Clang<19
> > > > * Make branch a separate operand
> > > >
> > > > Changes from v1:
> > > >
> > > > * Use asmSymbolicName for readability
> > >
> > > But it still fails on gcc-5:
> > >
> > > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > > invalid operand
> > > asm goto(
> > > ^
> > >
> > > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> > >
> >
> > How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
> https://godbolt.org/z/GTnf3vPaT
I could reproduce the issue on v6.8-rc5 using arm64 defconfig
and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
In file included from ./include/linux/jump_label.h:112:0,
from ./include/linux/static_key.h:1,
from ./include/linux/kasan-enabled.h:5,
from ./arch/arm64/include/asm/cache.h:41,
from ./include/linux/cache.h:6,
from ./include/linux/slab.h:15,
from ./include/linux/resource_ext.h:11,
from ./include/linux/acpi.h:13,
from arch/arm64/mm/fault.c:10:
arch/arm64/mm/fault.c: In function 'do_page_fault':
./arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
invalid operand
asm goto(
^
./arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
invalid operand
There are also a few warnings due to unrecognized options:
arch/arm64/mm/fault.c: At top level:
cc1: warning: unrecognized command line option '-Wno-shift-negative-value'
cc1: warning: unrecognized command line option '-Wno-stringop-overflow'
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 14:42 ` Geert Uytterhoeven
@ 2024-02-19 15:41 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-19 15:41 UTC (permalink / raw)
To: Geert Uytterhoeven, Arnd Bergmann
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ard,
>
> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > > > > The generic constraint "i" seems to be copied from x86 or arm (and with
> > > > > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > > > > not with -fPIE/-fPIC in GCC's aarch64 port.
> > > >
> > > > Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> > > > jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> > > >
> > > > > The machine constraint "S", which denotes a symbol or label reference
> > > > > with a constant offset, supports PIC and has been available in GCC since
> > > > > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > > > > "S" on a symbol with a constant offset [1] (e.g.
> > > > > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > > > > include/linux/netfilter.h), so we use "i" as a fallback.
> > > >
> > > > https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> > > > i.e. after 2012 ...
> > > >
> > > > > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > > > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> > > > >
> > > > > ---
> > > > > Changes from
> > > > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > > > >
> > > > > * Use "Si" as Ard suggested to support Clang<19
> > > > > * Make branch a separate operand
> > > > >
> > > > > Changes from v1:
> > > > >
> > > > > * Use asmSymbolicName for readability
> > > >
> > > > But it still fails on gcc-5:
> > > >
> > > > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > > > invalid operand
> > > > asm goto(
> > > > ^
> > > >
> > > > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> > > >
> > >
> > > How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
>
> > https://godbolt.org/z/GTnf3vPaT
>
> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
>
OK, I managed to do so as well.
And GCC 6.4 from the same source works correctly.
Not sure whether there are any plans to bump the minimal GCC version
any time soon (cc'ing Arnd), but we should probably drop this change
until that happens.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-19 15:41 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-19 15:41 UTC (permalink / raw)
To: Geert Uytterhoeven, Arnd Bergmann
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ard,
>
> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > > > > The generic constraint "i" seems to be copied from x86 or arm (and with
> > > > > a redundant generic operand modifier "c"). It works with -fno-PIE but
> > > > > not with -fPIE/-fPIC in GCC's aarch64 port.
> > > >
> > > > Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64:
> > > > jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> > > >
> > > > > The machine constraint "S", which denotes a symbol or label reference
> > > > > with a constant offset, supports PIC and has been available in GCC since
> > > > > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > > > > "S" on a symbol with a constant offset [1] (e.g.
> > > > > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > > > > include/linux/netfilter.h), so we use "i" as a fallback.
> > > >
> > > > https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> > > > i.e. after 2012 ...
> > > >
> > > > > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > > > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> > > > >
> > > > > ---
> > > > > Changes from
> > > > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > > > >
> > > > > * Use "Si" as Ard suggested to support Clang<19
> > > > > * Make branch a separate operand
> > > > >
> > > > > Changes from v1:
> > > > >
> > > > > * Use asmSymbolicName for readability
> > > >
> > > > But it still fails on gcc-5:
> > > >
> > > > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > > > invalid operand
> > > > asm goto(
> > > > ^
> > > >
> > > > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> > > >
> > >
> > > How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
>
> > https://godbolt.org/z/GTnf3vPaT
>
> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
>
OK, I managed to do so as well.
And GCC 6.4 from the same source works correctly.
Not sure whether there are any plans to bump the minimal GCC version
any time soon (cc'ing Arnd), but we should probably drop this change
until that happens.
_______________________________________________
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] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 15:41 ` Ard Biesheuvel
@ 2024-02-19 17:06 ` Arnd Bergmann
-1 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2024-02-19 17:06 UTC (permalink / raw)
To: Ard Biesheuvel, Geert Uytterhoeven
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>> > https://godbolt.org/z/GTnf3vPaT
>>
>> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
>> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
>> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
>>
>
> OK, I managed to do so as well.
>
> And GCC 6.4 from the same source works correctly.
>
> Not sure whether there are any plans to bump the minimal GCC version
> any time soon (cc'ing Arnd), but we should probably drop this change
> until that happens.
From what I can tell, we may as well formally raise the minimum
gcc version to 8.1+ already, as that is a version that is
actually used in distros, and we have been on 5.1+ for a few
years already.
Not sure if there are any other benefits to gcc-8 besides
allowing minor cleanups.
gcc-9 would bring --std=gnu2x support, but it may be too
early for that.
Arnd
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-19 17:06 ` Arnd Bergmann
0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2024-02-19 17:06 UTC (permalink / raw)
To: Ard Biesheuvel, Geert Uytterhoeven
Cc: Fangrui Song, Catalin Marinas, Will Deacon, linux-arm-kernel,
Jisheng Zhang, Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>> > https://godbolt.org/z/GTnf3vPaT
>>
>> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
>> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
>> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
>>
>
> OK, I managed to do so as well.
>
> And GCC 6.4 from the same source works correctly.
>
> Not sure whether there are any plans to bump the minimal GCC version
> any time soon (cc'ing Arnd), but we should probably drop this change
> until that happens.
From what I can tell, we may as well formally raise the minimum
gcc version to 8.1+ already, as that is a version that is
actually used in distros, and we have been on 5.1+ for a few
years already.
Not sure if there are any other benefits to gcc-8 besides
allowing minor cleanups.
gcc-9 would bring --std=gnu2x support, but it may be too
early for that.
Arnd
_______________________________________________
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] 26+ messages in thread* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 17:06 ` Arnd Bergmann
@ 2024-02-19 18:22 ` Mark Rutland
-1 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2024-02-19 18:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ard Biesheuvel, Geert Uytterhoeven, Fangrui Song,
Catalin Marinas, Will Deacon, linux-arm-kernel, Jisheng Zhang,
Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, Feb 19, 2024 at 06:06:19PM +0100, Arnd Bergmann wrote:
> On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> > On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> >> > https://godbolt.org/z/GTnf3vPaT
> >>
> >> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> >> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> >> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> >>
> >
> > OK, I managed to do so as well.
> >
> > And GCC 6.4 from the same source works correctly.
> >
> > Not sure whether there are any plans to bump the minimal GCC version
> > any time soon (cc'ing Arnd), but we should probably drop this change
> > until that happens.
>
> From what I can tell, we may as well formally raise the minimum
> gcc version to 8.1+ already, as that is a version that is
> actually used in distros, and we have been on 5.1+ for a few
> years already.
>
> Not sure if there are any other benefits to gcc-8 besides
> allowing minor cleanups.
Arguably a minor cleanup, but on arm64 that'd allow us to get rid of the old
mcount-based ftrace implementation and rely on -fpatchable-function-entry.
On its own that'd save ~130 lines of asm and ~70 lines of C, but it'd also
remove some constraints on other features (e.g. the mcount-based form's graph
tracer isn't compatible with pointer authentication), it would simplify a few
things going forwards (e.g. the implementation of RELIABLE_STACKTRACE, since we
could rely on having ftrace_regs and a single trampoline), and the remaining
support would be better tested.
I've wanted to remove the old ftrace implementation for a while, but on its own
it was never important/urgent enough to justify bumping to GCC 8+.
Mark.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-19 18:22 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2024-02-19 18:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ard Biesheuvel, Geert Uytterhoeven, Fangrui Song,
Catalin Marinas, Will Deacon, linux-arm-kernel, Jisheng Zhang,
Dave Martin, Peter Smith, llvm, linux-kernel
On Mon, Feb 19, 2024 at 06:06:19PM +0100, Arnd Bergmann wrote:
> On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> > On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> >> > https://godbolt.org/z/GTnf3vPaT
> >>
> >> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> >> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> >> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> >>
> >
> > OK, I managed to do so as well.
> >
> > And GCC 6.4 from the same source works correctly.
> >
> > Not sure whether there are any plans to bump the minimal GCC version
> > any time soon (cc'ing Arnd), but we should probably drop this change
> > until that happens.
>
> From what I can tell, we may as well formally raise the minimum
> gcc version to 8.1+ already, as that is a version that is
> actually used in distros, and we have been on 5.1+ for a few
> years already.
>
> Not sure if there are any other benefits to gcc-8 besides
> allowing minor cleanups.
Arguably a minor cleanup, but on arm64 that'd allow us to get rid of the old
mcount-based ftrace implementation and rely on -fpatchable-function-entry.
On its own that'd save ~130 lines of asm and ~70 lines of C, but it'd also
remove some constraints on other features (e.g. the mcount-based form's graph
tracer isn't compatible with pointer authentication), it would simplify a few
things going forwards (e.g. the implementation of RELIABLE_STACKTRACE, since we
could rely on having ftrace_regs and a single trampoline), and the remaining
support would be better tested.
I've wanted to remove the old ftrace implementation for a while, but on its own
it was never important/urgent enough to justify bumping to GCC 8+.
Mark.
_______________________________________________
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] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 18:22 ` Mark Rutland
@ 2024-02-20 7:55 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-20 7:55 UTC (permalink / raw)
To: Mark Rutland
Cc: Arnd Bergmann, Geert Uytterhoeven, Fangrui Song, Catalin Marinas,
Will Deacon, linux-arm-kernel, Jisheng Zhang, Dave Martin,
Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 19:22, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Feb 19, 2024 at 06:06:19PM +0100, Arnd Bergmann wrote:
> > On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> > > On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > >> > https://godbolt.org/z/GTnf3vPaT
> > >>
> > >> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> > >> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> > >> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> > >>
> > >
> > > OK, I managed to do so as well.
> > >
> > > And GCC 6.4 from the same source works correctly.
> > >
> > > Not sure whether there are any plans to bump the minimal GCC version
> > > any time soon (cc'ing Arnd), but we should probably drop this change
> > > until that happens.
> >
> > From what I can tell, we may as well formally raise the minimum
> > gcc version to 8.1+ already, as that is a version that is
> > actually used in distros, and we have been on 5.1+ for a few
> > years already.
> >
> > Not sure if there are any other benefits to gcc-8 besides
> > allowing minor cleanups.
>
> Arguably a minor cleanup, but on arm64 that'd allow us to get rid of the old
> mcount-based ftrace implementation and rely on -fpatchable-function-entry.
> On its own that'd save ~130 lines of asm and ~70 lines of C, but it'd also
> remove some constraints on other features (e.g. the mcount-based form's graph
> tracer isn't compatible with pointer authentication), it would simplify a few
> things going forwards (e.g. the implementation of RELIABLE_STACKTRACE, since we
> could rely on having ftrace_regs and a single trampoline), and the remaining
> support would be better tested.
>
> I've wanted to remove the old ftrace implementation for a while, but on its own
> it was never important/urgent enough to justify bumping to GCC 8+.
>
I don't think this is minor, tbh. Supporting two versions of the
highly complex tracing infrastructure for a toolchain that is only
used in CI seems like a waste of time and effort.
I checked x86, and it needs at least GCC 7 for retpoline support, so I
reckon at least GCC 5/6 support might be dropped there as well.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-20 7:55 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-20 7:55 UTC (permalink / raw)
To: Mark Rutland
Cc: Arnd Bergmann, Geert Uytterhoeven, Fangrui Song, Catalin Marinas,
Will Deacon, linux-arm-kernel, Jisheng Zhang, Dave Martin,
Peter Smith, llvm, linux-kernel
On Mon, 19 Feb 2024 at 19:22, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Feb 19, 2024 at 06:06:19PM +0100, Arnd Bergmann wrote:
> > On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> > > On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > >> > https://godbolt.org/z/GTnf3vPaT
> > >>
> > >> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> > >> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> > >> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> > >>
> > >
> > > OK, I managed to do so as well.
> > >
> > > And GCC 6.4 from the same source works correctly.
> > >
> > > Not sure whether there are any plans to bump the minimal GCC version
> > > any time soon (cc'ing Arnd), but we should probably drop this change
> > > until that happens.
> >
> > From what I can tell, we may as well formally raise the minimum
> > gcc version to 8.1+ already, as that is a version that is
> > actually used in distros, and we have been on 5.1+ for a few
> > years already.
> >
> > Not sure if there are any other benefits to gcc-8 besides
> > allowing minor cleanups.
>
> Arguably a minor cleanup, but on arm64 that'd allow us to get rid of the old
> mcount-based ftrace implementation and rely on -fpatchable-function-entry.
> On its own that'd save ~130 lines of asm and ~70 lines of C, but it'd also
> remove some constraints on other features (e.g. the mcount-based form's graph
> tracer isn't compatible with pointer authentication), it would simplify a few
> things going forwards (e.g. the implementation of RELIABLE_STACKTRACE, since we
> could rely on having ftrace_regs and a single trampoline), and the remaining
> support would be better tested.
>
> I've wanted to remove the old ftrace implementation for a while, but on its own
> it was never important/urgent enough to justify bumping to GCC 8+.
>
I don't think this is minor, tbh. Supporting two versions of the
highly complex tracing infrastructure for a toolchain that is only
used in CI seems like a waste of time and effort.
I checked x86, and it needs at least GCC 7 for retpoline support, so I
reckon at least GCC 5/6 support might be dropped there as well.
_______________________________________________
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] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-20 7:55 ` Ard Biesheuvel
@ 2024-02-20 8:49 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-20 8:49 UTC (permalink / raw)
To: Mark Rutland
Cc: Arnd Bergmann, Geert Uytterhoeven, Fangrui Song, Catalin Marinas,
Will Deacon, linux-arm-kernel, Jisheng Zhang, Dave Martin,
Peter Smith, llvm, linux-kernel
On Tue, 20 Feb 2024 at 08:55, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 19 Feb 2024 at 19:22, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Mon, Feb 19, 2024 at 06:06:19PM +0100, Arnd Bergmann wrote:
> > > On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> > > > On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > >> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > >> > https://godbolt.org/z/GTnf3vPaT
> > > >>
> > > >> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> > > >> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> > > >> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> > > >>
> > > >
> > > > OK, I managed to do so as well.
> > > >
> > > > And GCC 6.4 from the same source works correctly.
> > > >
> > > > Not sure whether there are any plans to bump the minimal GCC version
> > > > any time soon (cc'ing Arnd), but we should probably drop this change
> > > > until that happens.
> > >
> > > From what I can tell, we may as well formally raise the minimum
> > > gcc version to 8.1+ already, as that is a version that is
> > > actually used in distros, and we have been on 5.1+ for a few
> > > years already.
> > >
> > > Not sure if there are any other benefits to gcc-8 besides
> > > allowing minor cleanups.
> >
> > Arguably a minor cleanup, but on arm64 that'd allow us to get rid of the old
> > mcount-based ftrace implementation and rely on -fpatchable-function-entry.
> > On its own that'd save ~130 lines of asm and ~70 lines of C, but it'd also
> > remove some constraints on other features (e.g. the mcount-based form's graph
> > tracer isn't compatible with pointer authentication), it would simplify a few
> > things going forwards (e.g. the implementation of RELIABLE_STACKTRACE, since we
> > could rely on having ftrace_regs and a single trampoline), and the remaining
> > support would be better tested.
> >
> > I've wanted to remove the old ftrace implementation for a while, but on its own
> > it was never important/urgent enough to justify bumping to GCC 8+.
> >
>
> I don't think this is minor, tbh. Supporting two versions of the
> highly complex tracing infrastructure for a toolchain that is only
> used in CI seems like a waste of time and effort.
>
> I checked x86, and it needs at least GCC 7 for retpoline support, so I
> reckon at least GCC 5/6 support might be dropped there as well.
Another data point: __GCC_ASM_FLAG_OUTPUTS__ needs GCC 6 or later on x86.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-20 8:49 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2024-02-20 8:49 UTC (permalink / raw)
To: Mark Rutland
Cc: Arnd Bergmann, Geert Uytterhoeven, Fangrui Song, Catalin Marinas,
Will Deacon, linux-arm-kernel, Jisheng Zhang, Dave Martin,
Peter Smith, llvm, linux-kernel
On Tue, 20 Feb 2024 at 08:55, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 19 Feb 2024 at 19:22, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Mon, Feb 19, 2024 at 06:06:19PM +0100, Arnd Bergmann wrote:
> > > On Mon, Feb 19, 2024, at 16:41, Ard Biesheuvel wrote:
> > > > On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > >> On Mon, Feb 19, 2024 at 11:57 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > >> > https://godbolt.org/z/GTnf3vPaT
> > > >>
> > > >> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> > > >> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> > > >> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> > > >>
> > > >
> > > > OK, I managed to do so as well.
> > > >
> > > > And GCC 6.4 from the same source works correctly.
> > > >
> > > > Not sure whether there are any plans to bump the minimal GCC version
> > > > any time soon (cc'ing Arnd), but we should probably drop this change
> > > > until that happens.
> > >
> > > From what I can tell, we may as well formally raise the minimum
> > > gcc version to 8.1+ already, as that is a version that is
> > > actually used in distros, and we have been on 5.1+ for a few
> > > years already.
> > >
> > > Not sure if there are any other benefits to gcc-8 besides
> > > allowing minor cleanups.
> >
> > Arguably a minor cleanup, but on arm64 that'd allow us to get rid of the old
> > mcount-based ftrace implementation and rely on -fpatchable-function-entry.
> > On its own that'd save ~130 lines of asm and ~70 lines of C, but it'd also
> > remove some constraints on other features (e.g. the mcount-based form's graph
> > tracer isn't compatible with pointer authentication), it would simplify a few
> > things going forwards (e.g. the implementation of RELIABLE_STACKTRACE, since we
> > could rely on having ftrace_regs and a single trampoline), and the remaining
> > support would be better tested.
> >
> > I've wanted to remove the old ftrace implementation for a while, but on its own
> > it was never important/urgent enough to justify bumping to GCC 8+.
> >
>
> I don't think this is minor, tbh. Supporting two versions of the
> highly complex tracing infrastructure for a toolchain that is only
> used in CI seems like a waste of time and effort.
>
> I checked x86, and it needs at least GCC 7 for retpoline support, so I
> reckon at least GCC 5/6 support might be dropped there as well.
Another data point: __GCC_ASM_FLAG_OUTPUTS__ needs GCC 6 or later on x86.
_______________________________________________
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] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
2024-02-19 15:41 ` Ard Biesheuvel
@ 2024-02-20 12:05 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2024-02-20 12:05 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Geert Uytterhoeven, Arnd Bergmann, Fangrui Song, Catalin Marinas,
linux-arm-kernel, Jisheng Zhang, Dave Martin, Peter Smith, llvm,
linux-kernel
On Mon, Feb 19, 2024 at 04:41:03PM +0100, Ard Biesheuvel wrote:
> On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > Changes from
> > > > > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > > > > >
> > > > > > * Use "Si" as Ard suggested to support Clang<19
> > > > > > * Make branch a separate operand
> > > > > >
> > > > > > Changes from v1:
> > > > > >
> > > > > > * Use asmSymbolicName for readability
> > > > >
> > > > > But it still fails on gcc-5:
> > > > >
> > > > > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > > > > invalid operand
> > > > > asm goto(
> > > > > ^
> > > > >
> > > > > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> > > > >
> > > >
> > > > How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
> >
> > > https://godbolt.org/z/GTnf3vPaT
> >
> > I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> > and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> > https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> >
>
> OK, I managed to do so as well.
>
> And GCC 6.4 from the same source works correctly.
>
> Not sure whether there are any plans to bump the minimal GCC version
> any time soon (cc'ing Arnd), but we should probably drop this change
> until that happens.
Yup, makes sense to me. I'll revert the original change and we can bring it
back later if we decide to bump the minimum GCC version.
Cheers,
Will
_______________________________________________
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] 26+ messages in thread
* Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
@ 2024-02-20 12:05 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2024-02-20 12:05 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Geert Uytterhoeven, Arnd Bergmann, Fangrui Song, Catalin Marinas,
linux-arm-kernel, Jisheng Zhang, Dave Martin, Peter Smith, llvm,
linux-kernel
On Mon, Feb 19, 2024 at 04:41:03PM +0100, Ard Biesheuvel wrote:
> On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > Changes from
> > > > > > arm64: jump_label: use constraint "S" instead of "i" (https://lore.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > > > > >
> > > > > > * Use "Si" as Ard suggested to support Clang<19
> > > > > > * Make branch a separate operand
> > > > > >
> > > > > > Changes from v1:
> > > > > >
> > > > > > * Use asmSymbolicName for readability
> > > > >
> > > > > But it still fails on gcc-5:
> > > > >
> > > > > arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > > > > invalid operand
> > > > > asm goto(
> > > > > ^
> > > > >
> > > > > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> > > > >
> > > >
> > > > How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
> >
> > > https://godbolt.org/z/GTnf3vPaT
> >
> > I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> > and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> > https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
> >
>
> OK, I managed to do so as well.
>
> And GCC 6.4 from the same source works correctly.
>
> Not sure whether there are any plans to bump the minimal GCC version
> any time soon (cc'ing Arnd), but we should probably drop this change
> until that happens.
Yup, makes sense to me. I'll revert the original change and we can bring it
back later if we decide to bump the minimum GCC version.
Cheers,
Will
^ permalink raw reply [flat|nested] 26+ messages in thread