linux-kernel - Re: [PATCH 1/3] arm64/sysreg: Support feature-specific fields with 'Feat' descriptor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5ac755f3-d7e4-421a-85bd-43d7aa7fbfde@sirena.org.uk>
Date: Thu, 9 Oct 2025 13:42:47 +0100
From: Mark Brown <broonie@...nel.org>
To: Sascha Bischoff <Sascha.Bischoff@....com>
Cc: "yuzenghui@...wei.com" <yuzenghui@...wei.com>,
	Joey Gouly <Joey.Gouly@....com>,
	Suzuki Poulose <Suzuki.Poulose@....com>, nd <nd@....com>,
	Mark Rutland <Mark.Rutland@....com>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	"kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	Catalin Marinas <Catalin.Marinas@....com>,
	"maz@...nel.org" <maz@...nel.org>,
	"oliver.upton@...ux.dev" <oliver.upton@...ux.dev>,
	"will@...nel.org" <will@...nel.org>
Subject: Re: [PATCH 1/3] arm64/sysreg: Support feature-specific fields with
 'Feat' descriptor

On Thu, Oct 09, 2025 at 09:48:36AM +0000, Sascha Bischoff wrote:
> On Tue, 2025-10-07 at 17:28 +0100, Mark Brown wrote:

> > logic for generating the conditionals in a manner that's not tied to
> > features with a layer on top that generates standard naming for
> > common
> > patterns like FEAT, but OTOH part of why that's not been done because
> > it's got a bunch of nasty issues so perhaps just doing the simpler
> > case
> > is fine.

> I agree that there are plenty of cases where sysreg fields change based
> on more than just architectural features. Whilst my intent for this
> change wasn't strictly to cover all of those cases, it could arguably
> be used for that. Effectively, all this change is doing is to add a
> prefix to the field definitions so it can be used however works best.

Yeah, I don't know that we need to cover all the cases in the user
visible syntax right now - my thought was more about the code.

> It does feel as if Feat was potentially the wrong name to choose.
> Something like Fieldset, Prefix, AltLayout, etc might be better. I'm
> rather open to suggestions here - naming is hard!

It is :)

> > > The Feat descriptor can be used in the following way (Feat acts as
> > > both an if and an else-if):

> > >         Sysreg  EXAMPLE 0    1    2    3    4
> > >         Feat    FEAT_A
> > >         Field   63:0    Foo
> > >         Feat    FEAT_B

> > This assumes that there will never be nesting of these conditions in
> > the architecture.

> My thinking was that for something like that one could do:

>         Feat    FEAT_A_B

> Or similar. Yes, it means that anything that is nested needs to be
> flattened, but it does allow one to describe that situation. In the
> end, all this effectively does is to add a prefix, and said prefix can
> be anything. Double so if we move away from calling it Feat, and use
> something more generic instead.

You could definitely do it that way, and for the limited implementation
you're proposing it probably works as well.  My thinking here is about
what happens with the syntax when someone comes along and implements
nesting of fatures - the existing syntax would be broken since you
couldn't distinguish between an implicit else and nested FEAT_.

> > Probably worth complaining if we end a sysreg with a feature/prefix
> > defined.

> This is already be caught as we hit the EndSysreg directive while still
> in the Feat block:

> Error at 4690: unhandled statement
> Error at 4690: Missing terminator for Feat block

> I can be more specific in catching that, if you like.

No, it's fine so long sa there's a warning.

> > This is only going to work if the whole register is in a FEAT_ block,
> > so
> > you coudn't have:

...

> > but then supporting partial registers does have entertaining
> > consequences for handling Res0 and Res1.  If we're OK with that
> > restriction then the program should complain if someone tries to 
> > define a smaller FEAT block.

> It was my intent to only support whole registers here. Anything else
> gets insanely complex rather quickly, and much of the benefit gets lost
> IMO. By only operating on whole Sysregs (or SysregFields) we are able
> to re-use the existing logic to ensure that definitions are complete,
> and the generator's state tracking remains simple. And, as you say, it
> would break the ResX, Unkn side of things if we did anything else.

> When I add the missing check I mentioned above, then it will complain
> if fewer than 64 bits are in a defined block.

Yes, I think it's a reasonable simplification for the implementation and
so long as the syntax in the file can cope with someone doing the more
complicated cases it seems fine to leave them for later.  It's not like
it's a small bit of extra work it'd be easy to tack on, there's some
thorny issues there.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)