[Qemu-devel] [PATCH] i386: Enable IA32_MISC_ENABLE MWAIT bit when exposing mwait/monitor

Wanpeng Li posted 1 patch 4 years, 11 months ago
Test asan passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1557813999-9175-1-git-send-email-wanpengli@tencent.com
target/i386/cpu.c | 3 +++
target/i386/cpu.h | 1 +
2 files changed, 4 insertions(+)
[Qemu-devel] [PATCH] i386: Enable IA32_MISC_ENABLE MWAIT bit when exposing mwait/monitor
Posted by Wanpeng Li 4 years, 11 months ago
From: Wanpeng Li <wanpengli@tencent.com>

The CPUID.01H:ECX[bit 3] ought to mirror the value of the MSR 
IA32_MISC_ENABLE MWAIT bit and as userspace has control of them 
both, it is userspace's job to configure both bits to match on 
the initial setup.

Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 target/i386/cpu.c | 3 +++
 target/i386/cpu.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 722c551..40b6108 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4729,6 +4729,9 @@ static void x86_cpu_reset(CPUState *s)
 
     env->pat = 0x0007040600070406ULL;
     env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT;
+    if (enable_cpu_pm) {
+        env->msr_ia32_misc_enable |= MSR_IA32_MISC_ENABLE_MWAIT;
+    }
 
     memset(env->dr, 0, sizeof(env->dr));
     env->dr[6] = DR6_FIXED_1;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0128910..b94c329 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -387,6 +387,7 @@ typedef enum X86Seg {
 #define MSR_IA32_MISC_ENABLE            0x1a0
 /* Indicates good rep/movs microcode on some processors: */
 #define MSR_IA32_MISC_ENABLE_DEFAULT    1
+#define MSR_IA32_MISC_ENABLE_MWAIT      (1ULL << 18)
 
 #define MSR_MTRRphysBase(reg)           (0x200 + 2 * (reg))
 #define MSR_MTRRphysMask(reg)           (0x200 + 2 * (reg) + 1)
-- 
2.7.4


Re: [Qemu-devel] [PATCH] i386: Enable IA32_MISC_ENABLE MWAIT bit when exposing mwait/monitor
Posted by Paolo Bonzini 4 years, 11 months ago
On 14/05/19 08:06, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> The CPUID.01H:ECX[bit 3] ought to mirror the value of the MSR 
> IA32_MISC_ENABLE MWAIT bit and as userspace has control of them 
> both, it is userspace's job to configure both bits to match on 
> the initial setup.

Queued, thanks.

Paolo

> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>  target/i386/cpu.c | 3 +++
>  target/i386/cpu.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 722c551..40b6108 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4729,6 +4729,9 @@ static void x86_cpu_reset(CPUState *s)
>  
>      env->pat = 0x0007040600070406ULL;
>      env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT;
> +    if (enable_cpu_pm) {
> +        env->msr_ia32_misc_enable |= MSR_IA32_MISC_ENABLE_MWAIT;
> +    }
>  
>      memset(env->dr, 0, sizeof(env->dr));
>      env->dr[6] = DR6_FIXED_1;
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 0128910..b94c329 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -387,6 +387,7 @@ typedef enum X86Seg {
>  #define MSR_IA32_MISC_ENABLE            0x1a0
>  /* Indicates good rep/movs microcode on some processors: */
>  #define MSR_IA32_MISC_ENABLE_DEFAULT    1
> +#define MSR_IA32_MISC_ENABLE_MWAIT      (1ULL << 18)
>  
>  #define MSR_MTRRphysBase(reg)           (0x200 + 2 * (reg))
>  #define MSR_MTRRphysMask(reg)           (0x200 + 2 * (reg) + 1)
> 


Re: [Qemu-devel] [PATCH] i386: Enable IA32_MISC_ENABLE MWAIT bit when exposing mwait/monitor
Posted by Eduardo Habkost 4 years, 11 months ago
On Mon, May 20, 2019 at 02:59:53PM +0200, Paolo Bonzini wrote:
> On 14/05/19 08:06, Wanpeng Li wrote:
> > From: Wanpeng Li <wanpengli@tencent.com>
> > 
> > The CPUID.01H:ECX[bit 3] ought to mirror the value of the MSR 
> > IA32_MISC_ENABLE MWAIT bit and as userspace has control of them 
> > both, it is userspace's job to configure both bits to match on 
> > the initial setup.
> 
> Queued, thanks.
> 
> Paolo
> 
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> > ---
> >  target/i386/cpu.c | 3 +++
> >  target/i386/cpu.h | 1 +
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 722c551..40b6108 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -4729,6 +4729,9 @@ static void x86_cpu_reset(CPUState *s)
> >  
> >      env->pat = 0x0007040600070406ULL;
> >      env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT;
> > +    if (enable_cpu_pm) {
> > +        env->msr_ia32_misc_enable |= MSR_IA32_MISC_ENABLE_MWAIT;
> > +    }

What if enable_cpu_pm is false but we're running TCG, or if
enable_cpu_pm is true but we're not using -cpu host?

Shouldn't this be conditional on
  (env->features[FEAT_1_ECX] & CPUID_EXT_MONITOR)
instead?

> >  
> >      memset(env->dr, 0, sizeof(env->dr));
> >      env->dr[6] = DR6_FIXED_1;
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 0128910..b94c329 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -387,6 +387,7 @@ typedef enum X86Seg {
> >  #define MSR_IA32_MISC_ENABLE            0x1a0
> >  /* Indicates good rep/movs microcode on some processors: */
> >  #define MSR_IA32_MISC_ENABLE_DEFAULT    1
> > +#define MSR_IA32_MISC_ENABLE_MWAIT      (1ULL << 18)
> >  
> >  #define MSR_MTRRphysBase(reg)           (0x200 + 2 * (reg))
> >  #define MSR_MTRRphysMask(reg)           (0x200 + 2 * (reg) + 1)
> > 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] i386: Enable IA32_MISC_ENABLE MWAIT bit when exposing mwait/monitor
Posted by Paolo Bonzini 4 years, 11 months ago
On 20/05/19 23:05, Eduardo Habkost wrote:
> What if enable_cpu_pm is false but we're running TCG, or if
> enable_cpu_pm is true but we're not using -cpu host?
> 
> Shouldn't this be conditional on
>   (env->features[FEAT_1_ECX] & CPUID_EXT_MONITOR)
> instead?

Yes, it should.  I fixed it locally.

Paolo