[PATCH v2 0/2] KVM: VMX: Explicitly track TDX VMs' root level

[PATCH v2 0/2] KVM: VMX: Explicitly track TDX VMs' root level

[Sean Christopherson]

Fix a bug where a misbehaving (or niave?) userspace can configure guest CPUID
such that KVM will use the "wrong" root level for its mirror EPTs and trip the
KVM_BUG_ON() in tdx_load_mmu_pgd().

v2:
 - Rewrite the comment in kvm_mmu_get_tdp_level(). [Rick]
 - Check for kvm_has_mirrored_tdp() in kvm_mmu_get_tdp_level(), and yell if
   arch.mirror_root_level isn't also set. [Rick]
 - Separate the removal of TDX_SHARED_BIT_PWL_{4,5} from the bug fix. [Rick]
 - Fix a buring => buried typo. [Binbin]
 - Collect Tested/Reviewed-by. [Binbin, Rick, Yan] (I applied 'em only to
   patch 1, please  holler if that doesn't seem right).

v1: https://lore.kernel.org/all/20260814224509.2342760-1-seanjc@google.com

Sean Christopherson (2):
  KVM: VMX: Explicitly track TDX VMs' root level instead of guessing it
    from CPUID
  KVM: VMX: Drop TDX_SHARED_BIT_PWL_{4,5} and dedup related code

 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/cpuid.c            | 14 --------------
 arch/x86/kvm/cpuid.h            |  1 -
 arch/x86/kvm/mmu/mmu.c          | 19 +++++++++++--------
 arch/x86/kvm/vmx/tdx.c          | 20 +++++++++++---------
 5 files changed, 23 insertions(+), 32 deletions(-)


base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v2 1/2] KVM: VMX: Explicitly track TDX VMs' root level instead of guessing it from CPUID

[Sean Christopherson]

Explicitly track the root level for TDX VMs instead of trying to infer the
depth of the paging tree based on an individual vCPU's CPUID information.
Applying KVM's existing logic to select the root level to TDX is flawed as
nothing *requires* userspace to fill in the correct guest.MAXPHYADDR for a
vCPU's CPUID.  Guessing at the correct root level is also ridiculous given
that userspace has already told KVM the root level during TD initialization.

Relying on userspace to set the expected/correct CPUID lets a misbehaving
userspace trip the KVM_BUG_ON() in tdx_load_mmu_pgd() by configuring guest
CPUID to use an "incorrect" guest.MAXPHYADDR.

Don't use kvm_gfn_direct_bits() to infer the mirror root level, as the
connection between TDX's one and only "direct" bit and the predetermined
root level is a TDX implementation detail.  I.e. avoid baking in the
assumption that there is exactly one "direct bits", that the one bit is a
pivot between normal and mirror roots, and that the pivot bit is the most
significant bit of the effective GPA space.  For the same reason, set the
root level and direct bits in TDX code, i.e. don't provide a helper in the
MMU, because from the MMU's perspective, they are two separate concepts.

Keep gfn_direct_bits even though it can be trivially derived from
mirror_root_level as saving a whole eight bytes per VM is meaningless,
keeping the TDX details buried in TDX would require a kvm_x86_ops hook, and
the value is queried fairly often and in hot paths.

And for the moment, keep the S-bit sanity check in tdx_load_mmu_pgd(), even
though it really only needs to ensure the incoming level matches the
preconfigured mirror root level.  Because KVM manually configures the
S-bit location, there's technically a risk that the S-bit location and
mirror root level could get out of sync.  That can be addressed by more
programmatically computing the S-bit, but that doesn't need to be done now.

Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Kai Huang <kai.huang@intel.com>
Cc: Yan Zhao <yan.y.zhao@intel.com>
Fixes: 20d913729c11 ("KVM: x86/mmu: Taking guest pa into consideration when calculate tdp level")
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Tested-by: Yan Zhao <yan.y.zhao@intel.com>
Tested-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/cpuid.c            | 14 --------------
 arch/x86/kvm/cpuid.h            |  1 -
 arch/x86/kvm/mmu/mmu.c          | 19 +++++++++++--------
 arch/x86/kvm/vmx/tdx.c          | 14 ++++++++++++--
 5 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 683bb8bf43a9..edd5ad2e5b52 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1406,6 +1406,7 @@ struct kvm_arch {
 	struct kvm_mmu_memory_cache split_desc_cache;
 
 	gfn_t gfn_direct_bits;
+	int mirror_root_level;
 
 	/*
 	 * Size of the CPU's dirty log buffer, i.e. VMX's PML buffer. A Zero
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index ddb022cb203a..34c609a60eef 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -483,20 +483,6 @@ int cpuid_query_maxphyaddr(struct kvm_vcpu *vcpu)
 	return 36;
 }
 
-int cpuid_query_maxguestphyaddr(struct kvm_vcpu *vcpu)
-{
-	struct kvm_cpuid_entry2 *best;
-
-	best = kvm_find_cpuid_entry(vcpu, 0x80000000);
-	if (!best || best->eax < 0x80000008)
-		goto not_found;
-	best = kvm_find_cpuid_entry(vcpu, 0x80000008);
-	if (best)
-		return (best->eax >> 16) & 0xff;
-not_found:
-	return 0;
-}
-
 /*
  * This "raw" version returns the reserved GPA bits without any adjustments for
  * encryption technologies that usurp bits.  The raw mask should be used if and
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 8d863f45585d..46bfe8699e67 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -68,7 +68,6 @@ void __init kvm_init_xstate_sizes(void);
 u32 xstate_required_size(u64 xstate_bv, bool compacted);
 
 int cpuid_query_maxphyaddr(struct kvm_vcpu *vcpu);
-int cpuid_query_maxguestphyaddr(struct kvm_vcpu *vcpu);
 u64 kvm_vcpu_reserved_gpa_bits_raw(struct kvm_vcpu *vcpu);
 
 static inline int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 064ecc33b926..611c78be8b41 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5953,19 +5953,22 @@ void __kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu,
 
 static inline int kvm_mmu_get_tdp_level(struct kvm_vcpu *vcpu)
 {
-	int maxpa;
-
-	if (vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM)
-		maxpa = cpuid_query_maxguestphyaddr(vcpu);
-	else
-		maxpa = cpuid_maxphyaddr(vcpu);
-
 	/* tdp_root_level is architecture forced level, use it if nonzero */
 	if (tdp_root_level)
 		return tdp_root_level;
 
+	/*
+	 * If the VM has mirror roots, then the root level is predefined as the
+	 * mirror root (and by extension the normal root) needs to match the
+	 * root level that was configured for the external page tables that are
+	 * being mirrored by KVM.
+	 */
+	if (kvm_has_mirrored_tdp(vcpu->kvm) &&
+	    !WARN_ON_ONCE(!vcpu->kvm->arch.mirror_root_level))
+		return vcpu->kvm->arch.mirror_root_level;
+
 	/* Use 5-level TDP if and only if it's useful/necessary. */
-	if (max_tdp_level == 5 && maxpa <= 48)
+	if (max_tdp_level == 5 && cpuid_maxphyaddr(vcpu) <= 48)
 		return 4;
 
 	return max_tdp_level;
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index b272c20586a7..e6b7da616817 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2760,6 +2760,13 @@ DEFINE_CLASS(tdx_vm_state_guard, tdx_vm_state_guard_t,
 	     if (!IS_ERR(_T)) tdx_release_vm_state_locks(_T),
 	     tdx_acquire_vm_state_locks(kvm), struct kvm *kvm);
 
+static __always_inline void tdx_set_mirror_root_level(struct kvm *kvm, int level)
+{
+	BUILD_BUG_ON(level != 4 && level != 5);
+
+	kvm->arch.mirror_root_level = level;
+}
+
 static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
 {
 	struct kvm_tdx_init_vm __user *user_data = u64_to_user_ptr(cmd->data);
@@ -2822,10 +2829,13 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
 	kvm_tdx->attributes = td_params->attributes;
 	kvm_tdx->xfam = td_params->xfam;
 
-	if (td_params->config_flags & TDX_CONFIG_FLAGS_MAX_GPAW)
+	if (td_params->config_flags & TDX_CONFIG_FLAGS_MAX_GPAW) {
 		kvm->arch.gfn_direct_bits = TDX_SHARED_BIT_PWL_5;
-	else
+		tdx_set_mirror_root_level(kvm, 5);
+	} else {
 		kvm->arch.gfn_direct_bits = TDX_SHARED_BIT_PWL_4;
+		tdx_set_mirror_root_level(kvm, 4);
+	}
 
 	kvm_tdx->state = TD_STATE_INITIALIZED;
 out:
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v2 2/2] KVM: VMX: Drop TDX_SHARED_BIT_PWL_{4,5} and dedup related code

[Sean Christopherson]

Fold the GPA => GFN conversion and bitshift logic for identifying the S-bit
given the EPT root level into tdx_set_mirror_root_level() to dedup the math
and drop TDX_SHARED_BIT_PWL_{4,5} in the process.  In addition to deduping
a small amount of code, using the level to compute the S-bit position more
or less eliminates the risk of the mirror_root_level and gfn_direct_bits
getting out of sync.

Opportunistically switch the sanity check in tdx_load_mmu_pgd() to check
the mirror root level, not the S-Bit location, now that it's all but
impossible for the two things to get out of sync.

For all intents and purposes, no functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/tdx.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index e6b7da616817..a0bc9f818f43 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -56,9 +56,6 @@
 bool enable_tdx __ro_after_init;
 module_param_named(tdx, enable_tdx, bool, 0444);
 
-#define TDX_SHARED_BIT_PWL_5 gpa_to_gfn(BIT_ULL(51))
-#define TDX_SHARED_BIT_PWL_4 gpa_to_gfn(BIT_ULL(47))
-
 static const struct tdx_sys_info *tdx_sysinfo;
 
 void tdh_vp_rd_failed(struct vcpu_tdx *tdx, char *uclass, u32 field, u64 err)
@@ -1609,10 +1606,7 @@ static int handle_tdvmcall(struct kvm_vcpu *vcpu)
 
 void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int pgd_level)
 {
-	u64 shared_bit = (pgd_level == 5) ? TDX_SHARED_BIT_PWL_5 :
-			  TDX_SHARED_BIT_PWL_4;
-
-	if (KVM_BUG_ON(shared_bit != kvm_gfn_direct_bits(vcpu->kvm), vcpu->kvm))
+	if (KVM_BUG_ON(pgd_level != vcpu->kvm->arch.mirror_root_level, vcpu->kvm))
 		return;
 
 	td_vmcs_write64(to_tdx(vcpu), SHARED_EPT_POINTER, root_hpa);
@@ -2765,6 +2759,7 @@ static __always_inline void tdx_set_mirror_root_level(struct kvm *kvm, int level
 	BUILD_BUG_ON(level != 4 && level != 5);
 
 	kvm->arch.mirror_root_level = level;
+	kvm->arch.gfn_direct_bits = gpa_to_gfn(BIT_ULL(level == 4 ? 47 : 51));
 }
 
 static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
@@ -2829,13 +2824,10 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
 	kvm_tdx->attributes = td_params->attributes;
 	kvm_tdx->xfam = td_params->xfam;
 
-	if (td_params->config_flags & TDX_CONFIG_FLAGS_MAX_GPAW) {
-		kvm->arch.gfn_direct_bits = TDX_SHARED_BIT_PWL_5;
+	if (td_params->config_flags & TDX_CONFIG_FLAGS_MAX_GPAW)
 		tdx_set_mirror_root_level(kvm, 5);
-	} else {
-		kvm->arch.gfn_direct_bits = TDX_SHARED_BIT_PWL_4;
+	else
 		tdx_set_mirror_root_level(kvm, 4);
-	}
 
 	kvm_tdx->state = TD_STATE_INITIALIZED;
 out:
-- 
2.55.0.887.g758fc8c411-goog

Re: [PATCH v2 1/2] KVM: VMX: Explicitly track TDX VMs' root level instead of guessing it from CPUID

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A malicious or buggy userspace can trigger a host-side WARN_ON_ONCE by attempting to create a vCPU for a TDX VM before successfully completing KVM_TDX_INIT_VM.
--

commit d9de404aca15fbfe936c1e65aa00fb63755a135e
Author: Sean Christopherson <seanjc@google.com>

KVM: VMX: Explicitly track TDX VMs' root level instead of guessing it from CPUID

This commit replaces guessing the root level from CPUID with explicit tracking
for TDX VMs. It sets the mirror root level based on the configuration provided
during KVM_TDX_INIT_VM.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 064ecc33b9267..611c78be8b411 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
[ ... ]
> @@ -5953,19 +5953,22 @@ void __kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu,
>  
>  static inline int kvm_mmu_get_tdp_level(struct kvm_vcpu *vcpu)
>  {
[ ... ]
>  	/* tdp_root_level is architecture forced level, use it if nonzero */
>  	if (tdp_root_level)
>  		return tdp_root_level;
>  
> +	/*
> +	 * If the VM has mirror roots, then the root level is predefined as the
> +	 * mirror root (and by extension the normal root) needs to match the
> +	 * root level that was configured for the external page tables that are
> +	 * being mirrored by KVM.
> +	 */
> +	if (kvm_has_mirrored_tdp(vcpu->kvm) &&
> +	    !WARN_ON_ONCE(!vcpu->kvm->arch.mirror_root_level))
> +		return vcpu->kvm->arch.mirror_root_level;

[Severity: High]
Does this regression allow a malicious or buggy userspace to trigger this
WARN_ON_ONCE and cause a host denial of service (especially with
panic_on_warn=1)?

If userspace creates a TDX VM via KVM_CREATE_VM (setting the VM type to
KVM_X86_TDX_VM) and then directly calls KVM_CREATE_VCPU without executing
KVM_TDX_INIT_VM first, it appears this warning will be reached.

During vCPU creation, the MMU context is initialized unconditionally before
the architecture-specific vCPU creation callback:

kvm_arch_vcpu_create()->kvm_mmu_create()->kvm_mmu_get_tdp_level()

Because KVM_TDX_INIT_VM was skipped, mirror_root_level remains 0, satisfying
the condition and triggering the kernel warning.

The architecture-specific callback that enforces the TD_STATE_INITIALIZED
requirement is invoked too late in the process to prevent this:

kvm_arch_vcpu_create()->kvm_x86_call(vcpu_create)()

Would it be safer to return an error or handle the uninitialized state rather
than triggering a host-side warning on user-controllable input?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826215258.937210-1-seanjc@google.com?part=1

Re: [PATCH v2 1/2] KVM: VMX: Explicitly track TDX VMs' root level instead of guessing it from CPUID

[Sean Christopherson]

On Wed, Aug 26, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 064ecc33b9267..611c78be8b411 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> [ ... ]
> > @@ -5953,19 +5953,22 @@ void __kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu,
> >  
> >  static inline int kvm_mmu_get_tdp_level(struct kvm_vcpu *vcpu)
> >  {
> [ ... ]
> >  	/* tdp_root_level is architecture forced level, use it if nonzero */
> >  	if (tdp_root_level)
> >  		return tdp_root_level;
> >  
> > +	/*
> > +	 * If the VM has mirror roots, then the root level is predefined as the
> > +	 * mirror root (and by extension the normal root) needs to match the
> > +	 * root level that was configured for the external page tables that are
> > +	 * being mirrored by KVM.
> > +	 */
> > +	if (kvm_has_mirrored_tdp(vcpu->kvm) &&
> > +	    !WARN_ON_ONCE(!vcpu->kvm->arch.mirror_root_level))
> > +		return vcpu->kvm->arch.mirror_root_level;
> 
> [Severity: High]
> Does this regression allow a malicious or buggy userspace to trigger this
> WARN_ON_ONCE and cause a host denial of service (especially with
> panic_on_warn=1)?

No, because panic_on_warn=1 isn't a DoS.  But yeah, the WARN is reachable, because
as Sashiko points out, the check on TD_STATE_INITIALIZED happens too late.  Given
that CPUID _can't_ be set yet, consuming per-vCPU information in __kvm_mmu_create()
is bizarre (it's a leftover from what APIs were available at the time).

I'm pretty sure we can sqaush the WARN and make the code more reasonable at the
same time by doing:

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 611c78be8b41..d43b8b6c3b17 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6837,7 +6837,7 @@ static int __kvm_mmu_create(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, struct k
 	 * other exception is for shadowing L1's 32-bit or PAE NPT on 64-bit
 	 * KVM; that horror is handled on-demand by mmu_alloc_special_roots().
 	 */
-	if (tdp_enabled && kvm_mmu_get_tdp_level(vcpu) > PT32E_ROOT_LEVEL)
+	if (tdp_enabled && kvm_mmu_get_max_tdp_level() > PT32E_ROOT_LEVEL)
 		return 0;
 
 	page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_DMA32);

> If userspace creates a TDX VM via KVM_CREATE_VM (setting the VM type to
> KVM_X86_TDX_VM) and then directly calls KVM_CREATE_VCPU without executing
> KVM_TDX_INIT_VM first, it appears this warning will be reached.
> 
> During vCPU creation, the MMU context is initialized unconditionally before
> the architecture-specific vCPU creation callback:
> 
> kvm_arch_vcpu_create()->kvm_mmu_create()->kvm_mmu_get_tdp_level()
> 
> Because KVM_TDX_INIT_VM was skipped, mirror_root_level remains 0, satisfying
> the condition and triggering the kernel warning.
> 
> The architecture-specific callback that enforces the TD_STATE_INITIALIZED
> requirement is invoked too late in the process to prevent this:
> 
> kvm_arch_vcpu_create()->kvm_x86_call(vcpu_create)()
> 
> Would it be safer to return an error or handle the uninitialized state rather
> than triggering a host-side warning on user-controllable input?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260826215258.937210-1-seanjc@google.com?part=1