Re: [PATCH 5/6] arm64: insn: operate on MSR/MRS sysreg field via defines

[Vladimir Murzin <vladimir.murzin@arm.com>]

Hi Ada,

On 9/28/26 14:30, Ada Couprie Diaz wrote:
> Replace the few instances of hard-coded system register offset and masks
> used to operate on MSR/MRS instructions with defines.
> This will allow re-use in future commits while making the connection
> between those values more explicit.
> 
> While we are here, mark `aarch64_insn_extract_system_reg()` `noinstr`
> so it can be safe to use in alternative pacthing callbacks.
> 
> Changing the mask used in `aarch64_insn_gen_mrs()` to exclude
> the lower bits does not change behaviour,
> as `aarch64_insn_encode_register()` already clears the bits
> used to encode the target register.
> 
> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> ---
>  arch/arm64/include/asm/insn.h | 3 +++
>  arch/arm64/lib/insn.c         | 8 ++++----
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index 40f13d28a5fd7..03620f67a3e47 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -753,6 +753,9 @@ static __always_inline u32 aarch64_insn_gen_dsb(enum aarch64_insn_mb_type type)
>  	return insn;
>  }
>  
> +#define AARCH64_INSN_SYSREG_OFFSET	5
> +#define AARCH64_INSN_SYSREG_MASK	GENMASK(19, 5)
> +

Sashiko has raised comment [1]

| Does this mask unintentionally exclude bit 20?
| The previous hardcoded mask was 0x1FFFE0, which is equivalent to
| GENMASK(20, 5) and includes 16 bits. Defining it as GENMASK(19, 5) yields
| a 15-bit mask.

[1] https://sashiko.dev/#/patchset/20260928133034.243541-1-ada.coupriediaz%40arm.com

Cheers
Vladimir