Re: [PATCH v5 12/22] x86/virt/tdx: Reset software states during TDX module shutdown

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

On Mon, Mar 16, 2026 at 05:06:49PM +0800, Huang, Kai wrote:
>
>> @@ -1179,6 +1179,7 @@ EXPORT_SYMBOL_FOR_KVM(tdx_enable);
>>  int tdx_module_shutdown(void)
>>  {
>>  	struct tdx_module_args args = {};
>> +	int ret, cpu;
>>  
>>  	/*
>>  	 * Shut down the TDX module and prepare handoff data for the next
>> @@ -1188,7 +1189,22 @@ int tdx_module_shutdown(void)
>>  	 * modules as new modules likely have higher handoff version.
>>  	 */
>>  	args.rcx = tdx_sysinfo.handoff.module_hv;
>> -	return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
>> +	ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
>> +	if (ret)
>> +		return ret;
>> +
>> +	tdx_module_status = TDX_MODULE_UNINITIALIZED;
>> +	sysinit_done = false;
>> +	sysinit_ret = 0;
>> +
>> +	/*
>> +	 * By reaching here CPUHP is disabled and all present CPUs
>> +	 * are online. It's safe to just loop all online CPUs and
>> +	 * reset the per-cpu flag.
>> +	 */
>> +	for_each_online_cpu(cpu)
>> +		per_cpu(tdx_lp_initialized, cpu) = false;
>
>Since you have removed the requirement that P-SEAMLDR.INSTALL must be done
>on all CPUs, and removed the relevant patch, the "all present CPUs are
>online" part isn't correct anymore.
>
>And using for_each_online_cpu() isn't enough since this doesn't reset the
>tdx_lp_initialized for offline CPUs.
>
>One way is to just use for_each_possible_cpu() here so tdx_lp_initialized
>for all CPUs are reset.  Since the "CPUHP is disabled" part is still correct
>AFAICT (since stop_machine() disables CPUHP internally during the
>operation), resetting tdx_lp_initialized for offline CPUs won't race with
>CPUHP.
>
>And assuming this series will be applied after Sean's VMXON series, we will

Yes.

>have a TDX-specific CPUHP callback tdx_online_cpu() in TDX x86 core to do
>tdx_cpu_enable(), which will then enable TDX again on the new-online CPU.

Good point.

Clearing tdx_lp_initialized for offlined CPUs makes sense, but I'd rather not
justify this through "enabling TDX on new-online CPUs" since many details
remain unclear. For example, there will be a SEAMCALL to disable TDX per-CPU.
It should be called when CPUs go offline so that those CPUs can be exempting
from doing SEAMLDR.INSTALL during module updates. tdx_lp_initialized should
have been cleared along with that "disable TDX per-CPU" SEAMCALL for offlined
CPUs.

I'm not arguing against for_each_present_cpu(). I just think discussing how
to support TDX module update with offlined CPUs in the comment would be a bit
premature. How about keeping it simple:

/*
 * Since the TDX module is shut down and gone, mark all CPUs (including
 * offlined ones) as uninitialied. This is called in stop_machine() (where
 * CPU hotplug is disabled), preventing races with other tdx_lp_initialized
 * accesses.
 */

>
>Btw, w/o Sean's VMXON series, currently only KVM provides the TDX-specific
>CPUHP callback.  So it seems if module update is done when KVM is not
>loaded,

w/o VMXON series, module update isn't possible if KVM isn't loaded.

>there will be no TDX-specific CPUHP callback to re-enable TDX for
>the new-online CPU.  This means any SEAMCALL on that CPU will fail before
>KVM module is loaded again (which will then re-register the TDX-specific
>CPUHP and run tdx_cpu_enable() for all online CPUs). 
>
>But I don't think we should consider this case.
>
>
Agreed.