Re: [PATCH 2/4] arm64: insn: fix system register inconsistencies

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

On 10/16/26 17:46, Ada Couprie Diaz wrote:
> Two registers in `enum aarch64_insn_special_register` are defined
> incorrectly :
>  - SPSR_IRQ is named SPSR_I*N*Q
>  - SP_EL2's encoding is incorrect and does not correspond to ant register

Nitpick: s/ant/any ?

> 
> Fix those issues and re-order the registers by value.
> Update `enum aarch64_insn_system_register` values and order to be in line
> with `enum aarch64_insn_special_register`, so that we can make
> `aarch64_insn_gen_mrs()` use the same mask as
> `aarch64_insn_extract_system_reg()` while we're here.
> 
> This does not change the behaviour, as `aarch64_insn_encode_register()`
> always masks the lower bits and the top bit of registers are always set.
> 

I think aarch64_insn_encode_register() is not relevant here since
before the change bit[20] was set via aarch64_insn_get_mrs_value()
which returns 0xD5300000. With the change we'd do not rely on
aarch64_insn_get_mrs_value() any more.

> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> ---
>  arch/arm64/include/asm/insn.h | 12 ++++++------
>  arch/arm64/lib/insn.c         |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index c91a8431c961e..2e45f6e80d15f 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -119,27 +119,27 @@ enum aarch64_insn_special_register {
>  	AARCH64_INSN_SPCLREG_SP_EL0	= 0xC208,
>  	AARCH64_INSN_SPCLREG_SPSEL	= 0xC210,
>  	AARCH64_INSN_SPCLREG_CURRENTEL	= 0xC212,
> -	AARCH64_INSN_SPCLREG_DAIF	= 0xDA11,
>  	AARCH64_INSN_SPCLREG_NZCV	= 0xDA10,
> +	AARCH64_INSN_SPCLREG_DAIF	= 0xDA11,

Reorder, as promised in commit message

>  	AARCH64_INSN_SPCLREG_FPCR	= 0xDA20,
>  	AARCH64_INSN_SPCLREG_DSPSR_EL0	= 0xDA28,
>  	AARCH64_INSN_SPCLREG_DLR_EL0	= 0xDA29,
>  	AARCH64_INSN_SPCLREG_SPSR_EL2	= 0xE200,
>  	AARCH64_INSN_SPCLREG_ELR_EL2	= 0xE201,
>  	AARCH64_INSN_SPCLREG_SP_EL1	= 0xE208,
> -	AARCH64_INSN_SPCLREG_SPSR_INQ	= 0xE218,
> +	AARCH64_INSN_SPCLREG_SPSR_IRQ	= 0xE218,

Rename, as promised in commit message

>  	AARCH64_INSN_SPCLREG_SPSR_ABT	= 0xE219,
>  	AARCH64_INSN_SPCLREG_SPSR_UND	= 0xE21A,
>  	AARCH64_INSN_SPCLREG_SPSR_FIQ	= 0xE21B,
>  	AARCH64_INSN_SPCLREG_SPSR_EL3	= 0xF200,
>  	AARCH64_INSN_SPCLREG_ELR_EL3	= 0xF201,
> -	AARCH64_INSN_SPCLREG_SP_EL2	= 0xF210
> +	AARCH64_INSN_SPCLREG_SP_EL2	= 0xF208,

Correct encoding, as promised in commit message

>  };
>  
>  enum aarch64_insn_system_register {
> -	AARCH64_INSN_SYSREG_TPIDR_EL1	= 0x4684,
> -	AARCH64_INSN_SYSREG_TPIDR_EL2	= 0x6682,
> -	AARCH64_INSN_SYSREG_SP_EL0	= 0x4208,
> +	AARCH64_INSN_SYSREG_SP_EL0	= 0xC208,
> +	AARCH64_INSN_SYSREG_TPIDR_EL1	= 0xC684,
> +	AARCH64_INSN_SYSREG_TPIDR_EL2	= 0xE682,

Aligns encoding (we basically account for bit[20] of mrs) with
aarch64_insn_special_register, again as promised in commit message

>  };
>  
>  enum aarch64_insn_variant {
> diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
> index 5c37f3a4c402b..94ee1aae2f5b7 100644
> --- a/arch/arm64/lib/insn.c
> +++ b/arch/arm64/lib/insn.c
> @@ -1566,7 +1566,7 @@ u32 aarch64_insn_gen_mrs(enum aarch64_insn_register result,
>  {
>  	u32 insn = aarch64_insn_get_mrs_value();
>  

insn is 0xD5300000, bit[20] is set

> -	insn &= ~GENMASK(19, 0);
> +	insn &= ~GENMASK(20, 5);

bit[20] is clear

>  	insn |= sysreg << 5;

bit[20] is set

>  	return aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RT,
>  					    insn, result);
> -- 2.43.0
> 

FWIW,

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