[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 at 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 at arm.com>