* [Qemu-devel] [PATCH v6 0/3] Add runnability info to query-cpu-definitions
@ 2016-10-07 20:28 Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-07 20:28 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list,
Michael Mueller, Christian Borntraeger, Cornelia Huck
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features()
2016-10-07 20:28 [Qemu-devel] [PATCH v6 0/3] Add runnability info to query-cpu-definitions Eduardo Habkost
@ 2016-10-07 20:29 ` Eduardo Habkost
2016-10-10 11:57 ` Igor Mammedov
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 2/3] target-i386: x86_cpu_load_features() function Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
2 siblings, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-07 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v6 2/3] target-i386: x86_cpu_load_features() function
2016-10-07 20:28 [Qemu-devel] [PATCH v6 0/3] Add runnability info to query-cpu-definitions Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
@ 2016-10-07 20:29 ` Eduardo Habkost
2016-10-10 12:25 ` Igor Mammedov
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
2 siblings, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-07 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-07 20:28 [Qemu-devel] [PATCH v6 0/3] Add runnability info to query-cpu-definitions Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 2/3] target-i386: x86_cpu_load_features() function Eduardo Habkost
@ 2016-10-07 20:29 ` Eduardo Habkost
2016-10-10 12:27 ` Igor Mammedov
2016-10-11 14:13 ` Igor Mammedov
2 siblings, 2 replies; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-07 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features()
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
@ 2016-10-10 11:57 ` Igor Mammedov
2016-10-10 17:04 ` Eduardo Habkost
0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2016-10-10 11:57 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 2/3] target-i386: x86_cpu_load_features() function
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 2/3] target-i386: x86_cpu_load_features() function Eduardo Habkost
@ 2016-10-10 12:25 ` Igor Mammedov
2016-10-10 16:58 ` [Qemu-devel] [PATCH] fixup! " Eduardo Habkost
0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2016-10-10 12:25 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
@ 2016-10-10 12:27 ` Igor Mammedov
2016-10-10 17:01 ` Eduardo Habkost
2016-10-11 14:13 ` Igor Mammedov
1 sibling, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2016-10-10 12:27 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH] fixup! target-i386: x86_cpu_load_features() function
2016-10-10 12:25 ` Igor Mammedov
@ 2016-10-10 16:58 ` Eduardo Habkost
2016-10-11 11:41 ` Igor Mammedov
0 siblings, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-10 16:58 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-10 12:27 ` Igor Mammedov
@ 2016-10-10 17:01 ` Eduardo Habkost
2016-10-11 11:45 ` Igor Mammedov
0 siblings, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-10 17:01 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features()
2016-10-10 11:57 ` Igor Mammedov
@ 2016-10-10 17:04 ` Eduardo Habkost
0 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-10 17:04 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH] fixup! target-i386: x86_cpu_load_features() function
2016-10-10 16:58 ` [Qemu-devel] [PATCH] fixup! " Eduardo Habkost
@ 2016-10-11 11:41 ` Igor Mammedov
2016-10-14 14:59 ` Eduardo Habkost
0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2016-10-11 11:41 UTC (permalink / raw)
To: Eduardo Habkost
Cc: libvir-list, qemu-devel, Markus Armbruster, dahi, Paolo Bonzini,
Jiri Denemark, Richard Henderson
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);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-10 17:01 ` Eduardo Habkost
@ 2016-10-11 11:45 ` Igor Mammedov
2016-10-11 11:58 ` Eduardo Habkost
0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2016-10-11 11:45 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-11 11:45 ` Igor Mammedov
@ 2016-10-11 11:58 ` Eduardo Habkost
2016-10-11 13:21 ` Igor Mammedov
0 siblings, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-11 11:58 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-11 11:58 ` Eduardo Habkost
@ 2016-10-11 13:21 ` Igor Mammedov
2016-10-11 13:24 ` Eduardo Habkost
0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2016-10-11 13:21 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-11 13:21 ` Igor Mammedov
@ 2016-10-11 13:24 ` Eduardo Habkost
2016-10-11 13:51 ` Igor Mammedov
0 siblings, 1 reply; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-11 13:24 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-11 13:24 ` Eduardo Habkost
@ 2016-10-11 13:51 ` Igor Mammedov
0 siblings, 0 replies; 19+ messages in thread
From: Igor Mammedov @ 2016-10-11 13:51 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, dahi, Paolo Bonzini, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
2016-10-10 12:27 ` Igor Mammedov
@ 2016-10-11 14:13 ` Igor Mammedov
2016-10-14 14:59 ` Eduardo Habkost
1 sibling, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2016-10-11 14:13 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, libvir-list, Markus Armbruster, dahi, Paolo Bonzini,
Jiri Denemark, Richard Henderson
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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH] fixup! target-i386: x86_cpu_load_features() function
2016-10-11 11:41 ` Igor Mammedov
@ 2016-10-14 14:59 ` Eduardo Habkost
0 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-14 14:59 UTC (permalink / raw)
To: Igor Mammedov
Cc: libvir-list, qemu-devel, Markus Armbruster, dahi, Paolo Bonzini,
Jiri Denemark, Richard Henderson
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
2016-10-11 14:13 ` Igor Mammedov
@ 2016-10-14 14:59 ` Eduardo Habkost
0 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2016-10-14 14:59 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, libvir-list, Markus Armbruster, dahi, Paolo Bonzini,
Jiri Denemark, Richard Henderson
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
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-10-14 14:59 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-07 20:28 [Qemu-devel] [PATCH v6 0/3] Add runnability info to query-cpu-definitions Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
2016-10-10 11:57 ` Igor Mammedov
2016-10-10 17:04 ` Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 2/3] target-i386: x86_cpu_load_features() function Eduardo Habkost
2016-10-10 12:25 ` Igor Mammedov
2016-10-10 16:58 ` [Qemu-devel] [PATCH] fixup! " Eduardo Habkost
2016-10-11 11:41 ` Igor Mammedov
2016-10-14 14:59 ` Eduardo Habkost
2016-10-07 20:29 ` [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
2016-10-10 12:27 ` Igor Mammedov
2016-10-10 17:01 ` Eduardo Habkost
2016-10-11 11:45 ` Igor Mammedov
2016-10-11 11:58 ` Eduardo Habkost
2016-10-11 13:21 ` Igor Mammedov
2016-10-11 13:24 ` Eduardo Habkost
2016-10-11 13:51 ` Igor Mammedov
2016-10-11 14:13 ` Igor Mammedov
2016-10-14 14:59 ` Eduardo Habkost
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.