* [PATCH v7 21/22] x86/virt/tdx: Document TDX module update
From: Chao Gao @ 2026-03-31 12:41 UTC (permalink / raw)
To: linux-kernel, linux-doc, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin,
Jonathan Corbet, Shuah Khan
In-Reply-To: <20260331124214.117808-1-chao.gao@intel.com>
Document TDX module update as a subsection of "TDX Host Kernel Support" to
provide background information and cover key points that developers and
users may need to know, for example:
- update is done in stop_machine() context
- update instructions and results
- update policy and tooling
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
---
v5:
- use "update" when refer to the update feature/concept [Kai]
---
Documentation/arch/x86/tdx.rst | 36 ++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
index 61670e7df2f7..fe9132d0f1fe 100644
--- a/Documentation/arch/x86/tdx.rst
+++ b/Documentation/arch/x86/tdx.rst
@@ -99,6 +99,42 @@ initialize::
[..] virt/tdx: module initialization failed ...
+TDX module Runtime Update
+-------------------------
+
+The TDX architecture includes a persistent SEAM loader (P-SEAMLDR) that
+runs in SEAM mode separately from the TDX module. The kernel can
+communicate with P-SEAMLDR to perform runtime updates of the TDX module.
+
+During update, the TDX module becomes unresponsive to other TDX operations.
+To prevent components using TDX (such as KVM) from experiencing unexpected
+errors during updates, updates are performed in stop_machine() context.
+
+TDX module update has complex compatibility requirements; the new module
+must be compatible with the current CPU, P-SEAMLDR, and running TDX module.
+Rather than implementing complex module selection and policy enforcement
+logic in the kernel, userspace is responsible for auditing and selecting
+appropriate updates.
+
+Updates use the standard firmware upload interface. See
+Documentation/driver-api/firmware/fw_upload.rst for detailed instructions.
+
+Successful updates are logged in dmesg:
+ [..] virt/tdx: version 1.5.20 -> 1.5.24
+
+If updates failed, running TDs may be killed and further TDX operations may
+not be possible until reboot. For detailed error information, see
+Documentation/ABI/testing/sysfs-devices-faux-tdx-host.
+
+Given the risk of losing existing TDs, userspace should verify that the
+update is compatible with the current system and properly validated before
+applying it.
+
+A reference userspace tool that implements necessary checks is available
+at:
+
+ https://github.com/intel/tdx-module-binaries
+
TDX Interaction to Other Kernel Components
------------------------------------------
--
2.47.3
^ permalink raw reply related
* [PATCH v7 22/22] x86/virt/seamldr: Log TDX module update failures
From: Chao Gao @ 2026-03-31 12:41 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260331124214.117808-1-chao.gao@intel.com>
Currently, there is no way to restore a TDX module from shutdown state to
running state. This means if errors occur after a successful module
shutdown, they are unrecoverable since the old module is gone but the new
module isn't installed. All subsequent SEAMCALLs to the TDX module will
fail, so TDs will be killed due to SEAMCALL failures.
Log a message to clarify that SEAMCALL errors are expected in this
scenario. This ensures that after update failures, the first message in
dmesg explains the situation rather than showing confusing call traces from
various code paths.
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>
Acked-by: Kai Huang <kai.huang@intel.com>
---
v4:
- Use pr_warn_once() instead of reinventing it [Yilun]
v3:
- Rephrase the changelog to eliminate the confusing uses of 'i.e.' and 'e.g.'
[Dave/Yilun]
---
arch/x86/virt/vmx/tdx/seamldr.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index b3f3b40627c3..a972f9ba6877 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -252,6 +252,11 @@ static void ack_state(void)
set_target_state(update_data.state + 1);
}
+static void print_update_failure_message(void)
+{
+ pr_err_once("update failed, SEAMCALLs will report failure until TDs killed\n");
+}
+
/*
* See multi_cpu_stop() from where this multi-cpu state-machine was
* adopted, and the rationale for touch_nmi_watchdog().
@@ -291,10 +296,13 @@ static int do_seamldr_install_module(void *seamldr_params)
break;
}
- if (ret)
+ if (ret) {
WRITE_ONCE(update_data.failed, true);
- else
+ if (curstate > MODULE_UPDATE_SHUTDOWN)
+ print_update_failure_message();
+ } else {
ack_state();
+ }
} else {
touch_nmi_watchdog();
rcu_momentary_eqs();
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v2 03/31] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects
From: Xu Yilun @ 2026-03-31 13:31 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-coco, linux-pci, dan.j.williams, x86, chao.gao, dave.jiang,
baolu.lu, yilun.xu, zhenzhong.duan, kvm, rick.p.edgecombe,
dave.hansen, kas, xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <4af194d0-f1d7-4acf-aeef-cf3bb3a4b10e@suse.com>
On Mon, Mar 30, 2026 at 04:31:43PM +0300, Nikolay Borisov wrote:
>
>
> On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
> <snip>
>
> > +/**
> > + * tdx_page_array_ctrl_leak() - Leak data pages and free the container
> > + * @array: The tdx_page_array to be leaked.
> > + *
> > + * Call this function when failed to reclaim the control pages. Free the root
> > + * page and the holding structures, but orphan the data pages, to prevent the
> > + * host from re-allocating and accessing memory that the hardware may still
> > + * consider private.
> > + */
> > +void tdx_page_array_ctrl_leak(struct tdx_page_array *array)
> > +{
> > + if (!array)
> > + return;
> > +
> > + kfree(array->pages);
> > + kfree(array->root);
> > + kfree(array);
> > +}
> > +EXPORT_SYMBOL_GPL(tdx_page_array_ctrl_leak);
>
> This instantly raises a red flag if by design an API has the ability to
OK. With the discussion in this thread, I tend to remove this leak API.
> simply leak memory. Under what conditions this might be required, can't we
> do something to gracefully handle the case when pages cannot be freed
> instantly, i.e queued freeing or some such ? Simply leaking them is a big
> NO.
It was intended to be called when failing to reclaim pages from secure
firmware, maybe because of firmware bug. In this case kernel has no idea
what to do. Leaking is a last resort here, don't expect things still work.
^ permalink raw reply
* Re: [PATCH v2 04/31] x86/virt/tdx: Support allocating contiguous pages for tdx_page_array
From: Xu Yilun @ 2026-03-31 13:37 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-coco, linux-pci, dan.j.williams, x86, chao.gao, dave.jiang,
baolu.lu, yilun.xu, zhenzhong.duan, kvm, rick.p.edgecombe,
dave.hansen, kas, xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <7d67722b-5107-4dc3-b8fb-04c3c86a1563@suse.com>
> > static struct tdx_page_array *
> > -tdx_page_array_alloc(unsigned int nr_pages)
> > +tdx_page_array_alloc(unsigned int nr_pages,
> > + int (*alloc_fn)(unsigned int nr_pages,
> > + struct page **pages, void *data),
> > + void *data)
>
> This interface seems cumbersome, since you will always have separate
> allocation paths:
>
> Contig, Bulk and Iommu mt pages let's just keep them separate. I.e the flow
> should be:
>
> 1. Do common allocation by calling tdx_page_array_alloc (you aren't passing
> the alloc function) you just get a bare-bones tdx_page_array struct
>
> 2. Do the specific allocation in either :
>
> tdx_page_array_create - for the bulk case
> tdx_page_array_alloc_contig - for the contig case
> tdx_page_array_create_iommu_mt - for the iommu case. Here you can open code
> tdx_alloc_pages_iommu_mt.
>
> And keep the specific clearly separate in each function.
Thank for the suggestion. There are also other concerns in this thread,
but I'll take this into consideration.
^ permalink raw reply
* Re: [PATCH v2 05/31] x86/virt/tdx: Extend tdx_page_array to support IOMMU_MT
From: Xu Yilun @ 2026-03-31 14:19 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: Williams, Dan J, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, x86@kernel.org, Gao, Chao, Xu, Yilun,
dave.hansen@linux.intel.com, kas@kernel.org,
baolu.lu@linux.intel.com, Jiang, Dave, Li, Xiaoyao,
Verma, Vishal L, Duan, Zhenzhong, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <828f174d49a1ecaec65ba1179e08c6b22e249297.camel@intel.com>
> > +static int tdx_alloc_pages_iommu_mt(unsigned int nr_pages, struct page **pages,
> > + void *data)
> > +{
> > + unsigned int iq_order = (unsigned int)(long)data;
> > + struct folio *t_iq, *t_ctxiq;
> > + int ret;
> > +
> > + /* TODO: folio_alloc_node() is preferred, but need numa info */
> > + t_iq = folio_alloc(GFP_KERNEL | __GFP_ZERO, iq_order);
> > + if (!t_iq)
> > + return -ENOMEM;
> > +
> > + t_ctxiq = folio_alloc(GFP_KERNEL | __GFP_ZERO, iq_order);
> > + if (!t_ctxiq) {
> > + ret = -ENOMEM;
> > + goto out_t_iq;
> > + }
> > +
> > + ret = tdx_alloc_pages_bulk(nr_pages - 2, pages + 2, NULL);
> > + if (ret)
> > + goto out_t_ctxiq;
> > +
> > + pages[0] = folio_page(t_iq, 0);
> > + pages[1] = folio_page(t_ctxiq, 0);
>
> To me it seems like this can't really be called a page array any more. The first
> two u64's are too special. Instead it's a special one-off ABI format passed via
> a page.
>
> BTW, I can't find TDH.IOMMU.SETUP in the docs. Any pointers?
https://cdrdv2.intel.com/v1/dl/getContent/858625
>
> > +
> > + return 0;
> > +
> > +out_t_ctxiq:
> > + folio_put(t_ctxiq);
> > +out_t_iq:
> > + folio_put(t_iq);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * tdx_page_array_create_iommu_mt() - Create a page array for IOMMU Memory Tables
> > + * @iq_order: The allocation order for the IOMMU Invalidation Queue.
> > + * @nr_mt_pages: Number of additional order-0 pages for the MT.
> > + *
> > + * Allocate and populate a specialized tdx_page_array for IOMMU_MT structures.
> > + * The resulting array consists of two multi-order folios (at index 0 and 1)
> > + * followed by the requested number of order-0 pages.
> > + *
> > + * Return: Fully populated tdx_page_array or NULL on failure> > + */
> > +struct tdx_page_array *
> > +tdx_page_array_create_iommu_mt(unsigned int iq_order, unsigned int nr_mt_pages)
> > +{
> > + unsigned int nr_pages = nr_mt_pages + 2;
>
> Consider the amount of tricks that are needed to coax the tdx_page_array to
> populate the handoff page as needed. It adds 2 pages here, then subtracts them
> later in the callback. Then tweaks the pa in tdx_page_array_populate() to add
> the length...
mm.. The tricky part is the specific memory requirement/allocation, the
common part is the pa list contained in a root page. Maybe we only model
the later, let the specific user does the memory allocation. Is that
closer to your "break concepts apart" idea?
>
> > + struct tdx_page_array *array;
> > + int populated;
> > +
> > + if (nr_pages > TDX_PAGE_ARRAY_MAX_NENTS)
> > + return NULL;
> > +
> > + array = tdx_page_array_alloc(nr_pages, tdx_alloc_pages_iommu_mt,
> > + (void *)(long)iq_order);
> > + if (!array)
> > + return NULL;
> > +
> > + populated = tdx_page_array_populate(array, 0);
> > + if (populated != nr_pages)
> > + goto out_free;
> > +
> > + return array;
> > +
> > +out_free:
> > + tdx_page_array_free(array);
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(tdx_page_array_create_iommu_mt);
^ permalink raw reply
* Re: [PATCH v2 06/31] x86/virt/tdx: Read global metadata for TDX Module Extensions/Connect
From: Xu Yilun @ 2026-03-31 14:23 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-coco, linux-pci, dan.j.williams, x86, chao.gao, dave.jiang,
baolu.lu, yilun.xu, zhenzhong.duan, kvm, rick.p.edgecombe,
dave.hansen, kas, xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <a68bb3d5-0f93-428f-ac8c-f3400d1fc028@suse.com>
> > +static int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
> > +{
> > + int ret = 0;
> > + u64 val;
> > +
> > + if (!ret && !(ret = read_sys_metadata_field(0x3100000100000000, &val)))
>
> nit: This is likely generated by a script/llm because I see no other
> explanation why !ret is being checked here...
>
Yes, the file was generated by script at the beginning.
^ permalink raw reply
* Re: [PATCH v7 06/22] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Dave Hansen @ 2026-03-31 14:58 UTC (permalink / raw)
To: Chao Gao, linux-coco, kvm, linux-kernel
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260331124214.117808-7-chao.gao@intel.com>
On 3/31/26 05:41, Chao Gao wrote:
> Expose them as tdx-host device attributes. Make seamldr attributes
> visible only when the update feature is supported, as that's their sole
> purpose. Unconditional exposure is also problematic because reading them
> triggers P-SEAMLDR calls that break KVM on CPUs with a specific erratum
> (to be enumerated and handled in a later patch).
The erratum is irrelevant, IMNHO. Or it *should* be irrelevant.
^ permalink raw reply
* Re: [PATCH v7 06/22] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Dave Hansen @ 2026-03-31 14:58 UTC (permalink / raw)
To: Chao Gao, linux-coco, kvm, linux-kernel
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260331124214.117808-7-chao.gao@intel.com>
On 3/31/26 05:41, Chao Gao wrote:
> +/*
> + * 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);
I don't like that these are really *exactly* the same, except for the
name but are defined so differently. I see three alternatives that are
better:
1. Open code *both* so they look the same
2. Define a macro that both can use that has a seamldr_## prefix
3. Move the code to a new file to avoid symbol conflicts.
I'd honestly be fine with any of those.
The other option is to just put the module information at the top
directory so the sysfs name differentiates things.
For real, how many things are going to be in the seamldr directory? If
it's just two, is it worth having a directory?
^ permalink raw reply
* Re: [PATCH v7 07/22] coco/tdx-host: Implement firmware upload sysfs ABI for TDX module updates
From: Dave Hansen @ 2026-03-31 15:04 UTC (permalink / raw)
To: Chao Gao, linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260331124214.117808-8-chao.gao@intel.com>
On 3/31/26 05:41, Chao Gao wrote:
> +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);
How would this non-zero offset be triggered? Wouldn't it take a bug in
the firmware upload code? If so, why bother? It's not like something bad
happens here.
^ permalink raw reply
* Re: [PATCH v5 1/2] dma-mapping: introduce DMA_ATTR_CC_SHARED for shared memory
From: Jason Gunthorpe @ 2026-03-31 15:08 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, 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: <20260325192352.437608-2-jiri@resnulli.us>
On Wed, Mar 25, 2026 at 08:23:51PM +0100, 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
> shared memory on behalf of the device.
>
> When a caller has already arranged for the memory to be shared
> 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_SHARED 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.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> v4->v5:
> - rebased on top od dma-mapping-for-next
> - s/decrypted/shared/
> 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(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply
* Re: [PATCH v5 2/2] dma-buf: heaps: system: add system_cc_shared heap for explicitly shared memory
From: Jason Gunthorpe @ 2026-03-31 15:08 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, 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: <20260325192352.437608-3-jiri@resnulli.us>
On Wed, Mar 25, 2026 at 08:23:52PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Add a new "system_cc_shared" dma-buf heap to allow userspace to
> allocate shared (decrypted) memory for confidential computing (CoCo)
> VMs.
>
> On CoCo VMs, guest memory is private by default. The hardware uses an
> encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
> Intel TDX) to control whether a given memory access is private or
> shared. The kernel's direct map is set up as private,
> so pages returned by alloc_pages() are private in the direct map
> by default. To make this memory usable for devices that do not support
> DMA to private memory (no TDISP support), it has to be explicitly
> shared. A couple of things are needed to properly handle
> shared memory for the dma-buf use case:
>
> - set_memory_decrypted() on the direct map after allocation:
> Besides clearing the encryption bit in the direct map PTEs, this
> also notifies the hypervisor about the page state change. On free,
> the inverse set_memory_encrypted() must be called before returning
> pages to the allocator. If re-encryption fails, pages
> are intentionally leaked to prevent shared memory from being
> reused as private.
>
> - pgprot_decrypted() for userspace and kernel virtual mappings:
> Any new mapping of the shared pages, be it to userspace via
> mmap or to kernel vmalloc space via vmap, creates PTEs independent
> of the direct map. These must also have the encryption bit cleared,
> otherwise accesses through them would see encrypted (garbage) data.
>
> - DMA_ATTR_CC_SHARED for DMA mapping:
> Since the pages are already shared, the DMA API needs to be
> informed via DMA_ATTR_CC_SHARED so it can map them correctly
> as unencrypted for device access.
>
> On non-CoCo VMs, the system_cc_shared heap is not registered
> to prevent misuse by userspace that does not understand
> the security implications of explicitly shared memory.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> v4->v5:
> - bools renamed: s/decrypted/cc_decrypted/
> - other renames: s/decrypted/decrypted/ - this included name of the heap
> v2->v3:
> - removed couple of leftovers from headers
> v1->v2:
> - fixed build errors on s390 by including mem_encrypt.h
> - converted system heap flag implementation to a separate heap
> ---
> drivers/dma-buf/heaps/system_heap.c | 103 ++++++++++++++++++++++++++--
> 1 file changed, 98 insertions(+), 5 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply
* Re: [PATCH v7 07/22] coco/tdx-host: Implement firmware upload sysfs ABI for TDX module updates
From: Dave Hansen @ 2026-03-31 15:11 UTC (permalink / raw)
To: Chao Gao, linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260331124214.117808-8-chao.gao@intel.com>
On 3/31/26 05:41, Chao Gao wrote:
> +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;
> +}
> +
This will be the seventh 'fw_upload_ops' and the third that has this
pattern of not needing a ->poll_complete() implementation. It seems like
allowing a NULL ->poll_complete or having a common stub for this pattern
would be worthwhile.
I also don't think you need to be that verbose. This would be fine:
/*
* The upload completed during tdx_fw_write().
* Never poll for completion.
*/
^ permalink raw reply
* Re: [PATCH v2 11/31] x86/virt/tdx: Make TDX Module initialize Extensions
From: Xu Yilun @ 2026-03-31 14:58 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: Williams, Dan J, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, x86@kernel.org, Gao, Chao, Xu, Yilun,
dave.hansen@linux.intel.com, kas@kernel.org,
baolu.lu@linux.intel.com, Jiang, Dave, Li, Xiaoyao,
Verma, Vishal L, Duan, Zhenzhong, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <5cbed31cdfcc7e43f283abc275848370239a3d40.camel@intel.com>
> > static int tdx_ext_mem_add(struct tdx_page_array *ext_mem)
> > {
> > struct tdx_module_args args = {
> > @@ -1572,6 +1589,17 @@ static int __maybe_unused init_tdx_ext(void)
> > if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> > return 0;
> >
> > + /*
> > + * With this errata, TDX should use movdir64b to clear private pages
> > + * when reclaiming them. See tdx_quirk_reset_paddr().
> > + *
> > + * Don't expect this errata on any TDX Extensions supported platform.
> > + * All features require TDX Extensions (including TDX Extensions
> > + * itself) will never call tdx_quirk_reset_paddr().
> > + */
> > + if (boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
> > + return -ENXIO;
>
> I don't know if we are going to want to sprinkle these over every new feature
> until the end of time. If this feature will only show up on the platforms with
> this erratum, then I say we just drop the check.
Make sense.
>
> > +
> > nr_pages = tdx_sysinfo.ext.memory_pool_required_pages;
> > /*
> > * memory_pool_required_pages == 0 means no need to add more pages,
> > @@ -1587,6 +1615,20 @@ static int __maybe_unused init_tdx_ext(void)
> > goto out_ext_mem;
> > }
> >
> > + /*
> > + * ext_required == 0 means no need to call TDH.EXT.INIT, the Extensions
> > + * are already working.
>
> How does this scenario happen exactly? And why not check it above at the
> beginning? Before the allocation, so it doesn't need to free.
>
> Is there a scenario where the memory needs to be given, but the extension is
> already inited?
mm.. you are right. It leads to something absurd.
I checked with TDX Module team again. The correct understanding is:
- TDX_FEATURES0_EXT bit shows Extensions is supported.
- optional feature bits are selected on TDH_SYS_CONFIG
- If one of the optional feature (e.g. TDX CONNECT) requires Extention,
memory_pool_required_pages > 0 && ext_required == 1. Otherwise no
need to initialize Extension.
So yes, I should check memory_pool_required_pages && ext_required at the
beginning.
>
> > + */
> > + if (tdx_sysinfo.ext.ext_required) {
> > + ret = tdx_ext_init();
> > + /*
> > + * Some pages may have been touched by the TDX module.
> > + * Flush cache before returning these pages to kernel.
> > + */
> > + if (ret)
> > + goto out_flush;
> > + }
> > +
> > /* Extension memory is never reclaimed once assigned */
> > tdx_page_array_ctrl_leak(ext_mem);
> >
> > @@ -1595,6 +1637,9 @@ static int __maybe_unused init_tdx_ext(void)
> >
> > return 0;
> >
> > +out_flush:
> > + if (ext_mem)
>
> For the error path we don't need to be efficient. But also why does it assume
> tdx_ext_init() can touch the pages, but tdx_ext_mem_add() can't?
The tdx_ext_mem_add() only collects memory, tdx_ext_init() does the
actual initialization for Extensions and touches the memory. But the
detail of when touching the pages is not specified in SPEC, do you think
host doesn't have to tell the difference, just flush when any one of
ext-SEAMCALLs is called?
>
>
> > + wbinvd_on_all_cpus();
> > out_ext_mem:
> > tdx_page_array_free(ext_mem);
> >
>
^ permalink raw reply
* Re: [PATCH v7 08/22] x86/virt/seamldr: Allocate and populate a module update request
From: Dave Hansen @ 2026-03-31 15:44 UTC (permalink / raw)
To: Chao Gao, linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260331124214.117808-9-chao.gao@intel.com>
On 3/31/26 05:41, Chao Gao wrote:
> + ptr = sig;
> + for (i = 0; i < sig_size / SZ_4K; i++) {
> + /*
> + * @sig is 4KB-aligned, but that does not imply PAGE_SIZE
> + * alignment when PAGE_SIZE != SZ_4K. Always include the
> + * in-page offset.
> + */
> + params->sigstruct_pa[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> + ((unsigned long)ptr & ~PAGE_MASK);
> + ptr += SZ_4K;
> + }
There are a billion things the mainline kernel _could_ do. Like a 32-bit
4/4 split or a non-4k PAGE_SIZE on x86. But the mainline kernel doesn't
*do* this.
Why add complexity to deal with something that doesn't exist?
^ permalink raw reply
* SVSM Development Call April 1st, 2026
From: Jörg Rödel @ 2026-03-31 16:22 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 v13 10/48] arm64: RMI: Ensure that the RMM has GPT entries for memory
From: Mathieu Poirier @ 2026-03-31 17:43 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Steven Price, kvm, kvmarm, Catalin Marinas, Marc Zyngier,
Will Deacon, James Morse, Oliver Upton, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
Emi Kisanuki, Vishal Annapurve
In-Reply-To: <152f5070-fda6-4381-bc40-4a70908c27c1@arm.com>
On Tue, Mar 31, 2026 at 12:05:47PM +0100, Suzuki K Poulose wrote:
> Hi Mathieu,
>
> On 30/03/2026 21:58, Mathieu Poirier wrote:
> > Hi,
> >
> > On Wed, Mar 18, 2026 at 03:53:34PM +0000, Steven Price wrote:
> > > The RMM may not be tracking all the memory of the system at boot. Create
> > > the necessary tracking state and GPTs within the RMM so that all boot
> > > memory can be delegated to the RMM as needed during runtime.
> > >
> > > Note: support is currently missing for SROs which means that if the RMM
> > > needs memory donating this will fail (and render CCA unusable in Linux).
> > >
> > > Signed-off-by: Steven Price <steven.price@arm.com>
> > > ---
> > > New patch for v13
> > > ---
> > > arch/arm64/kvm/rmi.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 89 insertions(+)
> > >
> > > diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> > > index 9590dff9a2c1..80aedc85e94a 100644
> > > --- a/arch/arm64/kvm/rmi.c
> > > +++ b/arch/arm64/kvm/rmi.c
> > > @@ -4,6 +4,7 @@
> > > */
> > > #include <linux/kvm_host.h>
> > > +#include <linux/memblock.h>
> > > #include <asm/kvm_pgtable.h>
> > > #include <asm/rmi_cmds.h>
> > > @@ -56,6 +57,18 @@ static int rmi_check_version(void)
> > > return 0;
> > > }
> > > +/*
> > > + * These are the 'default' sizes when passing 0 as the tracking_region_size.
> > > + * TODO: Support other granule sizes
> > > + */
> > > +#ifdef CONFIG_PAGE_SIZE_4KB
> > > +#define RMM_GRANULE_TRACKING_SIZE SZ_1G
> > > +#elif defined(CONFIG_PAGE_SIZE_16KB)
> > > +#define RMM_GRANULE_TRACKING_SIZE SZ_32M
> > > +#elif defined(CONFIG_PAGE_SIZE_64KB)
> > > +#define RMM_GRANULE_TRACKING_SIZE SZ_512M
> > > +#endif
> > > +
> > > static int rmi_configure(void)
> > > {
> > > struct rmm_config *config __free(free_page) = NULL;
> > > @@ -95,6 +108,80 @@ static int rmi_configure(void)
> > > return 0;
> > > }
> > > +static int rmi_verify_memory_tracking(phys_addr_t start, phys_addr_t end)
> > > +{
> > > + start = ALIGN_DOWN(start, RMM_GRANULE_TRACKING_SIZE);
> >
> > This will produce an error on systems where the start of system memory is not
> > aligned to RMM_GRANULE_TRACKING_SIZE. For instance, on QEMU-SBSA the system
> > memory starts at 0x100_4300_0000. With the above and RMM_GRANULE_TRACKING_SIZE
> > set to SZ_1G, @start becomes 0x100_4000_0000, which falls outside the memory map
> > known to the TF-A. I fixed it with these modifications:
>
> Thanks for raising this. This would need to be addressed in the RMM
> spec, I have raised it with the team and will be addressed soon.
>
> >
> > LINUX:
> >
> > diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> > index 10ff1c3bddaf..21bfbbe2f047 100644
> > --- a/arch/arm64/kvm/rmi.c
> > +++ b/arch/arm64/kvm/rmi.c
> > @@ -424,7 +424,9 @@ static int rmi_configure(void)
> > static int rmi_verify_memory_tracking(phys_addr_t start, phys_addr_t end)
> > {
> > - start = ALIGN_DOWN(start, RMM_GRANULE_TRACKING_SIZE);
> > + phys_addr_t offset;
> > +
> > + offset = start - ALIGN_DOWN(start, RMM_GRANULE_TRACKING_SIZE);
> > end = ALIGN(end, RMM_GRANULE_TRACKING_SIZE);
> > while (start < end) {
> > @@ -439,7 +441,13 @@ static int rmi_verify_memory_tracking(phys_addr_t start, phys_addr_t end)
> > start);
> > return -ENODEV;
> > }
> > - start += RMM_GRANULE_TRACKING_SIZE;
> > +
> > + if (offset) {
> > + start += (RMM_GRANULE_TRACKING_SIZE - offset);
> > + offset = 0;
> > + } else {
> > + start += RMM_GRANULE_TRACKING_SIZE;
> > + }
> > }
> > return 0;
> >
> > RMM:
> >
> > diff --git a/runtime/rmi/granule.c b/runtime/rmi/granule.c
> > index cef521fc0869..60358d9ee81e 100644
> > --- a/runtime/rmi/granule.c
> > +++ b/runtime/rmi/granule.c
> > @@ -209,9 +209,11 @@ void smc_granule_tracking_get(unsigned long addr,
> > return;
> > }
> > +#if 0
> > if (!ALIGNED(addr, RMM_INTERNAL_TRACKING_REGION_SIZE)) {
> > return;
> > }
> > +#endif
> > g = find_granule(addr);
> > if (g != NULL) {
> >
> > This is likely not the right fix but hopefully provides some guidance. Send me
> > your patches when you have an idea and I'll test them.
>
> We will send you the update once it is fixed in the RMM spec. The rough idea
> is to remove the ALIGNMENT restrictions and return a Range that
> the host can iterate over to find "regions" with the same type of
> memory.
>
Ok, thanks for looking into this.
>
> Cheers
> Suzuki
>
>
> >
> > Thanks,
> > Mathieu
> >
> >
> > > + end = ALIGN(end, RMM_GRANULE_TRACKING_SIZE);
> > > +
> > > + while (start < end) {
> > > + unsigned long ret, category, state;
> > > +
> > > + ret = rmi_granule_tracking_get(start, &category, &state);
> > > + if (ret != RMI_SUCCESS ||
> > > + state != RMI_TRACKING_FINE ||
> > > + category != RMI_MEM_CATEGORY_CONVENTIONAL) {
> > > + /* TODO: Set granule tracking in this case */
> > > + kvm_err("Granule tracking for region isn't fine/conventional: %llx",
> > > + start);
> > > + return -ENODEV;
> > > + }
> > > + start += RMM_GRANULE_TRACKING_SIZE;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static unsigned long rmi_l0gpt_size(void)
> > > +{
> > > + return 1UL << (30 + FIELD_GET(RMI_FEATURE_REGISTER_1_L0GPTSZ,
> > > + rmm_feat_reg1));
> > > +}
> > > +
> > > +static int rmi_create_gpts(phys_addr_t start, phys_addr_t end)
> > > +{
> > > + unsigned long l0gpt_sz = rmi_l0gpt_size();
> > > +
> > > + start = ALIGN_DOWN(start, l0gpt_sz);
> > > + end = ALIGN(end, l0gpt_sz);
> > > +
> > > + while (start < end) {
> > > + int ret = rmi_gpt_l1_create(start);
> > > +
> > > + if (ret && ret != RMI_ERROR_GPT) {
> > > + /*
> > > + * FIXME: Handle SRO so that memory can be donated for
> > > + * the tables.
> > > + */
> > > + kvm_err("GPT Level1 table missing for %llx\n", start);
> > > + return -ENOMEM;
> > > + }
> > > + start += l0gpt_sz;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int rmi_init_metadata(void)
> > > +{
> > > + phys_addr_t start, end;
> > > + const struct memblock_region *r;
> > > +
> > > + for_each_mem_region(r) {
> > > + int ret;
> > > +
> > > + start = memblock_region_memory_base_pfn(r) << PAGE_SHIFT;
> > > + end = memblock_region_memory_end_pfn(r) << PAGE_SHIFT;
> > > + ret = rmi_verify_memory_tracking(start, end);
> > > + if (ret)
> > > + return ret;
> > > + ret = rmi_create_gpts(start, end);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int rmm_check_features(void)
> > > {
> > > if (kvm_lpa2_is_enabled() && !rmi_has_feature(RMI_FEATURE_REGISTER_0_LPA2)) {
> > > @@ -120,6 +207,8 @@ void kvm_init_rmi(void)
> > > return;
> > > if (rmi_configure())
> > > return;
> > > + if (rmi_init_metadata())
> > > + return;
> > > /* Future patch will enable static branch kvm_rmi_is_available */
> > > }
> > > --
> > > 2.43.0
> > >
> > >
>
^ permalink raw reply
* Re: [PATCH v2 3/5] x86/virt/tdx: Add SEAMCALL wrapper for TDH.SYS.DISABLE
From: Verma, Vishal L @ 2026-03-31 18:22 UTC (permalink / raw)
To: kas@kernel.org, Edgecombe, Rick P
Cc: seanjc@google.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
x86@kernel.org, tglx@kernel.org, pbonzini@redhat.com,
linux-coco@lists.linux.dev, kvm@vger.kernel.org
In-Reply-To: <acu4raBTjdFcfVlS@thinkstation>
On Tue, 2026-03-31 at 13:18 +0100, Kiryl Shutsemau wrote:
> On Mon, Mar 30, 2026 at 07:25:22PM +0000, Edgecombe, Rick P wrote:
> > > I assumed that if the SEAMCALL fails other SEAMCALLs suppose to be
> > > functional. Hm?
> >
> > The behavior should be that once you make this seamcall (assuming it's
> > supported) that no other seamcalls can be made. They will return an
> > error. Do you think something else would be better? If it's an old TDX
> > module, nothing happens of course.
>
> I guess the actual behaviour is dependant on the return code. It is
> obviously going to be the case for TDX_SUCCESS. And from the discussion,
> I guess that's true for TDX_SYS_BUSY and TDX_INTERRUPTED_RESUMABLE.
>
> What about other cases? The spec draft also lists TDX_SYS_NOT_READY and
> TDX_SYS_SHUTDOWN.
I think these are safe too - TDX_SYS_SHUTDOWN means the module has
already been shutdown, which this seamcall would've done, so things
should be in the same state either way.
TDX_SYS_NOT_READY means the module hasn't been initialized yet. This
seamcall should just exit, and the module is already blocking any
seamcall that need the module to be initialized. The seamcalls to
initialize the module will be allowed, as they are after a sys_disable
call anyway.
>
> I wounder if it can affect the kernel. Consider the case when kexec
> (crash kernel start) happens due to crash on TDX module.
>
> Will we be able to shutdown TDX module cleanly and make kexec safe?
Hm -are the semantics for what happens if there is a crash in the
module defined? I think Linux should expect that sys_disable should
either start doing its shutdown work, or exit with one of the other
defined exit statuses. Anything else would be considered a module bug.
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-03-31 19:13 UTC (permalink / raw)
To: Yan Zhao
Cc: Rick P Edgecombe, Dave Hansen, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Kai Huang, Xiaoyao Li,
dave.hansen@linux.intel.com, kas@kernel.org, mingo@redhat.com,
pbonzini@redhat.com, binbin.wu@linux.intel.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
Isaku Yamahata, sagis@google.com, Vishal Annapurve, bp@alien8.de,
tglx@kernel.org, yilun.xu@linux.intel.com, x86@kernel.org
In-Reply-To: <acYrxIxoENyZhKCV@yzhao56-desk.sh.intel.com>
On Fri, Mar 27, 2026, Yan Zhao wrote:
> On Thu, Mar 26, 2026 at 12:57:26AM +0800, Edgecombe, Rick P wrote:
> > On Wed, 2026-03-25 at 17:10 +0800, Yan Zhao wrote:
> > > > I don't really understand what this is saying.
> > > >
> > > > Is the concern that KVM might want to set up page tables for memory
> > > > that differ from how it was allocated? I'm a bit worried that this
> > > > assumes something about folios that doesn't always hold.
> > > >
> > > > I think the hugetlbfs gigantic support uses folios in at least a
> > > > few spots today.
> > > Below is the background of this problem. I'll try to include a short
> > > summary in the next version's patch logs.
> >
> > While this patchset is kind of pre-work for TDX huge pages, the reason
> > to separate it out and push it earlier is because it has some value on
> > it's own. So I'd think to focus mostly on the impact of the change
> > today.
> >
> > How about this justification:
> > 1. Because KVM handles guest memory as PFNs, and the SEAMCALLs under
> > discussion are only used there, PFN is more natural.
> >
> > 2. The struct page was partly making sure we didn't pass a wrong arg
> > (typical type safety) and partly ensuring that KVM doesn't pass non-
> > convertible memory, however the SEAMCALLs themselves can check this for
> > the kernel. So the case is already covered by warnings.
> >
> > In conclusion, the PFN is more natural and the original purpose of
> > struct page is already covered.
Most importantly, having core TDX make assumptions based on the struct page and/or
folio will create subtle dependencies that are easily avoided.
> > Sean said somewhere IIRC that he would have NAKed the struct page thing
> > if he had seen it, for even the base support.
Yes.
> > And the two points above don't actually require discussion of even huge
> > pages. So does it actually add any value to dive into the issues you list
> > below?
> I wanted to mention the issues listed below because I'm not sure if anyone has
> the same question as me: why do we have to convert struct page to PFN if they
> can both achieve the same purpose, given that currently all private memory
> allocated by gmem has struct page backing?
From https://lore.kernel.org/all/aWgyhmTJphGQqO0Y@google.com:
: I'm not at all opposed to backing guest_memfd with "struct page", quite the
: opposite. What I don't want is to bake assumptions into KVM code that doesn't
: _require_ struct page, because that has cause KVM immense pain in the past.
:
: And I'm strongly opposed to KVM special-casing TDX or anything else, precisely
: because we struggled through all that pain so that KVM would work better with
: memory that isn't backed by "struct page", or more specifically, memory that has
: an associated "struct page", but isn't managed by core MM, e.g. isn't refcounted.
^ permalink raw reply
* Re: [PATCH v2 2/5] x86/virt/tdx: Pull kexec cache flush logic into arch/x86
From: Sean Christopherson @ 2026-03-31 19:22 UTC (permalink / raw)
To: Vishal Verma
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, Paolo Bonzini,
linux-kernel, linux-coco, kvm, Kai Huang
In-Reply-To: <20260323-fuller_tdx_kexec_support-v2-2-87a36409e051@intel.com>
On Mon, Mar 23, 2026, Vishal Verma wrote:
> From: Rick Edgecombe <rick.p.edgecombe@intel.com>
>
> KVM tries to take care of some required cache flushing earlier in the
> kexec path in order to be kind to some long standing races that can occur
> later in the operation. Until recently, VMXOFF was handled within KVM.
> Since VMX being enabled is required to make a SEAMCALL, it had the best
> per-cpu scoped operation to plug the flushing into. So it is kicked off
> from there.
>
> This early kexec cache flushing in KVM happens via a syscore shutdown
> callback. Now that VMX enablement control has moved to arch/x86, which has
> grown its own syscore shutdown callback, it no longer make sense for it to
> live in KVM. It fits better with the TDX enablement managing code.
>
> In addition, future changes will add a SEAMCALL that happens immediately
> before VMXOFF, which means the cache flush in KVM will be too late to
> flush the cache before the last SEAMCALL. So move it to the newly added TDX
> arch/x86 syscore shutdown handler.
>
> Since tdx_cpu_flush_cache_for_kexec() is no longer needed by KVM, make it
> static and remove the export. Since it is also not part of an operation
> spread across disparate components, remove the redundant comments and
> verbose naming.
>
> In the existing KVM based code, CPU offline also funnels through
> tdx_cpu_flush_cache_for_kexec(). So the centralization to the arch/x86
> syscore shutdown callback elides this CPU offline time behavior. However,
> WBINVD is already generally done at CPU offline as matter of course. So
> don't bother adding TDX specific logic for this, and rely on the normal
> WBINVD to handle it.
>
> Acked-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Ingoring the potential fixup needed for the existing bug...
Acked-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/tdx.h | 6 ------
> arch/x86/kvm/vmx/tdx.c | 10 ----------
> arch/x86/virt/vmx/tdx/tdx.c | 39 ++++++++++++++++++++-------------------
> 3 files changed, 20 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 2917b3451491..7674fc530090 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -205,11 +205,5 @@ 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; }
> #endif /* CONFIG_INTEL_TDX_HOST */
>
> -#ifdef CONFIG_KEXEC_CORE
> -void tdx_cpu_flush_cache_for_kexec(void);
> -#else
> -static inline void tdx_cpu_flush_cache_for_kexec(void) { }
> -#endif
> -
> #endif /* !__ASSEMBLER__ */
> #endif /* _ASM_X86_TDX_H */
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index b7264b533feb..50a5cfdbd33e 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -440,16 +440,6 @@ void tdx_disable_virtualization_cpu(void)
> tdx_flush_vp(&arg);
> }
> local_irq_restore(flags);
> -
> - /*
> - * Flush cache now if kexec is possible: this is necessary to avoid
> - * having dirty private memory cachelines when the new kernel boots,
> - * but WBINVD is a relatively expensive operation and doing it during
> - * kexec can exacerbate races in native_stop_other_cpus(). Do it
> - * now, since this is a safe moment and there is going to be no more
> - * TDX activity on this CPU from this point on.
> - */
> - tdx_cpu_flush_cache_for_kexec();
> }
>
> #define TDX_SEAMCALL_RETRIES 10000
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index cb9b3210ab71..0802d0fd18a4 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -224,8 +224,28 @@ static int tdx_offline_cpu(unsigned int cpu)
> return 0;
> }
>
> +static void tdx_cpu_flush_cache(void)
> +{
> + lockdep_assert_preemption_disabled();
> +
> + if (!this_cpu_read(cache_state_incoherent))
> + return;
> +
> + wbinvd();
> + this_cpu_write(cache_state_incoherent, false);
> +}
> +
> static void tdx_shutdown_cpu(void *ign)
> {
> + /*
> + * Flush cache now if kexec is possible: this is necessary to avoid
> + * having dirty private memory cachelines when the new kernel boots,
> + * but WBINVD is a relatively expensive operation and doing it during
> + * kexec can exacerbate races in native_stop_other_cpus(). Do it
> + * now, since this is a safe moment and there is going to be no more
> + * TDX activity on this CPU from this point on.
> + */
> + tdx_cpu_flush_cache();
> x86_virt_put_ref(X86_FEATURE_VMX);
> }
>
> @@ -1920,22 +1940,3 @@ 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);
> -
> -#ifdef CONFIG_KEXEC_CORE
> -void tdx_cpu_flush_cache_for_kexec(void)
> -{
> - lockdep_assert_preemption_disabled();
Is there a pre-existing bug here that gets propagate to tdx_shutdown_cpu()? When
called from kvm_offline_cpu(), preemption won't be fully disabled, but per-CPU
access are fine because the task is pinned to the target CPU.
See https://lore.kernel.org/all/aUVx20ZRjOzKgKqy@google.com
> -
> - if (!this_cpu_read(cache_state_incoherent))
> - return;
> -
> - /*
> - * Private memory cachelines need to be clean at the time of
> - * kexec. Write them back now, as the caller promises that
> - * there should be no more SEAMCALLs on this CPU.
> - */
> - wbinvd();
> - this_cpu_write(cache_state_incoherent, false);
> -}
> -EXPORT_SYMBOL_FOR_KVM(tdx_cpu_flush_cache_for_kexec);
> -#endif
>
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH v2 1/5] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Sean Christopherson @ 2026-03-31 19:30 UTC (permalink / raw)
To: Vishal Verma
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, Paolo Bonzini,
linux-kernel, linux-coco, kvm
In-Reply-To: <20260323-fuller_tdx_kexec_support-v2-1-87a36409e051@intel.com>
On Mon, Mar 23, 2026, Vishal Verma wrote:
> diff --git a/arch/x86/kvm/vmx/tdx_errno.h b/arch/x86/include/asm/shared/tdx_errno.h
> similarity index 64%
> rename from arch/x86/kvm/vmx/tdx_errno.h
> rename to arch/x86/include/asm/shared/tdx_errno.h
> index 6ff4672c4181..8bf6765cf082 100644
> --- a/arch/x86/kvm/vmx/tdx_errno.h
> +++ b/arch/x86/include/asm/shared/tdx_errno.h
> @@ -1,14 +1,15 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> -/* architectural status code for SEAMCALL */
> -
> -#ifndef __KVM_X86_TDX_ERRNO_H
> -#define __KVM_X86_TDX_ERRNO_H
> +#ifndef _ASM_X86_SHARED_TDX_ERRNO_H
> +#define _ASM_X86_SHARED_TDX_ERRNO_H
> +#include <asm/trapnr.h>
>
> +/* Upper 32 bit of the TDX error code encodes the status */
> #define TDX_SEAMCALL_STATUS_MASK 0xFFFFFFFF00000000ULL
>
> /*
> - * TDX SEAMCALL Status Codes (returned in RAX)
> + * TDX Status Codes (returned in RAX)
> */
> +#define TDX_SUCCESS 0ULL
> #define TDX_NON_RECOVERABLE_VCPU 0x4000000100000000ULL
> #define TDX_NON_RECOVERABLE_TD 0x4000000200000000ULL
> #define TDX_NON_RECOVERABLE_TD_NON_ACCESSIBLE 0x6000000500000000ULL
> @@ -17,6 +18,7 @@
> #define TDX_OPERAND_INVALID 0xC000010000000000ULL
> #define TDX_OPERAND_BUSY 0x8000020000000000ULL
> #define TDX_PREVIOUS_TLB_EPOCH_BUSY 0x8000020100000000ULL
> +#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
> #define TDX_PAGE_METADATA_INCORRECT 0xC000030000000000ULL
> #define TDX_VCPU_NOT_ASSOCIATED 0x8000070200000000ULL
> #define TDX_KEY_GENERATION_FAILED 0x8000080000000000ULL
> @@ -28,6 +30,20 @@
> #define TDX_EPT_ENTRY_STATE_INCORRECT 0xC0000B0D00000000ULL
> #define TDX_METADATA_FIELD_NOT_READABLE 0xC0000C0200000000ULL
>
> +/*
> + * SW-defined error codes.
> + *
> + * Bits 47:40 == 0xFF indicate Reserved status code class that never used by
> + * TDX module.
> + */
> +#define TDX_ERROR _BITULL(63)
> +#define TDX_NON_RECOVERABLE _BITULL(62)
> +#define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40))
> +#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _ULL(0xFFFF0000))
> +
> +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP)
> +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)
I don't think the host's SW-defined error codes should be used by the guest. The
guest can't even make SEAMCALLs. So unless I'm misunderstanding the purpose, I
don't think it makes sense to move these into tdx_errno.h.
Regardless, please split this up into two patches:
1. Move tdx_errno.h
2. Land more #defines in tdx_errno.h
Because IIUC, tdx_errno.h holds *only* architecturally defined values, which makes
(1) super duper trivial to review and ack.
^ permalink raw reply
* Re: [PATCH v2 3/5] x86/virt/tdx: Add SEAMCALL wrapper for TDH.SYS.DISABLE
From: Edgecombe, Rick P @ 2026-03-31 21:36 UTC (permalink / raw)
To: Verma, Vishal L, kas@kernel.org
Cc: seanjc@google.com, bp@alien8.de, x86@kernel.org, hpa@zytor.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, pbonzini@redhat.com,
linux-coco@lists.linux.dev, kvm@vger.kernel.org
In-Reply-To: <b9aa84d76c131132b5268712a44200e804c31aeb.camel@intel.com>
On Tue, 2026-03-31 at 18:22 +0000, Verma, Vishal L wrote:
> >
> > I guess the actual behaviour is dependant on the return code. It is
> > obviously going to be the case for TDX_SUCCESS. And from the discussion,
> > I guess that's true for TDX_SYS_BUSY and TDX_INTERRUPTED_RESUMABLE.
> >
> > What about other cases? The spec draft also lists TDX_SYS_NOT_READY and
> > TDX_SYS_SHUTDOWN.
>
> I think these are safe too - TDX_SYS_SHUTDOWN means the module has
> already been shutdown, which this seamcall would've done, so things
> should be in the same state either way.
>
> TDX_SYS_NOT_READY means the module hasn't been initialized yet. This
> seamcall should just exit, and the module is already blocking any
> seamcall that need the module to be initialized. The seamcalls to
> initialize the module will be allowed, as they are after a sys_disable
> call anyway.
Should the seamcall return success in the case where it would return
TDX_SYS_NOT_READY? It is in basically a reset state right? The errors we care
about are actual errors (TDX_SW_ERROR), so it makes no difference to the code in
the patch. But it might be a nicer API for the seamcall?
>
> >
> > I wounder if it can affect the kernel. Consider the case when kexec
> > (crash kernel start) happens due to crash on TDX module.
> >
> > Will we be able to shutdown TDX module cleanly and make kexec safe?
>
> Hm -are the semantics for what happens if there is a crash in the
> module defined? I think Linux should expect that sys_disable should
> either start doing its shutdown work, or exit with one of the other
> defined exit statuses. Anything else would be considered a module bug.
We often have the question come up about how much we should to guard against
bugs in the TDX module. I tend to also think we should not do defensive
programming, same as we do for the kernel. If it's easy to handle something or
emit a warning it's nice, but otherwise the solution for such cases should be to
fix the TDX module bug.
But for the kdump case, we don't actually need sys disable to succeed. The kdump
kernel will not load the TDX module. And as for the errata, this already needs a
special situation to be a problem. But even if it happens, I'd think better to
try to the kdump. Not sure what the fix would be for that scenario, even if we
allowed for a large complexity budget. So best effort seems good.
Does it seem reasonable?
^ permalink raw reply
* Re: [PATCH v2 1/5] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Edgecombe, Rick P @ 2026-03-31 21:46 UTC (permalink / raw)
To: Verma, Vishal L, seanjc@google.com
Cc: bp@alien8.de, x86@kernel.org, kas@kernel.org, hpa@zytor.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, pbonzini@redhat.com,
linux-coco@lists.linux.dev, kvm@vger.kernel.org
In-Reply-To: <acwgvBzK01IrAAeU@google.com>
On Tue, 2026-03-31 at 12:30 -0700, Sean Christopherson wrote:
> +#define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40))
> > +#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _ULL(0xFFFF0000))
> > +
> > +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP)
> > +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)
>
> I don't think the host's SW-defined error codes should be used by the guest. The
> guest can't even make SEAMCALLs. So unless I'm misunderstanding the purpose, I
> don't think it makes sense to move these into tdx_errno.h.
Seems reasonable.
>
> Regardless, please split this up into two patches:
>
> 1. Move tdx_errno.h
> 2. Land more #defines in tdx_errno.h
>
> Because IIUC, tdx_errno.h holds *only* architecturally defined values, which makes
> (1) super duper trivial to review and ack.
Thanks!
^ permalink raw reply
* Re: [PATCH 1/2] x86/tdx: Fix off-by-one in port I/O handling
From: Kuppuswamy Sathyanarayanan @ 2026-03-31 21:57 UTC (permalink / raw)
To: Kiryl Shutsemau (Meta), Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86
Cc: H . Peter Anvin, Rick Edgecombe, Borys Tsyrulnikov, linux-kernel,
linux-coco, kvm, stable
In-Reply-To: <20260331112430.71425-2-kas@kernel.org>
Hi Kirill,
On 3/31/2026 4:24 AM, Kiryl Shutsemau (Meta) wrote:
> handle_in() and handle_out() in arch/x86/coco/tdx/tdx.c use:
>
> u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
>
> GENMASK(h, l) includes bit h. For size=1 (INB), this produces
> GENMASK(8, 0) = 0x1FF (9 bits) instead of GENMASK(7, 0) = 0xFF (8
> bits). The mask is one bit too wide for all I/O sizes.
>
> Fix the mask calculation.
>
> Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
> Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Cc: stable@vger.kernel.org
> ---
LGTM. Can you include a link to the bug report or related discussion in
the commit log? It will help understand the impact of this issue.
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> arch/x86/coco/tdx/tdx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 7b2833705d47..4d7f71d50122 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -693,7 +693,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
> .r13 = PORT_READ,
> .r14 = port,
> };
> - u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
> + u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
> bool success;
>
> /*
> @@ -713,7 +713,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
>
> static bool handle_out(struct pt_regs *regs, int size, int port)
> {
> - u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
> + u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
>
> /*
> * Emulate the I/O write via hypercall. More info about ABI can be found
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply
* Re: [PATCH 1/2] x86/tdx: Fix off-by-one in port I/O handling
From: Huang, Kai @ 2026-03-31 22:06 UTC (permalink / raw)
To: x86@kernel.org, mingo@redhat.com, kas@kernel.org, tglx@kernel.org,
bp@alien8.de, dave.hansen@linux.intel.com
Cc: Edgecombe, Rick P, hpa@zytor.com,
sathyanarayanan.kuppuswamy@linux.intel.com,
linux-kernel@vger.kernel.org, tsyrulnikov.borys@gmail.com,
kvm@vger.kernel.org, stable@vger.kernel.org,
linux-coco@lists.linux.dev
In-Reply-To: <20260331112430.71425-2-kas@kernel.org>
On Tue, 2026-03-31 at 12:24 +0100, Kiryl Shutsemau (Meta) wrote:
> handle_in() and handle_out() in arch/x86/coco/tdx/tdx.c use:
>
> u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
>
> GENMASK(h, l) includes bit h. For size=1 (INB), this produces
> GENMASK(8, 0) = 0x1FF (9 bits) instead of GENMASK(7, 0) = 0xFF (8
> bits). The mask is one bit too wide for all I/O sizes.
>
> Fix the mask calculation.
>
> Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
> Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Cc: stable@vger.kernel.org
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH 2/2] x86/tdx: Fix zero-extension for 32-bit port I/O
From: Kuppuswamy Sathyanarayanan @ 2026-03-31 22:10 UTC (permalink / raw)
To: Kiryl Shutsemau (Meta), Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86
Cc: H . Peter Anvin, Rick Edgecombe, Borys Tsyrulnikov, linux-kernel,
linux-coco, kvm, stable
In-Reply-To: <20260331112430.71425-3-kas@kernel.org>
Hi Kiril,
On 3/31/2026 4:24 AM, Kiryl Shutsemau (Meta) wrote:
> According to x86 architecture rules, 32-bit operations zero-extend the
> result to 64 bits. The current implementation of handle_in() only masks
> the lower 32 bits, which preserves the upper 32 bits of RAX when a
> 32-bit port IN instruction is emulated.
>
> Update handle_in() to zero out the entire RAX register when the I/O size
> is 4 bytes to ensure correct zero-extension. For smaller sizes (1 or 2
> bytes), continue to preserve the unaffected upper bits.
>
> Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
> Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Cc: stable@vger.kernel.org
> ---
If you have bug or discussion link, please include it.
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> arch/x86/coco/tdx/tdx.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 4d7f71d50122..b9b9a2d75119 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -703,8 +703,17 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
> */
> success = !__tdx_hypercall(&args);
>
> - /* Update part of the register affected by the emulated instruction */
> - regs->ax &= ~mask;
> + /*
> + * Update part of the register affected by the emulated instruction.
> + *
> + * 32-bit operands generate a 32-bit result, zero-extended to a 64-bit
> + * result.
> + */
> + if (size < 4)
> + regs->ax &= ~mask;
> + else
> + regs->ax = 0;
The logic would be more readable as:
if (size == 4)
regs->ax = 0;
else
regs->ax &= ~mask;
> +
> if (success)
> regs->ax |= args.r11 & mask;
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ 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