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 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 | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index b60269174d95..cecbd0b95a15 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1996,6 +1996,37 @@ 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
+ * the faulty bitmask value.
+ */
+static void verify_required_features(const struct cpuinfo_x86 *c)
+{
+	u32 missing[NCAPINTS] = REQUIRED_MASK_INITIALIZER;
+	char cap_buf[16];
+	u32 error = 0;
+	unsigned int i;
+
+	for (i = 0; i < NCAPINTS; i++) {
+		missing[i] &= ~c->x86_capability[i];
+		error |= missing[i];
+	}
+
+	if (!error)
+		return;		/* All good */
+
+	/*
+	 * At least one required feature is missing. Print a warning,
+	 * and taint the kernel.
+	 */
+	pr_warn("cpu %d: missing required feature(s):", c->cpu_index);
+	for_each_set_bit(i, (void *)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...
  */
@@ -2125,6 +2156,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

On 3/2/2026 7:25 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
> + * the faulty bitmask value.

Aren't we printing the faulty feature name instead of the bitmask value?

> + */
> +static void verify_required_features(const struct cpuinfo_x86 *c)
> +{
> +	u32 missing[NCAPINTS] = REQUIRED_MASK_INITIALIZER;
> +	char cap_buf[16];
> +	u32 error = 0;
> +	unsigned int i;
> +

X86 prefers reverse Xmas order for variable declarations.

> +	for (i = 0; i < NCAPINTS; i++) {
> +		missing[i] &= ~c->x86_capability[i];
> +		error |= missing[i];
> +	}
> +
> +	if (!error)
> +		return;		/* All good */
> +

The tail comments should be avoided. This one is completely unnecessary
here.


> +	/*
> +	 * At least one required feature is missing. Print a warning,
> +	 * and taint the kernel.
> +	 */

The "print a warning, and taint the kernel" part seems redundant.
Probably there is no need for a comment here as well.

> +	pr_warn("cpu %d: missing required feature(s):", c->cpu_index);
> +	for_each_set_bit(i, (void *)missing, NCAPINTS << 5)

for_each_set_bit() typically expects unsigned long *. Do you run into
any issue if you use that?

> +		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...
>   */
> @@ -2125,6 +2156,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>  	mcheck_cpu_init(c);
>  
>  	numa_add_cpu(smp_processor_id());
> +
> +	verify_required_features(c);
>  }
>  
>  /*

Thanks for the detailed review!

On 2026-03-10 at 00:05:24 -0700, Sohil Mehta wrote:
>On 3/2/2026 7:25 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
>> + * the faulty bitmask value.
>
>Aren't we printing the faulty feature name instead of the bitmask value?

Right, I think that was the previous idea for this function, thanks for spotting
that.

>
>> + */
>> +static void verify_required_features(const struct cpuinfo_x86 *c)
>> +{
>> +	u32 missing[NCAPINTS] = REQUIRED_MASK_INITIALIZER;
>> +	char cap_buf[16];
>> +	u32 error = 0;
>> +	unsigned int i;
>> +
>
>X86 prefers reverse Xmas order for variable declarations.

Sure, I'll clear it up.

>
>> +	for (i = 0; i < NCAPINTS; i++) {
>> +		missing[i] &= ~c->x86_capability[i];
>> +		error |= missing[i];
>> +	}
>> +
>> +	if (!error)
>> +		return;		/* All good */
>> +
>
>The tail comments should be avoided. This one is completely unnecessary
>here.

Fair enough, I can remove it.

>
>> +	/*
>> +	 * At least one required feature is missing. Print a warning,
>> +	 * and taint the kernel.
>> +	 */
>
>The "print a warning, and taint the kernel" part seems redundant.
>Probably there is no need for a comment here as well.

I would keep the 'At least one required feature is missing' so it's more
readable for someone looking at this for the first time. Looking at it now I
agree the second sentence doesn't help with anything though.

>
>> +	pr_warn("cpu %d: missing required feature(s):", c->cpu_index);
>> +	for_each_set_bit(i, (void *)missing, NCAPINTS << 5)
>
>for_each_set_bit() typically expects unsigned long *. Do you run into
>any issue if you use that?


I set some required features to disabled in the x86_capability[] array for a
test and it worked fine. But you're right, it should be a unsigned long *. I'll
change it and retest.

-- 
Kind regards
Maciej Wieczór-Retman