* Re: [RFC PATCH v5 00/45] TDX: Dynamic PAMT + S-EPT Hugepage
From: Sean Christopherson @ 2026-02-04 14:38 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Yan Zhao, Vishal Annapurve,
Ackerley Tng, Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aXuVR0kq_K1TYwlR@char.us.oracle.com>
On Thu, Jan 29, 2026, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 28, 2026 at 05:14:32PM -0800, Sean Christopherson wrote:
> > This is a combined series of Dynamic PAMT (from Rick), and S-EPT hugepage
> > support (from Yan). Except for some last minute tweaks to the DPAMT array
> > args stuff, a version of this based on a Google-internal kernel has been
> > moderately well tested (thanks Vishal!). But overall it's still firmly RFC
> > as I have deliberately NOT addressed others feedback from v4 of DPAMT and v3
>
> What does PAMT stand for? Is there a design document somewhere?
>
> > of S-EPT hugepage (mostly lack of cycles), and there's at least one patch in
> > here that shouldn't be merged as-is (the quick-and-dirty switch from struct
> > page to raw pfns).
> >
> > My immediate goal is to solidify the designs for DPAMT and S-EPT hugepage.
> > Given the substantial design changes I am proposing, posting an end-to-end
> > RFC seemed like a much better method than trying to communicate my thoughts
> > piecemeal.
> >
> > As for landing these series, I think the fastest overall approach would be
> > to land patches 1-4 asap (tangentially related cleanups and fixes), agree
>
> Should they be split out as non-RFC then?
Yeah, I'll do that soonish. I posted the kitchen sink so that people could
review the entire thing without having to chase down 4+ series/patches.
> > on a design (hopefully), and then hand control back to Rick and Yan to polish
> > their respective series for merge.
> >
> > I also want to land the VMXON series[*] before DPAMT, because there's a nasty
> > wart where KVM wires up a DPAMT-specific hook even if DPAMT is disabled,
> > because KVM's ordering needs to set the vendor hooks before tdx_sysinfo is
> > ready. Decoupling VMXON from KVM solves that problem, because it lets the
> > TDX subsystem parse sysinfo before TDX is loaded.
> >
> > Beyond that dependency, I am comfortable landing both DPAMT and S-EPT hugepage
> > support without any other prereqs, i.e. without an in-tree way to light up
> > the S-EPT hugepage code due to lack of hugepage support in guest_memfd.
>
> Can there be test-cases? Or simple code posted for QEMU which is the
> tool that 99% of kernel engineers use?
No? The core limitation is that KVM doesn't yet support hugepages for private
memory. No amount userspace code can overcome that limitation.
We can and do have tests and VMM support, but it's all out-of-tree (for now).
All I'm saying here is that I'm ok landing the S-EPT hugepage code in advance of
guest_memfd hugepage support, e.g. so that we don't end up in a stalemate due to
cyclical dependecies, or one big megaseries.
^ permalink raw reply
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Kiryl Shutsemau @ 2026-02-04 11:22 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, x86, linux-kernel, tglx, mingo, bp,
dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <20260203174946.1198053-2-prsampat@amd.com>
On Tue, Feb 03, 2026 at 11:49:45AM -0600, Pratik R. Sampat wrote:
> Confidential computing guests require memory to be accepted before use.
> The unaccepted memory bitmap maintained by firmware does not track
> most hotplugged memory ranges apart from system memory annotated to be
> cold plugged at boot.
>
> Explicitly validate and transition the newly added memory to a private
> state, making it usable by the guest.
>
> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
> ---
> drivers/firmware/efi/unaccepted_memory.c | 47 ++++++++++++++++++++++++
> include/linux/mm.h | 5 +++
> mm/memory_hotplug.c | 2 +
> 3 files changed, 54 insertions(+)
>
> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
> index c2c067eff634..359779133cb4 100644
> --- a/drivers/firmware/efi/unaccepted_memory.c
> +++ b/drivers/firmware/efi/unaccepted_memory.c
> @@ -209,6 +209,53 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
> return ret;
> }
>
> +/*
> + * Unaccepted memory bitmap only covers initial boot memory and not the
> + * hotpluggable range that is part of SRAT parsing. However, some initial memory
> + * with the attribute EFI_MEMORY_HOT_PLUGGABLE can indicate boot time memory
> + * that can be hot-removed. Hence post acceptance, only for that range update
> + * the unaccepted bitmap to reflect this change.
> + */
> +void accept_hotplug_memory(phys_addr_t start, unsigned long size)
> +{
> + struct efi_unaccepted_memory *unaccepted;
> + unsigned long range_start, range_len;
> + phys_addr_t end = start + size;
> + u64 phys_base, unit_size;
> + unsigned long flags;
> +
> + unaccepted = efi_get_unaccepted_table();
> + if (!unaccepted)
> + return;
This can be tricky.
If we boot a VM with <4GiB of memory and all of it is pre-accepted by
BIOS, the table will not be allocated.
But it doesn't mean that hotplugged memory above should not be accepted.
I don't think there is a way to detect such cases.
Your check is probably the best we can do, but it means VMs are going to
crash if memory accept is required by no table.
This is ugly situation.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* [Invitation] bi-weekly guest_memfd upstream call on 2026-02-05
From: David Hildenbrand (arm) @ 2026-02-04 11:17 UTC (permalink / raw)
To: linux-coco@lists.linux.dev, linux-mm@kvack.org, KVM
Hi,
Our next guest_memfd upstream call is scheduled for Thursday,
2026-02-07 at 8:00 - 9:00am (GMT-08:00) Pacific Time - Vancouver.
We'll be using the following Google meet:
http://meet.google.com/wxp-wtju-jzw
The meeting notes can be found at [1], where we also link recordings and
collect current guest_memfd upstream proposals. If you want an google
calendar invitation that also covers all future meetings, just write me
or Ackerley a mail.
I want to briefly talk about the pfncache[2] patches and how
synchronization with truncation etc. is supposed to work; then well see
which other topics pop up. :)
To put something to discuss onto the agenda, reply to this mail or add
them to the "Topics/questions for next meeting(s)" section in the
meeting notes as a comment.
[1]
https://docs.google.com/document/d/1M6766BzdY1Lhk7LiR5IqVR8B8mG3cr-cxTxOrAosPOk/edit?usp=sharing
[2] https://lore.kernel.org/all/20251203144159.6131-1-itazur@amazon.com/
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v3 23/26] x86/virt/tdx: Enable TDX Module runtime updates
From: Tony Lindgren @ 2026-02-04 10:03 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, linux-kernel, kvm, x86, 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, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin
In-Reply-To: <aXdMfzIxF6KR7VCe@tlindgre-MOBL1>
On Mon, Jan 26, 2026 at 01:14:07PM +0200, Tony Lindgren wrote:
> On Fri, Jan 23, 2026 at 06:55:31AM -0800, Chao Gao wrote:
> > --- a/arch/x86/include/asm/tdx.h
> > +++ b/arch/x86/include/asm/tdx.h
> > @@ -32,6 +32,9 @@
> > #define TDX_SUCCESS 0ULL
> > #define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
> >
> > +/* Bit definitions of TDX_FEATURES0 metadata field */
> > +#define TDX_FEATURES0_TD_PRESERVING BIT(1)
> > +#define TDX_FEATURES0_NO_RBP_MOD BIT(18)
> > #ifndef __ASSEMBLER__
> >
> > #include <uapi/asm/mce.h>
>
> How about let's put these defines into arch/x86/include/asm/shared/tdx.h
> instead? And use BIT_ULL?
Sorry I was confused. No need to move these defines to
arch/x86/include/asm/shared/tdx.h as far as I can tell.
The BIT_ULL comment still remains though.
Regards,
Tony
^ permalink raw reply
* Re: [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Yan Zhao @ 2026-02-04 9:45 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-10-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:41PM -0800, Sean Christopherson wrote:
> Massage .free_external_spt() into .reclaim_external_sp() to free up (pun
> intended) "free" for actually freeing memory, and to allow TDX to do more
> than just "free" the S-EPT entry. Specifically, nullify external_spt to
> leak the S-EPT page if reclaiming the page fails, as that detail and
> implementation choice has no business living in the TDP MMU.
>
> Use "sp" instead of "spt" even though "spt" is arguably more accurate, as
> "spte" and "spt" are dangerously close in name, and because the key
> parameter is a kvm_mmu_page, not a pointer to an S-EPT page table.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 2 +-
> arch/x86/include/asm/kvm_host.h | 4 ++--
> arch/x86/kvm/mmu/tdp_mmu.c | 13 ++-----------
> arch/x86/kvm/vmx/tdx.c | 27 ++++++++++++---------------
> 4 files changed, 17 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index 57eb1f4832ae..c17cedc485c9 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -95,8 +95,8 @@ KVM_X86_OP_OPTIONAL_RET0(set_identity_map_addr)
> KVM_X86_OP_OPTIONAL_RET0(get_mt_mask)
> KVM_X86_OP(load_mmu_pgd)
> KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
> -KVM_X86_OP_OPTIONAL_RET0(free_external_spt)
> KVM_X86_OP_OPTIONAL(remove_external_spte)
> +KVM_X86_OP_OPTIONAL(reclaim_external_sp)
> 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 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).
> /* Update external page table from spte getting removed, and flush TLB. */
> void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 27ac520f2a89..18764dbc97ea 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -456,17 +456,8 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> old_spte, FROZEN_SPTE, level, shared);
> }
>
> - if (is_mirror_sp(sp) &&
> - WARN_ON(kvm_x86_call(free_external_spt)(kvm, base_gfn, sp->role.level,
> - sp->external_spt))) {
> - /*
> - * Failed to free page table page in mirror page table and
> - * there is nothing to do further.
> - * Intentionally leak the page to prevent the kernel from
> - * accessing the encrypted page.
> - */
> - sp->external_spt = NULL;
> - }
> + if (is_mirror_sp(sp))
> + kvm_x86_call(reclaim_external_sp)(kvm, base_gfn, sp);
>
> call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
> }
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 30494f9ceb31..66bc3ceb5e17 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1783,27 +1783,24 @@ static void tdx_track(struct kvm *kvm)
> kvm_make_all_cpus_request(kvm, KVM_REQ_OUTSIDE_GUEST_MODE);
> }
>
> -static int tdx_sept_free_private_spt(struct kvm *kvm, gfn_t gfn,
> - enum pg_level level, void *private_spt)
> +static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
> + struct kvm_mmu_page *sp)
Passing in "sp" and having "reclaim_private_sp" in the function name is bit
confusing.
Strictly speaking, only sp->external_spt is private, while the sp and sp->spt
are just mirroring the external spt.
But I understand it's for setting sp->external_spt to NULL on error.
> {
> - struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> -
> /*
> - * free_external_spt() is only called after hkid is freed when TD is
> - * tearing down.
> * KVM doesn't (yet) zap page table pages in mirror page table while
> * TD is active, though guest pages mapped in mirror page table could be
> * zapped during TD is active, e.g. for shared <-> private conversion
> * and slot move/deletion.
> + *
> + * In other words, KVM should only free mirror page tables after the
> + * TD's hkid is freed, when the TD is being torn down.
> + *
> + * If the S-EPT PTE can't be removed for any reason, intentionally leak
> + * the page to prevent the kernel from accessing the encrypted page.
> */
> - if (KVM_BUG_ON(is_hkid_assigned(kvm_tdx), kvm))
> - return -EIO;
> -
> - /*
> - * The HKID assigned to this TD was already freed and cache was
> - * already flushed. We don't have to flush again.
> - */
> - return tdx_reclaim_page(virt_to_page(private_spt));
> + 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;
> }
>
> static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> @@ -3617,7 +3614,7 @@ void __init tdx_hardware_setup(void)
> vt_x86_ops.vm_size = max_t(unsigned int, vt_x86_ops.vm_size, sizeof(struct kvm_tdx));
>
> vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
> - vt_x86_ops.free_external_spt = tdx_sept_free_private_spt;
> + vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
> vt_x86_ops.remove_external_spte = tdx_sept_remove_private_spte;
> vt_x86_ops.protected_apic_has_interrupt = tdx_protected_apic_has_interrupt;
> }
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ 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-04 9:06 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-9-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:40PM -0800, Sean Christopherson wrote:
> 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).
>
> Implementing rules-based logic in a common chokepoint will also make it
> easier to reason about the correctness of splitting hugepages when support
> for S-EPT hugepages comes along.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 43 +++++++++++++-------------------------
> 1 file changed, 14 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 8743cd020d12..27ac520f2a89 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -359,25 +359,6 @@ static void tdp_mmu_unlink_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
> spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
> }
>
> -static void remove_external_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
> - int level)
> -{
> - /*
> - * External (TDX) SPTEs are limited to PG_LEVEL_4K, and external
> - * PTs are removed in a special order, involving free_external_spt().
> - * But remove_external_spte() will be called on non-leaf PTEs via
> - * __tdp_mmu_zap_root(), so avoid the error the former would return
> - * in this case.
> - */
> - if (!is_last_spte(old_spte, level))
> - return;
> -
> - /* Zapping leaf spte is allowed only when write lock is held. */
> - lockdep_assert_held_write(&kvm->mmu_lock);
> -
> - kvm_x86_call(remove_external_spte)(kvm, gfn, level, old_spte);
> -}
> -
> /**
> * handle_removed_pt() - handle a page table removed from the TDP structure
> *
> @@ -473,11 +454,6 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> }
> handle_changed_spte(kvm, kvm_mmu_page_as_id(sp), sptep, gfn,
> old_spte, FROZEN_SPTE, level, shared);
> -
> - if (is_mirror_sp(sp)) {
> - KVM_BUG_ON(shared, kvm);
> - remove_external_spte(kvm, gfn, old_spte, level);
> - }
> }
>
> if (is_mirror_sp(sp) &&
> @@ -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.
Besides, need "KVM_BUG_ON(shared, kvm)" in this case.
> + kvm_x86_call(remove_external_spte)(kvm, gfn, level, old_spte);
> }
>
> static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> @@ -725,12 +712,10 @@ static u64 tdp_mmu_set_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
>
> /*
> * Users that do non-atomic setting of PTEs don't operate on mirror
> - * roots, so don't handle it and bug the VM if it's seen.
> + * roots. Bug the VM as this path doesn't propagate such writes to the
> + * external page tables.
> */
> - if (is_mirror_sptep(sptep)) {
> - KVM_BUG_ON(is_shadow_present_pte(new_spte), kvm);
> - remove_external_spte(kvm, gfn, old_spte, level);
> - }
> + KVM_BUG_ON(is_mirror_sptep(sptep) && is_shadow_present_pte(new_spte), kvm);
>
> return old_spte;
> }
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ 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-04 7:38 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-7-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:38PM -0800, Sean Christopherson wrote:
> Fold set_external_spte_present() into __tdp_mmu_set_spte_atomic() in
> anticipation of supporting hugepage splitting, at which point other paths
> will also set shadow-present external SPTEs.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 82 +++++++++++++++++---------------------
> 1 file changed, 36 insertions(+), 46 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 56ad056e6042..6fb48b217f5b 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -495,33 +495,6 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
> }
>
> -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)
> -{
> - int ret;
> -
> - lockdep_assert_held(&kvm->mmu_lock);
> -
> - if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> - return -EIO;
> -
> - /*
> - * We need to lock out other updates to the SPTE until the external
> - * page table has been modified. Use FROZEN_SPTE similar to
> - * the zapping case.
> - */
> - if (!try_cmpxchg64(rcu_dereference(sptep), old_spte, FROZEN_SPTE))
> - return -EBUSY;
> -
> - ret = kvm_x86_call(set_external_spte)(kvm, gfn, level, new_spte);
> - if (ret)
> - __kvm_tdp_mmu_write_spte(sptep, *old_spte);
> - else
> - __kvm_tdp_mmu_write_spte(sptep, new_spte);
> - return ret;
> -}
> -
> /**
> * handle_changed_spte - handle bookkeeping associated with an SPTE change
> * @kvm: kvm instance
> @@ -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?
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?
> + /*
> + * 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->level, new_spte);
> if (ret)
> - return ret;
> - } else {
> - u64 *sptep = rcu_dereference(iter->sptep);
> -
> - /*
> - * 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 CPU modified the SPTE, try_cmpxchg64()
> - * updates iter->old_spte with the current value, so the caller
> - * operates on fresh data, e.g. if it retries
> - * tdp_mmu_set_spte_atomic()
> - */
> - if (!try_cmpxchg64(sptep, &iter->old_spte, new_spte))
> - return -EBUSY;
> + __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
> + * CPU modified the SPTE, try_cmpxchg64() updates iter->old_spte with
> + * 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))
> + return -EBUSY;
> +
> return 0;
> }
>
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Huang, Kai @ 2026-02-04 6:45 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org,
binbin.wu@linux.intel.com, pbonzini@redhat.com, mingo@redhat.com,
Yamahata, Isaku, ackerleytng@google.com, tglx@kernel.org,
sagis@google.com, Edgecombe, Rick P, bp@alien8.de,
Annapurve, Vishal, x86@kernel.org
In-Reply-To: <aYKr7XODY-p6YLYa@google.com>
On Tue, 2026-02-03 at 18:16 -0800, Sean Christopherson wrote:
> On Tue, Feb 03, 2026, Kai Huang wrote:
> > On Tue, 2026-02-03 at 12:12 -0800, Sean Christopherson wrote:
> > > On Tue, Feb 03, 2026, Kai Huang wrote:
> > > > On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > > > > Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> > > > > so that x86's TDX can update per-page metadata on allocation and free().
> > > > >
> > > > > Name the allocator page_get() to align with __get_free_page(), e.g. to
> > > > > communicate that it returns an "unsigned long", not a "struct page", and
> > > > > to avoid collisions with macros, e.g. with alloc_page.
> > > > >
> > > > > Suggested-by: Kai Huang <kai.huang@intel.com>
> > > > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > >
> > > > I thought it could be more generic for allocating an object, but not just a
> > > > page.
> > > >
> > > > E.g., I thought we might be able to use it to allocate a structure which has
> > > > "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> > > > it seems you abandoned this idea. May I ask why? Just want to understand
> > > > the reasoning here.
> > >
> > > Because that requires more complexity and there's no known use case, and I don't
> > > see an obvious way for a use case to come along. All of the motiviations for a
> > > custom allocation scheme that I can think of apply only to full pages, or fit
> > > nicely in a kmem_cache.
> > >
> > > Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
> > > "page" usage. Further splitting the "page" case doesn't require modifications to
> > > the "kmem_cache" case, whereas providing a fully generic solution would require
> > > additional changes, e.g. to handle this code:
> > >
> > > page = (void *)__get_free_page(gfp_flags);
> > > if (page && mc->init_value)
> > > memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
> > >
> > > It certainly wouldn't be much complexity, but this code is already a bit awkward,
> > > so I don't think it makes sense to add support for something that will probably
> > > never be used.
> >
> > For this particular piece of code, we can add a helper for allocating normal
> > page table pages, get rid of mc->init_value completely and hook mc-page_get()
> > to that helper.
>
> Hmm, I like the idea, but I don't think it would be a net positive. In practice,
> x86's "normal" page tables stop being normal, because KVM now initializes all
> SPTEs with BIT(63)=1 on x86-64. And that would also incur an extra RETPOLINE on
> all those allocations.
No argument on this. People hate indirect calls I guess. :-)
>
> > A bonus is we can then call that helper in all places when KVM needs to
> > allocate a page for normal page table instead of just calling
> > get_zerod_pages() directly, e.g., like the one in
> > tdp_mmu_alloc_sp_for_split(),
>
> Huh. Actually, that's a bug, but not the one you probably expect. At a glance,
> it looks like KVM incorrectly zeroing the page instead of initializing it with
> SHADOW_NONPRESENT_VALUE. But it's actually a "performance" bug, because KVM
> doesn't actually need to pre-initialize the page: either the page will never be
> used, or every SPTE will be initialized as a child SPTE.
>
> So that one _should_ be different, e.g. should be:
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index a32192c35099..36afd67601fc 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1456,7 +1456,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
> if (!sp)
> return NULL;
>
> - sp->spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> + sp->spt = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
> if (!sp->spt)
> goto err_spt;
>
If we look from "performance" perspective, yeah indeed, albeit we probably
not gonna see any performance difference.
But no more arguments. I just think it will be less error-prone if we have
a consistent way for allocating the same object (no matter what it is), but
it's just a theoretical thing.
^ 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-04 6:41 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: <aYJU8Som706YkIEO@google.com>
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?".
>
> > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > > index 5688c77616e3..30494f9ceb31 100644
> > > --- a/arch/x86/kvm/vmx/tdx.c
> > > +++ b/arch/x86/kvm/vmx/tdx.c
> > > @@ -1664,18 +1664,58 @@ static int tdx_mem_page_aug(struct kvm *kvm, gfn_t gfn,
> > > return 0;
> > > }
> > >
> > > +static struct page *tdx_spte_to_external_spt(struct kvm *kvm, gfn_t gfn,
> > > + u64 new_spte, enum pg_level level)
> > > +{
> > > + struct kvm_mmu_page *sp = spte_to_child_sp(new_spte);
> > > +
> > > + if (KVM_BUG_ON(!sp->external_spt, kvm) ||
> > > + KVM_BUG_ON(sp->role.level + 1 != level, kvm) ||
> > > + KVM_BUG_ON(sp->gfn != gfn, kvm))
> > > + return NULL;
> > Could we remove the KVM_BUG_ON()s, and ...
> >
> > > + return virt_to_page(sp->external_spt);
> > > +}
> > > +
> > > +static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn,
> > > + enum pg_level level, u64 mirror_spte)
> > > +{
> > > + gpa_t gpa = gfn_to_gpa(gfn);
> > > + u64 err, entry, level_state;
> > > + struct page *external_spt;
> > > +
> > > + external_spt = tdx_spte_to_external_spt(kvm, gfn, mirror_spte, level);
> > > + if (!external_spt)
> > add a KVM_BUG_ON() here?
> > It could save KVM_BUG_ON()s and have KVM_BUG_ON() match -EIO :)
>
> We could, but I don't want to, because if we're going to bother with sanity checks,
> I want the resulting WARNs to be precise. I.e. I want the WARN to capture *why*
> tdx_spte_to_external_spt() failed, to make debug/triage easier.
Ok.
> > 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.
But I don't have strong opinion :)
> > And mirror_spte --> new_spte?
>
> Hmm, ya, I made that change later, but it can probably be shifted here.
>
> > > - WARN_ON_ONCE(!is_shadow_present_pte(mirror_spte) ||
> > > - (mirror_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
> > > + WARN_ON_ONCE((mirror_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
> > Also check this for tdx_sept_link_private_spt()?
>
> Eh, we could, but I don't think it's necessary. make_nonleaf_spte() is hardcoded
> to set full permissions (and I don't see that changing any time soon), whereas
> leaf SPTE protections are much more dynamic.
Makes sense.
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Sean Christopherson @ 2026-02-04 2:16 UTC (permalink / raw)
To: Kai Huang
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Xiaoyao Li,
Yan Y Zhao, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
Isaku Yamahata, sagis@google.com, tglx@kernel.org,
Rick P Edgecombe, bp@alien8.de, Vishal Annapurve, x86@kernel.org
In-Reply-To: <a2bf6a8d9f9b61ae7264afc37d9925cf2e1f3ea9.camel@intel.com>
On Tue, Feb 03, 2026, Kai Huang wrote:
> On Tue, 2026-02-03 at 12:12 -0800, Sean Christopherson wrote:
> > On Tue, Feb 03, 2026, Kai Huang wrote:
> > > On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > > > Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> > > > so that x86's TDX can update per-page metadata on allocation and free().
> > > >
> > > > Name the allocator page_get() to align with __get_free_page(), e.g. to
> > > > communicate that it returns an "unsigned long", not a "struct page", and
> > > > to avoid collisions with macros, e.g. with alloc_page.
> > > >
> > > > Suggested-by: Kai Huang <kai.huang@intel.com>
> > > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > >
> > > I thought it could be more generic for allocating an object, but not just a
> > > page.
> > >
> > > E.g., I thought we might be able to use it to allocate a structure which has
> > > "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> > > it seems you abandoned this idea. May I ask why? Just want to understand
> > > the reasoning here.
> >
> > Because that requires more complexity and there's no known use case, and I don't
> > see an obvious way for a use case to come along. All of the motiviations for a
> > custom allocation scheme that I can think of apply only to full pages, or fit
> > nicely in a kmem_cache.
> >
> > Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
> > "page" usage. Further splitting the "page" case doesn't require modifications to
> > the "kmem_cache" case, whereas providing a fully generic solution would require
> > additional changes, e.g. to handle this code:
> >
> > page = (void *)__get_free_page(gfp_flags);
> > if (page && mc->init_value)
> > memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
> >
> > It certainly wouldn't be much complexity, but this code is already a bit awkward,
> > so I don't think it makes sense to add support for something that will probably
> > never be used.
>
> For this particular piece of code, we can add a helper for allocating normal
> page table pages, get rid of mc->init_value completely and hook mc-page_get()
> to that helper.
Hmm, I like the idea, but I don't think it would be a net positive. In practice,
x86's "normal" page tables stop being normal, because KVM now initializes all
SPTEs with BIT(63)=1 on x86-64. And that would also incur an extra RETPOLINE on
all those allocations.
> A bonus is we can then call that helper in all places when KVM needs to
> allocate a page for normal page table instead of just calling
> get_zerod_pages() directly, e.g., like the one in
> tdp_mmu_alloc_sp_for_split(),
Huh. Actually, that's a bug, but not the one you probably expect. At a glance,
it looks like KVM incorrectly zeroing the page instead of initializing it with
SHADOW_NONPRESENT_VALUE. But it's actually a "performance" bug, because KVM
doesn't actually need to pre-initialize the page: either the page will never be
used, or every SPTE will be initialized as a child SPTE.
So that one _should_ be different, e.g. should be:
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index a32192c35099..36afd67601fc 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1456,7 +1456,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
if (!sp)
return NULL;
- sp->spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+ sp->spt = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
if (!sp->spt)
goto err_spt;
> so that we can have a consistent way for allocating normal page table pages.
^ permalink raw reply related
* Re: [RFC PATCH v5 04/45] KVM: x86: Make "external SPTE" ops that can fail RET0 static calls
From: Sean Christopherson @ 2026-02-04 1:16 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Kai Huang,
Xiaoyao Li, Yan Y Zhao, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org, mingo@redhat.com,
pbonzini@redhat.com, binbin.wu@linux.intel.com,
ackerleytng@google.com, Isaku Yamahata, sagis@google.com,
tglx@kernel.org, bp@alien8.de, Vishal Annapurve, x86@kernel.org
In-Reply-To: <e3feb0224cf2665a71ba6147e4e3e3bb30f96760.camel@intel.com>
On Fri, Jan 30, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-01-29 at 17:28 -0800, Sean Christopherson wrote:
> >
> > Hmm, that's probably doable, but definitely in a separate patch.
> > E.g. something
> > like:
>
> I think it would be a good change. But after more consideration, I
> think the original patch is good on its own. Better to turn a bug into
> a deterministic thing, than an opportunity to consume stack. Seems to
> be what you intended.
>
> Another idea would be to have a variant that returns an error instead
> of 0 so the callers can have there error logic triggered, but it's all
> incremental value on top of this.
I don't like that idea, at all. First and foremost, I don't want to litter KVM
with WARNs for things that simply can't happen. I'm fine adding infrastructure
that hides the sanity checks, but I don't want to bleed that into callers.
The other aspect I dislike is that returning a specific errno could lead to all
sorts of weirdness and hidden dependencies.
All in all, I think we'd be increasing the chances of creating bugs just to harden
against issues that in all likelihood will never happen.
^ permalink raw reply
* Re: [PATCH v7 0/2] SEV-SNP: Add KVM support for SNP certificate fetching
From: Sean Christopherson @ 2026-02-04 0:10 UTC (permalink / raw)
To: Sean Christopherson, kvm, Michael Roth
Cc: linux-coco, linux-kernel, pbonzini, jroedel, thomas.lendacky,
liam.merwick, huibo.wang
In-Reply-To: <20260109231732.1160759-1-michael.roth@amd.com>
On Fri, 09 Jan 2026 17:17:31 -0600, Michael Roth wrote:
> This patchset is also available at:
>
> https://github.com/amdese/linux/commits/snp-certs-v7
>
> and is based on top of kvm/next (0499add8efd7)
>
>
> [...]
Applied to kvm-x86 svm, thanks!
[1/2] KVM: Introduce KVM_EXIT_SNP_REQ_CERTS for SNP certificate-fetching
https://github.com/kvm-x86/linux/commit/fa9893fadbc2
[2/2] KVM: SEV: Add KVM_SEV_SNP_ENABLE_REQ_CERTS command
https://github.com/kvm-x86/linux/commit/20c3c4108d58
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-02-03 23:54 UTC (permalink / raw)
To: Sean Christopherson
Cc: Dave Hansen, linux-coco, linux-kernel, kvm, x86, reinette.chatre,
ira.weiny, kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve,
paulmck, nik.borisov, zhenzhong.duan, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, Farrah Chen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <aYIXFmT-676oN6j0@google.com>
>On Fri, Jan 30, 2026 at 8:23 AM Dave Hansen <dave.hansen@intel.com> wrote:
>> On 1/30/26 00:08, Chao Gao wrote:
>> > AFAIK, this is a CPU implementation issue. The actual requirement is to
>> > evict (flush and invalidate) all VMCSs __cached in SEAM mode__, but big
>> > cores implement this by evicting the __entire__ VMCS cache. So, the
>> > current VMCS is invalidated and cleared.
>>
>> But why is this a P-SEAMLDR thing and not a TDX module thing?
>
>My guess is that it's because the P-SEAMLDR code loads and prepares the new TDX-
>Module by constructing the VMCS used for SEAMCALL using direct writes to memory
>(unless that TDX behavior has changed in the last few years). And so it needs
>to ensure that in-memory representation is synchronized with the VMCS cache.
>
>Hmm, but that doesn't make sense _if_ it really truly is SEAMRET that does the VMCS
>cache invalidation, because flushing the VMCS cache would ovewrite the in-memory
>state.
My understanding is:
1. SEAMCALL/SEAMRET use VMCSs.
2. P-SEAMLDR is single-threaded (likely for simplicity). So, it uses a _single_
global VMCS and only one CPU can call P-SEAMLDR calls at a time.
3. After SEAMRET from P-SEAMLDR, _if_ the global VMCS isn't flushed, other CPUs
cannot enter P-SEAMLDR because the global VMCS would be corrupted. (note the
global VMCS is cached by the original CPU).
4. To make P-SEAMLDR callable on all CPUs, SEAMRET instruction flush VMCSs.
The flush cannot be performed by the host VMM since the global VMCS is not
visible to it. P-SEAMLDR cannot do it either because SEAMRET is its final
instruction and requires a valid VMCS.
The TDX Module has per-CPU VMCSs, so it doesn't has this problem.
I'll check if SEAM ISA architects can join to explain this in more detail.
>
>> It seems like a bug, or at least a P-SEAMLDR implementation issue the
>> needs to get fixed.
>
>Yeah, 'tis odd behavior. IMO, that's all the more reason the TDX subsystem should
>hide the quirk from the rest of the kernel.
>
>[*] https://lore.kernel.org/all/20251010220403.987927-1-seanjc@google.com
^ permalink raw reply
* [PATCH v2 3/3] x86/sev: Rename SNP_FEATURES_PRESENT->SNP_FEATURES_IMPL
From: Kim Phillips @ 2026-02-03 22:24 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Kim Phillips, stable
In-Reply-To: <20260203222405.4065706-1-kim.phillips@amd.com>
Rename SNP_FEATURES_PRESENT->SNP_FEATURES_IMPL to denote its
counterpart relationship with SNP_FEATURES_IMPL_REQ.
Fixes: 8c29f0165405 ("x86/sev: Add SEV-SNP guest feature negotiation support")
Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Nikunj A Dadhania <nikunj@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v2: new this series
arch/x86/boot/compressed/sev.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 2b639703b8dd..aca5313d193c 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -198,11 +198,11 @@ bool sev_es_check_ghcb_fault(unsigned long address)
#endif
/*
- * SNP_FEATURES_PRESENT is the mask of SNP features that are implemented
+ * SNP_FEATURES_IMPL is the mask of SNP features that are implemented
* by the guest kernel. As and when a new feature is implemented in the
* guest kernel, a corresponding bit should be added to the mask.
*/
-#define SNP_FEATURES_PRESENT (MSR_AMD64_SNP_DEBUG_SWAP | \
+#define SNP_FEATURES_IMPL (MSR_AMD64_SNP_DEBUG_SWAP | \
MSR_AMD64_SNP_SECURE_TSC | \
SNP_FEATURE_SECURE_AVIC)
@@ -211,7 +211,7 @@ u64 snp_get_unsupported_features(u64 status)
if (!(status & MSR_AMD64_SEV_SNP_ENABLED))
return 0;
- return status & SNP_FEATURES_IMPL_REQ & ~SNP_FEATURES_PRESENT;
+ return status & SNP_FEATURES_IMPL_REQ & ~SNP_FEATURES_IMPL;
}
void snp_check_features(void)
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] KVM: SEV: Add support for IBPB-on-Entry
From: Kim Phillips @ 2026-02-03 22:24 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Kim Phillips
In-Reply-To: <20260203222405.4065706-1-kim.phillips@amd.com>
AMD EPYC 5th generation and above processors support IBPB-on-Entry
for SNP guests. By invoking an Indirect Branch Prediction Barrier
(IBPB) on VMRUN, old indirect branch predictions are prevented
from influencing indirect branches within the guest.
SNP guests may choose to enable IBPB-on-Entry by setting
SEV_FEATURES bit 21 (IbpbOnEntry).
Host support for IBPB on Entry is indicated by CPUID
Fn8000_001F[IbpbOnEntry], bit 31.
If supported, indicate support for IBPB on Entry in
sev_supported_vmsa_features bit 23 (IbpbOnEntry).
For more info, refer to page 615, Section 15.36.17 "Side-Channel
Protection", AMD64 Architecture Programmer's Manual Volume 2: System
Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
v2: Added Tom's Reviewed-by.
v1: https://lore.kernel.org/kvm/20260126224205.1442196-3-kim.phillips@amd.com/
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 9 ++++++++-
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index c01fdde465de..3ce5dff36f78 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -459,6 +459,7 @@
#define X86_FEATURE_ALLOWED_SEV_FEATURES (19*32+27) /* Allowed SEV Features */
#define X86_FEATURE_SVSM (19*32+28) /* "svsm" SVSM present */
#define X86_FEATURE_HV_INUSE_WR_ALLOWED (19*32+30) /* Allow Write to in-use hypervisor-owned pages */
+#define X86_FEATURE_IBPB_ON_ENTRY (19*32+31) /* SEV-SNP IBPB on VM Entry */
/* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
#define X86_FEATURE_NO_NESTED_DATA_BP (20*32+ 0) /* No Nested Data Breakpoints */
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index edde36097ddc..eebc65ec948f 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -306,6 +306,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
+#define SVM_SEV_FEAT_IBPB_ON_ENTRY BIT(21)
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ea515cf41168..8a6d25db0c00 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3165,8 +3165,15 @@ void __init sev_hardware_setup(void)
cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
- if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
+ if (!sev_snp_enabled)
+ return;
+ /* the following feature bit checks are SNP specific */
+
+ if (tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+
+ if (cpu_feature_enabled(X86_FEATURE_IBPB_ON_ENTRY))
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_IBPB_ON_ENTRY;
}
void sev_hardware_unsetup(void)
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] x86/sev: Allow IBPB-on-Entry feature for SNP guests
From: Kim Phillips @ 2026-02-03 22:24 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Kim Phillips, stable
In-Reply-To: <20260203222405.4065706-1-kim.phillips@amd.com>
The SEV-SNP IBPB-on-Entry feature does not require a guest-side
implementation. The feature was added in Zen5 h/w, after the first
SNP Zen implementation, and thus was not accounted for when the
initial set of SNP features were added to the kernel.
In its abundant precaution, commit 8c29f0165405 ("x86/sev: Add SEV-SNP
guest feature negotiation support") included SEV_STATUS' IBPB-on-Entry
bit as a reserved bit, thereby masking guests from using the feature.
Allow guests to make use of IBPB-on-Entry when supported by the
hypervisor, as the bit is now architecturally defined and safe to
expose.
Fixes: 8c29f0165405 ("x86/sev: Add SEV-SNP guest feature negotiation support")
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: Michael Roth <michael.roth@amd.com>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v2:
- Change title (Nikunj)
- Add reviews-by (Nikunj, Tom)
- Change the description to more generally explain what the patch does (Boris)
v1: https://lore.kernel.org/kvm/20260126224205.1442196-2-kim.phillips@amd.com/
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/coco/sev/core.c | 1 +
arch/x86/include/asm/msr-index.h | 5 ++++-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index c8c1464b3a56..2b639703b8dd 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -188,6 +188,7 @@ bool sev_es_check_ghcb_fault(unsigned long address)
MSR_AMD64_SNP_RESERVED_BIT13 | \
MSR_AMD64_SNP_RESERVED_BIT15 | \
MSR_AMD64_SNP_SECURE_AVIC | \
+ MSR_AMD64_SNP_RESERVED_BITS19_22 | \
MSR_AMD64_SNP_RESERVED_MASK)
#ifdef CONFIG_AMD_SECURE_AVIC
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 9ae3b11754e6..13f608117411 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -122,6 +122,7 @@ static const char * const sev_status_feat_names[] = {
[MSR_AMD64_SNP_VMSA_REG_PROT_BIT] = "VMSARegProt",
[MSR_AMD64_SNP_SMT_PROT_BIT] = "SMTProt",
[MSR_AMD64_SNP_SECURE_AVIC_BIT] = "SecureAVIC",
+ [MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT] = "IBPBOnEntry",
};
/*
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 4d3566bb1a93..9016a6b00bc7 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -735,7 +735,10 @@
#define MSR_AMD64_SNP_SMT_PROT BIT_ULL(MSR_AMD64_SNP_SMT_PROT_BIT)
#define MSR_AMD64_SNP_SECURE_AVIC_BIT 18
#define MSR_AMD64_SNP_SECURE_AVIC BIT_ULL(MSR_AMD64_SNP_SECURE_AVIC_BIT)
-#define MSR_AMD64_SNP_RESV_BIT 19
+#define MSR_AMD64_SNP_RESERVED_BITS19_22 GENMASK_ULL(22, 19)
+#define MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT 23
+#define MSR_AMD64_SNP_IBPB_ON_ENTRY BIT_ULL(MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT)
+#define MSR_AMD64_SNP_RESV_BIT 24
#define MSR_AMD64_SNP_RESERVED_MASK GENMASK_ULL(63, MSR_AMD64_SNP_RESV_BIT)
#define MSR_AMD64_SAVIC_CONTROL 0xc0010138
#define MSR_AMD64_SAVIC_EN_BIT 0
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/3] KVM: SEV: Add support for IBPB-on-Entry
From: Kim Phillips @ 2026-02-03 22:24 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Kim Phillips
AMD EPYC 5th generation and above processors support IBPB-on-Entry
for SNP guests. By invoking an Indirect Branch Prediction Barrier
(IBPB) on VMRUN, old indirect branch predictions are prevented
from influencing indirect branches within the guest.
The first patch is guest-side support which unmasks the Zen5+ feature
bit to allow kernel guests to set the feature.
The second patch is host-side support that checks the CPUID and
then sets the feature bit in the VMSA supported features mask.
The third patch is a trivial #define rename that was a result of
the review discussion from v1's 2/2, to clarify SEV features
that are implemented in the guest.
Based on https://github.com/kvm-x86/linux kvm-x86/next
(currently v6.19-rc6-182-ge944fe2c09f4).
This v2 series now also available here:
https://github.com/AMDESE/linux/tree/ibpb-on-entry-latest
Advance qemu bits (to add ibpb-on-entry=on/off switch) available here:
https://github.com/AMDESE/qemu/tree/ibpb-on-entry-latest
Qemu bits will be posted upstream once kernel bits are merged.
They depend on Naveen Rao's "target/i386: SEV: Add support for
enabling VMSA SEV features":
https://lore.kernel.org/qemu-devel/cover.1761648149.git.naveen@kernel.org/
---
v2:
- Change first patch's title (Nikunj)
- Add reviews-by (Nikunj, Tom)
- Change second patch's description to more generally explain what the patch does (Boris)
- Add new, third patch renaming SNP_FEATURES_PRESENT->SNP_FEATURES_IMPL
v1: https://lore.kernel.org/kvm/20260126224205.1442196-1-kim.phillips@amd.com/
Kim Phillips (3):
x86/sev: Allow IBPB-on-Entry feature for SNP guests
KVM: SEV: Add support for IBPB-on-Entry
x86/sev: Rename SNP_FEATURES_PRESENT->SNP_FEATURES_IMPL
arch/x86/boot/compressed/sev.c | 7 ++++---
arch/x86/coco/sev/core.c | 1 +
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/msr-index.h | 5 ++++-
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 9 ++++++++-
6 files changed, 19 insertions(+), 5 deletions(-)
base-commit: e944fe2c09f405a2e2d147145c9b470084bc4c9a
--
2.43.0
^ permalink raw reply
* Re: [RFC PATCH v5 02/45] KVM: x86/mmu: Update iter->old_spte if cmpxchg64 on mirror SPTE "fails"
From: Huang, Kai @ 2026-02-03 21:34 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
Yamahata, Isaku, sagis@google.com, tglx@kernel.org,
Edgecombe, Rick P, bp@alien8.de, Annapurve, Vishal,
x86@kernel.org
In-Reply-To: <aYJVRQMW8yeTkRxR@google.com>
On Tue, 2026-02-03 at 12:06 -0800, Sean Christopherson wrote:
> On Tue, Feb 03, 2026, Kai Huang wrote:
> > On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > > Pass a pointer to iter->old_spte, not simply its value, when setting an
> > > external SPTE in __tdp_mmu_set_spte_atomic(), so that the iterator's value
> > > will be updated if the cmpxchg64 to freeze the mirror SPTE fails. The bug
> > > is currently benign as TDX is mutualy exclusive with all paths that do
> > > "local" retry", e.g. clear_dirty_gfn_range() and wrprot_gfn_range().
> > >
> > > Fixes: 77ac7079e66d ("KVM: x86/tdp_mmu: Propagate building mirror page tables")
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> >
> > Reviewed-by: Kai Huang <kai.huang@intel.com>
> >
> > Btw, do we need to cc stable?
>
> Probably not? The bug is benign until dirty logging comes along, and if someone
> backports that support (if it ever manifests) to an older kernel, it's firmly
> that person's responsibility to pick up dependencies like this.
Makes sense. :-)
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Huang, Kai @ 2026-02-03 21:29 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
Yamahata, Isaku, sagis@google.com, tglx@kernel.org,
Edgecombe, Rick P, bp@alien8.de, Annapurve, Vishal,
x86@kernel.org
In-Reply-To: <aYJWvKagesT3FPfI@google.com>
On Tue, 2026-02-03 at 12:12 -0800, Sean Christopherson wrote:
> On Tue, Feb 03, 2026, Kai Huang wrote:
> > On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > > Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> > > so that x86's TDX can update per-page metadata on allocation and free().
> > >
> > > Name the allocator page_get() to align with __get_free_page(), e.g. to
> > > communicate that it returns an "unsigned long", not a "struct page", and
> > > to avoid collisions with macros, e.g. with alloc_page.
> > >
> > > Suggested-by: Kai Huang <kai.huang@intel.com>
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> >
> > I thought it could be more generic for allocating an object, but not just a
> > page.
> >
> > E.g., I thought we might be able to use it to allocate a structure which has
> > "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> > it seems you abandoned this idea. May I ask why? Just want to understand
> > the reasoning here.
>
> Because that requires more complexity and there's no known use case, and I don't
> see an obvious way for a use case to come along. All of the motiviations for a
> custom allocation scheme that I can think of apply only to full pages, or fit
> nicely in a kmem_cache.
>
> Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
> "page" usage. Further splitting the "page" case doesn't require modifications to
> the "kmem_cache" case, whereas providing a fully generic solution would require
> additional changes, e.g. to handle this code:
>
> page = (void *)__get_free_page(gfp_flags);
> if (page && mc->init_value)
> memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
>
> It certainly wouldn't be much complexity, but this code is already a bit awkward,
> so I don't think it makes sense to add support for something that will probably
> never be used.
For this particular piece of code, we can add a helper for allocating normal
page table pages, get rid of mc->init_value completely and hook mc-
>page_get() to that helper.
A bonus is we can then call that helper in all places when KVM needs to
allocate a page for normal page table instead of just calling
get_zerod_pages() directly, e.g., like the one in
tdp_mmu_alloc_sp_for_split(), so that we can have a consistent way for
allocating normal page table pages.
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Huang, Kai @ 2026-02-03 21:18 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
Yamahata, Isaku, sagis@google.com, tglx@kernel.org,
Edgecombe, Rick P, bp@alien8.de, Annapurve, Vishal,
x86@kernel.org
In-Reply-To: <aYJX3usu7FzPrFWa@google.com>
On Tue, 2026-02-03 at 12:17 -0800, Sean Christopherson wrote:
> On Tue, Feb 03, 2026, Kai Huang wrote:
> > On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > > int (*set_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > > u64 mirror_spte);
> > > -
> > > - /* Update external page tables for page table about to be freed. */
> > > void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> > > struct kvm_mmu_page *sp);
> > > -
> > > - /* Update external page table from spte getting removed, and flush TLB. */
> >
> > The above two comments are still useful to me.
> >
> > Not sure why do you want to remove them, especially in _this_ patch?
>
> My intent was to replace the individual comments with a more generic comment for
> all of the "external" hooks. For things like "and flush TLB", IMO those comments
> belong at the call sites, not at this point. E.g. _KVM_ doesn't require a TLB
> flush in all cases. And so for the definition of the hooks, I would prefer a more
> generic comment, so that if there are details that matter to the usage, they are
> documented there.
I see. You actually mentioned "propagate changes in mirror page tables to
the external pages" in the new comment, so all make sense to me now.
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Sean Christopherson @ 2026-02-03 21:17 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: Kai Huang, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Xiaoyao Li, Yan Y Zhao, dave.hansen@linux.intel.com,
kas@kernel.org, mingo@redhat.com, binbin.wu@linux.intel.com,
pbonzini@redhat.com, Isaku Yamahata, ackerleytng@google.com,
linux-kernel@vger.kernel.org, sagis@google.com, tglx@kernel.org,
bp@alien8.de, Vishal Annapurve, x86@kernel.org
In-Reply-To: <c01b2f81e025dd38be90d3820260c488c7eb22ce.camel@intel.com>
On Tue, Feb 03, 2026, Rick P Edgecombe wrote:
> On Tue, 2026-02-03 at 12:12 -0800, Sean Christopherson wrote:
> > > E.g., I thought we might be able to use it to allocate a structure which has
> > > "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> > > it seems you abandoned this idea. May I ask why? Just want to understand
> > > the reasoning here.
> >
> > Because that requires more complexity and there's no known use case, and I
> > don't see an obvious way for a use case to come along. All of the
> > motiviations for a custom allocation scheme that I can think of apply only to
> > full pages, or fit nicely in a kmem_cache.
> >
> > Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
> > "page" usage. Further splitting the "page" case doesn't require modifications
> > to the "kmem_cache" case, whereas providing a fully generic solution would
> > require additional changes, e.g. to handle this code:
> >
> > page = (void *)__get_free_page(gfp_flags);
> > if (page && mc->init_value)
> > memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
> >
> > It certainly wouldn't be much complexity, but this code is already a bit
> > awkward, so I don't think it makes sense to add support for something that
> > will probably never be used.
>
> The thing that the design needlessly works around is that we can rely on that
> there are only two DPAMT pages per 2MB range. We don't need the dynamic page
> count allocations.
>
> This means we don't need to pass around the list of pages that lets arch/x86
> take as many pages as it needs. We can maybe just pass in a struct like Kai was
> suggesting to the get/put helpers. So I was in the process of trying to morph
> this series in that direction to get rid of the complexity resulting from the
> dynamic assumption.
>
> This was what I had done in response to v4 discussions, so now retrofitting it
> into this new ops scheme. Care to warn me off of this before I have something to
> show?
That's largely orthogonal to this change. This change is about preparing the
DPAMT when S-EPT page is allocated versus being installed. The fact that DPAMT
requires at most two pages versus a more dynamic maximum is irrelevant.
The caches aren't about dynamic sizes (though they play nicely with them), they're
about:
(a) not having to deal with allocating under spinlock
(b) not having to free memory that goes unused (for a single page fault)
(c) batching allocations for performance reasons (with the caveat that I doubt
anyone has measured the performance impact in many, many years).
None of those talking points change at all if KVM needs to provide 2 pages versus
N pages. The max number of pages needed for page tables is pretty much the same
thing as DPAMT, just with a higher max (4/5 vs. 2). In both cases, the allocated
pages may or may not be consumed for any given fault.
For the leaf pages (including the hugepage splitting cases), which don't utilize
KVM's kvm_mmu_memory_cache, I wouldn't expect the KVM details to change all that
much. In fact, they shouldn't change at all, because tracking 2 pages versus N
pages in "struct tdx_pamt_cache" is a detail that is 100% buried in the TDX subsystem
(which was pretty much the entire goal of my design).
Though maybe I'm misunderstanding what you have in mind?
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Edgecombe, Rick P @ 2026-02-03 20:33 UTC (permalink / raw)
To: seanjc@google.com, Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
Yamahata, Isaku, ackerleytng@google.com,
linux-kernel@vger.kernel.org, sagis@google.com, tglx@kernel.org,
bp@alien8.de, Annapurve, Vishal, x86@kernel.org
In-Reply-To: <aYJWvKagesT3FPfI@google.com>
On Tue, 2026-02-03 at 12:12 -0800, Sean Christopherson wrote:
> > E.g., I thought we might be able to use it to allocate a structure which has
> > "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> > it seems you abandoned this idea. May I ask why? Just want to understand
> > the reasoning here.
>
> Because that requires more complexity and there's no known use case, and I
> don't see an obvious way for a use case to come along. All of the
> motiviations for a custom allocation scheme that I can think of apply only to
> full pages, or fit nicely in a kmem_cache.
>
> Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
> "page" usage. Further splitting the "page" case doesn't require modifications
> to the "kmem_cache" case, whereas providing a fully generic solution would
> require additional changes, e.g. to handle this code:
>
> page = (void *)__get_free_page(gfp_flags);
> if (page && mc->init_value)
> memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
>
> It certainly wouldn't be much complexity, but this code is already a bit
> awkward, so I don't think it makes sense to add support for something that
> will probably never be used.
The thing that the design needlessly works around is that we can rely on that
there are only two DPAMT pages per 2MB range. We don't need the dynamic page
count allocations.
This means we don't need to pass around the list of pages that lets arch/x86
take as many pages as it needs. We can maybe just pass in a struct like Kai was
suggesting to the get/put helpers. So I was in the process of trying to morph
this series in that direction to get rid of the complexity resulting from the
dynamic assumption.
This was what I had done in response to v4 discussions, so now retrofitting it
into this new ops scheme. Care to warn me off of this before I have something to
show?
^ permalink raw reply
* Re: [RFC PATCH v5 11/45] x86/tdx: Add helpers to check return status codes
From: Sean Christopherson @ 2026-02-03 20:32 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: Dave Hansen, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Kai Huang, Xiaoyao Li, Yan Y Zhao, dave.hansen@linux.intel.com,
kas@kernel.org, binbin.wu@linux.intel.com, mingo@redhat.com,
pbonzini@redhat.com, ackerleytng@google.com,
linux-kernel@vger.kernel.org, Isaku Yamahata, sagis@google.com,
tglx@kernel.org, bp@alien8.de, Vishal Annapurve, x86@kernel.org
In-Reply-To: <8c5aca4bacb31475a510e6a109956e7fa4a63de5.camel@intel.com>
On Fri, Jan 30, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-01-29 at 12:35 -0800, Sean Christopherson wrote:
> > On Thu, Jan 29, 2026, Dave Hansen wrote:
> > > On 1/28/26 17:14, Sean Christopherson wrote:
> > > ...
> > > > err = tdh_mng_vpflushdone(&kvm_tdx->td);
> > > > - if (err == TDX_FLUSHVP_NOT_DONE)
> > > > + if (IS_TDX_FLUSHVP_NOT_DONE(err))
> > > > goto out;
> > > > if (TDX_BUG_ON(err, TDH_MNG_VPFLUSHDONE, kvm)) {
> > >
> > > I really despise the non-csopeable, non-ctaggable, non-greppable names
> > > like this. Sometimes it's unavoidable. Is it really unavoidable here?
> > >
> > > Something like this is succinct enough and doesn't have any magic ##
> > > macro definitions:
> > >
> > > TDX_ERR_EQ(err, TDX_FLUSHVP_NOT_DONE)
>
> I like the editor friendliness. The only downside is that it puts the onus on
> the caller to make sure supported defines are passed into TDX_ERR_EQ().
Eh, that's easy enough to handle with a static_assert().
> Today there are a few special cases like IS_TDX_NON_RECOVERABLE().
Why bother with a wrapper for that one? It's a single bit, just test that bit.
For me, providing IS_TDX_NON_RECOVERABLE() is _more_ confusing, because it
suggests that there's a NON_RECOVERABLE error, when in fact (IIUC) it's more or
less a modifier.
> I don't know, I'm ok either way. I lean towards keeping it as in this patch
> because we already had an error code bit interpretation bug:
> https://lore.kernel.org/kvm/24d2f165-f854-4996-89cf-28d644c592a3@intel.com/
>
> So the centralization of bit interpretation seems like a real win.
>
> > FWIW, I have zero preference on this. I included the patch purely because it was
> > already there.
>
> Ha, actually we all had a long thread on this:
> https://lore.kernel.org/kvm/70484aa1b553ca250d893f80b2687b5d915e5309.camel@intel.com/
Oh, it's _that_ discussion :-)
What I meant was, "I don't have a strong preference between TDX_ERR_EQ() and
this patch". What I didn't like was tdx_operand_invalid(), because that reads
like a command and it's not at all clear that it's macro-like in behavior.
I'd vote for IS_TDX_ERR() over TDX_ERR_EQ(), but either works for me (as does
this patch).
> I see now that we closed it with you but never got Dave's final buy in.
^ 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-03 20:17 UTC (permalink / raw)
To: Kai Huang
Cc: x86@kernel.org, dave.hansen@linux.intel.com, kas@kernel.org,
bp@alien8.de, mingo@redhat.com, pbonzini@redhat.com,
tglx@kernel.org, Rick P Edgecombe, ackerleytng@google.com,
sagis@google.com, Vishal Annapurve, linux-kernel@vger.kernel.org,
Yan Y Zhao, Xiaoyao Li, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Isaku Yamahata,
binbin.wu@linux.intel.com
In-Reply-To: <4fae16cdcc368d33f128c3a79c788b905b83ffe7.camel@intel.com>
On Tue, Feb 03, 2026, Kai Huang wrote:
> On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > int (*set_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > u64 mirror_spte);
> > -
> > - /* Update external page tables for page table about to be freed. */
> > void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> > struct kvm_mmu_page *sp);
> > -
> > - /* Update external page table from spte getting removed, and flush TLB. */
>
> The above two comments are still useful to me.
>
> Not sure why do you want to remove them, especially in _this_ patch?
My intent was to replace the individual comments with a more generic comment for
all of the "external" hooks. For things like "and flush TLB", IMO those comments
belong at the call sites, not at this point. E.g. _KVM_ doesn't require a TLB
flush in all cases. And so for the definition of the hooks, I would prefer a more
generic comment, so that if there are details that matter to the usage, they are
documented there.
> > void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > u64 mirror_spte);
> >
> > +
>
> Unintentional change?
Ya.
>
> > bool (*has_wbinvd_exit)(void);
> >
> > u64 (*get_l2_tsc_offset)(struct kvm_vcpu *vcpu);
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 3911ac9bddfd..9b5a6861e2a4 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -6690,11 +6690,13 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
> > vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache;
> > vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO;
> >
> > - vcpu->arch.mmu_shadow_page_cache.init_value =
> > - SHADOW_NONPRESENT_VALUE;
> > + vcpu->arch.mmu_shadow_page_cache.init_value = SHADOW_NONPRESENT_VALUE;
> > if (!vcpu->arch.mmu_shadow_page_cache.init_value)
> > vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO;
>
> Ditto. Not sure this adjustment is intentional?
Heh, I'm pretty sure it was intentional, but yeah, doesn't belong here.
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Sean Christopherson @ 2026-02-03 20:12 UTC (permalink / raw)
To: Kai Huang
Cc: x86@kernel.org, dave.hansen@linux.intel.com, kas@kernel.org,
bp@alien8.de, mingo@redhat.com, pbonzini@redhat.com,
tglx@kernel.org, Rick P Edgecombe, ackerleytng@google.com,
sagis@google.com, Vishal Annapurve, linux-kernel@vger.kernel.org,
Yan Y Zhao, Xiaoyao Li, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Isaku Yamahata,
binbin.wu@linux.intel.com
In-Reply-To: <de05853257e9cc66998101943f78a4b7e6e3d741.camel@intel.com>
On Tue, Feb 03, 2026, Kai Huang wrote:
> On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> > so that x86's TDX can update per-page metadata on allocation and free().
> >
> > Name the allocator page_get() to align with __get_free_page(), e.g. to
> > communicate that it returns an "unsigned long", not a "struct page", and
> > to avoid collisions with macros, e.g. with alloc_page.
> >
> > Suggested-by: Kai Huang <kai.huang@intel.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
>
> I thought it could be more generic for allocating an object, but not just a
> page.
>
> E.g., I thought we might be able to use it to allocate a structure which has
> "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> it seems you abandoned this idea. May I ask why? Just want to understand
> the reasoning here.
Because that requires more complexity and there's no known use case, and I don't
see an obvious way for a use case to come along. All of the motiviations for a
custom allocation scheme that I can think of apply only to full pages, or fit
nicely in a kmem_cache.
Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
"page" usage. Further splitting the "page" case doesn't require modifications to
the "kmem_cache" case, whereas providing a fully generic solution would require
additional changes, e.g. to handle this code:
page = (void *)__get_free_page(gfp_flags);
if (page && mc->init_value)
memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
It certainly wouldn't be much complexity, but this code is already a bit awkward,
so I don't think it makes sense to add support for something that will probably
never be used.
^ 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