Use such helper in order to replace the code in
x86_msr_copy_from_buffer. Note the introduced helper should not be
directly called and instead x86_msr_get_entry should be used that will
properly deal with const and non-const inputs.
Note this requires making the raw fields uint64_t so that it can
accommodate the maximum size of MSRs values, and in turn removing the
truncation tests.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v4:
- Rename _x86_msr_get_entry to x86_msr_get_entry_const.
- Add newline before endif.
Changes since v3:
- New in this version.
---
tools/tests/cpu-policy/test-cpu-policy.c | 48 +++++++++++++++++++-----
xen/include/xen/lib/x86/msr.h | 20 +++++++++-
xen/lib/x86/msr.c | 41 ++++++++++----------
3 files changed, 76 insertions(+), 33 deletions(-)
diff --git a/tools/tests/cpu-policy/test-cpu-policy.c b/tools/tests/cpu-policy/test-cpu-policy.c
index 3f777fc1fc..686d7a886c 100644
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -386,16 +386,6 @@ static void test_msr_deserialise_failure(void)
.msr = { .idx = 0xce, .flags = 1 },
.rc = -EINVAL,
},
- {
- .name = "truncated val",
- .msr = { .idx = 0xce, .val = ~0ull },
- .rc = -EOVERFLOW,
- },
- {
- .name = "truncated val",
- .msr = { .idx = 0x10a, .val = ~0ull },
- .rc = -EOVERFLOW,
- },
};
printf("Testing MSR deserialise failure:\n");
@@ -644,6 +634,43 @@ static void test_cpuid_get_leaf_failure(void)
}
}
+static void test_msr_get_entry(void)
+{
+ static const struct test {
+ const char *name;
+ unsigned int idx;
+ bool success;
+ } tests[] = {
+ {
+ .name = "bad msr index",
+ .idx = -1,
+ },
+ {
+ .name = "good msr index",
+ .idx = 0xce,
+ .success = true,
+ },
+ };
+ const struct msr_policy pc;
+ const uint64_t *ec;
+ struct msr_policy p;
+ uint64_t *e;
+
+ /* Constness build test. */
+ ec = x86_msr_get_entry(&pc, 0);
+ e = x86_msr_get_entry(&p, 0);
+
+ printf("Testing MSR get leaf:\n");
+
+ for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
+ {
+ const struct test *t = &tests[i];
+
+ if ( !!x86_msr_get_entry(&pc, t->idx) != t->success )
+ fail(" Test %s failed\n", t->name);
+ }
+}
+
static void test_is_compatible_success(void)
{
static struct test {
@@ -763,6 +790,7 @@ int main(int argc, char **argv)
test_msr_serialise_success();
test_msr_deserialise_failure();
+ test_msr_get_entry();
test_is_compatible_success();
test_is_compatible_failure();
diff --git a/xen/include/xen/lib/x86/msr.h b/xen/include/xen/lib/x86/msr.h
index 48ba4a59c0..4d84b7cf27 100644
--- a/xen/include/xen/lib/x86/msr.h
+++ b/xen/include/xen/lib/x86/msr.h
@@ -17,7 +17,7 @@ struct msr_policy
* is dependent on real hardware support.
*/
union {
- uint32_t raw;
+ uint64_t raw;
struct {
uint32_t :31;
bool cpuid_faulting:1;
@@ -32,7 +32,7 @@ struct msr_policy
* fixed in hardware.
*/
union {
- uint32_t raw;
+ uint64_t raw;
struct {
bool rdcl_no:1;
bool ibrs_all:1;
@@ -91,6 +91,22 @@ int x86_msr_copy_from_buffer(struct msr_policy *policy,
const msr_entry_buffer_t msrs, uint32_t nr_entries,
uint32_t *err_msr);
+/**
+ * Get a MSR entry from a policy object.
+ *
+ * @param policy The msr_policy object.
+ * @param idx The index.
+ * @returns a pointer to the requested leaf or NULL in case of error.
+ *
+ * Do not call this function directly and instead use x86_msr_get_entry that
+ * will deal with both const and non-const policies returning a pointer with
+ * constness matching that of the input.
+ */
+const uint64_t *x86_msr_get_entry_const(const struct msr_policy *policy,
+ uint32_t idx);
+#define x86_msr_get_entry(p, i) \
+ ((__typeof__(&(p)->platform_info.raw))x86_msr_get_entry_const(p, i))
+
#endif /* !XEN_LIB_X86_MSR_H */
/*
diff --git a/xen/lib/x86/msr.c b/xen/lib/x86/msr.c
index 7d71e92a38..e9b337dd70 100644
--- a/xen/lib/x86/msr.c
+++ b/xen/lib/x86/msr.c
@@ -74,6 +74,8 @@ int x86_msr_copy_from_buffer(struct msr_policy *p,
for ( i = 0; i < nr_entries; i++ )
{
+ uint64_t *val;
+
if ( copy_from_buffer_offset(&data, msrs, i, 1) )
return -EFAULT;
@@ -83,31 +85,13 @@ int x86_msr_copy_from_buffer(struct msr_policy *p,
goto err;
}
- switch ( data.idx )
+ val = x86_msr_get_entry(p, data.idx);
+ if ( !val )
{
- /*
- * Assign data.val to p->field, checking for truncation if the
- * backing storage for field is smaller than uint64_t
- */
-#define ASSIGN(field) \
-({ \
- if ( (typeof(p->field))data.val != data.val ) \
- { \
- rc = -EOVERFLOW; \
- goto err; \
- } \
- p->field = data.val; \
-})
-
- case MSR_INTEL_PLATFORM_INFO: ASSIGN(platform_info.raw); break;
- case MSR_ARCH_CAPABILITIES: ASSIGN(arch_caps.raw); break;
-
-#undef ASSIGN
-
- default:
rc = -ERANGE;
goto err;
}
+ *val = data.val;
}
return 0;
@@ -119,6 +103,21 @@ int x86_msr_copy_from_buffer(struct msr_policy *p,
return rc;
}
+const uint64_t *x86_msr_get_entry_const(const struct msr_policy *policy,
+ uint32_t idx)
+{
+ switch ( idx )
+ {
+ case MSR_INTEL_PLATFORM_INFO:
+ return &policy->platform_info.raw;
+
+ case MSR_ARCH_CAPABILITIES:
+ return &policy->arch_caps.raw;
+ }
+
+ return NULL;
+}
+
/*
* Local variables:
* mode: C
--
2.33.0