Re: [PATCH v1 5/5] x86/cpu: Don't reset boot CPU cpuinfo in identify_cpu()

[Borislav Petkov]

On Fri, Jul 03, 2026 at 05:20:46PM -0700, Ihor Solodrai wrote:
> identify_cpu() used to rebuild c->x86_capability from scratch:
> init_cpu_info() memsets the capability array to zero and
> identify_cpu() then re-reads CPUID. On the boot CPU identify_cpu()
> runs after local_irq_enable() and before alternatives are patched. An
> interrupt delivered in the window between the memset() and
> get_cpu_cap() therefore will observe cleared x86_capability [1].
> 
> However the boot CPU's capabilities have already been scanned, with
> interrupts off, by early_identify_cpu(). So move the init_cpu_info()
> call out of identify_cpu() into its callers, and skip it for the boot
> CPU on 64-bit.
> 
> Secondary CPUs call init_cpu_info() then identify_cpu() from
> identify_secondary_cpu(), with interrupts off, exactly as
> before. 32-bit still resets in identify_boot_cpu(): it relies on the
> reset for the no-CPUID cpuid_level default.

Ok, this commit message is all over the place. Please structure it something
like this:

1. Prepare the context for the explanation briefly.

2. Explain the problem at hand.

3. "It happens because of <...>"

4. "Fix it by doing X"

5. "(Potentially do Y)."

And some of those above are optional depending on the issue being
explained.

Also, do not talk about what your patches do - that should (hopefully) be
visible from the diff itself. Rather, talk about *why* you're doing what
you're doing.

In the patches where you do only mechanical movement, add

"No functional changes."

to the commit message to denote that.


> [1] https://lore.kernel.org/bpf/20260610175651.647515-1-ihor.solodrai@xxxxxxxxx/
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/common.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 8f093c6c7ccc..cd3d7c37c174 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1986,8 +1986,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>  
>  	c->loops_per_jiffy = loops_per_jiffy;
>  
> -	init_cpu_info(c);
> -
>  	if (!cpuid_feature())
>  		identify_cpu_without_cpuid(c);
>  
> @@ -2156,6 +2154,13 @@ static void identify_cpu_32(struct cpuinfo_x86 *c)
>  
>  static __init void identify_boot_cpu(void)
>  {
> +	/*
> +	 * The boot CPU's capabilities were already scanned in early_identify_cpu().
> +	 * However 32-bit still needs to init_cpu_info() here for the no-CPUID
> +	 * cpuid_level default.
> +	 */
> +	if (IS_ENABLED(CONFIG_X86_32))
> +		init_cpu_info(&boot_cpu_data);
>  	identify_cpu(&boot_cpu_data);
>  	if (HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT))
>  		pr_info("CET detected: Indirect Branch Tracking enabled\n");
> @@ -2178,6 +2183,7 @@ void identify_secondary_cpu(unsigned int cpu)
>  		*c = boot_cpu_data;
>  	c->cpu_index = cpu;
>  
> +	init_cpu_info(c);
>  	identify_cpu(c);
>  	identify_cpu_32(c);

Also, when you look at the final version, do you see how identify_cpu() and
identify_cpu_32() are soo close together so that you can basically move the
body of identify_cpu_32() to the end of identify_cpu() and then you don't need
that helper either.

Other than that, the flows do look better. 32-bit still needs dealing with the
no-CPUID case but that's for another day. I can't really find it in me to
care, actually, 32-bit needs to go away and we'll simplify a looot of code. Oh
well.

Thx.

-- 
Regards/Gruss,
    Boris.

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