[PATCH 0/2] arm64: insn: clean-up memory barrier generation

[PATCH 0/2] arm64: insn: clean-up memory barrier generation

[Ada Couprie Diaz]

Hello,

This is a small follow-up from [0], taking Vladimir's comments into
account and going a bit further in cleaning-up DMB/DSB generation.

The first patch prepares for inlining and cleans-up the enum cases,
encoding the values directly and matching their numerical order.

The second patch inlines the barrier type check and the instruction
generation functions, allowing checking at compile time for invalid
barrier types (only non-deprecated types are allowed now) and safe
usage in alternative callbacks.

The "empty" switch-case statements with a default `BUILD_BUG` might
not be the best way to check for valid type, but I thought that
it might be a bit better than a three-/six-way && if, do let me know !

Thanks in advance,
Ada

[0]: https://lore.kernel.org/r/20260928133034.243541-1-ada.coupriediaz@arm.com 

Ada Couprie Diaz (2):
  arm64: insn: clean-up memory barrier types
  arm64: insn: make `aarch64_insn_gen_d{m,s}b()` alternative-safe

 arch/arm64/include/asm/insn.h | 68 +++++++++++++++++++++++++++--------
 arch/arm64/lib/insn.c         | 58 ------------------------------
 arch/arm64/net/bpf_jit.h      |  2 +-
 3 files changed, 55 insertions(+), 73 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.43.0

[PATCH 1/2] arm64: insn: clean-up memory barrier types

[Ada Couprie Diaz]

Replace magic-numbers for offset and mask with shared defines.

Re-order the types enum by value, matching the Arm ARM, and
define them with their proper values, simplifying their use.

Arm ARM DDI 0487 version M.c[0] deprecated a number of types
used as options for the DMB and DSB instructions, with no change
in behaviour.
The deprecated types are not used in the kernel, so remove them
from our instruction generation framework and add the missing
`AARCH64_INSN_MB_OSH` case in `__get_barrier_crm_val()`.

This simplifies the switch-case and now covers all the enum types,
which allows for making the DSB and DMB generation non instrumentable
in a follow-up commit.

[0]: https://support.arm.com/documentation/ddi0487/mc/
     (See C6.2.150, C6.2.152)

Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
 arch/arm64/include/asm/insn.h | 22 ++++++++----------
 arch/arm64/lib/insn.c         | 44 ++++++++++++-----------------------
 2 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 1ce75a442638..2d19f48aab81 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -7,6 +7,7 @@
  */
 #ifndef	__ASM_INSN_H
 #define	__ASM_INSN_H
+#include <linux/bits.h>
 #include <linux/build_bug.h>
 #include <linux/types.h>
 
@@ -297,19 +298,16 @@ enum aarch64_insn_mem_order_type {
 	AARCH64_INSN_MEM_ORDER_ACQREL,
 };
 
+#define AARCH64_INSN_MB_TYPE_OFFSET 8
+#define AARCH64_INSN_MB_TYPE_MASK GENMASK(11, AARCH64_INSN_MB_TYPE_OFFSET)
+
 enum aarch64_insn_mb_type {
-	AARCH64_INSN_MB_SY,
-	AARCH64_INSN_MB_ST,
-	AARCH64_INSN_MB_LD,
-	AARCH64_INSN_MB_ISH,
-	AARCH64_INSN_MB_ISHST,
-	AARCH64_INSN_MB_ISHLD,
-	AARCH64_INSN_MB_NSH,
-	AARCH64_INSN_MB_NSHST,
-	AARCH64_INSN_MB_NSHLD,
-	AARCH64_INSN_MB_OSH,
-	AARCH64_INSN_MB_OSHST,
-	AARCH64_INSN_MB_OSHLD,
+	AARCH64_INSN_MB_OSH	= 0b0011 << AARCH64_INSN_MB_TYPE_OFFSET,
+	AARCH64_INSN_MB_NSH	= 0b0111 << AARCH64_INSN_MB_TYPE_OFFSET,
+	AARCH64_INSN_MB_ISH	= 0b1011 << AARCH64_INSN_MB_TYPE_OFFSET,
+	AARCH64_INSN_MB_LD	= 0b1101 << AARCH64_INSN_MB_TYPE_OFFSET,
+	AARCH64_INSN_MB_ST	= 0b1110 << AARCH64_INSN_MB_TYPE_OFFSET,
+	AARCH64_INSN_MB_SY	= 0b1111 << AARCH64_INSN_MB_TYPE_OFFSET,
 };
 
 #define	__AARCH64_INSN_FUNCS(abbr, mask, val)				\
diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
index e70ac0238515..fa61a631d687 100644
--- a/arch/arm64/lib/insn.c
+++ b/arch/arm64/lib/insn.c
@@ -1561,60 +1561,46 @@ u32 aarch64_insn_gen_extr(enum aarch64_insn_variant variant,
 	return aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RM, insn, Rm);
 }
 
-static u32 __get_barrier_crm_val(enum aarch64_insn_mb_type type)
+static bool __is_valid_barrier_type(enum aarch64_insn_mb_type type)
 {
 	switch (type) {
-	case AARCH64_INSN_MB_SY:
-		return 0xf;
-	case AARCH64_INSN_MB_ST:
-		return 0xe;
-	case AARCH64_INSN_MB_LD:
-		return 0xd;
-	case AARCH64_INSN_MB_ISH:
-		return 0xb;
-	case AARCH64_INSN_MB_ISHST:
-		return 0xa;
-	case AARCH64_INSN_MB_ISHLD:
-		return 0x9;
+	case AARCH64_INSN_MB_OSH:
 	case AARCH64_INSN_MB_NSH:
-		return 0x7;
-	case AARCH64_INSN_MB_NSHST:
-		return 0x6;
-	case AARCH64_INSN_MB_NSHLD:
-		return 0x5;
+	case AARCH64_INSN_MB_ISH:
+	case AARCH64_INSN_MB_LD:
+	case AARCH64_INSN_MB_ST:
+	case AARCH64_INSN_MB_SY:
+		return true;
 	default:
 		pr_err("%s: unknown barrier type %d\n", __func__, type);
-		return AARCH64_BREAK_FAULT;
+		return false;
 	}
 }
 
 u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type)
 {
-	u32 opt;
 	u32 insn;
 
-	opt = __get_barrier_crm_val(type);
-	if (opt == AARCH64_BREAK_FAULT)
+	if (!__is_valid_barrier_type(type))
 		return AARCH64_BREAK_FAULT;
 
 	insn = aarch64_insn_get_dmb_value();
-	insn &= ~GENMASK(11, 8);
-	insn |= (opt << 8);
+	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
+	insn |= type;
 
 	return insn;
 }
 
 u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type)
 {
-	u32 opt, insn;
+	u32 insn;
 
-	opt = __get_barrier_crm_val(type);
-	if (opt == AARCH64_BREAK_FAULT)
+	if (!__is_valid_barrier_type(type))
 		return AARCH64_BREAK_FAULT;
 
 	insn = aarch64_insn_get_dsb_base_value();
-	insn &= ~GENMASK(11, 8);
-	insn |= (opt << 8);
+	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
+	insn |= type;
 
 	return insn;
 }
-- 
2.43.0

Re: [PATCH 1/2] arm64: insn: clean-up memory barrier types

[Vladimir Murzin]

Hi Ada,

On 10/16/26 17:51, Ada Couprie Diaz wrote:
> Replace magic-numbers for offset and mask with shared defines.
> 
> Re-order the types enum by value, matching the Arm ARM, and
> define them with their proper values, simplifying their use.
> 
> Arm ARM DDI 0487 version M.c[0] deprecated a number of types
> used as options for the DMB and DSB instructions, with no change
> in behaviour.
> The deprecated types are not used in the kernel, so remove them
> from our instruction generation framework and add the missing
> `AARCH64_INSN_MB_OSH` case in `__get_barrier_crm_val()`.
> 

Nitpick: this is slightly confusing since __get_barrier_crm_val()
gets converted into  __is_valid_barrier_type()

> This simplifies the switch-case and now covers all the enum types,
> which allows for making the DSB and DMB generation non instrumentable
> in a follow-up commit.
> 
> [0]: https://support.arm.com/documentation/ddi0487/mc/
>      (See C6.2.150, C6.2.152)
> 
> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> ---
>  arch/arm64/include/asm/insn.h | 22 ++++++++----------
>  arch/arm64/lib/insn.c         | 44 ++++++++++++-----------------------
>  2 files changed, 25 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index 1ce75a442638..2d19f48aab81 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -7,6 +7,7 @@
>   */
>  #ifndef	__ASM_INSN_H
>  #define	__ASM_INSN_H
> +#include <linux/bits.h>
>  #include <linux/build_bug.h>
>  #include <linux/types.h>
>  
> @@ -297,19 +298,16 @@ enum aarch64_insn_mem_order_type {
>  	AARCH64_INSN_MEM_ORDER_ACQREL,
>  };
>  
> +#define AARCH64_INSN_MB_TYPE_OFFSET 8
> +#define AARCH64_INSN_MB_TYPE_MASK GENMASK(11, AARCH64_INSN_MB_TYPE_OFFSET)
> +
>  enum aarch64_insn_mb_type {
> -	AARCH64_INSN_MB_SY,
> -	AARCH64_INSN_MB_ST,
> -	AARCH64_INSN_MB_LD,
> -	AARCH64_INSN_MB_ISH,
> -	AARCH64_INSN_MB_ISHST,
> -	AARCH64_INSN_MB_ISHLD,
> -	AARCH64_INSN_MB_NSH,
> -	AARCH64_INSN_MB_NSHST,
> -	AARCH64_INSN_MB_NSHLD,
> -	AARCH64_INSN_MB_OSH,
> -	AARCH64_INSN_MB_OSHST,
> -	AARCH64_INSN_MB_OSHLD,
> +	AARCH64_INSN_MB_OSH	= 0b0011 << AARCH64_INSN_MB_TYPE_OFFSET,
> +	AARCH64_INSN_MB_NSH	= 0b0111 << AARCH64_INSN_MB_TYPE_OFFSET,
> +	AARCH64_INSN_MB_ISH	= 0b1011 << AARCH64_INSN_MB_TYPE_OFFSET,
> +	AARCH64_INSN_MB_LD	= 0b1101 << AARCH64_INSN_MB_TYPE_OFFSET,
> +	AARCH64_INSN_MB_ST	= 0b1110 << AARCH64_INSN_MB_TYPE_OFFSET,
> +	AARCH64_INSN_MB_SY	= 0b1111 << AARCH64_INSN_MB_TYPE_OFFSET,
>  };
>  
>  #define	__AARCH64_INSN_FUNCS(abbr, mask, val)				\
> diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
> index e70ac0238515..fa61a631d687 100644
> --- a/arch/arm64/lib/insn.c
> +++ b/arch/arm64/lib/insn.c
> @@ -1561,60 +1561,46 @@ u32 aarch64_insn_gen_extr(enum aarch64_insn_variant variant,
>  	return aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RM, insn, Rm);
>  }
>  
> -static u32 __get_barrier_crm_val(enum aarch64_insn_mb_type type)
> +static bool __is_valid_barrier_type(enum aarch64_insn_mb_type type)
>  {
>  	switch (type) {
> -	case AARCH64_INSN_MB_SY:
> -		return 0xf;
> -	case AARCH64_INSN_MB_ST:
> -		return 0xe;
> -	case AARCH64_INSN_MB_LD:
> -		return 0xd;
> -	case AARCH64_INSN_MB_ISH:
> -		return 0xb;
> -	case AARCH64_INSN_MB_ISHST:
> -		return 0xa;
> -	case AARCH64_INSN_MB_ISHLD:
> -		return 0x9;
> +	case AARCH64_INSN_MB_OSH:
>  	case AARCH64_INSN_MB_NSH:
> -		return 0x7;
> -	case AARCH64_INSN_MB_NSHST:
> -		return 0x6;
> -	case AARCH64_INSN_MB_NSHLD:
> -		return 0x5;
> +	case AARCH64_INSN_MB_ISH:
> +	case AARCH64_INSN_MB_LD:
> +	case AARCH64_INSN_MB_ST:
> +	case AARCH64_INSN_MB_SY:
> +		return true;
>  	default:
>  		pr_err("%s: unknown barrier type %d\n", __func__, type);
> -		return AARCH64_BREAK_FAULT;
> +		return false;
>  	}
>  }
>  
>  u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type)
>  {
> -	u32 opt;
>  	u32 insn;
>  
> -	opt = __get_barrier_crm_val(type);
> -	if (opt == AARCH64_BREAK_FAULT)
> +	if (!__is_valid_barrier_type(type))
>  		return AARCH64_BREAK_FAULT;
>  
>  	insn = aarch64_insn_get_dmb_value();
> -	insn &= ~GENMASK(11, 8);
> -	insn |= (opt << 8);
> +	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
> +	insn |= type;
>  
>  	return insn;
>  }
>  
>  u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type)
>  {
> -	u32 opt, insn;
> +	u32 insn;
>  
> -	opt = __get_barrier_crm_val(type);
> -	if (opt == AARCH64_BREAK_FAULT)
> +	if (!__is_valid_barrier_type(type))
>  		return AARCH64_BREAK_FAULT;
>  
>  	insn = aarch64_insn_get_dsb_base_value();
> -	insn &= ~GENMASK(11, 8);
> -	insn |= (opt << 8);
> +	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
> +	insn |= type;
>  
>  	return insn;
>  }
> -- 2.43.0
> 

FWIW,

Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>

Re: [PATCH 1/2] arm64: insn: clean-up memory barrier types

[Ada Couprie Diaz]

Hi Vladimir,

On 19/10/2026 11:07, Vladimir Murzin wrote:
> Hi Ada,
>
> On 10/16/26 17:51, Ada Couprie Diaz wrote:
>> Replace magic-numbers for offset and mask with shared defines.
>>
>> Re-order the types enum by value, matching the Arm ARM, and
>> define them with their proper values, simplifying their use.
>>
>> Arm ARM DDI 0487 version M.c[0] deprecated a number of types
>> used as options for the DMB and DSB instructions, with no change
>> in behaviour.
>> The deprecated types are not used in the kernel, so remove them
>> from our instruction generation framework and add the missing
>> `AARCH64_INSN_MB_OSH` case in `__get_barrier_crm_val()`.
>>
> Nitpick: this is slightly confusing since __get_barrier_crm_val()
> gets converted into  __is_valid_barrier_type()
Ah yes, as with patch 2 I did forget this when I reworked this part
of the changes, my mistake. Thanks !
>> This simplifies the switch-case and now covers all the enum types,
>> which allows for making the DSB and DMB generation non instrumentable
>> in a follow-up commit.
>>
>> [0]: https://support.arm.com/documentation/ddi0487/mc/
>>       (See C6.2.150, C6.2.152)
>>
>> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
>> ---
>>   arch/arm64/include/asm/insn.h | 22 ++++++++----------
>>   arch/arm64/lib/insn.c         | 44 ++++++++++++-----------------------
>>   2 files changed, 25 insertions(+), 41 deletions(-)
>> [...]
> FWIW,
>
> Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
Thanks for having a look !
Kind regards,
Ada

[PATCH 2/2] arm64: insn: make `aarch64_insn_gen_d{m,s}b()` alternative-safe

[Ada Couprie Diaz]

Move `aarch64_insn_gen_d{m,s}b()` and `__get_barrier_crm_val()`
to `insn.h` and mark them `__always_inline`.
Replace the error print by an inline build-time check
now that the compiler can verify all call sites.

As BPF is the only caller, with static arguments, only accept
non-deprecated types for each barrier, as per Arm ARM DDI 0487
version M.c[0], and update `A64_DMB_ISH` accordingly.

As all functions are now inlined and do not call any instrumentable
function, they are now safe to call in alternative callbacks.

[0]: https://support.arm.com/documentation/ddi0487/mc/
         (See C6.2.150, C6.2.152)

Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
 arch/arm64/include/asm/insn.h | 46 +++++++++++++++++++++++++++++++++--
 arch/arm64/lib/insn.c         | 44 ---------------------------------
 arch/arm64/net/bpf_jit.h      |  2 +-
 3 files changed, 45 insertions(+), 47 deletions(-)

diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 2d19f48aab81..c91a8431c961 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -703,8 +703,50 @@ u32 aarch64_insn_gen_cas(enum aarch64_insn_register result,
 			 enum aarch64_insn_register value,
 			 enum aarch64_insn_size_type size,
 			 enum aarch64_insn_mem_order_type order);
-u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type);
-u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type);
+
+static __always_inline u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type)
+{
+	u32 insn;
+
+	switch (type) {
+	case AARCH64_INSN_MB_LD:
+	case AARCH64_INSN_MB_ST:
+	case AARCH64_INSN_MB_SY:
+		break;
+	default:
+		BUILD_BUG_ON_MSG(1, "invalid dmb type");
+	}
+
+	insn = aarch64_insn_get_dmb_value();
+	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
+	insn |= type;
+
+	return insn;
+}
+
+static __always_inline u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type)
+{
+	u32 insn;
+
+	switch (type) {
+	case AARCH64_INSN_MB_OSH:
+	case AARCH64_INSN_MB_NSH:
+	case AARCH64_INSN_MB_ISH:
+	case AARCH64_INSN_MB_LD:
+	case AARCH64_INSN_MB_ST:
+	case AARCH64_INSN_MB_SY:
+		break;
+	default:
+		BUILD_BUG_ON_MSG(1, "invalid dsb type");
+	}
+
+	insn = aarch64_insn_get_dsb_base_value();
+	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
+	insn |= type;
+
+	return insn;
+}
+
 u32 aarch64_insn_gen_mrs(enum aarch64_insn_register result,
 			 enum aarch64_insn_system_register sysreg);
 
diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
index fa61a631d687..5c37f3a4c402 100644
--- a/arch/arm64/lib/insn.c
+++ b/arch/arm64/lib/insn.c
@@ -1561,50 +1561,6 @@ u32 aarch64_insn_gen_extr(enum aarch64_insn_variant variant,
 	return aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RM, insn, Rm);
 }
 
-static bool __is_valid_barrier_type(enum aarch64_insn_mb_type type)
-{
-	switch (type) {
-	case AARCH64_INSN_MB_OSH:
-	case AARCH64_INSN_MB_NSH:
-	case AARCH64_INSN_MB_ISH:
-	case AARCH64_INSN_MB_LD:
-	case AARCH64_INSN_MB_ST:
-	case AARCH64_INSN_MB_SY:
-		return true;
-	default:
-		pr_err("%s: unknown barrier type %d\n", __func__, type);
-		return false;
-	}
-}
-
-u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type)
-{
-	u32 insn;
-
-	if (!__is_valid_barrier_type(type))
-		return AARCH64_BREAK_FAULT;
-
-	insn = aarch64_insn_get_dmb_value();
-	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
-	insn |= type;
-
-	return insn;
-}
-
-u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type)
-{
-	u32 insn;
-
-	if (!__is_valid_barrier_type(type))
-		return AARCH64_BREAK_FAULT;
-
-	insn = aarch64_insn_get_dsb_base_value();
-	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
-	insn |= type;
-
-	return insn;
-}
-
 u32 aarch64_insn_gen_mrs(enum aarch64_insn_register result,
 			 enum aarch64_insn_system_register sysreg)
 {
diff --git a/arch/arm64/net/bpf_jit.h b/arch/arm64/net/bpf_jit.h
index b2fe6e6dcf44..08d514b40a5b 100644
--- a/arch/arm64/net/bpf_jit.h
+++ b/arch/arm64/net/bpf_jit.h
@@ -324,7 +324,7 @@
 #define A64_NOP    A64_HINT(AARCH64_INSN_HINT_NOP)
 
 /* DMB */
-#define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_ISH)
+#define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_SY)
 
 /* ADR */
 #define A64_ADR(Rd, offset) \
-- 
2.43.0

Re: [PATCH 2/2] arm64: insn: make `aarch64_insn_gen_d{m,s}b()` alternative-safe

[Vladimir Murzin]

Hi Ada,

On 10/16/26 17:51, Ada Couprie Diaz wrote:
> Move `aarch64_insn_gen_d{m,s}b()` and `__get_barrier_crm_val()`

__get_barrier_crm_val()/__is_valid_barrier_type() has gone, so
reference to it confuses.

> to `insn.h` and mark them `__always_inline`.
> Replace the error print by an inline build-time check
> now that the compiler can verify all call sites.

My understanding is that error printing is not alternative-safe,
so we replace it with a build-time check. The latter requires the
barrier type to be visible to the compiler, hence moving the
helper to a header and marking it __always_inline (with the side
effect that the code may be inlined into noinstr functions).

However, is it safe to assume that the barrier type will always
be visible to the compiler? This is true today, but I can imagine
a scenario where the barrier type is passed through a helper
defined in a separate translation units, making it impossible for
the compiler to observe its value at build time. I'm not sure
whether this is something we should worry about right now.

Another thing I'm wondering about is how we plan to make other
helpers alternative-safe. Do we eventually intend to move all of
them into header?

I also noticed that we're not entirely consistent with error
reporting across helpers. In several cases, we simply return
AARCH64_BREAK_FAULT without printing anything to the log. Perhaps
a better approach to making these helpers alternative-safe would
be to remove error printing altogether, mark them noinstr, and
return AARCH64_BREAK_FAULT whenever we cannot construct a valid
opcode?

> 
> As BPF is the only caller, with static arguments, only accept
> non-deprecated types for each barrier, as per Arm ARM DDI 0487
> version M.c[0], and update `A64_DMB_ISH` accordingly.
> 
> As all functions are now inlined and do not call any instrumentable
> function, they are now safe to call in alternative callbacks.
> 
> [0]: https://support.arm.com/documentation/ddi0487/mc/
>          (See C6.2.150, C6.2.152)
> 
> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> ---
>  arch/arm64/include/asm/insn.h | 46 +++++++++++++++++++++++++++++++++--
>  arch/arm64/lib/insn.c         | 44 ---------------------------------
>  arch/arm64/net/bpf_jit.h      |  2 +-
>  3 files changed, 45 insertions(+), 47 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index 2d19f48aab81..c91a8431c961 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -703,8 +703,50 @@ u32 aarch64_insn_gen_cas(enum aarch64_insn_register result,
>  			 enum aarch64_insn_register value,
>  			 enum aarch64_insn_size_type size,
>  			 enum aarch64_insn_mem_order_type order);
> -u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type);
> -u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type);
> +
> +static __always_inline u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type)
> +{
> +	u32 insn;
> +
> +	switch (type) {
> +	case AARCH64_INSN_MB_LD:
> +	case AARCH64_INSN_MB_ST:
> +	case AARCH64_INSN_MB_SY:
> +		break;
> +	default:
> +		BUILD_BUG_ON_MSG(1, "invalid dmb type");
> +	}
> +
> +	insn = aarch64_insn_get_dmb_value();
> +	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
> +	insn |= type;
> +
> +	return insn;
> +}
> +
> +static __always_inline u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type)
> +{
> +	u32 insn;
> +
> +	switch (type) {
> +	case AARCH64_INSN_MB_OSH:
> +	case AARCH64_INSN_MB_NSH:
> +	case AARCH64_INSN_MB_ISH:
> +	case AARCH64_INSN_MB_LD:
> +	case AARCH64_INSN_MB_ST:
> +	case AARCH64_INSN_MB_SY:
> +		break;
> +	default:
> +		BUILD_BUG_ON_MSG(1, "invalid dsb type");
> +	}
> +
> +	insn = aarch64_insn_get_dsb_base_value();
> +	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
> +	insn |= type;
> +
> +	return insn;
> +}
> +
>  u32 aarch64_insn_gen_mrs(enum aarch64_insn_register result,
>  			 enum aarch64_insn_system_register sysreg);
>  
> diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
> index fa61a631d687..5c37f3a4c402 100644
> --- a/arch/arm64/lib/insn.c
> +++ b/arch/arm64/lib/insn.c
> @@ -1561,50 +1561,6 @@ u32 aarch64_insn_gen_extr(enum aarch64_insn_variant variant,
>  	return aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RM, insn, Rm);
>  }
>  
> -static bool __is_valid_barrier_type(enum aarch64_insn_mb_type type)
> -{
> -	switch (type) {
> -	case AARCH64_INSN_MB_OSH:
> -	case AARCH64_INSN_MB_NSH:
> -	case AARCH64_INSN_MB_ISH:
> -	case AARCH64_INSN_MB_LD:
> -	case AARCH64_INSN_MB_ST:
> -	case AARCH64_INSN_MB_SY:
> -		return true;
> -	default:
> -		pr_err("%s: unknown barrier type %d\n", __func__, type);
> -		return false;
> -	}
> -}
> -
> -u32 aarch64_insn_gen_dmb(enum aarch64_insn_mb_type type)
> -{
> -	u32 insn;
> -
> -	if (!__is_valid_barrier_type(type))
> -		return AARCH64_BREAK_FAULT;
> -
> -	insn = aarch64_insn_get_dmb_value();
> -	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
> -	insn |= type;
> -
> -	return insn;
> -}
> -
> -u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type)
> -{
> -	u32 insn;
> -
> -	if (!__is_valid_barrier_type(type))
> -		return AARCH64_BREAK_FAULT;
> -
> -	insn = aarch64_insn_get_dsb_base_value();
> -	insn &= ~AARCH64_INSN_MB_TYPE_MASK;
> -	insn |= type;
> -
> -	return insn;
> -}
> -
>  u32 aarch64_insn_gen_mrs(enum aarch64_insn_register result,
>  			 enum aarch64_insn_system_register sysreg)
>  {
> diff --git a/arch/arm64/net/bpf_jit.h b/arch/arm64/net/bpf_jit.h
> index b2fe6e6dcf44..08d514b40a5b 100644
> --- a/arch/arm64/net/bpf_jit.h
> +++ b/arch/arm64/net/bpf_jit.h
> @@ -324,7 +324,7 @@
>  #define A64_NOP    A64_HINT(AARCH64_INSN_HINT_NOP)
>  
>  /* DMB */
> -#define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_ISH)
> +#define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_SY)
>  

ISH - This option has the same behavior as SY and is deprecated.    

so conversion is valid.

Cheers
Vladimir

>  /* ADR */
>  #define A64_ADR(Rd, offset) \
> -- 2.43.0
> 

Re: [PATCH 2/2] arm64: insn: make `aarch64_insn_gen_d{m,s}b()` alternative-safe

[Ada Couprie Diaz]

Hi Vladimir,

On 19/10/2026 11:49, Vladimir Murzin wrote:
> Hi Ada,
>
> On 10/16/26 17:51, Ada Couprie Diaz wrote:
>> Move `aarch64_insn_gen_d{m,s}b()` and `__get_barrier_crm_val()`
> __get_barrier_crm_val()/__is_valid_barrier_type() has gone, so
> reference to it confuses.
Ah, my bad indeed, I forgot to take it out when reworking this part.
>> to `insn.h` and mark them `__always_inline`.
>> Replace the error print by an inline build-time check
>> now that the compiler can verify all call sites.
> My understanding is that error printing is not alternative-safe,
> so we replace it with a build-time check. The latter requires the
> barrier type to be visible to the compiler, hence moving the
> helper to a header and marking it __always_inline (with the side
> effect that the code may be inlined into noinstr functions).
That is correct !
> However, is it safe to assume that the barrier type will always
> be visible to the compiler? This is true today, but I can imagine
> a scenario where the barrier type is passed through a helper
> defined in a separate translation units, making it impossible for
> the compiler to observe its value at build time. I'm not sure
> whether this is something we should worry about right now.
You are right that an indirect call with variable barrier types would
lead to a build failure, even for "valid" code.
The current helpers in BPF are defines and have fixed barrier types,
which works great with this.

I think that being able to statically verify the validity of instruction
generation is a strongly desired characteristic, so I would be happy
prohibiting other uses that would prevent this.
However, I understand that there is a tradeoff here and that
not everyone might find the benefit worth it.

As it stands, it doesn't impact users of the `aarch64_insn_...`
functions, so now might be the time to think about what we
want here, rather than later when there would be other constraints.
> Another thing I'm wondering about is how we plan to make other
> helpers alternative-safe. Do we eventually intend to move all of
> them into header?
>
> I also noticed that we're not entirely consistent with error
> reporting across helpers. In several cases, we simply return
> AARCH64_BREAK_FAULT without printing anything to the log. Perhaps
> a better approach to making these helpers alternative-safe would
> be to remove error printing altogether, mark them noinstr, and
> return AARCH64_BREAK_FAULT whenever we cannot construct a valid
> opcode?
There's indeed a lot of inconsistency and other helpers not
covered here. Originally[1], I did try to make all the helpers used
in alternative patching safe and this already required to touch
many different helpers which were not really consistent.

As the goal was to make alternative patching safe, I did indeed
move as many helper functions to be `__always_inline` and thus
to the header, as I wanted to benefit from the static validation
that provided.

Trying to phrase my thinking process : to me, it feels like
generating instructions is something that, as much as possible,
should never be able to fail at runtime. Because then, what do
we do if we tried to generate an instruction, and fail ?
For alternative patching, that might be an immediate `BUG()`,
but there might be little to work around the issue
until a fix is made upstream, which could have otherwise been
detected during building...
All of that to say that we /could/ standardize on returning
`AARCH64_BREAK_FAULT` when failing to generate an instruction,
but to me it feels more like a liability that we should try to remove.

Very interested by other's thoughts on this question !

>> As BPF is the only caller, with static arguments, only accept
>> non-deprecated types for each barrier, as per Arm ARM DDI 0487
>> version M.c[0], and update `A64_DMB_ISH` accordingly.
>>
>> As all functions are now inlined and do not call any instrumentable
>> function, they are now safe to call in alternative callbacks.
>>
>> [0]: https://support.arm.com/documentation/ddi0487/mc/
>>           (See C6.2.150, C6.2.152)
>>
>> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
>> ---
>> [...]
>> diff --git a/arch/arm64/net/bpf_jit.h b/arch/arm64/net/bpf_jit.h
>> index b2fe6e6dcf44..08d514b40a5b 100644
>> --- a/arch/arm64/net/bpf_jit.h
>> +++ b/arch/arm64/net/bpf_jit.h
>> @@ -324,7 +324,7 @@
>>   #define A64_NOP    A64_HINT(AARCH64_INSN_HINT_NOP)
>>   
>>   /* DMB */
>> -#define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_ISH)
>> +#define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_SY)
>>   
> ISH - This option has the same behavior as SY and is deprecated.
>
> so conversion is valid.
>
> Cheers
> Vladimir
>
Thanks for the review !
Kind regards,
Ada

[1]: 
https://lore.kernel.org/r/20250923174903.76283-1-ada.coupriediaz@arm.com