| Message ID | 20201201150157.223625-6-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Miscellaneous improvements | expand |
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. > 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; ?) Thanks Eric > return 0; > } > >
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. Thanks, Alex > > Thanks > > Eric >> return 0; >> } >> >>
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 >
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); - return 0; }
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 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(-)