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

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

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

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

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

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.

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

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.

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

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

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

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

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

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

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