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

[Eduardo Habkost]

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?


> 
> 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.

-- 
Eduardo