All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] libx86: Remaining serialisation logic
@ 2019-02-25 15:47 Andrew Cooper
  2019-02-25 15:47 ` [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects Andrew Cooper
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Andrew Cooper @ 2019-02-25 15:47 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Sergey Dyasli, Wei Liu, Jan Beulich, Roger Pau Monné

Andrew Cooper (2):
  libx86: Introduce a helper to deserialise cpuid_policy objects
  tools/cpu-policy: Add unit tests

Roger Pau Monné (1):
  libx86: introduce a helper to deserialise msr_policy objects

 tools/tests/Makefile                     |   1 +
 tools/tests/cpu-policy/.gitignore        |   1 +
 tools/tests/cpu-policy/Makefile          |  28 ++++
 tools/tests/cpu-policy/test-cpu-policy.c | 258 +++++++++++++++++++++++++++++++
 xen/include/xen/lib/x86/cpuid.h          |  23 ++-
 xen/include/xen/lib/x86/msr.h            |  21 +++
 xen/lib/x86/cpuid.c                      | 109 +++++++++++++
 xen/lib/x86/msr.c                        |  67 ++++++++
 xen/lib/x86/private.h                    |  17 ++
 9 files changed, 524 insertions(+), 1 deletion(-)
 create mode 100644 tools/tests/cpu-policy/.gitignore
 create mode 100644 tools/tests/cpu-policy/Makefile
 create mode 100644 tools/tests/cpu-policy/test-cpu-policy.c

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects
  2019-02-25 15:47 [PATCH v2 0/3] libx86: Remaining serialisation logic Andrew Cooper
@ 2019-02-25 15:47 ` Andrew Cooper
  2019-02-27 12:31   ` Wei Liu
  2019-02-25 15:47 ` [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects Andrew Cooper
  2019-02-25 15:47 ` [PATCH v2 3/3] tools/cpu-policy: Add unit tests Andrew Cooper
  2 siblings, 1 reply; 11+ messages in thread
From: Andrew Cooper @ 2019-02-25 15:47 UTC (permalink / raw)
  To: Xen-devel
  Cc: Sergey Dyasli, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné

From: Roger Pau Monné <roger.pau@citrix.com>

As with the serialise side, Xen's copy_from_guest API is used, with a
compatibility wrapper for the userspace build.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
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>

v2:
 * Fix comment
---
 xen/include/xen/lib/x86/msr.h | 21 ++++++++++++++
 xen/lib/x86/msr.c             | 67 +++++++++++++++++++++++++++++++++++++++++++
 xen/lib/x86/private.h         | 14 +++++++++
 3 files changed, 102 insertions(+)

diff --git a/xen/include/xen/lib/x86/msr.h b/xen/include/xen/lib/x86/msr.h
index e2cfbb1..6236622 100644
--- a/xen/include/xen/lib/x86/msr.h
+++ b/xen/include/xen/lib/x86/msr.h
@@ -48,6 +48,27 @@ typedef xen_msr_entry_t msr_entry_buffer_t[];
 int x86_msr_copy_to_buffer(const struct msr_policy *policy,
                            msr_entry_buffer_t msrs, uint32_t *nr_entries);
 
+/**
+ * Unserialise an msr_policy object from an array of msrs.
+ *
+ * @param policy     The msr_policy object to unserialise into.
+ * @param msrs       The array of msrs to unserialise from.
+ * @param nr_entries The number of entries in 'msrs'.
+ * @param err_msr    Optional hint filled on error.
+ * @returns -errno
+ *
+ * Reads at most MSR_MAX_SERIALISED_ENTRIES.  May fail for a number of reasons
+ * based on the content in an individual 'msrs' entry, including the MSR index
+ * not being valid in the policy, the flags field being nonzero, or if the
+ * value provided would truncate when stored in the policy.  In such cases,
+ * the optional err_* pointer is filled in to aid diagnostics.
+ *
+ * No content validation is performed on the data stored in the policy object.
+ */
+int x86_msr_copy_from_buffer(struct msr_policy *policy,
+                             const msr_entry_buffer_t msrs, uint32_t nr_entries,
+                             uint32_t *err_msr);
+
 #endif /* !XEN_LIB_X86_MSR_H */
 
 /*
diff --git a/xen/lib/x86/msr.c b/xen/lib/x86/msr.c
index 60fb567..1524037 100644
--- a/xen/lib/x86/msr.c
+++ b/xen/lib/x86/msr.c
@@ -47,6 +47,73 @@ int x86_msr_copy_to_buffer(const struct msr_policy *p,
     return 0;
 }
 
+int x86_msr_copy_from_buffer(struct msr_policy *p,
+                             const msr_entry_buffer_t msrs, uint32_t nr_entries,
+                             uint32_t *err_msr)
+{
+    unsigned int i;
+    xen_msr_entry_t data;
+    int rc;
+
+    /*
+     * A well formed caller is expected to pass an array with entries in
+     * order, and without any repetitions.  However, due to per-vendor
+     * differences, and in the case of upgrade or levelled scenarios, we
+     * typically expect fewer than MAX entries to be passed.
+     *
+     * Detecting repeated entries is prohibitively complicated, so we don't
+     * bother.  That said, one way or another if more than MAX entries are
+     * passed, something is wrong.
+     */
+    if ( nr_entries > MSR_MAX_SERIALISED_ENTRIES )
+        return -E2BIG;
+
+    for ( i = 0; i < nr_entries; i++ )
+    {
+        if ( copy_from_buffer_offset(&data, msrs, i, 1) )
+            return -EFAULT;
+
+        if ( data.flags ) /* .flags MBZ */
+        {
+            rc = -EINVAL;
+            goto err;
+        }
+
+        switch ( data.idx )
+        {
+            /*
+             * Assign data.val to 'field', checking for truncation if the
+             * backing storage for 'field' is smaller than uint64_t
+             */
+#define ASSIGN(field)                           \
+({                                              \
+    if ( (typeof(field))data.val != data.val )  \
+    {                                           \
+        rc = -EOVERFLOW;                        \
+        goto err;                               \
+    }                                           \
+    field = data.val;                           \
+})
+
+        case MSR_INTEL_PLATFORM_INFO: ASSIGN(p->plaform_info.raw); break;
+
+#undef ASSIGN
+
+        default:
+            rc = -ERANGE;
+            goto err;
+        }
+    }
+
+    return 0;
+
+ err:
+    if ( err_msr )
+        *err_msr = data.idx;
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
index 3ee99aa..e0ff2da 100644
--- a/xen/lib/x86/private.h
+++ b/xen/lib/x86/private.h
@@ -12,6 +12,7 @@
 #include <asm/msr-index.h>
 
 #define copy_to_buffer_offset copy_to_guest_offset
+#define copy_from_buffer_offset copy_from_guest_offset
 
 #else
 
@@ -44,6 +45,19 @@ static inline bool test_bit(unsigned int bit, const void *vaddr)
     0;                                                  \
 })
 
+/* memcpy(), but with copy_from_guest_offset()'s API. */
+#define copy_from_buffer_offset(dst, src, index, nr)    \
+({                                                      \
+    const typeof(*(src)) *src_ = (src);                 \
+    typeof(*(dst)) *dst_ = (dst);                       \
+    typeof(index) index_ = (index);                     \
+    typeof(nr) nr_ = (nr), i_;                          \
+                                                        \
+    for ( i_ = 0; i_ < nr_; i_++ )                      \
+        dst_[i_] = src_[index_ + i_];                   \
+    0;                                                  \
+})
+
 #endif /* __XEN__ */
 
 #endif /* XEN_LIB_X86_PRIVATE_H */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects
  2019-02-25 15:47 [PATCH v2 0/3] libx86: Remaining serialisation logic Andrew Cooper
  2019-02-25 15:47 ` [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects Andrew Cooper
@ 2019-02-25 15:47 ` Andrew Cooper
  2019-02-27 12:31   ` Wei Liu
  2019-02-25 15:47 ` [PATCH v2 3/3] tools/cpu-policy: Add unit tests Andrew Cooper
  2 siblings, 1 reply; 11+ messages in thread
From: Andrew Cooper @ 2019-02-25 15:47 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, Jan Beulich, Sergey Dyasli

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@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>

v2:
 * Avoid explicit type overlap.
 * Fix comment.
 * Use array_access_nospec(), with a dummy implementation for userspace
   builds.
---
 xen/include/xen/lib/x86/cpuid.h |  21 ++++++++
 xen/lib/x86/cpuid.c             | 109 ++++++++++++++++++++++++++++++++++++++++
 xen/lib/x86/private.h           |   3 ++
 3 files changed, 133 insertions(+)

diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
index 22d43ef..767a33b 100644
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -319,6 +319,27 @@ typedef xen_cpuid_leaf_t cpuid_leaf_buffer_t[];
 int x86_cpuid_copy_to_buffer(const struct cpuid_policy *policy,
                              cpuid_leaf_buffer_t leaves, uint32_t *nr_entries);
 
+/**
+ * Unserialise a cpuid_policy object from an array of cpuid leaves.
+ *
+ * @param policy      The cpuid_policy to unserialise into.
+ * @param leaves      The array of leaves to unserialise from.
+ * @param nr_entries  The number of entries in 'leaves'.
+ * @param err_leaf    Optional hint filled on error.
+ * @param err_subleaf Optional hint filled on error.
+ * @returns -errno
+ *
+ * Reads at most CPUID_MAX_SERIALISED_LEAVES.  May return -ERANGE if an
+ * incoming leaf is out of range of cpuid_policy, in which case the optional
+ * err_* pointers are filled to aid diagnostics.
+ *
+ * No content validation of in-range leaves is performed.
+ */
+int x86_cpuid_copy_from_buffer(struct cpuid_policy *policy,
+                               const cpuid_leaf_buffer_t leaves,
+                               uint32_t nr_entries, uint32_t *err_leaf,
+                               uint32_t *err_subleaf);
+
 #endif /* !XEN_LIB_X86_CPUID_H */
 
 /*
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index 5a3159b..6c60ba8 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -233,6 +233,115 @@ int x86_cpuid_copy_to_buffer(const struct cpuid_policy *p,
     return 0;
 }
 
+int x86_cpuid_copy_from_buffer(struct cpuid_policy *p,
+                               const cpuid_leaf_buffer_t leaves,
+                               uint32_t nr_entries, uint32_t *err_leaf,
+                               uint32_t *err_subleaf)
+{
+    unsigned int i;
+    xen_cpuid_leaf_t data;
+
+    /*
+     * A well formed caller is expected to pass an array with leaves in order,
+     * and without any repetitions.  However, due to per-vendor differences,
+     * and in the case of upgrade or levelled scenarios, we typically expect
+     * fewer than MAX leaves to be passed.
+     *
+     * Detecting repeated entries is prohibitively complicated, so we don't
+     * bother.  That said, one way or another if more than MAX leaves are
+     * passed, something is wrong.
+     */
+    if ( nr_entries > CPUID_MAX_SERIALISED_LEAVES )
+        return -E2BIG;
+
+    for ( i = 0; i < nr_entries; ++i )
+    {
+        struct cpuid_leaf l;
+
+        if ( copy_from_buffer_offset(&data, leaves, i, 1) )
+            return -EFAULT;
+
+        l = (struct cpuid_leaf){ data.a, data.b, data.c, data.d };
+
+        switch ( data.leaf )
+        {
+        case 0 ... ARRAY_SIZE(p->basic.raw) - 1:
+            switch ( data.leaf )
+            {
+            case 0x4:
+                if ( data.subleaf >= ARRAY_SIZE(p->cache.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->cache.raw, data.subleaf) = l;
+                break;
+
+            case 0x7:
+                if ( data.subleaf >= ARRAY_SIZE(p->feat.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->feat.raw, data.subleaf) = l;
+                break;
+
+            case 0xb:
+                if ( data.subleaf >= ARRAY_SIZE(p->topo.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->topo.raw, data.subleaf) = l;
+                break;
+
+            case 0xd:
+                if ( data.subleaf >= ARRAY_SIZE(p->xstate.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->xstate.raw, data.subleaf) = l;
+                break;
+
+            default:
+                if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                    goto out_of_range;
+
+                array_access_nospec(p->basic.raw, data.leaf) = l;
+                break;
+            }
+            break;
+
+        case 0x40000000:
+            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                goto out_of_range;
+
+            p->hv_limit = l.a;
+            break;
+
+        case 0x40000100:
+            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                goto out_of_range;
+
+            p->hv2_limit = l.a;
+            break;
+
+        case 0x80000000 ... 0x80000000 + ARRAY_SIZE(p->extd.raw) - 1:
+            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                goto out_of_range;
+
+            array_access_nospec(p->extd.raw, data.leaf & 0xffff) = l;
+            break;
+
+        default:
+            goto out_of_range;
+        }
+    }
+
+    return 0;
+
+ out_of_range:
+    if ( err_leaf )
+        *err_leaf = data.leaf;
+    if ( err_subleaf )
+        *err_subleaf = data.subleaf;
+
+    return -ERANGE;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
index e0ff2da..6fb5022 100644
--- a/xen/lib/x86/private.h
+++ b/xen/lib/x86/private.h
@@ -6,6 +6,7 @@
 #include <xen/bitops.h>
 #include <xen/kernel.h>
 #include <xen/lib.h>
+#include <xen/nospec.h>
 #include <xen/types.h>
 
 #include <asm/guest_access.h>
@@ -32,6 +33,8 @@ static inline bool test_bit(unsigned int bit, const void *vaddr)
     return addr[bit / 8] & (1u << (bit % 8));
 }
 
+#define array_access_nospec(a, i) (a)[(i)]
+
 /* memcpy(), but with copy_to_guest_offset()'s API. */
 #define copy_to_buffer_offset(dst, index, src, nr)      \
 ({                                                      \
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] tools/cpu-policy: Add unit tests
  2019-02-25 15:47 [PATCH v2 0/3] libx86: Remaining serialisation logic Andrew Cooper
  2019-02-25 15:47 ` [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects Andrew Cooper
  2019-02-25 15:47 ` [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects Andrew Cooper
@ 2019-02-25 15:47 ` Andrew Cooper
  2019-02-27 12:36   ` Wei Liu
  2019-03-11 15:18   ` Jan Beulich
  2 siblings, 2 replies; 11+ messages in thread
From: Andrew Cooper @ 2019-02-25 15:47 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Sergey Dyasli, Wei Liu, Jan Beulich, Roger Pau Monné

These 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>

v2:
 * Pospone the AFL fuzzer for now.  It needs a bit more work.
 * Exit early for malloc() failures, which won't occur in practice.
---
 tools/tests/Makefile                     |   1 +
 tools/tests/cpu-policy/.gitignore        |   1 +
 tools/tests/cpu-policy/Makefile          |  28 ++++
 tools/tests/cpu-policy/test-cpu-policy.c | 258 +++++++++++++++++++++++++++++++
 xen/include/xen/lib/x86/cpuid.h          |   2 +-
 5 files changed, 289 insertions(+), 1 deletion(-)
 create mode 100644 tools/tests/cpu-policy/.gitignore
 create mode 100644 tools/tests/cpu-policy/Makefile
 create mode 100644 tools/tests/cpu-policy/test-cpu-policy.c

diff --git a/tools/tests/Makefile b/tools/tests/Makefile
index a9fc50d..067a380 100644
--- a/tools/tests/Makefile
+++ b/tools/tests/Makefile
@@ -5,6 +5,7 @@ CFLAGS  += $(CFLAGS_libxenctrl)
 LDLIBS += $(LDLIBS_libxenctrl)
 
 SUBDIRS-y :=
+SUBDIRS-$(CONFIG_X86) += cpu-policy
 SUBDIRS-$(CONFIG_X86) += mce-test
 SUBDIRS-y += mem-sharing
 ifeq ($(XEN_TARGET_ARCH),__fixme__)
diff --git a/tools/tests/cpu-policy/.gitignore b/tools/tests/cpu-policy/.gitignore
new file mode 100644
index 0000000..83bdb6b
--- /dev/null
+++ b/tools/tests/cpu-policy/.gitignore
@@ -0,0 +1 @@
+test-cpu-policy
diff --git a/tools/tests/cpu-policy/Makefile b/tools/tests/cpu-policy/Makefile
new file mode 100644
index 0000000..eeed7f3
--- /dev/null
+++ b/tools/tests/cpu-policy/Makefile
@@ -0,0 +1,28 @@
+XEN_ROOT = $(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+.PHONY: all
+all: test-cpu-policy
+
+.PHONY: clean
+clean:
+	$(RM) -f -- *.o .*.d .*.d2 test-cpu-policy
+
+.PHONY: distclean
+distclean: clean
+	$(RM) -f -- *~
+
+.PHONY: install
+install: all
+
+.PHONY: uninstall
+
+CFLAGS += -Werror $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -O3
+CFLAGS += $(APPEND_CFLAGS)
+
+vpath %.c ../../../xen/lib/x86
+
+test-cpu-policy: test-cpu-policy.o msr.o cpuid.o
+	$(CC) $(CFLAGS) $^ -o $@
+
+-include $(DEPS_INCLUDE)
diff --git a/tools/tests/cpu-policy/test-cpu-policy.c b/tools/tests/cpu-policy/test-cpu-policy.c
new file mode 100644
index 0000000..d13963e
--- /dev/null
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -0,0 +1,258 @@
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <err.h>
+
+#include <xen-tools/libs.h>
+#include <xen/lib/x86/cpuid.h>
+#include <xen/lib/x86/msr.h>
+#include <xen/domctl.h>
+
+static unsigned int nr_failures;
+#define fail(fmt, ...)                          \
+({                                              \
+    nr_failures++;                              \
+    printf(fmt, ##__VA_ARGS__);                 \
+})
+
+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,
+        },
+    };
+
+    printf("Testing CPUID serialise success:\n");
+
+    for ( size_t 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 )
+            err(1, "%s() malloc failure", __func__);
+
+        rc = x86_cpuid_copy_to_buffer(&t->p, leaves, &nr);
+
+        if ( rc != 0 )
+        {
+            fail("  Test %s, expected rc 0, got %d\n",
+                 t->name, rc);
+            goto test_done;
+        }
+
+        if ( nr != t->nr_leaves )
+        {
+            fail("  Test %s, expected %u leaves, got %u\n",
+                 t->name, t->nr_leaves, nr);
+            goto test_done;
+        }
+
+    test_done:
+        free(leaves);
+    }
+}
+
+static void test_msr_serialise_success(void)
+{
+    static const struct test {
+        struct msr_policy p;
+        const char *name;
+        unsigned int nr_msrs;
+    } tests[] = {
+        {
+            .name = "empty policy",
+            .nr_msrs = MSR_MAX_SERIALISED_ENTRIES,
+        },
+    };
+
+    printf("Testing MSR serialise success:\n");
+
+    for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
+    {
+        const struct test *t = &tests[i];
+        unsigned int nr = t->nr_msrs;
+        xen_msr_entry_t *msrs = malloc(nr * sizeof(*msrs));
+        int rc;
+
+        if ( !msrs )
+            err(1, "%s() malloc failure", __func__);
+
+        rc = x86_msr_copy_to_buffer(&t->p, msrs, &nr);
+
+        if ( rc != 0 )
+        {
+            fail("  Test %s, expected rc 0, got %d\n",
+                 t->name, rc);
+            goto test_done;
+        }
+
+        if ( nr != t->nr_msrs )
+        {
+            fail("  Test %s, expected %u msrs, got %u\n",
+                 t->name, t->nr_msrs, nr);
+            goto test_done;
+        }
+
+    test_done:
+        free(msrs);
+    }
+}
+
+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 },
+        },
+    };
+
+    printf("Testing CPUID deserialise failure:\n");
+
+    for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
+    {
+        const struct test *t = &tests[i];
+        uint32_t err_leaf = ~0u, err_subleaf = ~0u;
+        int rc;
+
+        /* No writes should occur.  Use NULL to catch errors. */
+        rc = x86_cpuid_copy_from_buffer(NULL, &t->leaf, 1,
+                                        &err_leaf, &err_subleaf);
+
+        if ( rc != -ERANGE )
+        {
+            fail("  Test %s, expected rc %d, got %d\n",
+                 t->name, -ERANGE, rc);
+            continue;
+        }
+
+        if ( err_leaf != t->leaf.leaf || err_subleaf != t->leaf.subleaf )
+        {
+            fail("  Test %s, expected err %08x:%08x, got %08x:%08x\n",
+                 t->name, t->leaf.leaf, t->leaf.subleaf,
+                 err_leaf, err_subleaf);
+            continue;
+        }
+    }
+}
+
+static void test_msr_deserialise_failure(void)
+{
+    static const struct test {
+        const char *name;
+        xen_msr_entry_t msr;
+        int rc;
+    } tests[] = {
+        {
+            .name = "bad msr index",
+            .msr = { .idx = 0xdeadc0de },
+            .rc = -ERANGE,
+        },
+        {
+            .name = "nonzero flags",
+            .msr = { .idx = 0xce, .flags = 1 },
+            .rc = -EINVAL,
+        },
+        {
+            .name = "truncated val",
+            .msr = { .idx = 0xce, .val = ~0ull },
+            .rc = -EOVERFLOW,
+        },
+    };
+
+    printf("Testing MSR deserialise failure:\n");
+
+    for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
+    {
+        const struct test *t = &tests[i];
+        uint32_t err_msr = ~0u;
+        int rc;
+
+        /* No writes should occur.  Use NULL to catch errors. */
+        rc = x86_msr_copy_from_buffer(NULL, &t->msr, 1, &err_msr);
+
+        if ( rc != t->rc )
+        {
+            fail("  Test %s, expected rc %d, got %d\n",
+                 t->name, t->rc, rc);
+            continue;
+        }
+
+        if ( err_msr != t->msr.idx )
+        {
+            fail("  Test %s, expected err_msr %#x, got %#x\n",
+                 t->name, t->msr.idx, err_msr);
+            continue;
+        }
+    }
+}
+
+int main(int argc, char **argv)
+{
+    printf("CPU Policy unit tests\n");
+
+    test_cpuid_serialise_success();
+    test_msr_serialise_success();
+
+    test_cpuid_deserialise_failure();
+    test_msr_deserialise_failure();
+
+    if ( nr_failures )
+        printf("Done: %u failures\n", nr_failures);
+    else
+        printf("Done: all ok\n");
+
+    return !!nr_failures;
+}
diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
index 767a33b..95b37b6 100644
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -66,8 +66,8 @@ static inline void cpuid_count_leaf(
 #undef XCHG
 
 #define CPUID_GUEST_NR_BASIC      (0xdu + 1)
-#define CPUID_GUEST_NR_FEAT       (0u + 1)
 #define CPUID_GUEST_NR_CACHE      (5u + 1)
+#define CPUID_GUEST_NR_FEAT       (0u + 1)
 #define CPUID_GUEST_NR_TOPO       (1u + 1)
 #define CPUID_GUEST_NR_XSTATE     (62u + 1)
 #define CPUID_GUEST_NR_EXTD_INTEL (0x8u + 1)
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects
  2019-02-25 15:47 ` [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects Andrew Cooper
@ 2019-02-27 12:31   ` Wei Liu
  2019-02-28 18:33     ` Andrew Cooper
  2019-03-11 16:00     ` Jan Beulich
  0 siblings, 2 replies; 11+ messages in thread
From: Wei Liu @ 2019-02-27 12:31 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Sergey Dyasli, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel

On Mon, Feb 25, 2019 at 03:47:13PM +0000, Andrew Cooper wrote:
> +
> +        switch ( data.idx )
> +        {
> +            /*
> +             * Assign data.val to 'field', checking for truncation if the
> +             * backing storage for 'field' is smaller than uint64_t
> +             */
> +#define ASSIGN(field)                           \
> +({                                              \
> +    if ( (typeof(field))data.val != data.val )  \
> +    {                                           \
> +        rc = -EOVERFLOW;                        \
> +        goto err;                               \
> +    }                                           \
> +    field = data.val;                           \

Missing parentheses around "field" in the macro. Although I don't think
it will break in practice, it is better to follow general macro writing
rules.

Other than this, this patch looks good to me.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects
  2019-02-25 15:47 ` [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects Andrew Cooper
@ 2019-02-27 12:31   ` Wei Liu
  2019-03-11 16:02     ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Wei Liu @ 2019-02-27 12:31 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Sergey Dyasli, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel

On Mon, Feb 25, 2019 at 03:47:14PM +0000, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] tools/cpu-policy: Add unit tests
  2019-02-25 15:47 ` [PATCH v2 3/3] tools/cpu-policy: Add unit tests Andrew Cooper
@ 2019-02-27 12:36   ` Wei Liu
  2019-03-11 15:18   ` Jan Beulich
  1 sibling, 0 replies; 11+ messages in thread
From: Wei Liu @ 2019-02-27 12:36 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Sergey Dyasli, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel

On Mon, Feb 25, 2019 at 03:47:15PM +0000, Andrew Cooper wrote:
> These 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>

Looks good to me. But I will let you and Jan figure out whether we
should use centralised gitignore or not.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects
  2019-02-27 12:31   ` Wei Liu
@ 2019-02-28 18:33     ` Andrew Cooper
  2019-03-11 16:00     ` Jan Beulich
  1 sibling, 0 replies; 11+ messages in thread
From: Andrew Cooper @ 2019-02-28 18:33 UTC (permalink / raw)
  To: Wei Liu; +Cc: Sergey Dyasli, Roger Pau Monné, Jan Beulich, Xen-devel

On 27/02/2019 12:31, Wei Liu wrote:
> On Mon, Feb 25, 2019 at 03:47:13PM +0000, Andrew Cooper wrote:
>> +
>> +        switch ( data.idx )
>> +        {
>> +            /*
>> +             * Assign data.val to 'field', checking for truncation if the
>> +             * backing storage for 'field' is smaller than uint64_t
>> +             */
>> +#define ASSIGN(field)                           \
>> +({                                              \
>> +    if ( (typeof(field))data.val != data.val )  \
>> +    {                                           \
>> +        rc = -EOVERFLOW;                        \
>> +        goto err;                               \
>> +    }                                           \
>> +    field = data.val;                           \
> Missing parentheses around "field" in the macro. Although I don't think
> it will break in practice, it is better to follow general macro writing
> rules.

Fixed.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] tools/cpu-policy: Add unit tests
  2019-02-25 15:47 ` [PATCH v2 3/3] tools/cpu-policy: Add unit tests Andrew Cooper
  2019-02-27 12:36   ` Wei Liu
@ 2019-03-11 15:18   ` Jan Beulich
  1 sibling, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2019-03-11 15:18 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Sergey Dyasli, Xen-devel, Wei Liu, Roger Pau Monne

>>> On 25.02.19 at 16:47, <andrew.cooper3@citrix.com> wrote:
> These 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>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects
  2019-02-27 12:31   ` Wei Liu
  2019-02-28 18:33     ` Andrew Cooper
@ 2019-03-11 16:00     ` Jan Beulich
  1 sibling, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2019-03-11 16:00 UTC (permalink / raw)
  To: Andrew Cooper, Wei Liu; +Cc: Sergey Dyasli, Xen-devel, Roger Pau Monne

>>> On 27.02.19 at 13:31, <wei.liu2@citrix.com> wrote:
> On Mon, Feb 25, 2019 at 03:47:13PM +0000, Andrew Cooper wrote:
>> +
>> +        switch ( data.idx )
>> +        {
>> +            /*
>> +             * Assign data.val to 'field', checking for truncation if the
>> +             * backing storage for 'field' is smaller than uint64_t
>> +             */
>> +#define ASSIGN(field)                           \
>> +({                                              \
>> +    if ( (typeof(field))data.val != data.val )  \
>> +    {                                           \
>> +        rc = -EOVERFLOW;                        \
>> +        goto err;                               \
>> +    }                                           \
>> +    field = data.val;                           \
> 
> Missing parentheses around "field" in the macro. Although I don't think
> it will break in practice, it is better to follow general macro writing
> rules.

With this, or even better with p-> pulled up here from the invocation,
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects
  2019-02-27 12:31   ` Wei Liu
@ 2019-03-11 16:02     ` Jan Beulich
  0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2019-03-11 16:02 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Sergey Dyasli, Xen-devel, Wei Liu, Roger Pau Monne

>>> On 27.02.19 at 13:31, <wei.liu2@citrix.com> wrote:
> On Mon, Feb 25, 2019 at 03:47:14PM +0000, Andrew Cooper wrote:
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-03-11 16:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-25 15:47 [PATCH v2 0/3] libx86: Remaining serialisation logic Andrew Cooper
2019-02-25 15:47 ` [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects Andrew Cooper
2019-02-27 12:31   ` Wei Liu
2019-02-28 18:33     ` Andrew Cooper
2019-03-11 16:00     ` Jan Beulich
2019-02-25 15:47 ` [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects Andrew Cooper
2019-02-27 12:31   ` Wei Liu
2019-03-11 16:02     ` Jan Beulich
2019-02-25 15:47 ` [PATCH v2 3/3] tools/cpu-policy: Add unit tests Andrew Cooper
2019-02-27 12:36   ` Wei Liu
2019-03-11 15:18   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.