[Qemu-devel] [PATCH v2] target/i386: Fixes to the check missing features routine

[Wainer dos Santos Moschetta]

The x86_cpu_class_check_missing_features() returns a list
of unavailable features compared to the host CPU. Currently it may
return empty strings for unnamed features as well as duplicated
names.

For example, the qmp "query-cpu-definitions" below shows one empty
string and repeated "mpx" entries:

(...)
{"execute": "query-cpu-definitions"}
(...)
        {
            "name": "Cascadelake-Server",
            "typename": "Cascadelake-Server-x86_64-cpu",
            "unavailable-features": [
                "hle",
                "rtm",
                "mpx",
                "avx512f",
                "avx512dq",
                "rdseed",
                "adx",
                "smap",
                "clflushopt",
                "clwb",
                "intel-pt",
                "avx512cd",
                "avx512bw",
                "avx512vl",
                "pku",
                "",
                "avx512vnni",
                "spec-ctrl",
                "ssbd",
                "3dnowprefetch",
                "xsavec",
                "xgetbv1",
                "mpx",
                "mpx",
                "avx512f",
                "avx512f",
                "avx512f",
                "pku"
            ],
(...)

Signed-off-by: Wainer dos Santos Moschetta <address@hidden>
Reviewed-by: Eduardo Habkost <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Caio Carrara <address@hidden>
---
v2:
 * Fixed typos. [eblake]
 * Removed unwanted manual test case. [ccarrara, ehabkost]
 * Not passing 'accel=kvm' on test's VM. [ehabkost]
 * Removed unneeded g_strdup() call. [ehabkost]
 * Formatted comment according to QEMU's coding style. [ehabkost]

v1: https://www.mail-archive.com/address@hidden/msg579404.html
---
 target/i386/cpu.c                   | 11 ++++++++-
 tests/acceptance/cpu_definitions.py | 35 +++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 tests/acceptance/cpu_definitions.py

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f81d35e1f9..014b91e608 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3615,19 +3615,28 @@ static void 
x86_cpu_class_check_missing_features(X86CPUClass *xcc,
 
     x86_cpu_filter_features(xc);
 
+    /* Auxiliary dictionary to avoid duplicate entries in the list. */
+    QDict *unique_feats_dict = qdict_new();
+
     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)) {
+                const char *fname = x86_cpu_feature_name(w, i);
+                if (!fname || qdict_haskey(unique_feats_dict, fname)) {
+                    continue;
+                }
                 strList *new = g_new0(strList, 1);
-                new->value = g_strdup(x86_cpu_feature_name(w, i));
+                new->value = g_strdup(fname);
                 *next = new;
                 next = &new->next;
+                qdict_put_null(unique_feats_dict, new->value);
             }
         }
     }
 
+    g_free(unique_feats_dict);
     object_unref(OBJECT(xc));
 }
 
diff --git a/tests/acceptance/cpu_definitions.py 
b/tests/acceptance/cpu_definitions.py
new file mode 100644
index 0000000000..4edad86799
--- /dev/null
+++ b/tests/acceptance/cpu_definitions.py
@@ -0,0 +1,35 @@
+# CPU definitions tests.
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Author:
+#  Wainer dos Santos Moschetta <address@hidden>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado import skip
+from avocado_qemu import Test
+
+
+class CPUDefinitions(Test):
+    """
+    Tests for the CPU definitions.
+
+    :avocado: enable
+    :avocado: tags=x86_64
+    """
+    def test_unavailable_features(self):
+        self.vm.add_args("-machine", "q35")
+        self.vm.launch()
+        cpu_definitions = self.vm.command('query-cpu-definitions')
+        self.assertTrue(len(cpu_definitions) > 0)
+        for cpu_model in cpu_definitions:
+            name = cpu_model.get('name')
+            unavailable_features = cpu_model.get('unavailable-features')
+
+            self.assertNotIn("", unavailable_features,
+                             name + " has unamed feature")
+            self.assertEqual(len(unavailable_features),
+                             len(set(unavailable_features)),
+                             name + " has duplicate feature")
-- 
2.19.1