[Qemu-devel] [PATCH v2 0/6] Add runnability info to query-cpu-definitions

[Qemu-devel] [PATCH v2 0/6] Add runnability info to query-cpu-definitions

[Eduardo Habkost]

This series extends query-cpu-definitions to include an extra
field: "unavailable-features". The new field can be used to find
out reasons that prevent the CPU model from running in the
current host.

This will return information based on the current machine and
accelerator only. In the future we may extend these mechanisms to
allow querying other machines and other accelerators without
restarting QEMU, but it will require some reorganization of
QEMU's main code.

This series is based on my 'x86-next' branch, at:
  git://github.com/ehabkost/qemu.git x86-next

Changes v1 -> v2:
* Fixed documentation to say "(since 2.7)"
* Removed @runnable field, improved documentation

Example command output:

  { "return": [
      {
        "unavailable-features": [ "kvm" ],
         "name": "host"
      },
      {
        "unavailable-features": [],
        "name": "qemu64"
      },
      {
        "unavailable-features": [],
        "name": "qemu32"
      },
      {
        "unavailable-features": ["npt", "fxsr-opt", "vme"],
        "name": "phenom"
      },
      {
        "unavailable-features": ["vme"],
        "name": "pentium3"
      },
      {
        "unavailable-features": ["vme"],
        "name": "pentium2"
      },
      {
        "unavailable-features": ["vme"],
        "name": "pentium"
      },
      {
        "unavailable-features": ["vme"],
        "name": "n270"
      },
      {
        "unavailable-features": ["vme"],
        "name": "kvm64"
      },
      {
        "unavailable-features": ["vme"],
        "name": "kvm32"
      },
      {
        "unavailable-features": ["vme"],
        "name": "coreduo"
      },
      {
        "unavailable-features": ["vme"],
        "name": "core2duo"
      },
      {
        "unavailable-features": ["vme"],
        "name": "athlon"
      },
      {
        "unavailable-features": ["vme"],
        "name": "Westmere"
      },
      {
        "unavailable-features": ["xsavec", "3dnowprefetch", "rdseed", "rtm", "invpcid", "erms", "avx2", "hle", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
        "name": "Skylake-Client"
      },
      {
        "unavailable-features": ["avx", "tsc-deadline", "x2apic", "vme"],
        "name": "SandyBridge"
      },
      {
        "unavailable-features": ["vme"],
        "name": "Penryn"
      },
      {
        "unavailable-features": ["tbm", "fma4", "xop", "3dnowprefetch", "misalignsse", "f16c", "avx", "fma", "vme"],
        "name": "Opteron_G5"
      },
      {
        "unavailable-features": ["fma4", "xop", "3dnowprefetch", "misalignsse", "avx", "vme"],
        "name": "Opteron_G4"
      },
      {
        "unavailable-features": ["misalignsse", "vme"],
        "name": "Opteron_G3"
      },
      {
        "unavailable-features": ["vme"],
        "name": "Opteron_G2"
      },
      {
        "unavailable-features": ["vme"],
        "name": "Opteron_G1"
      },
      {
        "unavailable-features": ["vme"],
        "name": "Nehalem"
      },
      {
        "unavailable-features": ["erms", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "vme"],
        "name": "IvyBridge"
      },
      {
        "unavailable-features": ["rtm", "invpcid", "erms", "avx2", "hle", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
        "name": "Haswell"
      },
      {
        "unavailable-features": ["invpcid", "erms", "avx2", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
        "name": "Haswell-noTSX"
      },
      {
        "unavailable-features": ["vme"],
        "name": "Conroe"
      },
      {
        "unavailable-features": ["3dnowprefetch", "rdseed", "rtm", "invpcid", "erms", "avx2", "hle", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
        "name": "Broadwell"
      },
      {
        "unavailable-features": ["3dnowprefetch", "rdseed", "invpcid", "erms", "avx2", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
        "name": "Broadwell-noTSX"
      },
      {
        "unavailable-features": ["vme"],
        "name": "486"
      }
  ]}

Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Michael Mueller <mimu@linux.vnet.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Jiri Denemark <jdenemar@redhat.com>
Cc: libvir-list@redhat.com

Eduardo Habkost (6):
  target-i386: List CPU models using subclass list
  target-i386: Move warning code outside x86_cpu_filter_features()
  target-i386: Define CPUID filtering functions before x86_cpu_list()
  qmp: Add runnability information to query-cpu-definitions
  target-i386: Use "-" instead of "_" on all feature names
  target-i386: Return runnability information on query-cpu-definitions

 qapi-schema.json      |  23 ++++-
 target-i386/cpu-qom.h |   4 +
 target-i386/cpu.c     | 262 +++++++++++++++++++++++++++++++++++---------------
 3 files changed, 209 insertions(+), 80 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH v2 1/6] target-i386: List CPU models using subclass list

[Eduardo Habkost]

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     | 111 +++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 86 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 a62d731..aaa239a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -678,6 +678,14 @@ static ObjectClass *x86_cpu_class_by_name(const char *cpu_model)
     return oc;
 }
 
+static char *x86_cpu_class_get_model_name(X86CPUClass *cc)
+{
+    const char *class_name = object_class_get_name(OBJECT_CLASS(cc));
+    assert(g_str_has_suffix(class_name, X86_CPU_TYPE_SUFFIX));
+    return g_strndup(class_name,
+                     strlen(class_name) - strlen(X86_CPU_TYPE_SUFFIX));
+}
+
 struct X86CPUDefinition {
     const char *name;
     uint32_t level;
@@ -1531,6 +1539,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.
@@ -2022,23 +2033,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++) {
@@ -2050,26 +2100,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;
-
-    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
-        CpuDefinitionInfoList *entry;
-        CpuDefinitionInfo *info;
+    ObjectClass *oc = data;
+    X86CPUClass *cc = X86_CPU_CLASS(oc);
+    CpuDefinitionInfoList **cpu_list = user_data;
+    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.5.5

[Qemu-devel] [PATCH v2 2/6] target-i386: Move warning code outside x86_cpu_filter_features()

[Eduardo Habkost]

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>
---
 target-i386/cpu.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index aaa239a..bd815f3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2165,9 +2165,6 @@ static int x86_cpu_filter_features(X86CPU *cpu)
         env->features[w] &= host_feat;
         cpu->filtered_features[w] = requested_features & ~env->features[w];
         if (cpu->filtered_features[w]) {
-            if (cpu->check_cpuid || cpu->enforce_cpuid) {
-                report_unavailable_features(w, cpu->filtered_features[w]);
-            }
             rv = 1;
         }
     }
@@ -2175,6 +2172,15 @@ static int x86_cpu_filter_features(X86CPU *cpu)
     return rv;
 }
 
+static void x86_cpu_report_filtered_features(X86CPU *cpu)
+{
+    FeatureWord w;
+
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        report_unavailable_features(w, cpu->filtered_features[w]);
+    }
+}
+
 static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
 {
     PropValue *pv;
@@ -2979,12 +2985,16 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         env->cpuid_level = 7;
     }
 
-    if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
-        error_setg(&local_err,
-                   kvm_enabled() ?
-                       "Host doesn't support requested features" :
-                       "TCG doesn't support requested features");
-        goto out;
+    if (x86_cpu_filter_features(cpu) &&
+        (cpu->check_cpuid || cpu->enforce_cpuid)) {
+        x86_cpu_report_filtered_features(cpu);
+        if (cpu->enforce_cpuid) {
+            error_setg(&local_err,
+                       kvm_enabled() ?
+                           "Host doesn't support requested features" :
+                           "TCG doesn't support requested features");
+            goto out;
+        }
     }
 
     /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
-- 
2.5.5

[Qemu-devel] [PATCH v2 3/6] target-i386: Define CPUID filtering functions before x86_cpu_list()

[Eduardo Habkost]

Just move code to another place so the it can be reused by the
query-cpu-definitions code.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 68 +++++++++++++++++++++++++++----------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bd815f3..f455d3d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2018,6 +2018,40 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
     }
 }
 
+/*
+ * 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)
+{
+    CPUX86State *env = &cpu->env;
+    FeatureWord w;
+    int rv = 0;
+
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        uint32_t host_feat =
+            x86_cpu_get_supported_feature_word(w, cpu->migratable);
+        uint32_t requested_features = env->features[w];
+        env->features[w] &= host_feat;
+        cpu->filtered_features[w] = requested_features & ~env->features[w];
+        if (cpu->filtered_features[w]) {
+            rv = 1;
+        }
+    }
+
+    return rv;
+}
+
+static void x86_cpu_report_filtered_features(X86CPU *cpu)
+{
+    FeatureWord w;
+
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        report_unavailable_features(w, cpu->filtered_features[w]);
+    }
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2147,40 +2181,6 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
     return r;
 }
 
-/*
- * 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)
-{
-    CPUX86State *env = &cpu->env;
-    FeatureWord w;
-    int rv = 0;
-
-    for (w = 0; w < FEATURE_WORDS; w++) {
-        uint32_t host_feat =
-            x86_cpu_get_supported_feature_word(w, cpu->migratable);
-        uint32_t requested_features = env->features[w];
-        env->features[w] &= host_feat;
-        cpu->filtered_features[w] = requested_features & ~env->features[w];
-        if (cpu->filtered_features[w]) {
-            rv = 1;
-        }
-    }
-
-    return rv;
-}
-
-static void x86_cpu_report_filtered_features(X86CPU *cpu)
-{
-    FeatureWord w;
-
-    for (w = 0; w < FEATURE_WORDS; w++) {
-        report_unavailable_features(w, cpu->filtered_features[w]);
-    }
-}
-
 static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
 {
     PropValue *pv;
-- 
2.5.5

[Qemu-devel] [PATCH v2 4/6] qmp: Add runnability information to query-cpu-definitions

[Eduardo Habkost]

Extend query-cpu-definitions schema to allow it to return two new
optional fields: "runnable" and "unavailable-features".
"runnable" will tell if the CPU model can be run in the current
host. "unavailable-features" will contain a list of CPU
properties that are preventing 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
Signed-off-by: Eduardo Habkost <ehabkost@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 8483bdf..43478e9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3005,11 +3005,32 @@
 # Virtual CPU definition.
 #
 # @name: the name of the CPU definition
+# @unavailable-features: #optional List of properties that prevent
+#                        the CPU model from running in the current
+#                        host. (since 2.7)
+#
+# @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. If
+# absolutely no extra information will be returned to explain why
+# the CPU model is not runnable, implementations may simply
+# return "type" as the property name.
+# 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' } }
+  'data': { 'name': 'str',
+            '*unavailable-features': [ 'str' ] } }
 
 ##
 # @query-cpu-definitions:
-- 
2.5.5

[Qemu-devel] [PATCH v2 5/6] target-i386: Use "-" instead of "_" on all feature names

[Eduardo Habkost]

This makes the feature name tables in feature_word_info all match
the actual QOM property names we use.

This will make the command-line interface more consistent,
allowing the QOM property names to be used as "-cpu" arguments
directly.

Add extra feat2prop() calls to x86_cpu_parse_featurestr() to keep
compatibility with the old that had underscores.

Cc: Jiri Denemark <jdenemar@redhat.com>
Cc: libvir-list@redhat.com
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f455d3d..b8519ba 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -189,7 +189,7 @@ static const char *feature_name[] = {
 };
 static const char *ext_feature_name[] = {
     "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",
@@ -209,17 +209,17 @@ static const char *ext2_feature_name[] = {
     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" /* AMD Page1GB */, "rdtscp",
+    NULL /* fxsr */, "fxsr-opt|ffxsr", "pdpe1gb" /* AMD Page1GB */, "rdtscp",
     NULL, "lm|i64", "3dnowext", "3dnow",
 };
 static const char *ext3_feature_name[] = {
-    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */,
+    "lahf-lm" /* AMD LahfSahf */, "cmp-legacy", "svm", "extapic" /* AMD ExtApicSpace */,
     "cr8legacy" /* AMD AltMovCr8 */, "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,
 };
 
@@ -235,8 +235,8 @@ static const char *ext4_feature_name[] = {
 };
 
 static const char *kvm_feature_name[] = {
-    "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,
@@ -246,9 +246,9 @@ static const char *kvm_feature_name[] = {
 };
 
 static const char *svm_feature_name[] = {
-    "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,
@@ -257,7 +257,7 @@ static const char *svm_feature_name[] = {
 };
 
 static const char *cpuid_7_0_ebx_feature_name[] = {
-    "fsgsbase", "tsc_adjust", NULL, "bmi1", "hle", "avx2", NULL, "smep",
+    "fsgsbase", "tsc-adjust", NULL, "bmi1", "hle", "avx2", NULL, "smep",
     "bmi2", "erms", "invpcid", "rtm", NULL, NULL, "mpx", NULL,
     "avx512f", NULL, "rdseed", "adx", "smap", NULL, "pcommit", "clflushopt",
     "clwb", NULL, "avx512pf", "avx512er", "avx512cd", NULL, NULL, NULL,
@@ -1941,8 +1941,8 @@ static PropertyInfo qdev_prop_spinlocks = {
     .set   = x86_set_hv_spinlocks,
 };
 
-/* Convert all '_' in a feature string option name to '-', to make feature
- * name conform to QOM property naming rule, which uses '-' instead of '_'.
+/* Convert all '_' in a feature string option name to '-', to keep compatibility
+ * with old feature names that used "_" instead of "-".
  */
 static inline void feat2prop(char *s)
 {
@@ -1971,8 +1971,10 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
     while (featurestr) {
         char *val;
         if (featurestr[0] == '+') {
+            feat2prop(featurestr);
             add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err);
         } else if (featurestr[0] == '-') {
+            feat2prop(featurestr);
             add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err);
         } else if ((val = strchr(featurestr, '='))) {
             *val = 0; val++;
@@ -3180,11 +3182,9 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
 
     names = g_strsplit(fi->feat_names[bitnr], "|", 0);
 
-    feat2prop(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);
     }
-- 
2.5.5

[Qemu-devel] [PATCH v2 6/6] target-i386: Return runnability information on query-cpu-definitions

[Eduardo Habkost]

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 v1 -> v2:
* Updated to the new schema: no @runnable field, and
  always report @unavailable-features as present
---
 target-i386/cpu.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b8519ba..65d1ffc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2054,6 +2054,45 @@ static void x86_cpu_report_filtered_features(X86CPU *cpu)
     }
 }
 
+/* Check for missing features that may prevent the CPU class from
+ * running using the current machine and accelerator.
+ */
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
+                                                 strList **missing_feats)
+{
+    X86CPU *xc;
+    FeatureWord w;
+
+    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))));
+    if (x86_cpu_filter_features(xc)) {
+        for (w = 0; w < FEATURE_WORDS; w++) {
+            FeatureWordInfo *fi = &feature_word_info[w];
+            uint32_t filtered = xc->filtered_features[w];
+            int i;
+            for (i = 0; i < 32; i++) {
+                if (filtered & (1UL << i)) {
+                    char **parts = g_strsplit(fi->feat_names[i], "|", 2);
+                    strList *new = g_new0(strList, 1);
+                    new->value = g_strdup(parts[0]);
+                    feat2prop(new->value);
+                    new->next = *missing_feats;
+                    *missing_feats = new;
+                    g_strfreev(parts);
+                }
+            }
+        }
+    }
+
+    object_unref(OBJECT(xc));
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2146,6 +2185,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.5.5

Re: [Qemu-devel] [PATCH v2 4/6] qmp: Add runnability information to query-cpu-definitions

[Jiri Denemark]

On Mon, Jun 06, 2016 at 17:05:41 -0300, Eduardo Habkost wrote:
> Extend query-cpu-definitions schema to allow it to return two new
> optional fields: "runnable" and "unavailable-features".
> "runnable" will tell if the CPU model can be run in the current
> host. "unavailable-features" will contain a list of CPU
> properties that are preventing the CPU model from running in the
> current host.

The commit message still talks about both "runnable" and
"unavailable-features" fields even though the former has been removed in
version 2 of the series.

Jirka

Re: [Qemu-devel] [PATCH v2 4/6] qmp: Add runnability information to query-cpu-definitions

[Eduardo Habkost]

On Thu, Jun 09, 2016 at 03:54:01PM +0200, Jiri Denemark wrote:
> On Mon, Jun 06, 2016 at 17:05:41 -0300, Eduardo Habkost wrote:
> > Extend query-cpu-definitions schema to allow it to return two new
> > optional fields: "runnable" and "unavailable-features".
> > "runnable" will tell if the CPU model can be run in the current
> > host. "unavailable-features" will contain a list of CPU
> > properties that are preventing the CPU model from running in the
> > current host.
> 
> The commit message still talks about both "runnable" and
> "unavailable-features" fields even though the former has been removed in
> version 2 of the series.

Oops, thanks for pointing it out.

Updated to:

    qmp: Add runnability information to query-cpu-definitions
    
    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.


-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/6] Add runnability info to query-cpu-definitions

[Eduardo Habkost]

Ping? No other feedback on this?

On Mon, Jun 06, 2016 at 05:05:37PM -0300, Eduardo Habkost wrote:
> This series extends query-cpu-definitions to include an extra
> field: "unavailable-features". The new field can be used to find
> out reasons that prevent the CPU model from running in the
> current host.
> 
> This will return information based on the current machine and
> accelerator only. In the future we may extend these mechanisms to
> allow querying other machines and other accelerators without
> restarting QEMU, but it will require some reorganization of
> QEMU's main code.
> 
> This series is based on my 'x86-next' branch, at:
>   git://github.com/ehabkost/qemu.git x86-next
> 
> Changes v1 -> v2:
> * Fixed documentation to say "(since 2.7)"
> * Removed @runnable field, improved documentation
> 
> Example command output:
> 
>   { "return": [
>       {
>         "unavailable-features": [ "kvm" ],
>          "name": "host"
>       },
>       {
>         "unavailable-features": [],
>         "name": "qemu64"
>       },
>       {
>         "unavailable-features": [],
>         "name": "qemu32"
>       },
>       {
>         "unavailable-features": ["npt", "fxsr-opt", "vme"],
>         "name": "phenom"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "pentium3"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "pentium2"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "pentium"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "n270"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "kvm64"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "kvm32"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "coreduo"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "core2duo"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "athlon"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "Westmere"
>       },
>       {
>         "unavailable-features": ["xsavec", "3dnowprefetch", "rdseed", "rtm", "invpcid", "erms", "avx2", "hle", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
>         "name": "Skylake-Client"
>       },
>       {
>         "unavailable-features": ["avx", "tsc-deadline", "x2apic", "vme"],
>         "name": "SandyBridge"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "Penryn"
>       },
>       {
>         "unavailable-features": ["tbm", "fma4", "xop", "3dnowprefetch", "misalignsse", "f16c", "avx", "fma", "vme"],
>         "name": "Opteron_G5"
>       },
>       {
>         "unavailable-features": ["fma4", "xop", "3dnowprefetch", "misalignsse", "avx", "vme"],
>         "name": "Opteron_G4"
>       },
>       {
>         "unavailable-features": ["misalignsse", "vme"],
>         "name": "Opteron_G3"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "Opteron_G2"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "Opteron_G1"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "Nehalem"
>       },
>       {
>         "unavailable-features": ["erms", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "vme"],
>         "name": "IvyBridge"
>       },
>       {
>         "unavailable-features": ["rtm", "invpcid", "erms", "avx2", "hle", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
>         "name": "Haswell"
>       },
>       {
>         "unavailable-features": ["invpcid", "erms", "avx2", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
>         "name": "Haswell-noTSX"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "Conroe"
>       },
>       {
>         "unavailable-features": ["3dnowprefetch", "rdseed", "rtm", "invpcid", "erms", "avx2", "hle", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
>         "name": "Broadwell"
>       },
>       {
>         "unavailable-features": ["3dnowprefetch", "rdseed", "invpcid", "erms", "avx2", "rdrand", "f16c", "avx", "tsc-deadline", "x2apic", "pcid", "fma", "vme"],
>         "name": "Broadwell-noTSX"
>       },
>       {
>         "unavailable-features": ["vme"],
>         "name": "486"
>       }
>   ]}
> 
> Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Cc: Michael Mueller <mimu@linux.vnet.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Jiri Denemark <jdenemar@redhat.com>
> Cc: libvir-list@redhat.com
> 
> Eduardo Habkost (6):
>   target-i386: List CPU models using subclass list
>   target-i386: Move warning code outside x86_cpu_filter_features()
>   target-i386: Define CPUID filtering functions before x86_cpu_list()
>   qmp: Add runnability information to query-cpu-definitions
>   target-i386: Use "-" instead of "_" on all feature names
>   target-i386: Return runnability information on query-cpu-definitions
> 
>  qapi-schema.json      |  23 ++++-
>  target-i386/cpu-qom.h |   4 +
>  target-i386/cpu.c     | 262 +++++++++++++++++++++++++++++++++++---------------
>  3 files changed, 209 insertions(+), 80 deletions(-)
> 
> -- 
> 2.5.5
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/6] Add runnability info to query-cpu-definitions

[Jiri Denemark]

On Mon, Jun 20, 2016 at 17:09:18 -0300, Eduardo Habkost wrote:
> 
> Ping? No other feedback on this?

The interface is fine from my point of view and I even have a working
libvirt code that consumes this.

Jirka

Re: [Qemu-devel] [PATCH v2 4/6] qmp: Add runnability information to query-cpu-definitions

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> Extend query-cpu-definitions schema to allow it to return two new
> optional fields: "runnable" and "unavailable-features".
> "runnable" will tell if the CPU model can be run in the current
> host. "unavailable-features" will contain a list of CPU
> properties that are preventing 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
> Signed-off-by: Eduardo Habkost <ehabkost@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 8483bdf..43478e9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3005,11 +3005,32 @@
>  # Virtual CPU definition.
>  #
>  # @name: the name of the CPU definition
> +# @unavailable-features: #optional List of properties that prevent
> +#                        the CPU model from running in the current
> +#                        host. (since 2.7)
> +#
> +# @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. If
> +# absolutely no extra information will be returned to explain why

Suggest "can be returned".

> +# the CPU model is not runnable, implementations may simply
> +# return "type" as the property name.
> +# 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' } }
> +  'data': { 'name': 'str',
> +            '*unavailable-features': [ 'str' ] } }
>  
>  ##
>  # @query-cpu-definitions:

With the commit message tidied up as per your reply to Jiri:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2 4/6] qmp: Add runnability information to query-cpu-definitions

[Eduardo Habkost]

On Wed, Jun 22, 2016 at 11:00:47AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Extend query-cpu-definitions schema to allow it to return two new
> > optional fields: "runnable" and "unavailable-features".
> > "runnable" will tell if the CPU model can be run in the current
> > host. "unavailable-features" will contain a list of CPU
> > properties that are preventing 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
> > Signed-off-by: Eduardo Habkost <ehabkost@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 8483bdf..43478e9 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3005,11 +3005,32 @@
> >  # Virtual CPU definition.
> >  #
> >  # @name: the name of the CPU definition
> > +# @unavailable-features: #optional List of properties that prevent
> > +#                        the CPU model from running in the current
> > +#                        host. (since 2.7)
> > +#
> > +# @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. If
> > +# absolutely no extra information will be returned to explain why
> 
> Suggest "can be returned".

I believe that "extra information can be returned" will always be
true (we just need an appropriate property name to represent that
information). But people writing architecture-specific code are
free to decide if the extra effort is worth it, or if "type" is
good enough for them.

> 
> > +# the CPU model is not runnable, implementations may simply
> > +# return "type" as the property name.
> > +# 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' } }
> > +  'data': { 'name': 'str',
> > +            '*unavailable-features': [ 'str' ] } }
> >  
> >  ##
> >  # @query-cpu-definitions:
> 
> With the commit message tidied up as per your reply to Jiri:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 4/6] qmp: Add runnability information to query-cpu-definitions

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Wed, Jun 22, 2016 at 11:00:47AM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > Extend query-cpu-definitions schema to allow it to return two new
>> > optional fields: "runnable" and "unavailable-features".
>> > "runnable" will tell if the CPU model can be run in the current
>> > host. "unavailable-features" will contain a list of CPU
>> > properties that are preventing 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
>> > Signed-off-by: Eduardo Habkost <ehabkost@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 8483bdf..43478e9 100644
>> > --- a/qapi-schema.json
>> > +++ b/qapi-schema.json
>> > @@ -3005,11 +3005,32 @@
>> >  # Virtual CPU definition.
>> >  #
>> >  # @name: the name of the CPU definition
>> > +# @unavailable-features: #optional List of properties that prevent
>> > +#                        the CPU model from running in the current
>> > +#                        host. (since 2.7)
>> > +#
>> > +# @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. If
>> > +# absolutely no extra information will be returned to explain why
>> 
>> Suggest "can be returned".
>
> I believe that "extra information can be returned" will always be
> true (we just need an appropriate property name to represent that
> information). But people writing architecture-specific code are
> free to decide if the extra effort is worth it, or if "type" is
> good enough for them.

Hmm.  What about: Implementations that choose not to provide specific
information return the property name "type".

>> > +# the CPU model is not runnable, implementations may simply
>> > +# return "type" as the property name.
>> > +# 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' } }
>> > +  'data': { 'name': 'str',
>> > +            '*unavailable-features': [ 'str' ] } }
>> >  
>> >  ##
>> >  # @query-cpu-definitions:
>> 
>> With the commit message tidied up as per your reply to Jiri:
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> Thanks!