Re: [PATCH v3 2/6] x86/microcode/core: Take a snapshot before and after applying microcode

[Borislav Petkov <bp@alien8.de>]

On Tue, Jan 03, 2023 at 10:02:08AM -0800, Ashok Raj wrote:
> Fixes: 1008c52c09dc ("x86/CPU: Add a microcode loader callback")

Why a Fixes tag? Do you have a failure scenario for current kernels?

If so, then it would need stable backporting.

If so, it would need the previous patch too.

> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 387578049de0..ac2e67156b9b 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -697,6 +697,7 @@ bool xen_set_default_idle(void);
>  #endif
>  
>  void __noreturn stop_this_cpu(void *dummy);
> +void microcode_store_cpu_caps(struct cpuinfo_x86 *info);

s/microcode_store_cpu_caps/store_cpu_caps/g

>  void microcode_check(struct cpuinfo_x86 *info);
>  
>  enum l1tf_mitigations {
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index b9c7529c920e..7c86c6fd07ae 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -2297,28 +2297,43 @@ void cpu_init_secondary(void)
>  #endif
>  
>  #ifdef CONFIG_MICROCODE_LATE_LOADING
> +
> +void microcode_store_cpu_caps(struct cpuinfo_x86 *info)
> +{
> +	/* Reload CPUID max function as it might've changed. */

Might've changed how?

> +	info->cpuid_level = cpuid_eax(0);
> +
> +	/*
> +	 * Copy all capability leafs to pick up the synthetic ones so that
> +	 * memcmp() below doesn't fail on that...

split that comment and put the second part...

> +	 */
> +	memcpy(info->x86_capability, &boot_cpu_data.x86_capability,
> +	       sizeof(info->x86_capability));
> +

... here:

	/*
	 * ... the ones coming from CPUID will get overwritten here:
	 */

> +	get_cpu_cap(info);
> +}
> +
>  /*
>   * The microcode loader calls this upon late microcode load to recheck features,
>   * only when microcode has been updated. Caller holds microcode_mutex and CPU
>   * hotplug lock.
>   */
> -void microcode_check(struct cpuinfo_x86 *info)
> +void microcode_check(struct cpuinfo_x86 *orig)
					   ^^^^^

Yeah, what dhansen said.

Thx.

-- 
Regards/Gruss,
    Boris.

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