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

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

[Chang S. Bae]

Hi all,

This revision has a couple of minor changes on top of 7.4-rc1:

 * Remove the fallback prototype (patch3)
 * Add a comment at the stop-machine's NMI handler call site (patch5)

V3 previously included several changes addressing Sashiko's feedback. For
the background and overall design, please refer to the more detailed V2
cover [*].

I'll first wait to see whether there is any further feedback from
Sashiko. Otherwise, I think this series is getting close to being ready.

The series is also available in this repository:
 git://github.com/intel-staging/microcode.git nmi-stop-machine_v4

Thanks,
Chang

[*] https://lore.kernel.org/20260331014251.86353-12-chang.seok.bae@intel.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                |  12 +-
 include/linux/stop_machine.h         |  38 ++++++
 kernel/stop_machine.c                | 180 ++++++++++++++++++++++++---
 8 files changed, 232 insertions(+), 58 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.53.0

[PATCH v4 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 v4 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
---
 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 1163736de1f9..a4b62f83efb6 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1824,4 +1824,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 v4 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
---
V3 -> V4: Remove the fallback prototype

  Tracking back the previous feedback on V1,
	  https://lore.kernel.org/lkml/87wm0zl8p2.ffs@tglx
  tglx gave this comment "That function should not be exposed at all when
  the config switch is off."

  So, the x86 microcode loader is the only user and the callsite should be
  compiled out when CONFIG_STOP_MACHINE_NMI=n.
---
 include/linux/stop_machine.h | 22 +++++++++++++++++
 kernel/stop_machine.c        | 46 ++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/include/linux/stop_machine.h b/include/linux/stop_machine.h
index b53b028296e0..af3218987f53 100644
--- a/include/linux/stop_machine.h
+++ b/include/linux/stop_machine.h
@@ -198,6 +198,28 @@ 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; }
 #endif /* CONFIG_STOP_MACHINE_NMI */
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 v4 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 __apic_send_IPI_self() cannot be used to send an
NMI so __apic_send_IPI() is used.

Signed-off-by: David Kaplan <david.kaplan@amd.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 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 v4 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>
---
V3 -> V4: Leave a note against coalesced NMI events in theory (Sashiko).
---
 arch/x86/kernel/nmi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 3c9f60d6ca5a..0d426e5e74ee 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,14 @@ static noinstr void default_do_nmi(struct pt_regs *regs)
 
 	instrumentation_begin();
 
+	/*
+	 * When an NMI stop-machine runs, the rest of the system should be idle,
+	 * so handle the stop-machine NMI first even if other NMIs might get
+	 * coalesced with it.
+	 */
+	if (stop_machine_nmi_handler())
+		goto out;
+
 	if (microcode_nmi_handler_enabled() && microcode_nmi_handler())
 		goto out;
 
-- 
2.53.0

[PATCH v4 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.

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 v4 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 58dce0b66179..f4bf1fdae6ba 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1325,6 +1325,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 0d426e5e74ee..e3660b8b8f38 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -391,9 +391,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 v4 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.

The handler already checks whether it is enabled, and how fast an offline
CPU handles the NMI does not matter. Instead of the static key, 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 e3660b8b8f38..11c965445bdc 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -553,7 +553,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;
 	}
@@ -715,7 +715,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 v4 9/9] x86/microcode: Remove microcode_nmi_handler_enable

[Chang S. Bae]

Remove the static key, 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 v4 8/9] x86/nmi: Simplify offline microcode handler invocation

[Chang S. Bae]

On 11/4/2026 10:23 AM, Chang S. Bae wrote:
> 
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index e3660b8b8f38..11c965445bdc 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -553,7 +553,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;
>   	}
> @@ -715,7 +715,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;
>   	}

There is a Sashiko comment on this:
https://sashiko.dev/#/patchset/20261104182322.578066-1-chang.seok.bae%40intel.com

I thought waking up soft-offlined SMT siblings with an NMI was benign. 
Right now the path is not explicitly gated by the use_nmi flag even 
though the underlying mechanism is NMI-based.

So perhaps it's better to have this reassurance ahead:

diff --git a/arch/x86/kernel/cpu/microcode/core.c 
b/arch/x86/kernel/cpu/microcode/core.c
index 330c62f6dd32..cf3c02622dc1 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -502,8 +502,11 @@ static void load_primary(unsigned int cpu)
         unsigned int nr_offl = cpumask_weight(&cpu_offline_mask);
         bool proceed = true;

-       /* Kick soft-offlined SMT siblings if required */
-       if (!cpu && nr_offl)
+       /*
+        * Kick soft-offlined SMT siblings if required.
+        * Note the mechanism is NMI base.
+        */
+       if (microcode_ops->use_nmi && !cpu && nr_offl)
                 proceed = kick_offline_cpus(nr_offl);

         /* If the soft-offlined CPUs did not respond, abort */
@@ -511,7 +514,7 @@ static void load_primary(unsigned int cpu)
                 __load_primary(cpu);

         /* Unconditionally release soft-offlined SMT siblings if 
required */
-       if (!cpu && nr_offl)
+       if (microcode_ops->use_nmi && !cpu && nr_offl)
                 release_offline_cpus();
  }

Thanks,
Chang

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

[Borislav Petkov]

On Wed, Nov 04, 2026 at 06:23:14PM +0000, Chang S. Bae wrote:
> 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)

Waay too many combinations of "stop", "multi" and "cpus" in this file, with
different ordering, causing me a headache.

Call that function __stop_machine() instead as it looks like it is going to be
the naked, no-locking, version which is internal only.

-- 
Regards/Gruss,
    Boris.

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

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

[Chang S. Bae]

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

On 11/4/26 13:56, Chang S. Bae wrote:
> On 11/4/2026 10:23 AM, Chang S. Bae wrote:
>>
>> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
>> index e3660b8b8f38..11c965445bdc 100644
>> --- a/arch/x86/kernel/nmi.c
>> +++ b/arch/x86/kernel/nmi.c
>> @@ -553,7 +553,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;
>>       }
>> @@ -715,7 +715,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;
>>       }
> 
> There is a Sashiko comment on this:
> https://sashiko.dev/#/patchset/20261104182322.578066-1- 
> chang.seok.bae%40intel.com
> 
> I thought waking up soft-offlined SMT siblings with an NMI was benign. 
> Right now the path is not explicitly gated by the use_nmi flag even 
> though the underlying mechanism is NMI-based.

As looking more, this looks like a bug to me. NMIs are raised to those 
CPUs, but not handled because the static key is off. So I ended up with 
this patch:


[-- Attachment #2: 0001-x86-microcode-Gate-soft-offlined-SMT-siblings-handli.patch --]
[-- Type: text/x-patch, Size: 2508 bytes --]

From 717f5931a0579bac239d80913b3aa4ef11e5dc7a Mon Sep 17 00:00:00 2001
From: "Chang S. Bae" <chang.seok.bae@intel.com>
Date: Fri, 6 Nov 2026 17:27:35 -0800
Subject: [PATCH] x86/microcode: Gate soft-offlined SMT siblings handling

Handling of soft-offlined SMT siblings relies on NMI events, but is
currently also executed even if use_nmi is false.

This causes a practical problem on !use_nmi systems. When SMT siblings
are soft-offlined, kick_offline_cpus() sends NMIs to them, but the
offline NMI handler is already switched off. Thus, those siblings appear
not responding, the rendezvous goes timeout, then finally the load fails.

Gate the wake-up and release paths on use_nmi. Otherwise, keep the
soft-offlined siblings in the offline state instead.

Fixes: 8f849ff63bcb ("x86/microcode: Handle "offline" CPUs correctly")
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: stable@vger.kernel.org
---
Now I ended up sorting out this could be a real issue on AMD systems. But
I appreciate if someone likely from AMD could test and confirm this fix
before moving on.
---
 arch/x86/kernel/cpu/microcode/core.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 0dd0c7241c57..f1bd399f5ebf 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -498,21 +498,36 @@ static void release_offline_cpus(void)
 		per_cpu(ucode_ctrl.ctrl, cpu) = SCTRL_DONE;
 }
 
+static void keep_offline_cpus(void)
+{
+	unsigned int cpu;
+
+	for_each_cpu(cpu, &cpu_offline_mask)
+		per_cpu(ucode_ctrl.result, cpu) = UCODE_OFFLINE;
+}
+
 static void load_primary(unsigned int cpu)
 {
 	unsigned int nr_offl = cpumask_weight(&cpu_offline_mask);
 	bool proceed = true;
 
-	/* Kick soft-offlined SMT siblings if required */
-	if (!cpu && nr_offl)
-		proceed = kick_offline_cpus(nr_offl);
+	/*
+	 * Kick soft-offlined SMT siblings via NMI events, when required and
+	 * NMI use is configured. Otherwise, keep them in the offline state.
+	 */
+	if (!cpu && nr_offl) {
+		if (microcode_ops->use_nmi)
+			proceed = kick_offline_cpus(nr_offl);
+		else
+			keep_offline_cpus();
+	}
 
 	/* If the soft-offlined CPUs did not respond, abort */
 	if (proceed)
 		__load_primary(cpu);
 
 	/* Unconditionally release soft-offlined SMT siblings if required */
-	if (!cpu && nr_offl)
+	if (microcode_ops->use_nmi && !cpu && nr_offl)
 		release_offline_cpus();
 }
 
-- 
2.53.0

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

[Borislav Petkov]

On Wed, Nov 04, 2026 at 06:23:15PM +0000, Chang S. Bae wrote:
> 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.

If that were the case then we should not use stop_machine(). But
stop_machine() prevents NMIs indirectly by simply not running anything else on
the CPUs, thus leading to no NMIs. Well, mostly. We can't do anything about
firmware gunk and so on.

David's original justifcation makes a lot more sense to me:

https://lore.kernel.org/r/20260125014224.249901-2-chang.seok.bae@intel.com

> 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.

This does not belong here.

> Add a build option to make it opt-in.

Also drop all text that explains the patch. Audit your whole set pls.

> 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
> ---
>  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 1163736de1f9..a4b62f83efb6 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1824,4 +1824,7 @@ config ARCH_HAS_CPU_ATTACK_VECTORS
>  config HAVE_ARCH_GET_SECUREBOOT
>  	def_bool EFI
>  
> +config STOP_MACHINE_NMI
> +	bool

What is the real point for that ugly ifdeffery introducing, yet another
Kconfig option in the zoo?

>  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.
> + */

I don't see what the point of that comment is.

> +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

I hate this ifdeffery.

> +	/* Used in the NMI stop_machine variant */
> +	bool			use_nmi;
> +	/* A separate function type to highlight noinstr requirement */

What?

This is the function that stop_machine_nmi calls. Pls keep commenting to the
point and at minimum.

> +	cpu_stop_nmisafe_fn_t	nmisafe_fn;
> +	/* cpumask of CPUs on which to raise an NMI */

Kinda obvious from the name, right?

> +	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);

Get rid of this.

> +
>  /* 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 {

If you already have "multi_stop_data" then this is "nmi_stop_data".

Please try not to come up with weird naming willy nilly.

> +	struct multi_stop_data	*data;
> +	int			ret;
> +};
> +
> +static DEFINE_PER_CPU(struct nmi_stop, nmi_stop);

static DEFINE_PER_CPU(struct nmi_stop_data, nmi_data);

> +
> +/*
> + * 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)

stop_machine_nmi()

> +{
> +	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();

This is just silly. We should fix objtool here.

Peter?

> +	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)

Call it what it is:

	stop_machine_callback()

or

	stop_machine_fn()

or so. Naming is very important.

> +{
> +	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
> 

-- 
Regards/Gruss,
    Boris.

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

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

[Chang S. Bae]

On 11/9/2026 6:46 PM, Borislav Petkov wrote:
> On Wed, Nov 04, 2026 at 06:23:15PM +0000, Chang S. Bae wrote:
>> +/*
>> + * 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)
> 
> stop_machine_nmi()
> 
>> +{
>> +	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();
> 
> This is just silly. We should fix objtool here.

Here is your previous thought:

   This one would be hard to fix because that's the

	err = msdata->fn(msdata->data);

   function pointer which objtool can't see through.

https://lore.kernel.org/lkml/20260127144947.GKaXjQi8R9cE17WYKM@fat_crate.local/

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

[Borislav Petkov]

On Mon, Nov 09, 2026 at 06:58:03PM -0800, Chang S. Bae wrote:
> Here is your previous thought:
> 
>   This one would be hard to fix because that's the
> 
> 	err = msdata->fn(msdata->data);
> 
>   function pointer which objtool can't see through.

That's why I pinged Peter whether he has some way to fix this

  call to {dynamic}()

gunk.

> https://lore.kernel.org/lkml/20260127144947.GKaXjQi8R9cE17WYKM@fat_crate.local

-- 
Regards/Gruss,
    Boris.

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