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

From: Mark Brown <broonie@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 4/8] arm64: Add sysreg header generation scripting
Date: Thu, 21 Apr 2022 16:46:49 +0100	[thread overview]
Message-ID: <YmF8aZuoAjukryR8@sirena.org.uk> (raw)
In-Reply-To: <YmF5sloTo4X5zAaT@lakrids>

[-- Attachment #1.1: Type: text/plain, Size: 2514 bytes --]

On Thu, Apr 21, 2022 at 04:35:14PM +0100, Mark Rutland wrote:
> On Thu, Apr 21, 2022 at 03:50:41PM +0100, Mark Brown wrote:

> > Until we need it that can just be equivalent to a comment, ready to kick
> > in once someone needs it.  I can update the existing (trivial but meh)
> > TTBRn register conversion to do that.

> FWIW, I'm happy either way (i.e. TTBRn to staty as-is, or be converted),
> given the duplication is trivial.

Yes, I don't think it's particularly *useful* to do this for TTBRn, but
it does demonstrate the mechanism which can then be applied to other
more interesting registers.

> > I was talking about a completely different issue, things like CPTR_EL2
> > where the register layout changes depending on some runtime
> > configuration.  You'd need two separate register layouts within a single
> > Sysreg which isn't too bad until you get to the point of having to name
> > things like RES0/RES1 and any name collisons in fields (I don't *think*
> > we have name collisons...).

> I understood that was a distinct problem; what I was suggesting is the
> same mechanism might help there, as e.g. it would allow us to do:

> 	# Creates CPTR_EL2__E2H1_RES0, etc
> 	SysregFields	CPTR_EL2__E2H1
> 		Comment	"CPTR_EL2 layout when HCR_EL2.E2H == 1"
> 		...
> 	EndSysregFields

...

> ... or where we have two names for the same register encoding (which is
> really silly, but does exist):

Right, that'd work but it has the disadvantage that we can't then
blindly use CPTR_EL2_ZEN_MASK or whatever.  That might be fine, it's a
small number of registers and could help someone spot being in the wrong
mode, but perhaps not.  The alternative is to teach the script about
multiple layouts for a single register.  I don't have a particularly
strong opinion there, and I think it's reasonable to punt until
converting the affected registers so people can look at concrete
suggestions and their impact on the relevant code.

> 	Sysreg	ICV_EOIR0_EL1	3	0	12	8	1
> 		Comment	"See ICx_EOIR0_EL1_*"
> 		NoFields
> 	EndSysreg
> 
> 	Sysreg	ICH_EOIR0_EL1	3	0	12	8	1
> 		Comment	"See ICx_EOIR0_EL1_*"
> 		NoFields
> 	EndSysreg
> 
> 	SysregFields	ICx_EOIR0_EL1
> 		Comment	"Shared layout for ICV_EOIR0_EL1 and ICH_EOIR0_EL1"
> 		Res0	63:24
> 		Field	23:0	INTID
> 	EndSysregFields

> I can believe there are problems with that, and/or maybe it's too ugly.

TBH that seems fine to me, at the level of generating the defines it's
just a shared layout.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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