| Message ID | 20220308012452.3468611-1-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/pmu: Use different raw event masks for AMD and Intel | expand |
On 8/3/2022 9:24 am, Jim Mattson wrote: > The third nybble of AMD's event select overlaps with Intel's IN_TX and > IN_TXCP bits. Therefore, we can't use AMD64_RAW_EVENT_MASK on Intel > platforms that support TSX. We already have pmu->reserved_bits as the first wall to check "can't use". > > Declare a raw_event_mask in the kvm_pmu structure, initialize it in > the vendor-specific pmu_refresh() functions, and use that mask for > PERF_TYPE_RAW configurations in reprogram_gp_counter(). > > Fixes: 710c47651431 ("KVM: x86/pmu: Use AMD64_RAW_EVENT_MASK for PERF_TYPE_RAW") Is it really a fix ? > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/pmu.c | 3 ++- > arch/x86/kvm/svm/pmu.c | 1 + > arch/x86/kvm/vmx/pmu_intel.c | 1 + > 4 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index c45ab8b5c37f..cacd27c1aa19 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -510,6 +510,7 @@ struct kvm_pmu { > u64 global_ctrl_mask; > u64 global_ovf_ctrl_mask; > u64 reserved_bits; > + u64 raw_event_mask; > u8 version; > struct kvm_pmc gp_counters[INTEL_PMC_MAX_GENERIC]; > struct kvm_pmc fixed_counters[INTEL_PMC_MAX_FIXED]; > diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c > index b1a02993782b..902b6d700215 100644 > --- a/arch/x86/kvm/pmu.c > +++ b/arch/x86/kvm/pmu.c > @@ -185,6 +185,7 @@ void reprogram_gp_counter(struct kvm_pmc *pmc, u64 eventsel) > u32 type = PERF_TYPE_RAW; > struct kvm *kvm = pmc->vcpu->kvm; > struct kvm_pmu_event_filter *filter; > + struct kvm_pmu *pmu = vcpu_to_pmu(pmc->vcpu); How about pmc_to_pmu(pmc) ? > bool allow_event = true; > > if (eventsel & ARCH_PERFMON_EVENTSEL_PIN_CONTROL) > @@ -221,7 +222,7 @@ void reprogram_gp_counter(struct kvm_pmc *pmc, u64 eventsel) > } > > if (type == PERF_TYPE_RAW) > - config = eventsel & AMD64_RAW_EVENT_MASK; > + config = eventsel & pmu->raw_event_mask; If the code base is current kvm/queue, the checks about (eventsel & HSW_IN_TX*) is still there. Not sure why we're wasting extra 'u64' space for nothing. > > if (pmc->current_config == eventsel && pmc_resume_counter(pmc)) > return; > diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c > index 886e8ac5cfaa..24eb935b6f85 100644 > --- a/arch/x86/kvm/svm/pmu.c > +++ b/arch/x86/kvm/svm/pmu.c > @@ -282,6 +282,7 @@ static void amd_pmu_refresh(struct kvm_vcpu *vcpu) > > pmu->counter_bitmask[KVM_PMC_GP] = ((u64)1 << 48) - 1; > pmu->reserved_bits = 0xfffffff000280000ull; > + pmu->raw_event_mask = AMD64_RAW_EVENT_MASK; > pmu->version = 1; > /* not applicable to AMD; but clean them to prevent any fall out */ > pmu->counter_bitmask[KVM_PMC_FIXED] = 0; > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > index 4e5b1eeeb77c..da71160a50d6 100644 > --- a/arch/x86/kvm/vmx/pmu_intel.c > +++ b/arch/x86/kvm/vmx/pmu_intel.c > @@ -485,6 +485,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->raw_event_mask = X86_RAW_EVENT_MASK; > > entry = kvm_find_cpuid_entry(vcpu, 0xa, 0); > if (!entry || !vcpu->kvm->arch.enable_pmu)
On 3/8/22 02:24, Jim Mattson wrote: > The third nybble of AMD's event select overlaps with Intel's IN_TX and > IN_TXCP bits. Therefore, we can't use AMD64_RAW_EVENT_MASK on Intel > platforms that support TSX. > > Declare a raw_event_mask in the kvm_pmu structure, initialize it in > the vendor-specific pmu_refresh() functions, and use that mask for > PERF_TYPE_RAW configurations in reprogram_gp_counter(). > > Fixes: 710c47651431 ("KVM: x86/pmu: Use AMD64_RAW_EVENT_MASK for PERF_TYPE_RAW") > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/pmu.c | 3 ++- > arch/x86/kvm/svm/pmu.c | 1 + > arch/x86/kvm/vmx/pmu_intel.c | 1 + > 4 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index c45ab8b5c37f..cacd27c1aa19 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -510,6 +510,7 @@ struct kvm_pmu { > u64 global_ctrl_mask; > u64 global_ovf_ctrl_mask; > u64 reserved_bits; > + u64 raw_event_mask; > u8 version; > struct kvm_pmc gp_counters[INTEL_PMC_MAX_GENERIC]; > struct kvm_pmc fixed_counters[INTEL_PMC_MAX_FIXED]; > diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c > index b1a02993782b..902b6d700215 100644 > --- a/arch/x86/kvm/pmu.c > +++ b/arch/x86/kvm/pmu.c > @@ -185,6 +185,7 @@ void reprogram_gp_counter(struct kvm_pmc *pmc, u64 eventsel) > u32 type = PERF_TYPE_RAW; > struct kvm *kvm = pmc->vcpu->kvm; > struct kvm_pmu_event_filter *filter; > + struct kvm_pmu *pmu = vcpu_to_pmu(pmc->vcpu); > bool allow_event = true; > > if (eventsel & ARCH_PERFMON_EVENTSEL_PIN_CONTROL) > @@ -221,7 +222,7 @@ void reprogram_gp_counter(struct kvm_pmc *pmc, u64 eventsel) > } > > if (type == PERF_TYPE_RAW) > - config = eventsel & AMD64_RAW_EVENT_MASK; > + config = eventsel & pmu->raw_event_mask; > > if (pmc->current_config == eventsel && pmc_resume_counter(pmc)) > return; > diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c > index 886e8ac5cfaa..24eb935b6f85 100644 > --- a/arch/x86/kvm/svm/pmu.c > +++ b/arch/x86/kvm/svm/pmu.c > @@ -282,6 +282,7 @@ static void amd_pmu_refresh(struct kvm_vcpu *vcpu) > > pmu->counter_bitmask[KVM_PMC_GP] = ((u64)1 << 48) - 1; > pmu->reserved_bits = 0xfffffff000280000ull; > + pmu->raw_event_mask = AMD64_RAW_EVENT_MASK; > pmu->version = 1; > /* not applicable to AMD; but clean them to prevent any fall out */ > pmu->counter_bitmask[KVM_PMC_FIXED] = 0; > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > index 4e5b1eeeb77c..da71160a50d6 100644 > --- a/arch/x86/kvm/vmx/pmu_intel.c > +++ b/arch/x86/kvm/vmx/pmu_intel.c > @@ -485,6 +485,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->raw_event_mask = X86_RAW_EVENT_MASK; > > entry = kvm_find_cpuid_entry(vcpu, 0xa, 0); > if (!entry || !vcpu->kvm->arch.enable_pmu) Queued, thanks. Paolo
On 3/8/22 11:28, Like Xu wrote: > On 8/3/2022 9:24 am, Jim Mattson wrote: >> The third nybble of AMD's event select overlaps with Intel's IN_TX and >> IN_TXCP bits. Therefore, we can't use AMD64_RAW_EVENT_MASK on Intel >> platforms that support TSX. > > We already have pmu->reserved_bits as the first wall to check "can't use". > >> >> Declare a raw_event_mask in the kvm_pmu structure, initialize it in >> the vendor-specific pmu_refresh() functions, and use that mask for >> PERF_TYPE_RAW configurations in reprogram_gp_counter(). >> >> Fixes: 710c47651431 ("KVM: x86/pmu: Use AMD64_RAW_EVENT_MASK for >> PERF_TYPE_RAW") > > Is it really a fix ? No, it's not, so I'm queuing it for 5.18 only. Paolo
On Tue, Mar 8, 2022 at 2:29 AM Like Xu <like.xu.linux@gmail.com> wrote: > > On 8/3/2022 9:24 am, Jim Mattson wrote: > > The third nybble of AMD's event select overlaps with Intel's IN_TX and > > IN_TXCP bits. Therefore, we can't use AMD64_RAW_EVENT_MASK on Intel > > platforms that support TSX. > > We already have pmu->reserved_bits as the first wall to check "can't use". That is only a safeguard for Intel platforms that *don't* support TSX. > > > > Declare a raw_event_mask in the kvm_pmu structure, initialize it in > > the vendor-specific pmu_refresh() functions, and use that mask for > > PERF_TYPE_RAW configurations in reprogram_gp_counter(). > > > > Fixes: 710c47651431 ("KVM: x86/pmu: Use AMD64_RAW_EVENT_MASK for PERF_TYPE_RAW") > > Is it really a fix ? When I submitted the commit referenced above, it was not my intention to introduce semantic changes on Intel platforms. I hadn't realized at the time that IN_TX and IN_TXCP overlapped with bits 11:8 of AMD's event select. But, you are right. Previously, the code would mask off IN_TX and IN_TXCP, just to or them back in again later. So, the aforementioned commit did not change the semantics of the existing code; it just rendered the statements to or the bits back in redundant.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index c45ab8b5c37f..cacd27c1aa19 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -510,6 +510,7 @@ struct kvm_pmu { u64 global_ctrl_mask; u64 global_ovf_ctrl_mask; u64 reserved_bits; + u64 raw_event_mask; u8 version; struct kvm_pmc gp_counters[INTEL_PMC_MAX_GENERIC]; struct kvm_pmc fixed_counters[INTEL_PMC_MAX_FIXED]; diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c index b1a02993782b..902b6d700215 100644 --- a/arch/x86/kvm/pmu.c +++ b/arch/x86/kvm/pmu.c @@ -185,6 +185,7 @@ void reprogram_gp_counter(struct kvm_pmc *pmc, u64 eventsel) u32 type = PERF_TYPE_RAW; struct kvm *kvm = pmc->vcpu->kvm; struct kvm_pmu_event_filter *filter; + struct kvm_pmu *pmu = vcpu_to_pmu(pmc->vcpu); bool allow_event = true; if (eventsel & ARCH_PERFMON_EVENTSEL_PIN_CONTROL) @@ -221,7 +222,7 @@ void reprogram_gp_counter(struct kvm_pmc *pmc, u64 eventsel) } if (type == PERF_TYPE_RAW) - config = eventsel & AMD64_RAW_EVENT_MASK; + config = eventsel & pmu->raw_event_mask; if (pmc->current_config == eventsel && pmc_resume_counter(pmc)) return; diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c index 886e8ac5cfaa..24eb935b6f85 100644 --- a/arch/x86/kvm/svm/pmu.c +++ b/arch/x86/kvm/svm/pmu.c @@ -282,6 +282,7 @@ static void amd_pmu_refresh(struct kvm_vcpu *vcpu) pmu->counter_bitmask[KVM_PMC_GP] = ((u64)1 << 48) - 1; pmu->reserved_bits = 0xfffffff000280000ull; + pmu->raw_event_mask = AMD64_RAW_EVENT_MASK; pmu->version = 1; /* not applicable to AMD; but clean them to prevent any fall out */ pmu->counter_bitmask[KVM_PMC_FIXED] = 0; diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c index 4e5b1eeeb77c..da71160a50d6 100644 --- a/arch/x86/kvm/vmx/pmu_intel.c +++ b/arch/x86/kvm/vmx/pmu_intel.c @@ -485,6 +485,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->raw_event_mask = X86_RAW_EVENT_MASK; entry = kvm_find_cpuid_entry(vcpu, 0xa, 0); if (!entry || !vcpu->kvm->arch.enable_pmu)
The third nybble of AMD's event select overlaps with Intel's IN_TX and IN_TXCP bits. Therefore, we can't use AMD64_RAW_EVENT_MASK on Intel platforms that support TSX. Declare a raw_event_mask in the kvm_pmu structure, initialize it in the vendor-specific pmu_refresh() functions, and use that mask for PERF_TYPE_RAW configurations in reprogram_gp_counter(). Fixes: 710c47651431 ("KVM: x86/pmu: Use AMD64_RAW_EVENT_MASK for PERF_TYPE_RAW") Signed-off-by: Jim Mattson <jmattson@google.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/pmu.c | 3 ++- arch/x86/kvm/svm/pmu.c | 1 + arch/x86/kvm/vmx/pmu_intel.c | 1 + 4 files changed, 5 insertions(+), 1 deletion(-)