[Qemu-devel] [PATCH 0/2] target-i386: fix W10 bug and bring CPUID levels closer to reality

[Qemu-devel] [PATCH 0/2] target-i386: fix W10 bug and bring CPUID levels closer to reality

[Radim Krčmář]

The first patch uses CPUID levels from the internet, which has a nice
side effect of making W10 insider preview bootable.
The second patch is a traditional buggy bug fix.


Radim Krčmář (2):
  target-i386: emulate CPUID level of real hardware
  target-i386: automatically raise cpuid level to 0xd

 target-i386/cpu.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

-- 
2.4.4

[Qemu-devel] [PATCH 1/2] target-i386: emulate CPUID level of real hardware

[Radim Krčmář]

W10 insider has a bug where it ignores CPUID level and interprets
CPUID.(EAX=07H, ECX=0H) incorrectly, because CPUID in fact returned
CPUID.(EAX=04H, ECX=0H);  this resulted in execution of unsupported
instructions.

While it's a Windows bug, there is no reason to emulate incorrect level;
and amend xlevel while at it.

I have used http://instlatx64.atw.hu/ as a source of CPUID and checked
that it matches Penryn Xeon X5472, Westmere Xeon W3520, SandyBridge
i5-2540M, and Haswell i5-4670T.

kvm64 and qemu64 were bumped to 0xD to avoid similar problems.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 target-i386/cpu.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4e7cdaaaa57e..d392cf46f517 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -677,7 +677,7 @@ struct X86CPUDefinition {
 static X86CPUDefinition builtin_x86_defs[] = {
     {
         .name = "qemu64",
-        .level = 4,
+        .level = 0xd,
         .vendor = CPUID_VENDOR_AMD,
         .family = 6,
         .model = 6,
@@ -753,7 +753,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
     },
     {
         .name = "kvm64",
-        .level = 5,
+        .level = 0xd,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 15,
         .model = 6,
@@ -864,7 +864,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
     },
     {
         .name = "pentium3",
-        .level = 2,
+        .level = 3,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 7,
@@ -889,8 +889,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
     },
     {
         .name = "n270",
-        /* original is on level 10 */
-        .level = 5,
+        .level = 10,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 28,
@@ -910,12 +909,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT2_NX,
         .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel(R) Atom(TM) CPU N270   @ 1.60GHz",
     },
     {
         .name = "Conroe",
-        .level = 4,
+        .level = 10,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 15,
@@ -932,12 +931,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
         .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)",
     },
     {
         .name = "Penryn",
-        .level = 4,
+        .level = 10,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 23,
@@ -955,12 +954,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
         .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)",
     },
     {
         .name = "Nehalem",
-        .level = 4,
+        .level = 11,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 26,
@@ -978,7 +977,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Core i7 9xx (Nehalem Class Core i7)",
     },
     {
@@ -1002,7 +1001,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Westmere E56xx/L56xx/X56xx (Nehalem-C)",
     },
     {
@@ -1031,7 +1030,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT3_LAHF_LM,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Xeon E312xx (Sandy Bridge)",
     },
     {
@@ -1063,7 +1062,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT3_LAHF_LM,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Xeon E3-12xx v2 (Ivy Bridge)",
     },
     {
@@ -1097,7 +1096,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Core Processor (Haswell, no TSX)",
     },    {
         .name = "Haswell",
@@ -1131,7 +1130,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EBX_RTM,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Core Processor (Haswell)",
     },
     {
@@ -1167,7 +1166,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EBX_SMAP,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Core Processor (Broadwell, no TSX)",
     },
     {
@@ -1203,7 +1202,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EBX_SMAP,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
-        .xlevel = 0x8000000A,
+        .xlevel = 0x80000008,
         .model_id = "Intel Core Processor (Broadwell)",
     },
     {
-- 
2.4.4

[Qemu-devel] [PATCH 2/2] target-i386: automatically raise cpuid level to 0xd

[Radim Krčmář]

We already bump to level 7 if features there are requested, so do the
same for 0xD.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 If we want this behavior, we should not do it by writing a case for
 every level.

 target-i386/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d392cf46f517..7a32ead690d2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2796,6 +2796,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         env->cpuid_level = 7;
     }
 
+    if (env->features[FEAT_XSAVE] && env->cpuid_level < 0xd) {
+        env->cpuid_level = 0xd;
+    }
+
     /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
      * CPUID[1].EDX.
      */
-- 
2.4.4

Re: [Qemu-devel] [PATCH 1/2] target-i386: emulate CPUID level of real hardware

[Paolo Bonzini]

On 18/06/2015 17:24, Radim Krčmář wrote:
> W10 insider has a bug where it ignores CPUID level and interprets
> CPUID.(EAX=07H, ECX=0H) incorrectly, because CPUID in fact returned
> CPUID.(EAX=04H, ECX=0H);  this resulted in execution of unsupported
> instructions.
> 
> While it's a Windows bug, there is no reason to emulate incorrect level;
> and amend xlevel while at it.
> 
> I have used http://instlatx64.atw.hu/ as a source of CPUID and checked
> that it matches Penryn Xeon X5472, Westmere Xeon W3520, SandyBridge
> i5-2540M, and Haswell i5-4670T.
> 
> kvm64 and qemu64 were bumped to 0xD to avoid similar problems.

This unfortunately has to be done only for new machine types.  Old types
will remain buggy forever.

Paolo

> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  target-i386/cpu.c | 37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4e7cdaaaa57e..d392cf46f517 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -677,7 +677,7 @@ struct X86CPUDefinition {
>  static X86CPUDefinition builtin_x86_defs[] = {
>      {
>          .name = "qemu64",
> -        .level = 4,
> +        .level = 0xd,
>          .vendor = CPUID_VENDOR_AMD,
>          .family = 6,
>          .model = 6,
> @@ -753,7 +753,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>      },
>      {
>          .name = "kvm64",
> -        .level = 5,
> +        .level = 0xd,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 15,
>          .model = 6,
> @@ -864,7 +864,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>      },
>      {
>          .name = "pentium3",
> -        .level = 2,
> +        .level = 3,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 6,
>          .model = 7,
> @@ -889,8 +889,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>      },
>      {
>          .name = "n270",
> -        /* original is on level 10 */
> -        .level = 5,
> +        .level = 10,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 6,
>          .model = 28,
> @@ -910,12 +909,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT2_NX,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_LAHF_LM,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel(R) Atom(TM) CPU N270   @ 1.60GHz",
>      },
>      {
>          .name = "Conroe",
> -        .level = 4,
> +        .level = 10,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 6,
>          .model = 15,
> @@ -932,12 +931,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_LAHF_LM,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)",
>      },
>      {
>          .name = "Penryn",
> -        .level = 4,
> +        .level = 10,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 6,
>          .model = 23,
> @@ -955,12 +954,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_LAHF_LM,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)",
>      },
>      {
>          .name = "Nehalem",
> -        .level = 4,
> +        .level = 11,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 6,
>          .model = 26,
> @@ -978,7 +977,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_LAHF_LM,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Core i7 9xx (Nehalem Class Core i7)",
>      },
>      {
> @@ -1002,7 +1001,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_LAHF_LM,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Westmere E56xx/L56xx/X56xx (Nehalem-C)",
>      },
>      {
> @@ -1031,7 +1030,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT3_LAHF_LM,
>          .features[FEAT_XSAVE] =
>              CPUID_XSAVE_XSAVEOPT,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Xeon E312xx (Sandy Bridge)",
>      },
>      {
> @@ -1063,7 +1062,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT3_LAHF_LM,
>          .features[FEAT_XSAVE] =
>              CPUID_XSAVE_XSAVEOPT,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Xeon E3-12xx v2 (Ivy Bridge)",
>      },
>      {
> @@ -1097,7 +1096,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID,
>          .features[FEAT_XSAVE] =
>              CPUID_XSAVE_XSAVEOPT,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Core Processor (Haswell, no TSX)",
>      },    {
>          .name = "Haswell",
> @@ -1131,7 +1130,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_RTM,
>          .features[FEAT_XSAVE] =
>              CPUID_XSAVE_XSAVEOPT,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Core Processor (Haswell)",
>      },
>      {
> @@ -1167,7 +1166,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_SMAP,
>          .features[FEAT_XSAVE] =
>              CPUID_XSAVE_XSAVEOPT,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Core Processor (Broadwell, no TSX)",
>      },
>      {
> @@ -1203,7 +1202,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_SMAP,
>          .features[FEAT_XSAVE] =
>              CPUID_XSAVE_XSAVEOPT,
> -        .xlevel = 0x8000000A,
> +        .xlevel = 0x80000008,
>          .model_id = "Intel Core Processor (Broadwell)",
>      },
>      {
> 

Re: [Qemu-devel] [PATCH 1/2] target-i386: emulate CPUID level of real hardware

[Radim Krčmář]

2015-06-18 17:29+0200, Paolo Bonzini:
> On 18/06/2015 17:24, Radim Krčmář wrote:
> > W10 insider has a bug where it ignores CPUID level and interprets
> > CPUID.(EAX=07H, ECX=0H) incorrectly, because CPUID in fact returned
> > CPUID.(EAX=04H, ECX=0H);  this resulted in execution of unsupported
> > instructions.
> > 
> > While it's a Windows bug, there is no reason to emulate incorrect level;
> > and amend xlevel while at it.
> > 
> > I have used http://instlatx64.atw.hu/ as a source of CPUID and checked
> > that it matches Penryn Xeon X5472, Westmere Xeon W3520, SandyBridge
> > i5-2540M, and Haswell i5-4670T.
> > 
> > kvm64 and qemu64 were bumped to 0xD to avoid similar problems.
> 
> This unfortunately has to be done only for new machine types.  Old types
> will remain buggy forever.

Ah, ok, which machine type should I target, 2.4?
And is patch 2 is only supposed to work with new machine types?

Thanks.

Re: [Qemu-devel] [PATCH 1/2] target-i386: emulate CPUID level of real hardware

[Paolo Bonzini]

On 18/06/2015 17:40, Radim Krčmář wrote:
> > This unfortunately has to be done only for new machine types.  Old types
> > will remain buggy forever.
>
> Ah, ok, which machine type should I target, 2.4?

Yes.

> And is patch 2 is only supposed to work with new machine types?

I'm a bit undecided there, since it only triggers with explicitly
specified CPU flags and the old code would basically never work (it's a
known bug, triggered by libvirt's "host-model" code in some cases).

Paolo

Re: [Qemu-devel] [PATCH 2/2] target-i386: automatically raise cpuid level to 0xd

[Eduardo Habkost]

On Thu, Jun 18, 2015 at 05:24:24PM +0200, Radim Krčmář wrote:
> We already bump to level 7 if features there are requested, so do the
> same for 0xD.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>

This breaks guest ABI and live-migration, as CPUID data is not part of
the migration stream (although we have considered including it in the
future).

If we are going to add more special cases like this, we must provide a
way to make QEMU honour an explicit "level" option from the config file
or command-line.

I have considered introducing "min-[x]level" and "max-{x]level"
properties to control automatic increasing of level/xlevel. The existing
X86CPUDefinition.level field could just control min_level, while
explicit "level=" on the command-line or config file would explicitly
force a specific value. Probably setting "max-level" on machine-type
compat code would be enough to restore the previous behavior.


> ---
>  If we want this behavior, we should not do it by writing a case for
>  every level.
> 
>  target-i386/cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index d392cf46f517..7a32ead690d2 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2796,6 +2796,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          env->cpuid_level = 7;
>      }
>  
> +    if (env->features[FEAT_XSAVE] && env->cpuid_level < 0xd) {
> +        env->cpuid_level = 0xd;
> +    }
> +
>      /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
>       * CPUID[1].EDX.
>       */
> -- 
> 2.4.4
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] target-i386: automatically raise cpuid level to 0xd

[Bandan Das]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Jun 18, 2015 at 05:24:24PM +0200, Radim Krčmář wrote:
>> We already bump to level 7 if features there are requested, so do the
>> same for 0xD.

But doesn't bumping to 7 for feat[ebx] have the potential to break
ABI too ?

>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>
> This breaks guest ABI and live-migration, as CPUID data is not part of
> the migration stream (although we have considered including it in the
> future).
>
> If we are going to add more special cases like this, we must provide a
> way to make QEMU honour an explicit "level" option from the config file
> or command-line.
>
> I have considered introducing "min-[x]level" and "max-{x]level"
> properties to control automatic increasing of level/xlevel. The existing
> X86CPUDefinition.level field could just control min_level, while
> explicit "level=" on the command-line or config file would explicitly
> force a specific value. Probably setting "max-level" on machine-type
> compat code would be enough to restore the previous behavior.
>
>
>> ---
>>  If we want this behavior, we should not do it by writing a case for
>>  every level.

Agreed, we should really have a more generic way of doing this.

Bandan

>>  target-i386/cpu.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index d392cf46f517..7a32ead690d2 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -2796,6 +2796,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>>          env->cpuid_level = 7;
>>      }
>>  
>> +    if (env->features[FEAT_XSAVE] && env->cpuid_level < 0xd) {
>> +        env->cpuid_level = 0xd;
>> +    }
>> +
>>      /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
>>       * CPUID[1].EDX.
>>       */
>> -- 
>> 2.4.4
>> 

Re: [Qemu-devel] [PATCH 2/2] target-i386: automatically raise cpuid level to 0xd

[Eduardo Habkost]

On Thu, Jun 18, 2015 at 01:12:32PM -0400, Bandan Das wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Thu, Jun 18, 2015 at 05:24:24PM +0200, Radim Krčmář wrote:
> >> We already bump to level 7 if features there are requested, so do the
> >> same for 0xD.
> 
> But doesn't bumping to 7 for feat[ebx] have the potential to break
> ABI too ?

It's not the auto-bumping that breaks ABI, but having older
machine-types now exposing different CPUID data to guests after applying
the patch.

The auto-bump to 7 was introduced at the same time we introduced the
first CPUID[7] features, so you couldn't have any running machines that
would break.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/2] target-i386: emulate CPUID level of real hardware

[Bandan Das]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/06/2015 17:24, Radim Krčmář wrote:
>> W10 insider has a bug where it ignores CPUID level and interprets
>> CPUID.(EAX=07H, ECX=0H) incorrectly, because CPUID in fact returned
>> CPUID.(EAX=04H, ECX=0H);  this resulted in execution of unsupported
>> instructions.
>> 
>> While it's a Windows bug, there is no reason to emulate incorrect level;
>> and amend xlevel while at it.
>> 
>> I have used http://instlatx64.atw.hu/ as a source of CPUID and checked
>> that it matches Penryn Xeon X5472, Westmere Xeon W3520, SandyBridge
>> i5-2540M, and Haswell i5-4670T.
>> 
>> kvm64 and qemu64 were bumped to 0xD to avoid similar problems.
>
> This unfortunately has to be done only for new machine types.  Old types
> will remain buggy forever.

When is it recommended to add a new machine type ? And conventionally,
new changes are to be made default and compat code takes care of the older
machine types right ?

Bandan


> Paolo
>
>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>> ---
>>  target-i386/cpu.c | 37 ++++++++++++++++++-------------------
>>  1 file changed, 18 insertions(+), 19 deletions(-)
>> 
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 4e7cdaaaa57e..d392cf46f517 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -677,7 +677,7 @@ struct X86CPUDefinition {
>>  static X86CPUDefinition builtin_x86_defs[] = {
>>      {
>>          .name = "qemu64",
>> -        .level = 4,
>> +        .level = 0xd,
>>          .vendor = CPUID_VENDOR_AMD,
>>          .family = 6,
>>          .model = 6,
>> @@ -753,7 +753,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>      },
>>      {
>>          .name = "kvm64",
>> -        .level = 5,
>> +        .level = 0xd,
>>          .vendor = CPUID_VENDOR_INTEL,
>>          .family = 15,
>>          .model = 6,
>> @@ -864,7 +864,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>      },
>>      {
>>          .name = "pentium3",
>> -        .level = 2,
>> +        .level = 3,
>>          .vendor = CPUID_VENDOR_INTEL,
>>          .family = 6,
>>          .model = 7,
>> @@ -889,8 +889,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>      },
>>      {
>>          .name = "n270",
>> -        /* original is on level 10 */
>> -        .level = 5,
>> +        .level = 10,
>>          .vendor = CPUID_VENDOR_INTEL,
>>          .family = 6,
>>          .model = 28,
>> @@ -910,12 +909,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_EXT2_NX,
>>          .features[FEAT_8000_0001_ECX] =
>>              CPUID_EXT3_LAHF_LM,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel(R) Atom(TM) CPU N270   @ 1.60GHz",
>>      },
>>      {
>>          .name = "Conroe",
>> -        .level = 4,
>> +        .level = 10,
>>          .vendor = CPUID_VENDOR_INTEL,
>>          .family = 6,
>>          .model = 15,
>> @@ -932,12 +931,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>>          .features[FEAT_8000_0001_ECX] =
>>              CPUID_EXT3_LAHF_LM,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)",
>>      },
>>      {
>>          .name = "Penryn",
>> -        .level = 4,
>> +        .level = 10,
>>          .vendor = CPUID_VENDOR_INTEL,
>>          .family = 6,
>>          .model = 23,
>> @@ -955,12 +954,12 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>>          .features[FEAT_8000_0001_ECX] =
>>              CPUID_EXT3_LAHF_LM,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)",
>>      },
>>      {
>>          .name = "Nehalem",
>> -        .level = 4,
>> +        .level = 11,
>>          .vendor = CPUID_VENDOR_INTEL,
>>          .family = 6,
>>          .model = 26,
>> @@ -978,7 +977,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
>>          .features[FEAT_8000_0001_ECX] =
>>              CPUID_EXT3_LAHF_LM,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Core i7 9xx (Nehalem Class Core i7)",
>>      },
>>      {
>> @@ -1002,7 +1001,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
>>          .features[FEAT_8000_0001_ECX] =
>>              CPUID_EXT3_LAHF_LM,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Westmere E56xx/L56xx/X56xx (Nehalem-C)",
>>      },
>>      {
>> @@ -1031,7 +1030,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_EXT3_LAHF_LM,
>>          .features[FEAT_XSAVE] =
>>              CPUID_XSAVE_XSAVEOPT,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Xeon E312xx (Sandy Bridge)",
>>      },
>>      {
>> @@ -1063,7 +1062,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_EXT3_LAHF_LM,
>>          .features[FEAT_XSAVE] =
>>              CPUID_XSAVE_XSAVEOPT,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Xeon E3-12xx v2 (Ivy Bridge)",
>>      },
>>      {
>> @@ -1097,7 +1096,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID,
>>          .features[FEAT_XSAVE] =
>>              CPUID_XSAVE_XSAVEOPT,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Core Processor (Haswell, no TSX)",
>>      },    {
>>          .name = "Haswell",
>> @@ -1131,7 +1130,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_7_0_EBX_RTM,
>>          .features[FEAT_XSAVE] =
>>              CPUID_XSAVE_XSAVEOPT,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Core Processor (Haswell)",
>>      },
>>      {
>> @@ -1167,7 +1166,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_7_0_EBX_SMAP,
>>          .features[FEAT_XSAVE] =
>>              CPUID_XSAVE_XSAVEOPT,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Core Processor (Broadwell, no TSX)",
>>      },
>>      {
>> @@ -1203,7 +1202,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>              CPUID_7_0_EBX_SMAP,
>>          .features[FEAT_XSAVE] =
>>              CPUID_XSAVE_XSAVEOPT,
>> -        .xlevel = 0x8000000A,
>> +        .xlevel = 0x80000008,
>>          .model_id = "Intel Core Processor (Broadwell)",
>>      },
>>      {
>> 

Re: [Qemu-devel] [PATCH 2/2] target-i386: automatically raise cpuid level to 0xd

[Radim Krčmář]

2015-06-18 12:50-0300, Eduardo Habkost:
> On Thu, Jun 18, 2015 at 05:24:24PM +0200, Radim Krčmář wrote:
> > We already bump to level 7 if features there are requested, so do the
> > same for 0xD.
> > 
> > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> 
> This breaks guest ABI and live-migration, as CPUID data is not part of
> the migration stream (although we have considered including it in the
> future).
> 
> If we are going to add more special cases like this, we must provide a
> way to make QEMU honour an explicit "level" option from the config file
> or command-line.

Thanks, I'll drop this patch.

> I have considered introducing "min-[x]level" and "max-{x]level"
> properties to control automatic increasing of level/xlevel. The existing
> X86CPUDefinition.level field could just control min_level, while
> explicit "level=" on the command-line or config file would explicitly
> force a specific value. Probably setting "max-level" on machine-type
> compat code would be enough to restore the previous behavior.

We'd need to set min-level at least to 7, to capture the raising we do
now, but a feature in level between default and 7 would result in a
different behavior, so we need to make it much uglier :/
We can add 'compat-level' bit for old machine types and raise to highest
habited function otherwise, optionally with controls you described.

Re: [Qemu-devel] [PATCH 2/2] target-i386: automatically raise cpuid level to 0xd

[Radim Krčmář]

2015-06-19 11:47+0200, Radim Krčmář:
> 2015-06-18 12:50-0300, Eduardo Habkost:
> > I have considered introducing "min-[x]level" and "max-{x]level"
> > properties to control automatic increasing of level/xlevel. The existing
> > X86CPUDefinition.level field could just control min_level, while
> > explicit "level=" on the command-line or config file would explicitly
> > force a specific value. Probably setting "max-level" on machine-type
> > compat code would be enough to restore the previous behavior.
> 
> We'd need to set min-level at least to 7, to capture the raising we do
                   ^^^^^^^^^

Should have been max-level.

(The alcohol level doesn't drop fast enough.)

> now, but a feature in level between default and 7 would result in a
> different behavior, so we need to make it much uglier :/
> We can add 'compat-level' bit for old machine types and raise to highest
> habited function otherwise, optionally with controls you described.

Re: [Qemu-devel] [PATCH 2/2] target-i386: automatically raise cpuid level to 0xd

[Radim Krčmář]

2015-06-19 11:47+0200, Radim Krčmář:
> 2015-06-18 12:50-0300, Eduardo Habkost:
> > I have considered introducing "min-[x]level" and "max-{x]level"
> > properties to control automatic increasing of level/xlevel. The existing
> > X86CPUDefinition.level field could just control min_level, while
> > explicit "level=" on the command-line or config file would explicitly
> > force a specific value. Probably setting "max-level" on machine-type
> > compat code would be enough to restore the previous behavior.
> 
> We'd need to set min-level at least to 7, to capture the raising we do
> now, but a feature in level between default and 7 would result in a
> different behavior, so we need to make it much uglier :/
> We can add 'compat-level' bit for old machine types and raise to highest
> habited function otherwise, optionally with controls you described.

No, features are only in 0x7 and 0xd, so the original solution is good.

(We should also be bumping the CPUID level when adding specific
 features, e.g. to at least 0xB when x2apic is selected.)