Re: 答复: [PATCH][v5] KVM: X86: support APERF/MPERF registers

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 5/31/2020 10:08 AM, Li,Rongqing wrote:
> 
> 
>> -----邮件原件-----
>> 发件人: Xiaoyao Li [mailto:xiaoyao.li@intel.com]
>> 发送时间: 2020年5月30日 18:40
>> 收件人: 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;
>> wei.huang2@amd.com
>> 主题: Re: [PATCH][v5] KVM: X86: support APERF/MPERF registers
>>
>> On 5/30/2020 12:35 PM, 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
>>> 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.
>>>
>>> And a per-VM capability is added to configure aperfmperf mode
>>>
>>
>> [...]
>>
>>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index
>>> cd708b0b460a..c960dda4251b 100644
>>> --- a/arch/x86/kvm/cpuid.c
>>> +++ b/arch/x86/kvm/cpuid.c
>>> @@ -122,6 +122,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;
>>> +	}
>>
>> In my understanding, KVM allows userspace to set a CPUID feature bit for
>> guest even if hardware doesn't support the feature.
>>
>> So what makes X86_FEATURE_APERFMPERF different here? Is there any
>> concern I miss?
>>
>> -Xiaoyao
> 
> Whether software emulation for aperf/mperf or pass-through depends on host cpu aperf/mperf feature.
>   
> Software emulation: the period of aperf/mperf in guest mode are accumulated as emulated value
> 

I know it that you want to ensure the correctness of exposure of 
aperf/mperf.

But there are so many features other than aperf/mperf that KVM reports 
the supported settings of them through KVM_GET_SUPPORTED_CPUID, but 
doesn't check nor force the correctness of userspace input. i.e., KVM 
allows userspace to set bogus CPUID settings as long as it doesn't break 
KVM (host kernel).

Indeed, bogus CPUID settings more than likely breaks the guest. But it's 
not KVM's fault. KVM just do what userspace wants.

IMO, If we really want to ensure the correctness of userspace provided 
CPUID settings, we need to return ERROR to userspace instead of fixing 
it siliently.

- Xiaoyao