1:40 p.m.
A recent glibc commit[1] added a blacklist to ensure it won't use
TSX on hosts that are known to have a broken TSX implementation.
Our existing Haswell CPU model has a blacklisted
family/model/stepping combination, so it has to be updated to
make sure guests will really use TSX. This is done by patch 5/5.
However, to do this safely we need to ensure the host CPU is not
a blacklisted one, so we won't mislead guests by exposing
known-to-be-good FMS values on a known-to-be-broken host. This is
done by patch 3/5.
[1]
https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2...
---
Cc: dgilbert(a)redhat.com
Cc: fweimer(a)redhat.com
Cc: carlos(a)redhat.com
Cc: triegel(a)redhat.com
Cc: berrange(a)redhat.com
Cc: jdenemar(a)redhat.com
Cc: pbonzini(a)redhat.com
Eduardo Habkost (5):
i386: Add explicit array size to x86_cpu_vendor_words2str()
i386: host_vendor_fms() helper function
i386/kvm: Blacklist TSX on known broken hosts
pc: Add 2.9 machine-types
i386: Change stepping of Haswell to non-blacklisted value
include/hw/i386/pc.h | 6 ++++++
target/i386/cpu.h | 1 +
hw/i386/pc_piix.c | 15 ++++++++++++---
hw/i386/pc_q35.c | 13 +++++++++++--
target/i386/cpu.c | 32 ++++++++++++++++++++++----------
target/i386/kvm.c | 17 +++++++++++++++++
6 files changed, 69 insertions(+), 15 deletions(-)
--
2.11.0.259.g40922b1
1:40 p.m.
New subject: [libvirt] [PATCH 1/5] i386: Add explicit array size to x86_cpu_vendor_words2str()
Add explicit array size to x86_cpu_vendor_words2str() to let the
compiler check if we are really passing an array of the right
size.
GCC still doesn't print a warning when the array is too small[1],
but clang already does.
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50584
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
target/i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a149c8dc42..25b802bb06 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -169,7 +169,7 @@
-static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
+static void x86_cpu_vendor_words2str(char dst[static (CPUID_VENDOR_SZ + 1)], uint32_t
vendor1,
uint32_t vendor2, uint32_t vendor3)
{
int i;
--
2.11.0.259.g40922b1
1:40 p.m.
New subject: [libvirt] [PATCH 2/5] i386: host_vendor_fms() helper function
Helper function for code that needs to check the host CPU
vendor/family/model/stepping values.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
target/i386/cpu.h | 1 +
target/i386/cpu.c | 28 ++++++++++++++++++++--------
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index a7f2f6099d..0f4a9d412b 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1423,6 +1423,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t
count,
void cpu_clear_apic_feature(CPUX86State *env);
void host_cpuid(uint32_t function, uint32_t count,
uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+void host_vendor_fms(char vendor[static (CPUID_VENDOR_SZ + 1)], int *family, int *model,
int *stepping);
/* helper.c */
int x86_cpu_handle_mmu_fault(CPUState *cpu, vaddr addr,
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 25b802bb06..9dbb2d98da 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -682,6 +682,25 @@ void host_cpuid(uint32_t function, uint32_t count,
*edx = vec[3];
}
+void host_vendor_fms(char vendor[static (CPUID_VENDOR_SZ + 1)], int *family, int *model,
int *stepping)
+{
+ uint32_t eax, ebx, ecx, edx;
+
+ host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
+ x86_cpu_vendor_words2str(vendor, ebx, edx, ecx);
+
+ host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
+ if (family) {
+ *family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
+ }
+ if (model) {
+ *model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
+ }
+ if (stepping) {
+ *stepping = eax & 0x0F;
+ }
+}
+
/* CPU class name definitions: */
#define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
@@ -1574,17 +1593,10 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
X86CPUClass *xcc = X86_CPU_CLASS(oc);
- uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
xcc->kvm_required = true;
- host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
- x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
-
- host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
- host_cpudef.family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
- host_cpudef.model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >>
12);
- host_cpudef.stepping = eax & 0x0F;
+ host_vendor_fms(host_cpudef.vendor, &host_cpudef.family, &host_cpudef.model,
&host_cpudef.stepping);
cpu_x86_fill_model_id(host_cpudef.model_id);
--
2.11.0.259.g40922b1
1:40 p.m.
New subject: [libvirt] [PATCH 3/5] i386/kvm: Blacklist TSX on known broken hosts
Some Intel CPUs are known to have a broken TSX implementation. A
microcode update from Intel disabled TSX on those CPUs, but
GET_SUPPORTED_CPUID might be reporting it as supported if the
hosts were not updated yet.
Manually fixup the GET_SUPPORTED_CPUID data to ensure we will
never enable TSX when running on those hosts.
Reference:
* glibc commit 2702856bf45c82cf8e69f2064f5aa15c0ceb6359:
https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2...
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
target/i386/kvm.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 10a9cd8f7f..3e99512640 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -272,6 +272,19 @@ static int get_para_features(KVMState *s)
return features;
}
+static bool host_tsx_blacklisted(void)
+{
+ int family, model, stepping;\
+ char vendor[CPUID_VENDOR_SZ + 1];
+
+ host_vendor_fms(vendor, &family, &model, &stepping);
+
+ /* Check if we are running on a Haswell host known to have broken TSX */
+ return !strcmp(vendor, CPUID_VENDOR_INTEL) &&
+ (family == 6) &&
+ ((model == 63 && stepping < 4) ||
+ model == 60 || model == 69 || model == 70);
+}
/* Returns the value for a specific register on the cpuid entry
*/
@@ -355,6 +368,10 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t
function,
}
} else if (function == 6 && reg == R_EAX) {
ret |= CPUID_6_EAX_ARAT; /* safe to allow because of emulated APIC */
+ } else if (function == 7 && index == 0 && reg == R_EBX) {
+ if (host_tsx_blacklisted()) {
+ ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
+ }
} else if (function == 0x80000001 && reg == R_EDX) {
/* On Intel, kvm returns cpuid according to the Intel spec,
* so add missing bits according to the AMD spec:
--
2.11.0.259.g40922b1
1:40 p.m.
New subject: [libvirt] [PATCH 4/5] pc: Add 2.9 machine-types
Cc: "Michael S. Tsirkin" <mst(a)redhat.com>
Cc: Laszlo Ersek <lersek(a)redhat.com>
Cc: Igor Mammedov <imammedo(a)redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Changes v1 -> v2:
* Added extra backslash to PC_COMPAT_2_8 definition
* Suggested-by: Laszlo Ersek <lersek(a)redhat.com>
---
include/hw/i386/pc.h | 2 ++
hw/i386/pc_piix.c | 15 ++++++++++++---
hw/i386/pc_q35.c | 13 +++++++++++--
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index b22e699c46..230e9e70c5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -375,6 +375,8 @@ int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_8 \
+ HW_COMPAT_2_8 \
+
#define PC_COMPAT_2_7 \
HW_COMPAT_2_7 \
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5e1adbe53c..9f102aa388 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -437,13 +437,24 @@ static void pc_i440fx_machine_options(MachineClass *m)
m->default_display = "std";
}
-static void pc_i440fx_2_8_machine_options(MachineClass *m)
+static void pc_i440fx_2_9_machine_options(MachineClass *m)
{
pc_i440fx_machine_options(m);
m->alias = "pc";
m->is_default = 1;
}
+DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL,
+ pc_i440fx_2_9_machine_options);
+
+static void pc_i440fx_2_8_machine_options(MachineClass *m)
+{
+ pc_i440fx_2_9_machine_options(m);
+ m->is_default = 0;
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
+}
+
DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
pc_i440fx_2_8_machine_options);
@@ -451,8 +462,6 @@ DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
static void pc_i440fx_2_7_machine_options(MachineClass *m)
{
pc_i440fx_2_8_machine_options(m);
- m->is_default = 0;
- m->alias = NULL;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d042fe0843..dd792a8547 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -301,19 +301,28 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_2_8_machine_options(MachineClass *m)
+static void pc_q35_2_9_machine_options(MachineClass *m)
{
pc_q35_machine_options(m);
m->alias = "q35";
}
+DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL,
+ pc_q35_2_9_machine_options);
+
+static void pc_q35_2_8_machine_options(MachineClass *m)
+{
+ pc_q35_2_9_machine_options(m);
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
+}
+
DEFINE_Q35_MACHINE(v2_8, "pc-q35-2.8", NULL,
pc_q35_2_8_machine_options);
static void pc_q35_2_7_machine_options(MachineClass *m)
{
pc_q35_2_8_machine_options(m);
- m->alias = NULL;
m->max_cpus = 255;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
}
--
2.11.0.259.g40922b1
6:21 a.m.
New subject: [libvirt] [Qemu-devel] [PATCH 4/5] pc: Add 2.9 machine-types
On 01/08/17 20:40, Eduardo Habkost wrote:
Cc: "Michael S. Tsirkin" <mst(a)redhat.com>
Cc: Laszlo Ersek <lersek(a)redhat.com>
Cc: Igor Mammedov <imammedo(a)redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Changes v1 -> v2:
* Added extra backslash to PC_COMPAT_2_8 definition
* Suggested-by: Laszlo Ersek <lersek(a)redhat.com>
---
include/hw/i386/pc.h | 2 ++
hw/i386/pc_piix.c | 15 ++++++++++++---
hw/i386/pc_q35.c | 13 +++++++++++--
3 files changed, 25 insertions(+), 5 deletions(-)
Reviewed-by: Laszlo Ersek <lersek(a)redhat.com>
Thanks
Laszlo
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index b22e699c46..230e9e70c5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -375,6 +375,8 @@ int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_8 \
+ HW_COMPAT_2_8 \
+
#define PC_COMPAT_2_7 \
HW_COMPAT_2_7 \
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5e1adbe53c..9f102aa388 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -437,13 +437,24 @@ static void pc_i440fx_machine_options(MachineClass *m)
m->default_display = "std";
}
-static void pc_i440fx_2_8_machine_options(MachineClass *m)
+static void pc_i440fx_2_9_machine_options(MachineClass *m)
{
pc_i440fx_machine_options(m);
m->alias = "pc";
m->is_default = 1;
}
+DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL,
+ pc_i440fx_2_9_machine_options);
+
+static void pc_i440fx_2_8_machine_options(MachineClass *m)
+{
+ pc_i440fx_2_9_machine_options(m);
+ m->is_default = 0;
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
+}
+
DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
pc_i440fx_2_8_machine_options);
@@ -451,8 +462,6 @@ DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
static void pc_i440fx_2_7_machine_options(MachineClass *m)
{
pc_i440fx_2_8_machine_options(m);
- m->is_default = 0;
- m->alias = NULL;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d042fe0843..dd792a8547 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -301,19 +301,28 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_2_8_machine_options(MachineClass *m)
+static void pc_q35_2_9_machine_options(MachineClass *m)
{
pc_q35_machine_options(m);
m->alias = "q35";
}
+DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL,
+ pc_q35_2_9_machine_options);
+
+static void pc_q35_2_8_machine_options(MachineClass *m)
+{
+ pc_q35_2_9_machine_options(m);
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
+}
+
DEFINE_Q35_MACHINE(v2_8, "pc-q35-2.8", NULL,
pc_q35_2_8_machine_options);
static void pc_q35_2_7_machine_options(MachineClass *m)
{
pc_q35_2_8_machine_options(m);
- m->alias = NULL;
m->max_cpus = 255;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
}
10:06 p.m.
New subject: [libvirt] [PATCH 4/5] pc: Add 2.9 machine-types
On Sun, Jan 08, 2017 at 05:40:40PM -0200, Eduardo Habkost wrote:
Cc: "Michael S. Tsirkin" <mst(a)redhat.com>
Cc: Laszlo Ersek <lersek(a)redhat.com>
Cc: Igor Mammedov <imammedo(a)redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
Do I understand it correctly that you are merging this through
another tree?
In that case
Reviewed-by: Michael S. Tsirkin <mst(a)redhat.com>
---
Changes v1 -> v2:
* Added extra backslash to PC_COMPAT_2_8 definition
* Suggested-by: Laszlo Ersek <lersek(a)redhat.com>
---
include/hw/i386/pc.h | 2 ++
hw/i386/pc_piix.c | 15 ++++++++++++---
hw/i386/pc_q35.c | 13 +++++++++++--
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index b22e699c46..230e9e70c5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -375,6 +375,8 @@ int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_8 \
+ HW_COMPAT_2_8 \
+
#define PC_COMPAT_2_7 \
HW_COMPAT_2_7 \
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5e1adbe53c..9f102aa388 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -437,13 +437,24 @@ static void pc_i440fx_machine_options(MachineClass *m)
m->default_display = "std";
}
-static void pc_i440fx_2_8_machine_options(MachineClass *m)
+static void pc_i440fx_2_9_machine_options(MachineClass *m)
{
pc_i440fx_machine_options(m);
m->alias = "pc";
m->is_default = 1;
}
+DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL,
+ pc_i440fx_2_9_machine_options);
+
+static void pc_i440fx_2_8_machine_options(MachineClass *m)
+{
+ pc_i440fx_2_9_machine_options(m);
+ m->is_default = 0;
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
+}
+
DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
pc_i440fx_2_8_machine_options);
@@ -451,8 +462,6 @@ DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
static void pc_i440fx_2_7_machine_options(MachineClass *m)
{
pc_i440fx_2_8_machine_options(m);
- m->is_default = 0;
- m->alias = NULL;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d042fe0843..dd792a8547 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -301,19 +301,28 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_2_8_machine_options(MachineClass *m)
+static void pc_q35_2_9_machine_options(MachineClass *m)
{
pc_q35_machine_options(m);
m->alias = "q35";
}
+DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL,
+ pc_q35_2_9_machine_options);
+
+static void pc_q35_2_8_machine_options(MachineClass *m)
+{
+ pc_q35_2_9_machine_options(m);
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
+}
+
DEFINE_Q35_MACHINE(v2_8, "pc-q35-2.8", NULL,
pc_q35_2_8_machine_options);
static void pc_q35_2_7_machine_options(MachineClass *m)
{
pc_q35_2_8_machine_options(m);
- m->alias = NULL;
m->max_cpus = 255;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
}
--
2.11.0.259.g40922b1
6:35 a.m.
New subject: [libvirt] [PATCH 4/5] pc: Add 2.9 machine-types
On Tue, Jan 10, 2017 at 06:06:33AM +0200, Michael S. Tsirkin wrote:
On Sun, Jan 08, 2017 at 05:40:40PM -0200, Eduardo Habkost wrote:
> Cc: "Michael S. Tsirkin" <mst(a)redhat.com>
> Cc: Laszlo Ersek <lersek(a)redhat.com>
> Cc: Igor Mammedov <imammedo(a)redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
Do I understand it correctly that you are merging this through
another tree?
I will apply it when including this series only if it is not
merged through another tree first. I believe multiple series
depend on adding the pc-2.9 machine-types. Feel free to apply it
to your tree if you want to.
In that case
Reviewed-by: Michael S. Tsirkin <mst(a)redhat.com>
Thanks!
> ---
> Changes v1 -> v2:
> * Added extra backslash to PC_COMPAT_2_8 definition
> * Suggested-by: Laszlo Ersek <lersek(a)redhat.com>
> ---
> include/hw/i386/pc.h | 2 ++
> hw/i386/pc_piix.c | 15 ++++++++++++---
> hw/i386/pc_q35.c | 13 +++++++++++--
> 3 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index b22e699c46..230e9e70c5 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -375,6 +375,8 @@ int e820_get_num_entries(void);
> bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
> #define PC_COMPAT_2_8 \
> + HW_COMPAT_2_8 \
> +
>
> #define PC_COMPAT_2_7 \
> HW_COMPAT_2_7 \
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 5e1adbe53c..9f102aa388 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -437,13 +437,24 @@ static void pc_i440fx_machine_options(MachineClass *m)
> m->default_display = "std";
> }
>
> -static void pc_i440fx_2_8_machine_options(MachineClass *m)
> +static void pc_i440fx_2_9_machine_options(MachineClass *m)
> {
> pc_i440fx_machine_options(m);
> m->alias = "pc";
> m->is_default = 1;
> }
>
> +DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL,
> + pc_i440fx_2_9_machine_options);
> +
> +static void pc_i440fx_2_8_machine_options(MachineClass *m)
> +{
> + pc_i440fx_2_9_machine_options(m);
> + m->is_default = 0;
> + m->alias = NULL;
> + SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
> +}
> +
> DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
> pc_i440fx_2_8_machine_options);
>
> @@ -451,8 +462,6 @@ DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
> static void pc_i440fx_2_7_machine_options(MachineClass *m)
> {
> pc_i440fx_2_8_machine_options(m);
> - m->is_default = 0;
> - m->alias = NULL;
> SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
> }
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index d042fe0843..dd792a8547 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -301,19 +301,28 @@ static void pc_q35_machine_options(MachineClass *m)
> m->max_cpus = 288;
> }
>
> -static void pc_q35_2_8_machine_options(MachineClass *m)
> +static void pc_q35_2_9_machine_options(MachineClass *m)
> {
> pc_q35_machine_options(m);
> m->alias = "q35";
> }
>
> +DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL,
> + pc_q35_2_9_machine_options);
> +
> +static void pc_q35_2_8_machine_options(MachineClass *m)
> +{
> + pc_q35_2_9_machine_options(m);
> + m->alias = NULL;
> + SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
> +}
> +
> DEFINE_Q35_MACHINE(v2_8, "pc-q35-2.8", NULL,
> pc_q35_2_8_machine_options);
>
> static void pc_q35_2_7_machine_options(MachineClass *m)
> {
> pc_q35_2_8_machine_options(m);
> - m->alias = NULL;
> m->max_cpus = 255;
> SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
> }
> --
> 2.11.0.259.g40922b1
--
Eduardo
1:40 p.m.
New subject: [libvirt] [PATCH 5/5] i386: Change stepping of Haswell to non-blacklisted value
glibc blacklists TSX on Haswell CPUs with model==60 and
stepping < 4. To make the Haswell CPU model more useful, make
those guests actually use TSX by changing CPU stepping to 4.
References:
* glibc commit 2702856bf45c82cf8e69f2064f5aa15c0ceb6359
https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2...
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
include/hw/i386/pc.h | 6 +++++-
target/i386/cpu.c | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 230e9e70c5..fcd9b4f541 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -376,7 +376,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_8 \
HW_COMPAT_2_8 \
-
+ {\
+ .driver = "Haswell-" TYPE_X86_CPU,\
+ .property = "stepping",\
+ .value = "1",\
+ },
#define PC_COMPAT_2_7 \
HW_COMPAT_2_7 \
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9dbb2d98da..003de7e74f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1190,7 +1190,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
.vendor = CPUID_VENDOR_INTEL,
.family = 6,
.model = 60,
- .stepping = 1,
+ .stepping = 4,
.features[FEAT_1_EDX] =
CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
--
2.11.0.259.g40922b1
5:35 a.m.
* Eduardo Habkost (ehabkost(a)redhat.com) wrote:
A recent glibc commit[1] added a blacklist to ensure it won't
use
TSX on hosts that are known to have a broken TSX implementation.
Our existing Haswell CPU model has a blacklisted
family/model/stepping combination, so it has to be updated to
make sure guests will really use TSX. This is done by patch 5/5.
However, to do this safely we need to ensure the host CPU is not
a blacklisted one, so we won't mislead guests by exposing
known-to-be-good FMS values on a known-to-be-broken host. This is
done by patch 3/5.
I'd just like to mke sure I understand the way this will fail in a migration;
lets say we have a guest that doesn't have the new libc and hosts
with a blacklisted CPU, and -cpu Haswell.
If I understand correctly then:
a) With 'enforce' the destination qemu will fail to start
printing an error about the host lack of tsx feature.
b) Without 'enforce' the destination will start but print
the same error as a warning, but the guest will probably
break as soon as it tries to use a tsx feature?
Any other combination?
Dave
[1]
https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2...
---
Cc: dgilbert(a)redhat.com
Cc: fweimer(a)redhat.com
Cc: carlos(a)redhat.com
Cc: triegel(a)redhat.com
Cc: berrange(a)redhat.com
Cc: jdenemar(a)redhat.com
Cc: pbonzini(a)redhat.com
Eduardo Habkost (5):
i386: Add explicit array size to x86_cpu_vendor_words2str()
i386: host_vendor_fms() helper function
i386/kvm: Blacklist TSX on known broken hosts
pc: Add 2.9 machine-types
i386: Change stepping of Haswell to non-blacklisted value
include/hw/i386/pc.h | 6 ++++++
target/i386/cpu.h | 1 +
hw/i386/pc_piix.c | 15 ++++++++++++---
hw/i386/pc_q35.c | 13 +++++++++++--
target/i386/cpu.c | 32 ++++++++++++++++++++++----------
target/i386/kvm.c | 17 +++++++++++++++++
6 files changed, 69 insertions(+), 15 deletions(-)
--
2.11.0.259.g40922b1
--
Dr. David Alan Gilbert / dgilbert(a)redhat.com / Manchester, UK
6:33 a.m.
On Mon, Jan 09, 2017 at 11:35:54AM +0000, Dr. David Alan Gilbert wrote:
* Eduardo Habkost (ehabkost(a)redhat.com) wrote:
> A recent glibc commit[1] added a blacklist to ensure it won't use
> TSX on hosts that are known to have a broken TSX implementation.
>
> Our existing Haswell CPU model has a blacklisted
> family/model/stepping combination, so it has to be updated to
> make sure guests will really use TSX. This is done by patch 5/5.
>
> However, to do this safely we need to ensure the host CPU is not
> a blacklisted one, so we won't mislead guests by exposing
> known-to-be-good FMS values on a known-to-be-broken host. This is
> done by patch 3/5.
I'd just like to mke sure I understand the way this will fail in a migration;
lets say we have a guest that doesn't have the new libc and hosts
with a blacklisted CPU, and -cpu Haswell.
If I understand correctly then:
a) With 'enforce' the destination qemu will fail to start
printing an error about the host lack of tsx feature.
Yes.
b) Without 'enforce' the destination will start but print
the same error as a warning, but the guest will probably
break as soon as it tries to use a tsx feature?
Yes. The general rule is: without "enforce", live migration can
break in unpredictable ways.
Without "enforce", QEMU will print a warning, and the VCPU will
run _without_ the TSX features on CPUID. If we're live-migrating,
it may break the guest if it tries to use a TSX feature, or break
migration if a TSX-related bit is already set on a MSR.
Any other combination?
Dave
> [1]
https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2...
>
> ---
> Cc: dgilbert(a)redhat.com
> Cc: fweimer(a)redhat.com
> Cc: carlos(a)redhat.com
> Cc: triegel(a)redhat.com
> Cc: berrange(a)redhat.com
> Cc: jdenemar(a)redhat.com
> Cc: pbonzini(a)redhat.com
>
> Eduardo Habkost (5):
> i386: Add explicit array size to x86_cpu_vendor_words2str()
> i386: host_vendor_fms() helper function
> i386/kvm: Blacklist TSX on known broken hosts
> pc: Add 2.9 machine-types
> i386: Change stepping of Haswell to non-blacklisted value
>
> include/hw/i386/pc.h | 6 ++++++
> target/i386/cpu.h | 1 +
> hw/i386/pc_piix.c | 15 ++++++++++++---
> hw/i386/pc_q35.c | 13 +++++++++++--
> target/i386/cpu.c | 32 ++++++++++++++++++++++----------
> target/i386/kvm.c | 17 +++++++++++++++++
> 6 files changed, 69 insertions(+), 15 deletions(-)
>
> --
> 2.11.0.259.g40922b1
>
--
Dr. David Alan Gilbert / dgilbert(a)redhat.com / Manchester, UK
--
Eduardo
6:38 a.m.
* Eduardo Habkost (ehabkost(a)redhat.com) wrote:
On Mon, Jan 09, 2017 at 11:35:54AM +0000, Dr. David Alan Gilbert
wrote:
> * Eduardo Habkost (ehabkost(a)redhat.com) wrote:
> > A recent glibc commit[1] added a blacklist to ensure it won't use
> > TSX on hosts that are known to have a broken TSX implementation.
> >
> > Our existing Haswell CPU model has a blacklisted
> > family/model/stepping combination, so it has to be updated to
> > make sure guests will really use TSX. This is done by patch 5/5.
> >
> > However, to do this safely we need to ensure the host CPU is not
> > a blacklisted one, so we won't mislead guests by exposing
> > known-to-be-good FMS values on a known-to-be-broken host. This is
> > done by patch 3/5.
>
> I'd just like to mke sure I understand the way this will fail in a migration;
> lets say we have a guest that doesn't have the new libc and hosts
> with a blacklisted CPU, and -cpu Haswell.
>
> If I understand correctly then:
> a) With 'enforce' the destination qemu will fail to start
> printing an error about the host lack of tsx feature.
Yes.
> b) Without 'enforce' the destination will start but print
> the same error as a warning, but the guest will probably
> break as soon as it tries to use a tsx feature?
Yes. The general rule is: without "enforce", live migration can
break in unpredictable ways.
Without "enforce", QEMU will print a warning, and the VCPU will
run _without_ the TSX features on CPUID. If we're live-migrating,
it may break the guest if it tries to use a TSX feature, or break
migration if a TSX-related bit is already set on a MSR.
OK, but you've been telling people to use "enforce" long enough that
they should have listened.
Are there any other cases we have to worry about; lets say a VM with the
new libc being migrated from an older QEMU, it suddenly changes
CPU ID to one that's supported; what happens?
I'm hoping the guest CPU ID is preserved with the TSX disabled until
a reboot?
Dave
>
> Any other combination?
>
> Dave
>
> > [1]
https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2...
> >
> > ---
> > Cc: dgilbert(a)redhat.com
> > Cc: fweimer(a)redhat.com
> > Cc: carlos(a)redhat.com
> > Cc: triegel(a)redhat.com
> > Cc: berrange(a)redhat.com
> > Cc: jdenemar(a)redhat.com
> > Cc: pbonzini(a)redhat.com
> >
> > Eduardo Habkost (5):
> > i386: Add explicit array size to x86_cpu_vendor_words2str()
> > i386: host_vendor_fms() helper function
> > i386/kvm: Blacklist TSX on known broken hosts
> > pc: Add 2.9 machine-types
> > i386: Change stepping of Haswell to non-blacklisted value
> >
> > include/hw/i386/pc.h | 6 ++++++
> > target/i386/cpu.h | 1 +
> > hw/i386/pc_piix.c | 15 ++++++++++++---
> > hw/i386/pc_q35.c | 13 +++++++++++--
> > target/i386/cpu.c | 32 ++++++++++++++++++++++----------
> > target/i386/kvm.c | 17 +++++++++++++++++
> > 6 files changed, 69 insertions(+), 15 deletions(-)
> >
> > --
> > 2.11.0.259.g40922b1
> >
> --
> Dr. David Alan Gilbert / dgilbert(a)redhat.com / Manchester, UK
--
Eduardo
--
Dr. David Alan Gilbert / dgilbert(a)redhat.com / Manchester, UK
7:04 a.m.
On Thu, Jan 12, 2017 at 12:38:00PM +0000, Dr. David Alan Gilbert wrote:
* Eduardo Habkost (ehabkost(a)redhat.com) wrote:
> On Mon, Jan 09, 2017 at 11:35:54AM +0000, Dr. David Alan Gilbert wrote:
> > * Eduardo Habkost (ehabkost(a)redhat.com) wrote:
> > > A recent glibc commit[1] added a blacklist to ensure it won't use
> > > TSX on hosts that are known to have a broken TSX implementation.
> > >
> > > Our existing Haswell CPU model has a blacklisted
> > > family/model/stepping combination, so it has to be updated to
> > > make sure guests will really use TSX. This is done by patch 5/5.
> > >
> > > However, to do this safely we need to ensure the host CPU is not
> > > a blacklisted one, so we won't mislead guests by exposing
> > > known-to-be-good FMS values on a known-to-be-broken host. This is
> > > done by patch 3/5.
> >
> > I'd just like to mke sure I understand the way this will fail in a
migration;
> > lets say we have a guest that doesn't have the new libc and hosts
> > with a blacklisted CPU, and -cpu Haswell.
> >
> > If I understand correctly then:
> > a) With 'enforce' the destination qemu will fail to start
> > printing an error about the host lack of tsx feature.
>
> Yes.
>
> > b) Without 'enforce' the destination will start but print
> > the same error as a warning, but the guest will probably
> > break as soon as it tries to use a tsx feature?
>
> Yes. The general rule is: without "enforce", live migration can
> break in unpredictable ways.
>
> Without "enforce", QEMU will print a warning, and the VCPU will
> run _without_ the TSX features on CPUID. If we're live-migrating,
> it may break the guest if it tries to use a TSX feature, or break
> migration if a TSX-related bit is already set on a MSR.
OK, but you've been telling people to use "enforce" long enough that
they should have listened.
Are there any other cases we have to worry about; lets say a VM with the
new libc being migrated from an older QEMU, it suddenly changes
CPU ID to one that's supported; what happens?
I assume you are talking about the stepping change, when
migrating from an old QEMU to a host that is _not_ on the
blacklist. In this case, the guest won't see any changes: CPUID
family/model/stepping will be kept the same for the whole life of
the VM (even if it is shut down), thanks to the machine-type
compatibility code.
I'm hoping the guest CPU ID is preserved with the TSX disabled
until
a reboot?
CPUID changes are effective immediately on migration. But guests
often notice the change only on the next reboot.
We could do something to make these problems less likely:
including CPUID data on the migration stream. I have considered
it on the past, but never implemented it. Maybe I should
reconsider that.
(This is another case where it would be interesting to have a
mechanism to let the destination host abort migration early: we
could make QEMU work in "enforce" mode when live-migrating, but
using the migrated CPUID data. This way CPUID changes would never
happen.)
Dave
>
> >
> > Any other combination?
> >
> > Dave
> >
> > > [1]
https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2...
> > >
> > > ---
> > > Cc: dgilbert(a)redhat.com
> > > Cc: fweimer(a)redhat.com
> > > Cc: carlos(a)redhat.com
> > > Cc: triegel(a)redhat.com
> > > Cc: berrange(a)redhat.com
> > > Cc: jdenemar(a)redhat.com
> > > Cc: pbonzini(a)redhat.com
> > >
> > > Eduardo Habkost (5):
> > > i386: Add explicit array size to x86_cpu_vendor_words2str()
> > > i386: host_vendor_fms() helper function
> > > i386/kvm: Blacklist TSX on known broken hosts
> > > pc: Add 2.9 machine-types
> > > i386: Change stepping of Haswell to non-blacklisted value
> > >
> > > include/hw/i386/pc.h | 6 ++++++
> > > target/i386/cpu.h | 1 +
> > > hw/i386/pc_piix.c | 15 ++++++++++++---
> > > hw/i386/pc_q35.c | 13 +++++++++++--
> > > target/i386/cpu.c | 32 ++++++++++++++++++++++----------
> > > target/i386/kvm.c | 17 +++++++++++++++++
> > > 6 files changed, 69 insertions(+), 15 deletions(-)
> > >
> > > --
> > > 2.11.0.259.g40922b1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert(a)redhat.com / Manchester, UK
>
> --
> Eduardo
--
Dr. David Alan Gilbert / dgilbert(a)redhat.com / Manchester, UK
--
Eduardo
2871
days inactive
2875
days old
12 comments
4 participants
participants (4)
-
Dr. David Alan Gilbert -
Eduardo Habkost -
Laszlo Ersek -
Michael S. Tsirkin