i386: Disable TOPOEXT by default on "-cpu host"

Message ID 20180809221852.15285-1-ehabkost@redhat.com
State New
Headers show
Series
  • i386: Disable TOPOEXT by default on "-cpu host"
Related show

Commit Message

Eduardo Habkost Aug. 9, 2018, 10:18 p.m.
Enabling TOPOEXT is always allowed, but it can't be enabled
blindly by "-cpu host" because it may make guests crash if the
rest of the cache topology information isn't provided or isn't
consistent.

This addresses the bug reported at:
https://bugzilla.redhat.com/show_bug.cgi?id=1613277

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Richard W.M. Jones Aug. 10, 2018, 7:41 a.m. | #1
On Thu, Aug 09, 2018 at 07:18:52PM -0300, Eduardo Habkost wrote:
> Enabling TOPOEXT is always allowed, but it can't be enabled
> blindly by "-cpu host" because it may make guests crash if the
> rest of the cache topology information isn't provided or isn't
> consistent.
> 
> This addresses the bug reported at:
> https://bugzilla.redhat.com/show_bug.cgi?id=1613277
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/cpu.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 723e02221e..3ac627978f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -849,6 +849,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>          },
>          .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
>          .tcg_features = TCG_EXT3_FEATURES,
> +        /*
> +         * TOPOEXT is always allowed but can't be enabled blindly by
> +         * "-cpu host", as it requires consistent cache topology info
> +         * to be provided so it doesn't confuse guests.
> +         */
> +        .no_autoenable_flags = CPUID_EXT3_TOPOEXT,
>      },
>      [FEAT_C000_0001_EDX] = {
>          .feat_names = {

Can confirm that this fixes the problem observed on the original AMD
Phenom machine, using qemu from git (@6ad908053) + your patch.
Therefore:

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Thanks,

Rich.
Moger, Babu Aug. 13, 2018, 1:50 p.m. | #2
Looks good. Did some basic testing.

Reviewed-by: Babu Moger <babu.moger@amd.com>

> -----Original Message-----
> From: Richard W.M. Jones <rjones@redhat.com>
> Sent: Friday, August 10, 2018 2:41 AM
> To: Eduardo Habkost <ehabkost@redhat.com>
> Cc: qemu-devel@nongnu.org; Paolo Bonzini <pbonzini@redhat.com>;
> Richard Henderson <rth@twiddle.net>; Moger, Babu
> <Babu.Moger@amd.com>
> Subject: Re: [PATCH] i386: Disable TOPOEXT by default on "-cpu host"
> 
> On Thu, Aug 09, 2018 at 07:18:52PM -0300, Eduardo Habkost wrote:
> > Enabling TOPOEXT is always allowed, but it can't be enabled
> > blindly by "-cpu host" because it may make guests crash if the
> > rest of the cache topology information isn't provided or isn't
> > consistent.
> >
> > This addresses the bug reported at:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1613277
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target/i386/cpu.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 723e02221e..3ac627978f 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -849,6 +849,12 @@ static FeatureWordInfo
> feature_word_info[FEATURE_WORDS] = {
> >          },
> >          .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
> >          .tcg_features = TCG_EXT3_FEATURES,
> > +        /*
> > +         * TOPOEXT is always allowed but can't be enabled blindly by
> > +         * "-cpu host", as it requires consistent cache topology info
> > +         * to be provided so it doesn't confuse guests.
> > +         */
> > +        .no_autoenable_flags = CPUID_EXT3_TOPOEXT,
> >      },
> >      [FEAT_C000_0001_EDX] = {
> >          .feat_names = {
> 
> Can confirm that this fixes the problem observed on the original AMD
> Phenom machine, using qemu from git (@6ad908053) + your patch.
> Therefore:
> 
> Tested-by: Richard W.M. Jones <rjones@redhat.com>
> 
> Thanks,
> 
> Rich.
> 
> --
> Richard Jones, Virtualization Group, Red Hat
> http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-p2v converts physical machines to virtual machines.  Boot with a
> live CD or over the network (PXE) and turn machines into KVM guests.
> http://libguestfs.org/virt-v2v

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 723e02221e..3ac627978f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -849,6 +849,12 @@  static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         },
         .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
         .tcg_features = TCG_EXT3_FEATURES,
+        /*
+         * TOPOEXT is always allowed but can't be enabled blindly by
+         * "-cpu host", as it requires consistent cache topology info
+         * to be provided so it doesn't confuse guests.
+         */
+        .no_autoenable_flags = CPUID_EXT3_TOPOEXT,
     },
     [FEAT_C000_0001_EDX] = {
         .feat_names = {