| Message ID | 20210511024214.280733-6-like.xu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/pmu: Add *basic* support to enable guest PEBS via DS | expand |
On Tue, May 11, 2021 at 10:42:03AM +0800, Like Xu wrote: > The mask value of fixed counter control register should be dynamic > adjusted with the number of fixed counters. This patch introduces a > variable that includes the reserved bits of fixed counter control > registers. This is needed for later Ice Lake fixed counter changes. > > Co-developed-by: Luwei Kang <luwei.kang@intel.com> > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > Signed-off-by: Like Xu <like.xu@linux.intel.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/vmx/pmu_intel.c | 6 +++++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 55efbacfc244..49b421bd3dd8 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -457,6 +457,7 @@ struct kvm_pmu { > unsigned nr_arch_fixed_counters; > unsigned available_event_types; > u64 fixed_ctr_ctrl; > + u64 fixed_ctr_ctrl_mask; > u64 global_ctrl; > u64 global_status; > u64 global_ovf_ctrl; > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > index d9dbebe03cae..ac7fe714e6c1 100644 > --- a/arch/x86/kvm/vmx/pmu_intel.c > +++ b/arch/x86/kvm/vmx/pmu_intel.c > @@ -400,7 +400,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > case MSR_CORE_PERF_FIXED_CTR_CTRL: > if (pmu->fixed_ctr_ctrl == data) > return 0; > - if (!(data & 0xfffffffffffff444ull)) { > + if (!(data & pmu->fixed_ctr_ctrl_mask)) { Don't we already have hardware with more than 3 fixed counters?
On 2021/5/17 16:18, Peter Zijlstra wrote: > On Tue, May 11, 2021 at 10:42:03AM +0800, Like Xu wrote: >> The mask value of fixed counter control register should be dynamic >> adjusted with the number of fixed counters. This patch introduces a >> variable that includes the reserved bits of fixed counter control >> registers. This is needed for later Ice Lake fixed counter changes. >> >> Co-developed-by: Luwei Kang <luwei.kang@intel.com> >> Signed-off-by: Luwei Kang <luwei.kang@intel.com> >> Signed-off-by: Like Xu <like.xu@linux.intel.com> >> --- >> arch/x86/include/asm/kvm_host.h | 1 + >> arch/x86/kvm/vmx/pmu_intel.c | 6 +++++- >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >> index 55efbacfc244..49b421bd3dd8 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -457,6 +457,7 @@ struct kvm_pmu { >> unsigned nr_arch_fixed_counters; >> unsigned available_event_types; >> u64 fixed_ctr_ctrl; >> + u64 fixed_ctr_ctrl_mask; >> u64 global_ctrl; >> u64 global_status; >> u64 global_ovf_ctrl; >> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c >> index d9dbebe03cae..ac7fe714e6c1 100644 >> --- a/arch/x86/kvm/vmx/pmu_intel.c >> +++ b/arch/x86/kvm/vmx/pmu_intel.c >> @@ -400,7 +400,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> case MSR_CORE_PERF_FIXED_CTR_CTRL: >> if (pmu->fixed_ctr_ctrl == data) >> return 0; >> - if (!(data & 0xfffffffffffff444ull)) { >> + if (!(data & pmu->fixed_ctr_ctrl_mask)) { > Don't we already have hardware with more than 3 fixed counters? Yes, so we update this mask based on the value of pmu->nr_arch_fixed_counters: + for (i = 0; i < pmu->nr_arch_fixed_counters; i++) + pmu->fixed_ctr_ctrl_mask &= ~(0xbull << (i * 4)); I assume this comment will not result in any code changes for this patch.
On Tue, May 18, 2021 at 03:55:13PM +0800, Xu, Like wrote: > On 2021/5/17 16:18, Peter Zijlstra wrote: > > On Tue, May 11, 2021 at 10:42:03AM +0800, Like Xu wrote: > > > The mask value of fixed counter control register should be dynamic > > > adjusted with the number of fixed counters. This patch introduces a > > > variable that includes the reserved bits of fixed counter control > > > registers. This is needed for later Ice Lake fixed counter changes. > > > > > > Co-developed-by: Luwei Kang <luwei.kang@intel.com> > > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > > > Signed-off-by: Like Xu <like.xu@linux.intel.com> > > > --- > > > arch/x86/include/asm/kvm_host.h | 1 + > > > arch/x86/kvm/vmx/pmu_intel.c | 6 +++++- > > > 2 files changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > > index 55efbacfc244..49b421bd3dd8 100644 > > > --- a/arch/x86/include/asm/kvm_host.h > > > +++ b/arch/x86/include/asm/kvm_host.h > > > @@ -457,6 +457,7 @@ struct kvm_pmu { > > > unsigned nr_arch_fixed_counters; > > > unsigned available_event_types; > > > u64 fixed_ctr_ctrl; > > > + u64 fixed_ctr_ctrl_mask; > > > u64 global_ctrl; > > > u64 global_status; > > > u64 global_ovf_ctrl; > > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > > > index d9dbebe03cae..ac7fe714e6c1 100644 > > > --- a/arch/x86/kvm/vmx/pmu_intel.c > > > +++ b/arch/x86/kvm/vmx/pmu_intel.c > > > @@ -400,7 +400,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > > > case MSR_CORE_PERF_FIXED_CTR_CTRL: > > > if (pmu->fixed_ctr_ctrl == data) > > > return 0; > > > - if (!(data & 0xfffffffffffff444ull)) { > > > + if (!(data & pmu->fixed_ctr_ctrl_mask)) { > > Don't we already have hardware with more than 3 fixed counters? > > Yes, so we update this mask based on the value of pmu->nr_arch_fixed_counters: Yes, I saw that, but the Changelog makes it appear this is only relevant to ice lake, which I think is not fully correct.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 55efbacfc244..49b421bd3dd8 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -457,6 +457,7 @@ struct kvm_pmu { unsigned nr_arch_fixed_counters; unsigned available_event_types; u64 fixed_ctr_ctrl; + u64 fixed_ctr_ctrl_mask; u64 global_ctrl; u64 global_status; u64 global_ovf_ctrl; diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c index d9dbebe03cae..ac7fe714e6c1 100644 --- a/arch/x86/kvm/vmx/pmu_intel.c +++ b/arch/x86/kvm/vmx/pmu_intel.c @@ -400,7 +400,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_CORE_PERF_FIXED_CTR_CTRL: if (pmu->fixed_ctr_ctrl == data) return 0; - if (!(data & 0xfffffffffffff444ull)) { + if (!(data & pmu->fixed_ctr_ctrl_mask)) { reprogram_fixed_counters(pmu, data); return 0; } @@ -470,6 +470,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) struct kvm_cpuid_entry2 *entry; union cpuid10_eax eax; union cpuid10_edx edx; + int i; pmu->nr_arch_gp_counters = 0; pmu->nr_arch_fixed_counters = 0; @@ -477,6 +478,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) pmu->counter_bitmask[KVM_PMC_FIXED] = 0; pmu->version = 0; pmu->reserved_bits = 0xffffffff00200000ull; + pmu->fixed_ctr_ctrl_mask = ~0ull; entry = kvm_find_cpuid_entry(vcpu, 0xa, 0); if (!entry) @@ -511,6 +513,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) ((u64)1 << edx.split.bit_width_fixed) - 1; } + for (i = 0; i < pmu->nr_arch_fixed_counters; i++) + pmu->fixed_ctr_ctrl_mask &= ~(0xbull << (i * 4)); pmu->global_ctrl = ((1ull << pmu->nr_arch_gp_counters) - 1) | (((1ull << pmu->nr_arch_fixed_counters) - 1) << INTEL_PMC_IDX_FIXED); pmu->global_ctrl_mask = ~pmu->global_ctrl;