* [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:31 ` Jonathan Neuschäfer
2016-09-30 7:55 ` Paolo Bonzini
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 02/11] target-i386: List CPU models using subclass list Eduardo Habkost
` (9 subsequent siblings)
10 siblings, 2 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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 wlil be kept.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v3 -> v4:
* New patch added to series
---
tests/test-x86-cpuid-compat.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 83162a4..7cff2b5 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,39 @@ 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:
+ * "-foo" overrides "+foo"
+ * "[+-]foo" overrides "foo=..."
+ * "foo_bar" should be translated to "foo-bar"
+ */
+ 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] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility Eduardo Habkost
@ 2016-09-29 21:31 ` Jonathan Neuschäfer
2016-09-30 7:55 ` Paolo Bonzini
1 sibling, 0 replies; 22+ messages in thread
From: Jonathan Neuschäfer @ 2016-09-29 21:31 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, libvir-list, Markus Armbruster, dahi, Paolo Bonzini,
Igor Mammedov, Jiri Denemark, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 270 bytes --]
On Thu, Sep 29, 2016 at 06:14:49PM -0300, Eduardo Habkost wrote:
> Add a new test case to ensure the existing behavior of the
> feature parsing code wlil be kept.
s/wlil/will/
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility Eduardo Habkost
2016-09-29 21:31 ` Jonathan Neuschäfer
@ 2016-09-30 7:55 ` Paolo Bonzini
2016-09-30 8:08 ` Jiri Denemark
2016-09-30 18:33 ` Eduardo Habkost
1 sibling, 2 replies; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-30 7:55 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: dahi, Igor Mammedov, Jiri Denemark, Markus Armbruster,
Richard Henderson, libvir-list
On 29/09/2016 23:14, Eduardo Habkost wrote:
> + * "-foo" overrides "+foo"
> + * "[+-]foo" overrides "foo=..."
Is this something that people are actually using? Can we detect it and
deprecate it in 2.8, and drop it in 2.9?
Paolo
> + * "foo_bar" should be translated to "foo-bar"
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility
2016-09-30 7:55 ` Paolo Bonzini
@ 2016-09-30 8:08 ` Jiri Denemark
2016-09-30 18:33 ` Eduardo Habkost
1 sibling, 0 replies; 22+ messages in thread
From: Jiri Denemark @ 2016-09-30 8:08 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Eduardo Habkost, qemu-devel, dahi, Igor Mammedov,
Markus Armbruster, Richard Henderson, libvir-list
On Fri, Sep 30, 2016 at 09:55:33 +0200, Paolo Bonzini wrote:
>
>
> On 29/09/2016 23:14, Eduardo Habkost wrote:
> > + * "-foo" overrides "+foo"
> > + * "[+-]foo" overrides "foo=..."
>
> Is this something that people are actually using? Can we detect it and
> deprecate it in 2.8, and drop it in 2.9?
Libvirt uses -cpu Model,+foo,-bar style, but we do not mix mix -foo and
+foo, or even [+-]foo and foo= if this is what you asked.
Jirka
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility
2016-09-30 7:55 ` Paolo Bonzini
2016-09-30 8:08 ` Jiri Denemark
@ 2016-09-30 18:33 ` Eduardo Habkost
2016-09-30 21:07 ` Paolo Bonzini
1 sibling, 1 reply; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-30 18:33 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, libvir-list, Markus Armbruster, dahi, Igor Mammedov,
Jiri Denemark, Richard Henderson
On Fri, Sep 30, 2016 at 09:55:33AM +0200, Paolo Bonzini wrote:
>
>
> On 29/09/2016 23:14, Eduardo Habkost wrote:
> > + * "-foo" overrides "+foo"
> > + * "[+-]foo" overrides "foo=..."
>
> Is this something that people are actually using? Can we detect it and
> deprecate it in 2.8, and drop it in 2.9?
We can, but I would like to keep the test cases there in 2.8, at
least. I will update the test case to note that this is legacy
behavior that we plan to remove in 2.9, and send a separate
follow-up patch to detect when people mix both formats.
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility
2016-09-30 18:33 ` Eduardo Habkost
@ 2016-09-30 21:07 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-30 21:07 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, libvir-list, Markus Armbruster, dahi, Igor Mammedov,
Jiri Denemark, Richard Henderson
On 30/09/2016 20:33, Eduardo Habkost wrote:
> On Fri, Sep 30, 2016 at 09:55:33AM +0200, Paolo Bonzini wrote:
>>
>>
>> On 29/09/2016 23:14, Eduardo Habkost wrote:
>>> + * "-foo" overrides "+foo"
>>> + * "[+-]foo" overrides "foo=..."
>>
>> Is this something that people are actually using? Can we detect it and
>> deprecate it in 2.8, and drop it in 2.9?
>
> We can, but I would like to keep the test cases there in 2.8, at
> least. I will update the test case to note that this is legacy
> behavior that we plan to remove in 2.9, and send a separate
> follow-up patch to detect when people mix both formats.
Yes, of course! Sounds like a very good plan.
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v4 02/11] target-i386: List CPU models using subclass list
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 03/11] target-i386: Disable VME by default with TCG Eduardo Habkost
` (8 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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] 22+ messages in thread* [Qemu-devel] [PATCH v4 03/11] target-i386: Disable VME by default with TCG
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 01/11] tests: Add test case for x86 feature parsing compatibility Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 02/11] target-i386: List CPU models using subclass list Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 04/11] target-i386: Make plus_features/minus_features QOM-based Eduardo Habkost
` (7 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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] 22+ messages in thread* [Qemu-devel] [PATCH v4 04/11] target-i386: Make plus_features/minus_features QOM-based
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (2 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 03/11] target-i386: Disable VME by default with TCG Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names Eduardo Habkost
` (6 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 108 +++++++++++-------------------------------------------
1 file changed, 22 insertions(+), 86 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3d3f64e..4eaec0e 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,14 @@ 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);
+ feat2prop(featurestr + 1);
+ 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);
+ feat2prop(featurestr + 1);
+ minus_features = g_list_append(minus_features,
+ g_strdup(featurestr + 1));
continue;
}
@@ -3066,6 +2990,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 +3016,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] 22+ messages in thread* [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (3 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 04/11] target-i386: Make plus_features/minus_features QOM-based Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-30 7:29 ` Jiri Denemark
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 06/11] target-i386: Register properties for feature aliases manually Eduardo Habkost
` (5 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4eaec0e..7795a7c 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,
@@ -3334,7 +3334,8 @@ 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 "_" */
+ assert(!strchr(names[0], '_'));
x86_cpu_register_bit_prop(cpu, names[0], &cpu->env.features[w], bitnr);
for (i = 1; names[i]; i++) {
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names Eduardo Habkost
@ 2016-09-30 7:29 ` Jiri Denemark
2016-09-30 13:42 ` Eduardo Habkost
0 siblings, 1 reply; 22+ messages in thread
From: Jiri Denemark @ 2016-09-30 7:29 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, dahi, Paolo Bonzini, Igor Mammedov, Markus Armbruster,
Richard Henderson, libvir-list
On Thu, Sep 29, 2016 at 18:14:53 -0300, Eduardo Habkost wrote:
> Instead of translating the feature name entries when adding
> property names, store the actual property names in the feature
> name array.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes series v3 -> v4:
> * New patch added to series
> ---
> target-i386/cpu.c | 31 ++++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4eaec0e..7795a7c 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",
> },
It wasn't quite obvious to me where this means we can't use the names
with underscores when talking to QEMU. So I tried it and apparently
underscores are just silently translated to dashes. It's backward
compatible this way. However, QEMU will always give us the names with
dashes, which means we have even more differences between libvirt's
feature names and QEMU's feature names. So assuming we'll have an
interface for querying supported CPU properties (and their aliases),
shouldn't the old underscore names be added as aliases? This way, we
could actually know that "ds-cpl" means "ds_cpl".
Jirka
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names
2016-09-30 7:29 ` Jiri Denemark
@ 2016-09-30 13:42 ` Eduardo Habkost
0 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-30 13:42 UTC (permalink / raw)
To: qemu-devel, dahi, Paolo Bonzini, Igor Mammedov, Markus Armbruster,
Richard Henderson, libvir-list
On Fri, Sep 30, 2016 at 09:29:58AM +0200, Jiri Denemark wrote:
> On Thu, Sep 29, 2016 at 18:14:53 -0300, Eduardo Habkost wrote:
> > Instead of translating the feature name entries when adding
> > property names, store the actual property names in the feature
> > name array.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes series v3 -> v4:
> > * New patch added to series
> > ---
> > target-i386/cpu.c | 31 ++++++++++++++++---------------
> > 1 file changed, 16 insertions(+), 15 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 4eaec0e..7795a7c 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",
> > },
>
> It wasn't quite obvious to me where this means we can't use the names
> with underscores when talking to QEMU. So I tried it and apparently
> underscores are just silently translated to dashes. It's backward
> compatible this way. However, QEMU will always give us the names with
> dashes, which means we have even more differences between libvirt's
> feature names and QEMU's feature names. So assuming we'll have an
> interface for querying supported CPU properties (and their aliases),
> shouldn't the old underscore names be added as aliases? This way, we
> could actually know that "ds-cpl" means "ds_cpl".
Good idea. Instead of translating property names silently we can
add aliases for the ones where underscores were actually used. I
will look into it.
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v4 06/11] target-i386: Register properties for feature aliases manually
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (4 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 07/11] target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas Eduardo Habkost
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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 property registration code and will
simplify code that needs to look up property names for CPUID
bits.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v3 -> v4:
* New patch added to series
---
target-i386/cpu.c | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7795a7c..c013ed0 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,
@@ -3323,28 +3323,19 @@ 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 "_" */
- 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]);
- 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)
@@ -3392,6 +3383,15 @@ 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);
+ 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);
+
x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v4 07/11] target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (5 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 06/11] target-i386: Register properties for feature aliases manually Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 08/11] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
` (3 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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 c013ed0..b36388e 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);
@@ -2963,8 +2977,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] 22+ messages in thread* [Qemu-devel] [PATCH v4 08/11] target-i386: Move warning code outside x86_cpu_filter_features()
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (6 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 07/11] target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 09/11] target-i386: x86_cpu_load_features() function Eduardo Habkost
` (2 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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 b36388e..b25657b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2163,14 +2163,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 =
@@ -2178,15 +2175,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)
@@ -3082,12 +3086,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] 22+ messages in thread* [Qemu-devel] [PATCH v4 09/11] target-i386: x86_cpu_load_features() function
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (7 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 08/11] target-i386: Move warning code outside x86_cpu_filter_features() Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-30 8:00 ` Paolo Bonzini
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 10/11] qmp: Add runnability information to query-cpu-definitions Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 11/11] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
10 siblings, 1 reply; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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 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 b25657b..e099892 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2993,34 +2993,14 @@ 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 configureured 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
@@ -3087,6 +3067,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] 22+ messages in thread* [Qemu-devel] [PATCH v4 10/11] qmp: Add runnability information to query-cpu-definitions
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (8 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 09/11] target-i386: x86_cpu_load_features() function Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 11/11] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
10 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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] 22+ messages in thread* [Qemu-devel] [PATCH v4 11/11] target-i386: Return runnability information on query-cpu-definitions
2016-09-29 21:14 [Qemu-devel] [PATCH v4 00/11] Add runnability info to query-cpu-definitions Eduardo Habkost
` (9 preceding siblings ...)
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 10/11] qmp: Add runnability information to query-cpu-definitions Eduardo Habkost
@ 2016-09-29 21:14 ` Eduardo Habkost
2016-09-30 8:02 ` Paolo Bonzini
10 siblings, 1 reply; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-29 21:14 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 e099892..4dd3aee 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
@@ -2032,6 +2053,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)
@@ -2124,6 +2195,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] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 11/11] target-i386: Return runnability information on query-cpu-definitions
2016-09-29 21:14 ` [Qemu-devel] [PATCH v4 11/11] target-i386: Return runnability information on query-cpu-definitions Eduardo Habkost
@ 2016-09-30 8:02 ` Paolo Bonzini
2016-09-30 13:40 ` Eduardo Habkost
0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-30 8:02 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: dahi, Igor Mammedov, Jiri Denemark, Markus Armbruster,
Richard Henderson, libvir-list
On 29/09/2016 23:14, Eduardo Habkost wrote:
> +/* 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];
> +}
> +
Could this be used to replace migratable_features?
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 11/11] target-i386: Return runnability information on query-cpu-definitions
2016-09-30 8:02 ` Paolo Bonzini
@ 2016-09-30 13:40 ` Eduardo Habkost
0 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2016-09-30 13:40 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, dahi, Igor Mammedov, Jiri Denemark, Markus Armbruster,
Richard Henderson, libvir-list
On Fri, Sep 30, 2016 at 10:02:49AM +0200, Paolo Bonzini wrote:
> On 29/09/2016 23:14, Eduardo Habkost wrote:
> > +/* 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];
> > +}
> > +
>
> Could this be used to replace migratable_features?
It can. I will do it in a follow-up patch.
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread