| Message ID | 1550135174-5423-13-git-send-email-wei.w.wang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Guest LBR Enabling | expand |
> +static void intel_pmu_get_global_status(struct kvm_pmu *pmu, > + struct msr_data *msr_info) > +{ > + u64 guest_debugctl, freeze_lbr_bits = DEBUGCTLMSR_FREEZE_LBRS_ON_PMI | > + DEBUGCTLMSR_LBR; > + > + if (!pmu->global_status) { > + msr_info->data = 0; > + return; > + } > + > + msr_info->data = pmu->global_status; > + if (pmu->version >= 4) { > + guest_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL); > + if ((guest_debugctl & freeze_lbr_bits) == freeze_lbr_bits) It should only check for the freeze bit, the freeze bit can be set even when LBRs are disabled. Also you seem to set the bit unconditionally? That doesn't seem right. It should only be set after an overflow. So the PMI injection needs to set it. -Andi
On Friday, February 15, 2019 12:32 AM, Andi Kleen wrote: > > > +static void intel_pmu_get_global_status(struct kvm_pmu *pmu, > > + struct msr_data *msr_info) > > +{ > > + u64 guest_debugctl, freeze_lbr_bits = > DEBUGCTLMSR_FREEZE_LBRS_ON_PMI | > > + DEBUGCTLMSR_LBR; > > + > > + if (!pmu->global_status) { > > + msr_info->data = 0; > > + return; > > + } > > + > > + msr_info->data = pmu->global_status; > > + if (pmu->version >= 4) { > > + guest_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL); > > + if ((guest_debugctl & freeze_lbr_bits) == freeze_lbr_bits) > > It should only check for the freeze bit, the freeze bit can be set even when > LBRs are disabled. > > Also you seem to set the bit unconditionally? > That doesn't seem right. It should only be set after an overflow. > > So the PMI injection needs to set it. OK. The freeze bits need to be cleared by IA32_PERF_GLOBAL_STATUS_RESET, which seems not supported by the perf code yet (thus guest won't clear them). Would handle_irq_v4 also need to be changed to support that? Best, Wei
On Fri, Feb 15, 2019 at 08:56:02AM +0000, Wang, Wei W wrote: > On Friday, February 15, 2019 12:32 AM, Andi Kleen wrote: > > > > > +static void intel_pmu_get_global_status(struct kvm_pmu *pmu, > > > + struct msr_data *msr_info) > > > +{ > > > + u64 guest_debugctl, freeze_lbr_bits = > > DEBUGCTLMSR_FREEZE_LBRS_ON_PMI | > > > + DEBUGCTLMSR_LBR; > > > + > > > + if (!pmu->global_status) { > > > + msr_info->data = 0; > > > + return; > > > + } > > > + > > > + msr_info->data = pmu->global_status; > > > + if (pmu->version >= 4) { > > > + guest_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL); > > > + if ((guest_debugctl & freeze_lbr_bits) == freeze_lbr_bits) > > > > It should only check for the freeze bit, the freeze bit can be set even when > > LBRs are disabled. > > > > Also you seem to set the bit unconditionally? > > That doesn't seem right. It should only be set after an overflow. > > > > So the PMI injection needs to set it. > > OK. The freeze bits need to be cleared by IA32_PERF_GLOBAL_STATUS_RESET, which seems not supported by the perf code yet (thus guest won't clear them). Would handle_irq_v4 also need to be changed to support that? In Arch Perfmon v4 it is cleared by the MSR_CORE_PERF_GLOBAL_OVF_CTRL write But the guest KVM pmu doesn't support v4 so far, so the only way to clear it is through DEBUGCTL. STATUS_RESET would only be needed to set it from the guest, which is not necessary at least for now (and would be also v4) At some point the guest PMU should probably be updated for v4, but it can be done separately from this. -Andi
On 02/15/2019 09:10 PM, Andi Kleen wrote: > > OK. The freeze bits need to be cleared by IA32_PERF_GLOBAL_STATUS_RESET, which seems not supported by the perf code yet (thus guest won't clear them). Would handle_irq_v4 also need to be changed to support that? > In Arch Perfmon v4 it is cleared by the MSR_CORE_PERF_GLOBAL_OVF_CTRL write Not very sure about this one. The spec 18.2.4.2 mentions "IA32_PERF_GLOBAL_STATUS_RESET provides additional bit fields to clear the new indicators.." IIUIC, the new freeze bits can only be cleared by RESET. > But the guest KVM pmu doesn't support v4 so far, so the only way to clear it is through DEBUGCTL. > > STATUS_RESET would only be needed to set it from the guest, which is not necessary at least for now > (and would be also v4) > > At some point the guest PMU should probably be updated for v4, but it can be done > separately from this. > Agree. I think the guest perf won't work in v4 mode if the KVM vPMU exposes it is v3. Probably we could also leave the freeze bits virtualization support to another series of vPMU v4 support? We would also need to use the STATUS_SET in v4 to set the freeze bits of GLOBAL_STATUS when entering the guest (instead of clearing the guest debugctl), so that we could achieve architectural emulation. Best, Wei
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c index bf40941..80d3fcf 100644 --- a/arch/x86/kvm/vmx/pmu_intel.c +++ b/arch/x86/kvm/vmx/pmu_intel.c @@ -420,6 +420,25 @@ static bool intel_pmu_access_lbr_msr(struct kvm_vcpu *vcpu, return ret; } +static void intel_pmu_get_global_status(struct kvm_pmu *pmu, + struct msr_data *msr_info) +{ + u64 guest_debugctl, freeze_lbr_bits = DEBUGCTLMSR_FREEZE_LBRS_ON_PMI | + DEBUGCTLMSR_LBR; + + if (!pmu->global_status) { + msr_info->data = 0; + return; + } + + msr_info->data = pmu->global_status; + if (pmu->version >= 4) { + guest_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL); + if ((guest_debugctl & freeze_lbr_bits) == freeze_lbr_bits) + msr_info->data |= GLOBAL_STATUS_LBRS_FROZEN; + } +} + static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) { struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); @@ -431,7 +450,7 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) msr_info->data = pmu->fixed_ctr_ctrl; return 0; case MSR_CORE_PERF_GLOBAL_STATUS: - msr_info->data = pmu->global_status; + intel_pmu_get_global_status(pmu, msr_info); return 0; case MSR_CORE_PERF_GLOBAL_CTRL: msr_info->data = pmu->global_ctrl;
Arch v4 supports streamlined Freeze_LBR_on_PMI, so we set the GLOBAL_STATUS_LBRS_FROZEN bit when the guest reads the global status msr with freezing lbr in use. Signed-off-by: Wei Wang <wei.w.wang@intel.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Andi Kleen <ak@linux.intel.com> --- arch/x86/kvm/vmx/pmu_intel.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)