[Qemu-devel] [PATCH] target-i386: set model=6 on qemu64 & qemu32 CPU models

[Qemu-devel] [PATCH] target-i386: set model=6 on qemu64 & qemu32 CPU models

[Eduardo Habkost]

There's no Intel CPU with family=6,model=2, and Linux and Windows guests
disable SEP when seeing that combination due to Pentium Pro erratum #82.

In addition to just having SEP ignored by guests, Skype (and maybe other
applications) runs sysenter directly without passing through ntdll on
Windows, and crashes because Windows ignored the SEP CPUID bit.

So, having model > 2 is a better default on qemu64 and qemu32 for two
reasons: making SEP really available for guests, and avoiding crashing
applications that work on bare metal.

model=3 would fix the problem, but it causes CPU enumeration problems
for Windows guests[1]. So this patch sets model=6, that matches "Athlon
(PM core)" on AMD and "P2 with on-die L2 cache" on Intel and it allows
Windows to use all CPUs as well as fixing sysenter.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=508623

Cc: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/i386/pc.h | 8 ++++++++
 target-i386/cpu.c    | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 7fb04d8..195e962 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -258,6 +258,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
             .driver = TYPE_X86_CPU,\
             .property = "pmu",\
             .value = "on",\
+        },{\
+            .driver   = "qemu64-" TYPE_X86_CPU,\
+            .property = "model",\
+            .value    = stringify(2),\
+        },{\
+            .driver   = "qemu32-" TYPE_X86_CPU,\
+            .property = "model",\
+            .value    = stringify(3),\
         }
 
 #define PC_COMPAT_1_4 \
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c36345e..36cfbce 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -544,7 +544,7 @@ static x86_def_t builtin_x86_defs[] = {
         .level = 4,
         .vendor = CPUID_VENDOR_AMD,
         .family = 6,
-        .model = 2,
+        .model = 6,
         .stepping = 3,
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
@@ -647,7 +647,7 @@ static x86_def_t builtin_x86_defs[] = {
         .level = 4,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
-        .model = 3,
+        .model = 6,
         .stepping = 3,
         .features[FEAT_1_EDX] =
             PPRO_FEATURES,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] target-i386: set model=6 on qemu64 & qemu32 CPU models

[Igor Mammedov]

On Tue, 10 Sep 2013 17:48:59 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> There's no Intel CPU with family=6,model=2, and Linux and Windows guests
> disable SEP when seeing that combination due to Pentium Pro erratum #82.
> 
> In addition to just having SEP ignored by guests, Skype (and maybe other
> applications) runs sysenter directly without passing through ntdll on
> Windows, and crashes because Windows ignored the SEP CPUID bit.
> 
> So, having model > 2 is a better default on qemu64 and qemu32 for two
> reasons: making SEP really available for guests, and avoiding crashing
> applications that work on bare metal.
> 
> model=3 would fix the problem, but it causes CPU enumeration problems
> for Windows guests[1]. So this patch sets model=6, that matches "Athlon
> (PM core)" on AMD and "P2 with on-die L2 cache" on Intel and it allows
> Windows to use all CPUs as well as fixing sysenter.
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=508623
> 
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/hw/i386/pc.h | 8 ++++++++
>  target-i386/cpu.c    | 4 ++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 7fb04d8..195e962 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -258,6 +258,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>              .driver = TYPE_X86_CPU,\
>              .property = "pmu",\
>              .value = "on",\
> +        },{\
> +            .driver   = "qemu64-" TYPE_X86_CPU,\
> +            .property = "model",\
> +            .value    = stringify(2),\
> +        },{\
> +            .driver   = "qemu32-" TYPE_X86_CPU,\
> +            .property = "model",\
> +            .value    = stringify(3),\
>          }
>  
>  #define PC_COMPAT_1_4 \
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index c36345e..36cfbce 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -544,7 +544,7 @@ static x86_def_t builtin_x86_defs[] = {
>          .level = 4,
>          .vendor = CPUID_VENDOR_AMD,
>          .family = 6,
> -        .model = 2,
> +        .model = 6,
>          .stepping = 3,
>          .features[FEAT_1_EDX] =
>              PPRO_FEATURES |
> @@ -647,7 +647,7 @@ static x86_def_t builtin_x86_defs[] = {
>          .level = 4,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 6,
> -        .model = 3,
> +        .model = 6,
>          .stepping = 3,
>          .features[FEAT_1_EDX] =
>              PPRO_FEATURES,

Reviewed-By: Igor Mammedov <imammedo@redhat.com>

Re: [Qemu-devel] [PATCH] target-i386: set model=6 on qemu64 & qemu32 CPU models

[Andreas Färber]

Am 11.09.2013 10:22, schrieb Igor Mammedov:
> On Tue, 10 Sep 2013 17:48:59 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
>> There's no Intel CPU with family=6,model=2, and Linux and Windows guests
>> disable SEP when seeing that combination due to Pentium Pro erratum #82.
>>
>> In addition to just having SEP ignored by guests, Skype (and maybe other
>> applications) runs sysenter directly without passing through ntdll on
>> Windows, and crashes because Windows ignored the SEP CPUID bit.
>>
>> So, having model > 2 is a better default on qemu64 and qemu32 for two
>> reasons: making SEP really available for guests, and avoiding crashing
>> applications that work on bare metal.
>>
>> model=3 would fix the problem, but it causes CPU enumeration problems
>> for Windows guests[1]. So this patch sets model=6, that matches "Athlon
>> (PM core)" on AMD and "P2 with on-die L2 cache" on Intel and it allows
>> Windows to use all CPUs as well as fixing sysenter.
>>
>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=508623
>>
>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  include/hw/i386/pc.h | 8 ++++++++
>>  target-i386/cpu.c    | 4 ++--
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 7fb04d8..195e962 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -258,6 +258,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>>              .driver = TYPE_X86_CPU,\
>>              .property = "pmu",\
>>              .value = "on",\
>> +        },{\
>> +            .driver   = "qemu64-" TYPE_X86_CPU,\
>> +            .property = "model",\
>> +            .value    = stringify(2),\
>> +        },{\
>> +            .driver   = "qemu32-" TYPE_X86_CPU,\
>> +            .property = "model",\
>> +            .value    = stringify(3),\
>>          }
>>  
>>  #define PC_COMPAT_1_4 \

Shouldn't this hunk be in PC_COMPAT_1_6 rather than alongside PMU, which
I believe was for 1_5?

Andreas

>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index c36345e..36cfbce 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -544,7 +544,7 @@ static x86_def_t builtin_x86_defs[] = {
>>          .level = 4,
>>          .vendor = CPUID_VENDOR_AMD,
>>          .family = 6,
>> -        .model = 2,
>> +        .model = 6,
>>          .stepping = 3,
>>          .features[FEAT_1_EDX] =
>>              PPRO_FEATURES |
>> @@ -647,7 +647,7 @@ static x86_def_t builtin_x86_defs[] = {
>>          .level = 4,
>>          .vendor = CPUID_VENDOR_INTEL,
>>          .family = 6,
>> -        .model = 3,
>> +        .model = 6,
>>          .stepping = 3,
>>          .features[FEAT_1_EDX] =
>>              PPRO_FEATURES,
> 
> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] target-i386: set model=6 on qemu64 & qemu32 CPU models

[Igor Mammedov]

On Wed, 11 Sep 2013 10:25:58 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 11.09.2013 10:22, schrieb Igor Mammedov:
> > On Tue, 10 Sep 2013 17:48:59 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> >> There's no Intel CPU with family=6,model=2, and Linux and Windows guests
> >> disable SEP when seeing that combination due to Pentium Pro erratum #82.
> >>
> >> In addition to just having SEP ignored by guests, Skype (and maybe other
> >> applications) runs sysenter directly without passing through ntdll on
> >> Windows, and crashes because Windows ignored the SEP CPUID bit.
> >>
> >> So, having model > 2 is a better default on qemu64 and qemu32 for two
> >> reasons: making SEP really available for guests, and avoiding crashing
> >> applications that work on bare metal.
> >>
> >> model=3 would fix the problem, but it causes CPU enumeration problems
> >> for Windows guests[1]. So this patch sets model=6, that matches "Athlon
> >> (PM core)" on AMD and "P2 with on-die L2 cache" on Intel and it allows
> >> Windows to use all CPUs as well as fixing sysenter.
> >>
> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=508623
> >>
> >> Cc: Andrea Arcangeli <aarcange@redhat.com>
> >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >> ---
> >>  include/hw/i386/pc.h | 8 ++++++++
> >>  target-i386/cpu.c    | 4 ++--
> >>  2 files changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >> index 7fb04d8..195e962 100644
> >> --- a/include/hw/i386/pc.h
> >> +++ b/include/hw/i386/pc.h
> >> @@ -258,6 +258,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >>              .driver = TYPE_X86_CPU,\
> >>              .property = "pmu",\
> >>              .value = "on",\
> >> +        },{\
> >> +            .driver   = "qemu64-" TYPE_X86_CPU,\
> >> +            .property = "model",\
> >> +            .value    = stringify(2),\
> >> +        },{\
> >> +            .driver   = "qemu32-" TYPE_X86_CPU,\
> >> +            .property = "model",\
> >> +            .value    = stringify(3),\
> >>          }
> >>  
> >>  #define PC_COMPAT_1_4 \
> 
> Shouldn't this hunk be in PC_COMPAT_1_6 rather than alongside PMU, which
> I believe was for 1_5?
grr, you are right.
my reviewed-by stands, provided compats are moved to PC_COMPAT_1_6.


> Andreas
> 
> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >> index c36345e..36cfbce 100644
> >> --- a/target-i386/cpu.c
> >> +++ b/target-i386/cpu.c
> >> @@ -544,7 +544,7 @@ static x86_def_t builtin_x86_defs[] = {
> >>          .level = 4,
> >>          .vendor = CPUID_VENDOR_AMD,
> >>          .family = 6,
> >> -        .model = 2,
> >> +        .model = 6,
> >>          .stepping = 3,
> >>          .features[FEAT_1_EDX] =
> >>              PPRO_FEATURES |
> >> @@ -647,7 +647,7 @@ static x86_def_t builtin_x86_defs[] = {
> >>          .level = 4,
> >>          .vendor = CPUID_VENDOR_INTEL,
> >>          .family = 6,
> >> -        .model = 3,
> >> +        .model = 6,
> >>          .stepping = 3,
> >>          .features[FEAT_1_EDX] =
> >>              PPRO_FEATURES,
> > 
> > Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> > 
> 
> 

Re: [Qemu-devel] [PATCH] target-i386: set model=6 on qemu64 & qemu32 CPU models

[Andreas Färber]

Am 11.09.2013 10:37, schrieb Igor Mammedov:
> On Wed, 11 Sep 2013 10:25:58 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Am 11.09.2013 10:22, schrieb Igor Mammedov:
>>> On Tue, 10 Sep 2013 17:48:59 -0300
>>> Eduardo Habkost <ehabkost@redhat.com> wrote:
>>>
>>>> There's no Intel CPU with family=6,model=2, and Linux and Windows guests
>>>> disable SEP when seeing that combination due to Pentium Pro erratum #82.
>>>>
>>>> In addition to just having SEP ignored by guests, Skype (and maybe other
>>>> applications) runs sysenter directly without passing through ntdll on
>>>> Windows, and crashes because Windows ignored the SEP CPUID bit.
>>>>
>>>> So, having model > 2 is a better default on qemu64 and qemu32 for two
>>>> reasons: making SEP really available for guests, and avoiding crashing
>>>> applications that work on bare metal.
>>>>
>>>> model=3 would fix the problem, but it causes CPU enumeration problems
>>>> for Windows guests[1]. So this patch sets model=6, that matches "Athlon
>>>> (PM core)" on AMD and "P2 with on-die L2 cache" on Intel and it allows
>>>> Windows to use all CPUs as well as fixing sysenter.
>>>>
>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=508623
>>>>
>>>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>>> ---
>>>>  include/hw/i386/pc.h | 8 ++++++++
>>>>  target-i386/cpu.c    | 4 ++--
>>>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>>> index 7fb04d8..195e962 100644
>>>> --- a/include/hw/i386/pc.h
>>>> +++ b/include/hw/i386/pc.h
>>>> @@ -258,6 +258,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>>>>              .driver = TYPE_X86_CPU,\
>>>>              .property = "pmu",\
>>>>              .value = "on",\
>>>> +        },{\
>>>> +            .driver   = "qemu64-" TYPE_X86_CPU,\
>>>> +            .property = "model",\
>>>> +            .value    = stringify(2),\
>>>> +        },{\
>>>> +            .driver   = "qemu32-" TYPE_X86_CPU,\
>>>> +            .property = "model",\
>>>> +            .value    = stringify(3),\
>>>>          }
>>>>  
>>>>  #define PC_COMPAT_1_4 \
>>
>> Shouldn't this hunk be in PC_COMPAT_1_6 rather than alongside PMU, which
>> I believe was for 1_5?
> grr, you are right.
> my reviewed-by stands, provided compats are moved to PC_COMPAT_1_6.

Fixed now that Stefan's net-next pull has been merged. Thanks, applied
to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

> 
> 
>> Andreas
>>
>>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>>>> index c36345e..36cfbce 100644
>>>> --- a/target-i386/cpu.c
>>>> +++ b/target-i386/cpu.c
>>>> @@ -544,7 +544,7 @@ static x86_def_t builtin_x86_defs[] = {
>>>>          .level = 4,
>>>>          .vendor = CPUID_VENDOR_AMD,
>>>>          .family = 6,
>>>> -        .model = 2,
>>>> +        .model = 6,
>>>>          .stepping = 3,
>>>>          .features[FEAT_1_EDX] =
>>>>              PPRO_FEATURES |
>>>> @@ -647,7 +647,7 @@ static x86_def_t builtin_x86_defs[] = {
>>>>          .level = 4,
>>>>          .vendor = CPUID_VENDOR_INTEL,
>>>>          .family = 6,
>>>> -        .model = 3,
>>>> +        .model = 6,
>>>>          .stepping = 3,
>>>>          .features[FEAT_1_EDX] =
>>>>              PPRO_FEATURES,
>>>
>>> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
>>>
>>
>>
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] target-i386: set model=6 on qemu64 & qemu32 CPU models

[Eduardo Habkost]

On Wed, Sep 11, 2013 at 11:53:46PM +0200, Andreas Färber wrote:
> Am 11.09.2013 10:37, schrieb Igor Mammedov:
> > On Wed, 11 Sep 2013 10:25:58 +0200
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> >> Am 11.09.2013 10:22, schrieb Igor Mammedov:
> >>> On Tue, 10 Sep 2013 17:48:59 -0300
> >>> Eduardo Habkost <ehabkost@redhat.com> wrote:
> >>>
> >>>> There's no Intel CPU with family=6,model=2, and Linux and Windows guests
> >>>> disable SEP when seeing that combination due to Pentium Pro erratum #82.
> >>>>
> >>>> In addition to just having SEP ignored by guests, Skype (and maybe other
> >>>> applications) runs sysenter directly without passing through ntdll on
> >>>> Windows, and crashes because Windows ignored the SEP CPUID bit.
> >>>>
> >>>> So, having model > 2 is a better default on qemu64 and qemu32 for two
> >>>> reasons: making SEP really available for guests, and avoiding crashing
> >>>> applications that work on bare metal.
> >>>>
> >>>> model=3 would fix the problem, but it causes CPU enumeration problems
> >>>> for Windows guests[1]. So this patch sets model=6, that matches "Athlon
> >>>> (PM core)" on AMD and "P2 with on-die L2 cache" on Intel and it allows
> >>>> Windows to use all CPUs as well as fixing sysenter.
> >>>>
> >>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=508623
> >>>>
> >>>> Cc: Andrea Arcangeli <aarcange@redhat.com>
> >>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >>>> ---
> >>>>  include/hw/i386/pc.h | 8 ++++++++
> >>>>  target-i386/cpu.c    | 4 ++--
> >>>>  2 files changed, 10 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>>> index 7fb04d8..195e962 100644
> >>>> --- a/include/hw/i386/pc.h
> >>>> +++ b/include/hw/i386/pc.h
> >>>> @@ -258,6 +258,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >>>>              .driver = TYPE_X86_CPU,\
> >>>>              .property = "pmu",\
> >>>>              .value = "on",\
> >>>> +        },{\
> >>>> +            .driver   = "qemu64-" TYPE_X86_CPU,\
> >>>> +            .property = "model",\
> >>>> +            .value    = stringify(2),\
> >>>> +        },{\
> >>>> +            .driver   = "qemu32-" TYPE_X86_CPU,\
> >>>> +            .property = "model",\
> >>>> +            .value    = stringify(3),\
> >>>>          }
> >>>>  
> >>>>  #define PC_COMPAT_1_4 \
> >>
> >> Shouldn't this hunk be in PC_COMPAT_1_6 rather than alongside PMU, which
> >> I believe was for 1_5?
> > grr, you are right.
> > my reviewed-by stands, provided compats are moved to PC_COMPAT_1_6.
> 
> Fixed now that Stefan's net-next pull has been merged. Thanks, applied
> to qom-cpu:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Thanks for catching and fixing it, and sorry for the confusion.

-- 
Eduardo