[PATCH v3 0/9] x86/microcode: Refactor NMI-based rendezvous mechanism to stop-machine

[PATCH v3 0/9] x86/microcode: Refactor NMI-based rendezvous mechanism to stop-machine

[Chang S. Bae]

Hi all,

First of all, apologies for such a long delay in posting the revision.
This series has almost fallen through the cracks unless Boris chimed in.
Even then, it took me a bit to revisit the series and post this.

Let me summarize the change since V2 [1]. Most of the changes are in the
stop-machine facility itself, addressing feedback from Sashiko, both from
the public [2] and from internal runs:

 * Simplify NMI handling to avoid false handling (patch2).
 * Use internal functions to avoid instrumentable functions (patch2).
 * Prevent use during boot (patch3).
 * Tighten to online CPUs only (patch3).

Each patch includes more detail (patch2-5). I've also noted some thoughts
on the Sashiko feedback where the concern looks less practical (patch5).

The patch set is available in this repository:
 git://github.com/intel-staging/microcode.git nmi-stop-machine_v3

Note this cover highlights a summary of changes only. Instead, V2 [1] has
lengthy cover letter that provides more background and design summary.

Thanks,
Chang

[1] V2: https://lore.kernel.org/20260331014251.86353-12-chang.seok.bae@intel.com
[2] https://sashiko.dev/#/patchset/20260331014251.86353-1-chang.seok.bae%40intel.com

Chang S. Bae (7):
  stop_machine: Refactor multi-CPU stop glue code
  stop_machine: Add NMI-based execution path
  stop_machine: Introduce stop_machine_nmi_cpuslocked()
  x86/microcode: Distinguish NMI control path on stop-machine callback
  x86/microcode: Use stop-machine NMI facility
  x86/nmi: Simplify offline microcode handler invocation
  x86/microcode: Remove microcode_nmi_handler_enable

David Kaplan (2):
  x86/apic: Implement self-NMI support
  x86/nmi: Support NMI stop-machine handler

 arch/Kconfig                         |   3 +
 arch/x86/Kconfig                     |   1 +
 arch/x86/include/asm/microcode.h     |  11 --
 arch/x86/kernel/apic/ipi.c           |  10 ++
 arch/x86/kernel/cpu/microcode/core.c |  35 ++----
 arch/x86/kernel/nmi.c                |   7 +-
 include/linux/stop_machine.h         |  43 +++++++
 kernel/stop_machine.c                | 180 ++++++++++++++++++++++++---
 8 files changed, 232 insertions(+), 58 deletions(-)


base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.53.0

[PATCH v3 1/9] stop_machine: Refactor multi-CPU stop glue code

[Chang S. Bae]

stop_machine_cpuslocked() currently configures struct multi_stop_data and
invokes the multi-stop operation. An upcoming stop_machine() variant will
have different configurations but the latter part will be shareable.

Extract the common part into stop_multi_cpus() to highlight each unique
front-end.

No functional change.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 kernel/stop_machine.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index d085ba1f4b44..8dcd5be67b85 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -582,18 +582,8 @@ static int __init cpu_stop_init(void)
 }
 early_initcall(cpu_stop_init);
 
-int stop_machine_cpuslocked(cpu_stop_fn_t fn, void *data,
-			    const struct cpumask *cpus)
+static int stop_multi_cpus(struct multi_stop_data *msdata)
 {
-	struct multi_stop_data msdata = {
-		.fn = fn,
-		.data = data,
-		.num_threads = num_online_cpus(),
-		.active_cpus = cpus,
-	};
-
-	lockdep_assert_cpus_held();
-
 	if (!stop_machine_initialized) {
 		/*
 		 * Handle the case where stop_machine() is called
@@ -603,19 +593,34 @@ int stop_machine_cpuslocked(cpu_stop_fn_t fn, void *data,
 		unsigned long flags;
 		int ret;
 
-		WARN_ON_ONCE(msdata.num_threads != 1);
+		WARN_ON_ONCE(msdata->num_threads != 1);
 
 		local_irq_save(flags);
 		hard_irq_disable();
-		ret = (*fn)(data);
+		ret = msdata->fn(msdata->data);
 		local_irq_restore(flags);
 
 		return ret;
 	}
 
 	/* Set the initial state and stop all online cpus. */
-	set_state(&msdata, MULTI_STOP_PREPARE);
-	return stop_cpus(cpu_online_mask, multi_cpu_stop, &msdata);
+	set_state(msdata, MULTI_STOP_PREPARE);
+	return stop_cpus(cpu_online_mask, multi_cpu_stop, msdata);
+}
+
+int stop_machine_cpuslocked(cpu_stop_fn_t fn, void *data,
+			    const struct cpumask *cpus)
+{
+	struct multi_stop_data msdata = {
+		.fn		= fn,
+		.data		= data,
+		.num_threads	= num_online_cpus(),
+		.active_cpus	= cpus,
+	};
+
+	lockdep_assert_cpus_held();
+
+	return stop_multi_cpus(&msdata);
 }
 
 int stop_machine(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus)
-- 
2.53.0

[PATCH v3 2/9] stop_machine: Add NMI-based execution path

[Chang S. Bae]

Currently multi_cpu_stop() executes the stop function in the
MULTI_STOP_RUN state. But NMIs can still preempt the execution. For use
cases requiring stricter isolation from them, provide an option to run
the stop function from NMI context.

Then, the NMI stop function must be built without instrumentation, as
exceptions may re-enable NMIs on return via IRET. Annotate the NMI
handler entirely with noinstr.

However, objtool cannot reliably determine whether the indirect call
target is located in a noinstr section, so it may emit false positives.
To avoid this, temporarily lift the instrumentation restriction around
the indirect call site and document the intention.

The x86 microcode loader is currently the primary user for this. But
other architectures are not expected to use it. Add a build option to
make it opt-in.

Originally-by: David Kaplan <david.kaplan@amd.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Suggested-by: Thomas Gleixner <tglx@kernel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Link: https://lore.kernel.org/lkml/20260129121729.GRaXtP2aeWkQKegxC2@fat_crate.local
Link: https://lore.kernel.org/lkml/87wm0zl8p2.ffs@tglx
---
V2 -> V3: Address Sashiko review
* Avoid instrumentable functions:
  smp_processor_id() -> raw_smp_processor_id()
  cpumask_test_and_clear_cpu() -> arch_test_and_clear_bit()
* Remove `done` tag to avoid false setting. Instead, data == NULL as indicator
---
 arch/Kconfig                 |  3 ++
 include/linux/stop_machine.h | 16 ++++++
 kernel/stop_machine.c        | 99 +++++++++++++++++++++++++++++++++++-
 3 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 45c657772362..afa564261e1c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1878,4 +1878,7 @@ config ARCH_HAS_CPU_ATTACK_VECTORS
 config HAVE_ARCH_GET_SECUREBOOT
 	def_bool EFI
 
+config STOP_MACHINE_NMI
+	bool
+
 endmenu
diff --git a/include/linux/stop_machine.h b/include/linux/stop_machine.h
index 84e7fb627ba4..b53b028296e0 100644
--- a/include/linux/stop_machine.h
+++ b/include/linux/stop_machine.h
@@ -20,6 +20,12 @@
  */
 typedef int (*cpu_stop_fn_t)(void *arg);
 
+/*
+ * Stop function variant runnable from NMI context. This makes the
+ * noinstr requirement explicit at the type level.
+ */
+typedef int (*cpu_stop_nmisafe_fn_t)(void *arg);
+
 #ifdef CONFIG_SMP
 
 struct cpu_stop_work {
@@ -186,4 +192,14 @@ stop_machine_from_inactive_cpu(cpu_stop_fn_t fn, void *data,
 }
 
 #endif	/* CONFIG_SMP || CONFIG_HOTPLUG_CPU */
+
+#ifdef CONFIG_STOP_MACHINE_NMI
+
+void arch_send_self_nmi(void);
+bool noinstr stop_machine_nmi_handler(void);
+
+#else
+static inline bool stop_machine_nmi_handler(void) { return false; }
+#endif /* CONFIG_STOP_MACHINE_NMI */
+
 #endif	/* _LINUX_STOP_MACHINE */
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 8dcd5be67b85..ad01b7c5266c 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -23,6 +23,7 @@
 #include <linux/atomic.h>
 #include <linux/nmi.h>
 #include <linux/sched/wake_q.h>
+#include <linux/delay.h>
 
 /*
  * Structure to determine completion condition and record errors.  May
@@ -175,6 +176,15 @@ struct multi_stop_data {
 
 	enum multi_stop_state	state;
 	atomic_t		thread_ack;
+
+#ifdef CONFIG_STOP_MACHINE_NMI
+	/* Used in the NMI stop_machine variant */
+	bool			use_nmi;
+	/* A separate function type to highlight noinstr requirement */
+	cpu_stop_nmisafe_fn_t	nmisafe_fn;
+	/* cpumask of CPUs on which to raise an NMI */
+	cpumask_var_t		nmi_cpus;
+#endif
 };
 
 static void set_state(struct multi_stop_data *msdata,
@@ -198,6 +208,8 @@ notrace void __weak stop_machine_yield(const struct cpumask *cpumask)
 	cpu_relax();
 }
 
+static int multi_stop_run(struct multi_stop_data *msdata);
+
 /* This is the cpu_stop function which stops the CPU. */
 static int multi_cpu_stop(void *data)
 {
@@ -236,7 +248,7 @@ static int multi_cpu_stop(void *data)
 				break;
 			case MULTI_STOP_RUN:
 				if (is_active)
-					err = msdata->fn(msdata->data);
+					err = multi_stop_run(msdata);
 				break;
 			default:
 				break;
@@ -713,3 +725,88 @@ int stop_machine_from_inactive_cpu(cpu_stop_fn_t fn, void *data,
 	mutex_unlock(&stop_cpus_mutex);
 	return ret ?: done.ret;
 }
+
+#ifdef CONFIG_STOP_MACHINE_NMI
+
+struct nmi_stop {
+	struct multi_stop_data	*data;
+	int			ret;
+};
+
+static DEFINE_PER_CPU(struct nmi_stop, nmi_stop);
+
+/*
+ * Instrumentation may trigger nested exceptions such as #INT3, #DB,
+ * or #PF. IRET from those would re-enable NMIs, which opposes the goal
+ * of this NMI stop-machine facility.
+ */
+bool noinstr stop_machine_nmi_handler(void)
+{
+	struct multi_stop_data *msdata = raw_cpu_read(nmi_stop.data);
+	unsigned int cpu = raw_smp_processor_id();
+	int ret;
+
+	if (!msdata || !arch_test_and_clear_bit(cpu, cpumask_bits(msdata->nmi_cpus)))
+		return false;
+
+	/*
+	 * The indirect call to @nmisafe_fn() is indistinguishable at
+	 * post-compilation. Temporarily enabling instrumentation avoids
+	 * objtool false positives.
+	 */
+	instrumentation_begin();
+	ret = msdata->nmisafe_fn(msdata->data);
+	instrumentation_end();
+
+	raw_cpu_write(nmi_stop.ret,  ret);
+	raw_cpu_write(nmi_stop.data, NULL);
+
+	return true;
+}
+
+static bool wait_for_nmi_handler(void)
+{
+	/* Conservative timeout */
+	unsigned long timeout = USEC_PER_SEC;
+
+	/* The handler clears up at the end */
+	while (this_cpu_read(nmi_stop.data) && timeout--)
+		udelay(1);
+
+	return !this_cpu_read(nmi_stop.data);
+}
+
+static int nmi_stop_run(struct multi_stop_data *msdata)
+{
+	/*
+	 * Save per-CPU state accessible from NMI context and raise a
+	 * self-NMI to execute the stop function from the NMI handler
+	 */
+	this_cpu_write(nmi_stop.data, msdata);
+	arch_send_self_nmi();
+
+	/*
+	 * Ensure the handler went through before reading the result.
+	 * Otherwise, make no stale state left behind.
+	 */
+	if (!wait_for_nmi_handler()) {
+		this_cpu_write(nmi_stop.data, NULL);
+		return -ETIMEDOUT;
+	}
+
+	return this_cpu_read(nmi_stop.ret);
+}
+
+static int multi_stop_run(struct multi_stop_data *msdata)
+{
+	return msdata->use_nmi ? nmi_stop_run(msdata) : msdata->fn(msdata->data);
+}
+
+#else
+
+static int multi_stop_run(struct multi_stop_data *msdata)
+{
+	return msdata->fn(msdata->data);
+}
+
+#endif /* CONFIG_STOP_MACHINE_NMI */
-- 
2.53.0

[PATCH v3 3/9] stop_machine: Introduce stop_machine_nmi_cpuslocked()

[Chang S. Bae]

With the NMI control logic in place, introduce an API to run the target
function from NMI context.

Originally-by: David Kaplan <david.kaplan@amd.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Link: https://lore.kernel.org/lkml/20260202105411.GVaYCCUygtEUNrMUtG@fat_crate.local
---
V2 -> V3: Address Sashiko review
* Ensure runtime only via `stop_machine_initialized`
* Ensure target CPUs online
* Add a fallback prototype
* Adjust function description (to align with mainline changes)
---
 include/linux/stop_machine.h | 27 +++++++++++++++++++++
 kernel/stop_machine.c        | 46 ++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/include/linux/stop_machine.h b/include/linux/stop_machine.h
index b53b028296e0..b05e65f4bb6d 100644
--- a/include/linux/stop_machine.h
+++ b/include/linux/stop_machine.h
@@ -198,8 +198,35 @@ stop_machine_from_inactive_cpu(cpu_stop_fn_t fn, void *data,
 void arch_send_self_nmi(void);
 bool noinstr stop_machine_nmi_handler(void);
 
+/**
+ * stop_machine_nmi_cpuslocked() - Freeze CPUs and run a function in NMI context
+ *
+ * @nmisafe_fn:	The function to run
+ * @data:	The data pointer for @nmisafe_fn()
+ * @cpus:	The CPUs to run @nmisafe_fn() on (NULL = one unspecified online CPU)
+ *
+ * Description: This stop_machine() variant runs @nmisafe_fn() from NMI context
+ * to prevent preemption by other NMIs. The callback must be built with noinstr.
+ * Other than that, the semantics match stop_machine_cpuslocked().
+ *
+ * Context: Must be called from within a cpus_read_lock() protected region.
+ * Avoid nested calls to cpus_read_lock().
+ *
+ * Return: 0 if all invocations of @nmisafe_fn return zero, -ENOMEM if cpumask
+ * allocation fails, -EINVAL if not initialized yet or any target CPU failed to
+ * receive NMI. Otherwise, the value returned by an arbitrarily chosen member of
+ * the set of calls to @nmisafe_fn that returned non-zero.
+ */
+int stop_machine_nmi_cpuslocked(cpu_stop_nmisafe_fn_t nmisafe_fn, void *data,
+				const struct cpumask *cpus);
+
 #else
 static inline bool stop_machine_nmi_handler(void) { return false; }
+static inline int
+stop_machine_nmi_cpulocked(cpu_stop_nmisafe_fn_t nmisafe_fn, void *data, conststruct cpumask *cpus)
+{
+	return 0;
+}
 #endif /* CONFIG_STOP_MACHINE_NMI */
 
 #endif	/* _LINUX_STOP_MACHINE */
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index ad01b7c5266c..2f3fe13cd3aa 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -802,6 +802,52 @@ static int multi_stop_run(struct multi_stop_data *msdata)
 	return msdata->use_nmi ? nmi_stop_run(msdata) : msdata->fn(msdata->data);
 }
 
+int stop_machine_nmi_cpuslocked(cpu_stop_nmisafe_fn_t nmisafe_fn, void *data,
+				const struct cpumask *cpus)
+{
+	struct multi_stop_data msdata = {
+		.nmisafe_fn	= nmisafe_fn,
+		.data		= data,
+		.num_threads	= num_online_cpus(),
+		.active_cpus	= cpus,
+		.use_nmi	= true,
+	};
+	int ret;
+
+	if (!stop_machine_initialized)
+		return -EINVAL;
+
+	if (!zalloc_cpumask_var(&msdata.nmi_cpus, GFP_KERNEL))
+		return -ENOMEM;
+
+	/*
+	 * NMI CPUs should be exactly those 'active' CPUs executing the
+	 * stop function. Follow the selection logic in multi_cpu_stop()
+	 * if not provided.
+	 */
+	if (!msdata.active_cpus)
+		cpumask_set_cpu(cpumask_first(cpu_online_mask), msdata.nmi_cpus);
+	else
+		cpumask_and(msdata.nmi_cpus, msdata.active_cpus, cpu_online_mask);
+
+	lockdep_assert_cpus_held();
+
+	ret = stop_multi_cpus(&msdata);
+
+	/*
+	 * The NMI handler clears each CPU bit. If any of those NMIs were
+	 * ever missed out, return error clearly.
+	 */
+	if (!cpumask_empty(msdata.nmi_cpus)) {
+		pr_err("CPUs %*pbl didn't run the stop_machine NMI handler.\n",
+		       cpumask_pr_args(msdata.nmi_cpus));
+		ret = -EINVAL;
+	}
+
+	free_cpumask_var(msdata.nmi_cpus);
+	return ret;
+}
+
 #else
 
 static int multi_stop_run(struct multi_stop_data *msdata)
-- 
2.53.0

[PATCH v3 4/9] x86/apic: Implement self-NMI support

[Chang S. Bae]

From: David Kaplan <david.kaplan@amd.com>

Add a function to send an NMI-to-self which is needed for stop-machine
NMI facility. Note that send_IPI_self() cannot be used to send an NMI so
send_IPI() is used.

Signed-off-by: David Kaplan <david.kaplan@amd.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
V2 -> V3: Use __apic_send_IPI() (Sashiko)
---
 arch/x86/kernel/apic/ipi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/apic/ipi.c b/arch/x86/kernel/apic/ipi.c
index c627bee3b14f..2bb70db5ee27 100644
--- a/arch/x86/kernel/apic/ipi.c
+++ b/arch/x86/kernel/apic/ipi.c
@@ -4,6 +4,7 @@
 #include <linux/delay.h>
 #include <linux/smp.h>
 #include <linux/string_choices.h>
+#include <linux/stop_machine.h>
 
 #include <asm/io_apic.h>
 
@@ -248,6 +249,15 @@ void default_send_IPI_self(int vector)
 	__default_send_IPI_shortcut(APIC_DEST_SELF, vector);
 }
 
+#ifdef CONFIG_STOP_MACHINE_NMI
+
+void arch_send_self_nmi(void)
+{
+	__apic_send_IPI(smp_processor_id(), NMI_VECTOR);
+}
+
+#endif
+
 #ifdef CONFIG_X86_32
 void default_send_IPI_mask_sequence_logical(const struct cpumask *mask, int vector)
 {
-- 
2.53.0

[PATCH v3 5/9] x86/nmi: Support NMI stop-machine handler

[Chang S. Bae]

From: David Kaplan <david.kaplan@amd.com>

Call the stop-machine handler from the NMI path in order to support the
NMI stop-machine.

Signed-off-by: David Kaplan <david.kaplan@amd.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
Sashiko raises a concern about coalescing edge-triggered events, such as
PMU overflows. But, not sure whether a practical issue or not especially
with this stop-machine use.

The current use of this facility has been to the microcode loader. Then
the mentioned case sounds like somewhat careless setup. Thus, the concern
looks theoretical, leaving the handling as-is.
---
 arch/x86/kernel/nmi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 3c9f60d6ca5a..26f5ca1ac8a5 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -24,6 +24,7 @@
 #include <linux/export.h>
 #include <linux/atomic.h>
 #include <linux/sched/clock.h>
+#include <linux/stop_machine.h>
 #include <linux/kvm_types.h>
 
 #include <asm/cpu_entry_area.h>
@@ -382,6 +383,9 @@ static noinstr void default_do_nmi(struct pt_regs *regs)
 
 	instrumentation_begin();
 
+	if (stop_machine_nmi_handler())
+		goto out;
+
 	if (microcode_nmi_handler_enabled() && microcode_nmi_handler())
 		goto out;
 
-- 
2.53.0

[PATCH v3 6/9] x86/microcode: Distinguish NMI control path on stop-machine callback

[Chang S. Bae]

load_cpus_stopped() currently centralizes the stop_machine() callback for
both NMI and NMI-less rendezvous. microcode_update_handler() alone is
enough for the latter.

While the NMI-based rendezvous finally reaches the same update handler,
it requires additional logic to trigger and process NMIs. That machinery
will be replaced by stop-machine facility.

As preparation for that conversion, split the callback path to make
NMI-specific steps explicit and clear. Rename the function to align with
the change.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 arch/x86/kernel/cpu/microcode/core.c | 30 ++++++++++++----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 0dd0c7241c57..9c006cc90047 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -531,7 +531,7 @@ void noinstr microcode_offline_nmi_handler(void)
 	wait_for_ctrl();
 }
 
-static noinstr bool microcode_update_handler(void)
+static noinstr int microcode_update_handler(void *unused)
 {
 	unsigned int cpu = raw_smp_processor_id();
 
@@ -547,7 +547,7 @@ static noinstr bool microcode_update_handler(void)
 	touch_nmi_watchdog();
 	instrumentation_end();
 
-	return true;
+	return 0;
 }
 
 /*
@@ -568,19 +568,15 @@ bool noinstr microcode_nmi_handler(void)
 		return false;
 
 	raw_cpu_write(ucode_ctrl.nmi_enabled, false);
-	return microcode_update_handler();
+	return microcode_update_handler(NULL) == 0;
 }
 
-static int load_cpus_stopped(void *unused)
+static int stop_cpu_in_nmi(void *unused)
 {
-	if (microcode_ops->use_nmi) {
-		/* Enable the NMI handler and raise NMI */
-		this_cpu_write(ucode_ctrl.nmi_enabled, true);
-		apic->send_IPI(smp_processor_id(), NMI_VECTOR);
-	} else {
-		/* Just invoke the handler directly */
-		microcode_update_handler();
-	}
+	/* Enable the NMI handler and raise NMI */
+	this_cpu_write(ucode_ctrl.nmi_enabled, true);
+	apic->send_IPI(smp_processor_id(), NMI_VECTOR);
+
 	return 0;
 }
 
@@ -617,13 +613,13 @@ static int load_late_stop_cpus(bool is_safe)
 	 */
 	store_cpu_caps(&prev_info);
 
-	if (microcode_ops->use_nmi)
+	if (microcode_ops->use_nmi) {
 		static_branch_enable_cpuslocked(&microcode_nmi_handler_enable);
-
-	stop_machine_cpuslocked(load_cpus_stopped, NULL, cpu_online_mask);
-
-	if (microcode_ops->use_nmi)
+		stop_machine_cpuslocked(stop_cpu_in_nmi, NULL, cpu_online_mask);
 		static_branch_disable_cpuslocked(&microcode_nmi_handler_enable);
+	} else {
+		stop_machine_cpuslocked(microcode_update_handler, NULL, cpu_online_mask);
+	}
 
 	/* Analyze the results */
 	for_each_cpu_and(cpu, cpu_present_mask, &cpus_booted_once_mask) {
-- 
2.53.0

[PATCH v3 7/9] x86/microcode: Use stop-machine NMI facility

[Chang S. Bae]

The existing NMI-based loading logic explicitly sends NMIs to online CPUs
and invokes microcode_update_handler() from the NMI context. The
stop-machine NMI variant already provides the mechanism on x86.

Replace the custom NMI control logic with stop_machine_nmi_cpuslocked().

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 arch/x86/Kconfig                     |  1 +
 arch/x86/include/asm/microcode.h     |  1 -
 arch/x86/kernel/cpu/microcode/core.c | 19 +++----------------
 arch/x86/kernel/nmi.c                |  3 ---
 4 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 15fd9ec5ecac..c0d8e172d708 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1328,6 +1328,7 @@ config MICROCODE_LATE_LOADING
 	bool "Late microcode loading (DANGEROUS)"
 	default n
 	depends on MICROCODE && SMP
+	select STOP_MACHINE_NMI
 	help
 	  Loading microcode late, when the system is up and executing instructions
 	  is a tricky business and should be avoided if possible. Just the sequence
diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 645e65ac1586..e6c09dc48efb 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -78,7 +78,6 @@ static inline u32 intel_get_microcode_revision(void)
 }
 #endif /* !CONFIG_CPU_SUP_INTEL */
 
-bool microcode_nmi_handler(void);
 void microcode_offline_nmi_handler(void);
 
 #ifdef CONFIG_MICROCODE_LATE_LOADING
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 9c006cc90047..73a46c6c74de 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -562,22 +562,9 @@ static noinstr int microcode_update_handler(void *unused)
  * path which must be NMI safe until the primary thread completed the
  * update.
  */
-bool noinstr microcode_nmi_handler(void)
+static noinstr int microcode_nmi_handler(void *data)
 {
-	if (!raw_cpu_read(ucode_ctrl.nmi_enabled))
-		return false;
-
-	raw_cpu_write(ucode_ctrl.nmi_enabled, false);
-	return microcode_update_handler(NULL) == 0;
-}
-
-static int stop_cpu_in_nmi(void *unused)
-{
-	/* Enable the NMI handler and raise NMI */
-	this_cpu_write(ucode_ctrl.nmi_enabled, true);
-	apic->send_IPI(smp_processor_id(), NMI_VECTOR);
-
-	return 0;
+	return microcode_update_handler(data);
 }
 
 static int load_late_stop_cpus(bool is_safe)
@@ -615,7 +602,7 @@ static int load_late_stop_cpus(bool is_safe)
 
 	if (microcode_ops->use_nmi) {
 		static_branch_enable_cpuslocked(&microcode_nmi_handler_enable);
-		stop_machine_cpuslocked(stop_cpu_in_nmi, NULL, cpu_online_mask);
+		stop_machine_nmi_cpuslocked(microcode_nmi_handler, NULL, cpu_online_mask);
 		static_branch_disable_cpuslocked(&microcode_nmi_handler_enable);
 	} else {
 		stop_machine_cpuslocked(microcode_update_handler, NULL, cpu_online_mask);
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 26f5ca1ac8a5..066ac297873e 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -386,9 +386,6 @@ static noinstr void default_do_nmi(struct pt_regs *regs)
 	if (stop_machine_nmi_handler())
 		goto out;
 
-	if (microcode_nmi_handler_enabled() && microcode_nmi_handler())
-		goto out;
-
 	/*
 	 * CPU-specific NMI must be processed before non-CPU-specific
 	 * NMI, otherwise we may lose it, because the CPU-specific
-- 
2.53.0

[PATCH v3 8/9] x86/nmi: Simplify offline microcode handler invocation

[Chang S. Bae]

The microcode loader now uses the stop-machine facility for NMI-based
loading. The offline handler is the only remaining user of the microcode
NMI static key.

But the handler already has internal guarding. Offline CPU doesn't matter
how fast it handles anyway, so remove the static key. Instead, check the
build option.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 arch/x86/kernel/nmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 066ac297873e..0227387fff54 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -548,7 +548,7 @@ DEFINE_IDTENTRY_RAW(exc_nmi)
 		raw_atomic_long_inc(&nsp->idt_calls);
 
 	if (arch_cpu_is_offline(smp_processor_id())) {
-		if (microcode_nmi_handler_enabled())
+		if (IS_ENABLED(CONFIG_MICROCODE_LATE_LOADING))
 			microcode_offline_nmi_handler();
 		return;
 	}
@@ -710,7 +710,7 @@ DEFINE_FREDENTRY_NMI(exc_nmi)
 	irqentry_state_t irq_state;
 
 	if (arch_cpu_is_offline(smp_processor_id())) {
-		if (microcode_nmi_handler_enabled())
+		if (IS_ENABLED(CONFIG_MICROCODE_LATE_LOADING))
 			microcode_offline_nmi_handler();
 		return;
 	}
-- 
2.53.0

[PATCH v3 9/9] x86/microcode: Remove microcode_nmi_handler_enable

[Chang S. Bae]

Remove it, as there is no more user.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 arch/x86/include/asm/microcode.h     | 10 ----------
 arch/x86/kernel/cpu/microcode/core.c |  8 ++------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index e6c09dc48efb..cb5c8e71fad7 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -80,14 +80,4 @@ static inline u32 intel_get_microcode_revision(void)
 
 void microcode_offline_nmi_handler(void);
 
-#ifdef CONFIG_MICROCODE_LATE_LOADING
-DECLARE_STATIC_KEY_FALSE(microcode_nmi_handler_enable);
-static __always_inline bool microcode_nmi_handler_enabled(void)
-{
-	return static_branch_unlikely(&microcode_nmi_handler_enable);
-}
-#else
-static __always_inline bool microcode_nmi_handler_enabled(void) { return false; }
-#endif
-
 #endif /* _ASM_X86_MICROCODE_H */
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 73a46c6c74de..330c62f6dd32 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -323,7 +323,6 @@ struct microcode_ctrl {
 	bool			nmi_enabled;
 };
 
-DEFINE_STATIC_KEY_FALSE(microcode_nmi_handler_enable);
 static DEFINE_PER_CPU(struct microcode_ctrl, ucode_ctrl);
 static atomic_t late_cpus_in, offline_in_nmi;
 static unsigned int loops_per_usec;
@@ -600,13 +599,10 @@ static int load_late_stop_cpus(bool is_safe)
 	 */
 	store_cpu_caps(&prev_info);
 
-	if (microcode_ops->use_nmi) {
-		static_branch_enable_cpuslocked(&microcode_nmi_handler_enable);
+	if (microcode_ops->use_nmi)
 		stop_machine_nmi_cpuslocked(microcode_nmi_handler, NULL, cpu_online_mask);
-		static_branch_disable_cpuslocked(&microcode_nmi_handler_enable);
-	} else {
+	else
 		stop_machine_cpuslocked(microcode_update_handler, NULL, cpu_online_mask);
-	}
 
 	/* Analyze the results */
 	for_each_cpu_and(cpu, cpu_present_mask, &cpus_booted_once_mask) {
-- 
2.53.0

Re: [PATCH v3 3/9] stop_machine: Introduce stop_machine_nmi_cpuslocked()

[kernel test robot]

Hi Chang,

kernel test robot noticed the following build errors:

[auto build test ERROR on 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907]

url:    https://github.com/intel-lab-lkp/linux/commits/Chang-S-Bae/stop_machine-Refactor-multi-CPU-stop-glue-code/20261015-211418
base:   24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
patch link:    https://lore.kernel.org/r/20261015211426.49114-4-chang.seok.bae%40intel.com
patch subject: [PATCH v3 3/9] stop_machine: Introduce stop_machine_nmi_cpuslocked()
config: i386-randconfig-1013-20261016 (https://download.01.org/0day-ci/archive/20261016/202610161559.cSgd4JA0-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.4.0-5) 12.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261016/202610161559.cSgd4JA0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610161559.cSgd4JA0-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/sched/sched.h:63,
                    from kernel/sched/rq-offsets.c:5:
>> include/linux/stop_machine.h:226:74: error: unknown type name 'conststruct'; did you mean 'const_true'?
     226 | stop_machine_nmi_cpulocked(cpu_stop_nmisafe_fn_t nmisafe_fn, void *data, conststruct cpumask *cpus)
         |                                                                          ^~~~~~~~~~~
         |                                                                          const_true
   make[3]: *** [scripts/Makefile.build:185: kernel/sched/rq-offsets.s] Error 1 shuffle=3873254352
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1425: prepare0] Error 2 shuffle=3873254352
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2 shuffle=3873254352
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2 shuffle=3873254352
   make: Target 'prepare' not remade because of errors.


vim +226 include/linux/stop_machine.h

   200	
   201	/**
   202	 * stop_machine_nmi_cpuslocked() - Freeze CPUs and run a function in NMI context
   203	 *
   204	 * @nmisafe_fn:	The function to run
   205	 * @data:	The data pointer for @nmisafe_fn()
   206	 * @cpus:	The CPUs to run @nmisafe_fn() on (NULL = one unspecified online CPU)
   207	 *
   208	 * Description: This stop_machine() variant runs @nmisafe_fn() from NMI context
   209	 * to prevent preemption by other NMIs. The callback must be built with noinstr.
   210	 * Other than that, the semantics match stop_machine_cpuslocked().
   211	 *
   212	 * Context: Must be called from within a cpus_read_lock() protected region.
   213	 * Avoid nested calls to cpus_read_lock().
   214	 *
   215	 * Return: 0 if all invocations of @nmisafe_fn return zero, -ENOMEM if cpumask
   216	 * allocation fails, -EINVAL if not initialized yet or any target CPU failed to
   217	 * receive NMI. Otherwise, the value returned by an arbitrarily chosen member of
   218	 * the set of calls to @nmisafe_fn that returned non-zero.
   219	 */
   220	int stop_machine_nmi_cpuslocked(cpu_stop_nmisafe_fn_t nmisafe_fn, void *data,
   221					const struct cpumask *cpus);
   222	
   223	#else
   224	static inline bool stop_machine_nmi_handler(void) { return false; }
   225	static inline int
 > 226	stop_machine_nmi_cpulocked(cpu_stop_nmisafe_fn_t nmisafe_fn, void *data, conststruct cpumask *cpus)
   227	{
   228		return 0;
   229	}
   230	#endif /* CONFIG_STOP_MACHINE_NMI */
   231	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki