diff mbox series

[02/13] x86/microcode: Do not load from filesystem for CPU hot add

Message ID 20221014200913.14644-3-ashok.raj@intel.com
State New
Headers
Series Make microcode loading more robust |

Commit Message

Ashok Raj Oct. 14, 2022, 8:09 p.m. UTC 
  When request_microcode_fw() is called, refresh_fw parameter must be set
only when late loading. Currently during CPU hotplug, the path is as
follows.

mc_device_add() -> microcode_init_cpu() ->
				request_microcode_fw(refresh_hw=true)

Consider if a new microcode file was just copied, but have not yet
performed a late load. Now adding a new CPU will result in loading that
microcode file vs the rest of the CPUs in the system would have a previous
revision of what was loaded earlier.

Moreover this doesn't need to be any different from logical cpu online
flow, microcode_update_cpu() and mc_device_add() seem to be doing the
opposite things.

Seems like there is no good use for the refresh_fw parameter in
microcode_init_cpu(), so simply drop it.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---
 arch/x86/kernel/cpu/microcode/core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Borislav Petkov Oct. 19, 2022, 5:52 p.m. UTC | #1 
On Fri, Oct 14, 2022 at 01:09:02PM -0700, Ashok Raj wrote:
> When request_microcode_fw() is called, refresh_fw parameter must be set
> only when late loading. Currently during CPU hotplug, the path is as
> follows.
> 
> mc_device_add() -> microcode_init_cpu() ->
> 				request_microcode_fw(refresh_hw=true)

So that ugly refresh_fw thing was getting on my nerves for a while
now so I took a long stern look today. And ended up axing off a lot
of ancient crud which was simply getting in the way and is completely
useless.

The diffstat says it all:

 arch/x86/include/asm/microcode.h      |   4 +-
 arch/x86/kernel/cpu/intel.c           |   1 -
 arch/x86/kernel/cpu/microcode/amd.c   |   5 +-
 arch/x86/kernel/cpu/microcode/core.c  | 214 ++++++++++++++++++-------------------------------------------------------------
 arch/x86/kernel/cpu/microcode/intel.c |   3 +-
 5 files changed, 52 insertions(+), 175 deletions(-)

and initial smoke-testing with CPU hotplug and suspend looks good.

I'll hammer on it some more in the coming days.

Patches as a reply to this message.

Thx.
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 6a41cee242f6..e4135b4fdbc6 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -599,7 +599,7 @@  static enum ucode_state microcode_resume_cpu(int cpu)
 	return UCODE_OK;
 }
 
-static enum ucode_state microcode_init_cpu(int cpu, bool refresh_fw)
+static enum ucode_state microcode_init_cpu(int cpu)
 {
 	enum ucode_state ustate;
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -614,7 +614,8 @@  static enum ucode_state microcode_init_cpu(int cpu, bool refresh_fw)
 	if (system_state != SYSTEM_RUNNING)
 		return UCODE_NFOUND;
 
-	ustate = microcode_ops->request_microcode_fw(cpu, &microcode_pdev->dev, refresh_fw);
+	ustate = microcode_ops->request_microcode_fw(cpu, &microcode_pdev->dev,
+						     false);
 	if (ustate == UCODE_NEW) {
 		pr_debug("CPU%d updated upon init\n", cpu);
 		apply_microcode_on_target(cpu);
@@ -633,7 +634,7 @@  static enum ucode_state microcode_update_cpu(int cpu)
 	if (uci->valid)
 		return microcode_resume_cpu(cpu);
 
-	return microcode_init_cpu(cpu, false);
+	return microcode_init_cpu(cpu);
 }
 
 static int mc_device_add(struct device *dev, struct subsys_interface *sif)
@@ -649,7 +650,7 @@  static int mc_device_add(struct device *dev, struct subsys_interface *sif)
 	if (err)
 		return err;
 
-	if (microcode_init_cpu(cpu, true) == UCODE_ERROR)
+	if (microcode_init_cpu(cpu) == UCODE_ERROR)
 		return -EINVAL;
 
 	return err;