Re: [PATCH v4 4/8] arm64: Add sysreg header generation scripting

[Mark Rutland <mark.rutland@arm.com>]

On Thu, Apr 21, 2022 at 02:00:17PM +0100, Mark Brown wrote:
> On Thu, Apr 21, 2022 at 10:47:42AM +0100, Mark Rutland wrote:
> > On Tue, Apr 19, 2022 at 11:43:25AM +0100, Mark Brown wrote:
> 
> > > | #define ID_AA64ISAR0_EL1_RNDR                   ARM64_SYSREG_BITMASK(63, 60)
> > > | #define ID_AA64ISAR0_EL1_RNDR_MASK              ARM64_SYSREG_BITMASK(63, 60)
> 
> > I think this got missed when s/ARM64_SYSREG_BITMASK()/GENMASK_ULL()/ happened.
> 
> Yes.
> 
> > > | #define ID_AA64ISAR0_EL1_RNDR_SHIFT             60
> > > | #define ID_AA64ISAR0_EL1_RNDR_WIDTH             4
> > > | #define ID_AA64ISAR0_EL1_RNDR_NI                ULL(0b0000)
> > > | #define ID_AA64ISAR0_EL1_RNDR_IMP               ULL(0b0001)
> 
> > Just to check, was there a reason for going for ULL() and GENMASK_ULL() rather
> > than UL() and GENMASK()?
> 
> > We generally use UL() today, since we treat `unsigned long` as the native
> > register size.
> 
> That's not been updated from what you originally had had

I think it was? The version in my arm64/sysreg-scripting branch doesn't
use either UL() or ULL(), and its example has:

| #define ID_AA64ISAR1_EL1_I8MM                   BITMASK(55, 52)
| #define ID_AA64ISAR1_EL1_I8MM_SHIFT             52
| #define ID_AA64ISAR1_EL1_I8MM_WIDTH             4
| #define ID_AA64ISAR1_EL1_I8MM_NI                0b0000
| #define ID_AA64ISAR1_EL1_I8MM_SUPPORTED         0b0001

> , I think I'd just been under the impression that there was a good
> reason for it that wasn't apparent to me.

FWIW, I have no strong opinion either way, so I'm happy for that to stay
as ULL and GENMASK_ULL(); I just wanted to check if there was a
rationale I'd missed.

> > > The script requires that all bits in the register be specified and that
> > > there be no overlapping fields. This helps the script spot errors in the
> > > input but means that the few registers which change layout at runtime
> > > depending on things like virtualisation settings will need some manual
> > > handling. No actual register conversions are done here but a header for
> > > the register data with some documention of the format is provided.
> 
> > It would be good to see an example of how we'd handle one of those, in case
> > that means we need to play around with naming or structure of the definitions a
> > bit.
> 
> My thinking here was that we might not want to handle those registers
> through the automated stuff at all.  I haven't yet come up with
> something that seems tasteful and viable for them, if I had a firm idea
> for what that should look like I'd probably have implemented it.

Sure, and I'm not expecting that we automate all of that, just that we
have an idea of how the manual bits would work with the automatic bits.
If the odd cases looks simple enough, we might be able to get away with
a couple of small additions to the scripting.

For example, for ESR_EL{1,2,3} today we define ESR_ELx_field
definitions rather than duplicate ESR_EL1_field / ESR_EL2_field /
ESR_EL3_field definitions. If the scripting has a mechanism to handle
that, then that might be good enough for the other odd cases.

For example, I think we could do something like:

# Define a set of fields without a specific register encoding, using the
# name `ESR_ELx`
SysregFields	ESR_ELx
	Res0	63:37
	Field	36:32	ISS2
	Field	31:26	EC
	Field	25	IL
	Field	24:0	ISS
EndSysregFields

# This could instead be SysregEncoding
Sysreg	ESR_EL1		3	0	5	2	0
	Comment		"See ESR_ELx for fields"
	# Don't create any field definitions for this reg, and don't
	# bother with the associated sanity checks
	NoFields	
EndSysreg

Sysreg	ESR_EL2		3	4	5	2	0
	Comment		"See ESR_ELx for fields"
	NoFields
EndSysreg

Sysreg	ESR_EL12	3	5	5	2	0
	Comment		"See ESR_ELx for fields"
	NoFields
EndSysreg

... and the `SysregFields` `NoFields`, and `Comment` mechanisms might be
good enough to cover the other odd cases we have (e.g. aliased
GIC registers, or different "views" for the same register).

Does that make sense, or have I misunderstood the point you were making?

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel