* Re: [RFC PATCH v5 16/45] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Sean Christopherson @ 2026-02-05 22:35 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYQ0l+C42gssMHHV@yzhao56-desk.sh.intel.com>
On Thu, Feb 05, 2026, Yan Zhao wrote:
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index f6e80aba5895..682c8a228b53 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1824,6 +1824,50 @@ u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data)
> > }
> > EXPORT_SYMBOL_FOR_KVM(tdh_mng_rd);
> >
> > +/* Number PAMT pages to be provided to TDX module per 2M region of PA */
> > +static int tdx_dpamt_entry_pages(void)
> > +{
> > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > + return 0;
> > +
> This function is not invoked when !tdx_supports_dynamic_pamt().
> So, probably we can just return the count below?
Or maybe WARN_ON_ONCE() and return 0? I have no strong preference.
> > + return tdx_sysinfo.tdmr.pamt_4k_entry_size * PTRS_PER_PTE / PAGE_SIZE;
> > +}
> > +
>
^ permalink raw reply
* Re: [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Sean Christopherson @ 2026-02-05 22:38 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYRBE1tICOiQ/RL0@yzhao56-desk.sh.intel.com>
On Thu, Feb 05, 2026, Yan Zhao wrote:
> On Wed, Feb 04, 2026 at 05:45:39PM +0800, Yan Zhao wrote:
> > On Wed, Jan 28, 2026 at 05:14:41PM -0800, Sean Christopherson wrote:
> > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > index d12ca0f8a348..b35a07ed11fb 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -1858,8 +1858,8 @@ struct kvm_x86_ops {
> > > u64 mirror_spte);
> > >
> > > /* Update external page tables for page table about to be freed. */
> > > - int (*free_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > > - void *external_spt);
> > > + void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> > > + struct kvm_mmu_page *sp);
> > Do you think "free" is still better than "reclaim" though TDX actually
> > invokes tdx_reclaim_page() to reclaim it on the TDX side?
> >
> > Naming it free_external_sp can be interpreted as freeing the sp->external_spt
> > externally (vs freeing it in tdp_mmu_free_sp_rcu_callback(). This naming also
> > allows for the future possibility of freeing sp->external_spt before the HKID is
> > freed (though this is unlikely).
> Oh. I found there's a free_external_sp() in patch 20.
>
> So, maybe reclaim_external_sp() --> remove_external_spt() ?
>
> Still think "sp" is not good :)
I think my vote would be for reclaim_external_spt(). I don't like "remove", because
similar to "free", I think most readers will assume success is guaranteed.
^ permalink raw reply
* Re: [RFC PATCH v5 06/45] KVM: x86/mmu: Fold set_external_spte_present() into its sole caller
From: Sean Christopherson @ 2026-02-05 23:06 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYL3izez+eZ34G/3@yzhao56-desk.sh.intel.com>
On Wed, Feb 04, 2026, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:14:38PM -0800, Sean Christopherson wrote:
> > @@ -626,6 +599,8 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> > struct tdp_iter *iter,
> > u64 new_spte)
> > {
> > + u64 *raw_sptep = rcu_dereference(iter->sptep);
> > +
> > /*
> > * The caller is responsible for ensuring the old SPTE is not a FROZEN
> > * SPTE. KVM should never attempt to zap or manipulate a FROZEN SPTE,
> > @@ -638,31 +613,46 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> > int ret;
> >
> > /*
> > - * Users of atomic zapping don't operate on mirror roots,
> > - * so don't handle it and bug the VM if it's seen.
> > + * KVM doesn't currently support zapping or splitting mirror
> > + * SPTEs while holding mmu_lock for read.
> > */
> > - if (KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> > + if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
> > + KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> > return -EBUSY;
> Should this be -EIO instead?
Yeah, probably.
> Though -EBUSY was introduced by commit 94faba8999b9 ('KVM: x86/tdp_mmu:
> Propagate tearing down mirror page tables')
>
> > - ret = set_external_spte_present(kvm, iter->sptep, iter->gfn,
> > - &iter->old_spte, new_spte, iter->level);
> Add "lockdep_assert_held(&kvm->mmu_lock)" for this case?
No, because I don't want to unnecessarily bleed TDX details into common MMU. Ah,
but there was a pre-existing lockdep in set_external_spte_present(). So I guess
that's arguably a functional change and should be called out in the changelog.
But I still want to drop the assertion (or maybe move it to TDX in a prep patch),
because ultimately the requirements around locking come from TDX, not from the
TDP MMU.
^ permalink raw reply
* Re: [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Sean Christopherson @ 2026-02-05 23:14 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYLqESiqkADxMGf9@yzhao56-desk.sh.intel.com>
On Wed, Feb 04, 2026, Yan Zhao wrote:
> On Tue, Feb 03, 2026 at 08:05:05PM +0000, Sean Christopherson wrote:
> > On Tue, Feb 03, 2026, Yan Zhao wrote:
> > > On Wed, Jan 28, 2026 at 05:14:37PM -0800, Sean Christopherson wrote:
> > > > static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
> > > > gfn_t gfn, u64 *old_spte,
> > > > u64 new_spte, int level)
> > > > {
> > > > - bool was_present = is_shadow_present_pte(*old_spte);
> > > > - bool is_present = is_shadow_present_pte(new_spte);
> > > > - bool is_leaf = is_present && is_last_spte(new_spte, level);
> > > > - int ret = 0;
> > > > -
> > > > - KVM_BUG_ON(was_present, kvm);
> > > > + int ret;
> > > >
> > > > lockdep_assert_held(&kvm->mmu_lock);
> > > > +
> > > > + if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> > > > + return -EIO;
> > > Why not move this check of is_shadow_present_pte() to tdx_sept_set_private_spte()
> > > as well?
> >
> > The series gets there eventually, but as of this commit, @old_spte isn't plumbed
> > into tdx_sept_set_private_spte().
> >
> > > Or also check !is_shadow_present_pte(new_spte) in TDP MMU?
> >
> > Not sure I understand this suggestion.
> Sorry. The accurate expression should be
> "what about moving !is_shadow_present_pte(new_spte) to TDP MMU as well?".
It's already there, in __tdp_mmu_set_spte_atomic():
/*
* KVM doesn't currently support zapping or splitting mirror
* SPTEs while holding mmu_lock for read.
*/
if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
return -EBUSY;
> > > And as Rick also mentioned, better to remove external in external_spt, e.g.
> > > something like pt_page.
> >
> > Yeah, maybe sept_spt?
> Hmm, here sept_spt is of type struct page, while sp->spt and sp->external_spt
> represents VA. Not sure if it will cause confusion.
How about sept_pt?
^ permalink raw reply
* CCC TEE I/O Meeting Agenda for February 5th at 8pm (PST)
From: dan.j.williams @ 2026-02-05 23:19 UTC (permalink / raw)
To: linux-coco; +Cc: david.quigley, mathieu.poirier
The meeting [1] is on for tonight. I have a late breaking conflict and
will need to leave early. Please respond to this message with topics for
opens. I have asked Dave Quigley to chair in my absence. We may not get
to the "IOMMU setup consensus" topic depending on the evidence
collection ABI discussion. The other topics are short.
Agenda:
- Opens
- PCI power management vs DOE power management vs TSM operations
- See pm_runtime_resume_and_get() usage [2]
- What's cooking for devsec-20260205?
- Next steps for device evidence collection ABI (Dave and Mathieu)
- IOMMU setup consensus (continued from Discord conversation [3])
[1]: Registration: https://zoom-lfx.platform.linuxfoundation.org/meeting/93219227394?password=80705486-101a-4e19-8f11-0baaf92104be&invite=true
[2]: https://github.com/l1k/linux/commit/f8343c60e130
[3]: https://discord.com/channels/1123803528377413762/1468465977376378910
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Yan Zhao @ 2026-02-06 2:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUarHf3KEwHGuJe@google.com>
On Thu, Feb 05, 2026 at 02:33:16PM -0800, Sean Christopherson wrote:
> On Thu, Feb 05, 2026, Yan Zhao wrote:
> > On Wed, Feb 04, 2026 at 06:23:38PM -0800, Sean Christopherson wrote:
> > > On Wed, Feb 04, 2026, Yan Zhao wrote:
> > > > On Wed, Jan 28, 2026 at 05:14:40PM -0800, Sean Christopherson wrote:
> > > > > @@ -590,10 +566,21 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> > > > > * the paging structure. Note the WARN on the PFN changing without the
> > > > > * SPTE being converted to a hugepage (leaf) or being zapped. Shadow
> > > > > * pages are kernel allocations and should never be migrated.
> > > > > + *
> > > > > + * When removing leaf entries from a mirror, immediately propagate the
> > > > > + * changes to the external page tables. Note, non-leaf mirror entries
> > > > > + * are handled by handle_removed_pt(), as TDX requires that all leaf
> > > > > + * entries are removed before the owning page table. Note #2, writes
> > > > > + * to make mirror PTEs shadow-present are propagated to external page
> > > > > + * tables by __tdp_mmu_set_spte_atomic(), as KVM needs to ensure the
> > > > > + * external page table was successfully updated before marking the
> > > > > + * mirror SPTE present.
> > > > > */
> > > > > if (was_present && !was_leaf &&
> > > > > (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> > > > > handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> > > > > + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
> > > > Should we check !is_present instead of !is_leaf?
> > > > e.g. a transition from a present leaf entry to a present non-leaf entry could
> > > > also trigger this if case.
> > >
> > > No, the !is_leaf check is very intentional. At this point in the series, S-EPT
> > > doesn't support hugepages. If KVM manages to install a leaf SPTE and replaces
> > > that SPTE with a non-leaf SPTE, then we absolutely want the KVM_BUG_ON() in
> > > tdx_sept_remove_private_spte() to fire:
> > >
> > > /* TODO: handle large pages. */
> > > if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> > > return -EIO;
> > But the op is named remove_external_spte().
> > And the check of "level != PG_LEVEL_4K" is for removing large leaf entries.
>
> I agree that the naming at this point in the series is unfortunate, but I don't
> see it as outright wrong. That the TDP MMU could theoretically replace the leaf
> SPTE with a non-leaf SPTE doesn't change the fact that the old leaf SPTE *is*
> being removed.
Hmm, I can't agree with that. But I won't insist if you think it's ok :)
> > Relying on this check is tricky and confusing.
>
> If it's still confusing at the end of the series, then I'm happy to discuss how
> we can make it less confusion. But as of this point in the series, I unfortunately
> don't see a better way to achieve my end goals (reducing the number of kvm_x86_ops
> hooks, and reducing how many TDX specific details bleed into common MMU code).
>
> There are "different" ways to incrementally move from where were at today, to where
> I want KVM to be, but I don't see them as "better". I.e. AFAICT, there's no way
> to move incrementally with reviewable patches while also maintaining perfect/ideal
> naming and flow.
Ok.
> > > And then later on, when S-EPT gains support for hugepages, "KVM: TDX: Add core
> > > support for splitting/demoting 2MiB S-EPT to 4KiB" doesn't need to touch code
> > > outside of arch/x86/kvm/vmx/tdx.c, because everything has already been plumbed
> > > in.
> > I haven't looked at the later patches for huge pages,
>
> Please do. As above, I don't think it's realistic to completely avoid some amount
> of "eww" in the intermediate stages.
WIP. Found some issues. Will comment after investigating further.
> > but plumbing here directly for splitting does not look right when it's
> > invoked under shared mmu_lock.
> > See the comment below.
> >
> > > > Besides, need "KVM_BUG_ON(shared, kvm)" in this case.
> > >
> > > Eh, we have lockdep_assert_held_write() in the S-EPT paths that require mmu_lock
> > > to be held for write. I don't think a KVM_BUG_ON() here would add meaningful
> > > value.
> > Hmm, I think KVM_BUG_ON(shared, kvm) is still useful.
> > If KVM invokes remove_external_spte() under shared mmu_lock, it needs to freeze
> > the entry first, similar to the sequence in __tdp_mmu_set_spte_atomic().
> >
> > i.e., invoking external x86 ops in handle_changed_spte() for mirror roots should
> > be !shared only.
>
> Sure, but...
>
> > Relying on the TDX code's lockdep_assert_held_write() for warning seems less
> > clear than having an explicit check here.
>
> ...that's TDX's responsibility to enforce, and I don't see any justification for
> something more than a lockdep assertion. As I've said elsewhere, several times,
My concern is that handle_changed_spte() can be invoked by callers other than
tdp_mmu_set_spte(). e.g.,
tdp_mmu_set_spte_atomic
| __tdp_mmu_set_spte_atomic
| | kvm_x86_call(set_external_spte)
| handle_changed_spte
| kvm_x86_call(set_external_spte)
When !is_frozen_spte(new_spte) and was_leaf, set_external_spte() may be invoked
twice for splitting under shared mmu_lock.
Therefore, I think it would be better to check for !shared and only invoke
set_external_spte() when !shared.
BTW: in the patch log, you mentioned that
: Invoke .remove_external_spte() in handle_changed_spte() as appropriate
: instead of relying on callers to do the right thing. Relying on callers
: to invoke .remove_external_spte() is confusing and brittle, e.g. subtly
: relies tdp_mmu_set_spte_atomic() never removing SPTEs, and removing an
: S-EPT entry in tdp_mmu_set_spte() is bizarre (yeah, the VM is bugged so
: it doesn't matter in practice, but it's still weird).
However, when tdp_mmu_set_spte_atomic() removes SPTEs in the future, it will
still need to follow the sequence in __tdp_mmu_set_spte_atomic() :
1. freeze, 2. set_external_spte(). 3. set the new_spte 4. handle_changed_spte().
So, do you think we should leave set_external_spte() in tdp_mmu_set_spte() for
exclusive mmu_lock scenarios instead of moving it to handle_changed_spte()?
> at some point we have to commit to getting the code right. Adding KVM_BUG_ON() in
> Every. Single. Call. does not yield more maintainable code. There are myriad
> things KVM can screw up, many of which have far, far more harmful impact than
> calling an S-EPT hook with mmu_lock held for read instead of write.
>
> The bar for adding a KVM_BUG_ON() is not simply "this shouldn't happen". It's,
> this shouldn't happen *and* at least one of (not a complete list):
>
> - we've either screwed this up badly more than once
> - it's really hard to get right
> - we might not notice if we do screw it up
> - KVM might corrupt data if we continue on
Thanks for sharing this bar.
^ permalink raw reply
* Re: [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Yan Zhao @ 2026-02-06 2:27 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUkVRedz9ngwu_1@google.com>
On Thu, Feb 05, 2026 at 03:14:29PM -0800, Sean Christopherson wrote:
> On Wed, Feb 04, 2026, Yan Zhao wrote:
> > On Tue, Feb 03, 2026 at 08:05:05PM +0000, Sean Christopherson wrote:
> > > On Tue, Feb 03, 2026, Yan Zhao wrote:
> > > > On Wed, Jan 28, 2026 at 05:14:37PM -0800, Sean Christopherson wrote:
> > > > > static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
> > > > > gfn_t gfn, u64 *old_spte,
> > > > > u64 new_spte, int level)
> > > > > {
> > > > > - bool was_present = is_shadow_present_pte(*old_spte);
> > > > > - bool is_present = is_shadow_present_pte(new_spte);
> > > > > - bool is_leaf = is_present && is_last_spte(new_spte, level);
> > > > > - int ret = 0;
> > > > > -
> > > > > - KVM_BUG_ON(was_present, kvm);
> > > > > + int ret;
> > > > >
> > > > > lockdep_assert_held(&kvm->mmu_lock);
> > > > > +
> > > > > + if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> > > > > + return -EIO;
> > > > Why not move this check of is_shadow_present_pte() to tdx_sept_set_private_spte()
> > > > as well?
> > >
> > > The series gets there eventually, but as of this commit, @old_spte isn't plumbed
> > > into tdx_sept_set_private_spte().
> > >
> > > > Or also check !is_shadow_present_pte(new_spte) in TDP MMU?
> > >
> > > Not sure I understand this suggestion.
> > Sorry. The accurate expression should be
> > "what about moving !is_shadow_present_pte(new_spte) to TDP MMU as well?".
>
> It's already there, in __tdp_mmu_set_spte_atomic():
>
> /*
> * KVM doesn't currently support zapping or splitting mirror
> * SPTEs while holding mmu_lock for read.
> */
> if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
> KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> return -EBUSY;
Ok. I was wondering why we don't include it directly in this patch, but it
doesn't matter.
> > > > And as Rick also mentioned, better to remove external in external_spt, e.g.
> > > > something like pt_page.
> > >
> > > Yeah, maybe sept_spt?
> > Hmm, here sept_spt is of type struct page, while sp->spt and sp->external_spt
> > represents VA. Not sure if it will cause confusion.
>
> How about sept_pt?
LGTM.
^ permalink raw reply
* Re: [RFC PATCH v5 06/45] KVM: x86/mmu: Fold set_external_spte_present() into its sole caller
From: Yan Zhao @ 2026-02-06 2:29 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUijQwl2Q6Q81DL@google.com>
> > > - ret = set_external_spte_present(kvm, iter->sptep, iter->gfn,
> > > - &iter->old_spte, new_spte, iter->level);
> > Add "lockdep_assert_held(&kvm->mmu_lock)" for this case?
>
> No, because I don't want to unnecessarily bleed TDX details into common MMU. Ah,
> but there was a pre-existing lockdep in set_external_spte_present(). So I guess
> that's arguably a functional change and should be called out in the changelog.
>
> But I still want to drop the assertion (or maybe move it to TDX in a prep patch),
> because ultimately the requirements around locking come from TDX, not from the
> TDP MMU.
LGTM.
^ permalink raw reply
* Re: [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Yan Zhao @ 2026-02-06 2:30 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUb0KvJynvYjr3h@google.com>
On Thu, Feb 05, 2026 at 02:38:08PM -0800, Sean Christopherson wrote:
> On Thu, Feb 05, 2026, Yan Zhao wrote:
> > On Wed, Feb 04, 2026 at 05:45:39PM +0800, Yan Zhao wrote:
> > > On Wed, Jan 28, 2026 at 05:14:41PM -0800, Sean Christopherson wrote:
> > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > > index d12ca0f8a348..b35a07ed11fb 100644
> > > > --- a/arch/x86/include/asm/kvm_host.h
> > > > +++ b/arch/x86/include/asm/kvm_host.h
> > > > @@ -1858,8 +1858,8 @@ struct kvm_x86_ops {
> > > > u64 mirror_spte);
> > > >
> > > > /* Update external page tables for page table about to be freed. */
> > > > - int (*free_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > > > - void *external_spt);
> > > > + void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> > > > + struct kvm_mmu_page *sp);
> > > Do you think "free" is still better than "reclaim" though TDX actually
> > > invokes tdx_reclaim_page() to reclaim it on the TDX side?
> > >
> > > Naming it free_external_sp can be interpreted as freeing the sp->external_spt
> > > externally (vs freeing it in tdp_mmu_free_sp_rcu_callback(). This naming also
> > > allows for the future possibility of freeing sp->external_spt before the HKID is
> > > freed (though this is unlikely).
> > Oh. I found there's a free_external_sp() in patch 20.
> >
> > So, maybe reclaim_external_sp() --> remove_external_spt() ?
> >
> > Still think "sp" is not good :)
>
> I think my vote would be for reclaim_external_spt(). I don't like "remove", because
> similar to "free", I think most readers will assume success is guaranteed.
Ok. reclaim_external_spt() looks good to me.
^ permalink raw reply
* Re: [RFC PATCH v5 16/45] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Yan Zhao @ 2026-02-06 2:32 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUbLVrxwDDZ2qh-@google.com>
On Thu, Feb 05, 2026 at 02:35:25PM -0800, Sean Christopherson wrote:
> On Thu, Feb 05, 2026, Yan Zhao wrote:
> > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > > index f6e80aba5895..682c8a228b53 100644
> > > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > > @@ -1824,6 +1824,50 @@ u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data)
> > > }
> > > EXPORT_SYMBOL_FOR_KVM(tdh_mng_rd);
> > >
> > > +/* Number PAMT pages to be provided to TDX module per 2M region of PA */
> > > +static int tdx_dpamt_entry_pages(void)
> > > +{
> > > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > > + return 0;
> > > +
> > This function is not invoked when !tdx_supports_dynamic_pamt().
> > So, probably we can just return the count below?
>
> Or maybe WARN_ON_ONCE() and return 0? I have no strong preference.
"WARN_ON_ONCE() and return 0" looks good to me.
> > > + return tdx_sysinfo.tdmr.pamt_4k_entry_size * PTRS_PER_PTE / PAGE_SIZE;
> > > +}
> > > +
> >
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Trilok Soni @ 2026-02-06 6:37 UTC (permalink / raw)
To: Salman Nabi, vvidwans, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <c29f4a15-1753-42ca-a4a4-ca429053163f@arm.com>
On 2/2/2026 7:52 AM, Salman Nabi wrote:
> Hi Trilok,
>
> On 1/31/26 01:35, Trilok Soni wrote:
>> On 1/19/2026 4:27 AM, Salman Nabi wrote:
>>> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
>>> activating firmware components without a reboot. Those components
>>> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
s/paylods/payloads
>>> usual way: via fwupd, FF-A or other secure storage methods, or via some
>>> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
>>> at system runtime, without requiring a reboot.
>>> The specification covers the SMCCC interface to list and query available
>>> components and eventually trigger the activation.
>>>
>>> Add a new directory under /sys/firmware to present firmware components
>>> capable of live activation. Each of them is a directory under lfa/,
>>> and is identified via its GUID. The activation will be triggered by echoing
>>> "1" into the "activate" file:
>>> ==========================================
>>> /sys/firmware/lfa # ls -l . 6c*
>>> .:
>>> total 0
>>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 47d4086d-4cfe-9846-9b95-2950cbbd5a00
>>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 6c0762a6-12f2-4b56-92cb-ba8f633606d9
>>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 d6d0eea7-fcea-d54b-9782-9934f234b6e4
>> Can you please explain or add a note on why we don't have name of the firmware
>> as the directory name and why you have selected GUID as top-level
>> directory name?
>
>
> We obtain the GUIDs of firmware components from the LFA agent in TF-A, which does not provide their names. For convenience, we have added a C structure in the driver to associate each firmware GUID with its corresponding name. Because new firmware components may be supported in the LFA agent before their GUID-to-name mapping is added to the driver, we avoid using the firmware name as the directory (kobject) name.
>
> Additionally, sysfs requires directory names to be unique among siblings. Since GUIDs are inherently unique, they provide a convenient, collision-free choice for directory names.
Thank you for the explanation. Please add these details in the commit text.
---Trilok Soni
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Yan Zhao @ 2026-02-06 9:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-21-seanjc@google.com>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 18764dbc97ea..01e3e4f4baa5 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -55,7 +55,8 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
>
> static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> {
> - free_page((unsigned long)sp->external_spt);
> + if (sp->external_spt)
> + kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
> free_page((unsigned long)sp->spt);
> kmem_cache_free(mmu_page_header_cache, sp);
> }
Strictly speaking, external_spt is not a control page. Its alloc/free are
different from normal control pages managed by TDX's code.
(1) alloc
tdx_alloc_control_page
__tdx_alloc_control_page
__tdx_pamt_get
spin_lock(&pamt_lock) ==> under process context
spin_unlock(&pamt_lock)
(2) free
tdp_mmu_free_sp_rcu_callback
tdp_mmu_free_sp
kvm_x86_call(free_external_sp)
tdx_free_control_page
__tdx_free_control_page
__tdx_pamt_put
spin_lock(&pamt_lock) ==> under softirq context
spin_unlock(&pamt_lock)
So, invoking __tdx_pamt_put() in the RCU callback triggers deadlock warning
(see the bottom for details).
> + /*
> + * TDX uses the external_spt cache to allocate S-EPT page table pages,
> + * which (a) don't need to be initialized by KVM as the TDX-Module will
> + * initialize the page (using the guest's encryption key), and (b) need
> + * to use a custom allocator to be compatible with Dynamic PAMT.
> + */
> + vt_x86_ops.alloc_external_sp = tdx_alloc_control_page;
> + vt_x86_ops.free_external_sp = tdx_free_control_page;
> +
> vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
> vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
> vt_x86_ops.remove_external_spte = tdx_sept_remove_private_spte;
================================
WARNING: inconsistent lock state
6.19.0-rc6-upstream+ #1078 Tainted: G S U
--------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/7/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
ffffffff9067b6f8 (pamt_lock){+.?.}-{3:3}, at: __tdx_pamt_put+0x80/0xf0
{SOFTIRQ-ON-W} state was registered at:
__lock_acquire+0x405/0xc10
lock_acquire.part.0+0x9c/0x210
lock_acquire+0x5e/0x100
_raw_spin_lock+0x37/0x80
__tdx_pamt_get+0xb8/0x150
__tdx_alloc_control_page+0x2e/0x60
__tdx_td_init+0x65/0x740 [kvm_intel]
tdx_td_init+0x147/0x240 [kvm_intel]
tdx_vm_ioctl+0x125/0x260 [kvm_intel]
vt_mem_enc_ioctl+0x17/0x30 [kvm_intel]
kvm_arch_vm_ioctl+0x4e0/0xb40 [kvm]
kvm_vm_ioctl+0x4f4/0xaf0 [kvm]
__x64_sys_ioctl+0x9d/0xf0
x64_sys_call+0xf38/0x1da0
do_syscall_64+0xc5/0xfc0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
irq event stamp: 252814
hardirqs last enabled at (252814): [<ffffffff8fa6f41a>] _raw_spin_unlock_irqrestore+0x5a/0x80
hardirqs last disabled at (252813): [<ffffffff8fa6f096>] _raw_spin_lock_irqsave+0x76/0x90
softirqs last enabled at (252798): [<ffffffff8e60f139>] handle_softirqs+0x309/0x460
softirqs last disabled at (252805): [<ffffffff8e60f401>] __irq_exit_rcu+0xe1/0x160
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(pamt_lock);
<Interrupt>
lock(pamt_lock);
*** DEADLOCK ***
1 lock held by swapper/7/0:
#0: ffffffff9077d660 (rcu_callback){....}-{0:0}, at: rcu_do_batch+0x153/0x620
stack backtrace:
CPU: 7 UID: 0 PID: 0 Comm: swapper/7 Tainted: G S U 6.19.0-rc6-upstream+ #1078 PREEMPT(voluntary) b8f4b38003dc2ca73352cf9d3d544aa826c4f5a9
Tainted: [S]=CPU_OUT_OF_SPEC, [U]=USER
Hardware name: Intel Corporation ArcherCity/ArcherCity, BIOS EGSDCRB1.SYS.0101.D29.2303301937 03/30/2023
Call Trace:
<IRQ>
show_stack+0x49/0x60
dump_stack_lvl+0x6f/0xb0
dump_stack+0x10/0x16
print_usage_bug.part.0+0x264/0x350
mark_lock_irq+0x4d6/0x9e0
? stack_trace_save+0x4a/0x70
? save_trace+0x66/0x2b0
mark_lock+0x1cf/0x6a0
mark_usage+0x4c/0x130
__lock_acquire+0x405/0xc10
? __this_cpu_preempt_check+0x13/0x20
lock_acquire.part.0+0x9c/0x210
? __tdx_pamt_put+0x80/0xf0
lock_acquire+0x5e/0x100
? __tdx_pamt_put+0x80/0xf0
_raw_spin_lock+0x37/0x80
? __tdx_pamt_put+0x80/0xf0
__tdx_pamt_put+0x80/0xf0
? __this_cpu_preempt_check+0x13/0x20
? sched_clock_noinstr+0x9/0x10
__tdx_free_control_page+0x22/0x40
tdx_free_control_page+0x38/0x50 [kvm_intel c135d3571385e160f086f9f6195fc72e4b6aa2b1]
tdp_mmu_free_sp_rcu_callback+0x24/0x50 [kvm 3932b137c28c130169e7e3615041bcec6cefc090]
? rcu_do_batch+0x1dc/0x620
rcu_do_batch+0x1e1/0x620
? rcu_do_batch+0x153/0x620
rcu_core+0x37d/0x4d0
rcu_core_si+0xe/0x20
handle_softirqs+0xdc/0x460
? hrtimer_interrupt+0x154/0x290
__irq_exit_rcu+0xe1/0x160
irq_exit_rcu+0xe/0x30
sysvec_apic_timer_interrupt+0xc0/0xf0
</IRQ>
<TASK>
asm_sysvec_apic_timer_interrupt+0x1b/0x20
RIP: 0010:cpuidle_enter_state+0x122/0x7a0
^ permalink raw reply
* Re: [RFC PATCH v5 37/45] KVM: x86/tdp_mmu: Alloc external_spt page for mirror page table splitting
From: Yan Zhao @ 2026-02-06 10:07 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-38-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:15:09PM -0800, Sean Christopherson wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Enhance tdp_mmu_alloc_sp_for_split() to allocate a page table page for the
> external page table for splitting the mirror page table.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Co-developed-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> [sean: use kvm_x86_ops.alloc_external_sp()]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 3b0da898824a..4f5b80f0ca03 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1447,7 +1447,7 @@ bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm,
> return spte_set;
> }
>
> -static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> +static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct tdp_iter *iter)
> {
> struct kvm_mmu_page *sp;
>
> @@ -1461,6 +1461,15 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> return NULL;
> }
>
> + if (is_mirror_sptep(iter->sptep)) {
tdp_mmu_alloc_sp_for_split() is invoked in tdp_mmu_split_huge_pages_root() after
rcu_read_unlock() is called.
So, it's incorrect to invoke is_mirror_sptep() which internally contains
rcu_dereference(), resulting in "WARNING: suspicious RCU usage".
> + sp->external_spt = (void *)kvm_x86_call(alloc_external_sp)(GFP_KERNEL_ACCOUNT);
> + if (!sp->external_spt) {
> + free_page((unsigned long)sp->spt);
> + kmem_cache_free(mmu_page_header_cache, sp);
> + return NULL;
> + }
> + }
> +
> return sp;
> }
>
> @@ -1540,7 +1549,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> else
> write_unlock(&kvm->mmu_lock);
>
> - sp = tdp_mmu_alloc_sp_for_split();
> + sp = tdp_mmu_alloc_sp_for_split(&iter);
>
> if (shared)
> read_lock(&kvm->mmu_lock);
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ permalink raw reply
* Re: [RFC PATCH v5 44/45] KVM: x86/mmu: Add support for splitting S-EPT hugepages on conversion
From: Yan Zhao @ 2026-02-06 10:14 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-45-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:15:16PM -0800, Sean Christopherson wrote:
> Add support for splitting S-EPT hugepages in preparation for converting a
> subset of a hugepage to be shared, as KVM must precisely zap/remove S-EPT
> entries to avoid clobbering guest memory (the lifetime of guest private
> memory is tied to the S-EPT). I.e. KVM needs to first split a hugepage so
> that only the to-be-converted small pages can be zapped.
>
> To avoid unnecessary work, e.g. if only the tail/end page of massive region
> isn't aligned to the conversion, explicitly detect unaligned head and tail
> pages relative to the max page size support by KVM, i.e. head/tail pages
> that will undergo partial conversion.
>
> To support splitting an S-EPT hugepage without a vCPU, add a per-VM PAMT
> cache, along with a mutex to guard the cache. Using a mutex, e.g. versus
> a spinlock, is important at it allows KVM to allocate memory *without*
> dropping the lock, i.e. so that the PAMT cache can be topped-up as needed
> without needed to juggle arch.tdp_mmu_external_cache_lock.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/kvm_host.h | 8 +++-
> arch/x86/kvm/mmu/mmu.c | 2 +-
> arch/x86/kvm/mmu/tdp_mmu.c | 72 +++++++++++++++++++++++++++++++--
> arch/x86/kvm/vmx/tdx.c | 34 +++++++++++++---
> arch/x86/kvm/vmx/tdx.h | 2 +
> 5 files changed, 107 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 385f1cf32d70..54dea90a53dc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1563,6 +1563,12 @@ struct kvm_arch {
> * the code to do so.
> */
> spinlock_t tdp_mmu_pages_lock;
> +
> + /*
> + * Protect the per-VM cache of pre-allocate pages used to populate the
> + * Dynamic PAMT when splitting S-EPT huge pages without a vCPU.
> + */
> + struct mutex tdp_mmu_external_cache_lock;
Missing "spin_lock_init(&kvm->arch.tdp_mmu_external_cache_lock);" in
kvm_mmu_init_tdp_mmu().
Will check the patch you replied next week.
> #endif /* CONFIG_X86_64 */
^ permalink raw reply
* Re: [RFC PATCH v5 22/45] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Yan Zhao @ 2026-02-06 10:20 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-23-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:54PM -0800, Sean Christopherson wrote:
> From: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>
> Add Dynamic PAMT support to KVM's S-EPT MMU by "getting" a PAMT page when
> adding guest memory (PAGE.ADD or PAGE.AUG), and "putting" the page when
> removing guest memory (PAGE.REMOVE).
>
> To access the per-vCPU PAMT caches without plumbing @vcpu throughout the
> TDP MMU, begrudginly use kvm_get_running_vcpu() to get the vCPU, and bug
> the VM If KVM attempts to set an S-EPT without an active vCPU. KVM only
> supports creating _new_ mappings in page (pre)fault paths, all of which
> require an active vCPU.
>
> The PAMT memory holds metadata for TDX-protected memory. With Dynamic
> PAMT, PAMT_4K is allocated on demand. The kernel supplies the TDX module
> with a few pages that cover 2M of host physical memory.
>
> PAMT memory can be reclaimed when the last user is gone. It can happen
> in a few code paths:
>
> - On TDH.PHYMEM.PAGE.RECLAIM in tdx_reclaim_td_control_pages() and
> tdx_reclaim_page().
>
> - On TDH.MEM.PAGE.REMOVE in tdx_sept_drop_private_spte().
>
> - In tdx_sept_zap_private_spte() for pages that were in the queue to be
> added with TDH.MEM.PAGE.ADD, but it never happened due to an error.
>
> - In tdx_sept_free_private_spt() for SEPT pages;
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [Minor log tweak]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 1 +
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/mmu/mmu.c | 4 +++
> arch/x86/kvm/vmx/tdx.c | 44 ++++++++++++++++++++++++++----
> arch/x86/kvm/vmx/tdx.h | 2 ++
> 5 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index 17dddada69fc..394dc29483a7 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -99,6 +99,7 @@ KVM_X86_OP_OPTIONAL(free_external_sp)
> KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
> KVM_X86_OP_OPTIONAL(remove_external_spte)
> KVM_X86_OP_OPTIONAL(reclaim_external_sp)
> +KVM_X86_OP_OPTIONAL_RET0(topup_external_cache)
> KVM_X86_OP(has_wbinvd_exit)
> KVM_X86_OP(get_l2_tsc_offset)
> KVM_X86_OP(get_l2_tsc_multiplier)
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 6e84dbc89e79..a6e4ab76b1b2 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1863,6 +1863,7 @@ struct kvm_x86_ops {
> struct kvm_mmu_page *sp);
> void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> u64 mirror_spte);
> + int (*topup_external_cache)(struct kvm_vcpu *vcpu, int min);
>
>
> bool (*has_wbinvd_exit)(void);
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 9b5a6861e2a4..4ecbf216d96f 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -605,6 +605,10 @@ static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu, bool maybe_indirect)
> PT64_ROOT_MAX_LEVEL);
> if (r)
> return r;
> +
> + r = kvm_x86_call(topup_external_cache)(vcpu, PT64_ROOT_MAX_LEVEL);
If this external cache is for PAMT pages allocation for guest pages only, here
the min count should be 1 instead of PT64_ROOT_MAX_LEVEL?
> + if (r)
> + return r;
> }
> r = kvm_mmu_topup_memory_cache(&vcpu->arch.mmu_shadow_page_cache,
> PT64_ROOT_MAX_LEVEL);
...
> void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
> @@ -3614,5 +3641,12 @@ void __init tdx_hardware_setup(void)
> vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
> vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
> vt_x86_ops.remove_external_spte = tdx_sept_remove_private_spte;
> +
> + /*
> + * FIXME: Wire up the PAMT hook iff DPAMT is supported, once VMXON is
> + * moved out of KVM and tdx_bringup() is folded into here.
> + */
> + vt_x86_ops.topup_external_cache = tdx_topup_external_pamt_cache;
> +
> vt_x86_ops.protected_apic_has_interrupt = tdx_protected_apic_has_interrupt;
> }
^ permalink raw reply
* Re: [PATCH v2 0/2] x86/virt/tdx: Print TDX module version to dmesg
From: Kiryl Shutsemau @ 2026-02-06 11:39 UTC (permalink / raw)
To: Verma, Vishal L, dave.hansen
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, Edgecombe, Rick P,
Huang, Kai, bp@alien8.de, mingo@redhat.com, Williams, Dan J,
tglx@linutronix.de, hpa@zytor.com, x86@kernel.org
In-Reply-To: <6d8d37740459963e6fd7f16a890a837b34ebdf17.camel@intel.com>
On Thu, Feb 05, 2026 at 07:05:39PM +0000, Verma, Vishal L wrote:
> On Fri, 2026-01-09 at 12:14 -0700, Vishal Verma wrote:
> > === Problem & Solution ===
> >
> > Currently, there is neither an ABI, nor any other way to determine from
> > the host system, what version of the TDX module is running. A sysfs ABI
> > for this has been proposed in [1], but it may need additional discussion.
> >
> > Many/most TDX developers already carry patches like this in their
> > development branches. It can be tricky to know which TDX module is
> > actually loaded on a system, and so this functionality has been needed
> > regularly for development and processing bug reports. Hence, it is
> > prudent to break out the patches to retrieve and print the TDX module
> > version, as those parts are very straightforward, and get some level of
> > debugability and traceability for TDX host systems.
> >
> > === Dependencies ===
> >
> > None. This is based on v6.19-rc4, and applies cleanly to tip.git.
> >
> > === Patch details ===
> >
> > Patch 1 is a prerequisite that adds the infrastructure to retrieve the
> > TDX module version from its global metadata. This was originally posted in [2].
> >
> > Patch 2 is based on a patch from Kai Huang [3], and prints the version to
> > dmesg during init.
> >
> > === Testing ===
> >
> > This has passed the usual suite of tests, including successful 0day
> > builds, KVM Unit tests, KVM selftests, a TD creation smoke test, and
> > selected KVM tests from the Avocado test suite.
> >
> > [1]: https://lore.kernel.org/all/20260105074350.98564-1-chao.gao@intel.com/
> > [2]: https://lore.kernel.org/all/20260105074350.98564-2-chao.gao@intel.com/
> > [3]: https://lore.kernel.org/all/57eaa1b17429315f8b5207774307f3c1dd40cf37.1730118186.git.kai.huang@intel.com/
> >
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
>
> Hi Kiryl, just wanted to check on the plan for this, I didn't see it
> merged in tip.git x86/tdx (or any other tip branch). Were you planning
> to take it through x86/tdx? Can I help with anything to move it along?
I guess it has to wait after the merge window at this point.
Dave, could you queue this after -rc1 is tagged?
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Kiryl Shutsemau @ 2026-02-06 12:03 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Pratik R. Sampat, linux-mm, linux-coco, x86, linux-kernel, tglx,
mingo, bp, dave.hansen, ardb, akpm, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <fc60cc71-80eb-4058-8d0a-dbd19a5c10ef@kernel.org>
On Thu, Feb 05, 2026 at 06:29:08PM +0100, David Hildenbrand (Arm) wrote:
> > Ideally, we want to know on boot:
> >
> > - what memory ranges are unaccepted - we have it;
> > - what memory range can be removed or added after boot - we don't have it
>
> The SRAT describes memory ranges where we can see hotplug memory. Is that
> too late? We calculate max_possible_pfn based on that.
The cleanest way would be to declare the ranges in EFI memory map, not
SRAT. It should be doable.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH v5 44/45] KVM: x86/mmu: Add support for splitting S-EPT hugepages on conversion
From: Sean Christopherson @ 2026-02-06 14:46 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYW+6WzOmCAvNRIH@yzhao56-desk.sh.intel.com>
On Fri, Feb 06, 2026, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:15:16PM -0800, Sean Christopherson wrote:
> > Add support for splitting S-EPT hugepages in preparation for converting a
> > subset of a hugepage to be shared, as KVM must precisely zap/remove S-EPT
> > entries to avoid clobbering guest memory (the lifetime of guest private
> > memory is tied to the S-EPT). I.e. KVM needs to first split a hugepage so
> > that only the to-be-converted small pages can be zapped.
> >
> > To avoid unnecessary work, e.g. if only the tail/end page of massive region
> > isn't aligned to the conversion, explicitly detect unaligned head and tail
> > pages relative to the max page size support by KVM, i.e. head/tail pages
> > that will undergo partial conversion.
> >
> > To support splitting an S-EPT hugepage without a vCPU, add a per-VM PAMT
> > cache, along with a mutex to guard the cache. Using a mutex, e.g. versus
> > a spinlock, is important at it allows KVM to allocate memory *without*
> > dropping the lock, i.e. so that the PAMT cache can be topped-up as needed
> > without needed to juggle arch.tdp_mmu_external_cache_lock.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/include/asm/kvm_host.h | 8 +++-
> > arch/x86/kvm/mmu/mmu.c | 2 +-
> > arch/x86/kvm/mmu/tdp_mmu.c | 72 +++++++++++++++++++++++++++++++--
> > arch/x86/kvm/vmx/tdx.c | 34 +++++++++++++---
> > arch/x86/kvm/vmx/tdx.h | 2 +
> > 5 files changed, 107 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 385f1cf32d70..54dea90a53dc 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1563,6 +1563,12 @@ struct kvm_arch {
> > * the code to do so.
> > */
> > spinlock_t tdp_mmu_pages_lock;
> > +
> > + /*
> > + * Protect the per-VM cache of pre-allocate pages used to populate the
> > + * Dynamic PAMT when splitting S-EPT huge pages without a vCPU.
> > + */
> > + struct mutex tdp_mmu_external_cache_lock;
> Missing "spin_lock_init(&kvm->arch.tdp_mmu_external_cache_lock);" in
> kvm_mmu_init_tdp_mmu().
>
> Will check the patch you replied next week.
It has the same bug. FWIW, I found and fixed the bug on our internal branch, but I
either missed the fixup when synchronizing back to the upstream branch, or I found
the issue after posting.
@@ -634,6 +642,7 @@ int tdx_vm_init(struct kvm *kvm)
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
tdx_init_pamt_cache(&kvm_tdx->pamt_cache);
+ mutex_init(&kvm_tdx->pamt_cache_lock);
kvm->arch.has_protected_state = true;
/*
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Sean Christopherson @ 2026-02-06 15:01 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYW5CbUvZrLogsWF@yzhao56-desk.sh.intel.com>
On Fri, Feb 06, 2026, Yan Zhao wrote:
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index 18764dbc97ea..01e3e4f4baa5 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -55,7 +55,8 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
> >
> > static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> > {
> > - free_page((unsigned long)sp->external_spt);
> > + if (sp->external_spt)
> > + kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
> > free_page((unsigned long)sp->spt);
> > kmem_cache_free(mmu_page_header_cache, sp);
> > }
> Strictly speaking, external_spt is not a control page. Its alloc/free are
> different from normal control pages managed by TDX's code.
Yeah, I called that out in the changelog. I'm definitley not wedded to
tdx_{alloc,free}_control_page(), but I am very much against tdx_{alloc,free}_page().
(arguably S-EPT pages aren't "control" pages, but they're not guest pages either)
> (1) alloc
> tdx_alloc_control_page
> __tdx_alloc_control_page
> __tdx_pamt_get
> spin_lock(&pamt_lock) ==> under process context
> spin_unlock(&pamt_lock)
>
> (2) free
> tdp_mmu_free_sp_rcu_callback
> tdp_mmu_free_sp
> kvm_x86_call(free_external_sp)
> tdx_free_control_page
> __tdx_free_control_page
> __tdx_pamt_put
> spin_lock(&pamt_lock) ==> under softirq context
> spin_unlock(&pamt_lock)
>
> So, invoking __tdx_pamt_put() in the RCU callback triggers deadlock warning
> (see the bottom for details).
Hrm. I can think of two options. Option #1 would be to use a raw spinlock and
disable IRQs:
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 823ec092b4e4..6348085d7dcb 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -2246,7 +2246,7 @@ static u64 tdh_phymem_pamt_remove(u64 pfn, u64 *pamt_pa_array)
}
/* Serializes adding/removing PAMT memory */
-static DEFINE_SPINLOCK(pamt_lock);
+static DEFINE_RAW_SPINLOCK(pamt_lock);
/* Bump PAMT refcount for the given page and allocate PAMT memory if needed */
int __tdx_pamt_get(u64 pfn, struct tdx_pamt_cache *cache)
@@ -2272,7 +2272,7 @@ int __tdx_pamt_get(u64 pfn, struct tdx_pamt_cache *cache)
if (ret)
goto out_free;
- scoped_guard(spinlock, &pamt_lock) {
+ scoped_guard(raw_spinlock_irqsave, &pamt_lock) {
/*
* Lost race to other tdx_pamt_add(). Other task has already allocated
* PAMT memory for the HPA.
@@ -2348,7 +2348,7 @@ void __tdx_pamt_put(u64 pfn)
if (!atomic_dec_and_test(pamt_refcount))
return;
- scoped_guard(spinlock, &pamt_lock) {
+ scoped_guard(raw_spinlock_irqsave, &pamt_lock) {
/* Lost race with tdx_pamt_get(). */
if (atomic_read(pamt_refcount))
return;
--
Option #2 would be to immediately free the page in tdx_sept_reclaim_private_sp(),
so that pages that freed via handle_removed_pt() don't defer freeing the S-EPT
page table (which, IIUC, is safe since the TDX-Module forces TLB flushes and exits).
I really, really don't like this option (if it even works).
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index ae7b9beb3249..4726011ad624 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2014,7 +2014,15 @@ static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
*/
if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
tdx_reclaim_page(virt_to_page(sp->external_spt)))
- sp->external_spt = NULL;
+ goto out;
+
+ /*
+ * Immediately free the control page, as the TDX subsystem doesn't
+ * support freeing pages from RCU callbacks.
+ */
+ tdx_free_control_page((unsigned long)sp->external_spt);
+out:
+ sp->external_spt = NULL;
}
void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
--
^ permalink raw reply related
* Re: [PATCH v2 0/2] x86/virt/tdx: Print TDX module version to dmesg
From: Dave Hansen @ 2026-02-06 15:10 UTC (permalink / raw)
To: Kiryl Shutsemau, Verma, Vishal L, dave.hansen
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, Edgecombe, Rick P,
Huang, Kai, bp@alien8.de, mingo@redhat.com, Williams, Dan J,
tglx@linutronix.de, hpa@zytor.com, x86@kernel.org
In-Reply-To: <aYXSd8B00OtKZcAU@thinkstation>
On 2/6/26 03:39, Kiryl Shutsemau wrote:
>> Hi Kiryl, just wanted to check on the plan for this, I didn't see it
>> merged in tip.git x86/tdx (or any other tip branch). Were you planning
>> to take it through x86/tdx? Can I help with anything to move it along?
> I guess it has to wait after the merge window at this point.
>
> Dave, could you queue this after -rc1 is tagged?
Sure.
Is there any other TDX stuff that needs to get picked up at the same
time that's been languishing?
^ permalink raw reply
* Re: [RFC PATCH v5 22/45] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Sean Christopherson @ 2026-02-06 16:03 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYXAdJV8rvWn4EQf@yzhao56-desk.sh.intel.com>
On Fri, Feb 06, 2026, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:14:54PM -0800, Sean Christopherson wrote:
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 6e84dbc89e79..a6e4ab76b1b2 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1863,6 +1863,7 @@ struct kvm_x86_ops {
> > struct kvm_mmu_page *sp);
> > void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > u64 mirror_spte);
> > + int (*topup_external_cache)(struct kvm_vcpu *vcpu, int min);
> >
> >
> > bool (*has_wbinvd_exit)(void);
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 9b5a6861e2a4..4ecbf216d96f 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -605,6 +605,10 @@ static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu, bool maybe_indirect)
> > PT64_ROOT_MAX_LEVEL);
> > if (r)
> > return r;
> > +
> > + r = kvm_x86_call(topup_external_cache)(vcpu, PT64_ROOT_MAX_LEVEL);
> If this external cache is for PAMT pages allocation for guest pages only, here
> the min count should be 1 instead of PT64_ROOT_MAX_LEVEL?
Oh! Right. Hmm, with that in mind, it seems like topup_external_cache() isn't
quite the right interace. It's not at all clear that, unlike the other caches,
the DPAMT cache isn't tied to the page tables, it's tied to the physical memory
being mapped into the guest.
At the very least, it seems like we should drop the @min parameter?
int (*topup_external_cache)(struct kvm *kvm, struct kvm_vcpu *vcpu);
Though if someone has a name that better captures what the cache is used for,
without bleeding too many details into common x86...
^ permalink raw reply
* Re: [RFC PATCH v5 37/45] KVM: x86/tdp_mmu: Alloc external_spt page for mirror page table splitting
From: Sean Christopherson @ 2026-02-06 16:09 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYW9UaK7tePxDuyh@yzhao56-desk.sh.intel.com>
On Fri, Feb 06, 2026, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:15:09PM -0800, Sean Christopherson wrote:
> > From: Isaku Yamahata <isaku.yamahata@intel.com>
> >
> > Enhance tdp_mmu_alloc_sp_for_split() to allocate a page table page for the
> > external page table for splitting the mirror page table.
> >
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > Co-developed-by: Yan Zhao <yan.y.zhao@intel.com>
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > [sean: use kvm_x86_ops.alloc_external_sp()]
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/kvm/mmu/tdp_mmu.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index 3b0da898824a..4f5b80f0ca03 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -1447,7 +1447,7 @@ bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm,
> > return spte_set;
> > }
> >
> > -static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> > +static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct tdp_iter *iter)
> > {
> > struct kvm_mmu_page *sp;
> >
> > @@ -1461,6 +1461,15 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> > return NULL;
> > }
> >
> > + if (is_mirror_sptep(iter->sptep)) {
> tdp_mmu_alloc_sp_for_split() is invoked in tdp_mmu_split_huge_pages_root() after
> rcu_read_unlock() is called.
>
> So, it's incorrect to invoke is_mirror_sptep() which internally contains
> rcu_dereference(), resulting in "WARNING: suspicious RCU usage".
Ah, now I see why the previous code pass in a bool. I don't love passing a bool,
but passing @iter is outright dangerous, so I guess this?
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index a32192c35099..4d92c0d19d7c 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1448,7 +1448,7 @@ bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm,
}
static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
- struct tdp_iter *iter)
+ bool is_mirror_sp)
{
struct kvm_mmu_page *sp;
@@ -1460,7 +1460,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
if (!sp->spt)
goto err_spt;
- if (is_mirror_sptep(iter->sptep)) {
+ if (is_mirror_sp) {
sp->external_spt = (void *)kvm_x86_call(alloc_external_sp)(GFP_KERNEL_ACCOUNT);
if (!sp->external_spt)
goto err_external_spt;
@@ -1525,6 +1525,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
gfn_t start, gfn_t end,
int target_level, bool shared)
{
+ const bool is_mirror_root = is_mirror_sp(root);
struct kvm_mmu_page *sp = NULL;
struct tdp_iter iter;
@@ -1557,7 +1558,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
else
write_unlock(&kvm->mmu_lock);
- sp = tdp_mmu_alloc_sp_for_split(kvm, &iter);
+ sp = tdp_mmu_alloc_sp_for_split(kvm, is_mirror_root);
if (shared)
read_lock(&kvm->mmu_lock);
^ permalink raw reply related
* Re: [PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
From: Xing, Cedric @ 2026-02-06 17:15 UTC (permalink / raw)
To: Chao Gao, linux-coco, linux-kernel, kvm, x86
Cc: reinette.chatre, ira.weiny, kai.huang, dan.j.williams, yilun.xu,
sagis, vannapurve, paulmck, nik.borisov, zhenzhong.duan, seanjc,
rick.p.edgecombe, kas, dave.hansen, vishal.l.verma, Farrah Chen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260123145645.90444-11-chao.gao@intel.com>
On 1/23/2026 8:55 AM, Chao Gao wrote:
[...]
> +
> +static void seamldr_init(struct device *dev)
> +{
> + const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> + int ret;
> +
> + if (WARN_ON_ONCE(!tdx_sysinfo))
> + return;
> +
> + if (!IS_ENABLED(CONFIG_INTEL_TDX_MODULE_UPDATE))
> + return;
> +
> + if (!tdx_supports_runtime_update(tdx_sysinfo))
> + pr_info("Current TDX Module cannot be updated. Consider BIOS updates\n");
> +
> + tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
I can't speak for others but the name "seamldr_upload" here doesn't look intuitive to me. Given this
FW node will show up in /sys/class/firmware/, I'd name it "tdx_module" to indicate to the user
clearly that this is for updating the TDX module.
-Cedric
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Sean Christopherson @ 2026-02-06 17:41 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYVPN5M7QQwu/r/n@yzhao56-desk.sh.intel.com>
On Fri, Feb 06, 2026, Yan Zhao wrote:
> On Thu, Feb 05, 2026 at 02:33:16PM -0800, Sean Christopherson wrote:
> > On Thu, Feb 05, 2026, Yan Zhao wrote:
> > > > > > if (was_present && !was_leaf &&
> > > > > > (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> > > > > > handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> > > > > > + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
> > > > > Should we check !is_present instead of !is_leaf?
> > > > > e.g. a transition from a present leaf entry to a present non-leaf entry could
> > > > > also trigger this if case.
> > > >
> > > > No, the !is_leaf check is very intentional. At this point in the series, S-EPT
> > > > doesn't support hugepages. If KVM manages to install a leaf SPTE and replaces
> > > > that SPTE with a non-leaf SPTE, then we absolutely want the KVM_BUG_ON() in
> > > > tdx_sept_remove_private_spte() to fire:
> > > >
> > > > /* TODO: handle large pages. */
> > > > if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> > > > return -EIO;
> > > But the op is named remove_external_spte().
> > > And the check of "level != PG_LEVEL_4K" is for removing large leaf entries.
> >
> > I agree that the naming at this point in the series is unfortunate, but I don't
> > see it as outright wrong. That the TDP MMU could theoretically replace the leaf
> > SPTE with a non-leaf SPTE doesn't change the fact that the old leaf SPTE *is*
> > being removed.
> Hmm, I can't agree with that. But I won't insist if you think it's ok :)
If the code is read through a TDX lens, then I agree, it's seems wrong. Because
then you *know* that TDX doesn't support back-to-back remove()=>add() operations
to handle a page split.
But from a TDP MMU perspective, this is entirely logical (ignoring that
link_external_spt() is gone at this point in the series).
else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf) {
kvm_x86_call(remove_external_spte)(kvm, gfn, level, old_spte);
/*
* Link the new page table if a hugepage is being split, i.e.
* if a leaf SPTE is being replaced with a non-leaf SPTE.
*/
if (is_present)
kvm_x86_call(link_external_spt)(kvm, gfn, level, ...);
}
And that is *exactly* why I want to get rid of the hyper-specific kvm_x86_ops
hooks. They bleed all kinds of implementation details all over the TDP MMU, which
makes it difficult to read and understand the relevant TDP MMU code if you don't
already know the TDX rules. And I absolutely do not want to effectively require
others to understand TDX's rules to be able to make changes to the TDP MMU.
> > > Relying on the TDX code's lockdep_assert_held_write() for warning seems less
> > > clear than having an explicit check here.
> >
> > ...that's TDX's responsibility to enforce, and I don't see any justification for
> > something more than a lockdep assertion. As I've said elsewhere, several times,
> My concern is that handle_changed_spte() can be invoked by callers other than
> tdp_mmu_set_spte(). e.g.,
>
> tdp_mmu_set_spte_atomic
> | __tdp_mmu_set_spte_atomic
> | | kvm_x86_call(set_external_spte)
> | handle_changed_spte
> | kvm_x86_call(set_external_spte)
>
> When !is_frozen_spte(new_spte) and was_leaf, set_external_spte() may be invoked
> twice for splitting under shared mmu_lock.
But we don't support that yet. I was going to punt dealing with that to the future,
but now that I've looked at it, I honestly think the problem is that I didn't go
far enough with the cleanup. I shouldn't have hedged.
What I said in "KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte()
for all":
: Ideally, KVM would provide a single pair of hooks to set S-EPT entries,
: one hook for setting SPTEs under write-lock and another for settings SPTEs
: under read-lock (e.g. to ensure the entire operation is "atomic", to allow
: for failure, etc.). Sadly, TDX's requirement that all child S-EPT entries
: are removed before the parent makes that impractical: the TDP MMU
: deliberately prunes non-leaf SPTEs and _then_ processes its children, thus
: making it quite important for the TDP MMU to differentiate between zapping
: leaf and non-leaf S-EPT entries.
isn't quite right. Ideally, KVM would provide *one* hook to set S-EPT entries.
Because the API to actually *set* the external SPTE shouldn't be any different
for read-lock vs. write-lost. I.e. the child S-EPT case remains the sole
exception. More below.
> Therefore, I think it would be better to check for !shared and only invoke
> set_external_spte() when !shared.
No. I am dead set against conditionally invoking set_external_spte() based on
shared vs. exclusive. The rules for how SPTE updates are forwarded to TDX, and
how TDX reacts to those updates, need to be defined purely on state transitions,
i.e. on old_spte vs. new_spte.
Actually, even that isn't a strong enough statement. *All* updates need to be
forwarded to TDX, and then TDX can react as appropriate.
> BTW: in the patch log, you mentioned that
>
> : Invoke .remove_external_spte() in handle_changed_spte() as appropriate
> : instead of relying on callers to do the right thing. Relying on callers
> : to invoke .remove_external_spte() is confusing and brittle, e.g. subtly
> : relies tdp_mmu_set_spte_atomic() never removing SPTEs, and removing an
> : S-EPT entry in tdp_mmu_set_spte() is bizarre (yeah, the VM is bugged so
> : it doesn't matter in practice, but it's still weird).
>
> However, when tdp_mmu_set_spte_atomic() removes SPTEs in the future, it will
> still need to follow the sequence in __tdp_mmu_set_spte_atomic() :
> 1. freeze, 2. set_external_spte(). 3. set the new_spte 4. handle_changed_spte().
>
> So, do you think we should leave set_external_spte() in tdp_mmu_set_spte() for
> exclusive mmu_lock scenarios instead of moving it to handle_changed_spte()?
Nope, as above, 100% the opposite. Over ~3 patches, e.g.
1. Drop the KVM_BUG_ON()s or move them to TDX
2. Morph the !is_frozen_spte() check into a KVM_MMU_WARN_ON()
3. Rework the code to rely on __handle_changed_spte() to propagate writes to S-EPT
That way, the _only_ path that updates external SPTEs is this:
if (was_present && !was_leaf &&
(is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
else if (is_mirror_sptep(sptep))
return kvm_x86_call(set_external_spte)(kvm, gfn, old_spte,
new_spte, level);
which is fully aligned with handle_changed_spte()'s role for !mirror roots: it
exists to react to changes (the sole exception to that being aging SPTEs, which
is a special case).
Compile-tested only.
---
arch/x86/kvm/mmu/tdp_mmu.c | 118 ++++++++++++++++++-------------------
1 file changed, 59 insertions(+), 59 deletions(-)
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 847f2fcb6740..33a321aedac0 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -464,7 +464,7 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
}
/**
- * handle_changed_spte - handle bookkeeping associated with an SPTE change
+ * __handle_changed_spte - handle bookkeeping associated with an SPTE change
* @kvm: kvm instance
* @as_id: the address space of the paging structure the SPTE was a part of
* @sptep: pointer to the SPTE
@@ -480,9 +480,9 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
* dirty logging updates are handled in common code, not here (see make_spte()
* and fast_pf_fix_direct_spte()).
*/
-static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
- gfn_t gfn, u64 old_spte, u64 new_spte,
- int level, bool shared)
+static int __handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
+ gfn_t gfn, u64 old_spte, u64 new_spte,
+ int level, bool shared)
{
bool was_present = is_shadow_present_pte(old_spte);
bool is_present = is_shadow_present_pte(new_spte);
@@ -518,7 +518,7 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
}
if (old_spte == new_spte)
- return;
+ return 0;
trace_kvm_tdp_mmu_spte_changed(as_id, gfn, level, old_spte, new_spte);
@@ -547,7 +547,7 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
"a temporary frozen SPTE.\n"
"as_id: %d gfn: %llx old_spte: %llx new_spte: %llx level: %d",
as_id, gfn, old_spte, new_spte, level);
- return;
+ return 0;
}
if (is_leaf != was_leaf)
@@ -559,30 +559,31 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
* SPTE being converted to a hugepage (leaf) or being zapped. Shadow
* pages are kernel allocations and should never be migrated.
*
- * When modifying leaf entries in mirrored page tables, propagate the
- * changes to the external SPTE. Bug the VM on failure, as callers
- * aren't prepared to handle errors, e.g. due to lock contention in the
- * TDX-Module. Note, changes to non-leaf mirror SPTEs are handled by
- * handle_removed_pt() (the TDX-Module requires that child entries are
- * removed before the parent SPTE), and changes to non-present mirror
- * SPTEs are handled by __tdp_mmu_set_spte_atomic() (KVM needs to set
- * the external SPTE while the mirror SPTE is frozen so that installing
- * a new SPTE is effectively an atomic operation).
+ * When modifying leaf entries in mirrored page tables, propagate all
+ * changes to the external SPTE.
*/
if (was_present && !was_leaf &&
(is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
- else if (was_leaf && is_mirror_sptep(sptep))
- KVM_BUG_ON(kvm_x86_call(set_external_spte)(kvm, gfn, old_spte,
- new_spte, level), kvm);
+ else if (is_mirror_sptep(sptep))
+ return kvm_x86_call(set_external_spte)(kvm, gfn, old_spte,
+ new_spte, level);
+
+ return 0;
+}
+
+static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
+ gfn_t gfn, u64 old_spte, u64 new_spte,
+ int level, bool shared)
+{
+ KVM_BUG_ON(__handle_changed_spte(kvm, as_id, sptep, gfn, old_spte,
+ new_spte, level, shared), kvm);
}
static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
struct tdp_iter *iter,
u64 new_spte)
{
- u64 *raw_sptep = rcu_dereference(iter->sptep);
-
/*
* The caller is responsible for ensuring the old SPTE is not a FROZEN
* SPTE. KVM should never attempt to zap or manipulate a FROZEN SPTE,
@@ -591,40 +592,6 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
*/
WARN_ON_ONCE(iter->yielded || is_frozen_spte(iter->old_spte));
- if (is_mirror_sptep(iter->sptep) && !is_frozen_spte(new_spte)) {
- int ret;
-
- /*
- * KVM doesn't currently support zapping or splitting mirror
- * SPTEs while holding mmu_lock for read.
- */
- if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
- KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
- return -EBUSY;
-
- /*
- * Temporarily freeze the SPTE until the external PTE operation
- * has completed, e.g. so that concurrent faults don't attempt
- * to install a child PTE in the external page table before the
- * parent PTE has been written.
- */
- if (!try_cmpxchg64(raw_sptep, &iter->old_spte, FROZEN_SPTE))
- return -EBUSY;
-
- /*
- * Update the external PTE. On success, set the mirror SPTE to
- * the desired value. On failure, restore the old SPTE so that
- * the SPTE isn't frozen in perpetuity.
- */
- ret = kvm_x86_call(set_external_spte)(kvm, iter->gfn, iter->old_spte,
- new_spte, iter->level);
- if (ret)
- __kvm_tdp_mmu_write_spte(iter->sptep, iter->old_spte);
- else
- __kvm_tdp_mmu_write_spte(iter->sptep, new_spte);
- return ret;
- }
-
/*
* Note, fast_pf_fix_direct_spte() can also modify TDP MMU SPTEs and
* does not hold the mmu_lock. On failure, i.e. if a different logical
@@ -632,7 +599,7 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
* the current value, so the caller operates on fresh data, e.g. if it
* retries tdp_mmu_set_spte_atomic()
*/
- if (!try_cmpxchg64(raw_sptep, &iter->old_spte, new_spte))
+ if (!try_cmpxchg64(rcu_dereference(iter->sptep), &iter->old_spte, new_spte))
return -EBUSY;
return 0;
@@ -663,14 +630,44 @@ static inline int __must_check tdp_mmu_set_spte_atomic(struct kvm *kvm,
lockdep_assert_held_read(&kvm->mmu_lock);
- ret = __tdp_mmu_set_spte_atomic(kvm, iter, new_spte);
+ /* KVM should never freeze SPTEs using higher level APIs. */
+ KVM_MMU_WARN_ON(is_frozen_spte(new_spte));
+
+ /*
+ * Temporarily freeze the SPTE until the external PTE operation has
+ * completed (unless the new SPTE itself will be frozen), e.g. so that
+ * concurrent faults don't attempt to install a child PTE in the
+ * external page table before the parent PTE has been written, or try
+ * to re-install a page table before the old one was removed.
+ */
+ if (is_mirror_sptep(iter->sptep))
+ ret = __tdp_mmu_set_spte_atomic(kvm, iter, FROZEN_SPTE);
+ else
+ ret = __tdp_mmu_set_spte_atomic(kvm, iter, new_spte);
if (ret)
return ret;
- handle_changed_spte(kvm, iter->as_id, iter->sptep, iter->gfn,
- iter->old_spte, new_spte, iter->level, true);
+ ret = __handle_changed_spte(kvm, iter->as_id, iter->sptep, iter->gfn,
+ iter->old_spte, new_spte, iter->level, true);
- return 0;
+ /*
+ * Unfreeze the mirror SPTE. If updating the external SPTE failed,
+ * restore the old SPTE so that the SPTE isn't frozen in perpetuity,
+ * otherwise set the mirror SPTE to the new desired value.
+ */
+ if (is_mirror_sptep(iter->sptep)) {
+ if (ret)
+ __kvm_tdp_mmu_write_spte(iter->sptep, iter->old_spte);
+ else
+ __kvm_tdp_mmu_write_spte(iter->sptep, new_spte);
+ } else {
+ /*
+ * Bug the VM if handling the change failed, as failure is only
+ * allowed if KVM couldn't update the external SPTE.
+ */
+ KVM_BUG_ON(ret, kvm);
+ }
+ return ret;
}
/*
@@ -1325,6 +1322,9 @@ static void kvm_tdp_mmu_age_spte(struct kvm *kvm, struct tdp_iter *iter)
{
u64 new_spte;
+ if (WARN_ON_ONCE(is_mirror_sptep(iter->sptep)))
+ return;
+
if (spte_ad_enabled(iter->old_spte)) {
iter->old_spte = tdp_mmu_clear_spte_bits_atomic(iter->sptep,
shadow_accessed_mask);
base-commit: c86be62d601ede14cbad1d0fb5af9b67d4172342
--
^ permalink raw reply related
* Re: [PATCH v2 0/2] x86/virt/tdx: Print TDX module version to dmesg
From: Edgecombe, Rick P @ 2026-02-06 18:35 UTC (permalink / raw)
To: Li, Xiaoyao, Hansen, Dave, kas@kernel.org, Verma, Vishal L,
dave.hansen@linux.intel.com
Cc: Gao, Chao, x86@kernel.org, bp@alien8.de, Huang, Kai,
hpa@zytor.com, mingo@redhat.com, Williams, Dan J,
tglx@linutronix.de, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org
In-Reply-To: <2235a1cf-7ccf-4134-80b5-8056537c6d33@intel.com>
On Fri, 2026-02-06 at 07:10 -0800, Dave Hansen wrote:
> Is there any other TDX stuff that needs to get picked up at the same
> time that's been languishing?
Xiaoyao is going to send a rebase of this after RC1:
https://lore.kernel.org/kvm/20250715091312.563773-1-xiaoyao.li@intel.com/
It has pretty wide agreement, and ack's from the KVM side. Also, we have
internal branches that are carrying forms of it. So if we merge it now we can
have less dependencies later.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox