[libvirt] [PATCH v6 0/3] Add runnability info to query-cpu-definitions
This series extends query-cpu-definitions to include an extra field: "unavailable-features". The new field can be used to find out reasons that prevent the CPU model from running in the current host. This will return information based on the current machine and accelerator only. In the future we may extend these mechanisms to allow querying other machines and other accelerators without restarting QEMU, but it will require some reorganization of QEMU's main code. To be able to implement this more cleanly, the series rework some of the feature/property name code. This series can be seen in the git branch at: https://github.com/ehabkost/qemu-hacks.git work/query-cpu-definitions-runnable-info The series is based on my x86-next branch: https://github.com/ehabkost/qemu.git x86-next Changes v5 -> v6: * Rebased to x86-next, that already has 8 of the previous patches from v5 applied * Removed x86_cpu_filter_features() from x86_cpu_load_features(), because some of the commands in the CPU model query API need info about CPU models before filtering * Recovered v3 of "target-i386: Move warning code outside x86_cpu_filter_features()" because now we can keep the simpler logic that checked the return value of x86_cpu_filter_features() Diff v5 -> v6: diff --git a/target-i386/cpu.c b/target-i386/cpu.c index d53e711..63330ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2052,6 +2052,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, } static void x86_cpu_load_features(X86CPU *cpu, Error **errp); +static int x86_cpu_filter_features(X86CPU *cpu); /* Check for missing features that may prevent the CPU class from * running using the current machine and accelerator. @@ -2085,6 +2086,8 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, next = &new->next; } + x86_cpu_filter_features(xc); + for (w = 0; w < FEATURE_WORDS; w++) { uint32_t filtered = xc->filtered_features[w]; int i; @@ -2234,11 +2237,14 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, /* * Filters CPU feature words based on host availability of each feature. + * + * Returns: 0 if all flags are supported by the host, non-zero otherwise. */ -static void x86_cpu_filter_features(X86CPU *cpu) +static int x86_cpu_filter_features(X86CPU *cpu) { CPUX86State *env = &cpu->env; FeatureWord w; + int rv = 0; for (w = 0; w < FEATURE_WORDS; w++) { uint32_t host_feat = @@ -2246,22 +2252,21 @@ static void x86_cpu_filter_features(X86CPU *cpu) uint32_t requested_features = env->features[w]; env->features[w] &= host_feat; cpu->filtered_features[w] = requested_features & ~env->features[w]; + if (cpu->filtered_features[w]) { + rv = 1; + } } + + return rv; } -/* Report list of filtered features to stderr. - * Returns true if some features were found to be filtered, false otherwise - */ -static bool x86_cpu_report_filtered_features(X86CPU *cpu) +static void x86_cpu_report_filtered_features(X86CPU *cpu) { FeatureWord w; - uint32_t filtered = 0; for (w = 0; w < FEATURE_WORDS; w++) { - filtered |= cpu->filtered_features[w]; report_unavailable_features(w, cpu->filtered_features[w]); } - return filtered; } static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props) @@ -3136,8 +3141,6 @@ static void x86_cpu_load_features(X86CPU *cpu, Error **errp) env->cpuid_xlevel2 = env->cpuid_min_xlevel2; } - x86_cpu_filter_features(cpu); - out: if (local_err != NULL) { error_propagate(errp, local_err); @@ -3176,8 +3179,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) goto out; } - if (cpu->check_cpuid || cpu->enforce_cpuid) { - if (x86_cpu_report_filtered_features(cpu) && cpu->enforce_cpuid) { + x86_cpu_filter_features(cpu); + + if (x86_cpu_filter_features(cpu) && + (cpu->check_cpuid || cpu->enforce_cpuid)) { + x86_cpu_report_filtered_features(cpu); + if (cpu->enforce_cpuid) { error_setg(&local_err, kvm_enabled() ? "Host doesn't support requested features" : Changes v4 -> v5: * New patch: "target-i386: Register aliases for feature names with underscores" * On patch: "tests: Add test case for x86 feature parsing compatibility": * Fix typo on commit message Reported-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> * Add comment noting that the "[+-]feature" compatibility mode will be removed soon * On patch: "target-i386: Make plus_features/minus_features QOM-based": * Removed feat2prop() call on , as we now have property aliases for the old names containing underscores * On patch: "target-i386: Remove underscores from feat_names arrays": * Remove the feat2prop() call from the alias registration loop, too * Commit message update to enumerate all code that uses feat_names * On patch: "target-i386: x86_cpu_load_features() function": * Fix typo on x86_cpu_load_features() comment Reported-by: Paolo Bonzini <pbonzini@redhat.com> Diff v4 ->v5: diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 4dd3aee..620889f 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2002,12 +2002,10 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, /* Compatibility syntax: */ if (featurestr[0] == '+') { - feat2prop(featurestr + 1); plus_features = g_list_append(plus_features, g_strdup(featurestr + 1)); continue; } else if (featurestr[0] == '-') { - feat2prop(featurestr + 1); minus_features = g_list_append(minus_features, g_strdup(featurestr + 1)); continue; @@ -3066,8 +3064,7 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; } -/* Load CPUID data based on configureured features - */ +/* Load CPUID data based on configured features */ static void x86_cpu_load_features(X86CPU *cpu, Error **errp) { CPUX86State *env = &cpu->env; @@ -3443,7 +3440,10 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu, return; } - /* Property names should use "-" instead of "_" */ + /* Property names should use "-" instead of "_". + * Old names containing underscores are registered as aliases + * using object_property_add_alias() + */ assert(!strchr(name, '_')); /* aliases don't use "|" delimiters anymore, they are registered * manually using object_property_add_alias() */ @@ -3496,7 +3496,6 @@ static void x86_cpu_initfn(Object *obj) } } - /* Alias for feature properties: */ object_property_add_alias(obj, "sse3", obj, "pni", &error_abort); object_property_add_alias(obj, "pclmuldq", obj, "pclmulqdq", &error_abort); object_property_add_alias(obj, "sse4-1", obj, "sse4.1", &error_abort); @@ -3505,6 +3504,28 @@ static void x86_cpu_initfn(Object *obj) object_property_add_alias(obj, "ffxsr", obj, "fxsr-opt", &error_abort); object_property_add_alias(obj, "i64", obj, "lm", &error_abort); + object_property_add_alias(obj, "ds_cpl", obj, "ds-cpl", &error_abort); + object_property_add_alias(obj, "tsc_adjust", obj, "tsc-adjust", &error_abort); + object_property_add_alias(obj, "fxsr_opt", obj, "fxsr-opt", &error_abort); + object_property_add_alias(obj, "lahf_lm", obj, "lahf-lm", &error_abort); + object_property_add_alias(obj, "cmp_legacy", obj, "cmp-legacy", &error_abort); + object_property_add_alias(obj, "nodeid_msr", obj, "nodeid-msr", &error_abort); + object_property_add_alias(obj, "perfctr_core", obj, "perfctr-core", &error_abort); + object_property_add_alias(obj, "perfctr_nb", obj, "perfctr-nb", &error_abort); + object_property_add_alias(obj, "kvm_nopiodelay", obj, "kvm-nopiodelay", &error_abort); + object_property_add_alias(obj, "kvm_mmu", obj, "kvm-mmu", &error_abort); + object_property_add_alias(obj, "kvm_asyncpf", obj, "kvm-asyncpf", &error_abort); + object_property_add_alias(obj, "kvm_steal_time", obj, "kvm-steal-time", &error_abort); + object_property_add_alias(obj, "kvm_pv_eoi", obj, "kvm-pv-eoi", &error_abort); + object_property_add_alias(obj, "kvm_pv_unhalt", obj, "kvm-pv-unhalt", &error_abort); + object_property_add_alias(obj, "svm_lock", obj, "svm-lock", &error_abort); + object_property_add_alias(obj, "nrip_save", obj, "nrip-save", &error_abort); + object_property_add_alias(obj, "tsc_scale", obj, "tsc-scale", &error_abort); + object_property_add_alias(obj, "vmcb_clean", obj, "vmcb-clean", &error_abort); + object_property_add_alias(obj, "pause_filter", obj, "pause-filter", &error_abort); + object_property_add_alias(obj, "sse4_1", obj, "sse4.1", &error_abort); + object_property_add_alias(obj, "sse4_2", obj, "sse4.2", &error_abort); + x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort); } diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c index 7cff2b5..260dd27 100644 --- a/tests/test-x86-cpuid-compat.c +++ b/tests/test-x86-cpuid-compat.c @@ -81,9 +81,14 @@ static void test_plus_minus(void) char *path; /* Rules: - * "-foo" overrides "+foo" - * "[+-]foo" overrides "foo=..." - * "foo_bar" should be translated to "foo-bar" + * 1)"-foo" overrides "+foo" + * 2) "[+-]foo" overrides "foo=..." + * 3) Old feature names with underscores (e.g. "sse4_2") + * should keep working + * + * Note: rules 1 and 2 are planned to be removed soon, but we + * need to keep compatibility for a while until we start + * warning users about it. */ qtest_start("-cpu pentium,-fpu,+fpu,-mce,mce=on,+cx8,cx8=off,+sse4_1,sse4_2=on"); path = get_cpu0_qom_path(); Changes v3 -> v4: * Removed patch "Define CPUID filtering functions before x86_cpu_list" * New patch: "tests: Add test case for x86 feature parsing compatibility" * New patch: "target-i386: Disable VME by default with TCG" * Disable VME by default on TCG to avoid returning bogus results for all CPU models in TCG mode * New patch: "target-i386: Make plus_features/minus_features QOM-based" * New patch: "target-i386: Remove underscores from property names" * New patch: "target-i386: Register properties for feature aliases manually" * New patch: "target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas" * New patch: "target-i386: x86_cpu_load_features() function" * On patch: "target-i386: Return runnability information on query-cpu-definitions" * Added code to handle unsupported XSAVE components cleanly * Use x86_cpu_load_features() function Changes v2 -> v3: * Small documentation reword * Suggested-by: Markus Armbruster <armbru@redhat.com> * Create a x86_cpu_feature_name() function, to isolate the code that returns the property name Changes v1 -> v2: * Fixed documentation to say "(since 2.7)" * Removed @runnable field, improved documentation Example command output: { "return": [ { "unavailable-features": [], "static": false, "name": "host" }, { "unavailable-features": [], "static": false, "name": "qemu64" }, { "unavailable-features": [], "static": false, "name": "qemu32" }, { "unavailable-features": ["npt", "sse4a", "3dnow", "3dnowext", "fxsr-opt", "mmxext"], "static": false, "name": "phenom" }, { "unavailable-features": [], "static": false, "name": "pentium3" }, { "unavailable-features": [], "static": false, "name": "pentium2" }, { "unavailable-features": [], "static": false, "name": "pentium" }, { "unavailable-features": [], "static": false, "name": "n270" }, { "unavailable-features": [], "static": false, "name": "kvm64" }, { "unavailable-features": [], "static": false, "name": "kvm32" }, { "unavailable-features": [], "static": false, "name": "coreduo" }, { "unavailable-features": [], "static": false, "name": "core2duo" }, { "unavailable-features": ["3dnow", "3dnowext", "mmxext"], "static": false, "name": "athlon" }, { "unavailable-features": [], "static": false, "name": "Westmere" }, { "unavailable-features": ["xgetbv1", "xsavec", "3dnowprefetch", "smap", "adx", "rdseed", "mpx", "rtm", "hle"], "static": false, "name": "Skylake-Client" }, { "unavailable-features": [], "static": false, "name": "SandyBridge" }, { "unavailable-features": [], "static": false, "name": "Penryn" }, { "unavailable-features": ["tbm", "fma4", "xop", "3dnowprefetch", "misalignsse", "sse4a"], "static": false, "name": "Opteron_G5" }, { "unavailable-features": ["fma4", "xop", "3dnowprefetch", "misalignsse", "sse4a"], "static": false, "name": "Opteron_G4" }, { "unavailable-features": ["misalignsse", "sse4a"], "static": false, "name": "Opteron_G3" }, { "unavailable-features": [], "static": false, "name": "Opteron_G2" }, { "unavailable-features": [], "static": false, "name": "Opteron_G1" }, { "unavailable-features": [], "static": false, "name": "Nehalem" }, { "unavailable-features": [], "static": false, "name": "IvyBridge" }, { "unavailable-features": ["rtm", "hle"], "static": false, "name": "Haswell" }, { "unavailable-features": [], "static": false, "name": "Haswell-noTSX" }, { "unavailable-features": [], "static": false, "name": "Conroe" }, { "unavailable-features": ["3dnowprefetch", "smap", "adx", "rdseed", "rtm", "hle"], "static": false, "name": "Broadwell" }, { "unavailable-features": ["3dnowprefetch", "smap", "adx", "rdseed"], "static": false, "name": "Broadwell-noTSX" }, { "unavailable-features": [], "static": false, "name": "486" } ]} Cc: David Hildenbrand <dahi@linux.vnet.ibm.com> Cc: Michael Mueller <mimu@linux.vnet.ibm.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: Cornelia Huck <cornelia.huck@de.ibm.com> Cc: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com Eduardo Habkost (3): target-i386: Move warning code outside x86_cpu_filter_features() target-i386: x86_cpu_load_features() function target-i386: Return runnability information on query-cpu-definitions target-i386/cpu.c | 169 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 137 insertions(+), 32 deletions(-) -- 2.7.4
x86_cpu_filter_features() will be reused by code that shouldn't print any warning. Move the warning code to a new x86_cpu_report_filtered_features() function, and call it from x86_cpu_realizefn(). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Recovered v3 of patch, because x86_cpu_load_features() won't call x86_cpu_filter_features() anymore and we can keep the previous logic in x86_cpu_realizefn() that checked x86_cpu_filter_features() return value Changes v4 -> v5: * (none) Changes v3 -> v4: * Made x86_cpu_filter_features() void, make x86_cpu_report_filtered_features() return true if some features were filtered --- target-i386/cpu.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 61240dd..1e8127b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2177,9 +2177,6 @@ static int x86_cpu_filter_features(X86CPU *cpu) env->features[w] &= host_feat; cpu->filtered_features[w] = requested_features & ~env->features[w]; if (cpu->filtered_features[w]) { - if (cpu->check_cpuid || cpu->enforce_cpuid) { - report_unavailable_features(w, cpu->filtered_features[w]); - } rv = 1; } } @@ -2187,6 +2184,15 @@ static int x86_cpu_filter_features(X86CPU *cpu) return rv; } +static void x86_cpu_report_filtered_features(X86CPU *cpu) +{ + FeatureWord w; + + for (w = 0; w < FEATURE_WORDS; w++) { + report_unavailable_features(w, cpu->filtered_features[w]); + } +} + static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props) { PropValue *pv; @@ -3080,12 +3086,16 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->cpuid_xlevel2 = env->cpuid_min_xlevel2; } - if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) { - error_setg(&local_err, - kvm_enabled() ? - "Host doesn't support requested features" : - "TCG doesn't support requested features"); - goto out; + if (x86_cpu_filter_features(cpu) && + (cpu->check_cpuid || cpu->enforce_cpuid)) { + x86_cpu_report_filtered_features(cpu); + if (cpu->enforce_cpuid) { + error_setg(&local_err, + kvm_enabled() ? + "Host doesn't support requested features" : + "TCG doesn't support requested features"); + goto out; + } } /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on -- 2.7.4
On Fri, 7 Oct 2016 17:29:00 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
x86_cpu_filter_features() will be reused by code that shouldn't print any warning. Move the warning code to a new x86_cpu_report_filtered_features() function, and call it from x86_cpu_realizefn().
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
--- Changes v5 -> v6: * Recovered v3 of patch, because x86_cpu_load_features() won't call x86_cpu_filter_features() anymore and we can keep the previous logic in x86_cpu_realizefn() that checked x86_cpu_filter_features() return value
Changes v4 -> v5: * (none)
Changes v3 -> v4: * Made x86_cpu_filter_features() void, make x86_cpu_report_filtered_features() return true if some features were filtered --- target-i386/cpu.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 61240dd..1e8127b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2177,9 +2177,6 @@ static int x86_cpu_filter_features(X86CPU *cpu) env->features[w] &= host_feat; cpu->filtered_features[w] = requested_features & ~env->features[w]; if (cpu->filtered_features[w]) { - if (cpu->check_cpuid || cpu->enforce_cpuid) { - report_unavailable_features(w, cpu->filtered_features[w]); - } rv = 1; } } @@ -2187,6 +2184,15 @@ static int x86_cpu_filter_features(X86CPU *cpu) return rv; }
+static void x86_cpu_report_filtered_features(X86CPU *cpu) +{ + FeatureWord w; + + for (w = 0; w < FEATURE_WORDS; w++) { + report_unavailable_features(w, cpu->filtered_features[w]); + } +} + static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props) { PropValue *pv; @@ -3080,12 +3086,16 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->cpuid_xlevel2 = env->cpuid_min_xlevel2; }
- if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) { - error_setg(&local_err, - kvm_enabled() ? - "Host doesn't support requested features" : - "TCG doesn't support requested features"); - goto out; + if (x86_cpu_filter_features(cpu) && + (cpu->check_cpuid || cpu->enforce_cpuid)) { + x86_cpu_report_filtered_features(cpu); + if (cpu->enforce_cpuid) { + error_setg(&local_err, + kvm_enabled() ? + "Host doesn't support requested features" : + "TCG doesn't support requested features"); + goto out; + } }
/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
On Mon, Oct 10, 2016 at 01:57:08PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:00 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
x86_cpu_filter_features() will be reused by code that shouldn't print any warning. Move the warning code to a new x86_cpu_report_filtered_features() function, and call it from x86_cpu_realizefn().
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Thanks. Applied to x86-next. -- Eduardo
When probing for CPU model information, we need to reuse the code that initializes CPUID fields, but not the remaining side-effects of x86_cpu_realizefn(). Move that code to a separate function that can be reused later. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Move x86_cpu_filter_features() outside x86_cpu_load_features(), as the CPU model querying API won't run x86_cpu_filter_features() on most cases Changes v4 -> v5: * Fix typo on x86_cpu_load_features() comment Reported-by: Paolo Bonzini <pbonzini@redhat.com> Changes series v3 -> v4: * New patch added to series --- target-i386/cpu.c | 67 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1e8127b..23cc19b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2993,34 +2993,13 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; } -#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) -#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) -static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +/* Load CPUID data based on configured features */ +static void x86_cpu_load_features(X86CPU *cpu, Error **errp) { - CPUState *cs = CPU(dev); - X86CPU *cpu = X86_CPU(dev); - X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); CPUX86State *env = &cpu->env; - Error *local_err = NULL; - static bool ht_warned; FeatureWord w; GList *l; - - if (xcc->kvm_required && !kvm_enabled()) { - char *name = x86_cpu_class_get_model_name(xcc); - error_setg(&local_err, "CPU model '%s' requires KVM", name); - g_free(name); - goto out; - } - - if (cpu->apic_id == UNASSIGNED_APIC_ID) { - error_setg(errp, "apic-id property was not initialized properly"); - return; - } + Error *local_err = NULL; /*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert @@ -3086,6 +3065,46 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->cpuid_xlevel2 = env->cpuid_min_xlevel2; } +out: + if (local_err != NULL) { + error_propagate(errp, local_err); + } +} + +#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) +#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) +static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +{ + CPUState *cs = CPU(dev); + X86CPU *cpu = X86_CPU(dev); + X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); + CPUX86State *env = &cpu->env; + Error *local_err = NULL; + static bool ht_warned; + + if (xcc->kvm_required && !kvm_enabled()) { + char *name = x86_cpu_class_get_model_name(xcc); + error_setg(&local_err, "CPU model '%s' requires KVM", name); + g_free(name); + goto out; + } + + if (cpu->apic_id == UNASSIGNED_APIC_ID) { + error_setg(errp, "apic-id property was not initialized properly"); + return; + } + + x86_cpu_load_features(cpu, &local_err); + if (local_err) { + goto out; + } + + x86_cpu_filter_features(cpu); + if (x86_cpu_filter_features(cpu) && (cpu->check_cpuid || cpu->enforce_cpuid)) { x86_cpu_report_filtered_features(cpu); -- 2.7.4
On Fri, 7 Oct 2016 17:29:01 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
When probing for CPU model information, we need to reuse the code that initializes CPUID fields, but not the remaining side-effects of x86_cpu_realizefn(). Move that code to a separate function that can be reused later.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Move x86_cpu_filter_features() outside x86_cpu_load_features(), as the CPU model querying API won't run x86_cpu_filter_features() on most cases
Changes v4 -> v5: * Fix typo on x86_cpu_load_features() comment Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Changes series v3 -> v4: * New patch added to series --- target-i386/cpu.c | 67 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 24 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1e8127b..23cc19b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2993,34 +2993,13 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; }
-#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) -#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) -static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +/* Load CPUID data based on configured features */ +static void x86_cpu_load_features(X86CPU *cpu, Error **errp) { - CPUState *cs = CPU(dev); - X86CPU *cpu = X86_CPU(dev); - X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); CPUX86State *env = &cpu->env; - Error *local_err = NULL; - static bool ht_warned; FeatureWord w; GList *l; - - if (xcc->kvm_required && !kvm_enabled()) { - char *name = x86_cpu_class_get_model_name(xcc); - error_setg(&local_err, "CPU model '%s' requires KVM", name); - g_free(name); - goto out; - } - - if (cpu->apic_id == UNASSIGNED_APIC_ID) { - error_setg(errp, "apic-id property was not initialized properly"); - return; - } + Error *local_err = NULL;
/*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert @@ -3086,6 +3065,46 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->cpuid_xlevel2 = env->cpuid_min_xlevel2; }
+out: + if (local_err != NULL) { + error_propagate(errp, local_err); + } +} + +#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) +#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) +static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +{ + CPUState *cs = CPU(dev); + X86CPU *cpu = X86_CPU(dev); + X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); + CPUX86State *env = &cpu->env; + Error *local_err = NULL; + static bool ht_warned; + + if (xcc->kvm_required && !kvm_enabled()) { + char *name = x86_cpu_class_get_model_name(xcc); + error_setg(&local_err, "CPU model '%s' requires KVM", name); + g_free(name); + goto out; + } + + if (cpu->apic_id == UNASSIGNED_APIC_ID) { + error_setg(errp, "apic-id property was not initialized properly"); + return; + } + + x86_cpu_load_features(cpu, &local_err); + if (local_err) { + goto out; + } + + x86_cpu_filter_features(cpu);
that makes 2 invocations of ^^ inside realize, see followup line vvvv [...]
if (x86_cpu_filter_features(cpu) && (cpu->check_cpuid || cpu->enforce_cpuid)) { x86_cpu_report_filtered_features(cpu);
On Mon, Oct 10, 2016 at 02:25:32PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:01 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
When probing for CPU model information, we need to reuse the code that initializes CPUID fields, but not the remaining side-effects of x86_cpu_realizefn(). Move that code to a separate function that can be reused later.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Move x86_cpu_filter_features() outside x86_cpu_load_features(), as the CPU model querying API won't run x86_cpu_filter_features() on most cases
Changes v4 -> v5: * Fix typo on x86_cpu_load_features() comment Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Changes series v3 -> v4: * New patch added to series --- target-i386/cpu.c | 67 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 24 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1e8127b..23cc19b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2993,34 +2993,13 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; }
-#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) -#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) -static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +/* Load CPUID data based on configured features */ +static void x86_cpu_load_features(X86CPU *cpu, Error **errp) { - CPUState *cs = CPU(dev); - X86CPU *cpu = X86_CPU(dev); - X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); CPUX86State *env = &cpu->env; - Error *local_err = NULL; - static bool ht_warned; FeatureWord w; GList *l; - - if (xcc->kvm_required && !kvm_enabled()) { - char *name = x86_cpu_class_get_model_name(xcc); - error_setg(&local_err, "CPU model '%s' requires KVM", name); - g_free(name); - goto out; - } - - if (cpu->apic_id == UNASSIGNED_APIC_ID) { - error_setg(errp, "apic-id property was not initialized properly"); - return; - } + Error *local_err = NULL;
/*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert @@ -3086,6 +3065,46 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->cpuid_xlevel2 = env->cpuid_min_xlevel2; }
+out: + if (local_err != NULL) { + error_propagate(errp, local_err); + } +} + +#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) +#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) +static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +{ + CPUState *cs = CPU(dev); + X86CPU *cpu = X86_CPU(dev); + X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); + CPUX86State *env = &cpu->env; + Error *local_err = NULL; + static bool ht_warned; + + if (xcc->kvm_required && !kvm_enabled()) { + char *name = x86_cpu_class_get_model_name(xcc); + error_setg(&local_err, "CPU model '%s' requires KVM", name); + g_free(name); + goto out; + } + + if (cpu->apic_id == UNASSIGNED_APIC_ID) { + error_setg(errp, "apic-id property was not initialized properly"); + return; + } + + x86_cpu_load_features(cpu, &local_err); + if (local_err) { + goto out; + } + + x86_cpu_filter_features(cpu);
that makes 2 invocations of ^^ inside realize, see followup line vvvv
Oops, leftover from v5. Thanks for spotting that! Fixup below. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 23cc19b..4294746 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -3103,8 +3103,6 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) goto out; } - x86_cpu_filter_features(cpu); - if (x86_cpu_filter_features(cpu) && (cpu->check_cpuid || cpu->enforce_cpuid)) { x86_cpu_report_filtered_features(cpu); -- 2.7.4 -- Eduardo
On Mon, 10 Oct 2016 13:58:25 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Oct 10, 2016 at 02:25:32PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:01 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
When probing for CPU model information, we need to reuse the code that initializes CPUID fields, but not the remaining side-effects of x86_cpu_realizefn(). Move that code to a separate function that can be reused later.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Move x86_cpu_filter_features() outside x86_cpu_load_features(), as the CPU model querying API won't run x86_cpu_filter_features() on most cases
Changes v4 -> v5: * Fix typo on x86_cpu_load_features() comment Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Changes series v3 -> v4: * New patch added to series --- target-i386/cpu.c | 67 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 24 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1e8127b..23cc19b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2993,34 +2993,13 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; }
-#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) -#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ - (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ - (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) -static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +/* Load CPUID data based on configured features */ +static void x86_cpu_load_features(X86CPU *cpu, Error **errp) { - CPUState *cs = CPU(dev); - X86CPU *cpu = X86_CPU(dev); - X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); CPUX86State *env = &cpu->env; - Error *local_err = NULL; - static bool ht_warned; FeatureWord w; GList *l; - - if (xcc->kvm_required && !kvm_enabled()) { - char *name = x86_cpu_class_get_model_name(xcc); - error_setg(&local_err, "CPU model '%s' requires KVM", name); - g_free(name); - goto out; - } - - if (cpu->apic_id == UNASSIGNED_APIC_ID) { - error_setg(errp, "apic-id property was not initialized properly"); - return; - } + Error *local_err = NULL;
/*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert @@ -3086,6 +3065,46 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->cpuid_xlevel2 = env->cpuid_min_xlevel2; }
+out: + if (local_err != NULL) { + error_propagate(errp, local_err); + } +} + +#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) +#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ + (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ + (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) +static void x86_cpu_realizefn(DeviceState *dev, Error **errp) +{ + CPUState *cs = CPU(dev); + X86CPU *cpu = X86_CPU(dev); + X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); + CPUX86State *env = &cpu->env; + Error *local_err = NULL; + static bool ht_warned; + + if (xcc->kvm_required && !kvm_enabled()) { + char *name = x86_cpu_class_get_model_name(xcc); + error_setg(&local_err, "CPU model '%s' requires KVM", name); + g_free(name); + goto out; + } + + if (cpu->apic_id == UNASSIGNED_APIC_ID) { + error_setg(errp, "apic-id property was not initialized properly"); + return; + } + + x86_cpu_load_features(cpu, &local_err); + if (local_err) { + goto out; + } + + x86_cpu_filter_features(cpu);
that makes 2 invocations of ^^ inside realize, see followup line vvvv
Oops, leftover from v5. Thanks for spotting that! Fixup below.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
--- target-i386/cpu.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 23cc19b..4294746 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -3103,8 +3103,6 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) goto out; }
- x86_cpu_filter_features(cpu); - if (x86_cpu_filter_features(cpu) && (cpu->check_cpuid || cpu->enforce_cpuid)) { x86_cpu_report_filtered_features(cpu);
On Tue, Oct 11, 2016 at 01:41:30PM +0200, Igor Mammedov wrote: [...]
Oops, leftover from v5. Thanks for spotting that! Fixup below.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Thanks. Patch + fixup applied to x86-next. -- Eduardo
Fill the "unavailable-features" field on the x86 implementation of query-cpu-definitions. Cc: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Call x86_cpu_filter_features(), now that x86_cpu_load_features() won't run it automatically Changes v4 -> v5: * (none) Changes v3 -> v4: * Handle missing XSAVE components cleanly, but looking up the original feature that required it * Use x86_cpu_load_features() function Changes v2 -> v3: * Create a x86_cpu_feature_name() function, to isolate the code that returns the property name Changes v1 -> v2: * Updated to the new schema: no @runnable field, and always report @unavailable-features as present --- target-i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 23cc19b..63330ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s) } } +/* Return the feature property name for a feature flag bit */ +static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) +{ + /* XSAVE components are automatically enabled by other features, + * so return the original feature name instead + */ + if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) { + int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr; + + if (comp < ARRAY_SIZE(x86_ext_save_areas) && + x86_ext_save_areas[comp].bits) { + w = x86_ext_save_areas[comp].feature; + bitnr = ctz32(x86_ext_save_areas[comp].bits); + } + } + + assert(bitnr < 32); + assert(w < FEATURE_WORDS); + return feature_word_info[w].feat_names[bitnr]; +} + /* Compatibily hack to maintain legacy +-feat semantic, * where +-feat overwrites any feature set by * feat=on|feat even if the later is parsed after +-feat @@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, } } +static void x86_cpu_load_features(X86CPU *cpu, Error **errp); +static int x86_cpu_filter_features(X86CPU *cpu); + +/* Check for missing features that may prevent the CPU class from + * running using the current machine and accelerator. + */ +static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } + + object_unref(OBJECT(xc)); +} + /* Print all cpuid feature names in featureset */ static void listflags(FILE *f, fprintf_function print, const char **featureset) @@ -2122,6 +2196,8 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data) info = g_malloc0(sizeof(*info)); info->name = x86_cpu_class_get_model_name(cc); + x86_cpu_class_check_missing_features(cc, &info->unavailable_features); + info->has_unavailable_features = true; entry = g_malloc0(sizeof(*entry)); entry->value = info; -- 2.7.4
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
Fill the "unavailable-features" field on the x86 implementation of query-cpu-definitions.
Cc: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Call x86_cpu_filter_features(), now that x86_cpu_load_features() won't run it automatically
Changes v4 -> v5: * (none)
Changes v3 -> v4: * Handle missing XSAVE components cleanly, but looking up the original feature that required it * Use x86_cpu_load_features() function
Changes v2 -> v3: * Create a x86_cpu_feature_name() function, to isolate the code that returns the property name
Changes v1 -> v2: * Updated to the new schema: no @runnable field, and always report @unavailable-features as present --- target-i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 23cc19b..63330ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s) } }
+/* Return the feature property name for a feature flag bit */ +static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) +{ + /* XSAVE components are automatically enabled by other features, + * so return the original feature name instead + */ + if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) { + int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr; + + if (comp < ARRAY_SIZE(x86_ext_save_areas) && + x86_ext_save_areas[comp].bits) { + w = x86_ext_save_areas[comp].feature; + bitnr = ctz32(x86_ext_save_areas[comp].bits); + } + } + + assert(bitnr < 32); + assert(w < FEATURE_WORDS); + return feature_word_info[w].feat_names[bitnr]; +} + /* Compatibily hack to maintain legacy +-feat semantic, * where +-feat overwrites any feature set by * feat=on|feat even if the later is parsed after +-feat @@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, } }
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp); +static int x86_cpu_filter_features(X86CPU *cpu); + +/* Check for missing features that may prevent the CPU class from + * running using the current machine and accelerator. + */ +static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } Shouldn't you add if (IS_AMD_CPU(env)) { fixup here, that realize does right after calling x86_cpu_filter_features()
+ object_unref(OBJECT(xc)); +} + /* Print all cpuid feature names in featureset */ static void listflags(FILE *f, fprintf_function print, const char **featureset) @@ -2122,6 +2196,8 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
info = g_malloc0(sizeof(*info)); info->name = x86_cpu_class_get_model_name(cc); + x86_cpu_class_check_missing_features(cc, &info->unavailable_features); + info->has_unavailable_features = true;
entry = g_malloc0(sizeof(*entry)); entry->value = info;
On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
Fill the "unavailable-features" field on the x86 implementation of query-cpu-definitions.
Cc: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Call x86_cpu_filter_features(), now that x86_cpu_load_features() won't run it automatically
Changes v4 -> v5: * (none)
Changes v3 -> v4: * Handle missing XSAVE components cleanly, but looking up the original feature that required it * Use x86_cpu_load_features() function
Changes v2 -> v3: * Create a x86_cpu_feature_name() function, to isolate the code that returns the property name
Changes v1 -> v2: * Updated to the new schema: no @runnable field, and always report @unavailable-features as present --- target-i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 23cc19b..63330ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s) } }
+/* Return the feature property name for a feature flag bit */ +static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) +{ + /* XSAVE components are automatically enabled by other features, + * so return the original feature name instead + */ + if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) { + int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr; + + if (comp < ARRAY_SIZE(x86_ext_save_areas) && + x86_ext_save_areas[comp].bits) { + w = x86_ext_save_areas[comp].feature; + bitnr = ctz32(x86_ext_save_areas[comp].bits); + } + } + + assert(bitnr < 32); + assert(w < FEATURE_WORDS); + return feature_word_info[w].feat_names[bitnr]; +} + /* Compatibily hack to maintain legacy +-feat semantic, * where +-feat overwrites any feature set by * feat=on|feat even if the later is parsed after +-feat @@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, } }
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp); +static int x86_cpu_filter_features(X86CPU *cpu); + +/* Check for missing features that may prevent the CPU class from + * running using the current machine and accelerator. + */ +static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } Shouldn't you add if (IS_AMD_CPU(env)) { fixup here, that realize does right after calling x86_cpu_filter_features()
What would it be useful for? The IS_AMD_CPU fixup runs after x86_cpu_filter_features() (so it doesn't affect filtered_features at all), and filtered_features is the only field used as input to build missing_feats. -- Eduardo
On Mon, 10 Oct 2016 14:01:10 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
Fill the "unavailable-features" field on the x86 implementation of query-cpu-definitions.
Cc: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v5 -> v6: * Call x86_cpu_filter_features(), now that x86_cpu_load_features() won't run it automatically
Changes v4 -> v5: * (none)
Changes v3 -> v4: * Handle missing XSAVE components cleanly, but looking up the original feature that required it * Use x86_cpu_load_features() function
Changes v2 -> v3: * Create a x86_cpu_feature_name() function, to isolate the code that returns the property name
Changes v1 -> v2: * Updated to the new schema: no @runnable field, and always report @unavailable-features as present --- target-i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 23cc19b..63330ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s) } }
+/* Return the feature property name for a feature flag bit */ +static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) +{ + /* XSAVE components are automatically enabled by other features, + * so return the original feature name instead + */ + if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) { + int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr; + + if (comp < ARRAY_SIZE(x86_ext_save_areas) && + x86_ext_save_areas[comp].bits) { + w = x86_ext_save_areas[comp].feature; + bitnr = ctz32(x86_ext_save_areas[comp].bits); + } + } + + assert(bitnr < 32); + assert(w < FEATURE_WORDS); + return feature_word_info[w].feat_names[bitnr]; +} + /* Compatibily hack to maintain legacy +-feat semantic, * where +-feat overwrites any feature set by * feat=on|feat even if the later is parsed after +-feat @@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, } }
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp); +static int x86_cpu_filter_features(X86CPU *cpu); + +/* Check for missing features that may prevent the CPU class from + * running using the current machine and accelerator. + */ +static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } Shouldn't you add if (IS_AMD_CPU(env)) { fixup here, that realize does right after calling x86_cpu_filter_features()
What would it be useful for? The IS_AMD_CPU fixup runs after x86_cpu_filter_features() (so it doesn't affect filtered_features at all), and filtered_features is the only field used as input to build missing_feats. For completeness of features returned by query-cpu-definitions, I'd guess. So that returned cpu definitions would match actually created cpus.
On Tue, Oct 11, 2016 at 01:45:21PM +0200, Igor Mammedov wrote:
On Mon, 10 Oct 2016 14:01:10 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: [...]
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } Shouldn't you add if (IS_AMD_CPU(env)) { fixup here, that realize does right after calling x86_cpu_filter_features()
What would it be useful for? The IS_AMD_CPU fixup runs after x86_cpu_filter_features() (so it doesn't affect filtered_features at all), and filtered_features is the only field used as input to build missing_feats. For completeness of features returned by query-cpu-definitions, I'd guess. So that returned cpu definitions would match actually created cpus.
For completeness of which query-cpu-definitions field, exactly? There's no field in the return value of query-cpu-definitions that would be affected by the AMD aliases. The AMD aliases don't affect runnability of the CPU model because they aren't included in the GET_SUPPORTED_CPUID check[1]. You would be right if we did return a copy of the low-level CPUID data that's seen by the guest, or if the AMD aliases were set up before x86_cpu_filter_features() (so they could affect filtered_features/unavailable-features), but that's not the case. [1] They aren't included in the GET_SUPPORTED_CPUID check because the existence of the AMD aliases depend only on the configured guest vendor ID, not on the host CPU. -- Eduardo
On Tue, 11 Oct 2016 08:58:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Oct 11, 2016 at 01:45:21PM +0200, Igor Mammedov wrote:
On Mon, 10 Oct 2016 14:01:10 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: [...]
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } Shouldn't you add if (IS_AMD_CPU(env)) { fixup here, that realize does right after calling x86_cpu_filter_features()
What would it be useful for? The IS_AMD_CPU fixup runs after x86_cpu_filter_features() (so it doesn't affect filtered_features at all), and filtered_features is the only field used as input to build missing_feats. For completeness of features returned by query-cpu-definitions, I'd guess. So that returned cpu definitions would match actually created cpus.
For completeness of which query-cpu-definitions field, exactly? There's no field in the return value of query-cpu-definitions that would be affected by the AMD aliases. The AMD aliases don't affect runnability of the CPU model because they aren't included in the GET_SUPPORTED_CPUID check[1].
You would be right if we did return a copy of the low-level CPUID data that's seen by the guest, or if the AMD aliases were set up before x86_cpu_filter_features() (so they could affect filtered_features/unavailable-features), but that's not the case.
[1] They aren't included in the GET_SUPPORTED_CPUID check because the existence of the AMD aliases depend only on the configured guest vendor ID, not on the host CPU.
Got it. I've tried to build with this patch but build fails with make -j32 CHK version_gen.h CC i386-linux-user/target-i386/cpu.o target-i386/cpu.c: In function ‘x86_cpu_definition_entry’: target-i386/cpu.c:2199:51: error: ‘CpuDefinitionInfo’ has no member named ‘unavailable_features’ x86_cpu_class_check_missing_features(cc, &info->unavailable_features); ^ target-i386/cpu.c:2200:9: error: ‘CpuDefinitionInfo’ has no member named ‘has_unavailable_features’ info->has_unavailable_features = true; Probably series misses a patch that adds it.
On Tue, Oct 11, 2016 at 03:21:05PM +0200, Igor Mammedov wrote:
On Tue, 11 Oct 2016 08:58:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Oct 11, 2016 at 01:45:21PM +0200, Igor Mammedov wrote:
On Mon, 10 Oct 2016 14:01:10 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: [...]
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } Shouldn't you add if (IS_AMD_CPU(env)) { fixup here, that realize does right after calling x86_cpu_filter_features()
What would it be useful for? The IS_AMD_CPU fixup runs after x86_cpu_filter_features() (so it doesn't affect filtered_features at all), and filtered_features is the only field used as input to build missing_feats. For completeness of features returned by query-cpu-definitions, I'd guess. So that returned cpu definitions would match actually created cpus.
For completeness of which query-cpu-definitions field, exactly? There's no field in the return value of query-cpu-definitions that would be affected by the AMD aliases. The AMD aliases don't affect runnability of the CPU model because they aren't included in the GET_SUPPORTED_CPUID check[1].
You would be right if we did return a copy of the low-level CPUID data that's seen by the guest, or if the AMD aliases were set up before x86_cpu_filter_features() (so they could affect filtered_features/unavailable-features), but that's not the case.
[1] They aren't included in the GET_SUPPORTED_CPUID check because the existence of the AMD aliases depend only on the configured guest vendor ID, not on the host CPU.
Got it.
I've tried to build with this patch but build fails with
make -j32 CHK version_gen.h CC i386-linux-user/target-i386/cpu.o target-i386/cpu.c: In function ‘x86_cpu_definition_entry’: target-i386/cpu.c:2199:51: error: ‘CpuDefinitionInfo’ has no member named ‘unavailable_features’ x86_cpu_class_check_missing_features(cc, &info->unavailable_features); ^ target-i386/cpu.c:2200:9: error: ‘CpuDefinitionInfo’ has no member named ‘has_unavailable_features’ info->has_unavailable_features = true;
Probably series misses a patch that adds it.
See git URLs on cover letter. Series is based on my x86-next branch. ] This series can be seen in the git branch at: ] https://github.com/ehabkost/qemu-hacks.git work/query-cpu-definitions-runnable-info ] ] The series is based on my x86-next branch: ] https://github.com/ehabkost/qemu.git x86-next -- Eduardo
On Tue, 11 Oct 2016 10:24:38 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Oct 11, 2016 at 03:21:05PM +0200, Igor Mammedov wrote:
On Tue, 11 Oct 2016 08:58:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Oct 11, 2016 at 01:45:21PM +0200, Igor Mammedov wrote:
On Mon, 10 Oct 2016 14:01:10 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: [...] > +static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, > + strList **missing_feats) > +{ > + X86CPU *xc; > + FeatureWord w; > + Error *err = NULL; > + strList **next = missing_feats; > + > + if (xcc->kvm_required && !kvm_enabled()) { > + strList *new = g_new0(strList, 1); > + new->value = g_strdup("kvm");; > + *missing_feats = new; > + return; > + } > + > + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); > + > + x86_cpu_load_features(xc, &err); > + if (err) { > + /* Errors at x86_cpu_load_features should never happen, > + * but in case it does, just report the model as not > + * runnable at all using the "type" property. > + */ > + strList *new = g_new0(strList, 1); > + new->value = g_strdup("type"); > + *next = new; > + next = &new->next; > + } > + > + x86_cpu_filter_features(xc); > + > + for (w = 0; w < FEATURE_WORDS; w++) { > + uint32_t filtered = xc->filtered_features[w]; > + int i; > + for (i = 0; i < 32; i++) { > + if (filtered & (1UL << i)) { > + strList *new = g_new0(strList, 1); > + new->value = g_strdup(x86_cpu_feature_name(w, i)); > + *next = new; > + next = &new->next; > + } > + } > + } Shouldn't you add if (IS_AMD_CPU(env)) { fixup here, that realize does right after calling x86_cpu_filter_features()
What would it be useful for? The IS_AMD_CPU fixup runs after x86_cpu_filter_features() (so it doesn't affect filtered_features at all), and filtered_features is the only field used as input to build missing_feats. For completeness of features returned by query-cpu-definitions, I'd guess. So that returned cpu definitions would match actually created cpus.
For completeness of which query-cpu-definitions field, exactly? There's no field in the return value of query-cpu-definitions that would be affected by the AMD aliases. The AMD aliases don't affect runnability of the CPU model because they aren't included in the GET_SUPPORTED_CPUID check[1].
You would be right if we did return a copy of the low-level CPUID data that's seen by the guest, or if the AMD aliases were set up before x86_cpu_filter_features() (so they could affect filtered_features/unavailable-features), but that's not the case.
[1] They aren't included in the GET_SUPPORTED_CPUID check because the existence of the AMD aliases depend only on the configured guest vendor ID, not on the host CPU.
Got it.
I've tried to build with this patch but build fails with
make -j32 CHK version_gen.h CC i386-linux-user/target-i386/cpu.o target-i386/cpu.c: In function ‘x86_cpu_definition_entry’: target-i386/cpu.c:2199:51: error: ‘CpuDefinitionInfo’ has no member named ‘unavailable_features’ x86_cpu_class_check_missing_features(cc, &info->unavailable_features); ^ target-i386/cpu.c:2200:9: error: ‘CpuDefinitionInfo’ has no member named ‘has_unavailable_features’ info->has_unavailable_features = true;
Probably series misses a patch that adds it.
See git URLs on cover letter. Series is based on my x86-next branch.
] This series can be seen in the git branch at: ] https://github.com/ehabkost/qemu-hacks.git work/query-cpu-definitions-runnable-info ] ] The series is based on my x86-next branch: ] https://github.com/ehabkost/qemu.git x86-next I've used this one from yesterday as base and it didn't have "qmp: Add runnability information to query-cpu-definitions"
I'll refetch and try again.
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
Fill the "unavailable-features" field on the x86 implementation of query-cpu-definitions.
Cc: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
--- Changes v5 -> v6: * Call x86_cpu_filter_features(), now that x86_cpu_load_features() won't run it automatically
Changes v4 -> v5: * (none)
Changes v3 -> v4: * Handle missing XSAVE components cleanly, but looking up the original feature that required it * Use x86_cpu_load_features() function
Changes v2 -> v3: * Create a x86_cpu_feature_name() function, to isolate the code that returns the property name
Changes v1 -> v2: * Updated to the new schema: no @runnable field, and always report @unavailable-features as present --- target-i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 23cc19b..63330ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s) } }
+/* Return the feature property name for a feature flag bit */ +static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) +{ + /* XSAVE components are automatically enabled by other features, + * so return the original feature name instead + */ + if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) { + int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr; + + if (comp < ARRAY_SIZE(x86_ext_save_areas) && + x86_ext_save_areas[comp].bits) { + w = x86_ext_save_areas[comp].feature; + bitnr = ctz32(x86_ext_save_areas[comp].bits); + } + } + + assert(bitnr < 32); + assert(w < FEATURE_WORDS); + return feature_word_info[w].feat_names[bitnr]; +} + /* Compatibily hack to maintain legacy +-feat semantic, * where +-feat overwrites any feature set by * feat=on|feat even if the later is parsed after +-feat @@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, } }
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp); +static int x86_cpu_filter_features(X86CPU *cpu); + +/* Check for missing features that may prevent the CPU class from + * running using the current machine and accelerator. + */ +static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, + strList **missing_feats) +{ + X86CPU *xc; + FeatureWord w; + Error *err = NULL; + strList **next = missing_feats; + + if (xcc->kvm_required && !kvm_enabled()) { + strList *new = g_new0(strList, 1); + new->value = g_strdup("kvm");; + *missing_feats = new; + return; + } + + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc)))); + + x86_cpu_load_features(xc, &err); + if (err) { + /* Errors at x86_cpu_load_features should never happen, + * but in case it does, just report the model as not + * runnable at all using the "type" property. + */ + strList *new = g_new0(strList, 1); + new->value = g_strdup("type"); + *next = new; + next = &new->next; + } + + x86_cpu_filter_features(xc); + + for (w = 0; w < FEATURE_WORDS; w++) { + uint32_t filtered = xc->filtered_features[w]; + int i; + for (i = 0; i < 32; i++) { + if (filtered & (1UL << i)) { + strList *new = g_new0(strList, 1); + new->value = g_strdup(x86_cpu_feature_name(w, i)); + *next = new; + next = &new->next; + } + } + } + + object_unref(OBJECT(xc)); +} + /* Print all cpuid feature names in featureset */ static void listflags(FILE *f, fprintf_function print, const char **featureset) @@ -2122,6 +2196,8 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
info = g_malloc0(sizeof(*info)); info->name = x86_cpu_class_get_model_name(cc); + x86_cpu_class_check_missing_features(cc, &info->unavailable_features); + info->has_unavailable_features = true;
entry = g_malloc0(sizeof(*entry)); entry->value = info;
On Tue, Oct 11, 2016 at 04:13:44PM +0200, Igor Mammedov wrote:
On Fri, 7 Oct 2016 17:29:02 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
Fill the "unavailable-features" field on the x86 implementation of query-cpu-definitions.
Cc: Jiri Denemark <jdenemar@redhat.com> Cc: libvir-list@redhat.com Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Thanks. Applied to x86-next. -- Eduardo
participants (2)
-
Eduardo Habkost -
Igor Mammedov