All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] x86: Clear feature bits disabled at compile-time
@ 2025-07-23  9:22 Maciej Wieczor-Retman
  2025-07-23  9:45 ` Greg KH
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Maciej Wieczor-Retman @ 2025-07-23  9:22 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck, Fenghua Yu,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu
  Cc: maciej.wieczor-retman, stable, Borislav Petkov, linux-kernel

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.

Add a DISABLED_MASK() macro that returns 32 bit chunks of the disabled
feature bits bitmask.

Initialize the cpu_caps_cleared and cpu_caps_set arrays with the
contents of the disabled and required bitmasks respectively. Then let
apply_forced_caps() clear/set these feature bits in the x86_capability.

Fixes: 6449dcb0cac7 ("x86: CPUID and CR3/CR4 flags for Linear Address Masking")
Fixes: 51c158f7aacc ("x86/cpufeatures: Add the CPU feature bit for FRED")
Fixes: 706d51681d63 ("x86/speculation: Support Enhanced IBRS on future CPUs")
Fixes: e7b6385b01d8 ("x86/cpufeatures: Add Intel SGX hardware bits")
Fixes: 6650cdd9a8cc ("x86/split_lock: Enable split lock detection by kernel")
Fixes: 701fb66d576e ("x86/cpufeatures: Add CPU feature flags for shadow stacks")
Fixes: ff4f82816dff ("x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions")
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
Cc: <stable@vger.kernel.org>
---
Changelog v2:
- Redo the patch to utilize a more generic solution, not just fix the
  LAM and FRED feature bits.
- Note more feature flags that shouldn't be present.
- Add fixes and cc tags.

 arch/x86/kernel/cpu/common.c       | 12 ++++++++++++
 arch/x86/tools/cpufeaturemasks.awk |  8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 77afca95cced..ba8b5fba8552 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1709,6 +1709,16 @@ static void __init cpu_parse_early_param(void)
 	}
 }
 
+static __init void init_cpu_cap(struct cpuinfo_x86 *c)
+{
+	int i;
+
+	for (i = 0; i < NCAPINTS; i++) {
+		cpu_caps_set[i] = REQUIRED_MASK(i);
+		cpu_caps_cleared[i] = DISABLED_MASK(i);
+	}
+}
+
 /*
  * Do minimum CPU detection early.
  * Fields really needed: vendor, cpuid_level, family, model, mask,
@@ -1782,6 +1792,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 	if (!pgtable_l5_enabled())
 		setup_clear_cpu_cap(X86_FEATURE_LA57);
 
+	init_cpu_cap(c);
+
 	detect_nopl();
 }
 
diff --git a/arch/x86/tools/cpufeaturemasks.awk b/arch/x86/tools/cpufeaturemasks.awk
index 173d5bf2d999..2e2412f7681f 100755
--- a/arch/x86/tools/cpufeaturemasks.awk
+++ b/arch/x86/tools/cpufeaturemasks.awk
@@ -82,6 +82,14 @@ END {
 		}
 		printf " 0\t\\\n";
 		printf "\t) & (1U << ((x) & 31)))\n\n";
+
+		printf "\n#define %s_MASK(x)\t\t\t\t\\\n", s;
+		printf "\t((\t\t\t\t";
+		for (i = 0; i < ncapints; i++) {
+			if (masks[i])
+				printf "\t\t\\\n\t\t(x) == %2d ? %s_MASK%d :", i, s, i;
+		}
+		printf " 0))\t\\\n\n";
 	}
 
 	printf "#endif /* _ASM_X86_CPUFEATUREMASKS_H */\n";
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23  9:22 [PATCH v2] x86: Clear feature bits disabled at compile-time Maciej Wieczor-Retman
@ 2025-07-23  9:45 ` Greg KH
  2025-07-23 11:46   ` Maciej Wieczor-Retman
  2025-07-23 13:46 ` Borislav Petkov
  2025-07-23 14:08 ` H. Peter Anvin
  2 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2025-07-23  9:45 UTC (permalink / raw)
  To: Maciej Wieczor-Retman
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck, Fenghua Yu,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu,
	stable, Borislav Petkov, linux-kernel

On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
> 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.
> 
> Add a DISABLED_MASK() macro that returns 32 bit chunks of the disabled
> feature bits bitmask.
> 
> Initialize the cpu_caps_cleared and cpu_caps_set arrays with the
> contents of the disabled and required bitmasks respectively. Then let
> apply_forced_caps() clear/set these feature bits in the x86_capability.
> 
> Fixes: 6449dcb0cac7 ("x86: CPUID and CR3/CR4 flags for Linear Address Masking")
> Fixes: 51c158f7aacc ("x86/cpufeatures: Add the CPU feature bit for FRED")
> Fixes: 706d51681d63 ("x86/speculation: Support Enhanced IBRS on future CPUs")
> Fixes: e7b6385b01d8 ("x86/cpufeatures: Add Intel SGX hardware bits")
> Fixes: 6650cdd9a8cc ("x86/split_lock: Enable split lock detection by kernel")
> Fixes: 701fb66d576e ("x86/cpufeatures: Add CPU feature flags for shadow stacks")
> Fixes: ff4f82816dff ("x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions")

That is fricken insane.

You are saying to people who backport stuff:
	This fixes a commit found in the following kernel releases:
		6.4
		6.9
		3.16.68 4.4.180 4.9.137 4.14.81 4.18.19 4.19
		5.11
		5.7
		6.6
		5.10

You didn't even sort this in any sane order, how was it generated?

What in the world is anyone supposed to do with this?

If you were sent a patch with this in it, what would you think?  What
could you do with it?

Please be reasonable and consider us overworked stable maintainers and
give us a chance to get things right.  As it is, this just makes things
worse...

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23  9:45 ` Greg KH
@ 2025-07-23 11:46   ` Maciej Wieczor-Retman
  2025-07-23 11:57     ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Maciej Wieczor-Retman @ 2025-07-23 11:46 UTC (permalink / raw)
  To: Greg KH
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck, Fenghua Yu,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu,
	stable, Borislav Petkov, linux-kernel

On 2025-07-23 at 11:45:22 +0200, Greg KH wrote:
>On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
>> 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.
>> 
>> Add a DISABLED_MASK() macro that returns 32 bit chunks of the disabled
>> feature bits bitmask.
>> 
>> Initialize the cpu_caps_cleared and cpu_caps_set arrays with the
>> contents of the disabled and required bitmasks respectively. Then let
>> apply_forced_caps() clear/set these feature bits in the x86_capability.
>> 
>> Fixes: 6449dcb0cac7 ("x86: CPUID and CR3/CR4 flags for Linear Address Masking")
>> Fixes: 51c158f7aacc ("x86/cpufeatures: Add the CPU feature bit for FRED")
>> Fixes: 706d51681d63 ("x86/speculation: Support Enhanced IBRS on future CPUs")
>> Fixes: e7b6385b01d8 ("x86/cpufeatures: Add Intel SGX hardware bits")
>> Fixes: 6650cdd9a8cc ("x86/split_lock: Enable split lock detection by kernel")
>> Fixes: 701fb66d576e ("x86/cpufeatures: Add CPU feature flags for shadow stacks")
>> Fixes: ff4f82816dff ("x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions")
>
>That is fricken insane.
>
>You are saying to people who backport stuff:
>	This fixes a commit found in the following kernel releases:
>		6.4
>		6.9
>		3.16.68 4.4.180 4.9.137 4.14.81 4.18.19 4.19
>		5.11
>		5.7
>		6.6
>		5.10
>
>You didn't even sort this in any sane order, how was it generated?
>
>What in the world is anyone supposed to do with this?
>
>If you were sent a patch with this in it, what would you think?  What
>could you do with it?
>
>Please be reasonable and consider us overworked stable maintainers and
>give us a chance to get things right.  As it is, this just makes things
>worse...
>
>greg k-h

Sorry, I certainly didn't want to add you more work.

I noted down which features are present in the x86_capability bitmask while
they're not compiled into the kernel. Then I noted down which commits added
these feature flags. So I suppose the order is from least to most significant
feature bit, which now I realize doesn't help much in backporting, again sorry.

Would a more fitting Fixes: commit be the one that changed how the feature flags
are used? At some point docs started stating to have them set only when features
are COMPILED & HARDWARE-SUPPORTED.

-- 
Kind regards
Maciej Wieczór-Retman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 11:46   ` Maciej Wieczor-Retman
@ 2025-07-23 11:57     ` Greg KH
  2025-07-23 13:03       ` Maciej Wieczor-Retman
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2025-07-23 11:57 UTC (permalink / raw)
  To: Maciej Wieczor-Retman
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck, Fenghua Yu,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu,
	stable, Borislav Petkov, linux-kernel

On Wed, Jul 23, 2025 at 01:46:44PM +0200, Maciej Wieczor-Retman wrote:
> On 2025-07-23 at 11:45:22 +0200, Greg KH wrote:
> >On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
> >> 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.
> >> 
> >> Add a DISABLED_MASK() macro that returns 32 bit chunks of the disabled
> >> feature bits bitmask.
> >> 
> >> Initialize the cpu_caps_cleared and cpu_caps_set arrays with the
> >> contents of the disabled and required bitmasks respectively. Then let
> >> apply_forced_caps() clear/set these feature bits in the x86_capability.
> >> 
> >> Fixes: 6449dcb0cac7 ("x86: CPUID and CR3/CR4 flags for Linear Address Masking")
> >> Fixes: 51c158f7aacc ("x86/cpufeatures: Add the CPU feature bit for FRED")
> >> Fixes: 706d51681d63 ("x86/speculation: Support Enhanced IBRS on future CPUs")
> >> Fixes: e7b6385b01d8 ("x86/cpufeatures: Add Intel SGX hardware bits")
> >> Fixes: 6650cdd9a8cc ("x86/split_lock: Enable split lock detection by kernel")
> >> Fixes: 701fb66d576e ("x86/cpufeatures: Add CPU feature flags for shadow stacks")
> >> Fixes: ff4f82816dff ("x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions")
> >
> >That is fricken insane.
> >
> >You are saying to people who backport stuff:
> >	This fixes a commit found in the following kernel releases:
> >		6.4
> >		6.9
> >		3.16.68 4.4.180 4.9.137 4.14.81 4.18.19 4.19
> >		5.11
> >		5.7
> >		6.6
> >		5.10
> >
> >You didn't even sort this in any sane order, how was it generated?
> >
> >What in the world is anyone supposed to do with this?
> >
> >If you were sent a patch with this in it, what would you think?  What
> >could you do with it?
> >
> >Please be reasonable and consider us overworked stable maintainers and
> >give us a chance to get things right.  As it is, this just makes things
> >worse...
> >
> >greg k-h
> 
> Sorry, I certainly didn't want to add you more work.
> 
> I noted down which features are present in the x86_capability bitmask while
> they're not compiled into the kernel. Then I noted down which commits added
> these feature flags. So I suppose the order is from least to most significant
> feature bit, which now I realize doesn't help much in backporting, again sorry.
> 
> Would a more fitting Fixes: commit be the one that changed how the feature flags
> are used? At some point docs started stating to have them set only when features
> are COMPILED & HARDWARE-SUPPORTED.

What would you want to see if you had to do something with a "Fixes:"
line?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 11:57     ` Greg KH
@ 2025-07-23 13:03       ` Maciej Wieczor-Retman
  2025-07-23 13:37         ` Greg KH
  2025-07-23 15:52         ` Xin Li
  0 siblings, 2 replies; 15+ messages in thread
From: Maciej Wieczor-Retman @ 2025-07-23 13:03 UTC (permalink / raw)
  To: Greg KH
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck, Fenghua Yu,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu,
	stable, Borislav Petkov, linux-kernel

On 2025-07-23 at 13:57:34 +0200, Greg KH wrote:
>On Wed, Jul 23, 2025 at 01:46:44PM +0200, Maciej Wieczor-Retman wrote:
>> On 2025-07-23 at 11:45:22 +0200, Greg KH wrote:
>> >On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
>> >> 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.
>> >> 
>> >> Add a DISABLED_MASK() macro that returns 32 bit chunks of the disabled
>> >> feature bits bitmask.
>> >> 
>> >> Initialize the cpu_caps_cleared and cpu_caps_set arrays with the
>> >> contents of the disabled and required bitmasks respectively. Then let
>> >> apply_forced_caps() clear/set these feature bits in the x86_capability.
>> >> 
>> >> Fixes: 6449dcb0cac7 ("x86: CPUID and CR3/CR4 flags for Linear Address Masking")
>> >> Fixes: 51c158f7aacc ("x86/cpufeatures: Add the CPU feature bit for FRED")
>> >> Fixes: 706d51681d63 ("x86/speculation: Support Enhanced IBRS on future CPUs")
>> >> Fixes: e7b6385b01d8 ("x86/cpufeatures: Add Intel SGX hardware bits")
>> >> Fixes: 6650cdd9a8cc ("x86/split_lock: Enable split lock detection by kernel")
>> >> Fixes: 701fb66d576e ("x86/cpufeatures: Add CPU feature flags for shadow stacks")
>> >> Fixes: ff4f82816dff ("x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions")
>> >
>> >That is fricken insane.
>> >
>> >You are saying to people who backport stuff:
>> >	This fixes a commit found in the following kernel releases:
>> >		6.4
>> >		6.9
>> >		3.16.68 4.4.180 4.9.137 4.14.81 4.18.19 4.19
>> >		5.11
>> >		5.7
>> >		6.6
>> >		5.10
>> >
>> >You didn't even sort this in any sane order, how was it generated?
>> >
>> >What in the world is anyone supposed to do with this?
>> >
>> >If you were sent a patch with this in it, what would you think?  What
>> >could you do with it?
>> >
>> >Please be reasonable and consider us overworked stable maintainers and
>> >give us a chance to get things right.  As it is, this just makes things
>> >worse...
>> >
>> >greg k-h
>> 
>> Sorry, I certainly didn't want to add you more work.
>> 
>> I noted down which features are present in the x86_capability bitmask while
>> they're not compiled into the kernel. Then I noted down which commits added
>> these feature flags. So I suppose the order is from least to most significant
>> feature bit, which now I realize doesn't help much in backporting, again sorry.
>> 
>> Would a more fitting Fixes: commit be the one that changed how the feature flags
>> are used? At some point docs started stating to have them set only when features
>> are COMPILED & HARDWARE-SUPPORTED.
>
>What would you want to see if you had to do something with a "Fixes:"
>line?

I suppose I'd want to see a Fixes:commit in a place that hasn't seen too many
changes. So the backport process doesn't hit too many infrastructure changes
since that makes things more tricky.

And I guess it would be great if the Fixes:commit pointed at some obvious error
that happened - like a place that could dereference a NULL pointer for example.

But I thought Fixes: was supposed to mark the origin point of some error the
patch is fixing?

In this case a documentation patch [1] changed how feature flags are supposed to
behave. But these flags were added in various points in the past. So what should
Fixes: point at then?

But anyway writing this now I get the feeling that [1] would be a better point
to mark for the "Fixes:" line.

[1] ea4e3bef4c94 ("Documentation/x86: Add documentation for /proc/cpuinfo feature flags")

>
>thanks,
>
>greg k-h

-- 
Kind regards
Maciej Wieczór-Retman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 13:03       ` Maciej Wieczor-Retman
@ 2025-07-23 13:37         ` Greg KH
  2025-07-23 15:52         ` Xin Li
  1 sibling, 0 replies; 15+ messages in thread
From: Greg KH @ 2025-07-23 13:37 UTC (permalink / raw)
  To: Maciej Wieczor-Retman
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck, Fenghua Yu,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu,
	stable, Borislav Petkov, linux-kernel

On Wed, Jul 23, 2025 at 03:03:27PM +0200, Maciej Wieczor-Retman wrote:
> On 2025-07-23 at 13:57:34 +0200, Greg KH wrote:
> >On Wed, Jul 23, 2025 at 01:46:44PM +0200, Maciej Wieczor-Retman wrote:
> >> On 2025-07-23 at 11:45:22 +0200, Greg KH wrote:
> >> >On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
> >> >> 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.
> >> >> 
> >> >> Add a DISABLED_MASK() macro that returns 32 bit chunks of the disabled
> >> >> feature bits bitmask.
> >> >> 
> >> >> Initialize the cpu_caps_cleared and cpu_caps_set arrays with the
> >> >> contents of the disabled and required bitmasks respectively. Then let
> >> >> apply_forced_caps() clear/set these feature bits in the x86_capability.
> >> >> 
> >> >> Fixes: 6449dcb0cac7 ("x86: CPUID and CR3/CR4 flags for Linear Address Masking")
> >> >> Fixes: 51c158f7aacc ("x86/cpufeatures: Add the CPU feature bit for FRED")
> >> >> Fixes: 706d51681d63 ("x86/speculation: Support Enhanced IBRS on future CPUs")
> >> >> Fixes: e7b6385b01d8 ("x86/cpufeatures: Add Intel SGX hardware bits")
> >> >> Fixes: 6650cdd9a8cc ("x86/split_lock: Enable split lock detection by kernel")
> >> >> Fixes: 701fb66d576e ("x86/cpufeatures: Add CPU feature flags for shadow stacks")
> >> >> Fixes: ff4f82816dff ("x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions")
> >> >
> >> >That is fricken insane.
> >> >
> >> >You are saying to people who backport stuff:
> >> >	This fixes a commit found in the following kernel releases:
> >> >		6.4
> >> >		6.9
> >> >		3.16.68 4.4.180 4.9.137 4.14.81 4.18.19 4.19
> >> >		5.11
> >> >		5.7
> >> >		6.6
> >> >		5.10
> >> >
> >> >You didn't even sort this in any sane order, how was it generated?
> >> >
> >> >What in the world is anyone supposed to do with this?
> >> >
> >> >If you were sent a patch with this in it, what would you think?  What
> >> >could you do with it?
> >> >
> >> >Please be reasonable and consider us overworked stable maintainers and
> >> >give us a chance to get things right.  As it is, this just makes things
> >> >worse...
> >> >
> >> >greg k-h
> >> 
> >> Sorry, I certainly didn't want to add you more work.
> >> 
> >> I noted down which features are present in the x86_capability bitmask while
> >> they're not compiled into the kernel. Then I noted down which commits added
> >> these feature flags. So I suppose the order is from least to most significant
> >> feature bit, which now I realize doesn't help much in backporting, again sorry.
> >> 
> >> Would a more fitting Fixes: commit be the one that changed how the feature flags
> >> are used? At some point docs started stating to have them set only when features
> >> are COMPILED & HARDWARE-SUPPORTED.
> >
> >What would you want to see if you had to do something with a "Fixes:"
> >line?
> 
> I suppose I'd want to see a Fixes:commit in a place that hasn't seen too many
> changes. So the backport process doesn't hit too many infrastructure changes
> since that makes things more tricky.
> 
> And I guess it would be great if the Fixes:commit pointed at some obvious error
> that happened - like a place that could dereference a NULL pointer for example.
> 
> But I thought Fixes: was supposed to mark the origin point of some error the
> patch is fixing?
> 
> In this case a documentation patch [1] changed how feature flags are supposed to
> behave. But these flags were added in various points in the past. So what should
> Fixes: point at then?
> 
> But anyway writing this now I get the feeling that [1] would be a better point
> to mark for the "Fixes:" line.
> 
> [1] ea4e3bef4c94 ("Documentation/x86: Add documentation for /proc/cpuinfo feature flags")

If you feel that this patch should be backported to only 5.10 and newer,
great, that's the correct marking place.  If not, you might want to
reconsider it :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23  9:22 [PATCH v2] x86: Clear feature bits disabled at compile-time Maciej Wieczor-Retman
  2025-07-23  9:45 ` Greg KH
@ 2025-07-23 13:46 ` Borislav Petkov
  2025-07-23 15:13   ` Maciej Wieczor-Retman
  2025-07-23 14:08 ` H. Peter Anvin
  2 siblings, 1 reply; 15+ messages in thread
From: Borislav Petkov @ 2025-07-23 13:46 UTC (permalink / raw)
  To: Maciej Wieczor-Retman
  Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, H. Peter Anvin,
	Kirill A. Shutemov, Alexander Potapenko, Peter Zijlstra (Intel),
	Xin Li, Sai Praneeth, Jethro Beekman, Jarkko Sakkinen,
	Sean Christopherson, Tony Luck, Fenghua Yu, Mike Rapoport (IBM),
	Kees Cook, Rick Edgecombe, Yu-cheng Yu, stable, Borislav Petkov,
	linux-kernel

On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
> +static __init void init_cpu_cap(struct cpuinfo_x86 *c)
> +{
> +	int i;
> +
> +	for (i = 0; i < NCAPINTS; i++) {
> +		cpu_caps_set[i] = REQUIRED_MASK(i);
> +		cpu_caps_cleared[i] = DISABLED_MASK(i);
> +	}
> +}

There's already apply_forced_caps(). Not another cap massaging function
please. Add that stuff there.

As to what the Fixes: tag should be - it should not have any Fixes: tag
because AFAICT, this has always been this way. So this fix should be
backported everywhere.

Thx.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23  9:22 [PATCH v2] x86: Clear feature bits disabled at compile-time Maciej Wieczor-Retman
  2025-07-23  9:45 ` Greg KH
  2025-07-23 13:46 ` Borislav Petkov
@ 2025-07-23 14:08 ` H. Peter Anvin
  2025-07-23 16:17   ` Maciej Wieczor-Retman
  2 siblings, 1 reply; 15+ messages in thread
From: H. Peter Anvin @ 2025-07-23 14:08 UTC (permalink / raw)
  To: Maciej Wieczor-Retman, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, Kirill A. Shutemov,
	Alexander Potapenko, Peter Zijlstra (Intel), Xin Li, Sai Praneeth,
	Jethro Beekman, Jarkko Sakkinen, Sean Christopherson, Tony Luck,
	Fenghua Yu, Mike Rapoport (IBM), Kees Cook, Rick Edgecombe,
	Yu-cheng Yu
  Cc: stable, Borislav Petkov, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On 2025-07-23 02:22, Maciej Wieczor-Retman wrote:
> 
>  arch/x86/kernel/cpu/common.c       | 12 ++++++++++++
>  arch/x86/tools/cpufeaturemasks.awk |  8 ++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 77afca95cced..ba8b5fba8552 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1709,6 +1709,16 @@ static void __init cpu_parse_early_param(void)
>  	}
>  }
>  
> +static __init void init_cpu_cap(struct cpuinfo_x86 *c)
> +{
> +	int i;
> +
> +	for (i = 0; i < NCAPINTS; i++) {
> +		cpu_caps_set[i] = REQUIRED_MASK(i);
> +		cpu_caps_cleared[i] = DISABLED_MASK(i);
> +	}
> +}
> +

No... just use an static array initializer for cpu_caps_cleared[].  You
don't even need to add any code at all.

Ironically enough, I actually had those macros generated in the original
awk script version, but they weren't used.

And you MUST NOT initialize cpu_caps_set[].  What we *can* and probably
should do is, at the very end of the process, verify that the final mask
conforms to cpu_caps_set[] and print a nasty message and set the
TAINT_CPU_OUT_OF_SPEC flag:

	add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK)

... but that is a separate patch, and doesn't need to be backported.

Xin send me a patch privately (attached) but it needs to have the
REQUIRED_MASK_INIT_VALUES part removed and made into a proper patch.

Xin didn't add an SoB on it, but you can use mine:

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>

	-hpa

[-- Attachment #2: xin.patch --]
[-- Type: text/x-patch, Size: 6630 bytes --]

Return-Path: <xin@zytor.com>
Received: from [192.168.7.202] ([71.202.166.45])
	(authenticated bits=0)
	by mail.zytor.com (8.18.1/8.17.1) with ESMTPSA id 56MIMkSj692590
	(version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NO)
	for <hpa@zytor.com>; Tue, 22 Jul 2025 11:22:47 -0700
DKIM-Filter: OpenDKIM Filter v2.11.0 mail.zytor.com 56MIMkSj692590
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zytor.com;
	s=2025062101; t=1753208567;
	bh=dyteAFsAJy5+VmQ/vZVASq8c7iaJ+iMBDZ5Nx+HCzmY=;
	h=Date:Subject:To:References:From:In-Reply-To:From;
	b=nnoRrPuX9IJ+o1R7h9A6G2yU1Z6Z25CzI24+q6sTmlatPY95L9YrzXCzyvXlk0bNl
	 ofB4oChO8SZeMR7Gda6mAq3onjvLSf4IdVNumFa34T1r3yhjlyQ14HjMUSeNMvsaBa
	 3sDt5+Lf48MxO1FEcDg/AO7SWl6qGbeCtBtJM2IQSMWIBb31ggqA0BdZcL9JYsNZwl
	 3mzCevgS0tXB8xztoUjOdP42LbaBGrcdyflSPklVaPH/kDQVx9qWFSKa0zGQVKeg0N
	 7ItKpH4We2plNju0g6uDwZBQBxyO38VQlCuQy+l/M/i6Hmn56rtFhzgymNsUAF54PV
	 XPPXWUquDp/Gw==
Message-ID: <9ffd7f0b-f196-41a1-83c2-4a830d1bb035@zytor.com>
Date: Tue, 22 Jul 2025 11:22:46 -0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] x86: Clear LAM and FRED feature bits
To: "H. Peter Anvin" <hpa@zytor.com>
References: <20250722074439.4069992-1-maciej.wieczor-retman@intel.com>
 <32382f60-79fb-4cfa-87b4-581f92c980da@zytor.com>
 <A4EB1016-8CF7-4609-BBF1-9AEC83B52A4F@zytor.com>
 <5654b3bc-15d4-443b-a7b1-2f9fd1d3e0aa@zytor.com>
 <AA38D754-3189-4905-AA3B-45A56065B68A@zytor.com>
 <91dd8b75-1191-4a3d-9f62-d94c325793fd@zytor.com>
Content-Language: en-US
From: Xin Li <xin@zytor.com>
Autocrypt: addr=xin@zytor.com; keydata=
 xsDNBGUPz1cBDACS/9yOJGojBFPxFt0OfTWuMl0uSgpwk37uRrFPTTLw4BaxhlFL0bjs6q+0
 2OfG34R+a0ZCuj5c9vggUMoOLdDyA7yPVAJU0OX6lqpg6z/kyQg3t4jvajG6aCgwSDx5Kzg5
 Rj3AXl8k2wb0jdqRB4RvaOPFiHNGgXCs5Pkux/qr0laeFIpzMKMootGa4kfURgPhRzUaM1vy
 bsMsL8vpJtGUmitrSqe5dVNBH00whLtPFM7IbzKURPUOkRRiusFAsw0a1ztCgoFczq6VfAVu
 raTye0L/VXwZd+aGi401V2tLsAHxxckRi9p3mc0jExPc60joK+aZPy6amwSCy5kAJ/AboYtY
 VmKIGKx1yx8POy6m+1lZ8C0q9b8eJ8kWPAR78PgT37FQWKYS1uAroG2wLdK7FiIEpPhCD+zH
 wlslo2ETbdKjrLIPNehQCOWrT32k8vFNEMLP5G/mmjfNj5sEf3IOKgMTMVl9AFjsINLHcxEQ
 6T8nGbX/n3msP6A36FDfdSEAEQEAAc0WWGluIExpIDx4aW5Aenl0b3IuY29tPsLBDQQTAQgA
 NxYhBIUq/WFSDTiOvUIqv2u9DlcdrjdRBQJlD89XBQkFo5qAAhsDBAsJCAcFFQgJCgsFFgID
 AQAACgkQa70OVx2uN1HUpgv/cM2fsFCQodLArMTX5nt9yqAWgA5t1srri6EgS8W3F+3Kitge
 tYTBKu6j5BXuXaX3vyfCm+zajDJN77JHuYnpcKKr13VcZi1Swv6Jx1u0II8DOmoDYLb1Q2ZW
 v83W55fOWJ2g72x/UjVJBQ0sVjAngazU3ckc0TeNQlkcpSVGa/qBIHLfZraWtdrNAQT4A1fa
 sWGuJrChBFhtKbYXbUCu9AoYmmbQnsx2EWoJy3h7OjtfFapJbPZql+no5AJ3Mk9eE5oWyLH+
 QWqtOeJM7kKvn/dBudokFSNhDUw06e7EoVPSJyUIMbYtUO7g2+Atu44G/EPP0yV0J4lRO6EA
 wYRXff7+I1jIWEHpj5EFVYO6SmBg7zF2illHEW31JAPtdDLDHYcZDfS41caEKOQIPsdzQkaQ
 oW2hchcjcMPAfyhhRzUpVHLPxLCetP8vrVhTvnaZUo0xaVYb3+wjP+D5j/3+hwblu2agPsaE
 vgVbZ8Fx3TUxUPCAdr/p73DGg57oHjgezsDNBGUPz1gBDAD4Mg7hMFRQqlzotcNSxatlAQNL
 MadLfUTFz8wUUa21LPLrHBkUwm8RujehJrzcVbPYwPXIO0uyL/F///CogMNx7Iwo6by43KOy
 g89wVFhyy237EY76j1lVfLzcMYmjBoTH95fJC/lVb5Whxil6KjSN/R/y3jfG1dPXfwAuZ/4N
 cMoOslWkfZKJeEut5aZTRepKKF54T5r49H9F7OFLyxrC/uI9UDttWqMxcWyCkHh0v1Di8176
 jjYRNTrGEfYfGxSp+3jYL3PoNceIMkqM9haXjjGl0W1B4BidK1LVYBNov0rTEzyr0a1riUrp
 Qk+6z/LHxCM9lFFXnqH7KWeToTOPQebD2B/Ah5CZlft41i8L6LOF/LCuDBuYlu/fI2nuCc8d
 m4wwtkou1Y/kIwbEsE/6RQwRXUZhzO6llfoN96Fczr/RwvPIK5SVMixqWq4QGFAyK0m/1ap4
 bhIRrdCLVQcgU4glo17vqfEaRcTW5SgX+pGs4KIPPBE5J/ABD6pBnUUAEQEAAcLA/AQYAQgA
 JhYhBIUq/WFSDTiOvUIqv2u9DlcdrjdRBQJlD89ZBQkFo5qAAhsMAAoJEGu9DlcdrjdR4C0L
 /RcjolEjoZW8VsyxWtXazQPnaRvzZ4vhmGOsCPr2BPtMlSwDzTlri8BBG1/3t/DNK4JLuwEj
 OAIE3fkkm+UG4Kjud6aNeraDI52DRVCSx6xff3bjmJsJJMb12mWglN6LjdF6K+PE+OTJUh2F
 dOhslN5C2kgl0dvUuevwMgQF3IljLmi/6APKYJHjkJpu1E6luZec/lRbetHuNFtbh3xgFIJx
 2RpgVDP4xB3f8r0I+y6ua+p7fgOjDLyoFjubRGed0Be45JJQEn7A3CSb6Xu7NYobnxfkwAGZ
 Q81a2XtvNS7Aj6NWVoOQB5KbM4yosO5+Me1V1SkX2jlnn26JPEvbV3KRFcwV5RnDxm4OQTSk
 PYbAkjBbm+tuJ/Sm+5Yp5T/BnKz21FoCS8uvTiziHj2H7Cuekn6F8EYhegONm+RVg3vikOpn
 gao85i4HwQTK9/D1wgJIQkdwWXVMZ6q/OALaBp82vQ2U9sjTyFXgDjglgh00VRAHP7u1Rcu4
 l75w1xInsg==
In-Reply-To: <91dd8b75-1191-4a3d-9f62-d94c325793fd@zytor.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 7/22/2025 11:13 AM, H. Peter Anvin wrote:
> On 2025-07-22 11:08, H. Peter Anvin wrote:
>>>
>>> Yes, something like:
>>>
>>> 	void __init init_cpu_cap(void)
>>> 	{
>>> 		for (i = 0; i < NCAPINTS; i++) {
>>> 			cpu_caps_set[i] = REQUIRED_MASK(i);
>>> 			cpu_caps_cleared[i] = DISABLED_MASK(i);
>>> 		}
>>> 	}
>>>
>>> And it would be better if it could be done at build time (to avoid
>>> changing Xen which has a dedicated startup code path):
>>>
>>> 	__u32 cpu_caps_{set,cleared}[NCAPINTS + NBUGINTS] = {
>>> 		{REQUIRED,DISABLED}_MASK(i),
>>> 	};
>>>
>>> And then apply_forced_caps() will do the rest automatically :)
>>
>> Yeah:
>>
>> u32 __init cpu_caps_cleared = { ... };
>>
>> ... which can come straight out of the awk script.
>>
> 
> Heck, I think I even did something like this in the first version, but
> tglx or someone else asked to axe it because it wasn't being used (yet.)  ;)
> 

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 5809534ecc98..eb075fbf337e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -703,8 +703,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_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
+__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned 
long)) = DISABLED_MASK_INIT_VALUES;
+__u32 cpu_caps_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned 
long)) = REQUIRED_MASK_INIT_VALUES;

  #ifdef CONFIG_X86_32
  /* The 32-bit entry code needs to find cpu_entry_area. */
diff --git a/arch/x86/tools/cpufeaturemasks.awk 
b/arch/x86/tools/cpufeaturemasks.awk
index 173d5bf2d999..6b3583304ee8 100755
--- a/arch/x86/tools/cpufeaturemasks.awk
+++ b/arch/x86/tools/cpufeaturemasks.awk
@@ -81,7 +81,13 @@ END {
                                 printf "\t\\\n\t\t((x) >> 5) == %2d ? 
%s_MASK%d :", i, s, i;
                 }
                 printf " 0\t\\\n";
-               printf "\t) & (1U << ((x) & 31)))\n\n";
+               printf "\t) & (1U << ((x) & 31)))\n";
+
+               printf "\n#define %s_MASK_INIT_VALUES\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";

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 13:46 ` Borislav Petkov
@ 2025-07-23 15:13   ` Maciej Wieczor-Retman
  2025-07-23 15:28     ` H. Peter Anvin
  0 siblings, 1 reply; 15+ messages in thread
From: Maciej Wieczor-Retman @ 2025-07-23 15:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, H. Peter Anvin,
	Kirill A. Shutemov, Alexander Potapenko, Peter Zijlstra (Intel),
	Xin Li, Sai Praneeth, Jethro Beekman, Jarkko Sakkinen,
	Sean Christopherson, Tony Luck, Fenghua Yu, Mike Rapoport (IBM),
	Kees Cook, Rick Edgecombe, Yu-cheng Yu, stable, Borislav Petkov,
	linux-kernel

On 2025-07-23 at 15:46:40 +0200, Borislav Petkov wrote:
>On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
>> +static __init void init_cpu_cap(struct cpuinfo_x86 *c)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < NCAPINTS; i++) {
>> +		cpu_caps_set[i] = REQUIRED_MASK(i);
>> +		cpu_caps_cleared[i] = DISABLED_MASK(i);
>> +	}
>> +}
>
>There's already apply_forced_caps(). Not another cap massaging function
>please. Add that stuff there.

I'll try that, but can't it overwrite some things? apply_forced_caps() is called
three times and cpu_caps_set/cleared are modified in between from what I can
see. init_cpu_cap() was supposed to only initialize these arrays.

>
>As to what the Fixes: tag should be - it should not have any Fixes: tag
>because AFAICT, this has always been this way. So this fix should be
>backported everywhere.

I found that in 5.9-rc1 the documentation for how /proc/cpuinfo should work was
merged [1]. I understand that from that point on, while one can't rely on a
feature's absence, it's a reliable convention that if a flag is present, then
the feature is working. So from 5.9 on, it seems like a bug when these features
show up as working while they're not due to not being compiled.

[1] ea4e3bef4c94 ("Documentation/x86: Add documentation for /proc/cpuinfo feature flags")

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

-- 
Kind regards
Maciej Wieczór-Retman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 15:13   ` Maciej Wieczor-Retman
@ 2025-07-23 15:28     ` H. Peter Anvin
  2025-07-23 17:13       ` Maciej Wieczor-Retman
  0 siblings, 1 reply; 15+ messages in thread
From: H. Peter Anvin @ 2025-07-23 15:28 UTC (permalink / raw)
  To: Maciej Wieczor-Retman, Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	Kirill A. Shutemov, Alexander Potapenko, Peter Zijlstra (Intel),
	Xin Li, Sai Praneeth, Jethro Beekman, Jarkko Sakkinen,
	Sean Christopherson, Tony Luck, Fenghua Yu, Mike Rapoport (IBM),
	Kees Cook, Rick Edgecombe, Yu-cheng Yu, stable, Borislav Petkov,
	linux-kernel

On July 23, 2025 8:13:07 AM PDT, Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> wrote:
>On 2025-07-23 at 15:46:40 +0200, Borislav Petkov wrote:
>>On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
>>> +static __init void init_cpu_cap(struct cpuinfo_x86 *c)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < NCAPINTS; i++) {
>>> +		cpu_caps_set[i] = REQUIRED_MASK(i);
>>> +		cpu_caps_cleared[i] = DISABLED_MASK(i);
>>> +	}
>>> +}
>>
>>There's already apply_forced_caps(). Not another cap massaging function
>>please. Add that stuff there.
>
>I'll try that, but can't it overwrite some things? apply_forced_caps() is called
>three times and cpu_caps_set/cleared are modified in between from what I can
>see. init_cpu_cap() was supposed to only initialize these arrays.
>
>>
>>As to what the Fixes: tag should be - it should not have any Fixes: tag
>>because AFAICT, this has always been this way. So this fix should be
>>backported everywhere.
>
>I found that in 5.9-rc1 the documentation for how /proc/cpuinfo should work was
>merged [1]. I understand that from that point on, while one can't rely on a
>feature's absence, it's a reliable convention that if a flag is present, then
>the feature is working. So from 5.9 on, it seems like a bug when these features
>show up as working while they're not due to not being compiled.
>
>[1] ea4e3bef4c94 ("Documentation/x86: Add documentation for /proc/cpuinfo feature flags")
>
>>
>>Thx.
>>
>>-- 
>>Regards/Gruss,
>>    Boris.
>>
>>https://people.kernel.org/tglx/notes-about-netiquette
>

What are you concerned it would overwrite? I'm confused.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 13:03       ` Maciej Wieczor-Retman
  2025-07-23 13:37         ` Greg KH
@ 2025-07-23 15:52         ` Xin Li
  2025-07-24  0:24           ` Xin Li
  1 sibling, 1 reply; 15+ messages in thread
From: Xin Li @ 2025-07-23 15:52 UTC (permalink / raw)
  To: Maciej Wieczor-Retman, Greg KH
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck, Fenghua Yu,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu,
	stable, Borislav Petkov, linux-kernel

On 7/23/2025 6:03 AM, Maciej Wieczor-Retman wrote:
> On 2025-07-23 at 13:57:34 +0200, Greg KH wrote:
>> On Wed, Jul 23, 2025 at 01:46:44PM +0200, Maciej Wieczor-Retman wrote:
>>> On 2025-07-23 at 11:45:22 +0200, Greg KH wrote:
>>>> On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
>>>>> 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.
>>>>>
>>>>> Add a DISABLED_MASK() macro that returns 32 bit chunks of the disabled
>>>>> feature bits bitmask.
>>>>>
>>>>> Initialize the cpu_caps_cleared and cpu_caps_set arrays with the
>>>>> contents of the disabled and required bitmasks respectively. Then let
>>>>> apply_forced_caps() clear/set these feature bits in the x86_capability.
>>>>>
>>>>> Fixes: 6449dcb0cac7 ("x86: CPUID and CR3/CR4 flags for Linear Address Masking")
>>>>> Fixes: 51c158f7aacc ("x86/cpufeatures: Add the CPU feature bit for FRED")
>>>>> Fixes: 706d51681d63 ("x86/speculation: Support Enhanced IBRS on future CPUs")
>>>>> Fixes: e7b6385b01d8 ("x86/cpufeatures: Add Intel SGX hardware bits")
>>>>> Fixes: 6650cdd9a8cc ("x86/split_lock: Enable split lock detection by kernel")
>>>>> Fixes: 701fb66d576e ("x86/cpufeatures: Add CPU feature flags for shadow stacks")
>>>>> Fixes: ff4f82816dff ("x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions")
>>>>
>>>> That is fricken insane.
>>>>
>>>> You are saying to people who backport stuff:
>>>> 	This fixes a commit found in the following kernel releases:
>>>> 		6.4
>>>> 		6.9
>>>> 		3.16.68 4.4.180 4.9.137 4.14.81 4.18.19 4.19
>>>> 		5.11
>>>> 		5.7
>>>> 		6.6
>>>> 		5.10
>>>>
>>>> You didn't even sort this in any sane order, how was it generated?
>>>>
>>>> What in the world is anyone supposed to do with this?
>>>>
>>>> If you were sent a patch with this in it, what would you think?  What
>>>> could you do with it?
>>>>
>>>> Please be reasonable and consider us overworked stable maintainers and
>>>> give us a chance to get things right.  As it is, this just makes things
>>>> worse...
>>>>
>>>> greg k-h
>>>
>>> Sorry, I certainly didn't want to add you more work.
>>>
>>> I noted down which features are present in the x86_capability bitmask while
>>> they're not compiled into the kernel. Then I noted down which commits added
>>> these feature flags. So I suppose the order is from least to most significant
>>> feature bit, which now I realize doesn't help much in backporting, again sorry.
>>>
>>> Would a more fitting Fixes: commit be the one that changed how the feature flags
>>> are used? At some point docs started stating to have them set only when features
>>> are COMPILED & HARDWARE-SUPPORTED.
>>
>> What would you want to see if you had to do something with a "Fixes:"
>> line?
> 
> I suppose I'd want to see a Fixes:commit in a place that hasn't seen too many
> changes. So the backport process doesn't hit too many infrastructure changes
> since that makes things more tricky.
> 
> And I guess it would be great if the Fixes:commit pointed at some obvious error
> that happened - like a place that could dereference a NULL pointer for example.
> 
> But I thought Fixes: was supposed to mark the origin point of some error the
> patch is fixing?
> 
> In this case a documentation patch [1] changed how feature flags are supposed to
> behave. But these flags were added in various points in the past. So what should
> Fixes: point at then?
> 
> But anyway writing this now I get the feeling that [1] would be a better point
> to mark for the "Fixes:" line.
> 
> [1] ea4e3bef4c94 ("Documentation/x86: Add documentation for /proc/cpuinfo feature flags")
> 

We need to investigate the root cause of this bug; and this seems like a
regression caused by my patch set that automatically generates CPU 
feature masks.

Just further debugging is needed to identify the culprit.

Thanks!
     Xin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 14:08 ` H. Peter Anvin
@ 2025-07-23 16:17   ` Maciej Wieczor-Retman
  0 siblings, 0 replies; 15+ messages in thread
From: Maciej Wieczor-Retman @ 2025-07-23 16:17 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	Kirill A. Shutemov, Alexander Potapenko, Peter Zijlstra (Intel),
	Xin Li, Sai Praneeth, Jethro Beekman, Jarkko Sakkinen,
	Sean Christopherson, Tony Luck, Fenghua Yu, Mike Rapoport (IBM),
	Kees Cook, Rick Edgecombe, Yu-cheng Yu, stable, Borislav Petkov,
	linux-kernel

On 2025-07-23 at 07:08:43 -0700, H. Peter Anvin wrote:
>On 2025-07-23 02:22, Maciej Wieczor-Retman wrote:
>> 
>>  arch/x86/kernel/cpu/common.c       | 12 ++++++++++++
>>  arch/x86/tools/cpufeaturemasks.awk |  8 ++++++++
>>  2 files changed, 20 insertions(+)
>> 
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index 77afca95cced..ba8b5fba8552 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -1709,6 +1709,16 @@ static void __init cpu_parse_early_param(void)
>>  	}
>>  }
>>  
>> +static __init void init_cpu_cap(struct cpuinfo_x86 *c)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < NCAPINTS; i++) {
>> +		cpu_caps_set[i] = REQUIRED_MASK(i);
>> +		cpu_caps_cleared[i] = DISABLED_MASK(i);
>> +	}
>> +}
>> +
>
>No... just use an static array initializer for cpu_caps_cleared[].  You
>don't even need to add any code at all.

Oh, right, that seems simpler

>
>Ironically enough, I actually had those macros generated in the original
>awk script version, but they weren't used.
>
>And you MUST NOT initialize cpu_caps_set[]. 

After thinking about it more I can see why doing that could be a problem, yes.

>What we *can* and probably should do is, at the very end of the process, verify
>that the final mask conforms to cpu_caps_set[] and print a nasty message and
>set the TAINT_CPU_OUT_OF_SPEC flag:
>
>	add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK)

Where would that end be though? End of identify_cpu()? apply_forced_caps() is
almost at the end of the function, and other functions don't seem to modify
x86_cpuinfo or cpu_caps_set[].

>... but that is a separate patch, and doesn't need to be backported.
>
>Xin send me a patch privately (attached) but it needs to have the
>REQUIRED_MASK_INIT_VALUES part removed and made into a proper patch.
>
>Xin didn't add an SoB on it, but you can use mine:
>
>Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>

Thanks, I'll work with that.

>
>	-hpa



-- 
Kind regards
Maciej Wieczór-Retman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 15:28     ` H. Peter Anvin
@ 2025-07-23 17:13       ` Maciej Wieczor-Retman
  2025-07-23 17:23         ` H. Peter Anvin
  0 siblings, 1 reply; 15+ messages in thread
From: Maciej Wieczor-Retman @ 2025-07-23 17:13 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	Kirill A. Shutemov, Alexander Potapenko, Peter Zijlstra (Intel),
	Xin Li, Sai Praneeth, Jethro Beekman, Jarkko Sakkinen,
	Sean Christopherson, Tony Luck, Fenghua Yu, Mike Rapoport (IBM),
	Kees Cook, Rick Edgecombe, Yu-cheng Yu, stable, Borislav Petkov,
	linux-kernel

On 2025-07-23 at 08:28:32 -0700, H. Peter Anvin wrote:
>On July 23, 2025 8:13:07 AM PDT, Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> wrote:
>>On 2025-07-23 at 15:46:40 +0200, Borislav Petkov wrote:
>>>On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
>>>> +static __init void init_cpu_cap(struct cpuinfo_x86 *c)
>>>> +{
>>>> +	int i;
>>>> +
>>>> +	for (i = 0; i < NCAPINTS; i++) {
>>>> +		cpu_caps_set[i] = REQUIRED_MASK(i);
>>>> +		cpu_caps_cleared[i] = DISABLED_MASK(i);
>>>> +	}
>>>> +}
>>>
>>>There's already apply_forced_caps(). Not another cap massaging function
>>>please. Add that stuff there.
>>
>>I'll try that, but can't it overwrite some things? apply_forced_caps() is called
>>three times and cpu_caps_set/cleared are modified in between from what I can
>>see. init_cpu_cap() was supposed to only initialize these arrays.
>>
>What are you concerned it would overwrite? I'm confused.

I thought that cpu_caps_set/cleared could change in-between apply_forced_caps()
calls. Therefore if we also applied the DISABLED_MASK() in every
apply_forced_caps() call I thought it might clear some flag that other function
might set.

But I've been looking at these calls for a while now and that doesn't seem
possible. Changes are made only if features are compiled, so it doesn't
interfere with the DISABLED_MASK().

Sorry for the confusion.

-- 
Kind regards
Maciej Wieczór-Retman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 17:13       ` Maciej Wieczor-Retman
@ 2025-07-23 17:23         ` H. Peter Anvin
  0 siblings, 0 replies; 15+ messages in thread
From: H. Peter Anvin @ 2025-07-23 17:23 UTC (permalink / raw)
  To: Maciej Wieczor-Retman
  Cc: Borislav Petkov, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	Kirill A. Shutemov, Alexander Potapenko, Peter Zijlstra (Intel),
	Xin Li, Sai Praneeth, Jethro Beekman, Jarkko Sakkinen,
	Sean Christopherson, Tony Luck, Fenghua Yu, Mike Rapoport (IBM),
	Kees Cook, Rick Edgecombe, Yu-cheng Yu, stable, Borislav Petkov,
	linux-kernel

On July 23, 2025 10:13:52 AM PDT, Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> wrote:
>On 2025-07-23 at 08:28:32 -0700, H. Peter Anvin wrote:
>>On July 23, 2025 8:13:07 AM PDT, Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> wrote:
>>>On 2025-07-23 at 15:46:40 +0200, Borislav Petkov wrote:
>>>>On Wed, Jul 23, 2025 at 11:22:49AM +0200, Maciej Wieczor-Retman wrote:
>>>>> +static __init void init_cpu_cap(struct cpuinfo_x86 *c)
>>>>> +{
>>>>> +	int i;
>>>>> +
>>>>> +	for (i = 0; i < NCAPINTS; i++) {
>>>>> +		cpu_caps_set[i] = REQUIRED_MASK(i);
>>>>> +		cpu_caps_cleared[i] = DISABLED_MASK(i);
>>>>> +	}
>>>>> +}
>>>>
>>>>There's already apply_forced_caps(). Not another cap massaging function
>>>>please. Add that stuff there.
>>>
>>>I'll try that, but can't it overwrite some things? apply_forced_caps() is called
>>>three times and cpu_caps_set/cleared are modified in between from what I can
>>>see. init_cpu_cap() was supposed to only initialize these arrays.
>>>
>>What are you concerned it would overwrite? I'm confused.
>
>I thought that cpu_caps_set/cleared could change in-between apply_forced_caps()
>calls. Therefore if we also applied the DISABLED_MASK() in every
>apply_forced_caps() call I thought it might clear some flag that other function
>might set.
>
>But I've been looking at these calls for a while now and that doesn't seem
>possible. Changes are made only if features are compiled, so it doesn't
>interfere with the DISABLED_MASK().
>
>Sorry for the confusion.
>

Any changes would be additive, or we would be in a world of hurt anyway.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] x86: Clear feature bits disabled at compile-time
  2025-07-23 15:52         ` Xin Li
@ 2025-07-24  0:24           ` Xin Li
  0 siblings, 0 replies; 15+ messages in thread
From: Xin Li @ 2025-07-24  0:24 UTC (permalink / raw)
  To: Maciej Wieczor-Retman, Greg KH
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Kirill A. Shutemov, Alexander Potapenko,
	Peter Zijlstra (Intel), Xin Li, Sai Praneeth, Jethro Beekman,
	Jarkko Sakkinen, Sean Christopherson, Tony Luck,
	Mike Rapoport (IBM), Kees Cook, Rick Edgecombe, Yu-cheng Yu,
	stable, Borislav Petkov, linux-kernel

On 7/23/2025 8:52 AM, Xin Li wrote:
> We need to investigate the root cause of this bug; and this seems like a
> regression caused by my patch set that automatically generates CPU 
> feature masks.

My bad, it's not caused by my patch set.

The bug is there for a long time before the patch set was merged.

And it seems we need a different patch for older Linux kernels.

Thanks!
     Xin


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-07-24  0:25 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-23  9:22 [PATCH v2] x86: Clear feature bits disabled at compile-time Maciej Wieczor-Retman
2025-07-23  9:45 ` Greg KH
2025-07-23 11:46   ` Maciej Wieczor-Retman
2025-07-23 11:57     ` Greg KH
2025-07-23 13:03       ` Maciej Wieczor-Retman
2025-07-23 13:37         ` Greg KH
2025-07-23 15:52         ` Xin Li
2025-07-24  0:24           ` Xin Li
2025-07-23 13:46 ` Borislav Petkov
2025-07-23 15:13   ` Maciej Wieczor-Retman
2025-07-23 15:28     ` H. Peter Anvin
2025-07-23 17:13       ` Maciej Wieczor-Retman
2025-07-23 17:23         ` H. Peter Anvin
2025-07-23 14:08 ` H. Peter Anvin
2025-07-23 16:17   ` Maciej Wieczor-Retman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.