* Re: [PATCH v4 06/16] x86/virt/tdx: Improve PAMT refcounts allocation for sparse memory
From: Edgecombe, Rick P @ 2025-11-26 20:47 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, tglx@linutronix.de, Yamahata, Isaku,
nik.borisov@suse.com, pbonzini@redhat.com, Annapurve, Vishal,
Gao, Chao, bp@alien8.de, x86@kernel.org
Cc: kirill.shutemov@linux.intel.com
In-Reply-To: <b3b465e6-cfa0-44d0-bdef-6d37bb26e6e0@suse.com>
Kiryl, curious if you have any comments on the below...
On Wed, 2025-11-26 at 16:45 +0200, Nikolay Borisov wrote:
> > +static int pamt_refcount_populate(pte_t *pte, unsigned long addr, void
> > *data)
> > +{
> > + struct page *page;
> > + pte_t entry;
> > +
> > + page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> > + if (!page)
> > return -ENOMEM;
> >
> > + entry = mk_pte(page, PAGE_KERNEL);
> > +
> > + spin_lock(&init_mm.page_table_lock);
> > + /*
> > + * PAMT refcount populations can overlap due to rounding of the
> > + * start/end pfn. Make sure the PAMT range is only populated once.
> > + */
> > + if (pte_none(ptep_get(pte)))
> > + set_pte_at(&init_mm, addr, pte, entry);
> > + else
> > + __free_page(page);
> > + spin_unlock(&init_mm.page_table_lock);
>
> nit: Wouldn't it be better to perform the pte_none() check before doing
> the allocation thus avoiding needless allocations? I.e do the
> alloc/mk_pte only after we are 100% sure we are going to use this entry.
Yes, but I'm also wondering why it needs init_mm.page_table_lock at all. Here is
my reasoning for why it doesn't:
apply_to_page_range() takes init_mm.page_table_lock internally when it modified
page tables in the address range (vmalloc). It needs to do this to avoid races
with other allocations that share the upper level page tables, which could be on
the ends of area that TDX reserves.
But pamt_refcount_populate() is only operating on the PTE's for the address
range that TDX code already controls. Vmalloc should not free the PMD underneath
the PTE operation because there is an allocation in any page tables it covers.
So we can skip the lock and also do the pte_none() check before the page
allocation as Nikolay suggests.
Same for the depopulate path.
^ permalink raw reply
* Re: [PATCH v4 16/16] Documentation/x86: Add documentation for TDX's Dynamic PAMT
From: Edgecombe, Rick P @ 2025-11-26 20:05 UTC (permalink / raw)
To: binbin.wu@linux.intel.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
Yamahata, Isaku, tglx@linutronix.de, Annapurve, Vishal, Gao, Chao,
bp@alien8.de, x86@kernel.org
In-Reply-To: <96775d7e-e0d9-404e-bdb0-8c95db7f09cf@linux.intel.com>
On Wed, 2025-11-26 at 18:33 +0800, Binbin Wu wrote:
> > +PAMT is memory that the TDX module needs to keep data about each page
> > +(think like struct page). It needs to handed to the TDX module for its
> ^
> be
Oh, yep, thanks.
^ permalink raw reply
* Re: [PATCH v4 04/16] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Edgecombe, Rick P @ 2025-11-26 17:56 UTC (permalink / raw)
To: binbin.wu@linux.intel.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
Yamahata, Isaku, tglx@linutronix.de, Annapurve, Vishal, Gao, Chao,
bp@alien8.de, x86@kernel.org
In-Reply-To: <51dc2009-ff3e-4419-9dab-b46db7b2e15d@linux.intel.com>
On Tue, 2025-11-25 at 09:50 +0800, Binbin Wu wrote:
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Thanks.
>
> One nit below.
>
> [...]
> > diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> > index 13ad2663488b..00ab0e550636 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> > @@ -33,6 +33,13 @@ static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> > sysinfo_tdmr->pamt_2m_entry_size = val;
> > if (!ret && !(ret = read_sys_metadata_field(0x9100000100000012, &val)))
> > sysinfo_tdmr->pamt_1g_entry_size = val;
> > + /*
> > + * Don't fail here if tdx_supports_dynamic_pamt() isn't supported. The
>
> A bit weird to say "if tdx_supports_dynamic_pamt() isn't supported", how about
> using "if dynamic PAMT isn't supported"?
Yes, good point, I'll use your suggestion.
^ permalink raw reply
* Re: [PATCH v4 06/16] x86/virt/tdx: Improve PAMT refcounts allocation for sparse memory
From: Nikolay Borisov @ 2025-11-26 14:45 UTC (permalink / raw)
To: Rick Edgecombe, bp, chao.gao, dave.hansen, isaku.yamahata,
kai.huang, kas, kvm, linux-coco, linux-kernel, mingo, pbonzini,
seanjc, tglx, vannapurve, x86, yan.y.zhao, xiaoyao.li, binbin.wu
Cc: Kirill A. Shutemov
In-Reply-To: <20251121005125.417831-7-rick.p.edgecombe@intel.com>
On 21.11.25 г. 2:51 ч., Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> init_pamt_metadata() allocates PAMT refcounts for all physical memory up
> to max_pfn. It might be suboptimal if the physical memory layout is
> discontinuous and has large holes.
>
> The refcount allocation vmalloc allocation. This is necessary to support a
nit: Something's odd with the first sentence, perhaps an "is a" before
is missing before "vmalloc"?
> large allocation size. The virtually contiguous property also makes it
> easy to find a specific 2MB range’s refcount since it can simply be
> indexed.
>
> Since vmalloc mappings support remapping during normal kernel runtime,
> switch to an approach that only populates refcount pages for the vmalloc
> mapping when there is actually memory for that range. This means any holes
> in the physical address space won’t use actual physical memory.
>
> The validity of this memory optimization is based on a couple assumptions:
> 1. Physical holes in the ram layout are commonly large enough for it to be
> worth it.
> 2. An alternative approach that looks the refcounts via some more layered
> data structure wouldn’t overly complicate the lookups. Or at least
> more than the complexity of managing the vmalloc mapping.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [Add feedback, update log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
<snip>
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 136 +++++++++++++++++++++++++++++++++---
> 1 file changed, 125 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index c28d4d11736c..edf9182ed86d 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -194,30 +194,135 @@ int tdx_cpu_enable(void)
> }
> EXPORT_SYMBOL_GPL(tdx_cpu_enable);
>
> -/*
> - * Allocate PAMT reference counters for all physical memory.
> - *
> - * It consumes 2MiB for every 1TiB of physical memory.
> - */
> -static int init_pamt_metadata(void)
> +/* Find PAMT refcount for a given physical address */
> +static atomic_t *tdx_find_pamt_refcount(unsigned long pfn)
> {
> - size_t size = DIV_ROUND_UP(max_pfn, PTRS_PER_PTE) * sizeof(*pamt_refcounts);
> + /* Find which PMD a PFN is in. */
> + unsigned long index = pfn >> (PMD_SHIFT - PAGE_SHIFT);
>
> - if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> - return 0;
> + return &pamt_refcounts[index];
> +}
>
> - pamt_refcounts = __vmalloc(size, GFP_KERNEL | __GFP_ZERO);
> - if (!pamt_refcounts)
> +/* Map a page into the PAMT refcount vmalloc region */
> +static int pamt_refcount_populate(pte_t *pte, unsigned long addr, void *data)
> +{
> + struct page *page;
> + pte_t entry;
> +
> + page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> + if (!page)
> return -ENOMEM;
>
> + entry = mk_pte(page, PAGE_KERNEL);
> +
> + spin_lock(&init_mm.page_table_lock);
> + /*
> + * PAMT refcount populations can overlap due to rounding of the
> + * start/end pfn. Make sure the PAMT range is only populated once.
> + */
> + if (pte_none(ptep_get(pte)))
> + set_pte_at(&init_mm, addr, pte, entry);
> + else
> + __free_page(page);
> + spin_unlock(&init_mm.page_table_lock);
nit: Wouldn't it be better to perform the pte_none() check before doing
the allocation thus avoiding needless allocations? I.e do the
alloc/mk_pte only after we are 100% sure we are going to use this entry.
> +
> return 0;
> }
<snip>
^ permalink raw reply
* Re: [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Kiryl Shutsemau @ 2025-11-26 13:35 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco, kvm,
Reinette Chatre, Chenyi Qiang, chao.p.peng
In-Reply-To: <33fe9716-ef3b-42f3-9806-4bd23fed6949@intel.com>
On Wed, Nov 26, 2025 at 08:17:18PM +0800, Xiaoyao Li wrote:
> On 11/26/2025 7:25 PM, Kiryl Shutsemau wrote:
> > On Wed, Nov 26, 2025 at 06:02:03PM +0800, Xiaoyao Li wrote:
> > > When the host enables split lock detection feature, the split lock from
> > > guests (normal or TDX) triggers #AC. The #AC caused by split lock access
> > > within a normal guest triggers a VM Exit and is handled in the host.
> > > The #AC caused by split lock access within a TDX guest does not trigger
> > > a VM Exit and instead it's delivered to the guest self.
> > >
> > > The default "warning" mode of handling split lock depends on being able
> > > to temporarily disable detection to recover from the split lock event.
> > > But the MSR that disables detection is not accessible to a guest.
> > >
> > > This means that TDX guests today can not disable the feature or use
> > > the "warning" mode (which is the default). But, they can use the "fatal"
> > > mode.
> > >
> > > Force TDX guests to use the "fatal" mode.
> > >
> > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > ---
> > > arch/x86/kernel/cpu/bus_lock.c | 17 ++++++++++++++++-
> > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
> > > index 981f8b1f0792..f278e4ea3dd4 100644
> > > --- a/arch/x86/kernel/cpu/bus_lock.c
> > > +++ b/arch/x86/kernel/cpu/bus_lock.c
> > > @@ -315,9 +315,24 @@ void bus_lock_init(void)
> > > wrmsrq(MSR_IA32_DEBUGCTLMSR, val);
> > > }
> > > +static bool split_lock_fatal(void)
> > > +{
> > > + if (sld_state == sld_fatal)
> > > + return true;
> > > +
> > > + /*
> > > + * TDX guests can not disable split lock detection.
> > > + * Force them into the fatal behavior.
> > > + */
> > > + if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
> > > + return true;
> > > +
> > > + return false;
> > > +}
> > > +
> > > bool handle_user_split_lock(struct pt_regs *regs, long error_code)
> > > {
> > > - if ((regs->flags & X86_EFLAGS_AC) || sld_state == sld_fatal)
> > > + if ((regs->flags & X86_EFLAGS_AC) || split_lock_fatal())
> > > return false;
> >
> > Maybe it would be cleaner to make it conditional on
> > cpu_model_supports_sld instead of special-casing TDX guest?
> >
> > #AC on any platfrom when we didn't asked for it suppose to be fatal, no?
>
> But TDX is the only one has such special non-architectural behavior.
>
> For example, for normal VMs under KVM, the behavior is x86 architectural.
> MSR_TEST_CTRL is not accessible to normal VMs, and no split lock #AC will be
> delivered to the normal VMs because it's handled by KVM.
How does it contradict what I suggested?
For both normal VMs and TDX guest, cpu_model_supports_sld will not be
set to true. So check for cpu_model_supports_sld here is going to be
NOP, unless #AC actually delivered, like we have in TDX case. Handling
it as fatal is sane behaviour in such case regardless if it TDX.
And we don't need to make the check explicitly about TDX guest.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Xiaoyao Li @ 2025-11-26 12:17 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco, kvm,
Reinette Chatre, Chenyi Qiang, chao.p.peng
In-Reply-To: <lvobu4gpfsjg63syubgy2jwcja72folflrst7bu2eqv6rhaqre@ttbkykphu32f>
On 11/26/2025 7:25 PM, Kiryl Shutsemau wrote:
> On Wed, Nov 26, 2025 at 06:02:03PM +0800, Xiaoyao Li wrote:
>> When the host enables split lock detection feature, the split lock from
>> guests (normal or TDX) triggers #AC. The #AC caused by split lock access
>> within a normal guest triggers a VM Exit and is handled in the host.
>> The #AC caused by split lock access within a TDX guest does not trigger
>> a VM Exit and instead it's delivered to the guest self.
>>
>> The default "warning" mode of handling split lock depends on being able
>> to temporarily disable detection to recover from the split lock event.
>> But the MSR that disables detection is not accessible to a guest.
>>
>> This means that TDX guests today can not disable the feature or use
>> the "warning" mode (which is the default). But, they can use the "fatal"
>> mode.
>>
>> Force TDX guests to use the "fatal" mode.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>> arch/x86/kernel/cpu/bus_lock.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
>> index 981f8b1f0792..f278e4ea3dd4 100644
>> --- a/arch/x86/kernel/cpu/bus_lock.c
>> +++ b/arch/x86/kernel/cpu/bus_lock.c
>> @@ -315,9 +315,24 @@ void bus_lock_init(void)
>> wrmsrq(MSR_IA32_DEBUGCTLMSR, val);
>> }
>>
>> +static bool split_lock_fatal(void)
>> +{
>> + if (sld_state == sld_fatal)
>> + return true;
>> +
>> + /*
>> + * TDX guests can not disable split lock detection.
>> + * Force them into the fatal behavior.
>> + */
>> + if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> bool handle_user_split_lock(struct pt_regs *regs, long error_code)
>> {
>> - if ((regs->flags & X86_EFLAGS_AC) || sld_state == sld_fatal)
>> + if ((regs->flags & X86_EFLAGS_AC) || split_lock_fatal())
>> return false;
>
> Maybe it would be cleaner to make it conditional on
> cpu_model_supports_sld instead of special-casing TDX guest?
>
> #AC on any platfrom when we didn't asked for it suppose to be fatal, no?
But TDX is the only one has such special non-architectural behavior.
For example, for normal VMs under KVM, the behavior is x86
architectural. MSR_TEST_CTRL is not accessible to normal VMs, and no
split lock #AC will be delivered to the normal VMs because it's handled
by KVM.
>> split_lock_warn(regs->ip);
>> return true;
>> --
>> 2.43.0
>>
>
^ permalink raw reply
* Re: [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Kiryl Shutsemau @ 2025-11-26 11:25 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco, kvm,
Reinette Chatre, Chenyi Qiang, chao.p.peng
In-Reply-To: <20251126100205.1729391-2-xiaoyao.li@intel.com>
On Wed, Nov 26, 2025 at 06:02:03PM +0800, Xiaoyao Li wrote:
> When the host enables split lock detection feature, the split lock from
> guests (normal or TDX) triggers #AC. The #AC caused by split lock access
> within a normal guest triggers a VM Exit and is handled in the host.
> The #AC caused by split lock access within a TDX guest does not trigger
> a VM Exit and instead it's delivered to the guest self.
>
> The default "warning" mode of handling split lock depends on being able
> to temporarily disable detection to recover from the split lock event.
> But the MSR that disables detection is not accessible to a guest.
>
> This means that TDX guests today can not disable the feature or use
> the "warning" mode (which is the default). But, they can use the "fatal"
> mode.
>
> Force TDX guests to use the "fatal" mode.
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/kernel/cpu/bus_lock.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
> index 981f8b1f0792..f278e4ea3dd4 100644
> --- a/arch/x86/kernel/cpu/bus_lock.c
> +++ b/arch/x86/kernel/cpu/bus_lock.c
> @@ -315,9 +315,24 @@ void bus_lock_init(void)
> wrmsrq(MSR_IA32_DEBUGCTLMSR, val);
> }
>
> +static bool split_lock_fatal(void)
> +{
> + if (sld_state == sld_fatal)
> + return true;
> +
> + /*
> + * TDX guests can not disable split lock detection.
> + * Force them into the fatal behavior.
> + */
> + if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
> + return true;
> +
> + return false;
> +}
> +
> bool handle_user_split_lock(struct pt_regs *regs, long error_code)
> {
> - if ((regs->flags & X86_EFLAGS_AC) || sld_state == sld_fatal)
> + if ((regs->flags & X86_EFLAGS_AC) || split_lock_fatal())
> return false;
Maybe it would be cleaner to make it conditional on
cpu_model_supports_sld instead of special-casing TDX guest?
#AC on any platfrom when we didn't asked for it suppose to be fatal, no?
> split_lock_warn(regs->ip);
> return true;
> --
> 2.43.0
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Kiryl Shutsemau @ 2025-11-26 11:12 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <20251125175753.1428857-3-prsampat@amd.com>
On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
> The unaccepted memory structure currently only supports accepting memory
> present at boot time. The unaccepted table uses a fixed-size bitmap
> reserved in memblock based on the initial memory layout, preventing
> dynamic addition of memory ranges after boot. This causes guest
> termination when memory is hot-added in a secure virtual machine due to
> accessing pages that have not transitioned to private before use.
How does the hot-pluggable memory look in EFI memory map? I thought
hot-pluggable ranges suppose to be declared thare. The cleanest solution
would be to have hot-pluggable and unaccepted indicated in EFI memory,
so we can size bitmap accordingly upfront.
> Extend the unaccepted memory framework to handle hotplugged memory by
> dynamically managing the unaccepted bitmap. Allocate a new bitmap when
> hotplugged ranges exceed the reserved bitmap capacity and switch to
> kernel-managed allocation.
>
> Hotplugged memory also follows the same acceptance policy using the
> accept_memory=[eager|lazy] kernel parameter to accept memory either
> up-front when added or before first use.
>
> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
> ---
> arch/x86/boot/compressed/efi.h | 1 +
> .../firmware/efi/libstub/unaccepted_memory.c | 1 +
> drivers/firmware/efi/unaccepted_memory.c | 83 +++++++++++++++++++
> include/linux/efi.h | 1 +
> include/linux/mm.h | 11 +++
> mm/memory_hotplug.c | 7 ++
> mm/page_alloc.c | 2 +
> 7 files changed, 106 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
> index 4f7027f33def..a220a1966cae 100644
> --- a/arch/x86/boot/compressed/efi.h
> +++ b/arch/x86/boot/compressed/efi.h
> @@ -102,6 +102,7 @@ struct efi_unaccepted_memory {
> u32 unit_size;
> u64 phys_base;
> u64 size;
> + bool mem_reserved;
> unsigned long *bitmap;
> };
>
Again, this is ABI break for kexec.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH 1/4] efi/libstub: Decouple memory bitmap from the unaccepted table
From: Kiryl Shutsemau @ 2025-11-26 11:08 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <20251125175753.1428857-2-prsampat@amd.com>
On Tue, Nov 25, 2025 at 11:57:50AM -0600, Pratik R. Sampat wrote:
> Memory hotplug in secure environments requires the unaccepted memory
> bitmap to grow as new memory is added. Currently, the bitmap is
> implemented as a flexible array member at the end of struct
> efi_unaccepted_memory, which is reserved by memblock at boot and cannot
> be resized without reallocating the entire structure.
>
> Replace the flexible array member with a pointer.
Well, it break interoperability between kernel before and after the
patch. Consider kexec from kernel without the patch to the kernel with
the patch and then back to older kernel. It is ABI break.
Is re-allocating the entire structure such a big pain?
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v4 16/16] Documentation/x86: Add documentation for TDX's Dynamic PAMT
From: Binbin Wu @ 2025-11-26 10:33 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, seanjc, tglx,
vannapurve, x86, yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-17-rick.p.edgecombe@intel.com>
On 11/21/2025 8:51 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Expand TDX documentation to include information on the Dynamic PAMT
> feature.
>
> The new section explains PAMT support in the TDX module and how Dynamic
> PAMT affects the kernel memory use.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [Add feedback, update log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> v3:
> - Trim down docs to be about things that user cares about, instead
> of development history and other details like this.
> ---
> Documentation/arch/x86/tdx.rst | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
> index 61670e7df2f7..8d45d31fee29 100644
> --- a/Documentation/arch/x86/tdx.rst
> +++ b/Documentation/arch/x86/tdx.rst
> @@ -99,6 +99,27 @@ initialize::
>
> [..] virt/tdx: module initialization failed ...
>
> +Dynamic PAMT
> +------------
> +
> +PAMT is memory that the TDX module needs to keep data about each page
> +(think like struct page). It needs to handed to the TDX module for its
^
be
> +exclusive use. For normal PAMT, this is installed when the TDX module
> +is first loaded and comes to about 0.4% of system memory.
> +
> +Dynamic PAMT is a TDX feature that allows VMM to allocate part of the
> +PAMT as needed (the parts for tracking 4KB size pages). The other page
> +sizes (1GB and 2MB) are still allocated statically at the time of
> +TDX module initialization. This reduces the amount of memory that TDX
> +uses while TDs are not in use.
> +
> +When Dynamic PAMT is in use, dmesg shows it like:
> + [..] virt/tdx: Enable Dynamic PAMT
> + [..] virt/tdx: 10092 KB allocated for PAMT
> + [..] virt/tdx: module initialized
> +
> +Dynamic PAMT is enabled automatically if supported.
> +
> TDX Interaction to Other Kernel Components
> ------------------------------------------
>
^ permalink raw reply
* [PATCH 2/2] x86/split_lock: Describe #AC handling in TDX guest kernel log
From: Xiaoyao Li @ 2025-11-26 10:02 UTC (permalink / raw)
To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Kiryl Shutsemau
Cc: x86, H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco,
kvm, Reinette Chatre, Chenyi Qiang, chao.p.peng, xiaoyao.li
In-Reply-To: <20251126100205.1729391-1-xiaoyao.li@intel.com>
X86_FEATURE_HYPERVISOR and X86_FEATURE_BUS_LOCK_DETECT are always
enumerated in a TDX guest because the corresponding CPUID values are
fixed to 1 by the TDX module. Similar to a normal guest, a TDX guest
never enumerates X86_FEATURE_SPLIT_LOCK_DETECT.
When "split_lock_detect=off", the TDX guest kernel log shows:
x86/split lock detection: disabled
and with other settings, it shows:
x86/split lock detection: #DB: ...
However, if the host enables split lock detection, a TDX guest receives
#AC regardless of its own "split_lock_detect" configuration. The actual
behavior does not match what the kernel log claims.
Call out the possible #AC behavior on TDX and highlight that this behavior
depends on the host's enabling of split lock detection.
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kernel/cpu/bus_lock.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
index f278e4ea3dd4..18695214d214 100644
--- a/arch/x86/kernel/cpu/bus_lock.c
+++ b/arch/x86/kernel/cpu/bus_lock.c
@@ -437,6 +437,9 @@ static void sld_state_show(void)
pr_info("#DB: setting system wide bus lock rate limit to %u/sec\n", bld_ratelimit.burst);
break;
}
+
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ pr_info("tdx: #AC depends on host configuration: crashing the kernel on kernel split_locks and sending SIGBUS on user-space split_locks\n");
}
void __init sld_setup(struct cpuinfo_x86 *c)
--
2.43.0
^ permalink raw reply related
* [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Xiaoyao Li @ 2025-11-26 10:02 UTC (permalink / raw)
To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Kiryl Shutsemau
Cc: x86, H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco,
kvm, Reinette Chatre, Chenyi Qiang, chao.p.peng, xiaoyao.li
In-Reply-To: <20251126100205.1729391-1-xiaoyao.li@intel.com>
When the host enables split lock detection feature, the split lock from
guests (normal or TDX) triggers #AC. The #AC caused by split lock access
within a normal guest triggers a VM Exit and is handled in the host.
The #AC caused by split lock access within a TDX guest does not trigger
a VM Exit and instead it's delivered to the guest self.
The default "warning" mode of handling split lock depends on being able
to temporarily disable detection to recover from the split lock event.
But the MSR that disables detection is not accessible to a guest.
This means that TDX guests today can not disable the feature or use
the "warning" mode (which is the default). But, they can use the "fatal"
mode.
Force TDX guests to use the "fatal" mode.
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kernel/cpu/bus_lock.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
index 981f8b1f0792..f278e4ea3dd4 100644
--- a/arch/x86/kernel/cpu/bus_lock.c
+++ b/arch/x86/kernel/cpu/bus_lock.c
@@ -315,9 +315,24 @@ void bus_lock_init(void)
wrmsrq(MSR_IA32_DEBUGCTLMSR, val);
}
+static bool split_lock_fatal(void)
+{
+ if (sld_state == sld_fatal)
+ return true;
+
+ /*
+ * TDX guests can not disable split lock detection.
+ * Force them into the fatal behavior.
+ */
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return true;
+
+ return false;
+}
+
bool handle_user_split_lock(struct pt_regs *regs, long error_code)
{
- if ((regs->flags & X86_EFLAGS_AC) || sld_state == sld_fatal)
+ if ((regs->flags & X86_EFLAGS_AC) || split_lock_fatal())
return false;
split_lock_warn(regs->ip);
return true;
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] x86/split_lock: Fix and enhancement for TDX guest
From: Xiaoyao Li @ 2025-11-26 10:02 UTC (permalink / raw)
To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Kiryl Shutsemau
Cc: x86, H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco,
kvm, Reinette Chatre, Chenyi Qiang, chao.p.peng, xiaoyao.li
Running a split lock test[1] inside a TDX guest under KVM triggers the
warning below. The test hangs but can be terminated.
x86/split lock detection: #AC: split_lock/1176 took a split_lock trap at address: 0x5630b30921f9
unchecked MSR access error: WRMSR to 0x33 (tried to write 0x0000000000000000) at rIP: 0xffffffff812a061f (native_write_msr+0xf/0x30)
Call Trace:
handle_user_split_lock
exc_alignment_check
asm_exc_alignment_check
It turns out that split lock detection is enabled (by the host) when the
TDX vCPU is running, and #AC is not intercepted but delivered directly to
the TDX guest. The default "warning" mode of split lock #AC handler in
the guest tries to handle the split lock by temporarily disabling
detection. However, the MSR that disables detection is not accessible to
a guest.
Patch 1 forces the TDX guest to always treat the split lock #AC as the
"fatal" mode. This prevents the TDX guest from attempting invalid MSR
writes.
Patch 2 enhances the sld_state_show() to indicate that the TDX guest can
receive #AC on split locks depending on the host's split lock detection
configuration.
Note that all the split lock issues on TDX guests are due to the
non-architectural behavior of TDX: a TDX guest can receive #AC even
though the split lock detection feature is not available and the
relevant MSR is not accessible.
One option is to make the behavior architectural for TDX guests by not
delivering the (unexpected) #AC to the TDX guest and letting the host
handle it instead. This is exactly how KVM handles split lock #AC for
normal VMs. This option also has the advantage that the TDX guest can
survive from split locks when the host mode is not fatal.
However, this option cannot replace current patches because it changes
the behavior of current TDX and would need to be opted-in by the host VMM
for compatibility. More importantly, it would be a new feature available
only in newer TDX modules, which means all existing TDX modules
cannot benefit from it.
We list the option here as an open to solicit feedback and determine
whether to pursue adding such feature to TDX module.
[1] https://github.com/xiaoyaoli-intel/splitlock/blob/main/splitlock.c
Xiaoyao Li (2):
x86/split_lock: Don't try to handle user split lock in TDX guest
x86/split_lock: Describe #AC handling in TDX guest kernel log
arch/x86/kernel/cpu/bus_lock.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
base-commit: ac3fd01e4c1efce8f2c054cdeb2ddd2fc0fb150d
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4 14/16] KVM: TDX: Reclaim PAMT memory
From: Binbin Wu @ 2025-11-26 8:53 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, seanjc, tglx,
vannapurve, x86, yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-15-rick.p.edgecombe@intel.com>
On 11/21/2025 8:51 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Call tdx_free_page() and tdx_pamt_put() on the paths that free TDX
> pages.
>
> 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;
>
> Add tdx_pamt_put() for memory that comes from guest_memfd and use
> tdx_free_page() for the rest.
External page table pages are not from guest_memfd, but tdx_pamt_put() is used
in tdx_sept_free_private_spt() for them.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [Minor log tweak]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> v4:
> - Rebasing on post-populate series required some changes on how PAMT
> refcounting was handled in the KVM_TDX_INIT_MEM_REGION path. Now
> instead of incrementing DPAMT refcount on the fake add in the fault
> path, it only increments it when tdh_mem_page_add() actually succeeds,
> like in tdx_mem_page_aug(). Because of this, the special handling for
> the case tdx_is_sept_zap_err_due_to_premap() cared about is unneeded.
>
> v3:
> - Minor log tweak to conform kvm/x86 style.
> ---
> arch/x86/kvm/vmx/tdx.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 24322263ac27..f8de50e7dc7f 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -360,7 +360,7 @@ static void tdx_reclaim_control_page(struct page *ctrl_page)
> if (tdx_reclaim_page(ctrl_page))
> return;
>
> - __free_page(ctrl_page);
> + tdx_free_page(ctrl_page);
> }
>
> struct tdx_flush_vp_arg {
> @@ -597,7 +597,7 @@ static void tdx_reclaim_td_control_pages(struct kvm *kvm)
>
> tdx_quirk_reset_page(kvm_tdx->td.tdr_page);
>
> - __free_page(kvm_tdx->td.tdr_page);
> + tdx_free_page(kvm_tdx->td.tdr_page);
> kvm_tdx->td.tdr_page = NULL;
> }
>
> @@ -1827,6 +1827,8 @@ static int tdx_sept_free_private_spt(struct kvm *kvm, gfn_t gfn,
> enum pg_level level, void *private_spt)
> {
> struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> + struct page *page = virt_to_page(private_spt);
> + int ret;
>
> /*
> * free_external_spt() is only called after hkid is freed when TD is
> @@ -1843,7 +1845,12 @@ static int tdx_sept_free_private_spt(struct kvm *kvm, gfn_t gfn,
> * 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));
> + ret = tdx_reclaim_page(virt_to_page(private_spt));
> + if (ret)
> + return ret;
> +
> + tdx_pamt_put(page);
> + return 0;
> }
>
> static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> @@ -1895,6 +1902,7 @@ static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> return;
>
> tdx_quirk_reset_page(page);
> + tdx_pamt_put(page);
> }
>
> void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
^ permalink raw reply
* Re: [PATCH v4 13/16] KVM: TDX: Handle PAMT allocation in fault path
From: Binbin Wu @ 2025-11-26 5:56 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, seanjc, tglx,
vannapurve, x86, yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-14-rick.p.edgecombe@intel.com>
On 11/21/2025 8:51 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Install PAMT pages for TDX call backs called during the fault path.
>
> There are two distinct cases when the kernel needs to allocate PAMT memory
> in the fault path: for SEPT page tables in tdx_sept_link_private_spt() and
> for leaf pages in tdx_sept_set_private_spte().
>
> These code paths run in atomic context. Previous changes have made the
> fault path top up the per-VCPU pool for memory allocations. Use it to do
> tdx_pamt_get/put() for the fault path operations.
>
> In the generic MMU these ops are inside functions that don’t always
> operate from the vCPU contexts (for example zap paths), which means they
> don’t have a struct kvm_vcpu handy. But for TDX they are always in a vCPU
> context. Since the pool of pre-allocated pages is on the vCPU, use
> kvm_get_running_vcpu() to get the vCPU. In case a new path appears where
> this is not the case, leave some KVM_BUG_ON()’s.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [Add feedback, update log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> v4:
> - Do prealloc.page_list initialization in tdx_td_vcpu_init() in case
> userspace doesn't call KVM_TDX_INIT_VCPU.
>
> v3:
> - Use new pre-allocation method
> - Updated log
> - Some extra safety around kvm_get_running_vcpu()
> ---
> arch/x86/kvm/vmx/tdx.c | 44 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 61a058a8f159..24322263ac27 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -683,6 +683,8 @@ int tdx_vcpu_create(struct kvm_vcpu *vcpu)
> if (!irqchip_split(vcpu->kvm))
> return -EINVAL;
>
> + INIT_LIST_HEAD(&tdx->prealloc.page_list);
> +
Should this change be moved to patch 12?
Because the pre-alloc page list has started to be used in patch 12 for external
page tables even without enabling dynamic PAMT.
> fpstate_set_confidential(&vcpu->arch.guest_fpu);
> vcpu->arch.apic->guest_apic_protected = true;
> INIT_LIST_HEAD(&tdx->vt.pi_wakeup_list);
>
[...]
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Binbin Wu @ 2025-11-26 5:21 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, seanjc, tglx,
vannapurve, x86, yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <7a6f5b4e-ad7b-4ad0-95fd-e1698f9b4e06@linux.intel.com>
On 11/26/2025 11:40 AM, Binbin Wu wrote:
>> index 260bb0e6eb44..61a058a8f159 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -1644,23 +1644,34 @@ static int tdx_mem_page_add(struct kvm *kvm, gfn_t gfn, enum pg_level level,
>> static void *tdx_alloc_external_fault_cache(struct kvm_vcpu *vcpu)
>> {
>> - struct vcpu_tdx *tdx = to_tdx(vcpu);
>> + struct page *page = get_tdx_prealloc_page(&to_tdx(vcpu)->prealloc);
>> - return kvm_mmu_memory_cache_alloc(&tdx->mmu_external_spt_cache);
>> + if (WARN_ON_ONCE(!page))
>> + return (void *)__get_free_page(GFP_ATOMIC | __GFP_ACCOUNT);
>
> kvm_mmu_memory_cache_alloc() calls BUG_ON() if the atomic allocation failed.
> Do we want to follow?
>
>
Please ignore this one.
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Binbin Wu @ 2025-11-26 3:40 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, seanjc, tglx,
vannapurve, x86, yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-13-rick.p.edgecombe@intel.com>
On 11/21/2025 8:51 AM, Rick Edgecombe wrote:
> In the KVM fault path page, tables and private pages need to be
"In the KVM fault path page, tables ..." should be
"In the KVM fault path, page tables ..."
> installed under a spin lock. This means that the operations around
> installing PAMT pages for them will not be able to allocate pages.
>
[...]
> @@ -141,7 +142,46 @@ int tdx_guest_keyid_alloc(void);
> u32 tdx_get_nr_guest_keyids(void);
> void tdx_guest_keyid_free(unsigned int keyid);
>
> -int tdx_pamt_get(struct page *page);
> +int tdx_dpamt_entry_pages(void);
> +
> +/*
> + * Simple structure for pre-allocating Dynamic
> + * PAMT pages outside of locks.
It's not just for Dynamic PAMT pages, but also external page table pages.
> + */
> +struct tdx_prealloc {
> + struct list_head page_list;
> + int cnt;
> +};
> +
> +static inline struct page *get_tdx_prealloc_page(struct tdx_prealloc *prealloc)
> +{
> + struct page *page;
> +
> + page = list_first_entry_or_null(&prealloc->page_list, struct page, lru);
> + if (page) {
> + list_del(&page->lru);
> + prealloc->cnt--;
> + }
> +
> + return page;
> +}
> +
> +static inline int topup_tdx_prealloc_page(struct tdx_prealloc *prealloc, unsigned int min_size)
> +{
> + while (prealloc->cnt < min_size) {
> + struct page *page = alloc_page(GFP_KERNEL_ACCOUNT);
> +
> + if (!page)
> + return -ENOMEM;
> +
> + list_add(&page->lru, &prealloc->page_list);
> + prealloc->cnt++;
> + }
> +
> + return 0;
> +}
> +
> +int tdx_pamt_get(struct page *page, struct tdx_prealloc *prealloc);
> void tdx_pamt_put(struct page *page);
>
> struct page *tdx_alloc_page(void);
> @@ -219,6 +259,7 @@ static inline int tdx_enable(void) { return -ENODEV; }
> static inline u32 tdx_get_nr_guest_keyids(void) { return 0; }
> static inline const char *tdx_dump_mce_info(struct mce *m) { return NULL; }
> static inline const struct tdx_sys_info *tdx_get_sysinfo(void) { return NULL; }
> +static inline int tdx_dpamt_entry_pages(void) { return 0; }
> #endif /* CONFIG_INTEL_TDX_HOST */
>
> #ifdef CONFIG_KEXEC_CORE
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 260bb0e6eb44..61a058a8f159 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1644,23 +1644,34 @@ static int tdx_mem_page_add(struct kvm *kvm, gfn_t gfn, enum pg_level level,
>
> static void *tdx_alloc_external_fault_cache(struct kvm_vcpu *vcpu)
> {
> - struct vcpu_tdx *tdx = to_tdx(vcpu);
> + struct page *page = get_tdx_prealloc_page(&to_tdx(vcpu)->prealloc);
>
> - return kvm_mmu_memory_cache_alloc(&tdx->mmu_external_spt_cache);
> + if (WARN_ON_ONCE(!page))
> + return (void *)__get_free_page(GFP_ATOMIC | __GFP_ACCOUNT);
kvm_mmu_memory_cache_alloc() calls BUG_ON() if the atomic allocation failed.
Do we want to follow?
> +
> + return page_address(page);
> }
>
> static int tdx_topup_external_fault_cache(struct kvm_vcpu *vcpu, unsigned int cnt)
> {
> - struct vcpu_tdx *tdx = to_tdx(vcpu);
> + struct tdx_prealloc *prealloc = &to_tdx(vcpu)->prealloc;
> + int min_fault_cache_size;
>
> - return kvm_mmu_topup_memory_cache(&tdx->mmu_external_spt_cache, cnt);
> + /* External page tables */
> + min_fault_cache_size = cnt;
> + /* Dynamic PAMT pages (if enabled) */
> + min_fault_cache_size += tdx_dpamt_entry_pages() * PT64_ROOT_MAX_LEVEL;
Is the value PT64_ROOT_MAX_LEVEL intended, since dynamic PAMT pages are only
needed for 4KB level?
> +
> + return topup_tdx_prealloc_page(prealloc, min_fault_cache_size);
> }
>
>
[...]
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Huang, Kai @ 2025-11-26 1:21 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Zhao, Yan Y, Wu, Binbin, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, Yamahata, Isaku, tglx@linutronix.de,
Annapurve, Vishal, Gao, Chao, Edgecombe, Rick P, bp@alien8.de,
x86@kernel.org
Cc: kirill.shutemov@linux.intel.com
In-Reply-To: <20251121005125.417831-8-rick.p.edgecombe@intel.com>
On Thu, 2025-11-20 at 16:51 -0800, Rick Edgecombe wrote:
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index cf51ccd16194..914213123d94 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -135,11 +135,17 @@ static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> return false; /* To be enabled when kernel is ready */
> }
>
> +void tdx_quirk_reset_page(struct page *page);
> +
> 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);
I don't think it's mandatory to move the declaration of tdx_quirk_reset_page()?
^ permalink raw reply
* Re: [PATCH v4 02/16] x86/tdx: Add helpers to check return status codes
From: Huang, Kai @ 2025-11-25 23:07 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Zhao, Yan Y, Wu, Binbin, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, Yamahata, Isaku, tglx@linutronix.de,
Annapurve, Vishal, Gao, Chao, Edgecombe, Rick P, bp@alien8.de,
x86@kernel.org
Cc: kirill.shutemov@linux.intel.com
In-Reply-To: <20251121005125.417831-3-rick.p.edgecombe@intel.com>
On Thu, 2025-11-20 at 16:51 -0800, Rick Edgecombe wrote:
> diff --git a/arch/x86/include/asm/shared/tdx_errno.h b/arch/x86/include/asm/shared/tdx_errno.h
> index 3aa74f6a6119..e302aed31b50 100644
> --- a/arch/x86/include/asm/shared/tdx_errno.h
> +++ b/arch/x86/include/asm/shared/tdx_errno.h
> @@ -5,7 +5,7 @@
> #include <asm/trapnr.h>
>
> /* Upper 32 bit of the TDX error code encodes the status */
> -#define TDX_SEAMCALL_STATUS_MASK 0xFFFFFFFF00000000ULL
> +#define TDX_STATUS_MASK 0xFFFFFFFF00000000ULL
>
> /*
> * TDX SEAMCALL Status Codes
> @@ -54,4 +54,49 @@
> #define TDX_OPERAND_ID_SEPT 0x92
> #define TDX_OPERAND_ID_TD_EPOCH 0xa9
>
> +#ifndef __ASSEMBLER__
> +#include <linux/bits.h>
> +#include <linux/types.h>
IMHO:
You might want to move #include <linux/bits.h> out of __ASSEMBLER__ to the top
of this file since macros like GENMASK_ULL() are used by SW-defined error codes
already. And you might want to move the inclusion of this header to the
previous patch when these error codes were moved to <asm/shared/tdx_errno.h>.
You may also move <linux/types.h> out of __ASSEMBLER__ since AFAICT this file is
assembly inclusion safe.
> +
> +static inline u64 TDX_STATUS(u64 err)
> +{
> + return err & TDX_STATUS_MASK;
> +}
> +
^ permalink raw reply
* Re: [PATCH v4 01/16] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Huang, Kai @ 2025-11-25 22:44 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Zhao, Yan Y, Wu, Binbin, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
tglx@linutronix.de, Yamahata, Isaku, linux-kernel@vger.kernel.org,
Annapurve, Vishal, Edgecombe, Rick P, Gao, Chao, bp@alien8.de,
x86@kernel.org
Cc: kirill.shutemov@linux.intel.com
In-Reply-To: <6968dcb446fb857b3f254030e487d889b464d7ce.camel@intel.com>
On Tue, 2025-11-25 at 22:30 +0000, Huang, Kai wrote:
> > /*
> > - * TDX SEAMCALL Status Codes (returned in RAX)
> > + * TDX SEAMCALL Status Codes
>
> Nit:
>
> I don't quite follow this change. Just curious: is it because "returned in RAX"
> doesn't apply to all error codes any more?
Also forgot to say, AFAICT these error codes will also be used by TDX guest,
therefore you might want to drop the "SEAMCALL" part from the status codes.
>
> > */
> > +#define TDX_SUCCESS 0ULL
> > #define TDX_NON_RECOVERABLE_VCPU 0x4000000100000000ULL
> > #define TDX_NON_RECOVERABLE_TD 0x4000000200000000ULL
> > #define TDX_NON_RECOVERABLE_TD_NON_ACCESSIBLE 0x6000000500000000ULL
> > @@ -17,6 +19,7 @@
> > #define TDX_OPERAND_INVALID 0xC000010000000000ULL
> > #define TDX_OPERAND_BUSY 0x8000020000000000ULL
> > #define TDX_PREVIOUS_TLB_EPOCH_BUSY 0x8000020100000000ULL
> > +#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
> > #define TDX_PAGE_METADATA_INCORRECT 0xC000030000000000ULL
> > #define TDX_VCPU_NOT_ASSOCIATED 0x8000070200000000ULL
> > #define TDX_KEY_GENERATION_FAILED 0x8000080000000000ULL
> > @@ -28,6 +31,20 @@
> > #define TDX_EPT_ENTRY_STATE_INCORRECT 0xC0000B0D00000000ULL
> > #define TDX_METADATA_FIELD_NOT_READABLE 0xC0000C0200000000ULL
^ permalink raw reply
* Re: [PATCH v4 01/16] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Huang, Kai @ 2025-11-25 22:30 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Zhao, Yan Y, Wu, Binbin, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, Yamahata, Isaku, tglx@linutronix.de,
Annapurve, Vishal, Gao, Chao, Edgecombe, Rick P, bp@alien8.de,
x86@kernel.org
Cc: kirill.shutemov@linux.intel.com
In-Reply-To: <20251121005125.417831-2-rick.p.edgecombe@intel.com>
> --- a/arch/x86/kvm/vmx/tdx_errno.h
> +++ b/arch/x86/include/asm/shared/tdx_errno.h
> @@ -1,14 +1,16 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> -/* architectural status code for SEAMCALL */
> +#ifndef _X86_SHARED_TDX_ERRNO_H
> +#define _X86_SHARED_TDX_ERRNO_H
AFAICT other files under asm/shared/ all use
#ifdef _ASM_X86_SHARED_xx_H
...
I guess it's better to follow this pattern.
>
> -#ifndef __KVM_X86_TDX_ERRNO_H
> -#define __KVM_X86_TDX_ERRNO_H
> +#include <asm/trapnr.h>
>
> +/* Upper 32 bit of the TDX error code encodes the status */
> #define TDX_SEAMCALL_STATUS_MASK 0xFFFFFFFF00000000ULL
>
> /*
> - * TDX SEAMCALL Status Codes (returned in RAX)
> + * TDX SEAMCALL Status Codes
Nit:
I don't quite follow this change. Just curious: is it because "returned in RAX"
doesn't apply to all error codes any more?
> */
> +#define TDX_SUCCESS 0ULL
> #define TDX_NON_RECOVERABLE_VCPU 0x4000000100000000ULL
> #define TDX_NON_RECOVERABLE_TD 0x4000000200000000ULL
> #define TDX_NON_RECOVERABLE_TD_NON_ACCESSIBLE 0x6000000500000000ULL
> @@ -17,6 +19,7 @@
> #define TDX_OPERAND_INVALID 0xC000010000000000ULL
> #define TDX_OPERAND_BUSY 0x8000020000000000ULL
> #define TDX_PREVIOUS_TLB_EPOCH_BUSY 0x8000020100000000ULL
> +#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
> #define TDX_PAGE_METADATA_INCORRECT 0xC000030000000000ULL
> #define TDX_VCPU_NOT_ASSOCIATED 0x8000070200000000ULL
> #define TDX_KEY_GENERATION_FAILED 0x8000080000000000ULL
> @@ -28,6 +31,20 @@
> #define TDX_EPT_ENTRY_STATE_INCORRECT 0xC0000B0D00000000ULL
> #define TDX_METADATA_FIELD_NOT_READABLE 0xC0000C0200000000ULL
>
> +/*
> + * SW-defined error codes.
> + *
> + * Bits 47:40 == 0xFF indicate Reserved status code class that never used by
> + * TDX module.
> + */
> +#define TDX_ERROR _BITULL(63)
> +#define TDX_NON_RECOVERABLE _BITULL(62)
> +#define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40))
> +#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _ULL(0xFFFF0000))
> +
> +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP)
> +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)
> +
> /*
> * TDX module operand ID, appears in 31:0 part of error code as
> * detail information
> @@ -37,4 +54,4 @@
> #define TDX_OPERAND_ID_SEPT 0x92
> #define TDX_OPERAND_ID_TD_EPOCH 0xa9
>
> -#endif /* __KVM_X86_TDX_ERRNO_H */
> +#endif /* _X86_SHARED_TDX_ERRNO_H */
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 6b338d7f01b7..2f3e16b93b4c 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -12,26 +12,6 @@
> #include <asm/trapnr.h>
I think you can remove <asm/trapnr.h> here since now <asm/tdx.h> is no longer
using any definitions from it. And <asm/shared/tdx_errno.h> now includes it
directly.
^ permalink raw reply
* Re: [PATCH v4 00/16] TDX: Enable Dynamic PAMT
From: Vishal Annapurve @ 2025-11-25 20:19 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, seanjc, tglx, x86,
yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-1-rick.p.edgecombe@intel.com>
On Thu, Nov 20, 2025 at 4:51 PM Rick Edgecombe
<rick.p.edgecombe@intel.com> wrote:
>
> Hi,
>
> This is 4th revision of Dynamic PAMT, which is a new feature that reduces
> the memory use of TDX. For background information, see the v3
> coverletter[0]. V4 mostly consists of incorporating the feedback from v3.
> Notably what it *doesn’t* change is the solution for pre-allocating DPAMT
> backing pages. (more info below in “Changes”)
>
> I think this series isn’t quite ready to ask for merging just yet. I’d
> appreciate another round of review especially looking for any issues in the
> refcount allocation/mapping and the pamt get/put lock-refcount dance. And
> hopefully collect some RBs.
>
> Sean/Paolo, we have mostly banished this all to TDX code except for “KVM:
> TDX: Add x86 ops for external spt cache” patch. That and “x86/virt/tdx:
> Add helpers to allow for pre-allocating pages” are probably the remaining
> possibly controversial parts of your domain. If you only have a short time
> to spend at this point, I’d point you at those two patches.
>
> Since most of the changes are in arch/x86, I’d think this feature could be
> a candidate for eventually merging through tip with Sean or Paolo’s ack.
> But it is currently based on kvm-x86/next in order to build on top of the
> post-populate cleanup series. Next time I’ll probably target tip if people
> think that is a good way forward.
>
> Changes
> =======
> There were two good suggestions around the pre-allocated pages solution
> last time, but neither ended up working out:
>
> 1. Dave suggested to use mempool_t instead of the linked list based
> structure, in order to not re-invent the wheel. This turned out to not
> quite fit. The problems were that there wasn’t really a “topup” mechanism,
> or an atomic fallback (which matches the kvm cache behavior). This results
> in very similar code being built around mempool that was built around the
> linked list. It was an overall harder to follow solution for not much code
> savings.
>
> I strongly considered going back to Kiryl’s original solution which passed
> a callback function pointer for allocating DPAMT pages, and an opaque
> void * that the callback could use to find the kvm_mmu_memory_cache. I
> thought that readability issues of passing the opaque void * between
> subsystems outweighed the small code duplication in the simple, familiar
> patterned linked list-based code. So I ended up leaving it.
>
> 2. Kai suggested (but later retracted the idea) that since the external
> page table cache was moved to TDX code, it could simply install DPAMT
> pages for the cache at topup time. Then the installation of DPAMT backing
> for S-EPT page tables could be done outside of the mmu_lock. It could also
> be argued that it makes the design simpler in a way, because the external
> page table cache acts like it did before. Anything in there could be simply
> used.
>
> At the time my argument against this was that whether a huge page would be
> installed (and thus, whether DPAMT backing was needed) for the guest
> private memory would not be known until later, so early install solution
> would need special late handling for TDX huge pages. After some internal
> discussions I at looked how we could simplify the series by punting on TDX
> huge pages needs.
>
> But it turns out that this other design was actually more complex and had
> more LOC than the previous solution. So it was dropped, and again, I went
> back to the original solution.
>
>
> I’m really starting to think that, while the overall solution here isn’t
> the most elegant, we might not have much more to squeeze from it. So
> design-wise, I think we should think about calling it done.
>
> Testing
> =======
> Based on kvm-x86/next (4531ff85d925). Testing was the usual, except I also
> tested with TDX modules that don't support DPAMT, and with the two
> optimization patches removed: “Improve PAMT refcounters allocation for
> sparse memory” and “x86/virt/tdx: Optimize tdx_alloc/free_page() helpers”.
>
> [0] https://lore.kernel.org/kvm/20250918232224.2202592-1-rick.p.edgecombe@intel.com/
>
>
> Kirill A. Shutemov (13):
> x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
> x86/tdx: Add helpers to check return status codes
> x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
> x86/virt/tdx: Allocate reference counters for PAMT memory
> x86/virt/tdx: Improve PAMT refcounts allocation for sparse memory
> x86/virt/tdx: Add tdx_alloc/free_page() helpers
> x86/virt/tdx: Optimize tdx_alloc/free_page() helpers
> KVM: TDX: Allocate PAMT memory for TD control structures
> KVM: TDX: Allocate PAMT memory for vCPU control structures
> KVM: TDX: Handle PAMT allocation in fault path
> KVM: TDX: Reclaim PAMT memory
> x86/virt/tdx: Enable Dynamic PAMT
> Documentation/x86: Add documentation for TDX's Dynamic PAMT
>
> Rick Edgecombe (3):
> x86/virt/tdx: Simplify tdmr_get_pamt_sz()
> KVM: TDX: Add x86 ops for external spt cache
> x86/virt/tdx: Add helpers to allow for pre-allocating pages
>
> Documentation/arch/x86/tdx.rst | 21 +
> arch/x86/coco/tdx/tdx.c | 10 +-
> arch/x86/include/asm/kvm-x86-ops.h | 3 +
> arch/x86/include/asm/kvm_host.h | 14 +-
> arch/x86/include/asm/shared/tdx.h | 8 +
> arch/x86/include/asm/shared/tdx_errno.h | 104 ++++
> arch/x86/include/asm/tdx.h | 78 ++-
> arch/x86/include/asm/tdx_global_metadata.h | 1 +
> arch/x86/kvm/mmu/mmu.c | 6 +-
> arch/x86/kvm/mmu/mmu_internal.h | 2 +-
> arch/x86/kvm/vmx/tdx.c | 160 ++++--
> arch/x86/kvm/vmx/tdx.h | 3 +-
> arch/x86/kvm/vmx/tdx_errno.h | 40 --
> arch/x86/virt/vmx/tdx/tdx.c | 587 +++++++++++++++++---
> arch/x86/virt/vmx/tdx/tdx.h | 5 +-
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 7 +
> 16 files changed, 854 insertions(+), 195 deletions(-)
> create mode 100644 arch/x86/include/asm/shared/tdx_errno.h
> delete mode 100644 arch/x86/kvm/vmx/tdx_errno.h
>
> --
> 2.51.2
>
acked-by: Vishal Annapurve <vannapurve@google.com>.
^ permalink raw reply
* SVSM Development Call November 26th, 2025
From: Jörg Rödel @ 2025-11-25 18:40 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the usual call for agenda items for the this weeks SVSM
development call. Please send me any agenda items you have in mind or
raise them in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* [RFC PATCH 4/4] mm: Add support for unaccepted memory hot-remove
From: Pratik R. Sampat @ 2025-11-25 17:57 UTC (permalink / raw)
To: linux-mm, linux-coco, linux-efi, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, david, osalvador,
thomas.lendacky, michael.roth, prsampat
In-Reply-To: <20251125175753.1428857-1-prsampat@amd.com>
Transition memory to shared during a hot-remove operation so that it can
be re-used by the hypervisor. During lazy acceptance, only memory that
was used has been accepted, therefore during hot-remove only mark pages
as shared that were previously accepted / made private.
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
arch/x86/coco/sev/core.c | 23 +++++++++++++++
arch/x86/include/asm/sev.h | 2 ++
arch/x86/include/asm/unaccepted_memory.h | 9 ++++++
drivers/firmware/efi/unaccepted_memory.c | 37 ++++++++++++++++++++++++
include/linux/mm.h | 7 +++++
mm/memory_hotplug.c | 2 ++
6 files changed, 80 insertions(+)
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index a5c9615a6e0c..c05fc91d10a1 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -621,6 +621,29 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE, 0);
}
+void snp_unaccept_memory(phys_addr_t start, phys_addr_t end)
+{
+ unsigned long vaddr, npages;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return;
+
+ vaddr = (unsigned long)__va(start);
+ npages = (end - start) >> PAGE_SHIFT;
+
+ /*
+ * Hotplugged memory can be set to shared externally. Attempting to
+ * re-share the memory (during hot-remove) will cause the pvalidate
+ * operation to not make any changes to the RMP table triggering the
+ * PVALIDATE_FAIL_NOUPDATE condition
+ *
+ * Since the memory hotplug case is unique, specify this intent so that
+ * if the page is part of hotplugged memory a pvalidate rescind
+ * operation is not performed
+ */
+ set_pages_state(vaddr, npages, SNP_PAGE_STATE_SHARED, SNP_PSC_SHARED_TO_SHARED);
+}
+
int snp_extend_hotplug_memory_state_bitmap(phys_addr_t start,
unsigned long size,
uint64_t unit_size)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index eb605892645c..8f3c5b878fd7 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -547,6 +547,7 @@ void __noreturn snp_abort(void);
void snp_dmi_setup(void);
int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call, struct svsm_attest_call *input);
void snp_accept_memory(phys_addr_t start, phys_addr_t end);
+void snp_unaccept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
@@ -639,6 +640,7 @@ static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
return -ENOTTY;
}
static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
+static inline void snp_unaccept_memory(phys_addr_t start, phys_addr_t end) { }
static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
static inline u64 sev_get_status(void) { return 0; }
static inline void sev_show_status(void) { }
diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
index abdf5472de9e..ad392294b71b 100644
--- a/arch/x86/include/asm/unaccepted_memory.h
+++ b/arch/x86/include/asm/unaccepted_memory.h
@@ -18,6 +18,15 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
}
}
+static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
+{
+ if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
+ snp_unaccept_memory(start, end);
+ } else {
+ panic("Cannot accept memory: unknown platform\n");
+ }
+}
+
static inline struct efi_unaccepted_memory *efi_get_unaccepted_table(void)
{
if (efi.unaccepted == EFI_INVALID_TABLE_ADDR)
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index 6796042a64aa..662cf0d6715f 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -301,6 +301,43 @@ int accept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_s
return 0;
}
+void unaccept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_size)
+{
+ u64 unit_size, phys_base, bit_start, bit_end, addr;
+ struct efi_unaccepted_memory *unacc_tbl;
+ unsigned long flags, *bitmap;
+ phys_addr_t start, end;
+ int i;
+
+ unacc_tbl = efi_get_unaccepted_table();
+ if (!unacc_tbl)
+ return;
+
+ phys_base = unacc_tbl->phys_base;
+ unit_size = unacc_tbl->unit_size;
+
+ start = mem_range_start - phys_base;
+ end = (mem_range_start + mem_range_size) - phys_base;
+
+ bit_start = start / unit_size;
+ bit_end = end / unit_size;
+
+ /* Only unaccept memory that was previously accepted in the range */
+ for (i = bit_start; i < bit_end; i++) {
+ spin_lock_irqsave(&unaccepted_memory_lock, flags);
+ bitmap = efi_get_unaccepted_bitmap();
+ if (!bitmap || test_bit(i, bitmap)) {
+ spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
+ continue;
+ }
+
+ addr = phys_base + i * unit_size;
+
+ arch_unaccept_memory(addr, addr + unit_size);
+ spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
+ }
+}
+
#ifdef CONFIG_PROC_VMCORE
static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
unsigned long pfn)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index bb43876e6c47..34d48693dc86 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4079,6 +4079,8 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
void accept_memory(phys_addr_t start, unsigned long size);
int accept_hotplug_memory(phys_addr_t mem_range_start,
unsigned long mem_range_size);
+void unaccept_hotplug_memory(phys_addr_t mem_range_start,
+ unsigned long mem_range_size);
bool mm_lazy_accept_enabled(void);
#else
@@ -4099,6 +4101,11 @@ static inline int accept_hotplug_memory(phys_addr_t mem_range_start,
return 0;
}
+static inline void unaccept_hotplug_memory(phys_addr_t mem_range_start,
+ unsigned long mem_range_size)
+{
+}
+
static inline bool mm_lazy_accept_enabled(void) { return false; }
#endif
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index bf8086682b66..0b14b14e53fe 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -2254,6 +2254,8 @@ static int try_remove_memory(u64 start, u64 size)
mem_hotplug_begin();
+ unaccept_hotplug_memory(start, size);
+
rc = memory_blocks_have_altmaps(start, size);
if (rc < 0) {
mem_hotplug_done();
--
2.51.1
^ permalink raw reply related
* [RFC PATCH 3/4] x86/sev: Introduce hotplug-aware SNP page state validation
From: Pratik R. Sampat @ 2025-11-25 17:57 UTC (permalink / raw)
To: linux-mm, linux-coco, linux-efi, x86, linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, david, osalvador,
thomas.lendacky, michael.roth, prsampat
In-Reply-To: <20251125175753.1428857-1-prsampat@amd.com>
When hot-removing memory in a SEV-SNP environment, pages must be set to
shared state so they can be reused by the hypervisor. This also applies
when memory is intended to be hotplugged back in later, as those pages
will need to be re-accepted after crossing the trust boundary.
However, memory can already be set to shared state externally. In such
cases, the pvalidate rescind operation will not change the validated bit
in the RMP table, setting the carry flag and causing the guest to
terminate.
Since memory hotplug is arguably unique, introduce a guest-maintained
memory state tracking structure that maintains a bitmap to track the
state (private vs shared) of all hotplugged memory supplemented with a
flag to indicate intent. This allows for memory that is already marked
as shared in the hotplug bitmap to avoid performing the pvalidate
rescind operation. Additionally, tracking page state changes from the
guest's perspective, enables the detection of inconsistencies if the
hypervisor changes states unexpectedly. For example, if the guest bitmap
reports memory as private but the hypervisor has already changed the RMP
state to shared, the guest detects this inconsistency when attempting to
share the memory and terminate rather than skipping over the pvalidate
rescind operation.
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
arch/x86/coco/sev/core.c | 104 +++++++++++++++++++++--
arch/x86/include/asm/sev.h | 32 +++++++
arch/x86/include/asm/unaccepted_memory.h | 13 +++
drivers/firmware/efi/unaccepted_memory.c | 2 +-
4 files changed, 143 insertions(+), 8 deletions(-)
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 14ef5908fb27..a5c9615a6e0c 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -46,6 +46,8 @@
#include <asm/cmdline.h>
#include <asm/msr.h>
+struct snp_hotplug_memory *snp_hp_mem;
+
/* AP INIT values as documented in the APM2 section "Processor Initialization State" */
#define AP_INIT_CS_LIMIT 0xffff
#define AP_INIT_DS_LIMIT 0xffff
@@ -453,9 +455,54 @@ static int vmgexit_psc(struct ghcb *ghcb, struct snp_psc_desc *desc)
return ret;
}
+static bool snp_hotplug_state_shared(unsigned long vaddr)
+{
+ phys_addr_t paddr = __pa(vaddr);
+ u64 hotplug_bit;
+
+ if (!snp_is_hotplug_memory(paddr))
+ return false;
+
+ hotplug_bit = (paddr - snp_hp_mem->phys_base) / snp_hp_mem->unit_size;
+
+ return !test_bit(hotplug_bit, snp_hp_mem->bitmap);
+}
+
+static void snp_set_hotplug_bit(unsigned long vaddr, bool private)
+{
+ phys_addr_t paddr = __pa(vaddr);
+ u64 hotplug_bit;
+
+ if (!snp_is_hotplug_memory(paddr))
+ return;
+
+ hotplug_bit = (paddr - snp_hp_mem->phys_base) / snp_hp_mem->unit_size;
+ if (private)
+ set_bit(hotplug_bit, snp_hp_mem->bitmap);
+ else
+ clear_bit(hotplug_bit, snp_hp_mem->bitmap);
+}
+
+static void set_hotplug_pages_state(struct snp_psc_desc *desc)
+{
+ struct psc_entry *e;
+ unsigned long vaddr;
+ bool op;
+ int i;
+
+ for (i = 0; i <= desc->hdr.end_entry; i++) {
+ e = &desc->entries[i];
+ vaddr = (unsigned long)pfn_to_kaddr(e->gfn);
+ op = e->operation == SNP_PAGE_STATE_PRIVATE;
+
+ snp_set_hotplug_bit(vaddr, op);
+ }
+}
+
static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long vaddr,
- unsigned long vaddr_end, int op)
+ unsigned long vaddr_end, int op, u8 psc_flags)
{
+ unsigned long vaddr_base;
struct ghcb_state state;
bool use_large_entry;
struct psc_hdr *hdr;
@@ -465,6 +512,7 @@ static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long
struct ghcb *ghcb;
int i;
+ vaddr_base = vaddr;
hdr = &data->hdr;
e = data->entries;
@@ -499,7 +547,8 @@ static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long
}
/* Page validation must be rescinded before changing to shared */
- if (op == SNP_PAGE_STATE_SHARED)
+ if (op == SNP_PAGE_STATE_SHARED &&
+ !(snp_hotplug_state_shared(vaddr_base) && (psc_flags & SNP_PSC_SHARED_TO_SHARED)))
pvalidate_pages(data);
local_irq_save(flags);
@@ -522,10 +571,12 @@ static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long
if (op == SNP_PAGE_STATE_PRIVATE)
pvalidate_pages(data);
+ set_hotplug_pages_state(data);
+
return vaddr;
}
-static void set_pages_state(unsigned long vaddr, unsigned long npages, int op)
+static void set_pages_state(unsigned long vaddr, unsigned long npages, int op, u8 psc_flags)
{
struct snp_psc_desc desc;
unsigned long vaddr_end;
@@ -538,7 +589,7 @@ static void set_pages_state(unsigned long vaddr, unsigned long npages, int op)
vaddr_end = vaddr + (npages << PAGE_SHIFT);
while (vaddr < vaddr_end)
- vaddr = __set_pages_state(&desc, vaddr, vaddr_end, op);
+ vaddr = __set_pages_state(&desc, vaddr, vaddr_end, op, psc_flags);
}
void snp_set_memory_shared(unsigned long vaddr, unsigned long npages)
@@ -546,7 +597,7 @@ void snp_set_memory_shared(unsigned long vaddr, unsigned long npages)
if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
return;
- set_pages_state(vaddr, npages, SNP_PAGE_STATE_SHARED);
+ set_pages_state(vaddr, npages, SNP_PAGE_STATE_SHARED, 0);
}
void snp_set_memory_private(unsigned long vaddr, unsigned long npages)
@@ -554,7 +605,7 @@ void snp_set_memory_private(unsigned long vaddr, unsigned long npages)
if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
return;
- set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
+ set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE, 0);
}
void snp_accept_memory(phys_addr_t start, phys_addr_t end)
@@ -567,7 +618,46 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
vaddr = (unsigned long)__va(start);
npages = (end - start) >> PAGE_SHIFT;
- set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
+ set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE, 0);
+}
+
+int snp_extend_hotplug_memory_state_bitmap(phys_addr_t start,
+ unsigned long size,
+ uint64_t unit_size)
+{
+ u64 hp_mem_size = DIV_ROUND_UP(size, unit_size * BITS_PER_BYTE);
+
+ if (snp_hp_mem) {
+ u64 old_size = snp_hp_mem->size;
+ unsigned long *bitmap;
+
+ bitmap = krealloc(snp_hp_mem->bitmap, hp_mem_size, GFP_KERNEL);
+ if (!bitmap)
+ return -ENOMEM;
+
+ memset(bitmap + old_size, 0, hp_mem_size - old_size);
+ snp_hp_mem->size = hp_mem_size;
+ snp_hp_mem->bitmap = bitmap;
+
+ return 0;
+ }
+
+ snp_hp_mem = kzalloc(sizeof(*snp_hp_mem), GFP_KERNEL);
+ if (!snp_hp_mem)
+ return -ENOMEM;
+
+ snp_hp_mem->bitmap = kzalloc(hp_mem_size, GFP_KERNEL);
+ if (!snp_hp_mem->bitmap) {
+ kfree(snp_hp_mem);
+ return -ENOMEM;
+ }
+
+ snp_hp_mem->phys_base = start;
+ snp_hp_mem->phys_end = start + hp_mem_size;
+ snp_hp_mem->size = hp_mem_size;
+ snp_hp_mem->unit_size = unit_size;
+
+ return 0;
}
static int vmgexit_ap_control(u64 event, struct sev_es_save_area *vmsa, u32 apic_id)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 465b19fd1a2d..eb605892645c 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -464,6 +464,38 @@ static __always_inline void sev_es_nmi_complete(void)
extern int __init sev_es_efi_map_ghcbs_cas(pgd_t *pgd);
extern void sev_enable(struct boot_params *bp);
+#define SNP_PSC_SHARED_TO_SHARED 0x1
+
+struct snp_hotplug_memory {
+ u64 phys_base;
+ u64 phys_end;
+ u32 unit_size;
+ u64 size;
+ /* bitmap bit unset: shared, set: private */
+ unsigned long *bitmap;
+};
+
+extern struct snp_hotplug_memory *snp_hp_mem;
+
+#ifdef CONFIG_UNACCEPTED_MEMORY
+int snp_extend_hotplug_memory_state_bitmap(phys_addr_t start,
+ unsigned long size,
+ uint64_t unit_size);
+static inline bool snp_is_hotplug_memory(phys_addr_t paddr)
+{
+ return snp_hp_mem && paddr >= snp_hp_mem->phys_base && paddr < snp_hp_mem->phys_end;
+}
+#else /* !CONFIG_UNACCEPTED_MEMORY */
+static inline int snp_extend_hotplug_memory_state_bitmap(phys_addr_t start,
+ unsigned long size,
+ uint64_t unit_size)
+{
+ return 0;
+}
+
+static inline bool snp_is_hotplug_memory(phys_addr_t paddr) { return false; }
+#endif
+
/*
* RMPADJUST modifies the RMP permissions of a page of a lesser-
* privileged (numerically higher) VMPL.
diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
index 5da80e68d718..abdf5472de9e 100644
--- a/arch/x86/include/asm/unaccepted_memory.h
+++ b/arch/x86/include/asm/unaccepted_memory.h
@@ -33,4 +33,17 @@ static inline unsigned long *efi_get_unaccepted_bitmap(void)
return NULL;
return __va(unaccepted->bitmap);
}
+
+static inline int arch_set_unaccepted_mem_state(phys_addr_t start, unsigned long size)
+{
+ struct efi_unaccepted_memory *unaccepted = efi_get_unaccepted_table();
+
+ if (!unaccepted)
+ return -EIO;
+
+ if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return snp_extend_hotplug_memory_state_bitmap(start, size, unaccepted->unit_size);
+
+ return 0;
+}
#endif
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index 8537812346e2..6796042a64aa 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -281,7 +281,7 @@ static int extend_unaccepted_bitmap(phys_addr_t mem_range_start,
unacc_tbl->bitmap = (unsigned long *)__pa(new_bitmap);
spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
- return 0;
+ return arch_set_unaccepted_mem_state(mem_range_start, mem_range_size);
}
int accept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_size)
--
2.51.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox