Re: [PATCH v4 19/24] x86/virt/tdx: Update tdx_sysinfo and check features post-update
From: Huang, Kai
Date: Wed Mar 04 2026 - 18:41:00 EST
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@xxxxxxxxx>
> Reviewed-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> Reviewed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx>
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;
> +}
> +