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

From: Alexandre Chartre <alexandre.chartre@oracle.com>
To: Marc Zyngier <maz@kernel.org>
Cc: will@kernel.org, catalin.marinas@arm.com,
	alexandru.elisei@arm.com, james.morse@arm.com,
	suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	konrad.wilk@oracle.com, alexandre.chartre@oracle.com
Subject: Re: [PATCH] KVM: arm64: Disabling disabled PMU counters wastes a lot of time
Date: Tue, 6 Jul 2021 15:50:35 +0200	[thread overview]
Message-ID: <ed86299d-c0d4-f73e-ff7d-86eefd2de650@oracle.com> (raw)
In-Reply-To: <e3843c2c-e20a-ef58-c795-1ba8f1d91ff6@oracle.com>

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()).

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

Thanks,

alex.