[v3,1/6] x86/microcode: Add a parameter to microcode_check() to store CPU capabilities

Message ID 20230103180212.333496-2-ashok.raj@intel.com
State New
Headers
Series Some fixes and cleanups for microcode |

Commit Message

Ashok Raj Jan. 3, 2023, 6:02 p.m. UTC
  This is a preparation before the next patch uses this to compare CPU
capabilities after performing an update.

Add a parameter to store CPU capabilities before performing a microcode
update.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: x86 <x86@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/include/asm/processor.h     |  2 +-
 arch/x86/kernel/cpu/common.c         | 12 +++++-------
 arch/x86/kernel/cpu/microcode/core.c |  3 ++-
 3 files changed, 8 insertions(+), 9 deletions(-)
  

Comments

Borislav Petkov Jan. 4, 2023, 6:21 p.m. UTC | #1
On Tue, Jan 03, 2023 at 10:02:07AM -0800, Ashok Raj wrote:
> This is a preparation before the next patch uses this to compare CPU

Once a patch is in git, the concept of "subsequent" or "next" patch becomes
ambiguous depending on how you're sorting them.

So you should strive for your commit messages to make sense on their own,
without referencing other "subsequent" or "next" patches.

> capabilities after performing an update.
> 
> Add a parameter to store CPU capabilities before performing a microcode
> update.

	" ... so that code later can do X."

And that is enough for an explanation.

> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 9cfca3d7d0e2..b9c7529c920e 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -2302,25 +2302,23 @@ void cpu_init_secondary(void)
>   * only when microcode has been updated. Caller holds microcode_mutex and CPU
>   * hotplug lock.

<--- I guess you can document that new parameter here.

>   */
> -void microcode_check(void)
> +void microcode_check(struct cpuinfo_x86 *info)

...
  

Patch

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 4e35c66edeb7..387578049de0 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -697,7 +697,7 @@  bool xen_set_default_idle(void);
 #endif
 
 void __noreturn stop_this_cpu(void *dummy);
-void microcode_check(void);
+void microcode_check(struct cpuinfo_x86 *info);
 
 enum l1tf_mitigations {
 	L1TF_MITIGATION_OFF,
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 9cfca3d7d0e2..b9c7529c920e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2302,25 +2302,23 @@  void cpu_init_secondary(void)
  * only when microcode has been updated. Caller holds microcode_mutex and CPU
  * hotplug lock.
  */
-void microcode_check(void)
+void microcode_check(struct cpuinfo_x86 *info)
 {
-	struct cpuinfo_x86 info;
-
 	perf_check_microcode();
 
 	/* Reload CPUID max function as it might've changed. */
-	info.cpuid_level = cpuid_eax(0);
+	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. The ones coming from CPUID will
 	 * get overwritten in get_cpu_cap().
 	 */
-	memcpy(&info.x86_capability, &boot_cpu_data.x86_capability, sizeof(info.x86_capability));
+	memcpy(&info->x86_capability, &boot_cpu_data.x86_capability, sizeof(info->x86_capability));
 
-	get_cpu_cap(&info);
+	get_cpu_cap(info);
 
-	if (!memcmp(&info.x86_capability, &boot_cpu_data.x86_capability, sizeof(info.x86_capability)))
+	if (!memcmp(&info->x86_capability, &boot_cpu_data.x86_capability, sizeof(info->x86_capability)))
 		return;
 
 	pr_warn("x86/CPU: CPU features have changed after loading microcode, but might not take effect.\n");
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index c4cd7328177b..d86a4f910a6b 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -439,6 +439,7 @@  static int __reload_late(void *info)
 static int microcode_reload_late(void)
 {
 	int old = boot_cpu_data.microcode, ret;
+	struct cpuinfo_x86 info;
 
 	pr_err("Attempting late microcode loading - it is dangerous and taints the kernel.\n");
 	pr_err("You should switch to early loading, if possible.\n");
@@ -448,7 +449,7 @@  static int microcode_reload_late(void)
 
 	ret = stop_machine_cpuslocked(__reload_late, NULL, cpu_online_mask);
 	if (ret == 0)
-		microcode_check();
+		microcode_check(&info);
 
 	pr_info("Reload completed, microcode revision: 0x%x -> 0x%x\n",
 		old, boot_cpu_data.microcode);