From 823efc5d26671e4737b2951d65b0565806ee43ab Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 15 Jun 2016 14:59:46 -0300 Subject: [PATCH 01/16] qdev: Don't stop applying globals on first error qdev_prop_set_globals_for_type() stops applying global properties on the first error. It is a leftover from when QEMU exited on any error when applying global property. Commit 25f8dd9 changed the fatal error to a warning, but neglected to drop the stopping. Fix that. For example, the following command-line will not set CPUID level to 3, but will warn only about "x86_64-cpu.vendor" being ignored. $ ./x86_64-softmmu/qemu-system-x86_64 \ -global x86_64-cpu.vendor=x \ -global x86_64-cpu.level=3 qemu-system-x86_64: Warning: global x86_64-cpu.vendor=x ignored: Property '.vendor' doesn't take value 'x' Fix this by not returning from qdev_prop_set_globals_for_type() on the first error. Cc: Markus Armbruster Reviewed-by: Markus Armbruster Reviewed-by: Marcel Apfelbaum Signed-off-by: Eduardo Habkost --- hw/core/qdev-properties.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index e3b2184a60..c10edeecf1 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -1088,7 +1088,6 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev, assert(prop->user_provided); error_reportf_err(err, "Warning: global %s.%s=%s ignored: ", prop->driver, prop->property, prop->value); - return; } } } From 8d76bfe8f8dbc7995bc0682d14f5f92583ae5b0a Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 15 Jun 2016 15:54:52 -0300 Subject: [PATCH 02/16] qdev: Eliminate qemu_add_globals() function The function is just a helper to handle the -global options, it can stay in vl.c like most qemu_opts_foreach() calls. Reviewed-by: Igor Mammedov Reviewed-by: Markus Armbruster Signed-off-by: Eduardo Habkost --- hw/core/qdev-properties-system.c | 21 +-------------------- include/qemu/config-file.h | 1 - vl.c | 16 +++++++++++++++- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index df38b8a03b..65d9fa9f53 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -1,5 +1,5 @@ /* - * qdev property parsing and global properties + * qdev property parsing * (parts specific for qemu-system-*) * * This file is based on code from hw/qdev-properties.c from @@ -394,22 +394,3 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd) } nd->instantiated = 1; } - -static int qdev_add_one_global(void *opaque, QemuOpts *opts, Error **errp) -{ - GlobalProperty *g; - - g = g_malloc0(sizeof(*g)); - g->driver = qemu_opt_get(opts, "driver"); - g->property = qemu_opt_get(opts, "property"); - g->value = qemu_opt_get(opts, "value"); - g->user_provided = true; - qdev_prop_register_global(g); - return 0; -} - -void qemu_add_globals(void) -{ - qemu_opts_foreach(qemu_find_opts("global"), - qdev_add_one_global, NULL, NULL); -} diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h index 3b8ecb0953..8603e86395 100644 --- a/include/qemu/config-file.h +++ b/include/qemu/config-file.h @@ -12,7 +12,6 @@ void qemu_add_opts(QemuOptsList *list); void qemu_add_drive_opts(QemuOptsList *list); int qemu_set_option(const char *str); int qemu_global_option(const char *str); -void qemu_add_globals(void); void qemu_config_write(FILE *fp); int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname); diff --git a/vl.c b/vl.c index 5cd0f2a7db..ebdeaa0b9d 100644 --- a/vl.c +++ b/vl.c @@ -2913,6 +2913,19 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size, loc_pop(&loc); } +static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) +{ + GlobalProperty *g; + + g = g_malloc0(sizeof(*g)); + g->driver = qemu_opt_get(opts, "driver"); + g->property = qemu_opt_get(opts, "property"); + g->value = qemu_opt_get(opts, "value"); + g->user_provided = true; + qdev_prop_register_global(g); + return 0; +} + int main(int argc, char **argv, char **envp) { int i; @@ -4442,7 +4455,8 @@ int main(int argc, char **argv, char **envp) qdev_prop_register_global(p); } } - qemu_add_globals(); + qemu_opts_foreach(qemu_find_opts("global"), + global_init_func, NULL, NULL); /* This checkpoint is required by replay to separate prior clock reading from the other reads, because timer polling functions query From 77280adbdf308af855844d921e5f16a873840568 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 15 Jun 2016 16:08:06 -0300 Subject: [PATCH 03/16] qdev: GlobalProperty.errp field The new field will allow error handling to be configured by qdev_prop_register_global() callers: &error_fatal and &error_abort can be used to make QEMU exit or abort if any errors are reported when applying the properties. While doing it, change the error message from "global %s.%s=%s ignored" to "can't apply global %s.%s=%s". Suggested-by: Paolo Bonzini Reviewed-by: Igor Mammedov Reviewed-by: Markus Armbruster Signed-off-by: Eduardo Habkost --- hw/core/qdev-properties.c | 11 ++++++++--- include/hw/qdev-core.h | 4 ++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index c10edeecf1..3c20c8e4b2 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -1085,9 +1085,14 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev, prop->used = true; object_property_parse(OBJECT(dev), prop->value, prop->property, &err); if (err != NULL) { - assert(prop->user_provided); - error_reportf_err(err, "Warning: global %s.%s=%s ignored: ", - prop->driver, prop->property, prop->value); + error_prepend(&err, "can't apply global %s.%s=%s: ", + prop->driver, prop->property, prop->value); + if (prop->errp) { + error_propagate(prop->errp, err); + } else { + assert(prop->user_provided); + error_reportf_err(err, "Warning: "); + } } } } diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 24aa0a7949..1d1f8612a9 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -259,6 +259,9 @@ struct PropertyInfo { * @user_provided: Set to true if property comes from user-provided config * (command-line or config file). * @used: Set to true if property was used when initializing a device. + * @errp: Error destination, used like first argument of error_setg() + * in case property setting fails later. If @errp is NULL, we + * print warnings instead of ignoring errors silently. */ typedef struct GlobalProperty { const char *driver; @@ -266,6 +269,7 @@ typedef struct GlobalProperty { const char *value; bool user_provided; bool used; + Error **errp; } GlobalProperty; /*** Board API. This should go away once we have a machine config file. ***/ From 39a3b377b89506ad15b8bc91fe2296f65b9f755a Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 15 Jun 2016 16:41:19 -0300 Subject: [PATCH 04/16] machine: Add machine_register_compat_props() function Move the compat_props handling to core machine code. Reviewed-by: Marcel Apfelbaum Reviewed-by: Markus Armbruster Signed-off-by: Eduardo Habkost --- hw/core/machine.c | 16 ++++++++++++++++ include/hw/boards.h | 1 + vl.c | 9 ++------- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 8f943013e5..052517d38d 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -560,6 +560,22 @@ static void machine_class_finalize(ObjectClass *klass, void *data) } } +void machine_register_compat_props(MachineState *machine) +{ + MachineClass *mc = MACHINE_GET_CLASS(machine); + int i; + GlobalProperty *p; + + if (!mc->compat_props) { + return; + } + + for (i = 0; i < mc->compat_props->len; i++) { + p = g_array_index(mc->compat_props, GlobalProperty *, i); + qdev_prop_register_global(p); + } +} + static const TypeInfo machine_info = { .name = TYPE_MACHINE, .parent = TYPE_OBJECT, diff --git a/include/hw/boards.h b/include/hw/boards.h index 3ed6155ee4..3e69eca038 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -40,6 +40,7 @@ int machine_kvm_shadow_mem(MachineState *machine); int machine_phandle_start(MachineState *machine); bool machine_dump_guest_core(MachineState *machine); bool machine_mem_merge(MachineState *machine); +void machine_register_compat_props(MachineState *machine); /** * CPUArchId: diff --git a/vl.c b/vl.c index ebdeaa0b9d..356713ea07 100644 --- a/vl.c +++ b/vl.c @@ -4448,13 +4448,8 @@ int main(int argc, char **argv, char **envp) exit (i == 1 ? 1 : 0); } - if (machine_class->compat_props) { - GlobalProperty *p; - for (i = 0; i < machine_class->compat_props->len; i++) { - p = g_array_index(machine_class->compat_props, GlobalProperty *, i); - qdev_prop_register_global(p); - } - } + machine_register_compat_props(current_machine); + qemu_opts_foreach(qemu_find_opts("global"), global_init_func, NULL, NULL); From adae837d40dea7100040136647e3de44898994df Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 15 Jun 2016 16:00:01 -0300 Subject: [PATCH 05/16] vl: Set errp to &error_abort on machine compat_props Use the new GlobalProperty.errp field to handle compat_props errors. Example output before this change: (with an intentionally broken entry added to PC_COMPAT_1_3 just for testing) $ qemu-system-x86_64 -machine pc-1.3 qemu-system-x86_64: hw/core/qdev-properties.c:1091: qdev_prop_set_globals_for_type: Assertion `prop->user_provided' failed. Aborted (core dumped) After: $ qemu-system-x86_64 -machine pc-1.3 Unexpected error in x86_cpuid_set_vendor() at /home/ehabkost/rh/proj/virt/qemu/target-i386/cpu.c:1688: qemu-system-x86_64: can't apply global cpu.vendor=x: Property '.vendor' doesn't take value 'x' Aborted (core dumped) Suggested-by: Paolo Bonzini Reviewed-by: Marcel Apfelbaum Reviewed-by: Markus Armbruster Signed-off-by: Eduardo Habkost --- hw/core/machine.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/core/machine.c b/hw/core/machine.c index 052517d38d..2fe6ff6f30 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -572,6 +572,8 @@ void machine_register_compat_props(MachineState *machine) for (i = 0; i < mc->compat_props->len; i++) { p = g_array_index(mc->compat_props, GlobalProperty *, i); + /* Machine compat_props must never cause errors: */ + p->errp = &error_abort; qdev_prop_register_global(p); } } From fb02d56e96d553088c5b4267a3c954a3e952a50a Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Thu, 9 Jun 2016 19:11:00 +0200 Subject: [PATCH 06/16] target-sparc: Use sparc_cpu_parse_features() directly Make SPARC target use sparc_cpu_parse_features() directly so it won't get in the way of switching other propertified targets to handling features as global properties. Signed-off-by: Igor Mammedov Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- target-sparc/cpu.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c index 5b74cfcd31..e4089f2074 100644 --- a/target-sparc/cpu.c +++ b/target-sparc/cpu.c @@ -101,9 +101,11 @@ static void cpu_sparc_disas_set_info(CPUState *cpu, disassemble_info *info) #endif } +static void sparc_cpu_parse_features(CPUState *cs, char *features, + Error **errp); + static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model) { - CPUClass *cc = CPU_GET_CLASS(cpu); CPUSPARCState *env = &cpu->env; char *s = g_strdup(cpu_model); char *featurestr, *name = strtok(s, ","); @@ -119,7 +121,7 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model) memcpy(env->def, def, sizeof(*def)); featurestr = strtok(NULL, ","); - cc->parse_features(CPU(cpu), featurestr, &err); + sparc_cpu_parse_features(CPU(cpu), featurestr, &err); g_free(s); if (err) { error_report_err(err); @@ -840,7 +842,6 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data) scc->parent_reset = cc->reset; cc->reset = sparc_cpu_reset; - cc->parse_features = sparc_cpu_parse_features; cc->has_work = sparc_cpu_has_work; cc->do_interrupt = sparc_cpu_do_interrupt; cc->cpu_exec_interrupt = sparc_cpu_exec_interrupt; From 7eb24386dbfb0b66464c7f856c1074c606efccda Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 17 Jun 2016 17:47:13 +0200 Subject: [PATCH 07/16] target-i386: TCG can support CPUID.07H:EBX.erms ERMS just says "rep movsb" and "rep stosb" are fast. It does not imply any new instruction, so we can support it easily. Signed-off-by: Paolo Bonzini Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3bd3cfc3ad..2227f2281c 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -358,10 +358,11 @@ static const char *cpuid_6_feature_name[] = { #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \ CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX | \ CPUID_7_0_EBX_PCOMMIT | CPUID_7_0_EBX_CLFLUSHOPT | \ - CPUID_7_0_EBX_CLWB | CPUID_7_0_EBX_MPX | CPUID_7_0_EBX_FSGSBASE) + CPUID_7_0_EBX_CLWB | CPUID_7_0_EBX_MPX | CPUID_7_0_EBX_FSGSBASE | \ + CPUID_7_0_EBX_ERMS) /* missing: CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2, - CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM, + CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM, CPUID_7_0_EBX_RDSEED */ #define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE) #define TCG_APM_FEATURES 0 From cf2887c9738451eb989c6c102af070dee2dc172a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 21 Jun 2016 14:04:40 +0200 Subject: [PATCH 08/16] target-i386: Avoid using locals outside their scope x86_cpu_parse_featurestr has a "val = num;" assignment just before num goes out of scope. Push num up to fix the issue. Signed-off-by: Paolo Bonzini Reviewed-by: Igor Mammedov Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- 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 2227f2281c..0b286e161c 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1975,6 +1975,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, const char *name; const char *val = NULL; char *eq = NULL; + char num[32]; /* Compatibility syntax: */ if (featurestr[0] == '+') { @@ -2000,7 +2001,6 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, if (!strcmp(name, "tsc-freq")) { int64_t tsc_freq; char *err; - char num[32]; tsc_freq = qemu_strtosz_suffix_unit(val, &err, QEMU_STRTOSZ_DEFSUFFIX_B, 1000); From 62a48a2a5798425997152dea3fc48708f9116c04 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Thu, 9 Jun 2016 19:11:01 +0200 Subject: [PATCH 09/16] cpu: Use CPUClass->parse_features() as convertor to global properties Currently CPUClass->parse_features() is used to parse -cpu features string and set properties on created CPU instances. But considering that features specified by -cpu apply to every created CPU instance, it doesn't make sense to parse the same features string for every CPU created. It also makes every target that cares about parsing features string explicitly call CPUClass->parse_features() parser, which gets in a way if we consider using generic device_add for CPU hotplug as device_add has not a clue about CPU specific hooks. Turns out we can use global properties mechanism to set properties on every created CPU instance for a given type. That way it's possible to convert CPU features into a set of global properties for CPU type specified by -cpu cpu_model and common Device.device_post_init() will apply them to CPU of given type automatically regardless whether it's manually created CPU or CPU created with help of device_add. Signed-off-by: Igor Mammedov Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- hw/arm/virt.c | 7 ++++--- include/qom/cpu.h | 2 +- qom/cpu.c | 41 +++++++++++++++++++++++++++++------------ target-i386/cpu.c | 26 ++++++++++++++++++++------ 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 6e098afd1f..ae90ca5771 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1261,6 +1261,7 @@ static void machvirt_init(MachineState *machine) for (n = 0; n < smp_cpus; n++) { ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]); + const char *typename = object_class_get_name(oc); CPUClass *cc = CPU_CLASS(oc); Object *cpuobj; Error *err = NULL; @@ -1270,10 +1271,10 @@ static void machvirt_init(MachineState *machine) error_report("Unable to find CPU definition"); exit(1); } - cpuobj = object_new(object_class_get_name(oc)); + /* convert -smp CPU options specified by the user into global props */ + cc->parse_features(typename, cpuopts, &err); + cpuobj = object_new(typename); - /* Handle any CPU options specified by the user */ - cc->parse_features(CPU(cpuobj), cpuopts, &err); g_free(cpuopts); if (err) { error_report_err(err); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 32f3af3e1c..cacb100f7b 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -134,7 +134,7 @@ typedef struct CPUClass { /*< public >*/ ObjectClass *(*class_by_name)(const char *cpu_model); - void (*parse_features)(CPUState *cpu, char *str, Error **errp); + void (*parse_features)(const char *typename, char *str, Error **errp); void (*reset)(CPUState *cpu); int reset_dump_flags; diff --git a/qom/cpu.c b/qom/cpu.c index 751e992de8..2a0d9fe10f 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -28,6 +28,7 @@ #include "exec/log.h" #include "qemu/error-report.h" #include "sysemu/sysemu.h" +#include "hw/qdev-properties.h" bool cpu_exists(int64_t id) { @@ -46,7 +47,7 @@ bool cpu_exists(int64_t id) CPUState *cpu_generic_init(const char *typename, const char *cpu_model) { char *str, *name, *featurestr; - CPUState *cpu; + CPUState *cpu = NULL; ObjectClass *oc; CPUClass *cc; Error *err = NULL; @@ -60,16 +61,18 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model) return NULL; } - cpu = CPU(object_new(object_class_get_name(oc))); - cc = CPU_GET_CLASS(cpu); - + cc = CPU_CLASS(oc); featurestr = strtok(NULL, ","); - cc->parse_features(cpu, featurestr, &err); + /* TODO: all callers of cpu_generic_init() need to be converted to + * call parse_features() only once, before calling cpu_generic_init(). + */ + cc->parse_features(object_class_get_name(oc), featurestr, &err); g_free(str); if (err != NULL) { goto out; } + cpu = CPU(object_new(object_class_get_name(oc))); object_property_set_bool(OBJECT(cpu), true, "realized", &err); out: @@ -282,25 +285,39 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model) return NULL; } -static void cpu_common_parse_features(CPUState *cpu, char *features, +static void cpu_common_parse_features(const char *typename, char *features, Error **errp) { char *featurestr; /* Single "key=value" string being parsed */ char *val; - Error *err = NULL; + static bool cpu_globals_initialized; + + /* TODO: all callers of ->parse_features() need to be changed to + * call it only once, so we can remove this check (or change it + * to assert(!cpu_globals_initialized). + * Current callers of ->parse_features() are: + * - machvirt_init() + * - cpu_generic_init() + * - cpu_x86_create() + */ + if (cpu_globals_initialized) { + return; + } + cpu_globals_initialized = true; featurestr = features ? strtok(features, ",") : NULL; while (featurestr) { val = strchr(featurestr, '='); if (val) { + GlobalProperty *prop = g_new0(typeof(*prop), 1); *val = 0; val++; - object_property_parse(OBJECT(cpu), val, featurestr, &err); - if (err) { - error_propagate(errp, err); - return; - } + prop->driver = typename; + prop->property = g_strdup(featurestr); + prop->value = g_strdup(val); + prop->errp = &error_fatal; + qdev_prop_register_global(prop); } else { error_setg(errp, "Expected key=value format, found %s.", featurestr); diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 0b286e161c..16977d5404 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1958,12 +1958,17 @@ static FeatureWordArray minus_features = { 0 }; /* Parse "+feature,-feature,feature=foo" CPU feature string */ -static void x86_cpu_parse_featurestr(CPUState *cs, char *features, +static void x86_cpu_parse_featurestr(const char *typename, char *features, Error **errp) { - X86CPU *cpu = X86_CPU(cs); char *featurestr; /* Single 'key=value" string being parsed */ Error *local_err = NULL; + static bool cpu_globals_initialized; + + if (cpu_globals_initialized) { + return; + } + cpu_globals_initialized = true; if (!features) { return; @@ -1976,6 +1981,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, const char *val = NULL; char *eq = NULL; char num[32]; + GlobalProperty *prop; /* Compatibility syntax: */ if (featurestr[0] == '+') { @@ -2013,7 +2019,12 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, name = "tsc-frequency"; } - object_property_parse(OBJECT(cpu), val, name, &local_err); + prop = g_new0(typeof(*prop), 1); + prop->driver = typename; + prop->property = g_strdup(name); + prop->value = g_strdup(val); + prop->errp = &error_fatal; + qdev_prop_register_global(prop); } if (local_err) { @@ -2202,9 +2213,11 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) { X86CPU *cpu = NULL; ObjectClass *oc; + CPUClass *cc; gchar **model_pieces; char *name, *features; Error *error = NULL; + const char *typename; model_pieces = g_strsplit(cpu_model, ",", 2); if (!model_pieces[0]) { @@ -2219,10 +2232,11 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) error_setg(&error, "Unable to find CPU definition: %s", name); goto out; } + cc = CPU_CLASS(oc); + typename = object_class_get_name(oc); - cpu = X86_CPU(object_new(object_class_get_name(oc))); - - x86_cpu_parse_featurestr(CPU(cpu), features, &error); + cc->parse_features(typename, features, &error); + cpu = X86_CPU(object_new(typename)); if (error) { goto out; } From 09f71b054a95161950a03fafc9023637929bd404 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Thu, 9 Jun 2016 19:11:02 +0200 Subject: [PATCH 10/16] arm: virt: Parse cpu_model only once Considering that features are converted to global properties and global properties are automatically applied to every new instance of created CPU (at object_new() time), there is no point in parsing cpu_model string every time a CPU created. So move parsing outside CPU creation loop and do it only once. Parsing also should be done before any CPU is created so that features would affect the first CPU a well. Signed-off-by: Igor Mammedov Reviewed-by: Peter Maydell Signed-off-by: Eduardo Habkost --- hw/arm/virt.c | 42 +++++++++++++++++++++--------------------- qom/cpu.c | 1 - 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index ae90ca5771..4dafd42be8 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1176,6 +1176,10 @@ static void machvirt_init(MachineState *machine) VirtGuestInfoState *guest_info_state = g_malloc0(sizeof *guest_info_state); VirtGuestInfo *guest_info = &guest_info_state->info; char **cpustr; + ObjectClass *oc; + const char *typename; + CPUClass *cc; + Error *err = NULL; bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); if (!cpu_model) { @@ -1259,27 +1263,24 @@ static void machvirt_init(MachineState *machine) create_fdt(vbi); + oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]); + if (!oc) { + error_report("Unable to find CPU definition"); + exit(1); + } + typename = object_class_get_name(oc); + + /* convert -smp CPU options specified by the user into global props */ + cc = CPU_CLASS(oc); + cc->parse_features(typename, cpustr[1], &err); + g_strfreev(cpustr); + if (err) { + error_report_err(err); + exit(1); + } + for (n = 0; n < smp_cpus; n++) { - ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]); - const char *typename = object_class_get_name(oc); - CPUClass *cc = CPU_CLASS(oc); - Object *cpuobj; - Error *err = NULL; - char *cpuopts = g_strdup(cpustr[1]); - - if (!oc) { - error_report("Unable to find CPU definition"); - exit(1); - } - /* convert -smp CPU options specified by the user into global props */ - cc->parse_features(typename, cpuopts, &err); - cpuobj = object_new(typename); - - g_free(cpuopts); - if (err) { - error_report_err(err); - exit(1); - } + Object *cpuobj = object_new(typename); if (!vms->secure) { object_property_set_bool(cpuobj, false, "has_el3", NULL); @@ -1310,7 +1311,6 @@ static void machvirt_init(MachineState *machine) object_property_set_bool(cpuobj, true, "realized", NULL); } - g_strfreev(cpustr); fdt_add_timer_nodes(vbi, gic_version); fdt_add_cpu_nodes(vbi); fdt_add_psci_node(vbi); diff --git a/qom/cpu.c b/qom/cpu.c index 2a0d9fe10f..f884a666a1 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -296,7 +296,6 @@ static void cpu_common_parse_features(const char *typename, char *features, * call it only once, so we can remove this check (or change it * to assert(!cpu_globals_initialized). * Current callers of ->parse_features() are: - * - machvirt_init() * - cpu_generic_init() * - cpu_x86_create() */ From 6aff24c6a61c6fec31e555c7748ba6085b7b2c06 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Thu, 9 Jun 2016 19:11:03 +0200 Subject: [PATCH 11/16] pc: Parse CPU features only once Considering that features are converted to global properties and global properties are automatically applied to every new instance of created CPU (at object_new() time), there is no point in parsing cpu_model string every time a CPU created. So move parsing outside CPU creation loop and do it only once. Parsing also should be done before any CPU is created so that features would affect the first CPU a well. Signed-off-by: Igor Mammedov Signed-off-by: Eduardo Habkost --- hw/i386/pc.c | 37 ++++++++++++++++++++++++++++--------- qom/cpu.c | 1 - target-i386/cpu.c | 44 -------------------------------------------- target-i386/cpu.h | 1 - 4 files changed, 28 insertions(+), 55 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index cd1745ebaf..d0df3c1013 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1039,21 +1039,17 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) } } -static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, +static X86CPU *pc_new_cpu(const char *typename, int64_t apic_id, Error **errp) { X86CPU *cpu = NULL; Error *local_err = NULL; - cpu = cpu_x86_create(cpu_model, &local_err); - if (local_err != NULL) { - goto out; - } + cpu = X86_CPU(object_new(typename)); object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err); object_property_set_bool(OBJECT(cpu), true, "realized", &local_err); -out: if (local_err) { error_propagate(errp, local_err); object_unref(OBJECT(cpu)); @@ -1065,7 +1061,8 @@ out: void pc_hot_add_cpu(const int64_t id, Error **errp) { X86CPU *cpu; - MachineState *machine = MACHINE(qdev_get_machine()); + ObjectClass *oc; + PCMachineState *pcms = PC_MACHINE(qdev_get_machine()); int64_t apic_id = x86_cpu_apic_id_from_index(id); Error *local_err = NULL; @@ -1093,7 +1090,9 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) return; } - cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err); + assert(pcms->possible_cpus->cpus[0].cpu); /* BSP is always present */ + oc = OBJECT_CLASS(CPU_GET_CLASS(pcms->possible_cpus->cpus[0].cpu)); + cpu = pc_new_cpu(object_class_get_name(oc), apic_id, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -1104,6 +1103,10 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) void pc_cpus_init(PCMachineState *pcms) { int i; + CPUClass *cc; + ObjectClass *oc; + const char *typename; + gchar **model_pieces; X86CPU *cpu = NULL; MachineState *machine = MACHINE(pcms); @@ -1116,6 +1119,22 @@ void pc_cpus_init(PCMachineState *pcms) #endif } + model_pieces = g_strsplit(machine->cpu_model, ",", 2); + if (!model_pieces[0]) { + error_report("Invalid/empty CPU model name"); + exit(1); + } + + oc = cpu_class_by_name(TYPE_X86_CPU, model_pieces[0]); + if (oc == NULL) { + error_report("Unable to find CPU definition: %s", model_pieces[0]); + exit(1); + } + typename = object_class_get_name(oc); + cc = CPU_CLASS(oc); + cc->parse_features(typename, model_pieces[1], &error_fatal); + g_strfreev(model_pieces); + /* Calculates the limit to CPU APIC ID values * * Limit for the APIC ID value, so that all @@ -1136,7 +1155,7 @@ void pc_cpus_init(PCMachineState *pcms) pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i); pcms->possible_cpus->len++; if (i < smp_cpus) { - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), + cpu = pc_new_cpu(typename, x86_cpu_apic_id_from_index(i), &error_fatal); pcms->possible_cpus->cpus[i].cpu = CPU(cpu); object_unref(OBJECT(cpu)); diff --git a/qom/cpu.c b/qom/cpu.c index f884a666a1..a9727a1e64 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -297,7 +297,6 @@ static void cpu_common_parse_features(const char *typename, char *features, * to assert(!cpu_globals_initialized). * Current callers of ->parse_features() are: * - cpu_generic_init() - * - cpu_x86_create() */ if (cpu_globals_initialized) { return; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 16977d5404..80e1767fe0 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2209,50 +2209,6 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp) } -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) -{ - X86CPU *cpu = NULL; - ObjectClass *oc; - CPUClass *cc; - gchar **model_pieces; - char *name, *features; - Error *error = NULL; - const char *typename; - - model_pieces = g_strsplit(cpu_model, ",", 2); - if (!model_pieces[0]) { - error_setg(&error, "Invalid/empty CPU model name"); - goto out; - } - name = model_pieces[0]; - features = model_pieces[1]; - - oc = x86_cpu_class_by_name(name); - if (oc == NULL) { - error_setg(&error, "Unable to find CPU definition: %s", name); - goto out; - } - cc = CPU_CLASS(oc); - typename = object_class_get_name(oc); - - cc->parse_features(typename, features, &error); - cpu = X86_CPU(object_new(typename)); - if (error) { - goto out; - } - -out: - if (error != NULL) { - error_propagate(errp, error); - if (cpu) { - object_unref(OBJECT(cpu)); - cpu = NULL; - } - } - g_strfreev(model_pieces); - return cpu; -} - X86CPU *cpu_x86_init(const char *cpu_model) { return X86_CPU(cpu_generic_init(TYPE_X86_CPU, cpu_model)); diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 474b0b937d..738958e0d5 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1234,7 +1234,6 @@ void x86_cpu_exec_enter(CPUState *cpu); void x86_cpu_exec_exit(CPUState *cpu); X86CPU *cpu_x86_init(const char *cpu_model); -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp); void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf); int cpu_x86_support_mca_broadcast(CPUX86State *env); From d6276d26bd92c683c7ca194b6f6f48fd8994c1ed Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 30 Jun 2016 15:12:17 -0300 Subject: [PATCH 12/16] target-i386: Show host and VM TSC frequencies on mismatch Improve the TSC frequency mismatch warning to show the host and VM TSC frequencies. Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Marcelo Tosatti Signed-off-by: Eduardo Habkost --- target-i386/kvm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index f3698f19b5..9679415e43 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -566,7 +566,9 @@ static int kvm_arch_set_tsc_khz(CPUState *cs) -ENOTSUP; if (cur_freq <= 0 || cur_freq != env->tsc_khz) { error_report("warning: TSC frequency mismatch between " - "VM and host, and TSC scaling unavailable"); + "VM (%" PRId64 " kHz) and host (%d kHz), " + "and TSC scaling unavailable", + env->tsc_khz, cur_freq); return r; } } From c35bd19a5c9140bce8b913cc5cefe6f071135bdb Mon Sep 17 00:00:00 2001 From: Evgeny Yakovlev Date: Fri, 24 Jun 2016 13:49:36 +0300 Subject: [PATCH 13/16] target-i386: Report hyperv feature words through qom This change adds hyperv feature words report through qom rpc. When VM is configured with hyperv features enabled libvirt will check that required feature words are set in cpuid leaf 40000003 through qom request. Currently qemu does not report hyperv feature words which prevents windows guests from starting with libvirt. To avoid conflicting with current hyperv properties all added feature words cannot be set directly with -cpu +feature yet. Signed-off-by: Evgeny Yakovlev Signed-off-by: Denis V. Lunev CC: Paolo Bonzini CC: Richard Henderson CC: Eduardo Habkost CC: Marcelo Tosatti Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 53 ++++++++++++++++++++++ target-i386/cpu.h | 3 ++ target-i386/kvm.c | 112 ++++++++++++++++++++++++++-------------------- 3 files changed, 120 insertions(+), 48 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 80e1767fe0..4e179b9b95 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -245,6 +245,47 @@ static const char *kvm_feature_name[] = { NULL, NULL, NULL, NULL, }; +static const char *hyperv_priv_feature_name[] = { + NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */, + NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */, + NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */, + NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */, + NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */, + NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, +}; + +static const char *hyperv_ident_feature_name[] = { + NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */, + NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */, + NULL /* hv_post_messages */, NULL /* hv_signal_events */, + NULL /* hv_create_port */, NULL /* hv_connect_port */, + NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */, + NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */, + NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, +}; + +static const char *hyperv_misc_feature_name[] = { + NULL /* hv_mwait */, NULL /* hv_guest_debugging */, + NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */, + NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */, + NULL, NULL, + NULL, NULL, NULL /* hv_guest_crash_msr */, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, +}; + static const char *svm_feature_name[] = { "npt", "lbrv", "svm_lock", "nrip_save", "tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists", @@ -412,6 +453,18 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX, .tcg_features = TCG_KVM_FEATURES, }, + [FEAT_HYPERV_EAX] = { + .feat_names = hyperv_priv_feature_name, + .cpuid_eax = 0x40000003, .cpuid_reg = R_EAX, + }, + [FEAT_HYPERV_EBX] = { + .feat_names = hyperv_ident_feature_name, + .cpuid_eax = 0x40000003, .cpuid_reg = R_EBX, + }, + [FEAT_HYPERV_EDX] = { + .feat_names = hyperv_misc_feature_name, + .cpuid_eax = 0x40000003, .cpuid_reg = R_EDX, + }, [FEAT_SVM] = { .feat_names = svm_feature_name, .cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX, diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 738958e0d5..3ae3e14f78 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -440,6 +440,9 @@ typedef enum FeatureWord { FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */ FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */ FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */ + FEAT_HYPERV_EAX, /* CPUID[4000_0003].EAX */ + FEAT_HYPERV_EBX, /* CPUID[4000_0003].EBX */ + FEAT_HYPERV_EDX, /* CPUID[4000_0003].EDX */ FEAT_SVM, /* CPUID[8000_000A].EDX */ FEAT_XSAVE, /* CPUID[EAX=0xd,ECX=1].EAX */ FEAT_6_EAX, /* CPUID[6].EAX */ diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 9679415e43..4193fe1746 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -576,6 +576,64 @@ static int kvm_arch_set_tsc_khz(CPUState *cs) return 0; } +static int hyperv_handle_properties(CPUState *cs) +{ + X86CPU *cpu = X86_CPU(cs); + CPUX86State *env = &cpu->env; + + if (cpu->hyperv_relaxed_timing) { + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE; + } + if (cpu->hyperv_vapic) { + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE; + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_APIC_ACCESS_AVAILABLE; + has_msr_hv_vapic = true; + } + if (cpu->hyperv_time && + kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) { + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE; + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE; + env->features[FEAT_HYPERV_EAX] |= 0x200; + has_msr_hv_tsc = true; + } + if (cpu->hyperv_crash && has_msr_hv_crash) { + env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE; + } + env->features[FEAT_HYPERV_EDX] |= HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE; + if (cpu->hyperv_reset && has_msr_hv_reset) { + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_RESET_AVAILABLE; + } + if (cpu->hyperv_vpindex && has_msr_hv_vpindex) { + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_VP_INDEX_AVAILABLE; + } + if (cpu->hyperv_runtime && has_msr_hv_runtime) { + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_VP_RUNTIME_AVAILABLE; + } + if (cpu->hyperv_synic) { + int sint; + + if (!has_msr_hv_synic || + kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_SYNIC, 0)) { + fprintf(stderr, "Hyper-V SynIC is not supported by kernel\n"); + return -ENOSYS; + } + + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_SYNIC_AVAILABLE; + env->msr_hv_synic_version = HV_SYNIC_VERSION_1; + for (sint = 0; sint < ARRAY_SIZE(env->msr_hv_synic_sint); sint++) { + env->msr_hv_synic_sint[sint] = HV_SYNIC_SINT_MASKED; + } + } + if (cpu->hyperv_stimer) { + if (!has_msr_hv_stimer) { + fprintf(stderr, "Hyper-V timers aren't supported by kernel\n"); + return -ENOSYS; + } + env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_SYNTIMER_AVAILABLE; + } + return 0; +} + static Error *invtsc_mig_blocker; #define KVM_MAX_CPUID_ENTRIES 100 @@ -635,56 +693,14 @@ int kvm_arch_init_vcpu(CPUState *cs) c = &cpuid_data.entries[cpuid_i++]; c->function = HYPERV_CPUID_FEATURES; - if (cpu->hyperv_relaxed_timing) { - c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE; + r = hyperv_handle_properties(cs); + if (r) { + return r; } - if (cpu->hyperv_vapic) { - c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE; - c->eax |= HV_X64_MSR_APIC_ACCESS_AVAILABLE; - has_msr_hv_vapic = true; - } - if (cpu->hyperv_time && - kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) { - c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE; - c->eax |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE; - c->eax |= 0x200; - has_msr_hv_tsc = true; - } - if (cpu->hyperv_crash && has_msr_hv_crash) { - c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE; - } - c->edx |= HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE; - if (cpu->hyperv_reset && has_msr_hv_reset) { - c->eax |= HV_X64_MSR_RESET_AVAILABLE; - } - if (cpu->hyperv_vpindex && has_msr_hv_vpindex) { - c->eax |= HV_X64_MSR_VP_INDEX_AVAILABLE; - } - if (cpu->hyperv_runtime && has_msr_hv_runtime) { - c->eax |= HV_X64_MSR_VP_RUNTIME_AVAILABLE; - } - if (cpu->hyperv_synic) { - int sint; + c->eax = env->features[FEAT_HYPERV_EAX]; + c->ebx = env->features[FEAT_HYPERV_EBX]; + c->edx = env->features[FEAT_HYPERV_EDX]; - if (!has_msr_hv_synic || - kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_SYNIC, 0)) { - fprintf(stderr, "Hyper-V SynIC is not supported by kernel\n"); - return -ENOSYS; - } - - c->eax |= HV_X64_MSR_SYNIC_AVAILABLE; - env->msr_hv_synic_version = HV_SYNIC_VERSION_1; - for (sint = 0; sint < ARRAY_SIZE(env->msr_hv_synic_sint); sint++) { - env->msr_hv_synic_sint[sint] = HV_SYNIC_SINT_MASKED; - } - } - if (cpu->hyperv_stimer) { - if (!has_msr_hv_stimer) { - fprintf(stderr, "Hyper-V timers aren't supported by kernel\n"); - return -ENOSYS; - } - c->eax |= HV_X64_MSR_SYNTIMER_AVAILABLE; - } c = &cpuid_data.entries[cpuid_i++]; c->function = HYPERV_CPUID_ENLIGHTMENT_INFO; if (cpu->hyperv_relaxed_timing) { From 87f8b626041ceaea9adcfdbd549359f0ca7b871d Mon Sep 17 00:00:00 2001 From: Ashok Raj Date: Wed, 22 Jun 2016 14:56:21 +0800 Subject: [PATCH 14/16] target-i386: kvm: Add basic Intel LMCE support This patch adds the support to inject SRAR and SRAO as LMCE, i.e. they are injected to only one VCPU rather than broadcast to all VCPUs. As KVM reports LMCE support on Intel platforms, this features is only available on Intel platforms. LMCE is disabled by default and can be enabled/disabled by cpu option 'lmce=on/off'. Signed-off-by: Ashok Raj [Haozhong: Enable LMCE only on Intel platforms Disable LMCE by default and add a cpu option 'lmce' Handle the error if LMCE is enabled w/o host support Remove MCG_LMCE_P from MCE_CAP_DEF Add migration support for LMCE Minor code style changes] Signed-off-by: Haozhong Zhang Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 4 +++- target-i386/cpu.h | 12 ++++++++++++ target-i386/kvm.c | 36 +++++++++++++++++++++++++++++++++--- target-i386/machine.c | 19 +++++++++++++++++++ 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 4e179b9b95..5bc7ce5d37 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2839,7 +2839,8 @@ static void mce_init(X86CPU *cpu) if (((cenv->cpuid_version >> 8) & 0xf) >= 6 && (cenv->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) == (CPUID_MCE | CPUID_MCA)) { - cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF; + cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF | + (cpu->enable_lmce ? MCG_LMCE_P : 0); cenv->mcg_ctl = ~(uint64_t)0; for (bank = 0; bank < MCE_BANKS_DEF; bank++) { cenv->mce_banks[bank * 4] = ~(uint64_t)0; @@ -3286,6 +3287,7 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0), DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id), DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true), + DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false), DEFINE_PROP_END_OF_LIST() }; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 3ae3e14f78..54c3fd6561 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -292,6 +292,7 @@ #define MCG_CTL_P (1ULL<<8) /* MCG_CAP register available */ #define MCG_SER_P (1ULL<<24) /* MCA recovery/new status bits */ +#define MCG_LMCE_P (1ULL<<27) /* Local Machine Check Supported */ #define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P) #define MCE_BANKS_DEF 10 @@ -301,6 +302,9 @@ #define MCG_STATUS_RIPV (1ULL<<0) /* restart ip valid */ #define MCG_STATUS_EIPV (1ULL<<1) /* ip points to correct instruction */ #define MCG_STATUS_MCIP (1ULL<<2) /* machine check in progress */ +#define MCG_STATUS_LMCE (1ULL<<3) /* Local MCE signaled */ + +#define MCG_EXT_CTL_LMCE_EN (1ULL<<0) /* Local MCE enabled */ #define MCI_STATUS_VAL (1ULL<<63) /* valid error */ #define MCI_STATUS_OVER (1ULL<<62) /* previous errors lost */ @@ -343,6 +347,7 @@ #define MSR_MCG_CAP 0x179 #define MSR_MCG_STATUS 0x17a #define MSR_MCG_CTL 0x17b +#define MSR_MCG_EXT_CTL 0x4d0 #define MSR_P6_EVNTSEL0 0x186 @@ -1114,6 +1119,7 @@ typedef struct CPUX86State { uint64_t mcg_cap; uint64_t mcg_ctl; + uint64_t mcg_ext_ctl; uint64_t mce_banks[MCE_BANKS_DEF*4]; uint64_t tsc_aux; @@ -1181,6 +1187,12 @@ struct X86CPU { */ bool enable_pmu; + /* LMCE support can be enabled/disabled via cpu option 'lmce=on/off'. It is + * disabled by default to avoid breaking migration between QEMU with + * different LMCE configurations. + */ + bool enable_lmce; + /* Compatibility bits for old machine types: */ bool enable_cpuid_0xb; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 4193fe1746..93275231ec 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -106,6 +106,8 @@ static int has_xsave; static int has_xcrs; static int has_pit_state2; +static bool has_msr_mcg_ext_ctl; + static struct kvm_cpuid2 *cpuid_cache; int kvm_has_pit_state2(void) @@ -382,10 +384,12 @@ static int kvm_get_mce_cap_supported(KVMState *s, uint64_t *mce_cap, static void kvm_mce_inject(X86CPU *cpu, hwaddr paddr, int code) { + CPUState *cs = CPU(cpu); CPUX86State *env = &cpu->env; uint64_t status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S; uint64_t mcg_status = MCG_STATUS_MCIP; + int flags = 0; if (code == BUS_MCEERR_AR) { status |= MCI_STATUS_AR | 0x134; @@ -394,10 +398,19 @@ static void kvm_mce_inject(X86CPU *cpu, hwaddr paddr, int code) status |= 0xc0; mcg_status |= MCG_STATUS_RIPV; } + + flags = cpu_x86_support_mca_broadcast(env) ? MCE_INJECT_BROADCAST : 0; + /* We need to read back the value of MSR_EXT_MCG_CTL that was set by the + * guest kernel back into env->mcg_ext_ctl. + */ + cpu_synchronize_state(cs); + if (env->mcg_ext_ctl & MCG_EXT_CTL_LMCE_EN) { + mcg_status |= MCG_STATUS_LMCE; + flags = 0; + } + cpu_x86_inject_mce(NULL, cpu, 9, status, mcg_status, paddr, - (MCM_ADDR_PHYS << 6) | 0xc, - cpu_x86_support_mca_broadcast(env) ? - MCE_INJECT_BROADCAST : 0); + (MCM_ADDR_PHYS << 6) | 0xc, flags); } static void hardware_memory_error(void) @@ -883,6 +896,10 @@ int kvm_arch_init_vcpu(CPUState *cs) unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK); if (unsupported_caps) { + if (unsupported_caps & MCG_LMCE_P) { + error_report("kvm: LMCE not supported"); + return -ENOTSUP; + } error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64, unsupported_caps); } @@ -903,6 +920,10 @@ int kvm_arch_init_vcpu(CPUState *cs) !!(c->ecx & CPUID_EXT_SMX); } + if (env->mcg_cap & MCG_LMCE_P) { + has_msr_mcg_ext_ctl = has_msr_feature_control = true; + } + c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0); if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) { /* for migration */ @@ -1723,6 +1744,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level) kvm_msr_entry_add(cpu, MSR_MCG_STATUS, env->mcg_status); kvm_msr_entry_add(cpu, MSR_MCG_CTL, env->mcg_ctl); + if (has_msr_mcg_ext_ctl) { + kvm_msr_entry_add(cpu, MSR_MCG_EXT_CTL, env->mcg_ext_ctl); + } for (i = 0; i < (env->mcg_cap & 0xff) * 4; i++) { kvm_msr_entry_add(cpu, MSR_MC0_CTL + i, env->mce_banks[i]); } @@ -2026,6 +2050,9 @@ static int kvm_get_msrs(X86CPU *cpu) if (env->mcg_cap) { kvm_msr_entry_add(cpu, MSR_MCG_STATUS, 0); kvm_msr_entry_add(cpu, MSR_MCG_CTL, 0); + if (has_msr_mcg_ext_ctl) { + kvm_msr_entry_add(cpu, MSR_MCG_EXT_CTL, 0); + } for (i = 0; i < (env->mcg_cap & 0xff) * 4; i++) { kvm_msr_entry_add(cpu, MSR_MC0_CTL + i, 0); } @@ -2154,6 +2181,9 @@ static int kvm_get_msrs(X86CPU *cpu) case MSR_MCG_CTL: env->mcg_ctl = msrs[i].data; break; + case MSR_MCG_EXT_CTL: + env->mcg_ext_ctl = msrs[i].data; + break; case MSR_IA32_MISC_ENABLE: env->msr_ia32_misc_enable = msrs[i].data; break; diff --git a/target-i386/machine.c b/target-i386/machine.c index cb9adf2b02..71c0e4dc47 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -896,6 +896,24 @@ static const VMStateDescription vmstate_tsc_khz = { } }; +static bool mcg_ext_ctl_needed(void *opaque) +{ + X86CPU *cpu = opaque; + CPUX86State *env = &cpu->env; + return cpu->enable_lmce && env->mcg_ext_ctl; +} + +static const VMStateDescription vmstate_mcg_ext_ctl = { + .name = "cpu/mcg_ext_ctl", + .version_id = 1, + .minimum_version_id = 1, + .needed = mcg_ext_ctl_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT64(env.mcg_ext_ctl, X86CPU), + VMSTATE_END_OF_LIST() + } +}; + VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, @@ -1022,6 +1040,7 @@ VMStateDescription vmstate_x86_cpu = { #ifdef TARGET_X86_64 &vmstate_pkru, #endif + &vmstate_mcg_ext_ctl, NULL } }; From 217f1b4a72153cf8d556e9d45919e9222c38d25e Mon Sep 17 00:00:00 2001 From: Haozhong Zhang Date: Thu, 23 Jun 2016 14:15:43 +0800 Subject: [PATCH 15/16] target-i386: Publish advised value of MSR_IA32_FEATURE_CONTROL via fw_cfg It's a prerequisite that certain bits of MSR_IA32_FEATURE_CONTROL should be set before some features (e.g. VMX and LMCE) can be used, which is usually done by the firmware. This patch adds a fw_cfg file "etc/msr_feature_control" which contains the advised value of MSR_IA32_FEATURE_CONTROL and can be used by guest firmware (e.g. SeaBIOS). Suggested-by: Paolo Bonzini Signed-off-by: Haozhong Zhang Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- hw/i386/pc.c | 29 +++++++++++++++++++++++++++++ target-i386/cpu.h | 4 ++++ 2 files changed, 33 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d0df3c1013..f56e225a99 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1166,6 +1166,34 @@ void pc_cpus_init(PCMachineState *pcms) smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]); } +static void pc_build_feature_control_file(PCMachineState *pcms) +{ + X86CPU *cpu = X86_CPU(pcms->possible_cpus->cpus[0].cpu); + CPUX86State *env = &cpu->env; + uint32_t unused, ecx, edx; + uint64_t feature_control_bits = 0; + uint64_t *val; + + cpu_x86_cpuid(env, 1, 0, &unused, &unused, &ecx, &edx); + if (ecx & CPUID_EXT_VMX) { + feature_control_bits |= FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX; + } + + if ((edx & (CPUID_EXT2_MCE | CPUID_EXT2_MCA)) == + (CPUID_EXT2_MCE | CPUID_EXT2_MCA) && + (env->mcg_cap & MCG_LMCE_P)) { + feature_control_bits |= FEATURE_CONTROL_LMCE; + } + + if (!feature_control_bits) { + return; + } + + val = g_malloc(sizeof(*val)); + *val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED); + fw_cfg_add_file(pcms->fw_cfg, "etc/msr_feature_control", val, sizeof(*val)); +} + static void pc_machine_done(Notifier *notifier, void *data) { @@ -1193,6 +1221,7 @@ void pc_machine_done(Notifier *notifier, void *data) acpi_setup(); if (pcms->fw_cfg) { pc_build_smbios(pcms->fw_cfg); + pc_build_feature_control_file(pcms); } } diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 54c3fd6561..5c7a2791f3 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -332,6 +332,10 @@ #define MSR_TSC_ADJUST 0x0000003b #define MSR_IA32_TSCDEADLINE 0x6e0 +#define FEATURE_CONTROL_LOCKED (1<<0) +#define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX (1<<2) +#define FEATURE_CONTROL_LMCE (1<<20) + #define MSR_P6_PERFCTR0 0xc1 #define MSR_IA32_SMBASE 0x9e From 40bfe48f1c78bb7905d58d8d603ca27063566bc9 Mon Sep 17 00:00:00 2001 From: Haozhong Zhang Date: Wed, 22 Jun 2016 14:56:23 +0800 Subject: [PATCH 16/16] target-i386: Enable LMCE for '-cpu host' if supported by host If -cpu host is used, LMCE will be automatically enabled when it's supported by host. Signed-off-by: Haozhong Zhang Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 5bc7ce5d37..fc209ee1cb 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1547,6 +1547,17 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, #ifdef CONFIG_KVM +static bool lmce_supported(void) +{ + uint64_t mce_cap; + + if (kvm_ioctl(kvm_state, KVM_X86_GET_MCE_CAP_SUPPORTED, &mce_cap) < 0) { + return false; + } + + return !!(mce_cap & MCG_LMCE_P); +} + static int cpu_x86_fill_model_id(char *str) { uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; @@ -1619,6 +1630,10 @@ static void host_x86_cpu_initfn(Object *obj) env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX); env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); + + if (lmce_supported()) { + object_property_set_bool(OBJECT(cpu), true, "lmce", &error_abort); + } } object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);