* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Aneesh Kumar K.V @ 2026-03-05 4:46 UTC (permalink / raw)
To: Dan Williams, linux-coco, linux-pci
Cc: gregkh, aik, yilun.xu, bhelgaas, alistair23, lukas, jgg,
Arnd Bergmann
In-Reply-To: <20260303000207.1836586-10-dan.j.williams@intel.com>
Dan Williams <dan.j.williams@intel.com> writes:
> After pci_tsm_bind() and pci_tsm_lock() the low level TSM driver is
> expected to populate PCI_TSM_EVIDENCE_TYPE_REPORT in its evidence store.
> This report is defined by the TDISP GET_DEVICE_INTERFACE_REPORT response
> payload.
>
> Add a helper to create encrypted MMIO descriptors from that report
> data. With those descriptors the TSM driver can use pci_tsm_mmio_setup() to
> inform ioremap() how to map the device per the device's expectations. The
> VM is expected to validate the interface with the relying party before
> accepting the device for operation.
>
> The helper also provides the obfuscated starting address for each
> encrypted MMIO range as the VM is never disclosed on the hpa that
> correlates to the gpa of the device's mmio. The obfuscated address is BAR
> relative.
>
> Based on an original patch by Aneesh [1]
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Link: https://lore.kernel.org/linux-coco/20251117140007.122062-8-aneesh.kumar@kernel.org/
> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> include/linux/ioport.h | 1 +
> include/linux/pci-tsm.h | 34 ++++++
> drivers/pci/tsm/core.c | 235 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 270 insertions(+)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 9afa30f9346f..1c106608c514 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -143,6 +143,7 @@ enum {
> IORES_DESC_RESERVED = 7,
> IORES_DESC_SOFT_RESERVED = 8,
> IORES_DESC_CXL = 9,
> + IORES_DESC_ENCRYPTED = 10,
> };
>
> /*
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index b70b4c0457c4..8869585230a3 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -194,12 +194,42 @@ struct pci_tsm_pf0 {
> struct pci_doe_mb *doe_mb;
> };
>
> +/**
> + * struct pci_tsm_mmio_entry - an encrypted MMIO range
> + * @res: MMIO address range (typically Guest Physical Address, GPA)
> + * @tsm_offset: Host Physical Address, HPA obfuscation offset added by the TSM.
> + * Translates report addresses to GPA.
> + */
> +struct pci_tsm_mmio_entry {
> + struct resource res;
> + u64 tsm_offset;
> +};
> +
> +struct pci_tsm_mmio {
> + int nr;
> + struct pci_tsm_mmio_entry mmio[] __counted_by(nr);
> +};
> +
> +static inline struct pci_tsm_mmio_entry *
> +pci_tsm_mmio_entry(struct pci_tsm_mmio *mmio, int idx)
> +{
> + return &mmio->mmio[idx];
> +}
> +
> +static inline struct resource *pci_tsm_mmio_resource(struct pci_tsm_mmio *mmio,
> + int idx)
> +{
> + return &mmio->mmio[idx].res;
> +}
> +
> /**
> * struct pci_tsm_devsec - context for tracking private/accepted PCI resources
> * @base_tsm: generic core "tsm" context
> + * @mmio: encrypted MMIO resources for this assigned device
> */
> struct pci_tsm_devsec {
> struct pci_tsm base_tsm;
> + struct pci_tsm_mmio *mmio;
> };
>
> /* physical function0 and capable of 'connect' */
> @@ -297,6 +327,10 @@ ssize_t pci_tsm_guest_req(struct pci_dev *pdev, enum pci_tsm_req_scope scope,
> struct pci_tsm_devsec *to_pci_tsm_devsec(struct pci_tsm *tsm);
> void pci_tsm_init_evidence(struct pci_tsm_evidence *evidence, int slot,
> enum hash_algo digest_algo);
> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> +void pci_tsm_mmio_teardown(struct pci_tsm_mmio *mmio);
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev);
> +int pci_tsm_mmio_free(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> #else
> static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> {
> diff --git a/drivers/pci/tsm/core.c b/drivers/pci/tsm/core.c
> index 039733fd19b1..e4f830b16d18 100644
> --- a/drivers/pci/tsm/core.c
> +++ b/drivers/pci/tsm/core.c
> @@ -15,6 +15,7 @@
> #include <linux/pci-tsm.h>
> #include <linux/sysfs.h>
> #include <linux/tsm.h>
> +#include <linux/unaligned.h>
> #include <linux/xarray.h>
> #include "../pci.h"
>
> @@ -558,6 +559,240 @@ static ssize_t dsm_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(dsm);
>
> +static void mmio_teardown(struct pci_tsm_mmio *mmio, int nr)
> +{
> + while (nr--)
> + remove_resource(pci_tsm_mmio_resource(mmio, nr));
> +}
> +
> +/**
> + * pci_tsm_mmio_setup() - mark device MMIO as encrypted in iomem
> + * @pdev: device owner of MMIO resources
> + * @mmio: container of an array of resources to mark encrypted
> + */
> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
> +{
> + int i;
> +
> + device_lock_assert(&pdev->dev);
> + if (pdev->dev.driver)
> + return -EBUSY;
> +
> + for (i = 0; i < mmio->nr; i++) {
> + struct resource *res = pci_tsm_mmio_resource(mmio, i);
> + int j;
> +
> + if (resource_size(res) == 0 || !res->end)
> + break;
> +
> + /* Only require the caller to set the range, init remainder */
> + *res = DEFINE_RES_NAMED_DESC(res->start, resource_size(res),
> + "PCI MMIO Encrypted",
> + IORESOURCE_MEM,
> + IORES_DESC_ENCRYPTED);
> +
> + for (j = 0; j < PCI_NUM_RESOURCES; j++)
> + if (resource_contains(pci_resource_n(pdev, j), res))
> + break;
> +
> + /* Request is outside of device MMIO */
> + if (j >= PCI_NUM_RESOURCES)
> + break;
> +
> + if (insert_resource(&iomem_resource, res) != 0)
> + break;
> + }
> +
> + if (i >= mmio->nr)
> + return 0;
> +
> + mmio_teardown(mmio, i);
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_setup);
> +
> +void pci_tsm_mmio_teardown(struct pci_tsm_mmio *mmio)
> +{
> + mmio_teardown(mmio, mmio->nr);
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_teardown);
> +
> +/*
> + * PCIe ECN TEE Device Interface Security Protocol (TDISP)
> + *
> + * Device Interface Report data object layout as defined by PCIe r7.0 section
> + * 11.3.11
> + */
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_TABLE BIT(0)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_PBA BIT(1)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_NON_TEE BIT(2)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_UPDATABLE BIT(3)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID GENMASK(31, 16)
> +
> +/* An interface report 'pfn' is 4K in size */
> +struct pci_tsm_devif_mmio {
> + __le64 pfn;
> + __le32 nr_pfns;
> + __le32 attributes;
> +};
> +
> +struct pci_tsm_devif_report {
> + __le16 interface_info;
> + __le16 reserved;
> + __le16 msi_x_message_control;
> + __le16 lnr_control;
> + __le32 tph_control;
> + __le32 mmio_range_count;
> + struct pci_tsm_devif_mmio mmio[];
> +};
> +
> +/**
> + * pci_tsm_mmio_alloc() - allocate encrypted MMIO range descriptor
> + * @pdev: device owner of MMIO ranges
> + * @report_data: TDISP Device Interface (DevIf) Report blob
> + * @report_sz: DevIf Report size
> + *
> + * Return: the encrypted MMIO range descriptor on success, NULL on failure
> + *
> + * Assumes that this is called within the live lifetime of a PCI device's
> + * association with a low level TSM.
> + */
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
> +{
> + struct pci_tsm *tsm = pdev->tsm;
> + struct pci_tsm_evidence *evidence = &tsm->evidence;
> + struct pci_tsm_evidence_object *report_obj = &evidence->obj[PCI_TSM_EVIDENCE_TYPE_REPORT];
> + struct tsm_dev *tsm_dev = tsm->tsm_dev;
> + u64 reporting_bar_base, last_reporting_end;
> + const struct pci_tsm_devif_report *report;
> + u32 mmio_range_count;
> + int last_bar = -1;
> + int i;
> +
> + guard(rwsem_read)(&evidence->lock);
> + if (report_obj->len < sizeof(struct pci_tsm_devif_report))
> + return NULL;
> +
> + if (dev_WARN_ONCE(&tsm_dev->dev, !IS_ALIGNED((unsigned long) report_obj->data, 8),
> + "misaligned report data\n"))
> + return NULL;
> +
> + report = report_obj->data;
> + mmio_range_count = __le32_to_cpu(report->mmio_range_count);
> +
> + /* check that the report object is self-consistent on mmio entries */
> + if (report_obj->len < struct_size(report, mmio, mmio_range_count))
> + return NULL;
> +
> + /* create pci_tsm_mmio descriptors from the report data */
> + struct pci_tsm_mmio *mmio __free(kfree) =
> + kzalloc(struct_size(mmio, mmio, mmio_range_count), GFP_KERNEL);
> + if (!mmio)
> + return NULL;
> +
> + for (i = 0; i < mmio_range_count; i++) {
> + u64 range_off;
> + struct range range;
> + const struct pci_tsm_devif_mmio *mmio_data = &report->mmio[i];
> + struct pci_tsm_mmio_entry *entry =
> + pci_tsm_mmio_entry(mmio, mmio->nr);
> + /* report values in are in terms of 4K pages */
> + u64 tsm_offset = __le64_to_cpu(mmio_data->pfn) * SZ_4K;
> + u64 size = __le32_to_cpu(mmio_data->nr_pfns) * SZ_4K;
> + u32 attr = __le32_to_cpu(mmio_data->attributes);
> + int bar = FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID,
> + attr);
> +
> + tsm_offset *= SZ_4K;
> + size *= SZ_4K;
> +
> + if (bar >= PCI_STD_NUM_BARS ||
> + !(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
> + pci_dbg(pdev, "Invalid reporting bar ID %d\n", bar);
> + return NULL;
> + }
> +
> + if (last_bar > bar) {
> + pci_dbg(pdev, "Reporting bar ID not in ascending order\n");
> + return NULL;
> + }
> +
> + if (last_bar < bar) {
> + /* transition to a new bar */
> + last_bar = bar;
> + /*
> + * The tsm_offset for the first range of the BAR
> + * corresponds to the BAR base.
> + */
> + reporting_bar_base = tsm_offset;
> + } else if (tsm_offset < last_reporting_end) {
> + pci_dbg(pdev, "Reporting ranges within BAR not in ascending order\n");
> + return NULL;
> + }
>
....
> + range_off = tsm_offset - reporting_bar_base;
>
range_off will always be zero? Should we do
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.
> +
> + last_reporting_end = tsm_offset + size;
> + if (last_reporting_end < tsm_offset) {
> + pci_dbg(pdev, "Reporting range overflow\n");
> + return NULL;
> + }
> +
> + range_off = tsm_offset - reporting_bar_base;
> + if (pci_resource_len(pdev, bar) < range_off + size) {
> + pci_dbg(pdev, "Reporting range larger than BAR size\n");
> + return NULL;
> + }
> +
> + range.start = pci_resource_start(pdev, bar) + range_off;
> + range.end = range.start + size - 1;
> +
> + if (FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_NON_TEE,
> + attr)) {
> + pci_dbg(pdev, "Skipping non-TEE range, BAR%d %pra\n",
> + bar, &range);
> + continue;
> + }
> +
> + /* Currently not supported */
> + if (FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_TABLE,
> + attr) ||
> + FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_PBA, attr)) {
> + pci_dbg(pdev, "Skipping MSIX range BAR%d %pra\n", bar,
> + &range);
> + continue;
> + }
> +
> + entry->res.start = range.start;
> + entry->res.end = range.end;
> + entry->tsm_offset = tsm_offset;
> + mmio->nr++;
> + }
> +
> + return_ptr(mmio);
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_alloc);
> +
> +/**
> + * pci_tsm_mmio_free() - free a pci_tsm_mmio instance
> + * @pdev: device owner of MMIO ranges
> + * @mmio: instance to free
> + *
> + * Returns 0 if @mmio was idle on entry, -EBUSY otherwise
> + */
> +int pci_tsm_mmio_free(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
> +{
> + for (int i = 0; i < mmio->nr; i++) {
> + struct resource *res = pci_tsm_mmio_resource(mmio, i);
> +
> + if (dev_WARN_ONCE(&pdev->dev, resource_assigned(res),
> + "MMIO resource still assigned %pr\n", res))
> + return -EBUSY;
> + }
> + kfree(mmio);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_free);
> +
> /**
> * pci_tsm_accept() - accept a device for private MMIO+DMA operation
> * @pdev: PCI device to accept
> --
> 2.52.0
^ permalink raw reply
* Re: [PATCH v4 16/24] x86/virt/seamldr: Install a new TDX Module
From: Xu Yilun @ 2026-03-05 4:22 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, Williams, Dan J,
bp@alien8.de
In-Reply-To: <6f1f835e27bef3462ae419906e25bd887eb8577b.camel@intel.com>
> > -static int do_seamldr_install_module(void *params)
> > +static int do_seamldr_install_module(void *seamldr_params)
>
> Nit:
>
> IMHO such renaming is just a noise to this patch, since in patch 10/11 it's
Agree. Otherwise
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> clear that the 'params' you passed in is seamldr_params. No?
>
> Perhaps just name it 'seamldr_params' at patch 11?
^ permalink raw reply
* Re: [PATCH v4 15/24] x86/virt/seamldr: Log TDX Module update failures
From: Xu Yilun @ 2026-03-05 4:18 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260212143606.534586-16-chao.gao@intel.com>
On Thu, Feb 12, 2026 at 06:35:18AM -0800, Chao Gao wrote:
> 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>
> ---
> 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 c59cdd5b1fe4..4e0a98404c7f 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -223,6 +223,11 @@ static void ack_state(void)
> set_target_state(tdp_data.state + 1);
> }
>
> +static void print_update_failure_message(void)
> +{
> + pr_err_once("update failed, SEAMCALLs will report failure until TDs killed\n");
> +}
The wrapper seems redundant but maybe too much indent if put the print
in the loop. Anyway, either is good to me, up to you.
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v4 13/24] x86/virt/seamldr: Shut down the current TDX module
From: Xu Yilun @ 2026-03-05 4:14 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260212143606.534586-14-chao.gao@intel.com>
On Thu, Feb 12, 2026 at 06:35:16AM -0800, Chao Gao wrote:
> The first step of TDX Module updates is shutting down the current TDX
> Module. This step also packs state information that needs to be
> preserved across updates as handoff data, which will be consumed by the
> updated module. The handoff data is stored internally in the SEAM range
> and is hidden from the kernel.
>
> To ensure a successful update, the new module must be able to consume
> the handoff data generated by the old module. Since handoff data layout
> may change between modules, the handoff data is versioned. Each module
> has a native handoff version and provides backward support for several
> older versions.
>
> The complete handoff versioning protocol is complex as it supports both
> module upgrades and downgrades. See details in Intel® Trust Domain
> Extensions (Intel® TDX) Module Base Architecture Specification, Revision
> 348549-007, Chapter 4.5.3 "Handoff Versioning".
>
> Ideally, the kernel needs to retrieve the handoff versions supported by
> the current module and the new module and select a version supported by
> both. But, since the Linux kernel only supports module upgrades, simply
> request the current module to generate handoff data using its highest
> supported version, expecting that the new module will likely support it.
>
> Note that only one CPU needs to call the TDX Module's shutdown API.
>
> 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>
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Xu Yilun @ 2026-03-05 4:12 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260212143606.534586-11-chao.gao@intel.com>
On Thu, Feb 12, 2026 at 06:35:13AM -0800, 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 at [1]. 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. The kernel does not
> validate P-SEAMLDR compatibility (for example, whether it supports 4KB
> or 16KB sigstruct); userspace must ensure the P-SEAMLDR version is
> compatible with the selected TDX Module by checking the minimum
> P-SEAMLDR version requirements at [2].
>
> 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>
^ permalink raw reply
* Re: [PATCH v4 09/24] x86/virt/seamldr: Check update limit before TDX Module updates
From: Xu Yilun @ 2026-03-05 4:09 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260212143606.534586-10-chao.gao@intel.com>
On Thu, Feb 12, 2026 at 06:35:12AM -0800, Chao Gao wrote:
> TDX maintains a log about each TDX Module which has been loaded. This
> log has a finite size which limits the number of TDX Module updates
> which can be performed.
>
> After each successful update, the remaining updates reduces by one. Once
> it reaches zero, further updates will fail until next reboot.
>
> Before updating the TDX Module, verify that the update limit has not been
> exceeded. Otherwise, P-SEAMLDR will detect this violation after the old TDX
> Module is gone and all TDs will be killed.
>
> Note that userspace should perform this check before updates. Perform this
> check in kernel as well to make the update process more robust.
>
> 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>
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Kalra, Ashish @ 2026-03-05 1:40 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: <7ab8d3af-b4f5-481c-ab2e-059ddd7e718e@intel.com>
Hello Dave and Sean,
On 3/4/2026 9:25 AM, Dave Hansen wrote:
> On 3/4/26 07:01, Sean Christopherson wrote:
>> I don't see any performance data in either posted version. Bluntly, this series
>> isn't going anywhere without data to guide us. E.g. comments like this from v1
>>
>> : And there is a cost associated with re-enabling the optimizations for all
>> : system RAM (even though it runs as a background kernel thread executing RMPOPT
>> : on different 1GB regions in parallel and with inline cond_resched()'s),
>> : we don't want to run this periodically.
>>
>> suggest there is meaningful cost associated with the scan.
>
> Well the RMP is 0.4% of the size of system memory, and I assume that you
> need to scan the whole table. There are surely shortcuts for 2M pages,
> but with 4k, that's ~8.5GB of RMP table for 2TB of memory. That's an
> awful lot of memory traffic for each CPU.
The RMPOPT instruction is optimized for 2M pages, so it checks that
all 512 2MB entries in that 1GB region are not assigned, i.e., for each
2MB RMP in the 1GB region containing the specified address it checks if
they are not assigned.
>
> It'll be annoying to keep a refcount per 1GB of paddr space.
>
> One other way to do it would be to loosely mirror the RMPOPT bitmap and
> keep our own bitmap of 1GB regions that _need_ RMPOPT run on them. Any
> private=>shared conversion sets a bit in the bitmap and schedules some
> work out in the future.
>
> It could also be less granular than that. Instead of any private=>shared
> conversion, the RMPOPT scan could be triggered on VM destruction which
> is much more likely to result in RMPOPT doing anything useful.
Yes, it will need to be more granular than scheduling RMPOPT work for any
private->shared conversion.
And that's what we are doing in v2 patch series, RMPOPT scan getting
triggered on VM destruction.
>
> BTW, I assume that the RMPOPT disable machinery is driven from the
> INVLPGB-like TLB invalidations that are a part of the SNP
> shared=>private conversions. It's a darn shame that RMPOPT wasn't
> broadcast in the same way. It would save the poor OS a lot of work. The
> RMPOPT table is per-cpu of course, but I'm not sure what keeps *a* CPU
> from broadcasting its success finding an SNP-free physical region to
> other CPUs.
The hardware does this broadcast for the RMPUPDATE instruction,
a broadcast will be sent in the RMPUPDATE instruction to clear matching entries
in other RMPOPT tables. This broadcast will be sent to all CPUs.
For the RMPOPT instruction itself, there is no such broadcast, but RMPOPT
instruction needs to be executed on only one thread per core, the
per-CPU RMPOPT table of the other sibling thread will be programmed while
executing the same instruction.
That's the reason, why we had this optimization to executing RMPOPT instruction
on only the primary thread as part of the v1 patch series and i believe we should
include this optimization as part of future series.
>
> tl;dr: I agree with you. The cost of these scans is going to be
> annoying, and it's going to need OS help to optimize it.
Here is some performance data:
Raw CPU cycles for a single RMPOPT instruction, func=0 :
RMPOPT during snp_rmptable_init() while booting:
....
[ 12.098580] SEV-SNP: RMPOPT max. CPU cycles 501460
[ 12.103839] SEV-SNP: RMPOPT min. CPU cycles 60
[ 12.108799] SEV-SNP: RMPOPT average cycles 139790
RMPOPT during SNP_INIT_EX, at CCP module load at boot:
[ 40.206619] SEV-SNP: RMPOPT max. CPU cycles 248083620
[ 40.206629] SEV-SNP: RMPOPT min. CPU cycles 60
[ 40.206629] SEV-SNP: RMPOPT average cycles 249820
RMPOPT after SNP guest shutdown:
...
[ 298.746893] SEV-SNP: RMPOPT max. CPU cycles 248083620
[ 298.746898] SEV-SNP: RMPOPT min. CPU cycles 60
[ 298.746900] SEV-SNP: RMPOPT average cycles 127859
I believe the min. CPU cycles is the case where RMPOPT fails early.
Raw CPU cycles for one complete iteration of executing RMPOPT (func=0) on all CPUs for the whole RAM:
This is for this complete loop with cond_resched() removed.
while (!kthread_should_stop()) {
phys_addr_t pa;
pr_info("RMP optimizations enabled on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
pa_start, pa_end);
start = rdtsc_ordered();
/*
* RMPOPT optimizations skip RMP checks at 1GB granularity if this range of
* memory does not contain any SNP guest memory.
*/
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE) {
/* Bit zero passes the function to the RMPOPT instruction. */
on_each_cpu_mask(cpu_online_mask, rmpopt,
(void *)(pa | RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS),
true);
}
end = rdtsc_ordered();
pr_info("RMPOPT cycles taken for physical address range 0x%016llx - 0x%016llx on all cpus %llu cycles\n",
pa_start, pa_end, end - start);
set_current_state(TASK_INTERRUPTIBLE);
schedule();
}
RMPOPT during snp_rmptable_init() while booting:
...
[ 12.114047] SEV-SNP: RMPOPT cycles taken for physical address range 0x0000000000000000 - 0x0000010380000000 on all cpus 1499496600 cycles
RMPOPT during SNP_INIT_EX:
...
[ 40.206630] SEV-SNP: RMPOPT cycles taken for physical address range 0x0000000000000000 - 0x0000010380000000 on all cpus 686519180 cycles
RMPOPT after SNP guest shutdown:
...
[ 298.746900] SEV-SNP: RMPOPT cycles taken for physical address range 0x0000000000000000 - 0x0000010380000000 on all cpus 369059160 cycles
Thanks,
Ashish
^ permalink raw reply
* Re: [PATCH v4 23/24] x86/virt/tdx: Document TDX Module updates
From: Huang, Kai @ 2026-03-04 23:49 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
Gao, Chao, x86@kernel.org
Cc: corbet@lwn.net, dave.hansen@linux.intel.com,
tony.lindgren@linux.intel.com, binbin.wu@linux.intel.com,
seanjc@google.com, kas@kernel.org, Chatre, Reinette,
Verma, Vishal L, nik.borisov@suse.com, mingo@redhat.com,
Weiny, Ira, hpa@zytor.com, Annapurve, Vishal, sagis@google.com,
Duan, Zhenzhong, Edgecombe, Rick P, paulmck@kernel.org,
tglx@kernel.org, yilun.xu@linux.intel.com, Williams, Dan J,
bp@alien8.de
In-Reply-To: <20260212143606.534586-24-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> Document TDX Module updates 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>
> ---
> Documentation/arch/x86/tdx.rst | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
> index 61670e7df2f7..01ae560c7f66 100644
> --- a/Documentation/arch/x86/tdx.rst
> +++ b/Documentation/arch/x86/tdx.rst
> @@ -99,6 +99,40 @@ initialize::
>
> [..] virt/tdx: module initialization failed ...
>
> +TDX Module Runtime Updates
> +--------------------------
> +
> +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 updates, 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.
During "updates" or "update"? The module only becomes unresponsive during
"one module runtime update", correct?
> +
> +TDX Module updates have 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
> +be not 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/confidential-computing.tdx.tdx-module.binaries
> +
> TDX Interaction to Other Kernel Components
> ------------------------------------------
>
I think I've confused what you mean by "updates" or "update".
Perhaps you mean the "steps" during module update as "updates"?
But to me you indeed said "update" in the changelog:
"
- update is done in stop_machine() context
- update instructions and results
- update policy and tooling
"
Please make this consistent at least.
With this clarified/addressed, feel free to add:
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH v4 19/24] x86/virt/tdx: Update tdx_sysinfo and check features post-update
From: Huang, Kai @ 2026-03-04 23:40 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-20-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> tdx_sysinfo contains all metadata of the active TDX module, including
> versions, supported features, and TDMR/TDCS/TDVPS information.
>
Nit: add "etc", since there are more staff besides the 3 things you listed.
> These
> values may change over updates. Blindly refreshing the entire tdx_sysinfo
> could disrupt running software, as it may subtly rely on the previous state
> unless proven otherwise.
>
> Adopt a conservative approach, like microcode updates, by only refreshing
> version information that does not affect functionality, while ignoring
> all other changes. This is acceptable as new modules are required to
> maintain backward compatibility.
>
> Any updates to metadata beyond versions should be justified and reviewed on
> a case-by-case basis.
>
> Note that preallocating a tdx_sys_info buffer before updates is to avoid
> having to handle -ENOMEM when updating tdx_sysinfo after a successful
> update.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
One bit below ...
[...]
>
> +/*
> + * Update tdx_sysinfo and check if any TDX module features changed after
> + * updates
s/updates/update? I don't see more than one update.
And it's more than "check module features being changed" since there are
other metadata fields which may have different values after update, right?
I would just remove this comment since I don't see it says more than just
repeating the code below (which also has comments saying the same thing, in
a more elaborated way).
> + */
> +int tdx_module_post_update(struct tdx_sys_info *info)
> +{
> + struct tdx_sys_info_version *old, *new;
> + int ret;
> +
> + /* Shouldn't fail as the update has succeeded */
> + ret = get_tdx_sys_info(info);
> + if (WARN_ONCE(ret, "version retrieval failed after update, replace TDX Module\n"))
> + return ret;
> +
> + old = &tdx_sysinfo.version;
> + new = &info->version;
> + pr_info("version %u.%u.%02u -> %u.%u.%02u\n", old->major_version,
> + old->minor_version,
> + old->update_version,
> + new->major_version,
> + new->minor_version,
> + new->update_version);
> +
> + /*
> + * Blindly refreshing the entire tdx_sysinfo could disrupt running
> + * software, as it may subtly rely on the previous state unless
> + * proven otherwise.
> + *
> + * Only refresh version information (including handoff version)
> + * that does not affect functionality, and ignore all other
> + * changes.
> + */
> + tdx_sysinfo.version = info->version;
> + tdx_sysinfo.handoff = info->handoff;
> +
> + if (!memcmp(&tdx_sysinfo, info, sizeof(*info)))
> + return 0;
> +
> + pr_info("TDX module features have changed after updates, but might not take effect.\n");
> + pr_info("Please consider updating your BIOS to install the TDX Module.\n");
> + return 0;
> +}
> +
^ permalink raw reply
* Re: [PATCH v4 18/24] x86/virt/tdx: Restore TDX Module state
From: Huang, Kai @ 2026-03-04 23:24 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-19-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> TDX Module state was packed as handoff data during module shutdown. After
> per-CPU initialization, the new module can restore TDX Module state from
> handoff data to preserve running TDs.
>
> Once the restoration is done, the TDX Module update is complete, which
> means the new module is ready to handle requests from the host and guests.
>
> Implement the new TDH.SYS.UPDATE SEAMCALL to restore TDX Module state
> and invoke it for one CPU.
Nit:
"for one CPU" -> "on one CPU since it only needs to be called once".
>
> Note that Intel® Trust Domain Extensions (Intel® TDX) Module Base
> Architecture Specification, Revision 348549-007, Chapter 4.5.5 states:
>
> If TDH.SYS.UPDATE returns an error, then the host VMM can continue
> with the non-update sequence (TDH.SYS.CONFIG, 15 TDH.SYS.KEY.CONFIG
> etc.). In this case all existing TDs are lost. Alternatively, the host
> VMM can request the P-SEAMLDR to update to another TDX Module. If that
> update is successful, existing TDs are preserved
>
> The two alternative error handling approaches are not implemented due to
> their complexity and unclear benefits.
Nit: use imperative mode:
Don't implement the two alternative ... due to ...
>
> Also note that the location and the format of handoff data is defined by
> the TDX Module. The new module knows where to get handoff data and how
> to parse it. The kernel doesn't need to provide its location, format etc.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH v4 17/24] x86/virt/seamldr: Do TDX per-CPU initialization after updates
From: Huang, Kai @ 2026-03-04 23:18 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-18-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> After installing the new TDX module, each CPU should be initialized
Nit:
"should be" -> "needs to be" ?
> again to make the CPU ready to run any other SEAMCALLs. So, call
> tdx_cpu_enable() on all CPUs.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH v4 16/24] x86/virt/seamldr: Install a new TDX Module
From: Huang, Kai @ 2026-03-04 23:17 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-17-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> Following the shutdown of the existing TDX Module, the update process
> continues with installing the new module. P-SEAMLDR provides the
> SEAMLDR.INSTALL SEAMCALL to perform this installation, which must be
> executed serially across all CPUs.
Nit:
Since you mentioned "serially" here, perhaps just add a sentence to mention
that it is guaranteed by the raw spinlock inside seamldr_call()?
>
> Implement SEAMLDR.INSTALL and execute it on every CPU.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Also a nit below ...
> ---
> arch/x86/virt/vmx/tdx/seamldr.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 4e0a98404c7f..4537311780b1 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -22,6 +22,7 @@
>
> /* P-SEAMLDR SEAMCALL leaf function */
> #define P_SEAMLDR_INFO 0x8000000000000000
> +#define P_SEAMLDR_INSTALL 0x8000000000000001
>
> #define SEAMLDR_MAX_NR_MODULE_4KB_PAGES 496
> #define SEAMLDR_MAX_NR_SIG_4KB_PAGES 4
> @@ -198,6 +199,7 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> enum tdp_state {
> TDP_START,
> TDP_SHUTDOWN,
> + TDP_CPU_INSTALL,
> TDP_DONE,
> };
>
> @@ -232,9 +234,10 @@ static void print_update_failure_message(void)
> * See multi_cpu_stop() from where this multi-cpu state-machine was
> * adopted, and the rationale for touch_nmi_watchdog()
> */
> -static int do_seamldr_install_module(void *params)
> +static int do_seamldr_install_module(void *seamldr_params)
Nit:
IMHO such renaming is just a noise to this patch, since in patch 10/11 it's
clear that the 'params' you passed in is seamldr_params. No?
Perhaps just name it 'seamldr_params' at patch 11?
> {
> enum tdp_state newstate, curstate = TDP_START;
> + struct tdx_module_args args = {};
> int cpu = smp_processor_id();
> bool primary;
> int ret = 0;
> @@ -253,6 +256,10 @@ static int do_seamldr_install_module(void *params)
> if (primary)
> ret = tdx_module_shutdown();
> break;
> + case TDP_CPU_INSTALL:
> + args.rcx = __pa(seamldr_params);
> + ret = seamldr_call(P_SEAMLDR_INSTALL, &args);
> + break;
> default:
> break;
> }
^ permalink raw reply
* Re: [PATCH v4 15/24] x86/virt/seamldr: Log TDX Module update failures
From: Huang, Kai @ 2026-03-04 23:08 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-16-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> 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>
Acked-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH v4 14/24] x86/virt/tdx: Reset software states during TDX Module shutdown
From: Huang, Kai @ 2026-03-04 23:06 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-15-chao.gao@intel.com>
> @@ -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,17 @@ 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;
> +
> + for_each_online_cpu(cpu)
> + per_cpu(tdx_lp_initialized, cpu) = false;
Maybe add a comment like:
/*
* By reaching here CPUHP is disabled and all present CPUs
* are online. It's safe to just loop all online CPUs and
* and reset the per-cpu flag.
*/
And maybe a helper function like reset_tdx_kernel_states() would be nice,
but it's also fine to me as-is:
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH v4 13/24] x86/virt/seamldr: Shut down the current TDX module
From: Huang, Kai @ 2026-03-04 22:59 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-14-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> The first step of TDX Module updates is shutting down the current TDX
> Module. This step also packs state information that needs to be
> preserved across updates as handoff data, which will be consumed by the
> updated module. The handoff data is stored internally in the SEAM range
> and is hidden from the kernel.
>
> To ensure a successful update, the new module must be able to consume
> the handoff data generated by the old module. Since handoff data layout
> may change between modules, the handoff data is versioned. Each module
> has a native handoff version and provides backward support for several
> older versions.
>
> The complete handoff versioning protocol is complex as it supports both
> module upgrades and downgrades. See details in Intel® Trust Domain
> Extensions (Intel® TDX) Module Base Architecture Specification, Revision
> 348549-007, Chapter 4.5.3 "Handoff Versioning".
>
> Ideally, the kernel needs to retrieve the handoff versions supported by
> the current module and the new module and select a version supported by
> both. But, since the Linux kernel only supports module upgrades, simply
Nit:
Again, ".. the Linux kernel only supports module upgrades ..." sounds like
describing the behaviour of the current kernel, but for now runtime update
is not supported yet.
I would change to " .. this implementation chooses to only support module
upgrades".
> request the current module to generate handoff data using its highest
> supported version, expecting that the new module will likely support it.
>
> Note that only one CPU needs to call the TDX Module's shutdown API.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
[...]
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 82bb82be8567..1c4da9540ae0 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -46,6 +46,7 @@
> #define TDH_PHYMEM_PAGE_WBINVD 41
> #define TDH_VP_WR 43
> #define TDH_SYS_CONFIG 45
> +#define TDH_SYS_SHUTDOWN 52
>
> /*
> * SEAMCALL leaf:
> @@ -118,4 +119,6 @@ struct tdmr_info_list {
> int max_tdmrs; /* How many 'tdmr_info's are allocated */
> };
>
> +int tdx_module_shutdown(void);
This (and future patches) makes couple of tdx_xx() functions visible out of
tdx.c. The alternative is to move the main "module update" function out of
seamldr.c to tdx.c, but that would require making couple of seamldr_xx()s
(and data structures probably) visible to tdx.c too.
I don't know which is better, so to make this series move forward:
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH v4 12/24] x86/virt/seamldr: Abort updates if errors occurred midway
From: Huang, Kai @ 2026-03-04 22:38 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-13-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> The TDX Module update process has multiple steps, each of which may
> encounter failures.
>
> The current state machine of updates proceeds to the next step regardless
> of errors. But continuing updates when errors occur midway is pointless.
>
> Abort the update by setting a flag to indicate that a CPU has encountered
> an error, forcing all CPUs to exit the execution loop. Note that failing
> CPUs do not acknowledge the current step. This keeps all other CPUs waiting
> in the current step (since advancing to the next step requires all CPUs to
> acknowledge the current step) until they detect the fault flag and exit the
> loop.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>
Reviewed-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* [Invitation] bi-weekly guest_memfd upstream call on 2026-03-05
From: David Hildenbrand (Arm) @ 2026-03-04 20:11 UTC (permalink / raw)
To: linux-coco@lists.linux.dev, linux-mm@kvack.org, KVM
Cc: ackerleytng, amit, aneeshkumar.kizhakeveetil, ashish.kalra,
dan.j.williams, eberman, fvdl, gshan, jackmanb, jackyli,
jthoughton, kalyazin, kevinloughlin, liruxin, michael.day,
michael.roth, mike.rapoport, mvaralar, pankaj.gupta, papaluri,
patrick.roy, Peter Xu, pheragu, pkondeti, psalian, seanjc,
shan.gavin, shivankg, sidtelang, suzuki.poulose, tabba, tatashin,
vannapurve, vbabka, wyihan
Hi,
Our next guest_memfd upstream call is scheduled for tomorrow, Thursday,
2026-03-05 at 8:00 - 9:00am (GMT-08:00) Pacific Time - Vancouver.
We'll be using the following Google meet:
http://meet.google.com/wxp-wtju-jzw
The meeting notes can be found at [1], where we also link recordings and
collect current guest_memfd upstream proposals. If you want an google
calendar invitation that also covers all future meetings, just write me
or Ackerley a mail.
For some reason, google seems to randomly drop all invitees from the
calendar entry ... so expect some annoying invitation mails until that
seems to stop.
In this meeting, Ackerley will give us an update on guest_memfd HugeTLB
support, which we were not able to squeeze into the last meeting. I'm
sure, other things will pop up around HugeTLB support and
expected/desired timelines.
To put something to discuss onto the agenda, reply to this mail or add
them to the "Topics/questions for next meeting(s)" section in the
meeting notes as a comment.
[1]
https://docs.google.com/document/d/1M6766BzdY1Lhk7LiR5IqVR8B8mG3cr-cxTxOrAosPOk/edit?usp=sharing
[2] https://lore.kernel.org/all/20251203144159.6131-1-itazur@amazon.com/
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: dan.j.williams @ 2026-03-04 17:14 UTC (permalink / raw)
To: Dan Williams, linux-coco, linux-pci
Cc: gregkh, aik, aneesh.kumar, yilun.xu, bhelgaas, alistair23, lukas,
jgg, Arnd Bergmann
In-Reply-To: <20260303000207.1836586-10-dan.j.williams@intel.com>
Dan Williams wrote:
> After pci_tsm_bind() and pci_tsm_lock() the low level TSM driver is
> expected to populate PCI_TSM_EVIDENCE_TYPE_REPORT in its evidence store.
> This report is defined by the TDISP GET_DEVICE_INTERFACE_REPORT response
> payload.
>
> Add a helper to create encrypted MMIO descriptors from that report
> data. With those descriptors the TSM driver can use pci_tsm_mmio_setup() to
> inform ioremap() how to map the device per the device's expectations. The
> VM is expected to validate the interface with the relying party before
> accepting the device for operation.
>
> The helper also provides the obfuscated starting address for each
> encrypted MMIO range as the VM is never disclosed on the hpa that
> correlates to the gpa of the device's mmio. The obfuscated address is BAR
> relative.
>
> Based on an original patch by Aneesh [1]
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Link: https://lore.kernel.org/linux-coco/20251117140007.122062-8-aneesh.kumar@kernel.org/
> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
[..]
> +/**
> + * pci_tsm_mmio_alloc() - allocate encrypted MMIO range descriptor
> + * @pdev: device owner of MMIO ranges
> + * @report_data: TDISP Device Interface (DevIf) Report blob
> + * @report_sz: DevIf Report size
> + *
> + * Return: the encrypted MMIO range descriptor on success, NULL on failure
> + *
> + * Assumes that this is called within the live lifetime of a PCI device's
> + * association with a low level TSM.
> + */
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
> +{
> + struct pci_tsm *tsm = pdev->tsm;
> + struct pci_tsm_evidence *evidence = &tsm->evidence;
> + struct pci_tsm_evidence_object *report_obj = &evidence->obj[PCI_TSM_EVIDENCE_TYPE_REPORT];
> + struct tsm_dev *tsm_dev = tsm->tsm_dev;
> + u64 reporting_bar_base, last_reporting_end;
> + const struct pci_tsm_devif_report *report;
> + u32 mmio_range_count;
> + int last_bar = -1;
> + int i;
> +
> + guard(rwsem_read)(&evidence->lock);
> + if (report_obj->len < sizeof(struct pci_tsm_devif_report))
> + return NULL;
> +
> + if (dev_WARN_ONCE(&tsm_dev->dev, !IS_ALIGNED((unsigned long) report_obj->data, 8),
> + "misaligned report data\n"))
> + return NULL;
Is this going to cause any implementation to need to copy the buffer
received from the low-level TSM? If so I would just mark 'struct
pci_tsm_devif_report' and 'struct pci_tsm_mmio_entry' as __packed and
drop this check.
> +
> + report = report_obj->data;
> + mmio_range_count = __le32_to_cpu(report->mmio_range_count);
> +
> + /* check that the report object is self-consistent on mmio entries */
> + if (report_obj->len < struct_size(report, mmio, mmio_range_count))
> + return NULL;
> +
> + /* create pci_tsm_mmio descriptors from the report data */
> + struct pci_tsm_mmio *mmio __free(kfree) =
> + kzalloc(struct_size(mmio, mmio, mmio_range_count), GFP_KERNEL);
> + if (!mmio)
> + return NULL;
> +
> + for (i = 0; i < mmio_range_count; i++) {
> + u64 range_off;
> + struct range range;
> + const struct pci_tsm_devif_mmio *mmio_data = &report->mmio[i];
> + struct pci_tsm_mmio_entry *entry =
> + pci_tsm_mmio_entry(mmio, mmio->nr);
> + /* report values in are in terms of 4K pages */
> + u64 tsm_offset = __le64_to_cpu(mmio_data->pfn) * SZ_4K;
> + u64 size = __le32_to_cpu(mmio_data->nr_pfns) * SZ_4K;
> + u32 attr = __le32_to_cpu(mmio_data->attributes);
> + int bar = FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID,
> + attr);
> +
> + tsm_offset *= SZ_4K;
> + size *= SZ_4K;
Whoops, these pfn to absolute address conversions were already performed above, will fix.
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Dave Hansen @ 2026-03-04 16:03 UTC (permalink / raw)
To: Andrew Cooper
Cc: Ashish.Kalra, KPrateek.Nayak, Michael.Roth, Nathan.Fontenot,
Tycho.Andersen, aik, ardb, babu.moger, bp, darwi, dave.hansen,
davem, dyoung, herbert, hpa, jackyli, jacobhxu, john.allen, kvm,
linux-coco, linux-crypto, linux-kernel, mingo, nikunj,
pawan.kumar.gupta, pbonzini, peterz, pgonda, rientjes, seanjc,
tglx, thomas.lendacky, x86, xin
In-Reply-To: <4ec520a1-68c7-4833-9e8f-edc610e5fdfa@citrix.com>
On 3/4/26 07:56, Andrew Cooper wrote:
> Logic asking hardware to optimise a 1G region because of no guest memory
> should at least WARN() if hardware comes back and says "well hang on now..."
It would be _nice_ to have a system where we can do a WARN(). But for
something that's just a lowly optimization, I'd rather that RMPOPT lose
the occasional race with a shared=>private conversion than have it take
a lock and _block_ those conversions.
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Andrew Cooper @ 2026-03-04 15:56 UTC (permalink / raw)
To: dave.hansen
Cc: Andrew Cooper, Ashish.Kalra, KPrateek.Nayak, Michael.Roth,
Nathan.Fontenot, Tycho.Andersen, aik, ardb, babu.moger, bp, darwi,
dave.hansen, davem, dyoung, herbert, hpa, jackyli, jacobhxu,
john.allen, kvm, linux-coco, linux-crypto, linux-kernel, mingo,
nikunj, pawan.kumar.gupta, pbonzini, peterz, pgonda, rientjes,
seanjc, tglx, thomas.lendacky, x86, xin
In-Reply-To: <a1701ab4-d80f-496c-bdb3-5d94d2d2f673@intel.com>
>> +/* + * 'val' is a system physical address aligned to 1GB OR'ed with
>> + * a function selection. Currently supported functions are 0 + *
>> (verify and report status) and 1 (report status). + */ +static void
>> rmpopt(void *val) +{ + asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc" +
>> : : "a" ((u64)val & PUD_MASK), "c" ((u64)val & 0x1) + : "memory",
>> "cc"); +}
> Doesn't this belong in:
>
> arch/x86/include/asm/special_insns.h
>
> Also, it's not reporting *any* status here, right? So why even talk
> about it if the kernel isn't doing any status checks? It just makes it
> more confusing.
The "c" (val & 0x1) constraint encodes whether this is a query or a
mutation, but both forms produce an answer via the carry flag.
Because it's void, it's a useless helper, and the overloading via one
parameter makes specifically poor code generation.
It should be:
static inline bool __rmpopt(unsigned long addr, unsigned int fn)
{
bool res;
asm volatile (".byte 0xf2, 0x0f, 0x01, 0xfc"
: "=ccc" (res)
: "a" (addr), "c" (fn));
return res;
}
with:
static inline bool rmpopt_query(unsigned long addr)
static inline bool rmpopt_set(unsigned long addr)
built on top.
Logic asking hardware to optimise a 1G region because of no guest memory
should at least WARN() if hardware comes back and says "well hang on now..."
The memory barrier isn't necessary and hinders the optimiser.
~Andrew
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Dave Hansen @ 2026-03-04 15:32 UTC (permalink / raw)
To: Sean Christopherson, Ashish Kalra
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: <7ab8d3af-b4f5-481c-ab2e-059ddd7e718e@intel.com>
On 3/4/26 07:25, Dave Hansen wrote:
> BTW, I assume that the RMPOPT disable machinery is driven from the
> INVLPGB-like TLB invalidations that are a part of the SNP
> shared=>private conversions. It's a darn shame that RMPOPT wasn't
> broadcast in the same way. It would save the poor OS a lot of work. The
> RMPOPT table is per-cpu of course, but I'm not sure what keeps *a* CPU
> from broadcasting its success finding an SNP-free physical region to
> other CPUs.
I guess the other dirt simple optimization would be to have one CPU to
the RMPOPT scan and then only IPI more CPUs if that first one succeeds.
That wouldn't be awful.
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Dave Hansen @ 2026-03-04 15:25 UTC (permalink / raw)
To: Sean Christopherson, Ashish Kalra
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: <aahH4XARlftClMrQ@google.com>
On 3/4/26 07:01, Sean Christopherson wrote:
> I don't see any performance data in either posted version. Bluntly, this series
> isn't going anywhere without data to guide us. E.g. comments like this from v1
>
> : And there is a cost associated with re-enabling the optimizations for all
> : system RAM (even though it runs as a background kernel thread executing RMPOPT
> : on different 1GB regions in parallel and with inline cond_resched()'s),
> : we don't want to run this periodically.
>
> suggest there is meaningful cost associated with the scan.
Well the RMP is 0.4% of the size of system memory, and I assume that you
need to scan the whole table. There are surely shortcuts for 2M pages,
but with 4k, that's ~8.5GB of RMP table for 2TB of memory. That's an
awful lot of memory traffic for each CPU.
It'll be annoying to keep a refcount per 1GB of paddr space.
One other way to do it would be to loosely mirror the RMPOPT bitmap and
keep our own bitmap of 1GB regions that _need_ RMPOPT run on them. Any
private=>shared conversion sets a bit in the bitmap and schedules some
work out in the future.
It could also be less granular than that. Instead of any private=>shared
conversion, the RMPOPT scan could be triggered on VM destruction which
is much more likely to result in RMPOPT doing anything useful.
BTW, I assume that the RMPOPT disable machinery is driven from the
INVLPGB-like TLB invalidations that are a part of the SNP
shared=>private conversions. It's a darn shame that RMPOPT wasn't
broadcast in the same way. It would save the poor OS a lot of work. The
RMPOPT table is per-cpu of course, but I'm not sure what keeps *a* CPU
from broadcasting its success finding an SNP-free physical region to
other CPUs.
tl;dr: I agree with you. The cost of these scans is going to be
annoying, and it's going to need OS help to optimize it.
^ permalink raw reply
* Re: [PATCH v4 0/4] x86/tdx: Clean up the definitions of TDX TD ATTRIBUTES
From: Nikolay Borisov @ 2026-03-04 15:02 UTC (permalink / raw)
To: Xiaoyao Li, Dave Hansen, Kiryl Shutsemau, Sean Christopherson,
Paolo Bonzini
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
linux-coco, kvm, linux-kernel, x86, Rick Edgecombe, Kai Huang,
binbin.wu, Tony Lindgren
In-Reply-To: <20260303030335.766779-1-xiaoyao.li@intel.com>
On 3.03.26 г. 5:03 ч., Xiaoyao Li wrote:
> The main purpose of this series was to remove redundant macros between
> core TDX and KVM, along with a typo fix. They were implemented as patch 1
> and patch 2.
>
> During the review of v1 and v2, there was encouragement to refine the
> names of the macros related to TD attributes to clarify their scope.
> Thus patch 3 and patch 4 are added.
>
> Note, Binbin suggested to rename tdx_attributes[] to tdx_td_attributes[]
> during v3 review. However, this v4 doesn't do it but leaves it for future
> cleanup to avoid making it more complicated because it also looks like
> it needs to rename "Attributes" to "TD Attributes" in tdx_dump_attributes(),
> which has user visibility change.
>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Sean Christopherson @ 2026-03-04 15:01 UTC (permalink / raw)
To: Ashish Kalra
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: <8dc0198f1261f5ae4b16388fc1ffad5ddb3895f9.1772486459.git.ashish.kalra@amd.com>
On Mon, Mar 02, 2026, Ashish Kalra wrote:
> @@ -500,6 +508,61 @@ static bool __init setup_rmptable(void)
> +/*
> + * 'val' is a system physical address aligned to 1GB OR'ed with
> + * a function selection. Currently supported functions are 0
> + * (verify and report status) and 1 (report status).
> + */
> +static void rmpopt(void *val)
> +{
> + asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> + : : "a" ((u64)val & PUD_MASK), "c" ((u64)val & 0x1)
> + : "memory", "cc");
> +}
> +
> +static int rmpopt_kthread(void *__unused)
> +{
> + phys_addr_t pa_start, pa_end;
> +
> + pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), PUD_SIZE);
> + pa_end = ALIGN(PFN_PHYS(max_pfn), PUD_SIZE);
> +
> + /* Limit memory scanning to the first 2 TB of RAM */
> + pa_end = (pa_end - pa_start) <= SZ_2T ? pa_end : pa_start + SZ_2T;
> +
> + while (!kthread_should_stop()) {
> + phys_addr_t pa;
> +
> + pr_info("RMP optimizations enabled on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
> + pa_start, pa_end);
> +
> + /*
> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this range of
> + * memory does not contain any SNP guest memory.
> + */
> + for (pa = pa_start; pa < pa_end; pa += PUD_SIZE) {
> + /* Bit zero passes the function to the RMPOPT instruction. */
> + on_each_cpu_mask(cpu_online_mask, rmpopt,
> + (void *)(pa | RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS),
> + true);
> +
> + /* Give a chance for other threads to run */
I'm not terribly concerned with other threads, but I am most definitely concerned
about other CPUs. IIUC, *every* time a guest_memfd file is destroyed, the kernel
will process *every* 2MiB chunk of memory, interrupting *every* CPU in the process.
Given that the whole point of RMPOPT is to allow running non-SNP and SNP VMs
side-by-side, inducing potentially significant jitter when stopping SNP VMs seems
like a dealbreaker.
Even using a kthread seems flawed, e.g. if all CPUs in the system are being used
to run VMs, then the kernel could be stealing cycles from an arbitrary VM/vCPU to
process RMPOPT. Contrast that with KVM's NX hugepage recovery thread, which is
spawned in the context of a specific VM so that recovering steady state performance
at the cost of periodically consuming CPU cycles is bound entirely to that VM.
I don't see any performance data in either posted version. Bluntly, this series
isn't going anywhere without data to guide us. E.g. comments like this from v1
: And there is a cost associated with re-enabling the optimizations for all
: system RAM (even though it runs as a background kernel thread executing RMPOPT
: on different 1GB regions in parallel and with inline cond_resched()'s),
: we don't want to run this periodically.
suggest there is meaningful cost associated with the scan.
^ permalink raw reply
* Re: [PATCH kernel 4/9] dma/swiotlb: Stop forcing SWIOTLB for TDISP devices
From: Jason Gunthorpe @ 2026-03-04 12:43 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: dan.j.williams, Robin Murphy, x86, linux-kernel, kvm, linux-pci,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Sean Christopherson, Paolo Bonzini,
Andy Lutomirski, Peter Zijlstra, Bjorn Helgaas, Marek Szyprowski,
Andrew Morton, Catalin Marinas, Michael Ellerman, Mike Rapoport,
Tom Lendacky, Ard Biesheuvel, Neeraj Upadhyay, Ashish Kalra,
Stefano Garzarella, Melody Wang, Seongman Lee, Joerg Roedel,
Nikunj A Dadhania, Michael Roth, Suravee Suthikulpanit,
Andi Kleen, Kuppuswamy Sathyanarayanan, Tony Luck,
David Woodhouse, Greg Kroah-Hartman, Denis Efremov, Geliang Tang,
Piotr Gregor, Michael S. Tsirkin, Alex Williamson, Arnd Bergmann,
Jesse Barnes, Jacob Pan, Yinghai Lu, Kevin Brodsky,
Jonathan Cameron, Aneesh Kumar K.V (Arm), Xu Yilun, Herbert Xu,
Kim Phillips, Konrad Rzeszutek Wilk, Stefano Stabellini,
Claire Chang, linux-coco, iommu
In-Reply-To: <5d669086-a5c8-4e55-8108-a9fff41cf094@amd.com>
On Wed, Mar 04, 2026 at 05:45:31PM +1100, Alexey Kardashevskiy wrote:
> > I suspect AMD needs to use their vTOM feature to allow shared memory
> > to remain available to TDISP RUN with a high/low address split.
>
> I could probably do something about it bit I wonder what is the real
> live use case which requires leaking SME mask, have a live example
> which I could try recreating?
We need shared memory allocated through a DMABUF heap:
https://lore.kernel.org/all/20260223095136.225277-1-jiri@resnulli.us/
To work with all PCI devices in the system, TDISP or not.
Without this the ability for a TDISP device to ingest (encrypted) data
requires all kinds of memcpy..
So the DMA API should see the DMA_ATTR_CC_DECRYPTED and setup the
correct dma_dddr_t either by choosing the shared alias for the TDISP
device's vTOM, or setting the C bit in a vIOMMU S1.
Jason
^ 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