Re: [PATCH v5 06/31] arm64/sysreg: Convert SPSR_ELx to automatic register generation
| Newsgroups | org.kernel.vger.kvm,dev.linux.lists.kvmarm,org.kernel.vger.linux-s390 |
|---|---|
| Message-ID | <[email protected]> |
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