* [PATCH v9 10/23] coco/tdx-host: Implement firmware upload sysfs ABI for TDX module updates
From: Chao Gao @ 2026-05-13 15:09 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
tl;dr: Select fw_upload for doing TDX module updates. The process of
selecting among available update images is complicated and nuanced. Punt
the selection policy out to userspace.
Long Version:
Linux kernel supports two primary firmware update mechanisms:
- request_firmware()
- firmware upload (or fw_upload)
The former is used by microcode updates, SEV firmware updates, etc. The
latter is used by CXL and FPGA firmware updates.
One key difference between them is: request_firmware() loads a named
file from the filesystem where the filename is kernel-controlled, while
fw_upload accepts firmware data directly from userspace.
Use fw_upload for TDX module updates as loading a named file isn't
suitable for TDX (see below for more reasons). Specifically, register
TDX faux device with fw_upload framework to expose sysfs interfaces
and implement operations to process data blobs supplied by userspace.
Why fw_upload instead of request_firmware()?
============================================
The explicit file selection capabilities of fw_upload is preferred over
the implicit file selection of request_firmware() for the following
reasons:
a. Intel distributes all versions of the TDX module, allowing admins to
load any version rather than always defaulting to the latest. This
flexibility is necessary because future extensions may require reverting to
a previous version to clear fatal errors.
b. Some module version series are platform-specific. For example, the 1.5.x
series is for certain platform generations, while the 2.0.x series is
intended for others.
c. The update policy for TDX module updates is non-linear at times. The
latest TDX module may not be compatible. For example, TDX module 1.5.x
may be updated to 1.5.y but not to 1.5.y+1. This policy is documented
separately in a file released along with each TDX module release.
So, the default policy of "request_firmware()" of "always load latest", is
not suitable for TDX. Userspace needs to deploy a more sophisticated policy
check (e.g., latest may not be compatible), and there is potential
operator choice to consider.
Just have userspace pick rather than add kernel mechanism to change the
default policy of request_firmware().
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Link: https://lore.kernel.org/kvm/01fc8946-eb84-46fa-9458-f345dd3f6033@intel.com/
---
Dave also suggested making .poll_complete() optional in fw_upload_ops.
That will be handled in a separate series.
v9:
- add a TL;DR to state the implementation choice up front [Dave]
- s/can_expose_seamldr()/supports_runtime_update()/ [Dave]
---
arch/x86/include/asm/seamldr.h | 1 +
arch/x86/virt/vmx/tdx/seamldr.c | 15 +++++
drivers/virt/coco/tdx-host/Kconfig | 2 +
drivers/virt/coco/tdx-host/tdx-host.c | 87 ++++++++++++++++++++++++++-
4 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
index c67e5bc910a9..ac6f80f7208b 100644
--- a/arch/x86/include/asm/seamldr.h
+++ b/arch/x86/include/asm/seamldr.h
@@ -32,5 +32,6 @@ struct seamldr_info {
static_assert(sizeof(struct seamldr_info) == 256);
int seamldr_get_info(struct seamldr_info *seamldr_info);
+int seamldr_install_module(const u8 *data, u32 size);
#endif /* _ASM_X86_SEAMLDR_H */
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 7269a239bc22..7b345000d7c3 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -6,6 +6,7 @@
*/
#define pr_fmt(fmt) "seamldr: " fmt
+#include <linux/mm.h>
#include <linux/spinlock.h>
#include <asm/seamldr.h>
@@ -41,3 +42,17 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
return seamldr_call(P_SEAMLDR_INFO, &args);
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
+
+/**
+ * seamldr_install_module - Install a new TDX module.
+ * @data: Pointer to the TDX module image.
+ * @size: Size of the TDX module image.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int seamldr_install_module(const u8 *data, u32 size)
+{
+ /* TODO: Update TDX module here */
+ return 0;
+}
+EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
diff --git a/drivers/virt/coco/tdx-host/Kconfig b/drivers/virt/coco/tdx-host/Kconfig
index d35d85ef91c0..ca600a39d97b 100644
--- a/drivers/virt/coco/tdx-host/Kconfig
+++ b/drivers/virt/coco/tdx-host/Kconfig
@@ -1,6 +1,8 @@
config TDX_HOST_SERVICES
tristate "TDX Host Services Driver"
depends on INTEL_TDX_HOST
+ select FW_LOADER
+ select FW_UPLOAD
default m
help
Enable access to TDX host services like module update and
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index a540d658757b..c4c099cf3de1 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -6,6 +6,7 @@
*/
#include <linux/device/faux.h>
+#include <linux/firmware.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/sysfs.h>
@@ -84,7 +85,7 @@ static struct attribute *seamldr_attrs[] = {
NULL,
};
-static umode_t seamldr_group_visible(struct kobject *kobj, struct attribute *attr, int idx)
+static bool supports_runtime_update(void)
{
const struct tdx_sys_info *sysinfo = tdx_get_sysinfo();
@@ -99,7 +100,12 @@ static umode_t seamldr_group_visible(struct kobject *kobj, struct attribute *att
if (boot_cpu_has_bug(X86_BUG_SEAMRET_INVD_VMCS))
return 0;
- return tdx_supports_runtime_update(sysinfo) ? attr->mode : 0;
+ return tdx_supports_runtime_update(sysinfo);
+}
+
+static umode_t seamldr_group_visible(struct kobject *kobj, struct attribute *attr, int idx)
+{
+ return supports_runtime_update() ? attr->mode : 0;
}
static const struct attribute_group seamldr_group = {
@@ -113,6 +119,81 @@ static const struct attribute_group *tdx_host_groups[] = {
NULL,
};
+static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
+ const u8 *data, u32 size)
+{
+ return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
+ u32 offset, u32 size, u32 *written)
+{
+ int ret;
+
+ ret = seamldr_install_module(data, size);
+ switch (ret) {
+ case 0:
+ *written = size;
+ return FW_UPLOAD_ERR_NONE;
+ default:
+ return FW_UPLOAD_ERR_FW_INVALID;
+ }
+}
+
+static enum fw_upload_err tdx_fw_poll_complete(struct fw_upload *fwl)
+{
+ /*
+ * The upload completed during tdx_fw_write().
+ * Never poll for completion.
+ */
+ return FW_UPLOAD_ERR_NONE;
+}
+
+
+static void tdx_fw_cancel(struct fw_upload *fwl)
+{
+ /*
+ * TDX module updates are not cancellable.
+ * Provide a no-op callback to satisfy fw_upload_ops.
+ */
+}
+
+static const struct fw_upload_ops tdx_fw_ops = {
+ .prepare = tdx_fw_prepare,
+ .write = tdx_fw_write,
+ .poll_complete = tdx_fw_poll_complete,
+ .cancel = tdx_fw_cancel,
+};
+
+static void seamldr_deinit(void *tdx_fwl)
+{
+ firmware_upload_unregister(tdx_fwl);
+}
+
+static int seamldr_init(struct device *dev)
+{
+ struct fw_upload *tdx_fwl;
+
+ if (!supports_runtime_update())
+ return 0;
+
+ tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "tdx_module",
+ &tdx_fw_ops, NULL);
+ if (IS_ERR(tdx_fwl))
+ return PTR_ERR(tdx_fwl);
+
+ return devm_add_action_or_reset(dev, seamldr_deinit, tdx_fwl);
+}
+
+static int tdx_host_probe(struct faux_device *fdev)
+{
+ return seamldr_init(&fdev->dev);
+}
+
+static const struct faux_device_ops tdx_host_ops = {
+ .probe = tdx_host_probe,
+};
+
static struct faux_device *fdev;
static int __init tdx_host_init(void)
@@ -120,7 +201,7 @@ static int __init tdx_host_init(void)
if (!x86_match_cpu(tdx_host_ids) || !tdx_get_sysinfo())
return -ENODEV;
- fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, NULL, tdx_host_groups);
+ fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, &tdx_host_ops, tdx_host_groups);
if (!fdev)
return -ENODEV;
--
2.52.0
^ permalink raw reply related
* [PATCH v9 11/23] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2026-05-13 15:09 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
There are two important ABIs here:
'struct tdx_image' - The on-disk and in-memory format for a TDX
module update image.
'struct seamldr_params' - The in-memory ABI passed to the TDX module
loader. Points to a single 'struct tdx_image'
Userspace supplies the update image in struct tdx_image format. The image
consists of a header followed by a sigstruct and the module binary.
P-SEAMLDR, however, consumes struct seamldr_params rather than the image
directly. Parse the struct tdx_image provided by userspace and populate a
matching struct seamldr_params.
Validate the struct tdx_image header before using it, because the header is
consumed solely by the kernel to locate the sigstruct and module within
the image. Do not validate the payload itself. The sigstruct and module
pages are passed through to P-SEAMLDR, which validates them as part of the
update flow.
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
sigstruct_pages_pa_list currently has only one entry, but it will grow to
four pages in the future. Keep it as an array for symmetry with
module_pages_pa_list and for extensibility.
v9:
- define a new format, basically don't use offset_of_module but use
sigstruct/module_nr_pages if the offset should be paged-aligned. this
saves alignment checks and bound checks.
- add a tdx_image_header struct to avoid using sizeof() for a struct with
variable array.
- rewrite the changelog to call out that this patch is to convert an ABI from
what userspace provides to an ABI the P-SEAMLDR consums.
- minimize casts and shifts and weird math
---
arch/x86/virt/vmx/tdx/seamldr.c | 126 +++++++++++++++++++++++++++++++-
1 file changed, 125 insertions(+), 1 deletion(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 7b345000d7c3..929203ec96f2 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt) "seamldr: " fmt
#include <linux/mm.h>
+#include <linux/slab.h>
#include <linux/spinlock.h>
#include <asm/seamldr.h>
@@ -16,6 +17,33 @@
/* P-SEAMLDR SEAMCALL leaf function */
#define P_SEAMLDR_INFO 0x8000000000000000
+#define SEAMLDR_MAX_NR_MODULE_PAGES 496
+#define SEAMLDR_MAX_NR_SIG_PAGES 1
+
+/*
+ * The seamldr_params "scenario" field specifies the operation mode:
+ * 0: Install TDX module from scratch (not used by kernel)
+ * 1: Update existing TDX module to a compatible version
+ */
+#define SEAMLDR_SCENARIO_UPDATE 1
+
+/*
+ * This is called the "SEAMLDR_PARAMS" data structure and is defined
+ * in "SEAM Loader (SEAMLDR) Interface Specification".
+ *
+ * It describes the TDX module that will be installed.
+ */
+struct seamldr_params {
+ u32 version;
+ u32 scenario;
+ u64 sigstruct_pages_pa_list[SEAMLDR_MAX_NR_SIG_PAGES];
+ u8 reserved[104];
+ u64 module_nr_pages;
+ u64 module_pages_pa_list[SEAMLDR_MAX_NR_MODULE_PAGES];
+} __packed;
+
+static_assert(sizeof(struct seamldr_params) == 4096);
+
/*
* Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
* interact with P-SEAMLDR simultaneously. Use raw version as the calls can
@@ -43,6 +71,89 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
+#define TDX_IMAGE_VERSION_2 0x200
+
+struct tdx_image_header {
+ u16 version; // This ABI is always 0x200
+ u16 checksum;
+ u8 signature[8];
+ u32 sigstruct_nr_pages;
+ u32 module_nr_pages;
+ u8 reserved[4076];
+} __packed;
+
+#define HEADER_SIZE sizeof(struct tdx_image_header)
+static_assert(HEADER_SIZE == 4096);
+
+/* Intel TDX module update ABI structure. aka. "TDX module blob". */
+struct tdx_image {
+ struct tdx_image_header header;
+ u8 payload[]; // Contains sigstruct pages followed by module pages
+};
+
+static void populate_pa_list(u64 *pa_list, u32 max_entries, const u8 *start, u32 nr_pages)
+{
+ int i;
+
+ nr_pages = MIN(nr_pages, max_entries);
+ for (i = 0; i < nr_pages; i++) {
+ pa_list[i] = vmalloc_to_pfn(start) << PAGE_SHIFT;
+ start += PAGE_SIZE;
+ }
+}
+
+static void populate_seamldr_params(struct seamldr_params *params,
+ const u8 *sig, u32 sig_nr_pages,
+ const u8 *mod, u32 mod_nr_pages)
+{
+ params->version = 0;
+ params->scenario = SEAMLDR_SCENARIO_UPDATE;
+ params->module_nr_pages = mod_nr_pages;
+
+ populate_pa_list(params->sigstruct_pages_pa_list, SEAMLDR_MAX_NR_SIG_PAGES,
+ sig, sig_nr_pages);
+ populate_pa_list(params->module_pages_pa_list, SEAMLDR_MAX_NR_MODULE_PAGES,
+ mod, mod_nr_pages);
+}
+
+static int init_seamldr_params(struct seamldr_params *params, const u8 *data, u32 size)
+{
+ const struct tdx_image *image = (const void *)data;
+ const struct tdx_image_header *header = &image->header;
+
+ u32 sigstruct_len = header->sigstruct_nr_pages * PAGE_SIZE;
+ u32 module_len = header->module_nr_pages * PAGE_SIZE;
+
+ u8 *header_start = (u8 *)header;
+ u8 *header_end = header_start + HEADER_SIZE;
+
+ u8 *sigstruct_start = header_end;
+ u8 *sigstruct_end = sigstruct_start + sigstruct_len;
+
+ u8 *module_start = sigstruct_end;
+
+ /* Check the calculated payload size against the data size. */
+ if (HEADER_SIZE + sigstruct_len + module_len != size)
+ return -EINVAL;
+
+ /*
+ * Don't care about user passing the wrong file, but protect
+ * kernel ABI by preventing accepting garbage.
+ */
+ if (header->version != TDX_IMAGE_VERSION_2)
+ return -EINVAL;
+
+ if (memcmp(header->signature, "TDX-BLOB", sizeof(header->signature)))
+ return -EINVAL;
+
+ if (memchr_inv(header->reserved, 0, sizeof(header->reserved)))
+ return -EINVAL;
+
+ populate_seamldr_params(params, sigstruct_start, header->sigstruct_nr_pages,
+ module_start, header->module_nr_pages);
+ return 0;
+}
+
/**
* seamldr_install_module - Install a new TDX module.
* @data: Pointer to the TDX module image.
@@ -52,7 +163,20 @@ EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
*/
int seamldr_install_module(const u8 *data, u32 size)
{
+ struct seamldr_params *params;
+ int ret;
+
+ params = kzalloc_obj(*params);
+ if (!params)
+ return -ENOMEM;
+
+ ret = init_seamldr_params(params, data, size);
+ if (ret)
+ goto out;
+
/* TODO: Update TDX module here */
- return 0;
+out:
+ kfree(params);
+ return ret;
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
--
2.52.0
^ permalink raw reply related
* [PATCH v9 12/23] x86/virt/seamldr: Introduce skeleton for TDX module updates
From: Chao Gao @ 2026-05-13 15:09 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
TDX module updates require careful synchronization with other TDX
operations. The requirements are (#1/#2 reflect current behavior that
must be preserved):
1. SEAMCALLs need to be callable from both process and IRQ contexts.
2. SEAMCALLs need to be able to run concurrently across CPUs
3. During updates, only update-related SEAMCALLs are permitted; all
other SEAMCALLs shouldn't be called.
4. During updates, all online CPUs must participate in the update work.
No single lock primitive satisfies all requirements. For instance,
rwlock_t handles #1/#2 but fails #4: CPUs spinning with IRQs disabled
cannot be directed to perform update work.
Use stop_machine() as it is the only well-understood mechanism that can
meet all requirements.
And TDX module updates consist of several steps (See Intel® Trust Domain
Extensions (Intel® TDX) Module Base Architecture Specification, Chapter
"TD-Preserving TDX module Update"). Ordering requirements between steps
mandate lockstep synchronization across all CPUs.
multi_cpu_stop() provides a good example of executing a multi-step task
in lockstep across CPUs, but it does not synchronize the individual
steps inside the callback itself.
Implement a similar state machine as the skeleton for TDX module
updates. Each state represents one step in the update flow, and the
state advances only after all CPUs acknowledge completion of the current
step. This acknowledgment mechanism provides the required lockstep
execution.
The update flow is intentionally simpler than multi_cpu_stop() in two ways:
a) use a spinlock to protect the control data instead of atomic_t and
explicit memory barriers.
b) omit touch_nmi_watchdog() and rcu_momentary_eqs(), which exist
there for debugging and are not strictly needed for this update flow
Potential alternative to stop_machine()
=======================================
An alternative approach is to lock all KVM entry points and kick all
vCPUs. Here, KVM entry points refer to KVM VM/vCPU ioctl entry points,
implemented in KVM common code (virt/kvm). Adding a locking mechanism
there would affect all architectures KVM supports. And to lock only TDX
vCPUs, new logic would be needed to identify TDX vCPUs, which the KVM
common code currently lacks. This would add significant complexity and
maintenance overhead to KVM for this TDX-specific use case, so don't take
this approach.
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>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v9:
- Extract control-data initialization into a separate helper. [Dave]
- Drop touch_nmi_watchdog() and rcu_momentary_eqs(), as they are not
needed here.
- Rename thread_ack to num_ack to make it clear that it counts the
number of acknowledgments.
- Rename set_target_state() to __set_target_state() to mark it as an
internal helper. Add a comment noting that __set_target_state()
does not take the lock, unlike ack_state().
- Update the changelog to explain why a spinlock is used instead of
atomic_t plus memory barriers to protect the control data.
---
arch/x86/virt/vmx/tdx/seamldr.c | 87 ++++++++++++++++++++++++++++++++-
1 file changed, 86 insertions(+), 1 deletion(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 929203ec96f2..7befe4a08f33 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -7,8 +7,10 @@
#define pr_fmt(fmt) "seamldr: " fmt
#include <linux/mm.h>
+#include <linux/nmi.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
+#include <linux/stop_machine.h>
#include <asm/seamldr.h>
@@ -154,6 +156,84 @@ static int init_seamldr_params(struct seamldr_params *params, const u8 *data, u3
return 0;
}
+/*
+ * During a TDX module update, all CPUs start from MODULE_UPDATE_START and
+ * progress to MODULE_UPDATE_DONE. Each state is associated with certain
+ * work. For some states, just one CPU needs to perform the work, while
+ * other CPUs just wait during those states.
+ */
+enum module_update_state {
+ MODULE_UPDATE_START,
+ MODULE_UPDATE_DONE,
+};
+
+static struct update_ctrl {
+ enum module_update_state state;
+ int num_ack;
+ /*
+ * Protect update_ctrl. Raw spinlock as it will be acquired from
+ * interrupt-disabled contexts.
+ */
+ raw_spinlock_t lock;
+} update_ctrl;
+
+/* Called with ctrl->lock held or during initialization. */
+static void __set_target_state(struct update_ctrl *ctrl,
+ enum module_update_state newstate)
+{
+ /* Reset ack counter. */
+ ctrl->num_ack = 0;
+ ctrl->state = newstate;
+}
+
+/* Last one to ack a state moves to the next state. */
+static void ack_state(struct update_ctrl *ctrl)
+{
+ raw_spin_lock(&ctrl->lock);
+
+ ctrl->num_ack++;
+ if (ctrl->num_ack == num_online_cpus())
+ __set_target_state(ctrl, ctrl->state + 1);
+
+ raw_spin_unlock(&ctrl->lock);
+}
+
+static void init_state(struct update_ctrl *ctrl)
+{
+ raw_spin_lock_init(&ctrl->lock);
+ __set_target_state(ctrl, MODULE_UPDATE_START + 1);
+}
+
+/*
+ * See multi_cpu_stop() from where this multi-cpu state-machine was
+ * adopted.
+ */
+static int do_seamldr_install_module(void *seamldr_params)
+{
+ enum module_update_state newstate, curstate = MODULE_UPDATE_START;
+ int ret = 0;
+
+ do {
+ newstate = READ_ONCE(update_ctrl.state);
+
+ if (curstate == newstate) {
+ cpu_relax();
+ continue;
+ }
+
+ curstate = newstate;
+ switch (curstate) {
+ /* TODO: add the update steps. */
+ default:
+ break;
+ }
+
+ ack_state(&update_ctrl);
+ } while (curstate != MODULE_UPDATE_DONE);
+
+ return ret;
+}
+
/**
* seamldr_install_module - Install a new TDX module.
* @data: Pointer to the TDX module image.
@@ -174,7 +254,12 @@ int seamldr_install_module(const u8 *data, u32 size)
if (ret)
goto out;
- /* TODO: Update TDX module here */
+ /* Ensure a stable set of online CPUs for the update process. */
+ cpus_read_lock();
+ init_state(&update_ctrl);
+ ret = stop_machine_cpuslocked(do_seamldr_install_module, params, cpu_online_mask);
+ cpus_read_unlock();
+
out:
kfree(params);
return ret;
--
2.52.0
^ permalink raw reply related
* [PATCH v9 13/23] x86/virt/seamldr: Abort updates after a failed step
From: Chao Gao @ 2026-05-13 15:09 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
A TDX module update is a multi-step process, and any step can fail.
The current update flow continues to later steps after an error.
Continuing after a failure can leave the TDX module in an unrecoverable
state.
One failure case must remain recoverable: update contention with an ongoing
TD build. The agreed kernel behavior for this case [1] is to fail the
update with -EBUSY so userspace can retry later.
Abort the update on any failure. This also makes the TD-build contention
case recoverable, because that failure occurs before any TDX module state
is changed. Apply the same rule to all errors instead of special-casing
-EBUSY.
Track per-step failures, stop the update loop once a failure is observed,
and do not advance the state machine to the next step.
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>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Link: https://lore.kernel.org/linux-coco/aQFmOZCdw64z14cJ@google.com/ # [1]
---
v9:
- Avoid nested if/else by deferring failure accounting to ack_state().
- Reduce indentation of the main flow.
- Convert the failed flag into a counter. This avoids a conditional
update of the flag; the counter can simply accumulate failures.
---
arch/x86/virt/vmx/tdx/seamldr.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 7befe4a08f33..48fe71319fea 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -170,6 +170,7 @@ enum module_update_state {
static struct update_ctrl {
enum module_update_state state;
int num_ack;
+ int num_failed;
/*
* Protect update_ctrl. Raw spinlock as it will be acquired from
* interrupt-disabled contexts.
@@ -187,12 +188,13 @@ static void __set_target_state(struct update_ctrl *ctrl,
}
/* Last one to ack a state moves to the next state. */
-static void ack_state(struct update_ctrl *ctrl)
+static void ack_state(struct update_ctrl *ctrl, int result)
{
raw_spin_lock(&ctrl->lock);
+ ctrl->num_failed += !!result;
ctrl->num_ack++;
- if (ctrl->num_ack == num_online_cpus())
+ if (ctrl->num_ack == num_online_cpus() && !ctrl->num_failed)
__set_target_state(ctrl, ctrl->state + 1);
raw_spin_unlock(&ctrl->lock);
@@ -202,6 +204,7 @@ static void init_state(struct update_ctrl *ctrl)
{
raw_spin_lock_init(&ctrl->lock);
__set_target_state(ctrl, MODULE_UPDATE_START + 1);
+ ctrl->num_failed = 0;
}
/*
@@ -228,8 +231,8 @@ static int do_seamldr_install_module(void *seamldr_params)
break;
}
- ack_state(&update_ctrl);
- } while (curstate != MODULE_UPDATE_DONE);
+ ack_state(&update_ctrl, ret);
+ } while (curstate != MODULE_UPDATE_DONE && !READ_ONCE(update_ctrl.num_failed));
return ret;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v9 14/23] x86/virt/seamldr: Shut down the current TDX module
From: Chao Gao @ 2026-05-13 15:09 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
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, Chapter
"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 this implementation chooses to only support 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.
Retrieve the module's handoff version from TDX global metadata and add an
update step to shut down the module. Module shutdown has global effect, so
it only needs to run on one CPU.
Note that the handoff information isn't cached in tdx_sysinfo. It is used
only for module shutdown, and is present only when the TDX module supports
updates. Caching it in get_tdx_sys_info() would require extra update-support
guards and refreshing the cached value across module updates.
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>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
---
v9:
- Use CPU0 as the primary CPU
---
arch/x86/include/asm/tdx_global_metadata.h | 4 ++++
arch/x86/virt/vmx/tdx/seamldr.c | 15 ++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.c | 19 ++++++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.h | 3 +++
arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 13 +++++++++++++
5 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index 40689c8dc67e..41150d546589 100644
--- a/arch/x86/include/asm/tdx_global_metadata.h
+++ b/arch/x86/include/asm/tdx_global_metadata.h
@@ -40,6 +40,10 @@ struct tdx_sys_info_td_conf {
u64 cpuid_config_values[128][2];
};
+struct tdx_sys_info_handoff {
+ u16 module_hv;
+};
+
struct tdx_sys_info {
struct tdx_sys_info_version version;
struct tdx_sys_info_features features;
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 48fe71319fea..6114cab46196 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -15,6 +15,7 @@
#include <asm/seamldr.h>
#include "seamcall_internal.h"
+#include "tdx.h"
/* P-SEAMLDR SEAMCALL leaf function */
#define P_SEAMLDR_INFO 0x8000000000000000
@@ -164,6 +165,7 @@ static int init_seamldr_params(struct seamldr_params *params, const u8 *data, u3
*/
enum module_update_state {
MODULE_UPDATE_START,
+ MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_DONE,
};
@@ -214,8 +216,16 @@ static void init_state(struct update_ctrl *ctrl)
static int do_seamldr_install_module(void *seamldr_params)
{
enum module_update_state newstate, curstate = MODULE_UPDATE_START;
+ int cpu = smp_processor_id();
+ bool primary;
int ret = 0;
+ /*
+ * Use CPU 0 to execute update steps that must run exactly once.
+ * Note CPU 0 is always online.
+ */
+ primary = cpu == 0;
+
do {
newstate = READ_ONCE(update_ctrl.state);
@@ -226,7 +236,10 @@ static int do_seamldr_install_module(void *seamldr_params)
curstate = newstate;
switch (curstate) {
- /* TODO: add the update steps. */
+ case MODULE_UPDATE_SHUTDOWN:
+ if (primary)
+ ret = tdx_module_shutdown();
+ break;
default:
break;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 1621695d7561..da3c1e857b26 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -321,7 +321,7 @@ static __init int build_tdx_memlist(struct list_head *tmb_list)
return ret;
}
-static __init int read_sys_metadata_field(u64 field_id, u64 *data)
+static int read_sys_metadata_field(u64 field_id, u64 *data)
{
struct tdx_module_args args = {};
int ret;
@@ -1267,6 +1267,23 @@ static __init int tdx_enable(void)
}
subsys_initcall(tdx_enable);
+int tdx_module_shutdown(void)
+{
+ struct tdx_sys_info_handoff handoff = {};
+ struct tdx_module_args args = {};
+ int ret;
+
+ ret = get_tdx_sys_info_handoff(&handoff);
+ WARN_ON_ONCE(ret);
+
+ /*
+ * Use the module's handoff version as it is the highest the
+ * module can produce and most likely supported by newer modules.
+ */
+ args.rcx = handoff.module_hv;
+ return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
+}
+
static bool is_pamt_page(unsigned long phys)
{
struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 76c5fb1e1ffe..f0c20dea0388 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
#define TDH_SYS_DISABLE 69
/*
@@ -108,4 +109,6 @@ struct tdmr_info_list {
int max_tdmrs; /* How many 'tdmr_info's are allocated */
};
+int tdx_module_shutdown(void);
+
#endif
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index d54d4227990c..e793dec688ab 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -100,6 +100,19 @@ static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_
return ret;
}
+static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
+{
+ int ret;
+ u64 val;
+
+ ret = read_sys_metadata_field(0x8900000100000000, &val);
+ if (ret)
+ return ret;
+
+ sysinfo_handoff->module_hv = val;
+ return 0;
+}
+
static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
{
int ret = 0;
--
2.52.0
^ permalink raw reply related
* [PATCH v9 15/23] x86/virt/tdx: Reset software states during TDX module shutdown
From: Chao Gao @ 2026-05-13 15:09 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
The TDX module requires a one-time global initialization (TDH.SYS.INIT) and
per-CPU initialization (TDH.SYS.LP.INIT) before use. These initializations
are guarded by software flags to prevent repetition.
After TDX module updates, the new TDX module requires the same global and
per-CPU initializations, but the existing software flags prevent
re-initialization.
Reset all software flags guarding the initialization flows to allow the
global and per-CPU initializations to be triggered again after updates.
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>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v9:
- use a global structure for TDX global state and use memset to
zero the whole structure [Dave]
---
arch/x86/virt/vmx/tdx/tdx.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index da3c1e857b26..20b3b33e4677 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1271,7 +1271,7 @@ int tdx_module_shutdown(void)
{
struct tdx_sys_info_handoff handoff = {};
struct tdx_module_args args = {};
- int ret;
+ int ret, cpu;
ret = get_tdx_sys_info_handoff(&handoff);
WARN_ON_ONCE(ret);
@@ -1281,7 +1281,21 @@ int tdx_module_shutdown(void)
* module can produce and most likely supported by newer modules.
*/
args.rcx = handoff.module_hv;
- return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
+ ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
+ if (ret)
+ return ret;
+
+ /*
+ * Clear global and per-CPU initialization flags so the new module
+ * can be fully re-initialized after a successful update.
+ *
+ * No locks needed as no concurrent accesses can occur here.
+ */
+ memset(&tdx_module_state, 0, sizeof(tdx_module_state));
+ for_each_possible_cpu(cpu)
+ per_cpu(tdx_lp_initialized, cpu) = false;
+
+ return 0;
}
static bool is_pamt_page(unsigned long phys)
--
2.52.0
^ permalink raw reply related
* [PATCH v9 17/23] x86/virt/seamldr: Do TDX per-CPU initialization after module installation
From: Chao Gao @ 2026-05-13 15:10 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
After installing the new TDX module, each CPU needs to be initialized
again to make the CPU ready to run any other SEAMCALLs. So, export and
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>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/tdx.h | 1 +
arch/x86/virt/vmx/tdx/seamldr.c | 4 ++++
arch/x86/virt/vmx/tdx/tdx.c | 2 +-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 27376db7ddac..5d750fe53669 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -107,6 +107,7 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
#ifdef CONFIG_INTEL_TDX_HOST
void tdx_init(void);
+int tdx_cpu_enable(void);
const char *tdx_dump_mce_info(struct mce *m);
const struct tdx_sys_info *tdx_get_sysinfo(void);
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 9d0e7e8c6c20..e4a3271051f6 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -177,6 +177,7 @@ enum module_update_state {
MODULE_UPDATE_START,
MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_CPU_INSTALL,
+ MODULE_UPDATE_CPU_INIT,
MODULE_UPDATE_DONE,
};
@@ -254,6 +255,9 @@ static int do_seamldr_install_module(void *seamldr_params)
case MODULE_UPDATE_CPU_INSTALL:
ret = seamldr_install(seamldr_params);
break;
+ case MODULE_UPDATE_CPU_INIT:
+ ret = tdx_cpu_enable();
+ break;
default:
break;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 20b3b33e4677..5e54da302f2d 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -106,7 +106,7 @@ static int try_init_module_global(void)
* (and TDX module global initialization SEAMCALL if not done) on local cpu to
* make this cpu be ready to run any other SEAMCALLs.
*/
-static int tdx_cpu_enable(void)
+int tdx_cpu_enable(void)
{
struct tdx_module_args args = {};
int ret;
--
2.52.0
^ permalink raw reply related
* [PATCH v9 16/23] x86/virt/seamldr: Install a new TDX module
From: Chao Gao @ 2026-05-13 15:09 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
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 on all CPUs.
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>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v9:
- Add a comment above seamldr_install()
---
arch/x86/virt/vmx/tdx/seamldr.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 6114cab46196..9d0e7e8c6c20 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -19,6 +19,7 @@
/* P-SEAMLDR SEAMCALL leaf function */
#define P_SEAMLDR_INFO 0x8000000000000000
+#define P_SEAMLDR_INSTALL 0x8000000000000001
#define SEAMLDR_MAX_NR_MODULE_PAGES 496
#define SEAMLDR_MAX_NR_SIG_PAGES 1
@@ -74,6 +75,15 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
+/* Call into P-SEAMLDR to install a TDX module update */
+static int seamldr_install(const struct seamldr_params *params)
+{
+ struct tdx_module_args args = {};
+
+ args.rcx = __pa(params);
+ return seamldr_call(P_SEAMLDR_INSTALL, &args);
+}
+
#define TDX_IMAGE_VERSION_2 0x200
struct tdx_image_header {
@@ -166,6 +176,7 @@ static int init_seamldr_params(struct seamldr_params *params, const u8 *data, u3
enum module_update_state {
MODULE_UPDATE_START,
MODULE_UPDATE_SHUTDOWN,
+ MODULE_UPDATE_CPU_INSTALL,
MODULE_UPDATE_DONE,
};
@@ -240,6 +251,9 @@ static int do_seamldr_install_module(void *seamldr_params)
if (primary)
ret = tdx_module_shutdown();
break;
+ case MODULE_UPDATE_CPU_INSTALL:
+ ret = seamldr_install(seamldr_params);
+ break;
default:
break;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v9 18/23] x86/virt/tdx: Restore TDX module state
From: Chao Gao @ 2026-05-13 15:10 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
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 on one CPU since it only needs to be called once.
For error handling, Intel® Trust Domain Extensions (Intel® TDX)
Module Base Architecture Specification, Chapter "Restore TDX Module
State after a TD-Preserving Update" states
If TDH.SYS.UPDATE returns an error, then the host VMM can continue
with the non-update sequence (TDH.SYS.CONFIG, 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.
Given the complexity and uncertain value of above recovery paths, simply
propagate errors.
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>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/virt/vmx/tdx/seamldr.c | 5 +++++
arch/x86/virt/vmx/tdx/tdx.c | 13 +++++++++++++
arch/x86/virt/vmx/tdx/tdx.h | 2 ++
3 files changed, 20 insertions(+)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index e4a3271051f6..6a39c9e3ef7d 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -178,6 +178,7 @@ enum module_update_state {
MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_CPU_INSTALL,
MODULE_UPDATE_CPU_INIT,
+ MODULE_UPDATE_RUN_UPDATE,
MODULE_UPDATE_DONE,
};
@@ -258,6 +259,10 @@ static int do_seamldr_install_module(void *seamldr_params)
case MODULE_UPDATE_CPU_INIT:
ret = tdx_cpu_enable();
break;
+ case MODULE_UPDATE_RUN_UPDATE:
+ if (primary)
+ ret = tdx_module_run_update();
+ break;
default:
break;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 5e54da302f2d..7eb1b67af656 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1298,6 +1298,19 @@ int tdx_module_shutdown(void)
return 0;
}
+int tdx_module_run_update(void)
+{
+ struct tdx_module_args args = {};
+ int ret;
+
+ ret = seamcall_prerr(TDH_SYS_UPDATE, &args);
+ if (ret)
+ return ret;
+
+ tdx_module_state.initialized = true;
+ return 0;
+}
+
static bool is_pamt_page(unsigned long phys)
{
struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index f0c20dea0388..bdfd0e1e337a 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -47,6 +47,7 @@
#define TDH_VP_WR 43
#define TDH_SYS_CONFIG 45
#define TDH_SYS_SHUTDOWN 52
+#define TDH_SYS_UPDATE 53
#define TDH_SYS_DISABLE 69
/*
@@ -110,5 +111,6 @@ struct tdmr_info_list {
};
int tdx_module_shutdown(void);
+int tdx_module_run_update(void);
#endif
--
2.52.0
^ permalink raw reply related
* [PATCH v9 19/23] x86/virt/tdx: Refresh TDX module version after update
From: Chao Gao @ 2026-05-13 15:10 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
The kernel exposes the TDX module version through sysfs so userspace can
check update compatibility. That information needs to remain accurate
across runtime updates.
A runtime update may change the module's update_version, so refresh the
cached version right after a successful update.
Drop __ro_after_init from tdx_sysinfo because it is now updated at runtime.
Do not refresh the rest of tdx_sysinfo, even if some values change across
updates. TDX module updates are backward compatible, so existing
tdx_sysinfo consumers, e.g. KVM, can continue to operate without seeing the
new values.
Refreshing the full structure would be risky. A tdx_sysinfo consumer may
initialize its TDX support based on the features originally reported in
tdx_sysinfo. If a runtime update adds new features and the full structure
is refreshed, that consumer could observe and use the newly reported
features without having performed the setup required to use them safely.
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
v9:
- don't print old and new version [Dave]
- explain why it's OK to hide changes from the tdx_sysinfo users [Dave]
- update versions in stop_machine context
- don't mention major/minor versions are idential across updates. That fact is
not relevant here.
---
arch/x86/virt/vmx/tdx/tdx.c | 6 +++++-
arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 7eb1b67af656..a04b69f77c6e 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -67,7 +67,7 @@ static struct tdmr_info_list tdx_tdmr_list;
/* All TDX-usable memory regions. Protected by mem_hotplug_lock. */
static LIST_HEAD(tdx_memlist);
-static struct tdx_sys_info tdx_sysinfo __ro_after_init;
+static struct tdx_sys_info tdx_sysinfo;
/*
* Do the module global initialization once and return its result.
@@ -1307,6 +1307,10 @@ int tdx_module_run_update(void)
if (ret)
return ret;
+ /* Shouldn't fail as the update has succeeded. */
+ ret = get_tdx_sys_info_version(&tdx_sysinfo.version);
+ WARN_ON_ONCE(ret);
+
tdx_module_state.initialized = true;
return 0;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index e793dec688ab..e49c300f23d4 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,7 +7,7 @@
* Include this file to other C file instead.
*/
-static __init int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
+static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
{
int ret = 0;
u64 val;
--
2.52.0
^ permalink raw reply related
* [PATCH v9 20/23] x86/virt/tdx: Reject updates during compatibility-sensitive operations
From: Chao Gao @ 2026-05-13 15:10 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
A TDX module erratum can corrupt TD state if a module update races with
a compatibility-sensitive operation. For example, if an update races
with TD build, the TD measurement hash may be corrupted, which can later
cause attestation failure.
Handle this by requesting the TDX module to detect such races during
TDH.SYS.SHUTDOWN and reject the update when one is found. Report the
failure to userspace as -EBUSY so the update can be retried.
The downside is that module updates can be blocked indefinitely if
compatibility-sensitive operations do not quiesce. In that case,
userspace must resolve the conflict and retry the update.
Do not pre-check whether the TDX module supports this race-detection
capability. If it does not, rely on the TDX module to reject module
shutdown.
== Alternatives ==
Two alternatives were considered and rejected [1]:
a. Fail TD build when the race occurs. This would complicate KVM error
handling and risk KVM uABI instability.
b. Allow the issue to leak through. This would make the problem harder to
detect and recover from.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/linux-coco/aQIbM5m09G0FYTzE@google.com/ # [1]
---
v9:
- Rewrite the changelog: focus on what the patch does and downsides then
the alternatives [Dave]
- Extract the movement of TDX_FEATURE0 bit definitions into a cleanup patch [Dave]
---
arch/x86/include/asm/tdx.h | 6 ++++--
arch/x86/virt/vmx/tdx/tdx.c | 30 ++++++++++++++++++++++++---
drivers/virt/coco/tdx-host/tdx-host.c | 2 ++
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 5d750fe53669..1e1bdc4ec9c8 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -29,11 +29,13 @@
/*
* TDX module SEAMCALL leaf function error codes
*/
-#define TDX_SUCCESS 0ULL
-#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
+#define TDX_SUCCESS 0ULL
+#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
+#define TDX_UPDATE_COMPAT_SENSITIVE 0x8000051200000000ULL
/* Bit definitions of TDX_FEATURES0 metadata field */
#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
+#define TDX_FEATURES0_UPDATE_COMPAT BIT_ULL(47)
#ifndef __ASSEMBLER__
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index a04b69f77c6e..2ab6f6efe6d1 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1267,11 +1267,14 @@ static __init int tdx_enable(void)
}
subsys_initcall(tdx_enable);
+#define TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE BIT(16)
+
int tdx_module_shutdown(void)
{
struct tdx_sys_info_handoff handoff = {};
struct tdx_module_args args = {};
int ret, cpu;
+ u64 err;
ret = get_tdx_sys_info_handoff(&handoff);
WARN_ON_ONCE(ret);
@@ -1281,9 +1284,30 @@ int tdx_module_shutdown(void)
* module can produce and most likely supported by newer modules.
*/
args.rcx = handoff.module_hv;
- ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
- if (ret)
- return ret;
+
+ /*
+ * This flag tells the TDX module to reject shutdown if it races
+ * with a "sensitive" ongoing operation. That eliminates exposure
+ * to a TDX erratum which can corrupt TDX guest states.
+ *
+ * This flag is not supported by all TDX modules and may cause
+ * the shutdown (and subsequent update procedure) to fail.
+ */
+ args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
+
+ err = seamcall(TDH_SYS_SHUTDOWN, &args);
+
+ /*
+ * The shutdown ran into a "sensitive" ongoing operation. Signal
+ * to userspace that it can retry.
+ */
+ if ((err & TDX_SEAMCALL_STATUS_MASK) == TDX_UPDATE_COMPAT_SENSITIVE)
+ return -EBUSY;
+
+ if (err) {
+ seamcall_err(TDH_SYS_SHUTDOWN, err, &args);
+ return -EIO;
+ }
/*
* Clear global and per-CPU initialization flags so the new module
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index c4c099cf3de1..ad116e56aa1a 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -135,6 +135,8 @@ static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
case 0:
*written = size;
return FW_UPLOAD_ERR_NONE;
+ case -EBUSY:
+ return FW_UPLOAD_ERR_BUSY;
default:
return FW_UPLOAD_ERR_FW_INVALID;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v9 21/23] x86/virt/tdx: Enable TDX module runtime updates
From: Chao Gao @ 2026-05-13 15:10 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-1-chao.gao@intel.com>
All pieces of TDX module runtime updates are in place. Enable it if it
is supported.
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: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/tdx.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 1e1bdc4ec9c8..ac042b369843 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -34,6 +34,7 @@
#define TDX_UPDATE_COMPAT_SENSITIVE 0x8000051200000000ULL
/* Bit definitions of TDX_FEATURES0 metadata field */
+#define TDX_FEATURES0_TD_PRESERVING BIT_ULL(1)
#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
#define TDX_FEATURES0_UPDATE_COMPAT BIT_ULL(47)
@@ -115,8 +116,7 @@ const struct tdx_sys_info *tdx_get_sysinfo(void);
static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinfo)
{
- /* To be enabled when kernel is ready. */
- return false;
+ return sysinfo->features.tdx_features0 & TDX_FEATURES0_TD_PRESERVING;
}
int tdx_guest_keyid_alloc(void);
--
2.52.0
^ permalink raw reply related
* [PATCH v9 22/23] coco/tdx-host: Document TDX module update compatibility criteria
From: Chao Gao @ 2026-05-13 15:10 UTC (permalink / raw)
To: kvm, linux-coco, x86, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, 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, Dan Williams
In-Reply-To: <20260513151045.1420990-1-chao.gao@intel.com>
The TDX module update protocol facilitates compatible runtime updates.
Document the compatibility criteria and indicators of update failures.
Note that runtime TDX module updates are an "update at your own risk"
operation; userspace is responsible for ensuring that the update meets
the compatibility criteria.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
---
v9:
- Reword the update error descriptions.
---
.../ABI/testing/sysfs-devices-faux-tdx-host | 40 +++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
index 65897fe6abc0..9e08db231da1 100644
--- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -26,3 +26,43 @@ Description: (RO) Report the number of remaining updates. TDX maintains a
See Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
Interface Specification, Chapter "SEAMLDR_INFO" and Chapter
"SEAMLDR.INSTALL" for more information.
+
+What: /sys/devices/faux/tdx_host/firmware/tdx_module
+Contact: linux-coco@lists.linux.dev
+Description: (Directory) The tdx_module directory implements the fw_upload
+ sysfs ABI, see Documentation/ABI/testing/sysfs-class-firmware
+ for the general description of the attributes @data, @cancel,
+ @error, @loading, @remaining_size, and @status. This ABI
+ facilitates "Compatible TDX module Updates". A compatible update
+ is one that meets the following criteria:
+
+ Does not interrupt or interfere with any current TDX
+ operation or TD VM.
+
+ Does not invalidate any previously consumed module metadata
+ values outside of the TEE_TCB_SVN_2 field (updated Security
+ Version Number) in TD Quotes.
+
+ Does not require validation of new module metadata fields. By
+ implication, new module features and capabilities are only
+ available by installing the module at reboot (BIOS or EFI
+ helper loaded).
+
+ See tdx_host/firmware/tdx_module/error for information on
+ update failure indicators.
+
+What: /sys/devices/faux/tdx_host/firmware/tdx_module/error
+Contact: linux-coco@lists.linux.dev
+Description: (RO) See Documentation/ABI/testing/sysfs-class-firmware for
+ baseline expectations for this file. The <ERROR> part in the
+ <STATUS>:<ERROR> format can be:
+
+ "device-busy": The update conflicted with an ongoing
+ compatibility-sensitive operation.
+
+ "firmware-invalid": The update failed for any other reason.
+
+ "firmware-invalid" may be fatal, causing all TDs and the TDX
+ module to be lost and preventing further TDX operations. This
+ occurs when reading /sys/devices/faux/tdx_host/version returns
+ -ENXIO.
--
2.52.0
^ permalink raw reply related
* [PATCH v9 23/23] x86/virt/tdx: Document TDX module update
From: Chao Gao @ 2026-05-13 15:10 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, linux-doc
Cc: binbin.wu, dave.hansen, djbw, 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: <20260513151045.1420990-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>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@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 1a3b5bac1021..9d2b7db166b5 100644
--- a/Documentation/arch/x86/tdx.rst
+++ b/Documentation/arch/x86/tdx.rst
@@ -73,6 +73,40 @@ initialize::
[..] virt/tdx: 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 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.
+
+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.
+
+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.52.0
^ permalink raw reply related
* Re: [PATCH v2 0/6] KVM: x86: Reg cleanups / prep work for APX
From: Paolo Bonzini @ 2026-05-13 16:50 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Chang S . Bae
In-Reply-To: <20260409224236.2021562-1-seanjc@google.com>
On 4/10/26 00:42, Sean Christopherson wrote:
> Clean up KVM's register tracking and storage, primarily to prepare for landing
> APX, which expands the maximum number of GPRs from 16 to 32.
Applied to kvm/next, thanks. Upon rereading the thread I honestly
didn't see anything worth changing in the commit message of patch 1.
Paolo
> v2:
> - Call out the RIP is effectively an "EX" reg too (in patch 2). [Paolo]
> - Rework the available/dirty APIs to have an explicit "clear" operation
> for available, and only a full "reset" for dirty. [Yosry, Paolo]
>
> v1: https://lore.kernel.org/all/20260311003346.2626238-1-seanjc@google.com
>
> Sean Christopherson (6):
> KVM: x86: Add dedicated storage for guest RIP
> KVM: x86: Drop the "EX" part of "EXREG" to avoid collision with APX
> KVM: nVMX: Do a bitwise-AND of regs_avail when switching active VMCS
> KVM: x86: Add wrapper APIs to reset dirty/available register masks
> KVM: x86: Track available/dirty register masks as "unsigned long"
> values
> KVM: x86: Use a proper bitmap for tracking available/dirty registers
>
> arch/x86/include/asm/kvm_host.h | 32 +++++++++--------
> arch/x86/kvm/kvm_cache_regs.h | 62 +++++++++++++++++++++++----------
> arch/x86/kvm/svm/sev.c | 2 +-
> arch/x86/kvm/svm/svm.c | 16 ++++-----
> arch/x86/kvm/svm/svm.h | 2 +-
> arch/x86/kvm/vmx/nested.c | 10 +++---
> arch/x86/kvm/vmx/tdx.c | 36 +++++++++----------
> arch/x86/kvm/vmx/vmx.c | 52 +++++++++++++--------------
> arch/x86/kvm/vmx/vmx.h | 24 ++++++-------
> arch/x86/kvm/x86.c | 20 +++++------
> 10 files changed, 143 insertions(+), 113 deletions(-)
>
>
> base-commit: b89df297a47e641581ee67793592e5c6ae0428f4
^ permalink raw reply
* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Jason Gunthorpe @ 2026-05-13 17:24 UTC (permalink / raw)
To: Mostafa Saleh
Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
Jiri Pirko, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <agSKQrSIhizCXKwx@google.com>
On Wed, May 13, 2026 at 02:27:14PM +0000, Mostafa Saleh wrote:
> > + /*
> > + * if platform supports memory encryption,
> > + * restricted mem pool is decrypted by default
> > + */
> > + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
> > + mem->unencrypted = true;
> > + set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
> > + rmem->size >> PAGE_SHIFT);
> > + } else {
> > + mem->unencrypted = false;
> > + }
>
> This breaks pKVM as it doesn’t set CC_ATTR_MEM_ENCRYPT, so all virtio
> traffic now fails.
How will pKVM signal what kind of memory the DMA needs then?
Does it use set_memory_decrypted()? How can it use
set_memory_decrypted() without offering CC_ATTR_MEM_ENCRYPT ?
> Also, by design, some drivers are clueless about bouncing, so
Oh? What does this mean? We take quite a dim view of drivers mis-using
the DMA API..
> I believe that the pool should have a way to control it’s property
> (encrypted or decrypted) and that takes priority over whatever
> attributes comes from allocation.
We should get here because dma_capable() fails, and then swiotlb needs
to return something that makes dma_capable() succeed. Yes, it should
return details about the thing it decided, but it shouldn't have been
pre-created with some idea how to make dma_capable() work.
If dma_capable() can fail, then swiotlb should know exactly what to do
to fix it.
If pkvm wants to use the hacky scheme where you force a swiotlb pool
configuration during arch init with force swiotlb that's a somewhat
different flow and, sure the forced pool should force do whatever it
is forced to.
But lets try to keep them seperated in the discussion..
> And that brings us to the same point whether it’s better to return
> the memory along with it’s state or we pass the requested state.
> I think for other cases it’s fine for the device/DMA-API to dictate
> the attrs, but not in restricted-dma case, the firmware just knows better.
The memory type must be returned back at some level so downstream
things can do the right transformation of the phys_addr_t.
One of the aspirational CC things that should work is a T=1 device
tries to DMA from a decrypted page, finds the address is above the dma
limit of the device, so it bounces it with SWIOTLB to an encrypted low
address page and then the DMA API internal flow switiches from working
with decrypted to encrypted phys_addr_t.
If we can make that work then maybe the flows are designed correctly.
Jason
^ permalink raw reply
* Re: SVSM Development Call May 13th, 2026
From: Jörg Rödel @ 2026-05-13 20:14 UTC (permalink / raw)
To: coconut-svsm, linux-coco
In-Reply-To: <6vr7ihpntcdikktgtlbudfblvahawkwwqtuv4pmr74j6mtp4yg@loyxw56it4jy>
Meeting minutes are here:
https://github.com/coconut-svsm/governance/pull/107
-Joerg
^ permalink raw reply
* Re: [PATCH v4 01/13] dma-direct: swiotlb: handle swiotlb alloc/free outside __dma_direct_alloc_pages
From: Aneesh Kumar K.V @ 2026-05-14 4:54 UTC (permalink / raw)
To: Mostafa Saleh
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <agSDPJMZkcI-uH8f@google.com>
Mostafa Saleh <smostafa@google.com> writes:
> On Tue, May 12, 2026 at 02:33:56PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Move swiotlb allocation out of __dma_direct_alloc_pages() and handle it in
>> dma_direct_alloc() / dma_direct_alloc_pages().
>>
>> This is needed for follow-up changes that simplify the handling of
>> memory encryption/decryption based on the DMA attribute flags.
>>
>> swiotlb backing pages are already mapped decrypted by
>> swiotlb_update_mem_attributes() and rmem_swiotlb_device_init(), so
>> dma-direct should not call dma_set_decrypted() on allocation nor
>> dma_set_encrypted() on free for swiotlb-backed memory.
>>
>> Update alloc/free paths to detect swiotlb-backed pages and skip
>> encrypt/decrypt transitions for those paths. Keep the existing highmem
>> rejection in dma_direct_alloc_pages() for swiotlb allocations.
>>
>> Only for "restricted-dma-pool", we currently set `for_alloc = true`, while
>> rmem_swiotlb_device_init() decrypts the whole pool up front. This pool is
>> typically used together with "shared-dma-pool", where the shared region is
>> accessed after remap/ioremap and the returned address is suitable for
>> decrypted memory access. So existing code paths remain valid.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> kernel/dma/direct.c | 44 +++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index ec887f443741..b958f150718a 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -125,9 +125,6 @@ static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
>>
>> WARN_ON_ONCE(!PAGE_ALIGNED(size));
>>
>> - if (is_swiotlb_for_alloc(dev))
>> - return dma_direct_alloc_swiotlb(dev, size);
>> -
>> gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
>> page = dma_alloc_contiguous(dev, size, gfp);
>> if (page) {
>> @@ -204,6 +201,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>> {
>> bool remap = false, set_uncached = false;
>> + bool mark_mem_decrypt = true;
>> struct page *page;
>> void *ret;
>>
>> @@ -250,11 +248,21 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>
>> + if (is_swiotlb_for_alloc(dev)) {
>> + page = dma_direct_alloc_swiotlb(dev, size);
>> + if (page) {
>> + mark_mem_decrypt = false;
>> + goto setup_page;
>> + }
>> + return NULL;
>> + }
>> +
>> /* we always manually zero the memory once we are done */
>> page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
>> if (!page)
>> return NULL;
>>
>> +setup_page:
>> /*
>> * dma_alloc_contiguous can return highmem pages depending on a
>> * combination the cma= arguments and per-arch setup. These need to be
>> @@ -281,7 +289,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> goto out_free_pages;
>> } else {
>> ret = page_address(page);
>> - if (dma_set_decrypted(dev, ret, size))
>> + if (mark_mem_decrypt && dma_set_decrypted(dev, ret, size))
>
> I am ok with that approach, but Jason was mentioning we shouldn’t
> special case swiotlb and make the allocator return the memory state
> (similar to the dma_page [1]) . I am also OK if you want to merge that
> part of my series with is.
>
> [1] https://lore.kernel.org/linux-iommu/20260408194750.2280873-1-smostafa@google.com/
>
I was not sure whether we need dma_page. As shown in this series, we can
simplify the allocation and free paths without adding new abstractions
like dma_page.
>
>> goto out_leak_pages;
>> }
>>
>> @@ -298,7 +306,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> return ret;
>>
>> out_encrypt_pages:
>> - if (dma_set_encrypted(dev, page_address(page), size))
>> + if (mark_mem_decrypt && dma_set_encrypted(dev, page_address(page), size))
>> return NULL;
>> out_free_pages:
>> __dma_direct_free_pages(dev, page, size);
>> @@ -310,6 +318,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> void dma_direct_free(struct device *dev, size_t size,
>> void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
>> {
>> + bool mark_mem_encrypted = true;
>> unsigned int page_order = get_order(size);
>>
>> if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> @@ -338,12 +347,15 @@ void dma_direct_free(struct device *dev, size_t size,
>> dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
>> return;
>>
>> + if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
>> + mark_mem_encrypted = false;
>> +
>> if (is_vmalloc_addr(cpu_addr)) {
>> vunmap(cpu_addr);
>> } else {
>> if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
>> arch_dma_clear_uncached(cpu_addr, size);
>> - if (dma_set_encrypted(dev, cpu_addr, size))
>> + if (mark_mem_encrypted && dma_set_encrypted(dev, cpu_addr, size))
>> return;
>> }
>>
>> @@ -359,6 +371,19 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>> if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>
>> + if (is_swiotlb_for_alloc(dev)) {
>> + page = dma_direct_alloc_swiotlb(dev, size);
>> + if (!page)
>> + return NULL;
>> +
>> + if (PageHighMem(page)) {
>
> My understanding is that rmem_swiotlb_device_init() asserts that there
> is no PageHighMem()? Also a similar check doesn’t exist in
> dma_direct_alloc().
>
The reason I added that HighMem check is that __dma_direct_alloc_pages()
already has that check.
page = dma_alloc_contiguous(dev, size, gfp);
if (page) {
if (dma_coherent_ok(dev, page_to_phys(page), size) &&
(allow_highmem || !PageHighMem(page)))
return page;
dma_free_contiguous(dev, page, size);
}
I understand that the current usage of swiotlb alloc is restricted to
restricted memory, and it will not return HighMem pages. I will drop
this hunk from the patch.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 02/13] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Aneesh Kumar K.V @ 2026-05-14 5:01 UTC (permalink / raw)
To: Mostafa Saleh
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <agSDk0QsEM0ZBCTA@google.com>
Mostafa Saleh <smostafa@google.com> writes:
> On Tue, May 12, 2026 at 02:33:57PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Propagate force_dma_unencrypted() into DMA_ATTR_CC_SHARED in the
>> dma-direct allocation path and use the attribute to drive the related
>> decisions.
>>
>> This updates dma_direct_alloc(), dma_direct_free(), and
>> dma_direct_alloc_pages() to fold the forced unencrypted case into attrs.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> kernel/dma/direct.c | 44 ++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 36 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index b958f150718a..0c2e1f8436ce 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -201,16 +201,31 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>> {
>> bool remap = false, set_uncached = false;
>> - bool mark_mem_decrypt = true;
>> + bool mark_mem_decrypt = false;
>> struct page *page;
>> void *ret;
>>
>> + /*
>> + * DMA_ATTR_CC_SHARED is not a caller-visible dma_alloc_*()
>> + * attribute. The direct allocator uses it internally after it has
>> + * decided that the backing pages must be shared/decrypted, so the
>> + * rest of the allocation path can consistently select DMA addresses,
>> + * choose compatible pools and restore encryption on free.
>> + */
>> + if (attrs & DMA_ATTR_CC_SHARED)
>> + return NULL;
>> +
>> + if (force_dma_unencrypted(dev)) {
>> + attrs |= DMA_ATTR_CC_SHARED;
>> + mark_mem_decrypt = true;
>> + }
>> +
>> size = PAGE_ALIGN(size);
>> if (attrs & DMA_ATTR_NO_WARN)
>> gfp |= __GFP_NOWARN;
>>
>> - if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> - !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev))
>> + if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
>> + DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev))
>> return dma_direct_alloc_no_mapping(dev, size, dma_handle, gfp);
>>
>> if (!dev_is_dma_coherent(dev)) {
>> @@ -244,7 +259,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> * Remapping or decrypting memory may block, allocate the memory from
>> * the atomic pools instead if we aren't allowed block.
>> */
>> - if ((remap || force_dma_unencrypted(dev)) &&
>> + if ((remap || (attrs & DMA_ATTR_CC_SHARED)) &&
>> dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>
>> @@ -318,11 +333,20 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> void dma_direct_free(struct device *dev, size_t size,
>> void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
>> {
>> - bool mark_mem_encrypted = true;
>> + bool mark_mem_encrypted = false;
>> unsigned int page_order = get_order(size);
>>
>> - if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> - !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev)) {
>> + /*
>> + * if the device had requested for an unencrypted buffer,
>> + * convert it to encrypted on free
>> + */
>> + if (force_dma_unencrypted(dev)) {
>> + attrs |= DMA_ATTR_CC_SHARED;
>> + mark_mem_encrypted = true;
>> + }
>> +
>> + if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
>> + DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev)) {
>> /* cpu_addr is a struct page cookie, not a kernel address */
>> dma_free_contiguous(dev, cpu_addr, size);
>> return;
>> @@ -365,10 +389,14 @@ void dma_direct_free(struct device *dev, size_t size,
>> struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp)
>> {
>> + unsigned long attrs = 0;
>> struct page *page;
>> void *ret;
>>
>> - if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
>> + if (force_dma_unencrypted(dev))
>> + attrs |= DMA_ATTR_CC_SHARED;
>> +
>> + if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>
> What about dma_direct_free_pages()? Nothing inside uses attrs, but
> that’s quite similar to dma_direct_alloc_pages()
>
> Also, at this point, shouldn’t this patch also remove
> force_dma_unencrypted() calls from dma_set_decrypted() and
> dma_set_encrypted()?
>
The final change have
void dma_direct_free_pages(struct device *dev, size_t size,
...
{
/*
* if the device had requested for an unencrypted buffer,
* convert it to encrypted on free
*/
bool mark_mem_encrypted = force_dma_unencrypted(dev);
That got added by "dma-direct: select DMA address encoding from DMA_ATTR_CC_SHARED "
I'll move that hunk into this patch. The overall goal is
dma_direct_alloc(.. attrs)
{
if (force_dma_unencrypted(dev))
attrs |= DMA_ATTR_CC_SHARED;
}
dma_direct_free(.. attrs)
{
if (force_dma_unencrypted(dev)) {
attrs |= DMA_ATTR_CC_SHARED;
mark_mem_encrypted = true;
}
if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
mark_mem_encrypted = false;
}
dma_direct_alloc_pages()
{
if (force_dma_unencrypted(dev))
attrs |= DMA_ATTR_CC_SHARED;
}
dma_direct_free_pages()
{
bool mark_mem_encrypted = force_dma_unencrypted(dev);
if (swiotlb_find_pool(dev, page_to_phys(page)))
mark_mem_encrypted = false;
}
-aneesh
^ permalink raw reply
* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V @ 2026-05-14 5:54 UTC (permalink / raw)
To: Mostafa Saleh
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <agSKQrSIhizCXKwx@google.com>
Mostafa Saleh <smostafa@google.com> writes:
> On Tue, May 12, 2026 at 02:33:59PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Teach swiotlb to distinguish between encrypted and decrypted bounce
>> buffer pools, and make allocation and mapping paths select a pool whose
>> state matches the requested DMA attributes.
>>
>> Add a decrypted flag to io_tlb_mem, initialize it for the default and
>> restricted pools, and propagate DMA_ATTR_CC_SHARED into swiotlb pool
>> allocation. Reject swiotlb alloc/map requests when the selected pool does
>> not match the required encrypted/decrypted state.
>>
>> Also return DMA addresses with the matching phys_to_dma_{encrypted,
>> unencrypted} helper so the DMA address encoding stays consistent with the
>> chosen pool.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> include/linux/dma-direct.h | 10 ++++
>> include/linux/swiotlb.h | 8 ++-
>> kernel/dma/direct.c | 14 +++--
>> kernel/dma/swiotlb.c | 108 +++++++++++++++++++++++++++----------
>> 4 files changed, 107 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>> index c249912456f9..94fad4e7c11e 100644
>> --- a/include/linux/dma-direct.h
>> +++ b/include/linux/dma-direct.h
>> @@ -77,6 +77,10 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
>> #ifndef phys_to_dma_unencrypted
>> #define phys_to_dma_unencrypted phys_to_dma
>> #endif
>> +
>> +#ifndef phys_to_dma_encrypted
>> +#define phys_to_dma_encrypted phys_to_dma
>> +#endif
>> #else
>> static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
>> {
>> @@ -90,6 +94,12 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>> {
>> return dma_addr_unencrypted(__phys_to_dma(dev, paddr));
>> }
>> +
>> +static inline dma_addr_t phys_to_dma_encrypted(struct device *dev,
>> + phys_addr_t paddr)
>> +{
>> + return dma_addr_encrypted(__phys_to_dma(dev, paddr));
>> +}
>> /*
>> * If memory encryption is supported, phys_to_dma will set the memory encryption
>> * bit in the DMA address, and dma_to_phys will clear it.
>> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
>> index 3dae0f592063..b3fa3c6e0169 100644
>> --- a/include/linux/swiotlb.h
>> +++ b/include/linux/swiotlb.h
>> @@ -81,6 +81,7 @@ struct io_tlb_pool {
>> struct list_head node;
>> struct rcu_head rcu;
>> bool transient;
>> + bool unencrypted;
>> #endif
>> };
>>
>> @@ -111,6 +112,7 @@ struct io_tlb_mem {
>> struct dentry *debugfs;
>> bool force_bounce;
>> bool for_alloc;
>> + bool unencrypted;
>> #ifdef CONFIG_SWIOTLB_DYNAMIC
>> bool can_grow;
>> u64 phys_limit;
>> @@ -282,7 +284,8 @@ static inline void swiotlb_sync_single_for_cpu(struct device *dev,
>> extern void swiotlb_print_info(void);
>>
>> #ifdef CONFIG_DMA_RESTRICTED_POOL
>> -struct page *swiotlb_alloc(struct device *dev, size_t size);
>> +struct page *swiotlb_alloc(struct device *dev, size_t size,
>> + unsigned long attrs);
>> bool swiotlb_free(struct device *dev, struct page *page, size_t size);
>>
>> static inline bool is_swiotlb_for_alloc(struct device *dev)
>> @@ -290,7 +293,8 @@ static inline bool is_swiotlb_for_alloc(struct device *dev)
>> return dev->dma_io_tlb_mem->for_alloc;
>> }
>> #else
>> -static inline struct page *swiotlb_alloc(struct device *dev, size_t size)
>> +static inline struct page *swiotlb_alloc(struct device *dev, size_t size,
>> + unsigned long attrs)
>> {
>> return NULL;
>> }
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index dc2907439b3d..97ae4fa10521 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -104,9 +104,10 @@ static void __dma_direct_free_pages(struct device *dev, struct page *page,
>> dma_free_contiguous(dev, page, size);
>> }
>>
>> -static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size)
>> +static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size,
>> + unsigned long attrs)
>> {
>> - struct page *page = swiotlb_alloc(dev, size);
>> + struct page *page = swiotlb_alloc(dev, size, attrs);
>>
>> if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
>> swiotlb_free(dev, page, size);
>> @@ -266,8 +267,12 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> gfp, attrs);
>>
>> if (is_swiotlb_for_alloc(dev)) {
>> - page = dma_direct_alloc_swiotlb(dev, size);
>> + page = dma_direct_alloc_swiotlb(dev, size, attrs);
>> if (page) {
>> + /*
>> + * swiotlb allocations comes from pool already marked
>> + * decrypted
>> + */
>> mark_mem_decrypt = false;
>> goto setup_page;
>> }
>> @@ -374,6 +379,7 @@ void dma_direct_free(struct device *dev, size_t size,
>> return;
>>
>> if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
>> + /* Swiotlb doesn't need a page attribute update on free */
>> mark_mem_encrypted = false;
>>
>> if (is_vmalloc_addr(cpu_addr)) {
>> @@ -403,7 +409,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>> gfp, attrs);
>>
>> if (is_swiotlb_for_alloc(dev)) {
>> - page = dma_direct_alloc_swiotlb(dev, size);
>> + page = dma_direct_alloc_swiotlb(dev, size, attrs);
>> if (!page)
>> return NULL;
>>
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index ab4eccbaa076..065663be282c 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>> @@ -259,10 +259,21 @@ void __init swiotlb_update_mem_attributes(void)
>> struct io_tlb_pool *mem = &io_tlb_default_mem.defpool;
>> unsigned long bytes;
>>
>> + /*
>> + * if platform support memory encryption, swiotlb buffers are
>> + * decrypted by default.
>> + */
>> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> + io_tlb_default_mem.unencrypted = true;
>> + else
>> + io_tlb_default_mem.unencrypted = false;
>> +
>> if (!mem->nslabs || mem->late_alloc)
>> return;
>> bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
>> - set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
>> +
>> + if (io_tlb_default_mem.unencrypted)
>> + set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
>> }
>>
>> static void swiotlb_init_io_tlb_pool(struct io_tlb_pool *mem, phys_addr_t start,
>> @@ -505,8 +516,10 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
>> if (!mem->slots)
>> goto error_slots;
>>
>> - set_memory_decrypted((unsigned long)vstart,
>> - (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
>> + if (io_tlb_default_mem.unencrypted)
>> + set_memory_decrypted((unsigned long)vstart,
>> + (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
>> +
>> swiotlb_init_io_tlb_pool(mem, virt_to_phys(vstart), nslabs, true,
>> nareas);
>> add_mem_pool(&io_tlb_default_mem, mem);
>> @@ -539,7 +552,9 @@ void __init swiotlb_exit(void)
>> tbl_size = PAGE_ALIGN(mem->end - mem->start);
>> slots_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), mem->nslabs));
>>
>> - set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
>> + if (io_tlb_default_mem.unencrypted)
>> + set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
>> +
>> if (mem->late_alloc) {
>> area_order = get_order(array_size(sizeof(*mem->areas),
>> mem->nareas));
>> @@ -563,6 +578,7 @@ void __init swiotlb_exit(void)
>> * @gfp: GFP flags for the allocation.
>> * @bytes: Size of the buffer.
>> * @phys_limit: Maximum allowed physical address of the buffer.
>> + * @unencrypted: true to allocate unencrypted memory, false for encrypted memory
>> *
>> * Allocate pages from the buddy allocator. If successful, make the allocated
>> * pages decrypted that they can be used for DMA.
>> @@ -570,7 +586,8 @@ void __init swiotlb_exit(void)
>> * Return: Decrypted pages, %NULL on allocation failure, or ERR_PTR(-EAGAIN)
>> * if the allocated physical address was above @phys_limit.
>> */
>> -static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
>> +static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes,
>> + u64 phys_limit, bool unencrypted)
>> {
>> unsigned int order = get_order(bytes);
>> struct page *page;
>> @@ -588,13 +605,13 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
>> }
>>
>> vaddr = phys_to_virt(paddr);
>> - if (set_memory_decrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> + if (unencrypted && set_memory_decrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> goto error;
>> return page;
>>
>> error:
>> /* Intentional leak if pages cannot be encrypted again. */
>> - if (!set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> + if (unencrypted && !set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> __free_pages(page, order);
>> return NULL;
>> }
>> @@ -604,30 +621,26 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
>> * @dev: Device for which a memory pool is allocated.
>> * @bytes: Size of the buffer.
>> * @phys_limit: Maximum allowed physical address of the buffer.
>> + * @attrs: DMA attributes for the allocation.
>> * @gfp: GFP flags for the allocation.
>> *
>> * Return: Allocated pages, or %NULL on allocation failure.
>> */
>> static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
>> - u64 phys_limit, gfp_t gfp)
>> + u64 phys_limit, unsigned long attrs, gfp_t gfp)
>> {
>> struct page *page;
>> - unsigned long attrs = 0;
>>
>> /*
>> * Allocate from the atomic pools if memory is encrypted and
>> * the allocation is atomic, because decrypting may block.
>> */
>> - if (!gfpflags_allow_blocking(gfp) && dev && force_dma_unencrypted(dev)) {
>> + if (!gfpflags_allow_blocking(gfp) && (attrs & DMA_ATTR_CC_SHARED)) {
>> void *vaddr;
>>
>> if (!IS_ENABLED(CONFIG_DMA_COHERENT_POOL))
>> return NULL;
>>
>> - /* swiotlb considered decrypted by default */
>> - if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> - attrs = DMA_ATTR_CC_SHARED;
>> -
>> return dma_alloc_from_pool(dev, bytes, &vaddr, gfp,
>> attrs, dma_coherent_ok);
>> }
>> @@ -638,7 +651,8 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
>> else if (phys_limit <= DMA_BIT_MASK(32))
>> gfp |= __GFP_DMA32;
>>
>> - while (IS_ERR(page = alloc_dma_pages(gfp, bytes, phys_limit))) {
>> + while (IS_ERR(page = alloc_dma_pages(gfp, bytes, phys_limit,
>> + !!(attrs & DMA_ATTR_CC_SHARED)))) {
>> if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
>> phys_limit < DMA_BIT_MASK(64) &&
>> !(gfp & (__GFP_DMA32 | __GFP_DMA)))
>> @@ -657,15 +671,18 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
>> * swiotlb_free_tlb() - free a dynamically allocated IO TLB buffer
>> * @vaddr: Virtual address of the buffer.
>> * @bytes: Size of the buffer.
>> + * @unencrypted: true if @vaddr was allocated decrypted and must be
>> + * re-encrypted before being freed
>> */
>> -static void swiotlb_free_tlb(void *vaddr, size_t bytes)
>> +static void swiotlb_free_tlb(void *vaddr, size_t bytes, bool unencrypted)
>> {
>> if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
>> dma_free_from_pool(NULL, vaddr, bytes))
>> return;
>>
>> /* Intentional leak if pages cannot be encrypted again. */
>> - if (!set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> + if (!unencrypted ||
>> + !set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> __free_pages(virt_to_page(vaddr), get_order(bytes));
>> }
>>
>> @@ -676,6 +693,7 @@ static void swiotlb_free_tlb(void *vaddr, size_t bytes)
>> * @nslabs: Desired (maximum) number of slabs.
>> * @nareas: Number of areas.
>> * @phys_limit: Maximum DMA buffer physical address.
>> + * @attrs: DMA attributes for the allocation.
>> * @gfp: GFP flags for the allocations.
>> *
>> * Allocate and initialize a new IO TLB memory pool. The actual number of
>> @@ -686,7 +704,8 @@ static void swiotlb_free_tlb(void *vaddr, size_t bytes)
>> */
>> static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
>> unsigned long minslabs, unsigned long nslabs,
>> - unsigned int nareas, u64 phys_limit, gfp_t gfp)
>> + unsigned int nareas, u64 phys_limit, unsigned long attrs,
>> + gfp_t gfp)
>> {
>> struct io_tlb_pool *pool;
>> unsigned int slot_order;
>> @@ -704,9 +723,10 @@ static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
>> if (!pool)
>> goto error;
>> pool->areas = (void *)pool + sizeof(*pool);
>> + pool->unencrypted = !!(attrs & DMA_ATTR_CC_SHARED);
>>
>> tlb_size = nslabs << IO_TLB_SHIFT;
>> - while (!(tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, gfp))) {
>> + while (!(tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, attrs, gfp))) {
>> if (nslabs <= minslabs)
>> goto error_tlb;
>> nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE);
>> @@ -724,7 +744,8 @@ static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
>> return pool;
>>
>> error_slots:
>> - swiotlb_free_tlb(page_address(tlb), tlb_size);
>> + swiotlb_free_tlb(page_address(tlb), tlb_size,
>> + !!(attrs & DMA_ATTR_CC_SHARED));
>> error_tlb:
>> kfree(pool);
>> error:
>> @@ -742,7 +763,9 @@ static void swiotlb_dyn_alloc(struct work_struct *work)
>> struct io_tlb_pool *pool;
>>
>> pool = swiotlb_alloc_pool(NULL, IO_TLB_MIN_SLABS, default_nslabs,
>> - default_nareas, mem->phys_limit, GFP_KERNEL);
>> + default_nareas, mem->phys_limit,
>> + mem->unencrypted ? DMA_ATTR_CC_SHARED : 0,
>> + GFP_KERNEL);
>> if (!pool) {
>> pr_warn_ratelimited("Failed to allocate new pool");
>> return;
>> @@ -762,7 +785,7 @@ static void swiotlb_dyn_free(struct rcu_head *rcu)
>> size_t tlb_size = pool->end - pool->start;
>>
>> free_pages((unsigned long)pool->slots, get_order(slots_size));
>> - swiotlb_free_tlb(pool->vaddr, tlb_size);
>> + swiotlb_free_tlb(pool->vaddr, tlb_size, pool->unencrypted);
>> kfree(pool);
>> }
>>
>> @@ -1232,6 +1255,7 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
>> nslabs = nr_slots(alloc_size);
>> phys_limit = min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
>> pool = swiotlb_alloc_pool(dev, nslabs, nslabs, 1, phys_limit,
>> + mem->unencrypted ? DMA_ATTR_CC_SHARED : 0,
>> GFP_NOWAIT);
>> if (!pool)
>> return -1;
>> @@ -1394,6 +1418,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
>> enum dma_data_direction dir, unsigned long attrs)
>> {
>> struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
>> + bool require_decrypted = false;
>> unsigned int offset;
>> struct io_tlb_pool *pool;
>> unsigned int i;
>> @@ -1411,6 +1436,16 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
>> if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
>>
>> + /*
>> + * if we are trying to swiotlb map a decrypted paddr or the paddr is encrypted
>> + * but the device is forcing decryption, use decrypted io_tlb_mem
>> + */
>> + if ((attrs & DMA_ATTR_CC_SHARED) || force_dma_unencrypted(dev))
>> + require_decrypted = true;
>> +
>> + if (require_decrypted != mem->unencrypted)
>> + return (phys_addr_t)DMA_MAPPING_ERROR;
>> +
>> /*
>> * The default swiotlb memory pool is allocated with PAGE_SIZE
>> * alignment. If a mapping is requested with larger alignment,
>> @@ -1608,8 +1643,14 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
>> if (swiotlb_addr == (phys_addr_t)DMA_MAPPING_ERROR)
>> return DMA_MAPPING_ERROR;
>>
>> - /* Ensure that the address returned is DMA'ble */
>> - dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
>> + /*
>> + * Use the allocated io_tlb_mem encryption type to determine dma addr.
>> + */
>> + if (dev->dma_io_tlb_mem->unencrypted)
>> + dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
>> + else
>> + dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
>> +
>> if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
>> __swiotlb_tbl_unmap_single(dev, swiotlb_addr, size, dir,
>> attrs | DMA_ATTR_SKIP_CPU_SYNC,
>> @@ -1773,7 +1814,8 @@ static inline void swiotlb_create_debugfs_files(struct io_tlb_mem *mem,
>>
>> #ifdef CONFIG_DMA_RESTRICTED_POOL
>>
>> -struct page *swiotlb_alloc(struct device *dev, size_t size)
>> +struct page *swiotlb_alloc(struct device *dev, size_t size,
>> + unsigned long attrs)
>> {
>> struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
>> struct io_tlb_pool *pool;
>> @@ -1784,6 +1826,9 @@ struct page *swiotlb_alloc(struct device *dev, size_t size)
>> if (!mem)
>> return NULL;
>>
>> + if (mem->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
>> + return NULL;
>> +
>> align = (1 << (get_order(size) + PAGE_SHIFT)) - 1;
>> index = swiotlb_find_slots(dev, 0, size, align, &pool);
>> if (index == -1)
>> @@ -1853,9 +1898,18 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
>> kfree(mem);
>> return -ENOMEM;
>> }
>> + /*
>> + * if platform supports memory encryption,
>> + * restricted mem pool is decrypted by default
>> + */
>> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
>> + mem->unencrypted = true;
>> + set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>> + rmem->size >> PAGE_SHIFT);
>> + } else {
>> + mem->unencrypted = false;
>> + }
>
> This breaks pKVM as it doesn’t set CC_ATTR_MEM_ENCRYPT, so all virtio
> traffic now fails.
>
> Also, by design, some drivers are clueless about bouncing, so
> I believe that the pool should have a way to control it’s property
> (encrypted or decrypted) and that takes priority over whatever
> attributes comes from allocation.
> And that brings us to the same point whether it’s better to return
> the memory along with it’s state or we pass the requested state.
> I think for other cases it’s fine for the device/DMA-API to dictate
> the attrs, but not in restricted-dma case, the firmware just knows better.
>
Is it that the pKVM guest kernel does not have awareness of
encrypted/decrypted DMA allocations? Instead, the firmware attaches
hypervisor-shared pages to the device via restricted-dma-pool? The
kernel then has swiotlb->for_alloc = true, and hence all DMA allocations
go through the restricted-dma-pool?
Given that pKVM supports pkvm_set_memory_encrypted() and
pkvm_set_memory_decrypted(), can we consider adding CC_ATTR_MEM_ENCRYPT
support to pKVM? It would also be good to investigate whether we can set
force_dma_unencrypted(dev) to true where needed.
I agree that this patch, as it stands, can break pKVM because we are now
missing the set_memory_decrypted() call required for pKVM to work.
We now mark the swiotlb io_tlb_mem as unencrypted/encrypted in the guest
using struct io_tlb_mem->unencrypted. I am not clear what we can use for
pKVM to conditionalize this so that it works for both protected and
unprotected guests.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V @ 2026-05-14 6:24 UTC (permalink / raw)
To: Jason Gunthorpe, Mostafa Saleh
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Petr Tesarik,
Alexey Kardashevskiy, Dan Williams, Xu Yilun, linuxppc-dev,
linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <20260513172450.GR7702@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
>> And that brings us to the same point whether it’s better to return
>> the memory along with it’s state or we pass the requested state.
>> I think for other cases it’s fine for the device/DMA-API to dictate
>> the attrs, but not in restricted-dma case, the firmware just knows better.
>
> The memory type must be returned back at some level so downstream
> things can do the right transformation of the phys_addr_t.
>
> One of the aspirational CC things that should work is a T=1 device
> tries to DMA from a decrypted page, finds the address is above the dma
> limit of the device, so it bounces it with SWIOTLB to an encrypted low
> address page and then the DMA API internal flow switiches from working
> with decrypted to encrypted phys_addr_t.
>
> If we can make that work then maybe the flows are designed correctly.
>
That is what this patch series aims to do. dma_direct_map_phys() will
derive the DMA address based on the attributes of the physical address:
if (attrs & DMA_ATTR_CC_SHARED)
dma_addr = phys_to_dma_unencrypted(dev, phys);
else
dma_addr = phys_to_dma_encrypted(dev, phys);
If that fails the dma_capable() check, we fall back to swiotlb_map():
if (unlikely(!dma_capable(dev, dma_addr, size, true, attrs)))
return swiotlb_map(dev, phys, size, dir, attrs);
We currently do not have an encrypted SWIOTLB pool, but once that is
supported, swiotlb_map() should do the right thing and return the
correct encrypted dma_addr_t based on io_tlb_mem->unencrypted:
if (dev->dma_io_tlb_mem->unencrypted) {
dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
attrs |= DMA_ATTR_CC_SHARED;
} else {
dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
}
-aneesh
^ permalink raw reply
* Re: [PATCH v4 03/13] dma-pool: track decrypted atomic pools and select them via attrs
From: Aneesh Kumar K.V @ 2026-05-14 7:00 UTC (permalink / raw)
To: Mostafa Saleh
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <agSED6BdSXBhbDYI@google.com>
Mostafa Saleh <smostafa@google.com> writes:
...
>> struct page *dma_alloc_from_pool(struct device *dev, size_t size,
>> - void **cpu_addr, gfp_t gfp,
>> + void **cpu_addr, gfp_t gfp, unsigned long attrs,
>> bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t))
>> {
>> - struct gen_pool *pool = NULL;
>> + struct dma_gen_pool *dma_pool = NULL;
>> struct page *page;
>> bool pool_found = false;
>>
>> - while ((pool = dma_guess_pool(pool, gfp))) {
>> + while ((dma_pool = dma_guess_pool(dma_pool, gfp))) {
>> +
>> + if (dma_pool->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
>> + continue;
>> +
>
> nit: If we fail to find a matching pool, a slightly misleading message
> is printed as pool_found = false
>
The message printed is
WARN(1, "Failed to get suitable pool for %s\n", dev_name(dev));
That is correct, isn’t it? The kernel failed to find a pool with the
correct encryption attribute. For example, the request was for an
encrypted allocation from the pool, but no encrypted pool was available.
>
>> pool_found = true;
>> - page = __dma_alloc_from_pool(dev, size, pool, cpu_addr,
>> + page = __dma_alloc_from_pool(dev, size, dma_pool->pool, cpu_addr,
>> phys_addr_ok);
>> if (page)
>> return page;
>> @@ -296,12 +345,14 @@ struct page *dma_alloc_from_pool(struct device *dev, size_t size,
-aneesh
^ permalink raw reply
* Re: [PATCH v14 10/44] arm64: RMI: Add support for SRO
From: Aneesh Kumar K.V @ 2026-05-14 8:01 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Steven Price, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Suzuki K Poulose, 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, Emi Kisanuki,
Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-11-steven.price@arm.com>
Steven Price <steven.price@arm.com> writes:
> +unsigned long rmi_sro_execute(struct rmi_sro_state *sro, gfp_t gfp)
> +{
> + unsigned long sro_handle;
> + struct arm_smccc_1_2_regs regs;
> + struct arm_smccc_1_2_regs *regs_in = &sro->regs;
> +
> + rmi_smccc_invoke(regs_in, ®s);
> +
> + sro_handle = regs.a1;
> +
> + while (RMI_RETURN_STATUS(regs.a0) == RMI_INCOMPLETE) {
> + bool can_cancel = RMI_RETURN_CAN_CANCEL(regs.a0);
> + int ret;
> +
> + switch (RMI_RETURN_MEMREQ(regs.a0)) {
> + case RMI_OP_MEM_REQ_NONE:
> + regs = (struct arm_smccc_1_2_regs){
> + SMC_RMI_OP_CONTINUE, sro_handle, 0
> + };
> + rmi_smccc_invoke(®s, ®s);
> + break;
> + case RMI_OP_MEM_REQ_DONATE:
> + ret = rmi_sro_donate(sro, sro_handle, regs.a2, ®s,
> + gfp);
> + break;
> + case RMI_OP_MEM_REQ_RECLAIM:
> + ret = rmi_sro_reclaim(sro, sro_handle, ®s);
> + break;
> + default:
> + ret = WARN_ON(1);
> + break;
> + }
> +
> + if (ret) {
> + if (can_cancel) {
> + /*
> + * FIXME: Handle cancelling properly!
> + *
> + * If the operation has failed due to memory
> + * allocation failure then the information on
> + * the memory allocation should be saved, so
> + * that the allocation can be repeated outside
> + * of any context which prevented the
> + * allocation.
> + */
> + }
> + if (WARN_ON(ret))
> + return ret;
> + }
> + }
> +
> + return regs.a0;
> +}
Can you also add support to return x1,x2 etc
This would help things like
static int rmi_rtt_dev_unmap(unsigned long rd_phys,
unsigned long base, unsigned long top,
unsigned long *out_ipa, unsigned long *out_desc,
unsigned long *rmi_ret)
{
unsigned long flags = RMI_ADDR_TYPE_SINGLE;
struct rmi_sro_state *sro __free(sro) =
rmi_sro_init(SMC_RMI_RTT_DEV_UNMAP, rd_phys, base, top, flags, NULL);
if (!sro)
return -ENOMEM;
*rmi_ret = rmi_sro_execute(sro);
if (*rmi_ret)
return 0;
*out_ipa = sro->regs.a1;
*out_desc = sro->regs.a2;
return 0;
}
-aneesh
^ permalink raw reply
* Re: [PATCH v4 03/13] dma-pool: track decrypted atomic pools and select them via attrs
From: Mostafa Saleh @ 2026-05-14 8:06 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <yq5abjeia315.fsf@kernel.org>
On Thu, May 14, 2026 at 8:01 AM Aneesh Kumar K.V
<aneesh.kumar@kernel.org> wrote:
>
> Mostafa Saleh <smostafa@google.com> writes:
>
> ...
>
> >> struct page *dma_alloc_from_pool(struct device *dev, size_t size,
> >> - void **cpu_addr, gfp_t gfp,
> >> + void **cpu_addr, gfp_t gfp, unsigned long attrs,
> >> bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t))
> >> {
> >> - struct gen_pool *pool = NULL;
> >> + struct dma_gen_pool *dma_pool = NULL;
> >> struct page *page;
> >> bool pool_found = false;
> >>
> >> - while ((pool = dma_guess_pool(pool, gfp))) {
> >> + while ((dma_pool = dma_guess_pool(dma_pool, gfp))) {
> >> +
> >> + if (dma_pool->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
> >> + continue;
> >> +
> >
> > nit: If we fail to find a matching pool, a slightly misleading message
> > is printed as pool_found = false
> >
>
> The message printed is
>
> WARN(1, "Failed to get suitable pool for %s\n", dev_name(dev));
>
> That is correct, isn’t it? The kernel failed to find a pool with the
> correct encryption attribute. For example, the request was for an
> encrypted allocation from the pool, but no encrypted pool was available.
>
Sure, I’d prefer a clearer print in that case, especially since that’s new code:
“Only {encrypted/decrypted} pool found for a {encrypted/decrypted} alloction”
But no strong opinion.
Thanks,
Mostafa
> >
> >> pool_found = true;
> >> - page = __dma_alloc_from_pool(dev, size, pool, cpu_addr,
> >> + page = __dma_alloc_from_pool(dev, size, dma_pool->pool, cpu_addr,
> >> phys_addr_ok);
> >> if (page)
> >> return page;
> >> @@ -296,12 +345,14 @@ struct page *dma_alloc_from_pool(struct device *dev, size_t size,
>
> -aneesh
^ permalink raw reply
* Re: [PATCH v14 10/44] arm64: RMI: Add support for SRO
From: Steven Price @ 2026-05-14 9:33 UTC (permalink / raw)
To: Aneesh Kumar K.V, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Suzuki K Poulose, 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, Emi Kisanuki, Vishal Annapurve,
WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <yq5a8q9ma08r.fsf@kernel.org>
On 14/05/2026 09:01, Aneesh Kumar K.V wrote:
> Steven Price <steven.price@arm.com> writes:
>
>> +unsigned long rmi_sro_execute(struct rmi_sro_state *sro, gfp_t gfp)
>> +{
>> + unsigned long sro_handle;
>> + struct arm_smccc_1_2_regs regs;
>> + struct arm_smccc_1_2_regs *regs_in = &sro->regs;
>> +
>> + rmi_smccc_invoke(regs_in, ®s);
>> +
>> + sro_handle = regs.a1;
>> +
>> + while (RMI_RETURN_STATUS(regs.a0) == RMI_INCOMPLETE) {
>> + bool can_cancel = RMI_RETURN_CAN_CANCEL(regs.a0);
>> + int ret;
>> +
>> + switch (RMI_RETURN_MEMREQ(regs.a0)) {
>> + case RMI_OP_MEM_REQ_NONE:
>> + regs = (struct arm_smccc_1_2_regs){
>> + SMC_RMI_OP_CONTINUE, sro_handle, 0
>> + };
>> + rmi_smccc_invoke(®s, ®s);
>> + break;
>> + case RMI_OP_MEM_REQ_DONATE:
>> + ret = rmi_sro_donate(sro, sro_handle, regs.a2, ®s,
>> + gfp);
>> + break;
>> + case RMI_OP_MEM_REQ_RECLAIM:
>> + ret = rmi_sro_reclaim(sro, sro_handle, ®s);
>> + break;
>> + default:
>> + ret = WARN_ON(1);
>> + break;
>> + }
>> +
>> + if (ret) {
>> + if (can_cancel) {
>> + /*
>> + * FIXME: Handle cancelling properly!
>> + *
>> + * If the operation has failed due to memory
>> + * allocation failure then the information on
>> + * the memory allocation should be saved, so
>> + * that the allocation can be repeated outside
>> + * of any context which prevented the
>> + * allocation.
>> + */
>> + }
>> + if (WARN_ON(ret))
>> + return ret;
>> + }
>> + }
>> +
>> + return regs.a0;
>> +}
>
> Can you also add support to return x1,x2 etc
Indeed that's going to be needed. Looking at this function again I don't
think we actually need the on-stack 'regs' any more. So the below (very
lightly tested) diff would use the regs from sro which also means they
will be there for the caller if it needs them.
Thanks,
Steve
---8<---
diff --git a/arch/arm64/kernel/rmi.c b/arch/arm64/kernel/rmi.c
index a8107ca9bb6d..58a0216be409 100644
--- a/arch/arm64/kernel/rmi.c
+++ b/arch/arm64/kernel/rmi.c
@@ -356,30 +356,29 @@ void rmi_sro_free(struct rmi_sro_state *sro)
unsigned long rmi_sro_execute(struct rmi_sro_state *sro, gfp_t gfp)
{
unsigned long sro_handle;
- struct arm_smccc_1_2_regs regs;
- struct arm_smccc_1_2_regs *regs_in = &sro->regs;
+ struct arm_smccc_1_2_regs *regs = &sro->regs;
- rmi_smccc_invoke(regs_in, ®s);
+ rmi_smccc_invoke(regs, regs);
- sro_handle = regs.a1;
+ sro_handle = regs->a1;
- while (RMI_RETURN_STATUS(regs.a0) == RMI_INCOMPLETE) {
- bool can_cancel = RMI_RETURN_CAN_CANCEL(regs.a0);
+ while (RMI_RETURN_STATUS(regs->a0) == RMI_INCOMPLETE) {
+ bool can_cancel = RMI_RETURN_CAN_CANCEL(regs->a0);
int ret;
- switch (RMI_RETURN_MEMREQ(regs.a0)) {
+ switch (RMI_RETURN_MEMREQ(regs->a0)) {
case RMI_OP_MEM_REQ_NONE:
- regs = (struct arm_smccc_1_2_regs){
+ *regs = (struct arm_smccc_1_2_regs){
SMC_RMI_OP_CONTINUE, sro_handle, 0
};
- rmi_smccc_invoke(®s, ®s);
+ rmi_smccc_invoke(regs, regs);
break;
case RMI_OP_MEM_REQ_DONATE:
- ret = rmi_sro_donate(sro, sro_handle, regs.a2, ®s,
+ ret = rmi_sro_donate(sro, sro_handle, regs->a2, regs,
gfp);
break;
case RMI_OP_MEM_REQ_RECLAIM:
- ret = rmi_sro_reclaim(sro, sro_handle, ®s);
+ ret = rmi_sro_reclaim(sro, sro_handle, regs);
break;
default:
ret = WARN_ON(1);
@@ -404,7 +403,7 @@ unsigned long rmi_sro_execute(struct rmi_sro_state *sro, gfp_t gfp)
}
}
- return regs.a0;
+ return regs->a0;
}
static int rmi_check_version(void)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox