Re: [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory

[Sean Christopherson <seanjc@google.com>]

On Mon, Jul 06, 2026, Rick P Edgecombe wrote:
> On Fri, 2026-07-03 at 11:15 +0800, Binbin Wu wrote:
> > > @@ -1669,16 +1683,29 @@ static struct page *tdx_spte_to_sept_pt(struct kvm
> > > *kvm, gfn_t gfn,
> > >   static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
> > >   				     enum pg_level level, u64 new_spte)
> > >   {
> > > +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> > > +	struct vcpu_tdx *tdx = to_tdx(vcpu);
> > 
> > Nit:
> > Is it better to move this after checking vcpu is not NULL?
> > Although tdx is not dereferenced in between, if vcpu is NULL,
> > it means container_of() does arithmetic to a NULL pointer.
> 
> Personally I'm on the fence. I'm going to leave it, because Sean did it that
> way:
> https://lore.kernel.org/kvm/20260129011517.3545883-23-seanjc@google.com/

Heh, because of course I'm omnipotent and never make mistakes.

I agree with the nit in the sense that this isn't a great pattern to encourage,
but IMO the root of the ugly pattern is the use of kvm_get_running_vcpu() (which
is sadly the lesser evil in this case).  I.e. I'm not terribly concerned about
this code leading to more "problems" in the future.  And I don't really want to
grab "tdx" later on because that deviates from the standard patterns in KVM and
incorrectly suggests there _is_ a need to pre-check for a non-NULL vCPU.

All that said, we can get the bost of both words by simply not caching "tdx";
there's only one use anyways.

diff --git arch/x86/kvm/vmx/tdx.c arch/x86/kvm/vmx/tdx.c
index ee073cacafbe..f53bac52449b 100644
--- arch/x86/kvm/vmx/tdx.c
+++ arch/x86/kvm/vmx/tdx.c
@@ -1684,7 +1684,6 @@ static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
                                     enum pg_level level, u64 new_spte)
 {
        struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
-       struct vcpu_tdx *tdx = to_tdx(vcpu);
        gpa_t gpa = gfn_to_gpa(gfn);
        u64 err, entry, level_state;
        struct page *sept_pt;
@@ -1697,7 +1696,7 @@ static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
        if (!sept_pt)
                return -EIO;
 
-       ret = tdx_pamt_get(page_to_pfn(sept_pt), &tdx->pamt_cache);
+       ret = tdx_pamt_get(page_to_pfn(sept_pt), &to_tdx(vcpu)->pamt_cache);
        if (ret)
                return ret;
 
@@ -1721,7 +1720,6 @@ static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level leve
        struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
        struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
        kvm_pfn_t pfn = spte_to_pfn(new_spte);
-       struct vcpu_tdx *tdx = to_tdx(vcpu);
        int ret;
 
        if (KVM_BUG_ON(!vcpu, kvm))
@@ -1733,7 +1731,7 @@ static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level leve
 
        WARN_ON_ONCE((new_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
 
-       ret = tdx_pamt_get(pfn, &tdx->pamt_cache);
+       ret = tdx_pamt_get(pfn, &to_tdx(vcpu)->pamt_cache);
        if (ret)
                return ret;