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

[Michael S. Tsirkin]

On Wed, Mar 28, 2018 at 03:31:23PM -0300, Eduardo Habkost wrote:
> On Wed, Mar 28, 2018 at 03:06:23AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Mar 27, 2018 at 06:36:46PM -0300, Eduardo Habkost wrote:
> > > On Tue, Mar 27, 2018 at 10:42:56PM +0300, Michael S. Tsirkin wrote:
> > > > 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.
> > > > > 
> > > [...]
> > > > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > > > > index d23fff1..95ed9eb 100644
> > > > > --- a/target/i386/kvm.c
> > > > > +++ b/target/i386/kvm.c
> > > > > @@ -999,6 +999,18 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > > > >          }
> > > > >      }
> > > > >  
> > > > > +    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);
> > > > > +        }
> > > > > +        if (kvm_vm_enable_cap(cs->kvm_state, 
> > > > > KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
> > > > > +            error_report("kvm: DISABLE EXITS not supported");
> > > > > +        }
> > > > > +    }
> > > > > +
> > > > >      qemu_add_vm_change_state_handler(cpu_update_state, env);
> > > > >  
> > > > >      c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> > > > 
> > > > Why not a bit per capability?
> > > > I can see how someone might want to disable mwait exists
> > > > but not the rest of them.
> > > 
> > > kvm-hint-dedicated=on should be used only if the physical CPU is
> > > dedicated to the VCPU task.  Are there any advantages of getting
> > > vmexits for HLT and PAUSE if no other task is going to use the
> > > CPU?
> > 
> > No but there are advantages to using mwait even without a dedicated host
> > CPU (VCPUs can wake up each other without exiting to hypervisor).
> 
> Are there any downsides?  What needs to be taken into account
> when deciding if mwait exits can be safely disabled?

Exit might take longer as it might need to wake up CPU
from a deep C state.
So one needs to know which C states are enabled on host
and whether any tasks on the same CPU are latency sensitive.

> 
> > 
> > Which is my point - there should be a separate flag to disable mwait
> > exiting only.
> 
> Adding new command-line option is possible, but not necessary for
> the dedicated-CPU use case.  This means this patch is already
> useful without adding new flags.

True.

> -- 
> Eduardo