Thread (12 messages) 12 messages, 4 authors, 2026-08-20

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

From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Date: 2026-08-19 17:35:23
Also in: kvm, lkml

On Wed, 2026-08-19 at 08:56 -0700, Sean Christopherson wrote:
FWIW, I don't view keying off gfn_direct_bits as being simpler.  It might be
less code, but conceptually it's more complex when reading
kvm_mmu_get_tdp_level(). E.g. the comment would need to explain the connection
between "direct bits" and the mirror root level, which most non-TDX readers
simply won't care about.

Hmm, but the comment I provided isn't very good either, as it too bleeds in
details about the S-bit pivot, and at the end of the day that's not the true
reason why the mirror root has/needs a predefined level.  The true reason is
very simple: KVM needs to mirror the external page tables, and obviously that
means using the same number of levels.

	/*
	 * 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 (vcpu->kvm->arch.mirror_root_level)
		return vcpu->kvm->arch.mirror_root_level;

But IMO that's a moot point, because this isn't a matter of simple vs.
complex. Keying of gfn_direct_bits is wrong/flawed, so whether or not it's
simpler is irrelevant.
I was just thinking that the patch was kind of doing two things with one change.
quoted hunk ↗ jump to hunk
All in all, this?

From: Sean Christopherson <seanjc@google.com>
Date: Fri, 14 Aug 2026 07:50:32 -0700
Subject: [PATCH] KVM: VMX: Explicitly track TDX VMs' root level instead of
 guessing it from CPUID

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 buring in TDX would require a kvm_x86_ops hook, and
the value is queried fairly often and in hot paths.

Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Xiaoyao Li <redacted>
Cc: Binbin Wu <redacted>
Cc: Kai Huang <redacted>
Cc: Yan Zhao <redacted>
Fixes: 20d913729c11 ("KVM: x86/mmu: Taking guest pa into consideration when
calculate tdp level")
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          | 18 ++++++++++--------
 arch/x86/kvm/vmx/tdx.c          | 20 +++++++++++---------
 5 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 283847619ff8..b2a74c69cc4a 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 c519e8e8d646..c78897510a1e 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5928,19 +5928,21 @@ 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 (vcpu->kvm->arch.mirror_root_level)
Elsewhere we use kvm_has_mirrored_tdp(vcpu->kvm) for these kind of checks. Would
be nice to be consistent and not add any uncertainty of whether
mirror_root_level can be set without kvm_has_mirrored_tdp() being true.
quoted hunk ↗ jump to hunk
+		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..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-
quoted
kvm))
+	if (KVM_BUG_ON(pgd_level != vcpu->kvm->arch.mirror_root_level, vcpu-
quoted
kvm))
 		return;
 
 	td_vmcs_write64(to_tdx(vcpu), SHARED_EPT_POINTER, root_hpa);
@@ -2760,6 +2754,14 @@ 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;
+	kvm->arch.gfn_direct_bits = gpa_to_gfn(BIT_ULL(level == 4 ? 47 :
51));
No need to remove TDX_SHARED_BIT_PWL_4/5 in this patch either anymore. Since
this lives in TDX code.

Otherwise,

Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
quoted hunk ↗ jump to hunk
+}
+
 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-
quoted
data);
@@ -2823,9 +2825,9 @@ static int tdx_td_init(struct kvm *kvm, struct
kvm_tdx_cmd *cmd)
 	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;
+		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:

base-commit: 1b731e5ded480bd1e5546aed35584238661ce72e
--
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help