diff mbox

i386: Allow monitor / mwait cpuid override

Message ID 1490624810-44752-1-git-send-email-agraf@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Alexander Graf March 27, 2017, 2:26 p.m. UTC
KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
instructions. There is work undergoing to enable actual execution
of these inside of KVM, but nobody really wants to expose the feature
to the guest by default, as it would eat up all of the host CPU.

So today there is no streamlined way to actually notify the guest that
it's ok to execute MONITOR / MWAIT, even when we want to explicitly
leave the guest in guest context.

This patch adds a new -cpu parameter called "mwait" which - when
enabled - force enables the MONITOR / MWAIT CPUID flag, even when
the underlying accel framework does not explicitly advertise support.

With that in place, we can explicitly allow users to specify that
they want have the guest execute MONITOR / MWAIT in its idle loop.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target/i386/cpu.c | 5 +++++
 target/i386/cpu.h | 1 +
 2 files changed, 6 insertions(+)

Comments

Eduardo Habkost March 27, 2017, 3:46 p.m. UTC | #1
On Mon, Mar 27, 2017 at 04:26:50PM +0200, Alexander Graf wrote:
> KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
> instructions. There is work undergoing to enable actual execution
> of these inside of KVM, but nobody really wants to expose the feature
> to the guest by default, as it would eat up all of the host CPU.

Isn't this something that should be reported using
KVM_GET_EMULATED_CPUID? (QEMU still doesn't know how to use
KVM_GET_EMULATED_CPUID, however.)

> 
> So today there is no streamlined way to actually notify the guest that
> it's ok to execute MONITOR / MWAIT, even when we want to explicitly
> leave the guest in guest context.

I'm not familiar with the variables involved in this decision.
How exactly would somebody (human or software) determine if it's
really ok to let the guest execute MONITOR / MWAIT?

Under what circumstances do you expect this to be used? Is this
just for debugging and development?

> 
> This patch adds a new -cpu parameter called "mwait" which - when
> enabled - force enables the MONITOR / MWAIT CPUID flag, even when
> the underlying accel framework does not explicitly advertise support.
> 

If you really want something that makes QEMU ignore what the
accel code is reporting, I would prefer a syntax that could be
used for other features too, like "-cpu ...,monitor=force".

> With that in place, we can explicitly allow users to specify that
> they want have the guest execute MONITOR / MWAIT in its idle loop.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target/i386/cpu.c | 5 +++++
>  target/i386/cpu.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 7aa7622..c44020b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3460,6 +3460,10 @@ static int x86_cpu_filter_features(X86CPU *cpu)
>              x86_cpu_get_supported_feature_word(w, false);
>          uint32_t requested_features = env->features[w];
>          env->features[w] &= host_feat;
> +        if (cpu->expose_monitor && (w == FEAT_1_ECX)) {
> +            /* Force monitor feature in */
> +            env->features[w] |= CPUID_EXT_MONITOR;
> +        }
>          cpu->filtered_features[w] = requested_features & ~env->features[w];
>          if (cpu->filtered_features[w]) {
>              rv = 1;
> @@ -3988,6 +3992,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> +    DEFINE_PROP_BOOL("mwait", X86CPU, expose_monitor, false),
>      DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
>      DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
>      DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 07401ad..7400d00 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1214,6 +1214,7 @@ struct X86CPU {
>      bool check_cpuid;
>      bool enforce_cpuid;
>      bool expose_kvm;
> +    bool expose_monitor;
>      bool migratable;
>      bool max_features; /* Enable all supported features automatically */
>      uint32_t apic_id;
> -- 
> 1.8.5.6
>
Alexander Graf March 27, 2017, 4:10 p.m. UTC | #2
On 27/03/2017 17:46, Eduardo Habkost wrote:
> On Mon, Mar 27, 2017 at 04:26:50PM +0200, Alexander Graf wrote:
>> KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
>> instructions. There is work undergoing to enable actual execution
>> of these inside of KVM, but nobody really wants to expose the feature
>> to the guest by default, as it would eat up all of the host CPU.
>
> Isn't this something that should be reported using
> KVM_GET_EMULATED_CPUID? (QEMU still doesn't know how to use
> KVM_GET_EMULATED_CPUID, however.)

Depends how you look at it. In KVM land there are basically 3 ways to 
deal with MONITOR/MWAIT:

   1) #VMEXIT on every execution, treat them as NOP
   2) let the guest natively execute them (looks like a busy loop for 
the host, but saves power)
   3) be smart in KVM about it, add actual emulation and adaptively 
allow for native mwait execution or emulated mwait which means we can 
run inside host context

>> So today there is no streamlined way to actually notify the guest that
>> it's ok to execute MONITOR / MWAIT, even when we want to explicitly
>> leave the guest in guest context.
>
> I'm not familiar with the variables involved in this decision.
> How exactly would somebody (human or software) determine if it's
> really ok to let the guest execute MONITOR / MWAIT?
>
> Under what circumstances do you expect this to be used? Is this
> just for debugging and development?

The main reason this is bubbling up at all are IPC intensive workloads. 
Imagine you have the following:

CPU0 goes idle (waiting on something)
CPU1 wants to wake up CPU0 (because wait time is over)

In a normal KVM environment what you get is that

CPU0 calls HLT (going into KVM to do other work)
CPU1 triggers IPI via emulated APIC to wake up CPU0

However with actual native MWAIT what happens is

CPU0 calls MWAIT, stays in guest context ("wasting" CPU time)
CPU1 writes a byte to a memory location, waking up CPU0

With that scheme, you get your IPC latency down by a *huge* margin.


When is that useful? When you only have a single VM on your host for 
example, so you want virtual machines for the sake of administration, 
not for overprovisioning.

>
>>
>> This patch adds a new -cpu parameter called "mwait" which - when
>> enabled - force enables the MONITOR / MWAIT CPUID flag, even when
>> the underlying accel framework does not explicitly advertise support.
>>
>
> If you really want something that makes QEMU ignore what the
> accel code is reporting, I would prefer a syntax that could be
> used for other features too, like "-cpu ...,monitor=force".

That sounds like a pretty nice idea and much more scalable. Let me see 
if I can somehow pull that off :).


Alex
Alexander Graf March 27, 2017, 4:22 p.m. UTC | #3
On 27/03/2017 17:46, Eduardo Habkost wrote:
> On Mon, Mar 27, 2017 at 04:26:50PM +0200, Alexander Graf wrote:
>> KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
>> instructions. There is work undergoing to enable actual execution
>> of these inside of KVM, but nobody really wants to expose the feature
>> to the guest by default, as it would eat up all of the host CPU.
>
> Isn't this something that should be reported using
> KVM_GET_EMULATED_CPUID? (QEMU still doesn't know how to use
> KVM_GET_EMULATED_CPUID, however.)
>
>>
>> So today there is no streamlined way to actually notify the guest that
>> it's ok to execute MONITOR / MWAIT, even when we want to explicitly
>> leave the guest in guest context.
>
> I'm not familiar with the variables involved in this decision.
> How exactly would somebody (human or software) determine if it's
> really ok to let the guest execute MONITOR / MWAIT?
>
> Under what circumstances do you expect this to be used? Is this
> just for debugging and development?
>
>>
>> This patch adds a new -cpu parameter called "mwait" which - when
>> enabled - force enables the MONITOR / MWAIT CPUID flag, even when
>> the underlying accel framework does not explicitly advertise support.
>>
>
> If you really want something that makes QEMU ignore what the
> accel code is reporting, I would prefer a syntax that could be
> used for other features too, like "-cpu ...,monitor=force".

Is there any QOM property that does this yet? I can't seem to find any 
tri-state (on/off/force) parsers and I want to make sure I don't 
reinvent the wheel.


Alex
Paolo Bonzini March 27, 2017, 4:27 p.m. UTC | #4
On 27/03/2017 18:22, Alexander Graf wrote:
>>
>> If you really want something that makes QEMU ignore what the
>> accel code is reporting, I would prefer a syntax that could be
>> used for other features too, like "-cpu ...,monitor=force".
> 
> Is there any QOM property that does this yet? I can't seem to find any
> tri-state (on/off/force) parsers and I want to make sure I don't
> reinvent the wheel.

No, but there's on/off/auto that you can copy.

Paolo
Eduardo Habkost March 27, 2017, 4:33 p.m. UTC | #5
On Mon, Mar 27, 2017 at 06:10:43PM +0200, Alexander Graf wrote:
> 
> 
> On 27/03/2017 17:46, Eduardo Habkost wrote:
> > On Mon, Mar 27, 2017 at 04:26:50PM +0200, Alexander Graf wrote:
> > > KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
> > > instructions. There is work undergoing to enable actual execution
> > > of these inside of KVM, but nobody really wants to expose the feature
> > > to the guest by default, as it would eat up all of the host CPU.
> > 
> > Isn't this something that should be reported using
> > KVM_GET_EMULATED_CPUID? (QEMU still doesn't know how to use
> > KVM_GET_EMULATED_CPUID, however.)
> 
> Depends how you look at it. In KVM land there are basically 3 ways to deal
> with MONITOR/MWAIT:
> 
>   1) #VMEXIT on every execution, treat them as NOP
>   2) let the guest natively execute them (looks like a busy loop for the
> host, but saves power)
>   3) be smart in KVM about it, add actual emulation and adaptively allow for
> native mwait execution or emulated mwait which means we can run inside host
> context

Which case is going to be enabled when using your patch? Don't
you want to make that configurable?

This looks like configuration that can't be easily represented
using the GET_SUPPORTED_CPUID/KVM_SET_CPUID abstraction, as the
ability to enable the feature can't be represented by a single
bit.

We already have a few cases where we make
kvm_arch_get_supported_cpuid() look at something other than just
GET_SUPPORTED_CPUID return values (most cases are related to
in-kernel irqchip). We can change kvm_arch_get_supported_cpuid()
to look at other flags (like one that would configure mwait
behavior), and decide to report CPUID_EXT_MONITOR on those cases.

Would something like the following make sense?

* Having a "mwait=(none|nop|native|smart)" option, to choose
  mwait behavior
* Making kvm_arch_get_supported_cpuid() return CPUID_EXT_MONITOR
  as supported only if the "mwait" option is not "none".

Then:
* "-cpu ...,monitor=on" would fail, because mwait=none would be
  the default
* "-cpu ...,mwait=none,monitor=on" would fail
* "-cpu ...,mwait=(nop|native|smart)" would fail if
  host KVM doesn't report the corresponding mwait behavior as
  supported
* "-cpu ...,mwait=(nop|native|smart),monitor=on" would work
* "-cpu ...,mwait=(nop|native|smart)" would also work, and
  probably should enable CPUID_EXT_MONITOR flag by default

We could still implement "monitor=force" for debugging, but I
think we need something safer than "just ignore what KVM is
telling us" for production.
Alexander Graf March 27, 2017, 4:42 p.m. UTC | #6
On 27/03/2017 18:33, Eduardo Habkost wrote:
> On Mon, Mar 27, 2017 at 06:10:43PM +0200, Alexander Graf wrote:
>>
>>
>> On 27/03/2017 17:46, Eduardo Habkost wrote:
>>> On Mon, Mar 27, 2017 at 04:26:50PM +0200, Alexander Graf wrote:
>>>> KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
>>>> instructions. There is work undergoing to enable actual execution
>>>> of these inside of KVM, but nobody really wants to expose the feature
>>>> to the guest by default, as it would eat up all of the host CPU.
>>>
>>> Isn't this something that should be reported using
>>> KVM_GET_EMULATED_CPUID? (QEMU still doesn't know how to use
>>> KVM_GET_EMULATED_CPUID, however.)
>>
>> Depends how you look at it. In KVM land there are basically 3 ways to deal
>> with MONITOR/MWAIT:
>>
>>   1) #VMEXIT on every execution, treat them as NOP
>>   2) let the guest natively execute them (looks like a busy loop for the
>> host, but saves power)
>>   3) be smart in KVM about it, add actual emulation and adaptively allow for
>> native mwait execution or emulated mwait which means we can run inside host
>> context
>
> Which case is going to be enabled when using your patch? Don't
> you want to make that configurable?
>
> This looks like configuration that can't be easily represented
> using the GET_SUPPORTED_CPUID/KVM_SET_CPUID abstraction, as the
> ability to enable the feature can't be represented by a single
> bit.
>
> We already have a few cases where we make
> kvm_arch_get_supported_cpuid() look at something other than just
> GET_SUPPORTED_CPUID return values (most cases are related to
> in-kernel irqchip). We can change kvm_arch_get_supported_cpuid()
> to look at other flags (like one that would configure mwait
> behavior), and decide to report CPUID_EXT_MONITOR on those cases.
>
> Would something like the following make sense?
>
> * Having a "mwait=(none|nop|native|smart)" option, to choose
>   mwait behavior
> * Making kvm_arch_get_supported_cpuid() return CPUID_EXT_MONITOR
>   as supported only if the "mwait" option is not "none".
>
> Then:
> * "-cpu ...,monitor=on" would fail, because mwait=none would be
>   the default
> * "-cpu ...,mwait=none,monitor=on" would fail
> * "-cpu ...,mwait=(nop|native|smart)" would fail if
>   host KVM doesn't report the corresponding mwait behavior as
>   supported
> * "-cpu ...,mwait=(nop|native|smart),monitor=on" would work
> * "-cpu ...,mwait=(nop|native|smart)" would also work, and
>   probably should enable CPUID_EXT_MONITOR flag by default
>
> We could still implement "monitor=force" for debugging, but I
> think we need something safer than "just ignore what KVM is
> telling us" for production.

I personally think we want the =force for any cpuid bit that we want to 
randomly poke into the expose CPUID bitmap and if a customer is daring 
enough, leave that to him as an option.

For anything else, I wouldn't overengineer this interface. I think 
eventually we want to just natively support emulated mwait with proper 
adaptive logic on when to trap vs execute mwait inside the guest (as 
mentioned in the kvm thread on this). Once that's implemented, KVM can 
just expose the MONITOR bit as supported and everyone gets it automatically.

If at that point, the adaptive interface performs much worse than the 
naive one, we can still think about pushing this into an enable_cap and 
add a specific mwait= option.


Alex
Eduardo Habkost March 27, 2017, 6:06 p.m. UTC | #7
On Mon, Mar 27, 2017 at 06:42:49PM +0200, Alexander Graf wrote:
> 
> 
> On 27/03/2017 18:33, Eduardo Habkost wrote:
> > On Mon, Mar 27, 2017 at 06:10:43PM +0200, Alexander Graf wrote:
> > > 
> > > 
> > > On 27/03/2017 17:46, Eduardo Habkost wrote:
> > > > On Mon, Mar 27, 2017 at 04:26:50PM +0200, Alexander Graf wrote:
> > > > > KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
> > > > > instructions. There is work undergoing to enable actual execution
> > > > > of these inside of KVM, but nobody really wants to expose the feature
> > > > > to the guest by default, as it would eat up all of the host CPU.
> > > > 
> > > > Isn't this something that should be reported using
> > > > KVM_GET_EMULATED_CPUID? (QEMU still doesn't know how to use
> > > > KVM_GET_EMULATED_CPUID, however.)
> > > 
> > > Depends how you look at it. In KVM land there are basically 3 ways to deal
> > > with MONITOR/MWAIT:
> > > 
> > >   1) #VMEXIT on every execution, treat them as NOP
> > >   2) let the guest natively execute them (looks like a busy loop for the
> > > host, but saves power)
> > >   3) be smart in KVM about it, add actual emulation and adaptively allow for
> > > native mwait execution or emulated mwait which means we can run inside host
> > > context
> > 
> > Which case is going to be enabled when using your patch? Don't
> > you want to make that configurable?

If the answer to this question is "no", then we can ignore all
the rest of my previous message. :)

> > 
> > This looks like configuration that can't be easily represented
> > using the GET_SUPPORTED_CPUID/KVM_SET_CPUID abstraction, as the
> > ability to enable the feature can't be represented by a single
> > bit.
> > 
> > We already have a few cases where we make
> > kvm_arch_get_supported_cpuid() look at something other than just
> > GET_SUPPORTED_CPUID return values (most cases are related to
> > in-kernel irqchip). We can change kvm_arch_get_supported_cpuid()
> > to look at other flags (like one that would configure mwait
> > behavior), and decide to report CPUID_EXT_MONITOR on those cases.
> > 
> > Would something like the following make sense?
> > 
> > * Having a "mwait=(none|nop|native|smart)" option, to choose
> >   mwait behavior
> > * Making kvm_arch_get_supported_cpuid() return CPUID_EXT_MONITOR
> >   as supported only if the "mwait" option is not "none".
> > 
> > Then:
> > * "-cpu ...,monitor=on" would fail, because mwait=none would be
> >   the default
> > * "-cpu ...,mwait=none,monitor=on" would fail
> > * "-cpu ...,mwait=(nop|native|smart)" would fail if
> >   host KVM doesn't report the corresponding mwait behavior as
> >   supported
> > * "-cpu ...,mwait=(nop|native|smart),monitor=on" would work
> > * "-cpu ...,mwait=(nop|native|smart)" would also work, and
> >   probably should enable CPUID_EXT_MONITOR flag by default
> > 
> > We could still implement "monitor=force" for debugging, but I
> > think we need something safer than "just ignore what KVM is
> > telling us" for production.
> 
> I personally think we want the =force for any cpuid bit that we want to
> randomly poke into the expose CPUID bitmap and if a customer is daring
> enough, leave that to him as an option.
> 
> For anything else, I wouldn't overengineer this interface. I think
> eventually we want to just natively support emulated mwait with proper
> adaptive logic on when to trap vs execute mwait inside the guest (as
> mentioned in the kvm thread on this). Once that's implemented, KVM can just
> expose the MONITOR bit as supported and everyone gets it automatically.
> 
> If at that point, the adaptive interface performs much worse than the naive
> one, we can still think about pushing this into an enable_cap and add a
> specific mwait= option.

Sounds good to me.

I thought the user still had to choose between the options above
somehow, and that KVM was not going to return MONITOR on
GET_SUPPORTED_CPUID because of that. If getting adaptative logic
that's good enough to add MONITOR to GET_SUPPORTED_CPUID is still
a possibility, then that's even better.
Gonglei (Arei) Feb. 27, 2018, 9:52 a.m. UTC | #8
Hi all,

Guests could achive good performance in 'Message Passing Workloads' 
scenarios when knowing the X86_FEATURE_MWAIT feature which is presented by qemu. 
the reason is that after knowing that feature, 
the guest could use mwait method, which saves VMEXIT, 
to do idle, and achives high performace in latency-sensitive scenario.

Is there any plan for this patch? 

Or May I send a updated version based on yours? @Alex?

Thanks,
-Gonglei


> -----Original Message-----
> From: Qemu-devel
> [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> Behalf Of Alexander Graf
> Sent: Monday, March 27, 2017 10:27 PM
> To: qemu-devel@nongnu.org
> Cc: Paolo Bonzini; Eduardo Habkost; Richard Henderson
> Subject: [Qemu-devel] [PATCH] i386: Allow monitor / mwait cpuid override
> 
> KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT
> instructions. There is work undergoing to enable actual execution
> of these inside of KVM, but nobody really wants to expose the feature
> to the guest by default, as it would eat up all of the host CPU.
> 
> So today there is no streamlined way to actually notify the guest that
> it's ok to execute MONITOR / MWAIT, even when we want to explicitly
> leave the guest in guest context.
> 
> This patch adds a new -cpu parameter called "mwait" which - when
> enabled - force enables the MONITOR / MWAIT CPUID flag, even when
> the underlying accel framework does not explicitly advertise support.
> 
> With that in place, we can explicitly allow users to specify that
> they want have the guest execute MONITOR / MWAIT in its idle loop.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target/i386/cpu.c | 5 +++++
>  target/i386/cpu.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 7aa7622..c44020b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3460,6 +3460,10 @@ static int x86_cpu_filter_features(X86CPU *cpu)
>              x86_cpu_get_supported_feature_word(w, false);
>          uint32_t requested_features = env->features[w];
>          env->features[w] &= host_feat;
> +        if (cpu->expose_monitor && (w == FEAT_1_ECX)) {
> +            /* Force monitor feature in */
> +            env->features[w] |= CPUID_EXT_MONITOR;
> +        }
>          cpu->filtered_features[w] = requested_features &
> ~env->features[w];
>          if (cpu->filtered_features[w]) {
>              rv = 1;
> @@ -3988,6 +3992,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> +    DEFINE_PROP_BOOL("mwait", X86CPU, expose_monitor, false),
>      DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
>      DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
>      DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 07401ad..7400d00 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1214,6 +1214,7 @@ struct X86CPU {
>      bool check_cpuid;
>      bool enforce_cpuid;
>      bool expose_kvm;
> +    bool expose_monitor;
>      bool migratable;
>      bool max_features; /* Enable all supported features automatically */
>      uint32_t apic_id;
> --
> 1.8.5.6
>
Alexander Graf Feb. 27, 2018, 11:55 p.m. UTC | #9
On 27.02.18 10:52, Gonglei (Arei) wrote:
> Hi all,
> 
> Guests could achive good performance in 'Message Passing Workloads' 
> scenarios when knowing the X86_FEATURE_MWAIT feature which is presented by qemu. 
> the reason is that after knowing that feature, 
> the guest could use mwait method, which saves VMEXIT, 
> to do idle, and achives high performace in latency-sensitive scenario.
> 
> Is there any plan for this patch? 
> 
> Or May I send a updated version based on yours? @Alex?

Oh, did I drop the ball on this one? If that's the case, sure, go ahead.


Alex
Paolo Bonzini Feb. 28, 2018, 11:50 a.m. UTC | #10
On 28/02/2018 00:55, Alexander Graf wrote:
> 
> 
> On 27.02.18 10:52, Gonglei (Arei) wrote:
>> Hi all,
>>
>> Guests could achive good performance in 'Message Passing Workloads' 
>> scenarios when knowing the X86_FEATURE_MWAIT feature which is presented by qemu. 
>> the reason is that after knowing that feature, 
>> the guest could use mwait method, which saves VMEXIT, 
>> to do idle, and achives high performace in latency-sensitive scenario.
>>
>> Is there any plan for this patch? 
>>
>> Or May I send a updated version based on yours? @Alex?
> 
> Oh, did I drop the ball on this one? If that's the case, sure, go ahead.

Hi, it is probably best to implement this feature based on the
HINT_DEDICATED cpuid bit that Wanpeng Li is adding.

Paolo
Wanpeng Li Feb. 28, 2018, 12:24 p.m. UTC | #11
On 2/28/18 7:50 PM, Paolo Bonzini wrote:

> On 28/02/2018 00:55, Alexander Graf wrote:

>>

>> On 27.02.18 10:52, Gonglei (Arei) wrote:

>>> Hi all,

>>>

>>> Guests could achive good performance in 'Message Passing Workloads'

>>> scenarios when knowing the X86_FEATURE_MWAIT feature which is presented by qemu.

>>> the reason is that after knowing that feature,

>>> the guest could use mwait method, which saves VMEXIT,

>>> to do idle, and achives high performace in latency-sensitive scenario.

>>>

>>> Is there any plan for this patch?

>>>

>>> Or May I send a updated version based on yours? @Alex?

>> Oh, did I drop the ball on this one? If that's the case, sure, go ahead.

> Hi, it is probably best to implement this feature based on the

> HINT_DEDICATED cpuid bit that Wanpeng Li is adding.


https://lkml.org/lkml/2018/2/13/624 As you pointed out, MWAIT/HLT/PAUSE 
non-exiting should be implemented at the same time and I'm still working 
on it. I will prepare patches for both the qemu and kvm stuffs. :)

Regards,
Wanpeng Li
diff mbox

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 7aa7622..c44020b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3460,6 +3460,10 @@  static int x86_cpu_filter_features(X86CPU *cpu)
             x86_cpu_get_supported_feature_word(w, false);
         uint32_t requested_features = env->features[w];
         env->features[w] &= host_feat;
+        if (cpu->expose_monitor && (w == FEAT_1_ECX)) {
+            /* Force monitor feature in */
+            env->features[w] |= CPUID_EXT_MONITOR;
+        }
         cpu->filtered_features[w] = requested_features & ~env->features[w];
         if (cpu->filtered_features[w]) {
             rv = 1;
@@ -3988,6 +3992,7 @@  static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
+    DEFINE_PROP_BOOL("mwait", X86CPU, expose_monitor, false),
     DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
     DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
     DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 07401ad..7400d00 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1214,6 +1214,7 @@  struct X86CPU {
     bool check_cpuid;
     bool enforce_cpuid;
     bool expose_kvm;
+    bool expose_monitor;
     bool migratable;
     bool max_features; /* Enable all supported features automatically */
     uint32_t apic_id;