* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Sean Christopherson @ 2026-07-07 0:25 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Kai Huang,
Dave Hansen, Yan Y Zhao, kirill.shutemov@linux.intel.com,
kas@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Vishal Annapurve, bp@alien8.de, Chao Gao, x86@kernel.org
In-Reply-To: <b9e2c377a21a53329cd70ac48309981957814686.camel@intel.com>
On Mon, Jul 06, 2026, Rick P Edgecombe wrote:
> On Mon, 2026-07-06 at 16:47 -0700, Sean Christopherson wrote:
> > On Mon, May 25, 2026, Rick Edgecombe wrote:
> > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > >
> > > Use control page helpers for allocating and freeing TD control structures,
> > > such these operations can work for Dynamic PAMT.
> > >
> > > The TDX module tracks some state for each page of physical memory that it
> > > might use. It calls this state the PAMT. It includes separate state for
> > > each page size a physical page could be utilized at within the TDX module
> > > (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> > > allocated dynamically. So the kernel must install PAMT backing for each 4KB
> > > page before gifting it to the TDX module, and tear it down after the page
> > > is reclaimed.
> > >
> > > TD-scoped control pages (TDR, TDCS) and vCPU-scoped control pages (TDVPR,
> > > TDCX) are all handed to the TDX module at 4KB page size and are therefore
> > > subject to this requirement. Replace the raw alloc_page()/__free_page()
> > > calls for these pages with tdx_alloc/free_control_page().
> > >
> > > Switching between special Dynamic PAMT operations or normal page
> > > alloc/free operations is handled internally in
> > > tdx_alloc/free_control_page(). So don't check for Dynamic PAMT around these
> > > calls. Just call them unconditionally. Similarly, drop the NULL checks
> > > before freeing, as tdx_free_control_page() handles NULL internally.
> > >
> > > No functional change intended when Dynamic PAMT is not in use.
> > >
> > > Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> >
> > Exactly what assistance was given, and when? I certainly didn't use any of these
> > tools, and given that Claude Opus 4.6 was released after I posted the v5 RFC, I
> > doubt Kirill did either.
>
> On this patch specifically, lots of "review this patch" type experiments IIRC.
> Probably mostly around the log.
>
> >
> > And in my strong opinion, even if AI tooling was used to rebase the patches, I
> > don't think that level of "assistance" should be presented this way. E.g. I would
> > rather a more informal:
> >
> > [ Rick: enhance log, rebase with help from AI tooling]
> >
> > Because unless I'm missing something, claiming that AI was used to write the patch
> > is misleading and disingenuous.
>
> Yea, I was just trying to follow the policy. It seems things are swinging the
> other way now. This works.
FWIW, my read of the official policy, even before any change in direction, is that
it only applies to using AI to write code. It's not like we given any credit in
the patch itself when changes are made in response to code review from human.
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Edgecombe, Rick P @ 2026-07-06 23:54 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kirill.shutemov@linux.intel.com,
kas@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, Gao, Chao, x86@kernel.org
In-Reply-To: <akw-dgBE_dycts0o@google.com>
On Mon, 2026-07-06 at 16:47 -0700, Sean Christopherson wrote:
> On Mon, May 25, 2026, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > Use control page helpers for allocating and freeing TD control structures,
> > such these operations can work for Dynamic PAMT.
> >
> > The TDX module tracks some state for each page of physical memory that it
> > might use. It calls this state the PAMT. It includes separate state for
> > each page size a physical page could be utilized at within the TDX module
> > (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> > allocated dynamically. So the kernel must install PAMT backing for each 4KB
> > page before gifting it to the TDX module, and tear it down after the page
> > is reclaimed.
> >
> > TD-scoped control pages (TDR, TDCS) and vCPU-scoped control pages (TDVPR,
> > TDCX) are all handed to the TDX module at 4KB page size and are therefore
> > subject to this requirement. Replace the raw alloc_page()/__free_page()
> > calls for these pages with tdx_alloc/free_control_page().
> >
> > Switching between special Dynamic PAMT operations or normal page
> > alloc/free operations is handled internally in
> > tdx_alloc/free_control_page(). So don't check for Dynamic PAMT around these
> > calls. Just call them unconditionally. Similarly, drop the NULL checks
> > before freeing, as tdx_free_control_page() handles NULL internally.
> >
> > No functional change intended when Dynamic PAMT is not in use.
> >
> > Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
>
> Exactly what assistance was given, and when? I certainly didn't use any of these
> tools, and given that Claude Opus 4.6 was released after I posted the v5 RFC, I
> doubt Kirill did either.
On this patch specifically, lots of "review this patch" type experiments IIRC.
Probably mostly around the log.
>
> And in my strong opinion, even if AI tooling was used to rebase the patches, I
> don't think that level of "assistance" should be presented this way. E.g. I would
> rather a more informal:
>
> [ Rick: enhance log, rebase with help from AI tooling]
>
> Because unless I'm missing something, claiming that AI was used to write the patch
> is misleading and disingenuous.
Yea, I was just trying to follow the policy. It seems things are swinging the
other way now. This works.
>
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > [sean: handle alloc+free+reclaim in one patch]
> > Co-developed-by: Sean Christopherson <seanjc@google.com>
>
> Where did this come from? I don't think me squashing two patches together
> warrants a Co-developed-by.
Gosh, I'm sorry. It looks like I added it. Will remove.
>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > [Rick: enhance log]
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Sean Christopherson @ 2026-07-06 23:47 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, tglx, vannapurve, x86,
chao.gao, yan.y.zhao, kai.huang, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-8-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Use control page helpers for allocating and freeing TD control structures,
> such these operations can work for Dynamic PAMT.
>
> The TDX module tracks some state for each page of physical memory that it
> might use. It calls this state the PAMT. It includes separate state for
> each page size a physical page could be utilized at within the TDX module
> (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> allocated dynamically. So the kernel must install PAMT backing for each 4KB
> page before gifting it to the TDX module, and tear it down after the page
> is reclaimed.
>
> TD-scoped control pages (TDR, TDCS) and vCPU-scoped control pages (TDVPR,
> TDCX) are all handed to the TDX module at 4KB page size and are therefore
> subject to this requirement. Replace the raw alloc_page()/__free_page()
> calls for these pages with tdx_alloc/free_control_page().
>
> Switching between special Dynamic PAMT operations or normal page
> alloc/free operations is handled internally in
> tdx_alloc/free_control_page(). So don't check for Dynamic PAMT around these
> calls. Just call them unconditionally. Similarly, drop the NULL checks
> before freeing, as tdx_free_control_page() handles NULL internally.
>
> No functional change intended when Dynamic PAMT is not in use.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
Exactly what assistance was given, and when? I certainly didn't use any of these
tools, and given that Claude Opus 4.6 was released after I posted the v5 RFC, I
doubt Kirill did either.
And in my strong opinion, even if AI tooling was used to rebase the patches, I
don't think that level of "assistance" should be presented this way. E.g. I would
rather a more informal:
[ Rick: enhance log, rebase with help from AI tooling]
Because unless I'm missing something, claiming that AI was used to write the patch
is misleading and disingenuous.
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [sean: handle alloc+free+reclaim in one patch]
> Co-developed-by: Sean Christopherson <seanjc@google.com>
Where did this come from? I don't think me squashing two patches together
warrants a Co-developed-by.
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> [Rick: enhance log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
^ permalink raw reply
* Re: [PATCH v8 13/46] KVM: guest_memfd: Add base support for KVM_SET_MEMORY_ATTRIBUTES2
From: Suzuki K Poulose @ 2026-07-06 22:35 UTC (permalink / raw)
To: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, willy, wyihan, yan.y.zhao, forkloop, pratyush,
aneesh.kumar, liam, Paolo Bonzini, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CAEvNRgFKbKfTMkqh_XF-igm07qYWfRwYJ5SH7wHcLZnqesCzTw@mail.gmail.com>
On 06/07/2026 19:17, Ackerley Tng wrote:
> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>
>>
>> [...snip...]
>>
>>> +static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>>> + size_t nr_pages, uint64_t attrs)
>>> +{
>>> + struct address_space *mapping = inode->i_mapping;
>>> + struct gmem_inode *gi = GMEM_I(inode);
>>> + pgoff_t end = start + nr_pages;
>>> + struct maple_tree *mt;
>>> + struct ma_state mas;
>>> + int r;
>>> +
>>> + mt = &gi->attributes;
>>> +
>>> + filemap_invalidate_lock(mapping);
>>> +
>>> + mas_init(&mas, mt, start);
>>> + r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
>>> + if (r)
>>> + goto out;
>>> +
>>> + /*
>>> + * From this point on guest_memfd has performed necessary
>>> + * checks and can proceed to do guest-breaking changes.
>>> + */
>>> +
>>> + kvm_gmem_invalidate_start(inode, start, end);
>>
>> I added support for Arm CCA KVM patches with the inplace conversion and
>> I am hitting the following issue.
>>
>> 1. I am supporting INIT_SHARED + MMAP flags.
>> 2. VMM creates the Gmem_fd with both the flags above.
>> 3. Uses the shared gmem-mmap to load the initial payloads (kernel, dtb).
>> 4. At the VM finalization time, Populate the loaded regions one by one
>> by
>> a) copying the images to a temparory buffer - Since CCA can't really
>> load the contents in-place.
>
> Sounds good :). I see that you blocked this in the kernel by returning
> -EOPNOTSUPP if (!src_page) [0].
We could do the copy in kernel with src_page == dst_page, but that would
affect the batching of Granule delegation (and at which point we might
need a temparory buffer in the kernel as big as the vma_pagesize)
>
>> b) Set the "region" to Private in the gmem_fd (via
>> SET_MEMORY_ATTRIBUTES2)
>> c) Invoke CCA backend to populate the private memory via
>> ioctl(KVM_ARM_RMI_POPULATE,..) [0]
>>
>
> This flow sounds right.
>
>> [0]
>> https://lore.kernel.org/all/20260513131757.116630-27-steven.price@arm.com/
>>
>>
>> 5. Additionally, VMM can mark the entire RAM to be private before the VM
>> starts running, again via SET_MEMORY_ATTRIBUTES2. On CCA, this
>> action is measured and doesn't require the Host to "commit" memory to
>> the VM.
>> Instead the host can lazily donate memory on a fault.
>>
>
> For both TDX and SNP, the host can also lazily donate memory,
> guest_memfd supports this.
>
>> But step (5) triggers the invalidation of both private and shared
>> mappings of the gmem area, from the kvm_gmem_invalidate_start()
>> above.
>>
>> This is because, the entire DRAM now has, some portions PRIVATE (the
>> loaded regions) and the rest are SHARED (from the Gmem_fd creation).
>> Thus, kvm_gmem_get_invalidate_filter(Dram_start, Dram_end) causes the
>> invalidation of both "PRIVATE" and "SHARED" regions, which results
>> in the destruction of the already loaded data and things go south.
>>
>
> This destruction will happen for TDX as well. I think we managed to get
> around this because we didn't apply conversion on the already-private
> ranges.
>
> IIUC on SNP, zapping pages in the stage 2 page tables doesn't destroy
> the data, so that's probably why it has been fine for SNP.
Additionally, the Guest at boot, will try to mark the entire DRAM
as Private (RIPAS_RAM in CCA), which would trigger this anyways.
Suzuki
>
>> When we know that the kvm_gmem_invalidate_xx is triggered by a
>> conversion, we don't need to invalidate the existing pages that
>> are in the requested state. i.e., the following patch on top of
>> this series does the trick for me :
>>
>>
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index a97fcac34a0e..62e0427a49f4 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -250,16 +250,23 @@ static void __kvm_gmem_invalidate_start(struct
>> gmem_file *f, pgoff_t start,
>> KVM_MMU_UNLOCK(kvm);
>> }
>>
>> +static void kvm_gmem_invalidate_start_filter(struct inode *inode,
>> pgoff_t start,
>> + pgoff_t end,
>> + enum kvm_gfn_range_filter
>> attr_filter)
>> +{
>> + struct gmem_file *f;
>> +
>> + kvm_gmem_for_each_file(f, inode)
>> + __kvm_gmem_invalidate_start(f, start, end, attr_filter);
>> +}
>> +
>> static void kvm_gmem_invalidate_start(struct inode *inode, pgoff_t start,
>> pgoff_t end)
>> {
>> enum kvm_gfn_range_filter attr_filter;
>> - struct gmem_file *f;
>> -
>> attr_filter = kvm_gmem_get_invalidate_filter(inode, start, end);
>>
>> - kvm_gmem_for_each_file(f, inode)
>> - __kvm_gmem_invalidate_start(f, start, end, attr_filter);
>> + kvm_gmem_invalidate_start_filter(inode, start, end, attr_filter);
>> }
>>
>> static void __kvm_gmem_invalidate_end(struct gmem_file *f, pgoff_t start,
>> @@ -724,9 +731,14 @@ static int __kvm_gmem_set_attributes(struct inode
>> *inode, pgoff_t start,
>> /*
>> * From this point on guest_memfd has performed necessary
>> * checks and can proceed to do guest-breaking changes.
>> + * Also, we don't have to invalidate the regions that
>> + * may already be in the requested state. Hence, we could
>> + * explicitly filter the invalidations to the opposite
>> + * state.
>> */
>>
>> - kvm_gmem_invalidate_start(inode, start, end);
>> + kvm_gmem_invalidate_start_filter(inode, start, end,
>> + to_private ? KVM_FILTER_SHARED :
>> KVM_FILTER_PRIVATE);
>>
>
> I think this makes sense. Thanks for catching this.
>
>> if (!to_private)
>> kvm_gmem_invalidate(inode, start, end);
>>
>>
>> Thoughts ?
>>
>> Suzuki
>>
>>
>>>
>>> [...snip...]
>>>
^ permalink raw reply
* Re: [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Edgecombe, Rick P @ 2026-07-06 21:52 UTC (permalink / raw)
To: seanjc@google.com
Cc: kirill.shutemov@linux.intel.com, linux-coco@lists.linux.dev,
Huang, Kai, kvm@vger.kernel.org, Hansen, Dave, Zhao, Yan Y,
kas@kernel.org, binbin.wu@linux.intel.com, mingo@redhat.com,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, linux-kernel@vger.kernel.org,
Gao, Chao, x86@kernel.org
In-Reply-To: <akwX9wUS_C-lc-I8@google.com>
On Mon, 2026-07-06 at 14:02 -0700, Sean Christopherson wrote:
> Heh, because of course I'm omnipotent and never make mistakes.
Just using failable Sean as a tie breaker.
>
> I agree with the nit in the sense that this isn't a great pattern to encourage,
> but IMO the root of the ugly pattern is the use of kvm_get_running_vcpu() (which
> is sadly the lesser evil in this case). I.e. I'm not terribly concerned about
> this code leading to more "problems" in the future. And I don't really want to
> grab "tdx" later on because that deviates from the standard patterns in KVM and
> incorrectly suggests there _is_ a need to pre-check for a non-NULL vCPU.
>
> All that said, we can get the bost of both words by simply not caching "tdx";
> there's only one use anyways.
Oh yea duh, that's better.
^ permalink raw reply
* Re: [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Edgecombe, Rick P @ 2026-07-06 21:44 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, yilun.xu@linux.intel.com,
x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, Hansen, Dave, dave.hansen@linux.intel.com,
kas@kernel.org, djbw@kernel.org, Fang, Peter,
linux-coco@lists.linux.dev
In-Reply-To: <20260702144614.59464-1-yilun.xu@linux.intel.com>
On Thu, 2026-07-02 at 22:46 +0800, Xu Yilun wrote:
> TDX uses the SEAMCALL instruction to invoke various TDX module
> functions. Just like the syscall, a SEAMCALL specifies the operation
> using a function number and parameters. Moreover, TDX also uses SEAMCALL
> versions to extend the functionalities of existing SEAMCALLs while
> keeping backward compatibility. Unlike syscall versions that assign
> brand new numbers, TDX segments the function number into a basic
> function number field and a version field. Together, they encode the new
> function number.
So isn't this pretty much like numbered seamcalls, except there is a special
format for generating seamcall2, seamcall3, etc? In the end you just use a
different number for a different version of the call. So it's just like
syscalls, except there is a pattern in the specific number for calls of the same
family.
>
> An existing SEAMCALL (TDH.VP.INIT) helper is already using the version
^ maybe swap the order of these two?
> field. However, having the caller pack the version into the function
^Can drop this?
> number open-codes the ABI layout, making the SEAMCALL helper definition
> obscure and error prone.
Do we need the second part of the sentence?
>
> Add a version field in struct tdx_module_args, so that most existing
> SEAMCALL helpers get a default "version == 0" behavior without code
> churn, while callers requiring extended functionalities can specify the
> version descriptively.
>
> As an internal implementation detail,
^ Is this needed to make it clear?
> encode the
> tdx_module_args.version in the function number before calling into
> assembly code.
>
> Two alternative schemes were considered:
>
> 1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
> However, this breaks naming consistency unless all existing stable
> function macros are changed to TDH_XXX_V0.
>
> 2. Add an explicit 'version' parameter to the base seamcall() API. This
> unnecessarily forces all stable SEAMCALL helpers to pass a
> meaningless '0' argument. Additionally, the magic '0' or '1' values
> at caller sites are not descriptive.
>
Dave was recently saying something to the effect of "make every word count". I
think we could lose some filler words.
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
How about a link to the thread where this was suggested.
> ---
> arch/x86/include/asm/shared/tdx.h | 2 ++
> arch/x86/virt/vmx/tdx/seamcall_internal.h | 19 ++++++++++++++++++-
> arch/x86/virt/vmx/tdx/tdx.h | 8 --------
> arch/x86/virt/vmx/tdx/tdx.c | 5 +++--
> 4 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index f20e91d7ac35..b9aac2de233a 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -143,6 +143,8 @@ struct tdx_module_args {
> u64 rbx;
> u64 rdi;
> u64 rsi;
> + /* ABI version, encoded in rax */
> + u8 version;
> };
>
> /* Used to communicate with the TDX module */
> diff --git a/arch/x86/virt/vmx/tdx/seamcall_internal.h b/arch/x86/virt/vmx/tdx/seamcall_internal.h
> index be5f446467df..7002e41cddad 100644
> --- a/arch/x86/virt/vmx/tdx/seamcall_internal.h
> +++ b/arch/x86/virt/vmx/tdx/seamcall_internal.h
> @@ -11,6 +11,7 @@
> #ifndef _X86_VIRT_SEAMCALL_INTERNAL_H
> #define _X86_VIRT_SEAMCALL_INTERNAL_H
>
> +#include <linux/bitfield.h>
> #include <linux/printk.h>
> #include <linux/types.h>
> #include <asm/archrandom.h>
> @@ -23,6 +24,22 @@ u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
>
> typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
>
> +/*
> + * SEAMCALL leaf:
> + *
> + * Bit 15:0 Leaf number
> + * Bit 23:16 Version number
> + */
> +#define SEAMCALL_VERSION_MASK GENMASK_U64(23, 16)
The annoying thing is that the path touched here is also used for seamldr calls
now, which afaict has no concept of version. Underscoring how much of a mess the
wrapper stack is.
> +
> +static __always_inline u64 __seamcall_encode_fn(sc_func_t func, u64 fn,
> + struct tdx_module_args *args)
> +{
I have the same question as Xiaoyao. We have so many wrappers already.
> + FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> +
> + return func(fn, args);
> +}
> +
> static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> struct tdx_module_args *args)
> {
> @@ -39,7 +56,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> */
> this_cpu_write(cache_state_incoherent, true);
>
> - return func(fn, args);
> + return __seamcall_encode_fn(func, fn, args);
> }
>
> static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index bdfd0e1e337a..63e3acfb5d0c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -50,14 +50,6 @@
> #define TDH_SYS_UPDATE 53
> #define TDH_SYS_DISABLE 69
>
> -/*
> - * SEAMCALL leaf:
> - *
> - * Bit 15:0 Leaf number
> - * Bit 23:16 Version number
> - */
> -#define TDX_VERSION_SHIFT 16
> -
> /* TDX page types */
> #define PT_NDA 0x0
> #define PT_RSVD 0x1
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 42df8ea464c4..7a89e29b118c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1910,10 +1910,11 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
> .rcx = vp->tdvpr_pa,
> .rdx = initial_rcx,
> .r8 = x2apicid,
> + /* apicid requires version == 1. */
> + .version = 1,
> };
>
> - /* apicid requires version == 1. */
> - return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
> + return seamcall(TDH_VP_INIT, &args);
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_vp_init);
>
^ permalink raw reply
* Re: [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Sean Christopherson @ 2026-07-06 21:02 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: binbin.wu@linux.intel.com, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Kai Huang, Dave Hansen, Yan Y Zhao,
kirill.shutemov@linux.intel.com, kas@kernel.org, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
tglx@kernel.org, Vishal Annapurve, bp@alien8.de, Chao Gao,
x86@kernel.org
In-Reply-To: <2bdcc07d98df66954b9508be537b6f6a7c9f88de.camel@intel.com>
On Mon, Jul 06, 2026, Rick P Edgecombe wrote:
> On Fri, 2026-07-03 at 11:15 +0800, Binbin Wu wrote:
> > > @@ -1669,16 +1683,29 @@ static struct page *tdx_spte_to_sept_pt(struct kvm
> > > *kvm, gfn_t gfn,
> > > static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
> > > enum pg_level level, u64 new_spte)
> > > {
> > > + struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> > > + struct vcpu_tdx *tdx = to_tdx(vcpu);
> >
> > Nit:
> > Is it better to move this after checking vcpu is not NULL?
> > Although tdx is not dereferenced in between, if vcpu is NULL,
> > it means container_of() does arithmetic to a NULL pointer.
>
> Personally I'm on the fence. I'm going to leave it, because Sean did it that
> way:
> https://lore.kernel.org/kvm/20260129011517.3545883-23-seanjc@google.com/
Heh, because of course I'm omnipotent and never make mistakes.
I agree with the nit in the sense that this isn't a great pattern to encourage,
but IMO the root of the ugly pattern is the use of kvm_get_running_vcpu() (which
is sadly the lesser evil in this case). I.e. I'm not terribly concerned about
this code leading to more "problems" in the future. And I don't really want to
grab "tdx" later on because that deviates from the standard patterns in KVM and
incorrectly suggests there _is_ a need to pre-check for a non-NULL vCPU.
All that said, we can get the bost of both words by simply not caching "tdx";
there's only one use anyways.
diff --git arch/x86/kvm/vmx/tdx.c arch/x86/kvm/vmx/tdx.c
index ee073cacafbe..f53bac52449b 100644
--- arch/x86/kvm/vmx/tdx.c
+++ arch/x86/kvm/vmx/tdx.c
@@ -1684,7 +1684,6 @@ static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
enum pg_level level, u64 new_spte)
{
struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
- struct vcpu_tdx *tdx = to_tdx(vcpu);
gpa_t gpa = gfn_to_gpa(gfn);
u64 err, entry, level_state;
struct page *sept_pt;
@@ -1697,7 +1696,7 @@ static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
if (!sept_pt)
return -EIO;
- ret = tdx_pamt_get(page_to_pfn(sept_pt), &tdx->pamt_cache);
+ ret = tdx_pamt_get(page_to_pfn(sept_pt), &to_tdx(vcpu)->pamt_cache);
if (ret)
return ret;
@@ -1721,7 +1720,6 @@ static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level leve
struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
kvm_pfn_t pfn = spte_to_pfn(new_spte);
- struct vcpu_tdx *tdx = to_tdx(vcpu);
int ret;
if (KVM_BUG_ON(!vcpu, kvm))
@@ -1733,7 +1731,7 @@ static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level leve
WARN_ON_ONCE((new_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
- ret = tdx_pamt_get(pfn, &tdx->pamt_cache);
+ ret = tdx_pamt_get(pfn, &to_tdx(vcpu)->pamt_cache);
if (ret)
return ret;
^ permalink raw reply related
* Re: [PATCH v6 00/11] Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-06 21:01 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, Gao, Chao, x86@kernel.org
In-Reply-To: <20260526023515.288829-1-rick.p.edgecombe@intel.com>
Sean,
On Mon, 2026-05-25 at 19:35 -0700, Rick Edgecombe wrote:
> KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
> KVM: TDX: Get/put PAMT pages when (un)mapping private memory
Would you be willing to take a look at these two patches that need ack's from
the KVM side? I'm hoping to make the next version of this the last one. So it
would be great to get any remaining comments before then.
Thanks.
^ permalink raw reply
* Re: [PATCH 00/15] Device Evidence and Trust for PCI Security Protocol (TDISP)
From: Dan Williams (nvidia) @ 2026-07-06 20:55 UTC (permalink / raw)
To: Jason Gunthorpe, Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Aaron Tomlin,
Alexey Kardashevskiy, Alistair Francis, Aneesh Kumar K.V,
Arnd Bergmann, Bjorn Helgaas, Daniel Gomez, Danilo Krummrich,
Dexuan Cui, Donald Hunter, Greg Kroah-Hartman, Jakub Kicinski,
Luis Chamberlain, Lukas Wunner, Petr Pavlu, Rafael J. Wysocki,
Robin Murphy, Sami Tolvanen, Samuel Ortiz, Saravana Kannan,
Will Deacon, Xu Yilun
In-Reply-To: <20260706125140.GB107792@ziepe.ca>
Jason Gunthorpe wrote:
> On Sun, Jul 05, 2026 at 03:08:04PM -0700, Dan Williams wrote:
> > * NONE: no usage of the device unless the trust is explicitly overridden
> > by user policy specified via a driver flag, module flag, or uapi (TBD).
> >
> > * ADVERSARY: needs acknowledgement from the bus and IOMMU / DMA layers
> > that the device is limited to strict IOMMU translation behavior. Drivers
> > can use this as a signal to limit functionality. This designation
> > implies follow-on IOMMU and bus enabling work for features like
> > arranging for the device to attach to a blocked IOMMU domain when
> > detached from a driver.
> >
> > * AUTO: typical / historical Linux driver model.
> >
> > * TCB: a trust level that only exists in Confidential Computing
> > environments. When acked by the IOMMU / DMA layer it enables the device
> > to issue direct-DMA to private/encrypted addresses or otherwise attach to
> > a secure vIOMMU within the TCB.
>
> I'm not sure I entirely like this one, certainly it needs to be
> possible to have both T=1 and ADVERSARY together.
T=1 and ADVERSARY are independent for link encryption and private MMIO.
In other words the device is placed into the TDISP RUN state independent
of its trust level. Downstream accesses to the device must have T=1, and
its upstream accesses will have T=1, but with force_dma_unencrypted() ==
true.
The only combination missing is T=1 with force_dma_unencrypted() == false &&
VIOMMU enforcing IOMMU_DOMAIN_DMA.
> Arguably the T=0/1 decision is much more like link encryption, it
> effects the transport of the DMA into the CPU. That it also impacts
> how the VIOMMU works is the only thing that make it sort of
> trust-like in this model.
>
> I'd also argue this list is missing "FULL" trust, which is the
> historical Linux behavior for a normal device. AUTO should be
> selecting between FULL/ADVERSARY based on things like the ACPI/etc as
> it does today.
Two notes,
1/ that is effectively how the UNSET level behaves. If the
bus has not set ADVERSARY before device_add() then the default behavior
is the AUTO level. Where AUTO means all of the automatic privileges a
device can be offered without needing any other coordination.
2/ The ambiguity and conflict occurs at ->dma_configure() time when the
bus and IOMMU layer want to reject the device's access to some privilege
by failing. When FULL is defined as !ADVERSARY then it is difficult to
describe the semantics when FULL trust honors rejections to private DMA
and when it falls back to shared operation.
The above more points to a need to have an explicit trust level for
adversarial private memory access. The address spaces are distinct
assets with different levels of trust.
UNSET: bus picks initial level, or leaves it to the device_core().
NONE:
ADVERSARY: Device can be in T=0, or T=1 mode (UNLOCKED, or RUN).
AUTO: Could rename this to be FULL or ALL or DEFAULT, I still keep
coming back to the "AUTO" name because the privileges are not
uniform based on the IOMMU / DMA topology and device capability.
Again, the TDISP state is independent. The TSM driver does
not get called to gatekeep and verify access in this mode.
TCB_ADVERSARY: or PRIVATE_ADVERSARY. Device can access private platform
resources iff an enforcing IOMMU is present.
TCB: or PRIVATE_FULL, automatically enable all access privileges
including private memory access.
Direct-dma to private memory without a Secure VIOMMU present
requires "TCB".
> In a CC VM if we have a T=0 device we probably want to operate it with
> ADVERSARY (there is no T=0 VIOMMU so this is equivilent to FULL)
>
> For a T=1 device we need to have the choice of FULL or ADVERSARY. When
> a VIOMMU is present ADVERSARY will further restrict the T=1 traffic as
> a defense in depth.
Yes, a mode that can setup IOMMU_DOMAIN_DMA for private memory
makes sense. It needs a new trust level to distinguish if the
device is trusted to access one or both address spaces.
> So, it is really necessary to have "TCB" here?
I think it is important to distinguish that the CC case adds another
class of memory to the trust decision.
> If the trust level is reduced to just be a command to the kernel how
> it should operate the device then it would be up to userspace to
> confirm things like T=1 before setting the trust.
This discussion gets strained for me when T=1 is used to mean both
"device is in TDISP RUN (with link encryption and private MMIO)" and
"device is in TDISP RUN + force_dma_unencrypted() == false".
As implemented, userspace can confirm the former before setting the
trust. The latter only gets confirmed / denied at ->dma_configure()
time, because that is the point at which driver policy has been applied
to set the operational trust level.
Otherwise, full bi-directional T=1 before setting the trust would
require an IOMMU to be blocking the device until that final confirmation
point. Given that is not always available the proposal is to defer
acknowledging the trust level with the TSM until ->dma_configure().
As far as userspace is concerned it has enabled T=1 at the device and
validated device evidence. Any failures to operate the device from that
point are either the TSM finding fault with the evidence the CC VM is
relying upon, or, in the new TCB_ADVERSARY case, the IOMMU layer says,
"no IOMMU present to enforce IOMMU_DOMAIN_DMA".
> The kernel would have to set the trust to NONE when security sensitive
> changes are detected.
>
> If we still want a kernel-side policy gate, eg kernel will not
> progress unless T=1 is met, then I think that would be better as an
> independent pair'd policy field, ie
> trust=FULL when policy=T=1, LINK_ENCRYPTION=1, etc
>
> As bundling the two into "TCB" is probably going to turn problematic
> as we have more and more combinations of policy conditions.
Agree that "TCB" alone is insufficient per above. However, a new
"trust when/if" ABI does not appear to scale.
Userspace policy simply needs to contend with the mechanisms that
"Device Core, BUS, IOMMU or TSM driver may fail to enable a device at a
given trust level".
NONE: Device core rejects device operation
ADVERSARY: reject device operation if an IOMMU to set IOMMU_DOMAIN_DMA
not available (not in current patches)
AUTO: no rejections, but no private memory access either
TCB_ADVERSARY: reject device operation if IOMMU_DOMAIN_DMA not
available, or TSM rejects the evidence used to enable
private memory access.
TCB: reject device operation if TSM rejects chosen evidence
Overrides and bypasses for those rejection decisions belong with
their respective drivers. For example, if a paravisor is handling
TDISP then there is no TSM driver to reject TCB operation.
Similarly ->dma_configure() when no IOMMU is present is a nop
today. New enabling could turn to enforcing, or userspace policy
otherwise understands the risks.
^ permalink raw reply
* Re: [PATCH v6 10/11] x86/virt/tdx: Enable Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-06 20:48 UTC (permalink / raw)
To: kas@kernel.org, Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, linux-kernel@vger.kernel.org,
seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
tglx@kernel.org, Annapurve, Vishal, bp@alien8.de,
kirill.shutemov@linux.intel.com, x86@kernel.org
In-Reply-To: <6d87002a7484cd44687c78d60319bea283a78562.camel@intel.com>
On Tue, 2026-06-30 at 18:20 -0700, Rick Edgecombe wrote:
> Looking... Actually there are some dynamic supported ones. Ok, I'll ask.
Thanks for this suggestion. Looks like we can drop the check.
^ permalink raw reply
* Re: [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Edgecombe, Rick P @ 2026-07-06 20:47 UTC (permalink / raw)
To: binbin.wu@linux.intel.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kirill.shutemov@linux.intel.com,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
tglx@kernel.org, Annapurve, Vishal, bp@alien8.de, Gao, Chao,
x86@kernel.org
In-Reply-To: <0c31fcdc-048c-4ff2-9e89-1ba112815c84@linux.intel.com>
On Fri, 2026-07-03 at 11:15 +0800, Binbin Wu wrote:
> > @@ -1669,16 +1683,29 @@ static struct page *tdx_spte_to_sept_pt(struct kvm
> > *kvm, gfn_t gfn,
> > static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
> > enum pg_level level, u64 new_spte)
> > {
> > + struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> > + struct vcpu_tdx *tdx = to_tdx(vcpu);
>
> Nit:
> Is it better to move this after checking vcpu is not NULL?
> Although tdx is not dereferenced in between, if vcpu is NULL,
> it means container_of() does arithmetic to a NULL pointer.
Personally I'm on the fence. I'm going to leave it, because Sean did it that
way:
https://lore.kernel.org/kvm/20260129011517.3545883-23-seanjc@google.com/
^ permalink raw reply
* Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()
From: Edgecombe, Rick P @ 2026-07-06 20:27 UTC (permalink / raw)
To: binbin.wu@linux.intel.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kirill.shutemov@linux.intel.com,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
tglx@kernel.org, Annapurve, Vishal, bp@alien8.de, Gao, Chao,
x86@kernel.org
In-Reply-To: <1694d6c2-2aaa-4277-ba83-97c4a3007417@linux.intel.com>
On Thu, 2026-07-02 at 15:39 +0800, Binbin Wu wrote:
> On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> > backing for the 2MB region covering the passed pfn. However, multiple
> > callers can concurrently operate on 4KB pages that fall within the same
> > 2MB region. When this happens only one Dynamic PAMT page pair needs to be
> > installed to cover the 2MB range. And when one page is freed, the Dynamic
> > PAMT backing cannot be freed until all pages in the range are no longer in
> > use. Make the helpers handle these races internally.
> >
> > Use the per-2MB refcounts from previous changes to track how many 4KB
> > pages are in use within each region. Gate the actual Dynamic PAMT add and
> > remove on refcount transitions (0->1 and 1->0). Serialize the refcount
> > check and SEAMCALL with a global spinlock so the read-decide-act sequence
> > is atomic. This also avoids TDX module BUSY errors, as Dynamic PAMT add
> > and remove SEAMCALLs take an internal TDX module locks at 2MB granularity,
> > so simultaneous attempts on the same region would conflict.
> >
> > The lock is global and heavyweight. Use simple conditional logic to keep
> > correctness obvious. This will be optimized in a later change.
> >
> > Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
>
> One nit below.
>
> [...]
>
> >
> > @@ -2057,10 +2061,26 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> > if (ret)
> > return ret;
> >
> > - tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> > - if (tdx_status != TDX_SUCCESS) {
> > - ret = -EIO;
> > - goto out_free;
> > + pamt_refcount = tdx_find_pamt_refcount(pfn);
>
> tdx_find_pamt_refcount() is tagged as __maybe_unused in the previous patch.
> I think the tag should be removed.
Oh yea, that was the intention. Will fix.
^ permalink raw reply
* Re: [PATCH v6 04/11] x86/virt/tdx: Allocate ref counts for Dynamic PAMT memory
From: Edgecombe, Rick P @ 2026-07-06 20:26 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, kirill.shutemov@linux.intel.com,
x86@kernel.org
In-Reply-To: <akusIp6UQW/pIDc3@intel.com>
On Mon, 2026-07-06 at 21:22 +0800, Chao Gao wrote:
> > +/*
> > + * Allocate PAMT reference counters for all physical memory.
> > + *
> > + * It consumes 2MiB for every 1TiB of physical memory.
> > + */
> > +static int init_pamt_refcounts(void)
> > +{
> > + size_t size = DIV_ROUND_UP(max_pfn, PTRS_PER_PTE) *
> > sizeof(*pamt_refcounts);
> > +
> > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > + return 0;
> > +
> > + pamt_refcounts = __vmalloc(size, GFP_KERNEL | __GFP_ZERO);
> > + if (!pamt_refcounts)
> > + return -ENOMEM;
> > +
> > + return 0;
> > +}
> > +
> > +static void free_pamt_refcounts(void)
> > +{
> > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > + return;
> > +
> > + vfree(pamt_refcounts);
> > + pamt_refcounts = NULL;
> > +}
>
> Both functions are only called during boot. They can be annotated with __init.
Yep, thanks.
>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
^ permalink raw reply
* Re: [PATCH v6 04/11] x86/virt/tdx: Allocate ref counts for Dynamic PAMT memory
From: Edgecombe, Rick P @ 2026-07-06 20:26 UTC (permalink / raw)
To: binbin.wu@linux.intel.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kirill.shutemov@linux.intel.com,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
tglx@kernel.org, Annapurve, Vishal, bp@alien8.de, Gao, Chao,
x86@kernel.org
In-Reply-To: <ca22e5c5-9742-44fa-9c47-e4134e832d49@linux.intel.com>
On Thu, 2026-07-02 at 15:20 +0800, Binbin Wu wrote:
> On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > The PAMT memory holds metadata for all possible TDX protected memory. Each
> > physical address range is covered by PAMT entries at three levels (1GB,
> > 2MB, 4KB). With Dynamic PAMT, the 4KB range of PAMT is allocated on
> > demand. The kernel supplies the TDX module with page pairs to store the
> > 4KB entries, which cover 2MB of host physical memory. The kernel must
> > provide this page pair before using pages from the range for TDX. If this
> > is not done, SEAMCALLs that give the pages to be protected by the TDX module
> > will fail.
> >
> > Allocate reference counters for every 2MB range to track TDX memory usage.
> > This can be used to handle concurrent get/put callers, in order to
> > accurately determine when the dynamic 4KB level of Dynamic PAMT needs to
> > be allocated and when it can be freed.
> >
> > This allocation will currently consume 2 MB for every 1 TB of address
> > space from 0 to max_pfn. The allocation size will depend on how the RAM is
> > physically laid out. In a worst case scenario where the entire 52-bit
> > address space is covered this would be 8GB. Then the DPAMT refcount
> > allocations could hypothetically cause the savings from Dynamic PAMT to go
> > negative on exotic platforms with sparse, small amounts of memory.
>
> Nit:
> Do you think it make sense to print the memory size allocated for
> DPAMT refcounts in the kernel log?
I think, no? Because it should normally be "small" on the scale of the system.
We have to draw the line somewhere.
>
> >
> > Future changes could reduce this refcount overhead to be only allocating
> > refcounts for physical ranges that contain memory that TDX can use.
> > However, this is left for future work.
> >
> > Assisted-by: Sashiko:claude-opus-4-6 GitHub Copilot:claude-opus-4-6 Sashiko:claude-opus-4-6
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Thanks, and for the others.
^ permalink raw reply
* Re: [PATCH v6 03/11] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Edgecombe, Rick P @ 2026-07-06 20:23 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, kirill.shutemov@linux.intel.com,
x86@kernel.org
In-Reply-To: <akugEqMk29Q9DJUY@intel.com>
On Mon, 2026-07-06 at 20:31 +0800, Chao Gao wrote:
> On Mon, May 25, 2026 at 07:35:07PM -0700, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > Add helpers to use when allocating or preparing pages that are handed to
> > the TDX-Module for use as control/S-EPT pages, and thus need Dynamic PAMT
>
> For the whole series: s/TDX-Module/TDX module, to match the existing
> convention.
Doh, I actually changed the whole series to TDX-module before realizing that
most of the references are "TDX module". So I thought to standardize on that
going forward. I missed this one when changing back.
>
> > +/*
> > + * Calculate the arg needed for operating on the DPAMT backing for
> > + * a given 4KB page.
> > + */
> > +static u64 pamt_2mb_arg(kvm_pfn_t pfn)
> > +{
> > + unsigned long hpa_2mb = ALIGN_DOWN(pfn << PAGE_SHIFT, PMD_SIZE);
>
> The changelog and the comment don't explain why the pfn is aligned down to a
> 2MB boundary. And ...
>
> > +
> > + return hpa_2mb | TDX_PS_2M;
> > +}
> > +
> > +/* Add PAMT backing for the given page. */
>
> ... this adds PAMT backing for the whole 2MB region containing the given page,
> not just the given page itself. Is it worth clarifying in the comment?
Yea that is a good comment.
>
> <snip>
>
> > +/*
> > + * Return a page that can be gifted to the TDX-Module for use as a "control"
> > + * page, i.e. pages that are used for control and S-EPT structures for a given
> > + * TDX guest, and bound to said guest's HKID and thus obtain TDX protections,
> > + * including PAMT tracking.
> > + */
>
> IIUC, S-EPT structures are not allocated through this helper. Why mention it?
Oh, yea that is removed with v6. I'll update it.
>
> > +struct page *tdx_alloc_control_page(void)
> > +{
> > + struct page *page;
> > +
> > + page = alloc_page(GFP_KERNEL_ACCOUNT);
> > + if (!page)
> > + return NULL;
> > +
> > + if (tdx_pamt_get(page_to_pfn(page))) {
> > + __free_page(page);
> > + return NULL;
> > + }
> > +
> > + return page;
> > +}
> > +EXPORT_SYMBOL_FOR_KVM(tdx_alloc_control_page);
> > +
> > +/*
> > + * Free a page that was gifted to the TDX-Module for use as a control/S-EPT
> > + * page. After this, the page is no longer protected by TDX.
> > + */
>
> Ditto.
>
> > +void tdx_free_control_page(struct page *page)
> > +{
> > + if (!page)
> > + return;
> > +
> > + tdx_pamt_put(page_to_pfn(page));
> > + __free_page(page);
> > +}
> > +EXPORT_SYMBOL_FOR_KVM(tdx_free_control_page);
>
> With above nits fixed,
>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
^ permalink raw reply
* Re: [PATCH v6 01/11] x86/virt/tdx: Simplify tdmr_get_pamt_sz()
From: Edgecombe, Rick P @ 2026-07-06 20:18 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, binbin.wu@linux.intel.com,
x86@kernel.org
In-Reply-To: <akdNPjf3zt6jPX1m@intel.com>
On Fri, 2026-07-03 at 13:48 +0800, Chao Gao wrote:
> On Mon, May 25, 2026 at 07:35:05PM -0700, Rick Edgecombe wrote:
> > Since the loop that iterates over it is gone, further simplify the code by
> > dropping the array of intermediate size and base storage. Just store the
> > values to their final locations.
>
> > Accept the small complication of having
> > to clear tdmr->pamt_4k_base in the error path, so that tdmr_do_pamt_func()
> > will not try to operate on the TDMR struct when attempting to free it.
>
> The clearing of tdmr->pamt_4k_base was dropped, so this section is a bit
> stale. Apart from this nit,
Oh, good point.
>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
Thanks, and for the others.
^ permalink raw reply
* Re: [PATCH v2 16/31] coco/tdx-host: Support Link TSM for TDX host
From: Ackerley Tng @ 2026-07-06 20:12 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-17-yilun.xu@linux.intel.com>
Xu Yilun <yilun.xu@linux.intel.com> writes:
>
> [...snip...]
>
> +static struct pci_tsm_ops tdx_tsm_link_ops = {
> + .probe = tdx_tsm_link_probe,
> + .remove = tdx_tsm_link_remove,
> + .connect = tdx_tsm_link_connect,
Since .connect is called from pci_tsm_connect(), which is in turn called
from connect_store, is the plan that the VMM should write to
/sys/bus/pci/devices/.../tsm/connect
to set up SPDM and IDE before starting the guest?
> + .disconnect = tdx_tsm_link_disconnect,
> +};
> +
>
> [...snip...]
>
^ permalink raw reply
* Re: [PATCH v2 30/31] coco/tdx-host: Implement IDE stream setup/teardown
From: Ackerley Tng @ 2026-07-06 20:05 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-31-yilun.xu@linux.intel.com>
Xu Yilun <yilun.xu@linux.intel.com> writes:
>
> [...snip...]
>
> static int tdx_tsm_link_connect(struct pci_dev *pdev)
> {
> struct tdx_tsm_link *tlink = to_tdx_tsm_link(pdev->tsm);
> @@ -362,7 +579,15 @@ static int tdx_tsm_link_connect(struct pci_dev *pdev)
> return PTR_ERR(tlink_spdm);
> }
>
> + struct tdx_tsm_link *tlink_ide __free(tdx_ide_stream_teardown) =
> + tdx_ide_stream_setup(tlink);
> + if (IS_ERR(tlink_ide)) {
> + pci_err(pdev, "fail to setup ide stream\n");
> + return PTR_ERR(tlink_ide);
> + }
> +
> retain_and_null_ptr(tlink_spdm);
> + retain_and_null_ptr(tlink_ide);
>
> return 0;
> }
Would it be possible to define .connect = tdx_tsm_link_connect in the
patch that sets up spdm, and then update tdx_tsm_link_connect in this
patch?
I think it would help in showing how the spdm is associated with
tdx_tsm_link_ops.
Would it also be possible to bring the definition of .probe in
tdx_tsm_link_ops till after this patch too?
> @@ -371,6 +596,7 @@ static void tdx_tsm_link_disconnect(struct pci_dev *pdev)
> {
> struct tdx_tsm_link *tlink = to_tdx_tsm_link(pdev->tsm);
>
> + tdx_ide_stream_teardown(tlink);
> tdx_spdm_session_teardown(tlink);
> }
>
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths
From: Jason Gunthorpe @ 2026-07-06 19:52 UTC (permalink / raw)
To: Tom Lendacky
Cc: Alexey Kardashevskiy, Aneesh Kumar K.V, Catalin Marinas, iommu,
linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Jiri Pirko, Mostafa Saleh, Petr Tesarik,
Dan Williams, Xu Yilun, linuxppc-dev, linux-s390,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Alexander Gordeev, Gerald Schaefer,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Sven Schnelle, x86
In-Reply-To: <d7afa22d-5a41-491f-b945-289ab78080e7@amd.com>
On Mon, Jul 06, 2026 at 01:45:33PM -0500, Tom Lendacky wrote:
> In that situation, with SME enabled on the host, the SWIOTLB must be
> mapped by the kernel without the encryption bit set and DMA addresses
> must be provided to devices without the encryption bit set. This is
> because if SWIOTLB is mapped encrypted, requiring the encryption bit be
> part of the DMA address, any device that cannot perform DMA at the
> address width where the encryption bit exists will fail to DMA properly.
Ideally there would be two SWIOTLBs and "decrypted" aka low address,
one is only used if the device requires it.
Alternatively we don't support this questionable security choice and
using swiotlb with an incapable device just fails. Use the iommu, or
don't force swiotlb.
Jason
^ permalink raw reply
* Re: [PATCH v2 28/31] coco/tdx-host: Parse ACPI KEYP table to init IDE for PCI host bridges
From: Ackerley Tng @ 2026-07-06 19:46 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-29-yilun.xu@linux.intel.com>
Xu Yilun <yilun.xu@linux.intel.com> writes:
> Parse the KEYP Key Configuration Units (KCU), to decide the max IDE
> streams supported for each host bridge.
>
> The KEYP table points to a number of KCU structures that each associates
> with a list of root ports (RP) via segment, bus, and devfn. Sanity check
> the KEYP table, ensure all RPs listed for each KCU are included in one
> host bridge. Then extact the max IDE streams supported to
> pci_host_bridge via pci_ide_set_nr_streams().
>
> Co-developed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> drivers/virt/coco/tdx-host/tdx-host.c | 111 ++++++++++++++++++++++++++
> 1 file changed, 111 insertions(+)
>
> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> index 4d127b7c2591..d5072a68b81a 100644
> --- a/drivers/virt/coco/tdx-host/tdx-host.c
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> @@ -5,6 +5,7 @@
> * Copyright (C) 2025 Intel Corporation
> */
>
> +#include <linux/acpi.h>
> #include <linux/bitfield.h>
> #include <linux/device/faux.h>
> #include <linux/dmar.h>
> @@ -12,6 +13,7 @@
> #include <linux/mod_devicetable.h>
> #include <linux/pci.h>
> #include <linux/pci-doe.h>
> +#include <linux/pci-ide.h>
> #include <linux/pci-tsm.h>
> #include <linux/tsm.h>
> #include <linux/vmalloc.h>
> @@ -474,6 +476,111 @@ static void unregister_link_tsm(void *link)
> tsm_unregister(link);
> }
>
> +#define KCU_STR_CAP_NUM_STREAMS GENMASK(8, 0)
> +
> +/* The bus_end is inclusive */
> +struct keyp_hb_info {
> + /* input */
> + u16 segment;
> + u8 bus_start;
> + u8 bus_end;
Would you consider using bus_last instead of bus_end to indicate that
bus_last is inclusive? Or would it be better to say nr_buses to avoid
the inclusive/exclusive problem?
> + /* output */
> + u8 nr_ide_streams;
> +};
> +
> +static bool keyp_info_match(struct acpi_keyp_rp_info *rp,
> + struct keyp_hb_info *hb)
> +{
> + return rp->segment == hb->segment && rp->bus >= hb->bus_start &&
> + rp->bus <= hb->bus_end;
> +}
> +
> +static int keyp_config_unit_handler(union acpi_subtable_headers *header,
> + void *arg, const unsigned long end)
> +{
> + struct acpi_keyp_config_unit *acpi_cu =
> + (struct acpi_keyp_config_unit *)&header->keyp;
> + struct keyp_hb_info *hb_info = arg;
> + int rp_size, rp_count, i;
> + void __iomem *addr;
> + bool match = false;
> + u32 cap;
> +
> + rp_size = acpi_cu->header.length - sizeof(*acpi_cu);
> + if (rp_size % sizeof(struct acpi_keyp_rp_info))
> + return -EINVAL;
> +
> + rp_count = rp_size / sizeof(struct acpi_keyp_rp_info);
> + if (!rp_count || rp_count != acpi_cu->root_port_count)
> + return -EINVAL;
> +
> + for (i = 0; i < rp_count; i++) {
> + struct acpi_keyp_rp_info *rp_info = &acpi_cu->rp_info[i];
> +
> + if (i == 0) {
> + match = keyp_info_match(rp_info, hb_info);
> + /* The host bridge already matches another KCU */
> + if (match && hb_info->nr_ide_streams)
> + return -EINVAL;
> +
> + continue;
> + }
> +
> + if (match ^ keyp_info_match(rp_info, hb_info))
> + return -EINVAL;
> + }
> +
> + if (!match)
> + return 0;
> +
> + addr = ioremap(acpi_cu->register_base_address, sizeof(cap));
> + if (!addr)
> + return -ENOMEM;
> + cap = ioread32(addr);
> + iounmap(addr);
> +
> + hb_info->nr_ide_streams = FIELD_GET(KCU_STR_CAP_NUM_STREAMS, cap) + 1;
> +
> + return 0;
> +}
> +
> +static u8 keyp_find_nr_ide_stream(u16 segment, u8 bus_start, u8 bus_end)
> +{
> + struct keyp_hb_info hb_info = {
> + .segment = segment,
> + .bus_start = bus_start,
> + .bus_end = bus_end,
> + };
> + int rc;
> +
> + rc = acpi_table_parse_keyp(ACPI_KEYP_TYPE_CONFIG_UNIT,
> + keyp_config_unit_handler, &hb_info);
> + if (rc < 0)
> + return 0;
> +
> + return hb_info.nr_ide_streams;
> +}
> +
> +static void keyp_setup_nr_ide_stream(struct pci_bus *bus)
> +{
> + struct pci_host_bridge *hb = pci_find_host_bridge(bus);
> + u8 nr_ide_streams;
> +
> + nr_ide_streams = keyp_find_nr_ide_stream(pci_domain_nr(bus),
> + bus->busn_res.start,
> + bus->busn_res.end);
> +
> + pci_ide_set_nr_streams(hb, nr_ide_streams);
> +}
> +
> +static void tdx_setup_nr_ide_stream(void)
Was wondering, is nr in the name meant as an abbreviation for number?
Why not just tdx_setup_ide_streams(), since this doesn't really set up a
specific number of streams.
Same for the naming of keyp_setup_nr_ide_stream().
> +{
> + struct pci_bus *bus = NULL;
> +
> + while ((bus = pci_find_next_bus(bus)))
> + keyp_setup_nr_ide_stream(bus);
> +}
> +
> static DEFINE_XARRAY(tlink_iommu_xa);
>
> static void tdx_iommu_clear(u64 iommu_id, struct tdx_page_array *iommu_mt)
> @@ -569,6 +676,8 @@ static int __maybe_unused tdx_connect_init(struct device *dev)
> if (ret)
> return ret;
>
> + tdx_setup_nr_ide_stream();
> +
> link = tsm_register(dev, &tdx_tsm_link_ops);
> if (IS_ERR(link))
> return dev_err_probe(dev, PTR_ERR(link),
> @@ -612,5 +721,7 @@ static void __exit tdx_host_exit(void)
> }
> module_exit(tdx_host_exit);
>
> +MODULE_IMPORT_NS("ACPI");
> +MODULE_IMPORT_NS("PCI_IDE");
> MODULE_DESCRIPTION("TDX Host Services");
> MODULE_LICENSE("GPL");
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v2 26/31] mm: Add __free() support for __free_page()
From: Ackerley Tng @ 2026-07-06 19:31 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-27-yilun.xu@linux.intel.com>
Xu Yilun <yilun.xu@linux.intel.com> writes:
> Allow for the declaration of struct page * variables that trigger
> __free_page() when they go out of scope.
>
> A example usage would be in the following patch:
>
> static struct pci_tsm *tdx_tsm_link_pf0_probe(...)
> {
> ...
>
> struct page *in_msg_page __free(__free_page) =
> alloc_page(GFP_KERNEL | __GFP_ZERO);
> if (!in_msg_page)
> return NULL;
>
> struct page *out_msg_page __free(__free_page) =
> alloc_page(GFP_KERNEL | __GFP_ZERO);
> if (!out_msg_page)
> return NULL;
>
> ...
>
> tlink->in_msg = no_free_ptr(in_msg_page);
> tlink->out_msg = no_free_ptr(out_msg_page);
>
> ...
> }
>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
> include/linux/gfp.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 51ef13ed756e..d37e5564234e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -391,6 +391,7 @@ extern void free_pages_nolock(struct page *page, unsigned int order);
> extern void free_pages(unsigned long addr, unsigned int order);
>
> #define __free_page(page) __free_pages((page), 0)
> +DEFINE_FREE(__free_page, struct page *, if (_T) __free_page(_T))
> #define free_page(addr) free_pages((addr), 0)
>
> void page_alloc_init_cpuhp(void);
> --
> 2.25.1
This is nice to have, independent of this series. Perhaps it should be
sent separately, and some existing function could be refactored to use
this, maybe __tdx_td_init?
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
^ permalink raw reply
* Re: [PATCH v2 23/31] coco/tdx-host: Setup all trusted IOMMUs on TDX Connect init
From: Ackerley Tng @ 2026-07-06 19:21 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-24-yilun.xu@linux.intel.com>
Xu Yilun <yilun.xu@linux.intel.com> writes:
> Setup all trusted IOMMUs on TDX Connect initialization and clear all on
> TDX Connect removal.
>
> Trusted IOMMU setup is the pre-condition for all following TDX Connect
> operations such as SPDM/IDE setup. It is more of a platform
> configuration than a standalone IOMMU configuration, so put the
> implementation in tdx-host driver.
>
I was wondering about hotplug. There's dmar_device_hotplug(). Does it
make sense to do TDH.IOMMU.SETUP from there?
For boot, is the platform-level configuration due to dependencies? Is it
because TDX is set up after IOMMUs and hence TDH.IOMMU.SETUP is done
from the creation of the faux device?
> There is no dedicated way to enumerate which IOMMU devices support
> trusted operations. The host has to call TDH.IOMMU.SETUP on all IOMMU
> devices and tell their trusted capability by the return value.
>
>
> [...snip...]
>
> @@ -119,6 +120,82 @@ static void unregister_link_tsm(void *link)
> tsm_unregister(link);
> }
>
> +static DEFINE_XARRAY(tlink_iommu_xa);
> +
> +static void tdx_iommu_clear(u64 iommu_id, struct tdx_page_array *iommu_mt)
> +{
> + u64 r;
> +
> + r = tdh_iommu_clear(iommu_id, iommu_mt);
> + if (r) {
> + pr_err("fail to clear tdx iommu 0x%llx\n", r);
> + goto leak;
> + }
> +
> + if (tdx_page_array_ctrl_release(iommu_mt, iommu_mt->nr_pages,
> + virt_to_phys(iommu_mt->root))) {
> + pr_err("fail to release iommu_mt pages\n");
> + goto leak;
> + }
> +
> + return;
> +
> +leak:
> + tdx_page_array_ctrl_leak(iommu_mt);
> +}
> +
> +static int tdx_iommu_enable_one(struct dmar_drhd_unit *drhd)
> +{
> + unsigned int nr_pages = tdx_sysinfo->connect.iommu_mt_page_count;
> + u64 r, iommu_id;
> + int ret;
> +
> + struct tdx_page_array *iommu_mt __free(tdx_page_array_free) =
> + tdx_page_array_create_iommu_mt(1, nr_pages);
> + if (!iommu_mt)
> + return -ENOMEM;
> +
> + r = tdh_iommu_setup(drhd->reg_base_addr, iommu_mt, &iommu_id);
> + /* This drhd doesn't support tdx mode, skip. */
> + if ((r & TDX_SEAMCALL_STATUS_MASK) == TDX_OPERAND_INVALID)
Extra space here before ==.
Is there a chance that TDX_OPERAND_INVALID might be returned for some
other reason other than that the drhd doesn't support TDX? I'm concerned
that some drhd would be silently skipped for some other error.
> + return 0;
> +
> + if (r) {
> + pr_err("fail to enable tdx mode for DRHD[0x%llx]\n",
> + drhd->reg_base_addr);
> + return -EFAULT;
> + }
> +
> + ret = xa_insert(&tlink_iommu_xa, (unsigned long)iommu_id,
> + no_free_ptr(iommu_mt), GFP_KERNEL);
> + if (ret) {
> + tdx_iommu_clear(iommu_id, iommu_mt);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void tdx_iommu_disable_all(void *data)
> +{
> + struct tdx_page_array *iommu_mt;
> + unsigned long iommu_id;
> +
> + xa_for_each(&tlink_iommu_xa, iommu_id, iommu_mt)
> + tdx_iommu_clear(iommu_id, iommu_mt);
> +}
> +
Is there an equivalent teardown operation? Like on kexec, if TDX is torn
down, would tdx_iommu_disable_all() need to be called to undo
everything, so that after TDX is started again tdx_iommu_enable_all()
would be set up again?
For hot-unplug, would the TDX IOMMU config need to be disabled?
> +static int tdx_iommu_enable_all(void)
> +{
> + int ret;
> +
> + ret = do_for_each_drhd_unit(tdx_iommu_enable_one);
> + if (ret)
> + tdx_iommu_disable_all(NULL);
> +
> + return ret;
> +}
> +
> static int __maybe_unused tdx_connect_init(struct device *dev)
> {
> struct tsm_dev *link;
> @@ -130,6 +207,14 @@ static int __maybe_unused tdx_connect_init(struct device *dev)
> if (!(tdx_sysinfo->features.tdx_features0 & TDX_FEATURES0_TDXCONNECT))
> return 0;
>
> + ret = tdx_iommu_enable_all();
> + if (ret)
> + return dev_err_probe(dev, ret, "Enable tdx iommu failed\n");
> +
> + ret = devm_add_action_or_reset(dev, tdx_iommu_disable_all, NULL);
> + if (ret)
> + return ret;
> +
> link = tsm_register(dev, &tdx_tsm_link_ops);
> if (IS_ERR(link))
> return dev_err_probe(dev, PTR_ERR(link),
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths
From: Tom Lendacky @ 2026-07-06 18:45 UTC (permalink / raw)
To: Alexey Kardashevskiy, Jason Gunthorpe, Aneesh Kumar K.V
Cc: Catalin Marinas, iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Jiri Pirko,
Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun, linuxppc-dev,
linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <fd135adc-a8a4-48e5-b649-2a29789b22d8@amd.com>
On 7/1/26 20:04, Alexey Kardashevskiy wrote:
> On 1/7/26 03:42, Jason Gunthorpe wrote:
>> On Mon, Jun 29, 2026 at 12:16:30PM +0530, Aneesh Kumar K.V wrote:
>>>>> Thinking about this more, I guess we should mark the swiotlb as
>>>>> cc_shared only with CC_ATTR_GUEST_MEM_ENCRYPT instead of
>>>>> CC_ATTR_MEM_ENCRYPT as we have below.
>>>>
>>>> The name cc_shared should be used for GUEST scenarios only.
>>>>
>>>> I guess there is some merit in keeping swiotlb using "decrypted" to
>>>> mean it usinig pgprot_decrypted and set_memory_decyped() which AMD
>>>> gives meaning to on both host and guest.
>>>
>>> Are you suggesting to change the struct io_tlb_mem::cc_shared back to
>>> struct io_tlb_mem::unencrypted?.
>>
>> Yes
>>
>>>> IDK what AMD should do on the host by default. I guess it should setup
>>>> a swiotlb pool of low dma addrs "unencrypted", but not "cc_shared"?
>>>>
>>>
>>> If by low DMA address you mean using an address with the C-bit
>>> cleared.
>>
>> Yes
>>
>>> The current code already does this and uses the swiotlb pool correctly
>>> on SME.
>>
>> Well, through the force_dma_unencrypted() hack...
>>
>>> The challenge arises when we want to force SWIOTLB
>>> bouncing even for devices that can handle encrypted DMA addresses (more
>>> on that below). For such a config force_dma_uencrypted(dev) will return
>>> false and swiotlb will be marked cc_shared/decrypted = true; This trip
>>> the new check we added.
>>
>> Yes, because cc_shared (guest) and unencrypted (host) are very
>> different things and we've mixed them:
>>
>>> if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
>>
>> I'm aruging force_dma_unencrypted should mean cc_shared and be
>> guest_only, but the SME hack breaks this.
>>
>>> We can also do
>>>
>>> if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
>>> /* swiotlb pool is incorrect for this device */
>>> if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
>>> return (phys_addr_t)DMA_MAPPING_ERROR;
>>>
>>> /* Force attrs to match the kind of memory in the pool */
>>> if (mem->cc_shared)
>>> *attrs |= DMA_ATTR_CC_SHARED;
>>> else
>>> *attrs &= ~DMA_ATTR_CC_SHARED;
>>> } else {
>>> /*
>>> * Host memory encryption where device requires an
>>> * unencrypted dma_addr_t due to dma mask limit
>>> */
>>> if (force_dma_unencrypted(dev))
>>> *attrs |= DMA_ATTR_CC_SHARED;
>>> else
>>> *attrs &= ~DMA_ATTR_CC_SHARED;
>>> }
>>
>> If we do this I would like to split the force_dma_.. functions into
>> guest and host, ie force_dma_cc_shared() and force_host_decrypted()
>
> imho force_dma_unencrypted() should not look at the mask at all (the
> mask should tell the DMA layer to use swiotlb, encrypted or not),
> instead, when we set up swiotlb - we could make it unencrypted if
> iommu=pt, otherwise encrypted (although this means IOMMU and defeats the
> purpose of swiotlb). But at least this patchset has enough plumbing to
> have swiotlb encrypted, right?
>
>> To make it clear there are two very different things here.
>>
>>> Here I see value in having DMA_ATTR_UNENCRYPTED. The question is do we
>>> need to split this into two flags and introduce the resulting code
>>> duplication.
>>
>> The external flag name should be DMA_ATTR_CC_SHARED and only used on
>> CC guest. Internally that turns into using set_memory_decrypted()
>> which works on guest and host for AMD. I don't know how to make the
>> host only case clearer and still keep the code efficient..
>>
>>>> The dma api has to detect, after the driver sets the dma limit, that
>>>> none of system memory is usable when:
>>>> - The direct path is being used
>>>> - phys to dma for 0 is outside the dma limit
>>>>
>>>> Then it should assume the arch has setup a swiotlb pool for it to use
>>>> to fix the high memory problem.
>>>>
>>>> Similar hackery would be needed in the dma alloc path to know that
>>>> decrypted can be used to fix the high memory problem like for GUEST.
>>>>
>>>> I guess some 'dev_cannot_reach_memory(dev)' sort of test in a
>>>> few key places? Setup with a static branch to be a nop on everything
>>>> but AMD, compiled out on every other arch.
>>>>
>>>
>>> If we are not able to reach the memory because of the memory encryption
>>> bit, then isn't dev_cannot_reach_memory(dev) the same as
>>> force_dma_unencrypted(dev)? If so, that is how it is already done.
>>
>> Sort of yes, but it is properly named to its purpose and not confused
>> with what should be a guest-only function.
>>
>>> x86/dma: Disable forced SWIOTLB bouncing for SME IOMMU passthrough
>>
>> Maybe as a crutch to get this series merged..
> feels okay but I do not really know the true meaning of "swiotlb=force"
> so adding Tom to the thread. Thanks,
With swiotlb=force all DMA would be bounce buffered through SWIOTLB. My
understanding (which could be completely wrong) for this setting is that
it is used mainly to validate a driver implementation (race conditions
between device memory access and driver memory access) and validate the
SWIOTLB implementation itself. I'm not sure why you would want to run
with swiotlb=force other than that.
In that situation, with SME enabled on the host, the SWIOTLB must be
mapped by the kernel without the encryption bit set and DMA addresses
must be provided to devices without the encryption bit set. This is
because if SWIOTLB is mapped encrypted, requiring the encryption bit be
part of the DMA address, any device that cannot perform DMA at the
address width where the encryption bit exists will fail to DMA properly.
Thanks,
Tom
>
>
^ permalink raw reply
* Re: [PATCH v2 22/31] iommu/vt-d: Export a helper to do function for each dmar_drhd_unit
From: Ackerley Tng @ 2026-07-06 18:35 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-23-yilun.xu@linux.intel.com>
Xu Yilun <yilun.xu@linux.intel.com> writes:
> Enable the tdx-host module to get VTBAR address for every IOMMU device.
> The VTBAR address is for TDX Module to identify the IOMMU device and
> setup its trusted configuraion.
>
I think this commit message doesn't match the commit subject or the
change, unless I'm misunderstanding something
> Suggested-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> include/linux/dmar.h | 2 ++
> drivers/iommu/intel/dmar.c | 16 ++++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 692b2b445761..cd8d9f440975 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -86,6 +86,8 @@ extern struct list_head dmar_drhd_units;
> dmar_rcu_check()) \
> if (i=drhd->iommu, 0) {} else
>
> +int do_for_each_drhd_unit(int (*fn)(struct dmar_drhd_unit *));
> +
> static inline bool dmar_rcu_check(void)
> {
> return rwsem_is_locked(&dmar_global_lock) ||
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 4f9571eee1d4..eea9ba691f99 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -2452,3 +2452,19 @@ bool dmar_platform_optin(void)
> return ret;
> }
> EXPORT_SYMBOL_GPL(dmar_platform_optin);
> +
> +int do_for_each_drhd_unit(int (*fn)(struct dmar_drhd_unit *))
> +{
> + struct dmar_drhd_unit *drhd;
> + int ret;
> +
> + guard(rwsem_read)(&dmar_global_lock);
> +
> + for_each_drhd_unit(drhd) {
> + ret = fn(drhd);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(do_for_each_drhd_unit);
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v8 13/46] KVM: guest_memfd: Add base support for KVM_SET_MEMORY_ATTRIBUTES2
From: Ackerley Tng @ 2026-07-06 18:17 UTC (permalink / raw)
To: Suzuki K Poulose, aik, andrew.jones, binbin.wu, brauner,
chao.p.peng, david, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, yan.y.zhao, forkloop,
pratyush, aneesh.kumar, liam, Paolo Bonzini, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <114e2488-97ed-4740-a8e8-1edd991f26c5@arm.com>
Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>
> [...snip...]
>
>> +static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>> + size_t nr_pages, uint64_t attrs)
>> +{
>> + struct address_space *mapping = inode->i_mapping;
>> + struct gmem_inode *gi = GMEM_I(inode);
>> + pgoff_t end = start + nr_pages;
>> + struct maple_tree *mt;
>> + struct ma_state mas;
>> + int r;
>> +
>> + mt = &gi->attributes;
>> +
>> + filemap_invalidate_lock(mapping);
>> +
>> + mas_init(&mas, mt, start);
>> + r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
>> + if (r)
>> + goto out;
>> +
>> + /*
>> + * From this point on guest_memfd has performed necessary
>> + * checks and can proceed to do guest-breaking changes.
>> + */
>> +
>> + kvm_gmem_invalidate_start(inode, start, end);
>
> I added support for Arm CCA KVM patches with the inplace conversion and
> I am hitting the following issue.
>
> 1. I am supporting INIT_SHARED + MMAP flags.
> 2. VMM creates the Gmem_fd with both the flags above.
> 3. Uses the shared gmem-mmap to load the initial payloads (kernel, dtb).
> 4. At the VM finalization time, Populate the loaded regions one by one
> by
> a) copying the images to a temparory buffer - Since CCA can't really
> load the contents in-place.
Sounds good :). I see that you blocked this in the kernel by returning
-EOPNOTSUPP if (!src_page) [0].
> b) Set the "region" to Private in the gmem_fd (via
> SET_MEMORY_ATTRIBUTES2)
> c) Invoke CCA backend to populate the private memory via
> ioctl(KVM_ARM_RMI_POPULATE,..) [0]
>
This flow sounds right.
> [0]
> https://lore.kernel.org/all/20260513131757.116630-27-steven.price@arm.com/
>
>
> 5. Additionally, VMM can mark the entire RAM to be private before the VM
> starts running, again via SET_MEMORY_ATTRIBUTES2. On CCA, this
> action is measured and doesn't require the Host to "commit" memory to
> the VM.
> Instead the host can lazily donate memory on a fault.
>
For both TDX and SNP, the host can also lazily donate memory,
guest_memfd supports this.
> But step (5) triggers the invalidation of both private and shared
> mappings of the gmem area, from the kvm_gmem_invalidate_start()
> above.
>
> This is because, the entire DRAM now has, some portions PRIVATE (the
> loaded regions) and the rest are SHARED (from the Gmem_fd creation).
> Thus, kvm_gmem_get_invalidate_filter(Dram_start, Dram_end) causes the
> invalidation of both "PRIVATE" and "SHARED" regions, which results
> in the destruction of the already loaded data and things go south.
>
This destruction will happen for TDX as well. I think we managed to get
around this because we didn't apply conversion on the already-private
ranges.
IIUC on SNP, zapping pages in the stage 2 page tables doesn't destroy
the data, so that's probably why it has been fine for SNP.
> When we know that the kvm_gmem_invalidate_xx is triggered by a
> conversion, we don't need to invalidate the existing pages that
> are in the requested state. i.e., the following patch on top of
> this series does the trick for me :
>
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index a97fcac34a0e..62e0427a49f4 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -250,16 +250,23 @@ static void __kvm_gmem_invalidate_start(struct
> gmem_file *f, pgoff_t start,
> KVM_MMU_UNLOCK(kvm);
> }
>
> +static void kvm_gmem_invalidate_start_filter(struct inode *inode,
> pgoff_t start,
> + pgoff_t end,
> + enum kvm_gfn_range_filter
> attr_filter)
> +{
> + struct gmem_file *f;
> +
> + kvm_gmem_for_each_file(f, inode)
> + __kvm_gmem_invalidate_start(f, start, end, attr_filter);
> +}
> +
> static void kvm_gmem_invalidate_start(struct inode *inode, pgoff_t start,
> pgoff_t end)
> {
> enum kvm_gfn_range_filter attr_filter;
> - struct gmem_file *f;
> -
> attr_filter = kvm_gmem_get_invalidate_filter(inode, start, end);
>
> - kvm_gmem_for_each_file(f, inode)
> - __kvm_gmem_invalidate_start(f, start, end, attr_filter);
> + kvm_gmem_invalidate_start_filter(inode, start, end, attr_filter);
> }
>
> static void __kvm_gmem_invalidate_end(struct gmem_file *f, pgoff_t start,
> @@ -724,9 +731,14 @@ static int __kvm_gmem_set_attributes(struct inode
> *inode, pgoff_t start,
> /*
> * From this point on guest_memfd has performed necessary
> * checks and can proceed to do guest-breaking changes.
> + * Also, we don't have to invalidate the regions that
> + * may already be in the requested state. Hence, we could
> + * explicitly filter the invalidations to the opposite
> + * state.
> */
>
> - kvm_gmem_invalidate_start(inode, start, end);
> + kvm_gmem_invalidate_start_filter(inode, start, end,
> + to_private ? KVM_FILTER_SHARED :
> KVM_FILTER_PRIVATE);
>
I think this makes sense. Thanks for catching this.
> if (!to_private)
> kvm_gmem_invalidate(inode, start, end);
>
>
> Thoughts ?
>
> Suzuki
>
>
>>
>> [...snip...]
>>
^ 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