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@xxxxxxxxx>
> 
> 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@xxxxxxxxx>
> ---

> 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...
>   */