Re: [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters

[Oliver Upton <oliver.upton@linux.dev>]

On Wed, Nov 20, 2024 at 08:38:22AM +0000, Marc Zyngier wrote:
> On Wed, 20 Nov 2024 00:52:30 +0000, Oliver Upton <oliver.upton@linux.dev> wrote:
> > +/*
> > + * Returns the PMU overflow state, which is true if there exists an event
> > + * counter where the values of the global enable control, PMOVSSET_EL0[n], and
> > + * PMINTENSET_EL1[n] are all 1.
> > + */
> > +static bool kvm_pmu_overflow_status(struct kvm_vcpu *vcpu)
> >  {
> > -	u64 reg = 0;
> > +	u64 reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0);
> >  
> > -	if ((kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) {
> > -		reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0);
> > -		reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1);
> > -	}
> > +	reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1);
> > +
> > +	/*
> > +	 * PMCR_EL0.E is the global enable control for event counters available
> > +	 * to EL0 and EL1.
> > +	 */
> > +	if (!(kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E))
> > +		reg &= kvm_pmu_hyp_counter_mask(vcpu);
> 
> So if the PMU is disabled at EL1, we remove the cycle counter from the
> list of counters that are considered for an overflow.
> 
> I don't think that's what you really want.

That's how it is written though, no? My understanding is that the fixed
counters are never reserved by EL2:

"""
The PE signals a request for:

[...]

 - The cycle counter, when the values of PMCR_EL0.E, PMOVSSET[31] and
   PMINTENSET[31] are all 1
"""

PMUOverflowCondition() and PMUCounterIsHyp() seem to match this too.
Unless I can't read, which happens a lot.

-- 
Thanks,
Oliver