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

[Alexandre Chartre]

On 7/6/21 7:36 PM, Marc Zyngier wrote:
> On Tue, 06 Jul 2021 15:52:46 +0100,
> Marc Zyngier <maz at kernel.org> wrote:
>>
>> On Tue, 06 Jul 2021 14:50:35 +0100,
>> Alexandre Chartre <alexandre.chartre at 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.
> 
> Please try this[1] for size, which is on top of Linus' tree as of this
> morning.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pmu/reset-values
> 

I have tried [1] and it works fine. Also tested with my patch on top (without
masking) and it also works fine.

Thanks,

alex.