Re: [PATCH v3 2/2] arm64/sysreg: Convert HFGITR_EL2 to automatic generation

From: Will Deacon
Date: Thu Apr 06 2023 - 11:05:33 EST


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