Re: [PATCH v4 19/24] x86/virt/tdx: Update tdx_sysinfo and check features post-update

["Huang, Kai" <kai.huang@intel.com>]

On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> tdx_sysinfo contains all metadata of the active TDX module, including
> versions, supported features, and TDMR/TDCS/TDVPS information. 
> 

Nit: add "etc", since there are more staff besides the 3 things you listed.

> These
> values may change over updates. Blindly refreshing the entire tdx_sysinfo
> could disrupt running software, as it may subtly rely on the previous state
> unless proven otherwise.
> 
> Adopt a conservative approach, like microcode updates, by only refreshing
> version information that does not affect functionality, while ignoring
> all other changes. This is acceptable as new modules are required to
> maintain backward compatibility.
> 
> Any updates to metadata beyond versions should be justified and reviewed on
> a case-by-case basis.
> 
> Note that preallocating a tdx_sys_info buffer before updates is to avoid
> having to handle -ENOMEM when updating tdx_sysinfo after a successful
> update.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>

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

One bit below ...

[...]

>  
> +/*
> + * Update tdx_sysinfo and check if any TDX module features changed after
> + * updates

s/updates/update?  I don't see more than one update.

And it's more than "check module features being changed" since there are
other metadata fields which may have different values after update, right?

I would just remove this comment since I don't see it says more than just
repeating the code below (which also has comments saying the same thing, in
a more elaborated way).

> + */
> +int tdx_module_post_update(struct tdx_sys_info *info)
> +{
> +	struct tdx_sys_info_version *old, *new;
> +	int ret;
> +
> +	/* Shouldn't fail as the update has succeeded */
> +	ret = get_tdx_sys_info(info);
> +	if (WARN_ONCE(ret, "version retrieval failed after update, replace TDX Module\n"))
> +		return ret;
> +
> +	old = &tdx_sysinfo.version;
> +	new = &info->version;
> +	pr_info("version %u.%u.%02u -> %u.%u.%02u\n", old->major_version,
> +						      old->minor_version,
> +						      old->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 updating your BIOS to install the TDX Module.\n");
> +	return 0;
> +}
> +