Re: [RESEND v13 03/10] KVM: x86/pmu: Use IA32_PERF_CAPABILITIES to adjust features visibility

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/01/21 07:04, Like Xu wrote:
On 2021/1/26 17:42, Paolo Bonzini wrote:
On 08/01/21 02:36, Like Xu wrote:

@@ -401,6 +398,9 @@ static void intel_pmu_init(struct kvm_vcpu *vcpu)
         pmu->fixed_counters[i].idx = i + INTEL_PMC_IDX_FIXED;
         pmu->fixed_counters[i].current_config = 0;
     }
+
+    vcpu->arch.perf_capabilities = guest_cpuid_has(vcpu, X86_FEATURE_PDCM) ?
+        vmx_get_perf_capabilities() : 0;

There is one thing I don't understand with this patch: intel_pmu_init is not called when CPUID is changed.  So I would have thought that anything that uses guest_cpuid_has must stay in intel_pmu_refresh.  As I understand it vcpu->arch.perf_capabilities is always set to 0 (vmx_get_perf_capabilities is never called), and kvm_set_msr_common would fail to set any bit in the MSR.  What am I missing?

In addition, the code of patch 4:

+    if (!intel_pmu_lbr_is_enabled(vcpu)) {
+        vcpu->arch.perf_capabilities &= ~PMU_CAP_LBR_FMT;
+        lbr_desc->records.nr = 0;
+    }

is not okay after MSR changes.  The value written by the host must be either rejected (with "return 1") or applied unchanged.

Fortunately I think this code is dead if you move the check in kvm_set_msr from patch 9 to patch 4.  However, in patch 9 vmx_get_perf_capabilities() must only set the LBR format bits if intel_pmu_lbr_is_compatible(vcpu).

Thanks for the guidance. How about handling it in this way:

In the intel_pmu_init():

     vcpu->arch.perf_capabilities = 0;
     lbr_desc->records.nr = 0;

In the intel_pmu_refresh():

     if (guest_cpuid_has(vcpu, X86_FEATURE_PDCM)) {
         vcpu->arch.perf_capabilities = vmx_get_perf_capabilities();
         if (!lbr_desc->records.nr)
             vcpu->arch.perf_capabilities &= ~PMU_CAP_LBR_FMT;
     }

In the vmx_set_msr():

     case MSR_IA32_PERF_CAPABILITIES:
         // set up lbr_desc->records.nr
         if (!intel_pmu_lbr_is_compatible(vcpu))

Maybe pass msr_info.data to intel_pmu_lbr_is_compatible? Otherwise seems okay, thanks Like.

Paolo




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux