When adding new words to a featureset, there is a reasonable amount of
boilerplate and it is preforable to split the addition into multiple patches.
GCC 12 spotted a real (transient) error which occurs when splitting additions
like this. Right now, FEATURESET_NR_ENTRIES is dynamically generated from the
highest numeric XEN_CPUFEATURE() value, and can be less than what the
FEATURESET_* constants suggest the length of a featureset bitmap ought to be.
This causes the policy <-> featureset converters to genuinely access
out-of-bounds on the featureset array.
Rework X86_NR_FEAT to be related to FEATURESET_* alone, allowing it
specifically to grow larger than FEATURESET_NR_ENTRIES.
Reported-by: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Roger Pau Monné <roger....@citrix.com>
CC: Wei Liu <w...@xen.org>
To preempt what I expect will be the first review question, no FEATURESET_*
can't become an enumeration, because the constants undergo token concatination
in the preprocess as part of making DECL_BITFIELD() work.
---
xen/arch/x86/cpu-policy.c | 7 +++++++
xen/arch/x86/include/asm/cpufeatures.h | 5 +----
xen/include/xen/lib/x86/cpu-policy.h | 4 ++--
xen/include/xen/lib/x86/cpuid-consts.h | 2 ++
xen/lib/x86/cpuid.c | 6 +++---
5 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 774c512a03bd..00416244a3d8 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -883,6 +883,13 @@ void __init init_dom0_cpuid_policy(struct domain *d)
static void __init __maybe_unused build_assertions(void)
{
+ /*
+ * Generally these are the same, but tend to differ when adding new
+ * infrastructure split across several patches. Simply confirm that the
+ * gen-cpuid.py X86_FEATURE_* bits fit within the bitmaps we operate on.
+ */
+ BUILD_BUG_ON(FEATURESET_NR_ENTRIES > X86_NR_FEAT);
+
/* Find some more clever allocation scheme if this trips. */
BUILD_BUG_ON(sizeof(struct cpu_policy) > PAGE_SIZE);
diff --git a/xen/arch/x86/include/asm/cpufeatures.h
b/xen/arch/x86/include/asm/cpufeatures.h
index 408ab4ba16a5..8989291bbfd6 100644
--- a/xen/arch/x86/include/asm/cpufeatures.h
+++ b/xen/arch/x86/include/asm/cpufeatures.h
@@ -2,10 +2,7 @@
* Explicitly intended for multiple inclusion.
*/
-#include <xen/lib/x86/cpuid-autogen.h>
-
-/* Number of capability words covered by the featureset words. */
-#define X86_NR_FEAT FEATURESET_NR_ENTRIES
+#include <xen/lib/x86/cpuid-consts.h>
/* Synthetic words follow the featureset words. */
#define X86_NR_SYNTH 1
diff --git a/xen/include/xen/lib/x86/cpu-policy.h
b/xen/include/xen/lib/x86/cpu-policy.h
index e9bda14a7595..01431de056c8 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -370,12 +370,12 @@ struct cpu_policy_errors
* Copy the featureset words out of a cpu_policy object.
*/
void x86_cpu_policy_to_featureset(const struct cpu_policy *p,
- uint32_t fs[FEATURESET_NR_ENTRIES]);
+ uint32_t fs[X86_NR_FEAT]);
/**
* Copy the featureset words back into a cpu_policy object.
*/
-void x86_cpu_featureset_to_policy(const uint32_t fs[FEATURESET_NR_ENTRIES],
+void x86_cpu_featureset_to_policy(const uint32_t fs[X86_NR_FEAT],
struct cpu_policy *p);
static inline uint64_t cpu_policy_xcr0_max(const struct cpu_policy *p)
diff --git a/xen/include/xen/lib/x86/cpuid-consts.h
b/xen/include/xen/lib/x86/cpuid-consts.h
index 6ca8c39a3df4..9fe931b8e31f 100644
--- a/xen/include/xen/lib/x86/cpuid-consts.h
+++ b/xen/include/xen/lib/x86/cpuid-consts.h
@@ -21,6 +21,8 @@
#define FEATURESET_7c1 14 /* 0x00000007:1.ecx */
#define FEATURESET_7d1 15 /* 0x00000007:1.edx */
+#define X86_NR_FEAT (FEATURESET_7d1 + 1)
+
#endif /* !XEN_LIB_X86_CONSTS_H */
/*
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index 68aafb404927..76f26e92af8d 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -61,7 +61,7 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor)
}
void x86_cpu_policy_to_featureset(
- const struct cpu_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
+ const struct cpu_policy *p, uint32_t fs[X86_NR_FEAT])
{
fs[FEATURESET_1d] = p->basic._1d;
fs[FEATURESET_1c] = p->basic._1c;
@@ -82,7 +82,7 @@ void x86_cpu_policy_to_featureset(
}
void x86_cpu_featureset_to_policy(
- const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpu_policy *p)
+ const uint32_t fs[X86_NR_FEAT], struct cpu_policy *p)
{
p->basic._1d = fs[FEATURESET_1d];
p->basic._1c = fs[FEATURESET_1c];
@@ -285,7 +285,7 @@ const uint32_t *x86_cpu_policy_lookup_deep_deps(uint32_t
feature)
static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
static const struct {
uint32_t feature;
- uint32_t fs[FEATURESET_NR_ENTRIES];
+ uint32_t fs[X86_NR_FEAT];
} deep_deps[] = INIT_DEEP_DEPS;
unsigned int start = 0, end = ARRAY_SIZE(deep_deps);
--
2.30.2
