Re: [PATCH 1/6] x86/microcode: Introduce staging option to reduce late-loading latency

[Chang S. Bae]

On 2/17/2025 5:33 AM, Borislav Petkov wrote:
> Why are you even touching this function instead of doing the staging in the
> beginning of load_late_stop_cpus()?

I thought staging is logically distinguishable. While 
load_late_stop_cpus() currently performs loading when CPUs are 
_stopped_, staging occurs on a non-critical path and remains 
interruptible. So, the function name itself seems misaligned with the 
staging process.

It looks like commit 4b753955e915 ("x86/microcode: Add per CPU result 
state") renamed microcode_reload_late() to the current name:

  -/*
  - * Reload microcode late on all CPUs. Wait for a sec until they
  - * all gather together.
  - */
  -static int microcode_reload_late(void)
  +static int load_late_stop_cpus(void)

which primarily narrowed the function’s scope to microcode rendezvous 
for late loading.


Given them all, maybe another option is to introduce a wrapper, instead 
of modifying load_late_locked() directly, like:

@@ -536,11 +536,6 @@ static int load_late_stop_cpus(bool is_safe)
        int old_rev = boot_cpu_data.microcode;
        struct cpuinfo_x86 prev_info;

-       if (!is_safe) {
-               pr_err("Late microcode loading without minimal revision 
check.\n");
-               pr_err("You should switch to early loading, if 
possible.\n");
-       }
-
        atomic_set(&late_cpus_in, num_online_cpus());
        atomic_set(&offline_in_nmi, 0);
        loops_per_usec = loops_per_jiffy / (TICK_NSEC / 1000);
@@ -674,6 +669,20 @@ static bool setup_cpus(void)
        return true;
 }

+static int load_late_apply(bool is_safe)
+{
+       if (!is_safe) {
+               pr_err("Late microcode loading without minimal revision 
check.\n");
+               pr_err("You should switch to early loading, if 
possible.\n");
+       }
+
+       /* Stage microcode without stopping CPUs */
+       if (microcode_ops->use_staging)
+               microcode_ops->stage_microcode();
+
+       return load_late_stop_cpus(is_safe);
+}
+
 static int load_late_locked(void)
 {
        if (!setup_cpus())
@@ -681,9 +690,9 @@ static int load_late_locked(void)

        switch (microcode_ops->request_microcode_fw(0, 
&microcode_pdev->dev)) {
        case UCODE_NEW:
-               return load_late_stop_cpus(false);
+               return load_late_apply(false);
        case UCODE_NEW_SAFE:
-               return load_late_stop_cpus(true);
+               return load_late_apply(true);
        case UCODE_NFOUND:
                return -ENOENT;
        default:

Thanks,
Chang