[Qemu-devel] [PATCH] target-i386: Disable cache info passthrough by default

[Qemu-devel] [PATCH] target-i386: Disable cache info passthrough by default

[Eduardo Habkost]

The host cache information may not make sense for the guest if the VM
CPU topology doesn't match the host CPU topology. To make sure we won't
expose broken cache information to the guest, disable cache info
passthrough by default, and add a new "host-cache-info" property that
can be used to enable the old behavior for users that really need it.

Cc: Benoît Canet <benoit@irqsave.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cfb8aa7..3a71f15 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -689,7 +689,6 @@ struct X86CPUDefinition {
     int stepping;
     FeatureWordArray features;
     char model_id[48];
-    bool cache_info_passthrough;
 };
 
 static X86CPUDefinition builtin_x86_defs[] = {
@@ -1416,6 +1415,7 @@ static X86CPUDefinition host_cpudef;
 
 static Property host_x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
+    DEFINE_PROP_BOOL("host-cache-info", X86CPU, cache_info_passthrough, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -1442,7 +1442,6 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
     cpu_x86_fill_model_id(host_cpudef.model_id);
 
     xcc->cpu_def = &host_cpudef;
-    host_cpudef.cache_info_passthrough = true;
 
     /* level, xlevel, xlevel2, and the feature words are initialized on
      * instance_init, because they require KVM to be initialized.
@@ -2076,7 +2075,6 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
     object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
     object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
     object_property_set_int(OBJECT(cpu), def->xlevel2, "xlevel2", errp);
-    cpu->cache_info_passthrough = def->cache_info_passthrough;
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
     for (w = 0; w < FEATURE_WORDS; w++) {
         env->features[w] = def->features[w];
-- 
2.1.0

Re: [Qemu-devel] [PATCH] target-i386: Disable cache info passthrough by default

[Igor Mammedov]

On Wed, 19 Aug 2015 10:08:22 -0700
Eduardo Habkost <ehabkost@redhat.com> wrote:

> The host cache information may not make sense for the guest if the VM
> CPU topology doesn't match the host CPU topology. To make sure we won't
> expose broken cache information to the guest, disable cache info
> passthrough by default, and add a new "host-cache-info" property that
> can be used to enable the old behavior for users that really need it.
> 
> Cc: Benoît Canet <benoit@irqsave.net>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index cfb8aa7..3a71f15 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -689,7 +689,6 @@ struct X86CPUDefinition {
>      int stepping;
>      FeatureWordArray features;
>      char model_id[48];
> -    bool cache_info_passthrough;
>  };
>  
>  static X86CPUDefinition builtin_x86_defs[] = {
> @@ -1416,6 +1415,7 @@ static X86CPUDefinition host_cpudef;
>  
>  static Property host_x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
> +    DEFINE_PROP_BOOL("host-cache-info", X86CPU, cache_info_passthrough, false),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> @@ -1442,7 +1442,6 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
>      cpu_x86_fill_model_id(host_cpudef.model_id);
>  
>      xcc->cpu_def = &host_cpudef;
> -    host_cpudef.cache_info_passthrough = true;
>  
>      /* level, xlevel, xlevel2, and the feature words are initialized on
>       * instance_init, because they require KVM to be initialized.
> @@ -2076,7 +2075,6 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
>      object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
>      object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
>      object_property_set_int(OBJECT(cpu), def->xlevel2, "xlevel2", errp);
> -    cpu->cache_info_passthrough = def->cache_info_passthrough;
Isn't that a guest visible change?

>      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
>      for (w = 0; w < FEATURE_WORDS; w++) {
>          env->features[w] = def->features[w];

Re: [Qemu-devel] [PATCH] target-i386: Disable cache info passthrough by default

[Eduardo Habkost]

On Tue, Sep 01, 2015 at 02:39:00PM +0200, Igor Mammedov wrote:
> On Wed, 19 Aug 2015 10:08:22 -0700
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > The host cache information may not make sense for the guest if the VM
> > CPU topology doesn't match the host CPU topology. To make sure we won't
> > expose broken cache information to the guest, disable cache info
> > passthrough by default, and add a new "host-cache-info" property that
> > can be used to enable the old behavior for users that really need it.
> > 
> > Cc: Benoît Canet <benoit@irqsave.net>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target-i386/cpu.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index cfb8aa7..3a71f15 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -689,7 +689,6 @@ struct X86CPUDefinition {
> >      int stepping;
> >      FeatureWordArray features;
> >      char model_id[48];
> > -    bool cache_info_passthrough;
> >  };
> >  
> >  static X86CPUDefinition builtin_x86_defs[] = {
> > @@ -1416,6 +1415,7 @@ static X86CPUDefinition host_cpudef;
> >  
> >  static Property host_x86_cpu_properties[] = {
> >      DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
> > +    DEFINE_PROP_BOOL("host-cache-info", X86CPU, cache_info_passthrough, false),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >  
> > @@ -1442,7 +1442,6 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
> >      cpu_x86_fill_model_id(host_cpudef.model_id);
> >  
> >      xcc->cpu_def = &host_cpudef;
> > -    host_cpudef.cache_info_passthrough = true;
> >  
> >      /* level, xlevel, xlevel2, and the feature words are initialized on
> >       * instance_init, because they require KVM to be initialized.
> > @@ -2076,7 +2075,6 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
> >      object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
> >      object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
> >      object_property_set_int(OBJECT(cpu), def->xlevel2, "xlevel2", errp);
> > -    cpu->cache_info_passthrough = def->cache_info_passthrough;
> Isn't that a guest visible change?

It is. I am not convinced we should spend time trying to make live
migration work with -cpu host unless we hear from an actual user that
depend on it, but in this case it is really simple to keep compatibility
by adding host-cache-info=on to PC_COMPAT_2_4, so I will send a new
version.

> 
> >      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
> >      for (w = 0; w < FEATURE_WORDS; w++) {
> >          env->features[w] = def->features[w];
> 

-- 
Eduardo