On Wed, Jun 13, 2018 at 06:00:22PM -0300, Eduardo Habkost wrote:
> On Tue, Jun 12, 2018 at 09:47:11PM +0300, Michael S. Tsirkin wrote:
> > With this flag, kvm allows guest to control host CPU power state. This
> > increases latency for other processes using same host CPU in an
> > unpredictable way, but if decreases idle entry/exit times for the
> > running VCPU.
> >
> > Follow-up patches will expose this capability to guest
> > (using mwait leaf).
> >
> > Based on a patch by Wanpeng Li <kernel...@gmail.com> .
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > include/sysemu/sysemu.h | 1 +
> > target/i386/kvm.c | 22 ++++++++++++++++++++++
> > vl.c | 6 ++++++
> > qemu-options.hx | 9 +++++++--
> > 4 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index e893f72f3b..b921c6f3b7 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -128,6 +128,7 @@ extern bool boot_strict;
> > extern uint8_t *boot_splash_filedata;
> > extern size_t boot_splash_filedata_size;
> > extern bool enable_mlock;
> > +extern bool enable_cpu_pm;
>
> After looking at patch 2/2, I see that the global variable is
> useful, and it's consistent with the existing enable_mlock
> variable.
>
> > extern uint8_t qemu_extra_params_fw[2];
> > extern QEMUClockType rtc_clock;
> > extern const char *mem_path;
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 44f70733e7..f093d55209 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -1357,6 +1357,28 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> > smram_machine_done.notify = register_smram_listener;
> > qemu_add_machine_init_done_notifier(&smram_machine_done);
> > }
> > +
> > + if (enable_cpu_pm) {
> > + int disable_exits = kvm_check_extension(s,
> > KVM_CAP_X86_DISABLE_EXITS);
> > + int ret;
> > +
> > +/* Work around for kernel header with a typo. TODO: fix header and drop. */
> > +#if defined(KVM_X86_DISABLE_EXITS_HTL) &&
> > !defined(KVM_X86_DISABLE_EXITS_HLT)
> > +#define KVM_X86_DISABLE_EXITS_HLT KVM_X86_DISABLE_EXITS_HTL
> > +#endif
> > + if (disable_exits) {
> > + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > + KVM_X86_DISABLE_EXITS_HLT |
> > + KVM_X86_DISABLE_EXITS_PAUSE);
> > + }
> > +
> > + ret = kvm_vm_enable_cap(s, KVM_CAP_X86_DISABLE_EXITS, 0,
> > + disable_exits);
>
> Isn't the kvm_vm_enable_cap() call supposed to be inside the "if
> (disable_exits)" block?
Doing it like this causes a warning if pm is requested but
disable halt is not supported.
But I can move it, sure - let me know.
> > + if (ret < 0) {
> > + error_report("kvm: guest stopping CPU not supported: %s",
> > strerror(-ret));
> > + }
> > + }
> > +
> > return 0;
> > }
> [...]
>
> --
> Eduardo
