lkml.org 
[lkml]   [2022]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v3 3/7] KVM: x86/pmu: Avoid using PEBS perf_events for normal counters
On Wed, Aug 31, 2022, Like Xu wrote:
> To address this issue, the reuse check has been revamped and KVM will

State what the patch does as a command, don't describe the effects in the past
tense.

> go back to do reprogram_counter() when any bit of guest PEBS_ENABLE
> msr has changed, which is similar to what global_ctrl_changed() does.

This managed to confuse the heck out of me. I misread reprogram_counter() as
reprogram_counters() because that's what I see in the diff, and then got all
turned around because this patch also stealthily renames global_ctrl_changed() to
reprogram_counters().

This is a very good example of when splitting a refactor with a bug fix can help
tremendously, e.g. with the refactor moved to a separate patch, this fix becomes:

diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 390d697efde1..d9b9a0f0db17 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -237,8 +237,8 @@ static bool pmc_resume_counter(struct kvm_pmc *pmc)
get_sample_period(pmc, pmc->counter)))
return false;

- if (!test_bit(pmc->idx, (unsigned long *)&pmc_to_pmu(pmc)->pebs_enable) &&
- pmc->perf_event->attr.precise_ip)
+ if (test_bit(pmc->idx, (unsigned long *)&pmc_to_pmu(pmc)->pebs_enable) !=
+ (!!pmc->perf_event->attr.precise_ip))
return false;

/* reuse perf_event to serve as pmc_reprogram_counter() does*/
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 5592b1259e1b..25b70a85bef5 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -431,7 +431,9 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
if (pmu->pebs_enable == data)
return 0;
if (!(data & pmu->pebs_enable_mask)) {
+ diff = pmu->pebs_enable ^ data;
pmu->pebs_enable = data;
+ reprogram_counters(pmu, diff);
return 0;
}
break;
which is much, much easier to describe and to follow.

TL;DR: I split this into two patches.

> Fixes: 79f3e3b58386 ("KVM: x86/pmu: Reprogram PEBS event to emulate guest PEBS counter")
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---

...

> -/* function is called when global control register has been updated. */
> -static void global_ctrl_changed(struct kvm_pmu *pmu, u64 data)
> +static void reprogram_counters(struct kvm_pmu *pmu, u64 diff)

\
 
 \ /
  Last update: 2022-09-23 00:36    [W:0.140 / U:2.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site