Messages in this thread | | | Date | Sat, 13 Apr 2024 12:12:59 +0800 | Subject | Re: [RFC PATCH 37/41] KVM: x86/pmu: Allow writing to fixed counter selector if counter is exposed | From | "Mi, Dapeng" <> |
| |
On 4/12/2024 6:03 AM, Sean Christopherson wrote: > On Fri, Jan 26, 2024, Xiong Zhang wrote: >> From: Mingwei Zhang <mizhang@google.com> >> >> Allow writing to fixed counter selector if counter is exposed. If this >> fixed counter is filtered out, this counter won't be enabled on HW. >> >> Passthrough PMU implements the context switch at VM Enter/Exit boundary the >> guest value cannot be directly written to HW since the HW PMU is owned by >> the host. Introduce a new field fixed_ctr_ctrl_hw in kvm_pmu to cache the >> guest value. which will be assigne to HW at PMU context restore. >> >> Since passthrough PMU intercept writes to fixed counter selector, there is >> no need to read the value at pmu context save, but still clear the fix >> counter ctrl MSR and counters when switching out to host PMU. >> >> Signed-off-by: Mingwei Zhang <mizhang@google.com> >> --- >> arch/x86/include/asm/kvm_host.h | 1 + >> arch/x86/kvm/vmx/pmu_intel.c | 28 ++++++++++++++++++++++++---- >> 2 files changed, 25 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >> index fd1c69371dbf..b02688ed74f7 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -527,6 +527,7 @@ struct kvm_pmu { >> unsigned nr_arch_fixed_counters; >> unsigned available_event_types; >> u64 fixed_ctr_ctrl; >> + u64 fixed_ctr_ctrl_hw; >> u64 fixed_ctr_ctrl_mask; > Before introduce more fields, can someone please send a patch/series to rename > the _mask fields? AFAIK, they all should be e.g. fixed_ctr_ctrl_rsvd, or something > to that effect.
Yeah, I remember I ever said to cook a patch to rename all these _mask fields. I would do it now.
> > Because I think we should avoid reinventing the naming wheel, and use "shadow" > instead of "hw", because KVM developers already know what "shadow" means. But > "mask" also has very specific meaning for shadowed fields. That, and "mask" is > a freaking awful name in the first place. > >> u64 global_ctrl; >> u64 global_status; >> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c >> index 713c2a7c7f07..93cfb86c1292 100644 >> --- a/arch/x86/kvm/vmx/pmu_intel.c >> +++ b/arch/x86/kvm/vmx/pmu_intel.c >> @@ -68,6 +68,25 @@ static int fixed_pmc_events[] = { >> [2] = PSEUDO_ARCH_REFERENCE_CYCLES, >> }; >> >> +static void reprogram_fixed_counters_in_passthrough_pmu(struct kvm_pmu *pmu, u64 data) > We need to come up with shorter names, this ain't Java. :-) Heh, that can be > another argument for "mediated", it saves three characters. > > And somewhat related, kernel style is <scope>_<blah>, i.e. > > static void mediated_pmu_reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data)
| |