Re: [Qemu-devel] [PATCH] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS

[Wanpeng Li]

2018-03-24 4:18 GMT+08:00 Eduardo Habkost <address@hidden>:
> On Fri, Mar 16, 2018 at 07:36:42AM -0700, Wanpeng Li wrote:
>> From: Wanpeng Li <address@hidden>
>>
>> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace 
>> with
>> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
>> in order that to improve latency in some workloads.
>>
>> Cc: Paolo Bonzini <address@hidden>
>> Cc: Radim Krčmář <address@hidden>
>> Cc: Eduardo Habkost <address@hidden>
>> Signed-off-by: Wanpeng Li <address@hidden>
>
>
> Thanks.
>
> Patch looks good (except for comment below), but I would like to
> see QEMU documentation mentioning what exactly are the practical
> consequences of setting "+kvm-hint-dedicated" (especially what
> could happen if people enable the flag without properly
> configuring vCPU pinning).
>
>
> [...]
>> +    if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
>> +        int disable_exits = kvm_check_extension(cs->kvm_state, 
>> KVM_CAP_X86_DISABLE_EXITS);
>> +        if (disable_exits) {
>> +            disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> +                              KVM_X86_DISABLE_EXITS_HLT |
>> +                              KVM_X86_DISABLE_EXITS_PAUSE);
>> +        }
>
> Documentation/virtual/kvm/api.txt says that KVM_FEATURE_PV_UNHALT
> shouldn't be enabled if disabling HLT exits.  This needs to be
> handled by QEMU.

This is handled by KVM(in kvm_update_cpuid()) currently to avoid kvm
userspace doing something crazy.
https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=caa057a2cad647fb368a12c8e6c410ac4c28e063

>
> Probably the simplest solution is to not allow kvm-hint-dedicated
> to be enabled if kvm-pv-unhalt is.  This should be mentioned in
> QEMU documentation, also, especially considering that we might
> enable kvm-pv-unhalt by default in future QEMU versions.

As Locking guy Waiman mentioned before:
> Generally speaking, unfair lock performs well for VMs with a small number of 
> vCPUs. Native qspinlock may perform better than pvqspinlock if there is vCPU 
> pinning and there is no vCPU over-commitment.
I think +kvm-hint-dedicated, -kvm-pv-unhalt is more suitable for vCPU
pinning and there is no vCPU over-commitment, on the contrary,
-kvm-hint-dedicated, +kvm-pv-unhalt is more prefer.

Regards,
Wanpeng Li