Re: [PATCH v5 06/31] arm64/sysreg: Convert SPSR_ELx to automatic register generation

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] SPSR_EL2 enum M incorrectly maps mode value 0b1001 to EL3h instead of EL2h.
- [Low] SPSR_EL1 and SPSR_EL2 mode enums use non-standard names (EL0, EL1tnv, EL1hnv).
--

--- Patch [6]: [PATCH v5 06/31] arm64/sysreg: Convert SPSR_ELx to automatic register generation ---
commit ca584536167a8b3942fb084c2775b73ad663d641
Author: Steffen Eiden <[email protected]>

arm64/sysreg: Convert SPSR_ELx to automatic register generation

This commit converts the SPSR_EL1, SPSR_EL2, and SPSR_EL12 registers to
automatic register generation. It removes the manual definitions in the
sysreg header and adds them to the sysreg generator tool.

> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index 7cb61aca3797f..2a4b94e8c62fd 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
[ ... ]
> @@ -401,6 +401,78 @@ Res0	3:1
>  Field	0	SO
>  EndSysreg
>  
> +Sysreg	SPSR_EL1	3	0	4	0	0
[ ... ]
> +Field	5	T
> +Field	4	M4
> +Enum	3:0	M
> +	0b0000	EL0

[Severity: Low]
Does this code deviate from the standard architectural nomenclature? The
architectural manual and existing kernel constants (like PSR_MODE_EL0t)
refer to this as EL0t rather than EL0.

> +	0b0100	EL1t
> +	0b0101	EL1h
> +	0b1000	EL1tnv
> +	0b1001	EL1hnv

[Severity: Low]
Are these names intended to be EL2t and EL2h instead? When FEAT_NV is
implemented, the architectural manual refers to these as EL2t and EL2h.
Using EL1tnv and EL1hnv might lead to mismatching macros when compared to
existing constants like PSR_MODE_EL2t.

> +EndEnum
> +EndSysreg
> +
> +Sysreg	SPSR_EL2	3	4	4	0	0
[ ... ]
> +Field	5	T
> +Field	4	M4
> +Enum	3:0	M
> +	0b0000	EL0
> +	0b0100	EL1t
> +	0b0101	EL1h
> +	0b1000	EL2t
> +	0b1001	EL3h

[Severity: Medium]
Could EL3h be a typo for EL2h here? According to the ARM Architecture
Reference Manual, 0b1001 corresponds to EL2h. Since SPSR_EL2 cannot hold
EL3 exception states, this might generate an incorrect SPSR_EL2_M_EL3h
macro while leaving out the definition for EL2h.

> +EndEnum
> +EndSysreg

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6