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

Maciej Wieczor-Retman posted 3 patches 2 weeks, 6 days ago
There is a newer version of this series
[PATCH v10 3/3] x86/cpu: Do a sanity check on required feature bits
Posted by Maciej Wieczor-Retman 2 weeks, 6 days ago
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 v10:
- Shorten the comment before the sanity check.
- cpu -> CPU in the warning.
- NCAPINTS << 5 -> NCAPINTS * 32

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

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 0e318f3d56cb..badf86a26e24 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2005,6 +2005,33 @@ 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.
+ */
+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 * 32)
+		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...
  */
@@ -2134,6 +2161,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 v10 3/3] x86/cpu: Do a sanity check on required feature bits
Posted by Pawan Gupta 2 weeks, 4 days ago
On Tue, Mar 17, 2026 at 08:00:22PM +0000, 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>

Reviewed-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Re: [PATCH v10 3/3] x86/cpu: Do a sanity check on required feature bits
Posted by Maciej Wieczor-Retman 2 weeks, 4 days ago
On 2026-03-18 at 17:02:51 -0700, Pawan Gupta wrote:
>On Tue, Mar 17, 2026 at 08:00:22PM +0000, 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>
>
>Reviewed-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

Thanks for looking at the patches!

From a bot review that Sohil showed me I realized there is one thing to fix in
this patch, namely possible unaligned access in for_each_set_bit(). So I
probably should clear the tags from this patch for the next version.

-- 
Kind regards
Maciej Wieczór-Retman
Re: [PATCH v10 3/3] x86/cpu: Do a sanity check on required feature bits
Posted by Sohil Mehta 2 weeks, 6 days ago
On 3/17/2026 1:00 PM, 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>
> ---

A minor concern below. Other than that,

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


>  
> +/*
> + * As a sanity check compare the final x86_capability bitmask with the initial
> + * predefined required feature bits.
> + */
> +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 * 32)
> +		pr_cont(" %s", x86_cap_name(i, cap_buf));
> +	pr_cont("\n");

Could multiple APs run into this concurrently? I am wondering if there
is some risk of garbled output because of the pr_warn()/pr_cont()
combination.

Though, I don't have an easy fix if it is an issue.

> +	add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
> +}
> +
>  /*
>   * This does the hard work of actually picking apart the CPU stuff...
>   */
> @@ -2134,6 +2161,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 v10 3/3] x86/cpu: Do a sanity check on required feature bits
Posted by Maciej Wieczor-Retman 2 weeks, 5 days ago
On 2026-03-17 at 13:43:54 -0700, Sohil Mehta wrote:
>On 3/17/2026 1:00 PM, 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>
>> ---
>
>A minor concern below. Other than that,
>
>Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>

Thanks :)

>
>>  
>> +/*
>> + * As a sanity check compare the final x86_capability bitmask with the initial
>> + * predefined required feature bits.
>> + */
>> +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 * 32)
>> +		pr_cont(" %s", x86_cap_name(i, cap_buf));
>> +	pr_cont("\n");
>
>Could multiple APs run into this concurrently? I am wondering if there
>is some risk of garbled output because of the pr_warn()/pr_cont()
>combination.

Just tested it on 224 CPU Xeon, basically I cleared a bunch of required features
from the x86_capability[] on all but the boot cpu. The warning output was serial
and no race conditions or mangled strings were observed.

Not sure if it can go wrong in any other way.

>Though, I don't have an easy fix if it is an issue.
>

-- 
Kind regards
Maciej Wieczór-Retman