* [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
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);
/* 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)
{
- 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 related
* [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
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)
+ 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 related
* [RFC PATCH v5 07/45] KVM: x86/mmu: Plumb the SPTE _pointer_ into the TDP MMU's handle_changed_spte()
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
Plumb the SPTE pointer into handle_changed_spte() so that remove leaf
mirror entries can be forwarded to TDX in handle_changed_spte(), instead
of effectively requiring callers to manually do so. Relying on each
caller to invoke .remove_external_spte() is confusing and brittle, e.g.
subtly relies tdp_mmu_set_spte_atomic() never removing SPTEs. This will
also allow consolidating all S-EPT updates into a single kvm_x86_ops hook.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/mmu/tdp_mmu.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 6fb48b217f5b..8743cd020d12 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -320,9 +320,9 @@ void kvm_tdp_mmu_alloc_root(struct kvm_vcpu *vcpu, bool mirror)
}
}
-static void handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn,
- u64 old_spte, u64 new_spte, int level,
- bool shared);
+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 void tdp_account_mmu_page(struct kvm *kvm, struct kvm_mmu_page *sp)
{
@@ -471,7 +471,7 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
old_spte = kvm_tdp_mmu_write_spte(sptep, old_spte,
FROZEN_SPTE, level);
}
- handle_changed_spte(kvm, kvm_mmu_page_as_id(sp), gfn,
+ handle_changed_spte(kvm, kvm_mmu_page_as_id(sp), sptep, gfn,
old_spte, FROZEN_SPTE, level, shared);
if (is_mirror_sp(sp)) {
@@ -499,6 +499,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
* @kvm: kvm instance
* @as_id: the address space of the paging structure the SPTE was a part of
+ * @sptep: pointer to the SPTE
* @gfn: the base GFN that was mapped by the SPTE
* @old_spte: The value of the SPTE before the change
* @new_spte: The value of the SPTE after the change
@@ -511,9 +512,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, gfn_t gfn,
- u64 old_spte, u64 new_spte, int level,
- bool shared)
+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)
{
bool was_present = is_shadow_present_pte(old_spte);
bool is_present = is_shadow_present_pte(new_spte);
@@ -685,8 +686,8 @@ static inline int __must_check tdp_mmu_set_spte_atomic(struct kvm *kvm,
if (ret)
return ret;
- handle_changed_spte(kvm, iter->as_id, iter->gfn, iter->old_spte,
- new_spte, iter->level, true);
+ handle_changed_spte(kvm, iter->as_id, iter->sptep, iter->gfn,
+ iter->old_spte, new_spte, iter->level, true);
return 0;
}
@@ -720,7 +721,7 @@ static u64 tdp_mmu_set_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
old_spte = kvm_tdp_mmu_write_spte(sptep, old_spte, new_spte, level);
- handle_changed_spte(kvm, as_id, gfn, old_spte, new_spte, level, false);
+ handle_changed_spte(kvm, as_id, sptep, gfn, old_spte, new_spte, level, false);
/*
* Users that do non-atomic setting of PTEs don't operate on mirror
--
2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply related
* [RFC PATCH v5 06/45] KVM: x86/mmu: Fold set_external_spte_present() into its sole caller
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
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;
- ret = set_external_spte_present(kvm, iter->sptep, iter->gfn,
- &iter->old_spte, new_spte, iter->level);
+ /*
+ * 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 related
* [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
Drop the dedicated .link_external_spt() for linking non-leaf S-EPT pages,
and instead funnel everything through .set_external_spte(). Using separate
hooks doesn't help prevent TDP MMU details from bleeding into TDX, and vice
versa; to the contrary, dedicated callbacks will result in _more_ pollution
when hugepage support is added, e.g. will require the TDP MMU to know
details about the splitting rules for TDX that aren't all that relevant to
the TDP MMU.
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.
However, that's the _only_ case that's truly special, and even that case
could be shoehorned into a single hook; it's just wouldn't be a net
positive.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm-x86-ops.h | 1 -
arch/x86/include/asm/kvm_host.h | 3 --
arch/x86/kvm/mmu/tdp_mmu.c | 37 +++---------------
arch/x86/kvm/vmx/tdx.c | 61 ++++++++++++++++++++----------
4 files changed, 48 insertions(+), 54 deletions(-)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index c18a033bee7e..57eb1f4832ae 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -94,7 +94,6 @@ KVM_X86_OP_OPTIONAL_RET0(set_tss_addr)
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(link_external_spt)
KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
KVM_X86_OP_OPTIONAL_RET0(free_external_spt)
KVM_X86_OP_OPTIONAL(remove_external_spte)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e441f270f354..d12ca0f8a348 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1853,9 +1853,6 @@ struct kvm_x86_ops {
void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, hpa_t root_hpa,
int root_level);
- /* Update external mapping with page table link. */
- int (*link_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
- void *external_spt);
/* Update the external page table from spte getting set. */
int (*set_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
u64 mirror_spte);
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 0feda295859a..56ad056e6042 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -495,31 +495,17 @@ 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 void *get_external_spt(gfn_t gfn, u64 new_spte, int level)
-{
- if (is_shadow_present_pte(new_spte) && !is_last_spte(new_spte, level)) {
- struct kvm_mmu_page *sp = spte_to_child_sp(new_spte);
-
- WARN_ON_ONCE(sp->role.level + 1 != level);
- WARN_ON_ONCE(sp->gfn != gfn);
- return sp->external_spt;
- }
-
- return NULL;
-}
-
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;
+
/*
* We need to lock out other updates to the SPTE until the external
* page table has been modified. Use FROZEN_SPTE similar to
@@ -528,18 +514,7 @@ static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sp
if (!try_cmpxchg64(rcu_dereference(sptep), old_spte, FROZEN_SPTE))
return -EBUSY;
- /*
- * Use different call to either set up middle level
- * external page table, or leaf.
- */
- if (is_leaf) {
- ret = kvm_x86_call(set_external_spte)(kvm, gfn, level, new_spte);
- } else {
- void *external_spt = get_external_spt(gfn, new_spte, level);
-
- KVM_BUG_ON(!external_spt, kvm);
- ret = kvm_x86_call(link_external_spt)(kvm, gfn, level, external_spt);
- }
+ ret = kvm_x86_call(set_external_spte)(kvm, gfn, level, new_spte);
if (ret)
__kvm_tdp_mmu_write_spte(sptep, *old_spte);
else
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;
+
+ 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)
+ return -EIO;
+
+ err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, external_spt,
+ &entry, &level_state);
+ if (unlikely(tdx_operand_busy(err)))
+ return -EBUSY;
+
+ if (TDX_BUG_ON_2(err, TDH_MEM_SEPT_ADD, entry, level_state, kvm))
+ return -EIO;
+
+ return 0;
+}
+
static int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn,
enum pg_level level, u64 mirror_spte)
{
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
kvm_pfn_t pfn = spte_to_pfn(mirror_spte);
+ if (KVM_BUG_ON(!is_shadow_present_pte(mirror_spte), kvm))
+ return -EIO;
+
+ if (!is_last_spte(mirror_spte, level))
+ return tdx_sept_link_private_spt(kvm, gfn, level, mirror_spte);
+
/* TODO: handle large pages. */
if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
return -EIO;
- 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);
/*
* Ensure pre_fault_allowed is read by kvm_arch_vcpu_pre_fault_memory()
@@ -1695,23 +1735,7 @@ static int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn,
return tdx_mem_page_aug(kvm, gfn, level, pfn);
}
-static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn,
- enum pg_level level, void *private_spt)
-{
- gpa_t gpa = gfn_to_gpa(gfn);
- struct page *page = virt_to_page(private_spt);
- u64 err, entry, level_state;
- err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, page, &entry,
- &level_state);
- if (unlikely(tdx_operand_busy(err)))
- return -EBUSY;
-
- if (TDX_BUG_ON_2(err, TDH_MEM_SEPT_ADD, entry, level_state, kvm))
- return -EIO;
-
- return 0;
-}
/*
* Ensure shared and private EPTs to be flushed on all vCPUs.
@@ -3592,7 +3616,6 @@ 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.link_external_spt = tdx_sept_link_private_spt;
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.remove_external_spte = tdx_sept_remove_private_spte;
--
2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply related
* [RFC PATCH v5 04/45] KVM: x86: Make "external SPTE" ops that can fail RET0 static calls
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
Define kvm_x86_ops .link_external_spt(), .set_external_spte(), and
.free_external_spt() as RET0 static calls so that an unexpected call to a
a default operation doesn't consume garbage.
Fixes: 77ac7079e66d ("KVM: x86/tdp_mmu: Propagate building mirror page tables")
Fixes: 94faba8999b9 ("KVM: x86/tdp_mmu: Propagate tearing down mirror page tables")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm-x86-ops.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index de709fb5bd76..c18a033bee7e 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -94,9 +94,9 @@ KVM_X86_OP_OPTIONAL_RET0(set_tss_addr)
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(link_external_spt)
-KVM_X86_OP_OPTIONAL(set_external_spte)
-KVM_X86_OP_OPTIONAL(free_external_spt)
+KVM_X86_OP_OPTIONAL_RET0(link_external_spt)
+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(has_wbinvd_exit)
KVM_X86_OP(get_l2_tsc_offset)
--
2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply related
* [RFC PATCH v5 03/45] KVM: TDX: Account all non-transient page allocations for per-TD structures
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
Account all non-transient allocations associated with a single TD (or its
vCPUs), as KVM's ABI is that allocations that are active for the lifetime
of a VM are accounted. Leave temporary allocations, i.e. allocations that
are freed within a single function/ioctl, unaccounted, to again align with
KVM's existing behavior, e.g. see commit dd103407ca31 ("KVM: X86: Remove
unnecessary GFP_KERNEL_ACCOUNT for temporary variables").
Fixes: 8d032b683c29 ("KVM: TDX: create/destroy VM structure")
Fixes: a50f673f25e0 ("KVM: TDX: Do TDX specific vcpu initialization")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/vmx/tdx.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 561461c9d131..5688c77616e3 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2397,7 +2397,7 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params,
atomic_inc(&nr_configured_hkid);
- tdr_page = alloc_page(GFP_KERNEL);
+ tdr_page = alloc_page(GFP_KERNEL_ACCOUNT);
if (!tdr_page)
goto free_hkid;
@@ -2405,12 +2405,12 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params,
/* TDVPS = TDVPR(4K page) + TDCX(multiple 4K pages), -1 for TDVPR. */
kvm_tdx->td.tdcx_nr_pages = tdx_sysinfo->td_ctrl.tdvps_base_size / PAGE_SIZE - 1;
tdcs_pages = kcalloc(kvm_tdx->td.tdcs_nr_pages, sizeof(*kvm_tdx->td.tdcs_pages),
- GFP_KERNEL);
+ GFP_KERNEL_ACCOUNT);
if (!tdcs_pages)
goto free_tdr;
for (i = 0; i < kvm_tdx->td.tdcs_nr_pages; i++) {
- tdcs_pages[i] = alloc_page(GFP_KERNEL);
+ tdcs_pages[i] = alloc_page(GFP_KERNEL_ACCOUNT);
if (!tdcs_pages[i])
goto free_tdcs;
}
@@ -2885,7 +2885,7 @@ static int tdx_td_vcpu_init(struct kvm_vcpu *vcpu, u64 vcpu_rcx)
int ret, i;
u64 err;
- page = alloc_page(GFP_KERNEL);
+ page = alloc_page(GFP_KERNEL_ACCOUNT);
if (!page)
return -ENOMEM;
tdx->vp.tdvpr_page = page;
@@ -2898,14 +2898,14 @@ static int tdx_td_vcpu_init(struct kvm_vcpu *vcpu, u64 vcpu_rcx)
tdx->vp.tdvpr_pa = page_to_phys(tdx->vp.tdvpr_page);
tdx->vp.tdcx_pages = kcalloc(kvm_tdx->td.tdcx_nr_pages, sizeof(*tdx->vp.tdcx_pages),
- GFP_KERNEL);
+ GFP_KERNEL_ACCOUNT);
if (!tdx->vp.tdcx_pages) {
ret = -ENOMEM;
goto free_tdvpr;
}
for (i = 0; i < kvm_tdx->td.tdcx_nr_pages; i++) {
- page = alloc_page(GFP_KERNEL);
+ page = alloc_page(GFP_KERNEL_ACCOUNT);
if (!page) {
ret = -ENOMEM;
goto free_tdcx;
--
2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply related
* [RFC PATCH v5 02/45] KVM: x86/mmu: Update iter->old_spte if cmpxchg64 on mirror SPTE "fails"
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
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>
---
arch/x86/kvm/mmu/tdp_mmu.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 9c26038f6b77..0feda295859a 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -509,10 +509,10 @@ static void *get_external_spt(gfn_t gfn, u64 new_spte, int level)
}
static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
- gfn_t gfn, u64 old_spte,
+ gfn_t gfn, u64 *old_spte,
u64 new_spte, int level)
{
- bool was_present = is_shadow_present_pte(old_spte);
+ 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;
@@ -525,7 +525,7 @@ static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sp
* page table has been modified. Use FROZEN_SPTE similar to
* the zapping case.
*/
- if (!try_cmpxchg64(rcu_dereference(sptep), &old_spte, FROZEN_SPTE))
+ if (!try_cmpxchg64(rcu_dereference(sptep), old_spte, FROZEN_SPTE))
return -EBUSY;
/*
@@ -541,7 +541,7 @@ static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sp
ret = kvm_x86_call(link_external_spt)(kvm, gfn, level, external_spt);
}
if (ret)
- __kvm_tdp_mmu_write_spte(sptep, old_spte);
+ __kvm_tdp_mmu_write_spte(sptep, *old_spte);
else
__kvm_tdp_mmu_write_spte(sptep, new_spte);
return ret;
@@ -670,7 +670,7 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
return -EBUSY;
ret = set_external_spte_present(kvm, iter->sptep, iter->gfn,
- iter->old_spte, new_spte, iter->level);
+ &iter->old_spte, new_spte, iter->level);
if (ret)
return ret;
} else {
--
2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply related
* [RFC PATCH v5 01/45] x86/tdx: Use pg_level in TDX APIs, not the TDX-Module's 0-based level
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: 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: <20260129011517.3545883-1-seanjc@google.com>
Rework the TDX APIs to take the kernel's 1-based pg_level enum, not the
TDX-Module's 0-based level. The APIs are _kernel_ APIs, not TDX-Module
APIs, and the kernel (and KVM) uses "enum pg_level" literally everywhere.
Using "enum pg_level" eliminates ambiguity when looking at the APIs (it's
NOT clear that "int level" refers to the TDX-Module's level), and will
allow for using existing helpers like page_level_size() when support for
hugepages is added to the S-EPT APIs.
No functional change intended.
Cc: Kai Huang <kai.huang@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Yan Zhao <yan.y.zhao@intel.com>
Cc: Vishal Annapurve <vannapurve@google.com>
Cc: Ackerley Tng <ackerleytng@google.com>
Acked-by: Kiryl Shutsemau <kas@kernel.org>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Tested-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Tested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/tdx.h | 14 ++++----------
arch/x86/kvm/vmx/tdx.c | 11 ++++-------
arch/x86/virt/vmx/tdx/tdx.c | 26 ++++++++++++++++++--------
3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 6b338d7f01b7..bc0d03e70fd6 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -189,19 +189,13 @@ static inline u64 mk_keyed_paddr(u16 hkid, struct page *page)
return ret;
}
-static inline int pg_level_to_tdx_sept_level(enum pg_level level)
-{
- WARN_ON_ONCE(level == PG_LEVEL_NONE);
- return level - 1;
-}
-
u64 tdh_vp_enter(struct tdx_vp *vp, struct tdx_module_args *args);
u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2);
-u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2);
+u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, enum pg_level level, struct page *page, u64 *ext_err1, u64 *ext_err2);
u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page);
-u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2);
-u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, int level, u64 *ext_err1, u64 *ext_err2);
+u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level, struct page *page, u64 *ext_err1, u64 *ext_err2);
+u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, enum pg_level level, u64 *ext_err1, u64 *ext_err2);
u64 tdh_mng_key_config(struct tdx_td *td);
u64 tdh_mng_create(struct tdx_td *td, u16 hkid);
u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp);
@@ -217,7 +211,7 @@ u64 tdh_vp_rd(struct tdx_vp *vp, u64 field, u64 *data);
u64 tdh_vp_wr(struct tdx_vp *vp, u64 field, u64 data, u64 mask);
u64 tdh_phymem_page_reclaim(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64 *tdx_size);
u64 tdh_mem_track(struct tdx_td *tdr);
-u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, u64 level, u64 *ext_err1, u64 *ext_err2);
+u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, enum pg_level level, u64 *ext_err1, u64 *ext_err2);
u64 tdh_phymem_cache_wb(bool resume);
u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td);
u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page);
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 5df9d32d2058..561461c9d131 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -1648,14 +1648,13 @@ static int tdx_mem_page_add(struct kvm *kvm, gfn_t gfn, enum pg_level level,
static int tdx_mem_page_aug(struct kvm *kvm, gfn_t gfn,
enum pg_level level, kvm_pfn_t pfn)
{
- int tdx_level = pg_level_to_tdx_sept_level(level);
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
struct page *page = pfn_to_page(pfn);
gpa_t gpa = gfn_to_gpa(gfn);
u64 entry, level_state;
u64 err;
- err = tdh_mem_page_aug(&kvm_tdx->td, gpa, tdx_level, page, &entry, &level_state);
+ err = tdh_mem_page_aug(&kvm_tdx->td, gpa, level, page, &entry, &level_state);
if (unlikely(tdx_operand_busy(err)))
return -EBUSY;
@@ -1699,12 +1698,11 @@ static int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn,
static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn,
enum pg_level level, void *private_spt)
{
- int tdx_level = pg_level_to_tdx_sept_level(level);
gpa_t gpa = gfn_to_gpa(gfn);
struct page *page = virt_to_page(private_spt);
u64 err, entry, level_state;
- err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, tdx_level, page, &entry,
+ err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, page, &entry,
&level_state);
if (unlikely(tdx_operand_busy(err)))
return -EBUSY;
@@ -1788,7 +1786,6 @@ static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
enum pg_level level, u64 mirror_spte)
{
struct page *page = pfn_to_page(spte_to_pfn(mirror_spte));
- int tdx_level = pg_level_to_tdx_sept_level(level);
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
gpa_t gpa = gfn_to_gpa(gfn);
u64 err, entry, level_state;
@@ -1808,7 +1805,7 @@ static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
return;
err = tdh_do_no_vcpus(tdh_mem_range_block, kvm, &kvm_tdx->td, gpa,
- tdx_level, &entry, &level_state);
+ level, &entry, &level_state);
if (TDX_BUG_ON_2(err, TDH_MEM_RANGE_BLOCK, entry, level_state, kvm))
return;
@@ -1824,7 +1821,7 @@ static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
* Race with TDH.VP.ENTER due to (0-step mitigation) and Guest TDCALLs.
*/
err = tdh_do_no_vcpus(tdh_mem_page_remove, kvm, &kvm_tdx->td, gpa,
- tdx_level, &entry, &level_state);
+ level, &entry, &level_state);
if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_REMOVE, entry, level_state, kvm))
return;
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 5ce4ebe99774..22c0f832cb37 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1516,6 +1516,12 @@ static void tdx_clflush_page(struct page *page)
clflush_cache_range(page_to_virt(page), PAGE_SIZE);
}
+static int pg_level_to_tdx_sept_level(enum pg_level level)
+{
+ WARN_ON_ONCE(level == PG_LEVEL_NONE);
+ return level - 1;
+}
+
noinstr u64 tdh_vp_enter(struct tdx_vp *td, struct tdx_module_args *args)
{
args->rcx = td->tdvpr_pa;
@@ -1556,10 +1562,11 @@ u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page
}
EXPORT_SYMBOL_FOR_KVM(tdh_mem_page_add);
-u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2)
+u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, enum pg_level level,
+ struct page *page, u64 *ext_err1, u64 *ext_err2)
{
struct tdx_module_args args = {
- .rcx = gpa | level,
+ .rcx = gpa | pg_level_to_tdx_sept_level(level),
.rdx = tdx_tdr_pa(td),
.r8 = page_to_phys(page),
};
@@ -1587,10 +1594,11 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page)
}
EXPORT_SYMBOL_FOR_KVM(tdh_vp_addcx);
-u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2)
+u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level,
+ struct page *page, u64 *ext_err1, u64 *ext_err2)
{
struct tdx_module_args args = {
- .rcx = gpa | level,
+ .rcx = gpa | pg_level_to_tdx_sept_level(level),
.rdx = tdx_tdr_pa(td),
.r8 = page_to_phys(page),
};
@@ -1606,10 +1614,11 @@ u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, int level, struct page *page, u
}
EXPORT_SYMBOL_FOR_KVM(tdh_mem_page_aug);
-u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, int level, u64 *ext_err1, u64 *ext_err2)
+u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, enum pg_level level,
+ u64 *ext_err1, u64 *ext_err2)
{
struct tdx_module_args args = {
- .rcx = gpa | level,
+ .rcx = gpa | pg_level_to_tdx_sept_level(level),
.rdx = tdx_tdr_pa(td),
};
u64 ret;
@@ -1822,10 +1831,11 @@ u64 tdh_mem_track(struct tdx_td *td)
}
EXPORT_SYMBOL_FOR_KVM(tdh_mem_track);
-u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, u64 level, u64 *ext_err1, u64 *ext_err2)
+u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, enum pg_level level,
+ u64 *ext_err1, u64 *ext_err2)
{
struct tdx_module_args args = {
- .rcx = gpa | level,
+ .rcx = gpa | pg_level_to_tdx_sept_level(level),
.rdx = tdx_tdr_pa(td),
};
u64 ret;
--
2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply related
* [RFC PATCH v5 00/45] TDX: Dynamic PAMT + S-EPT Hugepage
From: Sean Christopherson @ 2026-01-29 1:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Sean Christopherson, Paolo Bonzini
Cc: linux-kernel, linux-coco, kvm, Kai Huang, Rick Edgecombe,
Yan Zhao, Vishal Annapurve, Ackerley Tng, Sagi Shahar, Binbin Wu,
Xiaoyao Li, Isaku Yamahata
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
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
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.
Outside of the guest_memfd arch hook for in-place conversion, S-EPT hugepage
support doesn't have any direction dependencies/conflicts with guest_memfd
hugepage or in-place conversion support (which is great, because it means we
didn't totally botch the design!). E.g. Vishal's been able to test this code
precisely because it applies relatively cleanly on an internal branch with a
whole pile of guest_memfd changes.
Applies on kvm-x86 next (specifically kvm-x86-next-2026.01.23).
[*] https://lore.kernel.org/all/20251206011054.494190-1-seanjc@google.com
P.S. I apologize if I clobbered any of the Author attribution or SoBs. I
was moving patches around and synchronizing between an internal tree
and this upstream version, so things may have gotten a bit wonky.
Isaku Yamahata (1):
KVM: x86/tdp_mmu: Alloc external_spt page for mirror page table
splitting
Kiryl Shutsemau (12):
x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
x86/tdx: Add helpers to check return status codes
x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
x86/virt/tdx: Allocate reference counters for PAMT memory
x86/virt/tdx: Improve PAMT refcounts allocation for sparse memory
x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
x86/virt/tdx: Optimize tdx_alloc/free_control_page() helpers
KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
KVM: TDX: Get/put PAMT pages when (un)mapping private memory
x86/virt/tdx: Enable Dynamic PAMT
Documentation/x86: Add documentation for TDX's Dynamic PAMT
x86/virt/tdx: Get/Put DPAMT page pair if and only if mapping size is
4KB
Rick Edgecombe (3):
x86/virt/tdx: Simplify tdmr_get_pamt_sz()
x86/tdx: Add APIs to support get/put of DPAMT entries from KVM, under
spinlock
KVM: x86/mmu: Prevent hugepage promotion for mirror roots in fault
path
Sean Christopherson (22):
x86/tdx: Use pg_level in TDX APIs, not the TDX-Module's 0-based level
KVM: x86/mmu: Update iter->old_spte if cmpxchg64 on mirror SPTE
"fails"
KVM: TDX: Account all non-transient page allocations for per-TD
structures
KVM: x86: Make "external SPTE" ops that can fail RET0 static calls
KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use
.set_external_spte() for all
KVM: x86/mmu: Fold set_external_spte_present() into its sole caller
KVM: x86/mmu: Plumb the SPTE _pointer_ into the TDP MMU's
handle_changed_spte()
KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in
handle_changed_spte()
KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page
allocator
KVM: x86/mmu: Allocate/free S-EPT pages using
tdx_{alloc,free}_control_page()
*** DO NOT MERGE *** x86/virt/tdx: Don't assume guest memory is backed
by struct page
x86/virt/tdx: Extend "reset page" quirk to support huge pages
KVM: x86/mmu: Plumb the old_spte into kvm_x86_ops.set_external_spte()
KVM: TDX: Hoist tdx_sept_remove_private_spte() above
set_private_spte()
KVM: TDX: Handle removal of leaf SPTEs in .set_private_spte()
KVM: TDX: Add helper to handle mapping leaf SPTE into S-EPT
KVM: TDX: Move S-EPT page demotion TODO to tdx_sept_set_private_spte()
KVM: x86/mmu: Add Dynamic PAMT support in TDP MMU for vCPU-induced
page split
KVM: guest_memfd: Add helpers to get start/end gfns give
gmem+slot+pgoff
*** DO NOT MERGE *** KVM: guest_memfd: Add pre-zap arch hook for
shared<=>private conversion
KVM: x86/mmu: Add support for splitting S-EPT hugepages on conversion
Xiaoyao Li (1):
x86/virt/tdx: Add API to demote a 2MB mapping to 512 4KB mappings
Yan Zhao (6):
x86/virt/tdx: Enhance tdh_mem_page_aug() to support huge pages
x86/virt/tdx: Enhance tdh_phymem_page_wbinvd_hkid() to invalidate huge
pages
KVM: TDX: Add core support for splitting/demoting 2MiB S-EPT to 4KiB
KVM: x86: Introduce hugepage_set_guest_inhibit()
KVM: TDX: Honor the guest's accept level contained in an EPT violation
KVM: TDX: Turn on PG_LEVEL_2M
Documentation/arch/x86/tdx.rst | 21 +
arch/x86/coco/tdx/tdx.c | 10 +-
arch/x86/include/asm/kvm-x86-ops.h | 9 +-
arch/x86/include/asm/kvm_host.h | 36 +-
arch/x86/include/asm/shared/tdx.h | 1 +
arch/x86/include/asm/shared/tdx_errno.h | 104 +++
arch/x86/include/asm/tdx.h | 127 ++--
arch/x86/include/asm/tdx_global_metadata.h | 1 +
arch/x86/kvm/Kconfig | 1 +
arch/x86/kvm/mmu.h | 4 +
arch/x86/kvm/mmu/mmu.c | 34 +-
arch/x86/kvm/mmu/mmu_internal.h | 11 -
arch/x86/kvm/mmu/tdp_mmu.c | 315 ++++----
arch/x86/kvm/mmu/tdp_mmu.h | 2 +
arch/x86/kvm/vmx/tdx.c | 468 +++++++++---
arch/x86/kvm/vmx/tdx.h | 5 +-
arch/x86/kvm/vmx/tdx_arch.h | 3 +
arch/x86/kvm/vmx/tdx_errno.h | 40 -
arch/x86/virt/vmx/tdx/tdx.c | 762 +++++++++++++++++---
arch/x86/virt/vmx/tdx/tdx.h | 6 +-
arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 7 +
include/linux/kvm_host.h | 5 +
include/linux/kvm_types.h | 2 +
virt/kvm/Kconfig | 4 +
virt/kvm/guest_memfd.c | 71 +-
virt/kvm/kvm_main.c | 7 +-
26 files changed, 1576 insertions(+), 480 deletions(-)
create mode 100644 arch/x86/include/asm/shared/tdx_errno.h
delete mode 100644 arch/x86/kvm/vmx/tdx_errno.h
base-commit: e81f7c908e1664233974b9f20beead78cde6343a
--
2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply
* Re: [PATCH 1/2] KVM: SEV: IBPB-on-Entry guest support
From: Kim Phillips @ 2026-01-29 0:38 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel, kvm, linux-coco, x86, Sean Christopherson,
Paolo Bonzini, K Prateek Nayak, Nikunj A Dadhania, Tom Lendacky,
Michael Roth, Naveen Rao, David Kaplan, stable
In-Reply-To: <20260128192312.GQaXpiIL4YFmQB2LKL@fat_crate.local>
Hi Boris,
On 1/28/26 1:23 PM, Borislav Petkov wrote:
> On Mon, Jan 26, 2026 at 04:42:04PM -0600, Kim Phillips wrote:
>> 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.
>>
>> Unmask the bit, to allow guests to take advantage of the feature on
>> hypervisor kernel versions that support it: Amend the SEV_STATUS MSR
>> SNP_RESERVED_MASK to exclude bit 23 (IbpbOnEntry).
> Do not explain what the patch does.
For that last paragraph, how about:
"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")
>> Cc: Nikunj A Dadhania <nikunj@amd.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> CC: Borislav Petkov (AMD) <bp@alien8.de>
>> CC: Michael Roth <michael.roth@amd.com>
>> Cc: stable@kernel.org
> I guess...
Hopefully a bitfield will be carved out for these
no-explicit-guest-implementation-required bits by hardware such that we
won't need to do this again.
>> 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)
> Why isn't this part of SNP_FEATURES_PRESENT?
>
> If this feature doesn't require guest-side support, then it is trivially
> present, no?
SNP_FEATURES_PRESENT is for the non-trivial variety: Its bits get set as
part of the patchseries that add the explicit guest support *code*.
I believe 'features' like PREVENT_HOST_IBS are similar in this regard.
>> +#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
>> --
> I guess this is a fix of sorts and I could take it in now once all review
> comments have been addressed...
Cool, thanks.
Kim
^ permalink raw reply
* Re: [PATCH v3 09/26] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Dave Hansen @ 2026-01-29 0:08 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
In-Reply-To: <20260123145645.90444-10-chao.gao@intel.com>
On 1/23/26 06:55, Chao Gao wrote:
...
> diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> index 901abbae2e61..a3f155977016 100644
> --- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> +++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> @@ -4,3 +4,28 @@ Description: (RO) Report the version of the loaded TDX Module. The TDX Module
> version is formatted as x.y.z, where "x" is the major version,
> "y" is the minor version and "z" is the update version. Versions
> are used for bug reporting, TDX Module updates and etc.
> +
> +What: /sys/devices/faux/tdx_host/seamldr/version
> +Contact: linux-coco@lists.linux.dev
> +Description: (RO) Report the version of the loaded SEAM loader. The SEAM
> + loader version is formatted as x.y.z, where "x" is the major
> + version, "y" is the minor version and "z" is the update version.
> + Versions are used for bug reporting and compatibility check.
checks ^
> +What: /sys/devices/faux/tdx_host/seamldr/num_remaining_updates
> +Contact: linux-coco@lists.linux.dev
> +Description: (RO) Report the number of remaining updates that can be performed.
> + The CPU keeps track of TCB versions for each TDX Module that
> + has been loaded. Since this tracking database has finite
> + capacity, there's a maximum number of Module updates that can
> + be performed.
Is it really the CPU? Or some SEAM software construct?
> + After each successful update, the number reduces by one. Once it
> + reaches zero, further updates will fail until next reboot. The
> + number is always zero if P-SEAMLDR doesn't support updates.
> +
> + See Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
> + Interface Specification Chapter 3.3 "SEAMLDR_INFO" and Chapter
> + 4.2 "SEAMLDR.INSTALL" for more information. The documentation is
> + available at:
> + https://cdrdv2-public.intel.com/739045/intel-tdx-seamldr-interface-specification.pdf
Zap the URL. It's just going bit rot. Keep the document name. That's
googleable.
> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> index 0424933b2560..f4ce89522806 100644
> --- a/drivers/virt/coco/tdx-host/tdx-host.c
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> @@ -11,6 +11,7 @@
> #include <linux/sysfs.h>
>
> #include <asm/cpu_device_id.h>
> +#include <asm/seamldr.h>
> #include <asm/tdx.h>
>
> static const struct x86_cpu_id tdx_host_ids[] = {
> @@ -40,7 +41,64 @@ static struct attribute *tdx_host_attrs[] = {
> &dev_attr_version.attr,
> NULL,
> };
> -ATTRIBUTE_GROUPS(tdx_host);
> +
> +struct attribute_group tdx_host_group = {
> + .attrs = tdx_host_attrs,
> +};
> +
> +#ifdef CONFIG_INTEL_TDX_MODULE_UPDATE
> +static ssize_t seamldr_version_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + const struct seamldr_info *info = seamldr_get_info();
Uhh... seamldr_get_info() calls down into the SEAMLDR. It happily zaps
the VMCS and this is surely a slow thing. This also has 0444 permissions
which means *ANYONE* can call this. Constantly. As fast as they can make
a few syscalls.
Right?
Are there any concerns about making SEAMLDR calls? Are there any
system-wide performance implications? How long of an interrupt-blocking
blip is there for this?
Also, what's the locking around seamldr_get_info()? It writes into a
global, shared structure. I guess you disabled interrupts so it's
preempt safe at least. <sigh>
I guess it won't change *that* much. But, sheesh, it seems like an
awfully bad idea to have lots of CPUs writing into a common data
structure all at the same time.
^ permalink raw reply
* Re: [PATCH v3 08/26] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Dave Hansen @ 2026-01-28 23:57 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-9-chao.gao@intel.com>
On 1/23/26 06:55, Chao Gao wrote:
> +static struct seamldr_info seamldr_info __aligned(256);
I also wonder if this should be __read_mostly or even read-only after
boot. Is it ever modified?
^ permalink raw reply
* Re: [PATCH v3 08/26] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Dave Hansen @ 2026-01-28 23:54 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-9-chao.gao@intel.com>
On 1/23/26 06:55, Chao Gao wrote:
> P-SEAMLDR returns its information e.g., version and supported features, in
> response to the SEAMLDR.INFO SEAMCALL.
>
> This information is useful for userspace. For example, the admin can decide
> which TDX module versions are compatible with the P-SEAMLDR according to
> the P-SEAMLDR version.
>
> Add and export seamldr_get_info() which retrieves P-SEAMLDR information by
I don't need to know what the function name is. That's in the code.
> invoking SEAMLDR.INFO SEAMCALL in preparation for exposing P-SEAMLDR
> version and other necessary information to userspace.
I also want to know what spec you are getting this out of.
I think it's also worth calling out that there are SEAMLDR calls for both:
SEAMLDR_INFO
and
SEAMLDR_SEAMINFO
Which is astonishingly confusing. Please have mercy on folks that are
looking through the docs for the first time and explain this.
> diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
> new file mode 100644
> index 000000000000..d1e9f6e16e8d
> --- /dev/null
> +++ b/arch/x86/include/asm/seamldr.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_SEAMLDR_H
> +#define _ASM_X86_SEAMLDR_H
> +
> +#include <linux/types.h>
> +
> +struct seamldr_info {
/*
* This called the "SEAMLDR_INFO" data structure and is defined
* in "SEAM Loader (SEAMLDR) Interface Specification".
*/
> + u32 version;
> + u32 attributes;
> + u32 vendor_id;
> + u32 build_date;
> + u16 build_num;
> + u16 minor_version;
> + u16 major_version;
> + u16 update_version;
> + u8 reserved0[4];
Why not label this:
u32 acm_x2apicid: /* unused by kernel */
?
> + u32 num_remaining_updates;
> + u8 reserved1[224];
> +} __packed;
> +
> +#ifdef CONFIG_INTEL_TDX_MODULE_UPDATE
> +const struct seamldr_info *seamldr_get_info(void);
> +#else
> +static inline const struct seamldr_info *seamldr_get_info(void) { return NULL; }
> +#endif
> +
> +#endif
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index b99d73f7bb08..6a83ae405fac 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -9,9 +9,16 @@
> #include <linux/irqflags.h>
> #include <linux/types.h>
>
> +#include <asm/seamldr.h>
> +
> #include "seamcall.h"
>
> -static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> +/* P-SEAMLDR SEAMCALL leaf function */
> +#define P_SEAMLDR_INFO 0x8000000000000000
/*
* The SEAMLDR.INFO documentation requires
* this to be aligned to a 256-byte boundary.
*/
> +static struct seamldr_info seamldr_info __aligned(256);
> +
> +static inline int seamldr_call(u64 fn, struct tdx_module_args *args)
> {
> unsigned long flags;
> u64 vmcs;
> @@ -54,3 +61,11 @@ static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> WARN_ONCE(1, "Failed to save/restore the current VMCS");
> return -EIO;
> }
> +
> +const struct seamldr_info *seamldr_get_info(void)
> +{
> + struct tdx_module_args args = { .rcx = __pa(&seamldr_info) };
> +
> + return seamldr_call(P_SEAMLDR_INFO, &args) ? NULL : &seamldr_info;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
I'd also prefer a
BUILD_BUG_ON(sizeof(struct seamldr_info) != 2048);
just as a sanity check. It doesn't cost anything and it makes sure that
as you muck around with reserved fields and padding that there's at
least one check making sure it's OK.
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Dave Hansen @ 2026-01-28 23:36 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-8-chao.gao@intel.com>
On 1/23/26 06:55, Chao Gao wrote:
...
> +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> +{
> + unsigned long flags;
> + u64 vmcs;
> + int ret;
> +
> + if (!is_seamldr_call(fn))
> + return -EINVAL;
Why is this here? We shouldn't be silently papering over kernel bugs.
This is a WARN_ON() at *best*, but it also begs the question of how a
non-SEAMLDR call even got here.
> + /*
> + * SEAMRET from P-SEAMLDR invalidates the current VMCS. Save/restore
> + * the VMCS across P-SEAMLDR SEAMCALLs to avoid clobbering KVM state.
> + * Disable interrupts as KVM is allowed to do VMREAD/VMWRITE in IRQ
> + * context (but not NMI context).
> + */
I think you mean:
WARN_ON(in_nmi());
> + local_irq_save(flags);
> +
> + asm goto("1: vmptrst %0\n\t"
> + _ASM_EXTABLE(1b, %l[error])
> + : "=m" (vmcs) : : "cc" : error);
I'd much rather this be wrapped up in a helper function. We shouldn't
have to look at the horrors of inline assembly like this.
But this *REALLY* wants the KVM folks to look at it. One argument is
that with the inline assembly this is nice and self-contained. The other
argument is that this completely ignores all existing KVM infrastructure
and is parallel VMCS management.
I'd be shocked if this is the one and only place in the whole kernel
that can unceremoniously zap VMX state.
I'd *bet* that you don't really need to do the vmptrld and that KVM can
figure it out because it can vmptrld on demand anyway. Something along
the lines of:
local_irq_disable();
list_for_each(handwaving...)
vmcs_clear();
ret = seamldr_prerr(fn, args);
local_irq_enable();
Basically, zap this CPU's vmcs state and then make KVM reload it at some
later time.
I'm sure Sean and Paolo will tell me if I'm crazy.
> diff --git a/drivers/virt/coco/tdx-host/Kconfig b/drivers/virt/coco/tdx-host/Kconfig
> index e58bad148a35..6a9199e6c2c6 100644
> --- a/drivers/virt/coco/tdx-host/Kconfig
> +++ b/drivers/virt/coco/tdx-host/Kconfig
> @@ -8,3 +8,13 @@ config TDX_HOST_SERVICES
>
> Say y or m if enabling support for confidential virtual machine
> support (CONFIG_INTEL_TDX_HOST). The module is called tdx_host.ko
> +
> +config INTEL_TDX_MODULE_UPDATE
> + bool "Intel TDX module runtime update"
> + depends on TDX_HOST_SERVICES
> + help
> + This enables the kernel to support TDX module runtime update. This
> + allows the admin to update the TDX module to another compatible
> + version without the need to terminate running TDX guests.
... as opposed to the method that the kernel has to update the module
without terminating guests? ;)
> + If unsure, say N.
Let's call this:
config
INTEL_TDX_ONLY_DISABLE_THIS_IF_YOU_HATE_SECURITY_AND_IF_YOU_DO_WHY_ARE_YOU_RUNNING_TDX?
Can we have question marks in config symbol names? ;)
But, seriously, what the heck? Who would disable security updates for
their confidential computing infrastructure? Is this some kind of
intelligence test for our users so that if someone disables it we can
just laugh at them?
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Dave Hansen @ 2026-01-28 23:04 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-8-chao.gao@intel.com>
On 1/23/26 06:55, Chao Gao wrote:
> SEAMRET from the P-SEAMLDR clears the current VMCS structure pointed
> to by the current-VMCS pointer. A VMM that invokes the P-SEAMLDR
> using SEAMCALL must reload the current-VMCS, if required, using the
> VMPTRLD instruction.
That seems pretty mean.
This is going to need a lot more justification for why this is an
absolutely necessary requirement.
KVM folks, are you OK with this?
^ permalink raw reply
* Re: [PATCH v3 06/26] x86/virt/tdx: Prepare to support P-SEAMLDR SEAMCALLs
From: Dave Hansen @ 2026-01-28 23:03 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-7-chao.gao@intel.com>
On 1/23/26 06:55, Chao Gao wrote:
> P-SEAMLDR is another component alongside the TDX module within the
> protected SEAM range. P-SEAMLDR can update the TDX module at runtime.
> Software can talk with P-SEAMLDR via SEAMCALLs with the bit 63 of RAX
> (leaf number) set to 1 (a.k.a P-SEAMLDR SEAMCALLs).
This text kinda bugs me. It's OK, but needs improvement.
First, don't explain the ABI in the changelog. Nobody cares that it's
bit 63.
Background:
The TDX architecture uses the "SEAMCALL" instruction to
communicate with SEAM mode software. Right now, the only SEAM
mode software that the kernel communicates with is the TDX
module. But, there are actually some components that run in SEAM
mode but that are separate from the TDX module: that SEAM
loaders. Right now, the only component that communicates with
them is the BIOS which loads the TDX module itself at boot. But,
to support updating the TDX module, the kernel now needs to be
able to talk to one of the the SEAM loaders: the Persistent
loader or "P-SEAMLDR".
Then do this part:
> P-SEAMLDR SEAMCALLs differ from SEAMCALLs of the TDX module in terms of
> error codes and the handling of the current VMCS.
Except I don't even know how the TDX module handles the current VMCS.
That probably needs to be in there. Or, it should be brought up in the
patch itself that implements this. Or, uplifted to the cover letter.
> In preparation for adding support for P-SEAMLDR SEAMCALLs, do the two
> following changes to SEAMCALL low-level helpers:
>
> 1) Tweak sc_retry() to retry on "lack of entropy" errors reported by
> P-SEAMLDR because it uses a different error code.
>
> 2) Add seamldr_err() to log error messages on P-SEAMLDR SEAMCALL failures.
> diff --git a/arch/x86/virt/vmx/tdx/seamcall.h b/arch/x86/virt/vmx/tdx/seamcall.h
> index 0912e03fabfe..256f71d6ca70 100644
> --- a/arch/x86/virt/vmx/tdx/seamcall.h
> +++ b/arch/x86/virt/vmx/tdx/seamcall.h
> @@ -34,15 +34,28 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> return func(fn, args);
> }
>
> +#define SEAMLDR_RND_NO_ENTROPY 0x8000000000030001ULL
<sigh>
#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
So they're not even close values. They're not consistent or even a bit
off or anything.
Honestly, this needs a justification for why this was done this way. Why
can't "SEAM mode" be a monolithic thing from the kernel's perspective?
> +#define SEAMLDR_SEAMCALL_MASK _BITUL(63)
> +
> +static inline bool is_seamldr_call(u64 fn)
> +{
> + return fn & SEAMLDR_SEAMCALL_MASK;
> +}
> +
> static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
> struct tdx_module_args *args)
> {
> + u64 retry_code = TDX_RND_NO_ENTROPY;
> int retry = RDRAND_RETRY_LOOPS;
> u64 ret;
>
> + if (unlikely(is_seamldr_call(fn)))
> + retry_code = SEAMLDR_RND_NO_ENTROPY;
(un)likey() has two uses:
1. It's in performance critical code and compilers have been
demonstrated to be generating bad code.
2. It's in code where it's not obvious what the fast path is
and (un)likey() makes the code more readable.
Which one is this?
Second, this is nitpicky, but I'd rather this be:
if (is_seamldr_call(fn))
retry_code = SEAMLDR_RND_NO_ENTROPY;
else
retry_code = TDX_RND_NO_ENTROPY;
or even:
retry_code = TDX_RND_NO_ENTROPY;
if (is_seamldr_call(fn))
retry_code = SEAMLDR_RND_NO_ENTROPY;
That makes it trivial that 'retry_code' can only have two values. It's
nitpicky because the original initialization is so close.
> do {
> ret = func(fn, args);
> - } while (ret == TDX_RND_NO_ENTROPY && --retry);
> + } while (ret == retry_code && --retry);
>
> return ret;
> }
> @@ -68,6 +81,16 @@ static inline void seamcall_err_ret(u64 fn, u64 err,
> args->r9, args->r10, args->r11);
> }
>
> +static inline void seamldr_err(u64 fn, u64 err, struct tdx_module_args *args)
> +{
> + /*
> + * Note: P-SEAMLDR leaf numbers are printed in hex as they have
> + * bit 63 set, making them hard to read and understand if printed
> + * in decimal
> + */
> + pr_err("P-SEAMLDR (%llx) failed: %#016llx\n", fn, err);
> +}
Oh, lovely.
Didn't you just propose changing the module SEAMCALL leaf numbers in
decimal? Isn't it a little crazy to do one in decimal and the other in hex?
I'd really rather just see the TDX documentation changed.
But, honestly, I'd probably just leave the thing in hex, drop this hunk,
and go thwack someone that writes TDX module documentation instead.
> static __always_inline int sc_retry_prerr(sc_func_t func,
> sc_err_func_t err_func,
> u64 fn, struct tdx_module_args *args)
> @@ -96,4 +119,7 @@ static __always_inline int sc_retry_prerr(sc_func_t func,
> #define seamcall_prerr_ret(__fn, __args) \
> sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
>
> +#define seamldr_prerr(__fn, __args) \
> + sc_retry_prerr(__seamcall, seamldr_err, (__fn), (__args))
> +
> #endif
So, honestly, for me, it's a NAK for this whole patch.
Go change the P-SEAMLDR to use the same error code as the TDX module,
and fix the documentation. No kernel changes, please.
^ permalink raw reply
* Re: [PATCH v3 24/26] x86/virt/seamldr: Extend sigstruct to 16KB
From: Huang, Kai @ 2026-01-28 23:01 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, kas@kernel.org, mingo@redhat.com,
seanjc@google.com, Weiny, Ira, Chatre, Reinette,
tglx@linutronix.de, nik.borisov@suse.com, Verma, Vishal L,
hpa@zytor.com, sagis@google.com, Annapurve, Vishal,
Duan, Zhenzhong, Edgecombe, Rick P, paulmck@kernel.org,
bp@alien8.de, yilun.xu@linux.intel.com, Williams, Dan J
In-Reply-To: <20260123145645.90444-25-chao.gao@intel.com>
On Fri, 2026-01-23 at 06:55 -0800, Chao Gao wrote:
> Currently, each TDX Module has a 4KB sigstruct that is passed to the
> P-SEAMLDR during module updates to authenticate the TDX Module binary.
>
> Future TDX Module versions will pack additional information into the
> sigstruct, which will exceed the current 4KB size limit.
>
> To accommodate this, the sigstruct is being extended to support up to
> 16KB. Update seamldr_params and tdx-blob structures to handle the larger
> sigstruct size.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> arch/x86/virt/vmx/tdx/seamldr.c | 28 +++++++++++++++++++---------
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index d2d85114d6c4..9e77b24f659c 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -29,6 +29,8 @@
> /* P-SEAMLDR can accept up to 496 4KB pages for TDX module binary */
> #define SEAMLDR_MAX_NR_MODULE_4KB_PAGES 496
>
> +#define SEAMLDR_MAX_NR_SIG_4KB_PAGES 4
> +
> /* scenario field in struct seamldr_params */
> #define SEAMLDR_SCENARIO_UPDATE 1
>
> @@ -40,8 +42,8 @@
> struct seamldr_params {
> u32 version;
> u32 scenario;
> - u64 sigstruct_pa;
> - u8 reserved[104];
> + u64 sigstruct_pa[SEAMLDR_MAX_NR_SIG_4KB_PAGES];
> + u8 reserved[80];
> u64 num_module_pages;
> u64 mod_pages_pa_list[SEAMLDR_MAX_NR_MODULE_4KB_PAGES];
> } __packed;
> @@ -121,7 +123,10 @@ static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned
> if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> return ERR_PTR(-EINVAL);
>
> - if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
> + if (sig_size > SEAMLDR_MAX_NR_SIG_4KB_PAGES * SZ_4K)
> + return ERR_PTR(-EINVAL);
> +
> + if (!IS_ALIGNED(module_size, SZ_4K) || !IS_ALIGNED(sig_size, SZ_4K) ||
> !IS_ALIGNED((unsigned long)module, SZ_4K) ||
> !IS_ALIGNED((unsigned long)sig, SZ_4K))
> return ERR_PTR(-EINVAL);
> @@ -132,12 +137,17 @@ static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned
>
> params->scenario = SEAMLDR_SCENARIO_UPDATE;
>
> - /*
> - * Don't assume @sig is page-aligned although it is 4KB-aligned.
> - * Always add the in-page offset to get the physical address.
> - */
> - params->sigstruct_pa = (vmalloc_to_pfn(sig) << PAGE_SHIFT) +
> - ((unsigned long)sig & ~PAGE_MASK);
> + ptr = sig;
> + for (i = 0; i < sig_size / SZ_4K; i++) {
> + /*
> + * Don't assume @sig is page-aligned although it is 4KB-aligned.
> + * Always add the in-page offset to get the physical address.
> + */
> + params->sigstruct_pa[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> + ((unsigned long)ptr & ~PAGE_MASK);
> + ptr += SZ_4K;
> + }
> +
> params->num_module_pages = module_size / SZ_4K;
>
> ptr = module;
Let's move the discussion here (from patch 13 -- sorry about that):
IIRC this patch just simply re-purposes couple of reserved space in
SEAMLDR_PARAMS (which is part of P-SEAMLDR ABI) w/o enumeration, explicit
opt-in whatever. The code change here doesn't even bump up its version.
IIUC, if this code run on an old platform where SEAMLDR.INSTALL still only
works with 4K SIGSTRUCT, the SEAMLDR.INSTALL will only see part of the
SIGSTRUCT thus will likely fail.
How can we know whether a given 'struct tdx_blob' can work on an platform or
not? Or am I missing anything?
^ permalink raw reply
* Re: [PATCH v3 13/26] x86/virt/seamldr: Allocate and populate a module update request
From: Huang, Kai @ 2026-01-28 22:33 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, bp@alien8.de,
dave.hansen@linux.intel.com, kas@kernel.org, mingo@redhat.com,
Weiny, Ira, linux-kernel@vger.kernel.org, seanjc@google.com,
Verma, Vishal L, nik.borisov@suse.com, Chatre, Reinette,
hpa@zytor.com, sagis@google.com, Chen, Farrah, Duan, Zhenzhong,
tglx@linutronix.de, Edgecombe, Rick P, paulmck@kernel.org,
Annapurve, Vishal, yilun.xu@linux.intel.com, x86@kernel.org,
Williams, Dan J
In-Reply-To: <aXny+UkkEzU425k6@intel.com>
On Wed, 2026-01-28 at 19:28 +0800, Gao, Chao wrote:
> On Tue, Jan 27, 2026 at 11:21:06AM +0800, Huang, Kai wrote:
> >
> > > +/*
> > > + * Allocate and populate a seamldr_params.
> > > + * Note that both @module and @sig should be vmalloc'd memory.
> > > + */
> > > +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
> > > + const void *sig, unsigned int sig_size)
> > > +{
> > > + struct seamldr_params *params;
> > > + const u8 *ptr;
> > > + int i;
> > > +
> > > + BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
> > > + if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> > > + return ERR_PTR(-EINVAL);
> > > +
> > > + if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
> > > + !IS_ALIGNED((unsigned long)module, SZ_4K) ||
> > > + !IS_ALIGNED((unsigned long)sig, SZ_4K))
> > > + return ERR_PTR(-EINVAL);
> > > +
> >
> > Based on the the blob format link below, we have
> >
> > struct tdx_blob
> > {
> > ...
> > _u64 sigstruct[256]; // 2KB sigstruct,intel_tdx_module.so.sigstruct
> > _u64 reserved2[256]; // Reserved space
> > ...
> > }
> >
> > So it's clear SIGSTRUCT is just 2KB and the second half 2KB is "reserved
> > space".
> >
> > Why is the "reserved space" treated as part of SIGSTRUCT here?
>
> Good question. Because the space is reserved for sigstruct expansion.
>
> The __current__ SEAMLDR ABI accepts one 4KB page, but all __existing__
> sigstructs are only 2KB.
>
Oh I see.
I think we have two perspectives here: 1) what P-SEAMLDR ABI requires for
module and sigstruct; 2) how does the kernel get them and pass to
alloc_seamldr_params().
IIUC, I now understand alloc_seamldr_params() is expecting the 'module',
'module_size', 'sig' and 'sig_size' to meet P-SEAMCALL's ABI.
Then would it be better to add a comment for the checks of 'module',
'module_size', 'sig' and 'sig_size' in alloc_seamldr_params() (below code)
that it is P-SEAMCALL ABI that has these requirement?
if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
!IS_ALIGNED((unsigned long)module, SZ_4K) ||
!IS_ALIGNED((unsigned long)sig, SZ_4K))
return ERR_PTR(-EINVAL);
Otherwise it's a bit confusing because these 4 arguments are passed to
alloc_seamldr_params() right from the layout of 'struct tdx_blob' which is a
"software-organized" structure which, theoretically, could have nothing to
do P-SEAMLDR ABI.
> so, tdx_blob currently defines a 2KB sigstruct field
> followed by 2KB of reserved space. We anticipate that sigstructs will
> eventually exceed 4KB, so we added reserved3[N*512] to accommodate future
> growth.
>
> You're right. The current tdx_blob definition doesn't clearly indicate that
> reserved2/3 are actually part of the sigstruct.
>
> Does this revised tdx_blob definition make that clearer and better align with
> this patch?
>
Yes it's clearer, from the perspective that how it matches your code to
calculate 'sig_size'.
> The idea is to make tdx_blob generic enough to clearly represent:
> a 4KB header, followed by 4KB-aligned sigstruct, followed by the TDX Module
> binary. Current SEAMLDR ABI details or current sigstruct sizes are irrelevant.
>
> struct tdx_blob
> {
> _u16 version; // Version number
> _u16 checksum; // Checksum of the entire blob should be zero
> _u32 offset_of_module; // Offset of the module binary intel_tdx_module.bin in bytes
> _u8 signature[8]; // Must be "TDX-BLOB"
> _u32 length; // The length in bytes of the entire blob
> _u32 reserved0; // Reserved space
> _u64 reserved1[509]; // Reserved space
> _u64 sigstruct[512 + N*512]; // sigstruct, 4KB aligned
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> _u8 module[]; // intel_tdx_module.bin, 4KB aligned, to the end of the file
> }
>
>
> >
A side topic:
I checked the SEAMLDR.INSTALL. It appears the only requirement of the
SIGSTRUCT is it is 4K aligned. There's no where in the ABI (certainly not
in SEAMLDR_PARAMS) to tell how does SEAMLDR.INSTALL verifies the size of
SIGSTRUCT.
Is this right?
When we bumping SIGSTRUCT to a larger size, do we have some kinda
enumeration that reports such?
From your patch 24, IIUC I don't see such enumeration or explicit opt-in,
because you just changes the layout of SEAMLDR_PARAM w/o even changing it's
version.
[...]
> > But I think if we add 'sigstruct' to the 'struct tdx_blob', e.g.,
> >
> > struct tdx_blob {
> > u16 version;
> > ...
> > u64 rsvd2[509];
> > u64 sigstruct[256];
> > u64 rsvd3[256];
> > u64 data;
> > } __packed;
> >
> > .. we can just use
> >
> > sig = blob->sigstruct;
> > sig_size = 2K (or 4K I don't quite follow);
> >
> > which is clearer to read IMHO?
>
> The problem is hard-coding the sigstruct size to 2KB/4KB. This will soon no
> longer hold.
>
> But
> sig = blob->data;
> sig_size = blob->offset_of_module - sizeof(struct tdx_blob);
>
> doesn't make that assumption, making it more future-proof.
Sure. I am certainly fine with making it future-proof (albeit arguably you
could also change the way that how sig_size is calculated in the future,
i.e., in your patch 24).
But the real point is the code here needs to reflect the 'struct tdx_blob'
description in the doc. But with the current doc I don't see they match to
each other:
The doc says SIGSTRUCT is 2K but the code says it's 4K.
So I think you need to update the 'struct tdx_blob' description in the doc
to justify such code.
Btw, I think the link
https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
is subject to change, both the link itself and it's content.
Do you think we should just make the layout of 'struct tdx_blob' as a
documentation patch and include that to this series?
^ permalink raw reply
* Re: [PATCH v3 2/2] x86/sev: Add support to unaccept memory after hot-remove
From: Pratik R. Sampat @ 2026-01-28 22:25 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-coco, x86, linux-kernel, tglx, mingo, bp,
dave.hansen, kas, ardb, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <20260128130800.40adb423627ff6d8ffce7ccd@linux-foundation.org>
Hi Dave, Andrew
On 1/28/26 3:08 PM, Andrew Morton wrote:
> On Wed, 28 Jan 2026 14:41:05 -0600 "Pratik R. Sampat" <prsampat@amd.com> wrote:
>
>> Transition memory to the shared state during a hot-remove operation so
>> that it can be re-used by the hypervisor. This also applies when memory
>> is intended to be hotplugged back in later, as those pages will need to
>> be re-accepted after crossing the trust boundary.
>>
>> ...
>>
>> @@ -623,6 +624,7 @@ static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
>> return -ENOTTY;
>> }
>> static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
>> +static inline void snp_unaccept_memory(phys_addr_t start, phys_addr_t end) { }
>> static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
>> static inline u64 sev_get_status(void) { return 0; }
>> static inline void sev_show_status(void) { }
>> diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
>> index f5937e9866ac..8715be843e65 100644
>> --- a/arch/x86/include/asm/unaccepted_memory.h
>> +++ b/arch/x86/include/asm/unaccepted_memory.h
>> @@ -18,6 +18,15 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
>> }
>> }
>>
>> +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
>> +{
>> + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
>> + snp_unaccept_memory(start, end);
>> + } else {
>> + panic("Cannot unaccept memory: unknown platform\n");
>
> Seems severe. Dropping a WARN() and continuing would be preferred.
>
> What exactly happened here? Am I correct in thinking that the check in
> snp_unaccept_memory() makes this a cant-happen?
>
You're right, a WARN() is probably more appropriate here.
Based on my rudimentary understanding of TDX from Kiryl, TDX module is
what maintains this metadata for the HPA.
So, maybe the WARN could just be:
"Cannot unaccept memory: VMM responsible for unaccepting memory" for
TDX? Or, we could let it fall-through for TDX (if and when there is
support for that)
>> --- a/drivers/firmware/efi/unaccepted_memory.c
>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>> @@ -157,6 +157,52 @@ void accept_memory(phys_addr_t start, unsigned long size)
>> spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>> }
>>
>> +void unaccept_memory(phys_addr_t start, unsigned long size)
>> +{
>> + unsigned long range_start, range_end, bitrange_end;
>> + struct efi_unaccepted_memory *unaccepted;
>> + phys_addr_t end = start + size;
>> + u64 unit_size, phys_base;
>> + unsigned long flags;
>> +
>> + unaccepted = efi_get_unaccepted_table();
>> + if (!unaccepted)
>> + return;
>> +
>> + phys_base = unaccepted->phys_base;
>> + unit_size = unaccepted->unit_size;
>> +
>> + if (start < unaccepted->phys_base)
>> + start = unaccepted->phys_base;
>
> max()?
>
>> + if (end < unaccepted->phys_base)
>> + return;
>> +
>> + start -= phys_base;
>> + end -= phys_base;
>> +
>> + /* Make sure not to overrun the bitmap */
>> + if (end > unaccepted->size * unit_size * BITS_PER_BYTE)
>> + end = unaccepted->size * unit_size * BITS_PER_BYTE;
>
> min()?
>
> If you like min() and max(). Sometimes I find them annoying - need to mentally
> expand them to figure out what's going on.
Same! :-)
That is why I just aped the implementation from its counterpart
accept_memory() but can definitely do it this way and shave off a couple
of lines.
Thanks,
--Pratik
>
>> + range_start = start / unit_size;
>> + bitrange_end = DIV_ROUND_UP(end, unit_size);
>> +
>> + /* Only unaccept memory that was previously accepted in the range */
>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>> + for_each_clear_bitrange_from(range_start, range_end, unaccepted->bitmap,
>> + bitrange_end) {
>> + unsigned long phys_start, phys_end;
>> + unsigned long len = range_end - range_start;
>> +
>> + phys_start = range_start * unit_size + phys_base;
>> + phys_end = range_end * unit_size + phys_base;
>> +
>> + arch_unaccept_memory(phys_start, phys_end);
>> + bitmap_set(unaccepted->bitmap, range_start, len);
>> + }
>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>> +}
>
^ permalink raw reply
* Re: [PATCH v3 2/2] x86/sev: Add support to unaccept memory after hot-remove
From: Dave Hansen @ 2026-01-28 21:15 UTC (permalink / raw)
To: Pratik R. Sampat, linux-mm, linux-coco, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, david, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <20260128204105.508855-3-prsampat@amd.com>
On 1/28/26 12:41, Pratik R. Sampat wrote:
> +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
> +{
> + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
> + snp_unaccept_memory(start, end);
> + } else {
> + panic("Cannot unaccept memory: unknown platform\n");
> + }
> +}
This panic() is pretty nasty.
Can't we just disable memory hotplug up front if it's:
!cc_platform_has(CC_ATTR_GUEST_SEV_SNP)
?
^ permalink raw reply
* Re: [PATCH v3 2/2] x86/sev: Add support to unaccept memory after hot-remove
From: Andrew Morton @ 2026-01-28 21:08 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, x86, linux-kernel, tglx, mingo, bp,
dave.hansen, kas, ardb, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <20260128204105.508855-3-prsampat@amd.com>
On Wed, 28 Jan 2026 14:41:05 -0600 "Pratik R. Sampat" <prsampat@amd.com> wrote:
> Transition memory to the shared state during a hot-remove operation so
> that it can be re-used by the hypervisor. This also applies when memory
> is intended to be hotplugged back in later, as those pages will need to
> be re-accepted after crossing the trust boundary.
>
> ...
>
> @@ -623,6 +624,7 @@ static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
> return -ENOTTY;
> }
> static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
> +static inline void snp_unaccept_memory(phys_addr_t start, phys_addr_t end) { }
> static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
> static inline u64 sev_get_status(void) { return 0; }
> static inline void sev_show_status(void) { }
> diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
> index f5937e9866ac..8715be843e65 100644
> --- a/arch/x86/include/asm/unaccepted_memory.h
> +++ b/arch/x86/include/asm/unaccepted_memory.h
> @@ -18,6 +18,15 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
> }
> }
>
> +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
> +{
> + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
> + snp_unaccept_memory(start, end);
> + } else {
> + panic("Cannot unaccept memory: unknown platform\n");
Seems severe. Dropping a WARN() and continuing would be preferred.
What exactly happened here? Am I correct in thinking that the check in
snp_unaccept_memory() makes this a cant-happen?
> --- a/drivers/firmware/efi/unaccepted_memory.c
> +++ b/drivers/firmware/efi/unaccepted_memory.c
> @@ -157,6 +157,52 @@ void accept_memory(phys_addr_t start, unsigned long size)
> spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> }
>
> +void unaccept_memory(phys_addr_t start, unsigned long size)
> +{
> + unsigned long range_start, range_end, bitrange_end;
> + struct efi_unaccepted_memory *unaccepted;
> + phys_addr_t end = start + size;
> + u64 unit_size, phys_base;
> + unsigned long flags;
> +
> + unaccepted = efi_get_unaccepted_table();
> + if (!unaccepted)
> + return;
> +
> + phys_base = unaccepted->phys_base;
> + unit_size = unaccepted->unit_size;
> +
> + if (start < unaccepted->phys_base)
> + start = unaccepted->phys_base;
max()?
> + if (end < unaccepted->phys_base)
> + return;
> +
> + start -= phys_base;
> + end -= phys_base;
> +
> + /* Make sure not to overrun the bitmap */
> + if (end > unaccepted->size * unit_size * BITS_PER_BYTE)
> + end = unaccepted->size * unit_size * BITS_PER_BYTE;
min()?
If you like min() and max(). Sometimes I find them annoying - need to mentally
expand them to figure out what's going on.
> + range_start = start / unit_size;
> + bitrange_end = DIV_ROUND_UP(end, unit_size);
> +
> + /* Only unaccept memory that was previously accepted in the range */
> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
> + for_each_clear_bitrange_from(range_start, range_end, unaccepted->bitmap,
> + bitrange_end) {
> + unsigned long phys_start, phys_end;
> + unsigned long len = range_end - range_start;
> +
> + phys_start = range_start * unit_size + phys_base;
> + phys_end = range_end * unit_size + phys_base;
> +
> + arch_unaccept_memory(phys_start, phys_end);
> + bitmap_set(unaccepted->bitmap, range_start, len);
> + }
> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> +}
^ permalink raw reply
* [PATCH v3 2/2] x86/sev: Add support to unaccept memory after hot-remove
From: Pratik R. Sampat @ 2026-01-28 20:41 UTC (permalink / raw)
To: linux-mm, linux-coco, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, david, osalvador,
thomas.lendacky, michael.roth, prsampat
In-Reply-To: <20260128204105.508855-1-prsampat@amd.com>
Transition memory to the shared state during a hot-remove operation so
that it can be re-used by the hypervisor. This also applies when memory
is intended to be hotplugged back in later, as those pages will need to
be re-accepted after crossing the trust boundary.
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
arch/x86/coco/sev/core.c | 13 ++++++
arch/x86/include/asm/sev.h | 2 +
arch/x86/include/asm/unaccepted_memory.h | 9 ++++
drivers/firmware/efi/unaccepted_memory.c | 56 ++++++++++++++++++++++++
include/linux/mm.h | 9 ++++
mm/memory_hotplug.c | 2 +
6 files changed, 91 insertions(+)
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 9ae3b11754e6..63d8f44b76eb 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -703,6 +703,19 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
}
+void snp_unaccept_memory(phys_addr_t start, phys_addr_t end)
+{
+ unsigned long vaddr, npages;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return;
+
+ vaddr = (unsigned long)__va(start);
+ npages = (end - start) >> PAGE_SHIFT;
+
+ set_pages_state(vaddr, npages, SNP_PAGE_STATE_SHARED);
+}
+
static int vmgexit_ap_control(u64 event, struct sev_es_save_area *vmsa, u32 apic_id)
{
bool create = event != SVM_VMGEXIT_AP_DESTROY;
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 0e6c0940100f..3327de663793 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -514,6 +514,7 @@ bool snp_init(struct boot_params *bp);
void snp_dmi_setup(void);
int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call, struct svsm_attest_call *input);
void snp_accept_memory(phys_addr_t start, phys_addr_t end);
+void snp_unaccept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
@@ -623,6 +624,7 @@ static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
return -ENOTTY;
}
static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
+static inline void snp_unaccept_memory(phys_addr_t start, phys_addr_t end) { }
static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
static inline u64 sev_get_status(void) { return 0; }
static inline void sev_show_status(void) { }
diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
index f5937e9866ac..8715be843e65 100644
--- a/arch/x86/include/asm/unaccepted_memory.h
+++ b/arch/x86/include/asm/unaccepted_memory.h
@@ -18,6 +18,15 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
}
}
+static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
+{
+ if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
+ snp_unaccept_memory(start, end);
+ } else {
+ panic("Cannot unaccept memory: unknown platform\n");
+ }
+}
+
static inline struct efi_unaccepted_memory *efi_get_unaccepted_table(void)
{
if (efi.unaccepted == EFI_INVALID_TABLE_ADDR)
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index 5a4c8b0f56c8..9f1d594dba33 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -157,6 +157,52 @@ void accept_memory(phys_addr_t start, unsigned long size)
spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
}
+void unaccept_memory(phys_addr_t start, unsigned long size)
+{
+ unsigned long range_start, range_end, bitrange_end;
+ struct efi_unaccepted_memory *unaccepted;
+ phys_addr_t end = start + size;
+ u64 unit_size, phys_base;
+ unsigned long flags;
+
+ unaccepted = efi_get_unaccepted_table();
+ if (!unaccepted)
+ return;
+
+ phys_base = unaccepted->phys_base;
+ unit_size = unaccepted->unit_size;
+
+ if (start < unaccepted->phys_base)
+ start = unaccepted->phys_base;
+ if (end < unaccepted->phys_base)
+ return;
+
+ start -= phys_base;
+ end -= phys_base;
+
+ /* Make sure not to overrun the bitmap */
+ if (end > unaccepted->size * unit_size * BITS_PER_BYTE)
+ end = unaccepted->size * unit_size * BITS_PER_BYTE;
+
+ range_start = start / unit_size;
+ bitrange_end = DIV_ROUND_UP(end, unit_size);
+
+ /* Only unaccept memory that was previously accepted in the range */
+ spin_lock_irqsave(&unaccepted_memory_lock, flags);
+ for_each_clear_bitrange_from(range_start, range_end, unaccepted->bitmap,
+ bitrange_end) {
+ unsigned long phys_start, phys_end;
+ unsigned long len = range_end - range_start;
+
+ phys_start = range_start * unit_size + phys_base;
+ phys_end = range_end * unit_size + phys_base;
+
+ arch_unaccept_memory(phys_start, phys_end);
+ bitmap_set(unaccepted->bitmap, range_start, len);
+ }
+ spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
+}
+
bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
{
struct efi_unaccepted_memory *unaccepted;
@@ -227,6 +273,16 @@ void accept_hotplug_memory(phys_addr_t start, unsigned long size)
arch_accept_memory(start, start + size);
}
+void unaccept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+ if (range_contains_unaccepted_memory(start, size)) {
+ unaccept_memory(start, size);
+ return;
+ }
+
+ arch_unaccept_memory(start, start + size);
+}
+
#ifdef CONFIG_PROC_VMCORE
static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
unsigned long pfn)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2d3c1ea40606..faefaa9b92c6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4504,7 +4504,9 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
void accept_memory(phys_addr_t start, unsigned long size);
+void unaccept_memory(phys_addr_t start, unsigned long size);
void accept_hotplug_memory(phys_addr_t start, unsigned long size);
+void unaccept_hotplug_memory(phys_addr_t start, unsigned long size);
#else
@@ -4518,10 +4520,17 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
{
}
+static inline void unaccept_memory(phys_addr_t start, unsigned long size)
+{
+}
+
static inline void accept_hotplug_memory(phys_addr_t start, unsigned long size)
{
}
+static inline void unaccept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+}
#endif
static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 549ccfd190ee..21b87f2af930 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -2240,6 +2240,8 @@ static int try_remove_memory(u64 start, u64 size)
mem_hotplug_begin();
+ unaccept_hotplug_memory(start, size);
+
rc = memory_blocks_have_altmaps(start, size);
if (rc < 0) {
mem_hotplug_done();
--
2.52.0
^ permalink raw reply related
* [PATCH v3 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Pratik R. Sampat @ 2026-01-28 20:41 UTC (permalink / raw)
To: linux-mm, linux-coco, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, david, osalvador,
thomas.lendacky, michael.roth, prsampat
In-Reply-To: <20260128204105.508855-1-prsampat@amd.com>
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 | 18 ++++++++++++++++++
include/linux/mm.h | 5 +++++
mm/memory_hotplug.c | 2 ++
3 files changed, 25 insertions(+)
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index c2c067eff634..5a4c8b0f56c8 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -209,6 +209,24 @@ 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, handle acceptance in accordance with the
+ * unaccepted bitmap. Otherwise, perform the state change for the memory range
+ * up-front.
+ */
+void accept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+ if (range_contains_unaccepted_memory(start, size)) {
+ accept_memory(start, size);
+ return;
+ }
+
+ arch_accept_memory(start, start + size);
+}
+
#ifdef CONFIG_PROC_VMCORE
static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
unsigned long pfn)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 15076261d0c2..2d3c1ea40606 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4504,6 +4504,7 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
void accept_memory(phys_addr_t start, unsigned long size);
+void accept_hotplug_memory(phys_addr_t start, unsigned long size);
#else
@@ -4517,6 +4518,10 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
{
}
+static inline void accept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+}
+
#endif
static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index a63ec679d861..549ccfd190ee 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1567,6 +1567,8 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
if (!strcmp(res->name, "System RAM"))
firmware_map_add_hotplug(start, start + size, "System RAM");
+ accept_hotplug_memory(start, size);
+
/* device_online() will take the lock when calling online_pages() */
mem_hotplug_done();
--
2.52.0
^ permalink raw reply related
* [PATCH v3 0/2] SEV-SNP Unaccepted Memory Hotplug
From: Pratik R. Sampat @ 2026-01-28 20:41 UTC (permalink / raw)
To: linux-mm, linux-coco, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, david, osalvador,
thomas.lendacky, michael.roth, prsampat
Guest memory hot-plug/remove via the QEMU monitor is used by virtual
machines to dynamically scale the memory capacity of a system with
virtually zero downtime to the guest. For confidential VMs, memory has
to be first accepted before it can be used. Add support to accept
memory that has been hot-added and revert back it's state for
hypervisors to be able to use the pages during hot-remove.
Usage (for SNP guests)
----------------------
Step1: Spawn a QEMU SNP guest with the additional parameter of slots and
maximum possible memory, along with the initial memory as below:
"-m X,slots=Y,maxmem=Z".
Step2: Once the guest is booted, launch the qemu monitor and hotplug
the memory as follows:
(qemu) object_add memory-backend-memfd,id=mem1,size=1G
(qemu) device_add pc-dimm,id=dimm1,memdev=mem1
Memory is accepted up-front when added to the guest.
If using auto-onlining by either:
a) echo online > /sys/devices/system/memory/auto_online_blocks, OR
b) enable CONFIG_MHP_DEFAULT_ONLINE_TYPE_* while compiling kernel
Memory should show up automatically.
Otherwise, memory can also be onlined by echoing 1 to the newly added
blocks in: /sys/devices/system/memory/memoryXX/online
Step3: memory can be hot-removed using the qemu monitor using:
(qemu) device_remove dimm1
(qemu) object_remove mem1
Tip: Enable the kvm_convert_memory event in QEMU to observe memory
conversions between private and shared during hotplug/remove.
The series is based on
git.kernel.org/pub/scm/virt/kvm/kvm.git next
Comments and feedback appreciated!
Changelog Patch v2..v3:
-----------------------
https://lore.kernel.org/all/20260112202300.43546-1-prsampat@amd.com/
1. Account for cold-plugged memory at boot and introduce proper handling
of the unaccepted bitmap during both hotplug and remove. (Kiryl)
2. #include<asm/unaccepted_memory.h> within memory_hotplug caused build
failures on non-x86 archs (Andrew). Instead of introducing
#if-deffery to have arch agnostic fall throughs, create hotplug
specific helper functions since we now also need to take care of
managing the bitmaps due to 1. as well.
Changelog RFC..Patch v2:
------------------------
https://lore.kernel.org/all/20251125175753.1428857-1-prsampat@amd.com/
Based on feedback from the RFC, reworked the series to accept memory
upfront on hotplug. This is done for two reasons:
1. Avoids modifying the unaccepted bitmap. Extending the bitmap would
require either:
* Dynamically allocating the bitmap, which would need changes to EFI
struct definitions, or
* Pre-allocating a larger bitmap to accommodate hotpluggable memory.
This poses challenges since e820 is parsed before SRAT, which
contains the actual memory ranges information.
2. There are currently no known use-cases that would benefit from lazy
acceptance of hotplugged ranges which warrants this additional
complexity.
Pratik R. Sampat (2):
mm/memory_hotplug: Add support to accept memory during hot-add
x86/sev: Add support to unaccept memory after hot-remove
arch/x86/coco/sev/core.c | 13 +++++
arch/x86/include/asm/sev.h | 2 +
arch/x86/include/asm/unaccepted_memory.h | 9 +++
drivers/firmware/efi/unaccepted_memory.c | 74 ++++++++++++++++++++++++
include/linux/mm.h | 14 +++++
mm/memory_hotplug.c | 4 ++
6 files changed, 116 insertions(+)
--
2.52.0
^ 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