Re: [PATCH v2] x86: Clear feature bits disabled at compile-time

[Xin Li <xin@zytor.com>]

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