* [Qemu-devel] [PATCH v5 01/12] tests: Add test case for x86 feature parsing compatibility
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 02/12] target-i386: List CPU models using subclass list Eduardo Habkost
` (11 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
Add a new test case to ensure the existing behavior of the
feature parsing code will be kept.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v4 -> v5:
* 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
Changes series v3 -> v4:
* New patch added to series
---
tests/test-x86-cpuid-compat.c | 44 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 83162a4..260dd27 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -3,6 +3,7 @@
#include "qapi/qmp/qlist.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qint.h"
+#include "qapi/qmp/qbool.h"
#include "libqtest.h"
static char *get_cpu0_qom_path(void)
@@ -34,6 +35,15 @@ static QObject *qom_get(const char *path, const char *prop)
return ret;
}
+static bool qom_get_bool(const char *path, const char *prop)
+{
+ QBool *value = qobject_to_qbool(qom_get(path, prop));
+ bool b = qbool_get_bool(value);
+
+ QDECREF(value);
+ return b;
+}
+
typedef struct CpuidTestArgs {
const char *cmdline;
const char *property;
@@ -66,10 +76,44 @@ static void add_cpuid_test(const char *name, const char *cmdline,
qtest_add_data_func(name, args, test_cpuid_prop);
}
+static void test_plus_minus(void)
+{
+ char *path;
+
+ /* Rules:
+ * 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();
+
+ g_assert_false(qom_get_bool(path, "fpu"));
+ g_assert_false(qom_get_bool(path, "mce"));
+ g_assert_true(qom_get_bool(path, "cx8"));
+
+ /* Test both the original and the alias feature names: */
+ g_assert_true(qom_get_bool(path, "sse4-1"));
+ g_assert_true(qom_get_bool(path, "sse4.1"));
+
+ g_assert_true(qom_get_bool(path, "sse4-2"));
+ g_assert_true(qom_get_bool(path, "sse4.2"));
+
+ qtest_end();
+ g_free(path);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
+ qtest_add_func("x86/cpuid/parsing-plus-minus", test_plus_minus);
+
/* Original level values for CPU models: */
add_cpuid_test("x86/cpuid/phenom/level",
"-cpu phenom", "level", 5);
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 02/12] target-i386: List CPU models using subclass list
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 01/12] tests: Add test case for x86 feature parsing compatibility Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 03/12] target-i386: Disable VME by default with TCG Eduardo Habkost
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
Instead of using the builtin_x86_defs array, use the QOM subclass
list to list CPU models on "-cpu ?" and "query-cpu-definitions".
Signed-off-by: Andreas Färber <afaerber@suse.de>
[ehabkost: copied code from a patch by Andreas:
"target-i386: QOM'ify CPU", from March 2012]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu-qom.h | 4 ++
target-i386/cpu.c | 103 ++++++++++++++++++++++++++++++++++++--------------
2 files changed, 78 insertions(+), 29 deletions(-)
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 5dde658..e724004 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -63,6 +63,10 @@ typedef struct X86CPUClass {
bool kvm_required;
+ /* Optional description of CPU model.
+ * If unavailable, cpu_def->model_id is used */
+ const char *model_description;
+
DeviceRealize parent_realize;
void (*parent_reset)(CPUState *cpu);
} X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0807e92..ac3646e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1628,6 +1628,9 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
cpu_x86_fill_model_id(host_cpudef.model_id);
xcc->cpu_def = &host_cpudef;
+ xcc->model_description =
+ "KVM processor with all supported host features "
+ "(only available in KVM mode)";
/* level, xlevel, xlevel2, and the feature words are initialized on
* instance_init, because they require KVM to be initialized.
@@ -2098,23 +2101,62 @@ static void listflags(FILE *f, fprintf_function print, const char **featureset)
}
}
-/* generate CPU information. */
+/* Sort alphabetically by type name, listing kvm_required models last. */
+static gint x86_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+ ObjectClass *class_a = (ObjectClass *)a;
+ ObjectClass *class_b = (ObjectClass *)b;
+ X86CPUClass *cc_a = X86_CPU_CLASS(class_a);
+ X86CPUClass *cc_b = X86_CPU_CLASS(class_b);
+ const char *name_a, *name_b;
+
+ if (cc_a->kvm_required != cc_b->kvm_required) {
+ /* kvm_required items go last */
+ return cc_a->kvm_required ? 1 : -1;
+ } else {
+ name_a = object_class_get_name(class_a);
+ name_b = object_class_get_name(class_b);
+ return strcmp(name_a, name_b);
+ }
+}
+
+static GSList *get_sorted_cpu_model_list(void)
+{
+ GSList *list = object_class_get_list(TYPE_X86_CPU, false);
+ list = g_slist_sort(list, x86_cpu_list_compare);
+ return list;
+}
+
+static void x86_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ ObjectClass *oc = data;
+ X86CPUClass *cc = X86_CPU_CLASS(oc);
+ CPUListState *s = user_data;
+ char *name = x86_cpu_class_get_model_name(cc);
+ const char *desc = cc->model_description;
+ if (!desc) {
+ desc = cc->cpu_def->model_id;
+ }
+
+ (*s->cpu_fprintf)(s->file, "x86 %16s %-48s\n",
+ name, desc);
+ g_free(name);
+}
+
+/* list available CPU models and flags */
void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
{
- X86CPUDefinition *def;
- char buf[256];
int i;
+ CPUListState s = {
+ .file = f,
+ .cpu_fprintf = cpu_fprintf,
+ };
+ GSList *list;
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
- def = &builtin_x86_defs[i];
- snprintf(buf, sizeof(buf), "%s", def->name);
- (*cpu_fprintf)(f, "x86 %16s %-48s\n", buf, def->model_id);
- }
-#ifdef CONFIG_KVM
- (*cpu_fprintf)(f, "x86 %16s %-48s\n", "host",
- "KVM processor with all supported host features "
- "(only available in KVM mode)");
-#endif
+ (*cpu_fprintf)(f, "Available CPUs:\n");
+ list = get_sorted_cpu_model_list();
+ g_slist_foreach(list, x86_cpu_list_entry, &s);
+ g_slist_free(list);
(*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
@@ -2126,26 +2168,29 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
}
}
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
{
- CpuDefinitionInfoList *cpu_list = NULL;
- X86CPUDefinition *def;
- int i;
+ ObjectClass *oc = data;
+ X86CPUClass *cc = X86_CPU_CLASS(oc);
+ CpuDefinitionInfoList **cpu_list = user_data;
+ CpuDefinitionInfoList *entry;
+ CpuDefinitionInfo *info;
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
- CpuDefinitionInfoList *entry;
- CpuDefinitionInfo *info;
-
- def = &builtin_x86_defs[i];
- info = g_malloc0(sizeof(*info));
- info->name = g_strdup(def->name);
+ info = g_malloc0(sizeof(*info));
+ info->name = x86_cpu_class_get_model_name(cc);
- entry = g_malloc0(sizeof(*entry));
- entry->value = info;
- entry->next = cpu_list;
- cpu_list = entry;
- }
+ entry = g_malloc0(sizeof(*entry));
+ entry->value = info;
+ entry->next = *cpu_list;
+ *cpu_list = entry;
+}
+CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+{
+ CpuDefinitionInfoList *cpu_list = NULL;
+ GSList *list = get_sorted_cpu_model_list();
+ g_slist_foreach(list, x86_cpu_definition_entry, &cpu_list);
+ g_slist_free(list);
return cpu_list;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 03/12] target-i386: Disable VME by default with TCG
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 01/12] tests: Add test case for x86 feature parsing compatibility Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 02/12] target-i386: List CPU models using subclass list Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 04/12] target-i386: Register aliases for feature names with underscores Eduardo Habkost
` (9 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
VME is already disabled automatically when using TCG. So, instead
of pretending it is there when reporting CPU model data on
query-cpu-* QMP commands (making every CPU model to be reported
as not runnable), we can disable it by default on all CPU models
when using TCG.
Do that by adding a tcg_default_props array that will work like
kvm_default_props.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ac3646e..3d3f64e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1550,6 +1550,14 @@ static PropValue kvm_default_props[] = {
{ NULL, NULL },
};
+/* TCG-specific defaults that override all CPU models when using TCG
+ */
+static PropValue tcg_default_props[] = {
+ { "vme", "off" },
+ { NULL, NULL },
+};
+
+
void x86_cpu_change_kvm_default(const char *prop, const char *value)
{
PropValue *pv;
@@ -2283,6 +2291,8 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
}
x86_cpu_apply_props(cpu, kvm_default_props);
+ } else if (tcg_enabled()) {
+ x86_cpu_apply_props(cpu, tcg_default_props);
}
env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 04/12] target-i386: Register aliases for feature names with underscores
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (2 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 03/12] target-i386: Disable VME by default with TCG Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 19:56 ` [Qemu-devel] [libvirt] " Eric Blake
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 05/12] target-i386: Make plus_features/minus_features QOM-based Eduardo Habkost
` (8 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
Registering the actual names containing underscores as aliases
will allow management software to be aware that the old
compatibility names are suported, and will make feat2prop() calls
unnecessary when using feature names.
Also, this will help us avoid making the code support underscores
on feature names that never had them in the first place. e.g.
"+tsc_deadline" was never supported and doesn't need to be
translated to "+tsc-deadline".
In other word: this will require less magic translation of
strings, and simple 1:1 match between the config options and
actual QOM properties.
Note that the underscores are still present in the
FeatureWordInfo::feat_names arrays, because
add_flagname_to_bitmaps() needs them to be kept. The next patches
will remove add_flagname_to_bitmaps() and will allow us to
finally remove the aliases from feat_names.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3d3f64e..6b5bf59 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3455,6 +3455,28 @@ static void x86_cpu_initfn(Object *obj)
}
}
+ 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);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [libvirt] [PATCH v5 04/12] target-i386: Register aliases for feature names with underscores
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 04/12] target-i386: Register aliases for feature names with underscores Eduardo Habkost
@ 2016-09-30 19:56 ` Eric Blake
2016-09-30 20:59 ` Eduardo Habkost
0 siblings, 1 reply; 18+ messages in thread
From: Eric Blake @ 2016-09-30 19:56 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]
On 09/30/2016 01:49 PM, Eduardo Habkost wrote:
> Registering the actual names containing underscores as aliases
> will allow management software to be aware that the old
> compatibility names are suported, and will make feat2prop() calls
s/suported/supported/
> unnecessary when using feature names.
>
> Also, this will help us avoid making the code support underscores
> on feature names that never had them in the first place. e.g.
> "+tsc_deadline" was never supported and doesn't need to be
> translated to "+tsc-deadline".
>
> In other word: this will require less magic translation of
> strings, and simple 1:1 match between the config options and
> actual QOM properties.
>
> Note that the underscores are still present in the
> FeatureWordInfo::feat_names arrays, because
> add_flagname_to_bitmaps() needs them to be kept. The next patches
> will remove add_flagname_to_bitmaps() and will allow us to
> finally remove the aliases from feat_names.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [libvirt] [PATCH v5 04/12] target-i386: Register aliases for feature names with underscores
2016-09-30 19:56 ` [Qemu-devel] [libvirt] " Eric Blake
@ 2016-09-30 20:59 ` Eduardo Habkost
0 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 20:59 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, libvir-list, dahi, Paolo Bonzini, Igor Mammedov,
Jiri Denemark, Richard Henderson
On Fri, Sep 30, 2016 at 02:56:42PM -0500, Eric Blake wrote:
> On 09/30/2016 01:49 PM, Eduardo Habkost wrote:
> > Registering the actual names containing underscores as aliases
> > will allow management software to be aware that the old
> > compatibility names are suported, and will make feat2prop() calls
>
> s/suported/supported/
Fixed, thanks.
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v5 05/12] target-i386: Make plus_features/minus_features QOM-based
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (3 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 04/12] target-i386: Register aliases for feature names with underscores Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-11-25 14:51 ` Markus Armbruster
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 06/12] target-i386: Remove underscores from feat_names arrays Eduardo Habkost
` (7 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
Instead of using custom feature name lookup code for
plus_features/minus_features, save the property names used in
"[+-]feature" and use object_property_set_bool() to set them.
We don't need a feat2prop() call because we now have alias
properties for the old names containing underscores.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v4 -> v5:
* Removed feat2prop() call, as we now have property aliases for
the old names containing underscores
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 106 +++++++++++-------------------------------------------
1 file changed, 20 insertions(+), 86 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6b5bf59..cef4ecd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -650,85 +650,6 @@ void host_cpuid(uint32_t function, uint32_t count,
*edx = vec[3];
}
-#define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c)))
-
-/* general substring compare of *[s1..e1) and *[s2..e2). sx is start of
- * a substring. ex if !NULL points to the first char after a substring,
- * otherwise the string is assumed to sized by a terminating nul.
- * Return lexical ordering of *s1:*s2.
- */
-static int sstrcmp(const char *s1, const char *e1,
- const char *s2, const char *e2)
-{
- for (;;) {
- if (!*s1 || !*s2 || *s1 != *s2)
- return (*s1 - *s2);
- ++s1, ++s2;
- if (s1 == e1 && s2 == e2)
- return (0);
- else if (s1 == e1)
- return (*s2);
- else if (s2 == e2)
- return (*s1);
- }
-}
-
-/* compare *[s..e) to *altstr. *altstr may be a simple string or multiple
- * '|' delimited (possibly empty) strings in which case search for a match
- * within the alternatives proceeds left to right. Return 0 for success,
- * non-zero otherwise.
- */
-static int altcmp(const char *s, const char *e, const char *altstr)
-{
- const char *p, *q;
-
- for (q = p = altstr; ; ) {
- while (*p && *p != '|')
- ++p;
- if ((q == p && !*s) || (q != p && !sstrcmp(s, e, q, p)))
- return (0);
- if (!*p)
- return (1);
- else
- q = ++p;
- }
-}
-
-/* search featureset for flag *[s..e), if found set corresponding bit in
- * *pval and return true, otherwise return false
- */
-static bool lookup_feature(uint32_t *pval, const char *s, const char *e,
- const char **featureset)
-{
- uint32_t mask;
- const char **ppc;
- bool found = false;
-
- for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc) {
- if (*ppc && !altcmp(s, e, *ppc)) {
- *pval |= mask;
- found = true;
- }
- }
- return found;
-}
-
-static void add_flagname_to_bitmaps(const char *flagname,
- FeatureWordArray words,
- Error **errp)
-{
- FeatureWord w;
- for (w = 0; w < FEATURE_WORDS; w++) {
- FeatureWordInfo *wi = &feature_word_info[w];
- if (lookup_feature(&words[w], flagname, NULL, wi->feat_names)) {
- break;
- }
- }
- if (w == FEATURE_WORDS) {
- error_setg(errp, "CPU feature %s not found", flagname);
- }
-}
-
/* CPU class name definitions: */
#define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
@@ -2015,8 +1936,7 @@ static inline void feat2prop(char *s)
* feat=on|feat even if the later is parsed after +-feat
* (i.e. "-x2apic,x2apic=on" will result in x2apic disabled)
*/
-static FeatureWordArray plus_features = { 0 };
-static FeatureWordArray minus_features = { 0 };
+static GList *plus_features, *minus_features;
/* Parse "+feature,-feature,feature=foo" CPU feature string
*/
@@ -2047,10 +1967,12 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
/* Compatibility syntax: */
if (featurestr[0] == '+') {
- add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err);
+ plus_features = g_list_append(plus_features,
+ g_strdup(featurestr + 1));
continue;
} else if (featurestr[0] == '-') {
- add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err);
+ minus_features = g_list_append(minus_features,
+ g_strdup(featurestr + 1));
continue;
}
@@ -3066,6 +2988,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
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);
@@ -3091,9 +3014,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
}
}
- for (w = 0; w < FEATURE_WORDS; w++) {
- cpu->env.features[w] |= plus_features[w];
- cpu->env.features[w] &= ~minus_features[w];
+ for (l = plus_features; l; l = l->next) {
+ const char *prop = l->data;
+ object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
+ if (local_err) {
+ goto out;
+ }
+ }
+
+ for (l = minus_features; l; l = l->next) {
+ const char *prop = l->data;
+ object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
+ if (local_err) {
+ goto out;
+ }
}
if (!kvm_enabled() || !cpu->expose_kvm) {
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v5 05/12] target-i386: Make plus_features/minus_features QOM-based
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 05/12] target-i386: Make plus_features/minus_features QOM-based Eduardo Habkost
@ 2016-11-25 14:51 ` Markus Armbruster
0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2016-11-25 14:51 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, libvir-list, dahi, Paolo Bonzini, Igor Mammedov,
Jiri Denemark, Richard Henderson
Eduardo Habkost <ehabkost@redhat.com> writes:
> Instead of using custom feature name lookup code for
> plus_features/minus_features, save the property names used in
> "[+-]feature" and use object_property_set_bool() to set them.
>
> We don't need a feat2prop() call because we now have alias
> properties for the old names containing underscores.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v4 -> v5:
> * Removed feat2prop() call, as we now have property aliases for
> the old names containing underscores
>
> Changes series v3 -> v4:
> * New patch added to series
> ---
> target-i386/cpu.c | 106 +++++++++++-------------------------------------------
> 1 file changed, 20 insertions(+), 86 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 6b5bf59..cef4ecd 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
[...]
> @@ -2047,10 +1967,12 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
>
> /* Compatibility syntax: */
> if (featurestr[0] == '+') {
> - add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err);
> + plus_features = g_list_append(plus_features,
> + g_strdup(featurestr + 1));
> continue;
> } else if (featurestr[0] == '-') {
> - add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err);
> + minus_features = g_list_append(minus_features,
> + g_strdup(featurestr + 1));
> continue;
> }
>
Since removes the only assignments to local_err other than its
initialization to null, it can't become non-null anymore. Coverity
points out:
*** CID 1365201: Possible Control flow issues (DEADCODE)
/target-i386/cpu.c: 2050 in x86_cpu_parse_featurestr()
2044 prop->value = g_strdup(val);
2045 prop->errp = &error_fatal;
2046 qdev_prop_register_global(prop);
2047 }
2048
2049 if (local_err) {
>>> CID 1365201: Possible Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "error_propagate(errp, local...".
2050 error_propagate(errp, local_err);
2051 }
2052 }
2053
2054 static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
2055 static int x86_cpu_filter_features(X86CPU *cpu);
[...]
Please clean this up.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v5 06/12] target-i386: Remove underscores from feat_names arrays
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (4 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 05/12] target-i386: Make plus_features/minus_features QOM-based Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 07/12] target-i386: Register properties for feature aliases manually Eduardo Habkost
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
Instead of translating the feature name entries when adding
property names, store the actual property names in the feature
name array.
For reference, here is the full list of functions that use
FeatureWordInfo::feat_names:
* x86_cpu_get_migratable_flags(): not affected, as it just
check for non-NULL values.
* report_unavailable_features(): informative only. It will
start printing feature names with hyphens.
* x86_cpu_list(): informative only. It will start printing
feature names with hyphens
* x86_cpu_register_feature_bit_props(): not affected, as it
was already calling feat2prop(). Now we can remove the
feat2prop() calls safely.
So, the only user-visible effect of this patch are the new names
being used in help and error messages for users.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v4 -> v5:
* Remove the feat2prop() call from the alias registration
loop, too
* Commit message update to enumerate all code that uses
feat_names
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cef4ecd..ec65aef 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -279,11 +279,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
[FEAT_1_ECX] = {
.feat_names = {
"pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
- "ds_cpl", "vmx", "smx", "est",
+ "ds-cpl", "vmx", "smx", "est",
"tm2", "ssse3", "cid", NULL,
"fma", "cx16", "xtpr", "pdcm",
- NULL, "pcid", "dca", "sse4.1|sse4_1",
- "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
+ NULL, "pcid", "dca", "sse4.1|sse4-1",
+ "sse4.2|sse4-2", "x2apic", "movbe", "popcnt",
"tsc-deadline", "aes", "xsave", "osxsave",
"avx", "f16c", "rdrand", "hypervisor",
},
@@ -303,7 +303,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
"nx|xd", NULL, "mmxext", NULL /* mmx */,
- NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb", "rdtscp",
+ NULL /* fxsr */, "fxsr-opt|ffxsr", "pdpe1gb", "rdtscp",
NULL, "lm|i64", "3dnowext", "3dnow",
},
.cpuid_eax = 0x80000001, .cpuid_reg = R_EDX,
@@ -311,13 +311,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
[FEAT_8000_0001_ECX] = {
.feat_names = {
- "lahf_lm", "cmp_legacy", "svm", "extapic",
+ "lahf-lm", "cmp-legacy", "svm", "extapic",
"cr8legacy", "abm", "sse4a", "misalignsse",
"3dnowprefetch", "osvw", "ibs", "xop",
"skinit", "wdt", NULL, "lwp",
- "fma4", "tce", NULL, "nodeid_msr",
- NULL, "tbm", "topoext", "perfctr_core",
- "perfctr_nb", NULL, NULL, NULL,
+ "fma4", "tce", NULL, "nodeid-msr",
+ NULL, "tbm", "topoext", "perfctr-core",
+ "perfctr-nb", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
.cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
@@ -339,8 +339,8 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
[FEAT_KVM] = {
.feat_names = {
- "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
- "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt",
+ "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
+ "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
@@ -400,9 +400,9 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
[FEAT_SVM] = {
.feat_names = {
- "npt", "lbrv", "svm_lock", "nrip_save",
- "tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists",
- NULL, NULL, "pause_filter", NULL,
+ "npt", "lbrv", "svm-lock", "nrip-save",
+ "tsc-scale", "vmcb-clean", "flushbyasid", "decodeassists",
+ NULL, NULL, "pause-filter", NULL,
"pfthreshold", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
@@ -414,7 +414,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
[FEAT_7_0_EBX] = {
.feat_names = {
- "fsgsbase", "tsc_adjust", NULL, "bmi1",
+ "fsgsbase", "tsc-adjust", NULL, "bmi1",
"hle", "avx2", NULL, "smep",
"bmi2", "erms", "invpcid", "rtm",
NULL, NULL, "mpx", NULL,
@@ -3332,11 +3332,15 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
names = g_strsplit(fi->feat_names[bitnr], "|", 0);
- feat2prop(names[0]);
+ /* Property names should use "-" instead of "_".
+ * Old names containing underscores are registered as aliases
+ * using object_property_add_alias()
+ */
+ assert(!strchr(names[0], '_'));
x86_cpu_register_bit_prop(cpu, names[0], &cpu->env.features[w], bitnr);
for (i = 1; names[i]; i++) {
- feat2prop(names[i]);
+ assert(!strchr(names[i], '_'));
object_property_add_alias(obj, names[i], obj, names[0],
&error_abort);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 07/12] target-i386: Register properties for feature aliases manually
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (5 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 06/12] target-i386: Remove underscores from feat_names arrays Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 08/12] target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas Eduardo Habkost
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
Instead of keeping the aliases inside the feature name arrays and
require parsing the strings, just register alias properties
manually. This simplifies the code for property registration and
lookup.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v4 -> v5:
* Refresh after the previous patches were changed
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 43 +++++++++++++++++++++----------------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ec65aef..c018122 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -278,12 +278,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
[FEAT_1_ECX] = {
.feat_names = {
- "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
+ "pni" /* Intel,AMD sse3 */, "pclmulqdq", "dtes64", "monitor",
"ds-cpl", "vmx", "smx", "est",
"tm2", "ssse3", "cid", NULL,
"fma", "cx16", "xtpr", "pdcm",
- NULL, "pcid", "dca", "sse4.1|sse4-1",
- "sse4.2|sse4-2", "x2apic", "movbe", "popcnt",
+ NULL, "pcid", "dca", "sse4.1",
+ "sse4.2", "x2apic", "movbe", "popcnt",
"tsc-deadline", "aes", "xsave", "osxsave",
"avx", "f16c", "rdrand", "hypervisor",
},
@@ -302,9 +302,9 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL /* cx8 */, NULL /* apic */, NULL, "syscall",
NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
- "nx|xd", NULL, "mmxext", NULL /* mmx */,
- NULL /* fxsr */, "fxsr-opt|ffxsr", "pdpe1gb", "rdtscp",
- NULL, "lm|i64", "3dnowext", "3dnow",
+ "nx", NULL, "mmxext", NULL /* mmx */,
+ NULL /* fxsr */, "fxsr-opt", "pdpe1gb", "rdtscp",
+ NULL, "lm", "3dnowext", "3dnow",
},
.cpuid_eax = 0x80000001, .cpuid_reg = R_EDX,
.tcg_features = TCG_EXT2_FEATURES,
@@ -3321,31 +3321,22 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
FeatureWord w,
int bitnr)
{
- Object *obj = OBJECT(cpu);
- int i;
- char **names;
FeatureWordInfo *fi = &feature_word_info[w];
+ const char *name = fi->feat_names[bitnr];
- if (!fi->feat_names[bitnr]) {
+ if (!name) {
return;
}
- names = g_strsplit(fi->feat_names[bitnr], "|", 0);
-
/* Property names should use "-" instead of "_".
* Old names containing underscores are registered as aliases
* using object_property_add_alias()
*/
- assert(!strchr(names[0], '_'));
- x86_cpu_register_bit_prop(cpu, names[0], &cpu->env.features[w], bitnr);
-
- for (i = 1; names[i]; i++) {
- assert(!strchr(names[i], '_'));
- object_property_add_alias(obj, names[i], obj, names[0],
- &error_abort);
- }
-
- g_strfreev(names);
+ assert(!strchr(name, '_'));
+ /* aliases don't use "|" delimiters anymore, they are registered
+ * manually using object_property_add_alias() */
+ assert(!strchr(name, '|'));
+ x86_cpu_register_bit_prop(cpu, name, &cpu->env.features[w], bitnr);
}
static void x86_cpu_initfn(Object *obj)
@@ -3393,6 +3384,14 @@ static void x86_cpu_initfn(Object *obj)
}
}
+ 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);
+ object_property_add_alias(obj, "sse4-2", obj, "sse4.2", &error_abort);
+ object_property_add_alias(obj, "xd", obj, "nx", &error_abort);
+ 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);
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 08/12] target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (6 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 07/12] target-i386: Register properties for feature aliases manually Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 09/12] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 UTC (permalink / raw)
To: qemu-devel
Cc: dahi, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Markus Armbruster, Richard Henderson, libvir-list
Instead of treating the FP and SSE bits as special cases, add
them to the x86_ext_save_areas array. This will simplify the code
that calculates the supported xsave components and the size of
the xsave area.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c018122..477990d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -535,6 +535,20 @@ typedef struct ExtSaveArea {
} ExtSaveArea;
static const ExtSaveArea x86_ext_save_areas[] = {
+ [XSTATE_FP_BIT] = {
+ /* x87 FP state component is always enabled if XSAVE is supported */
+ .feature = FEAT_1_ECX, .bits = CPUID_EXT_XSAVE,
+ /* x87 state is in the legacy region of the XSAVE area */
+ .offset = 0,
+ .size = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader),
+ },
+ [XSTATE_SSE_BIT] = {
+ /* SSE state component is always enabled if XSAVE is supported */
+ .feature = FEAT_1_ECX, .bits = CPUID_EXT_XSAVE,
+ /* SSE state is in the legacy region of the XSAVE area */
+ .offset = 0,
+ .size = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader),
+ },
[XSTATE_YMM_BIT] =
{ .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
.offset = offsetof(X86XSaveArea, avx_state),
@@ -568,9 +582,9 @@ static const ExtSaveArea x86_ext_save_areas[] = {
static uint32_t xsave_area_size(uint64_t mask)
{
int i;
- uint64_t ret = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader);
+ uint64_t ret = 0;
- for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+ for (i = 0; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
if ((mask >> i) & 1) {
ret = MAX(ret, esa->offset + esa->size);
@@ -2961,8 +2975,8 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
return;
}
- mask = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
- for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+ mask = 0;
+ for (i = 0; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
if (env->features[esa->feature] & esa->bits) {
mask |= (1ULL << i);
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 09/12] target-i386: Move warning code outside x86_cpu_filter_features()
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (7 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 08/12] target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 10/12] target-i386: x86_cpu_load_features() function Eduardo Habkost
` (3 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 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 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 | 41 ++++++++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 477990d..636e629 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2161,14 +2161,11 @@ 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 int x86_cpu_filter_features(X86CPU *cpu)
+static void 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 =
@@ -2176,15 +2173,22 @@ static int 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]) {
- if (cpu->check_cpuid || cpu->enforce_cpuid) {
- report_unavailable_features(w, 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)
+{
+ 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)
@@ -3080,12 +3084,15 @@ 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;
+ x86_cpu_filter_features(cpu);
+ if (cpu->check_cpuid || cpu->enforce_cpuid) {
+ if (x86_cpu_report_filtered_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;
+ }
}
/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 10/12] target-i386: x86_cpu_load_features() function
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (8 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 09/12] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 11/12] qmp: Add runnability information to query-cpu-definitions Eduardo Habkost
` (2 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 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 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 | 66 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 42 insertions(+), 24 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 636e629..e4b45e3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2991,34 +2991,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
@@ -3085,6 +3064,45 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
}
x86_cpu_filter_features(cpu);
+
+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;
+ }
+
if (cpu->check_cpuid || cpu->enforce_cpuid) {
if (x86_cpu_report_filtered_features(cpu) && cpu->enforce_cpuid) {
error_setg(&local_err,
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v5 11/12] qmp: Add runnability information to query-cpu-definitions
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (9 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 10/12] target-i386: x86_cpu_load_features() function Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-10-07 20:25 ` Eduardo Habkost
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 12/12] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
2016-10-07 18:54 ` [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
12 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 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
Add a new optional field to query-cpu-definitions schema:
"unavailable-features". It will contain a list of QOM properties
that prevent the CPU model from running in the current host.
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
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v3 -> v4:
* Changed doc to "since 2.8" as we missed 2.7
* Reported-by: Eric Blake <eblake@redhat.com>
Changes v2 -> v3:
* Small documentation reword
* Suggested-by: Markus Armbruster <armbru@redhat.com>
Changes v1 -> v2:
* Remove @runnable field, non-empty @unavailable-features is
enough to report CPU model as not runnable.
* Documentation updates:
* Changed to "(since 2.7)";
* Add more details about the exact meaning of
unavailable-features, and what it would mean to see
read-only QOM properties in the list, and that
implementations can return "type" if there's
no extra information available;
---
qapi-schema.json | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index c3dcf11..abb163b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3114,10 +3114,31 @@
# QEMU version, machine type, machine options and accelerator options.
# A static model is always migration-safe. (since 2.8)
#
+# @unavailable-features: #optional List of properties that prevent
+# the CPU model from running in the current
+# host. (since 2.8)
+#
+# @unavailable-features is a list of QOM property names that
+# represent CPU model attributes that prevent the CPU from running.
+# If the QOM property is read-only, that means there's no known
+# way to make the CPU model run in the current host. Implementations
+# that choose not to provide specific information return the
+# property name "type".
+# If the property is read-write, it means that it MAY be possible
+# to run the CPU model in the current host if that property is
+# changed. Management software can use it as hints to suggest or
+# choose an alternative for the user, or just to generate meaningful
+# error messages explaining why the CPU model can't be used.
+# If @unavailable-features is an empty list, the CPU model is
+# runnable using the current host and machine-type.
+# If @unavailable-features is not present, runnability
+# information for the CPU is not available.
+#
# Since: 1.2.0
##
{ 'struct': 'CpuDefinitionInfo',
- 'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool' } }
+ 'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool',
+ '*unavailable-features': [ 'str' ] } }
##
# @query-cpu-definitions:
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v5 11/12] qmp: Add runnability information to query-cpu-definitions
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 11/12] qmp: Add runnability information to query-cpu-definitions Eduardo Habkost
@ 2016-10-07 20:25 ` Eduardo Habkost
0 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-10-07 20:25 UTC (permalink / raw)
To: qemu-devel
Cc: Michael Mueller, Christian Borntraeger, libvir-list,
Markus Armbruster, dahi, Paolo Bonzini, Cornelia Huck,
Igor Mammedov, Jiri Denemark, Richard Henderson
On Fri, Sep 30, 2016 at 03:49:45PM -0300, Eduardo Habkost wrote:
> Add a new optional field to query-cpu-definitions schema:
> "unavailable-features". It will contain a list of QOM properties
> that prevent the CPU model from running in the current host.
>
> 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
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Applied this to x86-next.
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v5 12/12] target-i386: Return runnability information on query-cpu-definitions
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (10 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 11/12] qmp: Add runnability information to query-cpu-definitions Eduardo Habkost
@ 2016-09-30 18:49 ` Eduardo Habkost
2016-10-07 18:54 ` [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:49 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 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 | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e4b45e3..620889f 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,56 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
}
}
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
+
+/* 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;
+ }
+
+ 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 +2193,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] 18+ messages in thread* Re: [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions
2016-09-30 18:49 [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions Eduardo Habkost
` (11 preceding siblings ...)
2016-09-30 18:49 ` [Qemu-devel] [PATCH v5 12/12] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
@ 2016-10-07 18:54 ` Eduardo Habkost
12 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2016-10-07 18:54 UTC (permalink / raw)
To: qemu-devel
Cc: Michael Mueller, Christian Borntraeger, libvir-list,
Markus Armbruster, dahi, Paolo Bonzini, Cornelia Huck,
Igor Mammedov, Jiri Denemark, Richard Henderson
On Fri, Sep 30, 2016 at 03:49:34PM -0300, Eduardo Habkost wrote:
[...]
> Eduardo Habkost (12):
> tests: Add test case for x86 feature parsing compatibility
> target-i386: List CPU models using subclass list
> target-i386: Disable VME by default with TCG
> target-i386: Register aliases for feature names with underscores
> target-i386: Make plus_features/minus_features QOM-based
> target-i386: Remove underscores from feat_names arrays
> target-i386: Register properties for feature aliases manually
> target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas
Patches 1-8 (above) were applied to x86-next. I will send v6 of
9-12 (below), with a small change.
> target-i386: Move warning code outside x86_cpu_filter_features()
> target-i386: x86_cpu_load_features() function
> qmp: Add runnability information to query-cpu-definitions
> target-i386: Return runnability information on query-cpu-definitions
>
> qapi-schema.json | 23 +-
> target-i386/cpu-qom.h | 4 +
> target-i386/cpu.c | 510 ++++++++++++++++++++++++++----------------
> tests/test-x86-cpuid-compat.c | 44 ++++
> 4 files changed, 388 insertions(+), 193 deletions(-)
>
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread