[PATCH 1/7] KVM: arm64: PMU: Have reset_pmu_reg() to clear a register

[Reiji Watanabe]

Hi Oliver,

On Sun, Jan 8, 2023 at 11:07 AM Oliver Upton <oliver.upton at linux.dev> wrote:
>
> On Thu, Dec 29, 2022 at 07:59:22PM -0800, Reiji Watanabe wrote:
> > On vCPU reset, PMCNTEN{SET,CLR}_EL1 and PMOVS{SET,CLR}_EL1 for
> > a vCPU are reset by reset_pmu_reg(). This function clears RAZ bits
> > of those registers corresponding to unimplemented event counters
> > on the vCPU, and sets bits corresponding to implemented event counters
> > to a predefined pseudo UNKNOWN value (some bits are set to 1).
> >
> > The function identifies (un)implemented event counters on the
> > vCPU based on the PMCR_EL1.N value on the host. Using the host
> > value for this would be problematic when KVM supports letting
> > userspace set PMCR_EL1.N to a value different from the host value
> > (some of the RAZ bits of those registers could end up being set to 1).
> >
> > Fix reset_pmu_reg() to clear the registers so that it can ensure
> > that all the RAZ bits are cleared even when the PMCR_EL1.N value
> > for the vCPU is different from the host value.
> >
> > Signed-off-by: Reiji Watanabe <reijiw at google.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 10 +---------
> >  1 file changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index c6cbfe6b854b..ec4bdaf71a15 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -604,19 +604,11 @@ static unsigned int pmu_visibility(const struct kvm_vcpu *vcpu,
> >
> >  static void reset_pmu_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> >  {
> > -     u64 n, mask = BIT(ARMV8_PMU_CYCLE_IDX);
> > -
> >       /* 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;
> > -     n &= ARMV8_PMU_PMCR_N_MASK;
> > -     if (n)
> > -             mask |= GENMASK(n - 1, 0);
> > -
> > -     reset_unknown(vcpu, r);
> > -     __vcpu_sys_reg(vcpu, r->reg) &= mask;
> > +     __vcpu_sys_reg(vcpu, r->reg) = 0;
>
> I've personally found KVM's UNKNOWN reset value to be tremendously
> useful in debugging guest behavior, as seeing that value is quite a
> 'smoking gun' IMO.
>
> Rather than zeroing the entire register, is it possible to instead
> derive the mask based on what userspace wrote to PMCR_EL1.N instead of
> what's in hardware?

Yeah, it's feasible.
Then, I am considering having KVM to clear new RAZ bits of those
registers based on the new PMCR_EL0.N value that userspace sets
when userspace updates PMCR_EL0.N, as clearing the new RAZ bits
should be done anyway by either KVM or userspace.
What do you think ?

Thank you,
Reiji