[Qemu-devel] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models

[Qemu-devel] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models

[Eduardo Habkost]

With the Intel microcode update that removed HLE and RTM, there will be
different kinds of Haswell and Broadwell CPUs out there: some that still
have the HLE and RTM features, and some that don't have the HLE and RTM
features. On both cases people may be willing to use the pc-*-2.3
machine-types.

So instead of making the CPU model results confusing by making it depend on the
machine-type, keep HLE and RTM on the existing Haswell and Broadwell CPU
models, and introduce "Haswell-noTSX" and "Broadwell-noTSX" CPU models later,
for people who have CPUs that don't have TSX feature available.

Eduardo Habkost (2):
  Revert "target-i386: Disable HLE and RTM on Haswell & Broadwell"
  target-i386: Haswell-noTSX and Broadwell-noTSX

 hw/i386/pc_piix.c |  4 ---
 hw/i386/pc_q35.c  |  4 ---
 target-i386/cpu.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 72 insertions(+), 10 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 1/2] Revert "target-i386: Disable HLE and RTM on Haswell & Broadwell"

[Eduardo Habkost]

This reverts commit 13704e4c455770d500d6b87b117e32f0d01252c9.

With the Intel microcode update that removed HLE and RTM, there will be
different kinds of Haswell and Broadwell CPUs out there: some that still
have the HLE and RTM features, and some that don't have the HLE and RTM
features. On both cases people may be willing to use the pc-*-2.3
machine-types.

So instead of making the CPU model results confusing by making it depend
on the machine-type, keep HLE and RTM on the existing Haswell and
Broadwell CPU models. The plan is to introduce "Haswell-noTSX" and
"Broadwell-noTSX" CPU models later, for people who have CPUs that don't
have TSX feature available.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 4 ----
 hw/i386/pc_q35.c  | 4 ----
 target-i386/cpu.c | 9 +++++----
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8eab4ba..0e32afa 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -331,10 +331,6 @@ static void pc_compat_2_2(MachineState *machine)
     x86_cpu_compat_set_features("Haswell", FEAT_1_ECX, 0, CPUID_EXT_RDRAND);
     x86_cpu_compat_set_features("Broadwell", FEAT_1_ECX, 0, CPUID_EXT_F16C);
     x86_cpu_compat_set_features("Broadwell", FEAT_1_ECX, 0, CPUID_EXT_RDRAND);
-    x86_cpu_compat_set_features("Haswell", FEAT_7_0_EBX,
-                                CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
-    x86_cpu_compat_set_features("Broadwell", FEAT_7_0_EBX,
-                                CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
 }
 
 static void pc_compat_2_1(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c0f21fe..5a7b11c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -310,10 +310,6 @@ static void pc_compat_2_2(MachineState *machine)
     x86_cpu_compat_set_features("Haswell", FEAT_1_ECX, 0, CPUID_EXT_RDRAND);
     x86_cpu_compat_set_features("Broadwell", FEAT_1_ECX, 0, CPUID_EXT_F16C);
     x86_cpu_compat_set_features("Broadwell", FEAT_1_ECX, 0, CPUID_EXT_RDRAND);
-    x86_cpu_compat_set_features("Haswell", FEAT_7_0_EBX,
-                                CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
-    x86_cpu_compat_set_features("Broadwell", FEAT_7_0_EBX,
-                                CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
 }
 
 static void pc_compat_2_1(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ed7e5d5..de3cdce 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1099,8 +1099,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT3_LAHF_LM,
         .features[FEAT_7_0_EBX] =
             CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
-            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
-            CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID,
+            CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
+            CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
+            CPUID_7_0_EBX_RTM,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
         .xlevel = 0x8000000A,
@@ -1133,9 +1134,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
         .features[FEAT_7_0_EBX] =
             CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
-            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
+            CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
             CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
-            CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
+            CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
             CPUID_7_0_EBX_SMAP,
         .features[FEAT_XSAVE] =
             CPUID_XSAVE_XSAVEOPT,
-- 
2.1.0

Re: [Qemu-devel] [PATCH 1/2] Revert "target-i386: Disable HLE and RTM on Haswell & Broadwell"

[Daniel P. Berrange]

On Fri, Mar 13, 2015 at 04:09:56PM -0300, Eduardo Habkost wrote:
> This reverts commit 13704e4c455770d500d6b87b117e32f0d01252c9.
> 
> With the Intel microcode update that removed HLE and RTM, there will be
> different kinds of Haswell and Broadwell CPUs out there: some that still
> have the HLE and RTM features, and some that don't have the HLE and RTM
> features. On both cases people may be willing to use the pc-*-2.3
> machine-types.
> 
> So instead of making the CPU model results confusing by making it depend
> on the machine-type, keep HLE and RTM on the existing Haswell and
> Broadwell CPU models. The plan is to introduce "Haswell-noTSX" and
> "Broadwell-noTSX" CPU models later, for people who have CPUs that don't
> have TSX feature available.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Yep, in this situation we need to support both "models" of CPU, so changing
based on machine type is inappropriate in this scenario.

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

[Qemu-devel] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX

[Eduardo Habkost]

With the Intel microcode update that removed HLE and RTM, there will be
different kinds of Haswell and Broadwell CPUs out there: some that still
have the HLE and RTM features, and some that don't have the HLE and RTM
features. On both cases people may be willing to use the pc-*-2.3
machine-types.

So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU
models, for hosts that have Haswell and Broadwell CPUs without TSX support.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index de3cdce..b693bab 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1073,6 +1073,39 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .model_id = "Intel Xeon E3-12xx v2 (Ivy Bridge)",
     },
     {
+        .name = "Haswell-noTSX",
+        .level = 0xd,
+        .vendor = CPUID_VENDOR_INTEL,
+        .family = 6,
+        .model = 60,
+        .stepping = 1,
+        .features[FEAT_1_EDX] =
+            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
+            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
+            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
+            CPUID_DE | CPUID_FP87,
+        .features[FEAT_1_ECX] =
+            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
+            CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
+            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
+            CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
+            CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
+            CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
+        .features[FEAT_8000_0001_EDX] =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
+            CPUID_EXT2_SYSCALL,
+        .features[FEAT_8000_0001_ECX] =
+            CPUID_EXT3_LAHF_LM,
+        .features[FEAT_7_0_EBX] =
+            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
+            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
+            CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID,
+        .features[FEAT_XSAVE] =
+            CPUID_XSAVE_XSAVEOPT,
+        .xlevel = 0x8000000A,
+        .model_id = "Intel Core Processor (Haswell, no TSX)",
+    },    {
         .name = "Haswell",
         .level = 0xd,
         .vendor = CPUID_VENDOR_INTEL,
@@ -1108,6 +1141,42 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .model_id = "Intel Core Processor (Haswell)",
     },
     {
+        .name = "Broadwell-noTSX",
+        .level = 0xd,
+        .vendor = CPUID_VENDOR_INTEL,
+        .family = 6,
+        .model = 61,
+        .stepping = 2,
+        .features[FEAT_1_EDX] =
+            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
+            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
+            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
+            CPUID_DE | CPUID_FP87,
+        .features[FEAT_1_ECX] =
+            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
+            CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
+            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
+            CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
+            CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
+            CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
+        .features[FEAT_8000_0001_EDX] =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
+            CPUID_EXT2_SYSCALL,
+        .features[FEAT_8000_0001_ECX] =
+            CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
+        .features[FEAT_7_0_EBX] =
+            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
+            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
+            CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
+            CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
+            CPUID_7_0_EBX_SMAP,
+        .features[FEAT_XSAVE] =
+            CPUID_XSAVE_XSAVEOPT,
+        .xlevel = 0x8000000A,
+        .model_id = "Intel Core Processor (Broadwell, no TSX)",
+    },
+    {
         .name = "Broadwell",
         .level = 0xd,
         .vendor = CPUID_VENDOR_INTEL,
-- 
2.1.0

Re: [Qemu-devel] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX

[Daniel P. Berrange]

On Fri, Mar 13, 2015 at 04:09:57PM -0300, Eduardo Habkost wrote:
> With the Intel microcode update that removed HLE and RTM, there will be
> different kinds of Haswell and Broadwell CPUs out there: some that still
> have the HLE and RTM features, and some that don't have the HLE and RTM
> features. On both cases people may be willing to use the pc-*-2.3
> machine-types.
> 
> So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU
> models, for hosts that have Haswell and Broadwell CPUs without TSX support.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

The addition of Haswell-noTSX looks good to me.

I'm unclear on whether we truely need Broadwell-noTSX though. Did
Intel actually ship any Broadwell production silicon in which the
microcode disables this feature, or was it only a problem on
pre-production samples of Broadwell ? If the latter, I'd say we
don't need to have a Broadwell-noTSX model added. Perhaps Jun/Don
can confirm from Intel's side.


> ---
>  target-i386/cpu.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index de3cdce..b693bab 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1073,6 +1073,39 @@ static X86CPUDefinition builtin_x86_defs[] = {
>          .model_id = "Intel Xeon E3-12xx v2 (Ivy Bridge)",
>      },
>      {
> +        .name = "Haswell-noTSX",
> +        .level = 0xd,
> +        .vendor = CPUID_VENDOR_INTEL,
> +        .family = 6,
> +        .model = 60,
> +        .stepping = 1,
> +        .features[FEAT_1_EDX] =
> +            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
> +            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
> +            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
> +            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
> +            CPUID_DE | CPUID_FP87,
> +        .features[FEAT_1_ECX] =
> +            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
> +            CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
> +            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
> +            CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
> +            CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
> +            CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
> +        .features[FEAT_8000_0001_EDX] =
> +            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
> +            CPUID_EXT2_SYSCALL,
> +        .features[FEAT_8000_0001_ECX] =
> +            CPUID_EXT3_LAHF_LM,
> +        .features[FEAT_7_0_EBX] =
> +            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
> +            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
> +            CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID,
> +        .features[FEAT_XSAVE] =
> +            CPUID_XSAVE_XSAVEOPT,
> +        .xlevel = 0x8000000A,
> +        .model_id = "Intel Core Processor (Haswell, no TSX)",
> +    },    {
>          .name = "Haswell",
>          .level = 0xd,
>          .vendor = CPUID_VENDOR_INTEL,
> @@ -1108,6 +1141,42 @@ static X86CPUDefinition builtin_x86_defs[] = {
>          .model_id = "Intel Core Processor (Haswell)",
>      },
>      {
> +        .name = "Broadwell-noTSX",
> +        .level = 0xd,
> +        .vendor = CPUID_VENDOR_INTEL,
> +        .family = 6,
> +        .model = 61,
> +        .stepping = 2,
> +        .features[FEAT_1_EDX] =
> +            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
> +            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
> +            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
> +            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
> +            CPUID_DE | CPUID_FP87,
> +        .features[FEAT_1_ECX] =
> +            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
> +            CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
> +            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
> +            CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
> +            CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
> +            CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
> +        .features[FEAT_8000_0001_EDX] =
> +            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
> +            CPUID_EXT2_SYSCALL,
> +        .features[FEAT_8000_0001_ECX] =
> +            CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
> +        .features[FEAT_7_0_EBX] =
> +            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
> +            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
> +            CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
> +            CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
> +            CPUID_7_0_EBX_SMAP,
> +        .features[FEAT_XSAVE] =
> +            CPUID_XSAVE_XSAVEOPT,
> +        .xlevel = 0x8000000A,
> +        .model_id = "Intel Core Processor (Broadwell, no TSX)",
> +    },
> +    {
>          .name = "Broadwell",
>          .level = 0xd,
>          .vendor = CPUID_VENDOR_INTEL,

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX

[Eduardo Habkost]

On Mon, Mar 16, 2015 at 10:24:51AM +0000, Daniel P. Berrange wrote:
> On Fri, Mar 13, 2015 at 04:09:57PM -0300, Eduardo Habkost wrote:
> > With the Intel microcode update that removed HLE and RTM, there will be
> > different kinds of Haswell and Broadwell CPUs out there: some that still
> > have the HLE and RTM features, and some that don't have the HLE and RTM
> > features. On both cases people may be willing to use the pc-*-2.3
> > machine-types.
> > 
> > So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU
> > models, for hosts that have Haswell and Broadwell CPUs without TSX support.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> The addition of Haswell-noTSX looks good to me.
> 
> I'm unclear on whether we truely need Broadwell-noTSX though. Did
> Intel actually ship any Broadwell production silicon in which the
> microcode disables this feature, or was it only a problem on
> pre-production samples of Broadwell ? If the latter, I'd say we
> don't need to have a Broadwell-noTSX model added. Perhaps Jun/Don
> can confirm from Intel's side.

I've talked to Don and Jun, and they confirmed that a Broadwell-noTSX
CPU model will be needed, too.

I see some Broadwell CPUs without TSX-NI on ark.intel.com, too, so the
TSX errata wouldn't be the only reason for needing the -noTSX model.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX

[Daniel P. Berrange]

On Thu, Mar 19, 2015 at 03:02:27PM -0300, Eduardo Habkost wrote:
> On Mon, Mar 16, 2015 at 10:24:51AM +0000, Daniel P. Berrange wrote:
> > On Fri, Mar 13, 2015 at 04:09:57PM -0300, Eduardo Habkost wrote:
> > > With the Intel microcode update that removed HLE and RTM, there will be
> > > different kinds of Haswell and Broadwell CPUs out there: some that still
> > > have the HLE and RTM features, and some that don't have the HLE and RTM
> > > features. On both cases people may be willing to use the pc-*-2.3
> > > machine-types.
> > > 
> > > So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU
> > > models, for hosts that have Haswell and Broadwell CPUs without TSX support.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > 
> > The addition of Haswell-noTSX looks good to me.
> > 
> > I'm unclear on whether we truely need Broadwell-noTSX though. Did
> > Intel actually ship any Broadwell production silicon in which the
> > microcode disables this feature, or was it only a problem on
> > pre-production samples of Broadwell ? If the latter, I'd say we
> > don't need to have a Broadwell-noTSX model added. Perhaps Jun/Don
> > can confirm from Intel's side.
> 
> I've talked to Don and Jun, and they confirmed that a Broadwell-noTSX
> CPU model will be needed, too.
> 
> I see some Broadwell CPUs without TSX-NI on ark.intel.com, too, so the
> TSX errata wouldn't be the only reason for needing the -noTSX model.

Ok, your patch looks good.

  Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models

[Andreas Färber]

Am 13.03.2015 um 20:09 schrieb Eduardo Habkost:
> With the Intel microcode update that removed HLE and RTM, there will be
> different kinds of Haswell and Broadwell CPUs out there: some that still
> have the HLE and RTM features, and some that don't have the HLE and RTM
> features. On both cases people may be willing to use the pc-*-2.3
> machine-types.
> 
> So instead of making the CPU model results confusing by making it depend on the
> machine-type, keep HLE and RTM on the existing Haswell and Broadwell CPU
> models, and introduce "Haswell-noTSX" and "Broadwell-noTSX" CPU models later,
> for people who have CPUs that don't have TSX feature available.
> 
> Eduardo Habkost (2):
>   Revert "target-i386: Disable HLE and RTM on Haswell & Broadwell"
>   target-i386: Haswell-noTSX and Broadwell-noTSX

No objections from a generic CPU point of view.

Only thing that comes to mind is whether it might make sense to
hierarchically make Broadwell the parent type of Broadwell-noTSX, to
avoid duplication. But then again we already have a lot of it. ;)

Regards,
Andreas

>  hw/i386/pc_piix.c |  4 ---
>  hw/i386/pc_q35.c  |  4 ---
>  target-i386/cpu.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 72 insertions(+), 10 deletions(-)

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models

[Eduardo Habkost]

On Fri, Mar 13, 2015 at 08:25:19PM +0100, Andreas Färber wrote:
> Am 13.03.2015 um 20:09 schrieb Eduardo Habkost:
> > With the Intel microcode update that removed HLE and RTM, there will be
> > different kinds of Haswell and Broadwell CPUs out there: some that still
> > have the HLE and RTM features, and some that don't have the HLE and RTM
> > features. On both cases people may be willing to use the pc-*-2.3
> > machine-types.
> > 
> > So instead of making the CPU model results confusing by making it depend on the
> > machine-type, keep HLE and RTM on the existing Haswell and Broadwell CPU
> > models, and introduce "Haswell-noTSX" and "Broadwell-noTSX" CPU models later,
> > for people who have CPUs that don't have TSX feature available.
> > 
> > Eduardo Habkost (2):
> >   Revert "target-i386: Disable HLE and RTM on Haswell & Broadwell"
> >   target-i386: Haswell-noTSX and Broadwell-noTSX
> 
> No objections from a generic CPU point of view.
> 
> Only thing that comes to mind is whether it might make sense to
> hierarchically make Broadwell the parent type of Broadwell-noTSX, to
> avoid duplication. But then again we already have a lot of it. ;)

The builtin_x86_defs[] table has no way to represent inheritance,
currently. Maybe one day when we move everything inside class_init
functions. :)

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models

[Andreas Färber]

Am 13.03.2015 um 20:45 schrieb Eduardo Habkost:
> On Fri, Mar 13, 2015 at 08:25:19PM +0100, Andreas Färber wrote:
>> Only thing that comes to mind is whether it might make sense to
>> hierarchically make Broadwell the parent type of Broadwell-noTSX, to
>> avoid duplication. But then again we already have a lot of it. ;)
> 
> The builtin_x86_defs[] table has no way to represent inheritance,
> currently. Maybe one day when we move everything inside class_init
> functions. :)

If x86_cpu_compat_set_features(), x86_cpu_list() and
arch_query_cpu_definitions() get fixed to no longer operate on the old
builtin_x86_defs[] but on object_class_foreach() or similar functions,
then you can register special CPU types in x86_cpu_register_types(),
like we do for the KVM host type already.

In this case I imagined a custom class_init with an &= ~... operation.
But I wasn't aware we're still using the array in so many places! :/

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)