Re: [PATCH v2 3/8] KVM: arm64: PMU: Preserve vCPU's PMCR_EL0.N value on vCPU reset

[Marc Zyngier <maz@kernel.org>]

On Fri, 20 Jan 2023 00:30:33 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Mon, Jan 16, 2023 at 05:35:37PM -0800, Reiji Watanabe wrote:
> > The number of PMU event counters is indicated in PMCR_EL0.N.
> > For a vCPU with PMUv3 configured, its value will be the same as
> > the host value by default. Userspace can set PMCR_EL0.N for the
> > vCPU to a lower value than the host value using KVM_SET_ONE_REG.
> > However, it is practically unsupported, as reset_pmcr() resets
> > PMCR_EL0.N to the host value on vCPU reset.
> > 
> > Change reset_pmcr() to preserve the vCPU's PMCR_EL0.N value on
> > vCPU reset so that userspace can limit the number of the PMU
> > event counter on the vCPU.
> > 
> > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > ---
> >  arch/arm64/kvm/pmu-emul.c | 6 ++++++
> >  arch/arm64/kvm/sys_regs.c | 4 +++-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> > index 24908400e190..937a272b00a5 100644
> > --- a/arch/arm64/kvm/pmu-emul.c
> > +++ b/arch/arm64/kvm/pmu-emul.c
> > @@ -213,6 +213,12 @@ void kvm_pmu_vcpu_init(struct kvm_vcpu *vcpu)
> >  
> >  	for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++)
> >  		pmu->pmc[i].idx = i;
> > +
> > +	/*
> > +	 * Initialize PMCR_EL0 for the vCPU with the host value so that
> > +	 * the value is available at the very first vCPU reset.
> > +	 */
> > +	__vcpu_sys_reg(vcpu, PMCR_EL0) = read_sysreg(pmcr_el0);
> 
> I think we need to derive a sanitised value for PMCR_EL0.N, as I believe
> nothing in the architecture prevents implementers from gluing together
> cores with varying numbers of PMCs. We probably haven't noticed it yet
> since it would appear all Arm designs have had 6 PMCs.

This brings back the question of late onlining. How do you cope with
with the onlining of such a CPU that has a smaller set of counters
than its online counterparts? This is at odds with the way the PMU
code works.

If you have a different set of counters, you are likely to have a
different PMU altogether:

[    1.192606] hw perfevents: enabled with armv8_cortex_a57 PMU driver, 7 counters available
[    1.201254] hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7 counters available

This isn't a broken system, but it has two set of cores which are
massively different, and two PMUs.

This really should tie back to the PMU type we're counting on, and to
the set of CPUs that implements it. We already have some
infrastructure to check for the affinity of the PMU vs the CPU we're
running on, and this is already visible to userspace.

Can't we just leave this responsibility to userspace?

Thanks,

	M.

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