All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Upton <oliver.upton@linux.dev>
To: Colton Lewis <coltonlewis@google.com>
Cc: Ben Horgan <ben.horgan@arm.com>,
	kvm@vger.kernel.org, pbonzini@redhat.com, corbet@lwn.net,
	linux@armlinux.org.uk, catalin.marinas@arm.com, will@kernel.org,
	maz@kernel.org, mizhang@google.com, joey.gouly@arm.com,
	suzuki.poulose@arm.com, yuzenghui@huawei.com,
	mark.rutland@arm.com, shuah@kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-perf-users@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums
Date: Fri, 27 Jun 2025 13:55:25 -0700	[thread overview]
Message-ID: <aF8FPTupC_VnAldN@linux.dev> (raw)
In-Reply-To: <gsntecv49aml.fsf@coltonlewis-kvm.c.googlers.com>

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/

  reply	other threads:[~2025-06-27 20:55 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-26 20:04 [PATCH v3 00/22] ARM64 PMU Partitioning Colton Lewis
2025-06-26 20:04 ` [PATCH v3 01/22] arm64: cpufeature: Add cpucap for HPMN0 Colton Lewis
2025-07-07 16:05   ` Mark Rutland
2025-07-08 22:34     ` Colton Lewis
2025-06-26 20:04 ` [PATCH v3 02/22] arm64: Generate sign macro for sysreg Enums Colton Lewis
2025-06-27  9:04   ` Ben Horgan
2025-06-27 20:45     ` Colton Lewis
2025-06-27 20:55       ` Oliver Upton [this message]
2025-06-30 17:42         ` Colton Lewis
2025-06-27 13:23   ` Marc Zyngier
2025-07-07 16:07   ` Mark Rutland
2025-06-26 20:04 ` [PATCH v3 03/22] KVM: arm64: Define PMI{CNTR,FILTR}_EL0 as undef_access Colton Lewis
2025-06-27 13:31   ` Marc Zyngier
2025-06-27 20:45     ` Colton Lewis
2025-06-26 20:04 ` [PATCH v3 04/22] KVM: arm64: Cleanup PMU includes Colton Lewis
2025-07-07 16:13   ` Mark Rutland
2025-07-08 22:37     ` Colton Lewis
2025-06-26 20:04 ` [PATCH v3 05/22] KVM: arm64: Reorganize PMU functions Colton Lewis
2025-06-26 20:04 ` [PATCH v3 06/22] perf: arm_pmuv3: Introduce method to partition the PMU Colton Lewis
2025-07-07 16:57   ` Mark Rutland
2025-07-07 19:07     ` Oliver Upton
2025-07-08 22:38       ` Colton Lewis
2025-07-08 22:41         ` Oliver Upton
2025-06-26 20:04 ` [PATCH v3 07/22] perf: arm_pmuv3: Generalize counter bitmasks Colton Lewis
2025-07-07 16:58   ` Mark Rutland
2025-07-08 22:38     ` Colton Lewis
2025-06-26 20:04 ` [PATCH v3 08/22] perf: arm_pmuv3: Keep out of guest counter partition Colton Lewis
2025-06-26 20:04 ` [PATCH v3 09/22] KVM: arm64: Correct kvm_arm_pmu_get_max_counters() Colton Lewis
2025-06-27 13:36   ` Marc Zyngier
2025-06-30 17:42     ` Colton Lewis
2025-06-26 20:04 ` [PATCH v3 10/22] KVM: arm64: Set up FGT for Partitioned PMU Colton Lewis
2025-06-27 15:01   ` Marc Zyngier
2025-06-27 20:45     ` Colton Lewis
2025-06-28  8:25       ` Marc Zyngier
2025-06-26 20:04 ` [PATCH v3 11/22] KVM: arm64: Writethrough trapped PMEVTYPER register Colton Lewis
2025-06-26 20:04 ` [PATCH v3 12/22] KVM: arm64: Use physical PMSELR for PMXEVTYPER if partitioned Colton Lewis
2025-06-26 20:04 ` [PATCH v3 13/22] KVM: arm64: Writethrough trapped PMOVS register Colton Lewis
2025-06-26 20:04 ` [PATCH v3 14/22] KVM: arm64: Write fast path PMU register handlers Colton Lewis
2025-06-26 20:04 ` [PATCH v3 15/22] KVM: arm64: Setup MDCR_EL2 to handle a partitioned PMU Colton Lewis
2025-06-26 20:04 ` [PATCH v3 16/22] KVM: arm64: Account for partitioning in PMCR_EL0 access Colton Lewis
2025-06-26 20:04 ` [PATCH v3 17/22] KVM: arm64: Context swap Partitioned PMU guest registers Colton Lewis
2025-06-26 20:04 ` [PATCH v3 18/22] KVM: arm64: Enforce PMU event filter at vcpu_load() Colton Lewis
2025-06-26 20:04 ` [PATCH v3 19/22] perf: arm_pmuv3: Handle IRQs for Partitioned PMU guest counters Colton Lewis
2025-06-26 20:04 ` [PATCH v3 20/22] KVM: arm64: Inject recorded guest interrupts Colton Lewis
2025-06-26 20:04 ` [PATCH v3 21/22] KVM: arm64: Add ioctl to partition the PMU when supported Colton Lewis
2025-06-26 20:04 ` [PATCH v3 22/22] KVM: arm64: selftests: Add test case for partitioned PMU Colton Lewis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aF8FPTupC_VnAldN@linux.dev \
    --to=oliver.upton@linux.dev \
    --cc=ben.horgan@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=coltonlewis@google.com \
    --cc=corbet@lwn.net \
    --cc=joey.gouly@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=mizhang@google.com \
    --cc=pbonzini@redhat.com \
    --cc=shuah@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.