Re: [PATCH v2 20/21] x86/virt/tdx: Update tdx_sysinfo and check features post-update

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

On Wed, Dec 03, 2025 at 03:41:50PM +0800, Binbin Wu wrote:
>
>
>On 10/1/2025 10:53 AM, Chao Gao wrote:
>
>[...]
>>  
>> +/*
>> + * Update tdx_sysinfo and check if any TDX module features changed after
>> + * updates
>> + */
>> +int tdx_module_post_update(struct tdx_sys_info *info)
>> +{
>> +	struct tdx_sys_info_version *cur, *new;
>> +	int ret;
>> +
>> +	/* Shouldn't fail as the update has succeeded */
>> +	ret = get_tdx_sys_info(info);
>> +	if (ret) {
>> +		WARN_ONCE(1, "version retrieval failed after update, replace TDX Module\n");
>
>Nit:
>Could be if (WARN_ONCE(ret, "..."))

ack.

>
>> +		return ret;
>> +	}
>> +
>> +	guard(mutex)(&tdx_module_lock);
>> +
>> +	cur = &tdx_sysinfo.version;
>
>Nit:
>After update, the current TDX module is the new TDX module already, may be
>better to use old instead of cur.

Indeed.

>
>> +	new = &info->version;
>> +	pr_info("version %u.%u.%02u -> %u.%u.%02u\n", cur->major_version,
>> +						      cur->minor_version,
>> +						      cur->update_version,
>> +						      new->major_version,
>> +						      new->minor_version,
>> +						      new->update_version);
>> +
>> +	/*
>> +	 * Blindly refreshing the entire tdx_sysinfo could disrupt running
>> +	 * software, as it may subtly rely on the previous state unless
>> +	 * proven otherwise.
>> +	 *
>> +	 * Only refresh version information (including handoff version)
>> +	 * that does not affect functionality, and ignore all other
>> +	 * changes.
>> +	 */
>> +	tdx_sysinfo.version	= info->version;
>> +	tdx_sysinfo.handoff	= info->handoff;
>> +
>> +	if (!memcmp(&tdx_sysinfo, info, sizeof(*info)))
>> +		return 0;
>> +
>> +	pr_info("TDX module features have changed after updates, but might not take effect.\n");
>> +	pr_info("Please consider a potential BIOS update.\n");
>
>
>BIOS update?
>I guess it's "TDX module update via BIOS"?

ok. I will update the log message.

>
>Does it mean after a system reboot, the change done by TD preserving update will
>be gone?

Yes. After reboot, the update will be gone. The (old) TDX module will be
reloaded by the BIOS.

>If we want the TDX module upgrade to be permanent, it needs to replace
>the TDX module binary the BIOS will load, right?

Yes.

>
>So the scenario of TD preserving update seems to be limited to security fixes?
>(I guess the security fixes will take effect directly after TD preserving
>update?)

Yes. Fixes (whether security, performance, or functional) will take effect. New
features won't. This series takes a minimalist approach, updating only the
module version and handoff-version while leaving all other TDX metadata
unchanged.

I think this conservative approach is appropriate for initial support. New
features can be gradually enabled later as we prove that other components
(e.g., KVM) are ready for new features introduced via runtime updates.

This enabling approach is aligned with current microcode updates.