Re: [PATCH 5/5] KVM: arm64: Remove redundant call to kvm_pmu_vcpu_reset()

From: Auger Eric <eric.auger@redhat.com>
To: Alexandru Elisei <alexandru.elisei@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, maz@kernel.org,
	james.morse@arm.com, julien.thierry.kdev@gmail.com,
	suzuki.poulose@arm.com
Subject: Re: [PATCH 5/5] KVM: arm64: Remove redundant call to kvm_pmu_vcpu_reset()
Date: Tue, 15 Dec 2020 10:15:12 +0100	[thread overview]
Message-ID: <594ae6f5-ad00-6279-e963-8b047c0209d6@redhat.com> (raw)
In-Reply-To: <eba17923-3e33-3e3d-0cf0-8f66d389146b@arm.com>

Hi Alexandru,

On 12/14/20 3:02 PM, Alexandru Elisei wrote:
> Hi Eric,
> 
> Thanks for having a look!
> 
> On 12/14/20 1:48 PM, Auger Eric wrote:
>> Alexandru,
>>
>> On 12/1/20 4:01 PM, Alexandru Elisei wrote:
>>> KVM_ARM_VCPU_INIT ioctl calls kvm_reset_vcpu(), which in turn resets the
>>> PMU with a call to kvm_pmu_vcpu_reset(). The function zeroes the PMU
>>> chained counters bitmap and stops all the counters with a perf event
>>> attached. Because it is called before the VCPU has had the chance to run,
>>> no perf events are in use and none are released.
>>>
>>> kvm_arm_pmu_v3_enable(), called by kvm_vcpu_first_run_init() only if the
>>> VCPU has not been initialized, also resets the PMU. kvm_pmu_vcpu_reset() in
>> This sounds strange to me. kvm_vcpu_first_run_init() only is called if
>> kvm_vcpu_initialized(vcpu) is true.
> 
> Typo on my part, the "not" should not be there. kvm_vcpu_first_run_init() is
> called only if kvm_vcpu_initialized() returns true in kvm_arch_vcpu_ioctl_run().
> 
>>> this case does the exact same thing as the previous call, so remove it.
>>>
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>>  arch/arm64/kvm/pmu-emul.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
>>> index 398f6df1bbe4..4ad66a532e38 100644
>>> --- a/arch/arm64/kvm/pmu-emul.c
>>> +++ b/arch/arm64/kvm/pmu-emul.c
>>> @@ -850,8 +850,6 @@ int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
>>>  		   return -EINVAL;
>>>  	}
>>>  
>>> -	kvm_pmu_vcpu_reset(vcpu);
>>> -
>> this patch does not apply for me (vcpu->arch.pmu.ready = true; ?)
> 
> I should have mentioned it in the cover letter, this is on top of Marc's pmu-undef
> branch:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pmu-undef
> 
> it should work on top of commit 7521c3a9e630 ("KVM: arm64: Get ready of the PMU
> ready state"), the branch was rebased since I sent the patches.
OK noted.

Thanks

Eric
> 
> Thanks,
> Alex
>>
>> Thanks
>>
>> Eric
>>>  	return 0;
>>>  }
>>>  
>>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm