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] [thread-next>] [day] [month] [year] [list]

Message-ID: <26d69b0d-3529-454f-9385-99a914bf1ebc@sirena.org.uk>
Date: Tue, 7 Oct 2025 17:28:12 +0100
From: Mark Brown <broonie@...nel.org>
To: Sascha Bischoff <Sascha.Bischoff@....com>
Cc: "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, nd <nd@....com>,
	Mark Rutland <Mark.Rutland@....com>,
	Catalin Marinas <Catalin.Marinas@....com>,
	"maz@...nel.org" <maz@...nel.org>,
	"oliver.upton@...ux.dev" <oliver.upton@...ux.dev>,
	Joey Gouly <Joey.Gouly@....com>,
	Suzuki Poulose <Suzuki.Poulose@....com>,
	"yuzenghui@...wei.com" <yuzenghui@...wei.com>,
	"will@...nel.org" <will@...nel.org>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>
Subject: Re: [PATCH 1/3] arm64/sysreg: Support feature-specific fields with
 'Feat' descriptor

On Tue, Oct 07, 2025 at 03:35:14PM +0000, Sascha Bischoff wrote:

> Some system register field encodings change based on the available and
> in-use architecture features. In order to support these different
> field encodings, introduce the Feat descriptor (Feat, ElseFeat,
> EndFeat) for describing such sysregs.

We have other system registers which change layouts based on things
other than features, ESR_ELx is probably the most entertaining of these
but there's others like PAR_EL1.  Ideally we should probably do the
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.

> 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.  I'm not sure I would want to assume that, even for plain
features though I can't think of any examples where it's an issue.
There are more serious issues with the implementation for practical
patterns with nesting (see below) which mean we might not want to deal
with that now but we should define the syntax for the file in a way that
will cope so I'd prefer not to have the implicit elses.

I'd also be inclined to say that something that's specificly for
features shouldn't repeat the FEAT_ so:

          Feat    A

instead, but that's purely a taste question.

> -	define("REG_" reg, "S" op0 "_" op1 "_C" crn "_C" crm "_" op2)
> -	define("SYS_" reg, "sys_reg(" op0 ", " op1 ", " crn ", " crm ", " op2 ")")
> +	feat = null
> +
> +	define(feat, "REG_" reg, "S" op0 "_" op1 "_C" crn "_C" crm "_" op2)
> +	define(feat, "SYS_" reg, "sys_reg(" op0 ", " op1 ", " crn ", " crm ", " op2 ")")
>  
> -	define("SYS_" reg "_Op0", op0)
> -	define("SYS_" reg "_Op1", op1)
> -	define("SYS_" reg "_CRn", crn)
> -	define("SYS_" reg "_CRm", crm)
> -	define("SYS_" reg "_Op2", op2)
> +	define(feat, "SYS_" reg "_Op0", op0)
> +	define(feat, "SYS_" reg "_Op1", op1)
> +	define(feat, "SYS_" reg "_CRn", crn)
> +	define(feat, "SYS_" reg "_CRm", crm)
> +	define(feat, "SYS_" reg "_Op2", op2)

Possibly it's worth having a reg_define() or something given the number
of things needing updating here?

> @@ -201,6 +205,7 @@ $1 == "EndSysreg" && block_current() == "Sysreg" {
>  	res0 = null
>  	res1 = null
>  	unkn = null
> +	feat = null
>  
>  	block_pop()
>  	next

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

> +$1 == "Feat" && (block_current() == "Sysreg" || block_current() == "SysregFields" || block_current() == "Feat") {

...

> +	next_bit = 63
> +
> +	res0 = "UL(0)"
> +	res1 = "UL(0)"
> +	unkn = "UL(0)"

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

        Sysreg  EXAMPLE 0    1    2    3    4
	Res0	63
        Feat    FEAT_A
        Field   62:1    Foo
        Feat    FEAT_B
        Field   62:32    Foo
        Field   31:1     Bar
	EndFeat
	Res0	0
	EndSysreg

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.

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