[PATCH v9 0/3] x86: Capability bits fix and required bits sanity check

[PATCH v9 0/3] x86: Capability bits fix and required bits sanity check

[Maciej Wieczor-Retman]

Series aims to fix the inconsistency between the cpuinfo behavior and
the documentation. Specifically the features that are not compiled are
still present in the cpuinfo bitmasks as enabled. This is not in line
with the documentation which specifies that not-compiled features are
not present in /proc/cpuinfo.

Along adding the disabled feature bitmask initializer array, the
complementary required bitmask initializer is also added. It can be used
to provide a sanity check, after the cpu identification is finished, to
make sure every required bit is set in the final bitmask. A warning with
the cpu number and all required bits that were not set is emitted in
case of the sanity check failure.

Before adding the sanity check a small cleanup can be done. Three places
open code an operation that retrieves either a feature string or, if the
string is not present, the feature number in word:bit format. One of
these places also doesn't check whether the string is actually there or
not. The cleanup patch fixes that and simplifies the other two
instances.

Patches are based on v7.0-rc3

Previous patchset versions:
v8: https://lore.kernel.org/all/cover.1772453012.git.m.wieczorretman@pm.me/
v7: https://lore.kernel.org/all/cover.1771936214.git.m.wieczorretman@pm.me/
v6: https://lore.kernel.org/all/cover.1771590895.git.m.wieczorretman@pm.me/
v5: https://lore.kernel.org/all/cover.1770908783.git.m.wieczorretman@pm.me/
v4: https://lore.kernel.org/all/20250724125346.2792543-1-maciej.wieczor-retman@intel.com/
v3: https://lore.kernel.org/all/20250724094554.2153919-1-maciej.wieczor-retman@intel.com/
v2: https://lore.kernel.org/all/20250723092250.3411923-1-maciej.wieczor-retman@intel.com/
v1: https://lore.kernel.org/all/20250722074439.4069992-1-maciej.wieczor-retman@intel.com/

Maciej Wieczor-Retman (3):
  x86/cpu: Clear feature bits disabled at compile-time
  x86/cpu: Check if feature string is non-zero
  x86/cpu: Do a sanity check on required feature bits

 arch/x86/include/asm/cpu.h         |  4 ++
 arch/x86/kernel/cpu/common.c       | 63 +++++++++++++++++++++++++++---
 arch/x86/kernel/cpu/cpuid-deps.c   | 23 +++--------
 arch/x86/tools/cpufeaturemasks.awk |  6 +++
 4 files changed, 72 insertions(+), 24 deletions(-)

-- 
2.53.0

[PATCH v9 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Maciej Wieczor-Retman]

From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

If some config options are disabled during compile time, they still are
enumerated in macros that use the x86_capability bitmask - cpu_has() or
this_cpu_has().

The features are also visible in /proc/cpuinfo even though they are not
enabled - which is contrary to what the documentation states about the
file. Examples of such feature flags are lam, fred, sgx, ibrs_enhanced,
split_lock_detect, user_shstk, avx_vnni and enqcmd.

Once the cpu_caps_cleared[] is initialized with the autogenerated
disabled bitmask apply_forced_caps() will clear the corresponding bits
in boot_cpu_data.x86_capability[] and other secondary CPUs'
cpu_data.x86_capability[]. Thus features disabled at compile time won't
show up in /proc/cpuinfo.

No BUGS are defined to be cleared at compile time, therefore only the
NCAPINTS part of cpu_caps_cleared[] is initialized. The NBUGINTS part is
set initialized to zero.

Reported-by: Farrah Chen <farrah.chen@intel.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220348
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v9:
- *_MASK_INITIALIZER -> *_MASK_INIT
- Remove Cc stable.
- Note that the BUGS part of cpu_caps_cleared[] is zeroed.

Changelog v6:
- Remove patch message portions that are not just describing the diff.

 arch/x86/kernel/cpu/common.c       | 3 ++-
 arch/x86/tools/cpufeaturemasks.awk | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index a8ff4376c286..76339e988304 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -735,7 +735,8 @@ static const char *table_lookup_model(struct cpuinfo_x86 *c)
 }
 
 /* Aligned to unsigned long to avoid split lock in atomic bitmap ops */
-__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
+__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long)) =
+	DISABLED_MASK_INIT;
 __u32 cpu_caps_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/tools/cpufeaturemasks.awk b/arch/x86/tools/cpufeaturemasks.awk
index 173d5bf2d999..9382bd15279a 100755
--- a/arch/x86/tools/cpufeaturemasks.awk
+++ b/arch/x86/tools/cpufeaturemasks.awk
@@ -82,6 +82,12 @@ END {
 		}
 		printf " 0\t\\\n";
 		printf "\t) & (1U << ((x) & 31)))\n\n";
+
+		printf "\n#define %s_MASK_INIT\t\t\t\\", s;
+		printf "\n\t{\t\t\t\t\t\t\\";
+		for (i = 0; i < ncapints; i++)
+			printf "\n\t\t%s_MASK%d,\t\t\t\\", s, i;
+		printf "\n\t}\n\n";
 	}
 
 	printf "#endif /* _ASM_X86_CPUFEATUREMASKS_H */\n";
-- 
2.53.0

[PATCH v9 2/3] x86/cpu: Check if feature string is non-zero

[Maciej Wieczor-Retman]

From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

In filter_cpuid_features, x86_cap_flags[] is read, but it's not verified
whether the string is non-zero which could lead to unwanted output.

In two more places there are open coded paths that try to retrieve a
feature string, and if there isn't one, the feature word and bit are
returned instead. While correcting filter_cpuid_features() with a helper
it's trivial to also clean up these open coded cases.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v9:
- 16 -> X86_CAP_BUF_SIZE.
- Add comment to the x86_cap_name().

Changelog v8:
- Move x86_cap_name() declaration from linux/cpu.h to the arch/cpu.h.
  Include arch/cpu.h in the cpuid-deps.c file instead of linux/cpu.h.

Changelog v7:
- sizeof(buf) -> 16
- Rebase onto 7.01-rc1.

Changelog v6:
- Remove parts of the patch message that are redundant and just copy
  what's visible in the diff.
- Redo the helper to use an external char buffer instead of a local
  static string.

 arch/x86/include/asm/cpu.h       |  4 ++++
 arch/x86/kernel/cpu/common.c     | 30 +++++++++++++++++++++++++-----
 arch/x86/kernel/cpu/cpuid-deps.c | 23 +++++------------------
 3 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index ad235dda1ded..93e8ad2786bf 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -9,6 +9,8 @@
 #include <linux/percpu.h>
 #include <asm/ibt.h>
 
+#define X86_CAP_BUF_SIZE			16
+
 #ifndef CONFIG_SMP
 #define cpu_physical_id(cpu)			boot_cpu_physical_apicid
 #define cpu_acpi_id(cpu)			0
@@ -67,4 +69,6 @@ int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type);
 
 extern struct cpumask cpus_stop_mask;
 
+const char *x86_cap_name(unsigned int bit, char *buf);
+
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 76339e988304..cfffbbda3d95 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -678,6 +678,7 @@ cpuid_dependent_features[] = {
 static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
 {
 	const struct cpuid_dependent_feature *df;
+	char feature_buf[X86_CAP_BUF_SIZE];
 
 	for (df = cpuid_dependent_features; df->feature; df++) {
 
@@ -700,7 +701,7 @@ static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
 			continue;
 
 		pr_warn("CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
-			x86_cap_flags[df->feature], df->level);
+			x86_cap_name(df->feature, feature_buf), df->level);
 	}
 }
 
@@ -1637,6 +1638,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
 
 	while (arg) {
 		bool found __maybe_unused = false;
+		char name_buf[X86_CAP_BUF_SIZE];
 		unsigned int bit;
 
 		opt = strsep(&arg, ",");
@@ -1657,10 +1659,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
 					setup_clear_cpu_cap(bit);
 				}
 				/* empty-string, i.e., ""-defined feature flags */
-				if (!x86_cap_flags[bit])
-					pr_cont(" %d:%d\n", bit >> 5, bit & 31);
-				else
-					pr_cont(" %s\n", x86_cap_flags[bit]);
+				pr_cont(" %s\n", x86_cap_name(bit, name_buf));
 
 				taint++;
 			}
@@ -1983,6 +1982,27 @@ static void generic_identify(struct cpuinfo_x86 *c)
 #endif
 }
 
+/*
+ * Return the feature "name" if available, otherwise return the
+ * X86_FEATURE_* numerals to make it easier to identify the feature.
+ */
+const char *x86_cap_name(unsigned int bit, char *buf)
+{
+	unsigned int word = bit >> 5;
+	const char *name = NULL;
+
+	if (likely(word < NCAPINTS))
+		name = x86_cap_flags[bit];
+	else if (likely(word < NCAPINTS + NBUGINTS))
+		name = x86_bug_flags[bit - 32 * NCAPINTS];
+
+	if (name)
+		return name;
+
+	snprintf(buf, X86_CAP_BUF_SIZE, "%u:%u", word, bit & 31);
+	return buf;
+}
+
 /*
  * This does the hard work of actually picking apart the CPU stuff...
  */
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 146f6f8b0650..49fd45b5fbd6 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -2,6 +2,8 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/module.h>
+
+#include <asm/cpu.h>
 #include <asm/cpufeature.h>
 
 struct cpuid_dep {
@@ -156,24 +158,9 @@ void setup_clear_cpu_cap(unsigned int feature)
 	do_clear_cpu_cap(NULL, feature);
 }
 
-/*
- * Return the feature "name" if available, otherwise return
- * the X86_FEATURE_* numerals to make it easier to identify
- * the feature.
- */
-static const char *x86_feature_name(unsigned int feature, char *buf)
-{
-	if (x86_cap_flags[feature])
-		return x86_cap_flags[feature];
-
-	snprintf(buf, 16, "%d*32+%2d", feature / 32, feature % 32);
-
-	return buf;
-}
-
 void check_cpufeature_deps(struct cpuinfo_x86 *c)
 {
-	char feature_buf[16], depends_buf[16];
+	char feature_buf[X86_CAP_BUF_SIZE], depends_buf[X86_CAP_BUF_SIZE];
 	const struct cpuid_dep *d;
 
 	for (d = cpuid_deps; d->feature; d++) {
@@ -185,8 +172,8 @@ void check_cpufeature_deps(struct cpuinfo_x86 *c)
 			 */
 			pr_warn_once("x86 CPU feature dependency check failure: CPU%d has '%s' enabled but '%s' disabled. Kernel might be fine, but no guarantees.\n",
 				     smp_processor_id(),
-				     x86_feature_name(d->feature, feature_buf),
-				     x86_feature_name(d->depends, depends_buf));
+				     x86_cap_name(d->feature, feature_buf),
+				     x86_cap_name(d->depends, depends_buf));
 		}
 	}
 }
-- 
2.53.0

[PATCH v9 3/3] x86/cpu: Do a sanity check on required feature bits

[Maciej Wieczor-Retman]

From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

After CPU identification concludes, do a sanity check by comparing the
final x86_capability bitmask with the pre-defined required feature bits.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
Acked-by: H. Peter Anvin (Intel) <hpa@zytor.com>
---
Changelog v9:
- REQUIRED_MASK_INITIALIZER -> REQUIRED_MASK_INIT
- Redo the comments.
- Fix reverse xmas order.
- Inside for_each_set_bit: (void *) -> (unsigned long *).
- 16 -> X86_CAP_BUF_SIZE.

Changelog v6:
- Add Peter's acked-by tag.
- Rename patch subject to imperative form.
- Add a char buffer to the x86_cap_name() call.

 arch/x86/kernel/cpu/common.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index cfffbbda3d95..43ee2d55a708 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2003,6 +2003,34 @@ const char *x86_cap_name(unsigned int bit, char *buf)
 	return buf;
 }
 
+/*
+ * As a sanity check compare the final x86_capability bitmask with the initial
+ * predefined required feature bits. In case of a mismatch emit a warning with
+ * all the missing feature names or X86_FEATURE_* numerals.
+ */
+static void verify_required_features(const struct cpuinfo_x86 *c)
+{
+	u32 missing[NCAPINTS] = REQUIRED_MASK_INIT;
+	char cap_buf[X86_CAP_BUF_SIZE];
+	unsigned int i;
+	u32 error = 0;
+
+	for (i = 0; i < NCAPINTS; i++) {
+		missing[i] &= ~c->x86_capability[i];
+		error |= missing[i];
+	}
+
+	if (!error)
+		return;
+
+	/* At least one required feature is missing */
+	pr_warn("cpu %d: missing required feature(s):", c->cpu_index);
+	for_each_set_bit(i, (unsigned long *)missing, NCAPINTS << 5)
+		pr_cont(" %s", x86_cap_name(i, cap_buf));
+	pr_cont("\n");
+	add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
+}
+
 /*
  * This does the hard work of actually picking apart the CPU stuff...
  */
@@ -2132,6 +2160,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	mcheck_cpu_init(c);
 
 	numa_add_cpu(smp_processor_id());
+
+	verify_required_features(c);
 }
 
 /*
-- 
2.53.0

Re: [PATCH v9 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Sohil Mehta]

On 3/10/2026 11:03 AM, Maciej Wieczor-Retman wrote:
> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> 
> If some config options are disabled during compile time, they still are
> enumerated in macros that use the x86_capability bitmask - cpu_has() or
> this_cpu_has().
> 
> The features are also visible in /proc/cpuinfo even though they are not
> enabled - which is contrary to what the documentation states about the
> file. Examples of such feature flags are lam, fred, sgx, ibrs_enhanced,
> split_lock_detect, user_shstk, avx_vnni and enqcmd.
> 

Do all of these features have compile time options today? For example, I
couldn't find X86_DISABLED_FEATURE_* equivalent for ibrs_enhanced,
split_lock_detect or avx_vnni. Maybe I am missing some obvious connection.

But, if these can't be disabled using config options, it would be better
to avoid any mention of them here.

> Once the cpu_caps_cleared[] is initialized with the autogenerated
> disabled bitmask apply_forced_caps() will clear the corresponding bits
> in boot_cpu_data.x86_capability[] and other secondary CPUs'

This sentence was a bit hard to read for me. How about?

Initialize cpu_caps_cleared[] with an autogenerated disabled bitmask.
During CPU init, apply_forced_caps() will clear the corresponding bits
in struct cpuinfo_x86 for each CPU. Thus features disabled at...

> cpu_data.x86_capability[]. Thus features disabled at compile time won't
> show up in /proc/cpuinfo.
> 
> No BUGS are defined to be cleared at compile time, therefore only the
> NCAPINTS part of cpu_caps_cleared[] is initialized. The NBUGINTS part is

..NCAPINTS part of cpu_caps_cleared[] is initialized using the macro.
The NBUGINTS part is set to zero.

> set initialized to zero.
> 

You don't need the word 'initialized' here.


> Reported-by: Farrah Chen <farrah.chen@intel.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220348
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---

>  arch/x86/kernel/cpu/common.c       | 3 ++-
>  arch/x86/tools/cpufeaturemasks.awk | 6 ++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 

The code changes look good to me,

Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>

Re: [PATCH v9 2/3] x86/cpu: Check if feature string is non-zero

[Sohil Mehta]

On 3/10/2026 11:03 AM, Maciej Wieczor-Retman wrote:
> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> 
> In filter_cpuid_features, x86_cap_flags[] is read, but it's not verified

filter_cpuid_features()

> whether the string is non-zero which could lead to unwanted output.
> 
> In two more places there are open coded paths that try to retrieve a
> feature string, and if there isn't one, the feature word and bit are
> returned instead. 

How about wording the next sentence as:

Add a common helper to fix filter_cpuid_features() as well as clean up
the open coded cases.

> While correcting filter_cpuid_features() with a helper
> it's trivial to also clean up these open coded cases.
> 
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---

> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index ad235dda1ded..93e8ad2786bf 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -9,6 +9,8 @@
>  #include <linux/percpu.h>
>  #include <asm/ibt.h>
>  
> +#define X86_CAP_BUF_SIZE			16
> +
>  #ifndef CONFIG_SMP
>  #define cpu_physical_id(cpu)			boot_cpu_physical_apicid
>  #define cpu_acpi_id(cpu)			0
> @@ -67,4 +69,6 @@ int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type);
>  
>  extern struct cpumask cpus_stop_mask;
>  
> +const char *x86_cap_name(unsigned int bit, char *buf);
> +

These declarations - X86_CAP_BUF_SIZE and x86_cap_name() are better
suited to asm/cpufeature.h instead of asm/cpu.h.


Also, it would make more sense to have the #define closer to the
function declaration. Maybe, right above it?


>  #endif /* _ASM_X86_CPU_H */

...

>  
> +/*
> + * Return the feature "name" if available, otherwise return the
> + * X86_FEATURE_* numerals to make it easier to identify the feature.
> + */

Should we add a sentence here to say that all callers must pass a buffer
of size X86_CAP_BUF_SIZE.

> +const char *x86_cap_name(unsigned int bit, char *buf)
> +{
> +	unsigned int word = bit >> 5;
> +	const char *name = NULL;
> +
> +	if (likely(word < NCAPINTS))
> +		name = x86_cap_flags[bit];
> +	else if (likely(word < NCAPINTS + NBUGINTS))
> +		name = x86_bug_flags[bit - 32 * NCAPINTS];
> +

Can we get rid of the two likely() annotations here? Is x86_cap_name()
called from any performance critical path?

> +	if (name)
> +		return name;
> +
> +	snprintf(buf, X86_CAP_BUF_SIZE, "%u:%u", word, bit & 31);
> +	return buf;
> +}
> +
>  /*
>   * This does the hard work of actually picking apart the CPU stuff...
>   */

Re: [PATCH v9 3/3] x86/cpu: Do a sanity check on required feature bits

[Sohil Mehta]

On 3/10/2026 11:03 AM, Maciej Wieczor-Retman wrote:
> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> 
> After CPU identification concludes, do a sanity check by comparing the
> final x86_capability bitmask with the pre-defined required feature bits.
> 
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> Acked-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> ---

> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index cfffbbda3d95..43ee2d55a708 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -2003,6 +2003,34 @@ const char *x86_cap_name(unsigned int bit, char *buf)
>  	return buf;
>  }
>  
> +/*
> + * As a sanity check compare the final x86_capability bitmask with the initial
> + * predefined required feature bits. In case of a mismatch emit a warning with
> + * all the missing feature names or X86_FEATURE_* numerals.
> + */

I feel the second sentence is more appropriate as part of the commit
message instead of a code comment.

> +static void verify_required_features(const struct cpuinfo_x86 *c)
> +{
> +	u32 missing[NCAPINTS] = REQUIRED_MASK_INIT;
> +	char cap_buf[X86_CAP_BUF_SIZE];
> +	unsigned int i;
> +	u32 error = 0;
> +
> +	for (i = 0; i < NCAPINTS; i++) {
> +		missing[i] &= ~c->x86_capability[i];
> +		error |= missing[i];
> +	}
> +
> +	if (!error)
> +		return;
> +
> +	/* At least one required feature is missing */
> +	pr_warn("cpu %d: missing required feature(s):", c->cpu_index);

s/cpu/CPU

I haven't seen cpu_index being used commonly. I don't know the nuance
between that and smp_processor_id(). It's probably the same in this case.

Also, it seems too noisy to print this for every CPU in a large core
count system. But, pr_warn_once() won't work well with the pr_cont()
implementation that you have.

I don't have a better suggestion except for a static variable. Maybe
someone else has a better idea..

My assumption is that typically we won't run into this on a production
system. But, VMs could encounter this. So, I am not sure.

> +	for_each_set_bit(i, (unsigned long *)missing, NCAPINTS << 5)

NCAPINTS * 32 is the common usage across arch/x86.

> +		pr_cont(" %s", x86_cap_name(i, cap_buf));
> +	pr_cont("\n");
> +	add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
> +}
> +
>  /*
>   * This does the hard work of actually picking apart the CPU stuff...
>   */
> @@ -2132,6 +2160,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>  	mcheck_cpu_init(c);
>  
>  	numa_add_cpu(smp_processor_id());
> +
> +	verify_required_features(c);
>  }
>  
>  /*

Re: [PATCH v9 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Maciej Wieczor-Retman]

On 2026-03-10 at 15:00:30 -0700, Sohil Mehta wrote:
>On 3/10/2026 11:03 AM, Maciej Wieczor-Retman wrote:
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> 
>> If some config options are disabled during compile time, they still are
>> enumerated in macros that use the x86_capability bitmask - cpu_has() or
>> this_cpu_has().
>> 
>> The features are also visible in /proc/cpuinfo even though they are not
>> enabled - which is contrary to what the documentation states about the
>> file. Examples of such feature flags are lam, fred, sgx, ibrs_enhanced,
>> split_lock_detect, user_shstk, avx_vnni and enqcmd.
>> 
>
>Do all of these features have compile time options today? For example, I
>couldn't find X86_DISABLED_FEATURE_* equivalent for ibrs_enhanced,
>split_lock_detect or avx_vnni. Maybe I am missing some obvious connection.
>
>But, if these can't be disabled using config options, it would be better
>to avoid any mention of them here.

Okay, I'll clean this up. I included these because previously I intended to
backport the changes as far as the documentation specified cpuinfo should work
this way. But yeah, now that we're not doing that it's best to remove them.
Thanks!

>
>> Once the cpu_caps_cleared[] is initialized with the autogenerated
>> disabled bitmask apply_forced_caps() will clear the corresponding bits
>> in boot_cpu_data.x86_capability[] and other secondary CPUs'
>
>This sentence was a bit hard to read for me. How about?
>
>Initialize cpu_caps_cleared[] with an autogenerated disabled bitmask.
>During CPU init, apply_forced_caps() will clear the corresponding bits
>in struct cpuinfo_x86 for each CPU. Thus features disabled at...

That works for me, the version above was my attempt to describe the diff as
little as possible while keeping the relevant information. Perhaps that sentence
got a bit too long.

>> cpu_data.x86_capability[]. Thus features disabled at compile time won't
>> show up in /proc/cpuinfo.
>> 
>> No BUGS are defined to be cleared at compile time, therefore only the
>> NCAPINTS part of cpu_caps_cleared[] is initialized. The NBUGINTS part is
>
>..NCAPINTS part of cpu_caps_cleared[] is initialized using the macro.
>The NBUGINTS part is set to zero.
>
>> set initialized to zero.
>> 
>
>You don't need the word 'initialized' here.

Okay, I'll remove it.

>
>> Reported-by: Farrah Chen <farrah.chen@intel.com>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220348
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>
>>  arch/x86/kernel/cpu/common.c       | 3 ++-
>>  arch/x86/tools/cpufeaturemasks.awk | 6 ++++++
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>> 
>
>The code changes look good to me,
>
>Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
>

Thanks :)

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 2/3] x86/cpu: Check if feature string is non-zero

[Maciej Wieczor-Retman]

On 2026-03-10 at 15:35:51 -0700, Sohil Mehta wrote:
>On 3/10/2026 11:03 AM, Maciej Wieczor-Retman wrote:
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> 
>> In filter_cpuid_features, x86_cap_flags[] is read, but it's not verified
>
>filter_cpuid_features()

Sure, I'll change it

>
>> whether the string is non-zero which could lead to unwanted output.
>> 
>> In two more places there are open coded paths that try to retrieve a
>> feature string, and if there isn't one, the feature word and bit are
>> returned instead. 
>
>How about wording the next sentence as:
>
>Add a common helper to fix filter_cpuid_features() as well as clean up
>the open coded cases.

Yes, that sounds okay.

>> While correcting filter_cpuid_features() with a helper
>> it's trivial to also clean up these open coded cases.
>> 
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>
>> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
>> index ad235dda1ded..93e8ad2786bf 100644
>> --- a/arch/x86/include/asm/cpu.h
>> +++ b/arch/x86/include/asm/cpu.h
>> @@ -9,6 +9,8 @@
>>  #include <linux/percpu.h>
>>  #include <asm/ibt.h>
>>  
>> +#define X86_CAP_BUF_SIZE			16
>> +
>>  #ifndef CONFIG_SMP
>>  #define cpu_physical_id(cpu)			boot_cpu_physical_apicid
>>  #define cpu_acpi_id(cpu)			0
>> @@ -67,4 +69,6 @@ int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type);
>>  
>>  extern struct cpumask cpus_stop_mask;
>>  
>> +const char *x86_cap_name(unsigned int bit, char *buf);
>> +
>
>These declarations - X86_CAP_BUF_SIZE and x86_cap_name() are better
>suited to asm/cpufeature.h instead of asm/cpu.h.
>
>
>Also, it would make more sense to have the #define closer to the
>function declaration. Maybe, right above it?

Sure, I was wondering what the best place would be. asm/cpufeature.h does make
more sense.

>
>>  #endif /* _ASM_X86_CPU_H */
>
>...
>
>>  
>> +/*
>> + * Return the feature "name" if available, otherwise return the
>> + * X86_FEATURE_* numerals to make it easier to identify the feature.
>> + */
>
>Should we add a sentence here to say that all callers must pass a buffer
>of size X86_CAP_BUF_SIZE.

Yes, perhaps it's a good idea.

>
>> +const char *x86_cap_name(unsigned int bit, char *buf)
>> +{
>> +	unsigned int word = bit >> 5;
>> +	const char *name = NULL;
>> +
>> +	if (likely(word < NCAPINTS))
>> +		name = x86_cap_flags[bit];
>> +	else if (likely(word < NCAPINTS + NBUGINTS))
>> +		name = x86_bug_flags[bit - 32 * NCAPINTS];
>> +
>
>Can we get rid of the two likely() annotations here? Is x86_cap_name()
>called from any performance critical path?

As far as I can tell not really but it does get called during boot time a bunch
of times. But is it an issue to sprinkle these likely() calls in non-critical
performance paths?

>
>> +	if (name)
>> +		return name;
>> +
>> +	snprintf(buf, X86_CAP_BUF_SIZE, "%u:%u", word, bit & 31);
>> +	return buf;
>> +}
>> +
>>  /*
>>   * This does the hard work of actually picking apart the CPU stuff...
>>   */

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 3/3] x86/cpu: Do a sanity check on required feature bits

[Maciej Wieczor-Retman]

On 2026-03-10 at 16:13:25 -0700, Sohil Mehta wrote:
>On 3/10/2026 11:03 AM, Maciej Wieczor-Retman wrote:
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> 
>> After CPU identification concludes, do a sanity check by comparing the
>> final x86_capability bitmask with the pre-defined required feature bits.
>> 
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> Acked-by: H. Peter Anvin (Intel) <hpa@zytor.com>
>> ---
>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index cfffbbda3d95..43ee2d55a708 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -2003,6 +2003,34 @@ const char *x86_cap_name(unsigned int bit, char *buf)
>>  	return buf;
>>  }
>>  
>> +/*
>> + * As a sanity check compare the final x86_capability bitmask with the initial
>> + * predefined required feature bits. In case of a mismatch emit a warning with
>> + * all the missing feature names or X86_FEATURE_* numerals.
>> + */
>
>I feel the second sentence is more appropriate as part of the commit
>message instead of a code comment.

Okay, yeah, I can move it there.

>
>> +static void verify_required_features(const struct cpuinfo_x86 *c)
>> +{
>> +	u32 missing[NCAPINTS] = REQUIRED_MASK_INIT;
>> +	char cap_buf[X86_CAP_BUF_SIZE];
>> +	unsigned int i;
>> +	u32 error = 0;
>> +
>> +	for (i = 0; i < NCAPINTS; i++) {
>> +		missing[i] &= ~c->x86_capability[i];
>> +		error |= missing[i];
>> +	}
>> +
>> +	if (!error)
>> +		return;
>> +
>> +	/* At least one required feature is missing */
>> +	pr_warn("cpu %d: missing required feature(s):", c->cpu_index);
>
>s/cpu/CPU

Sure

>
>I haven't seen cpu_index being used commonly. I don't know the nuance
>between that and smp_processor_id(). It's probably the same in this case.

cpu_index gets set to 0 on the boot CPU before identify_cpu() is called and
is set to the cpu variable in identify_secondary_cpu(). So I think it should
work okay in this case.

>Also, it seems too noisy to print this for every CPU in a large core
>count system. But, pr_warn_once() won't work well with the pr_cont()
>implementation that you have.
>
>I don't have a better suggestion except for a static variable. Maybe
>someone else has a better idea..
>
>My assumption is that typically we won't run into this on a production
>system. But, VMs could encounter this. So, I am not sure.

As it is a sanity check the assumption is this should not happen unless
something is quite wrong on the system. At that point more warning logs seem
helpful rather than noisy. I only managed to trigger this warning by modifying
the boot code to clear some capability bits just before this function. Not sure
if there are any other trivial ways to trigger this by accident.

>
>> +	for_each_set_bit(i, (unsigned long *)missing, NCAPINTS << 5)
>
>NCAPINTS * 32 is the common usage across arch/x86.

Oh, indeed, I'll change it.

>
>> +		pr_cont(" %s", x86_cap_name(i, cap_buf));
>> +	pr_cont("\n");
>> +	add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
>> +}
>> +
>>  /*
>>   * This does the hard work of actually picking apart the CPU stuff...
>>   */
>> @@ -2132,6 +2160,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>>  	mcheck_cpu_init(c);
>>  
>>  	numa_add_cpu(smp_processor_id());
>> +
>> +	verify_required_features(c);
>>  }
>>  
>>  /*
>

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 3/3] x86/cpu: Do a sanity check on required feature bits

[Maciej Wieczor-Retman]

On 2026-03-11 at 14:33:29 +0000, Maciej Wieczor-Retman wrote:
>On 2026-03-10 at 16:13:25 -0700, Sohil Mehta wrote:
>>On 3/10/2026 11:03 AM, Maciej Wieczor-Retman wrote:
>>> +/*
>>> + * As a sanity check compare the final x86_capability bitmask with the initial
>>> + * predefined required feature bits. In case of a mismatch emit a warning with
>>> + * all the missing feature names or X86_FEATURE_* numerals.
>>> + */
>>
>>I feel the second sentence is more appropriate as part of the commit
>>message instead of a code comment.
>
>Okay, yeah, I can move it there.

On second thought, looking at it more, it's probably quite visible from the diff
what's gonna happen on mismatch. Perhaps it's better if I just remove this line
from the comment.

-- 
Kind regards
Maciej Wieczór-Retman