[Qemu-devel] [RFC 0/7] CPUID fixes for 1.5

[Qemu-devel] [RFC 0/7] CPUID fixes for 1.5

[Eduardo Habkost]

This series contain multiple CPUID fixes for 1.5. As we still don't have static
QOM properties on the CPU class available yet, I implemented machine-typed
compatibility using simple static-variable-based mechanisms.

I am sending this as RFC as it depends on the previous feature-words series I
have sent.

Git tree for testing:
  git://github.com/ehabkost/qemu-hacks.git work/cpu-feat-fixes-various.v1

Borislav Petkov (1):
  target-i386: n270 can MOVBE

Eduardo Habkost (6):
  target-i386: Introduce generic CPUID feature compat function
  target-i386: Introduce compat function to set CPUID 'level'
  target-i386: Introduce compat function to set CPUID 'model'
  pc: Use separate init functions for pc-*-1.4
  target-i386: change CPUID model of 486 to 8
  target-i386: Disable direct passthrough of PMU CPUID leaf by default

 hw/i386/pc_piix.c |  12 +++++--
 hw/i386/pc_q35.c  |  10 +++++-
 target-i386/cpu.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 target-i386/cpu.h |  13 +++++++
 4 files changed, 129 insertions(+), 6 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [RFC 1/7] target-i386: Introduce generic CPUID feature compat function

[Eduardo Habkost]

Introduce x86_cpu_compat_set_features(), that can be used to set/unset
feature bits on specific CPU models for machine-type compatibility.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 26 ++++++++++++++++++++++++++
 target-i386/cpu.h |  4 ++++
 2 files changed, 30 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5bcb79c..a207474 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -970,6 +970,32 @@ static x86_def_t builtin_x86_defs[] = {
     },
 };
 
+/**
+ * x86_cpu_compat_set_features:
+ * @cpu_model: CPU model name to be changed. If NULL, all CPU models are changed
+ * @w: Identifies the feature word to be changed.
+ * @feat_add: Feature bits to be added to feature word
+ * @feat_remove: Feature bits to be removed from feature word
+ *
+ * Change CPU model feature bits for compatibility.
+ *
+ * This function may be used by machine-type compatibility functions
+ * to enable or disable feature bits on specific CPU models.
+ */
+void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
+                                 uint32_t feat_add, uint32_t feat_remove)
+{
+    x86_def_t *def;
+    int i;
+    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+        def = &builtin_x86_defs[i];
+        if (!cpu_model || !strcmp(cpu_model, def->name)) {
+            def->features[w] |= feat_add;
+            def->features[w] &= ~feat_remove;
+        }
+    }
+}
+
 #ifdef CONFIG_KVM
 static int cpu_x86_fill_model_id(char *str)
 {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index aca78fc..60581e1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1254,6 +1254,10 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
 
 void disable_kvm_pv_eoi(void);
 
+void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
+                                 uint32_t feat_add, uint32_t feat_remove);
+
+
 /* Return name of 32-bit register, from a R_* constant */
 const char *get_register_name_32(unsigned int reg);
 
-- 
1.8.1.4

Re: [Qemu-devel] [RFC 1/7] target-i386: Introduce generic CPUID feature compat function

[Andreas Färber]

Am 25.04.2013 20:43, schrieb Eduardo Habkost:
> Introduce x86_cpu_compat_set_features(), that can be used to set/unset
> feature bits on specific CPU models for machine-type compatibility.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 26 ++++++++++++++++++++++++++
>  target-i386/cpu.h |  4 ++++
>  2 files changed, 30 insertions(+)

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

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

[Qemu-devel] [RFC 2/7] target-i386: Introduce compat function to set CPUID 'level'

[Eduardo Habkost]

Introduce x86_cpu_compat_set_level(), which can be used by machine-type
init functions to keep compatibility when CPU models are changed.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 22 ++++++++++++++++++++++
 target-i386/cpu.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a207474..6ef5842 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -996,6 +996,28 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
     }
 }
 
+/**
+ * x86_cpu_compat_set_level:
+ * @cpu_model: CPU model name to be changed. If NULL, all CPU models are changed
+ * @level: New value for 'level' field on CPU model
+ *
+ * Change CPU model 'level' field for compatibility.
+ *
+ * This function may be used by machine-type compatibility functions
+ * to set a backwards-compatible value for the 'level' field on CPU models.
+ */
+void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level)
+{
+    x86_def_t *def;
+    int i;
+    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+        def = &builtin_x86_defs[i];
+        if (!cpu_model || !strcmp(cpu_model, def->name)) {
+            def->level = level;
+        }
+    }
+}
+
 #ifdef CONFIG_KVM
 static int cpu_x86_fill_model_id(char *str)
 {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 60581e1..86a5988 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1256,6 +1256,7 @@ void disable_kvm_pv_eoi(void);
 
 void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
                                  uint32_t feat_add, uint32_t feat_remove);
+void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
1.8.1.4

Re: [Qemu-devel] [RFC 2/7] target-i386: Introduce compat function to set CPUID 'level'

[Igor Mammedov]

On Thu, 25 Apr 2013 15:43:01 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Introduce x86_cpu_compat_set_level(), which can be used by machine-type
> init functions to keep compatibility when CPU models are changed.
Doesn't seems to be used in this series. Why adding dead code, all that should
be replaced by static properties in next release. I plan to respin properties
series shortly after 1.5.

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 22 ++++++++++++++++++++++
>  target-i386/cpu.h |  1 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index a207474..6ef5842 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -996,6 +996,28 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
>      }
>  }
>  
> +/**
> + * x86_cpu_compat_set_level:
> + * @cpu_model: CPU model name to be changed. If NULL, all CPU models are changed
> + * @level: New value for 'level' field on CPU model
> + *
> + * Change CPU model 'level' field for compatibility.
> + *
> + * This function may be used by machine-type compatibility functions
> + * to set a backwards-compatible value for the 'level' field on CPU models.
> + */
> +void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level)
> +{
> +    x86_def_t *def;
> +    int i;
> +    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
> +        def = &builtin_x86_defs[i];
> +        if (!cpu_model || !strcmp(cpu_model, def->name)) {
> +            def->level = level;
> +        }
> +    }
> +}
> +
>  #ifdef CONFIG_KVM
>  static int cpu_x86_fill_model_id(char *str)
>  {
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 60581e1..86a5988 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1256,6 +1256,7 @@ void disable_kvm_pv_eoi(void);
>  
>  void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
>                                   uint32_t feat_add, uint32_t feat_remove);
> +void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level);
>  
>  
>  /* Return name of 32-bit register, from a R_* constant */
> -- 
> 1.8.1.4
> 
> 


-- 
Regards,
  Igor

Re: [Qemu-devel] [RFC 2/7] target-i386: Introduce compat function to set CPUID 'level'

[Eduardo Habkost]

On Fri, Apr 26, 2013 at 05:16:26PM +0200, Igor Mammedov wrote:
> On Thu, 25 Apr 2013 15:43:01 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > Introduce x86_cpu_compat_set_level(), which can be used by machine-type
> > init functions to keep compatibility when CPU models are changed.
> Doesn't seems to be used in this series. Why adding dead code, all that should
> be replaced by static properties in next release. I plan to respin properties
> series shortly after 1.5.

I was planning to include another patch that changed the "level" field
on a CPU model, but in the end I left it out (it didn't look like a
necessary bug fix for 1.5). When this series gets out of RFC status, I
will remove the patch.

-- 
Eduardo

[Qemu-devel] [RFC 3/7] target-i386: Introduce compat function to set CPUID 'model'

[Eduardo Habkost]

Introduce x86_cpu_compat_set_model(), which can be used by machine-type
init functions to change the 'model' value of CPU models for
compatibility.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 22 ++++++++++++++++++++++
 target-i386/cpu.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6ef5842..8ce088e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1018,6 +1018,28 @@ void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level)
     }
 }
 
+/**
+ * x86_cpu_compat_set_model:
+ * @cpu_model: CPU model name to be changed. If NULL, all CPU models are changed
+ * @model: New value for 'model' field on CPU model
+ *
+ * Change CPU model 'model' field for compatibility.
+ *
+ * This function may be used by machine-type compatibility functions
+ * to set a backwards-compatible value for the 'model' field on CPU models.
+ */
+void x86_cpu_compat_set_model(const char *cpu_model, uint32_t model)
+{
+    x86_def_t *def;
+    int i;
+    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+        def = &builtin_x86_defs[i];
+        if (!cpu_model || !strcmp(cpu_model, def->name)) {
+            def->model = model;
+        }
+    }
+}
+
 #ifdef CONFIG_KVM
 static int cpu_x86_fill_model_id(char *str)
 {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 86a5988..1cd5d19 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1257,6 +1257,7 @@ void disable_kvm_pv_eoi(void);
 void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
                                  uint32_t feat_add, uint32_t feat_remove);
 void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level);
+void x86_cpu_compat_set_model(const char *cpu_model, uint32_t model);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
1.8.1.4

[Qemu-devel] [RFC 4/7] pc: Use separate init functions for pc-*-1.4

[Eduardo Habkost]

The pc-*-1.4 machine-types will have some compatibility calls, so make
them use different init functions from pc-*-1.5.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 9 +++++++--
 hw/i386/pc_q35.c  | 7 ++++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 943758a..20708dc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -235,10 +235,15 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
              initrd_filename, cpu_model, 1, 1);
 }
 
+static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
+{
+    pc_init_pci(args);
+}
+
 static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
 {
     enable_compat_apic_id_mode();
-    pc_init_pci(args);
+    pc_init_pci_1_4(args);
 }
 
 /* PC machine init function for pc-0.14 to pc-1.2 */
@@ -309,7 +314,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
 static QEMUMachine pc_i440fx_machine_v1_4 = {
     .name = "pc-i440fx-1.4",
     .desc = "Standard PC (i440FX + PIIX, 1996)",
-    .init = pc_init_pci,
+    .init = pc_init_pci_1_4,
     .max_cpus = 255,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_4,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6ac1a89..7eb4a75 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -209,6 +209,11 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     }
 }
 
+static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
+{
+    pc_q35_init(args);
+}
+
 static QEMUMachine pc_q35_machine_v1_5 = {
     .name = "pc-q35-1.5",
     .alias = "q35",
@@ -221,7 +226,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
 static QEMUMachine pc_q35_machine_v1_4 = {
     .name = "pc-q35-1.4",
     .desc = "Standard PC (Q35 + ICH9, 2009)",
-    .init = pc_q35_init,
+    .init = pc_q35_init_1_4,
     .max_cpus = 255,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_4,
-- 
1.8.1.4

[Qemu-devel] [RFC 5/7] target-i386: n270 can MOVBE

[Eduardo Habkost]

From: Borislav Petkov <bp@suse.de>

The Atom core (cpu name "n270" in QEMU speak) supports MOVBE. This is
needed when booting 3.8 and later linux kernels built with the MATOM
target because we require MOVBE in order to boot properly now.

Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Borislav Petkov <bp@suse.de>
[ehabkost: added compat code to disable MOVBE on pc-*-1.4 and older]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 target-i386/cpu.c | 3 ++-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 20708dc..615d8f4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -237,6 +237,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
 
 static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
 {
+    x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     pc_init_pci(args);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 7eb4a75..3240203 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -211,6 +211,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
 static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
 {
+    x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     pc_q35_init(args);
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8ce088e..592fed8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -656,7 +656,8 @@ static x86_def_t builtin_x86_defs[] = {
             /* Some CPUs got no CPUID_SEP */
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
-            CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR,
+            CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR |
+            CPUID_EXT_MOVBE,
         .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_NX,
-- 
1.8.1.4

Re: [Qemu-devel] [RFC 5/7] target-i386: n270 can MOVBE

[Andreas Färber]

Am 25.04.2013 20:43, schrieb Eduardo Habkost:
> From: Borislav Petkov <bp@suse.de>
> 
> The Atom core (cpu name "n270" in QEMU speak) supports MOVBE. This is
> needed when booting 3.8 and later linux kernels built with the MATOM
> target because we require MOVBE in order to boot properly now.
> 
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> [ehabkost: added compat code to disable MOVBE on pc-*-1.4 and older]
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/i386/pc_piix.c | 1 +
>  hw/i386/pc_q35.c  | 1 +
>  target-i386/cpu.c | 3 ++-
>  3 files changed, 4 insertions(+), 1 deletion(-)

Thanks, rebased and applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 20708dc..615d8f4 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -237,6 +237,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
>  
>  static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
>  {
> +    x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      pc_init_pci(args);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 7eb4a75..3240203 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -211,6 +211,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>  
>  static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
>  {
> +    x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      pc_q35_init(args);
>  }
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 8ce088e..592fed8 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -656,7 +656,8 @@ static x86_def_t builtin_x86_defs[] = {
>              /* Some CPUs got no CPUID_SEP */
>          .features[FEAT_1_ECX] =
>              CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
> -            CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR,
> +            CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR |
> +            CPUID_EXT_MOVBE,
>          .features[FEAT_8000_0001_EDX] =
>              (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
>              CPUID_EXT2_NX,
> 


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

[Qemu-devel] [RFC 6/7] target-i386: change CPUID model of 486 to 8

[Eduardo Habkost]

This changes the model number of 486 to 8 (DX4) which matches the
feature set presented, and actually has the CPUID instruction.

This adds compatibility calls to the pc-1.4 init function, to keep
model=0 on pc-1.4 and older.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This patch contains a chunk from a larger patch, sent by H. Peter Anvin
at:
  Message-Id: <1362017554-1260-1-git-send-email-hpa@zytor.com>
  http://marc.info/?l=qemu-devel&m=136202143002609

I hope I'm doing the right thing by keeping hpa's signed-off-by line.
---
 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 target-i386/cpu.c | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 615d8f4..9372f77 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -238,6 +238,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
 static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
 {
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
+    x86_cpu_compat_set_model("486", 0);
     pc_init_pci(args);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 3240203..fc566fd 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -212,6 +212,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
 {
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
+    x86_cpu_compat_set_model("486", 0);
     pc_q35_init(args);
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 592fed8..4fc7527 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -587,7 +587,7 @@ static x86_def_t builtin_x86_defs[] = {
         .level = 1,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 4,
-        .model = 0,
+        .model = 8,
         .stepping = 0,
         .features[FEAT_1_EDX] =
             I486_FEATURES,
-- 
1.8.1.4

Re: [Qemu-devel] [RFC 6/7] target-i386: change CPUID model of 486 to 8

[Eduardo Habkost]

CCing H. Peter Anvin.

(Sorry, I thought git-send-email was configured to CC everybody
mentioned in the SoB lines).

On Thu, Apr 25, 2013 at 03:43:05PM -0300, Eduardo Habkost wrote:
> This changes the model number of 486 to 8 (DX4) which matches the
> feature set presented, and actually has the CPUID instruction.
> 
> This adds compatibility calls to the pc-1.4 init function, to keep
> model=0 on pc-1.4 and older.
> 
> Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> This patch contains a chunk from a larger patch, sent by H. Peter Anvin
> at:
>   Message-Id: <1362017554-1260-1-git-send-email-hpa@zytor.com>
>   http://marc.info/?l=qemu-devel&m=136202143002609
> 
> I hope I'm doing the right thing by keeping hpa's signed-off-by line.
> ---
>  hw/i386/pc_piix.c | 1 +
>  hw/i386/pc_q35.c  | 1 +
>  target-i386/cpu.c | 2 +-
>  3 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 615d8f4..9372f77 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -238,6 +238,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
>  static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
>  {
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
> +    x86_cpu_compat_set_model("486", 0);
>      pc_init_pci(args);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 3240203..fc566fd 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -212,6 +212,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>  static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
>  {
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
> +    x86_cpu_compat_set_model("486", 0);
>      pc_q35_init(args);
>  }
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 592fed8..4fc7527 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -587,7 +587,7 @@ static x86_def_t builtin_x86_defs[] = {
>          .level = 1,
>          .vendor = CPUID_VENDOR_INTEL,
>          .family = 4,
> -        .model = 0,
> +        .model = 8,
>          .stepping = 0,
>          .features[FEAT_1_EDX] =
>              I486_FEATURES,
> -- 
> 1.8.1.4
> 

-- 
Eduardo

[Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Eduardo Habkost]

The current code handling the CPUID 0xA leaf simply forwards all data
from GET_SUPPORTED_CPUID directly to the guest, breaking migration
between hosts with different number of PMU counters.

This patch disables this behavior, except on older machine-types (for
compatibility) and on the "host" CPU model.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Gleb Natapov <gleb@redhat.com>
---
 hw/i386/pc_piix.c |  1 +
 hw/i386/pc_q35.c  |  1 +
 target-i386/cpu.c | 25 ++++++++++++++++++++++++-
 target-i386/cpu.h |  7 +++++++
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9372f77..bbc7064 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -239,6 +239,7 @@ static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
 {
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_model("486", 0);
+    x86_cpu_enable_pmu_passthrough();
     pc_init_pci(args);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index fc566fd..9718e94 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -213,6 +213,7 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
 {
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_model("486", 0);
+    x86_cpu_enable_pmu_passthrough();
     pc_q35_init(args);
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4fc7527..602d00f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -245,6 +245,16 @@ static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
         (1 << KVM_FEATURE_PV_EOI) |
         (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
 
+/* Enables passthrough of CPUID leaf 0xA by default, for compatibility with old
+ * machine-types.
+ */
+static bool default_pmu_passthrough;
+
+void x86_cpu_enable_pmu_passthrough(void)
+{
+    default_pmu_passthrough = true;
+}
+
 void disable_kvm_pv_eoi(void)
 {
     kvm_default_features &= ~(1UL << KVM_FEATURE_PV_EOI);
@@ -375,6 +385,12 @@ typedef struct x86_def_t {
     int stepping;
     FeatureWordArray features;
     char model_id[48];
+
+    /* Enable direct passthrough of PMU leaf from the GET_SUPPORTED_CPUID
+     * data returned by the kernel. This is not migration-safe and should
+     * never be enabled by default.
+     */
+    bool cpuid_pmu_passthrough;
 } x86_def_t;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -1120,6 +1136,8 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
     x86_cpu_def->features[FEAT_KVM] =
         kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
 
+    x86_cpu_def->cpuid_pmu_passthrough = true;
+
 #endif /* CONFIG_KVM */
 }
 
@@ -1735,9 +1753,13 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
         return;
     }
 
+    /* Defaults & compat bits that are not in the builtin_x86_defs table: */
     if (kvm_enabled()) {
         def->features[FEAT_KVM] |= kvm_default_features;
     }
+    if (default_pmu_passthrough) {
+        def->cpuid_pmu_passthrough = true;
+    }
     def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
 
     object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
@@ -1755,6 +1777,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
     env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
     env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
     env->cpuid_xlevel2 = def->xlevel2;
+    env->cpuid_pmu_passthrough = def->cpuid_pmu_passthrough;
 
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 }
@@ -1986,7 +2009,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 0xA:
         /* Architectural Performance Monitoring Leaf */
-        if (kvm_enabled()) {
+        if (kvm_enabled() && env->cpuid_pmu_passthrough) {
             KVMState *s = cs->kvm_state;
 
             *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1cd5d19..12c8bf1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -843,6 +843,12 @@ typedef struct CPUX86State {
     uint32_t cpuid_vendor3;
     uint32_t cpuid_version;
     FeatureWordArray features;
+
+    /* Enable direct passthrough of PMU leaf from the GET_SUPPORTED_CPUID
+     * data returned by the kernel. This is not migration-safe and should
+     * never be enabled by default.
+     */
+    bool cpuid_pmu_passthrough;
     uint32_t cpuid_model[12];
     uint32_t cpuid_apic_id;
 
@@ -1258,6 +1264,7 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
                                  uint32_t feat_add, uint32_t feat_remove);
 void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level);
 void x86_cpu_compat_set_model(const char *cpu_model, uint32_t model);
+void x86_cpu_enable_pmu_passthrough(void);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
1.8.1.4

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Igor Mammedov]

On Thu, 25 Apr 2013 15:43:06 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> The current code handling the CPUID 0xA leaf simply forwards all data
> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> between hosts with different number of PMU counters.
> 
> This patch disables this behavior, except on older machine-types (for
> compatibility) and on the "host" CPU model.
Please, make it static property and use compat properties.
Result will be simpler and  much less will have to be redone/discarded after
converting to the rest to properties and sub-classes.

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> ---
>  hw/i386/pc_piix.c |  1 +
>  hw/i386/pc_q35.c  |  1 +
>  target-i386/cpu.c | 25 ++++++++++++++++++++++++-
>  target-i386/cpu.h |  7 +++++++
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 9372f77..bbc7064 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -239,6 +239,7 @@ static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
>  {
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      x86_cpu_compat_set_model("486", 0);
> +    x86_cpu_enable_pmu_passthrough();
>      pc_init_pci(args);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index fc566fd..9718e94 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -213,6 +213,7 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
>  {
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      x86_cpu_compat_set_model("486", 0);
> +    x86_cpu_enable_pmu_passthrough();
>      pc_q35_init(args);
>  }
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4fc7527..602d00f 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -245,6 +245,16 @@ static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
>          (1 << KVM_FEATURE_PV_EOI) |
>          (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
>  
> +/* Enables passthrough of CPUID leaf 0xA by default, for compatibility with old
> + * machine-types.
> + */
> +static bool default_pmu_passthrough;
> +
> +void x86_cpu_enable_pmu_passthrough(void)
> +{
> +    default_pmu_passthrough = true;
> +}
> +
>  void disable_kvm_pv_eoi(void)
>  {
>      kvm_default_features &= ~(1UL << KVM_FEATURE_PV_EOI);
> @@ -375,6 +385,12 @@ typedef struct x86_def_t {
>      int stepping;
>      FeatureWordArray features;
>      char model_id[48];
> +
> +    /* Enable direct passthrough of PMU leaf from the GET_SUPPORTED_CPUID
> +     * data returned by the kernel. This is not migration-safe and should
> +     * never be enabled by default.
> +     */
> +    bool cpuid_pmu_passthrough;
>  } x86_def_t;
>  
>  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> @@ -1120,6 +1136,8 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
>      x86_cpu_def->features[FEAT_KVM] =
>          kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
>  
> +    x86_cpu_def->cpuid_pmu_passthrough = true;
> +
>  #endif /* CONFIG_KVM */
>  }
>  
> @@ -1735,9 +1753,13 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
>          return;
>      }
>  
> +    /* Defaults & compat bits that are not in the builtin_x86_defs table: */
>      if (kvm_enabled()) {
>          def->features[FEAT_KVM] |= kvm_default_features;
>      }
> +    if (default_pmu_passthrough) {
> +        def->cpuid_pmu_passthrough = true;
> +    }
>      def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
>  
>      object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
> @@ -1755,6 +1777,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
>      env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
>      env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
>      env->cpuid_xlevel2 = def->xlevel2;
> +    env->cpuid_pmu_passthrough = def->cpuid_pmu_passthrough;
>  
>      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
>  }
> @@ -1986,7 +2009,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          break;
>      case 0xA:
>          /* Architectural Performance Monitoring Leaf */
> -        if (kvm_enabled()) {
> +        if (kvm_enabled() && env->cpuid_pmu_passthrough) {
>              KVMState *s = cs->kvm_state;
>  
>              *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX);
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 1cd5d19..12c8bf1 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -843,6 +843,12 @@ typedef struct CPUX86State {
>      uint32_t cpuid_vendor3;
>      uint32_t cpuid_version;
>      FeatureWordArray features;
> +
> +    /* Enable direct passthrough of PMU leaf from the GET_SUPPORTED_CPUID
> +     * data returned by the kernel. This is not migration-safe and should
> +     * never be enabled by default.
> +     */
> +    bool cpuid_pmu_passthrough;
>      uint32_t cpuid_model[12];
>      uint32_t cpuid_apic_id;
>  
> @@ -1258,6 +1264,7 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
>                                   uint32_t feat_add, uint32_t feat_remove);
>  void x86_cpu_compat_set_level(const char *cpu_model, uint32_t level);
>  void x86_cpu_compat_set_model(const char *cpu_model, uint32_t model);
> +void x86_cpu_enable_pmu_passthrough(void);
>  
>  
>  /* Return name of 32-bit register, from a R_* constant */
> -- 
> 1.8.1.4
> 


-- 
Regards,
  Igor

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Eduardo Habkost]

On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
> On Thu, 25 Apr 2013 15:43:06 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > The current code handling the CPUID 0xA leaf simply forwards all data
> > from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> > between hosts with different number of PMU counters.
> > 
> > This patch disables this behavior, except on older machine-types (for
> > compatibility) and on the "host" CPU model.
> Please, make it static property and use compat properties.
> Result will be simpler and  much less will have to be redone/discarded after
> converting to the rest to properties and sub-classes.

I was going to say that static properties were too much work to be done
in time for 1.5, but you are right: in this specific case adding a
static property for the cpuid_pmu_passthrough field looks very easy. I
will give it a try.

I will probably try to make the "model" field a static property as well.
Then only x86_cpu_compat_set_features() would be kept, as converting
feature flags to static properties will probably require more work.

Thanks,

-- 
Eduardo

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Andreas Färber]

Am 26.04.2013 17:31, schrieb Eduardo Habkost:
> On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
>> On Thu, 25 Apr 2013 15:43:06 -0300
>> Eduardo Habkost <ehabkost@redhat.com> wrote:
>>
>>> The current code handling the CPUID 0xA leaf simply forwards all data
>>> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
>>> between hosts with different number of PMU counters.
>>>
>>> This patch disables this behavior, except on older machine-types (for
>>> compatibility) and on the "host" CPU model.
>> Please, make it static property and use compat properties.
>> Result will be simpler and  much less will have to be redone/discarded after
>> converting to the rest to properties and sub-classes.
> 
> I was going to say that static properties were too much work to be done
> in time for 1.5, but you are right: in this specific case adding a
> static property for the cpuid_pmu_passthrough field looks very easy. I
> will give it a try.

I am hoping to get as initial set (though not all) of the static
properties still into 1.5. Using them to fix CPUID bugs can then be done
during Hard Freeze. :)

Andreas

-- 
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] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Igor Mammedov]

On Fri, 26 Apr 2013 17:33:18 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 26.04.2013 17:31, schrieb Eduardo Habkost:
> > On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
> >> On Thu, 25 Apr 2013 15:43:06 -0300
> >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> >>
> >>> The current code handling the CPUID 0xA leaf simply forwards all data
> >>> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> >>> between hosts with different number of PMU counters.
> >>>
> >>> This patch disables this behavior, except on older machine-types (for
> >>> compatibility) and on the "host" CPU model.
> >> Please, make it static property and use compat properties.
> >> Result will be simpler and  much less will have to be redone/discarded after
> >> converting to the rest to properties and sub-classes.
> > 
> > I was going to say that static properties were too much work to be done
> > in time for 1.5, but you are right: in this specific case adding a
> > static property for the cpuid_pmu_passthrough field looks very easy. I
> > will give it a try.
> 
> I am hoping to get as initial set (though not all) of the static
> properties still into 1.5. Using them to fix CPUID bugs can then be done
> during Hard Freeze. :)
patch "[PATCH 02/10] target-i386: cpu: convert existing dynamic properties
into static properties" should be enough for using model,level compat
properties.

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


-- 
Regards,
  Igor

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Eduardo Habkost]

On Fri, Apr 26, 2013 at 05:39:15PM +0200, Igor Mammedov wrote:
> On Fri, 26 Apr 2013 17:33:18 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
> > Am 26.04.2013 17:31, schrieb Eduardo Habkost:
> > > On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
> > >> On Thu, 25 Apr 2013 15:43:06 -0300
> > >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >>
> > >>> The current code handling the CPUID 0xA leaf simply forwards all data
> > >>> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> > >>> between hosts with different number of PMU counters.
> > >>>
> > >>> This patch disables this behavior, except on older machine-types (for
> > >>> compatibility) and on the "host" CPU model.
> > >> Please, make it static property and use compat properties.
> > >> Result will be simpler and  much less will have to be redone/discarded after
> > >> converting to the rest to properties and sub-classes.
> > > 
> > > I was going to say that static properties were too much work to be done
> > > in time for 1.5, but you are right: in this specific case adding a
> > > static property for the cpuid_pmu_passthrough field looks very easy. I
> > > will give it a try.
> > 
> > I am hoping to get as initial set (though not all) of the static
> > properties still into 1.5. Using them to fix CPUID bugs can then be done
> > during Hard Freeze. :)
> patch "[PATCH 02/10] target-i386: cpu: convert existing dynamic properties
> into static properties" should be enough for using model,level compat
> properties.

...except that we don't have X86CPU model subclasses yet, so we can't
set model-specific compat properties using compat_props.

Maybe we can make compat_props work for pmu-passthrough, but it may be
not completely straightforward because I would like to keep it enabled
by default on -cpu host.

-- 
Eduardo

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Igor Mammedov]

On Fri, 26 Apr 2013 14:30:40 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Fri, Apr 26, 2013 at 05:39:15PM +0200, Igor Mammedov wrote:
> > On Fri, 26 Apr 2013 17:33:18 +0200
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> > > Am 26.04.2013 17:31, schrieb Eduardo Habkost:
> > > > On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
> > > >> On Thu, 25 Apr 2013 15:43:06 -0300
> > > >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > >>
> > > >>> The current code handling the CPUID 0xA leaf simply forwards all data
> > > >>> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> > > >>> between hosts with different number of PMU counters.
> > > >>>
> > > >>> This patch disables this behavior, except on older machine-types (for
> > > >>> compatibility) and on the "host" CPU model.
> > > >> Please, make it static property and use compat properties.
> > > >> Result will be simpler and  much less will have to be redone/discarded after
> > > >> converting to the rest to properties and sub-classes.
> > > > 
> > > > I was going to say that static properties were too much work to be done
> > > > in time for 1.5, but you are right: in this specific case adding a
> > > > static property for the cpuid_pmu_passthrough field looks very easy. I
> > > > will give it a try.
> > > 
> > > I am hoping to get as initial set (though not all) of the static
> > > properties still into 1.5. Using them to fix CPUID bugs can then be done
> > > during Hard Freeze. :)
> > patch "[PATCH 02/10] target-i386: cpu: convert existing dynamic properties
> > into static properties" should be enough for using model,level compat
> > properties.
> 
> ...except that we don't have X86CPU model subclasses yet, so we can't
> set model-specific compat properties using compat_props.
X86CPU could serve here

> 
> Maybe we can make compat_props work for pmu-passthrough, but it may be
> not completely straightforward because I would like to keep it enabled
> by default on -cpu host.
Maybe for 'host' hack realize() to override default, will do and later we
can move it to host_subclass.

it's easier to re-factor than compact functions
> 
> -- 
> Eduardo
> 


-- 
Regards,
  Igor

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Eduardo Habkost]

On Fri, Apr 26, 2013 at 07:41:10PM +0200, Igor Mammedov wrote:
> On Fri, 26 Apr 2013 14:30:40 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Fri, Apr 26, 2013 at 05:39:15PM +0200, Igor Mammedov wrote:
> > > On Fri, 26 Apr 2013 17:33:18 +0200
> > > Andreas Färber <afaerber@suse.de> wrote:
> > > 
> > > > Am 26.04.2013 17:31, schrieb Eduardo Habkost:
> > > > > On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
> > > > >> On Thu, 25 Apr 2013 15:43:06 -0300
> > > > >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > > >>
> > > > >>> The current code handling the CPUID 0xA leaf simply forwards all data
> > > > >>> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> > > > >>> between hosts with different number of PMU counters.
> > > > >>>
> > > > >>> This patch disables this behavior, except on older machine-types (for
> > > > >>> compatibility) and on the "host" CPU model.
> > > > >> Please, make it static property and use compat properties.
> > > > >> Result will be simpler and  much less will have to be redone/discarded after
> > > > >> converting to the rest to properties and sub-classes.
> > > > > 
> > > > > I was going to say that static properties were too much work to be done
> > > > > in time for 1.5, but you are right: in this specific case adding a
> > > > > static property for the cpuid_pmu_passthrough field looks very easy. I
> > > > > will give it a try.
> > > > 
> > > > I am hoping to get as initial set (though not all) of the static
> > > > properties still into 1.5. Using them to fix CPUID bugs can then be done
> > > > during Hard Freeze. :)
> > > patch "[PATCH 02/10] target-i386: cpu: convert existing dynamic properties
> > > into static properties" should be enough for using model,level compat
> > > properties.
> > 
> > ...except that we don't have X86CPU model subclasses yet, so we can't
> > set model-specific compat properties using compat_props.
> X86CPU could serve here

How?

If we need to set model=8 only on the "486" CPU model, how would we do
that in the compat_props table without subclasses?

> 
> > 
> > Maybe we can make compat_props work for pmu-passthrough, but it may be
> > not completely straightforward because I would like to keep it enabled
> > by default on -cpu host.
> Maybe for 'host' hack realize() to override default, will do and later we
> can move it to host_subclass.

Yes, it's doable. And I don't think we even need to hack methods, we
just need a x86_def_t field that indicates that this is a model that
overrides the default (and that field would be true only for "host").

But I still don't see how it would be possible to use compat_props for
the 486 model=8 fix or for the n270 movbe fix.

-- 
Eduardo

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Igor Mammedov]

On Fri, 26 Apr 2013 16:01:15 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Fri, Apr 26, 2013 at 07:41:10PM +0200, Igor Mammedov wrote:
> > On Fri, 26 Apr 2013 14:30:40 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> > > On Fri, Apr 26, 2013 at 05:39:15PM +0200, Igor Mammedov wrote:
> > > > On Fri, 26 Apr 2013 17:33:18 +0200
> > > > Andreas Färber <afaerber@suse.de> wrote:
> > > > 
> > > > > Am 26.04.2013 17:31, schrieb Eduardo Habkost:
> > > > > > On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
> > > > > >> On Thu, 25 Apr 2013 15:43:06 -0300
> > > > > >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > > > >>
> > > > > >>> The current code handling the CPUID 0xA leaf simply forwards all data
> > > > > >>> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> > > > > >>> between hosts with different number of PMU counters.
> > > > > >>>
> > > > > >>> This patch disables this behavior, except on older machine-types (for
> > > > > >>> compatibility) and on the "host" CPU model.
> > > > > >> Please, make it static property and use compat properties.
> > > > > >> Result will be simpler and  much less will have to be redone/discarded after
> > > > > >> converting to the rest to properties and sub-classes.
> > > > > > 
> > > > > > I was going to say that static properties were too much work to be done
> > > > > > in time for 1.5, but you are right: in this specific case adding a
> > > > > > static property for the cpuid_pmu_passthrough field looks very easy. I
> > > > > > will give it a try.
> > > > > 
> > > > > I am hoping to get as initial set (though not all) of the static
> > > > > properties still into 1.5. Using them to fix CPUID bugs can then be done
> > > > > during Hard Freeze. :)
> > > > patch "[PATCH 02/10] target-i386: cpu: convert existing dynamic properties
> > > > into static properties" should be enough for using model,level compat
> > > > properties.
> > > 
> > > ...except that we don't have X86CPU model subclasses yet, so we can't
> > > set model-specific compat properties using compat_props.
> > X86CPU could serve here
> 
> How?
> 
> If we need to set model=8 only on the "486" CPU model, how would we do
> that in the compat_props table without subclasses?
for model, it would be its possible with custom setter, might be preferred way
since it's local to cpu.c, and could be easily replaced with with one-liner once
sub-classes are in tree.

> 
> > 
> > > 
> > > Maybe we can make compat_props work for pmu-passthrough, but it may be
> > > not completely straightforward because I would like to keep it enabled
> > > by default on -cpu host.
> > Maybe for 'host' hack realize() to override default, will do and later we
> > can move it to host_subclass.
> 
> Yes, it's doable. And I don't think we even need to hack methods, we
> just need a x86_def_t field that indicates that this is a model that
> overrides the default (and that field would be true only for "host").
Extended x86_def_t might be harder to handle when switching to sub-classes,
although it's hard to say without comparing patches for both ways.

> 
> But I still don't see how it would be possible to use compat_props for
> the 486 model=8 fix or for the n270 movbe fix.
> 
It's impossible to implement "n270  movbe" fixup with compat props currently,
so 5/7 + 1/7 looks ok. We can drop them once features converted to static properties
+ sub-classes.

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Eduardo Habkost]

On Tue, Apr 30, 2013 at 07:04:23PM +0200, Igor Mammedov wrote:
> On Fri, 26 Apr 2013 16:01:15 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Fri, Apr 26, 2013 at 07:41:10PM +0200, Igor Mammedov wrote:
> > > On Fri, 26 Apr 2013 14:30:40 -0300
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > 
> > > > On Fri, Apr 26, 2013 at 05:39:15PM +0200, Igor Mammedov wrote:
> > > > > On Fri, 26 Apr 2013 17:33:18 +0200
> > > > > Andreas Färber <afaerber@suse.de> wrote:
> > > > > 
> > > > > > Am 26.04.2013 17:31, schrieb Eduardo Habkost:
> > > > > > > On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote:
> > > > > > >> On Thu, 25 Apr 2013 15:43:06 -0300
> > > > > > >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > > > > >>
> > > > > > >>> The current code handling the CPUID 0xA leaf simply forwards all data
> > > > > > >>> from GET_SUPPORTED_CPUID directly to the guest, breaking migration
> > > > > > >>> between hosts with different number of PMU counters.
> > > > > > >>>
> > > > > > >>> This patch disables this behavior, except on older machine-types (for
> > > > > > >>> compatibility) and on the "host" CPU model.
> > > > > > >> Please, make it static property and use compat properties.
> > > > > > >> Result will be simpler and  much less will have to be redone/discarded after
> > > > > > >> converting to the rest to properties and sub-classes.
> > > > > > > 
> > > > > > > I was going to say that static properties were too much work to be done
> > > > > > > in time for 1.5, but you are right: in this specific case adding a
> > > > > > > static property for the cpuid_pmu_passthrough field looks very easy. I
> > > > > > > will give it a try.
> > > > > > 
> > > > > > I am hoping to get as initial set (though not all) of the static
> > > > > > properties still into 1.5. Using them to fix CPUID bugs can then be done
> > > > > > during Hard Freeze. :)
> > > > > patch "[PATCH 02/10] target-i386: cpu: convert existing dynamic properties
> > > > > into static properties" should be enough for using model,level compat
> > > > > properties.
> > > > 
> > > > ...except that we don't have X86CPU model subclasses yet, so we can't
> > > > set model-specific compat properties using compat_props.
> > > X86CPU could serve here
> > 
> > How?
> > 
> > If we need to set model=8 only on the "486" CPU model, how would we do
> > that in the compat_props table without subclasses?
> for model, it would be its possible with custom setter, might be preferred way
> since it's local to cpu.c, and could be easily replaced with with one-liner once
> sub-classes are in tree.

A custom setter has to be set inside class_init, because instance_init
is run _after_ global properties are set, but we still don't have
subclasses so we can't have a different setter for "486" or a
compat_props item that would affect only "486". How exactly would this
solution look like? I can't see it.

(My main question isn't about the implementation inside cpu.c, but how
the machine-type code inside pc_piix.c would look like).


> 
> > 
> > > 
> > > > 
> > > > Maybe we can make compat_props work for pmu-passthrough, but it may be
> > > > not completely straightforward because I would like to keep it enabled
> > > > by default on -cpu host.
> > > Maybe for 'host' hack realize() to override default, will do and later we
> > > can move it to host_subclass.
> > 
> > Yes, it's doable. And I don't think we even need to hack methods, we
> > just need a x86_def_t field that indicates that this is a model that
> > overrides the default (and that field would be true only for "host").
> Extended x86_def_t might be harder to handle when switching to sub-classes,
> although it's hard to say without comparing patches for both ways.

I believe it's impossible to implement any behavior that will change
depending on the CPU model (enabling PMU passthrough on "host",
disabling on all other models) without extending x86_def_t. x86_def_t
data is the only thing we get back from cpu_x86_find_by_name().

Would you prefer to change cpu_x86_find_by_name() to get a X86CPU* as
argument, too? I was trying to avoid that, but it is doable.


> 
> > 
> > But I still don't see how it would be possible to use compat_props for
> > the 486 model=8 fix or for the n270 movbe fix.
> > 
> It's impossible to implement "n270  movbe" fixup with compat props currently,
> so 5/7 + 1/7 looks ok. We can drop them once features converted to static properties
> + sub-classes.

True. But why this doesn't apply to 6/7 (486 model=8) as well?

-- 
Eduardo

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Andreas Färber]

Am 30.04.2013 21:57, schrieb Eduardo Habkost:
> On Tue, Apr 30, 2013 at 07:04:23PM +0200, Igor Mammedov wrote:
>> It's impossible to implement "n270  movbe" fixup with compat props currently,
>> so 5/7 + 1/7 looks ok. We can drop them once features converted to static properties
>> + sub-classes.
> 
> True. But why this doesn't apply to 6/7 (486 model=8) as well?

Eduardo, this series is "RFC". Are you going to grant permission to pick
individual agreed-on patches from it or are you planning to resend?

Andreas

-- 
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] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Eduardo Habkost]

On Wed, May 01, 2013 at 01:14:48PM +0200, Andreas Färber wrote:
> Am 30.04.2013 21:57, schrieb Eduardo Habkost:
> > On Tue, Apr 30, 2013 at 07:04:23PM +0200, Igor Mammedov wrote:
> >> It's impossible to implement "n270  movbe" fixup with compat props currently,
> >> so 5/7 + 1/7 looks ok. We can drop them once features converted to static properties
> >> + sub-classes.
> > 
> > True. But why this doesn't apply to 6/7 (486 model=8) as well?
> 
> Eduardo, this series is "RFC". Are you going to grant permission to pick
> individual agreed-on patches from it or are you planning to resend?

I was planning to rebase and resend because of the other changes
entering qom-cpu and to address the concerns about the other patches.
But if you want to pick patches directly from this RFC series, I don't
mind at all. It's up to you.

-- 
Eduardo

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Andreas Färber]

Am 02.05.2013 16:43, schrieb Eduardo Habkost:
> On Wed, May 01, 2013 at 01:14:48PM +0200, Andreas Färber wrote:
>> Am 30.04.2013 21:57, schrieb Eduardo Habkost:
>>> On Tue, Apr 30, 2013 at 07:04:23PM +0200, Igor Mammedov wrote:
>>>> It's impossible to implement "n270  movbe" fixup with compat props currently,
>>>> so 5/7 + 1/7 looks ok. We can drop them once features converted to static properties
>>>> + sub-classes.
>>>
>>> True. But why this doesn't apply to 6/7 (486 model=8) as well?
>>
>> Eduardo, this series is "RFC". Are you going to grant permission to pick
>> individual agreed-on patches from it or are you planning to resend?
> 
> I was planning to rebase and resend because of the other changes
> entering qom-cpu and to address the concerns about the other patches.
> But if you want to pick patches directly from this RFC series, I don't
> mind at all. It's up to you.

OK. Could you look at the series I posted yesterday? If you're okay with
it and Anthony doesn't get a heart attack due to my qdev creativity ;)
I'd like to rebase the remainder of this series on it.

Andreas

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