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

Chao Gao posted 21 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v2 20/21] x86/virt/tdx: Update tdx_sysinfo and check features post-update
Posted by Chao Gao 4 months, 1 week ago
tdx_sysinfo contains all metadata of the active TDX module, including
versions, supported features, and TDMR/TDCS/TDVPS information. These
elements 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>
Tested-by: Farrah Chen <farrah.chen@intel.com>
---
v2:
 - don't add a separate function for version and feature checks. Do them
   directly in tdx_module_post_update()
 - add a comment about preallocating a tdx_sys_info buffer in
   seamldr_install_module().
---
 arch/x86/virt/vmx/tdx/seamldr.c | 12 ++++++++-
 arch/x86/virt/vmx/tdx/tdx.c     | 47 +++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h     |  2 ++
 3 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index a8ca6966beac..a72f6b0b27e9 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -350,6 +350,16 @@ int seamldr_install_module(const u8 *data, u32 size)
 	if (!info->num_remaining_updates)
 		return -ENOSPC;
 
+	/*
+	 * Preallocating a tdx_sys_info buffer before updates is to avoid having to
+	 * handle -ENOMEM when updating tdx_sysinfo after a successful update.
+	 */
+	struct tdx_sys_info *sysinfo __free(kfree) = kzalloc(sizeof(*sysinfo),
+							     GFP_KERNEL);
+	if (!sysinfo)
+		return -ENOMEM;
+
+
 	struct seamldr_params *params __free(free_seamldr_params) =
 						init_seamldr_params(data, size);
 	if (IS_ERR(params))
@@ -367,6 +377,6 @@ int seamldr_install_module(const u8 *data, u32 size)
 	if (ret)
 		return ret;
 
-	return 0;
+	return tdx_module_post_update(sysinfo);
 }
 EXPORT_SYMBOL_GPL_FOR_MODULES(seamldr_install_module, "tdx-host");
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 7613fd16a0ce..128e6ffba736 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1241,6 +1241,53 @@ int tdx_module_run_update(void)
 	return 0;
 }
 
+/*
+ * 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");
+		return ret;
+	}
+
+	guard(mutex)(&tdx_module_lock);
+
+	cur = &tdx_sysinfo.version;
+	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");
+	return 0;
+}
+
 static bool is_pamt_page(unsigned long phys)
 {
 	struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 983c01c6949a..ca76126880ee 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -3,6 +3,7 @@
 #define _X86_VIRT_TDX_H
 
 #include <linux/bits.h>
+#include <asm/tdx_global_metadata.h>
 
 /*
  * This file contains both macros and data structures defined by the TDX
@@ -124,5 +125,6 @@ int tdx_module_shutdown(void);
 void tdx_module_set_error(void);
 int tdx_cpu_enable(void);
 int tdx_module_run_update(void);
+int tdx_module_post_update(struct tdx_sys_info *info);
 
 #endif
-- 
2.47.3
Re: [PATCH v2 20/21] x86/virt/tdx: Update tdx_sysinfo and check features post-update
Posted by Binbin Wu 2 months, 1 week ago

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, "..."))

> +		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.

> +	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"?

Does it mean after a system reboot, the change done by TD preserving update will
be gone? If we want the TDX module upgrade to be permanent, it needs to replace
the TDX module binary the BIOS will load, right?

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?)


> +	return 0;
> +}
> +
>  static bool is_pamt_page(unsigned long phys)
>  {
>  	struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 983c01c6949a..ca76126880ee 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -3,6 +3,7 @@
>  #define _X86_VIRT_TDX_H
>  
>  #include <linux/bits.h>
> +#include <asm/tdx_global_metadata.h>
>  
>  /*
>   * This file contains both macros and data structures defined by the TDX
> @@ -124,5 +125,6 @@ int tdx_module_shutdown(void);
>  void tdx_module_set_error(void);
>  int tdx_cpu_enable(void);
>  int tdx_module_run_update(void);
> +int tdx_module_post_update(struct tdx_sys_info *info);
>  
>  #endif
Re: [PATCH v2 20/21] x86/virt/tdx: Update tdx_sysinfo and check features post-update
Posted by Chao Gao 2 weeks, 6 days ago
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.
Re: [PATCH v2 20/21] x86/virt/tdx: Update tdx_sysinfo and check features post-update
Posted by dan.j.williams@intel.com 1 week, 6 days ago
Chao Gao wrote:
[..]
> 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.

New features over updates, and revised software contracts (broken
contracts) over updates are not "compatible updates". So it is not a
"conservative approach" to start, it is more a hard barrier that updates
of that nature are out-of-scope via this interface. The Documentation
needs to be clear that the problems caused by update are the Module's
problems, not new kernel problems.

If you look at other platform mutating update mechanisms like ACPI
runtime update and CXL runtime update, the class of updates that expose
new features / contracts require reset. TDX is not special in this
regard.