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

[Igor Mammedov <imammedo@redhat.com>]

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>
> > 
> 
>