[PATCH v12 4/4] x86/cpu: Clear feature bits whose dependencies were cleared

Maciej Wieczor-Retman posted 4 patches 6 days ago
[PATCH v12 4/4] x86/cpu: Clear feature bits whose dependencies were cleared
Posted by Maciej Wieczor-Retman 6 days ago
From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

After cpu_caps_cleared[] is initialized with DISABLED_MASK_INIT,
features present in disabled bitmasks are cleared from x86_capability[].
However features that depend on them and are not part of any disabled
mask are not cleared by anything. They can trigger the warning in
check_cpufeature_deps(), as before both features would show up as
enabled even though they weren't. The uncleared features can also still
falsely show up in /proc/cpuinfo.

Take such cases into account before applying the cpu_caps_cleared[]
array.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v12:
- Move the check from check_cpufeature_deps() to apply_forced_caps().
  Redo last paragraph of the patch message.

Changelog v11:
- Add this patch to the series.

 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/kernel/cpu/common.c      |  2 ++
 arch/x86/kernel/cpu/cpuid-deps.c  | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 3ddc1d33399b..e7a62990a9c2 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -77,6 +77,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 
 extern void setup_clear_cpu_cap(unsigned int bit);
 extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
+void clear_feature_disabled_deps(struct cpuinfo_x86 *c);
 void check_cpufeature_deps(struct cpuinfo_x86 *c);
 
 #define setup_force_cpu_cap(bit) do {			\
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8ad0da7012dd..2e24bdb3acaa 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -955,6 +955,8 @@ static void apply_forced_caps(struct cpuinfo_x86 *c)
 {
 	int i;
 
+	clear_feature_disabled_deps(c);
+
 	for (i = 0; i < NCAPINTS + NBUGINTS; i++) {
 		c->x86_capability[i] &= ~cpu_caps_cleared[i];
 		c->x86_capability[i] |= cpu_caps_set[i];
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 4354c0dd6d66..83060c0013c1 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -158,6 +158,20 @@ void setup_clear_cpu_cap(unsigned int feature)
 	do_clear_cpu_cap(NULL, feature);
 }
 
+/*
+ * If the dependency was cleared through the disabled bitmasks while the
+ * feature wasn't it also needs to be cleared.
+ */
+void clear_feature_disabled_deps(struct cpuinfo_x86 *c)
+{
+	const struct cpuid_dep *d;
+
+	for (d = cpuid_deps; d->feature; d++) {
+		if (!DISABLED_MASK_BIT_SET(d->feature) && DISABLED_MASK_BIT_SET(d->depends))
+			set_bit(d->feature, (unsigned long *)cpu_caps_cleared);
+	}
+}
+
 void check_cpufeature_deps(struct cpuinfo_x86 *c)
 {
 	char feature_buf[X86_NAMELESS_FEAT_BUFLEN], depends_buf[X86_NAMELESS_FEAT_BUFLEN];
-- 
2.53.0
Re: [PATCH v12 4/4] x86/cpu: Clear feature bits whose dependencies were cleared
Posted by H. Peter Anvin 5 days, 13 hours ago
On 2026-03-27 08:11, Maciej Wieczor-Retman wrote:
> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> 
> After cpu_caps_cleared[] is initialized with DISABLED_MASK_INIT,
> features present in disabled bitmasks are cleared from x86_capability[].
> However features that depend on them and are not part of any disabled
> mask are not cleared by anything. They can trigger the warning in
> check_cpufeature_deps(), as before both features would show up as
> enabled even though they weren't. The uncleared features can also still
> falsely show up in /proc/cpuinfo.
> 
> Take such cases into account before applying the cpu_caps_cleared[]
> array.
> 
> +/*
> + * If the dependency was cleared through the disabled bitmasks while the
> + * feature wasn't it also needs to be cleared.
> + */
> +void clear_feature_disabled_deps(struct cpuinfo_x86 *c)
> +{
> +	const struct cpuid_dep *d;
> +
> +	for (d = cpuid_deps; d->feature; d++) {
> +		if (!DISABLED_MASK_BIT_SET(d->feature) && DISABLED_MASK_BIT_SET(d->depends))
> +			set_bit(d->feature, (unsigned long *)cpu_caps_cleared);
> +	}
> +}
> +

Thinking about it, if this is discovered at runtime, it is really too late,
because in a sense we have miscompiled the kernel with unwanted code (as we
should have compiled out these features just as with other DISABLED features.)

This ultimately reflects a failed dependency in Kconfig.cpufeatures, which may
have caused kconfig to do the Wrong Thing[TM]. So at the very least it might
be a good thing to print a message here saying Kconfig.cpufeatures should be
fixed.

The better option, which I don't know how difficult it would be, would be to
make the dependencies available to Kconfig. This sounds like something that
would fall in the scope of Ahmed's rework rather than this patchset, though
(Ahmed, would you agree?)

	-hpa
Re: [PATCH v12 4/4] x86/cpu: Clear feature bits whose dependencies were cleared
Posted by Ahmed S. Darwish 2 days, 17 hours ago
On Fri, 27 Mar 2026, H. Peter Anvin wrote:
>
> Thinking about it, if this is discovered at runtime, it is really too late,
> because in a sense we have miscompiled the kernel with unwanted code (as we
> should have compiled out these features just as with other DISABLED features.)
>
> This ultimately reflects a failed dependency in Kconfig.cpufeatures, which may
> have caused kconfig to do the Wrong Thing[TM]. So at the very least it might
> be a good thing to print a message here saying Kconfig.cpufeatures should be
> fixed.
>
> The better option, which I don't know how difficult it would be, would be to
> make the dependencies available to Kconfig. This sounds like something that
> would fall in the scope of Ahmed's rework rather than this patchset, though
> (Ahmed, would you agree?)
>

Definitely!

If the CPUID model sent some days ago is to be merged, (*) then all these
dependencies should be encoded within x86-cpuid-db, especially that it now
covers all the synthetic X86_FEATURE flags as well.

x86-cpuid-db can then flatten the Directed Acyclic Graph of dependencies,
and auto-generate a flat map which the kernel can use at configuration,
compile, and run time.

And if a dependency cycle (or other dependency annotation failures) exist,
then this can be detected early on; e.g., at the project's CI level, which
auto generates kcpuid CSV and the C99 fields at <asm/cpuid/leaf_types.h>.

Thanks,
Ahmed

(*) https://lore.kernel.org/lkml/20260327021645.555257-1-darwi@linutronix.de
Re: [PATCH v12 4/4] x86/cpu: Clear feature bits whose dependencies were cleared
Posted by Maciej Wieczor-Retman 1 day ago
On 2026-03-30 at 23:41:57 +0200, Ahmed S. Darwish wrote:
>On Fri, 27 Mar 2026, H. Peter Anvin wrote:
>>
>> Thinking about it, if this is discovered at runtime, it is really too late,
>> because in a sense we have miscompiled the kernel with unwanted code (as we
>> should have compiled out these features just as with other DISABLED features.)
>>
>> This ultimately reflects a failed dependency in Kconfig.cpufeatures, which may
>> have caused kconfig to do the Wrong Thing[TM]. So at the very least it might
>> be a good thing to print a message here saying Kconfig.cpufeatures should be
>> fixed.
>>
>> The better option, which I don't know how difficult it would be, would be to
>> make the dependencies available to Kconfig. This sounds like something that
>> would fall in the scope of Ahmed's rework rather than this patchset, though
>> (Ahmed, would you agree?)
>>
>
>Definitely!
>
>If the CPUID model sent some days ago is to be merged, (*) then all these
>dependencies should be encoded within x86-cpuid-db, especially that it now
>covers all the synthetic X86_FEATURE flags as well.

Hi! I'm still testing/browsing through the cpuid patchset. But I was wondering
what did you mean about encoding these dependencies? That they are encoded in
your patchset or that they should be?

-- 
Kind regards
Maciej Wieczór-Retman
Re: [PATCH v12 4/4] x86/cpu: Clear feature bits whose dependencies were cleared
Posted by Maciej Wieczor-Retman 2 days, 3 hours ago
On 2026-03-30 at 23:41:57 +0200, Ahmed S. Darwish wrote:
>On Fri, 27 Mar 2026, H. Peter Anvin wrote:
>>
>> Thinking about it, if this is discovered at runtime, it is really too late,
>> because in a sense we have miscompiled the kernel with unwanted code (as we
>> should have compiled out these features just as with other DISABLED features.)
>>
>> This ultimately reflects a failed dependency in Kconfig.cpufeatures, which may
>> have caused kconfig to do the Wrong Thing[TM]. So at the very least it might
>> be a good thing to print a message here saying Kconfig.cpufeatures should be
>> fixed.
>>
>> The better option, which I don't know how difficult it would be, would be to
>> make the dependencies available to Kconfig. This sounds like something that
>> would fall in the scope of Ahmed's rework rather than this patchset, though
>> (Ahmed, would you agree?)
>>
>
>Definitely!
>
>If the CPUID model sent some days ago is to be merged, (*) then all these
>dependencies should be encoded within x86-cpuid-db, especially that it now
>covers all the synthetic X86_FEATURE flags as well.
>
>x86-cpuid-db can then flatten the Directed Acyclic Graph of dependencies,
>and auto-generate a flat map which the kernel can use at configuration,
>compile, and run time.
>
>And if a dependency cycle (or other dependency annotation failures) exist,
>then this can be detected early on; e.g., at the project's CI level, which
>auto generates kcpuid CSV and the C99 fields at <asm/cpuid/leaf_types.h>.
>
>Thanks,
>Ahmed
>
>(*) https://lore.kernel.org/lkml/20260327021645.555257-1-darwi@linutronix.de

Cool, so I assume for now a warning should suffice and in the longer view the
x86-cpuid-db should help resolve the problem more broadly?

-- 
Kind regards
Maciej Wieczór-Retman
Re: [PATCH v12 4/4] x86/cpu: Clear feature bits whose dependencies were cleared
Posted by Maciej Wieczor-Retman 3 days, 5 hours ago
On 2026-03-27 at 19:22:27 -0700, H. Peter Anvin wrote:
>On 2026-03-27 08:11, Maciej Wieczor-Retman wrote:
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> 
>> After cpu_caps_cleared[] is initialized with DISABLED_MASK_INIT,
>> features present in disabled bitmasks are cleared from x86_capability[].
>> However features that depend on them and are not part of any disabled
>> mask are not cleared by anything. They can trigger the warning in
>> check_cpufeature_deps(), as before both features would show up as
>> enabled even though they weren't. The uncleared features can also still
>> falsely show up in /proc/cpuinfo.
>> 
>> Take such cases into account before applying the cpu_caps_cleared[]
>> array.
>> 
>> +/*
>> + * If the dependency was cleared through the disabled bitmasks while the
>> + * feature wasn't it also needs to be cleared.
>> + */
>> +void clear_feature_disabled_deps(struct cpuinfo_x86 *c)
>> +{
>> +	const struct cpuid_dep *d;
>> +
>> +	for (d = cpuid_deps; d->feature; d++) {
>> +		if (!DISABLED_MASK_BIT_SET(d->feature) && DISABLED_MASK_BIT_SET(d->depends))
>> +			set_bit(d->feature, (unsigned long *)cpu_caps_cleared);
>> +	}
>> +}
>> +
>
>Thinking about it, if this is discovered at runtime, it is really too late,
>because in a sense we have miscompiled the kernel with unwanted code (as we
>should have compiled out these features just as with other DISABLED features.)
>
>This ultimately reflects a failed dependency in Kconfig.cpufeatures, which may
>have caused kconfig to do the Wrong Thing[TM]. So at the very least it might
>be a good thing to print a message here saying Kconfig.cpufeatures should be
>fixed.

So how about a warning (for any future cases) and add a new patch to fix SGX (so
the current known problem is solved)?

>The better option, which I don't know how difficult it would be, would be to
>make the dependencies available to Kconfig. This sounds like something that
>would fall in the scope of Ahmed's rework rather than this patchset, though
>(Ahmed, would you agree?)
>
>	-hpa
>
>

-- 
Kind regards
Maciej Wieczór-Retman