Re: [PATCH v6 09/22] x86/virt/seamldr: Introduce skeleton for TDX module updates

[Chao Gao <chao.gao@intel.com>]

>+static void set_target_state(enum module_update_state state)
>+{
>+	/* Reset ack counter. */
>+	update_data.thread_ack = num_online_cpus();

...

>+static void ack_state(void)
>+{
>+	guard(raw_spinlock)(&update_data.lock);
>+	update_data.thread_ack--;
>+	if (!update_data.thread_ack)
>+		set_target_state(update_data.state + 1);
>+}
>+
>+/*
>+ * See multi_cpu_stop() from where this multi-cpu state-machine was
>+ * adopted, and the rationale for touch_nmi_watchdog().
>+ */
>+static int do_seamldr_install_module(void *seamldr_params)
>+{
>+	enum module_update_state newstate, curstate = MODULE_UPDATE_START;
>+	int ret = 0;
>+
>+	do {
>+		/* Chill out and re-read update_data. */
>+		cpu_relax();
>+		newstate = READ_ONCE(update_data.state);
>+
>+		if (newstate != curstate) {
>+			curstate = newstate;
>+			switch (curstate) {
>+			/* TODO: add the update steps. */
>+			default:
>+				break;
>+			}
>+
>+			ack_state();
>+		} else {
>+			touch_nmi_watchdog();
>+			rcu_momentary_eqs();
>+		}
>+	} while (curstate != MODULE_UPDATE_DONE);
>+
>+	return ret;
>+}
>+
> DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
> 	    if (!IS_ERR_OR_NULL(_T)) free_seamldr_params(_T))
> 
>@@ -197,7 +270,7 @@ int seamldr_install_module(const u8 *data, u32 size)
> 	if (IS_ERR(params))
> 		return PTR_ERR(params);
> 
>-	/* TODO: Update TDX module here */
>-	return 0;
>+	set_target_state(MODULE_UPDATE_START + 1);
>+	return stop_machine(do_seamldr_install_module, params, cpu_online_mask);

I'm reviewing feedback from sashiko:

https://sashiko.dev/#/patchset/20260326084448.29947-1-chao.gao%40intel.com

It identifies a valid race between CPU hotplug and state machine management. If
a CPU goes offline after set_target_state() but before stop_machine(),
thread_ack never reaches zero, causing all CPUs to spin indefinitely with
interrupts disabled.

The fix is: acquire cpus_read_lock() before set_target_state() and use
stop_machine_cpuslocked(). i.e.,

diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index ed6a092b11e2..6f9d80a3a76f 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -270,7 +270,8 @@ int seamldr_install_module(const u8 *data, u32 size)
	if (IS_ERR(params))
		return PTR_ERR(params);
 
+	guard(cpus_read_lock)();
	set_target_state(MODULE_UPDATE_START + 1);
-	return stop_machine(do_seamldr_install_module, params, cpu_online_mask);
+	return stop_machine_cpuslocked(do_seamldr_install_module, params, cpu_online_mask);
 }
 EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");