* Re: [RFC PATCH v5 22/45] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Yan Zhao @ 2026-02-06 10:20 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-23-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:14:54PM -0800, Sean Christopherson wrote:
> From: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>
> Add Dynamic PAMT support to KVM's S-EPT MMU by "getting" a PAMT page when
> adding guest memory (PAGE.ADD or PAGE.AUG), and "putting" the page when
> removing guest memory (PAGE.REMOVE).
>
> To access the per-vCPU PAMT caches without plumbing @vcpu throughout the
> TDP MMU, begrudginly use kvm_get_running_vcpu() to get the vCPU, and bug
> the VM If KVM attempts to set an S-EPT without an active vCPU. KVM only
> supports creating _new_ mappings in page (pre)fault paths, all of which
> require an active vCPU.
>
> The PAMT memory holds metadata for TDX-protected memory. With Dynamic
> PAMT, PAMT_4K is allocated on demand. The kernel supplies the TDX module
> with a few pages that cover 2M of host physical memory.
>
> PAMT memory can be reclaimed when the last user is gone. It can happen
> in a few code paths:
>
> - On TDH.PHYMEM.PAGE.RECLAIM in tdx_reclaim_td_control_pages() and
> tdx_reclaim_page().
>
> - On TDH.MEM.PAGE.REMOVE in tdx_sept_drop_private_spte().
>
> - In tdx_sept_zap_private_spte() for pages that were in the queue to be
> added with TDH.MEM.PAGE.ADD, but it never happened due to an error.
>
> - In tdx_sept_free_private_spt() for SEPT pages;
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [Minor log tweak]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 1 +
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/mmu/mmu.c | 4 +++
> arch/x86/kvm/vmx/tdx.c | 44 ++++++++++++++++++++++++++----
> arch/x86/kvm/vmx/tdx.h | 2 ++
> 5 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index 17dddada69fc..394dc29483a7 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -99,6 +99,7 @@ KVM_X86_OP_OPTIONAL(free_external_sp)
> KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
> KVM_X86_OP_OPTIONAL(remove_external_spte)
> KVM_X86_OP_OPTIONAL(reclaim_external_sp)
> +KVM_X86_OP_OPTIONAL_RET0(topup_external_cache)
> KVM_X86_OP(has_wbinvd_exit)
> KVM_X86_OP(get_l2_tsc_offset)
> KVM_X86_OP(get_l2_tsc_multiplier)
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 6e84dbc89e79..a6e4ab76b1b2 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1863,6 +1863,7 @@ struct kvm_x86_ops {
> struct kvm_mmu_page *sp);
> void (*remove_external_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> u64 mirror_spte);
> + int (*topup_external_cache)(struct kvm_vcpu *vcpu, int min);
>
>
> bool (*has_wbinvd_exit)(void);
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 9b5a6861e2a4..4ecbf216d96f 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -605,6 +605,10 @@ static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu, bool maybe_indirect)
> PT64_ROOT_MAX_LEVEL);
> if (r)
> return r;
> +
> + r = kvm_x86_call(topup_external_cache)(vcpu, PT64_ROOT_MAX_LEVEL);
If this external cache is for PAMT pages allocation for guest pages only, here
the min count should be 1 instead of PT64_ROOT_MAX_LEVEL?
> + if (r)
> + return r;
> }
> r = kvm_mmu_topup_memory_cache(&vcpu->arch.mmu_shadow_page_cache,
> PT64_ROOT_MAX_LEVEL);
...
> void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
> @@ -3614,5 +3641,12 @@ void __init tdx_hardware_setup(void)
> vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
> vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
> vt_x86_ops.remove_external_spte = tdx_sept_remove_private_spte;
> +
> + /*
> + * FIXME: Wire up the PAMT hook iff DPAMT is supported, once VMXON is
> + * moved out of KVM and tdx_bringup() is folded into here.
> + */
> + vt_x86_ops.topup_external_cache = tdx_topup_external_pamt_cache;
> +
> vt_x86_ops.protected_apic_has_interrupt = tdx_protected_apic_has_interrupt;
> }
^ permalink raw reply
* Re: [RFC PATCH v5 44/45] KVM: x86/mmu: Add support for splitting S-EPT hugepages on conversion
From: Yan Zhao @ 2026-02-06 10:14 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-45-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:15:16PM -0800, Sean Christopherson wrote:
> Add support for splitting S-EPT hugepages in preparation for converting a
> subset of a hugepage to be shared, as KVM must precisely zap/remove S-EPT
> entries to avoid clobbering guest memory (the lifetime of guest private
> memory is tied to the S-EPT). I.e. KVM needs to first split a hugepage so
> that only the to-be-converted small pages can be zapped.
>
> To avoid unnecessary work, e.g. if only the tail/end page of massive region
> isn't aligned to the conversion, explicitly detect unaligned head and tail
> pages relative to the max page size support by KVM, i.e. head/tail pages
> that will undergo partial conversion.
>
> To support splitting an S-EPT hugepage without a vCPU, add a per-VM PAMT
> cache, along with a mutex to guard the cache. Using a mutex, e.g. versus
> a spinlock, is important at it allows KVM to allocate memory *without*
> dropping the lock, i.e. so that the PAMT cache can be topped-up as needed
> without needed to juggle arch.tdp_mmu_external_cache_lock.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/kvm_host.h | 8 +++-
> arch/x86/kvm/mmu/mmu.c | 2 +-
> arch/x86/kvm/mmu/tdp_mmu.c | 72 +++++++++++++++++++++++++++++++--
> arch/x86/kvm/vmx/tdx.c | 34 +++++++++++++---
> arch/x86/kvm/vmx/tdx.h | 2 +
> 5 files changed, 107 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 385f1cf32d70..54dea90a53dc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1563,6 +1563,12 @@ struct kvm_arch {
> * the code to do so.
> */
> spinlock_t tdp_mmu_pages_lock;
> +
> + /*
> + * Protect the per-VM cache of pre-allocate pages used to populate the
> + * Dynamic PAMT when splitting S-EPT huge pages without a vCPU.
> + */
> + struct mutex tdp_mmu_external_cache_lock;
Missing "spin_lock_init(&kvm->arch.tdp_mmu_external_cache_lock);" in
kvm_mmu_init_tdp_mmu().
Will check the patch you replied next week.
> #endif /* CONFIG_X86_64 */
^ permalink raw reply
* Re: [RFC PATCH v5 37/45] KVM: x86/tdp_mmu: Alloc external_spt page for mirror page table splitting
From: Yan Zhao @ 2026-02-06 10:07 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-38-seanjc@google.com>
On Wed, Jan 28, 2026 at 05:15:09PM -0800, Sean Christopherson wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Enhance tdp_mmu_alloc_sp_for_split() to allocate a page table page for the
> external page table for splitting the mirror page table.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Co-developed-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> [sean: use kvm_x86_ops.alloc_external_sp()]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 3b0da898824a..4f5b80f0ca03 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1447,7 +1447,7 @@ bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm,
> return spte_set;
> }
>
> -static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> +static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct tdp_iter *iter)
> {
> struct kvm_mmu_page *sp;
>
> @@ -1461,6 +1461,15 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> return NULL;
> }
>
> + if (is_mirror_sptep(iter->sptep)) {
tdp_mmu_alloc_sp_for_split() is invoked in tdp_mmu_split_huge_pages_root() after
rcu_read_unlock() is called.
So, it's incorrect to invoke is_mirror_sptep() which internally contains
rcu_dereference(), resulting in "WARNING: suspicious RCU usage".
> + sp->external_spt = (void *)kvm_x86_call(alloc_external_sp)(GFP_KERNEL_ACCOUNT);
> + if (!sp->external_spt) {
> + free_page((unsigned long)sp->spt);
> + kmem_cache_free(mmu_page_header_cache, sp);
> + return NULL;
> + }
> + }
> +
> return sp;
> }
>
> @@ -1540,7 +1549,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> else
> write_unlock(&kvm->mmu_lock);
>
> - sp = tdp_mmu_alloc_sp_for_split();
> + sp = tdp_mmu_alloc_sp_for_split(&iter);
>
> if (shared)
> read_lock(&kvm->mmu_lock);
> --
> 2.53.0.rc1.217.geba53bf80e-goog
>
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Yan Zhao @ 2026-02-06 9:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <20260129011517.3545883-21-seanjc@google.com>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 18764dbc97ea..01e3e4f4baa5 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -55,7 +55,8 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
>
> static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> {
> - free_page((unsigned long)sp->external_spt);
> + if (sp->external_spt)
> + kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
> free_page((unsigned long)sp->spt);
> kmem_cache_free(mmu_page_header_cache, sp);
> }
Strictly speaking, external_spt is not a control page. Its alloc/free are
different from normal control pages managed by TDX's code.
(1) alloc
tdx_alloc_control_page
__tdx_alloc_control_page
__tdx_pamt_get
spin_lock(&pamt_lock) ==> under process context
spin_unlock(&pamt_lock)
(2) free
tdp_mmu_free_sp_rcu_callback
tdp_mmu_free_sp
kvm_x86_call(free_external_sp)
tdx_free_control_page
__tdx_free_control_page
__tdx_pamt_put
spin_lock(&pamt_lock) ==> under softirq context
spin_unlock(&pamt_lock)
So, invoking __tdx_pamt_put() in the RCU callback triggers deadlock warning
(see the bottom for details).
> + /*
> + * TDX uses the external_spt cache to allocate S-EPT page table pages,
> + * which (a) don't need to be initialized by KVM as the TDX-Module will
> + * initialize the page (using the guest's encryption key), and (b) need
> + * to use a custom allocator to be compatible with Dynamic PAMT.
> + */
> + vt_x86_ops.alloc_external_sp = tdx_alloc_control_page;
> + vt_x86_ops.free_external_sp = tdx_free_control_page;
> +
> vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
> vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
> vt_x86_ops.remove_external_spte = tdx_sept_remove_private_spte;
================================
WARNING: inconsistent lock state
6.19.0-rc6-upstream+ #1078 Tainted: G S U
--------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/7/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
ffffffff9067b6f8 (pamt_lock){+.?.}-{3:3}, at: __tdx_pamt_put+0x80/0xf0
{SOFTIRQ-ON-W} state was registered at:
__lock_acquire+0x405/0xc10
lock_acquire.part.0+0x9c/0x210
lock_acquire+0x5e/0x100
_raw_spin_lock+0x37/0x80
__tdx_pamt_get+0xb8/0x150
__tdx_alloc_control_page+0x2e/0x60
__tdx_td_init+0x65/0x740 [kvm_intel]
tdx_td_init+0x147/0x240 [kvm_intel]
tdx_vm_ioctl+0x125/0x260 [kvm_intel]
vt_mem_enc_ioctl+0x17/0x30 [kvm_intel]
kvm_arch_vm_ioctl+0x4e0/0xb40 [kvm]
kvm_vm_ioctl+0x4f4/0xaf0 [kvm]
__x64_sys_ioctl+0x9d/0xf0
x64_sys_call+0xf38/0x1da0
do_syscall_64+0xc5/0xfc0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
irq event stamp: 252814
hardirqs last enabled at (252814): [<ffffffff8fa6f41a>] _raw_spin_unlock_irqrestore+0x5a/0x80
hardirqs last disabled at (252813): [<ffffffff8fa6f096>] _raw_spin_lock_irqsave+0x76/0x90
softirqs last enabled at (252798): [<ffffffff8e60f139>] handle_softirqs+0x309/0x460
softirqs last disabled at (252805): [<ffffffff8e60f401>] __irq_exit_rcu+0xe1/0x160
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(pamt_lock);
<Interrupt>
lock(pamt_lock);
*** DEADLOCK ***
1 lock held by swapper/7/0:
#0: ffffffff9077d660 (rcu_callback){....}-{0:0}, at: rcu_do_batch+0x153/0x620
stack backtrace:
CPU: 7 UID: 0 PID: 0 Comm: swapper/7 Tainted: G S U 6.19.0-rc6-upstream+ #1078 PREEMPT(voluntary) b8f4b38003dc2ca73352cf9d3d544aa826c4f5a9
Tainted: [S]=CPU_OUT_OF_SPEC, [U]=USER
Hardware name: Intel Corporation ArcherCity/ArcherCity, BIOS EGSDCRB1.SYS.0101.D29.2303301937 03/30/2023
Call Trace:
<IRQ>
show_stack+0x49/0x60
dump_stack_lvl+0x6f/0xb0
dump_stack+0x10/0x16
print_usage_bug.part.0+0x264/0x350
mark_lock_irq+0x4d6/0x9e0
? stack_trace_save+0x4a/0x70
? save_trace+0x66/0x2b0
mark_lock+0x1cf/0x6a0
mark_usage+0x4c/0x130
__lock_acquire+0x405/0xc10
? __this_cpu_preempt_check+0x13/0x20
lock_acquire.part.0+0x9c/0x210
? __tdx_pamt_put+0x80/0xf0
lock_acquire+0x5e/0x100
? __tdx_pamt_put+0x80/0xf0
_raw_spin_lock+0x37/0x80
? __tdx_pamt_put+0x80/0xf0
__tdx_pamt_put+0x80/0xf0
? __this_cpu_preempt_check+0x13/0x20
? sched_clock_noinstr+0x9/0x10
__tdx_free_control_page+0x22/0x40
tdx_free_control_page+0x38/0x50 [kvm_intel c135d3571385e160f086f9f6195fc72e4b6aa2b1]
tdp_mmu_free_sp_rcu_callback+0x24/0x50 [kvm 3932b137c28c130169e7e3615041bcec6cefc090]
? rcu_do_batch+0x1dc/0x620
rcu_do_batch+0x1e1/0x620
? rcu_do_batch+0x153/0x620
rcu_core+0x37d/0x4d0
rcu_core_si+0xe/0x20
handle_softirqs+0xdc/0x460
? hrtimer_interrupt+0x154/0x290
__irq_exit_rcu+0xe1/0x160
irq_exit_rcu+0xe/0x30
sysvec_apic_timer_interrupt+0xc0/0xf0
</IRQ>
<TASK>
asm_sysvec_apic_timer_interrupt+0x1b/0x20
RIP: 0010:cpuidle_enter_state+0x122/0x7a0
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Trilok Soni @ 2026-02-06 6:37 UTC (permalink / raw)
To: Salman Nabi, vvidwans, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <c29f4a15-1753-42ca-a4a4-ca429053163f@arm.com>
On 2/2/2026 7:52 AM, Salman Nabi wrote:
> Hi Trilok,
>
> On 1/31/26 01:35, Trilok Soni wrote:
>> On 1/19/2026 4:27 AM, Salman Nabi wrote:
>>> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
>>> activating firmware components without a reboot. Those components
>>> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
s/paylods/payloads
>>> usual way: via fwupd, FF-A or other secure storage methods, or via some
>>> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
>>> at system runtime, without requiring a reboot.
>>> The specification covers the SMCCC interface to list and query available
>>> components and eventually trigger the activation.
>>>
>>> Add a new directory under /sys/firmware to present firmware components
>>> capable of live activation. Each of them is a directory under lfa/,
>>> and is identified via its GUID. The activation will be triggered by echoing
>>> "1" into the "activate" file:
>>> ==========================================
>>> /sys/firmware/lfa # ls -l . 6c*
>>> .:
>>> total 0
>>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 47d4086d-4cfe-9846-9b95-2950cbbd5a00
>>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 6c0762a6-12f2-4b56-92cb-ba8f633606d9
>>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 d6d0eea7-fcea-d54b-9782-9934f234b6e4
>> Can you please explain or add a note on why we don't have name of the firmware
>> as the directory name and why you have selected GUID as top-level
>> directory name?
>
>
> We obtain the GUIDs of firmware components from the LFA agent in TF-A, which does not provide their names. For convenience, we have added a C structure in the driver to associate each firmware GUID with its corresponding name. Because new firmware components may be supported in the LFA agent before their GUID-to-name mapping is added to the driver, we avoid using the firmware name as the directory (kobject) name.
>
> Additionally, sysfs requires directory names to be unique among siblings. Since GUIDs are inherently unique, they provide a convenient, collision-free choice for directory names.
Thank you for the explanation. Please add these details in the commit text.
---Trilok Soni
^ permalink raw reply
* Re: [RFC PATCH v5 16/45] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Yan Zhao @ 2026-02-06 2:32 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUbLVrxwDDZ2qh-@google.com>
On Thu, Feb 05, 2026 at 02:35:25PM -0800, Sean Christopherson wrote:
> On Thu, Feb 05, 2026, Yan Zhao wrote:
> > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > > index f6e80aba5895..682c8a228b53 100644
> > > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > > @@ -1824,6 +1824,50 @@ u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data)
> > > }
> > > EXPORT_SYMBOL_FOR_KVM(tdh_mng_rd);
> > >
> > > +/* Number PAMT pages to be provided to TDX module per 2M region of PA */
> > > +static int tdx_dpamt_entry_pages(void)
> > > +{
> > > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > > + return 0;
> > > +
> > This function is not invoked when !tdx_supports_dynamic_pamt().
> > So, probably we can just return the count below?
>
> Or maybe WARN_ON_ONCE() and return 0? I have no strong preference.
"WARN_ON_ONCE() and return 0" looks good to me.
> > > + return tdx_sysinfo.tdmr.pamt_4k_entry_size * PTRS_PER_PTE / PAGE_SIZE;
> > > +}
> > > +
> >
^ permalink raw reply
* Re: [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Yan Zhao @ 2026-02-06 2:30 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUb0KvJynvYjr3h@google.com>
On Thu, Feb 05, 2026 at 02:38:08PM -0800, Sean Christopherson wrote:
> On Thu, Feb 05, 2026, Yan Zhao wrote:
> > On Wed, Feb 04, 2026 at 05:45:39PM +0800, Yan Zhao wrote:
> > > On Wed, Jan 28, 2026 at 05:14:41PM -0800, Sean Christopherson wrote:
> > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > > index d12ca0f8a348..b35a07ed11fb 100644
> > > > --- a/arch/x86/include/asm/kvm_host.h
> > > > +++ b/arch/x86/include/asm/kvm_host.h
> > > > @@ -1858,8 +1858,8 @@ struct kvm_x86_ops {
> > > > u64 mirror_spte);
> > > >
> > > > /* Update external page tables for page table about to be freed. */
> > > > - int (*free_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > > > - void *external_spt);
> > > > + void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> > > > + struct kvm_mmu_page *sp);
> > > Do you think "free" is still better than "reclaim" though TDX actually
> > > invokes tdx_reclaim_page() to reclaim it on the TDX side?
> > >
> > > Naming it free_external_sp can be interpreted as freeing the sp->external_spt
> > > externally (vs freeing it in tdp_mmu_free_sp_rcu_callback(). This naming also
> > > allows for the future possibility of freeing sp->external_spt before the HKID is
> > > freed (though this is unlikely).
> > Oh. I found there's a free_external_sp() in patch 20.
> >
> > So, maybe reclaim_external_sp() --> remove_external_spt() ?
> >
> > Still think "sp" is not good :)
>
> I think my vote would be for reclaim_external_spt(). I don't like "remove", because
> similar to "free", I think most readers will assume success is guaranteed.
Ok. reclaim_external_spt() looks good to me.
^ permalink raw reply
* Re: [RFC PATCH v5 06/45] KVM: x86/mmu: Fold set_external_spte_present() into its sole caller
From: Yan Zhao @ 2026-02-06 2:29 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUijQwl2Q6Q81DL@google.com>
> > > - ret = set_external_spte_present(kvm, iter->sptep, iter->gfn,
> > > - &iter->old_spte, new_spte, iter->level);
> > Add "lockdep_assert_held(&kvm->mmu_lock)" for this case?
>
> No, because I don't want to unnecessarily bleed TDX details into common MMU. Ah,
> but there was a pre-existing lockdep in set_external_spte_present(). So I guess
> that's arguably a functional change and should be called out in the changelog.
>
> But I still want to drop the assertion (or maybe move it to TDX in a prep patch),
> because ultimately the requirements around locking come from TDX, not from the
> TDP MMU.
LGTM.
^ permalink raw reply
* Re: [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Yan Zhao @ 2026-02-06 2:27 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUkVRedz9ngwu_1@google.com>
On Thu, Feb 05, 2026 at 03:14:29PM -0800, Sean Christopherson wrote:
> On Wed, Feb 04, 2026, Yan Zhao wrote:
> > On Tue, Feb 03, 2026 at 08:05:05PM +0000, Sean Christopherson wrote:
> > > On Tue, Feb 03, 2026, Yan Zhao wrote:
> > > > On Wed, Jan 28, 2026 at 05:14:37PM -0800, Sean Christopherson wrote:
> > > > > static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
> > > > > gfn_t gfn, u64 *old_spte,
> > > > > u64 new_spte, int level)
> > > > > {
> > > > > - bool was_present = is_shadow_present_pte(*old_spte);
> > > > > - bool is_present = is_shadow_present_pte(new_spte);
> > > > > - bool is_leaf = is_present && is_last_spte(new_spte, level);
> > > > > - int ret = 0;
> > > > > -
> > > > > - KVM_BUG_ON(was_present, kvm);
> > > > > + int ret;
> > > > >
> > > > > lockdep_assert_held(&kvm->mmu_lock);
> > > > > +
> > > > > + if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> > > > > + return -EIO;
> > > > Why not move this check of is_shadow_present_pte() to tdx_sept_set_private_spte()
> > > > as well?
> > >
> > > The series gets there eventually, but as of this commit, @old_spte isn't plumbed
> > > into tdx_sept_set_private_spte().
> > >
> > > > Or also check !is_shadow_present_pte(new_spte) in TDP MMU?
> > >
> > > Not sure I understand this suggestion.
> > Sorry. The accurate expression should be
> > "what about moving !is_shadow_present_pte(new_spte) to TDP MMU as well?".
>
> It's already there, in __tdp_mmu_set_spte_atomic():
>
> /*
> * KVM doesn't currently support zapping or splitting mirror
> * SPTEs while holding mmu_lock for read.
> */
> if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
> KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> return -EBUSY;
Ok. I was wondering why we don't include it directly in this patch, but it
doesn't matter.
> > > > And as Rick also mentioned, better to remove external in external_spt, e.g.
> > > > something like pt_page.
> > >
> > > Yeah, maybe sept_spt?
> > Hmm, here sept_spt is of type struct page, while sp->spt and sp->external_spt
> > represents VA. Not sure if it will cause confusion.
>
> How about sept_pt?
LGTM.
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Yan Zhao @ 2026-02-06 2:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYUarHf3KEwHGuJe@google.com>
On Thu, Feb 05, 2026 at 02:33:16PM -0800, Sean Christopherson wrote:
> On Thu, Feb 05, 2026, Yan Zhao wrote:
> > On Wed, Feb 04, 2026 at 06:23:38PM -0800, Sean Christopherson wrote:
> > > On Wed, Feb 04, 2026, Yan Zhao wrote:
> > > > On Wed, Jan 28, 2026 at 05:14:40PM -0800, Sean Christopherson wrote:
> > > > > @@ -590,10 +566,21 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> > > > > * the paging structure. Note the WARN on the PFN changing without the
> > > > > * SPTE being converted to a hugepage (leaf) or being zapped. Shadow
> > > > > * pages are kernel allocations and should never be migrated.
> > > > > + *
> > > > > + * When removing leaf entries from a mirror, immediately propagate the
> > > > > + * changes to the external page tables. Note, non-leaf mirror entries
> > > > > + * are handled by handle_removed_pt(), as TDX requires that all leaf
> > > > > + * entries are removed before the owning page table. Note #2, writes
> > > > > + * to make mirror PTEs shadow-present are propagated to external page
> > > > > + * tables by __tdp_mmu_set_spte_atomic(), as KVM needs to ensure the
> > > > > + * external page table was successfully updated before marking the
> > > > > + * mirror SPTE present.
> > > > > */
> > > > > if (was_present && !was_leaf &&
> > > > > (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> > > > > handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> > > > > + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
> > > > Should we check !is_present instead of !is_leaf?
> > > > e.g. a transition from a present leaf entry to a present non-leaf entry could
> > > > also trigger this if case.
> > >
> > > No, the !is_leaf check is very intentional. At this point in the series, S-EPT
> > > doesn't support hugepages. If KVM manages to install a leaf SPTE and replaces
> > > that SPTE with a non-leaf SPTE, then we absolutely want the KVM_BUG_ON() in
> > > tdx_sept_remove_private_spte() to fire:
> > >
> > > /* TODO: handle large pages. */
> > > if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> > > return -EIO;
> > But the op is named remove_external_spte().
> > And the check of "level != PG_LEVEL_4K" is for removing large leaf entries.
>
> I agree that the naming at this point in the series is unfortunate, but I don't
> see it as outright wrong. That the TDP MMU could theoretically replace the leaf
> SPTE with a non-leaf SPTE doesn't change the fact that the old leaf SPTE *is*
> being removed.
Hmm, I can't agree with that. But I won't insist if you think it's ok :)
> > Relying on this check is tricky and confusing.
>
> If it's still confusing at the end of the series, then I'm happy to discuss how
> we can make it less confusion. But as of this point in the series, I unfortunately
> don't see a better way to achieve my end goals (reducing the number of kvm_x86_ops
> hooks, and reducing how many TDX specific details bleed into common MMU code).
>
> There are "different" ways to incrementally move from where were at today, to where
> I want KVM to be, but I don't see them as "better". I.e. AFAICT, there's no way
> to move incrementally with reviewable patches while also maintaining perfect/ideal
> naming and flow.
Ok.
> > > And then later on, when S-EPT gains support for hugepages, "KVM: TDX: Add core
> > > support for splitting/demoting 2MiB S-EPT to 4KiB" doesn't need to touch code
> > > outside of arch/x86/kvm/vmx/tdx.c, because everything has already been plumbed
> > > in.
> > I haven't looked at the later patches for huge pages,
>
> Please do. As above, I don't think it's realistic to completely avoid some amount
> of "eww" in the intermediate stages.
WIP. Found some issues. Will comment after investigating further.
> > but plumbing here directly for splitting does not look right when it's
> > invoked under shared mmu_lock.
> > See the comment below.
> >
> > > > Besides, need "KVM_BUG_ON(shared, kvm)" in this case.
> > >
> > > Eh, we have lockdep_assert_held_write() in the S-EPT paths that require mmu_lock
> > > to be held for write. I don't think a KVM_BUG_ON() here would add meaningful
> > > value.
> > Hmm, I think KVM_BUG_ON(shared, kvm) is still useful.
> > If KVM invokes remove_external_spte() under shared mmu_lock, it needs to freeze
> > the entry first, similar to the sequence in __tdp_mmu_set_spte_atomic().
> >
> > i.e., invoking external x86 ops in handle_changed_spte() for mirror roots should
> > be !shared only.
>
> Sure, but...
>
> > Relying on the TDX code's lockdep_assert_held_write() for warning seems less
> > clear than having an explicit check here.
>
> ...that's TDX's responsibility to enforce, and I don't see any justification for
> something more than a lockdep assertion. As I've said elsewhere, several times,
My concern is that handle_changed_spte() can be invoked by callers other than
tdp_mmu_set_spte(). e.g.,
tdp_mmu_set_spte_atomic
| __tdp_mmu_set_spte_atomic
| | kvm_x86_call(set_external_spte)
| handle_changed_spte
| kvm_x86_call(set_external_spte)
When !is_frozen_spte(new_spte) and was_leaf, set_external_spte() may be invoked
twice for splitting under shared mmu_lock.
Therefore, I think it would be better to check for !shared and only invoke
set_external_spte() when !shared.
BTW: in the patch log, you mentioned that
: Invoke .remove_external_spte() in handle_changed_spte() as appropriate
: instead of relying on callers to do the right thing. Relying on callers
: to invoke .remove_external_spte() is confusing and brittle, e.g. subtly
: relies tdp_mmu_set_spte_atomic() never removing SPTEs, and removing an
: S-EPT entry in tdp_mmu_set_spte() is bizarre (yeah, the VM is bugged so
: it doesn't matter in practice, but it's still weird).
However, when tdp_mmu_set_spte_atomic() removes SPTEs in the future, it will
still need to follow the sequence in __tdp_mmu_set_spte_atomic() :
1. freeze, 2. set_external_spte(). 3. set the new_spte 4. handle_changed_spte().
So, do you think we should leave set_external_spte() in tdp_mmu_set_spte() for
exclusive mmu_lock scenarios instead of moving it to handle_changed_spte()?
> at some point we have to commit to getting the code right. Adding KVM_BUG_ON() in
> Every. Single. Call. does not yield more maintainable code. There are myriad
> things KVM can screw up, many of which have far, far more harmful impact than
> calling an S-EPT hook with mmu_lock held for read instead of write.
>
> The bar for adding a KVM_BUG_ON() is not simply "this shouldn't happen". It's,
> this shouldn't happen *and* at least one of (not a complete list):
>
> - we've either screwed this up badly more than once
> - it's really hard to get right
> - we might not notice if we do screw it up
> - KVM might corrupt data if we continue on
Thanks for sharing this bar.
^ permalink raw reply
* CCC TEE I/O Meeting Agenda for February 5th at 8pm (PST)
From: dan.j.williams @ 2026-02-05 23:19 UTC (permalink / raw)
To: linux-coco; +Cc: david.quigley, mathieu.poirier
The meeting [1] is on for tonight. I have a late breaking conflict and
will need to leave early. Please respond to this message with topics for
opens. I have asked Dave Quigley to chair in my absence. We may not get
to the "IOMMU setup consensus" topic depending on the evidence
collection ABI discussion. The other topics are short.
Agenda:
- Opens
- PCI power management vs DOE power management vs TSM operations
- See pm_runtime_resume_and_get() usage [2]
- What's cooking for devsec-20260205?
- Next steps for device evidence collection ABI (Dave and Mathieu)
- IOMMU setup consensus (continued from Discord conversation [3])
[1]: Registration: https://zoom-lfx.platform.linuxfoundation.org/meeting/93219227394?password=80705486-101a-4e19-8f11-0baaf92104be&invite=true
[2]: https://github.com/l1k/linux/commit/f8343c60e130
[3]: https://discord.com/channels/1123803528377413762/1468465977376378910
^ permalink raw reply
* Re: [RFC PATCH v5 05/45] KVM: TDX: Drop kvm_x86_ops.link_external_spt(), use .set_external_spte() for all
From: Sean Christopherson @ 2026-02-05 23:14 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYLqESiqkADxMGf9@yzhao56-desk.sh.intel.com>
On Wed, Feb 04, 2026, Yan Zhao wrote:
> On Tue, Feb 03, 2026 at 08:05:05PM +0000, Sean Christopherson wrote:
> > On Tue, Feb 03, 2026, Yan Zhao wrote:
> > > On Wed, Jan 28, 2026 at 05:14:37PM -0800, Sean Christopherson wrote:
> > > > static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sptep,
> > > > gfn_t gfn, u64 *old_spte,
> > > > u64 new_spte, int level)
> > > > {
> > > > - bool was_present = is_shadow_present_pte(*old_spte);
> > > > - bool is_present = is_shadow_present_pte(new_spte);
> > > > - bool is_leaf = is_present && is_last_spte(new_spte, level);
> > > > - int ret = 0;
> > > > -
> > > > - KVM_BUG_ON(was_present, kvm);
> > > > + int ret;
> > > >
> > > > lockdep_assert_held(&kvm->mmu_lock);
> > > > +
> > > > + if (KVM_BUG_ON(is_shadow_present_pte(*old_spte), kvm))
> > > > + return -EIO;
> > > Why not move this check of is_shadow_present_pte() to tdx_sept_set_private_spte()
> > > as well?
> >
> > The series gets there eventually, but as of this commit, @old_spte isn't plumbed
> > into tdx_sept_set_private_spte().
> >
> > > Or also check !is_shadow_present_pte(new_spte) in TDP MMU?
> >
> > Not sure I understand this suggestion.
> Sorry. The accurate expression should be
> "what about moving !is_shadow_present_pte(new_spte) to TDP MMU as well?".
It's already there, in __tdp_mmu_set_spte_atomic():
/*
* KVM doesn't currently support zapping or splitting mirror
* SPTEs while holding mmu_lock for read.
*/
if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
return -EBUSY;
> > > And as Rick also mentioned, better to remove external in external_spt, e.g.
> > > something like pt_page.
> >
> > Yeah, maybe sept_spt?
> Hmm, here sept_spt is of type struct page, while sp->spt and sp->external_spt
> represents VA. Not sure if it will cause confusion.
How about sept_pt?
^ permalink raw reply
* Re: [RFC PATCH v5 06/45] KVM: x86/mmu: Fold set_external_spte_present() into its sole caller
From: Sean Christopherson @ 2026-02-05 23:06 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYL3izez+eZ34G/3@yzhao56-desk.sh.intel.com>
On Wed, Feb 04, 2026, Yan Zhao wrote:
> On Wed, Jan 28, 2026 at 05:14:38PM -0800, Sean Christopherson wrote:
> > @@ -626,6 +599,8 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> > struct tdp_iter *iter,
> > u64 new_spte)
> > {
> > + u64 *raw_sptep = rcu_dereference(iter->sptep);
> > +
> > /*
> > * The caller is responsible for ensuring the old SPTE is not a FROZEN
> > * SPTE. KVM should never attempt to zap or manipulate a FROZEN SPTE,
> > @@ -638,31 +613,46 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> > int ret;
> >
> > /*
> > - * Users of atomic zapping don't operate on mirror roots,
> > - * so don't handle it and bug the VM if it's seen.
> > + * KVM doesn't currently support zapping or splitting mirror
> > + * SPTEs while holding mmu_lock for read.
> > */
> > - if (KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> > + if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
> > + KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> > return -EBUSY;
> Should this be -EIO instead?
Yeah, probably.
> Though -EBUSY was introduced by commit 94faba8999b9 ('KVM: x86/tdp_mmu:
> Propagate tearing down mirror page tables')
>
> > - ret = set_external_spte_present(kvm, iter->sptep, iter->gfn,
> > - &iter->old_spte, new_spte, iter->level);
> Add "lockdep_assert_held(&kvm->mmu_lock)" for this case?
No, because I don't want to unnecessarily bleed TDX details into common MMU. Ah,
but there was a pre-existing lockdep in set_external_spte_present(). So I guess
that's arguably a functional change and should be called out in the changelog.
But I still want to drop the assertion (or maybe move it to TDX in a prep patch),
because ultimately the requirements around locking come from TDX, not from the
TDP MMU.
^ permalink raw reply
* Re: [RFC PATCH v5 09/45] KVM: x86: Rework .free_external_spt() into .reclaim_external_sp()
From: Sean Christopherson @ 2026-02-05 22:38 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYRBE1tICOiQ/RL0@yzhao56-desk.sh.intel.com>
On Thu, Feb 05, 2026, Yan Zhao wrote:
> On Wed, Feb 04, 2026 at 05:45:39PM +0800, Yan Zhao wrote:
> > On Wed, Jan 28, 2026 at 05:14:41PM -0800, Sean Christopherson wrote:
> > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > index d12ca0f8a348..b35a07ed11fb 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -1858,8 +1858,8 @@ struct kvm_x86_ops {
> > > u64 mirror_spte);
> > >
> > > /* Update external page tables for page table about to be freed. */
> > > - int (*free_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> > > - void *external_spt);
> > > + void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> > > + struct kvm_mmu_page *sp);
> > Do you think "free" is still better than "reclaim" though TDX actually
> > invokes tdx_reclaim_page() to reclaim it on the TDX side?
> >
> > Naming it free_external_sp can be interpreted as freeing the sp->external_spt
> > externally (vs freeing it in tdp_mmu_free_sp_rcu_callback(). This naming also
> > allows for the future possibility of freeing sp->external_spt before the HKID is
> > freed (though this is unlikely).
> Oh. I found there's a free_external_sp() in patch 20.
>
> So, maybe reclaim_external_sp() --> remove_external_spt() ?
>
> Still think "sp" is not good :)
I think my vote would be for reclaim_external_spt(). I don't like "remove", because
similar to "free", I think most readers will assume success is guaranteed.
^ permalink raw reply
* Re: [RFC PATCH v5 16/45] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Sean Christopherson @ 2026-02-05 22:35 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYQ0l+C42gssMHHV@yzhao56-desk.sh.intel.com>
On Thu, Feb 05, 2026, Yan Zhao wrote:
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index f6e80aba5895..682c8a228b53 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1824,6 +1824,50 @@ u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data)
> > }
> > EXPORT_SYMBOL_FOR_KVM(tdh_mng_rd);
> >
> > +/* Number PAMT pages to be provided to TDX module per 2M region of PA */
> > +static int tdx_dpamt_entry_pages(void)
> > +{
> > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > + return 0;
> > +
> This function is not invoked when !tdx_supports_dynamic_pamt().
> So, probably we can just return the count below?
Or maybe WARN_ON_ONCE() and return 0? I have no strong preference.
> > + return tdx_sysinfo.tdmr.pamt_4k_entry_size * PTRS_PER_PTE / PAGE_SIZE;
> > +}
> > +
>
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Sean Christopherson @ 2026-02-05 22:33 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: <aYQtIK/Lq5T3ad6V@yzhao56-desk.sh.intel.com>
On Thu, Feb 05, 2026, Yan Zhao wrote:
> On Wed, Feb 04, 2026 at 06:23:38PM -0800, Sean Christopherson wrote:
> > On Wed, Feb 04, 2026, Yan Zhao wrote:
> > > On Wed, Jan 28, 2026 at 05:14:40PM -0800, Sean Christopherson wrote:
> > > > @@ -590,10 +566,21 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> > > > * the paging structure. Note the WARN on the PFN changing without the
> > > > * SPTE being converted to a hugepage (leaf) or being zapped. Shadow
> > > > * pages are kernel allocations and should never be migrated.
> > > > + *
> > > > + * When removing leaf entries from a mirror, immediately propagate the
> > > > + * changes to the external page tables. Note, non-leaf mirror entries
> > > > + * are handled by handle_removed_pt(), as TDX requires that all leaf
> > > > + * entries are removed before the owning page table. Note #2, writes
> > > > + * to make mirror PTEs shadow-present are propagated to external page
> > > > + * tables by __tdp_mmu_set_spte_atomic(), as KVM needs to ensure the
> > > > + * external page table was successfully updated before marking the
> > > > + * mirror SPTE present.
> > > > */
> > > > if (was_present && !was_leaf &&
> > > > (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> > > > handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> > > > + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
> > > Should we check !is_present instead of !is_leaf?
> > > e.g. a transition from a present leaf entry to a present non-leaf entry could
> > > also trigger this if case.
> >
> > No, the !is_leaf check is very intentional. At this point in the series, S-EPT
> > doesn't support hugepages. If KVM manages to install a leaf SPTE and replaces
> > that SPTE with a non-leaf SPTE, then we absolutely want the KVM_BUG_ON() in
> > tdx_sept_remove_private_spte() to fire:
> >
> > /* TODO: handle large pages. */
> > if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> > return -EIO;
> But the op is named remove_external_spte().
> And the check of "level != PG_LEVEL_4K" is for removing large leaf entries.
I agree that the naming at this point in the series is unfortunate, but I don't
see it as outright wrong. That the TDP MMU could theoretically replace the leaf
SPTE with a non-leaf SPTE doesn't change the fact that the old leaf SPTE *is*
being removed.
> Relying on this check is tricky and confusing.
If it's still confusing at the end of the series, then I'm happy to discuss how
we can make it less confusion. But as of this point in the series, I unfortunately
don't see a better way to achieve my end goals (reducing the number of kvm_x86_ops
hooks, and reducing how many TDX specific details bleed into common MMU code).
There are "different" ways to incrementally move from where were at today, to where
I want KVM to be, but I don't see them as "better". I.e. AFAICT, there's no way
to move incrementally with reviewable patches while also maintaining perfect/ideal
naming and flow.
> > And then later on, when S-EPT gains support for hugepages, "KVM: TDX: Add core
> > support for splitting/demoting 2MiB S-EPT to 4KiB" doesn't need to touch code
> > outside of arch/x86/kvm/vmx/tdx.c, because everything has already been plumbed
> > in.
> I haven't looked at the later patches for huge pages,
Please do. As above, I don't think it's realistic to completely avoid some amount
of "eww" in the intermediate stages.
> but plumbing here directly for splitting does not look right when it's
> invoked under shared mmu_lock.
> See the comment below.
>
> > > Besides, need "KVM_BUG_ON(shared, kvm)" in this case.
> >
> > Eh, we have lockdep_assert_held_write() in the S-EPT paths that require mmu_lock
> > to be held for write. I don't think a KVM_BUG_ON() here would add meaningful
> > value.
> Hmm, I think KVM_BUG_ON(shared, kvm) is still useful.
> If KVM invokes remove_external_spte() under shared mmu_lock, it needs to freeze
> the entry first, similar to the sequence in __tdp_mmu_set_spte_atomic().
>
> i.e., invoking external x86 ops in handle_changed_spte() for mirror roots should
> be !shared only.
Sure, but...
> Relying on the TDX code's lockdep_assert_held_write() for warning seems less
> clear than having an explicit check here.
...that's TDX's responsibility to enforce, and I don't see any justification for
something more than a lockdep assertion. As I've said elsewhere, several times,
at some point we have to commit to getting the code right. Adding KVM_BUG_ON() in
Every. Single. Call. does not yield more maintainable code. There are myriad
things KVM can screw up, many of which have far, far more harmful impact than
calling an S-EPT hook with mmu_lock held for read instead of write.
The bar for adding a KVM_BUG_ON() is not simply "this shouldn't happen". It's,
this shouldn't happen *and* at least one of (not a complete list):
- we've either screwed this up badly more than once
- it's really hard to get right
- we might not notice if we do screw it up
- KVM might corrupt data if we continue on
^ permalink raw reply
* Re: [PATCH v2 0/2] x86/virt/tdx: Print TDX module version to dmesg
From: Verma, Vishal L @ 2026-02-05 19:05 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org
Cc: Gao, Chao, Edgecombe, Rick P, dave.hansen@linux.intel.com,
Huang, Kai, kas@kernel.org, bp@alien8.de, mingo@redhat.com,
Williams, Dan J, tglx@linutronix.de, hpa@zytor.com,
x86@kernel.org
In-Reply-To: <20260109-tdx_print_module_version-v2-0-e10e4ca5b450@intel.com>
On Fri, 2026-01-09 at 12:14 -0700, Vishal Verma wrote:
> === Problem & Solution ===
>
> Currently, there is neither an ABI, nor any other way to determine from
> the host system, what version of the TDX module is running. A sysfs ABI
> for this has been proposed in [1], but it may need additional discussion.
>
> Many/most TDX developers already carry patches like this in their
> development branches. It can be tricky to know which TDX module is
> actually loaded on a system, and so this functionality has been needed
> regularly for development and processing bug reports. Hence, it is
> prudent to break out the patches to retrieve and print the TDX module
> version, as those parts are very straightforward, and get some level of
> debugability and traceability for TDX host systems.
>
> === Dependencies ===
>
> None. This is based on v6.19-rc4, and applies cleanly to tip.git.
>
> === Patch details ===
>
> Patch 1 is a prerequisite that adds the infrastructure to retrieve the
> TDX module version from its global metadata. This was originally posted in [2].
>
> Patch 2 is based on a patch from Kai Huang [3], and prints the version to
> dmesg during init.
>
> === Testing ===
>
> This has passed the usual suite of tests, including successful 0day
> builds, KVM Unit tests, KVM selftests, a TD creation smoke test, and
> selected KVM tests from the Avocado test suite.
>
> [1]: https://lore.kernel.org/all/20260105074350.98564-1-chao.gao@intel.com/
> [2]: https://lore.kernel.org/all/20260105074350.98564-2-chao.gao@intel.com/
> [3]: https://lore.kernel.org/all/57eaa1b17429315f8b5207774307f3c1dd40cf37.1730118186.git.kai.huang@intel.com/
>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Hi Kiryl, just wanted to check on the plan for this, I didn't see it
merged in tip.git x86/tdx (or any other tip branch). Were you planning
to take it through x86/tdx? Can I help with anything to move it along?
Thank you,
Vishal
^ 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-05 17:29 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Pratik R. Sampat, linux-mm, linux-coco, x86, linux-kernel, tglx,
mingo, bp, dave.hansen, ardb, akpm, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <aYS8O0phOE41IuEF@thinkstation>
On 2/5/26 17:08, Kiryl Shutsemau wrote:
> On Thu, Feb 05, 2026 at 04:48:13PM +0100, David Hildenbrand (Arm) wrote:
>> On 2/5/26 11:48, Kiryl Shutsemau wrote:
>>>
>>> You are calling arch_accept_memory() on every memory allocation if the
>>> memory is not represented in the bitmap. Hard NAK.
>>
>> In which scenarios would we not have memory represented in the bitmap?
>> Guests with <4 GiB? (how does kexec work?) Anything else?
>
> We create the bitmap that covers all unaccepted memory.
Good! :)
>
> What memory is unaccepted is up to BIOS. Current implementation of edk2
> accepts the memory in the first 4G range of physical address space. It
> means we won't have bitmap for this range (unaccepted->phys_base >= 4G).
Ah, okay, this comes from the BIOS.
>
> If the whole VM is smaller than 4G we won't have the bitmap at all.
>
> We can allocate bitmap for all possible memory. Maybe upto max_possible_pfn?
> But we might not know the value in EFI stub. It costs 4k per 64GiB of
> physical address space.
That's what I would do. 4k per 64GiB sounds reasonable.
>
> Ideally, we want to know on boot:
>
> - what memory ranges are unaccepted - we have it;
> - what memory range can be removed or added after boot - we don't have it
The SRAT describes memory ranges where we can see hotplug memory. Is
that too late? We calculate max_possible_pfn based on that.
(don't ask me about special CXL windows and how they are advertised :) )
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Dave Hansen @ 2026-02-05 16:37 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: <aYTFZv9Mf_FqWM_k@google.com>
On 2/5/26 08:29, Sean Christopherson wrote:
> No, this isn't the explanation. I found the explanation in the pseudocode for
> SEAMRET. The "successful VM-Entry" path says this:
>
> current-VMCS = current-VMCS.VMCS-link-pointer
> IF inP_SEAMLDR == 1; THEN
> If current-VMCS != FFFFFFFF_FFFFFFFFH; THEN
> Ensure data for VMCS referenced by current-VMC is in memory
> Initialize implementation-specific data in all VMCS referenced by current-VMCS
> Set launch state of VMCS referenced by current-VMCS to “clear”
> current-VMCS = FFFFFFFF_FFFFFFFFH
> FI;
> inP_SEAMLDR = 0
> FI;
Yes, in version 002 of the spec. It wasn't there in 001.
The basic problem is that the SEAM VMCSes need to get flushed when the
TDX module is being loaded. The TDX module never loads itself, thus the
"inP_SEAMLDR == 1" check. It sounds like there was already an existing
thing in microcode to just flush VMCSes and invalidate "current-VMCS".
It was much easier for microcode to just jump over to that existing
thing than to surgically target the SEAM VMCSes, or somehow avoid
zapping "current-VMCS". It makes total sense for the microcoders to have
gone this route.
I'm seeing if it can get changed back to the 001 version so we just
don't even have to deal with this whole mess.
^ permalink raw reply
* Re: [PATCH v3 07/26] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Sean Christopherson @ 2026-02-05 16:29 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: <aYKKnf7K3lRdUcxl@intel.com>
On Wed, Feb 04, 2026, Chao Gao wrote:
> >On Fri, Jan 30, 2026 at 8:23 AM Dave Hansen <dave.hansen@intel.com> wrote:
> >> On 1/30/26 00:08, Chao Gao wrote:
> >> > AFAIK, this is a CPU implementation issue. The actual requirement is to
> >> > evict (flush and invalidate) all VMCSs __cached in SEAM mode__, but big
> >> > cores implement this by evicting the __entire__ VMCS cache. So, the
> >> > current VMCS is invalidated and cleared.
> >>
> >> But why is this a P-SEAMLDR thing and not a TDX module thing?
> >
> >My guess is that it's because the P-SEAMLDR code loads and prepares the new TDX-
> >Module by constructing the VMCS used for SEAMCALL using direct writes to memory
> >(unless that TDX behavior has changed in the last few years). And so it needs
> >to ensure that in-memory representation is synchronized with the VMCS cache.
> >
> >Hmm, but that doesn't make sense _if_ it really truly is SEAMRET that does the VMCS
> >cache invalidation, because flushing the VMCS cache would ovewrite the in-memory
> >state.
>
> My understanding is:
>
> 1. SEAMCALL/SEAMRET use VMCSs.
>
> 2. P-SEAMLDR is single-threaded (likely for simplicity). So, it uses a _single_
> global VMCS and only one CPU can call P-SEAMLDR calls at a time.
>
> 3. After SEAMRET from P-SEAMLDR, _if_ the global VMCS isn't flushed, other CPUs
> cannot enter P-SEAMLDR because the global VMCS would be corrupted. (note the
> global VMCS is cached by the original CPU).
>
> 4. To make P-SEAMLDR callable on all CPUs, SEAMRET instruction flush VMCSs.
> The flush cannot be performed by the host VMM since the global VMCS is not
> visible to it. P-SEAMLDR cannot do it either because SEAMRET is its final
> instruction and requires a valid VMCS.
No, this isn't the explanation. I found the explanation in the pseudocode for
SEAMRET. The "successful VM-Entry" path says this:
current-VMCS = current-VMCS.VMCS-link-pointer
IF inP_SEAMLDR == 1; THEN
If current-VMCS != FFFFFFFF_FFFFFFFFH; THEN
Ensure data for VMCS referenced by current-VMC is in memory
Initialize implementation-specific data in all VMCS referenced by current-VMCS
Set launch state of VMCS referenced by current-VMCS to “clear”
current-VMCS = FFFFFFFF_FFFFFFFFH
FI;
inP_SEAMLDR = 0
FI;
I.e. my guess about firmware (probably XuCode?) doing direct writes was correct,
I just guessed wrong on which VMCS. Or rather, I didn't guess "all".
> The TDX Module has per-CPU VMCSs, so it doesn't has this problem.
>
> I'll check if SEAM ISA architects can join to explain this in more detail.
^ 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 16:08 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Pratik R. Sampat, linux-mm, linux-coco, x86, linux-kernel, tglx,
mingo, bp, dave.hansen, ardb, akpm, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <ce4334f8-5e54-4dbf-9be0-059279ef1962@kernel.org>
On Thu, Feb 05, 2026 at 04:48:13PM +0100, David Hildenbrand (Arm) wrote:
> On 2/5/26 11:48, Kiryl Shutsemau wrote:
> > 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:
> > > >
> > > > 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.
>
> In which scenarios would we not have memory represented in the bitmap?
> Guests with <4 GiB? (how does kexec work?) Anything else?
We create the bitmap that covers all unaccepted memory.
What memory is unaccepted is up to BIOS. Current implementation of edk2
accepts the memory in the first 4G range of physical address space. It
means we won't have bitmap for this range (unaccepted->phys_base >= 4G).
If the whole VM is smaller than 4G we won't have the bitmap at all.
We can allocate bitmap for all possible memory. Maybe upto max_possible_pfn?
But we might not know the value in EFI stub. It costs 4k per 64GiB of
physical address space.
Ideally, we want to know on boot:
- what memory ranges are unaccepted - we have it;
- what memory range can be removed or added after boot - we don't have it
Then we can allocate bitmap that covers all this memory.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH v5 00/45] TDX: Dynamic PAMT + S-EPT Hugepage
From: Dave Hansen @ 2026-02-05 16:01 UTC (permalink / raw)
To: Sean Christopherson
Cc: Konrad Rzeszutek Wilk, 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: <aYS85EsXu_xuQXSI@google.com>
On 2/5/26 07:53, Sean Christopherson wrote:
> I'm a-ok starting with a topic branch. If maintaining that branch becomes too
> costly, then we can always revisit things. And that would probably be good
> motiviation to beat guest_memfd hugepage into shape 🙂
Sounds like a plan.
^ permalink raw reply
* Re: [RFC PATCH v5 00/45] TDX: Dynamic PAMT + S-EPT Hugepage
From: Sean Christopherson @ 2026-02-05 15:53 UTC (permalink / raw)
To: Dave Hansen
Cc: Konrad Rzeszutek Wilk, 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: <e60e0929-f3b0-463d-8c82-dc9170e401eb@intel.com>
On Wed, Feb 04, 2026, Dave Hansen wrote:
> 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 was thinking pushing to Linus' tree, but a topic branch could likely provide
almost as much value?
> 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.
I'm a-ok starting with a topic branch. If maintaining that branch becomes too
costly, then we can always revisit things. And that would probably be good
motiviation to beat guest_memfd hugepage into shape :-)
^ 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-05 15:48 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: <aYR0vqwyFPo3EKAi@thinkstation>
On 2/5/26 11:48, Kiryl Shutsemau wrote:
> 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:
>>>
>>> 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.
In which scenarios would we not have memory represented in the bitmap?
Guests with <4 GiB? (how does kexec work?) Anything else?
--
Cheers,
David
^ 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: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
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