libx86: Helper for clearing out-of-range CPUID leaves

When merging a levelled policy, stale out-of-range leaves may remain.
Introduce a helper to clear them, and test a number of the subtle corner
cases.

The logic based on cpuid_policy_xstates() is liable to need changing when XCR0
has bit 63 defined.  Leave BUILD_BUG_ON()'s behind with comments in all all
impacted areas, which includes in x86_cpuid_policy_fill_native().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/tools/tests/cpu-policy/test-cpu-policy.c b/tools/tests/cpu-policy/test-cpu-policy.c
index fd96c0b896fd1fbb73c1c2e0eeb5a4f7a6248559..cb2c920e82feb3e124602c62f27d5372127414b6 100644 (file)
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -20,6 +20,17 @@ static unsigned int nr_failures;
     printf(fmt, ##__VA_ARGS__);                 \
 })
 
+#define memdup(ptr)                             \
+({                                              \
+    typeof(*(ptr)) *p_ = (ptr);                 \
+    void *n_ = malloc(sizeof(*p_));             \
+                                                \
+    if ( !n_ )                                  \
+        err(1, "%s malloc failure", __func__);  \
+                                                \
+    memcpy(n_, p_, sizeof(*p_));                \
+})
+
 static void test_vendor_identification(void)
 {
     static const struct test {
@@ -345,6 +356,151 @@ static void test_msr_deserialise_failure(void)
     }
 }
 
+static void test_cpuid_out_of_range_clearing(void)
+{
+    static const struct test {
+        const char *name;
+        unsigned int nr_markers;
+        struct cpuid_policy p;
+    } tests[] = {
+        {
+            .name = "basic",
+            .nr_markers = 1,
+            .p = {
+                /* Retains marker in leaf 0.  Clears others. */
+                .basic.max_leaf = 0,
+                .basic.vendor_ebx = 0xc2,
+
+                .basic.raw_fms = 0xc2,
+                .cache.raw[0].a = 0xc2,
+                .feat.raw[0].a = 0xc2,
+                .topo.raw[0].a = 0xc2,
+                .xstate.raw[0].a = 0xc2,
+                .xstate.raw[1].a = 0xc2,
+            },
+        },
+        {
+            .name = "cache",
+            .nr_markers = 1,
+            .p = {
+                /* Retains marker in subleaf 0.  Clears others. */
+                .basic.max_leaf = 4,
+                .cache.raw[0] = { .a = 1, .b = 0xc2 },
+
+                .cache.raw[1].b = 0xc2,
+                .feat.raw[0].a = 0xc2,
+                .topo.raw[0].a = 0xc2,
+                .xstate.raw[0].a = 0xc2,
+                .xstate.raw[1].a = 0xc2,
+            },
+        },
+        {
+            .name = "feat",
+            .nr_markers = 1,
+            .p = {
+                /* Retains marker in subleaf 0.  Clears others. */
+                .basic.max_leaf = 7,
+                .feat.raw[0].b = 0xc2,
+
+                .feat.raw[1].b = 0xc2,
+                .topo.raw[0].a = 0xc2,
+                .xstate.raw[0].a = 0xc2,
+                .xstate.raw[1].a = 0xc2,
+            },
+        },
+        {
+            .name = "topo",
+            .nr_markers = 1,
+            .p = {
+                /* Retains marker in subleaf 0.  Clears others. */
+                .basic.max_leaf = 0xb,
+                .topo.raw[0] = { .b = 0xc2, .c = 0x0100 },
+
+                .topo.raw[1].b = 0xc2,
+                .xstate.raw[0].a = 0xc2,
+                .xstate.raw[1].a = 0xc2,
+            },
+        },
+        {
+            .name = "xstate x87",
+            .nr_markers = 2,
+            .p = {
+                /* First two subleaves always valid.  Others cleared. */
+                .basic.max_leaf = 0xd,
+                .xstate.raw[0].a = 1,
+                .xstate.raw[0].b = 0xc2,
+                .xstate.raw[1].b = 0xc2,
+
+                .xstate.raw[2].b = 0xc2,
+                .xstate.raw[3].b = 0xc2,
+            },
+        },
+        {
+            .name = "xstate sse",
+            .nr_markers = 2,
+            .p = {
+                /* First two subleaves always valid.  Others cleared. */
+                .basic.max_leaf = 0xd,
+                .xstate.raw[0].a = 2,
+                .xstate.raw[0].b = 0xc2,
+                .xstate.raw[1].b = 0xc2,
+
+                .xstate.raw[2].b = 0xc2,
+                .xstate.raw[3].b = 0xc2,
+            },
+        },
+        {
+            .name = "xstate avx",
+            .nr_markers = 3,
+            .p = {
+                /* Third subleaf also valid.  Others cleared. */
+                .basic.max_leaf = 0xd,
+                .xstate.raw[0].a = 7,
+                .xstate.raw[0].b = 0xc2,
+                .xstate.raw[1].b = 0xc2,
+                .xstate.raw[2].b = 0xc2,
+
+                .xstate.raw[3].b = 0xc2,
+            },
+        },
+        {
+            .name = "extd",
+            .nr_markers = 1,
+            .p = {
+                /* Retains marker in leaf 0.  Clears others. */
+                .extd.max_leaf = 0,
+                .extd.vendor_ebx = 0xc2,
+
+                .extd.raw_fms = 0xc2,
+            },
+        },
+    };
+
+    printf("Testing CPUID out-of-range clearing:\n");
+
+    for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
+    {
+        const struct test *t = &tests[i];
+        struct cpuid_policy *p = memdup(&t->p);
+        void *ptr;
+        unsigned int nr_markers;
+
+        x86_cpuid_policy_clear_out_of_range_leaves(p);
+
+        /* Count the number of 0xc2's still remaining. */
+        for ( ptr = p, nr_markers = 0;
+              (ptr = memchr(ptr, 0xc2, (void *)p + sizeof(*p) - ptr));
+              ptr++, nr_markers++ )
+            ;
+
+        if ( nr_markers != t->nr_markers )
+             fail("  Test %s fail - expected %u markers, got %u\n",
+                  t->name, t->nr_markers, nr_markers);
+
+        free(p);
+    }
+}
+
 int main(int argc, char **argv)
 {
     printf("CPU Policy unit tests\n");
@@ -352,9 +508,10 @@ int main(int argc, char **argv)
     test_vendor_identification();
 
     test_cpuid_serialise_success();
-    test_msr_serialise_success();
-
     test_cpuid_deserialise_failure();
+    test_cpuid_out_of_range_clearing();
+
+    test_msr_serialise_success();
     test_msr_deserialise_failure();
 
     if ( nr_failures )

diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
index ed7d7b41feae8b3649548051314d99a9511ef729..261859831983724e9df26d4d2cd3778ecb312b07 100644 (file)
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -331,6 +331,17 @@ const uint32_t *x86_cpuid_lookup_deep_deps(uint32_t feature);
  */
 void x86_cpuid_policy_fill_native(struct cpuid_policy *p);
 
+/**
+ * Clear leaf data beyond the policies max leaf/subleaf settings.
+ *
+ * Policy serialisation purposefully omits out-of-range leaves, because there
+ * are a large number of them due to vendor differences.  However, when
+ * constructing new policies (e.g. levelling down), it is possible to end up
+ * with out-of-range leaves with stale content in them.  This helper clears
+ * them.
+ */
+void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p);
+
 #ifdef __XEN__
 #include <public/arch-x86/xen.h>
 typedef XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_leaf_buffer_t;

diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index a82cdb27aa226b044869861812acfd1fc760a0fa..24f425eed5c2b7658fe85afb4e639cc1c055506c 100644 (file)
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -2,6 +2,13 @@
 
 #include <xen/lib/x86/cpuid.h>
 
+static void zero_leaves(struct cpuid_leaf *l,
+                        unsigned int first, unsigned int last)
+{
+    if ( first <= last )
+        memset(&l[first], 0, sizeof(*l) * (last - first + 1));
+}
+
 unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx)
 {
     switch ( ebx )
@@ -146,6 +153,9 @@ void x86_cpuid_policy_fill_native(struct cpuid_policy *p)
 
         xstates = cpuid_policy_xstates(p);
 
+        /* This logic will probably need adjusting when XCR0[63] gets used. */
+        BUILD_BUG_ON(ARRAY_SIZE(p->xstate.raw) > 63);
+
         for ( i = 2; i < min_t(unsigned int, 63,
                                ARRAY_SIZE(p->xstate.raw)); ++i )
         {
@@ -163,6 +173,59 @@ void x86_cpuid_policy_fill_native(struct cpuid_policy *p)
     recalculate_synth(p);
 }
 
+void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p)
+{
+    unsigned int i;
+
+    zero_leaves(p->basic.raw, p->basic.max_leaf + 1,
+                ARRAY_SIZE(p->basic.raw) - 1);
+
+    if ( p->basic.max_leaf < 4 )
+        memset(p->cache.raw, 0, sizeof(p->cache.raw));
+    else
+    {
+        for ( i = 0; (i < ARRAY_SIZE(p->cache.raw) &&
+                      p->cache.subleaf[i].type); ++i )
+            ;
+
+        zero_leaves(p->cache.raw, i, ARRAY_SIZE(p->cache.raw) - 1);
+    }
+
+    if ( p->basic.max_leaf < 7 )
+        memset(p->feat.raw, 0, sizeof(p->feat.raw));
+    else
+        zero_leaves(p->feat.raw, p->feat.max_subleaf + 1,
+                    ARRAY_SIZE(p->feat.raw) - 1);
+
+    if ( p->basic.max_leaf < 0xb )
+        memset(p->topo.raw, 0, sizeof(p->topo.raw));
+    else
+    {
+        for ( i = 0; (i < ARRAY_SIZE(p->topo.raw) &&
+                      p->topo.subleaf[i].type); ++i )
+            ;
+
+        zero_leaves(p->topo.raw, i, ARRAY_SIZE(p->topo.raw) - 1);
+    }
+
+    if ( p->basic.max_leaf < 0xd || !cpuid_policy_xstates(p) )
+        memset(p->xstate.raw, 0, sizeof(p->xstate.raw));
+    else
+    {
+        /* This logic will probably need adjusting when XCR0[63] gets used. */
+        BUILD_BUG_ON(ARRAY_SIZE(p->xstate.raw) > 63);
+
+        /* First two leaves always valid.  Rest depend on xstates. */
+        i = max(2, 64 - __builtin_clzll(cpuid_policy_xstates(p)));
+
+        zero_leaves(p->xstate.raw, i,
+                    ARRAY_SIZE(p->xstate.raw) - 1);
+    }
+
+    zero_leaves(p->extd.raw, (p->extd.max_leaf & 0xffff) + 1,
+                ARRAY_SIZE(p->extd.raw) - 1);
+}
+
 const uint32_t *x86_cpuid_lookup_deep_deps(uint32_t feature)
 {
     static const uint32_t deep_features[] = INIT_DEEP_FEATURES;

diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
index f5b195e46d35b1fb80df7c48278c5098b260255e..b793181464f3ca15b9aafb250cdca8c546d0a027 100644 (file)
--- a/xen/lib/x86/private.h
+++ b/xen/lib/x86/private.h
@@ -21,6 +21,7 @@
 #include <inttypes.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <string.h>
 
 #include <xen/asm/msr-index.h>
 #include <xen/asm/x86-vendors.h>