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@xxxxxxxxx>
> 
> 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@xxxxxxxxx>
> Acked-by: H. Peter Anvin (Intel) <hpa@xxxxxxxxx>
> ---

> 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);
>  }
>  
>  /*