* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Kiryl Shutsemau @ 2026-02-05 10:51 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: David Hildenbrand (arm), linux-mm, linux-coco, x86, linux-kernel,
tglx, mingo, bp, dave.hansen, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <550c89ae-de6e-45f7-89a2-ccc815f8d5a2@amd.com>
On Wed, Feb 04, 2026 at 09:50:01PM -0600, Pratik R. Sampat wrote:
>
>
> On 2/4/26 1:59 PM, David Hildenbrand (arm) wrote:
> > On 2/4/26 12:22, Kiryl Shutsemau wrote:
> >> On Tue, Feb 03, 2026 at 11:49:45AM -0600, Pratik R. Sampat wrote:
> >>> Confidential computing guests require memory to be accepted before use.
> >>> The unaccepted memory bitmap maintained by firmware does not track
> >>> most hotplugged memory ranges apart from system memory annotated to be
> >>> cold plugged at boot.
> >>>
> >>> Explicitly validate and transition the newly added memory to a private
> >>> state, making it usable by the guest.
> >>>
> >>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
> >>> ---
> >>> drivers/firmware/efi/unaccepted_memory.c | 47 ++++++++++++++++++++++++
> >>> include/linux/mm.h | 5 +++
> >>> mm/memory_hotplug.c | 2 +
> >>> 3 files changed, 54 insertions(+)
> >>>
> >>> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
> >>> index c2c067eff634..359779133cb4 100644
> >>> --- a/drivers/firmware/efi/unaccepted_memory.c
> >>> +++ b/drivers/firmware/efi/unaccepted_memory.c
> >>> @@ -209,6 +209,53 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
> >>> return ret;
> >>> }
> >>> +/*
> >>> + * Unaccepted memory bitmap only covers initial boot memory and not the
> >>> + * hotpluggable range that is part of SRAT parsing. However, some initial memory
> >>> + * with the attribute EFI_MEMORY_HOT_PLUGGABLE can indicate boot time memory
> >>> + * that can be hot-removed. Hence post acceptance, only for that range update
> >>> + * the unaccepted bitmap to reflect this change.
> >>> + */
> >>> +void accept_hotplug_memory(phys_addr_t start, unsigned long size)
> >>> +{
> >>> + struct efi_unaccepted_memory *unaccepted;
> >>> + unsigned long range_start, range_len;
> >>> + phys_addr_t end = start + size;
> >>> + u64 phys_base, unit_size;
> >>> + unsigned long flags;
> >>> +
> >>> + unaccepted = efi_get_unaccepted_table();
> >>> + if (!unaccepted)
> >>> + return;
> >>
> >> This can be tricky.
> >>
> >> If we boot a VM with <4GiB of memory and all of it is pre-accepted by
> >> BIOS, the table will not be allocated.
> >>
> >> But it doesn't mean that hotplugged memory above should not be accepted.
> >>
> >> I don't think there is a way to detect such cases.
> >>
> >> Your check is probably the best we can do, but it means VMs are going to
> >> crash if memory accept is required by no table.
> >>
> >> This is ugly situation.
>
> Agreed. Breaking hotplug for VMs under 4G is absolutely not the way to go.
>
> Would it be worse if we call arch_accept_memory() if the table doesn't exist?
> The table is primarily to operate on the bitmap's entry. We could wrap these
> accept calls within an arch check for TDX and SNP guest if the unaccepted table
> is NULL. Or, less preferably convert the panic() of the existing
> arch_[accept/unaccept]_memory() to a WARN() instead.
I think you try to workaround a lack of proper design. I think the right
way would be to make unaccepted hotpluggable ranges declared upfront in
the EFI memory map, so kernel can allocate bitmap for all of it on boot
and not playing guessing game.
If it required EFI spec modification, let's do it.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Kiryl Shutsemau @ 2026-02-05 10:48 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: David Hildenbrand (arm), linux-mm, linux-coco, x86, linux-kernel,
tglx, mingo, bp, dave.hansen, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <70be936e-e49d-4485-8d1e-416fdf8f40a4@amd.com>
On Wed, Feb 04, 2026 at 09:50:09PM -0600, Pratik R. Sampat wrote:
>
>
> On 2/4/26 2:00 PM, David Hildenbrand (arm) wrote:
> >> #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();
> >>
> >
> > I really hate that accepting (and un-accepting) hotplugged memory is different to accepting ordinary boot memory.
> >
> > Is there really no way we can get a reasonable implementation where we just call a generic accept_memory() and it will know what to do?
> >
>
> Sure, that shouldn't be impossible.
>
> The only reason I initially kept them separate is because we accept and update
> the bitmap unconditionally. This mainly applies to cold-plugged memory since
> their bitmap state after remove shouldn't matter. However, as we are now
> correctly setting the bits in the hot-remove path we should be fine accepting
> from the for_each_set_bitrange_from() logic within accept_memory(), I think.
>
> Something like so?
>
> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
> index d11e7836200a..e56adfd382f8 100644
> --- a/drivers/firmware/efi/unaccepted_memory.c
> +++ b/drivers/firmware/efi/unaccepted_memory.c
> @@ -36,6 +36,7 @@ void accept_memory(phys_addr_t start, unsigned long size)
> unsigned long range_start, range_end;
> struct accept_range range, *entry;
> phys_addr_t end = start + size;
> + phys_addr_t bitmap_end;
> unsigned long flags;
> u64 unit_size;
>
> @@ -44,6 +45,21 @@ void accept_memory(phys_addr_t start, unsigned long size)
> return;
>
> unit_size = unaccepted->unit_size;
> + bitmap_end = unaccepted->phys_base + unaccepted->size * unit_size * BITS_PER_BYTE;
> +
> + /* Memory completely beyond bitmap: hotplug memory, accept unconditionally */
> + if (start >= bitmap_end) {
> + arch_accept_memory(start, end);
> + return;
> + }
> +
> + /* Memory partially beyond bitmap */
> + if (end > bitmap_end) {
> + arch_accept_memory(bitmap_end, end);
> + end = bitmap_end;
> + }
You are calling arch_accept_memory() on every memory allocation if the
memory is not represented in the bitmap. Hard NAK.
>
> /*
> * Only care for the part of the range that is represented
>
> unaccept_hotplug_memory() truly doesn't do anything special for hotplug so I
> could just re-name it unaccept_memory().
>
> Thanks!
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Yan Zhao @ 2026-02-05 7:04 UTC (permalink / raw)
To: Sean Christopherson, 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: <aYMVEX5OO22/Y72/@yzhao56-desk.sh.intel.com>
On Wed, Feb 04, 2026 at 05:45:39PM +0800, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:14:41PM -0800, Sean Christopherson wrote:
> > Massage .free_external_spt() into .reclaim_external_sp() to free up (pun
> > intended) "free" for actually freeing memory, and to allow TDX to do more
> > than just "free" the S-EPT entry. Specifically, nullify external_spt to
> > leak the S-EPT page if reclaiming the page fails, as that detail and
> > implementation choice has no business living in the TDP MMU.
> >
> > Use "sp" instead of "spt" even though "spt" is arguably more accurate, as
> > "spte" and "spt" are dangerously close in name, and because the key
> > parameter is a kvm_mmu_page, not a pointer to an S-EPT page table.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/include/asm/kvm-x86-ops.h | 2 +-
> > arch/x86/include/asm/kvm_host.h | 4 ++--
> > arch/x86/kvm/mmu/tdp_mmu.c | 13 ++-----------
> > arch/x86/kvm/vmx/tdx.c | 27 ++++++++++++---------------
> > 4 files changed, 17 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> > index 57eb1f4832ae..c17cedc485c9 100644
> > --- a/arch/x86/include/asm/kvm-x86-ops.h
> > +++ b/arch/x86/include/asm/kvm-x86-ops.h
> > @@ -95,8 +95,8 @@ KVM_X86_OP_OPTIONAL_RET0(set_identity_map_addr)
> > KVM_X86_OP_OPTIONAL_RET0(get_mt_mask)
> > KVM_X86_OP(load_mmu_pgd)
> > KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
> > -KVM_X86_OP_OPTIONAL_RET0(free_external_spt)
> > KVM_X86_OP_OPTIONAL(remove_external_spte)
> > +KVM_X86_OP_OPTIONAL(reclaim_external_sp)
> > KVM_X86_OP(has_wbinvd_exit)
> > KVM_X86_OP(get_l2_tsc_offset)
> > KVM_X86_OP(get_l2_tsc_multiplier)
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index d12ca0f8a348..b35a07ed11fb 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1858,8 +1858,8 @@ struct kvm_x86_ops {
> > u64 mirror_spte);
> >
> > /* Update external page tables for page table about to be freed. */
> > - int (*free_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > - void *external_spt);
> > + void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> > + struct kvm_mmu_page *sp);
> Do you think "free" is still better than "reclaim" though TDX actually
> invokes tdx_reclaim_page() to reclaim it on the TDX side?
>
> Naming it free_external_sp can be interpreted as freeing the sp->external_spt
> externally (vs freeing it in tdp_mmu_free_sp_rcu_callback(). This naming also
> allows for the future possibility of freeing sp->external_spt before the HKID is
> freed (though this is unlikely).
Oh. I found there's a free_external_sp() in patch 20.
So, maybe reclaim_external_sp() --> remove_external_spt() ?
Still think "sp" is not good :)
> > /* Update external page table from spte getting removed, and flush TLB. */
> > void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index 27ac520f2a89..18764dbc97ea 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -456,17 +456,8 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> > old_spte, FROZEN_SPTE, level, shared);
> > }
> >
> > - if (is_mirror_sp(sp) &&
> > - WARN_ON(kvm_x86_call(free_external_spt)(kvm, base_gfn, sp->role.level,
> > - sp->external_spt))) {
> > - /*
> > - * Failed to free page table page in mirror page table and
> > - * there is nothing to do further.
> > - * Intentionally leak the page to prevent the kernel from
> > - * accessing the encrypted page.
> > - */
> > - sp->external_spt = NULL;
> > - }
> > + if (is_mirror_sp(sp))
> > + kvm_x86_call(reclaim_external_sp)(kvm, base_gfn, sp);
> >
> > call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
> > }
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index 30494f9ceb31..66bc3ceb5e17 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -1783,27 +1783,24 @@ static void tdx_track(struct kvm *kvm)
> > kvm_make_all_cpus_request(kvm, KVM_REQ_OUTSIDE_GUEST_MODE);
> > }
> >
> > -static int tdx_sept_free_private_spt(struct kvm *kvm, gfn_t gfn,
> > - enum pg_level level, void *private_spt)
> > +static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
> > + struct kvm_mmu_page *sp)
> Passing in "sp" and having "reclaim_private_sp" in the function name is bit
> confusing.
> Strictly speaking, only sp->external_spt is private, while the sp and sp->spt
> are just mirroring the external spt.
>
> But I understand it's for setting sp->external_spt to NULL on error.
>
> > {
> > - struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> > -
> > /*
> > - * free_external_spt() is only called after hkid is freed when TD is
> > - * tearing down.
> > * KVM doesn't (yet) zap page table pages in mirror page table while
> > * TD is active, though guest pages mapped in mirror page table could be
> > * zapped during TD is active, e.g. for shared <-> private conversion
> > * and slot move/deletion.
> > + *
> > + * In other words, KVM should only free mirror page tables after the
> > + * TD's hkid is freed, when the TD is being torn down.
> > + *
> > + * If the S-EPT PTE can't be removed for any reason, intentionally leak
> > + * the page to prevent the kernel from accessing the encrypted page.
> > */
> > - if (KVM_BUG_ON(is_hkid_assigned(kvm_tdx), kvm))
> > - return -EIO;
> > -
> > - /*
> > - * The HKID assigned to this TD was already freed and cache was
> > - * already flushed. We don't have to flush again.
> > - */
> > - return tdx_reclaim_page(virt_to_page(private_spt));
> > + if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
> > + tdx_reclaim_page(virt_to_page(sp->external_spt)))
> > + sp->external_spt = NULL;
> > }
> >
> > static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> > @@ -3617,7 +3614,7 @@ void __init tdx_hardware_setup(void)
> > vt_x86_ops.vm_size = max_t(unsigned int, vt_x86_ops.vm_size, sizeof(struct kvm_tdx));
> >
> > vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
> > - vt_x86_ops.free_external_spt = tdx_sept_free_private_spt;
> > + vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
> > vt_x86_ops.remove_external_spte = tdx_sept_remove_private_spte;
> > vt_x86_ops.protected_apic_has_interrupt = tdx_protected_apic_has_interrupt;
> > }
> > --
> > 2.53.0.rc1.217.geba53bf80e-goog
> >
^ permalink raw reply
* Re: [RFC PATCH v5 16/45] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Yan Zhao @ 2026-02-05 6:11 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-17-seanjc@google.com>
> +void tdx_quirk_reset_page(struct page *page);
Looks this change is unnecessary.
> int tdx_guest_keyid_alloc(void);
> u32 tdx_get_nr_guest_keyids(void);
> void tdx_guest_keyid_free(unsigned int keyid);
>
> -void tdx_quirk_reset_page(struct page *page);
> +struct page *__tdx_alloc_control_page(gfp_t gfp);
> +void __tdx_free_control_page(struct page *page);
> +
> +static inline unsigned long tdx_alloc_control_page(gfp_t gfp)
> +{
> + struct page *page = __tdx_alloc_control_page(gfp);
> +
> + if (!page)
> + return 0;
> +
> + return (unsigned long)page_address(page);
> +}
> +
> +static inline void tdx_free_control_page(unsigned long addr)
> +{
> + if (!addr)
> + return;
> +
> + __tdx_free_control_page(virt_to_page(addr));
> +}
>
> struct tdx_td {
> /* TD root structure: */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index f6e80aba5895..682c8a228b53 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1824,6 +1824,50 @@ u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data)
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_mng_rd);
>
> +/* Number PAMT pages to be provided to TDX module per 2M region of PA */
> +static int tdx_dpamt_entry_pages(void)
> +{
> + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> + return 0;
> +
This function is not invoked when !tdx_supports_dynamic_pamt().
So, probably we can just return the count below?
> + return tdx_sysinfo.tdmr.pamt_4k_entry_size * PTRS_PER_PTE / PAGE_SIZE;
> +}
> +
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Yan Zhao @ 2026-02-05 5:39 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: <aYP_Ko3FGRriGXWR@google.com>
On Wed, Feb 04, 2026 at 06:23:38PM -0800, Sean Christopherson wrote:
> On Wed, Feb 04, 2026, Yan Zhao wrote:
> > On Wed, Jan 28, 2026 at 05:14:40PM -0800, Sean Christopherson wrote:
> > > @@ -590,10 +566,21 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> > > * the paging structure. Note the WARN on the PFN changing without the
> > > * SPTE being converted to a hugepage (leaf) or being zapped. Shadow
> > > * pages are kernel allocations and should never be migrated.
> > > + *
> > > + * When removing leaf entries from a mirror, immediately propagate the
> > > + * changes to the external page tables. Note, non-leaf mirror entries
> > > + * are handled by handle_removed_pt(), as TDX requires that all leaf
> > > + * entries are removed before the owning page table. Note #2, writes
> > > + * to make mirror PTEs shadow-present are propagated to external page
> > > + * tables by __tdp_mmu_set_spte_atomic(), as KVM needs to ensure the
> > > + * external page table was successfully updated before marking the
> > > + * mirror SPTE present.
> > > */
> > > if (was_present && !was_leaf &&
> > > (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> > > handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> > > + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
> > Should we check !is_present instead of !is_leaf?
> > e.g. a transition from a present leaf entry to a present non-leaf entry could
> > also trigger this if case.
>
> No, the !is_leaf check is very intentional. At this point in the series, S-EPT
> doesn't support hugepages. If KVM manages to install a leaf SPTE and replaces
> that SPTE with a non-leaf SPTE, then we absolutely want the KVM_BUG_ON() in
> tdx_sept_remove_private_spte() to fire:
>
> /* TODO: handle large pages. */
> if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> return -EIO;
But the op is named remove_external_spte().
And the check of "level != PG_LEVEL_4K" is for removing large leaf entries.
Relying on this check is tricky and confusing.
> And then later on, when S-EPT gains support for hugepages, "KVM: TDX: Add core
> support for splitting/demoting 2MiB S-EPT to 4KiB" doesn't need to touch code
> outside of arch/x86/kvm/vmx/tdx.c, because everything has already been plumbed
> in.
I haven't looked at the later patches for huge pages, but plumbing here directly
for splitting does not look right when it's invoked under shared mmu_lock.
See the comment below.
> > Besides, need "KVM_BUG_ON(shared, kvm)" in this case.
>
> Eh, we have lockdep_assert_held_write() in the S-EPT paths that require mmu_lock
> to be held for write. I don't think a KVM_BUG_ON() here would add meaningful
> value.
Hmm, I think KVM_BUG_ON(shared, kvm) is still useful.
If KVM invokes remove_external_spte() under shared mmu_lock, it needs to freeze
the entry first, similar to the sequence in __tdp_mmu_set_spte_atomic().
i.e., invoking external x86 ops in handle_changed_spte() for mirror roots should
be !shared only.
Relying on the TDX code's lockdep_assert_held_write() for warning seems less
clear than having an explicit check here.
^ permalink raw reply
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Pratik R. Sampat @ 2026-02-05 3:50 UTC (permalink / raw)
To: David Hildenbrand (arm), linux-mm, linux-coco, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <d07cbb96-ba30-4dcf-ad14-0479d6a989f2@kernel.org>
On 2/4/26 2:00 PM, David Hildenbrand (arm) wrote:
>> #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();
>>
>
> I really hate that accepting (and un-accepting) hotplugged memory is different to accepting ordinary boot memory.
>
> Is there really no way we can get a reasonable implementation where we just call a generic accept_memory() and it will know what to do?
>
Sure, that shouldn't be impossible.
The only reason I initially kept them separate is because we accept and update
the bitmap unconditionally. This mainly applies to cold-plugged memory since
their bitmap state after remove shouldn't matter. However, as we are now
correctly setting the bits in the hot-remove path we should be fine accepting
from the for_each_set_bitrange_from() logic within accept_memory(), I think.
Something like so?
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index d11e7836200a..e56adfd382f8 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -36,6 +36,7 @@ void accept_memory(phys_addr_t start, unsigned long size)
unsigned long range_start, range_end;
struct accept_range range, *entry;
phys_addr_t end = start + size;
+ phys_addr_t bitmap_end;
unsigned long flags;
u64 unit_size;
@@ -44,6 +45,21 @@ void accept_memory(phys_addr_t start, unsigned long size)
return;
unit_size = unaccepted->unit_size;
+ bitmap_end = unaccepted->phys_base + unaccepted->size * unit_size * BITS_PER_BYTE;
+
+ /* Memory completely beyond bitmap: hotplug memory, accept unconditionally */
+ if (start >= bitmap_end) {
+ arch_accept_memory(start, end);
+ return;
+ }
+
+ /* Memory partially beyond bitmap */
+ if (end > bitmap_end) {
+ arch_accept_memory(bitmap_end, end);
+ end = bitmap_end;
+ }
/*
* Only care for the part of the range that is represented
unaccept_hotplug_memory() truly doesn't do anything special for hotplug so I
could just re-name it unaccept_memory().
Thanks!
^ permalink raw reply related
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Pratik R. Sampat @ 2026-02-05 3:50 UTC (permalink / raw)
To: David Hildenbrand (arm), Kiryl Shutsemau
Cc: linux-mm, linux-coco, x86, linux-kernel, tglx, mingo, bp,
dave.hansen, ardb, akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <29b4dc97-bb01-42fd-8ccd-f4cb2886ccd3@kernel.org>
On 2/4/26 1:59 PM, David Hildenbrand (arm) wrote:
> On 2/4/26 12:22, Kiryl Shutsemau wrote:
>> On Tue, Feb 03, 2026 at 11:49:45AM -0600, Pratik R. Sampat wrote:
>>> Confidential computing guests require memory to be accepted before use.
>>> The unaccepted memory bitmap maintained by firmware does not track
>>> most hotplugged memory ranges apart from system memory annotated to be
>>> cold plugged at boot.
>>>
>>> Explicitly validate and transition the newly added memory to a private
>>> state, making it usable by the guest.
>>>
>>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>>> ---
>>> drivers/firmware/efi/unaccepted_memory.c | 47 ++++++++++++++++++++++++
>>> include/linux/mm.h | 5 +++
>>> mm/memory_hotplug.c | 2 +
>>> 3 files changed, 54 insertions(+)
>>>
>>> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
>>> index c2c067eff634..359779133cb4 100644
>>> --- a/drivers/firmware/efi/unaccepted_memory.c
>>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>>> @@ -209,6 +209,53 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
>>> return ret;
>>> }
>>> +/*
>>> + * Unaccepted memory bitmap only covers initial boot memory and not the
>>> + * hotpluggable range that is part of SRAT parsing. However, some initial memory
>>> + * with the attribute EFI_MEMORY_HOT_PLUGGABLE can indicate boot time memory
>>> + * that can be hot-removed. Hence post acceptance, only for that range update
>>> + * the unaccepted bitmap to reflect this change.
>>> + */
>>> +void accept_hotplug_memory(phys_addr_t start, unsigned long size)
>>> +{
>>> + struct efi_unaccepted_memory *unaccepted;
>>> + unsigned long range_start, range_len;
>>> + phys_addr_t end = start + size;
>>> + u64 phys_base, unit_size;
>>> + unsigned long flags;
>>> +
>>> + unaccepted = efi_get_unaccepted_table();
>>> + if (!unaccepted)
>>> + return;
>>
>> This can be tricky.
>>
>> If we boot a VM with <4GiB of memory and all of it is pre-accepted by
>> BIOS, the table will not be allocated.
>>
>> But it doesn't mean that hotplugged memory above should not be accepted.
>>
>> I don't think there is a way to detect such cases.
>>
>> Your check is probably the best we can do, but it means VMs are going to
>> crash if memory accept is required by no table.
>>
>> This is ugly situation.
Agreed. Breaking hotplug for VMs under 4G is absolutely not the way to go.
Would it be worse if we call arch_accept_memory() if the table doesn't exist?
The table is primarily to operate on the bitmap's entry. We could wrap these
accept calls within an arch check for TDX and SNP guest if the unaccepted table
is NULL. Or, less preferably convert the panic() of the existing
arch_[accept/unaccept]_memory() to a WARN() instead.
>
> It's all starting to feel .... very hacky, sorry to say.
>
> This should all be easier. If we expect memory hotplug (SRAT), why can't we just allocate the bitmap properly?
>
The unaccepted bitmap allocation happens a lot earlier than SRAT parsing. So to
get the right range, either we have to duplicate some of that parsing logic
earlier, or, replace the memblock allocated bitmap later. The first one is a
bit more hacky, but the second one would require us to the change the original
unaccepted struct from a flexible array to a pointer which might break kexec.
Neither of the approaches seem less intrusive than the other unfortunately.
--Pratik
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Sean Christopherson @ 2026-02-05 2:23 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: <aYMMHVvwDjZ7Lz9l@yzhao56-desk.sh.intel.com>
On Wed, Feb 04, 2026, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:14:40PM -0800, Sean Christopherson wrote:
> > @@ -590,10 +566,21 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> > * the paging structure. Note the WARN on the PFN changing without the
> > * SPTE being converted to a hugepage (leaf) or being zapped. Shadow
> > * pages are kernel allocations and should never be migrated.
> > + *
> > + * When removing leaf entries from a mirror, immediately propagate the
> > + * changes to the external page tables. Note, non-leaf mirror entries
> > + * are handled by handle_removed_pt(), as TDX requires that all leaf
> > + * entries are removed before the owning page table. Note #2, writes
> > + * to make mirror PTEs shadow-present are propagated to external page
> > + * tables by __tdp_mmu_set_spte_atomic(), as KVM needs to ensure the
> > + * external page table was successfully updated before marking the
> > + * mirror SPTE present.
> > */
> > if (was_present && !was_leaf &&
> > (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> > handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> > + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
> Should we check !is_present instead of !is_leaf?
> e.g. a transition from a present leaf entry to a present non-leaf entry could
> also trigger this if case.
No, the !is_leaf check is very intentional. At this point in the series, S-EPT
doesn't support hugepages. If KVM manages to install a leaf SPTE and replaces
that SPTE with a non-leaf SPTE, then we absolutely want the KVM_BUG_ON() in
tdx_sept_remove_private_spte() to fire:
/* TODO: handle large pages. */
if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
return -EIO;
And then later on, when S-EPT gains support for hugepages, "KVM: TDX: Add core
support for splitting/demoting 2MiB S-EPT to 4KiB" doesn't need to touch code
outside of arch/x86/kvm/vmx/tdx.c, because everything has already been plumbed
in.
> Besides, need "KVM_BUG_ON(shared, kvm)" in this case.
Eh, we have lockdep_assert_held_write() in the S-EPT paths that require mmu_lock
to be held for write. I don't think a KVM_BUG_ON() here would add meaningful
value.
^ permalink raw reply
* Re: [GIT PULL] PCIe Link Encryption fixes for 6.19
From: pr-tracker-bot @ 2026-02-04 23:25 UTC (permalink / raw)
To: dan.j.williams
Cc: Linus Torvalds, linux-coco, linux-pci, linux-kernel,
Alexey Kardashevskiy, Xu Yilun, Aneesh Kumar K.V (Arm)
In-Reply-To: <6983bd3a3a3ae_68d310016@dwillia2-mobl4.notmuch>
The pull request you sent on Wed, 4 Feb 2026 13:42:18 -0800:
> git://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm tags/tsm-fixes-for-6.19
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/f14faaf3a1fb3b9e4cf2e56269711fb85fba9458
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: SVSM Development Call February 4, 2026
From: Jörg Rödel @ 2026-02-04 21:50 UTC (permalink / raw)
To: coconut-svsm, linux-coco
In-Reply-To: <lcirfa4dfkyv2522vuwr4dth2lbzi2sunhcubojuarg6v2myfx@bf33fftemnmj>
Meeting minutes are ready:
https://github.com/coconut-svsm/governance/pull/95
-Jörg
^ permalink raw reply
* [GIT PULL] PCIe Link Encryption fixes for 6.19
From: dan.j.williams @ 2026-02-04 21:42 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-coco, linux-pci, linux-kernel, Alexey Kardashevskiy,
Xu Yilun, Aneesh Kumar K.V (Arm)
Hi Linus, please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm tags/tsm-fixes-for-6.19
...to receive a small collection of fixes for 6.19. The largest change
is reverting part of an ABI that never shipped in a released kernel
(Documentation/ABI/testing/sysfs-class-tsm). The fix / replacement for
that is too large to squeeze in at this late date. The rest is a
collection of small fixups summarized in the tag message.
It has appeared in linux-next with no reports at last check.
Given the tsm.git tree merged the PCIe Link Encryption support for the
AMD "ccp" driver, Alexey asked that the fixes also go through the
tsm.git with an ack from Tom. Bjorn acked taking ide.c fixes through
tsm.git as well.
---
The following changes since commit 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7:
Linux 6.19-rc6 (2026-01-18 15:42:45 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm tags/tsm-fixes-for-6.19
for you to fetch changes up to c2012263047689e495e81c96d7d5b0586299578d:
crypto/ccp: Allow multiple streams on the same root bridge (2026-01-30 14:27:53 -0800)
----------------------------------------------------------------
tsm fixes for 6.19
- Fix multiple streams per host bridge for SEV-TIO
- Drop the TSM ABI for reporting IDE streams (to be replaced)
- Fix virtual function enumeration
- Fix reserved stream ID initialization
- Fix unused variable compiler warning
----------------------------------------------------------------
Alexey Kardashevskiy (2):
crypto/ccp: Use PCI bridge defaults for IDE
crypto/ccp: Allow multiple streams on the same root bridge
Dan Williams (1):
Revert "PCI/TSM: Report active IDE streams"
Li Ming (2):
PCI/IDE: Fix off by one error calculating VF RID range
PCI/IDE: Fix reading a wrong reg for unused sel stream initialization
Thomas Weißschuh (1):
coco/tsm: Remove unused variable tsm_rwsem
Documentation/ABI/testing/sysfs-class-tsm | 10 ----------
drivers/crypto/ccp/sev-dev-tsm.c | 15 +--------------
drivers/pci/ide.c | 10 +++-------
drivers/virt/coco/tsm-core.c | 30 ------------------------------
include/linux/pci-ide.h | 4 +---
include/linux/tsm.h | 3 ---
6 files changed, 5 insertions(+), 67 deletions(-)
^ permalink raw reply
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: David Hildenbrand (arm) @ 2026-02-04 20:00 UTC (permalink / raw)
To: Pratik R. Sampat, linux-mm, linux-coco, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <20260203174946.1198053-2-prsampat@amd.com>
> #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();
>
I really hate that accepting (and un-accepting) hotplugged memory is
different to accepting ordinary boot memory.
Is there really no way we can get a reasonable implementation where we
just call a generic accept_memory() and it will know what to do?
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: David Hildenbrand (arm) @ 2026-02-04 19:59 UTC (permalink / raw)
To: Kiryl Shutsemau, Pratik R. Sampat
Cc: linux-mm, linux-coco, x86, linux-kernel, tglx, mingo, bp,
dave.hansen, ardb, akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <aYMjjyVVax5cml9B@thinkstation>
On 2/4/26 12:22, Kiryl Shutsemau wrote:
> On Tue, Feb 03, 2026 at 11:49:45AM -0600, Pratik R. Sampat wrote:
>> Confidential computing guests require memory to be accepted before use.
>> The unaccepted memory bitmap maintained by firmware does not track
>> most hotplugged memory ranges apart from system memory annotated to be
>> cold plugged at boot.
>>
>> Explicitly validate and transition the newly added memory to a private
>> state, making it usable by the guest.
>>
>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>> ---
>> drivers/firmware/efi/unaccepted_memory.c | 47 ++++++++++++++++++++++++
>> include/linux/mm.h | 5 +++
>> mm/memory_hotplug.c | 2 +
>> 3 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
>> index c2c067eff634..359779133cb4 100644
>> --- a/drivers/firmware/efi/unaccepted_memory.c
>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>> @@ -209,6 +209,53 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
>> return ret;
>> }
>>
>> +/*
>> + * Unaccepted memory bitmap only covers initial boot memory and not the
>> + * hotpluggable range that is part of SRAT parsing. However, some initial memory
>> + * with the attribute EFI_MEMORY_HOT_PLUGGABLE can indicate boot time memory
>> + * that can be hot-removed. Hence post acceptance, only for that range update
>> + * the unaccepted bitmap to reflect this change.
>> + */
>> +void accept_hotplug_memory(phys_addr_t start, unsigned long size)
>> +{
>> + struct efi_unaccepted_memory *unaccepted;
>> + unsigned long range_start, range_len;
>> + phys_addr_t end = start + size;
>> + u64 phys_base, unit_size;
>> + unsigned long flags;
>> +
>> + unaccepted = efi_get_unaccepted_table();
>> + if (!unaccepted)
>> + return;
>
> This can be tricky.
>
> If we boot a VM with <4GiB of memory and all of it is pre-accepted by
> BIOS, the table will not be allocated.
>
> But it doesn't mean that hotplugged memory above should not be accepted.
>
> I don't think there is a way to detect such cases.
>
> Your check is probably the best we can do, but it means VMs are going to
> crash if memory accept is required by no table.
>
> This is ugly situation.
It's all starting to feel .... very hacky, sorry to say.
This should all be easier. If we expect memory hotplug (SRAT), why can't
we just allocate the bitmap properly?
--
Cheers,
David
^ permalink raw reply
* Re: [RFC PATCH v5 00/45] TDX: Dynamic PAMT + S-EPT Hugepage
From: Dave Hansen @ 2026-02-04 15:09 UTC (permalink / raw)
To: Sean Christopherson, Konrad Rzeszutek Wilk
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Yan Zhao, Vishal Annapurve,
Ackerley Tng, Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYNaA7Td23xKHoHK@google.com>
On 2/4/26 06:38, Sean Christopherson wrote:
...
> We can and do have tests and VMM support, but it's all out-of-tree (for now).
> All I'm saying here is that I'm ok landing the S-EPT hugepage code in advance of
> guest_memfd hugepage support, e.g. so that we don't end up in a stalemate due to
> cyclical dependecies, or one big megaseries.
Does "landing" mean having it sit in some topic branch, or pushing to Linus?
I'm all for getting these hellish dependency chains out of the way, but
we usually try pretty hard to avoid having dead/unreachable code in
mainline.
If it is something you want to do in mainline, we should probably do a
bit of cross-x86/kvm brainstorming to make sure there's no other option.
^ permalink raw reply
* Re: [RFC PATCH v5 00/45] TDX: Dynamic PAMT + S-EPT Hugepage
From: Sean Christopherson @ 2026-02-04 14:38 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Yan Zhao, Vishal Annapurve,
Ackerley Tng, Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aXuVR0kq_K1TYwlR@char.us.oracle.com>
On Thu, Jan 29, 2026, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 28, 2026 at 05:14:32PM -0800, Sean Christopherson wrote:
> > This is a combined series of Dynamic PAMT (from Rick), and S-EPT hugepage
> > support (from Yan). Except for some last minute tweaks to the DPAMT array
> > args stuff, a version of this based on a Google-internal kernel has been
> > moderately well tested (thanks Vishal!). But overall it's still firmly RFC
> > as I have deliberately NOT addressed others feedback from v4 of DPAMT and v3
>
> What does PAMT stand for? Is there a design document somewhere?
>
> > of S-EPT hugepage (mostly lack of cycles), and there's at least one patch in
> > here that shouldn't be merged as-is (the quick-and-dirty switch from struct
> > page to raw pfns).
> >
> > My immediate goal is to solidify the designs for DPAMT and S-EPT hugepage.
> > Given the substantial design changes I am proposing, posting an end-to-end
> > RFC seemed like a much better method than trying to communicate my thoughts
> > piecemeal.
> >
> > As for landing these series, I think the fastest overall approach would be
> > to land patches 1-4 asap (tangentially related cleanups and fixes), agree
>
> Should they be split out as non-RFC then?
Yeah, I'll do that soonish. I posted the kitchen sink so that people could
review the entire thing without having to chase down 4+ series/patches.
> > on a design (hopefully), and then hand control back to Rick and Yan to polish
> > their respective series for merge.
> >
> > I also want to land the VMXON series[*] before DPAMT, because there's a nasty
> > wart where KVM wires up a DPAMT-specific hook even if DPAMT is disabled,
> > because KVM's ordering needs to set the vendor hooks before tdx_sysinfo is
> > ready. Decoupling VMXON from KVM solves that problem, because it lets the
> > TDX subsystem parse sysinfo before TDX is loaded.
> >
> > Beyond that dependency, I am comfortable landing both DPAMT and S-EPT hugepage
> > support without any other prereqs, i.e. without an in-tree way to light up
> > the S-EPT hugepage code due to lack of hugepage support in guest_memfd.
>
> Can there be test-cases? Or simple code posted for QEMU which is the
> tool that 99% of kernel engineers use?
No? The core limitation is that KVM doesn't yet support hugepages for private
memory. No amount userspace code can overcome that limitation.
We can and do have tests and VMM support, but it's all out-of-tree (for now).
All I'm saying here is that I'm ok landing the S-EPT hugepage code in advance of
guest_memfd hugepage support, e.g. so that we don't end up in a stalemate due to
cyclical dependecies, or one big megaseries.
^ permalink raw reply
* Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add
From: Kiryl Shutsemau @ 2026-02-04 11:22 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, x86, linux-kernel, tglx, mingo, bp,
dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <20260203174946.1198053-2-prsampat@amd.com>
On Tue, Feb 03, 2026 at 11:49:45AM -0600, Pratik R. Sampat wrote:
> Confidential computing guests require memory to be accepted before use.
> The unaccepted memory bitmap maintained by firmware does not track
> most hotplugged memory ranges apart from system memory annotated to be
> cold plugged at boot.
>
> Explicitly validate and transition the newly added memory to a private
> state, making it usable by the guest.
>
> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
> ---
> drivers/firmware/efi/unaccepted_memory.c | 47 ++++++++++++++++++++++++
> include/linux/mm.h | 5 +++
> mm/memory_hotplug.c | 2 +
> 3 files changed, 54 insertions(+)
>
> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
> index c2c067eff634..359779133cb4 100644
> --- a/drivers/firmware/efi/unaccepted_memory.c
> +++ b/drivers/firmware/efi/unaccepted_memory.c
> @@ -209,6 +209,53 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
> return ret;
> }
>
> +/*
> + * Unaccepted memory bitmap only covers initial boot memory and not the
> + * hotpluggable range that is part of SRAT parsing. However, some initial memory
> + * with the attribute EFI_MEMORY_HOT_PLUGGABLE can indicate boot time memory
> + * that can be hot-removed. Hence post acceptance, only for that range update
> + * the unaccepted bitmap to reflect this change.
> + */
> +void accept_hotplug_memory(phys_addr_t start, unsigned long size)
> +{
> + struct efi_unaccepted_memory *unaccepted;
> + unsigned long range_start, range_len;
> + phys_addr_t end = start + size;
> + u64 phys_base, unit_size;
> + unsigned long flags;
> +
> + unaccepted = efi_get_unaccepted_table();
> + if (!unaccepted)
> + return;
This can be tricky.
If we boot a VM with <4GiB of memory and all of it is pre-accepted by
BIOS, the table will not be allocated.
But it doesn't mean that hotplugged memory above should not be accepted.
I don't think there is a way to detect such cases.
Your check is probably the best we can do, but it means VMs are going to
crash if memory accept is required by no table.
This is ugly situation.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* [Invitation] bi-weekly guest_memfd upstream call on 2026-02-05
From: David Hildenbrand (arm) @ 2026-02-04 11:17 UTC (permalink / raw)
To: linux-coco@lists.linux.dev, linux-mm@kvack.org, KVM
Hi,
Our next guest_memfd upstream call is scheduled for Thursday,
2026-02-07 at 8:00 - 9:00am (GMT-08:00) Pacific Time - Vancouver.
We'll be using the following Google meet:
http://meet.google.com/wxp-wtju-jzw
The meeting notes can be found at [1], where we also link recordings and
collect current guest_memfd upstream proposals. If you want an google
calendar invitation that also covers all future meetings, just write me
or Ackerley a mail.
I want to briefly talk about the pfncache[2] patches and how
synchronization with truncation etc. is supposed to work; then well see
which other topics pop up. :)
To put something to discuss onto the agenda, reply to this mail or add
them to the "Topics/questions for next meeting(s)" section in the
meeting notes as a comment.
[1]
https://docs.google.com/document/d/1M6766BzdY1Lhk7LiR5IqVR8B8mG3cr-cxTxOrAosPOk/edit?usp=sharing
[2] https://lore.kernel.org/all/20251203144159.6131-1-itazur@amazon.com/
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v3 23/26] x86/virt/tdx: Enable TDX Module runtime updates
From: Tony Lindgren @ 2026-02-04 10:03 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin
In-Reply-To: <aXdMfzIxF6KR7VCe@tlindgre-MOBL1>
On Mon, Jan 26, 2026 at 01:14:07PM +0200, Tony Lindgren wrote:
> On Fri, Jan 23, 2026 at 06:55:31AM -0800, Chao Gao wrote:
> > --- a/arch/x86/include/asm/tdx.h
> > +++ b/arch/x86/include/asm/tdx.h
> > @@ -32,6 +32,9 @@
> > #define TDX_SUCCESS 0ULL
> > #define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
> >
> > +/* Bit definitions of TDX_FEATURES0 metadata field */
> > +#define TDX_FEATURES0_TD_PRESERVING BIT(1)
> > +#define TDX_FEATURES0_NO_RBP_MOD BIT(18)
> > #ifndef __ASSEMBLER__
> >
> > #include <uapi/asm/mce.h>
>
> How about let's put these defines into arch/x86/include/asm/shared/tdx.h
> instead? And use BIT_ULL?
Sorry I was confused. No need to move these defines to
arch/x86/include/asm/shared/tdx.h as far as I can tell.
The BIT_ULL comment still remains though.
Regards,
Tony
^ permalink raw reply
* Re: [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Yan Zhao @ 2026-02-04 9:45 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-10-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:41PM -0800, Sean Christopherson wrote:
> Massage .free_external_spt() into .reclaim_external_sp() to free up (pun
> intended) "free" for actually freeing memory, and to allow TDX to do more
> than just "free" the S-EPT entry. Specifically, nullify external_spt to
> leak the S-EPT page if reclaiming the page fails, as that detail and
> implementation choice has no business living in the TDP MMU.
>
> Use "sp" instead of "spt" even though "spt" is arguably more accurate, as
> "spte" and "spt" are dangerously close in name, and because the key
> parameter is a kvm_mmu_page, not a pointer to an S-EPT page table.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 2 +-
> arch/x86/include/asm/kvm_host.h | 4 ++--
> arch/x86/kvm/mmu/tdp_mmu.c | 13 ++-----------
> arch/x86/kvm/vmx/tdx.c | 27 ++++++++++++---------------
> 4 files changed, 17 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index 57eb1f4832ae..c17cedc485c9 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -95,8 +95,8 @@ KVM_X86_OP_OPTIONAL_RET0(set_identity_map_addr)
> KVM_X86_OP_OPTIONAL_RET0(get_mt_mask)
> KVM_X86_OP(load_mmu_pgd)
> KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
> -KVM_X86_OP_OPTIONAL_RET0(free_external_spt)
> KVM_X86_OP_OPTIONAL(remove_external_spte)
> +KVM_X86_OP_OPTIONAL(reclaim_external_sp)
> KVM_X86_OP(has_wbinvd_exit)
> KVM_X86_OP(get_l2_tsc_offset)
> KVM_X86_OP(get_l2_tsc_multiplier)
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index d12ca0f8a348..b35a07ed11fb 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1858,8 +1858,8 @@ struct kvm_x86_ops {
> u64 mirror_spte);
>
> /* Update external page tables for page table about to be freed. */
> - int (*free_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> - void *external_spt);
> + void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> + struct kvm_mmu_page *sp);
Do you think "free" is still better than "reclaim" though TDX actually
invokes tdx_reclaim_page() to reclaim it on the TDX side?
Naming it free_external_sp can be interpreted as freeing the sp->external_spt
externally (vs freeing it in tdp_mmu_free_sp_rcu_callback(). This naming also
allows for the future possibility of freeing sp->external_spt before the HKID is
freed (though this is unlikely).
> /* Update external page table from spte getting removed, and flush TLB. */
> void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 27ac520f2a89..18764dbc97ea 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -456,17 +456,8 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> old_spte, FROZEN_SPTE, level, shared);
> }
>
> - if (is_mirror_sp(sp) &&
> - WARN_ON(kvm_x86_call(free_external_spt)(kvm, base_gfn, sp->role.level,
> - sp->external_spt))) {
> - /*
> - * Failed to free page table page in mirror page table and
> - * there is nothing to do further.
> - * Intentionally leak the page to prevent the kernel from
> - * accessing the encrypted page.
> - */
> - sp->external_spt = NULL;
> - }
> + if (is_mirror_sp(sp))
> + kvm_x86_call(reclaim_external_sp)(kvm, base_gfn, sp);
>
> call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
> }
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 30494f9ceb31..66bc3ceb5e17 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1783,27 +1783,24 @@ static void tdx_track(struct kvm *kvm)
> kvm_make_all_cpus_request(kvm, KVM_REQ_OUTSIDE_GUEST_MODE);
> }
>
> -static int tdx_sept_free_private_spt(struct kvm *kvm, gfn_t gfn,
> - enum pg_level level, void *private_spt)
> +static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
> + struct kvm_mmu_page *sp)
Passing in "sp" and having "reclaim_private_sp" in the function name is bit
confusing.
Strictly speaking, only sp->external_spt is private, while the sp and sp->spt
are just mirroring the external spt.
But I understand it's for setting sp->external_spt to NULL on error.
> {
> - struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> -
> /*
> - * free_external_spt() is only called after hkid is freed when TD is
> - * tearing down.
> * KVM doesn't (yet) zap page table pages in mirror page table while
> * TD is active, though guest pages mapped in mirror page table could be
> * zapped during TD is active, e.g. for shared <-> private conversion
> * and slot move/deletion.
> + *
> + * In other words, KVM should only free mirror page tables after the
> + * TD's hkid is freed, when the TD is being torn down.
> + *
> + * If the S-EPT PTE can't be removed for any reason, intentionally leak
> + * the page to prevent the kernel from accessing the encrypted page.
> */
> - if (KVM_BUG_ON(is_hkid_assigned(kvm_tdx), kvm))
> - return -EIO;
> -
> - /*
> - * The HKID assigned to this TD was already freed and cache was
> - * already flushed. We don't have to flush again.
> - */
> - return tdx_reclaim_page(virt_to_page(private_spt));
> + if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
> + tdx_reclaim_page(virt_to_page(sp->external_spt)))
> + sp->external_spt = NULL;
> }
>
> static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> @@ -3617,7 +3614,7 @@ void __init tdx_hardware_setup(void)
> vt_x86_ops.vm_size = max_t(unsigned int, vt_x86_ops.vm_size, sizeof(struct kvm_tdx));
>
> vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
> - vt_x86_ops.free_external_spt = tdx_sept_free_private_spt;
> + vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
> vt_x86_ops.remove_external_spte = tdx_sept_remove_private_spte;
> vt_x86_ops.protected_apic_has_interrupt = tdx_protected_apic_has_interrupt;
> }
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Yan Zhao @ 2026-02-04 9:06 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-9-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:40PM -0800, Sean Christopherson wrote:
> Invoke .remove_external_spte() in handle_changed_spte() as appropriate
> instead of relying on callers to do the right thing. Relying on callers
> to invoke .remove_external_spte() is confusing and brittle, e.g. subtly
> relies tdp_mmu_set_spte_atomic() never removing SPTEs, and removing an
> S-EPT entry in tdp_mmu_set_spte() is bizarre (yeah, the VM is bugged so
> it doesn't matter in practice, but it's still weird).
>
> Implementing rules-based logic in a common chokepoint will also make it
> easier to reason about the correctness of splitting hugepages when support
> for S-EPT hugepages comes along.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 43 +++++++++++++-------------------------
> 1 file changed, 14 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 8743cd020d12..27ac520f2a89 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -359,25 +359,6 @@ static void tdp_mmu_unlink_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
> spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
> }
>
> -static void remove_external_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
> - int level)
> -{
> - /*
> - * External (TDX) SPTEs are limited to PG_LEVEL_4K, and external
> - * PTs are removed in a special order, involving free_external_spt().
> - * But remove_external_spte() will be called on non-leaf PTEs via
> - * __tdp_mmu_zap_root(), so avoid the error the former would return
> - * in this case.
> - */
> - if (!is_last_spte(old_spte, level))
> - return;
> -
> - /* Zapping leaf spte is allowed only when write lock is held. */
> - lockdep_assert_held_write(&kvm->mmu_lock);
> -
> - kvm_x86_call(remove_external_spte)(kvm, gfn, level, old_spte);
> -}
> -
> /**
> * handle_removed_pt() - handle a page table removed from the TDP structure
> *
> @@ -473,11 +454,6 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> }
> handle_changed_spte(kvm, kvm_mmu_page_as_id(sp), sptep, gfn,
> old_spte, FROZEN_SPTE, level, shared);
> -
> - if (is_mirror_sp(sp)) {
> - KVM_BUG_ON(shared, kvm);
> - remove_external_spte(kvm, gfn, old_spte, level);
> - }
> }
>
> if (is_mirror_sp(sp) &&
> @@ -590,10 +566,21 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> * the paging structure. Note the WARN on the PFN changing without the
> * SPTE being converted to a hugepage (leaf) or being zapped. Shadow
> * pages are kernel allocations and should never be migrated.
> + *
> + * When removing leaf entries from a mirror, immediately propagate the
> + * changes to the external page tables. Note, non-leaf mirror entries
> + * are handled by handle_removed_pt(), as TDX requires that all leaf
> + * entries are removed before the owning page table. Note #2, writes
> + * to make mirror PTEs shadow-present are propagated to external page
> + * tables by __tdp_mmu_set_spte_atomic(), as KVM needs to ensure the
> + * external page table was successfully updated before marking the
> + * mirror SPTE present.
> */
> if (was_present && !was_leaf &&
> (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
Should we check !is_present instead of !is_leaf?
e.g. a transition from a present leaf entry to a present non-leaf entry could
also trigger this if case.
Besides, need "KVM_BUG_ON(shared, kvm)" in this case.
> + kvm_x86_call(remove_external_spte)(kvm, gfn, level, old_spte);
> }
>
> static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> @@ -725,12 +712,10 @@ static u64 tdp_mmu_set_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
>
> /*
> * Users that do non-atomic setting of PTEs don't operate on mirror
> - * roots, so don't handle it and bug the VM if it's seen.
> + * roots. Bug the VM as this path doesn't propagate such writes to the
> + * external page tables.
> */
> - if (is_mirror_sptep(sptep)) {
> - KVM_BUG_ON(is_shadow_present_pte(new_spte), kvm);
> - remove_external_spte(kvm, gfn, old_spte, level);
> - }
> + KVM_BUG_ON(is_mirror_sptep(sptep) && is_shadow_present_pte(new_spte), kvm);
>
> return old_spte;
> }
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ permalink raw reply
* Re: [RFC PATCH v5 06/45] KVM: x86/mmu: Fold set_external_spte_present() into its sole caller
From: Yan Zhao @ 2026-02-04 7:38 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-7-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:38PM -0800, Sean Christopherson wrote:
> Fold set_external_spte_present() into __tdp_mmu_set_spte_atomic() in
> anticipation of supporting hugepage splitting, at which point other paths
> will also set shadow-present external SPTEs.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 82 +++++++++++++++++---------------------
> 1 file changed, 36 insertions(+), 46 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 56ad056e6042..6fb48b217f5b 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -495,33 +495,6 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
> }
>
> -static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
> - gfn_t gfn, u64 *old_spte,
> - u64 new_spte, int level)
> -{
> - int ret;
> -
> - lockdep_assert_held(&kvm->mmu_lock);
> -
> - if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> - return -EIO;
> -
> - /*
> - * We need to lock out other updates to the SPTE until the external
> - * page table has been modified. Use FROZEN_SPTE similar to
> - * the zapping case.
> - */
> - if (!try_cmpxchg64(rcu_dereference(sptep), old_spte, FROZEN_SPTE))
> - return -EBUSY;
> -
> - ret = kvm_x86_call(set_external_spte)(kvm, gfn, level, new_spte);
> - if (ret)
> - __kvm_tdp_mmu_write_spte(sptep, *old_spte);
> - else
> - __kvm_tdp_mmu_write_spte(sptep, new_spte);
> - return ret;
> -}
> -
> /**
> * handle_changed_spte - handle bookkeeping associated with an SPTE change
> * @kvm: kvm instance
> @@ -626,6 +599,8 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> struct tdp_iter *iter,
> u64 new_spte)
> {
> + u64 *raw_sptep = rcu_dereference(iter->sptep);
> +
> /*
> * The caller is responsible for ensuring the old SPTE is not a FROZEN
> * SPTE. KVM should never attempt to zap or manipulate a FROZEN SPTE,
> @@ -638,31 +613,46 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> int ret;
>
> /*
> - * Users of atomic zapping don't operate on mirror roots,
> - * so don't handle it and bug the VM if it's seen.
> + * KVM doesn't currently support zapping or splitting mirror
> + * SPTEs while holding mmu_lock for read.
> */
> - if (KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> + if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
> + KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> return -EBUSY;
Should this be -EIO instead?
Though -EBUSY was introduced by commit 94faba8999b9 ('KVM: x86/tdp_mmu:
Propagate tearing down mirror page tables')
> - ret = set_external_spte_present(kvm, iter->sptep, iter->gfn,
> - &iter->old_spte, new_spte, iter->level);
Add "lockdep_assert_held(&kvm->mmu_lock)" for this case?
> + /*
> + * Temporarily freeze the SPTE until the external PTE operation
> + * has completed, e.g. so that concurrent faults don't attempt
> + * to install a child PTE in the external page table before the
> + * parent PTE has been written.
> + */
> + if (!try_cmpxchg64(raw_sptep, &iter->old_spte, FROZEN_SPTE))
> + return -EBUSY;
> +
> + /*
> + * Update the external PTE. On success, set the mirror SPTE to
> + * the desired value. On failure, restore the old SPTE so that
> + * the SPTE isn't frozen in perpetuity.
> + */
> + ret = kvm_x86_call(set_external_spte)(kvm, iter->gfn,
> + iter->level, new_spte);
> if (ret)
> - return ret;
> - } else {
> - u64 *sptep = rcu_dereference(iter->sptep);
> -
> - /*
> - * Note, fast_pf_fix_direct_spte() can also modify TDP MMU SPTEs
> - * and does not hold the mmu_lock. On failure, i.e. if a
> - * different logical CPU modified the SPTE, try_cmpxchg64()
> - * updates iter->old_spte with the current value, so the caller
> - * operates on fresh data, e.g. if it retries
> - * tdp_mmu_set_spte_atomic()
> - */
> - if (!try_cmpxchg64(sptep, &iter->old_spte, new_spte))
> - return -EBUSY;
> + __kvm_tdp_mmu_write_spte(iter->sptep, iter->old_spte);
> + else
> + __kvm_tdp_mmu_write_spte(iter->sptep, new_spte);
> + return ret;
> }
>
> + /*
> + * Note, fast_pf_fix_direct_spte() can also modify TDP MMU SPTEs and
> + * does not hold the mmu_lock. On failure, i.e. if a different logical
> + * CPU modified the SPTE, try_cmpxchg64() updates iter->old_spte with
> + * the current value, so the caller operates on fresh data, e.g. if it
> + * retries tdp_mmu_set_spte_atomic()
> + */
> + if (!try_cmpxchg64(raw_sptep, &iter->old_spte, new_spte))
> + return -EBUSY;
> +
> return 0;
> }
>
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Huang, Kai @ 2026-02-04 6:45 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org,
binbin.wu@linux.intel.com, pbonzini@redhat.com, mingo@redhat.com,
Yamahata, Isaku, ackerleytng@google.com, tglx@kernel.org,
sagis@google.com, Edgecombe, Rick P, bp@alien8.de,
Annapurve, Vishal, x86@kernel.org
In-Reply-To: <aYKr7XODY-p6YLYa@google.com>
On Tue, 2026-02-03 at 18:16 -0800, Sean Christopherson wrote:
> On Tue, Feb 03, 2026, Kai Huang wrote:
> > On Tue, 2026-02-03 at 12:12 -0800, Sean Christopherson wrote:
> > > On Tue, Feb 03, 2026, Kai Huang wrote:
> > > > On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > > > > Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> > > > > so that x86's TDX can update per-page metadata on allocation and free().
> > > > >
> > > > > Name the allocator page_get() to align with __get_free_page(), e.g. to
> > > > > communicate that it returns an "unsigned long", not a "struct page", and
> > > > > to avoid collisions with macros, e.g. with alloc_page.
> > > > >
> > > > > Suggested-by: Kai Huang <kai.huang@intel.com>
> > > > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > >
> > > > I thought it could be more generic for allocating an object, but not just a
> > > > page.
> > > >
> > > > E.g., I thought we might be able to use it to allocate a structure which has
> > > > "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> > > > it seems you abandoned this idea. May I ask why? Just want to understand
> > > > the reasoning here.
> > >
> > > Because that requires more complexity and there's no known use case, and I don't
> > > see an obvious way for a use case to come along. All of the motiviations for a
> > > custom allocation scheme that I can think of apply only to full pages, or fit
> > > nicely in a kmem_cache.
> > >
> > > Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
> > > "page" usage. Further splitting the "page" case doesn't require modifications to
> > > the "kmem_cache" case, whereas providing a fully generic solution would require
> > > additional changes, e.g. to handle this code:
> > >
> > > page = (void *)__get_free_page(gfp_flags);
> > > if (page && mc->init_value)
> > > memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
> > >
> > > It certainly wouldn't be much complexity, but this code is already a bit awkward,
> > > so I don't think it makes sense to add support for something that will probably
> > > never be used.
> >
> > For this particular piece of code, we can add a helper for allocating normal
> > page table pages, get rid of mc->init_value completely and hook mc-page_get()
> > to that helper.
>
> Hmm, I like the idea, but I don't think it would be a net positive. In practice,
> x86's "normal" page tables stop being normal, because KVM now initializes all
> SPTEs with BIT(63)=1 on x86-64. And that would also incur an extra RETPOLINE on
> all those allocations.
No argument on this. People hate indirect calls I guess. :-)
>
> > A bonus is we can then call that helper in all places when KVM needs to
> > allocate a page for normal page table instead of just calling
> > get_zerod_pages() directly, e.g., like the one in
> > tdp_mmu_alloc_sp_for_split(),
>
> Huh. Actually, that's a bug, but not the one you probably expect. At a glance,
> it looks like KVM incorrectly zeroing the page instead of initializing it with
> SHADOW_NONPRESENT_VALUE. But it's actually a "performance" bug, because KVM
> doesn't actually need to pre-initialize the page: either the page will never be
> used, or every SPTE will be initialized as a child SPTE.
>
> So that one _should_ be different, e.g. should be:
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index a32192c35099..36afd67601fc 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1456,7 +1456,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
> if (!sp)
> return NULL;
>
> - sp->spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> + sp->spt = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
> if (!sp->spt)
> goto err_spt;
>
If we look from "performance" perspective, yeah indeed, albeit we probably
not gonna see any performance difference.
But no more arguments. I just think it will be less error-prone if we have
a consistent way for allocating the same object (no matter what it is), but
it's just a theoretical thing.
^ permalink raw reply
* Re: [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Yan Zhao @ 2026-02-04 6:41 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYJU8Som706YkIEO@google.com>
On Tue, Feb 03, 2026 at 08:05:05PM +0000, Sean Christopherson wrote:
> On Tue, Feb 03, 2026, Yan Zhao wrote:
> > On Wed, Jan 28, 2026 at 05:14:37PM -0800, Sean Christopherson wrote:
> > > static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
> > > gfn_t gfn, u64 *old_spte,
> > > u64 new_spte, int level)
> > > {
> > > - bool was_present = is_shadow_present_pte(*old_spte);
> > > - bool is_present = is_shadow_present_pte(new_spte);
> > > - bool is_leaf = is_present && is_last_spte(new_spte, level);
> > > - int ret = 0;
> > > -
> > > - KVM_BUG_ON(was_present, kvm);
> > > + int ret;
> > >
> > > lockdep_assert_held(&kvm->mmu_lock);
> > > +
> > > + if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> > > + return -EIO;
> > Why not move this check of is_shadow_present_pte() to tdx_sept_set_private_spte()
> > as well?
>
> The series gets there eventually, but as of this commit, @old_spte isn't plumbed
> into tdx_sept_set_private_spte().
>
> > Or also check !is_shadow_present_pte(new_spte) in TDP MMU?
>
> Not sure I understand this suggestion.
Sorry. The accurate expression should be
"what about moving !is_shadow_present_pte(new_spte) to TDP MMU as well?".
>
> > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > > index 5688c77616e3..30494f9ceb31 100644
> > > --- a/arch/x86/kvm/vmx/tdx.c
> > > +++ b/arch/x86/kvm/vmx/tdx.c
> > > @@ -1664,18 +1664,58 @@ static int tdx_mem_page_aug(struct kvm *kvm, gfn_t gfn,
> > > return 0;
> > > }
> > >
> > > +static struct page *tdx_spte_to_external_spt(struct kvm *kvm, gfn_t gfn,
> > > + u64 new_spte, enum pg_level level)
> > > +{
> > > + struct kvm_mmu_page *sp = spte_to_child_sp(new_spte);
> > > +
> > > + if (KVM_BUG_ON(!sp->external_spt, kvm) ||
> > > + KVM_BUG_ON(sp->role.level + 1 != level, kvm) ||
> > > + KVM_BUG_ON(sp->gfn != gfn, kvm))
> > > + return NULL;
> > Could we remove the KVM_BUG_ON()s, and ...
> >
> > > + return virt_to_page(sp->external_spt);
> > > +}
> > > +
> > > +static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn,
> > > + enum pg_level level, u64 mirror_spte)
> > > +{
> > > + gpa_t gpa = gfn_to_gpa(gfn);
> > > + u64 err, entry, level_state;
> > > + struct page *external_spt;
> > > +
> > > + external_spt = tdx_spte_to_external_spt(kvm, gfn, mirror_spte, level);
> > > + if (!external_spt)
> > add a KVM_BUG_ON() here?
> > It could save KVM_BUG_ON()s and have KVM_BUG_ON() match -EIO :)
>
> We could, but I don't want to, because if we're going to bother with sanity checks,
> I want the resulting WARNs to be precise. I.e. I want the WARN to capture *why*
> tdx_spte_to_external_spt() failed, to make debug/triage easier.
Ok.
> > And as Rick also mentioned, better to remove external in external_spt, e.g.
> > something like pt_page.
>
> Yeah, maybe sept_spt?
Hmm, here sept_spt is of type struct page, while sp->spt and sp->external_spt
represents VA. Not sure if it will cause confusion.
But I don't have strong opinion :)
> > And mirror_spte --> new_spte?
>
> Hmm, ya, I made that change later, but it can probably be shifted here.
>
> > > - WARN_ON_ONCE(!is_shadow_present_pte(mirror_spte) ||
> > > - (mirror_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
> > > + WARN_ON_ONCE((mirror_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
> > Also check this for tdx_sept_link_private_spt()?
>
> Eh, we could, but I don't think it's necessary. make_nonleaf_spte() is hardcoded
> to set full permissions (and I don't see that changing any time soon), whereas
> leaf SPTE protections are much more dynamic.
Makes sense.
^ permalink raw reply
* Re: [RFC PATCH v5 19/45] KVM: Allow owner of kvm_mmu_memory_cache to provide a custom page allocator
From: Sean Christopherson @ 2026-02-04 2:16 UTC (permalink / raw)
To: Kai Huang
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Xiaoyao Li,
Yan Y Zhao, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
Isaku Yamahata, sagis@google.com, tglx@kernel.org,
Rick P Edgecombe, bp@alien8.de, Vishal Annapurve, x86@kernel.org
In-Reply-To: <a2bf6a8d9f9b61ae7264afc37d9925cf2e1f3ea9.camel@intel.com>
On Tue, Feb 03, 2026, Kai Huang wrote:
> On Tue, 2026-02-03 at 12:12 -0800, Sean Christopherson wrote:
> > On Tue, Feb 03, 2026, Kai Huang wrote:
> > > On Wed, 2026-01-28 at 17:14 -0800, Sean Christopherson wrote:
> > > > Extend "struct kvm_mmu_memory_cache" to support a custom page allocator
> > > > so that x86's TDX can update per-page metadata on allocation and free().
> > > >
> > > > Name the allocator page_get() to align with __get_free_page(), e.g. to
> > > > communicate that it returns an "unsigned long", not a "struct page", and
> > > > to avoid collisions with macros, e.g. with alloc_page.
> > > >
> > > > Suggested-by: Kai Huang <kai.huang@intel.com>
> > > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > >
> > > I thought it could be more generic for allocating an object, but not just a
> > > page.
> > >
> > > E.g., I thought we might be able to use it to allocate a structure which has
> > > "pair of DPAMT pages" so it could be assigned to 'struct kvm_mmu_page'. But
> > > it seems you abandoned this idea. May I ask why? Just want to understand
> > > the reasoning here.
> >
> > Because that requires more complexity and there's no known use case, and I don't
> > see an obvious way for a use case to come along. All of the motiviations for a
> > custom allocation scheme that I can think of apply only to full pages, or fit
> > nicely in a kmem_cache.
> >
> > Specifically, the "cache" logic is already bifurcated between "kmem_cache' and
> > "page" usage. Further splitting the "page" case doesn't require modifications to
> > the "kmem_cache" case, whereas providing a fully generic solution would require
> > additional changes, e.g. to handle this code:
> >
> > page = (void *)__get_free_page(gfp_flags);
> > if (page && mc->init_value)
> > memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
> >
> > It certainly wouldn't be much complexity, but this code is already a bit awkward,
> > so I don't think it makes sense to add support for something that will probably
> > never be used.
>
> For this particular piece of code, we can add a helper for allocating normal
> page table pages, get rid of mc->init_value completely and hook mc-page_get()
> to that helper.
Hmm, I like the idea, but I don't think it would be a net positive. In practice,
x86's "normal" page tables stop being normal, because KVM now initializes all
SPTEs with BIT(63)=1 on x86-64. And that would also incur an extra RETPOLINE on
all those allocations.
> A bonus is we can then call that helper in all places when KVM needs to
> allocate a page for normal page table instead of just calling
> get_zerod_pages() directly, e.g., like the one in
> tdp_mmu_alloc_sp_for_split(),
Huh. Actually, that's a bug, but not the one you probably expect. At a glance,
it looks like KVM incorrectly zeroing the page instead of initializing it with
SHADOW_NONPRESENT_VALUE. But it's actually a "performance" bug, because KVM
doesn't actually need to pre-initialize the page: either the page will never be
used, or every SPTE will be initialized as a child SPTE.
So that one _should_ be different, e.g. should be:
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index a32192c35099..36afd67601fc 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1456,7 +1456,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
if (!sp)
return NULL;
- sp->spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+ sp->spt = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
if (!sp->spt)
goto err_spt;
> so that we can have a consistent way for allocating normal page table pages.
^ permalink raw reply related
* Re: [RFC PATCH v5 04/45] KVM: x86: Make "external SPTE" ops that can fail RET0 static calls
From: Sean Christopherson @ 2026-02-04 1:16 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Kai Huang,
Xiaoyao Li, Yan Y Zhao, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org, mingo@redhat.com,
pbonzini@redhat.com, binbin.wu@linux.intel.com,
ackerleytng@google.com, Isaku Yamahata, sagis@google.com,
tglx@kernel.org, bp@alien8.de, Vishal Annapurve, x86@kernel.org
In-Reply-To: <e3feb0224cf2665a71ba6147e4e3e3bb30f96760.camel@intel.com>
On Fri, Jan 30, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-01-29 at 17:28 -0800, Sean Christopherson wrote:
> >
> > Hmm, that's probably doable, but definitely in a separate patch.
> > E.g. something
> > like:
>
> I think it would be a good change. But after more consideration, I
> think the original patch is good on its own. Better to turn a bug into
> a deterministic thing, than an opportunity to consume stack. Seems to
> be what you intended.
>
> Another idea would be to have a variant that returns an error instead
> of 0 so the callers can have there error logic triggered, but it's all
> incremental value on top of this.
I don't like that idea, at all. First and foremost, I don't want to litter KVM
with WARNs for things that simply can't happen. I'm fine adding infrastructure
that hides the sanity checks, but I don't want to bleed that into callers.
The other aspect I dislike is that returning a specific errno could lead to all
sorts of weirdness and hidden dependencies.
All in all, I think we'd be increasing the chances of creating bugs just to harden
against issues that in all likelihood will never happen.
^ permalink raw reply
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