diff mbox

[v2] target-i386: Disable cache info passthrough by default

Message ID 1441203551-15403-1-git-send-email-ehabkost@redhat.com
State New
Headers show

Commit Message

Eduardo Habkost Sept. 2, 2015, 2:19 p.m. UTC
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>
---
Changes v1 -> v2:
* Rebased on top of:
  Subject: [PATCH 0/3] pc-*-2.5 machine-types
* Added host-cache-info=on to PC_COMPAT_2_4 to keep compatibility
  on migration
  Reported-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/i386/pc.h | 7 ++++++-
 target-i386/cpu.c    | 4 +---
 2 files changed, 7 insertions(+), 4 deletions(-)

Comments

Igor Mammedov Sept. 2, 2015, 2:41 p.m. UTC | #1
On Wed,  2 Sep 2015 11:19:11 -0300
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>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Changes v1 -> v2:
> * Rebased on top of:
>   Subject: [PATCH 0/3] pc-*-2.5 machine-types
> * Added host-cache-info=on to PC_COMPAT_2_4 to keep compatibility
>   on migration
>   Reported-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/i386/pc.h | 7 ++++++-
>  target-i386/cpu.c    | 4 +---
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 21c1cff..d41b901 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -285,7 +285,12 @@ int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
>  #define PC_COMPAT_2_4 \
> -        HW_COMPAT_2_4
> +        HW_COMPAT_2_4 \
> +        {\
> +            .driver   = "host" "-" TYPE_X86_CPU,\
> +            .property = "host-cache-info",\
> +            .value    = "on",\
> +        },
>  
>  #define PC_COMPAT_2_3 \
>          PC_COMPAT_2_4 \
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 32fee00..152ced2 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];
Eduardo Habkost Oct. 16, 2015, 7:33 p.m. UTC | #2
On Wed, Sep 02, 2015 at 11:19:11AM -0300, Eduardo Habkost 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>

Oops, I was waiting for the pc-2.5 series to get in, then I forgot about
it.

Applied to x86 tree.
diff mbox

Patch

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 21c1cff..d41b901 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -285,7 +285,12 @@  int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
 #define PC_COMPAT_2_4 \
-        HW_COMPAT_2_4
+        HW_COMPAT_2_4 \
+        {\
+            .driver   = "host" "-" TYPE_X86_CPU,\
+            .property = "host-cache-info",\
+            .value    = "on",\
+        },
 
 #define PC_COMPAT_2_3 \
         PC_COMPAT_2_4 \
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 32fee00..152ced2 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];