* [PATCH v2 03/16] fs/resctrl: Add info/kernel_mode file to show kernel mode options
From: Babu Moger @ 2026-03-12 20:36 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
Add resctrl_kernel_mode_show() and the "kernel_mode" info file to
display supported kernel modes and the current one (e.g. for PLZA).
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: New patch to handle PLZA interfaces with /sys/fs/resctrl/info/ directory.
https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
---
fs/resctrl/rdtgroup.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 9d6d74af4874..081da61bfe84 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -984,6 +984,41 @@ static int rdt_last_cmd_status_show(struct kernfs_open_file *of,
return 0;
}
+/*
+ * Supported resctrl kernel modes for info/kernel_mode. Names match
+ * user-visible strings.
+ */
+static struct resctrl_kmode kmodes[RESCTRL_KERNEL_MODES_NUM] = {
+ {"inherit_ctrl_and_mon", INHERIT_CTRL_AND_MON},
+ {"global_assign_ctrl_inherit_mon", GLOBAL_ASSIGN_CTRL_INHERIT_MON},
+ {"global_assign_ctrl_assign_mon", GLOBAL_ASSIGN_CTRL_ASSIGN_MON},
+};
+
+/**
+ * resctrl_kernel_mode_show() - Show supported and current resctrl kernel modes
+ * @of: kernfs file handle.
+ * @s: seq_file to write to.
+ * @v: unused.
+ *
+ * Writes one line per supported mode. The currently active mode is shown as
+ * [name]; other supported modes are shown as name.
+ */
+static int resctrl_kernel_mode_show(struct kernfs_open_file *of,
+ struct seq_file *s, void *v)
+{
+ int i;
+
+ mutex_lock(&rdtgroup_mutex);
+ for (i = 0; i < RESCTRL_KERNEL_MODES_NUM; i++) {
+ if (resctrl_kcfg.kmode_cur & kmodes[i].val)
+ seq_printf(s, "[%s]\n", kmodes[i].name);
+ else if (resctrl_kcfg.kmode & kmodes[i].val)
+ seq_printf(s, "%s\n", kmodes[i].name);
+ }
+ mutex_unlock(&rdtgroup_mutex);
+ return 0;
+}
+
void *rdt_kn_parent_priv(struct kernfs_node *kn)
{
/*
@@ -1885,6 +1920,13 @@ static struct rftype res_common_files[] = {
.seq_show = rdt_last_cmd_status_show,
.fflags = RFTYPE_TOP_INFO,
},
+ {
+ .name = "kernel_mode",
+ .mode = 0444,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .seq_show = resctrl_kernel_mode_show,
+ .fflags = RFTYPE_TOP_INFO,
+ },
{
.name = "mbm_assign_on_mkdir",
.mode = 0644,
--
2.43.0
^ permalink raw reply related
* [PATCH v2 02/16] fs, x86/resctrl: Add architecture routines for kernel mode initialization
From: Babu Moger @ 2026-03-12 20:36 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
Implement the resctrl kernel mode (kmode) arch initialization.
- Add resctrl_arch_get_kmode_cfg() to fill the default kernel mode
(INHERIT_CTRL_AND_MON). This can be extended later (e.g. for PLZA) to set
additional modes.
- Add global resctrl_kcfg and resctrl_kmode_init() to initialize default
values.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: New patch to handle PLZA interfaces with /sys/fs/resctrl/info/ directory.
https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
---
arch/x86/kernel/cpu/resctrl/core.c | 7 +++++++
fs/resctrl/rdtgroup.c | 10 ++++++++++
2 files changed, 17 insertions(+)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 7667cf7c4e94..4c3ab2d93909 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -892,6 +892,13 @@ bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
}
}
+void resctrl_arch_get_kmode_cfg(struct resctrl_kmode_cfg *kcfg)
+{
+ kcfg->kmode = INHERIT_CTRL_AND_MON;
+ kcfg->kmode_cur = INHERIT_CTRL_AND_MON;
+ kcfg->k_rdtgrp = NULL;
+}
+
static __init bool get_mem_config(void)
{
struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_MBA];
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 5da305bd36c9..9d6d74af4874 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -76,6 +76,9 @@ static void rdtgroup_destroy_root(void);
struct dentry *debugfs_resctrl;
+/* Current resctrl kernel mode config (kmode, kmode_cur, k_rdtgrp). */
+struct resctrl_kmode_cfg resctrl_kcfg;
+
/*
* Memory bandwidth monitoring event to use for the default CTRL_MON group
* and each new CTRL_MON group created by the user. Only relevant when
@@ -2204,6 +2207,11 @@ static void io_alloc_init(void)
}
}
+static void resctrl_kmode_init(void)
+{
+ resctrl_arch_get_kmode_cfg(&resctrl_kcfg);
+}
+
void resctrl_file_fflags_init(const char *config, unsigned long fflags)
{
struct rftype *rft;
@@ -4554,6 +4562,8 @@ int resctrl_init(void)
io_alloc_init();
+ resctrl_kmode_init();
+
ret = resctrl_l3_mon_resource_init();
if (ret)
return ret;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 01/16] fs/resctrl: Add kernel mode (kmode) data structures and arch hook
From: Babu Moger @ 2026-03-12 20:36 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <cover.1773347820.git.babu.moger@amd.com>
Add resctrl_kmode, resctrl_kmode_cfg, kernel mode bit defines, and
resctrl_arch_get_kmode_cfg() for resctrl kernel mode (e.g. PLZA) support.
INHERIT_CTRL_AND_MON: kernel and user space use the same CLOSID/RMID.
GLOBAL_ASSIGN_CTRL_INHERIT_MON: When active, CLOSID/control group can be
assigned for all kernel work while all kernel work uses same RMID as user
space.
GLOBAL_ASSIGN_CTRL_ASSIGN_MON: When active the same resource group (CLOSID
and RMID) can be assigned to all the kernel work. This could be any group,
including the default group.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: New patch to handle PLZA interfaces with /sys/fs/resctrl/info/ directory.
https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
---
include/linux/resctrl.h | 10 ++++++++++
include/linux/resctrl_types.h | 30 ++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 006e57fd7ca5..2c36d1ac392f 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -699,6 +699,16 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
*/
bool resctrl_arch_get_io_alloc_enabled(struct rdt_resource *r);
+/**
+ * resctrl_arch_get_kmode_cfg() - Get resctrl kernel mode configuration
+ * @kcfg: Filled with current kernel mode config (kmode, kmode_cur, k_rdtgrp).
+ *
+ * Used by the arch (e.g. x86) to report which kernel mode is active and,
+ * when a global assign mode is in use, which rdtgroup is assigned to
+ * kernel work.
+ */
+void resctrl_arch_get_kmode_cfg(struct resctrl_kmode_cfg *kcfg);
+
extern unsigned int resctrl_rmid_realloc_threshold;
extern unsigned int resctrl_rmid_realloc_limit;
diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
index a5f56faa18d2..6b78b08eab29 100644
--- a/include/linux/resctrl_types.h
+++ b/include/linux/resctrl_types.h
@@ -65,7 +65,37 @@ enum resctrl_event_id {
QOS_NUM_EVENTS,
};
+/**
+ * struct resctrl_kmode - Resctrl kernel mode descriptor
+ * @name: Human-readable name of the kernel mode.
+ * @val: Bitmask value for the kernel mode (e.g. INHERIT_CTRL_AND_MON).
+ */
+struct resctrl_kmode {
+ char name[32];
+ u32 val;
+};
+
+/**
+ * struct resctrl_kmode_cfg - Resctrl kernel mode configuration
+ * @kmode: Requested kernel mode.
+ * @kmode_cur: Currently active kernel mode.
+ * @k_rdtgrp: Resource control structure in use, or NULL otherwise.
+ */
+struct resctrl_kmode_cfg {
+ u32 kmode;
+ u32 kmode_cur;
+ struct rdtgroup *k_rdtgrp;
+};
+
#define QOS_NUM_L3_MBM_EVENTS (QOS_L3_MBM_LOCAL_EVENT_ID - QOS_L3_MBM_TOTAL_EVENT_ID + 1)
#define MBM_STATE_IDX(evt) ((evt) - QOS_L3_MBM_TOTAL_EVENT_ID)
+/* Resctrl kernel mode bits (e.g. for PLZA). */
+#define INHERIT_CTRL_AND_MON BIT(0) /* Kernel uses same CLOSID/RMID as user. */
+/* One CLOSID for all kernel work; RMID inherited from user. */
+#define GLOBAL_ASSIGN_CTRL_INHERIT_MON BIT(1)
+/* One resource group (CLOSID+RMID) for all kernel work. */
+#define GLOBAL_ASSIGN_CTRL_ASSIGN_MON BIT(2)
+#define RESCTRL_KERNEL_MODES_NUM 3
+
#endif /* __LINUX_RESCTRL_TYPES_H */
--
2.43.0
^ permalink raw reply related
* [PATCH v2 00/16] fs,x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem
From: Babu Moger @ 2026-03-12 20:36 UTC (permalink / raw)
To: corbet, tony.luck, reinette.chatre, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, babu.moger, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
This series adds support for Privilege-Level Zero Association (PLZA) to the
resctrl subsystem. PLZA is an AMD feature that allows specifying a CLOSID
and/or RMID for execution in kernel mode (privilege level zero), so that
kernel work is not subject to the same resource constrains as the current
user-space task. This avoids kernel operations being aggressively throttled
when a task's memory bandwidth is heavily limited.
The feature documentation is not yet publicly available, but it is expected
to be released in the next few weeks. In the meantime, a brief description
of the features is provided below.
Privilege Level Zero Association (PLZA)
Privilege Level Zero Association (PLZA) allows the hardware to
automatically associate execution in Privilege Level Zero (CPL=0) with a
specific COS (Class of Service) and/or RMID (Resource Monitoring
Identifier). The QoS feature set already has a mechanism to associate
execution on each logical processor with an RMID or COS. PLZA allows the
system to override this per-thread association for a thread that is
executing with CPL=0.
------------------------------------------------------------------------
The series introduces the feature in a way that supports the interface in
a generic manner to accomodate MPAM or other vendor specific implimentation.
Below is the detailed requirements provided by Reinette:
https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
Summary:
1. Kernel-mode/PLZA controls and status should be exposed under the resctrl
info directory:/sys/fs/resctrl/info/, not as a separate or arch-specific path.
2. Add two info files
a. kernel_mode
Purpose: Control how resource allocation and monitoring apply in kernel mode
(e.g. inherit from task vs global assign).
Read: List supported modes and show current one (e.g. with [brackets]).
Write: Set current mode by name (e.g. inherit_ctrl_and_mon, global_assign_ctrl_assign_mon).
b. kernel_mode_assignment
Purpose: When a “global assign” kernel mode is active, specify which resctrl group
(CLOSID/RMID) is used for kernel work.
Read: Show the assigned group in a path-like form (e.g. //, ctrl1//, ctrl1/mon1/).
Write: Assign or clear the group used for kernel mode (and optionally clear with an empty write).
The patches are based on top of commit (v7.0.0-rc3)
839e91ce3f41b (tip/master) Merge branch into tip/master: 'x86/tdx'
------------------------------------------------------------------------
Examples: kernel_mode and kernel_mode_assignment
All paths below are under /sys/fs/resctrl/ (e.g. info/kernel_mode means
/sys/fs/resctrl/info/kernel_mode). Resctrl must be mounted and the platform
must support the relevant modes (e.g. AMD with PLZA).
1) kernel_mode — show and set the current kernel mode
Read supported modes and which one is active (current in brackets):
$ cat info/kernel_mode
[inherit_ctrl_and_mon]
global_assign_ctrl_inherit_mon
global_assign_ctrl_assign_mon
Set the active mode (e.g. use one CLOSID+RMID for all kernel work):
$ echo "global_assign_ctrl_assign_mon" > info/kernel_mode
$ cat info/kernel_mode
inherit_ctrl_and_mon
global_assign_ctrl_inherit_mon
[global_assign_ctrl_assign_mon]
Mode meanings:
- inherit_ctrl_and_mon: kernel uses same CLOSID/RMID as the current task (default).
- global_assign_ctrl_inherit_mon: one CLOSID for all kernel work; RMID inherited from user.
- global_assign_ctrl_assign_mon: one resource group (CLOSID+RMID) for all kernel work.
2) kernel_mode_assignment — show and set which group is used for kernel work
Only relevant when kernel_mode is not "inherit_ctrl_and_mon". Read the
currently assigned group (path format is "CTRL_MON/MON/"):
$ cat info/kernel_mode_assignment
//
"//" means the default CTRL_MON group is assigned. Assign a specific
group instead (e.g. a CTRL_MON group "ctrl1", or a MON group "mon1" under it):
$ echo "ctrl1//" > info/kernel_mode_assignment
$ cat info/kernel_mode_assignment
ctrl1//
$ echo "ctrl1/mon1/" > info/kernel_mode_assignment
$ cat info/kernel_mode_assignment
ctrl1/mon1/
Clear the assignment (no dedicated group for kernel work):
$ echo >> info/kernel_mode_assignment
$ cat info/kernel_mode_assignment
Kmode is not configured
Errors (e.g. invalid group name or unsupported mode) are reported in
info/last_cmd_status.
---
v2:
This is similar to RFC with new proposal. Names of the some interfaces
are not final. Lets fix that later as we move forward.
Separated the two features: Global Bandwidth Enforcement (GLBE) and
Privilege Level Zero Association (PLZA).
This series only adds support for PLZA.
Used the name of the feature as kmode instead of PLZA. That can be changed as well.
Tony suggested using global variables to store the kernel mode
CLOSID and RMID. However, the kernel mode CLOSID and RMID are
coming from rdtgroup structure with the new interface. Accessing
them requires holding the associated lock, which would make the
context switch path unnecessarily expensive. So, dropped the idea.
https://lore.kernel.org/lkml/aXuxVSbk1GR2ttzF@agluck-desk3/
Let me know if there are other ways to optimize this.
Patch 1: Data structures and arch hook: Add resctrl_kmode,
resctrl_kmode_cfg, kernel-mode bits, and resctrl_arch_get_kmode_cfg()
for generic resctrl kernel mode (e.g. PLZA).
Patch 2: Implement resctrl_arch_get_kmode_cfg() on x86, add global resctrl_kcfg
and resctrl_kmode_init() to set default kmode.
Patch 3: Add info/kernel_mode and resctrl_kernel_mode_show() to list supported
kernel modes and show the current one in brackets.
Patch 4: Add x86 PLZA support and boot option rdt=plza.
Patch 5: Add supported modes from CPUID.
Patch 6: Add rdt_kmode_enable_key and arch enable/disable helpers so PLZA only
touches fast paths when enabled.
Patch 7: Add MSR_IA32_PQR_PLZA_ASSOC, bit defines, and union qos_pqr_plza_assoc
for programming PLZA.
Patch 8: Add Per-CPU and per-task state.
Patch 9: Add resctrl_arch_configure_kmode() and resctrl_arch_set_kmode()
to program PLZA per domain and set/clear it on a CPU.
Patch 10: In the sched-in path, program MSR_IA32_PQR_PLZA_ASSOC from task or
per-CPU kmode; only write when kmode changes; guard with rdt_kmode_enable_key.
Patch 11: Add write handler so the current kernel mode can be set by name.
Patch 12: Add info/kernel_mode_assignment and show which rdtgroup is assigned
for kernel mode in CTRL_MON/MON/ form.
Patch 13: Add write handler to assign/clear the group used for kernel mode;
enforce single assignment and clear on rmdir.
Patch 14: Update per-CPU PLZA state when its cpu_mask changes (add/remove CPUs)
via cpus_write_kmode() and helpers.
Patch 15: Refactor so task list respects t->kmode when the group has kmode (PLZA),
so tasks are shown correctly.
Patch 16: Add arch helper to set task kmode.
--------------------------------------------------------------------------------
v1 : https://lore.kernel.org/lkml/cover.1769029977.git.babu.moger@amd.com/
Babu Moger (16):
fs/resctrl: Add kernel mode (kmode) data structures and arch hook
fs, x86/resctrl: Add architecture routines for kernel mode
initialization
fs/resctrl: Add info/kernel_mode file to show kernel mode options
x86/resctrl: Support Privilege-Level Zero Association (PLZA)
x86/resctrl: Initialize supported kernel modes when CPUID reports PLZA
resctrl: Introduce kmode static key enable/disable helpers
x86/resctrl: Add data structures and definitions for PLZA
configuration
x86/resctrl: Add per-CPU and per-task kernel mode state
x86,fs/resctrl: Add the functionality to configure PLZA
x86/resctrl: Add PLZA state tracking and context switch handling
fs/resctrl: Add write handler for info/kernel_mode
fs/resctrl: Add info/kernel_mode_assignment to show kernel-mode
rdtgroup
fs/resctrl: Add write interface for kernel_mode_assignment
fs/resctrl: Update kmode configuration when cpu_mask changes
x86/resctrl: Refactor show_rdt_tasks() to support PLZA tasks
fs/resctrl: Add per-task kmode enable support via rdtgroup
.../admin-guide/kernel-parameters.txt | 2 +-
Documentation/filesystems/resctrl.rst | 69 ++
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/msr-index.h | 7 +
arch/x86/include/asm/resctrl.h | 92 ++-
arch/x86/kernel/cpu/resctrl/core.c | 12 +
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 77 +++
arch/x86/kernel/cpu/resctrl/internal.h | 26 +
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +
arch/x86/kernel/cpu/scattered.c | 1 +
fs/resctrl/internal.h | 2 +
fs/resctrl/rdtgroup.c | 635 +++++++++++++++++-
include/linux/resctrl.h | 40 ++
include/linux/resctrl_types.h | 30 +
include/linux/sched.h | 2 +
15 files changed, 989 insertions(+), 9 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4 09/24] x86/virt/seamldr: Check update limit before TDX Module updates
From: Dave Hansen @ 2026-03-12 20:23 UTC (permalink / raw)
To: Chao Gao, linux-coco, linux-kernel, kvm, x86
Cc: reinette.chatre, ira.weiny, kai.huang, dan.j.williams, yilun.xu,
sagis, vannapurve, paulmck, nik.borisov, zhenzhong.duan, seanjc,
rick.p.edgecombe, kas, dave.hansen, vishal.l.verma, binbin.wu,
tony.lindgren, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin
In-Reply-To: <20260212143606.534586-10-chao.gao@intel.com>
On 2/12/26 06:35, Chao Gao wrote:
> Note that userspace should perform this check before updates. Perform this
> check in kernel as well to make the update process more robust.
How many of these patches are to be "more robust"?
If you don't need it to "turn the lights on", I say kick it out.
^ permalink raw reply
* Re: [PATCH v4 07/24] coco/tdx-host: Implement firmware upload sysfs ABI for TDX Module updates
From: Dave Hansen @ 2026-03-12 20:20 UTC (permalink / raw)
To: Chao Gao, linux-coco, linux-kernel, kvm, x86
Cc: reinette.chatre, ira.weiny, kai.huang, dan.j.williams, yilun.xu,
sagis, vannapurve, paulmck, nik.borisov, zhenzhong.duan, seanjc,
rick.p.edgecombe, kas, dave.hansen, vishal.l.verma, binbin.wu,
tony.lindgren, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin
In-Reply-To: <20260212143606.534586-8-chao.gao@intel.com>
On 2/12/26 06:35, Chao Gao wrote:
> + * @data: Pointer to the TDX module update blob. It should be vmalloc'd
> + * memory.
Why?!?! What does it matter? Why enforce this?
^ permalink raw reply
* Re: [PATCH v4 08/24] x86/virt/seamldr: Block TDX Module updates if any CPU is offline
From: Dave Hansen @ 2026-03-12 20:20 UTC (permalink / raw)
To: Chao Gao, linux-coco, linux-kernel, kvm, x86
Cc: reinette.chatre, ira.weiny, kai.huang, dan.j.williams, yilun.xu,
sagis, vannapurve, paulmck, nik.borisov, zhenzhong.duan, seanjc,
rick.p.edgecombe, kas, dave.hansen, vishal.l.verma, binbin.wu,
tony.lindgren, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin
In-Reply-To: <20260212143606.534586-9-chao.gao@intel.com>
On 2/12/26 06:35, Chao Gao wrote:
> P-SEAMLDR requires every CPU to call SEAMLDR.INSTALL during updates. So,
> every CPU should be online during updates.
Gah, how did another one of these creep in? We've already fixed like a
half dozen of these.
There needs to be a *LONG* justification why there is no other choice
here. There are very good reasons to leave CPUs offline forever.
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Dave Hansen @ 2026-03-12 20:15 UTC (permalink / raw)
To: Huang, Kai, Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, tglx@kernel.org, dave.hansen@linux.intel.com,
bp@alien8.de, kas@kernel.org, Chatre, Reinette, mingo@redhat.com,
Weiny, Ira, seanjc@google.com, Verma, Vishal L,
nik.borisov@suse.com, binbin.wu@linux.intel.com, hpa@zytor.com,
Annapurve, Vishal, Chen, Farrah, Duan, Zhenzhong,
sagis@google.com, linux-kernel@vger.kernel.org,
paulmck@kernel.org, Edgecombe, Rick P, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <75dae98e3b70b66a56bde9b57992fc7d45671c36.camel@intel.com>
On 2/24/26 02:25, Huang, Kai wrote:
> But I agree making it IRQ safe is the simplest way so that we don't need to
> worry about the deadlock.
Uh, we don't just disable interrupts so we don't have to worry about
things. We disable them when we *need* to functionally.
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Dave Hansen @ 2026-03-12 20:14 UTC (permalink / raw)
To: Chao Gao, linux-coco, linux-kernel, kvm, x86
Cc: reinette.chatre, ira.weiny, kai.huang, dan.j.williams, yilun.xu,
sagis, vannapurve, paulmck, nik.borisov, zhenzhong.duan, seanjc,
rick.p.edgecombe, kas, dave.hansen, vishal.l.verma, binbin.wu,
tony.lindgren, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260212143606.534586-5-chao.gao@intel.com>
On 2/12/26 06:35, Chao Gao wrote:
> +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> +{
> + /*
> + * Serialize P-SEAMLDR calls and disable interrupts as the calls
> + * can be made from IRQ context.
> + */
> + guard(raw_spinlock_irqsave)(&seamldr_lock);
> + return seamcall_prerr(fn, args);
> +}
What SEAMLDR calls are getting made from IRQ context?
Why does this need to be raw_?
^ permalink raw reply
* Re: [PATCH v4 09/24] x86/virt/seamldr: Check update limit before TDX Module updates
From: Edgecombe, Rick P @ 2026-03-12 19:21 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: Huang, Kai, dave.hansen@linux.intel.com,
tony.lindgren@linux.intel.com, binbin.wu@linux.intel.com,
seanjc@google.com, Weiny, Ira, Chatre, Reinette, Verma, Vishal L,
nik.borisov@suse.com, mingo@redhat.com, kas@kernel.org,
Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
tglx@kernel.org, paulmck@kernel.org, hpa@zytor.com, bp@alien8.de,
yilun.xu@linux.intel.com, Williams, Dan J
In-Reply-To: <20260212143606.534586-10-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> TDX maintains a log about each TDX Module which has been loaded. This
> log has a finite size which limits the number of TDX Module updates
> which can be performed.
>
> After each successful update, the remaining updates reduces by one. Once
> it reaches zero, further updates will fail until next reboot.
>
> Before updating the TDX Module, verify that the update limit has not been
> exceeded. Otherwise, P-SEAMLDR will detect this violation after the old TDX
> Module is gone and all TDs will be killed.
>
> Note that userspace should perform this check before updates. Perform this
> check in kernel as well to make the update process more robust.
What happens if we drop this patch? The IIUC the idea is userspace needs to know
what they are doing already.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v4 19/24] x86/virt/tdx: Update tdx_sysinfo and check features post-update
From: Edgecombe, Rick P @ 2026-03-12 18:48 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: Huang, Kai, dave.hansen@linux.intel.com,
tony.lindgren@linux.intel.com, binbin.wu@linux.intel.com,
seanjc@google.com, Weiny, Ira, Chatre, Reinette, Verma, Vishal L,
nik.borisov@suse.com, mingo@redhat.com, kas@kernel.org,
Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
tglx@kernel.org, paulmck@kernel.org, hpa@zytor.com, bp@alien8.de,
yilun.xu@linux.intel.com, Williams, Dan J
In-Reply-To: <20260212143606.534586-20-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> > tdx_sysinfo contains all metadata of the active TDX module, including
> > versions, supported features, and TDMR/TDCS/TDVPS information. These
> > values may change over updates. Blindly refreshing the entire tdx_sysinfo
> > could disrupt running software, as it may subtly rely on the previous state
> > unless proven otherwise.
> >
> > Adopt a conservative approach, like microcode updates, by only refreshing
> > version information that does not affect functionality, while ignoring
> > all other changes. This is acceptable as new modules are required to
> > maintain backward compatibility.
> >
> > Any updates to metadata beyond versions should be justified and reviewed on
> > a case-by-case basis.
> >
> > Note that preallocating a tdx_sys_info buffer before updates is to avoid
> > having to handle -ENOMEM when updating tdx_sysinfo after a successful
> > update.
> >
> > Signed-off-by: Chao Gao <chao.gao@intel.com>
> > Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> > Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> > ---
> > v3:
> > - use 'old' instead of 'cur' as the local variable to represent the
> > sysinfo of the previous module [Binbin]
> > - combine if(ret) and WARN_ONCE(1, ...) to WARN_ONCE(ret, ...) [Binbin]
> > - Improve the print log messages after detecting new features from updates.
> > [Binbin]
> >
> > v2:
> > - don't add a separate function for version and feature checks. Do them
> > directly in tdx_module_post_update()
> > - add a comment about preallocating a tdx_sys_info buffer in
> > seamldr_install_module().
> > ---
> > arch/x86/virt/vmx/tdx/seamldr.c | 11 ++++++++-
> > arch/x86/virt/vmx/tdx/tdx.c | 43 +++++++++++++++++++++++++++++++++
> > arch/x86/virt/vmx/tdx/tdx.h | 3 +++
> > 3 files changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> > index 0ca802234695..3f37cc6c68ff 100644
> > --- a/arch/x86/virt/vmx/tdx/seamldr.c
> > +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> > @@ -315,6 +315,15 @@ int seamldr_install_module(const u8 *data, u32 size)
> > if (WARN_ON_ONCE(!is_vmalloc_addr(data)))
> > return -EINVAL;
> >
> > + /*
> > + * Preallocating a tdx_sys_info buffer before updates is to avoid having to
> > + * handle -ENOMEM when updating tdx_sysinfo after a successful update.
> > + */
> > + struct tdx_sys_info *sysinfo __free(kfree) = kzalloc(sizeof(*sysinfo),
> > + GFP_KERNEL);
> > + if (!sysinfo)
> > + return -ENOMEM;
> > +
> > struct seamldr_params *params __free(free_seamldr_params) =
> > init_seamldr_params(data, size);
> > if (IS_ERR(params))
> > @@ -332,6 +341,6 @@ int seamldr_install_module(const u8 *data, u32 size)
> > if (ret)
> > return ret;
> >
> > - return 0;
> > + return tdx_module_post_update(sysinfo);
> > }
> > EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index a8adb2c97e2f..3f5edbc33a4f 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1218,6 +1218,49 @@ int tdx_module_run_update(void)
> > return 0;
> > }
> >
> > +/*
> > + * Update tdx_sysinfo and check if any TDX module features changed after
> > + * updates
> > + */
> > +int tdx_module_post_update(struct tdx_sys_info *info)
> > +{
> > + struct tdx_sys_info_version *old, *new;
> > + int ret;
> > +
> > + /* Shouldn't fail as the update has succeeded */
> > + ret = get_tdx_sys_info(info);
> > + if (WARN_ONCE(ret, "version retrieval failed after update, replace TDX Module\n"))
> > + return ret;
> > +
> > + old = &tdx_sysinfo.version;
> > + new = &info->version;
> > + pr_info("version %u.%u.%02u -> %u.%u.%02u\n", old->major_version,
> > + old->minor_version,
> > + old->update_version,
> > + new->major_version,
> > + new->minor_version,
> > + new->update_version);
> > +
> > + /*
> > + * Blindly refreshing the entire tdx_sysinfo could disrupt running
> > + * software, as it may subtly rely on the previous state unless
> > + * proven otherwise.
> > + *
> > + * Only refresh version information (including handoff version)
> > + * that does not affect functionality, and ignore all other
> > + * changes.
> > + */
> > + tdx_sysinfo.version = info->version;
> > + tdx_sysinfo.handoff = info->handoff;
> > +
> > + if (!memcmp(&tdx_sysinfo, info, sizeof(*info)))
> > + return 0;
Ahh, late day reviewing this yesterday, I read this as memcpy(). This seems like
a good approach. I'd only wonder if this should either be a stronger warning, or
we can skip checking the TDX module is behaving. We rely on a lot already. But
feel free to disregard it.
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > +
> > + pr_info("TDX module features have changed after updates, but might not take effect.\n");
> > + pr_info("Please consider updating your BIOS to install the TDX Module.\n");
> > + return 0;
> > +}
> > +
> > 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 0887debfd139..d1807a476d3b 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.h
> > +++ b/arch/x86/virt/vmx/tdx/tdx.h
> > @@ -4,6 +4,8 @@
> >
> > #include <linux/bits.h>
> >
> > +#include <asm/tdx_global_metadata.h>
> > +
> > /*
> > * This file contains both macros and data structures defined by the TDX
> > * architecture and Linux defined software data structures and functions.
> > @@ -122,5 +124,6 @@ struct tdmr_info_list {
> >
> > int tdx_module_shutdown(void);
> > int tdx_module_run_update(void);
> > +int tdx_module_post_update(struct tdx_sys_info *info);
> >
> > #endif
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Andrew Cooper @ 2026-03-12 18:33 UTC (permalink / raw)
To: Sean Christopherson
Cc: Andrew Cooper, chang.seok.bae, kas, kvm, linux-coco, linux-kernel,
pbonzini, x86
In-Reply-To: <abMGHGMTfz-qaPxI@google.com>
On 12/03/2026 6:29 pm, Sean Christopherson wrote:
> On Thu, Mar 12, 2026, Andrew Cooper wrote:
>>> Have you measured performance/latency overhead if KVM goes straight to context
>>> switching R16-R31 at entry/exit? With PUSH2/POP2, it's "only" 8 more instructions
>>> on each side.
>>>
>>> If the overhead is in the noise, I'd be very strongly inclined to say KVM should
>>> swap at entry/exit regardless of kernel behavior so that we don't have to special
>>> case accesses on the back end.
>> I tried raising this point at plumbers but I don't think it came through
>> well.
>>
>> You can't unconditionally use PUSH2/POP2 in the VMExit, because at that
>> point in time it's the guest's XCR0 in context. If the guest has APX
>> disabled, PUSH2 in the VMExit path will #UD.
> Oh good gravy, so that's what the spec means by "inherited XCR0-sensitivity".
>
>> You either need two VMExit handlers, one APX and one non-APX and choose
>> based on the guest XCR0 value, or you need a branch prior to regaining
>> speculative safety, or you need to save/restore XCR0 as the first
>> action. It's horrible any way you look at it.
> Yeah, no kidding. And now that KVM loads host XCR0 outside of the fastpath,
> moving it back in just to load APX registers and take on all that complexity
> makes zero sense.
>
>> I've asked both Intel and AMD for changes to VT-x/SVM to have a proper
>> host/guest split of XCR0 which hardware manages on entry/exit. It's the
>> only viable option in my opinion, but it's still an unknown period of
>> time away and not going to exist in the first APX-capable hardware.
> +1, especially hardware already swaps XCR0 for SEV-ES+ guests.
>
> Thanks Andy!
To be clear, I've got tumbleweeds from one, and "oh yeah, we'll think
about that" from the other. Some extra requests for this would not go
amiss.
~Andrew
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-03-12 18:29 UTC (permalink / raw)
To: Andrew Cooper
Cc: chang.seok.bae, kas, kvm, linux-coco, linux-kernel, pbonzini, x86
In-Reply-To: <e76107e3-b93a-4154-859f-d22de0ee6f78@citrix.com>
On Thu, Mar 12, 2026, Andrew Cooper wrote:
> > Have you measured performance/latency overhead if KVM goes straight to context
> > switching R16-R31 at entry/exit? With PUSH2/POP2, it's "only" 8 more instructions
> > on each side.
> >
> > If the overhead is in the noise, I'd be very strongly inclined to say KVM should
> > swap at entry/exit regardless of kernel behavior so that we don't have to special
> > case accesses on the back end.
>
> I tried raising this point at plumbers but I don't think it came through
> well.
>
> You can't unconditionally use PUSH2/POP2 in the VMExit, because at that
> point in time it's the guest's XCR0 in context. If the guest has APX
> disabled, PUSH2 in the VMExit path will #UD.
Oh good gravy, so that's what the spec means by "inherited XCR0-sensitivity".
> You either need two VMExit handlers, one APX and one non-APX and choose
> based on the guest XCR0 value, or you need a branch prior to regaining
> speculative safety, or you need to save/restore XCR0 as the first
> action. It's horrible any way you look at it.
Yeah, no kidding. And now that KVM loads host XCR0 outside of the fastpath,
moving it back in just to load APX registers and take on all that complexity
makes zero sense.
> I've asked both Intel and AMD for changes to VT-x/SVM to have a proper
> host/guest split of XCR0 which hardware manages on entry/exit. It's the
> only viable option in my opinion, but it's still an unknown period of
> time away and not going to exist in the first APX-capable hardware.
+1, especially hardware already swaps XCR0 for SEV-ES+ guests.
Thanks Andy!
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Andrew Cooper @ 2026-03-12 18:11 UTC (permalink / raw)
To: seanjc
Cc: Andrew Cooper, chang.seok.bae, kas, kvm, linux-coco, linux-kernel,
pbonzini, x86
In-Reply-To: <abL8SW5JS1aV5goa@google.com>
> Have you measured performance/latency overhead if KVM goes straight to context
> switching R16-R31 at entry/exit? With PUSH2/POP2, it's "only" 8 more instructions
> on each side.
>
> If the overhead is in the noise, I'd be very strongly inclined to say KVM should
> swap at entry/exit regardless of kernel behavior so that we don't have to special
> case accesses on the back end.
I tried raising this point at plumbers but I don't think it came through
well.
You can't unconditionally use PUSH2/POP2 in the VMExit, because at that
point in time it's the guest's XCR0 in context. If the guest has APX
disabled, PUSH2 in the VMExit path will #UD.
You either need two VMExit handlers, one APX and one non-APX and choose
based on the guest XCR0 value, or you need a branch prior to regaining
speculative safety, or you need to save/restore XCR0 as the first
action. It's horrible any way you look at it.
I've asked both Intel and AMD for changes to VT-x/SVM to have a proper
host/guest split of XCR0 which hardware manages on entry/exit. It's the
only viable option in my opinion, but it's still an unknown period of
time away and not going to exist in the first APX-capable hardware.
~Andrew
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Edgecombe, Rick P @ 2026-03-12 18:05 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Williams, Dan J, dave.hansen@linux.intel.com, kas@kernel.org,
Chatre, Reinette, Weiny, Ira, linux-kernel@vger.kernel.org,
mingo@redhat.com, Verma, Vishal L, nik.borisov@suse.com,
seanjc@google.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, Annapurve, Vishal, Duan, Zhenzhong,
sagis@google.com, paulmck@kernel.org, hpa@zytor.com,
tglx@kernel.org, yilun.xu@linux.intel.com, x86@kernel.org,
bp@alien8.de
In-Reply-To: <abLJGel27U8xuU6q@intel.com>
On Thu, 2026-03-12 at 22:09 +0800, Chao Gao wrote:
> On Thu, Mar 12, 2026 at 10:00:20AM +0800, Edgecombe, Rick P wrote:
> > On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> > > TDX Module updates require careful synchronization with other TDX
> > > operations on the host. During updates, only update-related SEAMCALLs are
> > > permitted; all other SEAMCALLs must be blocked.
> > >
> > > However, SEAMCALLs can be invoked from different contexts (normal and IRQ
> > > context) and run in parallel across CPUs. And, all TD vCPUs must remain
> > > out of guest mode during updates.
> > >
> >
> > Above it says only update-related SEAMCALLs are permitted. Does that not already
> > exclude SEAMCALLs that might allow entering the TD?
>
> Those SEAMCALLs would return errors, and TDs would be killed if those errors
> aren't handled properly.
>
> One may argue that we can handle errors and retry after updates. But this just
> provides a new form of synchronization, which is not as clean as the
> well-defined synchronization provided by the kernel.
Ah ok, so it's not about the SEAMCALL resulting in entering the TD, it's about
SEAMCALLs that are operating on TDs. That makes sense. But probably don't focus
on the TD entering part then. It's just any seamcalls that are not allowed
should not be called and they need to be excluded. It's simpler.
>
> >
> > > No single lock primitive can satisfy
> > > all these synchronization requirements, so stop_machine() is used as the
> > > only well-understood mechanism that can meet them all.
> > >
> > > The TDX Module update process consists of several steps as described in
> > > Intel® Trust Domain Extensions (Intel® TDX) Module Base Architecture
> > > Specification, Revision 348549-007, Chapter 4.5 "TD-Preserving TDX Module
> > > Update"
> > >
> > > - shut down the old module
> > > - install the new module
> > > - global and per-CPU initialization
> > > - restore state information
> > >
> > > Some steps must execute on a single CPU, others must run serially across
> > > all CPUs, and some can run concurrently on all CPUs. There are also
> > > ordering requirements between steps, so all CPUs must work in a step-locked
> > > manner.
> >
> > Does the fact that they can run on other CPUs add any synchronization
> > requirements? If not I'd leave it off.
>
> I'm not sure I understand the concern.
>
> Lockstep synchronization is needed specifically because we have both multiple
> CPUs and multiple steps.
>
> If updates only required a single CPU, stop_machine() would be sufficient.
The last part "some can run concurrently on all CPUs", how does it affect the
design? They can run concurrently, but don't have to... So it's a non-
requirement?
It seems the main argument here is, this thing has lots of complex ordering
requirements. So we do it lockstep as a simple pattern to bring sanity. It's a
fine fuzzy argument I think. The way you list the types of requirements all
specifically has me trying to find the connection between each requirement and
lockstep. That is where I get lost. If the reader doesn't need to do the work of
understanding, don't ask them. And if they do, it probably needs to be clearer.
>
> >
> > >
> > > In summary, TDX Module updates create two requirements:
> >
> > The stop_machine() part seems more like a solution then a requirement.
> >
> > >
> > > 1. The entire update process must use stop_machine() to synchronize with
> > > other TDX workloads
> > > 2. Update steps must be performed in a step-locked manner
> > >
> > > To prepare for implementing concrete TDX Module update steps, establish
> > > the framework by mimicking multi_cpu_stop(), which is a good example of
> > > performing a multi-step task in step-locked manner.
> > >
> >
> > Offline Chao pointed that Paul suggested this after considering refactoring out
> > the common code. I think it might still be worth mentioning why you can't use
> > multi_cpu_stop() directly. I guess there are some differences. what are they.
>
> To be clear, Paul didn't actually suggest this approach. His feedback indicated
> he wasn't concerned about duplicating some of multi_cpu_stop()'s code, i.e., no
> need to refactor out some common code.
Right, sorry for oversimplifying.
>
> https://lore.kernel.org/all/a7affba9-0cea-4493-b868-392158b59d83@paulmck-laptop/#t
>
> We can't use multi_cpu_stop() directly because it only provides lockstep
> execution for its own infrastructure, not for the function it runs. If we
> passed a function that performs steps A, B, and C to multi_cpu_stop(), there's
> no guarantee that all CPUs complete step A before any CPU begins step B.
If it could be said more concisely, it seems relevant.
>
> >
> > > Specifically, use a
> > > global state machine to control each CPU's work and require all CPUs to
> > > acknowledge completion before proceeding to the next step.
> >
> > Maybe add a bit more about the reasoning for requiring the other steps to ack.
> > Tie it back to the lockstep part.
> >
>
> Ok. How about:
>
> Specifically, add a global state machine where each state represents a step in
> the above update flow. The state advances only after all CPUs acknowledge
> completing their work in the current state. This acknowledgment mechanism is
> what ensures lockstep execution.
Looks good.
>
> <snip>
>
> > > +static int do_seamldr_install_module(void *params)
> > > +{
> > > + enum tdp_state newstate, curstate = TDP_START;
> > > + int ret = 0;
> > > +
> > > + do {
> > > + /* Chill out and re-read tdp_data */
> > > + cpu_relax();
> > > + newstate = READ_ONCE(tdp_data.state);
> > > +
> > > + if (newstate != curstate) {
> > > + curstate = newstate;
> > > + switch (curstate) {
> >
> > Maybe a little comment here like "todo add the steps".
>
> Sure.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-03-12 17:47 UTC (permalink / raw)
To: Chang S. Bae
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel
In-Reply-To: <8853f8d5-57e6-4ea4-b9b5-8a0182d0d012@intel.com>
On Thu, Mar 12, 2026, Chang S. Bae wrote:
> On 3/11/2026 12:01 PM, Paolo Bonzini wrote:
> >
> > On the other hand, the extra 16 regs[] entries would be more or less
> > unused, the ugly switch statements wouldn't go away. In other words,
> > most of your remarks to Changseok's patches would remain...
>
> I think so...
>
> If the host kernel ever starts using EGPRs, the state would need to be
> switched in the entry code. At that point, they would likely be saved
> somewhere other than XSAVE buffer. In turn, the guest state would also need
> to be saved to regs[] on VM exit.
>
> However, that is sort of what-if scenarios at best. The host kernel still
> manages EGPR context switching through XSAVE. Saving EGPRs into regs[] would
> introduce an oddity to synchronize between two buffers: regs[] and
> gfpu->fpstate, which looks like unnecessary complexity.
>
> So while ugly, the switch statements are a bit of a trade-off here. Also
> bits 16-31 in the extended regs_avail will remain unset with APX=y.
Have you measured performance/latency overhead if KVM goes straight to context
switching R16-R31 at entry/exit? With PUSH2/POP2, it's "only" 8 more instructions
on each side.
If the overhead is in the noise, I'd be very strongly inclined to say KVM should
swap at entry/exit regardless of kernel behavior so that we don't have to special
case accesses on the back end.
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Edgecombe, Rick P @ 2026-03-12 16:56 UTC (permalink / raw)
To: Gao, Chao, Zhao, Yan Y
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
dave.hansen@linux.intel.com, kas@kernel.org, Chatre, Reinette,
seanjc@google.com, linux-kernel@vger.kernel.org,
binbin.wu@linux.intel.com, Weiny, Ira, nik.borisov@suse.com,
mingo@redhat.com, Verma, Vishal L, tony.lindgren@linux.intel.com,
Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
tglx@kernel.org, paulmck@kernel.org, hpa@zytor.com, bp@alien8.de,
yilun.xu@linux.intel.com, x86@kernel.org, Williams, Dan J
In-Reply-To: <abLPaonGMoEm2GFg@intel.com>
On Thu, 2026-03-12 at 22:36 +0800, Chao Gao wrote:
> > > + if (blob->version != 0x100) {
> > Do we need a macro for this 0x100?
>
> Maybe not, as this is a one-off check (i.e., the version/macro won't be used
> anywhere else). If someone has a strong opinion on this, I can add one.
Seems like kind of a magic number as it is. What would the macro name be, and
would it make the code more understandable?
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Chang S. Bae @ 2026-03-12 16:34 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel
In-Reply-To: <7ec084f8-812e-42f2-8470-e416fa7ee848@redhat.com>
On 3/11/2026 12:01 PM, Paolo Bonzini wrote:
>
> On the other hand, the extra 16 regs[]
> entries would be more or less unused, the ugly switch statements
> wouldn't go away. In other words, most of your remarks to Changseok's
> patches would remain...
I think so...
If the host kernel ever starts using EGPRs, the state would need to be
switched in the entry code. At that point, they would likely be saved
somewhere other than XSAVE buffer. In turn, the guest state would also
need to be saved to regs[] on VM exit.
However, that is sort of what-if scenarios at best. The host kernel
still manages EGPR context switching through XSAVE. Saving EGPRs into
regs[] would introduce an oddity to synchronize between two buffers:
regs[] and gfpu->fpstate, which looks like unnecessary complexity.
So while ugly, the switch statements are a bit of a trade-off here. Also
bits 16-31 in the extended regs_avail will remain unset with APX=y.
^ permalink raw reply
* Re: [PATCH v4 24/24] [NOT-FOR-REVIEW] x86/virt/seamldr: Save and restore current VMCS
From: Dave Hansen @ 2026-03-12 15:31 UTC (permalink / raw)
To: Vishal Annapurve, Chao Gao
Cc: Huang, Kai, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, sagis@google.com,
Duan, Zhenzhong, Edgecombe, Rick P, paulmck@kernel.org,
tglx@kernel.org, yilun.xu@linux.intel.com, Williams, Dan J,
bp@alien8.de
In-Reply-To: <CAGtprH-tAohbEuQ=BBx=MeUv_hcWmbTADcLMfyi7cVHGo+0ukA@mail.gmail.com>
On 3/12/26 08:26, Vishal Annapurve wrote:
>> 1. Save/restore the current VMCS around P-SEAMLDR calls. This produces ugly
>> assembly code [1] and doesn't play well with #MCE or #NMI if they
>> need to use the current VMCS.
> I see that significant discussion has already occurred regarding this.
>
> My question intends to better understand the current state, Do we have
> a known scenario today in upstream implementation where #MCE/#NMI need
> to use the current VMCS?
Nope, no known cases.
But, to be honest, it's not even something we should have to reason
about on the software side. It's foisting too much complexity and future
burden on software, so it's getting fixed.
^ permalink raw reply
* Re: [PATCH v4 24/24] [NOT-FOR-REVIEW] x86/virt/seamldr: Save and restore current VMCS
From: Vishal Annapurve @ 2026-03-12 15:26 UTC (permalink / raw)
To: Chao Gao
Cc: Huang, Kai, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, sagis@google.com,
Duan, Zhenzhong, Edgecombe, Rick P, paulmck@kernel.org,
tglx@kernel.org, yilun.xu@linux.intel.com, Williams, Dan J,
bp@alien8.de
In-Reply-To: <abJ90EMUnEAy763n@intel.com>
On Thu, Mar 12, 2026 at 1:48 AM Chao Gao <chao.gao@intel.com> wrote:
> But I agree that following the X86_BUG_TDX_PW_MCE is better in consistency
> and extensibility. So, here is the refined patch:
>
>
> From 46e89a50803d6568eb60bd8ec866ac3fd9f6e6da Mon Sep 17 00:00:00 2001
> From: Chao Gao <chao.gao@intel.com>
> Date: Tue, 10 Mar 2026 18:49:41 -0700
> Subject: [PATCH] coco/tdx-host: Don't expose P-SEAMLDR features on CPUs with
> erratum
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Some TDX-capable CPUs have an erratum, as documented in Intel® Trust
> Domain CPU Architectural Extensions (May 2021 edition) Chapter 2.3:
>
> SEAMRET from the P-SEAMLDR clears the current VMCS structure pointed
> to by the current-VMCS pointer. A VMM that invokes the P-SEAMLDR using
> SEAMCALL must reload the current-VMCS, if required, using the VMPTRLD
> instruction.
>
> Clearing the current VMCS behind KVM's back will break KVM.
>
> This erratum is not present when IA32_VMX_BASIC[60] is set. Add a CPU
> bug bit for this erratum and refuse to expose P-SEAMLDR features (e.g.,
> TDX module updates) on affected CPUs. Also, emit a message to clarify
> why P-SEAMLDR features are disabled for affected CPUs.
>
> == Alternatives ==
> Two workarounds were considered but both were rejected:
>
> 1. Save/restore the current VMCS around P-SEAMLDR calls. This produces ugly
> assembly code [1] and doesn't play well with #MCE or #NMI if they
> need to use the current VMCS.
I see that significant discussion has already occurred regarding this.
My question intends to better understand the current state, Do we have
a known scenario today in upstream implementation where #MCE/#NMI need
to use the current VMCS?
>
> 2. Move KVM's VMCS tracking logic to the TDX core code, which would break
> the boundary between KVM and the TDX core code [2].
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Link: https://lore.kernel.org/kvm/fedb3192-e68c-423c-93b2-a4dc2f964148@intel.com/ # [1]
> Link: https://lore.kernel.org/kvm/aYIXFmT-676oN6j0@google.com/ # [2]
> ---
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/include/asm/vmx.h | 1 +
> arch/x86/virt/vmx/tdx/tdx.c | 12 ++++++++++++
> drivers/virt/coco/tdx-host/tdx-host.c | 5 +++++
> 4 files changed, 19 insertions(+)
>
^ permalink raw reply
* Re: [PATCH v2 02/19] device core: Fix kernel-doc warnings in base.h
From: Greg KH @ 2026-03-12 14:45 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Dan Williams, linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, jgg
In-Reply-To: <20260309163932.000073e4@huawei.com>
On Mon, Mar 09, 2026 at 04:39:32PM +0000, Jonathan Cameron wrote:
> On Mon, 2 Mar 2026 16:01:50 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > In preparation for adding new fields to 'struct device_private' fix up
> > existing kernel-doc warnings in this header file of the form:
> >
> > Warning: drivers/base/base.h:59 struct member 'subsys' not described in
> > 'subsys_private'
> > Warning: drivers/base/base.h:59 struct member 'devices_kset' not described
> > in 'subsys_private'
> > Warning: drivers/base/base.h:59 struct member 'interfaces' not described in
> > 'subsys_private'
> > Warning: drivers/base/base.h:59 struct member 'mutex' not described in
> > 'subsys_private'
> >
> > ...which are simple replacements of " - " with ": ".
> >
> > Add new descriptions for these previously undescribed fields:
> >
> > Warning: drivers/base/base.h:58 struct member 'drivers_autoprobe' not
> > described in 'subsys_private'
> > Warning: drivers/base/base.h:117 struct member 'deferred_probe_reason' not
> > described in 'device_private'
> >
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> Maybe if the rest looks 'slow' can send this one ahead?
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
I can takt this now, thanks.
greg k-h
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Greg KH @ 2026-03-12 14:44 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu, bhelgaas,
alistair23, lukas, jgg, Christoph Hellwig, Jason Gunthorpe,
Marek Szyprowski, Robin Murphy, Roman Kisel, Samuel Ortiz,
Rafael J. Wysocki, Danilo Krummrich
In-Reply-To: <20260303000207.1836586-4-dan.j.williams@intel.com>
On Mon, Mar 02, 2026 at 04:01:51PM -0800, Dan Williams wrote:
> An "accepted" device is one that is allowed to access private memory within
> a Trusted Computing Boundary (TCB). The concept of "acceptance" is distinct
> from other device properties like usb_device::authorized, or
> tb_switch::authorized. The entry to the accepted state is a violent
> operation in which the device will reject MMIO requests that are not
> encrypted, and the device enters a new IOMMU protection domain to allow it
> to access addresses that were previously off-limits.
Trying to mix/match "acceptance" with "authorized" is going to be a
nightmare, what's the combination that can happen here over time?
We need to either "trust" or "not trust" the device, and the bus can
decide what to do with that value (if anything). The DMA layer can then
use that value to do:
> Subsystems like the DMA mapping layer, that need to modify their behavior
> based on the accept state, may only have access to the base 'struct
> device'.
^this.
> It is also likely that the concept of TCB acceptance grows beyond
> PCI devices over time. For these reasons, introduce the concept of
> acceptance in 'struct device_private' which is device common, but only
> suitable for buses and built-in infrastructure to consume.
Busses are what can control this, but please, let's not make this a
cc-only type thing. We have the idea of trust starting to propagate
through a number of different busses, let's get it right here, so we
don't have to have all of these different bus-specific hacks like we do
today.
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Roman Kisel <romank@linux.microsoft.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/base/Kconfig | 4 +++
> drivers/base/Makefile | 1 +
> drivers/base/base.h | 9 +++++++
> include/linux/device.h | 22 ++++++++++++++++
> drivers/base/coco.c | 58 ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 94 insertions(+)
> create mode 100644 drivers/base/coco.c
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 1786d87b29e2..d4743bf978ec 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -249,4 +249,8 @@ config FW_DEVLINK_SYNC_STATE_TIMEOUT
> command line option on every system/board your kernel is expected to
> work on.
>
> +config CONFIDENTIAL_DEVICES
> + depends on ARCH_HAS_CC_PLATFORM
> + bool
> +
> endmenu
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 8074a10183dc..e11052cd5253 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
> obj-$(CONFIG_ACPI) += physical_location.o
> +obj-$(CONFIG_CONFIDENTIAL_DEVICES) += coco.o
>
> obj-y += test/
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index b68355f5d6e3..1ae9a1679504 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -119,8 +119,13 @@ struct driver_type {
> * @dead: This device is currently either in the process of or has been
> * removed from the system. Any asynchronous events scheduled for this
> * device should exit without taking any action.
> + * @cc_accepted: track the TEE acceptance state of the device for deferred
> + * probing, MMIO mapping type, and SWIOTLB bypass for private memory DMA.
> *
> * Nothing outside of the driver core should ever touch these fields.
> + *
> + * All bitfield flags are manipulated under device_lock() to avoid
> + * read-modify-write collisions.
> */
> struct device_private {
> struct klist klist_children;
> @@ -136,6 +141,10 @@ struct device_private {
> struct driver_type driver_type;
> #endif
> u8 dead:1;
> +#ifdef CONFIG_CONFIDENTIAL_DEVICES
> + u8 cc_accepted:1;
> +#endif
Just make this:
u8 trusted:1;
no need for an #ifdef.
> +
> };
> #define to_device_private_parent(obj) \
> container_of(obj, struct device_private, knode_parent)
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 0be95294b6e6..4470365d772b 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1191,6 +1191,28 @@ static inline bool device_link_test(const struct device_link *link, u32 flags)
> return !!(link->flags & flags);
> }
>
> +/* Confidential Device state helpers */
> +#ifdef CONFIG_CONFIDENTIAL_DEVICES
> +int device_cc_accept(struct device *dev);
> +int device_cc_reject(struct device *dev);
> +bool device_cc_accepted(struct device *dev);
> +#else
> +static inline int device_cc_accept(struct device *dev)
No __must_hold() usage? That's best to check this at build time, not
just relying on:
> +{
> + lockdep_assert_held(&dev->mutex);
runtime checks.
Same for all the calls here.
> +/**
> + * device_cc_accept(): Mark a device as able to access private memory
> + * @dev: device to accept
> + *
> + * Confidential bus drivers use this helper to accept devices. For example, PCI
> + * has a sysfs ABI to accept devices after relying party attestation.
> + *
> + * Given that moving a device into confidential / private operation implicates
> + * changes to MMIO mapping attributes and DMA mappings, the transition must be
> + * done while the device is idle (driver detached).
> + */
> +int device_cc_accept(struct device *dev)
> +{
> + lockdep_assert_held(&dev->mutex);
> +
> + if (dev->driver)
> + return -EBUSY;
So you are saying that once a driver is bound, it is "trusted"? That's
fine, but maybe you don't want to do that in the core, shouldn't that be
a bus-specific thing?
this could then be:
int device_trust(struct device *dev);
int device_untrust(struct device *dev); /* ugh, bad name, pick something else? */
bool device_trusted(struct device *dev);
but note, do you ever want to move a device from trusted to untrusted?
What would cause that?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2026-03-12 14:36 UTC (permalink / raw)
To: Yan Zhao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <abIlr/gnH/Emc/9X@yzhao56-desk.sh.intel.com>
>> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
>> + const void *sig, unsigned int sig_size)
>> +{
>> + struct seamldr_params *params;
>> + const u8 *ptr;
>> + int i;
>> +
>> + if (WARN_ON_ONCE(!is_vmalloc_addr(module) || !is_vmalloc_addr(sig)))
>> + return ERR_PTR(-EINVAL);
>> +
>> + if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
>> + return ERR_PTR(-EINVAL);
>> +
>> + if (sig_size > SEAMLDR_MAX_NR_SIG_4KB_PAGES * SZ_4K)
>> + return ERR_PTR(-EINVAL);
>> +
>> + /*
>> + * Check that input buffers satisfy P-SEAMLDR's size and alignment
>> + * constraints so they can be passed directly to P-SEAMLDR without
>> + * relocation or copy.
>> + */
>> + if (!IS_ALIGNED(module_size, SZ_4K) || !IS_ALIGNED(sig_size, SZ_4K) ||
>> + !IS_ALIGNED((unsigned long)module, SZ_4K) ||
>> + !IS_ALIGNED((unsigned long)sig, SZ_4K))
>> + return ERR_PTR(-EINVAL);
>> +
>> + params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
>> + if (!params)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + params->scenario = SEAMLDR_SCENARIO_UPDATE;
>
>Add a comment for why params->version isn't initialized explicitly?
Because the page is zero-allocated, the version is implicitly 0.
But I just found that 16KB sigstructs require version 1, so I'll make the
version explicit:
/* Only version 1 supports >4KB sigstruct */
if (sig_size > SZ_4K)
params->version = 1;
else
params->version = 0;
Note that we can't always use version 1 since existing P-SEAMLDR versions don't
support it.
<snip>
>> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
>> +{
>> + const struct tdx_blob *blob = (const void *)data;
>> + int module_size, sig_size;
>> + const void *sig, *module;
>> +
>> + if (size < sizeof(struct tdx_blob) || blob->offset_of_module >= size)
>> + return ERR_PTR(-EINVAL);
>> +
>> + if (blob->version != 0x100) {
>Do we need a macro for this 0x100?
Maybe not, as this is a one-off check (i.e., the version/macro won't be used
anywhere else). If someone has a strong opinion on this, I can add one.
^ permalink raw reply
* Re: [PATCH v4 09/24] x86/virt/seamldr: Check update limit before TDX Module updates
From: Chao Gao @ 2026-03-12 14:13 UTC (permalink / raw)
To: Yan Zhao
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, zhenzhong.duan, seanjc, rick.p.edgecombe, kas,
dave.hansen, vishal.l.verma, binbin.wu, tony.lindgren,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <abImiYEAOH29tq/H@yzhao56-desk.sh.intel.com>
On Thu, Mar 12, 2026 at 10:35:53AM +0800, Yan Zhao wrote:
>On Thu, Feb 12, 2026 at 06:35:12AM -0800, Chao Gao wrote:
>> TDX maintains a log about each TDX Module which has been loaded. This
>> log has a finite size which limits the number of TDX Module updates
>> which can be performed.
>>
>> After each successful update, the remaining updates reduces by one. Once
>> it reaches zero, further updates will fail until next reboot.
>>
>> Before updating the TDX Module, verify that the update limit has not been
>> exceeded. Otherwise, P-SEAMLDR will detect this violation after the old TDX
>> Module is gone and all TDs will be killed.
>>
>> Note that userspace should perform this check before updates. Perform this
>> check in kernel as well to make the update process more robust.
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> ---
>> arch/x86/virt/vmx/tdx/seamldr.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
>> index 694243f1f220..733b13215691 100644
>> --- a/arch/x86/virt/vmx/tdx/seamldr.c
>> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
>> @@ -52,6 +52,16 @@ EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>> */
>> int seamldr_install_module(const u8 *data, u32 size)
>> {
>> + struct seamldr_info info;
>> + int ret;
>> +
>> + ret = seamldr_get_info(&info);
>> + if (ret)
>> + return ret;
>> +
>> + if (!info.num_remaining_updates)
>> + return -ENOSPC;
>seamldr_install_module() is invoked by tdx_fw_write().
>Why don't we put the check of info.num_remaining_updates in tdx_fw_prepare()?
Putting sanity checks in a preparatory step makes sense. Will do.
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Chao Gao @ 2026-03-12 14:09 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org, Huang, Kai,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, Weiny, Ira,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, kas@kernel.org, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, tglx@kernel.org,
paulmck@kernel.org, hpa@zytor.com, bp@alien8.de,
yilun.xu@linux.intel.com, Williams, Dan J
In-Reply-To: <daa8080e1195f586af331dcf31ee5239c4ecbd15.camel@intel.com>
On Thu, Mar 12, 2026 at 10:00:20AM +0800, Edgecombe, Rick P wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> TDX Module updates require careful synchronization with other TDX
>> operations on the host. During updates, only update-related SEAMCALLs are
>> permitted; all other SEAMCALLs must be blocked.
>>
>> However, SEAMCALLs can be invoked from different contexts (normal and IRQ
>> context) and run in parallel across CPUs. And, all TD vCPUs must remain
>> out of guest mode during updates.
>>
>
>Above it says only update-related SEAMCALLs are permitted. Does that not already
>exclude SEAMCALLs that might allow entering the TD?
Those SEAMCALLs would return errors, and TDs would be killed if those errors
aren't handled properly.
One may argue that we can handle errors and retry after updates. But this just
provides a new form of synchronization, which is not as clean as the
well-defined synchronization provided by the kernel.
>
>> No single lock primitive can satisfy
>> all these synchronization requirements, so stop_machine() is used as the
>> only well-understood mechanism that can meet them all.
>>
>> The TDX Module update process consists of several steps as described in
>> Intel® Trust Domain Extensions (Intel® TDX) Module Base Architecture
>> Specification, Revision 348549-007, Chapter 4.5 "TD-Preserving TDX Module
>> Update"
>>
>> - shut down the old module
>> - install the new module
>> - global and per-CPU initialization
>> - restore state information
>>
>> Some steps must execute on a single CPU, others must run serially across
>> all CPUs, and some can run concurrently on all CPUs. There are also
>> ordering requirements between steps, so all CPUs must work in a step-locked
>> manner.
>
>Does the fact that they can run on other CPUs add any synchronization
>requirements? If not I'd leave it off.
I'm not sure I understand the concern.
Lockstep synchronization is needed specifically because we have both multiple
CPUs and multiple steps.
If updates only required a single CPU, stop_machine() would be sufficient.
>
>>
>> In summary, TDX Module updates create two requirements:
>
>The stop_machine() part seems more like a solution then a requirement.
>
>>
>> 1. The entire update process must use stop_machine() to synchronize with
>> other TDX workloads
>> 2. Update steps must be performed in a step-locked manner
>>
>> To prepare for implementing concrete TDX Module update steps, establish
>> the framework by mimicking multi_cpu_stop(), which is a good example of
>> performing a multi-step task in step-locked manner.
>>
>
>Offline Chao pointed that Paul suggested this after considering refactoring out
>the common code. I think it might still be worth mentioning why you can't use
>multi_cpu_stop() directly. I guess there are some differences. what are they.
To be clear, Paul didn't actually suggest this approach. His feedback indicated
he wasn't concerned about duplicating some of multi_cpu_stop()'s code, i.e., no
need to refactor out some common code.
https://lore.kernel.org/all/a7affba9-0cea-4493-b868-392158b59d83@paulmck-laptop/#t
We can't use multi_cpu_stop() directly because it only provides lockstep
execution for its own infrastructure, not for the function it runs. If we
passed a function that performs steps A, B, and C to multi_cpu_stop(), there's
no guarantee that all CPUs complete step A before any CPU begins step B.
>
>> Specifically, use a
>> global state machine to control each CPU's work and require all CPUs to
>> acknowledge completion before proceeding to the next step.
>
>Maybe add a bit more about the reasoning for requiring the other steps to ack.
>Tie it back to the lockstep part.
>
Ok. How about:
Specifically, add a global state machine where each state represents a step in
the above update flow. The state advances only after all CPUs acknowledge
completing their work in the current state. This acknowledgment mechanism is
what ensures lockstep execution.
<snip>
>> +static int do_seamldr_install_module(void *params)
>> +{
>> + enum tdp_state newstate, curstate = TDP_START;
>> + int ret = 0;
>> +
>> + do {
>> + /* Chill out and re-read tdp_data */
>> + cpu_relax();
>> + newstate = READ_ONCE(tdp_data.state);
>> +
>> + if (newstate != curstate) {
>> + curstate = newstate;
>> + switch (curstate) {
>
>Maybe a little comment here like "todo add the steps".
Sure.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox