Re: [PATCH v8 09/21] x86/virt/seamldr: Introduce skeleton for TDX module updates
From: Dave Hansen
Date: Wed May 06 2026 - 16:46:11 EST
On 5/6/26 06:00, Chao Gao wrote:
> On Thu, Apr 30, 2026 at 01:03:53PM -0700, Dave Hansen wrote:
>> On 4/27/26 08:28, Chao Gao wrote:
>>> +static struct {
>>> + enum module_update_state state;
>>> + int thread_ack;
>>
>> multi_stop_data has an atomic_t for this.
>>
>> You have an int.
>>
>> Which one is right?
>
> You pointed out that using atomic_t and memory barriers for synchronization was
> overly complicated. So, I switched to use a spinlock, and thread_ack can now be
> a plain int.
Good point.
Could you make this a bit more obvious, please?
I honestly don't like guard(). Maybe I'm old-fashioned, but I really,
really like critical sections to be, well, explicit *sections* of code.
Second, you have two functions defined next to each other with similar
names:
static void ack_state(void)
static void set_target_state(enum module_update_state state)
Both of which manipulate the same data. One takes the lock. One doesn't.
That could be fixed with comments.