[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 5/6] x86/cpu-policy: Disentangle X86_NR_FEAT and FEATURESET_NR_ENTRIES


  • To: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Thu, 4 May 2023 20:39:23 +0100
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Thu, 04 May 2023 19:39:40 +0000
  • Ironport-data: A9a23:kJpQzKuPcYcZMGFx809kXMhjeOfnVCdeMUV32f8akzHdYApBsoF/q tZmKTyCa6yKMGX2foh0aY209hkDu5SAxtFrHQVkpHsxHigR+JbJXdiXEBz9bniYRiHhoOCLz O1FM4Wdc5pkJpP4jk3wWlQ0hSAkjclkfpKlVKiffHg3HVQ+IMsYoUoLs/YjhYJ1isSODQqIu Nfjy+XSI1bg0DNvWo4uw/vrRChH4bKj6Vv0gnRkPaoQ5AKGyyFPZH4iDfrZw0XQE9E88tGSH 44v/JnhlkvF8hEkDM+Sk7qTWiXmlZaLYGBiIlIPM0STqkAqSh4ai87XB9JFAatjsB2bnsgZ9 Tl4ncfYpTHFnEH7sL91vxFwS0mSNEDdkVPNCSDXXce7lyUqf5ZwqhnH4Y5f0YAwo45K7W9yG fMwEz8saE25nNiPkY2FStA3r50DE5HxBdZK0p1g5Wmx4fcORJnCR+PB5MNC3Sd2jcdLdRrcT 5NHM3w1Nk2GOkARfA5NU/rSn8/x7pX7WxRepEiYuuwc5G/LwRYq+LPsLMDUapqBQsA9ckOw/ zqZrj+gXEhDXDCZ4WSP9nzzud3dpwDce4NIN7GX7uYyonTGkwT/DzVJDADm8JFVkHWWRNZ3O 0ESvC00osAa5EGtC9XwQRC8iHqFpQIHHcpdFfUg7wOAwbaS5ByWbkAGRDNcbN0ttOctWCcnk FSOmrvU6SdH6ePPDyjHr/HN8G30YHJORYMfWcMaZTAKwt++mpoJt0PwcNZaS4fsruKtAwill lhmsxMCa6UvYd8jjvvrpgie2WLz+fAlXSZuuFyJAzvNAhdRIdf8Otf2sQWzAeNodt7xc7WXg JQTdyFyBsgqBIrFqiGCSf5l8FqBt6fca220bbKC8vAcG9WRF52LJ9o4DMlWfhsBDyr9UWaBj LXvkQ1Q/oRPG3ChcLV6ZYm8Y+xzk/i7T467CqmFNoERCnSUSDJrAQk0PRLAt4wTuBFEfV4D1 WezLp/3UCdy5VVPxzuqXeYNuYIWKtQF7TqLH/jTlk33uYdykVbJEd/pxnPSNLFmhE5FyS2Jm +ti2zyikUgEDrCkOHKPrub+7zkidBAGOHw/kOQPHsbrH+asMDtJ5yP5qV/5R7FYog==
  • Ironport-hdrordr: A9a23:dcGjLqjOHA+0jiMQx7THnu5Q8nBQXg4ji2hC6mlwRA09TyX4rb HKoB1/73LJYVkqN03I9ervBED4ewK5yXct2/h3AV7AZniFhILLFu1fBOLZqlWLJ8SZzI9gPM 9bGJSWY+eAbmSS4/yb3OFte+xQueVu78iT9JjjJ2YEd3ANV0l/hz0JcjpzGHcGODWvWPICZe GhDtIunUvbRZwNBv7Le0U4Yw==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <jbeulich@xxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.