1. Patchew
  2. linux
  3. [PATCH v3 00/22] ARM64 PMU Partitioning
  4. View patch

[PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums

Colton Lewis posted 22 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Posted by Colton Lewis 3 months, 1 week ago 
There's no reason Enums shouldn't be equivalent to UnsignedEnums and
explicitly specify they are unsigned. This will avoid the annoyance I
had with HPMN0.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 arch/arm64/tools/gen-sysreg.awk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
index f2a1732cb1f6..fa21a632d9b7 100755
--- a/arch/arm64/tools/gen-sysreg.awk
+++ b/arch/arm64/tools/gen-sysreg.awk
@@ -308,6 +308,7 @@ $1 == "Enum" && (block_current() == "Sysreg" || block_current() == "SysregFields
 	parse_bitdef(reg, field, $2)
 
 	define_field(reg, field, msb, lsb)
+	define_field_sign(reg, field, "false")
 
 	next
 }
-- 
2.50.0.727.gbf7dc18ff4-goog
Re: [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Posted by Mark Rutland 3 months ago 
On Thu, Jun 26, 2025 at 08:04:38PM +0000, Colton Lewis wrote:
> There's no reason Enums shouldn't be equivalent to UnsignedEnums and
> explicitly specify they are unsigned. This will avoid the annoyance I
> had with HPMN0.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>

I realise you've already agreed to drop this, so the below is mostly for
the sake of the archive.

We deliberately kept Enum separate from SignedEnum and UnsignedEnum as
there are enumerated ID fields which have no ordering, and are neither
signed nor unsigned (e.g. CTR_EL0.L1Ip).

That was discussed previously at:

  https://lore.kernel.org/all/Y5MrVC3d8MPhvshE@FVFF77S0Q05N/

We could consider adding a comment to that effect in gen-sysreg.awk, but
that needn't be part of this series.

Mark.

> ---
>  arch/arm64/tools/gen-sysreg.awk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
> index f2a1732cb1f6..fa21a632d9b7 100755
> --- a/arch/arm64/tools/gen-sysreg.awk
> +++ b/arch/arm64/tools/gen-sysreg.awk
> @@ -308,6 +308,7 @@ $1 == "Enum" && (block_current() == "Sysreg" || block_current() == "SysregFields
>  	parse_bitdef(reg, field, $2)
>  
>  	define_field(reg, field, msb, lsb)
> +	define_field_sign(reg, field, "false")
>  
>  	next
>  }
> -- 
> 2.50.0.727.gbf7dc18ff4-goog
>
Re: [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Posted by Marc Zyngier 3 months, 1 week ago 
On Thu, 26 Jun 2025 21:04:38 +0100,
Colton Lewis <coltonlewis@google.com> wrote:
> 
> There's no reason Enums shouldn't be equivalent to UnsignedEnums and
> explicitly specify they are unsigned. This will avoid the annoyance I
> had with HPMN0.

And randomly break unsuspecting cases for which a value is neither
signed not unsigned, but an actual *enumeration*.

If you have issues with HPMN0, start by explaining those, because
ranting in a commit message doesn't help much.

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Posted by Ben Horgan 3 months, 1 week ago 
Hi Colton,

On 6/26/25 21:04, Colton Lewis wrote:
> There's no reason Enums shouldn't be equivalent to UnsignedEnums and
> explicitly specify they are unsigned. This will avoid the annoyance I
> had with HPMN0.
An Enum can be annotated with the field's sign by updating it to 
UnsignedEnum or SignedEnum. This is explained in [1].

With this change ID_AA64PFR1_EL1.MTE_frac would be marked as unsigned 
when it should really be considered signed.

Enum	43:40	MTE_frac
	0b0000	ASYNC
	0b1111	NI
EndEnum


> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>   arch/arm64/tools/gen-sysreg.awk | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
> index f2a1732cb1f6..fa21a632d9b7 100755
> --- a/arch/arm64/tools/gen-sysreg.awk
> +++ b/arch/arm64/tools/gen-sysreg.awk
> @@ -308,6 +308,7 @@ $1 == "Enum" && (block_current() == "Sysreg" || block_current() == "SysregFields
>   	parse_bitdef(reg, field, $2)
>   
>   	define_field(reg, field, msb, lsb)
> +	define_field_sign(reg, field, "false")
>   
>   	next
>   }

Thanks,

Ben

[1] 
https://lore.kernel.org/all/20221207-arm64-sysreg-helpers-v4-1-25b6b3fb9d18@kernel.org/
Re: [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Posted by Colton Lewis 3 months, 1 week ago 
Hi Ben. Thanks for the review.

Ben Horgan <ben.horgan@arm.com> writes:

> Hi Colton,

> On 6/26/25 21:04, Colton Lewis wrote:
>> There's no reason Enums shouldn't be equivalent to UnsignedEnums and
>> explicitly specify they are unsigned. This will avoid the annoyance I
>> had with HPMN0.
> An Enum can be annotated with the field's sign by updating it to
> UnsignedEnum or SignedEnum. This is explained in [1].

> With this change ID_AA64PFR1_EL1.MTE_frac would be marked as unsigned
> when it should really be considered signed.

> Enum	43:40	MTE_frac
> 	0b0000	ASYNC
> 	0b1111	NI
> EndEnum

Thanks for the explanation. I made this a separate commit because I
considered people might object and HPMN0 is already an UnsignedEnum in
my previous commit.

Do you think it would be a good idea to make plain Enums signed by
default or should I just remove this commit from the series?


>> Signed-off-by: Colton Lewis <coltonlewis@google.com>
>> ---
>>    arch/arm64/tools/gen-sysreg.awk | 1 +
>>    1 file changed, 1 insertion(+)

>> diff --git a/arch/arm64/tools/gen-sysreg.awk  
>> b/arch/arm64/tools/gen-sysreg.awk
>> index f2a1732cb1f6..fa21a632d9b7 100755
>> --- a/arch/arm64/tools/gen-sysreg.awk
>> +++ b/arch/arm64/tools/gen-sysreg.awk
>> @@ -308,6 +308,7 @@ $1 == "Enum" && (block_current() == "Sysreg" ||  
>> block_current() == "SysregFields
>>    	parse_bitdef(reg, field, $2)

>>    	define_field(reg, field, msb, lsb)
>> +	define_field_sign(reg, field, "false")

>>    	next
>>    }

> Thanks,

> Ben

> [1]
> https://lore.kernel.org/all/20221207-arm64-sysreg-helpers-v4-1-25b6b3fb9d18@kernel.org/
Re: [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Posted by Oliver Upton 3 months, 1 week ago 
On Fri, Jun 27, 2025 at 08:45:38PM +0000, Colton Lewis wrote:
> Hi Ben. Thanks for the review.
> 
> Ben Horgan <ben.horgan@arm.com> writes:
> 
> > Hi Colton,
> 
> > On 6/26/25 21:04, Colton Lewis wrote:
> > > There's no reason Enums shouldn't be equivalent to UnsignedEnums and
> > > explicitly specify they are unsigned. This will avoid the annoyance I
> > > had with HPMN0.
> > An Enum can be annotated with the field's sign by updating it to
> > UnsignedEnum or SignedEnum. This is explained in [1].
> 
> > With this change ID_AA64PFR1_EL1.MTE_frac would be marked as unsigned
> > when it should really be considered signed.
> 
> > Enum	43:40	MTE_frac
> > 	0b0000	ASYNC
> > 	0b1111	NI
> > EndEnum
> 
> Thanks for the explanation. I made this a separate commit because I
> considered people might object and HPMN0 is already an UnsignedEnum in
> my previous commit.
> 
> Do you think it would be a good idea to make plain Enums signed by
> default or should I just remove this commit from the series?

It is presumptive to associate a sign with an enumeration. Generally
speaking, the only fields that can do signed / unsigned comparisons are
the Feature ID register fields.

So please drop this and only keep the change for HPMN0.

Thanks,
Oliver

> > > Signed-off-by: Colton Lewis <coltonlewis@google.com>
> > > ---
> > >    arch/arm64/tools/gen-sysreg.awk | 1 +
> > >    1 file changed, 1 insertion(+)
> 
> > > diff --git a/arch/arm64/tools/gen-sysreg.awk
> > > b/arch/arm64/tools/gen-sysreg.awk
> > > index f2a1732cb1f6..fa21a632d9b7 100755
> > > --- a/arch/arm64/tools/gen-sysreg.awk
> > > +++ b/arch/arm64/tools/gen-sysreg.awk
> > > @@ -308,6 +308,7 @@ $1 == "Enum" && (block_current() == "Sysreg" ||
> > > block_current() == "SysregFields
> > >    	parse_bitdef(reg, field, $2)
> 
> > >    	define_field(reg, field, msb, lsb)
> > > +	define_field_sign(reg, field, "false")
> 
> > >    	next
> > >    }
> 
> > Thanks,
> 
> > Ben
> 
> > [1]
> > https://lore.kernel.org/all/20221207-arm64-sysreg-helpers-v4-1-25b6b3fb9d18@kernel.org/
Re: [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Posted by Colton Lewis 3 months, 1 week ago 
Oliver Upton <oliver.upton@linux.dev> writes:

> On Fri, Jun 27, 2025 at 08:45:38PM +0000, Colton Lewis wrote:
>> Hi Ben. Thanks for the review.

>> Ben Horgan <ben.horgan@arm.com> writes:

>> > Hi Colton,

>> > On 6/26/25 21:04, Colton Lewis wrote:
>> > > There's no reason Enums shouldn't be equivalent to UnsignedEnums and
>> > > explicitly specify they are unsigned. This will avoid the annoyance I
>> > > had with HPMN0.
>> > An Enum can be annotated with the field's sign by updating it to
>> > UnsignedEnum or SignedEnum. This is explained in [1].

>> > With this change ID_AA64PFR1_EL1.MTE_frac would be marked as unsigned
>> > when it should really be considered signed.

>> > Enum	43:40	MTE_frac
>> > 	0b0000	ASYNC
>> > 	0b1111	NI
>> > EndEnum

>> Thanks for the explanation. I made this a separate commit because I
>> considered people might object and HPMN0 is already an UnsignedEnum in
>> my previous commit.

>> Do you think it would be a good idea to make plain Enums signed by
>> default or should I just remove this commit from the series?

> It is presumptive to associate a sign with an enumeration. Generally
> speaking, the only fields that can do signed / unsigned comparisons are
> the Feature ID register fields.

> So please drop this and only keep the change for HPMN0.

Done.


> Thanks,
> Oliver

>> > > Signed-off-by: Colton Lewis <coltonlewis@google.com>
>> > > ---
>> > >    arch/arm64/tools/gen-sysreg.awk | 1 +
>> > >    1 file changed, 1 insertion(+)

>> > > diff --git a/arch/arm64/tools/gen-sysreg.awk
>> > > b/arch/arm64/tools/gen-sysreg.awk
>> > > index f2a1732cb1f6..fa21a632d9b7 100755
>> > > --- a/arch/arm64/tools/gen-sysreg.awk
>> > > +++ b/arch/arm64/tools/gen-sysreg.awk
>> > > @@ -308,6 +308,7 @@ $1 == "Enum" && (block_current() == "Sysreg" ||
>> > > block_current() == "SysregFields
>> > >    	parse_bitdef(reg, field, $2)

>> > >    	define_field(reg, field, msb, lsb)
>> > > +	define_field_sign(reg, field, "false")

>> > >    	next
>> > >    }

>> > Thanks,

>> > Ben

>> > [1]
>> >  
>> https://lore.kernel.org/all/20221207-arm64-sysreg-helpers-v4-1-25b6b3fb9d18@kernel.org/

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.