| Message ID | 20230306-arm64-fgt-reg-gen-v3-2-decba93cbaab@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/sysregs: Generate definitions fine grained traps control registers | expand |
On Thu, Mar 23, 2023 at 08:44:54PM +0000, Mark Brown wrote: > Automatically generate the Hypervisor Fine-Grained Instruction Trap > Register as per DDI0601 2022-12, currently we only have a definition for > the register name not any of the contents. No functional change. > > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > arch/arm64/include/asm/sysreg.h | 1 - > arch/arm64/tools/sysreg | 65 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 65 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index e5ca9ece1606..c48b41c9b0cc 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -419,7 +419,6 @@ > #define SYS_MDCR_EL2 sys_reg(3, 4, 1, 1, 1) > #define SYS_CPTR_EL2 sys_reg(3, 4, 1, 1, 2) > #define SYS_HSTR_EL2 sys_reg(3, 4, 1, 1, 3) > -#define SYS_HFGITR_EL2 sys_reg(3, 4, 1, 1, 6) > #define SYS_HACR_EL2 sys_reg(3, 4, 1, 1, 7) > > #define SYS_TTBR0_EL2 sys_reg(3, 4, 2, 0, 0) > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > index 60829a9409f0..c06097a8b921 100644 > --- a/arch/arm64/tools/sysreg > +++ b/arch/arm64/tools/sysreg > @@ -1941,6 +1941,71 @@ Sysreg HFGWTR_EL2 3 4 1 1 5 > Fields HFGxTR_EL2 > EndSysreg > > +Sysreg HFGITR_EL2 3 4 1 1 6 > +Res0 63:61 > +Field 60 COSPRCTX > +Field 59 nGCSEPP > +Field 58 nGCSSTR_EL1 > +Field 57 nGCSPUSHM_EL1 These aren't in the Arm ARM afaict ^^^ > +Field 56 nBRBIALL > +Field 55 nBRBINJ > +Field 54 DCCVAC > +Field 53 SVC_EL1 > +Field 52 SVC_EL0 > +Field 51 ERET > +Field 50 CPPRCTX > +Field 49 DVPRCTX > +Field 48 CFPRCTX > +Field 47 TLBIVAALE1 > +Field 46 TLBIVALE1 > +Field 45 TLBIVAAE1 > +Field 44 TLBSIDE1 This is a typo ^^^ (I stopped reviewing at this point) Can't we generate this file from the architecture xml? That would hopefully avoid typos like this and make review less tedious. Will
On Thu, Apr 06, 2023 at 03:46:54PM +0100, Will Deacon wrote: > On Thu, Mar 23, 2023 at 08:44:54PM +0000, Mark Brown wrote: > > Automatically generate the Hypervisor Fine-Grained Instruction Trap > > Register as per DDI0601 2022-12, currently we only have a definition for > > the register name not any of the contents. No functional change. > > +Res0 63:61 > > +Field 60 COSPRCTX > > +Field 59 nGCSEPP > > +Field 58 nGCSSTR_EL1 > > +Field 57 nGCSPUSHM_EL1 > These aren't in the Arm ARM afaict ^^^ Yes, as mentioned in the cover letter these are as per DDI0601 2022-12, the current at time of posting the patch latest release of the architecture XML. They should appear in the next release of the ARM, the XML is updated more frequently. The XML can be seen on the web here: https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/HFGITR-EL2--Hypervisor-Fine-Grained-Instruction-Trap-Register?lang=en > Can't we generate this file from the architecture xml? That would hopefully > avoid typos like this and make review less tedious. I agree that this seems like a sensible idea however there has previously been pushback on the idea of providing tooling to do that, and we would want to manually integrate the output of any such tool since there are a number of cases where for legacy or usability reasons we rename or combine fields. The cases where we use a Fields block to cover identical ELx versions are another issue. I also note that while the XML is viewable on the web AFAICT the only directly downloadable version of the architecture XML available externally is in PDF format which is not entirely helpful for this purpose.
On Thu, Apr 06, 2023 at 04:02:18PM +0100, Mark Brown wrote: > On Thu, Apr 06, 2023 at 03:46:54PM +0100, Will Deacon wrote: > > On Thu, Mar 23, 2023 at 08:44:54PM +0000, Mark Brown wrote: > > > > Automatically generate the Hypervisor Fine-Grained Instruction Trap > > > Register as per DDI0601 2022-12, currently we only have a definition for > > > the register name not any of the contents. No functional change. > > > > +Res0 63:61 > > > +Field 60 COSPRCTX > > > +Field 59 nGCSEPP > > > +Field 58 nGCSSTR_EL1 > > > +Field 57 nGCSPUSHM_EL1 > > > These aren't in the Arm ARM afaict ^^^ > > Yes, as mentioned in the cover letter these are as per DDI0601 2022-12, > the current at time of posting the patch latest release of the > architecture XML. They should appear in the next release of the ARM, > the XML is updated more frequently. The XML can be seen on the web > here: > > https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/HFGITR-EL2--Hypervisor-Fine-Grained-Instruction-Trap-Register?lang=en Thanks -- I just wanted to check that they were final. Happy to trust that xml. > > Can't we generate this file from the architecture xml? That would hopefully > > avoid typos like this and make review less tedious. > > I agree that this seems like a sensible idea however there has > previously been pushback on the idea of providing tooling to do that, > and we would want to manually integrate the output of any such tool > since there are a number of cases where for legacy or usability reasons > we rename or combine fields. The cases where we use a Fields block to > cover identical ELx versions are another issue. > > I also note that while the XML is viewable on the web AFAICT the only > directly downloadable version of the architecture XML available > externally is in PDF format which is not entirely helpful for this > purpose. Sorry, I didn't mean to automate this in the tree, just that you could do it locally when you generate the patch (as I suspect this must be tedious for you to write out by hand too!). We've had a string of typos in the definitions so far, and it would be nice to take steps to avoid that for future changes. Will
On Thu, Apr 06, 2023 at 04:04:57PM +0100, Will Deacon wrote: > On Thu, Apr 06, 2023 at 04:02:18PM +0100, Mark Brown wrote: > > On Thu, Apr 06, 2023 at 03:46:54PM +0100, Will Deacon wrote: > > > Can't we generate this file from the architecture xml? That would hopefully > > > avoid typos like this and make review less tedious. > > I agree that this seems like a sensible idea however there has > > previously been pushback on the idea of providing tooling to do that, > > and we would want to manually integrate the output of any such tool > > since there are a number of cases where for legacy or usability reasons > > we rename or combine fields. The cases where we use a Fields block to > > cover identical ELx versions are another issue. > > I also note that while the XML is viewable on the web AFAICT the only > > directly downloadable version of the architecture XML available > > externally is in PDF format which is not entirely helpful for this > > purpose. > Sorry, I didn't mean to automate this in the tree, just that you could > do it locally when you generate the patch (as I suspect this must be > tedious for you to write out by hand too!). We've had a string of typos > in the definitions so far, and it would be nice to take steps to avoid > that for future changes. Yeah, normally it's not too bad (and it can be useful to make sure you've actually looked at the entire register definition properly) but I did just about get annoyed enough to write something locally while doing this register. You could certainly get a good chunk of the way there, especially for simple fields (enums would need manual naming IIRC) - if it hadn't been for the pushback mentioned above combined with what's on developer.arm.com I'd probably have got round to doing something already.
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index e5ca9ece1606..c48b41c9b0cc 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -419,7 +419,6 @@ #define SYS_MDCR_EL2 sys_reg(3, 4, 1, 1, 1) #define SYS_CPTR_EL2 sys_reg(3, 4, 1, 1, 2) #define SYS_HSTR_EL2 sys_reg(3, 4, 1, 1, 3) -#define SYS_HFGITR_EL2 sys_reg(3, 4, 1, 1, 6) #define SYS_HACR_EL2 sys_reg(3, 4, 1, 1, 7) #define SYS_TTBR0_EL2 sys_reg(3, 4, 2, 0, 0) diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg index 60829a9409f0..c06097a8b921 100644 --- a/arch/arm64/tools/sysreg +++ b/arch/arm64/tools/sysreg @@ -1941,6 +1941,71 @@ Sysreg HFGWTR_EL2 3 4 1 1 5 Fields HFGxTR_EL2 EndSysreg +Sysreg HFGITR_EL2 3 4 1 1 6 +Res0 63:61 +Field 60 COSPRCTX +Field 59 nGCSEPP +Field 58 nGCSSTR_EL1 +Field 57 nGCSPUSHM_EL1 +Field 56 nBRBIALL +Field 55 nBRBINJ +Field 54 DCCVAC +Field 53 SVC_EL1 +Field 52 SVC_EL0 +Field 51 ERET +Field 50 CPPRCTX +Field 49 DVPRCTX +Field 48 CFPRCTX +Field 47 TLBIVAALE1 +Field 46 TLBIVALE1 +Field 45 TLBIVAAE1 +Field 44 TLBSIDE1 +Field 43 TLBIVAE1 +Field 42 TLBIVMALLE1 +Field 41 TLBIRVAALE1 +Field 40 TLBIRVALE1 +Field 39 TLBIRVAAE1 +Field 38 TLBIRVAE1 +Field 37 TLBIRVAALE1IS +Field 36 TLBIRVALE1IS +Field 35 TLBIRVAAE1IS +Field 34 TLBIRAALE1IS +Field 33 TLBIVAALE1IS +Field 32 TLBIVALE1IS +Field 31 TLBIVAAE1IS +Field 30 TLBIASIDE1IS +Field 29 TLBIVAE1IS +Field 28 TLBIVVMALLE1IS +Field 27 TLBIRVAALE1OS +Field 26 TLBIRVALE1OS +Field 25 TLBIRVAAE1OS +Field 24 TLBIRVAE1OS +Field 23 TLBIVAALE1OS +Field 22 TLBIVALE1OS +Field 21 TLBIVAAE1OS +Field 20 TLVIASIDE1OS +Field 19 TLBIVAE1OS +Field 18 TLBIVMALLE1OS +Field 17 ATS1E1WP +Field 16 ATS1E1RP +Field 15 ATS1E0W +Field 14 ATS1E0R +Field 13 ATS1E1W +Field 12 ATS1E1R +Field 11 DCZVA +Field 10 DCCIVAC +Field 9 DCCVADP +Field 8 DCCVAP +Field 7 DCCVAU +Field 6 DCCISW +Field 5 DCCSW +Field 4 DCISW +Field 3 DCIVAC +Field 2 ICIVAU +Field 1 ICIALLU +Field 0 ICIALLUIS +EndSysreg + Sysreg ZCR_EL2 3 4 1 2 0 Fields ZCR_ELx EndSysreg
Automatically generate the Hypervisor Fine-Grained Instruction Trap Register as per DDI0601 2022-12, currently we only have a definition for the register name not any of the contents. No functional change. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/include/asm/sysreg.h | 1 - arch/arm64/tools/sysreg | 65 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-)