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

[Kiryl Shutsemau]

On Sun, Mar 15, 2026 at 06:58:29AM -0700, Chao Gao wrote:
> TDX module updates require careful synchronization with other TDX
> operations. The requirements are (#1/#2 reflect current behavior that
> must be preserved):
> 
> 1. SEAMCALLs need to be callable from both process and IRQ contexts.
> 2. SEAMCALLs need to be able to run concurrently across CPUs
> 3. During updates, only update-related SEAMCALLs are permitted; all
>    other SEAMCALLs shouldn't be called.
> 4. During updates, all online CPUs must participate in the update work.
> 
> No single lock primitive satisfies all requirements. For instance,
> rwlock_t handles #1/#2 but fails #4: CPUs spinning with IRQs disabled
> cannot be directed to perform update work.
> 
> Use stop_machine() as it is the only well-understood mechanism that can
> meet all requirements.
> 
> And TDX module updates consist of several steps (See Intel® Trust Domain
> Extensions (Intel® TDX) Module Base Architecture Specification, Revision
> 348549-007, Chapter 4.5 "TD-Preserving TDX module Update"). Ordering
> requirements between steps mandate lockstep synchronization across all
> CPUs.
> 
> multi_cpu_stop() is a good example of performing a multi-step task in
> lockstep. But it doesn't synchronize steps within the callback function
> it takes. So, implement one based on its pattern to establish the
> skeleton for TDX module updates. Specifically, add a global state
> machine where each state represents a step in the update flow. The state
> advances only after all CPUs acknowledge completing their work in the
> current state. This acknowledgment mechanism is what ensures lockstep
> execution.
> 
> Potential alternative to stop_machine()
> =======================================
> An alternative approach is to lock all KVM entry points and kick all
> vCPUs. Here, KVM entry points refer to KVM VM/vCPU ioctl entry points,
> implemented in KVM common code (virt/kvm). Adding a locking mechanism
> there would affect all architectures KVM supports. And to lock only TDX
> vCPUs, new logic would be needed to identify TDX vCPUs, which the KVM
> common code currently lacks. This would add significant complexity and
> maintenance overhead to KVM for this TDX-specific use case.
> 
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> Reviewed-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> Reviewed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>

Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>

One question below.

> +static struct {
> +	enum module_update_state state;
> +	int thread_ack;
> +	/*
> +	 * Protect update_data. Raw spinlock as it will be acquired from
> +	 * interrupt-disabled contexts.
> +	 */
> +	raw_spinlock_t lock;
> +} update_data = {
> +	.lock = __RAW_SPIN_LOCK_UNLOCKED(update_data.lock)
> +};

multi_stop_cpu() used atomic_t for thread_ack insead of spinlock.
Any particular reason you took different direction?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov