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

[Huang, Kai]

> +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)
> +};
> +
> +static void set_target_state(enum module_update_state state)
> +{
> +	/* Reset ack counter. */
> +	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)
> +{
> +	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;
> +}

The read 

    newstate = READ_ONCE(update_data.state);

.. is done out of the spinlock, but AFAICT it is fine.

The worst case is couple of additional loops on old state.  The thread_ack
is protected by the lock thus no ack will get lost.

Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx>