Messages in this thread Patch in this message | | | Subject | Re: [PATCH v5 10/16] KVM: x86: Set PEBS_UNAVAIL in IA32_MISC_ENABLE when PEBS is enabled | From | Liuxiangdong <> | Date | Tue, 20 Apr 2021 16:30:20 +0800 |
| |
On 2021/4/15 11:20, Like Xu wrote: > The bit 12 represents "Processor Event Based Sampling Unavailable (RO)" : > 1 = PEBS is not supported. > 0 = PEBS is supported. > > A write to this PEBS_UNAVL available bit will bring #GP(0) when guest PEBS > is enabled. Some PEBS drivers in guest may care about this bit. > > Signed-off-by: Like Xu <like.xu@linux.intel.com> > --- > arch/x86/kvm/vmx/pmu_intel.c | 2 ++ > arch/x86/kvm/x86.c | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > index 58f32a55cc2e..c846d3eef7a7 100644 > --- a/arch/x86/kvm/vmx/pmu_intel.c > +++ b/arch/x86/kvm/vmx/pmu_intel.c > @@ -588,6 +588,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) > bitmap_set(pmu->all_valid_pmc_idx, INTEL_PMC_IDX_FIXED_VLBR, 1); > > if (vcpu->arch.perf_capabilities & PERF_CAP_PEBS_FORMAT) { > + vcpu->arch.ia32_misc_enable_msr &= ~MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL; > if (vcpu->arch.perf_capabilities & PERF_CAP_PEBS_BASELINE) { > pmu->pebs_enable_mask = ~pmu->global_ctrl; > pmu->reserved_bits &= ~ICL_EVENTSEL_ADAPTIVE; > @@ -597,6 +598,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) > } > pmu->pebs_data_cfg_mask = ~0xff00000full; > } else { > + vcpu->arch.ia32_misc_enable_msr |= MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL; > pmu->pebs_enable_mask = > ~((1ull << pmu->nr_arch_gp_counters) - 1); > }
I guess what we want is
if (vcpu->arch.perf_capabilities & PERF_CAP_PEBS_FORMAT) { vcpu->arch.ia32_misc_enable_msr &= ~MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL; if (vcpu->arch.perf_capabilities & PERF_CAP_PEBS_BASELINE) { pmu->pebs_enable_mask = ~pmu->global_ctrl; pmu->reserved_bits &= ~ICL_EVENTSEL_ADAPTIVE; for (i = 0; i < pmu->nr_arch_fixed_counters; i++) { pmu->fixed_ctr_ctrl_mask &= ~(1ULL << (INTEL_PMC_IDX_FIXED + i * 4)); } pmu->pebs_data_cfg_mask = ~0xff00000full; } else { pmu->pebs_enable_mask = ~((1ull << pmu->nr_arch_gp_counters) - 1); } } else { vcpu->arch.ia32_misc_enable_msr |= MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL; vcpu->arch.perf_capabilities &= ~PERF_CAP_PEBS_MASK; }
But here is
if (vcpu->arch.perf_capabilities & PERF_CAP_PEBS_FORMAT) { vcpu->arch.ia32_misc_enable_msr &= ~MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL; if (vcpu->arch.perf_capabilities & PERF_CAP_PEBS_BASELINE) { pmu->pebs_enable_mask = ~pmu->global_ctrl; pmu->reserved_bits &= ~ICL_EVENTSEL_ADAPTIVE; for (i = 0; i < pmu->nr_arch_fixed_counters; i++) { pmu->fixed_ctr_ctrl_mask &= ~(1ULL << (INTEL_PMC_IDX_FIXED + i * 4)); } pmu->pebs_data_cfg_mask = ~0xff00000full; } else { vcpu->arch.ia32_misc_enable_msr |= MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL; pmu->pebs_enable_mask = ~((1ull << pmu->nr_arch_gp_counters) - 1); } } else { vcpu->arch.perf_capabilities &= ~PERF_CAP_PEBS_MASK; }
Wrong else branch?
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 1a64e816e06d..ed38f1dada63 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3126,6 +3126,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > break; > case MSR_IA32_MISC_ENABLE: > data &= ~MSR_IA32_MISC_ENABLE_EMON; > + if (!msr_info->host_initiated && > + (vcpu->arch.perf_capabilities & PERF_CAP_PEBS_FORMAT) && > + (data & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL)) > + return 1; > if (!kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT) && > ((vcpu->arch.ia32_misc_enable_msr ^ data) & MSR_IA32_MISC_ENABLE_MWAIT)) { > if (!guest_cpuid_has(vcpu, X86_FEATURE_XMM3))
| |