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.0On 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...
> */
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
© 2016 - 2026 Red Hat, Inc.