* SVSM Development Call March 18, 2026
From: Jörg Rödel @ 2026-03-17 17:06 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the call for agenda items for this weeks SVSM development call. Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory
From: Mostafa Saleh @ 2026-03-17 15:40 UTC (permalink / raw)
To: Jiri Pirko
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, jstultz, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, jgg, leon,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <xdy5anped2koy47cuxbbqocyypisl7lagwvpuokpzpggohk2dp@yilc5ihictph>
On Tue, Mar 17, 2026 at 02:37:02PM +0100, Jiri Pirko wrote:
> Tue, Mar 17, 2026 at 02:24:13PM +0100, smostafa@google.com wrote:
> >Hi Jiri,
> >
> >On Thu, Mar 05, 2026 at 01:36:39PM +0100, Jiri Pirko wrote:
> >> From: Jiri Pirko <jiri@nvidia.com>
> >>
> >> Confidential computing (CoCo) VMs/guests, such as AMD SEV and Intel TDX,
> >> run with encrypted/protected memory which creates a challenge
> >> for devices that do not support DMA to it (no TDISP support).
> >>
> >> For kernel-only DMA operations, swiotlb bounce buffering provides a
> >> transparent solution by copying data through decrypted memory.
> >> However, the only way to get this memory into userspace is via the DMA
> >> API's dma_alloc_pages()/dma_mmap_pages() type interfaces which limits
> >> the use of the memory to a single DMA device, and is incompatible with
> >> pin_user_pages().
> >>
> >> These limitations are particularly problematic for the RDMA subsystem
> >> which makes heavy use of pin_user_pages() and expects flexible memory
> >> usage between many different DMA devices.
> >>
> >> This patch series enables userspace to explicitly request decrypted
> >> (shared) memory allocations from the dma-buf system heap.
> >> Userspace can mmap this memory and pass the dma-buf fd to other
> >> existing importers such as RDMA or DRM devices to access the
> >> memory. The DMA API is improved to allow the dma heap exporter to DMA
> >> map the shared memory to each importing device.
> >
> >I have been looking into a similar problem with restricted-dma[1] and
> >the inability of the DMA API to recognize that a block of memory is
> >already decrypted.
> >
> >However, in your case, adding a new attr “DMA_ATTR_CC_DECRYPTED” works
> >well as dma-buf owns the memory, and is both responsible for the
> >set_memory_decrypted() and passing the DMA attrs.
> >
> >On the other hand, for restricted-dma, the memory decryption is deep
> >in the DMA direct memory allocation and the DMA API callers (for ex
> >virtio drivers) are clueless about it and can’t pass any attrs.
> >My proposal was specific to restricted-dma and won’t work for your case.
> >
> >I am wondering if the kernel should have a more solid, unified method
> >for identifying already-decrypted memory instead. Perhaps we need a
> >way for the DMA API to natively recognize the encryption state of a
> >physical page (working alongside force_dma_unencrypted(dev)), rather
> >than relying on caller-provided attributes?
>
> I actually had it originally implemented probably in the similar way you
> suggest. I had a bit in page/folio struct to indicate the
> "shared/decrypted" state. However I was told that adding such bit is
> basically a no-go. Isn't that right?
>
Yes, I believe it’s discouraged to add new fields to the struct page.
But I see the memory encryption API is spilling in different places
and I am not sure if that’s a good enough justification for that or
maybe we just need to re-architect it.
For the restricted-dma stuff, we don’t actually care about the
address, a device can either handle encryption or not, so relying on
force_dma_unencrypted(struct device *) which is implemented by the
architecture is enough, and we just need to integrate that so it
can be used from SWIOTLB and DMA-direct (and other places)
consistently. (although that might not be a simple as it sounds)
I am not sure in the dma-buf case if that would be enough, but
another way to have this per page and to avoid encoding this in
struct page, is to push this problem to the arch code and it can
rely on things as the page table (I believe ARM CCA have a bit
for that)
Anyway, I think there should be some boundaries in the kernel that
defines that instead of each subsystem having its assumptions,
especially memory encryption/decryption problems that can easily
cause security issues.
Thanks,
Mostafa
>
> >
> >[1] https://lore.kernel.org/all/20260305170335.963568-1-smostafa@google.com/
> >
> >Thanks,
> >Mostafa
> >
> >
> >>
> >> Jiri Pirko (2):
> >> dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
> >> dma-buf: heaps: system: add system_cc_decrypted heap for explicitly
> >> decrypted memory
> >>
> >> drivers/dma-buf/heaps/system_heap.c | 103 ++++++++++++++++++++++++++--
> >> include/linux/dma-mapping.h | 6 ++
> >> include/trace/events/dma.h | 3 +-
> >> kernel/dma/direct.h | 14 +++-
> >> 4 files changed, 117 insertions(+), 9 deletions(-)
> >>
> >> --
> >> 2.51.1
> >>
^ permalink raw reply
* Re: [PATCH v5 06/22] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Dave Hansen @ 2026-03-17 15:34 UTC (permalink / raw)
To: Kiryl Shutsemau, Chao Gao
Cc: x86, linux-coco, kvm, linux-kernel, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu
In-Reply-To: <abkmceqUtMTnLI0V@thinkstation>
On 3/17/26 03:06, Kiryl Shutsemau wrote:
>> + return sysfs_emit(buf, "%u.%u.%02u\n", info.major_version,
>> + info.minor_version,
>> + info.update_version);
> Maybe a comment why %02u used for update_version?
Could we just stick this in a:
#define TDX_VERSION_FMT "%u.%u.%02u"
and document it there, once and for all?
Users can do:
return sysfs_emit(buf, TDX_VERSION_FMT "\n", ...
There are going to be a couple of these, I expect.
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Lukas Wunner @ 2026-03-17 14:13 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, jgg, Donald Hunter, Jakub Kicinski
In-Reply-To: <69b88bfe2662f_40c01002c@dwillia2-mobl4.notmuch>
On Mon, Mar 16, 2026 at 04:02:22PM -0700, Dan Williams wrote:
> Dan Williams wrote:
> > Lukas Wunner wrote:
> > > This doesn't look like it's ever been tested, so at the very least
> > > it should be marked RFC in the subject to convey that it's not yet
> > > in a cut-and-dried state.
> >
> > The 16MB limit has indeed not been tested, the test script in this set
> > was using smaller than 64K payloads to check out the interface.
>
> So 16MB works ok, slow, but works. A given attribute in this
> implementation never exceeds the limit
Famous last words.
If you look at netlink_dump(), it sizes the skb based on
nlk->max_recvmsg_len. If that's larger than 64k, you'll
try to fill as much as possible of that space with a single
netlink attribute. The computation of "available" in your
patch doesn't take the 65531 bytes limit for a netlink attribute
into account so it looks like you'll end up overflowing the length
of the netlink attribute.
Unfortunately nla_put() doesn't prevent such overflows, it does
all the size calculations with an int, not a u16.
Thanks,
Lukas
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory
From: Jiri Pirko @ 2026-03-17 13:37 UTC (permalink / raw)
To: Mostafa Saleh
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, jstultz, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, jgg, leon,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <ablV_f_l7wD2m63E@google.com>
Tue, Mar 17, 2026 at 02:24:13PM +0100, smostafa@google.com wrote:
>Hi Jiri,
>
>On Thu, Mar 05, 2026 at 01:36:39PM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Confidential computing (CoCo) VMs/guests, such as AMD SEV and Intel TDX,
>> run with encrypted/protected memory which creates a challenge
>> for devices that do not support DMA to it (no TDISP support).
>>
>> For kernel-only DMA operations, swiotlb bounce buffering provides a
>> transparent solution by copying data through decrypted memory.
>> However, the only way to get this memory into userspace is via the DMA
>> API's dma_alloc_pages()/dma_mmap_pages() type interfaces which limits
>> the use of the memory to a single DMA device, and is incompatible with
>> pin_user_pages().
>>
>> These limitations are particularly problematic for the RDMA subsystem
>> which makes heavy use of pin_user_pages() and expects flexible memory
>> usage between many different DMA devices.
>>
>> This patch series enables userspace to explicitly request decrypted
>> (shared) memory allocations from the dma-buf system heap.
>> Userspace can mmap this memory and pass the dma-buf fd to other
>> existing importers such as RDMA or DRM devices to access the
>> memory. The DMA API is improved to allow the dma heap exporter to DMA
>> map the shared memory to each importing device.
>
>I have been looking into a similar problem with restricted-dma[1] and
>the inability of the DMA API to recognize that a block of memory is
>already decrypted.
>
>However, in your case, adding a new attr “DMA_ATTR_CC_DECRYPTED” works
>well as dma-buf owns the memory, and is both responsible for the
>set_memory_decrypted() and passing the DMA attrs.
>
>On the other hand, for restricted-dma, the memory decryption is deep
>in the DMA direct memory allocation and the DMA API callers (for ex
>virtio drivers) are clueless about it and can’t pass any attrs.
>My proposal was specific to restricted-dma and won’t work for your case.
>
>I am wondering if the kernel should have a more solid, unified method
>for identifying already-decrypted memory instead. Perhaps we need a
>way for the DMA API to natively recognize the encryption state of a
>physical page (working alongside force_dma_unencrypted(dev)), rather
>than relying on caller-provided attributes?
I actually had it originally implemented probably in the similar way you
suggest. I had a bit in page/folio struct to indicate the
"shared/decrypted" state. However I was told that adding such bit is
basically a no-go. Isn't that right?
>
>[1] https://lore.kernel.org/all/20260305170335.963568-1-smostafa@google.com/
>
>Thanks,
>Mostafa
>
>
>>
>> Jiri Pirko (2):
>> dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
>> dma-buf: heaps: system: add system_cc_decrypted heap for explicitly
>> decrypted memory
>>
>> drivers/dma-buf/heaps/system_heap.c | 103 ++++++++++++++++++++++++++--
>> include/linux/dma-mapping.h | 6 ++
>> include/trace/events/dma.h | 3 +-
>> kernel/dma/direct.h | 14 +++-
>> 4 files changed, 117 insertions(+), 9 deletions(-)
>>
>> --
>> 2.51.1
>>
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory
From: Mostafa Saleh @ 2026-03-17 13:24 UTC (permalink / raw)
To: Jiri Pirko
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, jstultz, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, jgg, leon,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260305123641.164164-1-jiri@resnulli.us>
Hi Jiri,
On Thu, Mar 05, 2026 at 01:36:39PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Confidential computing (CoCo) VMs/guests, such as AMD SEV and Intel TDX,
> run with encrypted/protected memory which creates a challenge
> for devices that do not support DMA to it (no TDISP support).
>
> For kernel-only DMA operations, swiotlb bounce buffering provides a
> transparent solution by copying data through decrypted memory.
> However, the only way to get this memory into userspace is via the DMA
> API's dma_alloc_pages()/dma_mmap_pages() type interfaces which limits
> the use of the memory to a single DMA device, and is incompatible with
> pin_user_pages().
>
> These limitations are particularly problematic for the RDMA subsystem
> which makes heavy use of pin_user_pages() and expects flexible memory
> usage between many different DMA devices.
>
> This patch series enables userspace to explicitly request decrypted
> (shared) memory allocations from the dma-buf system heap.
> Userspace can mmap this memory and pass the dma-buf fd to other
> existing importers such as RDMA or DRM devices to access the
> memory. The DMA API is improved to allow the dma heap exporter to DMA
> map the shared memory to each importing device.
I have been looking into a similar problem with restricted-dma[1] and
the inability of the DMA API to recognize that a block of memory is
already decrypted.
However, in your case, adding a new attr “DMA_ATTR_CC_DECRYPTED” works
well as dma-buf owns the memory, and is both responsible for the
set_memory_decrypted() and passing the DMA attrs.
On the other hand, for restricted-dma, the memory decryption is deep
in the DMA direct memory allocation and the DMA API callers (for ex
virtio drivers) are clueless about it and can’t pass any attrs.
My proposal was specific to restricted-dma and won’t work for your case.
I am wondering if the kernel should have a more solid, unified method
for identifying already-decrypted memory instead. Perhaps we need a
way for the DMA API to natively recognize the encryption state of a
physical page (working alongside force_dma_unencrypted(dev)), rather
than relying on caller-provided attributes?
[1] https://lore.kernel.org/all/20260305170335.963568-1-smostafa@google.com/
Thanks,
Mostafa
>
> Jiri Pirko (2):
> dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
> dma-buf: heaps: system: add system_cc_decrypted heap for explicitly
> decrypted memory
>
> drivers/dma-buf/heaps/system_heap.c | 103 ++++++++++++++++++++++++++--
> include/linux/dma-mapping.h | 6 ++
> include/trace/events/dma.h | 3 +-
> kernel/dma/direct.h | 14 +++-
> 4 files changed, 117 insertions(+), 9 deletions(-)
>
> --
> 2.51.1
>
^ permalink raw reply
* Re: [PATCH v5 12/22] x86/virt/tdx: Reset software states during TDX module shutdown
From: Huang, Kai @ 2026-03-17 11:08 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
yilun.xu@linux.intel.com, dave.hansen@linux.intel.com,
bp@alien8.de, tony.lindgren@linux.intel.com, mingo@redhat.com,
Weiny, Ira, pbonzini@redhat.com, seanjc@google.com,
Verma, Vishal L, nik.borisov@suse.com, binbin.wu@linux.intel.com,
kas@kernel.org, sagis@google.com, Annapurve, Vishal,
hpa@zytor.com, Edgecombe, Rick P, linux-kernel@vger.kernel.org,
Chatre, Reinette, tglx@kernel.org, paulmck@kernel.org,
x86@kernel.org, Williams, Dan J
In-Reply-To: <abkQWsqR7BURMZxc@intel.com>
On Tue, 2026-03-17 at 16:27 +0800, Gao, Chao wrote:
> On Mon, Mar 16, 2026 at 05:06:49PM +0800, Huang, Kai wrote:
> >
> > > @@ -1179,6 +1179,7 @@ EXPORT_SYMBOL_FOR_KVM(tdx_enable);
> > > int tdx_module_shutdown(void)
> > > {
> > > struct tdx_module_args args = {};
> > > + int ret, cpu;
> > >
> > > /*
> > > * Shut down the TDX module and prepare handoff data for the next
> > > @@ -1188,7 +1189,22 @@ int tdx_module_shutdown(void)
> > > * modules as new modules likely have higher handoff version.
> > > */
> > > args.rcx = tdx_sysinfo.handoff.module_hv;
> > > - return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
> > > + ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + tdx_module_status = TDX_MODULE_UNINITIALIZED;
> > > + sysinit_done = false;
> > > + sysinit_ret = 0;
> > > +
> > > + /*
> > > + * By reaching here CPUHP is disabled and all present CPUs
> > > + * are online. It's safe to just loop all online CPUs and
> > > + * reset the per-cpu flag.
> > > + */
> > > + for_each_online_cpu(cpu)
> > > + per_cpu(tdx_lp_initialized, cpu) = false;
> >
> > Since you have removed the requirement that P-SEAMLDR.INSTALL must be done
> > on all CPUs, and removed the relevant patch, the "all present CPUs are
> > online" part isn't correct anymore.
> >
> > And using for_each_online_cpu() isn't enough since this doesn't reset the
> > tdx_lp_initialized for offline CPUs.
> >
> > One way is to just use for_each_possible_cpu() here so tdx_lp_initialized
> > for all CPUs are reset. Since the "CPUHP is disabled" part is still correct
> > AFAICT (since stop_machine() disables CPUHP internally during the
> > operation), resetting tdx_lp_initialized for offline CPUs won't race with
> > CPUHP.
> >
> > And assuming this series will be applied after Sean's VMXON series, we will
>
> Yes.
>
> > have a TDX-specific CPUHP callback tdx_online_cpu() in TDX x86 core to do
> > tdx_cpu_enable(), which will then enable TDX again on the new-online CPU.
>
> Good point.
>
> Clearing tdx_lp_initialized for offlined CPUs makes sense, but I'd rather not
> justify this through "enabling TDX on new-online CPUs" since many details
> remain unclear. For example, there will be a SEAMCALL to disable TDX per-CPU.
Sorry which SEAMCALL are you referring to?
> It should be called when CPUs go offline so that those CPUs can be exempting
> from doing SEAMLDR.INSTALL during module updates. tdx_lp_initialized should
> have been cleared along with that "disable TDX per-CPU" SEAMCALL for offlined
> CPUs.
Hmm.. It sounds like we are missing some background here. Could you
elaborate what are the steps to update module on a subset of CPUs?
^ permalink raw reply
* Re: [PATCH v5 08/22] x86/virt/seamldr: Allocate and populate a module update request
From: Kiryl Shutsemau @ 2026-03-17 10:59 UTC (permalink / raw)
To: Chao Gao
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-9-chao.gao@intel.com>
On Sun, Mar 15, 2026 at 06:58:28AM -0700, Chao Gao wrote:
> P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX module
> update requests. This structure contains physical addresses pointing to
> the module binary and its signature file (or sigstruct), along with an
> update scenario field.
>
> TDX modules are distributed in the tdx_blob format defined in
> blob_structure.txt from the "Intel TDX module Binaries Repository". A
> tdx_blob contains a header, sigstruct, and module binary. This is also the
> format supplied by the userspace to the kernel.
>
> Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
> accordingly. This structure will be passed to P-SEAMLDR to initiate the
> update.
>
> Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
> a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
> Specification" will be published separately. P-SEAMLDR compatibility
> validation (such as 4KB vs 16KB sigstruct support) is left to userspace,
> which must verify the P-SEAMLDR version meets the TDX module's minimum
> requirements.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> v5:
> - use a macro for tdx_blob version (0x100) [Yan]
> - don't do alignment checking for the binary/sigstruct [Rick]
> - drop blob's sigstruct and validation checking
> - set seamldr_params.version to 1 when necessary
> - drop the link to blob_structure.txt which might be unstable [Kai]
>
> v4:
> - Remove checksum verification as it is optional
> - Convert comments to is_vmalloc_addr() checks [Kai]
> - Explain size/alignment checks in alloc_seamldr_params() [Kai]
>
> v3:
> - Print tdx_blob version in hex [Binbin]
> - Drop redundant sigstruct alignment check [Yilun]
> - Note buffers passed from firmware upload infrastructure are
> vmalloc()'d above alloc_seamldr_params()
> ---
> arch/x86/virt/vmx/tdx/seamldr.c | 141 ++++++++++++++++++++++++++++++++
> 1 file changed, 141 insertions(+)
>
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 7114326d7569..20cb6c797ce5 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -7,6 +7,7 @@
> #define pr_fmt(fmt) "seamldr: " fmt
>
> #include <linux/mm.h>
> +#include <linux/slab.h>
> #include <linux/spinlock.h>
>
> #include <asm/seamldr.h>
> @@ -16,6 +17,33 @@
> /* P-SEAMLDR SEAMCALL leaf function */
> #define P_SEAMLDR_INFO 0x8000000000000000
>
> +#define SEAMLDR_MAX_NR_MODULE_4KB_PAGES 496
> +#define SEAMLDR_MAX_NR_SIG_4KB_PAGES 4
> +
> +/*
> + * The seamldr_params "scenario" field specifies the operation mode:
> + * 0: Install TDX module from scratch (not used by kernel)
> + * 1: Update existing TDX module to a compatible version
> + */
> +#define SEAMLDR_SCENARIO_UPDATE 1
> +
> +/*
> + * This is called the "SEAMLDR_PARAMS" data structure and is defined
> + * in "SEAM Loader (SEAMLDR) Interface Specification".
> + *
> + * It describes the TDX module that will be installed.
> + */
> +struct seamldr_params {
> + u32 version;
> + u32 scenario;
> + u64 sigstruct_pa[SEAMLDR_MAX_NR_SIG_4KB_PAGES];
> + u8 reserved[80];
> + u64 num_module_pages;
> + u64 mod_pages_pa_list[SEAMLDR_MAX_NR_MODULE_4KB_PAGES];
> +} __packed;
> +
> +static_assert(sizeof(struct seamldr_params) == 4096);
> +
> /*
> * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> * interact with P-SEAMLDR simultaneously. Use raw version as the calls can
> @@ -41,6 +69,114 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
> }
> EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>
> +static void free_seamldr_params(struct seamldr_params *params)
> +{
> + free_page((unsigned long)params);
> +}
> +
> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
> + const void *sig, unsigned int sig_size)
> +{
> + struct seamldr_params *params;
> + const u8 *ptr;
> + int i;
> +
> + if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> + return ERR_PTR(-EINVAL);
> +
> + if (sig_size > SEAMLDR_MAX_NR_SIG_4KB_PAGES * SZ_4K)
> + return ERR_PTR(-EINVAL);
> +
> + params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
> + if (!params)
> + return ERR_PTR(-ENOMEM);
> +
> + /*
> + * Only use version 1 when required (sigstruct > 4KB) for backward
> + * compatibility with P-SEAMLDR that lacks version 1 support.
> + */
> + if (sig_size > SZ_4K)
> + params->version = 1;
> + else
> + params->version = 0;
> +
> + params->scenario = SEAMLDR_SCENARIO_UPDATE;
> +
> + ptr = sig;
> + for (i = 0; i < sig_size / SZ_4K; i++) {
> + /*
> + * Don't assume @sig is page-aligned although it is 4KB-aligned.
> + * Always add the in-page offset to get the physical address.
> + */
I don't follow this. If @sig is 4k aligned in VA, it is page aligned.
If you want to handle case when @sig is not 4k aligned, than this is
broken. You need to bump ptr to the next 4k boundary, not by 4k.
> + params->sigstruct_pa[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> + ((unsigned long)ptr & ~PAGE_MASK);
> + ptr += SZ_4K;
> + }
> +
> + params->num_module_pages = module_size / SZ_4K;
> +
> + ptr = module;
> + for (i = 0; i < params->num_module_pages; i++) {
> + params->mod_pages_pa_list[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> + ((unsigned long)ptr & ~PAGE_MASK);
> + ptr += SZ_4K;
Same here.
> + }
> +
> + return params;
> +}
> +
> +/*
> + * Intel TDX module blob. Its format is defined at:
> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
> + *
> + * Note this structure differs from the reference above: the two variable-length
> + * fields "@sigstruct" and "@module" are represented as a single "@data" field
> + * here and split programmatically using the offset_of_module value.
> + */
> +struct tdx_blob {
> + u16 version;
> + u16 checksum;
> + u32 offset_of_module;
> + u8 signature[8];
> + u32 length;
> + u32 reserved0;
> + u64 reserved1[509];
> + u8 data[];
> +} __packed;
> +
> +/* Supported versions of the tdx_blob */
> +#define TDX_BLOB_VERSION_1 0x100
> +
> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> +{
> + const struct tdx_blob *blob = (const void *)data;
> + int module_size, sig_size;
> + const void *sig, *module;
> +
> + /* Ensure the size is valid otherwise reading any field from the blob may overflow. */
> + if (size <= sizeof(struct tdx_blob) || size <= blob->offset_of_module)
> + return ERR_PTR(-EINVAL);
> +
> + if (blob->version != TDX_BLOB_VERSION_1) {
> + pr_err("unsupported blob version: %x\n", blob->version);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + /* Split the blob into a sigstruct and a module. */
> + sig = blob->data;
> + sig_size = blob->offset_of_module - sizeof(struct tdx_blob);
> + module = data + blob->offset_of_module;
> + module_size = size - blob->offset_of_module;
> +
> + if (sig_size <= 0 || module_size <= 0 || blob->length != size)
> + return ERR_PTR(-EINVAL);
Maybe add a comment somewhere that block->offset_of_module is relative
to start of struct tdx_blob, not blob->data and blob->length includes
length of struct tdx_blob.
It can be either way and it is better to give a reader a hint.
> +
> + return alloc_seamldr_params(module, module_size, sig, sig_size);
> +}
> +
> +DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
> + if (!IS_ERR_OR_NULL(_T)) free_seamldr_params(_T))
> +
> /**
> * seamldr_install_module - Install a new TDX module.
> * @data: Pointer to the TDX module update blob.
> @@ -50,6 +186,11 @@ EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
> */
> int seamldr_install_module(const u8 *data, u32 size)
> {
> + struct seamldr_params *params __free(free_seamldr_params) =
> + init_seamldr_params(data, size);
> + if (IS_ERR(params))
> + return PTR_ERR(params);
> +
> /* TODO: Update TDX module here */
> return 0;
> }
> --
> 2.47.3
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v5 07/22] coco/tdx-host: Implement firmware upload sysfs ABI for TDX module updates
From: Kiryl Shutsemau @ 2026-03-17 10:20 UTC (permalink / raw)
To: Chao Gao
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-8-chao.gao@intel.com>
On Sun, Mar 15, 2026 at 06:58:27AM -0700, Chao Gao wrote:
> Linux kernel supports two primary firmware update mechanisms:
> - request_firmware()
> - firmware upload (or fw_upload)
>
> The former is used by microcode updates, SEV firmware updates, etc. The
> latter is used by CXL and FPGA firmware updates.
>
> One key difference between them is: request_firmware() loads a named
> file from the filesystem where the filename is kernel-controlled, while
> fw_upload accepts firmware data directly from userspace.
>
> Use fw_upload for TDX module updates as loading a named file isn't
> suitable for TDX (see below for more reasons). Specifically, register
> TDX faux device with fw_upload framework to expose sysfs interfaces
> and implement operations to process data blobs supplied by userspace.
>
> Implementation notes:
> 1. P-SEAMLDR processes the entire update at once rather than
> chunk-by-chunk, so .write() is called only once per update; so the
> offset should be always 0.
> 2. An update completes synchronously within .write(), meaning
> .poll_complete() is only called after the update succeeds and so always
> returns success
>
> Why fw_upload instead of request_firmware()?
> ============================================
> The explicit file selection capabilities of fw_upload is preferred over
> the implicit file selection of request_firmware() for the following
> reasons:
>
> a. Intel distributes all versions of the TDX module, allowing admins to
> load any version rather than always defaulting to the latest. This
> flexibility is necessary because future extensions may require reverting to
> a previous version to clear fatal errors.
>
> b. Some module version series are platform-specific. For example, the 1.5.x
> series is for certain platform generations, while the 2.0.x series is
> intended for others.
>
> c. The update policy for TDX module updates is non-linear at times. The
> latest TDX module may not be compatible. For example, TDX module 1.5.x
> may be updated to 1.5.y but not to 1.5.y+1. This policy is documented
> separately in a file released along with each TDX module release.
>
> So, the default policy of "request_firmware()" of "always load latest", is
> not suitable for TDX. Userspace needs to deploy a more sophisticated policy
> check (e.g., latest may not be compatible), and there is potential
> operator choice to consider.
>
> Just have userspace pick rather than add kernel mechanism to change the
> default policy of request_firmware().
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
One minor thing below.
> ---
> v5:
> - remove a tail comment [Yan]
> - remove is_vmalloc_addr() check [Dave]
> - use devm_add_action_or_reset() for deinit [Yilun]
> - remove global tdx_fwl [Yilun]
> - clarify request_firmware() doesn't take filename from userspace
> [Rick]
>
> v4:
> - make tdx_fwl static [Kai]
> - don't support update canceling [Yilun]
> - explain why seamldr_init() doesn't return an error [Kai]
> - bail out if TDX module updates are not supported [Kai]
> - name the firmware "tdx_module" instead of "seamldr_upload" [Cedric]
>
> v3:
> - clear "cancel_request" in the "prepare" phase [Binbin]
> - Don't fail the whole tdx-host device if seamldr_init() met an error
> [Yilun]
> - Add kdoc for seamldr_install_module() and verify that the input
> buffer is vmalloc'd. [Yilun]
> ---
> arch/x86/include/asm/seamldr.h | 1 +
> arch/x86/include/asm/tdx.h | 6 ++
> arch/x86/virt/vmx/tdx/seamldr.c | 15 +++++
> drivers/virt/coco/tdx-host/Kconfig | 2 +
> drivers/virt/coco/tdx-host/tdx-host.c | 87 +++++++++++++++++++++++++++
> 5 files changed, 111 insertions(+)
>
> diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
> index c67e5bc910a9..ac6f80f7208b 100644
> --- a/arch/x86/include/asm/seamldr.h
> +++ b/arch/x86/include/asm/seamldr.h
> @@ -32,5 +32,6 @@ struct seamldr_info {
> static_assert(sizeof(struct seamldr_info) == 256);
>
> int seamldr_get_info(struct seamldr_info *seamldr_info);
> +int seamldr_install_module(const u8 *data, u32 size);
>
> #endif /* _ASM_X86_SEAMLDR_H */
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index cb2219302dfc..b3a7301e77c6 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -103,6 +103,12 @@ int tdx_enable(void);
> const char *tdx_dump_mce_info(struct mce *m);
> const struct tdx_sys_info *tdx_get_sysinfo(void);
>
> +static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinfo)
> +{
> + /* To be enabled when kernel is ready. */
> + return false;
> +}
> +
> int tdx_guest_keyid_alloc(void);
> u32 tdx_get_nr_guest_keyids(void);
> void tdx_guest_keyid_free(unsigned int keyid);
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 7c0cbab2c4c0..7114326d7569 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -6,6 +6,7 @@
> */
> #define pr_fmt(fmt) "seamldr: " fmt
>
> +#include <linux/mm.h>
> #include <linux/spinlock.h>
>
> #include <asm/seamldr.h>
> @@ -39,3 +40,17 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
> return seamldr_call(P_SEAMLDR_INFO, &args);
> }
> EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
> +
> +/**
> + * seamldr_install_module - Install a new TDX module.
> + * @data: Pointer to the TDX module update blob.
> + * @size: Size of the TDX module update blob.
> + *
> + * Returns 0 on success, negative error code on failure.
> + */
> +int seamldr_install_module(const u8 *data, u32 size)
> +{
> + /* TODO: Update TDX module here */
> + return 0;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
> diff --git a/drivers/virt/coco/tdx-host/Kconfig b/drivers/virt/coco/tdx-host/Kconfig
> index d35d85ef91c0..ca600a39d97b 100644
> --- a/drivers/virt/coco/tdx-host/Kconfig
> +++ b/drivers/virt/coco/tdx-host/Kconfig
> @@ -1,6 +1,8 @@
> config TDX_HOST_SERVICES
> tristate "TDX Host Services Driver"
> depends on INTEL_TDX_HOST
> + select FW_LOADER
> + select FW_UPLOAD
> default m
> help
> Enable access to TDX host services like module update and
> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> index 8d46e3c039ba..1b93d20406c1 100644
> --- a/drivers/virt/coco/tdx-host/tdx-host.c
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/device/faux.h>
> +#include <linux/firmware.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> #include <linux/sysfs.h>
> @@ -94,8 +95,94 @@ static const struct attribute_group seamldr_group = {
> .attrs = seamldr_attrs,
> };
>
> +static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
> + const u8 *data, u32 size)
> +{
> + return FW_UPLOAD_ERR_NONE;
> +}
> +
> +static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
> + u32 offset, u32 size, u32 *written)
> +{
> + int ret;
> +
> + /*
> + * tdx_fw_write() always processes all data on the first call with
> + * offset == 0. Since it never returns partial success (it either
> + * succeeds completely or fails), there is no subsequent call with
> + * non-zero offsets.
> + */
> + WARN_ON_ONCE(offset);
> + ret = seamldr_install_module(data, size);
> + switch (ret) {
> + case 0:
> + *written = size;
> + return FW_UPLOAD_ERR_NONE;
> + case -EBUSY:
> + return FW_UPLOAD_ERR_BUSY;
> + case -EIO:
> + return FW_UPLOAD_ERR_HW_ERROR;
> + case -ENOSPC:
> + return FW_UPLOAD_ERR_WEAROUT;
> + case -ENOMEM:
> + return FW_UPLOAD_ERR_RW_ERROR;
> + default:
> + return FW_UPLOAD_ERR_FW_INVALID;
> + }
> +}
> +
> +static enum fw_upload_err tdx_fw_poll_complete(struct fw_upload *fwl)
> +{
> + /*
> + * TDX module updates are completed in the previous phase
> + * (tdx_fw_write()). If any error occurred, the previous phase
> + * would return an error code to abort the update process. In
> + * other words, reaching this point means the update succeeded.
> + */
> + return FW_UPLOAD_ERR_NONE;
> +}
> +
> +/*
> + * TDX module updates cannot be cancelled. Provide a stub function since
> + * the firmware upload framework requires a .cancel operation.
> + */
> +static void tdx_fw_cancel(struct fw_upload *fwl)
> +{
> +}
> +
> +static const struct fw_upload_ops tdx_fw_ops = {
> + .prepare = tdx_fw_prepare,
> + .write = tdx_fw_write,
> + .poll_complete = tdx_fw_poll_complete,
> + .cancel = tdx_fw_cancel,
> +};
> +
> +static void seamldr_deinit(void *tdx_fwl)
> +{
> + firmware_upload_unregister(tdx_fwl);
> +}
> +
> static int seamldr_init(struct device *dev)
> {
> + const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> + struct fw_upload *tdx_fwl;
> + int ret;
> +
> + if (WARN_ON_ONCE(!tdx_sysinfo))
> + return -EIO;
> +
> + if (!tdx_supports_runtime_update(tdx_sysinfo))
> + return 0;
Hm. Do we still want to register seamldr_group for this case?
Maybe move it up before the check?
> +
> + tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "tdx_module",
> + &tdx_fw_ops, NULL);
> + if (IS_ERR(tdx_fwl))
> + return PTR_ERR(tdx_fwl);
> +
> + ret = devm_add_action_or_reset(dev, seamldr_deinit, tdx_fwl);
> + if (ret)
> + return ret;
> +
> return devm_device_add_group(dev, &seamldr_group);
> }
>
> --
> 2.47.3
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v5 06/22] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Kiryl Shutsemau @ 2026-03-17 10:06 UTC (permalink / raw)
To: Chao Gao
Cc: x86, linux-coco, kvm, linux-kernel, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu
In-Reply-To: <20260315135920.354657-7-chao.gao@intel.com>
On Sun, Mar 15, 2026 at 06:58:26AM -0700, Chao Gao wrote:
> TDX module updates require userspace to select the appropriate module
> to load. Expose necessary information to facilitate this decision. Two
> values are needed:
>
> - P-SEAMLDR version: for compatibility checks between TDX module and
> P-SEAMLDR
> - num_remaining_updates: indicates how many updates can be performed
>
> Expose them as tdx-host device attributes. Register these attributes
> during device probe rather than creation, making it easier to hide them
> when P-SEAMLDR calls are unsafe (due to CPU erratum).
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Few nitpicks below.
> ---
> v5:
> - fix typos [Binbin]
> - register seamldr_group during device probe
> v4:
> - Make seamldr attribute permission "0400" [Dave]
> - Don't include implementation details in OS ABI docs [Dave]
> - Tag tdx_host_group as static [Kai]
>
> v3:
> - use #ifdef rather than .is_visible() to control P-SEAMLDR sysfs
> visibility [Yilun]
> ---
> .../ABI/testing/sysfs-devices-faux-tdx-host | 23 +++++++
> drivers/virt/coco/tdx-host/tdx-host.c | 68 ++++++++++++++++++-
> 2 files changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> index 2cf682b65acf..44b8356aed6b 100644
> --- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> +++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> @@ -4,3 +4,26 @@ Description: (RO) Report the version of the loaded TDX module. The TDX module
> version is formatted as x.y.z, where "x" is the major version,
> "y" is the minor version and "z" is the update version. Versions
> are used for bug reporting, TDX module updates etc.
> +
> +What: /sys/devices/faux/tdx_host/seamldr/version
> +Contact: linux-coco@lists.linux.dev
> +Description: (RO) Report the version of the loaded SEAM loader. The SEAM
> + loader version is formatted as x.y.z, where "x" is the major
> + version, "y" is the minor version and "z" is the update version.
> + Versions are used for bug reporting and compatibility checks.
> +
> +What: /sys/devices/faux/tdx_host/seamldr/num_remaining_updates
> +Contact: linux-coco@lists.linux.dev
> +Description: (RO) Report the number of remaining updates. TDX maintains a
> + log about each TDX module that has been loaded. This log has
> + a finite size, which limits the number of TDX module updates
> + that can be performed.
> +
> + After each successful update, the number reduces by one. Once it
> + reaches zero, further updates will fail until next reboot. The
> + number is always zero if the P-SEAMLDR doesn't support updates.
> +
> + See Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
> + Interface Specification, Revision 343755-003, Chapter 3.3
> + "SEAMLDR_INFO" and Chapter 4.2 "SEAMLDR.INSTALL" for more
> + information.
Do you think revision and chapter numbers useful here?
> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> index 0424933b2560..8d46e3c039ba 100644
> --- a/drivers/virt/coco/tdx-host/tdx-host.c
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> @@ -11,6 +11,7 @@
> #include <linux/sysfs.h>
>
> #include <asm/cpu_device_id.h>
> +#include <asm/seamldr.h>
> #include <asm/tdx.h>
>
> static const struct x86_cpu_id tdx_host_ids[] = {
> @@ -42,6 +43,71 @@ static struct attribute *tdx_host_attrs[] = {
> };
> ATTRIBUTE_GROUPS(tdx_host);
>
> +static ssize_t seamldr_version_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct seamldr_info info;
> + int ret;
> +
> + ret = seamldr_get_info(&info);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, "%u.%u.%02u\n", info.major_version,
> + info.minor_version,
> + info.update_version);
Maybe a comment why %02u used for update_version?
> +}
> +
> +static ssize_t num_remaining_updates_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct seamldr_info info;
> + int ret;
> +
> + ret = seamldr_get_info(&info);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, "%u\n", info.num_remaining_updates);
> +}
> +
> +/*
> + * Open-code DEVICE_ATTR_ADMIN_RO to specify a different 'show' function
> + * for P-SEAMLDR version as version_show() is used for TDX module version.
> + *
> + * Admin-only readable as reading these attributes calls into P-SEAMLDR,
> + * which may have potential performance and system impact.
> + */
> +static struct device_attribute dev_attr_seamldr_version =
> + __ATTR(version, 0400, seamldr_version_show, NULL);
> +static DEVICE_ATTR_ADMIN_RO(num_remaining_updates);
> +
> +static struct attribute *seamldr_attrs[] = {
> + &dev_attr_seamldr_version.attr,
> + &dev_attr_num_remaining_updates.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group seamldr_group = {
> + .name = "seamldr",
> + .attrs = seamldr_attrs,
> +};
> +
> +static int seamldr_init(struct device *dev)
> +{
> + return devm_device_add_group(dev, &seamldr_group);
> +}
> +
> +static int tdx_host_probe(struct faux_device *fdev)
> +{
> + return seamldr_init(&fdev->dev);
> +}
> +
> +static const struct faux_device_ops tdx_host_ops = {
> + .probe = tdx_host_probe,
> +};
> +
> static struct faux_device *fdev;
>
> static int __init tdx_host_init(void)
> @@ -49,7 +115,7 @@ static int __init tdx_host_init(void)
> if (!x86_match_cpu(tdx_host_ids) || !tdx_get_sysinfo())
> return -ENODEV;
>
> - fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, NULL, tdx_host_groups);
> + fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, &tdx_host_ops, tdx_host_groups);
> if (!fdev)
> return -ENODEV;
>
> --
> 2.47.3
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v5 05/22] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Kiryl Shutsemau @ 2026-03-17 10:00 UTC (permalink / raw)
To: Chao Gao
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, Farrah Chen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin
In-Reply-To: <abio7iAJhGdeEEcE@intel.com>
On Tue, Mar 17, 2026 at 09:05:50AM +0800, Chao Gao wrote:
> >> +
> >> +int seamldr_get_info(struct seamldr_info *seamldr_info)
> >> +{
> >> + /*
> >> + * Use slow_virt_to_phys() since @seamldr_info may be allocated on
> >> + * the stack.
> >> + */
> >> + struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
> >> +
> >> + return seamldr_call(P_SEAMLDR_INFO, &args);
> >
> >On what condition this information can change?
>
> After each update, the num_remaining_updates field will change.
>
> >
> >I see the next patch calls this on every _show operation. Would we
> >benefit from caching the response?
>
> Yes, caching would save a few cycles, but adds complexity. we'd need to keep
> the cache synchronized and invalidate it when values change, likely requiring
> additional state tracking.
>
> Dave recommended allocating the structure on the stack [*] when I raised the
> caching idea.
>
> This isn't a hot path, so I prefer keeping it "simple and stupid".
Fair enough.
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v5 04/22] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Kiryl Shutsemau @ 2026-03-17 9:59 UTC (permalink / raw)
To: Chao Gao
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, Farrah Chen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin
In-Reply-To: <abijs4Ij97L4Rp9E@intel.com>
On Tue, Mar 17, 2026 at 08:43:31AM +0800, Chao Gao wrote:
> >> +/*
> >> + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> >> + * interact with P-SEAMLDR simultaneously. Use raw version as the calls can
> >> + * be made with interrupts disabled.
> >
> >Hm. I am not sure how it explains use of raw_spinlock. What's wrong with
> >using plain spinlock with interrupts disabled?
>
> We mustn't acquire sleeping lock with interrupts disabled. in RT kernel, a plain
> spinlock becomes a sleeping lock.
>
> The lock doc [*] states:
> On PREEMPT_RT kernels, these lock types are converted to sleeping locks:
> local_lock
> spinlock_t
> rwlock_t
>
> *:https://docs.kernel.org/locking/locktypes.html
>
> and also see the test I did:
>
> https://lore.kernel.org/kvm/abPEit+Ckeqmypv+@intel.com/
>
> How about:
>
> /*
> * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> * interact with P-SEAMLDR simultaneously. Use raw version as the calls can
> * be made with interrupts disabled, where plain spinlocks are prohibited in
> * PREEMPT_RT kernels as they become sleeping locks.
> */
Looks better. Thanks for the explanation.
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 3/4] x86/virt/tdx: Add SEAMCALL wrapper for TDH.SYS.DISABLE
From: Kiryl Shutsemau @ 2026-03-17 9:47 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: tglx@kernel.org, Hansen, Dave, seanjc@google.com, bp@alien8.de,
x86@kernel.org, ackerleytng@google.com, hpa@zytor.com,
linux-kernel@vger.kernel.org, mingo@redhat.com, Huang, Kai,
kvm@vger.kernel.org, linux-coco@lists.linux.dev,
pbonzini@redhat.com, Verma, Vishal L, Gao, Chao
In-Reply-To: <ac341cedf0e387f2ba5cf3fc9d2ad5fcaa2ec3c8.camel@intel.com>
On Mon, Mar 16, 2026 at 09:15:13PM +0000, Edgecombe, Rick P wrote:
> On Mon, 2026-03-16 at 11:51 +0000, Kiryl Shutsemau wrote:
> > On Fri, Mar 06, 2026 at 05:03:57PM -0800, Rick Edgecombe wrote:
> > > From: Vishal Verma <vishal.l.verma@intel.com>
> > >
> > > Some early TDX-capable platforms have an erratum where a partial write
> > > to TDX private memory can cause a machine check on a subsequent read.
> > > On these platforms, kexec and kdump have been disabled in these cases,
> > > because the old kernel cannot safely hand off TDX state to the new
> > > kernel. Later TDX modules support the TDH.SYS.DISABLE SEAMCALL, which
> > > provides a way to cleanly disable TDX and allow kexec to proceed.
> >
> > Does it need to be enumerated?
> >
> > I don't see this SEAMCALL be covered in the public documentation.
> > </me looking around>
> > Ah! Found it the the draft. So the feature is not yet finalized.
> >
> > "Support of TDH.SYS.DISABLE is enumerated by TDX_FEATURES0. SYS_DISABLE
> > (bit 53)"
> >
> > I am seeing the next patch calling it unconditionally. Is it okay?
>
> We debated checking the feature bit before allowing kexec, but decided it was
> simpler to just blindly call and ignore the errors. The reasoning was that this
> is already a somewhat exotic scenario being addressed, and future modules will
> have the feature. So maintaining a check for the feature bit only helps a little
> bit, for a short time. And then only if the user would rather have kexec blocked
> than attempt it. Do you think it is worth it?
No, I see very limited reason to support stale TDX modules. Users are
expected to keep the module up-to-date, so skipping enumeration should
be okay. But it deserves explanation in the commit message or a comment.
> > > This can be a long running operation, and the time needed largely
> > > depends on the amount of memory that has been allocated to TDs. If all
> > > TDs have been destroyed prior to the sys_disable call, then it is fast,
> > > with only needing to override the TDX module memory.
> > >
> > > After the SEAMCALL completes, the TDX module is disabled and all memory
> > > resources allocated to TDX are freed and reset. The next kernel can then
> > > re-initialize the TDX module from scratch via the normal TDX bring-up
> > > sequence.
> > >
> > > The SEAMCALL may be interrupted by an interrupt. In this case, it
> > > returns TDX_INTERRUPTED_RESUMABLE, and it must be retried in a loop
> > > until the operation completes successfully.
> > >
> > > Add a tdx_sys_disable() helper, which implements the retry loop around
> > > the SEAMCALL to provide this functionality.
> > >
> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > > ---
> > > arch/x86/include/asm/tdx.h | 3 +++
> > > arch/x86/virt/vmx/tdx/tdx.c | 18 ++++++++++++++++++
> > > arch/x86/virt/vmx/tdx/tdx.h | 1 +
> > > 3 files changed, 22 insertions(+)
> > >
> > > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> > > index f0826b0a512a..baaf43a09e99 100644
> > > --- a/arch/x86/include/asm/tdx.h
> > > +++ b/arch/x86/include/asm/tdx.h
> > > @@ -173,6 +173,8 @@ static inline int pg_level_to_tdx_sept_level(enum pg_level level)
> > > return level - 1;
> > > }
> > >
> > > +void tdx_sys_disable(void);
> > > +
> > > u64 tdh_vp_enter(struct tdx_vp *vp, struct tdx_module_args *args);
> > > u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
> > > u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2);
> > > @@ -204,6 +206,7 @@ static inline void tdx_init(void) { }
> > > 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 void tdx_sys_disable(void) { }
> > > #endif /* CONFIG_INTEL_TDX_HOST */
> > >
> > > #endif /* !__ASSEMBLER__ */
> > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > > index 0802d0fd18a4..68bd2618dde4 100644
> > > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > > @@ -37,6 +37,7 @@
> > > #include <asm/msr.h>
> > > #include <asm/cpufeature.h>
> > > #include <asm/tdx.h>
> > > +#include <asm/shared/tdx_errno.h>
> > > #include <asm/cpu_device_id.h>
> > > #include <asm/processor.h>
> > > #include <asm/mce.h>
> > > @@ -1940,3 +1941,20 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page)
> > > return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
> > > }
> > > EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_hkid);
> > > +
> > > +void tdx_sys_disable(void)
> > > +{
> > > + struct tdx_module_args args = {};
> > > +
> > > + /*
> > > + * SEAMCALLs that can return TDX_INTERRUPTED_RESUMABLE are guaranteed
> > > + * to make forward progress between interrupts, so it is safe to loop
> > > + * unconditionally here.
> > > + *
> > > + * This is a 'destructive' SEAMCALL, in that no other SEAMCALL can be
> > > + * run after this until a full reinitialization is done.
> > > + */
> > > + while (seamcall(TDH_SYS_DISABLE, &args) == TDX_INTERRUPTED_RESUMABLE)
> > > + ;
> >
> > Silently ignore any other errors?
>
> Do you think it's worth a warn? There are a couple other considerations.
> - Kai brought up offline that we should handle TDX_SYS_BUSY here too.
> - Previous kexec patches had trouble solving races around tdx enabling. So we
> have to handle the seamcall failures.
>
> So we have to exclude a few different errors in different ways. And then the
> warn worthy error codes either don't impact anything, or the new kernel will
> fail to initialize the TDX module and give notice there.
The delayed error is harder to debug. It can be useful to leave a
breadcrumbs.
Also, do we want to make try_init_module_global() return failure after
tdx_sys_disable()? I guess, TDH_SYS_LP_INIT will fail anyway, so it
shouldn't matter.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v5 12/22] x86/virt/tdx: Reset software states during TDX module shutdown
From: Chao Gao @ 2026-03-17 8:27 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, kas@kernel.org, seanjc@google.com,
Chatre, Reinette, Weiny, Ira, binbin.wu@linux.intel.com,
Verma, Vishal L, nik.borisov@suse.com, mingo@redhat.com,
pbonzini@redhat.com, tony.lindgren@linux.intel.com,
sagis@google.com, Annapurve, Vishal, hpa@zytor.com,
Edgecombe, Rick P, paulmck@kernel.org, tglx@kernel.org,
yilun.xu@linux.intel.com, Williams, Dan J, bp@alien8.de
In-Reply-To: <996e42a31101bb90575e7b2b6f33b3bd607b5e6e.camel@intel.com>
On Mon, Mar 16, 2026 at 05:06:49PM +0800, Huang, Kai wrote:
>
>> @@ -1179,6 +1179,7 @@ EXPORT_SYMBOL_FOR_KVM(tdx_enable);
>> int tdx_module_shutdown(void)
>> {
>> struct tdx_module_args args = {};
>> + int ret, cpu;
>>
>> /*
>> * Shut down the TDX module and prepare handoff data for the next
>> @@ -1188,7 +1189,22 @@ int tdx_module_shutdown(void)
>> * modules as new modules likely have higher handoff version.
>> */
>> args.rcx = tdx_sysinfo.handoff.module_hv;
>> - return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
>> + ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
>> + if (ret)
>> + return ret;
>> +
>> + tdx_module_status = TDX_MODULE_UNINITIALIZED;
>> + sysinit_done = false;
>> + sysinit_ret = 0;
>> +
>> + /*
>> + * By reaching here CPUHP is disabled and all present CPUs
>> + * are online. It's safe to just loop all online CPUs and
>> + * reset the per-cpu flag.
>> + */
>> + for_each_online_cpu(cpu)
>> + per_cpu(tdx_lp_initialized, cpu) = false;
>
>Since you have removed the requirement that P-SEAMLDR.INSTALL must be done
>on all CPUs, and removed the relevant patch, the "all present CPUs are
>online" part isn't correct anymore.
>
>And using for_each_online_cpu() isn't enough since this doesn't reset the
>tdx_lp_initialized for offline CPUs.
>
>One way is to just use for_each_possible_cpu() here so tdx_lp_initialized
>for all CPUs are reset. Since the "CPUHP is disabled" part is still correct
>AFAICT (since stop_machine() disables CPUHP internally during the
>operation), resetting tdx_lp_initialized for offline CPUs won't race with
>CPUHP.
>
>And assuming this series will be applied after Sean's VMXON series, we will
Yes.
>have a TDX-specific CPUHP callback tdx_online_cpu() in TDX x86 core to do
>tdx_cpu_enable(), which will then enable TDX again on the new-online CPU.
Good point.
Clearing tdx_lp_initialized for offlined CPUs makes sense, but I'd rather not
justify this through "enabling TDX on new-online CPUs" since many details
remain unclear. For example, there will be a SEAMCALL to disable TDX per-CPU.
It should be called when CPUs go offline so that those CPUs can be exempting
from doing SEAMLDR.INSTALL during module updates. tdx_lp_initialized should
have been cleared along with that "disable TDX per-CPU" SEAMCALL for offlined
CPUs.
I'm not arguing against for_each_present_cpu(). I just think discussing how
to support TDX module update with offlined CPUs in the comment would be a bit
premature. How about keeping it simple:
/*
* Since the TDX module is shut down and gone, mark all CPUs (including
* offlined ones) as uninitialied. This is called in stop_machine() (where
* CPU hotplug is disabled), preventing races with other tdx_lp_initialized
* accesses.
*/
>
>Btw, w/o Sean's VMXON series, currently only KVM provides the TDX-specific
>CPUHP callback. So it seems if module update is done when KVM is not
>loaded,
w/o VMXON series, module update isn't possible if KVM isn't loaded.
>there will be no TDX-specific CPUHP callback to re-enable TDX for
>the new-online CPU. This means any SEAMCALL on that CPU will fail before
>KVM module is loaded again (which will then re-register the TDX-specific
>CPUHP and run tdx_cpu_enable() for all online CPUs).
>
>But I don't think we should consider this case.
>
>
Agreed.
^ permalink raw reply
* Re: [PATCH v4 1/2] dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
From: Jiri Pirko @ 2026-03-17 8:26 UTC (permalink / raw)
To: Robin Murphy
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, jstultz, tjmercier,
christian.koenig, m.szyprowski, jgg, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <15fcfa5e-c4b9-4b56-8f84-20dd5f66d643@arm.com>
Mon, Mar 16, 2026 at 07:25:55PM +0100, robin.murphy@arm.com wrote:
>On 2026-03-16 12:58 pm, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Current CC designs don't place a vIOMMU in front of untrusted devices.
>> Instead, the DMA API forces all untrusted device DMA through swiotlb
>> bounce buffers (is_swiotlb_force_bounce()) which copies data into
>> decrypted memory on behalf of the device.
>>
>> When a caller has already arranged for the memory to be decrypted
>> via set_memory_decrypted(), the DMA API needs to know so it can map
>> directly using the unencrypted physical address rather than bounce
>> buffering. Following the pattern of DMA_ATTR_MMIO, add
>> DMA_ATTR_CC_DECRYPTED for this purpose. Like the MMIO case, only the
>> caller knows what kind of memory it has and must inform the DMA API
>> for it to work correctly.
>
>Echoing Jason's point, if the intent of this is to indicate shared memory,
>please call it DMA_ATTR_CC_SHARED. Yes, some of the existing APIs are badly
>named because they conflated intent with implementation details; that is no
>reason to keep wilfully making the same mistake.
>
>At least with Arm CCA, the architecture enforces *confidentiality* pretty
>much orthogonally to encryption - if your threat model excludes physical
>attacks against DRAM, you can still have Realms isolated from each other (and
>of course other execution states) without even implementing the memory
>encryption feature; conversely if you do have it, then even all the
>shared/host memory may still be physically encrypted, it just has its own
>context (key) distinct from the Realm ones. Similarly, while it's not a
>"true" CoCo environment, pKVM has a similar notion of shared vs. private
>which can benefit from piggy-backing off much of the CoCo infrastructure in
>places like the DMA layer, but has nothing whatsoever to do with actual
>encryption.
>
>Furthermore, "shared" is just shorter and more readable, even before I invoke
>the previous discussion of why it should be "unencrypted" rather than
>"decrypted" anyway ;)
Okay, fair points. I'll rename it to shared for "v5". Thanks!
>
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>> v3->v4:
>> - added some sanity checks to dma_map_phys and dma_unmap_phys
>> - enhanced documentation of DMA_ATTR_CC_DECRYPTED attr
>> v1->v2:
>> - rebased on top of recent dma-mapping-fixes
>> ---
>> include/linux/dma-mapping.h | 10 ++++++++++
>> include/trace/events/dma.h | 3 ++-
>> kernel/dma/direct.h | 14 +++++++++++---
>> kernel/dma/mapping.c | 13 +++++++++++--
>> 4 files changed, 34 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>> index 29973baa0581..476964d2b22f 100644
>> --- a/include/linux/dma-mapping.h
>> +++ b/include/linux/dma-mapping.h
>> @@ -85,6 +85,16 @@
>> * a cacheline must have this attribute for this to be considered safe.
>> */
>> #define DMA_ATTR_CPU_CACHE_CLEAN (1UL << 11)
>> +/*
>> + * DMA_ATTR_CC_DECRYPTED: Indicates the DMA mapping is decrypted (shared) for
>> + * confidential computing guests. For normal system memory the caller must have
>> + * called set_memory_decrypted(), and pgprot_decrypted must be used when
>> + * creating CPU PTEs for the mapping. The same decrypted semantic may be passed
>> + * to the vIOMMU when it sets up the IOPTE. For MMIO use together with
>
>That being "the vIOMMU" that you said doesn't exist, and which is explicitly
>not supported?...
Yeah, I wanted to draw the full picture. I can put a not like "(when it
is going to be introduced)" or something like that to be clear.
>
>> + * DMA_ATTR_MMIO to indicate decrypted MMIO. Unless DMA_ATTR_MMIO is provided
>> + * a struct page is required.
>> + */
>> +#define DMA_ATTR_CC_DECRYPTED (1UL << 12)
>> /*
>> * A dma_addr_t can hold any valid DMA or bus address for the platform. It can
>> diff --git a/include/trace/events/dma.h b/include/trace/events/dma.h
>> index 33e99e792f1a..b8082d5177c4 100644
>> --- a/include/trace/events/dma.h
>> +++ b/include/trace/events/dma.h
>> @@ -32,7 +32,8 @@ TRACE_DEFINE_ENUM(DMA_NONE);
>> { DMA_ATTR_ALLOC_SINGLE_PAGES, "ALLOC_SINGLE_PAGES" }, \
>> { DMA_ATTR_NO_WARN, "NO_WARN" }, \
>> { DMA_ATTR_PRIVILEGED, "PRIVILEGED" }, \
>> - { DMA_ATTR_MMIO, "MMIO" })
>> + { DMA_ATTR_MMIO, "MMIO" }, \
>> + { DMA_ATTR_CC_DECRYPTED, "CC_DECRYPTED" })
>> DECLARE_EVENT_CLASS(dma_map,
>> TP_PROTO(struct device *dev, phys_addr_t phys_addr, dma_addr_t dma_addr,
>> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
>> index e89f175e9c2d..c047a9d0fda3 100644
>> --- a/kernel/dma/direct.h
>> +++ b/kernel/dma/direct.h
>> @@ -84,16 +84,24 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
>> dma_addr_t dma_addr;
>> if (is_swiotlb_force_bounce(dev)) {
>> - if (attrs & DMA_ATTR_MMIO)
>> - return DMA_MAPPING_ERROR;
>> + if (!(attrs & DMA_ATTR_CC_DECRYPTED)) {
>> + if (attrs & DMA_ATTR_MMIO)
>> + return DMA_MAPPING_ERROR;
>> - return swiotlb_map(dev, phys, size, dir, attrs);
>> + return swiotlb_map(dev, phys, size, dir, attrs);
>> + }
>> + } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
>> + return DMA_MAPPING_ERROR;
>> }
>> if (attrs & DMA_ATTR_MMIO) {
>> dma_addr = phys;
>> if (unlikely(!dma_capable(dev, dma_addr, size, false)))
>> goto err_overflow;
>> + } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
>> + dma_addr = phys_to_dma_unencrypted(dev, phys);
>> + if (unlikely(!dma_capable(dev, dma_addr, size, false)))
>> + goto err_overflow;
>> } else {
>> dma_addr = phys_to_dma(dev, phys);
>> if (unlikely(!dma_capable(dev, dma_addr, size, true)) ||
>> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
>> index 3928a509c44c..abb0c88b188b 100644
>> --- a/kernel/dma/mapping.c
>> +++ b/kernel/dma/mapping.c
>> @@ -157,6 +157,7 @@ dma_addr_t dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
>> {
>> const struct dma_map_ops *ops = get_dma_ops(dev);
>> bool is_mmio = attrs & DMA_ATTR_MMIO;
>> + bool is_cc_decrypted = attrs & DMA_ATTR_CC_DECRYPTED;
>> dma_addr_t addr = DMA_MAPPING_ERROR;
>> BUG_ON(!valid_dma_direction(dir));
>> @@ -165,8 +166,11 @@ dma_addr_t dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
>> return DMA_MAPPING_ERROR;
>> if (dma_map_direct(dev, ops) ||
>> - (!is_mmio && arch_dma_map_phys_direct(dev, phys + size)))
>> + (!is_mmio && !is_cc_decrypted &&
>> + arch_dma_map_phys_direct(dev, phys + size)))
>> addr = dma_direct_map_phys(dev, phys, size, dir, attrs);
>> + else if (is_cc_decrypted)
>> + return DMA_MAPPING_ERROR;
>> else if (use_dma_iommu(dev))
>
>...although, why *shouldn't* this be allowed with a vIOMMU? (Especially given
>that a vIOMMU for untrusted devices can be emulated by the host VMM without
>the CoCo hypervisor having to care at all - again, at least on Arm and other
>architectures where IOMMUs are regular driver model devices)
Well, when iommu path is able to consume the attr, this restriction
should be lifted. This is basically a sanity check for the
dma_map_phys() caller.
>
>> addr = iommu_dma_map_phys(dev, phys, size, dir, attrs);
>> else if (ops->map_phys)
>
>Or indeed any other non-direct ops? Obviously all the legacy architectures
>like Alpha are never going to see this or care, but I could imagine Xen and
>possibly PowerPC might.
Same here.
>
>Thanks,
>Robin.
>
>> @@ -203,11 +207,16 @@ void dma_unmap_phys(struct device *dev, dma_addr_t addr, size_t size,
>> {
>> const struct dma_map_ops *ops = get_dma_ops(dev);
>> bool is_mmio = attrs & DMA_ATTR_MMIO;
>> + bool is_cc_decrypted = attrs & DMA_ATTR_CC_DECRYPTED;
>> BUG_ON(!valid_dma_direction(dir));
>> +
>> if (dma_map_direct(dev, ops) ||
>> - (!is_mmio && arch_dma_unmap_phys_direct(dev, addr + size)))
>> + (!is_mmio && !is_cc_decrypted &&
>> + arch_dma_unmap_phys_direct(dev, addr + size)))
>> dma_direct_unmap_phys(dev, addr, size, dir, attrs);
>> + else if (is_cc_decrypted)
>> + return;
>> else if (use_dma_iommu(dev))
>> iommu_dma_unmap_phys(dev, addr, size, dir, attrs);
>> else if (ops->unmap_phys)
>
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Xu Yilun @ 2026-03-17 5:13 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Aneesh Kumar K.V, Dan Williams, linux-coco, linux-pci, gregkh,
aik, bhelgaas, alistair23, lukas, Arnd Bergmann
In-Reply-To: <20260313133658.GD1586734@nvidia.com>
On Fri, Mar 13, 2026 at 10:36:58AM -0300, Jason Gunthorpe wrote:
> On Fri, Mar 13, 2026 at 06:23:51PM +0800, Xu Yilun wrote:
>
> > My understanding is, it is the obfuscated host start pfn of this range,
> > if this range has offset to the BAR start, this field should also be
> > offsetted.
>
> The OS must get an idea of the bar layout out of the report, so there
> have to be restrictions on how it is formed otherwise it is
> unparsible. IMHO the PCI spec created this very general mechanism but
Yeah, now I think I've fully understood the problem. The essential thing
is some of the MMIO ranges could be hidden from the report, at least the
MSIX/PBA ranges must be hidden if not locked/private. This makes guest
hard to find the bar layout out of the report.
> the CPU CC specs need to constrain it to be usable by an OS.
>
> > > range_off = tsm_offset & (pci_resource_len(pdev, bar) - 1);
> > >
> > > So that we correctly handle if the interface report is reporting a range
> > > within a bar. The only requirement here is bar address should be aligned
> > > to its size and mmio_reporting_offset should not add offsets in that range.
This is the solution on CPU CC side, which requires the TSM to choose a
mmio_reporting_offset aligned to the MAX BAR size so that the
obfuscation won't blur the bar offset info. This is a trade off, the
larger the bar_size, the weaker the obfuscation. Is there a worst case,
that a device has a large bar in high address and a small bar in low
address, the small bar address has no intersection with the
mmio_reporting_offset. Then the obfuscated address ends up being amusing
0xabcd012345000, which clearly leaks the mmio_reporting_offset and the
physical BAR address...
And I've no idea why unlocked MSIX/PBA must be hidden? How about other
non-TEE ranges, must be hidden or mustn't? Is there a possibility we
enforce DSM to present all ranges, then the layout is clear to OS?
>
> Right.
>
> Jason
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Dan Williams @ 2026-03-17 1:45 UTC (permalink / raw)
To: Jakub Kicinski, Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, jgg, Donald Hunter
In-Reply-To: <20260314111245.76d18d73@kernel.org>
Jakub Kicinski wrote:
> On Mon, 2 Mar 2026 16:01:56 -0800 Dan Williams wrote:
> > The implementation adheres to the guideline from:
> > Documentation/userspace-api/netlink/genetlink-legacy.rst
> >
> > New Netlink families should never respond to a DO operation with
> > multiple replies, with ``NLM_F_MULTI`` set. Use a filtered dump
> > instead.
>
> My understanding of F_MULTI is that deserializer is supposed to
> continue deserializing into current object. IOW if we have:
>
> struct does_this {
> int really;
> int have_to;
> int be_netlink;
> };
Heh, sensing a subtle message here...
> You can send "really" and "be_netlink" in one message and "have_to"
> in the next, and receiver should reconstruct them into a single struct.
>
> If F_MULTI is not set - receiver assumes that the next message is a new
> struct. And the whole dump returns a list of structs.
>
> So IOW I think what you're doing is a bit too.. inventive.
Fair, but see below, satisfying the requirements here are stuck in the
liminal space between sysfs and netlink...
> Do you have plans to add more commands?
Yes, future work like teaching the kernel how to cache device evidence
and re-challenge a device after error or power-loss recovery [1]. It may
even supplant some sysfs interfaces that would be better with
transactional semantics.
For example, a LOCK operation that returns a session cookie and a
RUN/ACCEPT operation that only succeeds if the session has not been
invalidated in the interim. sysfs would require userspace locking for
such a semantic.
[1]: http://lore.kernel.org/69a9de4791667_6423c1006c@dwillia2-mobl4.notmuch
> The read-only stuff feels like it could be a sysfs API?
In fact, the original genesis of a proposal in this space was sysfs back
at Plumbers 2024 [2].
As the number of attributes, modifiers, and transactions grew the
feedback in the BoF was to move to a more suitable uAPI, netlink.
Yes, a subset of the objects here could move to sysfs [3], but that does
relieve the main need here which is an interface that can dump a fresh
copy of the device measurements (settings and device data up to 16MB in
size), signed by the device, with a nonce provided by relying party
(userspace).
[2]: https://lpc.events/event/18/contributions/1955/
[3]: http://lore.kernel.org/20260219124119.GD723117@nvidia.com
> The main strength of Netlink is "do" commands with multiple optional
> attrs.
Yes, that is attractive and saves a pile of bug prone ioctl handling.
The gap I need to fill first though is a uAPI that allows for large
blobs to be fetched after being regenerated / reformatted besed on some
input attributes.
"Multi message netlink attributes" while inventive, feels less awkward
and more future proof than a sysfs binary attribute scheme to do the
same.
^ permalink raw reply
* Re: [PATCH v5 05/22] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Chao Gao @ 2026-03-17 1:05 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, Farrah Chen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin
In-Reply-To: <abgBFmEhlsdmxa1k@thinkstation>
>> +
>> +int seamldr_get_info(struct seamldr_info *seamldr_info)
>> +{
>> + /*
>> + * Use slow_virt_to_phys() since @seamldr_info may be allocated on
>> + * the stack.
>> + */
>> + struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
>> +
>> + return seamldr_call(P_SEAMLDR_INFO, &args);
>
>On what condition this information can change?
After each update, the num_remaining_updates field will change.
>
>I see the next patch calls this on every _show operation. Would we
>benefit from caching the response?
Yes, caching would save a few cycles, but adds complexity. we'd need to keep
the cache synchronized and invalidate it when values change, likely requiring
additional state tracking.
Dave recommended allocating the structure on the stack [*] when I raised the
caching idea.
This isn't a hot path, so I prefer keeping it "simple and stupid".
[*]: https://lore.kernel.org/all/56a62e54-59f7-4423-9f01-4472d5c3815d@intel.com/
>
>> +}
>> +EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>> --
>> 2.47.3
>>
>
>--
> Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v5 04/22] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-03-17 0:43 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, Farrah Chen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin
In-Reply-To: <abf-3xRg7ouZw-F4@thinkstation>
>> +/*
>> + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
>> + * interact with P-SEAMLDR simultaneously. Use raw version as the calls can
>> + * be made with interrupts disabled.
>
>Hm. I am not sure how it explains use of raw_spinlock. What's wrong with
>using plain spinlock with interrupts disabled?
We mustn't acquire sleeping lock with interrupts disabled. in RT kernel, a plain
spinlock becomes a sleeping lock.
The lock doc [*] states:
On PREEMPT_RT kernels, these lock types are converted to sleeping locks:
local_lock
spinlock_t
rwlock_t
*:https://docs.kernel.org/locking/locktypes.html
and also see the test I did:
https://lore.kernel.org/kvm/abPEit+Ckeqmypv+@intel.com/
How about:
/*
* Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
* interact with P-SEAMLDR simultaneously. Use raw version as the calls can
* be made with interrupts disabled, where plain spinlocks are prohibited in
* PREEMPT_RT kernels as they become sleeping locks.
*/
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Dan Williams @ 2026-03-16 23:02 UTC (permalink / raw)
To: Dan Williams, Lukas Wunner, Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, jgg, Donald Hunter, Jakub Kicinski
In-Reply-To: <69b8646c8aeb1_452b100aa@dwillia2-mobl4.notmuch>
Dan Williams wrote:
> Lukas Wunner wrote:
> > On Mon, Mar 02, 2026 at 04:01:56PM -0800, Dan Williams wrote:
> > > +definitions:
> > > + -
> > > + type: const
> > > + name: max-object-size
> > > + value: 0x01000000
> > [...]
> > > + -
> > > + name: val
> > > + type: binary
> > > + checks:
> > > + max-len: max-obj-size
> >
> > The length of a netlink attribute is a 16-bit value, so a 16 MByte value
> > (0x01000000) won't fit.
> >
> > Moreover you're referencing max-obj-size but are defining max-object-size.
>
> Good catch, not sure why the tooling did not complain.
This ends up not being a problem because 'val' is not referenced by a
request, only a reply. The automatic code generation does not generate
an nla_policy for replies. It should just be deleted because, see
below...
> > This doesn't look like it's ever been tested, so at the very least
> > it should be marked RFC in the subject to convey that it's not yet
> > in a cut-and-dried state.
>
> The 16MB limit has indeed not been tested, the test script in this set
> was using smaller than 64K payloads to check out the interface.
So 16MB works ok, slow, but works. A given attribute in this
implementation never exceeds the limit because the protocol is for
assembly of a bulk attribute over multiple messages.
> The RFC comment for this piece is fair. The whole became a v2 based on
> the maturity of other proposals that were in v1. This "evidence"
> proposal deserves its own conversation.
>
> > The two top-most commits on my development branch have solved the
> > size problem and may serve as a template:
> >
> > https://github.com/l1k/linux/commits/doe
>
> I was concerned that gets the same "too inventive" feedback from netdev
> folks, but I ended up triggering the same with a broken alternate
> proposal.
I think your patches address the performance problem if userspace passes
large recieve buffers, but I do not think we need to make that a
userspace requirement.
^ permalink raw reply
* Re: [PATCH 3/4] x86/virt/tdx: Add SEAMCALL wrapper for TDH.SYS.DISABLE
From: Edgecombe, Rick P @ 2026-03-16 21:15 UTC (permalink / raw)
To: kas@kernel.org
Cc: tglx@kernel.org, Hansen, Dave, seanjc@google.com, bp@alien8.de,
x86@kernel.org, ackerleytng@google.com, hpa@zytor.com,
linux-kernel@vger.kernel.org, mingo@redhat.com, Huang, Kai,
kvm@vger.kernel.org, linux-coco@lists.linux.dev,
pbonzini@redhat.com, Verma, Vishal L, Gao, Chao
In-Reply-To: <abftAJkiU3Q24wYQ@thinkstation>
On Mon, 2026-03-16 at 11:51 +0000, Kiryl Shutsemau wrote:
> On Fri, Mar 06, 2026 at 05:03:57PM -0800, Rick Edgecombe wrote:
> > From: Vishal Verma <vishal.l.verma@intel.com>
> >
> > Some early TDX-capable platforms have an erratum where a partial write
> > to TDX private memory can cause a machine check on a subsequent read.
> > On these platforms, kexec and kdump have been disabled in these cases,
> > because the old kernel cannot safely hand off TDX state to the new
> > kernel. Later TDX modules support the TDH.SYS.DISABLE SEAMCALL, which
> > provides a way to cleanly disable TDX and allow kexec to proceed.
>
> Does it need to be enumerated?
>
> I don't see this SEAMCALL be covered in the public documentation.
> </me looking around>
> Ah! Found it the the draft. So the feature is not yet finalized.
>
> "Support of TDH.SYS.DISABLE is enumerated by TDX_FEATURES0. SYS_DISABLE
> (bit 53)"
>
> I am seeing the next patch calling it unconditionally. Is it okay?
We debated checking the feature bit before allowing kexec, but decided it was
simpler to just blindly call and ignore the errors. The reasoning was that this
is already a somewhat exotic scenario being addressed, and future modules will
have the feature. So maintaining a check for the feature bit only helps a little
bit, for a short time. And then only if the user would rather have kexec blocked
than attempt it. Do you think it is worth it?
>
> > This can be a long running operation, and the time needed largely
> > depends on the amount of memory that has been allocated to TDs. If all
> > TDs have been destroyed prior to the sys_disable call, then it is fast,
> > with only needing to override the TDX module memory.
> >
> > After the SEAMCALL completes, the TDX module is disabled and all memory
> > resources allocated to TDX are freed and reset. The next kernel can then
> > re-initialize the TDX module from scratch via the normal TDX bring-up
> > sequence.
> >
> > The SEAMCALL may be interrupted by an interrupt. In this case, it
> > returns TDX_INTERRUPTED_RESUMABLE, and it must be retried in a loop
> > until the operation completes successfully.
> >
> > Add a tdx_sys_disable() helper, which implements the retry loop around
> > the SEAMCALL to provide this functionality.
> >
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > ---
> > arch/x86/include/asm/tdx.h | 3 +++
> > arch/x86/virt/vmx/tdx/tdx.c | 18 ++++++++++++++++++
> > arch/x86/virt/vmx/tdx/tdx.h | 1 +
> > 3 files changed, 22 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> > index f0826b0a512a..baaf43a09e99 100644
> > --- a/arch/x86/include/asm/tdx.h
> > +++ b/arch/x86/include/asm/tdx.h
> > @@ -173,6 +173,8 @@ static inline int pg_level_to_tdx_sept_level(enum pg_level level)
> > return level - 1;
> > }
> >
> > +void tdx_sys_disable(void);
> > +
> > u64 tdh_vp_enter(struct tdx_vp *vp, struct tdx_module_args *args);
> > u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
> > u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2);
> > @@ -204,6 +206,7 @@ static inline void tdx_init(void) { }
> > 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 void tdx_sys_disable(void) { }
> > #endif /* CONFIG_INTEL_TDX_HOST */
> >
> > #endif /* !__ASSEMBLER__ */
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index 0802d0fd18a4..68bd2618dde4 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -37,6 +37,7 @@
> > #include <asm/msr.h>
> > #include <asm/cpufeature.h>
> > #include <asm/tdx.h>
> > +#include <asm/shared/tdx_errno.h>
> > #include <asm/cpu_device_id.h>
> > #include <asm/processor.h>
> > #include <asm/mce.h>
> > @@ -1940,3 +1941,20 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page)
> > return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
> > }
> > EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_hkid);
> > +
> > +void tdx_sys_disable(void)
> > +{
> > + struct tdx_module_args args = {};
> > +
> > + /*
> > + * SEAMCALLs that can return TDX_INTERRUPTED_RESUMABLE are guaranteed
> > + * to make forward progress between interrupts, so it is safe to loop
> > + * unconditionally here.
> > + *
> > + * This is a 'destructive' SEAMCALL, in that no other SEAMCALL can be
> > + * run after this until a full reinitialization is done.
> > + */
> > + while (seamcall(TDH_SYS_DISABLE, &args) == TDX_INTERRUPTED_RESUMABLE)
> > + ;
>
> Silently ignore any other errors?
Do you think it's worth a warn? There are a couple other considerations.
- Kai brought up offline that we should handle TDX_SYS_BUSY here too.
- Previous kexec patches had trouble solving races around tdx enabling. So we
have to handle the seamcall failures.
So we have to exclude a few different errors in different ways. And then the
warn worthy error codes either don't impact anything, or the new kernel will
fail to initialize the TDX module and give notice there.
I don't have a strong objection. It seems to be a judgment call of whether the
complexity is worth the benefit.
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Dan Williams @ 2026-03-16 20:13 UTC (permalink / raw)
To: Lukas Wunner, Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, jgg, Donald Hunter, Jakub Kicinski
In-Reply-To: <abWq1oiE-YkksxNu@wunner.de>
Lukas Wunner wrote:
> On Mon, Mar 02, 2026 at 04:01:56PM -0800, Dan Williams wrote:
> > +definitions:
> > + -
> > + type: const
> > + name: max-object-size
> > + value: 0x01000000
> [...]
> > + -
> > + name: val
> > + type: binary
> > + checks:
> > + max-len: max-obj-size
>
> The length of a netlink attribute is a 16-bit value, so a 16 MByte value
> (0x01000000) won't fit.
>
> Moreover you're referencing max-obj-size but are defining max-object-size.
Good catch, not sure why the tooling did not complain.
> This doesn't look like it's ever been tested, so at the very least
> it should be marked RFC in the subject to convey that it's not yet
> in a cut-and-dried state.
The 16MB limit has indeed not been tested, the test script in this set
was using smaller than 64K payloads to check out the interface.
The RFC comment for this piece is fair. The whole became a v2 based on
the maturity of other proposals that were in v1. This "evidence"
proposal deserves its own conversation.
> The two top-most commits on my development branch have solved the
> size problem and may serve as a template:
>
> https://github.com/l1k/linux/commits/doe
I was concerned that gets the same "too inventive" feedback from netdev
folks, but I ended up triggering the same with a broken alternate
proposal.
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Kalra, Ashish @ 2026-03-16 19:03 UTC (permalink / raw)
To: Dave Hansen, Sean Christopherson
Cc: tglx, mingo, bp, dave.hansen, x86, hpa, peterz, thomas.lendacky,
herbert, davem, ardb, pbonzini, aik, Michael.Roth, KPrateek.Nayak,
Tycho.Andersen, Nathan.Fontenot, jackyli, pgonda, rientjes,
jacobhxu, xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj,
john.allen, darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <5102edd8-8eaa-4688-b3f7-3004c4cbc8f3@intel.com>
Hello Dave,
On 3/11/2026 5:20 PM, Dave Hansen wrote:
> On 3/11/26 14:24, Kalra, Ashish wrote:
> ...
>> There are 2 active SNP VMs here, with one SNP VM being terminated, the other SNP VM is still running, both VMs are configured with 100GB guest RAM:
>>
>> When this loop is executed when the SNP guest terminates:
>>
>> [ 232.789187] SEV-SNP: RMPOPT execution time 391609638 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~391 ms
>>
>> [ 234.647462] SEV-SNP: RMPOPT execution time 457933019 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~457 ms
>
> That's better, but it's not quite what am looking for.
>
> The most important case (IMNHO) is when RMPOPT falls flat on its face:
> it tries to optimize the full 2TB of memory and manages to optimize nothing.
>
> I doubt that two 100GB VMs will get close to that case. It's
> theoretically possible, but unlikely.
>
> You also didn't mention 4k vs. 2M vs. 1G mappings.
>
>> Now, there are a couple of additional RMPOPT optimizations which can be applied to this loop :
>>
>> 1). RMPOPT can skip the bulk of its work if another CPU has already optimized that region.
>> The optimal thing may be to optimize all memory on one CPU first, and then let all the others
>> run RMPOPT in parallel.
>
> Ahh, so the RMP table itself caches the result of the RMPOPT in its 1G
> metadata, then the CPUs can just copy it into their core-local
> optimization table at RMPOPT time?
>
> That's handy.
>
> *But*, for the purposes of finding pathological behavior, it's actually
> contrary to what I think I was asking for which was having all 1G pages
> filled with some private memory. If the system was in the state I want
> to see tested, that optimization won't function.
True that in this case RMPOPT will not do any optimizations and the system performance will be worst, but actually
if you see in this case, for this loop which we are considering, the loop will actually have the smallest runtime.
More on this below.
>
>> [ 363.926595] SEV-SNP: RMPOPT execution time 317016656 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~317 ms
>>
>> [ 365.415243] SEV-SNP: RMPOPT execution time 369659769 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~369 ms.
>>
>> So, with these two optimizations applied, there is like a ~16-20% performance improvement (when SNP guest terminates) in the execution of this loop
>> which is executing RMPOPT on upto 2TB of RAM on all CPUs.
>>
>> Any thoughts, feedback on the performance numbers ?
>
> 16-20% isn't horrible, but it isn't really a fundamental change.
>
> It would also be nice to see elapsed time for each CPU. Having one
> pegged CPU for 400ms and 99 mostly idle ones is way different than
> having 100 pegged CPUs for 400ms.
>
> That's why I was interested in "how long it takes per-cpu".
>
> But you could get some pretty good info with your new optimized loop:
>
> start = ktime_get();
>
> for (pa = pa_start; pa < pa_end; pa += PUD_SIZE)
> rmpopt() // current CPU
>
> middle = ktime_get();
>
> for (pa = pa_start; pa < pa_end; pa += PUD_SIZE)
> on_each_cpu_mask(...) // remote CPUs
>
> end = ktime_get();
>
> If you do that ^ with a system:
>
> 1. full of private memory
Again, for this case RMPOPT fails to do any optimizations, but for this loop which we are considering, this case will have the smallest runtime.
> 2. empty of private memory
> 3. empty again
In both these cases, RMPOPT does the best optimizations for system performance, but for the loop which we are considering, these cases will have
the longest runtime, as in this case RMPOPT has to check *all* the RMP entries in each 1GB region (and for every 1G region it is executed for) and
so each RMPOPT instruction and this loop itself will take the maximum time.
Here are the actual numbers:
These measurements are done with the *new* optimized loop:
...
/* Only one thread per core needs to issue RMPOPT instruction */
for_each_online_cpu(cpu) {
if (!topology_is_primary_thread(cpu))
continue;
cpumask_set_cpu(cpu, cpus);
}
...
start = ktime_get();
/*
* RMPOPT is optimized to skip the bulk of its work if another CPU has already
* optimized that region. Optimize all memory on one CPU first, and then let all
* the others run RMPOPT in parallel.
*/
cpumask_clear_cpu(smp_processor_id(), cpus);
/* current CPU */
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE)
rmpopt((void *)(pa | RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS));
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE) {
/* Bit zero passes the function to the RMPOPT instruction. */
on_each_cpu_mask(cpus, rmpopt,
(void *)(pa | RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS),
true);
}
end = ktime_get();
elapsed_ns = ktime_to_ns(ktime_sub(end, start));
pr_info("RMPOPT execution time %llu ns for physical address range 0x%016llx - 0x%016llx on all cpus\n",
elapsed_ns, pa_start, pa_end);
...
Case 2 and 3:
When the following loop is executed, after SNP is enabled at snp_rmptable_init(), the RMP table does not have any assigned pages, which is
essentially case 2.
So the loop has the worst runtime, as can be seen below:
[ 12.961935] SEV-SNP: RMP optimizations enabled on physical address range @1GB alignment [0x0000000000000000 - 0x0000020000000000]
[ 13.286659] SEV-SNP: RMPOPT execution time 311135734 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~311 ms.
At this point, i simulate the case you are looking for, where the RAM is full of private memory/assigned pages, essentially case 1.
In other words, i simulated a case, where the first 4K page at every 1GB boundary is an assigned page.
This means that RMPOPT will exit immediately and early as it finds an assigned page on the first page it checks in every 1GB range, as below:
...
for (pfn = 0; pfn < max_pfn; pfn += (1 << (PUD_SHIFT - PAGE_SHIFT)))
rmp_make_private(pfn, 0, PG_LEVEL_4K, 0, true);
...
And so RMPOPT instruction itself and executing this loop after programming the RMP table as above has the smallest runtime:
[ 13.430801] SEV-SNP: RMP optimizations enabled on physical address range @1GB alignment [0x0000000000000000 - 0x0000020000000000]
[ 13.539667] SEV-SNP: RMPOPT execution time 95275588 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~95 ms.
To summarize, these two are the worst and best performance numbers for this loop which we are considering.
Best runtime for the loop:
When RMPOPT exits early as it finds an assigned page on the first RMP entry it checks in the 1GB -> ~95ms.
Worst runtime for the loop:
When RMPOPT does not find any assigned page in the full 1GB range it is checking -> ~311ms.
So looking at this range [95ms - 311ms], we need to decide if we want to use the kthread approach ?
>
> You'll hopefully see:
>
> 1. RMPOPT fall on its face. Worst case scenario (what I want to
> see most)
> 2. RMPOPT sees great success, but has to scan the RMP at least
> once. Remote CPUs get a free ride on the first CPU's scan.
> Largest (middle-start) vs. (end-middle)/nr_cpus delta.
> 3. RMPOPT best case. Everything is already optimized.
>
>> Ideally we should be issuing RMPOPTs to only optimize the 1G regions that contained memory associated with that guest and that should be
>> significantly less than the whole 2TB RAM range.
>>
>> But that is something we planned for 1GB hugetlb guest_memfd support getting merged and which i believe has dependency on:
>> 1). in-place conversion for guest_memfd,
>> 2). 2M hugepage support for guest_memfd and finally
>> 3). 1GB hugeTLB support for guest_memfd.
>
> It's a no-brainer to do RMPOPT when you have 1GB pages around. You'll
> see zero argument from me.
>
Yes.
> Doing things per-guest and for smaller pages gets a little bit harder to
> reason about. In the end, this is all about trying to optimize against
> the RMP table which is a global resource. It's going to get wonky if
> RMPOPT is driven purely by guest-local data. There are lots of potential
> pitfalls.
>
> For now, let's just do it as simply as possible. Get maximum bang for
> our buck with minimal data structures and see how that works out. It
> might end up being a:
>
> queue_delayed_work()
>
> to do some cleanup a few seconds out after each SNP guest terminates. If
> a bunch of guests terminate all at once it'll at least only do a single
> set of IPIs.
Again, looking at the numbers above, what are your suggestions for
1). using the kthread approach OR
2). probably scheduling it for later execution after SNP guest termination via a workqueue OR
3). use some additional data structure like a bitmap to track 1G pages in guest_memfd
to do the RMP re-optimizations.
Thanks,
Ashish
^ permalink raw reply
* Re: [PATCH v4 1/2] dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
From: Robin Murphy @ 2026-03-16 18:25 UTC (permalink / raw)
To: Jiri Pirko, dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, jgg, leon,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260316125857.617836-2-jiri@resnulli.us>
On 2026-03-16 12:58 pm, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Current CC designs don't place a vIOMMU in front of untrusted devices.
> Instead, the DMA API forces all untrusted device DMA through swiotlb
> bounce buffers (is_swiotlb_force_bounce()) which copies data into
> decrypted memory on behalf of the device.
>
> When a caller has already arranged for the memory to be decrypted
> via set_memory_decrypted(), the DMA API needs to know so it can map
> directly using the unencrypted physical address rather than bounce
> buffering. Following the pattern of DMA_ATTR_MMIO, add
> DMA_ATTR_CC_DECRYPTED for this purpose. Like the MMIO case, only the
> caller knows what kind of memory it has and must inform the DMA API
> for it to work correctly.
Echoing Jason's point, if the intent of this is to indicate shared
memory, please call it DMA_ATTR_CC_SHARED. Yes, some of the existing
APIs are badly named because they conflated intent with implementation
details; that is no reason to keep wilfully making the same mistake.
At least with Arm CCA, the architecture enforces *confidentiality*
pretty much orthogonally to encryption - if your threat model excludes
physical attacks against DRAM, you can still have Realms isolated from
each other (and of course other execution states) without even
implementing the memory encryption feature; conversely if you do have
it, then even all the shared/host memory may still be physically
encrypted, it just has its own context (key) distinct from the Realm
ones. Similarly, while it's not a "true" CoCo environment, pKVM has a
similar notion of shared vs. private which can benefit from
piggy-backing off much of the CoCo infrastructure in places like the DMA
layer, but has nothing whatsoever to do with actual encryption.
Furthermore, "shared" is just shorter and more readable, even before I
invoke the previous discussion of why it should be "unencrypted" rather
than "decrypted" anyway ;)
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> v3->v4:
> - added some sanity checks to dma_map_phys and dma_unmap_phys
> - enhanced documentation of DMA_ATTR_CC_DECRYPTED attr
> v1->v2:
> - rebased on top of recent dma-mapping-fixes
> ---
> include/linux/dma-mapping.h | 10 ++++++++++
> include/trace/events/dma.h | 3 ++-
> kernel/dma/direct.h | 14 +++++++++++---
> kernel/dma/mapping.c | 13 +++++++++++--
> 4 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 29973baa0581..476964d2b22f 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -85,6 +85,16 @@
> * a cacheline must have this attribute for this to be considered safe.
> */
> #define DMA_ATTR_CPU_CACHE_CLEAN (1UL << 11)
> +/*
> + * DMA_ATTR_CC_DECRYPTED: Indicates the DMA mapping is decrypted (shared) for
> + * confidential computing guests. For normal system memory the caller must have
> + * called set_memory_decrypted(), and pgprot_decrypted must be used when
> + * creating CPU PTEs for the mapping. The same decrypted semantic may be passed
> + * to the vIOMMU when it sets up the IOPTE. For MMIO use together with
That being "the vIOMMU" that you said doesn't exist, and which is
explicitly not supported?...
> + * DMA_ATTR_MMIO to indicate decrypted MMIO. Unless DMA_ATTR_MMIO is provided
> + * a struct page is required.
> + */
> +#define DMA_ATTR_CC_DECRYPTED (1UL << 12)
>
> /*
> * A dma_addr_t can hold any valid DMA or bus address for the platform. It can
> diff --git a/include/trace/events/dma.h b/include/trace/events/dma.h
> index 33e99e792f1a..b8082d5177c4 100644
> --- a/include/trace/events/dma.h
> +++ b/include/trace/events/dma.h
> @@ -32,7 +32,8 @@ TRACE_DEFINE_ENUM(DMA_NONE);
> { DMA_ATTR_ALLOC_SINGLE_PAGES, "ALLOC_SINGLE_PAGES" }, \
> { DMA_ATTR_NO_WARN, "NO_WARN" }, \
> { DMA_ATTR_PRIVILEGED, "PRIVILEGED" }, \
> - { DMA_ATTR_MMIO, "MMIO" })
> + { DMA_ATTR_MMIO, "MMIO" }, \
> + { DMA_ATTR_CC_DECRYPTED, "CC_DECRYPTED" })
>
> DECLARE_EVENT_CLASS(dma_map,
> TP_PROTO(struct device *dev, phys_addr_t phys_addr, dma_addr_t dma_addr,
> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
> index e89f175e9c2d..c047a9d0fda3 100644
> --- a/kernel/dma/direct.h
> +++ b/kernel/dma/direct.h
> @@ -84,16 +84,24 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
> dma_addr_t dma_addr;
>
> if (is_swiotlb_force_bounce(dev)) {
> - if (attrs & DMA_ATTR_MMIO)
> - return DMA_MAPPING_ERROR;
> + if (!(attrs & DMA_ATTR_CC_DECRYPTED)) {
> + if (attrs & DMA_ATTR_MMIO)
> + return DMA_MAPPING_ERROR;
>
> - return swiotlb_map(dev, phys, size, dir, attrs);
> + return swiotlb_map(dev, phys, size, dir, attrs);
> + }
> + } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
> + return DMA_MAPPING_ERROR;
> }
>
> if (attrs & DMA_ATTR_MMIO) {
> dma_addr = phys;
> if (unlikely(!dma_capable(dev, dma_addr, size, false)))
> goto err_overflow;
> + } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
> + dma_addr = phys_to_dma_unencrypted(dev, phys);
> + if (unlikely(!dma_capable(dev, dma_addr, size, false)))
> + goto err_overflow;
> } else {
> dma_addr = phys_to_dma(dev, phys);
> if (unlikely(!dma_capable(dev, dma_addr, size, true)) ||
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 3928a509c44c..abb0c88b188b 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -157,6 +157,7 @@ dma_addr_t dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
> {
> const struct dma_map_ops *ops = get_dma_ops(dev);
> bool is_mmio = attrs & DMA_ATTR_MMIO;
> + bool is_cc_decrypted = attrs & DMA_ATTR_CC_DECRYPTED;
> dma_addr_t addr = DMA_MAPPING_ERROR;
>
> BUG_ON(!valid_dma_direction(dir));
> @@ -165,8 +166,11 @@ dma_addr_t dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
> return DMA_MAPPING_ERROR;
>
> if (dma_map_direct(dev, ops) ||
> - (!is_mmio && arch_dma_map_phys_direct(dev, phys + size)))
> + (!is_mmio && !is_cc_decrypted &&
> + arch_dma_map_phys_direct(dev, phys + size)))
> addr = dma_direct_map_phys(dev, phys, size, dir, attrs);
> + else if (is_cc_decrypted)
> + return DMA_MAPPING_ERROR;
> else if (use_dma_iommu(dev))
...although, why *shouldn't* this be allowed with a vIOMMU? (Especially
given that a vIOMMU for untrusted devices can be emulated by the host
VMM without the CoCo hypervisor having to care at all - again, at least
on Arm and other architectures where IOMMUs are regular driver model
devices)
> addr = iommu_dma_map_phys(dev, phys, size, dir, attrs);
> else if (ops->map_phys)
Or indeed any other non-direct ops? Obviously all the legacy
architectures like Alpha are never going to see this or care, but I
could imagine Xen and possibly PowerPC might.
Thanks,
Robin.
> @@ -203,11 +207,16 @@ void dma_unmap_phys(struct device *dev, dma_addr_t addr, size_t size,
> {
> const struct dma_map_ops *ops = get_dma_ops(dev);
> bool is_mmio = attrs & DMA_ATTR_MMIO;
> + bool is_cc_decrypted = attrs & DMA_ATTR_CC_DECRYPTED;
>
> BUG_ON(!valid_dma_direction(dir));
> +
> if (dma_map_direct(dev, ops) ||
> - (!is_mmio && arch_dma_unmap_phys_direct(dev, addr + size)))
> + (!is_mmio && !is_cc_decrypted &&
> + arch_dma_unmap_phys_direct(dev, addr + size)))
> dma_direct_unmap_phys(dev, addr, size, dir, attrs);
> + else if (is_cc_decrypted)
> + return;
> else if (use_dma_iommu(dev))
> iommu_dma_unmap_phys(dev, addr, size, dir, attrs);
> else if (ops->unmap_phys)
^ permalink raw reply
* Re: [PATCH v5 05/22] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Kiryl Shutsemau @ 2026-03-16 13:12 UTC (permalink / raw)
To: Chao Gao
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, Farrah Chen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin
In-Reply-To: <20260315135920.354657-6-chao.gao@intel.com>
On Sun, Mar 15, 2026 at 06:58:25AM -0700, Chao Gao wrote:
> P-SEAMLDR returns its information such as version number, in response to
> the SEAMLDR.INFO SEAMCALL.
>
> This information is useful for userspace. For example, the admin can decide
> which TDX module versions are compatible with the P-SEAMLDR according to
> the P-SEAMLDR version.
>
> Retrieve P-SEAMLDR information in preparation for exposing P-SEAMLDR
> version and other necessary information to userspace. Export the new kAPI
> for use by tdx-host.ko.
>
> Note that there are two distinct P-SEAMLDR APIs with similar names:
>
> SEAMLDR.INFO: Returns a SEAMLDR_INFO structure containing SEAMLDR
> information such as version and remaining updates.
>
> SEAMLDR.SEAMINFO: Returns a SEAMLDR_SEAMINFO structure containing SEAM
> and system information such as Convertible Memory
> Regions (CMRs) and number of CPUs and sockets.
>
> The former is used here.
>
> For details, see "Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
> Interface Specification" revision 343755-003.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> ---
> Kai also suggested merging this patch with the first use of the new
> kAPI, so we don't need to add a comment for slow_virt_to_phys() (as the
> reason can be seen from the call site). I am fine with it, but the
> changelog may be a bit lengthy.
>
> v5:
> - add a comment for slow_virt_to_phys() [Kai]
> v4:
> - put seamldr_info on stack [Dave]
> - improve changelogs to explain SEAMLDR.INFO and SEAMLDR.SEAMINFO [Dave]
> - add P-SEAMLDR spec information in the changelog [Dave]
> - add proper comments above ABI structure definition [Dave]
> - add unused ABI structure fields rather than marking them as reserved
> to better align with the specc [Dave] (I omitted "not used by kernel"
> tags since there are 5-6 such fields and maintaining these tags would
> be tedious.)
> ---
> arch/x86/include/asm/seamldr.h | 36 +++++++++++++++++++++++++++++++++
> arch/x86/virt/vmx/tdx/seamldr.c | 19 ++++++++++++++++-
> 2 files changed, 54 insertions(+), 1 deletion(-)
> create mode 100644 arch/x86/include/asm/seamldr.h
>
> diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
> new file mode 100644
> index 000000000000..c67e5bc910a9
> --- /dev/null
> +++ b/arch/x86/include/asm/seamldr.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_SEAMLDR_H
> +#define _ASM_X86_SEAMLDR_H
> +
> +#include <linux/types.h>
> +
> +/*
> + * This is called the "SEAMLDR_INFO" data structure and is defined
> + * in "SEAM Loader (SEAMLDR) Interface Specification".
> + *
> + * The SEAMLDR.INFO documentation requires this to be aligned to a
> + * 256-byte boundary.
> + */
> +struct seamldr_info {
> + u32 version;
> + u32 attributes;
> + u32 vendor_id;
> + u32 build_date;
> + u16 build_num;
> + u16 minor_version;
> + u16 major_version;
> + u16 update_version;
> + u32 acm_x2apicid;
> + u32 num_remaining_updates;
> + u8 seam_info[128];
> + u8 seam_ready;
> + u8 seam_debug;
> + u8 p_seam_ready;
> + u8 reserved[93];
> +} __packed __aligned(256);
> +
> +static_assert(sizeof(struct seamldr_info) == 256);
> +
> +int seamldr_get_info(struct seamldr_info *seamldr_info);
> +
> +#endif /* _ASM_X86_SEAMLDR_H */
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 7ed9be89017c..7c0cbab2c4c0 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -8,8 +8,13 @@
>
> #include <linux/spinlock.h>
>
> +#include <asm/seamldr.h>
> +
> #include "seamcall_internal.h"
>
> +/* P-SEAMLDR SEAMCALL leaf function */
> +#define P_SEAMLDR_INFO 0x8000000000000000
> +
> /*
> * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> * interact with P-SEAMLDR simultaneously. Use raw version as the calls can
> @@ -17,8 +22,20 @@
> */
> static DEFINE_RAW_SPINLOCK(seamldr_lock);
>
> -static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> +static int seamldr_call(u64 fn, struct tdx_module_args *args)
> {
> guard(raw_spinlock)(&seamldr_lock);
> return seamcall_prerr(fn, args);
> }
> +
> +int seamldr_get_info(struct seamldr_info *seamldr_info)
> +{
> + /*
> + * Use slow_virt_to_phys() since @seamldr_info may be allocated on
> + * the stack.
> + */
> + struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
> +
> + return seamldr_call(P_SEAMLDR_INFO, &args);
On what condition this information can change?
I see the next patch calls this on every _show operation. Would we
benefit from caching the response?
> +}
> +EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
> --
> 2.47.3
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox