Re: [PATCH v4 24/24] [NOT-FOR-REVIEW] x86/virt/seamldr: Save and restore current VMCS

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

On Thu, Mar 12, 2026 at 06:06:22AM +0800, Huang, Kai wrote:
>
>>  static const struct x86_cpu_id tdx_host_ids[] = {
>> 	X86_MATCH_FEATURE(X86_FEATURE_TDX_HOST_PLATFORM, NULL),
>> @@ -175,6 +177,7 @@ static int seamldr_init(struct device *dev)
>>  {
>> 	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
>> 	struct fw_upload *tdx_fwl;
>> +	u64 basic_msr;
>>  
>> 	if (WARN_ON_ONCE(!tdx_sysinfo))
>> 		return -EIO;
>> @@ -182,6 +185,15 @@ static int seamldr_init(struct device *dev)
>> 	if (!tdx_supports_runtime_update(tdx_sysinfo))
>> 		return 0;
>>  
>> +	/*
>> +	 * Some TDX-capable CPUs have an erratum where the current VMCS may
>> +	 * be cleared after calling into P-SEAMLDR. Ensure no such erratum
>> +	 * exists before exposing any P-SEAMLDR functions.
>> +	 */
>> +	rdmsrq(MSR_IA32_VMX_BASIC, basic_msr);
>> +	if (!(basic_msr & VMX_BASIC_PRESERVE_CURRENT_VMCS))
>> +		return 0;
>> +
>
>IIUC this silently disables runtime update and user won't be able to have
>any clue to tell what went wrong (while the user can see the module supports
>this feature and apparently the kernel should support it)?

I'll add some logging.

>
>Since we already have a X86_BUG_TDX_PW_MCE which is detected during kernel
>boot in tdx_init(), shouldn't we just follow so that the user can at least
>see the CPU has this erratum?
>
>Another advantage is, if in the future some other kernel code needs to know
>this erratum, it can just consult this flag.

Thanks!

I didn't do that because I wasn't sure if adding a bug bit was justified
without another use case (i.e., this is a one-off check).

But I agree that following the X86_BUG_TDX_PW_MCE is better in consistency
and extensibility. So, here is the refined patch:


From 46e89a50803d6568eb60bd8ec866ac3fd9f6e6da Mon Sep 17 00:00:00 2001
From: Chao Gao <chao.gao@intel.com>
Date: Tue, 10 Mar 2026 18:49:41 -0700
Subject: [PATCH] coco/tdx-host: Don't expose P-SEAMLDR features on CPUs with
 erratum
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Some TDX-capable CPUs have an erratum, as documented in Intel® Trust
Domain CPU Architectural Extensions (May 2021 edition) Chapter 2.3:

  SEAMRET from the P-SEAMLDR clears the current VMCS structure pointed
  to by the current-VMCS pointer. A VMM that invokes the P-SEAMLDR using
  SEAMCALL must reload the current-VMCS, if required, using the VMPTRLD
  instruction.

Clearing the current VMCS behind KVM's back will break KVM.

This erratum is not present when IA32_VMX_BASIC[60] is set. Add a CPU
bug bit for this erratum and refuse to expose P-SEAMLDR features (e.g.,
TDX module updates) on affected CPUs. Also, emit a message to clarify
why P-SEAMLDR features are disabled for affected CPUs.

== Alternatives ==
Two workarounds were considered but both were rejected:

1. Save/restore the current VMCS around P-SEAMLDR calls. This produces ugly
   assembly code [1] and doesn't play well with #MCE or #NMI if they
   need to use the current VMCS.

2. Move KVM's VMCS tracking logic to the TDX core code, which would break
   the boundary between KVM and the TDX core code [2].

Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/kvm/fedb3192-e68c-423c-93b2-a4dc2f964148@intel.com/ # [1]
Link: https://lore.kernel.org/kvm/aYIXFmT-676oN6j0@google.com/ # [2]
---
 arch/x86/include/asm/cpufeatures.h    |  1 +
 arch/x86/include/asm/vmx.h            |  1 +
 arch/x86/virt/vmx/tdx/tdx.c           | 12 ++++++++++++
 drivers/virt/coco/tdx-host/tdx-host.c |  5 +++++
 4 files changed, 19 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index c3b53beb1300..dab518122946 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -570,4 +570,5 @@
 #define X86_BUG_ITS_NATIVE_ONLY		X86_BUG( 1*32+ 8) /* "its_native_only" CPU is affected by ITS, VMX is not affected */
 #define X86_BUG_TSA			X86_BUG( 1*32+ 9) /* "tsa" CPU is affected by Transient Scheduler Attacks */
 #define X86_BUG_VMSCAPE			X86_BUG( 1*32+10) /* "vmscape" CPU is affected by VMSCAPE attacks from guests */
+#define X86_BUG_SEAMRET_INVD_VMCS	X86_BUG( 1*32+11) /* "seamret_invd_vmcs" SEAMRET may clear the current VMCS */
 #endif /* _ASM_X86_CPUFEATURES_H */
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index c85c50019523..a467b681e62d 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -135,6 +135,7 @@
 #define VMX_BASIC_INOUT				BIT_ULL(54)
 #define VMX_BASIC_TRUE_CTLS			BIT_ULL(55)
 #define VMX_BASIC_NO_HW_ERROR_CODE_CC		BIT_ULL(56)
+#define VMX_BASIC_NO_SEAMRET_INVD_VMCS		BIT_ULL(60)
 
 static inline u32 vmx_basic_vmcs_revision_id(u64 vmx_basic)
 {
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 2caedc985fbd..06c8f957a6db 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -39,6 +39,7 @@
 #include <asm/cpu_device_id.h>
 #include <asm/processor.h>
 #include <asm/mce.h>
+#include <asm/vmx.h>
 
 #include "seamcall_internal.h"
 #include "tdx.h"
@@ -1453,6 +1454,8 @@ static struct notifier_block tdx_memory_nb = {
 
 static void __init check_tdx_erratum(void)
 {
+	u64 basic_msr;
+
	/*
	 * These CPUs have an erratum.  A partial write from non-TD
	 * software (e.g. via MOVNTI variants or UC/WC mapping) to TDX
@@ -1464,6 +1467,15 @@ static void __init check_tdx_erratum(void)
	case INTEL_EMERALDRAPIDS_X:
		setup_force_cpu_bug(X86_BUG_TDX_PW_MCE);
	}
+
+	/*
+	 * Some TDX-capable CPUs have an erratum where the current VMCS may
+	 * be cleared after calling into P-SEAMLDR. Ensure no such erratum
+	 * exists before exposing any P-SEAMLDR functions.
+	 */
+	rdmsrq(MSR_IA32_VMX_BASIC, basic_msr);
+	if (!(basic_msr & VMX_BASIC_NO_SEAMRET_INVD_VMCS))
+		setup_force_cpu_bug(X86_BUG_SEAMRET_INVD_VMCS);
 }
 
 void __init tdx_init(void)
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 891cc6a083e0..7e9496e215f6 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -182,6 +182,11 @@ static int seamldr_init(struct device *dev)
	if (!tdx_supports_runtime_update(tdx_sysinfo))
		return 0;
 
+	if (boot_cpu_has_bug(X86_BUG_SEAMRET_INVD_VMCS)) {
+		pr_info("Cannot talk with P-SEAMLDR due to seamret_invd_vmcs bug\n");
+		return 0;
+	}
+
	tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "tdx_module",
					   &tdx_fw_ops, NULL);
	if (IS_ERR(tdx_fwl))
-- 
2.47.3


>
>And btw,
>
>Which code base was this patch generated?  If I read correctly, in this
>series seamldr_init() is a void function but doesn't return anything.

This patch is based on a new version. Yilun suggested changing seamldr_init()
to return an error:

https://lore.kernel.org/kvm/aaEP8CbLCc69U45Z@yilunxu-OptiPlex-7050/