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

[Marc Zyngier <maz@kernel.org>]

On Wed, 20 Nov 2024 08:54:45 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> 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
> """

Ah, fair enough. I had the misguided impression that there would be
another control for that, but this is actually called out in D13.11.2
("The PMU does not provide any control that a hypervisor can use to
reserve the cycle counter for its own use.").

I stand corrected.

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

Nah, that's definitely me.

FWIW, and with the follow-up fixes:

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

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