On 04.01.19 at 16:33, <andrew.cooper3@citrix.com> wrote:

The AFL harness currently notices that there are cases where we optimse the
serialised stream by omitting data beyond the various maximum leaves.

Both sets of tests will be extended with further libx86 work.

Fix the sorting of the CPUID_GUEST_NR_* constants, noticed while writing the
unit tests.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Sergey Dyasli <sergey.dyasli@citrix.com>
---
 tools/fuzz/cpu-policy/.gitignore          |   1 +
 tools/fuzz/cpu-policy/Makefile            |  27 ++++
 tools/fuzz/cpu-policy/afl-policy-fuzzer.c | 117 ++++++++++++++
 tools/tests/Makefile                      |   1 +
 tools/tests/cpu-policy/.gitignore         |   1 +

Did we somehow come to the conclusion that the central .gitignore
at the root of the tree is not the way to go in the future?

andrewcoop@andrewcoop:/local/xen.git$ git ls-files | grep gitignore
.gitignore
tools/tests/vhpet/.gitignore
xen/tools/kconfig/.gitignore
xen/tools/kconfig/lxdialog/.gitignore
xen/xsm/flask/.gitignore

--- /dev/null
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -0,0 +1,247 @@
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <xen-tools/libs.h>
+#include <xen/lib/x86/cpuid.h>
+#include <xen/lib/x86/msr.h>
+#include <xen/domctl.h>
+
+static void test_cpuid_serialise_success(void)
+{
+    static const struct test {
+        struct cpuid_policy p;
+        const char *name;
+        unsigned int nr_leaves;
+    } tests[] = {
+        {
+            .name = "empty policy",
+            .nr_leaves = 4,
+        },
+    };
+    unsigned int i;
+
+    printf("Testing CPUID serialise success:\n");
+
+    for ( i = 0; i < ARRAY_SIZE(tests); ++i )
+    {
+        const struct test *t = &tests[i];
+        unsigned int nr = t->nr_leaves;
+        xen_cpuid_leaf_t *leaves = malloc(nr * sizeof(*leaves));
+        int rc;
+
+        if ( !leaves )
+            goto test_done;

Shouldn't you leave some indication of the test not having got run?

+static void test_cpuid_deserialise_failure(void)
+{
+    static const struct test {
+        const char *name;
+        xen_cpuid_leaf_t leaf;
+    } tests[] = {
+        {
+            .name = "incorrect basic subleaf",
+            .leaf = { .leaf = 0, .subleaf = 0 },
+        },
+        {
+            .name = "incorrect hv1 subleaf",
+            .leaf = { .leaf = 0x40000000, .subleaf = 0 },
+        },
+        {
+            .name = "incorrect hv2 subleaf",
+            .leaf = { .leaf = 0x40000100, .subleaf = 0 },
+        },
+        {
+            .name = "incorrect extd subleaf",
+            .leaf = { .leaf = 0x80000000, .subleaf = 0 },
+        },
+        {
+            .name = "OoB basic leaf",
+            .leaf = { .leaf = CPUID_GUEST_NR_BASIC },
+        },
+        {
+            .name = "OoB cache leaf",
+            .leaf = { .leaf = 0x4, .subleaf = CPUID_GUEST_NR_CACHE },
+        },
+        {
+            .name = "OoB feat leaf",
+            .leaf = { .leaf = 0x7, .subleaf = CPUID_GUEST_NR_FEAT },
+        },
+        {
+            .name = "OoB topo leaf",
+            .leaf = { .leaf = 0xb, .subleaf = CPUID_GUEST_NR_TOPO },
+        },
+        {
+            .name = "OoB xstate leaf",
+            .leaf = { .leaf = 0xd, .subleaf = CPUID_GUEST_NR_XSTATE },
+        },
+        {
+            .name = "OoB extd leaf",
+            .leaf = { .leaf = 0x80000000 | CPUID_GUEST_NR_EXTD },
+        },
+    };
+    unsigned int i;
+
+    printf("Testing CPUID deserialise failure:\n");
+
+    for ( i = 0; i < ARRAY_SIZE(tests); ++i )
+    {
+        const struct test *t = &tests[i];
+        uint32_t err_leaf = ~0u, err_subleaf = ~0u;
+        int rc;
+
+        rc = x86_cpuid_copy_from_buffer(NULL, &t->leaf, 1,
+                                        &err_leaf, &err_subleaf);
+
+        if ( rc != -ERANGE )
+        {
+            printf("  Test %s, expected rc %d, got %d\n",
+                   t->name, -ERANGE, rc);
+            continue;

Perhaps drop this? The subsequent test ought to apply regardless
of error code.