答复: [PATCH] [v3] kvm: x86: support APERF/MPERF registers

["Li,Rongqing" <lirongqing@baidu.com>]

> -----邮件原件-----
> 发件人: Wei Huang [mailto:wei.huang2@amd.com]
> 发送时间: 2020年5月2日 5:31
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; x86@kernel.org;
> hpa@zytor.com; bp@alien8.de; mingo@redhat.com; tglx@linutronix.de;
> jmattson@google.com; wanpengli@tencent.com; vkuznets@redhat.com;
> sean.j.christopherson@intel.com; pbonzini@redhat.com; xiaoyao.li@intel.com
> 主题: Re: [PATCH] [v3] kvm: x86: support APERF/MPERF registers
> 重要性: 高
> 
> On 04/30 06:45, Li RongQing wrote:
> > Guest kernel reports a fixed cpu frequency in /proc/cpuinfo, this is
> > confused to user when turbo is enable, and aperf/mperf can be used to
> > show current cpu frequency after 7d5905dc14a
> > "(x86 / CPU: Always show current CPU frequency in /proc/cpuinfo)"
> > so guest should support aperf/mperf capability
> >
> > this patch implements aperf/mperf by three mode: none, software
>   ^^^^
>   This
> 
> > emulation, and pass-through
> >
> > none: default mode, guest does not support aperf/mperf
> >
> > software emulation: the period of aperf/mperf in guest mode are
> > accumulated as emulated value
> >
> > pass-though: it is only suitable for KVM_HINTS_REALTIME, Because that
> > hint guarantees we have a 1:1 vCPU:CPU binding and guaranteed no
> > over-commit.
> 
> If we save/restore the values of aperf/mperf properly during vcpu migration
> among different cores, is pinning still required?
> 

I think it can be as a new mode, it maybe add more msr operation, like write

> >
> > and a per-VM capability is added to configure aperfmperf mode
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > Signed-off-by: Chai Wen <chaiwen@baidu.com>
> > Signed-off-by: Jia Lina <jialina01@baidu.com>
> > ---
> > diff v2:
> > support aperfmperf pass though
> > move common codes to kvm_get_msr_common
> >
> > diff v1:
> > 1. support AMD, but not test
> 
> pt-mode doesn't work doesn't work on AMD. See below.
> 
> > 2. support per-vm capability to enable  Documentation/virt/kvm/api.rst
> > | 10 ++++++++++  arch/x86/include/asm/kvm_host.h | 11 +++++++++++
> >  arch/x86/kvm/cpuid.c            | 13 ++++++++++++-
> >  arch/x86/kvm/svm.c              |  8 ++++++++
> >  arch/x86/kvm/vmx/vmx.c          |  6 ++++++
> >  arch/x86/kvm/x86.c              | 42
> +++++++++++++++++++++++++++++++++++++++++
> >  arch/x86/kvm/x86.h              | 15 +++++++++++++++
> >  include/uapi/linux/kvm.h        |  1 +
> >  8 files changed, 105 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/virt/kvm/api.rst
> > b/Documentation/virt/kvm/api.rst index efbbe570aa9b..c3be3b6a1717
> > 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -6109,3 +6109,13 @@ KVM can therefore start protected VMs.
> >  This capability governs the KVM_S390_PV_COMMAND ioctl and the
> > KVM_MP_STATE_LOAD MP_STATE. KVM_SET_MP_STATE can fail for
> protected
> > guests when the state change is invalid.
> > +
> > +8.23 KVM_CAP_APERFMPERF
> > +----------------------------
> > +
> > +:Architectures: x86
> > +:Parameters: args[0] is aperfmperf mode;
> > +             0 for not support, 1 for software emulation, 2 for
> > +pass-through
> > +:Returns: 0 on success; -1 on error
> > +
> > +This capability indicates that KVM supports APERF and MPERF MSR
> > +registers
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index 42a2d0d3984a..81477f676f60
> > 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -820,6 +820,9 @@ struct kvm_vcpu_arch {
> >
> >  	/* AMD MSRC001_0015 Hardware Configuration */
> >  	u64 msr_hwcr;
> > +
> > +	u64 v_mperf;
> > +	u64 v_aperf;
> >  };
> >
> >  struct kvm_lpage_info {
> > @@ -885,6 +888,12 @@ enum kvm_irqchip_mode {
> >  	KVM_IRQCHIP_SPLIT,        /* created with KVM_CAP_SPLIT_IRQCHIP
> */
> >  };
> >
> > +enum kvm_aperfmperf_mode {
> > +	KVM_APERFMPERF_NONE,
> > +	KVM_APERFMPERF_SOFT,      /* software emulate aperfmperf */
> > +	KVM_APERFMPERF_PT,        /* pass-through aperfmperf to guest */
> > +};
> > +
> >  #define APICV_INHIBIT_REASON_DISABLE    0
> >  #define APICV_INHIBIT_REASON_HYPERV     1
> >  #define APICV_INHIBIT_REASON_NESTED     2
> > @@ -982,6 +991,8 @@ struct kvm_arch {
> >
> >  	struct kvm_pmu_event_filter *pmu_event_filter;
> >  	struct task_struct *nx_lpage_recovery_thread;
> > +
> > +	enum kvm_aperfmperf_mode aperfmperf_mode;
> >  };
> >
> >  struct kvm_vm_stat {
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index
> > 901cd1fdecd9..7a64ea2c3eef 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -124,6 +124,14 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
> >  					   MSR_IA32_MISC_ENABLE_MWAIT);
> >  	}
> >
> > +	best = kvm_find_cpuid_entry(vcpu, 6, 0);
> > +	if (best) {
> > +		if (guest_has_aperfmperf(vcpu->kvm) &&
> > +			boot_cpu_has(X86_FEATURE_APERFMPERF))
> > +			best->ecx |= 1;
> > +		else
> > +			best->ecx &= ~1;
> > +	}
> >  	/* Update physical-address width */
> >  	vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
> >  	kvm_mmu_reset_context(vcpu);
> > @@ -558,7 +566,10 @@ static inline int __do_cpuid_func(struct
> kvm_cpuid_array *array, u32 function)
> >  	case 6: /* Thermal management */
> >  		entry->eax = 0x4; /* allow ARAT */
> >  		entry->ebx = 0;
> > -		entry->ecx = 0;
> > +		if (boot_cpu_has(X86_FEATURE_APERFMPERF))
> > +			entry->ecx = 0x1;
> > +		else
> > +			entry->ecx = 0x0;
> >  		entry->edx = 0;
> >  		break;
> >  	/* function 7 has additional index. */ diff --git
> > a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index
> > 851e9cc79930..5646b6475049 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
>                  ^^^^^^^^^
> The latest kernel moves svm-related files to arch/x86/kvm/svm directory. You
> need to update your patches.
> 
> > @@ -2292,6 +2292,14 @@ static int svm_create_vcpu(struct kvm_vcpu
> *vcpu)
> >  	svm->msrpm = page_address(msrpm_pages);
> >  	svm_vcpu_init_msrpm(svm->msrpm);
> >
> > +	if (guest_aperfmperf_soft(vcpu->kvm)) {
> > +		set_msr_interception(svm->msrpm, MSR_IA32_MPERF, 1, 0);
> > +		set_msr_interception(svm->msrpm, MSR_IA32_APERF, 1, 0);
> > +	} else if (guest_aperfmperf_pt(vcpu->kvm)) {
> > +		set_msr_interception(svm->msrpm, MSR_IA32_MPERF, 0, 0);
> > +		set_msr_interception(svm->msrpm, MSR_IA32_APERF, 0, 0);
> > +	}
> 
> The bit setting for KVM_APERFMPERF_SOFT and KVM_APERFMPERF_PT is
> incorrect. set_msr_interception() takes read/write as parameters. When they
> are 1, it means svm doesn't intercept this specific MSR. So, you code should
> look like:
> 
>      if (guest_aperfmperf_soft(vcpu->kvm)) {
>              set_msr_interception(svm->msrpm, MSR_IA32_MPERF, 0, 0);
>              set_msr_interception(svm->msrpm, MSR_IA32_APERF, 0, 0);
>      } else if (guest_aperfmperf_pt(vcpu->kvm)) {
>              set_msr_interception(svm->msrpm, MSR_IA32_MPERF, 1, 0);
>              set_msr_interception(svm->msrpm, MSR_IA32_APERF, 1, 0);
>      }
> 
> 


Thanks, I will fix it


-Li