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

[PATCH] 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.

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, and
the value is queried fairly often and in hot paths.

Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Binbin Wu <binbin.wu@linux.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")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---

Compile-tested only, and found by inspection, i.e. I haven't proven that this
works, or that there's actually a bug.  But I'm pretty sure there's a bug.

Came across this when looking at our out-of-tree intrahost migration code, and
wanted to assert that could be at most one mirror root per TDX VM.

 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/cpuid.c            | 14 --------------
 arch/x86/kvm/cpuid.h            |  1 -
 arch/x86/kvm/mmu.h              |  9 +++++++++
 arch/x86/kvm/mmu/mmu.c          | 16 ++++++++--------
 arch/x86/kvm/vmx/tdx.c          | 12 +++---------
 6 files changed, 21 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.h b/arch/x86/kvm/mmu.h
index 2ae7f9ed4cf8..cc37e210787c 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -389,6 +389,15 @@ static inline gpa_t kvm_translate_gpa(struct kvm_vcpu *vcpu,
 						     exception, pte_access);
 }
 
+static __always_inline void kvm_mmu_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 inline bool kvm_has_mirrored_tdp(const struct kvm *kvm)
 {
 	return kvm->arch.vm_type == KVM_X86_TDX_VM;
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c519e8e8d646..a51a852c1bec 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5928,19 +5928,19 @@ 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 fixed as the gfn
+	 * used to select between the normal and mirror root must be covered.
+	 */
+	if (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..6d8e9befd9e6 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);
@@ -2823,9 +2817,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;
+		kvm_mmu_set_mirror_root_level(kvm, 5);
 	else
-		kvm->arch.gfn_direct_bits = TDX_SHARED_BIT_PWL_4;
+		kvm_mmu_set_mirror_root_level(kvm, 4);
 
 	kvm_tdx->state = TD_STATE_INITIALIZED;
 out:

base-commit: 1b731e5ded480bd1e5546aed35584238661ce72e
-- 
2.55.0.691.gc56d675ccc-goog

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

[Edgecombe, Rick P]

On Fri, 2026-08-14 at 15:45 -0700, Sean Christopherson wrote:
> 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.

Hmm, yea. But to me the text is a little ambiguous what "configuring guest
CPUID" means. There are the two configurations of CPUID that happen and the goof
was due to forgetting that there is no enforcement between the first "directly
configurable bits" (where "userspace has already told KVM the root level during
TD initialization" happens), and the second that happens via normal SET_CPUID.

Doing a KVM_BUG_ON() if tdx code sees a different level than what was processed
in setup_tdparams_eptp_controls() seems good to me.

> 
> 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, and
> the value is queried fairly often and in hot paths.

gfn_direct_bits comes directly from the the initial configuration, so why do we
need to add mirror_root_level in this patch? The old code calculated shared bit
with a conditional, so we could easily compute level with an inverted
conditional. The mirror_root_level caching is then a separate
cleanup/enhancments.

Ohhh, because to calculate it kvm_mmu_get_tdp_level() would embed some TDX
specifics there.

Wait, no, this knowledge embeds in kvm_mmu_set_mirror_root_level() anyway. So
I'd think to just have the below. If comparing gfn_direct_bits to
gfn_direct_bits doesn't make sense, then let's just drop the KVM_BUG_ON().


 arch/x86/kvm/cpuid.c   | 14 --------------
 arch/x86/kvm/cpuid.h   |  1 -
 arch/x86/kvm/mmu.h     |  5 +++++
 arch/x86/kvm/mmu/mmu.c | 16 ++++++++--------
 4 files changed, 13 insertions(+), 23 deletions(-)

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.h b/arch/x86/kvm/mmu.h
index 2ae7f9ed4cf8..e950f656007e 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -389,6 +389,11 @@ static inline gpa_t kvm_translate_gpa(struct kvm_vcpu
*vcpu,
                                                     exception, pte_access);
 }
 
+static inline unsigned int kvm_mmu_get_mirror_root_level(struct kvm *kvm)
+{
+       return kvm->arch.gfn_direct_bits == BIT_ULL(47) ? 4 : 5;
+}
+
 static inline bool kvm_has_mirrored_tdp(const struct kvm *kvm)
 {
        return kvm->arch.vm_type == KVM_X86_TDX_VM;
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 064ecc33b926..025871403597 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5953,19 +5953,19 @@ 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 fixed as the gfn
+        * used to select between the normal and mirror root must be covered.
+        */
+       if (vcpu->kvm->arch.gfn_direct_bits)
+               return kvm_mmu_get_mirror_root_level(vcpu->kvm);
+
        /* 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;


> 
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Binbin Wu <binbin.wu@linux.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")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> 
> Compile-tested only, and found by inspection, i.e. I haven't proven that this
> works, or that there's actually a bug.  But I'm pretty sure there's a bug.
> 
> Came across this when looking at our out-of-tree intrahost migration code, and
> wanted to assert that could be at most one mirror root per TDX VM.

I regression tested it in our CI. I don't have a TDX setup the past two days to
try to reproduce it specifically. By inspection it also looks like a bug to me.

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

[Sean Christopherson]

On Wed, Aug 19, 2026, Rick P Edgecombe wrote:
> On Fri, 2026-08-14 at 15:45 -0700, Sean Christopherson wrote:
> > 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.
> 
> Hmm, yea. But to me the text is a little ambiguous what "configuring guest
> CPUID" means. There are the two configurations of CPUID that happen and the goof
> was due to forgetting that there is no enforcement between the first "directly
> configurable bits" (where "userspace has already told KVM the root level during
> TD initialization" happens), and the second that happens via normal SET_CPUID.
> 
> Doing a KVM_BUG_ON() if tdx code sees a different level than what was processed
> in setup_tdparams_eptp_controls() seems good to me.
> 
> > 
> > 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, and
> > the value is queried fairly often and in hot paths.
> 
> gfn_direct_bits comes directly from the the initial configuration, so why do we
> need to add mirror_root_level in this patch? The old code calculated shared bit
> with a conditional, so we could easily compute level with an inverted
> conditional. The mirror_root_level caching is then a separate
> cleanup/enhancments.
> 
> Ohhh, because to calculate it kvm_mmu_get_tdp_level() would embed some TDX
> specifics there.
> 
> Wait, no, this knowledge embeds in kvm_mmu_set_mirror_root_level() anyway. So
> I'd think to just have the below. 

I started with that, but I didn't like bleeding that level of detail into the
MMU.  Or rather, I didn't like baking in the assumption that there is exactly
one "direct bits", that the one bit is a pivot between normal and mirror root,
and that the pivot bit is the most significant bit of the effective GPA space.

On the other hand, the MMU already knows about mirror roots, and needs to know
that mirror roots can have predetermined levels, so explicitly storing that level
doesn't add new assumptions.

> If comparing gfn_direct_bits to gfn_direct_bits doesn't make sense, then
> let's just drop the KVM_BUG_ON().

Why?  Defense in depth is often useful.