[PATCH 1/3] KVM: arm64: Narrow PMU sysreg reset values to architectural requirements

[Marc Zyngier]

On Thu, 15 Jul 2021 12:51:49 +0100,
Robin Murphy <robin.murphy at arm.com> wrote:
> 
> On 2021-07-15 12:11, Marc Zyngier wrote:
> > Hi Alex,
> > 
> > On Wed, 14 Jul 2021 16:48:07 +0100,
> > Alexandru Elisei <alexandru.elisei at arm.com> wrote:
> >> 
> >> Hi Marc,
> >> 
> >> On 7/13/21 2:58 PM, Marc Zyngier wrote:
> >>> A number of the PMU sysregs expose reset values that are not in
> >>> compliant with the architecture (set bits in the RES0 ranges,
> >>> for example).
> >>> 
> >>> This in turn has the effect that we need to pointlessly mask
> >>> some register when using them.
> >>> 
> >>> Let's start by making sure we don't have illegal values in the
> >>> shadow registers at reset time. This affects all the registers
> >>> that dedicate one bit per counter, the counters themselves,
> >>> PMEVTYPERn_EL0 and PMSELR_EL0.
> >>> 
> >>> Reported-by: Alexandre Chartre <alexandre.chartre at oracle.com>
> >>> Signed-off-by: Marc Zyngier <maz at kernel.org>
> >>> ---
> >>>   arch/arm64/kvm/sys_regs.c | 46 ++++++++++++++++++++++++++++++++++++---
> >>>   1 file changed, 43 insertions(+), 3 deletions(-)
> >>> 
> >>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> >>> index f6f126eb6ac1..95ccb8f45409 100644
> >>> --- a/arch/arm64/kvm/sys_regs.c
> >>> +++ b/arch/arm64/kvm/sys_regs.c
> >>> @@ -603,6 +603,44 @@ static unsigned int pmu_visibility(const struct kvm_vcpu *vcpu,
> >>>   	return REG_HIDDEN;
> >>>   }
> >>>   +static void reset_pmu_reg(struct kvm_vcpu *vcpu, const struct
> >>> sys_reg_desc *r)
> >>> +{
> >>> +	u64 n, mask;
> >>> +
> >>> +	/* No PMU available, any PMU reg may UNDEF... */
> >>> +	if (!kvm_arm_support_pmu_v3())
> >>> +		return;
> >>> +
> >>> +	n = read_sysreg(pmcr_el0) >> ARMV8_PMU_PMCR_N_SHIFT;
> >> 
> >> Isn't this going to cause a lot of unnecessary traps with NV? Is
> >> that going to be a problem?
> > 
> > We'll get a new traps at L2 VM creation if we expose a PMU to the L1
> > guest, and if L2 gets one too. I don't think that's a real problem, as
> > the performance of an L2 PMU is bound to be hilarious, and if we are
> > really worried about that, we can always cache it locally. Which is
> > likely the best thing to do if you think of big-little.
> > 
> > Let's not think of big-little.
> > 
> > Another thing is that we could perfectly ignore the number of counter
> > on the host and always expose the architectural maximum, given that
> > the PMU is completely emulated. With that, no trap.
> 
> Although that would deliberately exacerbate the existing problem of
> guest counters mysteriously under-reporting due to the host event
> getting multiplexed, thus arguably make the L2 PMU even less useful.

Oh, absolutely. But the current implementation of the PMU emulation
would be pretty terrible on NV anyway.

> But then trying to analyse application performance under NV at all
> seems to stand a high chance of being akin to shovelling fog, so...

Indeed. Not to mention that there is no (publicly available) HW to
measure performance on anyway...

	M.

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