Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Chao Gao
Date: Fri Mar 13 2026 - 08:16:50 EST
On Thu, Mar 12, 2026 at 01:40:44PM -0700, Dave Hansen wrote:
>On 2/12/26 06:35, Chao Gao wrote:
>> +static void set_target_state(enum tdp_state state)
>> +{
>> + /* Reset ack counter. */
>> + atomic_set(&tdp_data.thread_ack, num_online_cpus());
>> + /* Ensure thread_ack is updated before the new state */
>> + smp_wmb();
>> + WRITE_ONCE(tdp_data.state, state);
>> +}
>
>This looks overly complicated.
>
>If it doesn't need to be scalable, just make it stupid and simple. Why
>not just protect the whole thing with a spinlock and be done with it?
Good suggestion. I copied this from multi_cpu_stop() without considering
whether it could be simplified.
Regarding scalability, I compared the update time and didn't see a
meaningful difference on a system with 240 CPUs.
I will make changes like this:
(Note: I'm also renaming tdp_data/tdp_state to update_data and
module_update_state for clarity, since "tdp" isn't obvious as Kai pointed
out.)
static struct {
enum module_update_state state;
- atomic_t thread_ack;
- atomic_t failed;
+ int thread_ack;
+ int failed;
+ raw_spinlock_t lock;
} update_data;
static void set_target_state(enum module_update_state state)
{
/* Reset ack counter. */
- atomic_set(&update_data.thread_ack, num_online_cpus());
- /*
- * Ensure thread_ack is updated before the new state.
- * Otherwise, other CPUs may see the new state and ack
- * it before thread_ack is reset. An ack before reset
- * is effectively lost, causing the system to wait
- * forever for thread_ack to become zero.
- */
- smp_wmb();
- WRITE_ONCE(update_data.state, state);
+ update_data.thread_ack = num_online_cpus();
+ update_data.state = state;
}
/* Last one to ack a state moves to the next state. */
static void ack_state(void)
{
- if (atomic_dec_and_test(&update_data.thread_ack))
+ guard(raw_spinlock)(&update_data.lock);
+ update_data.thread_ack--;
+ if (!update_data.thread_ack)
set_target_state(update_data.state + 1);
}