Re: [PATCH] KVM: arm64: Disabling disabled PMU counters wastes a lot of time

[Marc Zyngier <maz@kernel.org>]

On Tue, 06 Jul 2021 14:50:35 +0100,
Alexandre Chartre <alexandre.chartre@oracle.com> wrote:
> 
> 
> Hi Marc,
> 
> On 6/29/21 3:16 PM, Alexandre Chartre wrote:
> > On 6/29/21 11:06 AM, Marc Zyngier wrote
> > [...]
> >> So the sysreg is the only thing we should consider, and I think we
> >> should drop the useless masking. There is at least another instance of
> >> this in the PMU code (kvm_pmu_overflow_status()), and apart from
> >> kvm_pmu_vcpu_reset(), only the sysreg accessors should care about the
> >> masking to sanitise accesses.
> >> 
> >> What do you think?
> >> 
> > 
> > I think you are right. PMCNTENSET_EL0 is already masked with kvm_pmu_valid_counter_mask()
> > so there's effectively no need to mask it again when we use it. I will send an additional
> > patch (on top of this one) to remove useless masking. Basically, changes would be:
> 
> I had a closer look and we can't remove the mask. The access
> functions (for pmcnten, pminten, pmovs), clear or set only the
> specified valid counter bits. This means that bits other than the
> valid counter bits never change in __vcpu_sys_reg(), and those bits
> are not necessarily zero because the initial value is
> 0x1de7ec7edbadc0deULL (set by reset_unknown()).

That's a bug that should be fixed on its own. Bits that are RAZ/WI in
the architecture shouldn't be kept in the shadow registers the first
place. I'll have a look.

> So I will resubmit initial patch, with just the commit message
> changes.

Please don't. I'm not papering over this kind of bug.

Thanks,

	M.

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