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

[Alexandre Chartre <alexandre.chartre@oracle.com>]

On 7/6/21 4:52 PM, Marc Zyngier wrote:
> 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.
> 

Right, I meant I will resubmit after -rc1 as you requested.

Thanks,

alex.