* Re: [PATCH v4 09/24] x86/virt/seamldr: Check update limit before TDX Module updates
From: Yan Zhao @ 2026-03-12 2:35 UTC (permalink / raw)
To: Chao Gao
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: <20260212143606.534586-10-chao.gao@intel.com>
On Thu, Feb 12, 2026 at 06:35:12AM -0800, Chao Gao wrote:
> TDX maintains a log about each TDX Module which has been loaded. This
> log has a finite size which limits the number of TDX Module updates
> which can be performed.
>
> After each successful update, the remaining updates reduces by one. Once
> it reaches zero, further updates will fail until next reboot.
>
> Before updating the TDX Module, verify that the update limit has not been
> exceeded. Otherwise, P-SEAMLDR will detect this violation after the old TDX
> Module is gone and all TDs will be killed.
>
> Note that userspace should perform this check before updates. Perform this
> check in kernel as well to make the update process more robust.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> ---
> 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()?
> if (WARN_ON_ONCE(!is_vmalloc_addr(data)))
> return -EINVAL;
>
> --
> 2.47.3
>
>
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Yan Zhao @ 2026-03-12 2:32 UTC (permalink / raw)
To: Chao Gao
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: <20260212143606.534586-11-chao.gao@intel.com>
On Thu, Feb 12, 2026 at 06:35:13AM -0800, Chao Gao wrote:
> P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX Module
> update requests. This structure contains physical addresses pointing to
> the module binary and its signature file (or sigstruct), along with an
> update scenario field.
>
> TDX Modules are distributed in the tdx_blob format defined at [1]. A
> tdx_blob contains a header, sigstruct, and module binary. This is also
> the format supplied by the userspace to the kernel.
>
> Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
> accordingly. This structure will be passed to P-SEAMLDR to initiate the
> update.
>
> Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
> a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
> Specification" will be published separately. The kernel does not
> validate P-SEAMLDR compatibility (for example, whether it supports 4KB
> or 16KB sigstruct); userspace must ensure the P-SEAMLDR version is
> compatible with the selected TDX Module by checking the minimum
> P-SEAMLDR version requirements at [2].
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/blob_structure.txt # [1]
> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json # [2]
> ---
> v4:
> - Remove checksum verification as it is optional
> - Convert comments to is_vmalloc_addr() checks [Kai]
> - Explain size/alignment checks in alloc_seamldr_params() [Kai]
>
> v3:
> - Print tdx_blob version in hex [Binbin]
> - Drop redundant sigstruct alignment check [Yilun]
> - Note buffers passed from firmware upload infrastructure are
> vmalloc()'d above alloc_seamldr_params()
> ---
> arch/x86/virt/vmx/tdx/seamldr.c | 152 ++++++++++++++++++++++++++++++++
> 1 file changed, 152 insertions(+)
>
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 733b13215691..718cb8396057 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -6,9 +6,11 @@
> */
> #define pr_fmt(fmt) "seamldr: " fmt
>
> +#include <linux/cleanup.h>
> #include <linux/cpuhplock.h>
> #include <linux/cpumask.h>
> #include <linux/mm.h>
> +#include <linux/slab.h>
> #include <linux/spinlock.h>
>
> #include <asm/seamldr.h>
> @@ -18,6 +20,33 @@
> /* P-SEAMLDR SEAMCALL leaf function */
> #define P_SEAMLDR_INFO 0x8000000000000000
>
> +#define SEAMLDR_MAX_NR_MODULE_4KB_PAGES 496
> +#define SEAMLDR_MAX_NR_SIG_4KB_PAGES 4
> +
> +/*
> + * The seamldr_params "scenario" field specifies the operation mode:
> + * 0: Install TDX Module from scratch (not used by kernel)
> + * 1: Update existing TDX Module to a compatible version
> + */
> +#define SEAMLDR_SCENARIO_UPDATE 1
> +
> +/*
> + * This is called the "SEAMLDR_PARAMS" data structure and is defined
> + * in "SEAM Loader (SEAMLDR) Interface Specification".
> + *
> + * It describes the TDX Module that will be installed.
> + */
> +struct seamldr_params {
> + u32 version;
> + u32 scenario;
> + u64 sigstruct_pa[SEAMLDR_MAX_NR_SIG_4KB_PAGES];
> + u8 reserved[80];
Calculate this size (i.e., 80) from 4096 - xxx ?
> + u64 num_module_pages;
> + u64 mod_pages_pa_list[SEAMLDR_MAX_NR_MODULE_4KB_PAGES];
> +} __packed;
> +
> +static_assert(sizeof(struct seamldr_params) == 4096);
> +
> /*
> * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> * interact with P-SEAMLDR simultaneously.
> @@ -42,6 +71,124 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
> }
> EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>
> +static void free_seamldr_params(struct seamldr_params *params)
> +{
> + free_page((unsigned long)params);
> +}
> +
> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
> + const void *sig, unsigned int sig_size)
> +{
> + struct seamldr_params *params;
> + const u8 *ptr;
> + int i;
> +
> + if (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?
> + ptr = sig;
> + for (i = 0; i < sig_size / SZ_4K; i++) {
> + /*
> + * Don't assume @sig is page-aligned although it is 4KB-aligned.
> + * Always add the in-page offset to get the physical address.
> + */
> + params->sigstruct_pa[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> + ((unsigned long)ptr & ~PAGE_MASK);
> + ptr += SZ_4K;
> + }
> +
> + params->num_module_pages = module_size / SZ_4K;
> +
> + ptr = module;
> + for (i = 0; i < params->num_module_pages; i++) {
> + params->mod_pages_pa_list[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
> + ((unsigned long)ptr & ~PAGE_MASK);
> + ptr += SZ_4K;
> + }
> +
> + return params;
> +}
> +
> +/*
> + * Intel TDX Module blob. Its format is defined at:
> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
> + *
> + * Note this structure differs from the reference above: the two variable-length
> + * fields "@sigstruct" and "@module" are represented as a single "@data" field
> + * here and split programmatically using the offset_of_module value.
> + */
> +struct tdx_blob {
> + u16 version;
> + u16 checksum;
> + u32 offset_of_module;
> + u8 signature[8];
> + u32 length;
> + u32 resv0;
> + u64 resv1[509];
> + u8 data[];
> +} __packed;
> +
> +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?
> + pr_err("unsupported blob version: %x\n", blob->version);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (blob->resv0 || memchr_inv(blob->resv1, 0, sizeof(blob->resv1))) {
> + pr_err("non-zero reserved fields\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + /* Split the blob into a sigstruct and a module */
> + sig = blob->data;
> + sig_size = blob->offset_of_module - sizeof(struct tdx_blob);
> + module = data + blob->offset_of_module;
> + module_size = size - blob->offset_of_module;
> +
> + if (sig_size <= 0 || module_size <= 0 || blob->length != size)
> + return ERR_PTR(-EINVAL);
> +
> + if (memcmp(blob->signature, "TDX-BLOB", 8)) {
> + pr_err("invalid signature\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return alloc_seamldr_params(module, module_size, sig, sig_size);
> +}
> +
> +DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
> + if (!IS_ERR_OR_NULL(_T)) free_seamldr_params(_T))
> +
> /**
> * seamldr_install_module - Install a new TDX module
> * @data: Pointer to the TDX module update blob. It should be vmalloc'd
> @@ -65,6 +212,11 @@ int seamldr_install_module(const u8 *data, u32 size)
> if (WARN_ON_ONCE(!is_vmalloc_addr(data)))
> return -EINVAL;
>
> + struct seamldr_params *params __free(free_seamldr_params) =
> + init_seamldr_params(data, size);
> + if (IS_ERR(params))
> + return PTR_ERR(params);
> +
> guard(cpus_read_lock)();
> if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
> pr_err("Cannot update the TDX Module if any CPU is offline\n");
> --
> 2.47.3
>
>
^ permalink raw reply
* Re: [PATCH v4 13/24] x86/virt/seamldr: Shut down the current TDX module
From: Edgecombe, Rick P @ 2026-03-12 2:34 UTC (permalink / raw)
To: Gao, Chao, Huang, Kai
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, Chatre, Reinette, Weiny, Ira, seanjc@google.com,
Verma, Vishal L, nik.borisov@suse.com, binbin.wu@linux.intel.com,
hpa@zytor.com, Annapurve, Vishal, sagis@google.com,
Duan, Zhenzhong, tglx@kernel.org, linux-kernel@vger.kernel.org,
paulmck@kernel.org, bp@alien8.de, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <aaqM+eFfQ9qmpzyT@intel.com>
On Fri, 2026-03-06 at 16:14 +0800, Chao Gao wrote:
> > This (and future patches) makes couple of tdx_xx() functions visible out of
> > tdx.c. The alternative is to move the main "module update" function out of
> > seamldr.c to tdx.c, but that would require making couple of seamldr_xx()s
> > (and data structures probably) visible to tdx.c too.
>
> Yes. I'll keep this organization unless someone strongly prefers moving the
> main "module update" function and related data structures to tdx.c.
>
> If neither approach is acceptable, a third option would be to remove seamldr.c
> entirely and merge it into tdx.c. This would mean adding ~360 LoC to an
> existing file that already has ~1900 LoC.
tdx.c will only get bigger, but the breakdown between these files in this series
is not super clear to me. I think the headers are not a problem. But the fact
that seamldr.c is making seamcalls indirectly is a bit strange.
I'd maybe vote to put it all into tdx.c at this stage of the enabling, but
leaving it seems ok to me too. Someday when TDX is more implemented we can see
what borders make more sense.
^ permalink raw reply
* Re: [PATCH v4 13/24] x86/virt/seamldr: Shut down the current TDX module
From: Edgecombe, Rick P @ 2026-03-12 2:17 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: 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-14-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> The first step of TDX Module updates is shutting down the current TDX
> Module. This step also packs state information that needs to be
> preserved across updates as handoff data, which will be consumed by the
> updated module. The handoff data is stored internally in the SEAM range
> and is hidden from the kernel.
>
> To ensure a successful update, the new module must be able to consume
> the handoff data generated by the old module. Since handoff data layout
> may change between modules, the handoff data is versioned. Each module
> has a native handoff version and provides backward support for several
> older versions.
>
> The complete handoff versioning protocol is complex as it supports both
> module upgrades and downgrades. See details in Intel® Trust Domain
> Extensions (Intel® TDX) Module Base Architecture Specification, Revision
> 348549-007, Chapter 4.5.3 "Handoff Versioning".
>
> Ideally, the kernel needs to retrieve the handoff versions supported by
> the current module and the new module and select a version supported by
> both. But, since the Linux kernel only supports module upgrades, simply
> request the current module to generate handoff data using its highest
> supported version, expecting that the new module will likely support it.
>
> Note that only one CPU needs to call the TDX Module's shutdown API.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> ---
> v4:
> - skip the whole handoff metadata if runtime updates are not supported
> [Yilun]
> v3:
> - remove autogeneration stuff in the changelog
> v2:
> - add a comment about how handoff version is chosen.
> - remove the first !ret in get_tdx_sys_info_handoff() as we edited the
> auto-generated code anyway
> - remove !! when determining whether a CPU is the primary one
> - remove unnecessary if-break nesting in TDP_SHUTDOWN
> ---
> arch/x86/include/asm/tdx_global_metadata.h | 5 +++++
> arch/x86/virt/vmx/tdx/seamldr.c | 10 ++++++++++
> arch/x86/virt/vmx/tdx/tdx.c | 15 +++++++++++++++
> arch/x86/virt/vmx/tdx/tdx.h | 3 +++
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 15 +++++++++++++++
> 5 files changed, 48 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index 40689c8dc67e..8a9ebd895e70 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -40,12 +40,17 @@ 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;
> struct tdx_sys_info_tdmr tdmr;
> struct tdx_sys_info_td_ctrl td_ctrl;
> struct tdx_sys_info_td_conf td_conf;
> + struct tdx_sys_info_handoff handoff;
> };
>
> #endif
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 70bc577e5957..c59cdd5b1fe4 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -18,6 +18,7 @@
> #include <asm/seamldr.h>
>
> #include "seamcall_internal.h"
> +#include "tdx.h"
>
> /* P-SEAMLDR SEAMCALL leaf function */
> #define P_SEAMLDR_INFO 0x8000000000000000
> @@ -196,6 +197,7 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> */
> enum tdp_state {
> TDP_START,
> + TDP_SHUTDOWN,
> TDP_DONE,
> };
>
> @@ -228,8 +230,12 @@ static void ack_state(void)
> static int do_seamldr_install_module(void *params)
> {
> enum tdp_state newstate, curstate = TDP_START;
> + int cpu = smp_processor_id();
> + bool primary;
> int ret = 0;
>
> + primary = cpumask_first(cpu_online_mask) == cpu;
> +
> do {
> /* Chill out and re-read tdp_data */
> cpu_relax();
> @@ -238,6 +244,10 @@ static int do_seamldr_install_module(void *params)
> if (newstate != curstate) {
> curstate = newstate;
> switch (curstate) {
> + case TDP_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 b65b2a609e81..f911c8c63800 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1176,6 +1176,21 @@ int tdx_enable(void)
> }
> EXPORT_SYMBOL_FOR_KVM(tdx_enable);
>
> +int tdx_module_shutdown(void)
> +{
> + struct tdx_module_args args = {};
> +
> + /*
> + * Shut down the TDX Module and prepare handoff data for the next
> + * TDX Module. This SEAMCALL requires a handoff version. Use the
> + * module's handoff version, as it is the highest version the
> + * module can produce and is more likely to be supported by new
> + * modules as new modules likely have higher handoff version.
> + */
> + args.rcx = tdx_sysinfo.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 82bb82be8567..1c4da9540ae0 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -46,6 +46,7 @@
> #define TDH_PHYMEM_PAGE_WBINVD 41
> #define TDH_VP_WR 43
> #define TDH_SYS_CONFIG 45
> +#define TDH_SYS_SHUTDOWN 52
>
> /*
> * SEAMCALL leaf:
> @@ -118,4 +119,6 @@ struct tdmr_info_list {
> int max_tdmrs; /* How many 'tdmr_info's are allocated */
> };
>
> +int tdx_module_shutdown(void);
> +
> #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 4c9917a9c2c3..6aee10c36489 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -100,6 +100,20 @@ static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf
> return ret;
> }
>
> +static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
> +{
> + int ret = 0;
> + u64 val;
> +
> + if (!tdx_supports_runtime_update(&tdx_sysinfo))
> + return 0;
DPAMT has a similar need to conditionally fetch metadata. The thing that is ugly
about this is it refers to the global copy while populating the tdx_sys_info
passed as a pointer. That is how DPAMT worked previously. I was going to change
it to something like this for DPAMT:
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 13ad2663488b..13e68d375065 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Automatically generated functions to read TDX global metadata.
+ * Functions to read TDX global metadata.
*
* This file doesn't compile on its own as it lacks of inclusion
* of SEAMCALL wrapper primitive which reads global metadata.
@@ -18,6 +18,17 @@ static int get_tdx_sys_info_features(struct
tdx_sys_info_features *sysinfo_featu
return ret;
}
+static int get_tdx_sys_info_tdmr_dpamt(struct tdx_sys_info_tdmr *sysinfo_tdmr)
+{
+ int ret = 0;
+ u64 val;
+
+ if (!ret && !(ret = read_sys_metadata_field(0x9100000100000013, &val)))
+ sysinfo_tdmr->pamt_page_bitmap_entry_bits = val;
+
+ return ret;
+}
+
static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
{
int ret = 0;
@@ -94,5 +105,12 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
+ /*
+ * Don't treat a module that doesn't support Dynamic PAMT
+ * as a failure. Only read the metadata optionally.
+ */
+ if (tdx_supports_dynamic_pamt(sysinfo))
+ ret = ret ?: get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
+
return ret;
}
Wait, looking at the later patches, in the post update caller it will refer to
the old sysinfo instead of the new one? It assumes a new module will not lose
runtime update ability?
Rest of the patch LGTM.
> +
> + if (!ret && !(ret = read_sys_metadata_field(0x8900000100000000, &val)))
> + sysinfo_handoff->module_hv = val;
> +
> + return ret;
> +}
> +
> static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> {
> int ret = 0;
> @@ -115,6 +129,7 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
> ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
> + ret = ret ?: get_tdx_sys_info_handoff(&sysinfo->handoff);
>
> return ret;
> }
^ permalink raw reply related
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Edgecombe, Rick P @ 2026-03-12 2:00 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-12-chao.gao@intel.com>
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?
> 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.
>
> 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.
> 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.
>
> 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.
>
> 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>
> ---
> v2:
> - refine the changlog to follow context-problem-solution structure
> - move alternative discussions at the end of the changelog
> - add a comment about state machine transition
> - Move rcu_momentary_eqs() call to the else branch.
> ---
> arch/x86/virt/vmx/tdx/seamldr.c | 70 ++++++++++++++++++++++++++++++++-
> 1 file changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 718cb8396057..21d572d75769 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -10,8 +10,10 @@
> #include <linux/cpuhplock.h>
> #include <linux/cpumask.h>
> #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>
>
> @@ -186,6 +188,68 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> return alloc_seamldr_params(module, module_size, sig, sig_size);
> }
>
> +/*
> + * During a TDX Module update, all CPUs start from TDP_START and progress
> + * to TDP_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 tdp_state {
> + TDP_START,
> + TDP_DONE,
> +};
> +
> +static struct {
> + enum tdp_state state;
> + atomic_t thread_ack;
> +} tdp_data;
> +
> +static void set_target_state(enum tdp_state state)
> +{
> + /* Reset ack counter. */
> + atomic_set(&tdp_data.thread_ack, num_online_cpus());
> + /* Ensure thread_ack is updated before the new state */
> + smp_wmb();
> + WRITE_ONCE(tdp_data.state, state);
> +}
> +
> +/* Last one to ack a state moves to the next state. */
> +static void ack_state(void)
> +{
> + if (atomic_dec_and_test(&tdp_data.thread_ack))
> + set_target_state(tdp_data.state + 1);
> +}
> +
> +/*
> + * See multi_cpu_stop() from where this multi-cpu state-machine was
> + * adopted, and the rationale for touch_nmi_watchdog()
> + */
> +static int do_seamldr_install_module(void *params)
> +{
> + 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".
> + default:
> + break;
> + }
> + ack_state();
> + } else {
> + touch_nmi_watchdog();
> + rcu_momentary_eqs();
> + }
> + } while (curstate != TDP_DONE);
> +
> + return ret;
> +}
> +
> DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
> if (!IS_ERR_OR_NULL(_T)) free_seamldr_params(_T))
>
> @@ -223,7 +287,11 @@ int seamldr_install_module(const u8 *data, u32 size)
> return -EBUSY;
> }
>
> - /* TODO: Update TDX Module here */
> + set_target_state(TDP_START + 1);
> + ret = stop_machine_cpuslocked(do_seamldr_install_module, params, cpu_online_mask);
> + if (ret)
> + return ret;
> +
> return 0;
> }
> EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
From: Jason Gunthorpe @ 2026-03-12 0:34 UTC (permalink / raw)
To: Jiri Pirko
Cc: Leon Romanovsky, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <phry3e2dtgxzxdqvrnqfuskangp4al64f2auithwme5kwkgepe@7qtftrhgv4l7>
On Mon, Mar 09, 2026 at 06:51:21PM +0100, Jiri Pirko wrote:
> Mon, Mar 09, 2026 at 04:18:57PM +0100, jgg@ziepe.ca wrote:
> >On Mon, Mar 09, 2026 at 04:02:33PM +0200, Leon Romanovsky wrote:
> >> On Mon, Mar 09, 2026 at 10:15:30AM -0300, Jason Gunthorpe wrote:
> >> > On Sun, Mar 08, 2026 at 12:19:48PM +0200, Leon Romanovsky wrote:
> >> >
> >> > > > +/*
> >> > > > + * DMA_ATTR_CC_DECRYPTED: Indicates memory that has been explicitly decrypted
> >> > > > + * (shared) for confidential computing guests. The caller must have
> >> > > > + * called set_memory_decrypted(). A struct page is required.
> >> > > > + */
> >> > > > +#define DMA_ATTR_CC_DECRYPTED (1UL << 12)
> >> > >
> >> > > While adding the new attribute is fine, I would expect additional checks in
> >> > > dma_map_phys() to ensure the attribute cannot be misused. For example,
> >> > > WARN_ON(attrs & (DMA_ATTR_CC_DECRYPTED | DMA_ATTR_MMIO)), along with a check
> >> > > that we are taking the direct path only.
> >> >
> >> > DECRYPYED and MMIO is something that needs to work, VFIO (inside a
> >> > TVM) should be using that combination.
> >>
> >> So this sentence "A struct page is required" from the comment above is
> >> not accurate.
> >
> >It would be clearer to say "Unless DMA_ATTR_MMIO is provided a struct
> >page is required"
> >
> >We need to audit if that works properly, IIRC it does, but I don't
> >remember.. Jiri?
>
> How can you do set_memory_decrypted if you don't have page/folio ?
Alot of device MMIO is decrypted by nature and can't be encrypted, so
you'd have to use both flags. eg in VFIO we'd want to do this.
Jason
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Dave Hansen @ 2026-03-11 22:20 UTC (permalink / raw)
To: Kalra, Ashish, Sean Christopherson
Cc: tglx, mingo, bp, dave.hansen, x86, hpa, peterz, thomas.lendacky,
herbert, davem, ardb, pbonzini, aik, Michael.Roth, KPrateek.Nayak,
Tycho.Andersen, Nathan.Fontenot, jackyli, pgonda, rientjes,
jacobhxu, xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj,
john.allen, darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cc9bf918-a14b-4619-a084-3f424fa16ea1@amd.com>
On 3/11/26 14:24, Kalra, Ashish wrote:
...
> There are 2 active SNP VMs here, with one SNP VM being terminated, the other SNP VM is still running, both VMs are configured with 100GB guest RAM:
>
> When this loop is executed when the SNP guest terminates:
>
> [ 232.789187] SEV-SNP: RMPOPT execution time 391609638 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~391 ms
>
> [ 234.647462] SEV-SNP: RMPOPT execution time 457933019 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~457 ms
That's better, but it's not quite what am looking for.
The most important case (IMNHO) is when RMPOPT falls flat on its face:
it tries to optimize the full 2TB of memory and manages to optimize nothing.
I doubt that two 100GB VMs will get close to that case. It's
theoretically possible, but unlikely.
You also didn't mention 4k vs. 2M vs. 1G mappings.
> Now, there are a couple of additional RMPOPT optimizations which can be applied to this loop :
>
> 1). RMPOPT can skip the bulk of its work if another CPU has already optimized that region.
> The optimal thing may be to optimize all memory on one CPU first, and then let all the others
> run RMPOPT in parallel.
Ahh, so the RMP table itself caches the result of the RMPOPT in its 1G
metadata, then the CPUs can just copy it into their core-local
optimization table at RMPOPT time?
That's handy.
*But*, for the purposes of finding pathological behavior, it's actually
contrary to what I think I was asking for which was having all 1G pages
filled with some private memory. If the system was in the state I want
to see tested, that optimization won't function.
> [ 363.926595] SEV-SNP: RMPOPT execution time 317016656 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~317 ms
>
> [ 365.415243] SEV-SNP: RMPOPT execution time 369659769 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~369 ms.
>
> So, with these two optimizations applied, there is like a ~16-20% performance improvement (when SNP guest terminates) in the execution of this loop
> which is executing RMPOPT on upto 2TB of RAM on all CPUs.
>
> Any thoughts, feedback on the performance numbers ?
16-20% isn't horrible, but it isn't really a fundamental change.
It would also be nice to see elapsed time for each CPU. Having one
pegged CPU for 400ms and 99 mostly idle ones is way different than
having 100 pegged CPUs for 400ms.
That's why I was interested in "how long it takes per-cpu".
But you could get some pretty good info with your new optimized loop:
start = ktime_get();
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE)
rmpopt() // current CPU
middle = ktime_get();
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE)
on_each_cpu_mask(...) // remote CPUs
end = ktime_get();
If you do that ^ with a system:
1. full of private memory
2. empty of private memory
3. empty again
You'll hopefully see:
1. RMPOPT fall on its face. Worst case scenario (what I want to
see most)
2. RMPOPT sees great success, but has to scan the RMP at least
once. Remote CPUs get a free ride on the first CPU's scan.
Largest (middle-start) vs. (end-middle)/nr_cpus delta.
3. RMPOPT best case. Everything is already optimized.
> Ideally we should be issuing RMPOPTs to only optimize the 1G regions that contained memory associated with that guest and that should be
> significantly less than the whole 2TB RAM range.
>
> But that is something we planned for 1GB hugetlb guest_memfd support getting merged and which i believe has dependency on:
> 1). in-place conversion for guest_memfd,
> 2). 2M hugepage support for guest_memfd and finally
> 3). 1GB hugeTLB support for guest_memfd.
It's a no-brainer to do RMPOPT when you have 1GB pages around. You'll
see zero argument from me.
Doing things per-guest and for smaller pages gets a little bit harder to
reason about. In the end, this is all about trying to optimize against
the RMP table which is a global resource. It's going to get wonky if
RMPOPT is driven purely by guest-local data. There are lots of potential
pitfalls.
For now, let's just do it as simply as possible. Get maximum bang for
our buck with minimal data structures and see how that works out. It
might end up being a:
queue_delayed_work()
to do some cleanup a few seconds out after each SNP guest terminates. If
a bunch of guests terminate all at once it'll at least only do a single
set of IPIs.
^ permalink raw reply
* Re: [PATCH v4 24/24] [NOT-FOR-REVIEW] x86/virt/seamldr: Save and restore current VMCS
From: Huang, Kai @ 2026-03-11 22:06 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <abFlCkNd7tqaUyAP@intel.com>
> static const struct x86_cpu_id tdx_host_ids[] = {
> X86_MATCH_FEATURE(X86_FEATURE_TDX_HOST_PLATFORM, NULL),
> @@ -175,6 +177,7 @@ static int seamldr_init(struct device *dev)
> {
> const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> struct fw_upload *tdx_fwl;
> + u64 basic_msr;
>
> if (WARN_ON_ONCE(!tdx_sysinfo))
> return -EIO;
> @@ -182,6 +185,15 @@ static int seamldr_init(struct device *dev)
> if (!tdx_supports_runtime_update(tdx_sysinfo))
> return 0;
>
> + /*
> + * Some TDX-capable CPUs have an erratum where the current VMCS may
> + * be cleared after calling into P-SEAMLDR. Ensure no such erratum
> + * exists before exposing any P-SEAMLDR functions.
> + */
> + rdmsrq(MSR_IA32_VMX_BASIC, basic_msr);
> + if (!(basic_msr & VMX_BASIC_PRESERVE_CURRENT_VMCS))
> + return 0;
> +
IIUC this silently disables runtime update and user won't be able to have
any clue to tell what went wrong (while the user can see the module supports
this feature and apparently the kernel should support it)?
Since we already have a X86_BUG_TDX_PW_MCE which is detected during kernel
boot in tdx_init(), shouldn't we just follow so that the user can at least
see the CPU has this erratum?
Another advantage is, if in the future some other kernel code needs to know
this erratum, it can just consult this flag.
And btw,
Which code base was this patch generated? If I read correctly, in this
series seamldr_init() is a void function but doesn't return anything.
^ permalink raw reply
* Re: [PATCH v2 5/7] KVM: guest_memfd: Add cleanup interface for guest teardown
From: Kalra, Ashish @ 2026-03-11 21:49 UTC (permalink / raw)
To: Ackerley Tng, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <CAEvNRgGdaA1ynF8jxQDPh9U0U8Q0RkE0=KJx4FNrh_=+dVRaLQ@mail.gmail.com>
Hello Ackerley,
On 3/11/2026 1:00 AM, Ackerley Tng wrote:
> "Kalra, Ashish" <ashish.kalra@amd.com> writes:
>
>> Hello Ackerley,
>>
>> On 3/9/2026 4:01 AM, Ackerley Tng wrote:
>>> Ashish Kalra <Ashish.Kalra@amd.com> writes:
>>>
>>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>>
>>>> Introduce kvm_arch_gmem_cleanup() to perform architecture-specific
>>>> cleanups when the last file descriptor for the guest_memfd inode is
>>>> closed. This typically occurs during guest shutdown and termination
>>>> and allows for final resource release.
>>>>
>>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>>> ---
>>>>
>>>> [...snip...]
>>>>
>>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>>> index 017d84a7adf3..2724dd1099f2 100644
>>>> --- a/virt/kvm/guest_memfd.c
>>>> +++ b/virt/kvm/guest_memfd.c
>>>> @@ -955,6 +955,14 @@ static void kvm_gmem_destroy_inode(struct inode *inode)
>>>>
>>>> static void kvm_gmem_free_inode(struct inode *inode)
>>>> {
>>>> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CLEANUP
>>>> + /*
>>>> + * Finalize cleanup for the inode once the last guest_memfd
>>>> + * reference is released. This usually occurs after guest
>>>> + * termination.
>>>> + */
>>>> + kvm_arch_gmem_cleanup();
>>>> +#endif
>>>
>>> Folks have already talked about the performance implications of doing
>>> the scan and rmpopt, I just want to call out that one VM could have more
>>> than one associated guest_memfd too.
>>
>> Yes, i have observed that kvm_gmem_free_inode() gets invoked multiple times
>> at SNP guest shutdown.
>>
>> And the same is true for kvm_gmem_destroy_inode() too.
>>
>>>
>>> I think the cleanup function should be thought of as cleanup for the
>>> inode (even if it doesn't take an inode pointer since it's not (yet)
>>> required).
>>>
>>> So, the gmem cleanup function should not handle deduplicating cleanup
>>> requests, but the arch function should, if the cleanup needs
>>> deduplicating.
>>
>> I agree, the arch function will have to handle deduplicating, and for that
>> the arch function will probably need to be passed the inode pointer,
>> to have a parameter to assist with deduplicating.
>>
>
> By the time .free_folio() is called, folio->mapping may no longer exist,
> so if we definitely want to deduplicate using something in the inode,
> .free_folio() won't be the right callback to use.
Ok.
>
> I was thinking that deduplicating using something in the folio would be
> better. Can rmpopt take a PFN range? Then there's really no
> deduplication, the cleanup would be nicely narrowed to whatever was just
> freed. Perhaps the PFNs could be aligned up to the nearest PMD or PUD
> size for rmpopt to do the right thing.
>
It will really be ideal if the cleanup can be narrowed down to whatever was just freed.
RMPOPT takes a SPA which is GB aligned, so if the PFNs are aligned to the nearest
PUD, then RMPOPT will be perfectly aligned to optimize the 1G regions that contained
memory associated with that guest being freed.
This will also be the most optimal way to use RMPOPT, as we only optimize the 1G regions
that contains memory associated with that guest, which should be much smaller than
optimizing the whole 2TB RAM.
And that's what the actual plans for RMPOPT are.
We had planned for a phased RMPOPT implementation.
In the first phase, we were planning to do RMP re-optimizations for entire 2TB
RAM.
Once 1GB hugetlb guest_memfd support is merged, we planned to support re-enabling
RMPOPT optimizations during 1GB page cleanup as a follow-on series.
But i believe this support is dependent on:
1). in-place conversion for guest_memfd,
2). 2M hugepage support for guest_memfd.
Another alternative we are considering is implementing a bitmap of 1GB regions in guest_memfd
that tracks when they are being freed and then issue RMPOPT on those 1GB regions.
(and this will be independent of the 1GB hugeTLB support for guest_memfd).
> Or perhaps some more tracking is required to check that the entire
> aligned range is freed before doing the rmpopt.
>
> I need to implement some of this tracking for guest_memfd HugeTLB
> support, so if the tracking is useful for you, we should discuss!
Yes, this tracking is going to be useful for RMPOPT.
Is this going to be implemented as part of the 1GB hugeTLB support for guest_memfd ?
>
>>>
>>> Also, .free_inode() is called through RCU, so it could be called after
>>> some delay. Could it be possible that .free_inode() ends up being called
>>> way after the associated VM gets torn down, or after KVM the module gets
>>> unloaded? Does rmpopt still work fine if KVM the module got unloaded?
>>
>> Yes, .free_inode() can probably get called after the associated VM has
>> been torn down and which should be fine for issuing RMPOPT to do
>> RMP re-optimizations.
>>
>> As far as about KVM module getting unloaded, then as part of the forthcoming patch-series,
>> during KVM module unload, X86_SNP_SHUTDOWN would be issued which means SNP would get
>> disabled and therefore, RMP checks are also disabled.
>>
>> And as CC_ATTR_HOST_SEV_SNP would then be cleared, therefore, snp_perform_rmp_optimization()
>> will simply return.
>>
>
> I think relying on CC_ATTR_HOST_SEV_SNP to skip optimization should be
> best as long as there are no races (like the .free_inode() will
> definitely not try to optimize when SNP is half shut down or something
> like that.
Yeah, i will have to take a look at such races.
>
>> Another option is to add a new guest_memfd superblock operation, and then do the
>> final guest_memfd cleanup using the .evict_inode() callback. This will then ensure
>> that the cleanup is not called through RCU and avoids any kind of delays, as following:
>>
>> +static void kvm_gmem_evict_inode(struct inode *inode)
>> +{
>> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CLEANUP
>> + kvm_arch_gmem_cleanup();
>> +#endif
>> + truncate_inode_pages_final(&inode->i_data);
>> + clear_inode(inode);
>> +}
>> +
>>
>
> At the point of .evict_inode(), CoCo-shared guest_memfd pages could
> still be pinned (for DMA or whatever, accidentally or maliciously), can
> rmpopt work on shared pages that might still be used for DMA?
>
Yes, RMPOPT should be safe to work here, as it checks the RMP table for assigned
or private pages in the 1GB range specified. For a 1GB range full of shared pages,
it will mark that range to be RMP optimized.
If all RMPUPDATE's for all private->shared pages conversion have been completed at
the point of .evict_inode(), then RMPOPT re-optimizations will work nicely.
> .invalidate_folio() and .free_folio() both actually happen on removal
> from guest_memfd ownership, though both are not exactly when the folio
> is completely not in use.
>
> Is the best time to optimize when the pages are truly freed?
>
Yes.
Thanks,
Ashish
>> @@ -971,6 +979,7 @@ static const struct super_operations kvm_gmem_super_operations = {
>> .alloc_inode = kvm_gmem_alloc_inode,
>> .destroy_inode = kvm_gmem_destroy_inode,
>> .free_inode = kvm_gmem_free_inode,
>> + .evict_inode = kvm_gmem_evict_inode,
>> };
>>
>>
>> Thanks,
>> Ashish
>>
>>>
>>> IIUC the current kmem_cache_free(kvm_gmem_inode_cachep, GMEM_I(inode));
>>> is fine because in kvm_gmem_exit(), there is a rcu_barrier() before
>>> kmem_cache_destroy(kvm_gmem_inode_cachep);.
>>>
>>>> kmem_cache_free(kvm_gmem_inode_cachep, GMEM_I(inode));
>>>> }
>>>>
>>>> --
>>>> 2.43.0
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Paolo Bonzini @ 2026-03-11 19:01 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Chang S . Bae
In-Reply-To: <20260311003346.2626238-1-seanjc@google.com>
On 3/11/26 01:33, Sean Christopherson wrote:
> Clean up KVM's register tracking and storage in preparation for landing APX,
> which expands the maximum number of GPRs from 16 to 32.
>
> This is kinda sorta an RFC, as there are some very opinionated changes. I.e.
> if you dislike something, please speak up.
>
> My thought is to treat R16-R31 as much like other GPRs as possible (though
> maybe we don't need to expand regs[] as sketched out in the last patch?).
The cleanups in patches 1-4 are nice.
For APX specifically, in abstract it's nice to treat R16-R31 as much as
possible as regular GPRs. 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...
Paolo
^ permalink raw reply
* Re: [PATCH v2 3/7] x86/sev: add support for RMPOPT instruction
From: Kalra, Ashish @ 2026-03-11 21:24 UTC (permalink / raw)
To: Dave Hansen, Sean Christopherson
Cc: tglx, mingo, bp, dave.hansen, x86, hpa, peterz, thomas.lendacky,
herbert, davem, ardb, pbonzini, aik, Michael.Roth, KPrateek.Nayak,
Tycho.Andersen, Nathan.Fontenot, jackyli, pgonda, rientjes,
jacobhxu, xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj,
john.allen, darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <d7ba3790-a959-4150-87e0-c87dea4d09c5@intel.com>
Hello Dave and Sean,
On 3/5/2026 1:40 PM, Dave Hansen wrote:
> On 3/5/26 11:22, Kalra, Ashish wrote:
>> But, these are the performance numbers you should be considering :
>>
>> RMPOPT during boot:
>>
>> [ 49.913402] SEV-SNP: RMPOPT largest cycles 1143020
>> [ 49.913407] SEV-SNP: RMPOPT smallest cycles 60
>> [ 49.913408] SEV-SNP: RMPOPT average cycles 5226
>>
>> RMPOPT after SNP guest shutdown:
>>
>> [ 276.435091] SEV-SNP: RMPOPT largest cycles 83680
>> [ 276.435096] SEV-SNP: RMPOPT smallest cycles 60
>> [ 276.435097] SEV-SNP: RMPOPT average cycles 5658
>
> First of all, I'd really appreciate wall clock measurements on these.
> It's just less math and guesswork. Cycles are easy to measure but hard
> to read. Please make these easier to read. Also, the per-RMPOPT numbers
> don't mean much. You have to scale it by the number of CPUs and memory
> (or 2TB) to get to a real, useful number.
>
> The thing that matters is how long this loop takes:
>
> for (pa = pa_start; pa < pa_end; pa += PUD_SIZE)
>
> and *especially* how long it takes per-cpu and when the system has a
> full 2TB load of memory.
>
> That will tell us how many resources this RMPOPT thing is going to take,
> which is the _real_ thing we need to know.
>
> Also, to some degree, the thing we care about here the *most* is the
> worst case scenario. I think the worst possible case is that there's one
> 4k private page in each 1GB of memory, and that it's the last 4k page.
> I'd like to see numbers for something close to *that*, not when there
> are no private pages.
>
> The two things you measured above are interesting, but they're only part
> of the story.
>
Here is the concerned performance data:
All these measurements are done with 2TB RAM installed on the server:
$ free -h
total used free shared buff/cache available
Mem: 2.0Ti 13Gi 1.9Ti 8.8Mi 1.6Gi 1.9Ti
Swap: 2.0Gi 0B 2.0Gi
For the loop executing RMPOPT on up-to 2TB of RAM on all CPUs:
..
start = ktime_get();
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE) {
/* Bit zero passes the function to the RMPOPT instruction. */
on_each_cpu_mask(cpu_online_mask, rmpopt,
(void *)(pa | RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS),
true);
}
end = ktime_get();
elapsed_ns = ktime_to_ns(ktime_sub(end, start));
...
There are 2 active SNP VMs here, with one SNP VM being terminated, the other SNP VM is still running, both VMs are configured with 100GB guest RAM:
When this loop is executed when the SNP guest terminates:
[ 232.789187] SEV-SNP: RMPOPT execution time 391609638 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~391 ms
[ 234.647462] SEV-SNP: RMPOPT execution time 457933019 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~457 ms
Now, there are a couple of additional RMPOPT optimizations which can be applied to this loop :
1). RMPOPT can skip the bulk of its work if another CPU has already optimized that region.
The optimal thing may be to optimize all memory on one CPU first, and then let all the others
run RMPOPT in parallel.
2). The other optimization being applied here is only executing RMPOPT on only thread per
core.
The code sequence being used here:
...
/* Only one thread per core needs to issue RMPOPT instruction */
for_each_online_cpu(cpu) {
if (!topology_is_primary_thread(cpu))
continue;
cpumask_set_cpu(cpu, cpus);
}
while (!kthread_should_stop()) {
...
start = ktime_get();
/*
* RMPOPT is optimized to skip the bulk of its work if another CPU has already
* optimized that region. Optimize all memory on one CPU first, and then let all
* the others run RMPOPT in parallel.
*/
cpumask_clear_cpu(smp_processor_id(), cpus);
/* current CPU */
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE)
rmpopt((void *)(pa | RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS));
for (pa = pa_start; pa < pa_end; pa += PUD_SIZE) {
/* Bit zero passes the function to the RMPOPT instruction. */
on_each_cpu_mask(cpus, rmpopt,
(void *)(pa | RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS),
true);
}
end = ktime_get();
elapsed_ns = ktime_to_ns(ktime_sub(end, start));
...
With these optimizations applied:
When this loop is executed when an SNP guest terminates, again with 2 active SNP VMs with 100GB guest RAM:
[ 363.926595] SEV-SNP: RMPOPT execution time 317016656 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~317 ms
[ 365.415243] SEV-SNP: RMPOPT execution time 369659769 ns for physical address range 0x0000000000000000 - 0x0000020000000000 on all cpus -> ~369 ms.
So, with these two optimizations applied, there is like a ~16-20% performance improvement (when SNP guest terminates) in the execution of this loop
which is executing RMPOPT on upto 2TB of RAM on all CPUs.
Any thoughts, feedback on the performance numbers ?
Ideally we should be issuing RMPOPTs to only optimize the 1G regions that contained memory associated with that guest and that should be
significantly less than the whole 2TB RAM range.
But that is something we planned for 1GB hugetlb guest_memfd support getting merged and which i believe has dependency on:
1). in-place conversion for guest_memfd,
2). 2M hugepage support for guest_memfd and finally
3). 1GB hugeTLB support for guest_memfd.
The other alternative probably will be to use Dave's suggestions to loosely mirror the RMPOPT bitmap and
keep our own bitmap of 1GB regions that _need_ RMPOPT run on them and probably this bitmap lives in
guest_memfd and we track when they are being freed and then issue RMPOPT on those 1GB regions
(and this will be independent of the 1GB hugeTLB support for guest_memfd).
Thanks,
Ashish
^ permalink raw reply
* Re: [PATCH v12 06/46] arm64: RMI: Define the user ABI
From: Marc Zyngier @ 2026-03-11 19:10 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Catalin Marinas, 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, Aneesh Kumar K . V, Emi Kisanuki,
Vishal Annapurve
In-Reply-To: <33053e22-6cc6-4d55-bc7f-01f873a15d28@arm.com>
On Mon, 02 Mar 2026 15:23:44 +0000,
Steven Price <steven.price@arm.com> wrote:
>
> >> + struct kvm_arm_rmi_populate {
> >> + __u64 base;
> >> + __u64 size;
> >> + __u64 source_uaddr;
> >> + __u32 flags;
> >> + __u32 reserved;
> >> + };
> >> +
> >> +Populate a region of protected address space by copying the data from the user
> >> +space pointer provided. This is only valid before any VCPUs have been run.
> >> +The ioctl might not populate the entire region and user space may have to
> >> +repeatedly call it (with updated pointers) to populate the entire region.
> >
> > size as a __u64 is odd, as the return value from the ioctl is a signed
> > int. This implies that you can't really report how many bytes you have
> > copied. Some form of consistency wouldn't hurt.
>
> Good spot. In practice this works because >2GB in one operation is
> highly unlikely to be processed in one go. But I guess I'll change this
> to have an output size argument. I guess I could make the kernel update
> all of base,size,source_uaddr which would simplify user space.
In a conversation with Suzuki, I suggested that splice(2) could be a
nicer way to express this, and allow asynchronous use with io-uring.
After all, having a guestmem backend for CCA is not exactly
outlandish, and having a splice implementation realistic enough.
Thoughts?
M.
--
Jazz isn't dead. It just smells funny.
^ permalink raw reply
* Re: [PATCH 4/7] KVM: x86: Add wrapper APIs to reset dirty/available register masks
From: Paolo Bonzini @ 2026-03-11 18:50 UTC (permalink / raw)
To: Sean Christopherson, Yosry Ahmed
Cc: Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel,
Chang S . Bae
In-Reply-To: <abFulxXuRziXj039@google.com>
On 3/11/26 14:31, Sean Christopherson wrote:
>> Not closely following this series and don't know this code well, but
>> this API is very confusing for me tbh. Especially in comparison with
>> kvm_reset_dirty_registers().
>>
>> Maybe rename this to kvm_clear_available_registers(), and pass in a
>> "clear_mask", then reverse the polarity:
>>
>> vcpu->arch.regs_avail &= ~clear_mask;
> Oh, yeah, I can do something like that. I originally misread the TDX code and
> thought it was explicitly setting regs_avail, and so came up with a roundabout
> name. I didn't revisit the naming or the polarity of the param once I realized
> all callers could use the same scheme.
>
> No small part of me is tempted to turn it into a straigh "set" though, unless I'm
> missing something, the whole &= business is an implementation quirk.
I like kvm_clear_available_registers() for this + removing the second
argument completely for kvm_reset_dirty_registers().
Paolo
^ permalink raw reply
* Re: [PATCH 2/7] KVM: x86: Drop the "EX" part of "EXREG" to avoid collision with APX
From: Paolo Bonzini @ 2026-03-11 18:46 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Chang S . Bae
In-Reply-To: <20260311003346.2626238-3-seanjc@google.com>
On 3/11/26 01:33, Sean Christopherson wrote:
> Now that NR_VCPU_REGS is no longer a thing, drop the "EX" is for
> extended (or maybe extra?") prefix from non-GRP registers to avoid a
> collision with APX (Advanced Performance Extensions), which adds:
>
> 16 additional general-purpose registers (GPRs) R16–R31, also referred
> to as Extended GPRs (EGPRs) in this document;
And also, now that RIP is effectively an EXREG.
Paolo
^ permalink raw reply
* Re: [PATCH 4/7] KVM: x86: Add wrapper APIs to reset dirty/available register masks
From: Yosry Ahmed @ 2026-03-11 18:28 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel, Chang S . Bae
In-Reply-To: <abFulxXuRziXj039@google.com>
On Wed, Mar 11, 2026 at 6:31 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Mar 10, 2026, Yosry Ahmed wrote:
> > On Tue, Mar 10, 2026 at 5:34 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > Add wrappers for setting regs_{avail,dirty} in anticipation of turning the
> > > fields into proper bitmaps, at which point direct writes won't work so
> > > well.
> > >
> > > Deliberately leave the initialization in kvm_arch_vcpu_create() as-is,
> > > because the regs_avail logic in particular is special in that it's the one
> > > and only place where KVM marks eagerly synchronized registers as available.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > > arch/x86/kvm/kvm_cache_regs.h | 19 +++++++++++++++++++
> > > arch/x86/kvm/svm/svm.c | 4 ++--
> > > arch/x86/kvm/vmx/nested.c | 4 ++--
> > > arch/x86/kvm/vmx/tdx.c | 2 +-
> > > arch/x86/kvm/vmx/vmx.c | 4 ++--
> > > 5 files changed, 26 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
> > > index ac1f9867a234..94e31cf38cb8 100644
> > > --- a/arch/x86/kvm/kvm_cache_regs.h
> > > +++ b/arch/x86/kvm/kvm_cache_regs.h
> > > @@ -105,6 +105,25 @@ static __always_inline bool kvm_register_test_and_mark_available(struct kvm_vcpu
> > > return arch___test_and_set_bit(reg, (unsigned long *)&vcpu->arch.regs_avail);
> > > }
> > >
> > > +static __always_inline void kvm_reset_available_registers(struct kvm_vcpu *vcpu,
> > > + u32 available_mask)
> >
> > Not closely following this series and don't know this code well, but
> > this API is very confusing for me tbh. Especially in comparison with
> > kvm_reset_dirty_registers().
> >
> > Maybe rename this to kvm_clear_available_registers(), and pass in a
> > "clear_mask", then reverse the polarity:
> >
> > vcpu->arch.regs_avail &= ~clear_mask;
>
> Oh, yeah, I can do something like that. I originally misread the TDX code and
> thought it was explicitly setting regs_avail, and so came up with a roundabout
> name. I didn't revisit the naming or the polarity of the param once I realized
> all callers could use the same scheme.
>
> No small part of me is tempted to turn it into a straigh "set" though, unless I'm
> missing something, the whole &= business is an implementation quirk.
Not sure what you mean here, this (for example)?
vcpu->arch.regs_avail = ~SVM_REGS_LAZY_LOAD_SET;
Does this mean all other bits in regs_avail should already be set for
all users so the &= is unnecessary? Or it doesn't matter if they're
set or not?
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
From: Jiri Pirko @ 2026-03-11 14:19 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Petr Tesarik, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, leon,
sean.anderson, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <20260309131736.GK1687929@ziepe.ca>
Mon, Mar 09, 2026 at 02:17:36PM +0100, jgg@ziepe.ca wrote:
>On Mon, Mar 09, 2026 at 01:56:10PM +0100, Petr Tesarik wrote:
>> I don't want to start a bikeshedding discussion, so if everyone else
>> likes this name, let's keep it. But maybe the "_CC" (meaning
>> Confidential Comptuing) is not necessary. IIUC it's the same concept as
>> set_page_encrypted(), set_page_decrypted(), which does not refer to
>> CoCo either.
>
>Frankly I hate that AMD got their "encrypted" "decrypted" naming baked
>into the CC related APIs.
>
>I'm not at all convinced that they "do not refer to CoCo" in the way
>Linux uses them and other arches absolutely make them 100% tied to coco.
>
>If we are going to bikeshed the name it should be DMA_ATTR_CC_SHARED
On the other hand, the encrypted/decrypted helpers could be always
renamed if it makes sense. Better to perhaps have DMA_ATTR_DECRYPTED to
have things consistently named now? If someone renames them all in the
future, so be it.
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: SEV: Add support for SNP BTB Isolation
From: Sean Christopherson @ 2026-03-11 14:15 UTC (permalink / raw)
To: Kim Phillips
Cc: linux-kernel, kvm, linux-coco, x86, Paolo Bonzini,
K Prateek Nayak, Nikunj A Dadhania, Tom Lendacky, Michael Roth,
Borislav Petkov, Borislav Petkov, Naveen Rao, David Kaplan,
Pawan Gupta
In-Reply-To: <20260311130611.2201214-4-kim.phillips@amd.com>
On Wed, Mar 11, 2026, Kim Phillips wrote:
> This feature ensures SNP guest Branch Target Buffers (BTBs) are not
> affected by context outside that guest. CPU hardware tracks each
> guest's BTB entries and can flush the BTB if it has been determined
> to be contaminated with any prediction information originating outside
> the particular guest's context.
>
> To mitigate possible performance penalties incurred by these flushes,
> it is recommended that the hypervisor run with SPEC_CTRL[IBRS] set.
> Note that using Automatic IBRS is not an equivalent option here, since
> it behaves differently when SEV-SNP is active. See commit acaa4b5c4c85
> ("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
> for more details.
>
> Indicate support for BTB Isolation in sev_supported_vmsa_features,
> bit 7.
This isn't very useful for the changelog. I can read the patch quite easily.
What would be useful is a description of the change in conversational language,
and an explanation of why it is the correct change. E.g. (not really, but you
get the idea)
Advertise support for BTB Ioslation via SEV_VMSA_FEATURES when SNP is
enabled, as all hardware that supports SNP also support BTB Isolation.
BTB Isolation is an optional feature that can be enabled by the guest to
sprinkle fairy dust on the CPU to completely prevent all speculative
execution attacks.
> SNP-active guests can enable (BTB) Isolation through SEV_Status
> bit 9 (SNPBTBIsolation).
That's not what the doc says:
SNP-active guests may choose to enable the Branch Target Buffer Isolation
mode through SEV_FEATURES bit 7 (BTBIsolation).
> For more info,> refer to page 615, Section 15.36.17 "Side-Channel
> Protection", AMD64 Architecture Programmer's Manual Volume 2: System
> Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
>
> Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>
> ---
> v2: No changes
> v1: https://lore.kernel.org/kvm/20260224180157.725159-4-kim.phillips@amd.com/
>
> arch/x86/include/asm/svm.h | 1 +
> arch/x86/kvm/svm/sev.c | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index edde36097ddc..2038461c1316 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -305,6 +305,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
> #define SVM_SEV_FEAT_RESTRICTED_INJECTION BIT(3)
> #define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
> #define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
> +#define SVM_SEV_FEAT_BTB_ISOLATION BIT(7)
> #define SVM_SEV_FEAT_SECURE_TSC BIT(9)
>
> #define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 3f9c1aa39a0a..ac29cf47dd08 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3167,6 +3167,9 @@ void __init sev_hardware_setup(void)
>
> if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
> sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
> +
> + if (sev_snp_enabled)
If BTB_ISOLATION is actually supported on *all* SNP hardware, then that needs to
be called out. Please also separate this from the core kernel changes, unless
there is some dependency on them. And if there _is_ a dependency, call that out.
Ugh, I'm getting deja vu. I suspect I had a long response typed out for v1 of
this patch, and rebooted my system before actually sending it.
Oh wait, no, you just made the same mistakes in two different patches. Please
revist https://lore.kernel.org/all/aaWog_UjW-M3412C@google.com.
In general, spamming patches without internalizing the feedback makes for grumpy
maintainers.
^ permalink raw reply
* Re: [PATCH 4/7] KVM: x86: Add wrapper APIs to reset dirty/available register masks
From: Sean Christopherson @ 2026-03-11 13:31 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel, Chang S . Bae
In-Reply-To: <CAO9r8zNBEOJrxbx8ob2KRLRKkd_aZjz8tyXGqXxs0=TJq4fU6Q@mail.gmail.com>
On Tue, Mar 10, 2026, Yosry Ahmed wrote:
> On Tue, Mar 10, 2026 at 5:34 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Add wrappers for setting regs_{avail,dirty} in anticipation of turning the
> > fields into proper bitmaps, at which point direct writes won't work so
> > well.
> >
> > Deliberately leave the initialization in kvm_arch_vcpu_create() as-is,
> > because the regs_avail logic in particular is special in that it's the one
> > and only place where KVM marks eagerly synchronized registers as available.
> >
> > No functional change intended.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/kvm/kvm_cache_regs.h | 19 +++++++++++++++++++
> > arch/x86/kvm/svm/svm.c | 4 ++--
> > arch/x86/kvm/vmx/nested.c | 4 ++--
> > arch/x86/kvm/vmx/tdx.c | 2 +-
> > arch/x86/kvm/vmx/vmx.c | 4 ++--
> > 5 files changed, 26 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
> > index ac1f9867a234..94e31cf38cb8 100644
> > --- a/arch/x86/kvm/kvm_cache_regs.h
> > +++ b/arch/x86/kvm/kvm_cache_regs.h
> > @@ -105,6 +105,25 @@ static __always_inline bool kvm_register_test_and_mark_available(struct kvm_vcpu
> > return arch___test_and_set_bit(reg, (unsigned long *)&vcpu->arch.regs_avail);
> > }
> >
> > +static __always_inline void kvm_reset_available_registers(struct kvm_vcpu *vcpu,
> > + u32 available_mask)
>
> Not closely following this series and don't know this code well, but
> this API is very confusing for me tbh. Especially in comparison with
> kvm_reset_dirty_registers().
>
> Maybe rename this to kvm_clear_available_registers(), and pass in a
> "clear_mask", then reverse the polarity:
>
> vcpu->arch.regs_avail &= ~clear_mask;
Oh, yeah, I can do something like that. I originally misread the TDX code and
thought it was explicitly setting regs_avail, and so came up with a roundabout
name. I didn't revisit the naming or the polarity of the param once I realized
all callers could use the same scheme.
No small part of me is tempted to turn it into a straigh "set" though, unless I'm
missing something, the whole &= business is an implementation quirk.
> Most callers are already passing in an inverse of a mask, so might as
> well pass the mask as-is and invert it here, and it helps make the
> name clear, we're passing in a bitmask to clear from regs_avail.
^ permalink raw reply
* [PATCH v2 3/3] KVM: SEV: Add support for SNP BTB Isolation
From: Kim Phillips @ 2026-03-11 13:06 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
In-Reply-To: <20260311130611.2201214-1-kim.phillips@amd.com>
This feature ensures SNP guest Branch Target Buffers (BTBs) are not
affected by context outside that guest. CPU hardware tracks each
guest's BTB entries and can flush the BTB if it has been determined
to be contaminated with any prediction information originating outside
the particular guest's context.
To mitigate possible performance penalties incurred by these flushes,
it is recommended that the hypervisor run with SPEC_CTRL[IBRS] set.
Note that using Automatic IBRS is not an equivalent option here, since
it behaves differently when SEV-SNP is active. See commit acaa4b5c4c85
("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
for more details.
Indicate support for BTB Isolation in sev_supported_vmsa_features,
bit 7.
SNP-active guests can enable (BTB) Isolation through SEV_Status
bit 9 (SNPBTBIsolation).
For more info, refer to page 615, Section 15.36.17 "Side-Channel
Protection", AMD64 Architecture Programmer's Manual Volume 2: System
Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v2: No changes
v1: https://lore.kernel.org/kvm/20260224180157.725159-4-kim.phillips@amd.com/
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index edde36097ddc..2038461c1316 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -305,6 +305,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_RESTRICTED_INJECTION BIT(3)
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
+#define SVM_SEV_FEAT_BTB_ISOLATION BIT(7)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 3f9c1aa39a0a..ac29cf47dd08 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3167,6 +3167,9 @@ void __init sev_hardware_setup(void)
if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+
+ if (sev_snp_enabled)
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_BTB_ISOLATION;
}
void sev_hardware_unsetup(void)
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
From: Kim Phillips @ 2026-03-11 13:06 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, stable
In-Reply-To: <20260311130611.2201214-1-kim.phillips@amd.com>
This is to prepare to allow legacy IBRS toggling on AMD systems,
where the BTB Isolation SEV-SNP feature can use it to optimize the
quick VM exit to re-entry path.
There is no reason this wasn't allowed in the first place, therefore
adding the cc: stable and Fixes: tags.
Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v2: No changes
v1: https://lore.kernel.org/kvm/20260224180157.725159-3-kim.phillips@amd.com/
arch/x86/kernel/cpu/bugs.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 957e0df38d90..c910da561044 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2152,11 +2152,6 @@ static void __init spectre_v2_select_mitigation(void)
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
}
- if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
- pr_err("IBRS selected but not Intel CPU. Switching to AUTO select\n");
- spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
- }
-
if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && !boot_cpu_has(X86_FEATURE_IBRS)) {
pr_err("IBRS selected but CPU doesn't have IBRS. Switching to AUTO select\n");
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
@@ -2251,7 +2246,7 @@ static void __init spectre_v2_apply_mitigation(void)
pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);
if (spectre_v2_in_ibrs_mode(spectre_v2_enabled)) {
- if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS) && spectre_v2_enabled != SPECTRE_V2_IBRS) {
msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
} else {
x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] cpu/bugs: Allow forcing Automatic IBRS with SNP enabled using spectre_v2=eibrs
From: Kim Phillips @ 2026-03-11 13:06 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, stable
In-Reply-To: <20260311130611.2201214-1-kim.phillips@amd.com>
To allow this, do the SNP check in spectre_v2_select_mitigation()
processing instead of the original commit's implementation in
cpu_set_bug_bits().
Since SPECTRE_V2_CMD_AUTO logic falls through to SPECTRE_V2_CMD_FORCE,
double-check if SPECTRE_V2_CMD_FORCE is used before allowing
SPECTRE_V2_EIBRS with SNP enabled.
Also mute SPECTRE_V2_IBRS_PERF_MSG if SNP is enabled on an AutoIBRS
capable machine, since, in that case, the message doesn't apply.
Fixes: acaa4b5c4c85 ("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v2:
- Address Dave Hansen's comment to adhere to using the IBRS_ENHANCED
Intel feature flag also for AutoIBRS.
v1:
https://lore.kernel.org/kvm/20260224180157.725159-2-kim.phillips@amd.com/
arch/x86/kernel/cpu/bugs.c | 12 ++++++++++--
arch/x86/kernel/cpu/common.c | 6 +-----
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 83f51cab0b1e..957e0df38d90 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2181,7 +2181,14 @@ static void __init spectre_v2_select_mitigation(void)
break;
fallthrough;
case SPECTRE_V2_CMD_FORCE:
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+ /*
+ * Unless forced, don't use AutoIBRS when SNP is enabled
+ * because it degrades host userspace indirect branch performance.
+ */
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) &&
+ (!boot_cpu_has(X86_FEATURE_SEV_SNP) ||
+ (boot_cpu_has(X86_FEATURE_SEV_SNP) &&
+ spectre_v2_cmd == SPECTRE_V2_CMD_FORCE))) {
spectre_v2_enabled = SPECTRE_V2_EIBRS;
break;
}
@@ -2261,7 +2268,8 @@ static void __init spectre_v2_apply_mitigation(void)
case SPECTRE_V2_IBRS:
setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) &&
+ !boot_cpu_has(X86_FEATURE_SEV_SNP))
pr_warn(SPECTRE_V2_IBRS_PERF_MSG);
break;
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index bb937bc4b00f..5aff1424a27d 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1486,13 +1486,9 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
/*
* AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
* flag and protect from vendor-specific bugs via the whitelist.
- *
- * Don't use AutoIBRS when SNP is enabled because it degrades host
- * userspace indirect branch performance.
*/
if ((x86_arch_cap_msr & ARCH_CAP_IBRS_ALL) ||
- (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
- !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
+ cpu_has(c, X86_FEATURE_AUTOIBRS)) {
setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
!(x86_arch_cap_msr & ARCH_CAP_PBRSB_NO))
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/3] KVM: SEV: Add support for BTB Isolation
From: Kim Phillips @ 2026-03-11 13:06 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
This feature ensures SNP guest Branch Target Buffers (BTBs) are not
affected by context outside that guest.
The first patch fixes a longstanding bug where users weren't able
to force Automatic IBRS on SNP enabled machines using spectre_v2=eibrs.
The second patch fixes another longstanding bug where users couldn't
select legacy / toggling SPEC_CTRL[IBRS] on AMD systems. Users of
the BTB Isolation feature may use IBRS to mitigate possible
performance degradation caused by BTB Isolation.
The third patch adds support for the feature by adding it to the
supported features bitmask.
Based on tip/master, currently 7726ce228780.
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
This series also available here:
https://github.com/AMDESE/linux/tree/btb-isol-latest
Advance qemu bits (to add btb-isol=on/off switch) available here:
https://github.com/AMDESE/qemu/tree/btb-isol-latest
Qemu bits will be posted upstream once kernel bits are merged.
They depend on Naveen Rao's "target/i386: SEV: Add support for
enabling VMSA SEV features":
https://lore.kernel.org/qemu-devel/cover.1761648149.git.naveen@kernel.org/
v2:
- Patch 1/3:
- Address Dave Hansen's comment to adhere to using the IBRS_ENHANCED
Intel feature flag also for AutoIBRS.
v1:
https://lore.kernel.org/kvm/20260224180157.725159-1-kim.phillips@amd.com/
Kim Phillips (3):
cpu/bugs: Allow forcing Automatic IBRS with SNP enabled using
spectre_v2=eibrs
cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
KVM: SEV: Add support for SNP BTB Isolation
arch/x86/include/asm/svm.h | 1 +
arch/x86/kernel/cpu/bugs.c | 19 +++++++++++--------
arch/x86/kernel/cpu/common.c | 6 +-----
arch/x86/kvm/svm/sev.c | 3 +++
4 files changed, 16 insertions(+), 13 deletions(-)
base-commit: 7726ce2287804e70b2bf2fc00f104530b603d3f3
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4 24/24] [NOT-FOR-REVIEW] x86/virt/seamldr: Save and restore current VMCS
From: Chao Gao @ 2026-03-11 12:50 UTC (permalink / raw)
To: 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-25-chao.gao@intel.com>
On Thu, Feb 12, 2026 at 06:35:27AM -0800, Chao Gao wrote:
>P-SEAMLDR calls clobber the current VMCS as documented in Intel® Trust
>Domain CPU Architectural Extensions (May 2021 edition) Chapter 2.3 [1]:
>
> 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.
>
>Save and restore the current VMCS using VMPTRST and VMPTRLD instructions
>to avoid breaking KVM.
>
>Signed-off-by: Chao Gao <chao.gao@intel.com>
>---
>This patch is needed for testing until microcode is updated to preserve
>the current VMCS across P-SEAMLDR calls. Otherwise, if some normal VMs
>are running before TDX Module updates, vmread/vmwrite errors may occur
>immediately after updates.
The agreed approach is to fix the CPU behavior rather than work around the
issue in the kernel. So, I'll include the following patch to handle this
erratum. Please let me know if you have any concerns.
From 04b53e83dc9daee1866e1c8f26e3d027e1a0be6a 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. Check for
the erratum and refuse to expose P-SEAMLDR features (e.g., TDX module
updates) on 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.
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/vmx.h | 1 +
drivers/virt/coco/tdx-host/tdx-host.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index c85c50019523..d066c50b9051 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -135,6 +135,7 @@
#define VMX_BASIC_INOUT BIT_ULL(54)
#define VMX_BASIC_TRUE_CTLS BIT_ULL(55)
#define VMX_BASIC_NO_HW_ERROR_CODE_CC BIT_ULL(56)
+#define VMX_BASIC_PRESERVE_CURRENT_VMCS BIT_ULL(60)
static inline u32 vmx_basic_vmcs_revision_id(u64 vmx_basic)
{
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 891cc6a083e0..13c23769d09d 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -12,8 +12,10 @@
#include <linux/sysfs.h>
#include <asm/cpu_device_id.h>
+#include <asm/msr.h>
#include <asm/seamldr.h>
#include <asm/tdx.h>
+#include <asm/vmx.h>
static const struct x86_cpu_id tdx_host_ids[] = {
X86_MATCH_FEATURE(X86_FEATURE_TDX_HOST_PLATFORM, NULL),
@@ -175,6 +177,7 @@ static int seamldr_init(struct device *dev)
{
const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
struct fw_upload *tdx_fwl;
+ u64 basic_msr;
if (WARN_ON_ONCE(!tdx_sysinfo))
return -EIO;
@@ -182,6 +185,15 @@ static int seamldr_init(struct device *dev)
if (!tdx_supports_runtime_update(tdx_sysinfo))
return 0;
+ /*
+ * Some TDX-capable CPUs have an erratum where the current VMCS may
+ * be cleared after calling into P-SEAMLDR. Ensure no such erratum
+ * exists before exposing any P-SEAMLDR functions.
+ */
+ rdmsrq(MSR_IA32_VMX_BASIC, basic_msr);
+ if (!(basic_msr & VMX_BASIC_PRESERVE_CURRENT_VMCS))
+ return 0;
+
tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "tdx_module",
&tdx_fw_ops, NULL);
if (IS_ERR(tdx_fwl))
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v2 4/4] dma: direct: set decrypted flag for remapped dma allocations
From: Mostafa Saleh @ 2026-03-11 12:24 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Suzuki K Poulose, linux-kernel, iommu, linux-coco,
Catalin Marinas, will, maz, tglx, robin.murphy, akpm, jgg,
steven.price
In-Reply-To: <yq5abjjl4o0j.fsf@kernel.org>
On Fri, Dec 26, 2025 at 02:29:24PM +0530, Aneesh Kumar K.V wrote:
> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>
> > Suzuki K Poulose <suzuki.poulose@arm.com> writes:
> >
> >> On 21/12/2025 16:09, Aneesh Kumar K.V (Arm) wrote:
> >>> Devices that are DMA non-coherent and need a remap were skipping
> >>> dma_set_decrypted(), leaving buffers encrypted even when the device
> >>> requires unencrypted access. Move the call after the remap
> >>> branch so both paths mark the allocation decrypted (or fail cleanly)
> >>> before use.
> >>>
> >>> Fixes: f3c962226dbe ("dma-direct: clean up the remapping checks in dma_direct_alloc")
> >>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> >>> ---
> >>> kernel/dma/direct.c | 8 +++-----
> >>> 1 file changed, 3 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> >>> index 3448d877c7c6..a62dc25524cc 100644
> >>> --- a/kernel/dma/direct.c
> >>> +++ b/kernel/dma/direct.c
> >>> @@ -271,9 +271,6 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> >>> if (remap) {
> >>> pgprot_t prot = dma_pgprot(dev, PAGE_KERNEL, attrs);
> >>>
> >>> - if (force_dma_unencrypted(dev))
> >>> - prot = pgprot_decrypted(prot);
> >>
> >> This would be problematic, isn't it ? We don't support decrypted on a
> >> vmap area for arm64. If we move this down, we might actually use the
> >> vmapped area. Not sure if other archs are fine with "decrypting" a
> >> "vmap" address.
> >>
> >> If we map the "vmap" address with pgprot_decrypted, we could go ahead
> >> and further map the linear map (i.e., page_address(page)) decrypted
> >> and get everything working.
> >
> > We still have the problem w.r.t free
> >
> > dma_direct_free():
> >
> > if (is_vmalloc_addr(cpu_addr)) {
> > vunmap(cpu_addr);
> > } else {
> > if (dma_set_encrypted(dev, cpu_addr, size))
> > return;
> > }
> >
>
> How about the below change?
>
> commit 8261c528961c6959b85de87c5659ce9081dc85b7
> Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> Date: Fri Dec 19 14:46:20 2025 +0530
>
> dma: direct: set decrypted flag for remapped DMA allocations
>
> Devices that are DMA non-coherent and require a remap were skipping
> dma_set_decrypted(), leaving DMA buffers encrypted even when the device
> requires unencrypted access. Move the call after the if (remap) branch
> so that both direct and remapped allocation paths correctly mark the
> allocation as decrypted (or fail cleanly) before use.
>
> If CMA allocations return highmem pages, treat this as an allocation
> error so that dma_direct_alloc() falls back to the standard allocation
> path. This is required because some architectures (e.g. arm64) cannot
> mark vmap addresses as decrypted, and highmem pages necessarily require
> a vmap remap. As a result, such allocations cannot be safely marked
> unencrypted for DMA.
>
> Other architectures (e.g. x86) do not have this limitation, but instead
> of making this architecture-specific, I have made the restriction apply
> when the device requires unencrypted DMA access. This was done for
> simplicity,
>
> Fixes: f3c962226dbe ("dma-direct: clean up the remapping checks in dma_direct_alloc")
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
Are there any cases this happened in CCA, the only cases I can see
remap is true are:
- PageHighMem(): Where that fails for CCA
- !dev_is_dma_coherent(): AFAIK, all devices with CCA must have an
SMMU, so direct DMA is only for virtualized devices which cannot
be incoherent.
Thanks,
Mostafa
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 7c0b55ca121f..811de37ad81c 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -264,6 +264,15 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> * remapped to return a kernel virtual address.
> */
> if (PageHighMem(page)) {
> + /*
> + * Unencrypted/shared DMA requires a linear-mapped buffer
> + * address to look up the PFN and set architecture-required PFN
> + * attributes. This is not possible with HighMem, so return
> + * failure.
> + */
> + if (force_dma_unencrypted(dev))
> + goto out_free_pages;
> +
> remap = true;
> set_uncached = false;
> }
> @@ -284,7 +293,13 @@ 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 (force_dma_unencrypted(dev)) {
> + void *lm_addr;
> +
> + lm_addr = page_address(page);
> + if (set_memory_decrypted((unsigned long)lm_addr, PFN_UP(size)))
> goto out_leak_pages;
> }
>
> @@ -349,8 +364,16 @@ void dma_direct_free(struct device *dev, size_t size,
> } 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 (force_dma_unencrypted(dev)) {
> + void *lm_addr;
> +
> + lm_addr = phys_to_virt(dma_to_phys(dev, dma_addr));
> + if (set_memory_encrypted((unsigned long)lm_addr, PFN_UP(size))) {
> + pr_warn_ratelimited("leaking DMA memory that can't be re-encrypted\n");
> return;
> + }
> }
>
> __dma_direct_free_pages(dev, dma_direct_to_page(dev, dma_addr), size);
^ permalink raw reply
* Re: [PATCH v2 5/7] KVM: guest_memfd: Add cleanup interface for guest teardown
From: Ackerley Tng @ 2026-03-11 6:00 UTC (permalink / raw)
To: Kalra, Ashish, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <98313534-af6a-4c00-a016-9d9010f145da@amd.com>
"Kalra, Ashish" <ashish.kalra@amd.com> writes:
> Hello Ackerley,
>
> On 3/9/2026 4:01 AM, Ackerley Tng wrote:
>> Ashish Kalra <Ashish.Kalra@amd.com> writes:
>>
>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>
>>> Introduce kvm_arch_gmem_cleanup() to perform architecture-specific
>>> cleanups when the last file descriptor for the guest_memfd inode is
>>> closed. This typically occurs during guest shutdown and termination
>>> and allows for final resource release.
>>>
>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>> ---
>>>
>>> [...snip...]
>>>
>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>> index 017d84a7adf3..2724dd1099f2 100644
>>> --- a/virt/kvm/guest_memfd.c
>>> +++ b/virt/kvm/guest_memfd.c
>>> @@ -955,6 +955,14 @@ static void kvm_gmem_destroy_inode(struct inode *inode)
>>>
>>> static void kvm_gmem_free_inode(struct inode *inode)
>>> {
>>> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CLEANUP
>>> + /*
>>> + * Finalize cleanup for the inode once the last guest_memfd
>>> + * reference is released. This usually occurs after guest
>>> + * termination.
>>> + */
>>> + kvm_arch_gmem_cleanup();
>>> +#endif
>>
>> Folks have already talked about the performance implications of doing
>> the scan and rmpopt, I just want to call out that one VM could have more
>> than one associated guest_memfd too.
>
> Yes, i have observed that kvm_gmem_free_inode() gets invoked multiple times
> at SNP guest shutdown.
>
> And the same is true for kvm_gmem_destroy_inode() too.
>
>>
>> I think the cleanup function should be thought of as cleanup for the
>> inode (even if it doesn't take an inode pointer since it's not (yet)
>> required).
>>
>> So, the gmem cleanup function should not handle deduplicating cleanup
>> requests, but the arch function should, if the cleanup needs
>> deduplicating.
>
> I agree, the arch function will have to handle deduplicating, and for that
> the arch function will probably need to be passed the inode pointer,
> to have a parameter to assist with deduplicating.
>
By the time .free_folio() is called, folio->mapping may no longer exist,
so if we definitely want to deduplicate using something in the inode,
.free_folio() won't be the right callback to use.
I was thinking that deduplicating using something in the folio would be
better. Can rmpopt take a PFN range? Then there's really no
deduplication, the cleanup would be nicely narrowed to whatever was just
freed. Perhaps the PFNs could be aligned up to the nearest PMD or PUD
size for rmpopt to do the right thing.
Or perhaps some more tracking is required to check that the entire
aligned range is freed before doing the rmpopt.
I need to implement some of this tracking for guest_memfd HugeTLB
support, so if the tracking is useful for you, we should discuss!
>>
>> Also, .free_inode() is called through RCU, so it could be called after
>> some delay. Could it be possible that .free_inode() ends up being called
>> way after the associated VM gets torn down, or after KVM the module gets
>> unloaded? Does rmpopt still work fine if KVM the module got unloaded?
>
> Yes, .free_inode() can probably get called after the associated VM has
> been torn down and which should be fine for issuing RMPOPT to do
> RMP re-optimizations.
>
> As far as about KVM module getting unloaded, then as part of the forthcoming patch-series,
> during KVM module unload, X86_SNP_SHUTDOWN would be issued which means SNP would get
> disabled and therefore, RMP checks are also disabled.
>
> And as CC_ATTR_HOST_SEV_SNP would then be cleared, therefore, snp_perform_rmp_optimization()
> will simply return.
>
I think relying on CC_ATTR_HOST_SEV_SNP to skip optimization should be
best as long as there are no races (like the .free_inode() will
definitely not try to optimize when SNP is half shut down or something
like that.
> Another option is to add a new guest_memfd superblock operation, and then do the
> final guest_memfd cleanup using the .evict_inode() callback. This will then ensure
> that the cleanup is not called through RCU and avoids any kind of delays, as following:
>
> +static void kvm_gmem_evict_inode(struct inode *inode)
> +{
> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CLEANUP
> + kvm_arch_gmem_cleanup();
> +#endif
> + truncate_inode_pages_final(&inode->i_data);
> + clear_inode(inode);
> +}
> +
>
At the point of .evict_inode(), CoCo-shared guest_memfd pages could
still be pinned (for DMA or whatever, accidentally or maliciously), can
rmpopt work on shared pages that might still be used for DMA?
.invalidate_folio() and .free_folio() both actually happen on removal
from guest_memfd ownership, though both are not exactly when the folio
is completely not in use.
Is the best time to optimize when the pages are truly freed?
> @@ -971,6 +979,7 @@ static const struct super_operations kvm_gmem_super_operations = {
> .alloc_inode = kvm_gmem_alloc_inode,
> .destroy_inode = kvm_gmem_destroy_inode,
> .free_inode = kvm_gmem_free_inode,
> + .evict_inode = kvm_gmem_evict_inode,
> };
>
>
> Thanks,
> Ashish
>
>>
>> IIUC the current kmem_cache_free(kvm_gmem_inode_cachep, GMEM_I(inode));
>> is fine because in kvm_gmem_exit(), there is a rcu_barrier() before
>> kmem_cache_destroy(kvm_gmem_inode_cachep);.
>>
>>> kmem_cache_free(kvm_gmem_inode_cachep, GMEM_I(inode));
>>> }
>>>
>>> --
>>> 2.43.0
^ 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