Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation

From: Mark Brown <broonie@kernel.org>
To: Marc Zyngier <maz@kernel.org>
Cc: Akihiko Odaki <akihiko.odaki@daynix.com>,
	linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Oliver Upton <oliver.upton@linux.dev>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	James Morse <james.morse@arm.com>, Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	asahi@lists.linux.dev, Alyssa Rosenzweig <alyssa@rosenzweig.io>,
	Sven Peter <sven@svenpeter.dev>, Hector Martin <marcan@marcan.st>
Subject: Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
Date: Mon, 19 Dec 2022 17:52:06 +0000	[thread overview]
Message-ID: <Y6CkxueMwWPBMV7h@sirena.org.uk> (raw)
In-Reply-To: <86o7rzpg6i.wl-maz@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 1952 bytes --]

On Mon, Dec 19, 2022 at 03:27:17PM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > fully represent everything in the spec yet.  For things like the
> > registers with multiple possible views it's much more effort which
> > shouldn't get in the way of progress on features but with something like
> > this just updating the tool so we can match the architecture spec is the
> > right thing.

> I was tempted to add a Namespace tag that wouldn't generate the sysreg
> #defines, but only generate the fields with a feature-specific
> namespace. For example:

I think this is roughly where we'd end up - I was using the term view
when thinking about it but that's just bikeshed.

> Sysreg	CCSIDR_EL1	3	1	0	0	0
> Res0	63:32
> Unkn	31:28
> Field	27:13	NumSets
> Field	12:3	Associativity
> Field	2:0	LineSize
> EndSysreg
> 
> Namespace CCIDX CCSIDR_EL1
> Res0	63:56
> Field	55:32	NumSets
> Res0	31:25
> Field	24:3	Associativity
> Field	2:0	LineSize
> EndSysreg

Yeah, something like that.  I think we also want a way to label bits in
the root register as only existing in namespaces/views for things where
there's no default (eg, where a feature adds two views at once or things
have been there since the base architecture), and I wasn't sure if it
made sense to nest the declaration of the views inside the Sysreg (I'm
tempted to think it's more trouble than it's worth especially on the
tooling side).

I also wanted to go through and do an audit of all the current registers
to make sure there were no nasty cases that'd complicate things.  I
don't think there'd be anything but...

> the later generating:

> #define CCIDR_EL1_CCIDX_RES0		(GENMASK(63, 56) | GENMASK(31, 25))
> #define	CCIDR_EL1_CCIDX_NumSets		GENMASK(55, 32)
> #define	CCIDR_EL1_CCIDX_Associativity	GENMASK(24, 3)
> #define CCIDR_EL1_CCIDX_LineSize	GENMASK(2, 0)

> Thoughts?

Definitely that for the output.

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