[PATCH 0/2] arm64: Drop more workarounds after binutils bump

linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/2] arm64: Drop more workarounds after binutils bump
@ 2025-06-07 10:40 Marc Zyngier
  2025-06-07 10:40 ` [PATCH 1/2] arm64: Remove .inst " Marc Zyngier
  2025-06-07 10:40 ` [PATCH 2/2] arm64: Restore UL() use in alternative macros " Marc Zyngier
  0 siblings, 2 replies; 4+ messages in thread
From: Marc Zyngier @ 2025-06-07 10:40 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel,
	Arnd Bergmann

Now that binutils 2.30 is our base-line, drop a couple of annoying
workarounds that we have been carrying for some time (I have some
history with the .inst one).

We *could* also start dropping some of the SVE macros now that gas is
known to support it, but I'm not convinced we want to go there yet.

Marc Zyngier (2):
  arm64: Remove .inst workarounds after binutils bump
  arm64: Restore UL() use in alternative macros after binutils bump

 arch/arm64/Kconfig                          |  3 --
 arch/arm64/Makefile                         |  4 ---
 arch/arm64/include/asm/alternative-macros.h | 39 ++++++++-------------
 arch/arm64/include/asm/sysreg.h             | 21 -----------
 tools/arch/arm64/include/asm/sysreg.h       | 21 -----------
 5 files changed, 14 insertions(+), 74 deletions(-)

-- 
2.39.2



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

* [PATCH 1/2] arm64: Remove .inst workarounds after binutils bump
  2025-06-07 10:40 [PATCH 0/2] arm64: Drop more workarounds after binutils bump Marc Zyngier
@ 2025-06-07 10:40 ` Marc Zyngier
  2025-06-09 16:45   ` Marc Zyngier
  2025-06-07 10:40 ` [PATCH 2/2] arm64: Restore UL() use in alternative macros " Marc Zyngier
  1 sibling, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2025-06-07 10:40 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel,
	Arnd Bergmann

Binutils version up to (and including) 2.25 have a pathological
behaviour when it comes to mixing .inst directive and arithmetic
involving labels. The assembler complains about non-constant
expressions and compilation stops pretty quickly.

Now that our base binutils version is 2.30, we can drop the hackish
workarounds we had in both the alternative and sysreg code.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/Kconfig                          |  3 ---
 arch/arm64/Makefile                         |  4 ---
 arch/arm64/include/asm/alternative-macros.h | 28 ++++++++-------------
 arch/arm64/include/asm/sysreg.h             | 21 ----------------
 tools/arch/arm64/include/asm/sysreg.h       | 21 ----------------
 5 files changed, 11 insertions(+), 66 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 55fc331af3371..95697700f1992 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -404,9 +404,6 @@ config ARCH_SUPPORTS_UPROBES
 config ARCH_PROC_KCORE_TEXT
 	def_bool y
 
-config BROKEN_GAS_INST
-	def_bool !$(as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n)
-
 config BUILTIN_RETURN_ADDRESS_STRIPS_PAC
 	bool
 	# Clang's __builtin_return_address() strips the PAC since 12.0.0
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 73a10f65ce8bc..212bf394ea43f 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -29,10 +29,6 @@ cc_has_k_constraint := $(call try-run,echo				\
 		return 0;						\
 	}' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
 
-ifeq ($(CONFIG_BROKEN_GAS_INST),y)
-$(warning Detected assembler with broken .inst; disassembly will be unreliable)
-endif
-
 # The GCC option -ffreestanding is required in order to compile code containing
 # ARM/NEON intrinsics in a non C99-compliant environment (such as the kernel)
 CC_FLAGS_FPU	:= -ffreestanding
diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
index c8c77f9e36d60..61c2cbe618b9f 100644
--- a/arch/arm64/include/asm/alternative-macros.h
+++ b/arch/arm64/include/asm/alternative-macros.h
@@ -40,17 +40,6 @@
 /*
  * alternative assembly primitive:
  *
- * If any of these .org directive fail, it means that insn1 and insn2
- * don't have the same length. This used to be written as
- *
- * .if ((664b-663b) != (662b-661b))
- * 	.error "Alternatives instruction length mismatch"
- * .endif
- *
- * but most assemblers die if insn1 or insn2 have a .inst. This should
- * be fixed in a binutils release posterior to 2.25.51.0.2 (anything
- * containing commit 4e4d08cf7399b606 or c1baaddf8861).
- *
  * Alternatives with callbacks do not generate replacement instructions.
  */
 #define __ALTERNATIVE_CFG(oldinstr, newinstr, cpucap, cfg_enabled)	\
@@ -65,8 +54,9 @@
 	"663:\n\t"							\
 	newinstr "\n"							\
 	"664:\n\t"							\
-	".org	. - (664b-663b) + (662b-661b)\n\t"			\
-	".org	. - (662b-661b) + (664b-663b)\n\t"			\
+	".if ((664b-663b) != (662b-661b))\n"				\
+	".error \"Alternatives instruction length mismatch\"\n"		\
+	".endif\n"							\
 	".previous\n"							\
 	".endif\n"
 
@@ -99,6 +89,12 @@
 	.byte \alt_len
 .endm
 
+.macro check_alternative_sizes
+	.if ((664b-663b) != (662b-661b))
+	.error "Alternatives instruction length mismatch"
+	.endif
+.endm
+
 .macro alternative_insn insn1, insn2, cap, enable = 1
 	.if \enable
 661:	\insn1
@@ -107,8 +103,7 @@
 	.popsection
 	.subsection 1
 663:	\insn2
-664:	.org	. - (664b-663b) + (662b-661b)
-	.org	. - (662b-661b) + (664b-663b)
+664:	check_alternative_sizes
 	.previous
 	.endif
 .endm
@@ -179,8 +174,7 @@
  */
 .macro alternative_endif
 664:
-	.org	. - (664b-663b) + (662b-661b)
-	.org	. - (662b-661b) + (664b-663b)
+	check_alternative_sizes
 	.if .Lasm_alt_mode==0
 	.previous
 	.endif
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index f1bb0d10c39a3..42084cdd3fce8 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -50,8 +50,6 @@
 #define sys_reg_CRm(id)	(((id) >> CRm_shift) & CRm_mask)
 #define sys_reg_Op2(id)	(((id) >> Op2_shift) & Op2_mask)
 
-#ifndef CONFIG_BROKEN_GAS_INST
-
 #ifdef __ASSEMBLY__
 // The space separator is omitted so that __emit_inst(x) can be parsed as
 // either an assembler directive or an assembler macro argument.
@@ -60,25 +58,6 @@
 #define __emit_inst(x)			".inst " __stringify((x)) "\n\t"
 #endif
 
-#else  /* CONFIG_BROKEN_GAS_INST */
-
-#ifndef CONFIG_CPU_BIG_ENDIAN
-#define __INSTR_BSWAP(x)		(x)
-#else  /* CONFIG_CPU_BIG_ENDIAN */
-#define __INSTR_BSWAP(x)		((((x) << 24) & 0xff000000)	| \
-					 (((x) <<  8) & 0x00ff0000)	| \
-					 (((x) >>  8) & 0x0000ff00)	| \
-					 (((x) >> 24) & 0x000000ff))
-#endif	/* CONFIG_CPU_BIG_ENDIAN */
-
-#ifdef __ASSEMBLY__
-#define __emit_inst(x)			.long __INSTR_BSWAP(x)
-#else  /* __ASSEMBLY__ */
-#define __emit_inst(x)			".long " __stringify(__INSTR_BSWAP(x)) "\n\t"
-#endif	/* __ASSEMBLY__ */
-
-#endif	/* CONFIG_BROKEN_GAS_INST */
-
 /*
  * Instructions for modifying PSTATE fields.
  * As per Arm ARM for v8-A, Section "C.5.1.3 op0 == 0b00, architectural hints,
diff --git a/tools/arch/arm64/include/asm/sysreg.h b/tools/arch/arm64/include/asm/sysreg.h
index 690b6ebd118f4..58aebe6e65da5 100644
--- a/tools/arch/arm64/include/asm/sysreg.h
+++ b/tools/arch/arm64/include/asm/sysreg.h
@@ -49,8 +49,6 @@
 #define sys_reg_CRm(id)	(((id) >> CRm_shift) & CRm_mask)
 #define sys_reg_Op2(id)	(((id) >> Op2_shift) & Op2_mask)
 
-#ifndef CONFIG_BROKEN_GAS_INST
-
 #ifdef __ASSEMBLY__
 // The space separator is omitted so that __emit_inst(x) can be parsed as
 // either an assembler directive or an assembler macro argument.
@@ -59,25 +57,6 @@
 #define __emit_inst(x)			".inst " __stringify((x)) "\n\t"
 #endif
 
-#else  /* CONFIG_BROKEN_GAS_INST */
-
-#ifndef CONFIG_CPU_BIG_ENDIAN
-#define __INSTR_BSWAP(x)		(x)
-#else  /* CONFIG_CPU_BIG_ENDIAN */
-#define __INSTR_BSWAP(x)		((((x) << 24) & 0xff000000)	| \
-					 (((x) <<  8) & 0x00ff0000)	| \
-					 (((x) >>  8) & 0x0000ff00)	| \
-					 (((x) >> 24) & 0x000000ff))
-#endif	/* CONFIG_CPU_BIG_ENDIAN */
-
-#ifdef __ASSEMBLY__
-#define __emit_inst(x)			.long __INSTR_BSWAP(x)
-#else  /* __ASSEMBLY__ */
-#define __emit_inst(x)			".long " __stringify(__INSTR_BSWAP(x)) "\n\t"
-#endif	/* __ASSEMBLY__ */
-
-#endif	/* CONFIG_BROKEN_GAS_INST */
-
 /*
  * Instructions for modifying PSTATE fields.
  * As per Arm ARM for v8-A, Section "C.5.1.3 op0 == 0b00, architectural hints,
-- 
2.39.2



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

* Re: [PATCH 1/2] arm64: Remove .inst workarounds after binutils bump
  2025-06-07 10:40 ` [PATCH 1/2] arm64: Remove .inst " Marc Zyngier
@ 2025-06-09 16:45   ` Marc Zyngier
  0 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2025-06-09 16:45 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel,
	Arnd Bergmann

On 2025-06-07 11:40, Marc Zyngier wrote:
> Binutils version up to (and including) 2.25 have a pathological
> behaviour when it comes to mixing .inst directive and arithmetic
> involving labels. The assembler complains about non-constant
> expressions and compilation stops pretty quickly.
> 
> Now that our base binutils version is 2.30, we can drop the hackish
> workarounds we had in both the alternative and sysreg code.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/Kconfig                          |  3 ---
>  arch/arm64/Makefile                         |  4 ---
>  arch/arm64/include/asm/alternative-macros.h | 28 ++++++++-------------
>  arch/arm64/include/asm/sysreg.h             | 21 ----------------
>  tools/arch/arm64/include/asm/sysreg.h       | 21 ----------------
>  5 files changed, 11 insertions(+), 66 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 55fc331af3371..95697700f1992 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -404,9 +404,6 @@ config ARCH_SUPPORTS_UPROBES
>  config ARCH_PROC_KCORE_TEXT
>  	def_bool y
> 
> -config BROKEN_GAS_INST
> -	def_bool !$(as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n)
> -
>  config BUILTIN_RETURN_ADDRESS_STRIPS_PAC
>  	bool
>  	# Clang's __builtin_return_address() strips the PAC since 12.0.0
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 73a10f65ce8bc..212bf394ea43f 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -29,10 +29,6 @@ cc_has_k_constraint := $(call try-run,echo				\
>  		return 0;						\
>  	}' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
> 
> -ifeq ($(CONFIG_BROKEN_GAS_INST),y)
> -$(warning Detected assembler with broken .inst; disassembly will be 
> unreliable)
> -endif
> -
>  # The GCC option -ffreestanding is required in order to compile code 
> containing
>  # ARM/NEON intrinsics in a non C99-compliant environment (such as the 
> kernel)
>  CC_FLAGS_FPU	:= -ffreestanding
> diff --git a/arch/arm64/include/asm/alternative-macros.h
> b/arch/arm64/include/asm/alternative-macros.h
> index c8c77f9e36d60..61c2cbe618b9f 100644
> --- a/arch/arm64/include/asm/alternative-macros.h
> +++ b/arch/arm64/include/asm/alternative-macros.h
> @@ -40,17 +40,6 @@
>  /*
>   * alternative assembly primitive:
>   *
> - * If any of these .org directive fail, it means that insn1 and insn2
> - * don't have the same length. This used to be written as
> - *
> - * .if ((664b-663b) != (662b-661b))
> - * 	.error "Alternatives instruction length mismatch"
> - * .endif
> - *
> - * but most assemblers die if insn1 or insn2 have a .inst. This should
> - * be fixed in a binutils release posterior to 2.25.51.0.2 (anything
> - * containing commit 4e4d08cf7399b606 or c1baaddf8861).
> - *
>   * Alternatives with callbacks do not generate replacement 
> instructions.
>   */
>  #define __ALTERNATIVE_CFG(oldinstr, newinstr, cpucap, cfg_enabled)	\
> @@ -65,8 +54,9 @@
>  	"663:\n\t"							\
>  	newinstr "\n"							\
>  	"664:\n\t"							\
> -	".org	. - (664b-663b) + (662b-661b)\n\t"			\
> -	".org	. - (662b-661b) + (664b-663b)\n\t"			\
> +	".if ((664b-663b) != (662b-661b))\n"				\
> +	".error \"Alternatives instruction length mismatch\"\n"		\
> +	".endif\n"							\
>  	".previous\n"							\

It appears[1] that clang decided to somehow mimic the broken GAS
behaviour with its integrated assembler, and is unable to assemble
the above until some version of clang 19 (both clang 17 and 18, as
available in Debian trixie, appear to be broken).

Passing "CLANG_FLAGS=-no-integrated-as" obviously works, but that's
not exactly desirable.

On the plus side, the test that drives BROKEN_GAS_INST doesn't fire
on any of the clang versions, so we can still drop that.

I'll repost an updated version towards the end of the week.

          M.

[1] https://lore.kernel.org/all/202506091850.ZQDMasOg-lkp@intel.com/
-- 
Jazz is not dead. It just smells funny...


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

* [PATCH 2/2] arm64: Restore UL() use in alternative macros after binutils bump
  2025-06-07 10:40 [PATCH 0/2] arm64: Drop more workarounds after binutils bump Marc Zyngier
  2025-06-07 10:40 ` [PATCH 1/2] arm64: Remove .inst " Marc Zyngier
@ 2025-06-07 10:40 ` Marc Zyngier
  1 sibling, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2025-06-07 10:40 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel,
	Arnd Bergmann

Binutils versions up to (and including) 2.27 are not handling L/UL
suffixes on constants, making it impossible to use the BIT() macro
in some of the alternative macros.

Now that binutils 2.30 is the base version, revert ba00c2a04fa54
("arm64: fix the build with binutils 2.27") and allow the use of the
BIT() macro in inline assembly, such as the alternative code.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/alternative-macros.h | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
index 61c2cbe618b9f..7d80d488c7faf 100644
--- a/arch/arm64/include/asm/alternative-macros.h
+++ b/arch/arm64/include/asm/alternative-macros.h
@@ -8,12 +8,7 @@
 #include <asm/cpucaps.h>
 #include <asm/insn-def.h>
 
-/*
- * Binutils 2.27.0 can't handle a 'UL' suffix on constants, so for the assembly
- * macros below we must use we must use `(1 << ARM64_CB_SHIFT)`.
- */
-#define ARM64_CB_SHIFT	15
-#define ARM64_CB_BIT	BIT(ARM64_CB_SHIFT)
+#define ARM64_CB_BIT	BIT(15)
 
 #if ARM64_NCAPS >= ARM64_CB_BIT
 #error "cpucaps have overflown ARM64_CB_BIT"
@@ -76,7 +71,7 @@
 	__ALTERNATIVE_CFG(oldinstr, newinstr, cpucap, IS_ENABLED(cfg))
 
 #define ALTERNATIVE_CB(oldinstr, cpucap, cb) \
-	__ALTERNATIVE_CFG_CB(oldinstr, (1 << ARM64_CB_SHIFT) | (cpucap), 1, cb)
+	__ALTERNATIVE_CFG_CB(oldinstr, ARM64_CB_BIT | (cpucap), 1, cb)
 #else
 
 #include <asm/assembler.h>
@@ -151,7 +146,7 @@
 .macro alternative_cb cap, cb
 	.set .Lasm_alt_mode, 0
 	.pushsection .altinstructions, "a"
-	altinstruction_entry 661f, \cb, (1 << ARM64_CB_SHIFT) | \cap, 662f-661f, 0
+	altinstruction_entry 661f, \cb, ARM64_CB_BIT | \cap, 662f-661f, 0
 	.popsection
 661:
 .endm
-- 
2.39.2



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

end of thread, other threads:[~2025-06-09 17:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-07 10:40 [PATCH 0/2] arm64: Drop more workarounds after binutils bump Marc Zyngier
2025-06-07 10:40 ` [PATCH 1/2] arm64: Remove .inst " Marc Zyngier
2025-06-09 16:45   ` Marc Zyngier
2025-06-07 10:40 ` [PATCH 2/2] arm64: Restore UL() use in alternative macros " Marc Zyngier

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