* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Sean Christopherson @ 2026-02-03 20:12 UTC (permalink / raw)
To: Kai Huang
Cc: x86@kernel.org, dave.hansen@linux.intel.com, kas@kernel.org,
bp@alien8.de, mingo@redhat.com, pbonzini@redhat.com,
tglx@kernel.org, Rick P Edgecombe, ackerleytng@google.com,
sagis@google.com, Vishal Annapurve, linux-kernel@vger.kernel.org,
Yan Y Zhao, Xiaoyao Li, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Isaku Yamahata,
binbin.wu@linux.intel.com
In-Reply-To: <de05853257e9cc66998101943f78a4b7e6e3d741.camel@intel.com>
On Tue, Feb 03, 2026, Kai Huang wrote:
> On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> > so that x86's TDX can update per-page metadata on allocation and free().
> >
> > Name the allocator page_get() to align with __get_free_page(), e.g. to
> > communicate that it returns an "unsigned long", not a "struct page", and
> > to avoid collisions with macros, e.g. with alloc_page.
> >
> > Suggested-by: Kai Huang <kai.huang@intel.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
>
> I thought it could be more generic for allocating an object, but not just a
> page.
>
> E.g., I thought we might be able to use it to allocate a structure which has
> "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> it seems you abandoned this idea. May I ask why? Just want to understand
> the reasoning here.
Because that requires more complexity and there's no known use case, and I don't
see an obvious way for a use case to come along. All of the motiviations for a
custom allocation scheme that I can think of apply only to full pages, or fit
nicely in a kmem_cache.
Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
"page" usage. Further splitting the "page" case doesn't require modifications to
the "kmem_cache" case, whereas providing a fully generic solution would require
additional changes, e.g. to handle this code:
page = (void *)__get_free_page(gfp_flags);
if (page && mc->init_value)
memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
It certainly wouldn't be much complexity, but this code is already a bit awkward,
so I don't think it makes sense to add support for something that will probably
never be used.
^ permalink raw reply
* Re: [RFC PATCH v5 02/45] KVM: x86/mmu: Update iter->old_spte if cmpxchg64 on mirror SPTE "fails"
From: Sean Christopherson @ 2026-02-03 20:06 UTC (permalink / raw)
To: Kai Huang
Cc: x86@kernel.org, dave.hansen@linux.intel.com, kas@kernel.org,
bp@alien8.de, mingo@redhat.com, pbonzini@redhat.com,
tglx@kernel.org, Rick P Edgecombe, ackerleytng@google.com,
sagis@google.com, Vishal Annapurve, linux-kernel@vger.kernel.org,
Yan Y Zhao, Xiaoyao Li, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Isaku Yamahata,
binbin.wu@linux.intel.com
In-Reply-To: <1c4bdb3613ebaf65b5dcf9a2268b06fa0c5a6ef3.camel@intel.com>
On Tue, Feb 03, 2026, Kai Huang wrote:
> On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > Pass a pointer to iter->old_spte, not simply its value, when setting an
> > external SPTE in __tdp_mmu_set_spte_atomic(), so that the iterator's value
> > will be updated if the cmpxchg64 to freeze the mirror SPTE fails. The bug
> > is currently benign as TDX is mutualy exclusive with all paths that do
> > "local" retry", e.g. clear_dirty_gfn_range() and wrprot_gfn_range().
> >
> > Fixes: 77ac7079e66d ("KVM: x86/tdp_mmu: Propagate building mirror page tables")
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
>
> Btw, do we need to cc stable?
Probably not? The bug is benign until dirty logging comes along, and if someone
backports that support (if it ever manifests) to an older kernel, it's firmly
that person's responsibility to pick up dependencies like this.
^ permalink raw reply
* Re: [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Sean Christopherson @ 2026-02-03 20:05 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYHLlTPeo2fzh02y@yzhao56-desk.sh.intel.com>
On Tue, Feb 03, 2026, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:14:37PM -0800, Sean Christopherson wrote:
> > static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
> > gfn_t gfn, u64 *old_spte,
> > u64 new_spte, int level)
> > {
> > - bool was_present = is_shadow_present_pte(*old_spte);
> > - bool is_present = is_shadow_present_pte(new_spte);
> > - bool is_leaf = is_present && is_last_spte(new_spte, level);
> > - int ret = 0;
> > -
> > - KVM_BUG_ON(was_present, kvm);
> > + int ret;
> >
> > lockdep_assert_held(&kvm->mmu_lock);
> > +
> > + if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> > + return -EIO;
> Why not move this check of is_shadow_present_pte() to tdx_sept_set_private_spte()
> as well?
The series gets there eventually, but as of this commit, @old_spte isn't plumbed
into tdx_sept_set_private_spte().
> Or also check !is_shadow_present_pte(new_spte) in TDP MMU?
Not sure I understand this suggestion.
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index 5688c77616e3..30494f9ceb31 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -1664,18 +1664,58 @@ static int tdx_mem_page_aug(struct kvm *kvm, gfn_t gfn,
> > return 0;
> > }
> >
> > +static struct page *tdx_spte_to_external_spt(struct kvm *kvm, gfn_t gfn,
> > + u64 new_spte, enum pg_level level)
> > +{
> > + struct kvm_mmu_page *sp = spte_to_child_sp(new_spte);
> > +
> > + if (KVM_BUG_ON(!sp->external_spt, kvm) ||
> > + KVM_BUG_ON(sp->role.level + 1 != level, kvm) ||
> > + KVM_BUG_ON(sp->gfn != gfn, kvm))
> > + return NULL;
> Could we remove the KVM_BUG_ON()s, and ...
>
> > + return virt_to_page(sp->external_spt);
> > +}
> > +
> > +static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn,
> > + enum pg_level level, u64 mirror_spte)
> > +{
> > + gpa_t gpa = gfn_to_gpa(gfn);
> > + u64 err, entry, level_state;
> > + struct page *external_spt;
> > +
> > + external_spt = tdx_spte_to_external_spt(kvm, gfn, mirror_spte, level);
> > + if (!external_spt)
> add a KVM_BUG_ON() here?
> It could save KVM_BUG_ON()s and have KVM_BUG_ON() match -EIO :)
We could, but I don't want to, because if we're going to bother with sanity checks,
I want the resulting WARNs to be precise. I.e. I want the WARN to capture *why*
tdx_spte_to_external_spt() failed, to make debug/triage easier.
> And as Rick also mentioned, better to remove external in external_spt, e.g.
> something like pt_page.
Yeah, maybe sept_spt?
> And mirror_spte --> new_spte?
Hmm, ya, I made that change later, but it can probably be shifted here.
> > - WARN_ON_ONCE(!is_shadow_present_pte(mirror_spte) ||
> > - (mirror_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
> > + WARN_ON_ONCE((mirror_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
> Also check this for tdx_sept_link_private_spt()?
Eh, we could, but I don't think it's necessary. make_nonleaf_spte() is hardcoded
to set full permissions (and I don't see that changing any time soon), whereas
leaf SPTE protections are much more dynamic.
^ permalink raw reply
* [PATCH v4 2/2] x86/sev: Add support to unaccept memory after hot-remove
From: Pratik R. Sampat @ 2026-02-03 17:49 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: <20260203174946.1198053-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 | 15 ++++++
drivers/firmware/efi/unaccepted_memory.c | 59 ++++++++++++++++++++++++
include/linux/mm.h | 4 ++
mm/memory_hotplug.c | 2 +
6 files changed, 95 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..91f01ad0ee03 100644
--- a/arch/x86/include/asm/unaccepted_memory.h
+++ b/arch/x86/include/asm/unaccepted_memory.h
@@ -18,6 +18,21 @@ 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)
+{
+ /*
+ * TDX platforms do not require the guest to transition pages on remove
+ * rather expect the VMM to remove the unplugged memory from SEPT
+ */
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
+ return;
+ } else 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 359779133cb4..d11e7836200a 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -256,6 +256,65 @@ void accept_hotplug_memory(phys_addr_t start, unsigned long size)
spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
}
+/*
+ * Cold-plugged memory used with lazy acceptance may partially set pages to
+ * private. On removal, iterate through the bitmap to unaccept those ranges.
+ * For hotplug ranges beyond the bitmap, unaccept unconditionally.
+ */
+void unaccept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+ unsigned long range_start, range_end, bitrange_end;
+ phys_addr_t bitmap_end, end = start + size;
+ struct efi_unaccepted_memory *unaccepted;
+ u64 phys_base, unit_size;
+ unsigned long flags;
+
+ unaccepted = efi_get_unaccepted_table();
+ if (!unaccepted)
+ return;
+
+ phys_base = unaccepted->phys_base;
+ unit_size = unaccepted->unit_size;
+ bitmap_end = phys_base + unaccepted->size * unit_size * BITS_PER_BYTE;
+
+ /* Unaccept the entire hotplug range beyond the bitmap immediately */
+ if (start >= bitmap_end) {
+ arch_unaccept_memory(start, end);
+ return;
+ }
+
+ start = max(start, phys_base);
+ if (end < phys_base)
+ return;
+
+ /* Unaccept ranges when start is within the bitmap but end is beyond */
+ if (end > bitmap_end) {
+ arch_unaccept_memory(bitmap_end, end);
+ end = bitmap_end;
+ }
+
+ start -= phys_base;
+ end -= phys_base;
+
+ range_start = start / unit_size;
+ bitrange_end = DIV_ROUND_UP(end, unit_size);
+
+ /* Only unaccept memory that was previously accepted in the bitmap */
+ 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);
+}
+
#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..49b194cddda7 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4505,6 +4505,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);
+void unaccept_hotplug_memory(phys_addr_t start, unsigned long size);
#else
@@ -4522,6 +4523,9 @@ 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 v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Pratik R. Sampat @ 2026-02-03 17:49 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: <20260203174946.1198053-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 | 47 ++++++++++++++++++++++++
include/linux/mm.h | 5 +++
mm/memory_hotplug.c | 2 +
3 files changed, 54 insertions(+)
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index c2c067eff634..359779133cb4 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -209,6 +209,53 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
return ret;
}
+/*
+ * Unaccepted memory bitmap only covers initial boot memory and not the
+ * hotpluggable range that is part of SRAT parsing. However, some initial memory
+ * with the attribute EFI_MEMORY_HOT_PLUGGABLE can indicate boot time memory
+ * that can be hot-removed. Hence post acceptance, only for that range update
+ * the unaccepted bitmap to reflect this change.
+ */
+void accept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+ struct efi_unaccepted_memory *unaccepted;
+ unsigned long range_start, range_len;
+ phys_addr_t end = start + size;
+ u64 phys_base, unit_size;
+ unsigned long flags;
+
+ unaccepted = efi_get_unaccepted_table();
+ if (!unaccepted)
+ return;
+
+ /* Accept hotplug range unconditionally */
+ arch_accept_memory(start, end);
+
+ phys_base = unaccepted->phys_base;
+ unit_size = unaccepted->unit_size;
+
+ /* Only update bitmap for the region that is represented by it */
+ if (start >= phys_base + unaccepted->size * unit_size * BITS_PER_BYTE)
+ return;
+
+ start = max(start, phys_base);
+ if (end < phys_base)
+ return;
+
+ start -= phys_base;
+ end -= phys_base;
+
+ /* Make sure not to overrun the bitmap */
+ end = min(end, unaccepted->size * unit_size * BITS_PER_BYTE);
+
+ range_start = start / unit_size;
+ range_len = DIV_ROUND_UP(end, unit_size) - range_start;
+
+ spin_lock_irqsave(&unaccepted_memory_lock, flags);
+ bitmap_clear(unaccepted->bitmap, range_start, range_len);
+ spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
+}
+
#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 v4 0/2] SEV-SNP Unaccepted Memory Hotplug
From: Pratik R. Sampat @ 2026-02-03 17:49 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 via 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 v3..v4:
-----------------------
https://lore.kernel.org/all/20260128204105.508855-1-prsampat@amd.com/
1. Unconditionally accept all hotplug pages and set bitmap for the ones
that are in the bit range (Kiryl)
2. Fix implementation similarly for unacceptance and merge
unaccept_memory() implementation within unaccept_hotplug_memory()
3. Use max()/min() when clamping memory ranges to operate in the
bitmap (Andrew)
4. Fall through arch_unaccept_memory() for TDX platforms (Kiryl).
However, retain the panic() similar to arch_accept_memory() since
it is a can't-happen scenario for other archs.
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 | 15 ++++
drivers/firmware/efi/unaccepted_memory.c | 106 +++++++++++++++++++++++
include/linux/mm.h | 9 ++
mm/memory_hotplug.c | 4 +
6 files changed, 149 insertions(+)
--
2.52.0
^ permalink raw reply
* SVSM Development Call February 4, 2026
From: Jörg Rödel @ 2026-02-03 17:06 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the call for agenda items for this weeks SVSM development call. Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Dave Hansen @ 2026-02-03 16:12 UTC (permalink / raw)
To: Sean Christopherson, Chao Gao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, rick.p.edgecombe, kas, dave.hansen,
vishal.l.verma, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin
In-Reply-To: <aYIXFmT-676oN6j0@google.com>
On 2/3/26 07:41, Sean Christopherson wrote:
>> It seems like a bug, or at least a P-SEAMLDR implementation issue the
>> needs to get fixed.
> Yeah, 'tis odd behavior. IMO, that's all the more reason the TDX subsystem should
> hide the quirk from the rest of the kernel.
>
> [*] https://lore.kernel.org/all/20251010220403.987927-1-seanjc@google.com
For now, I say treat it as a bug. Don't deal with it in the series.
If it truly is unfixable P-SEAMLDR behavior, then Intel can issue and
erratum for it and we can add (ugly) code to follow Sean's suggestion.
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Sean Christopherson @ 2026-02-03 15:41 UTC (permalink / raw)
To: Chao Gao
Cc: Dave Hansen, linux-coco, linux-kernel, kvm, x86, reinette.chatre,
ira.weiny, kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve,
paulmck, nik.borisov, zhenzhong.duan, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, Farrah Chen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <aYHmUCLRYL+JX1ga@intel.com>
On Tue, Feb 03, 2026, Chao Gao wrote:
> >>> 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.
> >>
> >> The idea is feasible. But just calling vmcs_clear() won't work. We need to
> >> reset all the tracking state associated with each VMCS. We should call
> >> vmclear_local_loaded_vmcss() instead, similar to what's done before VMXOFF.
> >>
> >>>
> >>> I'm sure Sean and Paolo will tell me if I'm crazy.
> >>
> >> To me, this approach needs more work since we need to either move
> >> vmclear_local_loaded_vmcss() to the kernel or allow KVM to register a callback.
> >>
> >> I don't think it's as straightforward as just doing the save/restore.
> >
> >Could you please just do me a favor and spend 20 minutes to see what
> >this looks like in practice and if the KVM folks hate it?
I hate it :-)
> Sure. KVM tracks the current VMCS and only executes vmptrld for a new VMCS if
> it differs from the current one. See arch/x86/kvm/vmx/vmx.c::vmx_vcpu_load_vmcs()
>
> prev = per_cpu(current_vmcs, cpu);
> if (prev != vmx->loaded_vmcs->vmcs) {
> per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs;
> vmcs_load(vmx->loaded_vmcs->vmcs);
> }
>
> By resetting current_vmcs to NULL during P-SEAMLDR calls, KVM is forced to do a
> vmptrld on the next VMCS load. So, we can implement seamldr_call() as:
>
> static int seamldr_call(u64 fn, struct tdx_module_args *args)
> {
> int ret;
>
> WARN_ON_ONCE(!is_seamldr_call(fn));
>
> /*
> * Serialize P-SEAMLDR calls since only a single CPU is allowed to
> * interact with P-SEAMLDR at a time.
> *
> * P-SEAMLDR calls invalidate the current VMCS. Exclude KVM access to
> * the VMCS by disabling interrupts. This is not safe against VMCS use
> * in NMIs, but there are none of those today.
> *
> * Set the per-CPU current_vmcs cache to NULL to force KVM to reload
> * the VMCS.
> */
> guard(raw_spinlock_irqsave)(&seamldr_lock);
> ret = seamcall_prerr(fn, args);
> this_cpu_write(current_vmcs, NULL);
>
> return ret;
> }
>
> This requires moving the per-CPU current_vmcs from KVM to the kernel, which
> should be trivial with Sean's VMXON series.
Trivial in code, but I am very strongly opposed to moving current_vmcs out of KVM.
As stated in the cover letter of the initial VMXON RFC[*]:
: Emphasis on "only", because leaving VMCS tracking and clearing in KVM is
: another key difference from Xin's series. The "light bulb" moment on that
: front is that TDX isn't a hypervisor, and isn't trying to be a hypervisor.
: Specifically, TDX should _never_ have it's own VMCSes (that are visible to the
: host; the TDX-Module has it's own VMCSes to do SEAMCALL/SEAMRET), and so there
: is simply no reason to move that functionality out of KVM.
TDX's "use" of a VMCS should be completely transparent to KVM, because otherwise
we are stepping over that line that says the TDX subsystem isn't a hypervisor.
I also really, really don't want to add a super special case rule to KVM's VMCS
tracking logic.
After reading through the rest of this discussion, I'm doubling down on that
stance, because I agree that this is decidely odd behavior.
Pulling in two other threads from this discussion:
On Wed, Jan 28, 2026 at 3:05 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> 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?
As above, I'm definitely not ok with the current VMCS being zapped out from
underneath KVM. As to whether or not I'm ok with the P-SEAMLDR behavior, I would
say that's more of a question for you, as it will fall on the TDX subsytem to
workaround the bug/quirk.
On Fri, Jan 30, 2026 at 8:23 AM Dave Hansen <dave.hansen@intel.com> wrote:
> On 1/30/26 00:08, Chao Gao wrote:
> > AFAIK, this is a CPU implementation issue. The actual requirement is to
> > evict (flush and invalidate) all VMCSs __cached in SEAM mode__, but big
> > cores implement this by evicting the __entire__ VMCS cache. So, the
> > current VMCS is invalidated and cleared.
>
> But why is this a P-SEAMLDR thing and not a TDX module thing?
My guess is that it's because the P-SEAMLDR code loads and prepares the new TDX-
Module by constructing the VMCS used for SEAMCALL using direct writes to memory
(unless that TDX behavior has changed in the last few years). And so it needs
to ensure that in-memory representation is synchronized with the VMCS cache.
Hmm, but that doesn't make sense _if_ it really truly is SEAMRET that does the VMCS
cache invalidation, because flushing the VMCS cache would ovewrite the in-memory
state.
> It seems like a bug, or at least a P-SEAMLDR implementation issue the
> needs to get fixed.
Yeah, 'tis odd behavior. IMO, that's all the more reason the TDX subsystem should
hide the quirk from the rest of the kernel.
[*] https://lore.kernel.org/all/20251010220403.987927-1-seanjc@google.com
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-02-03 12:15 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, Farrah Chen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <fedb3192-e68c-423c-93b2-a4dc2f964148@intel.com>
>>> 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.
>>
>> The idea is feasible. But just calling vmcs_clear() won't work. We need to
>> reset all the tracking state associated with each VMCS. We should call
>> vmclear_local_loaded_vmcss() instead, similar to what's done before VMXOFF.
>>
>>>
>>> I'm sure Sean and Paolo will tell me if I'm crazy.
>>
>> To me, this approach needs more work since we need to either move
>> vmclear_local_loaded_vmcss() to the kernel or allow KVM to register a callback.
>>
>> I don't think it's as straightforward as just doing the save/restore.
>
>Could you please just do me a favor and spend 20 minutes to see what
>this looks like in practice and if the KVM folks hate it?
Sure. KVM tracks the current VMCS and only executes vmptrld for a new VMCS if
it differs from the current one. See arch/x86/kvm/vmx/vmx.c::vmx_vcpu_load_vmcs()
prev = per_cpu(current_vmcs, cpu);
if (prev != vmx->loaded_vmcs->vmcs) {
per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs;
vmcs_load(vmx->loaded_vmcs->vmcs);
}
By resetting current_vmcs to NULL during P-SEAMLDR calls, KVM is forced to do a
vmptrld on the next VMCS load. So, we can implement seamldr_call() as:
static int seamldr_call(u64 fn, struct tdx_module_args *args)
{
int ret;
WARN_ON_ONCE(!is_seamldr_call(fn));
/*
* Serialize P-SEAMLDR calls since only a single CPU is allowed to
* interact with P-SEAMLDR at a time.
*
* P-SEAMLDR calls invalidate the current VMCS. Exclude KVM access to
* the VMCS by disabling interrupts. This is not safe against VMCS use
* in NMIs, but there are none of those today.
*
* Set the per-CPU current_vmcs cache to NULL to force KVM to reload
* the VMCS.
*/
guard(raw_spinlock_irqsave)(&seamldr_lock);
ret = seamcall_prerr(fn, args);
this_cpu_write(current_vmcs, NULL);
return ret;
}
This requires moving the per-CPU current_vmcs from KVM to the kernel, which
should be trivial with Sean's VMXON series.
And I tested this. Without this_cpu_write(), vmread/vmwrite errors occur after
TDX Module updates. But with it, no errors.
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Huang, Kai @ 2026-02-03 11:16 UTC (permalink / raw)
To: seanjc@google.com, x86@kernel.org, dave.hansen@linux.intel.com,
kas@kernel.org, bp@alien8.de, mingo@redhat.com,
pbonzini@redhat.com, tglx@kernel.org
Cc: Edgecombe, Rick P, ackerleytng@google.com, sagis@google.com,
Annapurve, Vishal, linux-kernel@vger.kernel.org, Zhao, Yan Y,
Li, Xiaoyao, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Yamahata, Isaku, binbin.wu@linux.intel.com
In-Reply-To: <20260129011517.3545883-21-seanjc@google.com>
On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> Now that kvm_mmu_memory_cache supports custom page allocators, wire up the
> S-EPT cache to use tdx_{alloc,free}_control_page() (arguably S-EPT pages
> aren't "control" pages, but they're not guest pages either). Using the
> TDX APIs will make S-EPT pages naturally play nice with Dynamic PAMT, by
> virtue of adding/removing PAMT entries when S-EPT pages are allocated and
> freed, as opposed to when they are added/removed from the S-EPT tree.
>
> Inserting into the PAMT entries on allocation does mean KVM will create
> unnecessary PAMT entries, e.g. once a vCPU stops faulting in memory, the
> remaining pages in the MMU cache will go unused. But in practice, odds
> are very good the containing 2MiB page will have other in-use S-EPT pages,
> i.e. will create PAMT entries anyways. And _if_ creating PAMT entries on
> allocation is problematic for memory consumption, that can be resolved by
> tweaking KVM's cache size.
>
> Suggested-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Some nits below ..
[...]
> int (*set_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> u64 mirror_spte);
> -
> - /* Update external page tables for page table about to be freed. */
> void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> struct kvm_mmu_page *sp);
> -
> - /* Update external page table from spte getting removed, and flush TLB. */
The above two comments are still useful to me.
Not sure why do you want to remove them, especially in _this_ patch?
> void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> u64 mirror_spte);
>
> +
Unintentional change?
> bool (*has_wbinvd_exit)(void);
>
> u64 (*get_l2_tsc_offset)(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 3911ac9bddfd..9b5a6861e2a4 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -6690,11 +6690,13 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
> vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache;
> vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO;
>
> - vcpu->arch.mmu_shadow_page_cache.init_value =
> - SHADOW_NONPRESENT_VALUE;
> + vcpu->arch.mmu_shadow_page_cache.init_value = SHADOW_NONPRESENT_VALUE;
> if (!vcpu->arch.mmu_shadow_page_cache.init_value)
> vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO;
Ditto. Not sure this adjustment is intentional?
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Huang, Kai @ 2026-02-03 10:56 UTC (permalink / raw)
To: seanjc@google.com, x86@kernel.org, dave.hansen@linux.intel.com,
kas@kernel.org, bp@alien8.de, mingo@redhat.com,
pbonzini@redhat.com, tglx@kernel.org
Cc: Edgecombe, Rick P, ackerleytng@google.com, sagis@google.com,
Annapurve, Vishal, linux-kernel@vger.kernel.org, Zhao, Yan Y,
Li, Xiaoyao, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Yamahata, Isaku, binbin.wu@linux.intel.com
In-Reply-To: <20260129011517.3545883-20-seanjc@google.com>
On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> so that x86's TDX can update per-page metadata on allocation and free().
>
> Name the allocator page_get() to align with __get_free_page(), e.g. to
> communicate that it returns an "unsigned long", not a "struct page", and
> to avoid collisions with macros, e.g. with alloc_page.
>
> Suggested-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
I thought it could be more generic for allocating an object, but not just a
page.
E.g., I thought we might be able to use it to allocate a structure which has
"pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
it seems you abandoned this idea. May I ask why? Just want to understand
the reasoning here.
Anyway:
Reviewed-by: Kai Huang <kai.huang@intel.com>
> ---
> include/linux/kvm_types.h | 2 ++
> virt/kvm/kvm_main.c | 7 ++++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index a568d8e6f4e8..87fa9deffdb7 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -112,6 +112,8 @@ struct kvm_mmu_memory_cache {
> gfp_t gfp_custom;
> u64 init_value;
> struct kmem_cache *kmem_cache;
> + unsigned long (*page_get)(gfp_t gfp);
> + void (*page_free)(unsigned long addr);
> int capacity;
> int nobjs;
> void **objects;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 571cf0d6ec01..7015edce5bd8 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -356,7 +356,10 @@ static inline void *mmu_memory_cache_alloc_obj(struct kvm_mmu_memory_cache *mc,
> if (mc->kmem_cache)
> return kmem_cache_alloc(mc->kmem_cache, gfp_flags);
>
> - page = (void *)__get_free_page(gfp_flags);
> + if (mc->page_get)
> + page = (void *)mc->page_get(gfp_flags);
> + else
> + page = (void *)__get_free_page(gfp_flags);
> if (page && mc->init_value)
> memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
> return page;
> @@ -416,6 +419,8 @@ void kvm_mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc)
> while (mc->nobjs) {
> if (mc->kmem_cache)
> kmem_cache_free(mc->kmem_cache, mc->objects[--mc->nobjs]);
> + else if (mc->page_free)
> + mc->page_free((unsigned long)mc->objects[--mc->nobjs]);
> else
> free_page((unsigned long)mc->objects[--mc->nobjs]);
> }
> --
> 2.53.0.rc1.217.geba53bf80e-goog
^ permalink raw reply
* Re: [RFC PATCH v5 04/45] KVM: x86: Make "external SPTE" ops that can fail RET0 static calls
From: Huang, Kai @ 2026-02-03 10:44 UTC (permalink / raw)
To: seanjc@google.com, Edgecombe, Rick P
Cc: kvm@vger.kernel.org, Li, Xiaoyao, linux-coco@lists.linux.dev,
Zhao, Yan Y, dave.hansen@linux.intel.com, kas@kernel.org,
binbin.wu@linux.intel.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
Yamahata, Isaku, ackerleytng@google.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, sagis@google.com, x86@kernel.org
In-Reply-To: <e3feb0224cf2665a71ba6147e4e3e3bb30f96760.camel@intel.com>
On Fri, 2026-01-30 at 17:32 +0000, Edgecombe, Rick P wrote:
> On Thu, 2026-01-29 at 17:28 -0800, Sean Christopherson wrote:
> >
> > Hmm, that's probably doable, but definitely in a separate patch.
> > E.g. something
> > like:
>
> I think it would be a good change. But after more consideration, I
> think the original patch is good on its own. Better to turn a bug into
> a deterministic thing, than an opportunity to consume stack. Seems to
> be what you intended.
>
> Another idea would be to have a variant that returns an error instead
> of 0 so the callers can have there error logic triggered, but it's all
> incremental value on top of this.
>
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Makes sense.
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [RFC PATCH v5 03/45] KVM: TDX: Account all non-transient page allocations for per-TD structures
From: Huang, Kai @ 2026-02-03 10:36 UTC (permalink / raw)
To: seanjc@google.com, x86@kernel.org, dave.hansen@linux.intel.com,
kas@kernel.org, bp@alien8.de, mingo@redhat.com,
pbonzini@redhat.com, tglx@kernel.org
Cc: Edgecombe, Rick P, ackerleytng@google.com, sagis@google.com,
Annapurve, Vishal, linux-kernel@vger.kernel.org, Zhao, Yan Y,
Li, Xiaoyao, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Yamahata, Isaku, binbin.wu@linux.intel.com
In-Reply-To: <20260129011517.3545883-4-seanjc@google.com>
On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> 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>
>
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [RFC PATCH v5 02/45] KVM: x86/mmu: Update iter->old_spte if cmpxchg64 on mirror SPTE "fails"
From: Huang, Kai @ 2026-02-03 10:30 UTC (permalink / raw)
To: seanjc@google.com, x86@kernel.org, dave.hansen@linux.intel.com,
kas@kernel.org, bp@alien8.de, mingo@redhat.com,
pbonzini@redhat.com, tglx@kernel.org
Cc: Edgecombe, Rick P, ackerleytng@google.com, sagis@google.com,
Annapurve, Vishal, linux-kernel@vger.kernel.org, Zhao, Yan Y,
Li, Xiaoyao, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Yamahata, Isaku, binbin.wu@linux.intel.com
In-Reply-To: <20260129011517.3545883-3-seanjc@google.com>
On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> Pass a pointer to iter->old_spte, not simply its value, when setting an
> external SPTE in __tdp_mmu_set_spte_atomic(), so that the iterator's value
> will be updated if the cmpxchg64 to freeze the mirror SPTE fails. The bug
> is currently benign as TDX is mutualy exclusive with all paths that do
> "local" retry", e.g. clear_dirty_gfn_range() and wrprot_gfn_range().
>
> Fixes: 77ac7079e66d ("KVM: x86/tdp_mmu: Propagate building mirror page tables")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Btw, do we need to cc stable?
^ permalink raw reply
* Re: [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Yan Zhao @ 2026-02-03 10:19 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-6-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:37PM -0800, Sean Christopherson wrote:
> 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;
Why not move this check of is_shadow_present_pte() to tdx_sept_set_private_spte()
as well?
Or also check !is_shadow_present_pte(new_spte) in TDP MMU?
> * 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;
Could we remove the KVM_BUG_ON()s, and ...
> + return virt_to_page(sp->external_spt);
> +}
> +
> +static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn,
> + enum pg_level level, u64 mirror_spte)
> +{
> + gpa_t gpa = gfn_to_gpa(gfn);
> + u64 err, entry, level_state;
> + struct page *external_spt;
> +
> + external_spt = tdx_spte_to_external_spt(kvm, gfn, mirror_spte, level);
> + if (!external_spt)
add a KVM_BUG_ON() here?
It could save KVM_BUG_ON()s and have KVM_BUG_ON() match -EIO :)
And as Rick also mentioned, better to remove external in external_spt, e.g.
something like pt_page.
And mirror_spte --> new_spte?
> + 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);
Also check this for tdx_sept_link_private_spt()?
> /*
> * 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
* Re: [RFC PATCH v5 02/45] KVM: x86/mmu: Update iter->old_spte if cmpxchg64 on mirror SPTE "fails"
From: Yan Zhao @ 2026-02-03 8:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-3-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:34PM -0800, Sean Christopherson wrote:
> Pass a pointer to iter->old_spte, not simply its value, when setting an
> external SPTE in __tdp_mmu_set_spte_atomic(), so that the iterator's value
> will be updated if the cmpxchg64 to freeze the mirror SPTE fails. The bug
> is currently benign as TDX is mutualy exclusive with all paths that do
> "local" retry", e.g. clear_dirty_gfn_range() and wrprot_gfn_range().
>
> Fixes: 77ac7079e66d ("KVM: x86/tdp_mmu: Propagate building mirror page tables")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> 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);
Do we need to add a comment explaining that when the above try_cmpxchg64()
succeeds, the value of *old_spte is unmodified?
> 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
* Re: [PATCH 1/2] KVM: SEV: IBPB-on-Entry guest support
From: Borislav Petkov @ 2026-02-02 17:12 UTC (permalink / raw)
To: Tom Lendacky
Cc: Kim Phillips, linux-kernel, kvm, linux-coco, x86,
Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Michael Roth, Naveen Rao, David Kaplan, stable
In-Reply-To: <3392d89c-ebf5-48f2-b498-a7dc532a0493@amd.com>
On Mon, Feb 02, 2026 at 10:09:19AM -0600, Tom Lendacky wrote:
> But I can see that getting stale because it isn't required to be updated
> for features that don't require an implementation in order for the guest
> to boot successfully. Whereas the SNP_FEATURES_IMPL_REQ is set with
> known values that require an implementation and all the reserved bits
> set. So it takes actual updating to get one of those features to work
> that are represented in that bitmap.
Ok, I guess we can rename that define SNP_FEATURES_IMPL to denote is the
counterpart of SNP_FEATURES_IMPL_REQ, so to speak.
@Kim, you can send a new version with the define renamed.
Due to it being too close to the merge window, it'll wait for after and then
it can go to stable later but I don't think that's a problem.
> That will tell us what the guest is running with, not what it can run with.
hm, ok, let's think about this more then. I don't have a clear use case for
a this-is-what-a-SNP-guest-can-run-with so let's deal with that later...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 1/2] KVM: SEV: IBPB-on-Entry guest support
From: Tom Lendacky @ 2026-02-02 16:09 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kim Phillips, linux-kernel, kvm, linux-coco, x86,
Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Michael Roth, Naveen Rao, David Kaplan, stable
In-Reply-To: <20260202154936.GAaYDHkOMpjFpoBe5m@fat_crate.local>
On 2/2/26 09:49, Borislav Petkov wrote:
> On Mon, Feb 02, 2026 at 09:38:50AM -0600, Tom Lendacky wrote:
>> I guess it really depends on the persons point of view. I agree that
>> renaming the SNP_FEATURES_PRESENT to SNP_FEATURES_IMPL(EMENTED) would
>> match up nicely with SNP_FEATURES_IMPL_REQ. Maybe that's all that is
>> needed...
>
> I guess...
>
> I still think it would be useful to have a common place that says which things
> in SEV_STATUS are supported and present in a guest, no?
But I can see that getting stale because it isn't required to be updated
for features that don't require an implementation in order for the guest
to boot successfully. Whereas the SNP_FEATURES_IMPL_REQ is set with
known values that require an implementation and all the reserved bits
set. So it takes actual updating to get one of those features to work
that are represented in that bitmap.
>
> Or are we going to dump that MSR like Joerg's patch from a while ago and
> that'll tell us what the guest supports?
That will tell us what the guest is running with, not what it can run with.
Thanks,
Tom
>
> Hmm.
>
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Salman Nabi @ 2026-02-02 15:52 UTC (permalink / raw)
To: Trilok Soni, vvidwans, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <b5fd72fa-fcde-4f7a-bfad-17465d7e3841@oss.qualcomm.com>
Hi Trilok,
On 1/31/26 01:35, Trilok Soni wrote:
> On 1/19/2026 4:27 AM, Salman Nabi wrote:
>> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
>> activating firmware components without a reboot. Those components
>> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
>> usual way: via fwupd, FF-A or other secure storage methods, or via some
>> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
>> at system runtime, without requiring a reboot.
>> The specification covers the SMCCC interface to list and query available
>> components and eventually trigger the activation.
>>
>> Add a new directory under /sys/firmware to present firmware components
>> capable of live activation. Each of them is a directory under lfa/,
>> and is identified via its GUID. The activation will be triggered by echoing
>> "1" into the "activate" file:
>> ==========================================
>> /sys/firmware/lfa # ls -l . 6c*
>> .:
>> total 0
>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 47d4086d-4cfe-9846-9b95-2950cbbd5a00
>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 6c0762a6-12f2-4b56-92cb-ba8f633606d9
>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 d6d0eea7-fcea-d54b-9782-9934f234b6e4
> Can you please explain or add a note on why we don't have name of the firmware
> as the directory name and why you have selected GUID as top-level
> directory name?
We obtain the GUIDs of firmware components from the LFA agent in TF-A, which does not provide their names. For convenience, we have added a C structure in the driver to associate each firmware GUID with its corresponding name. Because new firmware components may be supported in the LFA agent before their GUID-to-name mapping is added to the driver, we avoid using the firmware name as the directory (kobject) name.
Additionally, sysfs requires directory names to be unique among siblings. Since GUIDs are inherently unique, they provide a convenient, collision-free choice for directory names.
>
> ---Trilok Soni
Many thanks,
Salman
^ permalink raw reply
* Re: [PATCH 1/2] KVM: SEV: IBPB-on-Entry guest support
From: Borislav Petkov @ 2026-02-02 15:49 UTC (permalink / raw)
To: Tom Lendacky
Cc: Kim Phillips, linux-kernel, kvm, linux-coco, x86,
Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Michael Roth, Naveen Rao, David Kaplan, stable
In-Reply-To: <6556bacb-2e81-4aa8-92e4-0ff8642f4ec9@amd.com>
On Mon, Feb 02, 2026 at 09:38:50AM -0600, Tom Lendacky wrote:
> I guess it really depends on the persons point of view. I agree that
> renaming the SNP_FEATURES_PRESENT to SNP_FEATURES_IMPL(EMENTED) would
> match up nicely with SNP_FEATURES_IMPL_REQ. Maybe that's all that is
> needed...
I guess...
I still think it would be useful to have a common place that says which things
in SEV_STATUS are supported and present in a guest, no?
Or are we going to dump that MSR like Joerg's patch from a while ago and
that'll tell us what the guest supports?
Hmm.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 1/2] KVM: SEV: IBPB-on-Entry guest support
From: Tom Lendacky @ 2026-02-02 15:38 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kim Phillips, linux-kernel, kvm, linux-coco, x86,
Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Michael Roth, Naveen Rao, David Kaplan, stable
In-Reply-To: <20260130154534.GCaXzSHgkEFnk5mX14@fat_crate.local>
On 1/30/26 09:45, Borislav Petkov wrote:
> On Fri, Jan 30, 2026 at 08:56:07AM -0600, Tom Lendacky wrote:
>> It can be added. Any of the features added to SNP_FEATURES_PRESENT that
>> aren't set in the SNP_FEATURES_IMPL_REQ bitmap are really a no-op. The
>> SNP_FEATURES_PRESENT bitmap is meant to contain whatever bits are set in
>> SNP_FEATURES_IMPL_REQ when an implementation has been implemented for the
>> guest.
>>
>> But, yeah, we could add all the bits that aren't set in
>> SNP_FEATURES_IMPL_REQ to SNP_FEATURES_PRESENT if it makes it clearer.
>
> Right, that's the question. SNP_FEATURES_PRESENT is used in the masking
> operation to get the unsupported features.
>
> But when we say a SNP feature is present, then, even if it doesn't need guest
> implementation, that feature is still present nonetheless.
>
> So our nomenclature is kinda imprecise here.
>
> I'd say, we can always rename SNP_FEATURES_PRESENT to denote what it is there
> for, i.e., the narrower functionality of the masking.
>
> Or, if we want to gather there *all* features that are present, then we can
> start adding them...
>
>> If we do that, it should probably be a separate patch (?) that also
>> rewords the comment above SNP_FEATURES_PRESENT
>
> ... yes, as a separate patch.
>
> Question is, what do we really wanna do here?
>
> Does it make sense and is it useful to have SNP_FEATURES_PRESENT contain *all*
> guest SNP features...
I guess it really depends on the persons point of view. I agree that
renaming the SNP_FEATURES_PRESENT to SNP_FEATURES_IMPL(EMENTED) would
match up nicely with SNP_FEATURES_IMPL_REQ. Maybe that's all that is
needed...
Thanks,
Tom
>
> Thx.
>
^ permalink raw reply
* Re: [PATCH v3 24/26] x86/virt/seamldr: Extend sigstruct to 16KB
From: Huang, Kai @ 2026-02-02 11:57 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
dave.hansen@linux.intel.com, kas@kernel.org, seanjc@google.com,
Chatre, Reinette, linux-kernel@vger.kernel.org,
tglx@linutronix.de, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, bp@alien8.de, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <aXy/S47ryxy0PwpM@intel.com>
On Fri, 2026-01-30 at 22:25 +0800, Gao, Chao wrote:
> > 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?
>
> Good question.
>
> This is actually userspace's responsibility. The kernel exposes P-SEAMLDR
> version to userspace, and for each module, the mapping file [*] lists the
> module's minimum P-SEAMLDR version requirements. This allows userspace to
> determine whether the existing P-SEAMLDR can load a specific TDX blob.
>
> If the kernel cannot load a module using the current P-SEAMLDR, that's
> userspace's fault.
>
> *: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json
Thanks for the info.
In this case, I am not sure why do you need to implement code (patch 13) to
firstly support 4K less SIGSTRUCT (with a confusing doc of 'tdx_blob' layout
definition), but here extend it to 16K in a second patch?
How about just merge this one to patch 13 and point out this fact in
changelog if needed?
E.g.,:
For a given TDX blob, not all SEAMLDR and TDX module versions support
runtime update for it. Intel publishes the requirement of the minimal
SEAMLDR and TDX module versions for it.
There's no hardware/firmware interface that the kernel could use to
detect and bail out early if such requirement is not met. It's
userspace's responsibility to make sure such requirement is met before
performing runtime update.
Actually, assuming the new spec which reflects 16KB SIGSTRUCT in
SEAMLDR_PARAMS will be published, I _think_ the fact that "some old SEMALDR
versions only support upto 4K SIGSTRUCT" probably doesn't matter anymore,
especially if we add the above to the changelog.
So I don't quite see why we need to keep this "extend SIGSTRUCT to 16KB" as
a separate patch?
Btw, we also need the updated doc of TDX blob layout too, to reflect what
your code is doing (regarding to how to calculate SIGSTRUCT base/size).
And we need to make sure the TDX blob layout is architectural.
^ permalink raw reply
* [PATCH RESEND] KVM: SEV: Enable SNP AP CPU hotplug
From: Jethro Beekman @ 2026-02-02 10:12 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, kvm,
linux-kernel, linux-coco
The GHCB protocol states that after AP CREATE (as opposed to CREATE_ON_INIT),
the hypervisor must immediately proceed to VMRUN. Update vCPU state on AP
CREATE so this happens.
vCPUs created after SNP_LAUNCH_FINISH don't go through snp_launch_update_vmsa.
Ensure the vCPU state is updated properly during VMCB initialization.
Signed-off-by: Jethro Beekman <jethro@fortanix.com>
---
arch/x86/kvm/svm/sev.c | 43 ++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index cdaca10b8773..9af1bd5b2071 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -960,6 +960,19 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
return 0;
}
+static void sev_es_finalize_vcpu(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.guest_state_protected = true;
+ /*
+ * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
+ * be _always_ ON. Enable it only after setting
+ * guest_state_protected because KVM_SET_MSRS allows dynamic
+ * toggling of LBRV (for performance reason) on write access to
+ * MSR_IA32_DEBUGCTLMSR when guest_state_protected is not set.
+ */
+ svm_enable_lbrv(vcpu);
+}
+
static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
int *error)
{
@@ -999,15 +1012,9 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
* do xsave/xrstor on it.
*/
fpstate_set_confidential(&vcpu->arch.guest_fpu);
- vcpu->arch.guest_state_protected = true;
- /*
- * SEV-ES guest mandates LBR Virtualization to be _always_ ON. Enable it
- * only after setting guest_state_protected because KVM_SET_MSRS allows
- * dynamic toggling of LBRV (for performance reason) on write access to
- * MSR_IA32_DEBUGCTLMSR when guest_state_protected is not set.
- */
- svm_enable_lbrv(vcpu);
+ sev_es_finalize_vcpu(vcpu);
+
return 0;
}
@@ -2480,15 +2487,7 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
- svm->vcpu.arch.guest_state_protected = true;
- /*
- * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
- * be _always_ ON. Enable it only after setting
- * guest_state_protected because KVM_SET_MSRS allows dynamic
- * toggling of LBRV (for performance reason) on write access to
- * MSR_IA32_DEBUGCTLMSR when guest_state_protected is not set.
- */
- svm_enable_lbrv(vcpu);
+ sev_es_finalize_vcpu(vcpu);
}
return 0;
@@ -4030,6 +4029,10 @@ static void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
/* Use the new VMSA */
svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
+ /* vCPU was added after SNP_LAUNCH_FINISH */
+ if (!vcpu->arch.guest_state_protected)
+ sev_es_finalize_vcpu(vcpu);
+
/* Mark the vCPU as runnable */
kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
@@ -4111,8 +4114,12 @@ static int sev_snp_ap_creation(struct vcpu_svm *svm)
* Unless Creation is deferred until INIT, signal the vCPU to update
* its state.
*/
- if (request != SVM_VMGEXIT_AP_CREATE_ON_INIT)
+ if (request != SVM_VMGEXIT_AP_CREATE_ON_INIT) {
+ if (target_vcpu->arch.mp_state == KVM_MP_STATE_UNINITIALIZED ||
+ target_vcpu->arch.mp_state == KVM_MP_STATE_INIT_RECEIVED)
+ kvm_set_mp_state(target_vcpu, KVM_MP_STATE_RUNNABLE);
kvm_make_request_and_kick(KVM_REQ_UPDATE_PROTECTED_GUEST_STATE, target_vcpu);
+ }
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH RESEND] KVM: SEV: Track SNP launch state and disallow invalid userspace interactions
From: Jethro Beekman @ 2026-02-02 10:11 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, kvm,
linux-kernel, linux-coco
Calling any of the SNP_LAUNCH_ ioctls after SNP_LAUNCH_FINISH results in a
kernel page fault due to RMP violation. Track SNP launch state and exit early.
vCPUs created after SNP_LAUNCH_FINISH won't have a guest VMSA automatically
created during SNP_LAUNCH_FINISH by converting the kernel-allocated VMSA. Don't
allocate a VMSA page, so that the vCPU is in a state similar to what it would
be after SNP AP destroy. This ensures pre_sev_run() prevents the vCPU from
running even if userspace makes the vCPU runnable.
Signed-off-by: Jethro Beekman <jethro@fortanix.com>
---
arch/x86/kvm/svm/sev.c | 43 ++++++++++++++++++++++++++----------------
arch/x86/kvm/svm/svm.h | 1 +
2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f59c65abe3cf..cdaca10b8773 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2205,6 +2205,9 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
if (!sev_snp_guest(kvm))
return -ENOTTY;
+ if (sev->snp_finished)
+ return -EINVAL;
+
if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
return -EFAULT;
@@ -2369,7 +2372,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
void __user *src;
int ret = 0;
- if (!sev_snp_guest(kvm) || !sev->snp_context)
+ if (!sev_snp_guest(kvm) || !sev->snp_context || sev->snp_finished)
return -EINVAL;
if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
@@ -2502,7 +2505,7 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
if (!sev_snp_guest(kvm))
return -ENOTTY;
- if (!sev->snp_context)
+ if (!sev->snp_context || sev->snp_finished)
return -EINVAL;
if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
@@ -2548,13 +2551,15 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
data->gctx_paddr = __psp_pa(sev->snp_context);
ret = sev_issue_cmd(kvm, SEV_CMD_SNP_LAUNCH_FINISH, data, &argp->error);
- /*
- * Now that there will be no more SNP_LAUNCH_UPDATE ioctls, private pages
- * can be given to the guest simply by marking the RMP entry as private.
- * This can happen on first access and also with KVM_PRE_FAULT_MEMORY.
- */
- if (!ret)
+ if (!ret) {
+ sev->snp_finished = true;
+ /*
+ * Now that there will be no more SNP_LAUNCH_UPDATE ioctls, private pages
+ * can be given to the guest simply by marking the RMP entry as private.
+ * This can happen on first access and also with KVM_PRE_FAULT_MEMORY.
+ */
kvm->arch.pre_fault_allowed = true;
+ }
kfree(id_auth);
@@ -3253,6 +3258,9 @@ void sev_free_vcpu(struct kvm_vcpu *vcpu)
svm = to_svm(vcpu);
+ if (!svm->sev_es.vmsa)
+ goto skip_vmsa_free;
+
/*
* If it's an SNP guest, then the VMSA was marked in the RMP table as
* a guest-owned page. Transition the page to hypervisor state before
@@ -4653,6 +4661,7 @@ void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
int sev_vcpu_create(struct kvm_vcpu *vcpu)
{
+ struct kvm_sev_info *sev = to_kvm_sev_info(vcpu->kvm);
struct vcpu_svm *svm = to_svm(vcpu);
struct page *vmsa_page;
@@ -4661,15 +4670,17 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
if (!sev_es_guest(vcpu->kvm))
return 0;
- /*
- * SEV-ES guests require a separate (from the VMCB) VMSA page used to
- * contain the encrypted register state of the guest.
- */
- vmsa_page = snp_safe_alloc_page();
- if (!vmsa_page)
- return -ENOMEM;
+ if (!sev->snp_finished) {
+ /*
+ * SEV-ES guests require a separate (from the VMCB) VMSA page used to
+ * contain the encrypted register state of the guest.
+ */
+ vmsa_page = snp_safe_alloc_page();
+ if (!vmsa_page)
+ return -ENOMEM;
- svm->sev_es.vmsa = page_address(vmsa_page);
+ svm->sev_es.vmsa = page_address(vmsa_page);
+ }
vcpu->arch.guest_tsc_protected = snp_is_secure_tsc_enabled(vcpu->kvm);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 01be93a53d07..59c328c13b2a 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -96,6 +96,7 @@ struct kvm_sev_info {
bool active; /* SEV enabled guest */
bool es_active; /* SEV-ES enabled guest */
bool need_init; /* waiting for SEV_INIT2 */
+ bool snp_finished; /* SNP guest measurement has been finalized */
unsigned int asid; /* ASID used for this guest */
unsigned int handle; /* SEV firmware handle */
int fd; /* SEV device fd */
--
2.43.0
^ permalink raw reply related
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