Re: [PATCH v3 09/22] KVM: arm64: Correct kvm_arm_pmu_get_max_counters()

[Colton Lewis]

Marc Zyngier <maz@xxxxxxxxxx> writes:

> On Thu, 26 Jun 2025 21:04:45 +0100,
> Colton Lewis <coltonlewis@xxxxxxxxxx> wrote:

> > Since cntr_mask is modified when the PMU is partitioned to remove some
> > bits, make sure the missing counters are added back to get the right
> > total.

> Please fix the subject of the patch to be more descriptive. It is
> worded like a bug fix, while it really is only a step in the patch
> series.

> Something like "Take partitioning into account for max number of
> counters" would go a long way.

Done.


> > Signed-off-by: Colton Lewis <coltonlewis@xxxxxxxxxx>
> > ---
> >   arch/arm64/kvm/pmu.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)

> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> > index 79b7ea037153..67216451b8ce 100644
> > --- a/arch/arm64/kvm/pmu.c
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -533,6 +533,8 @@ static bool pmu_irq_is_valid(struct kvm *kvm, int  
> > irq)
> >   u8 kvm_arm_pmu_get_max_counters(struct kvm *kvm)
> >   {
> >   	struct arm_pmu *arm_pmu = kvm->arch.arm_pmu;
> > +	u8 counters;
> > +

> nit: superfluous blank line.

I'll remove.


> >   	/*
> >   	 * PMUv3 requires that all event counters are capable of counting any
> > @@ -545,7 +547,12 @@ u8 kvm_arm_pmu_get_max_counters(struct kvm *kvm)
> >   	 * The arm_pmu->cntr_mask considers the fixed counter(s) as well.
> >   	 * Ignore those and return only the general-purpose counters.
> >   	 */
> > -	return bitmap_weight(arm_pmu->cntr_mask,  
> > ARMV8_PMU_MAX_GENERAL_COUNTERS);
> > +	counters = bitmap_weight(arm_pmu->cntr_mask,  
> > ARMV8_PMU_MAX_GENERAL_COUNTERS);
> > +
> > +	if (kvm_pmu_is_partitioned(arm_pmu))
> > +		counters += arm_pmu->hpmn_max;

> Why the check? Why can't we rely on hpmn_max to always give us the
> correct value?

Imagine no partitioning. Then the value of hpmn_max that would be give a
correct result here is 0 but 0 is also a valid value (anything from 0 to
the number of host counters inclusive) indicating we are partitioned and
want to give only the cycle counter to the guest.

My predicate to determine if the PMU is partitoned relies on hpmn_max
being a valid value only when the PMU is partitioned.

Making hpmn_max 0 here would require introducing another flag to check
partitioning.

> Thanks,

> 	M.

> --
> Without deviation from the norm, progress is not possible.