[PATCH v8 0/3] x86: Capability bits fix and required bits sanity check

[PATCH v8 0/3] x86: Capability bits fix and required bits sanity check

[Maciej Wieczor-Retman]

Series aims to fix the inconsistency between the cpuinfo behavior and
the documentation. Specifically the features that are not compiled are
still present in the cpuinfo bitmasks as enabled. This is not in line
with the documentation which specifies that not-compiled features are
not present in /proc/cpuinfo.

Along adding the disabled feature bitmask initializer array, the
complementary required bitmask initializer is also added. It can be used
to provide a sanity check, after the cpu identification is finished, to
make sure every required bit is set in the final bitmask. A warning with
the cpu number and all required bits that were not set is emitted in
case of the sanity check failure.

Before adding the sanity check a small cleanup can be done. Three places
open code an operation that retrieves either a feature string or, if the
string is not present, the feature number in word:bit format. One of
these places also doesn't check whether the string is actually there or
not. The cleanup patch fixes that and simplifies the other two
instances.

Patches are based on v7.0-rc2

Previous patchset versions:
v7: https://lore.kernel.org/all/cover.1771936214.git.m.wieczorretman@pm.me/
v6: https://lore.kernel.org/all/cover.1771590895.git.m.wieczorretman@pm.me/
v5: https://lore.kernel.org/all/cover.1770908783.git.m.wieczorretman@pm.me/
v4: https://lore.kernel.org/all/20250724125346.2792543-1-maciej.wieczor-retman@intel.com/
v3: https://lore.kernel.org/all/20250724094554.2153919-1-maciej.wieczor-retman@intel.com/
v2: https://lore.kernel.org/all/20250723092250.3411923-1-maciej.wieczor-retman@intel.com/
v1: https://lore.kernel.org/all/20250722074439.4069992-1-maciej.wieczor-retman@intel.com/

Maciej Wieczor-Retman (3):
  x86/cpu: Clear feature bits disabled at compile-time
  x86/cpu: Check if feature string is non-zero
  x86/cpu: Do a sanity check on required feature bits

 arch/x86/include/asm/cpu.h         |  2 +
 arch/x86/kernel/cpu/common.c       | 62 +++++++++++++++++++++++++++---
 arch/x86/kernel/cpu/cpuid-deps.c   | 20 ++--------
 arch/x86/tools/cpufeaturemasks.awk |  6 +++
 4 files changed, 67 insertions(+), 23 deletions(-)

-- 
2.53.0

[PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Maciej Wieczor-Retman]

From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

If some config options are disabled during compile time, they still are
enumerated in macros that use the x86_capability bitmask - cpu_has() or
this_cpu_has().

The features are also visible in /proc/cpuinfo even though they are not
enabled - which is contrary to what the documentation states about the
file. Examples of such feature flags are lam, fred, sgx, ibrs_enhanced,
split_lock_detect, user_shstk, avx_vnni and enqcmd.

Once the cpu_caps_cleared array is initialized with the autogenerated
disabled bitmask apply_forced_caps() will clear the corresponding bits
in boot_cpu_data.x86_capability[] and other secondary cpus'
cpu_data.x86_capability[]. Thus features disabled at compile time won't
show up in /proc/cpuinfo.

Reported-by: Farrah Chen <farrah.chen@intel.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220348
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
Cc: <stable@vger.kernel.org> # 6.18.x
---
Changelog v6:
- Remove patch message portions that are not just describing the diff.

 arch/x86/kernel/cpu/common.c       | 3 ++-
 arch/x86/tools/cpufeaturemasks.awk | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 1c3261cae40c..9aa11224a038 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -732,7 +732,8 @@ static const char *table_lookup_model(struct cpuinfo_x86 *c)
 }
 
 /* Aligned to unsigned long to avoid split lock in atomic bitmap ops */
-__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
+__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long)) =
+	DISABLED_MASK_INITIALIZER;
 __u32 cpu_caps_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/tools/cpufeaturemasks.awk b/arch/x86/tools/cpufeaturemasks.awk
index 173d5bf2d999..b7f4e775a365 100755
--- a/arch/x86/tools/cpufeaturemasks.awk
+++ b/arch/x86/tools/cpufeaturemasks.awk
@@ -82,6 +82,12 @@ END {
 		}
 		printf " 0\t\\\n";
 		printf "\t) & (1U << ((x) & 31)))\n\n";
+
+		printf "\n#define %s_MASK_INITIALIZER\t\t\t\\", s;
+		printf "\n\t{\t\t\t\t\t\t\\";
+		for (i = 0; i < ncapints; i++)
+			printf "\n\t\t%s_MASK%d,\t\t\t\\", s, i;
+		printf "\n\t}\n\n";
 	}
 
 	printf "#endif /* _ASM_X86_CPUFEATUREMASKS_H */\n";
-- 
2.53.0

[PATCH v8 2/3] x86/cpu: Check if feature string is non-zero

[Maciej Wieczor-Retman]

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 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       |  2 ++
 arch/x86/kernel/cpu/common.c     | 26 +++++++++++++++++++++-----
 arch/x86/kernel/cpu/cpuid-deps.c | 20 +++-----------------
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index ad235dda1ded..fdf9566e2272 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -67,4 +67,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 9aa11224a038..b60269174d95 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -675,6 +675,7 @@ cpuid_dependent_features[] = {
 static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
 {
 	const struct cpuid_dependent_feature *df;
+	char feature_buf[16];
 
 	for (df = cpuid_dependent_features; df->feature; df++) {
 
@@ -697,7 +698,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);
 	}
 }
 
@@ -1634,6 +1635,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
 
 	while (arg) {
 		bool found __maybe_unused = false;
+		char name_buf[16];
 		unsigned int bit;
 
 		opt = strsep(&arg, ",");
@@ -1654,10 +1656,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++;
 			}
@@ -1980,6 +1979,23 @@ static void generic_identify(struct cpuinfo_x86 *c)
 #endif
 }
 
+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, 16, "%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..dfd79b06ab7b 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -2,6 +2,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/module.h>
+#include <asm/cpu.h>
 #include <asm/cpufeature.h>
 
 struct cpuid_dep {
@@ -156,21 +157,6 @@ 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];
@@ -185,8 +171,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.0

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

[Maciej Wieczor-Retman]

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

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Borislav Petkov]

On Mon, Mar 02, 2026 at 03:25:10PM +0000, Maciej Wieczor-Retman wrote:
> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> 
> If some config options are disabled during compile time, they still are
> enumerated in macros that use the x86_capability bitmask - cpu_has() or
> this_cpu_has().
> 
> The features are also visible in /proc/cpuinfo even though they are not
> enabled - which is contrary to what the documentation states about the
> file. Examples of such feature flags are lam, fred, sgx, ibrs_enhanced,
> split_lock_detect, user_shstk, avx_vnni and enqcmd.
> 
> Once the cpu_caps_cleared array is initialized with the autogenerated
> disabled bitmask apply_forced_caps() will clear the corresponding bits
> in boot_cpu_data.x86_capability[] and other secondary cpus'

All your text: s/cpu/CPU/g

> cpu_data.x86_capability[]. Thus features disabled at compile time won't
> show up in /proc/cpuinfo.
> 
> Reported-by: Farrah Chen <farrah.chen@intel.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220348
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> Cc: <stable@vger.kernel.org> # 6.18.x

So why is this going to stable anyway?

What is the serious issue this is fixing? Really...?

> ---
> Changelog v6:
> - Remove patch message portions that are not just describing the diff.
> 
>  arch/x86/kernel/cpu/common.c       | 3 ++-
>  arch/x86/tools/cpufeaturemasks.awk | 6 ++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 1c3261cae40c..9aa11224a038 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -732,7 +732,8 @@ static const char *table_lookup_model(struct cpuinfo_x86 *c)
>  
>  /* Aligned to unsigned long to avoid split lock in atomic bitmap ops */
> -__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
> +__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long)) =
> +	DISABLED_MASK_INITIALIZER;

DISABLED_MASK_INIT is kinda obvious already.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Maciej Wieczor-Retman]

On 2026-03-02 at 20:31:42 +0100, Borislav Petkov wrote:
>On Mon, Mar 02, 2026 at 03:25:10PM +0000, Maciej Wieczor-Retman wrote:
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>>
>> If some config options are disabled during compile time, they still are
>> enumerated in macros that use the x86_capability bitmask - cpu_has() or
>> this_cpu_has().
>>
>> The features are also visible in /proc/cpuinfo even though they are not
>> enabled - which is contrary to what the documentation states about the
>> file. Examples of such feature flags are lam, fred, sgx, ibrs_enhanced,
>> split_lock_detect, user_shstk, avx_vnni and enqcmd.
>>
>> Once the cpu_caps_cleared array is initialized with the autogenerated
>> disabled bitmask apply_forced_caps() will clear the corresponding bits
>> in boot_cpu_data.x86_capability[] and other secondary cpus'
>
>All your text: s/cpu/CPU/g

Sure, I'll change it.

>
>> cpu_data.x86_capability[]. Thus features disabled at compile time won't
>> show up in /proc/cpuinfo.
>>
>> Reported-by: Farrah Chen <farrah.chen@intel.com>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220348
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> Cc: <stable@vger.kernel.org> # 6.18.x
>
>So why is this going to stable anyway?
>
>What is the serious issue this is fixing? Really...?

The documentation from at least 5.10 onwards promises to have flags in cpuinfo
only if they're truly compiled and enabled. So I thought that incosistency can
be corrected from that point on. For the 6.18 stable kernel this particular
patch applies cleanly because it already started using the awk script. For the
older ones I took Greg's advice and prepared separate patch that worked before
the awk script was introduced.

>> ---
>> Changelog v6:
>> - Remove patch message portions that are not just describing the diff.
>>
>>  arch/x86/kernel/cpu/common.c       | 3 ++-
>>  arch/x86/tools/cpufeaturemasks.awk | 6 ++++++
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index 1c3261cae40c..9aa11224a038 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -732,7 +732,8 @@ static const char *table_lookup_model(struct cpuinfo_x86 *c)
>>
>>  /* Aligned to unsigned long to avoid split lock in atomic bitmap ops */
>> -__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
>> +__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long)) =
>> +	DISABLED_MASK_INITIALIZER;
>
>DISABLED_MASK_INIT is kinda obvious already.

Okay, I'll shorten it.

>
>--
>Regards/Gruss,
>    Boris.
>
>https://people.kernel.org/tglx/notes-about-netiquette

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Borislav Petkov]

On Mon, Mar 02, 2026 at 07:48:41PM +0000, Maciej Wieczor-Retman wrote:
> The documentation from at least 5.10 onwards promises to have flags in cpuinfo
> only if they're truly compiled and enabled. So I thought that incosistency can
> be corrected from that point on. For the 6.18 stable kernel this particular
> patch applies cleanly because it already started using the awk script. For the
> older ones I took Greg's advice and prepared separate patch that worked before
> the awk script was introduced.

I don't think you got my question, lemme try again: 

How serious is this bug so that you want to backport it to stable?

So what if some flags appear in /proc/cpuinfo even if they're not compiled in?

Is the cat going to catch fire or no one cares...?

IOW, does it really need to go to stable and if so, what's the grave bug it is
fixing?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Maciej Wieczor-Retman]

On 2026-03-02 at 21:25:04 +0100, Borislav Petkov wrote:
>On Mon, Mar 02, 2026 at 07:48:41PM +0000, Maciej Wieczor-Retman wrote:
>> The documentation from at least 5.10 onwards promises to have flags in cpuinfo
>> only if they're truly compiled and enabled. So I thought that incosistency can
>> be corrected from that point on. For the 6.18 stable kernel this particular
>> patch applies cleanly because it already started using the awk script. For the
>> older ones I took Greg's advice and prepared separate patch that worked before
>> the awk script was introduced.
>
>I don't think you got my question, lemme try again: 
>
>How serious is this bug so that you want to backport it to stable?
>
>So what if some flags appear in /proc/cpuinfo even if they're not compiled in?
>
>Is the cat going to catch fire or no one cares...?
>
>IOW, does it really need to go to stable and if so, what's the grave bug it is
>fixing?

I don't think it's some big threat. But then would you agree it'd be a good idea
to backport a change to the documentation that the cpuinfo isn't as reliable as
the documentation entry says it is? I would imagine the documentation should be
kept as accurate as possible.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Borislav Petkov]

On Mon, Mar 02, 2026 at 08:38:35PM +0000, Maciej Wieczor-Retman wrote:
> I don't think it's some big threat. But then would you agree it'd be a good idea
> to backport a change to the documentation that the cpuinfo isn't as reliable as
> the documentation entry says it is? I would imagine the documentation should be
> kept as accurate as possible.

Point me to which rule your argumentation applies, pls:

Documentation/process/stable-kernel-rules.rst

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Maciej Wieczor-Retman]

On 2026-03-02 at 21:59:47 +0100, Borislav Petkov wrote:
>On Mon, Mar 02, 2026 at 08:38:35PM +0000, Maciej Wieczor-Retman wrote:
>> I don't think it's some big threat. But then would you agree it'd be a good idea
>> to backport a change to the documentation that the cpuinfo isn't as reliable as
>> the documentation entry says it is? I would imagine the documentation should be
>> kept as accurate as possible.
>
>Point me to which rule your argumentation applies, pls:
>
>Documentation/process/stable-kernel-rules.rst
>
>--
>Regards/Gruss,
>    Boris.
>
>https://people.kernel.org/tglx/notes-about-netiquette

Maybe it could fall under the 'some "oh, that's not good" issue'? :)

But I see your point, I'll drop sending it to stable.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Borislav Petkov]

On March 2, 2026 9:22:47 PM UTC, Maciej Wieczor-Retman <m.wieczorretman@pm.me> wrote:
>Maybe it could fall under the 'some "oh, that's not good" issue'? :)

More like "no one noticed/complained until now so why are we making waves and generating unnecessary work now" thing...

-- 
Small device. Typos and formatting crap

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Sohil Mehta]

On 3/2/2026 7:25 AM, Maciej Wieczor-Retman wrote:

>  /* Aligned to unsigned long to avoid split lock in atomic bitmap ops */
> -__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
> +__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long)) =
> +	DISABLED_MASK_INITIALIZER;

IIUC, DISABLED_MASK_INITIALIZER only contains the X86_FEATURE_* bits.
So, the NBUGINTS bits in cpu_caps_cleared[] are implicitly set to 0.

Should that be mentioned in the comment above? It wasn't obvious to me
when I first looked at it.

>  __u32 cpu_caps_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/tools/cpufeaturemasks.awk b/arch/x86/tools/cpufeaturemasks.awk
> index 173d5bf2d999..b7f4e775a365 100755
> --- a/arch/x86/tools/cpufeaturemasks.awk
> +++ b/arch/x86/tools/cpufeaturemasks.awk
> @@ -82,6 +82,12 @@ END {
>  		}
>  		printf " 0\t\\\n";
>  		printf "\t) & (1U << ((x) & 31)))\n\n";
> +
> +		printf "\n#define %s_MASK_INITIALIZER\t\t\t\\", s;
> +		printf "\n\t{\t\t\t\t\t\t\\";
> +		for (i = 0; i < ncapints; i++)
> +			printf "\n\t\t%s_MASK%d,\t\t\t\\", s, i;
> +		printf "\n\t}\n\n";
>  	}
>  
>  	printf "#endif /* _ASM_X86_CPUFEATUREMASKS_H */\n";

Re: [PATCH v8 2/3] x86/cpu: Check if feature string is non-zero

[Sohil Mehta]

On 3/2/2026 7:25 AM, Maciej Wieczor-Retman wrote:

> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 9aa11224a038..b60269174d95 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -675,6 +675,7 @@ cpuid_dependent_features[] = {
>  static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
>  {
>  	const struct cpuid_dependent_feature *df;
> +	char feature_buf[16];
>  

The usage of number 16 isn't intuitive here. Might be useful to use a
macro here. See below..

>  	for (df = cpuid_dependent_features; df->feature; df++) {
>  
> @@ -697,7 +698,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);
>  	}
>  }
>  
> @@ -1634,6 +1635,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
>  
>  	while (arg) {
>  		bool found __maybe_unused = false;
> +		char name_buf[16];
>  		unsigned int bit;
>  
>  		opt = strsep(&arg, ",");
> @@ -1654,10 +1656,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++;
>  			}
> @@ -1980,6 +1979,23 @@ static void generic_identify(struct cpuinfo_x86 *c)
>  #endif
>  }
>  

It would be useful to have a comment here because the function is called
from multiple files. You can probably reuse the one from the deleted
x86_feature_name().

/*
 * 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, 16, "%u:%u", word, bit & 31);

The buffer size of 16 is expected to be in sync with the callers and
easy to mess up. Also, the callers wouldn't know that it needs to be 16
because of this snprintf(). Should we have a simple define for it?

Maybe something like,

#define X86_CAP_BUF_SIZE 16

> +	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..dfd79b06ab7b 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -2,6 +2,7 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/module.h>

Nit:

You can consider adding a newline here.

> +#include <asm/cpu.h>
>  #include <asm/cpufeature.h>
>  

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

[Sohil Mehta]

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

Re: [PATCH v8 1/3] x86/cpu: Clear feature bits disabled at compile-time

[Maciej Wieczor-Retman]

On 2026-03-09 at 16:47:49 -0700, Sohil Mehta wrote:
>On 3/2/2026 7:25 AM, Maciej Wieczor-Retman wrote:
>
>>  /* Aligned to unsigned long to avoid split lock in atomic bitmap ops */
>> -__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
>> +__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long)) =
>> +	DISABLED_MASK_INITIALIZER;
>
>IIUC, DISABLED_MASK_INITIALIZER only contains the X86_FEATURE_* bits.
>So, the NBUGINTS bits in cpu_caps_cleared[] are implicitly set to 0.
>
>Should that be mentioned in the comment above? It wasn't obvious to me
>when I first looked at it.

As I understand the features can be compile time disabled while bugs can't? So
it wouldn't be practical to initialize the BUGS part of cpu_caps_cleared. But I
suppose it doesn't hurt to clarify it in the patch message. 

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v8 2/3] x86/cpu: Check if feature string is non-zero

[Maciej Wieczor-Retman]

On 2026-03-09 at 23:23:26 -0700, Sohil Mehta wrote:
>On 3/2/2026 7:25 AM, Maciej Wieczor-Retman wrote:
>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index 9aa11224a038..b60269174d95 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -675,6 +675,7 @@ cpuid_dependent_features[] = {
>>  static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
>>  {
>>  	const struct cpuid_dependent_feature *df;
>> +	char feature_buf[16];
>>  
>
>The usage of number 16 isn't intuitive here. Might be useful to use a
>macro here. See below..
>
...
>
>It would be useful to have a comment here because the function is called
>from multiple files. You can probably reuse the one from the deleted
>x86_feature_name().

Sure, that's a good idea.

>
>/*
> * 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, 16, "%u:%u", word, bit & 31);
>
>The buffer size of 16 is expected to be in sync with the callers and
>easy to mess up. Also, the callers wouldn't know that it needs to be 16
>because of this snprintf(). Should we have a simple define for it?
>
>Maybe something like,
>
>#define X86_CAP_BUF_SIZE 16

Yes, thanks, I probably should've done that frome the start :)

>
>> +	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..dfd79b06ab7b 100644
>> --- a/arch/x86/kernel/cpu/cpuid-deps.c
>> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
>> @@ -2,6 +2,7 @@
>>  #include <linux/kernel.h>
>>  #include <linux/init.h>
>>  #include <linux/module.h>
>
>Nit:
>
>You can consider adding a newline here.

Okay.

>> +#include <asm/cpu.h>
>>  #include <asm/cpufeature.h>
>>  

-- 
Kind regards
Maciej Wieczór-Retman

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

[Maciej Wieczor-Retman]

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