* Re: [PATCH v3 02/26] x86/virt/tdx: Use %# prefix for hex values in SEAMCALL error messages
From: Binbin Wu @ 2026-01-28 1:34 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, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260123145645.90444-3-chao.gao@intel.com>
On 1/23/2026 10:55 PM, Chao Gao wrote:
> "%#" format specifier automatically adds the "0x" prefix and has one less
> character than "0x%".
>
> For conciseness, replace "0x%" with "%#" when printing hexadecimal values
> in SEAMCALL error messages.
>
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> ---
> "0x%" is also used to print TDMR ranges. I didn't convert them to reduce
> code churn, but if they should be converted for consistency, I'm happy
> to do that.
Generally, is there any preference for coding in Linux kernel about
"0x%" VS. "%#"? Or developers just make their own choices?
>
> v2: new
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index dbc7cb08ca53..2218bb42af40 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -63,16 +63,16 @@ typedef void (*sc_err_func_t)(u64 fn, u64 err, struct tdx_module_args *args);
>
> static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
> {
> - pr_err("SEAMCALL (%llu) failed: 0x%016llx\n", fn, err);
> + pr_err("SEAMCALL (%llu) failed: %#016llx\n", fn, err);
> }
>
> static inline void seamcall_err_ret(u64 fn, u64 err,
> struct tdx_module_args *args)
> {
> seamcall_err(fn, err, args);
> - pr_err("RCX 0x%016llx RDX 0x%016llx R08 0x%016llx\n",
> + pr_err("RCX %#016llx RDX %#016llx R08 %#016llx\n",
> args->rcx, args->rdx, args->r8);
> - pr_err("R09 0x%016llx R10 0x%016llx R11 0x%016llx\n",
> + pr_err("R09 %#016llx R10 %#016llx R11 %#016llx\n",
> args->r9, args->r10, args->r11);
> }
>
^ permalink raw reply
* Re: [PATCH v3 01/26] x86/virt/tdx: Print SEAMCALL leaf numbers in decimal
From: Binbin Wu @ 2026-01-28 1:28 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, Kirill A. Shutemov, Farrah Chen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260123145645.90444-2-chao.gao@intel.com>
On 1/23/2026 10:55 PM, Chao Gao wrote:
> Both TDX spec and kernel defines SEAMCALL leaf numbers as decimal. Printing
> them in hex makes no sense. Correct it.
>
> Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> ---
> v2:
> - print leaf numbers with %llu
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 5ce4ebe99774..dbc7cb08ca53 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -63,7 +63,7 @@ typedef void (*sc_err_func_t)(u64 fn, u64 err, struct tdx_module_args *args);
>
> static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
> {
> - pr_err("SEAMCALL (0x%016llx) failed: 0x%016llx\n", fn, err);
> + pr_err("SEAMCALL (%llu) failed: 0x%016llx\n", fn, err);
> }
>
> static inline void seamcall_err_ret(u64 fn, u64 err,
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Vedashree Vidwans @ 2026-01-27 23:01 UTC (permalink / raw)
To: Salman Nabi, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <20260119122729.287522-2-salman.nabi@arm.com>
Hello,
On 1/19/26 04:27, Salman Nabi wrote:
> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
> activating firmware components without a reboot. Those components
> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
> usual way: via fwupd, FF-A or other secure storage methods, or via some
> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
> at system runtime, without requiring a reboot.
> The specification covers the SMCCC interface to list and query available
> components and eventually trigger the activation.
>
> Add a new directory under /sys/firmware to present firmware components
> capable of live activation. Each of them is a directory under lfa/,
> and is identified via its GUID. The activation will be triggered by echoing
> "1" into the "activate" file:
> ==========================================
> /sys/firmware/lfa # ls -l . 6c*
> .:
> total 0
> drwxr-xr-x 2 0 0 0 Jan 19 11:33 47d4086d-4cfe-9846-9b95-2950cbbd5a00
> drwxr-xr-x 2 0 0 0 Jan 19 11:33 6c0762a6-12f2-4b56-92cb-ba8f633606d9
> drwxr-xr-x 2 0 0 0 Jan 19 11:33 d6d0eea7-fcea-d54b-9782-9934f234b6e4
>
> 6c0762a6-12f2-4b56-92cb-ba8f633606d9:
> total 0
> --w------- 1 0 0 4096 Jan 19 11:33 activate
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 activation_capable
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 activation_pending
> --w------- 1 0 0 4096 Jan 19 11:33 cancel
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 cpu_rendezvous
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 current_version
> -rw-r--r-- 1 0 0 4096 Jan 19 11:33 force_cpu_rendezvous
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 may_reset_cpu
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 name
> -r--r--r-- 1 0 0 4096 Jan 19 11:33 pending_version
> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 # grep . *
> grep: activate: Permission denied
> activation_capable:1
> activation_pending:1
> grep: cancel: Permission denied
> cpu_rendezvous:1
> current_version:0.0
> force_cpu_rendezvous:1
> may_reset_cpu:0
> name:TF-RMM
> pending_version:0.0
> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 # echo 1 > activate
> [ 2825.797871] Arm LFA: firmware activation succeeded.
> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 #
> ==========================================
>
> [1] https://developer.arm.com/documentation/den0147/latest/
>
> Signed-off-by: Salman Nabi <salman.nabi@arm.com>
> ---
> drivers/firmware/smccc/Kconfig | 8 +
> drivers/firmware/smccc/Makefile | 1 +
> drivers/firmware/smccc/lfa_fw.c | 668 ++++++++++++++++++++++++++++++++
> 3 files changed, 677 insertions(+)
> create mode 100644 drivers/firmware/smccc/lfa_fw.c
>
> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
> index 15e7466179a6..ff7ca49486b0 100644
> --- a/drivers/firmware/smccc/Kconfig
> +++ b/drivers/firmware/smccc/Kconfig
> @@ -23,3 +23,11 @@ config ARM_SMCCC_SOC_ID
> help
> Include support for the SoC bus on the ARM SMCCC firmware based
> platforms providing some sysfs information about the SoC variant.
> +
> +config ARM_LFA
> + tristate "Arm Live Firmware activation support"
> + depends on HAVE_ARM_SMCCC_DISCOVERY
> + default y
> + help
> + Include support for triggering Live Firmware Activation, which
> + allows to upgrade certain firmware components without a reboot.
> diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
> index 40d19144a860..a6dd01558a94 100644
> --- a/drivers/firmware/smccc/Makefile
> +++ b/drivers/firmware/smccc/Makefile
> @@ -2,3 +2,4 @@
> #
> obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o kvm_guest.o
> obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o
> +obj-$(CONFIG_ARM_LFA) += lfa_fw.o
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> new file mode 100644
> index 000000000000..ce54049b7190
> --- /dev/null
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -0,0 +1,668 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Arm Limited
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/stop_machine.h>
> +#include <linux/string.h>
> +#include <linux/sysfs.h>
> +#include <linux/arm-smccc.h>
> +#include <linux/psci.h>
> +#include <uapi/linux/psci.h>
> +#include <linux/uuid.h>
> +#include <linux/array_size.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "Arm LFA: " fmt
> +
> +/* LFA v1.0b0 specification */
> +#define LFA_1_0_FN_BASE 0xc40002e0
> +#define LFA_1_0_FN(n) (LFA_1_0_FN_BASE + (n))
> +
> +#define LFA_1_0_FN_GET_VERSION LFA_1_0_FN(0)
> +#define LFA_1_0_FN_CHECK_FEATURE LFA_1_0_FN(1)
> +#define LFA_1_0_FN_GET_INFO LFA_1_0_FN(2)
> +#define LFA_1_0_FN_GET_INVENTORY LFA_1_0_FN(3)
> +#define LFA_1_0_FN_PRIME LFA_1_0_FN(4)
> +#define LFA_1_0_FN_ACTIVATE LFA_1_0_FN(5)
> +#define LFA_1_0_FN_CANCEL LFA_1_0_FN(6)
> +
> +/* CALL_AGAIN flags (returned by SMC) */
> +#define LFA_PRIME_CALL_AGAIN BIT(0)
> +#define LFA_ACTIVATE_CALL_AGAIN BIT(0)
> +
> +/* LFA return values */
> +#define LFA_SUCCESS 0
> +#define LFA_NOT_SUPPORTED 1
> +#define LFA_BUSY 2
> +#define LFA_AUTH_ERROR 3
> +#define LFA_NO_MEMORY 4
> +#define LFA_CRITICAL_ERROR 5
> +#define LFA_DEVICE_ERROR 6
> +#define LFA_WRONG_STATE 7
> +#define LFA_INVALID_PARAMETERS 8
> +#define LFA_COMPONENT_WRONG_STATE 9
> +#define LFA_INVALID_ADDRESS 10
> +#define LFA_ACTIVATION_FAILED 11
> +
> +#define LFA_ERROR_STRING(name) \
> + [name] = #name
> +
> +static const char * const lfa_error_strings[] = {
> + LFA_ERROR_STRING(LFA_SUCCESS),
> + LFA_ERROR_STRING(LFA_NOT_SUPPORTED),
> + LFA_ERROR_STRING(LFA_BUSY),
> + LFA_ERROR_STRING(LFA_AUTH_ERROR),
> + LFA_ERROR_STRING(LFA_NO_MEMORY),
> + LFA_ERROR_STRING(LFA_CRITICAL_ERROR),
> + LFA_ERROR_STRING(LFA_DEVICE_ERROR),
> + LFA_ERROR_STRING(LFA_WRONG_STATE),
> + LFA_ERROR_STRING(LFA_INVALID_PARAMETERS),
> + LFA_ERROR_STRING(LFA_COMPONENT_WRONG_STATE),
> + LFA_ERROR_STRING(LFA_INVALID_ADDRESS),
> + LFA_ERROR_STRING(LFA_ACTIVATION_FAILED)
> +};
> +
> +enum image_attr_names {
> + LFA_ATTR_NAME,
> + LFA_ATTR_CURRENT_VERSION,
> + LFA_ATTR_PENDING_VERSION,
> + LFA_ATTR_ACT_CAPABLE,
> + LFA_ATTR_ACT_PENDING,
> + LFA_ATTR_MAY_RESET_CPU,
> + LFA_ATTR_CPU_RENDEZVOUS,
> + LFA_ATTR_FORCE_CPU_RENDEZVOUS,
> + LFA_ATTR_ACTIVATE,
> + LFA_ATTR_CANCEL,
> + LFA_ATTR_NR_IMAGES
> +};
> +
> +struct image_props {
> + struct list_head image_node;
> + const char *image_name;
> + int fw_seq_id;
> + u64 current_version;
> + u64 pending_version;
> + bool activation_capable;
> + bool activation_pending;
> + bool may_reset_cpu;
> + bool cpu_rendezvous;
> + bool cpu_rendezvous_forced;
> + struct kobject *image_dir;
> + struct kobj_attribute image_attrs[LFA_ATTR_NR_IMAGES];
> +};
> +static LIST_HEAD(lfa_fw_images);
> +
> +/* A UUID split over two 64-bit registers */
> +struct uuid_regs {
> + u64 uuid_lo;
> + u64 uuid_hi;
> +};
> +
> +static const struct fw_image_uuid {
> + const char *name;
> + const char *uuid;
> +} fw_images_uuids[] = {
> + {
> + .name = "TF-A BL31 runtime",
> + .uuid = "47d4086d-4cfe-9846-9b95-2950cbbd5a00",
> + },
> + {
> + .name = "BL33 non-secure payload",
> + .uuid = "d6d0eea7-fcea-d54b-9782-9934f234b6e4",
> + },
> + {
> + .name = "TF-RMM",
> + .uuid = "6c0762a6-12f2-4b56-92cb-ba8f633606d9",
> + },
> +};
> +
> +static struct kobject *lfa_dir;
> +static DEFINE_MUTEX(lfa_lock);
> +static struct workqueue_struct *fw_images_update_wq;
> +static struct work_struct fw_images_update_work;
> +
> +static int update_fw_images_tree(void);
> +
> +static void delete_fw_image_node(struct image_props *attrs)
> +{
> + int i;
> +
> + for (i = 0; i < LFA_ATTR_NR_IMAGES; i++)
> + sysfs_remove_file(attrs->image_dir, &attrs->image_attrs[i].attr);
> +
> + kobject_put(attrs->image_dir);
> + list_del(&attrs->image_node);
> + kfree(attrs);
> +}
> +
> +static void remove_invalid_fw_images(struct work_struct *work)
> +{
> + struct image_props *attrs, *tmp;
> +
> + mutex_lock(&lfa_lock);
> +
> + /*
> + * Remove firmware images including directories that are no longer
> + * present in the LFA agent after updating the existing ones.
> + */
> + list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node) {
> + if (attrs->fw_seq_id == -1)
> + delete_fw_image_node(attrs);
> + }
> +
> + mutex_unlock(&lfa_lock);
> +}
> +
> +static void set_image_flags(struct image_props *attrs, int seq_id,
> + u32 image_flags, u64 reg_current_ver,
> + u64 reg_pending_ver)
> +{
> + attrs->fw_seq_id = seq_id;
> + attrs->current_version = reg_current_ver;
> + attrs->pending_version = reg_pending_ver;
> + attrs->activation_capable = !!(image_flags & BIT(0));
> + attrs->activation_pending = !!(image_flags & BIT(1));
> + attrs->may_reset_cpu = !!(image_flags & BIT(2));
> + /* cpu_rendezvous_optional bit has inverse logic in the spec */
> + attrs->cpu_rendezvous = !(image_flags & BIT(3));
> +}
> +
> +static unsigned long get_nr_lfa_components(void)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + reg.a0 = LFA_1_0_FN_GET_INFO;
> + reg.a1 = 0; /* lfa_info_selector = 0 */
> +
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 != LFA_SUCCESS)
> + return reg.a0;
> +
> + return reg.a1;
> +}
> +
> +static int lfa_cancel(void *data)
> +{
> + struct image_props *attrs = data;
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + reg.a0 = LFA_1_0_FN_CANCEL;
> + reg.a1 = attrs->fw_seq_id;
> + arm_smccc_1_2_invoke(®, ®);
> +
> + /*
> + * When firmware activation is called with "skip_cpu_rendezvous=1",
> + * LFA_CANCEL can fail with LFA_BUSY if the activation could not be
> + * cancelled.
> + */
> + if (reg.a0 == LFA_SUCCESS) {
> + pr_info("Activation cancelled for image %s\n",
> + attrs->image_name);
> + } else {
> + pr_err("Firmware activation could not be cancelled: %s\n",
> + lfa_error_strings[-reg.a0]);
> + return -EINVAL;
> + }
> +
> + return reg.a0;
> +}
> +
> +static int call_lfa_activate(void *data)
> +{
> + struct image_props *attrs = data;
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + reg.a0 = LFA_1_0_FN_ACTIVATE;
> + reg.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> + /*
> + * As we do not support updates requiring a CPU reset (yet),
> + * we pass 0 in reg.a3 and reg.a4, holding the entry point and context
> + * ID respectively.
> + * cpu_rendezvous_forced is set by the administrator, via sysfs,
> + * cpu_rendezvous is dictated by each firmware component.
> + */
> + reg.a2 = !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous);
> +
> + for (;;) {
> + arm_smccc_1_2_invoke(®, ®);
> +
> + if ((long)reg.a0 < 0) {
> + pr_err("ACTIVATE for image %s failed: %s\n",
> + attrs->image_name, lfa_error_strings[-reg.a0]);
> + return reg.a0;
> + }
> + if (!(reg.a1 & LFA_ACTIVATE_CALL_AGAIN))
> + break; /* ACTIVATE successful */
> + }
The implementation uses same 'struct arm_smccc_1_2_regs reg' as
input and output for arm_smccc_1_2_invoke(). Here, reg.a0 (function ID),
reg.a1 (fw_seq_id) and reg.a2 (cpu rendezvous) are initialized once
before the loop and arm_smccc_1_2_invoke() overwrites the whole register
set on every iteration. That means inputs (a0, a1, a2) can be clobbered
between loop iterations unless reassigned each time.
Suggestion: Re-initialize input members of reg on each loop iteration or
use a separate 'struct arm_smccc_1_2_regs' for output to avoid input
corruption.
> +
> + return reg.a0;
> +}
> +
> +static int activate_fw_image(struct image_props *attrs)
> +{
> + int ret;
> +
> + mutex_lock(&lfa_lock);
> + if (attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous)
> + ret = stop_machine(call_lfa_activate, attrs, cpu_online_mask);
> + else
> + ret = call_lfa_activate(attrs);
> +
> + if (ret != 0) {
> + mutex_unlock(&lfa_lock);
> + return lfa_cancel(attrs);
> + }
> +
> + /*
> + * Invalidate fw_seq_ids (-1) for all images as the seq_ids and the
> + * number of firmware images in the LFA agent may change after a
> + * successful activation attempt. Negate all image flags as well.
> + */
> + attrs = NULL;
> + list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> + set_image_flags(attrs, -1, 0b1000, 0, 0);
> + }
> +
> + update_fw_images_tree();
> +
> + /*
> + * Removing non-valid image directories at the end of an activation.
> + * We can't remove the sysfs attributes while in the respective
> + * _store() handler, so have to postpone the list removal to a
> + * workqueue.
> + */
> + INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
> + queue_work(fw_images_update_wq, &fw_images_update_work);
> + mutex_unlock(&lfa_lock);
> +
> + return ret;
> +}
> +
> +static int prime_fw_image(struct image_props *attrs)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> + int ret;
> +
> + mutex_lock(&lfa_lock);
> + /* Avoid SMC calls on invalid firmware images */
> + if (attrs->fw_seq_id == -1) {
> + pr_err("Arm LFA: Invalid firmware sequence id\n");
> + mutex_unlock(&lfa_lock);
> +
> + return -ENODEV;
> + }
> +
> + if (attrs->may_reset_cpu) {
> + pr_err("CPU reset not supported by kernel driver\n");
> + mutex_unlock(&lfa_lock);
> +
> + return -EINVAL;
> + }
> +
> + /*
> + * LFA_PRIME/ACTIVATE will return 1 in reg.a1 if the firmware
> + * priming/activation is still in progress. In that case
> + * LFA_PRIME/ACTIVATE will need to be called again.
> + * reg.a1 will become 0 once the prime/activate process completes.
> + */
> + reg.a0 = LFA_1_0_FN_PRIME;
> + reg.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> + for (;;) {
> + arm_smccc_1_2_invoke(®, ®);
> +
> + if ((long)reg.a0 < 0) {
> + pr_err("LFA_PRIME for image %s failed: %s\n",
> + attrs->image_name, lfa_error_strings[-reg.a0]);
> + mutex_unlock(&lfa_lock);
> +
> + return reg.a0;
> + }
> + if (!(reg.a1 & LFA_PRIME_CALL_AGAIN)) {
> + ret = 0;
> + break; /* PRIME successful */
> + }
> + }
Similar comment to call_lfa_activate(). Suggestion to either re-assign
'struct arm_smccc_1_2_regs' input values on each loop iteration or use a
separate 'struct arm_smccc_1_2_regs' for output to avoid input
corruption between loop iterations. This matches the intended
'CALL_AGAIN' protocal while keeping the inputs stable across retries.
> +
> + mutex_unlock(&lfa_lock);
> + return ret;
The introduction of separate 'ret' cariable does not appear necessary
for functional correctness. The SMCCC status is conveyed via reg.a0 on
each iteration, so returning reg.a0 should preserve existing behavior.
If 'ret' must be kept, consider initializing it to 0 at declaration
time. That avoids setting ret = 0 inside 'PRIME successful' path and
leads to simpler control flow.
> +}
> +
> +static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_NAME]);
> +
> + return sysfs_emit(buf, "%s\n", attrs->image_name);
> +}
> +
> +static ssize_t activation_capable_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACT_CAPABLE]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->activation_capable);
> +}
> +
> +static ssize_t activation_pending_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACT_PENDING]);
> + struct arm_smccc_1_2_regs reg = { 0 };
> +
> + /*
> + * Activation pending status can change anytime thus we need to update
> + * and return its current value
> + */
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = attrs->fw_seq_id;
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == LFA_SUCCESS)
> + attrs->activation_pending = !!(reg.a3 & BIT(1));
> +
> + return sysfs_emit(buf, "%d\n", attrs->activation_pending);
> +}
> +
> +static ssize_t may_reset_cpu_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_MAY_RESET_CPU]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->may_reset_cpu);
> +}
> +
> +static ssize_t cpu_rendezvous_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_CPU_RENDEZVOUS]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->cpu_rendezvous);
> +}
> +
> +static ssize_t force_cpu_rendezvous_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_FORCE_CPU_RENDEZVOUS]);
> + int ret;
> +
> + ret = kstrtobool(buf, &attrs->cpu_rendezvous_forced);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static ssize_t force_cpu_rendezvous_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_FORCE_CPU_RENDEZVOUS]);
> +
> + return sysfs_emit(buf, "%d\n", attrs->cpu_rendezvous_forced);
> +}
> +
> +static ssize_t current_version_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_CURRENT_VERSION]);
> + u32 maj, min;
> +
> + maj = attrs->current_version >> 32;
> + min = attrs->current_version & 0xffffffff;
> + return sysfs_emit(buf, "%u.%u\n", maj, min);
> +}
> +
> +static ssize_t pending_version_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACT_PENDING]);
> + struct arm_smccc_1_2_regs reg = { 0 };
> + u32 maj, min;
> +
> + /*
> + * Similar to activation pending, this value can change following an
> + * update, we need to retrieve fresh info instead of stale information.
> + */
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = attrs->fw_seq_id;
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == LFA_SUCCESS) {
> + if (reg.a5 != 0 && attrs->activation_pending)
> + {
> + attrs->pending_version = reg.a5;
> + maj = reg.a5 >> 32;
> + min = reg.a5 & 0xffffffff;
> + }
> + }
> +
> + return sysfs_emit(buf, "%u.%u\n", maj, min);
> +}
> +
> +static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_ACTIVATE]);
> + int ret;
> +
> + ret = prime_fw_image(attrs);
> + if (ret) {
> + pr_err("Firmware prime failed: %s\n",
> + lfa_error_strings[-ret]);
> + return -ECANCELED;
> + }
> +
> + ret = activate_fw_image(attrs);
> + if (ret) {
> + pr_err("Firmware activation failed: %s\n",
> + lfa_error_strings[-ret]);
> + return -ECANCELED;
> + }
> +
> + pr_info("Firmware activation succeeded\n");
> +
> + return count;
> +}
> +
> +static ssize_t cancel_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct image_props *attrs = container_of(attr, struct image_props,
> + image_attrs[LFA_ATTR_CANCEL]);
> + int ret;
> +
> + ret = lfa_cancel(attrs);
> + if (ret != 0)
> + return ret;
> +
> + return count;
> +}
> +
> +static struct kobj_attribute image_attrs_group[LFA_ATTR_NR_IMAGES] = {
> + [LFA_ATTR_NAME] = __ATTR_RO(name),
> + [LFA_ATTR_CURRENT_VERSION] = __ATTR_RO(current_version),
> + [LFA_ATTR_PENDING_VERSION] = __ATTR_RO(pending_version),
> + [LFA_ATTR_ACT_CAPABLE] = __ATTR_RO(activation_capable),
> + [LFA_ATTR_ACT_PENDING] = __ATTR_RO(activation_pending),
> + [LFA_ATTR_MAY_RESET_CPU] = __ATTR_RO(may_reset_cpu),
> + [LFA_ATTR_CPU_RENDEZVOUS] = __ATTR_RO(cpu_rendezvous),
> + [LFA_ATTR_FORCE_CPU_RENDEZVOUS] = __ATTR_RW(force_cpu_rendezvous),
> + [LFA_ATTR_ACTIVATE] = __ATTR_WO(activate),
> + [LFA_ATTR_CANCEL] = __ATTR_WO(cancel)
> +};
> +
> +static void clean_fw_images_tree(void)
> +{
> + struct image_props *attrs, *tmp;
> +
> + list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node)
> + delete_fw_image_node(attrs);
> +}
> +
> +static int update_fw_image_node(char *fw_uuid, int seq_id,
> + u32 image_flags, u64 reg_current_ver,
> + u64 reg_pending_ver)
> +{
> + const char *image_name = "(unknown)";
> + struct image_props *attrs;
> + int ret;
> +
> + /*
> + * If a fw_image is already in the images list then we just update
> + * its flags and seq_id instead of trying to recreate it.
> + */
> + list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> + if (!strcmp(attrs->image_dir->name, fw_uuid)) {
> + set_image_flags(attrs, seq_id, image_flags,
> + reg_current_ver, reg_pending_ver);
> + return 0;
> + }
> + }
> +
> + attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> + if (!attrs)
> + return -ENOMEM;
> +
> + for (int i = 0; i < ARRAY_SIZE(fw_images_uuids); i++) {
> + if (!strcmp(fw_images_uuids[i].uuid, fw_uuid))
> + image_name = fw_images_uuids[i].name;
> + }
I would recommend using fw_uuid as the image_name when UUID is not
found in fw_images_uuids[], currently the driver assigns 'unknown'
in such case.
There is a valid possibility that platform-specific FW images, not
listed in fw_images_uuids[], are used by LFA agent for live FW
activation. In such scenarios, falling back to 'unknown' would lose
important information especially when errors surface in
call_lfa_activate(). Using UUID directly would indicate which
image failed or behaved unexpectedly.
Thank you,
Veda
> +
> + attrs->image_dir = kobject_create_and_add(fw_uuid, lfa_dir);
> + if (!attrs->image_dir)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&attrs->image_node);
> + attrs->image_name = image_name;
> + attrs->cpu_rendezvous_forced = 1;
> + set_image_flags(attrs, seq_id, image_flags, reg_current_ver,
> + reg_pending_ver);
> +
> + /*
> + * The attributes for each sysfs file are constant (handler functions,
> + * name and permissions are the same within each directory), but we
> + * need a per-directory copy regardless, to get a unique handle
> + * for each directory, so that container_of can do its magic.
> + * Also this requires an explicit sysfs_attr_init(), since it's a new
> + * copy, to make LOCKDEP happy.
> + */
> + memcpy(attrs->image_attrs, image_attrs_group,
> + sizeof(attrs->image_attrs));
> + for (int i = 0; i < LFA_ATTR_NR_IMAGES; i++) {
> + struct attribute *attr = &attrs->image_attrs[i].attr;
> +
> + sysfs_attr_init(attr);
> + ret = sysfs_create_file(attrs->image_dir, attr);
> + if (ret) {
> + pr_err("creating sysfs file for uuid %s: %d\n",
> + fw_uuid, ret);
> + clean_fw_images_tree();
> +
> + return ret;
> + }
> + }
> + list_add(&attrs->image_node, &lfa_fw_images);
> +
> + return ret;
> +}
> +
> +static int update_fw_images_tree(void)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> + struct uuid_regs image_uuid;
> + char image_id_str[40];
> + int ret, num_of_components;
> +
> + num_of_components = get_nr_lfa_components();
> + if (num_of_components <= 0) {
> + pr_err("Error getting number of LFA components\n");
> + return -ENODEV;
> + }
> +
> + for (int i = 0; i < num_of_components; i++) {
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = i; /* fw_seq_id under consideration */
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == LFA_SUCCESS) {
> + image_uuid.uuid_lo = reg.a1;
> + image_uuid.uuid_hi = reg.a2;
> +
> + snprintf(image_id_str, sizeof(image_id_str), "%pUb",
> + &image_uuid);
> + ret = update_fw_image_node(image_id_str, i,
> + reg.a3, reg.a4, reg.a5);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int __init lfa_init(void)
> +{
> + struct arm_smccc_1_2_regs reg = { 0 };
> + int err;
> +
> + reg.a0 = LFA_1_0_FN_GET_VERSION;
> + arm_smccc_1_2_invoke(®, ®);
> + if (reg.a0 == -LFA_NOT_SUPPORTED) {
> + pr_info("Live Firmware activation: no firmware agent found\n");
> + return -ENODEV;
> + }
> +
> + fw_images_update_wq = alloc_workqueue("fw_images_update_wq",
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
> + if (!fw_images_update_wq) {
> + pr_err("Live Firmware Activation: Failed to allocate workqueue.\n");
> +
> + return -ENOMEM;
> + }
> +
> + pr_info("Live Firmware Activation: detected v%ld.%ld\n",
> + reg.a0 >> 16, reg.a0 & 0xffff);
> +
> + lfa_dir = kobject_create_and_add("lfa", firmware_kobj);
> + if (!lfa_dir)
> + return -ENOMEM;
> +
> + mutex_lock(&lfa_lock);
> + err = update_fw_images_tree();
> + if (err != 0)
> + kobject_put(lfa_dir);
> +
> + mutex_unlock(&lfa_lock);
> + return err;
> +}
> +module_init(lfa_init);
> +
> +static void __exit lfa_exit(void)
> +{
> + flush_workqueue(fw_images_update_wq);
> + destroy_workqueue(fw_images_update_wq);
> +
> + mutex_lock(&lfa_lock);
> + clean_fw_images_tree();
> + mutex_unlock(&lfa_lock);
> +
> + kobject_put(lfa_dir);
> +}
> +module_exit(lfa_exit);
> +
> +MODULE_DESCRIPTION("ARM Live Firmware Activation (LFA)");
> +MODULE_LICENSE("GPL");
^ permalink raw reply
* Re: [PATCH 2/2] KVM: SEV: Add support for IBPB-on-Entry
From: Kim Phillips @ 2026-01-27 20:56 UTC (permalink / raw)
To: Nikunj A. Dadhania, linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak, Tom Lendacky,
Michael Roth, Borislav Petkov, Borislav Petkov, Naveen Rao,
David Kaplan
In-Reply-To: <4dea11f9-6034-489b-acaf-9a150818d1a1@amd.com>
On 1/27/26 12:38 AM, Nikunj A. Dadhania wrote:
Hi Nikunj,
> On 1/27/2026 4:12 AM, Kim Phillips wrote:
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index ea515cf41168..8a6d25db0c00 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -3165,8 +3165,15 @@ void __init sev_hardware_setup(void)
>> cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
>> sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
>>
>> - if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
>> + if (!sev_snp_enabled)
>> + return;
>> + /* the following feature bit checks are SNP specific */
>> +
>
> The early return seems to split up the SNP features unnecessarily.
>
> Keeping everything under `if (sev_snp_enabled)` is cleaner IMO -
> it's clear that these features belong together. Plus, when
> someone adds the next SNP feature, they won't have to think about
> whether it goes before or after the return. The comment about
> "SNP specific" features becomes redundant as well.
The SNP 'togetherness' semantics are maintained whether under an
'if (sev_snp_enabled)' body, or after an 'if (!sev_snp_enabled) return;'.
Only SNP-specific things are being done in the trailing part of the function,
so it naturally lends itself to do the early return. It makes it more
readable by eliminating the unnecessary indentation created by an
'if (sev_snp_enabled)' body.
Meanwhile, I agree with your comments on the first patch in the series.
Thanks for your review,
Kim
^ permalink raw reply
* Re: [PATCH v3 26/26] coco/tdx-host: Set and document TDX Module update expectations
From: dan.j.williams @ 2026-01-27 17:23 UTC (permalink / raw)
To: Chao Gao, dan.j.williams
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, yilun.xu, sagis, vannapurve, paulmck, nik.borisov,
zhenzhong.duan, seanjc, rick.p.edgecombe, kas, dave.hansen,
vishal.l.verma
In-Reply-To: <aXis76vQhWi3RvEB@intel.com>
Chao Gao wrote:
[..]
> >So, remove "compat_capable" ABI. Amend the "error" ABI documentation
> >with the details for avoiding failures and the risk of running updates
> >on configurations that support update but not collision avoidance.
>
> Got it. I will modify this patch as follows:
Overall, looks good to me. You can add:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
...after a few additional fixups below:
> diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> index a3f155977016..0a68e68375fa 100644
> --- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> +++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> @@ -29,3 +29,57 @@ Description: (RO) Report the number of remaining updates that can be performed.
> 4.2 "SEAMLDR.INSTALL" for more information. The documentation is
> available at:
> https://cdrdv2-public.intel.com/739045/intel-tdx-seamldr-interface-specification.pdf
> +
> +What: /sys/devices/faux/tdx_host/firmware/seamldr_upload
> +Contact: linux-coco@lists.linux.dev
> +Description: (Directory) The seamldr_upload directory implements the
> + fw_upload sysfs ABI, see
> + Documentation/ABI/testing/sysfs-class-firmware for the general
> + description of the attributes @data, @cancel, @error, @loading,
> + @remaining_size, and @status. This ABI facilitates "Compatible
> + TDX Module Updates". A compatible update is one that meets the
> + following criteria:
> +
> + Does not interrupt or interfere with any current TDX
> + operation or TD VM.
> +
> + Does not invalidate any previously consumed Module metadata
> + values outside of the TEE_TCB_SVN_2 field (updated Security
> + Version Number) in TD Quotes.
> +
> + Does not require validation of new Module metadata fields. By
> + implication, new Module features and capabilities are only
> + available by installing the Module at reboot (BIOS or EFI
> + helper loaded).
> +
> + See tdx_host/firmware/seamldr_upload/error for more details.
> +
> +What: /sys/devices/faux/tdx_host/firmware/seamldr_upload/error
> +Contact: linux-coco@lists.linux.dev
> +Description: (RO) See Documentation/ABI/testing/sysfs-class-firmware for
> + baseline expectations for this file. The <ERROR> part in the
> + <STATUS>:<ERROR> format can be:
> +
> + "device-busy": Compatibility checks failed or not all CPUs
> + are online
> + "flash-wearout": the number of updates reached the limit.
> + "read-write-error": Memory allocation failed.
> + "hw-error": Cannot communicate with P-SEAMLDR or TDX Module
> + "firmware-invalid": The TDX Module to be installed is invalid
> + or other unexpected errors occurred.
> +
> + "hw-error" or "firmware-invalid" may be fatal, causing all TDs
> + and the TDX Module to be lost and preventing further TDX
> + operations. This occurs when /sys/devices/faux/tdx_host/version
> + becomes unreadable after update failures.
I would specify the exact unambiguous errno value that gets returned on
read when the version become indeterminate, like ENXIO.
> + and the (previous) TDX Module stay running.
> +
> + On certain earlier TDX Module versions, incompatible updates may
> + not trigger "device-busy" errors but instead cause TD
> + attestation failures.
I would just leave this out. It bitrots quickly and does not provide
any actionable information. This is not the kernel's responsibility...
> +
> + See version_select_and_load.py [1] documentation for how to
> + detect compatible updates and whether the current platform
> + components catch errors or let them leak and cause potential TD
> + attestation failures.
> + [1]: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/version_select_and_load.py
...that detail about what happens when compat detection is missing
belongs in the tooling documentation. That documentation does not exist
yet, so this link needs to be replaced with a pointer to documentation
before this goes upstream. I am assuming that we want to create an
actual package that distributions can pick up as project? It might be
worth going through the exercise of packaging the binaries and the tool
as an rpm or deb to get that work bootstrapped.
"version_select_and_load" probably wants a better name like "tdxctl" or
similar.
Note that a tdxctl project would also attract features related to TDX
Connect to wrap common flows around the tdx_host device sysfs ABIs.
^ permalink raw reply
* SVSM Development Call January 28, 2026
From: Jörg Rödel @ 2026-01-27 17:06 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the call for agenda items for this weeks SVSM development call. Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH v3 26/26] coco/tdx-host: Set and document TDX Module update expectations
From: Chao Gao @ 2026-01-27 12:17 UTC (permalink / raw)
To: dan.j.williams
Cc: linux-coco, linux-kernel, kvm, x86, reinette.chatre, ira.weiny,
kai.huang, yilun.xu, sagis, vannapurve, paulmck, nik.borisov,
zhenzhong.duan, seanjc, rick.p.edgecombe, kas, dave.hansen,
vishal.l.verma
In-Reply-To: <6977e73a7a121_30951002f@dwillia2-mobl4.notmuch>
On Mon, Jan 26, 2026 at 02:14:18PM -0800, dan.j.williams@intel.com wrote:
>Chao Gao wrote:
>> In rare cases, TDX Module updates may cause TD management operations to
>> fail if they occur during phases of the TD lifecycle that are sensitive
>> to update compatibility.
>
>No. The TDX Module wants to be able to claim that some updates are
>compatible when they are not. If Linux takes on additional exclusions it
>modestly increases the scope of changes that can be included in an
>update. It is not possible to claim "rare" if module updates routinely
>include that problematic scope.
>
>> But not all combinations of P-SEAMLDR, kernel, and TDX Module have the
>> capability to detect and prevent said incompatibilities. Completely
>> disabling TDX Module updates on platforms without the capability would
>> be overkill, as these incompatibility cases are rare and can be
>> addressed by userspace through coordinated scheduling of updates and TD
>> management operations.
>
>"Completely disabling" is not the tradeoff. The tradeoff is whether or
>not the TDX Module meets Linux compatible update requirements or not.
>
>> To set clear expectations for TDX Module updates, expose the capability
>> to detect and prevent these incompatibility cases via sysfs and
>> document the compatibility criteria and indications when those criteria
>> are violated.
>
>Linux derives no benefit from a "compat_capable" kernel ABI. Yes, the
>internals must export the error condition on collision. I am not
>debating that nor revisiting the decision of pre-update-fail, vs
>post-collision-notify. However, if the module violates the Linux
>expectations that is the module's issue to document or preclude. The
>fact that the compatibility contract is ambiguous to the kernel is a
>feature. It puts the onus squarely on module updates to be documented
>(or tools updated to understand) as meeting or violating Linux
>compatibility expectations.
>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> ---
>> v3:
>> - new, based on a reference patch from Dan Williams
>
>One of the details that is missing is the protocol (module documentation
>or tooling) to determine ahead of time if an update is compatible. That
>obviates the need for "compat_capable" ABI which serves no long term
>purpose. Specifically, the expectation is "run non-compatible updates at
>your own operational risk".
Agreed. We need to add metadata like crypto library version or equivalent
abstraction to the mapping file. This enables userspace to determine whether
module updates meet Linux compatibility requirements. I'll submit a request
for this metadata.
And actually, userspace can already determine if the TDX module supports
"collision avoidance" by reading the "tdx_features0" field from the mapping
file [1].
[1]: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json
>
>So, remove "compat_capable" ABI. Amend the "error" ABI documentation
>with the details for avoiding failures and the risk of running updates
>on configurations that support update but not collision avoidance.
Got it. I will modify this patch as follows:
diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
index a3f155977016..0a68e68375fa 100644
--- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -29,3 +29,57 @@ Description: (RO) Report the number of remaining updates that can be performed.
4.2 "SEAMLDR.INSTALL" for more information. The documentation is
available at:
https://cdrdv2-public.intel.com/739045/intel-tdx-seamldr-interface-specification.pdf
+
+What: /sys/devices/faux/tdx_host/firmware/seamldr_upload
+Contact: linux-coco@lists.linux.dev
+Description: (Directory) The seamldr_upload directory implements the
+ fw_upload sysfs ABI, see
+ Documentation/ABI/testing/sysfs-class-firmware for the general
+ description of the attributes @data, @cancel, @error, @loading,
+ @remaining_size, and @status. This ABI facilitates "Compatible
+ TDX Module Updates". A compatible update is one that meets the
+ following criteria:
+
+ Does not interrupt or interfere with any current TDX
+ operation or TD VM.
+
+ Does not invalidate any previously consumed Module metadata
+ values outside of the TEE_TCB_SVN_2 field (updated Security
+ Version Number) in TD Quotes.
+
+ Does not require validation of new Module metadata fields. By
+ implication, new Module features and capabilities are only
+ available by installing the Module at reboot (BIOS or EFI
+ helper loaded).
+
+ See tdx_host/firmware/seamldr_upload/error for more details.
+
+What: /sys/devices/faux/tdx_host/firmware/seamldr_upload/error
+Contact: linux-coco@lists.linux.dev
+Description: (RO) See Documentation/ABI/testing/sysfs-class-firmware for
+ baseline expectations for this file. The <ERROR> part in the
+ <STATUS>:<ERROR> format can be:
+
+ "device-busy": Compatibility checks failed or not all CPUs
+ are online
+ "flash-wearout": the number of updates reached the limit.
+ "read-write-error": Memory allocation failed.
+ "hw-error": Cannot communicate with P-SEAMLDR or TDX Module
+ "firmware-invalid": The TDX Module to be installed is invalid
+ or other unexpected errors occurred.
+
+ "hw-error" or "firmware-invalid" may be fatal, causing all TDs
+ and the TDX Module to be lost and preventing further TDX
+ operations. This occurs when /sys/devices/faux/tdx_host/version
+ becomes unreadable after update failures. For other errors, TDs
+ and the (previous) TDX Module stay running.
+
+ On certain earlier TDX Module versions, incompatible updates may
+ not trigger "device-busy" errors but instead cause TD
+ attestation failures.
+
+ See version_select_and_load.py [1] documentation for how to
+ detect compatible updates and whether the current platform
+ components catch errors or let them leak and cause potential TD
+ attestation failures.
+ [1]: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/version_select_and_load.py
^ permalink raw reply related
* Re: [PATCH kernel 2/2] crypto/ccp: Allow multiple streams on the same root bridge
From: dan.j.williams @ 2026-01-27 6:59 UTC (permalink / raw)
To: Alexey Kardashevskiy, dan.j.williams, linux-crypto
Cc: linux-kernel, Ashish Kalra, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, x86, linux-coco, Pratik R . Sampat
In-Reply-To: <196ef0d2-93ac-4872-831b-e803e02b5d95@amd.com>
Alexey Kardashevskiy wrote:
> On 24/1/26 09:59, dan.j.williams@intel.com wrote:
> > Alexey Kardashevskiy wrote:
> >> IDE stream IDs are responsibility of a platform and in some cases
> >> TSM allocates the numbers. AMD SEV TIO though leaves it to the host
> >> OS. Mistakenly stream ID is hard coded to be the same as a traffic
> >> class.
> >
> > I scratched my head at this comment, but now realize that you are
> > saying the existing code used the local @tc, not that the hardware
> > stream ID is in any way related to traffic class, right?
>
> When I did that in the first place, I also wanted to try different
> traffic classes so I just took a shortcut here.
>
> > It would help to detail what the end user visible effects of this
> > bug are. The TSM framework does not allow for multiple streams per
> > PF, so I wonder what scenario is being fixed?
>
> There is no way in the current upstream code to specify this TC so the
> only visible effect is that 2 devices under the same bridge can work
> now, previously the second device would fail to allocate a stream.
>
> > Lastly, are you expecting tsm.git#fixes to pick this up? I am
> > assuming that this goes through crypto.git and tsm.git can just stay
> > focused on core fixes.
>
> I was kinda hoping that Tom acks these (as he did) and you could take
> them. Thanks,
Ok, so can you refresh the changelog to call out the user visible
effects? Something like:
---
With SEV-TIO the low-level TSM driver is responsible for allocating a
Stream ID. The Stream ID needs to be unique within each IDE partner
port. Fix the Stream ID selection to reuse the host bridge stream
resource id which is a pool of 256 ids per host bridge on AMD platforms.
Otherwise, only one device per-host bridge can establish Selective
Stream IDE.
---
Send a v2, and I will pick it up.
^ permalink raw reply
* Re: [PATCH 2/2] KVM: SEV: Add support for IBPB-on-Entry
From: Nikunj A. Dadhania @ 2026-01-27 6:38 UTC (permalink / raw)
To: Kim Phillips, linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak, Tom Lendacky,
Michael Roth, Borislav Petkov, Borislav Petkov, Naveen Rao,
David Kaplan
In-Reply-To: <20260126224205.1442196-3-kim.phillips@amd.com>
On 1/27/2026 4:12 AM, Kim Phillips wrote:
> AMD EPYC 5th generation and above processors support IBPB-on-Entry
> for SNP guests. By invoking an Indirect Branch Prediction Barrier
> (IBPB) on VMRUN, old indirect branch predictions are prevented
> from influencing indirect branches within the guest.
>
> SNP guests may choose to enable IBPB-on-Entry by setting
> SEV_FEATURES bit 21 (IbpbOnEntry).
>
> Host support for IBPB on Entry is indicated by CPUID
> Fn8000_001F[IbpbOnEntry], bit 31.
>
> If supported, indicate support for IBPB on Entry in
> sev_supported_vmsa_features bit 23 (IbpbOnEntry).
>
> 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>
> ---
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/include/asm/svm.h | 1 +
> arch/x86/kvm/svm/sev.c | 9 ++++++++-
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index c01fdde465de..3ce5dff36f78 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -459,6 +459,7 @@
> #define X86_FEATURE_ALLOWED_SEV_FEATURES (19*32+27) /* Allowed SEV Features */
> #define X86_FEATURE_SVSM (19*32+28) /* "svsm" SVSM present */
> #define X86_FEATURE_HV_INUSE_WR_ALLOWED (19*32+30) /* Allow Write to in-use hypervisor-owned pages */
> +#define X86_FEATURE_IBPB_ON_ENTRY (19*32+31) /* SEV-SNP IBPB on VM Entry */
>
> /* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
> #define X86_FEATURE_NO_NESTED_DATA_BP (20*32+ 0) /* No Nested Data Breakpoints */
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index edde36097ddc..eebc65ec948f 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -306,6 +306,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
> #define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
> #define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
> #define SVM_SEV_FEAT_SECURE_TSC BIT(9)
> +#define SVM_SEV_FEAT_IBPB_ON_ENTRY BIT(21)
>
> #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 ea515cf41168..8a6d25db0c00 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3165,8 +3165,15 @@ void __init sev_hardware_setup(void)
> cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
> sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
>
> - if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
> + if (!sev_snp_enabled)
> + return;
> + /* the following feature bit checks are SNP specific */
> +
The early return seems to split up the SNP features unnecessarily.
Keeping everything under `if (sev_snp_enabled)` is cleaner IMO -
it's clear that these features belong together. Plus, when
someone adds the next SNP feature, they won't have to think about
whether it goes before or after the return. The comment about
"SNP specific" features becomes redundant as well.
> + if (tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
> sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
> +
> + if (cpu_feature_enabled(X86_FEATURE_IBPB_ON_ENTRY))
> + sev_supported_vmsa_features |= SVM_SEV_FEAT_IBPB_ON_ENTRY;
Regards,
Nikunj
^ permalink raw reply
* Re: [PATCH 1/2] KVM: SEV: IBPB-on-Entry guest support
From: Nikunj A. Dadhania @ 2026-01-27 6:19 UTC (permalink / raw)
To: Kim Phillips, linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak, Tom Lendacky,
Michael Roth, Borislav Petkov, Borislav Petkov, Naveen Rao,
David Kaplan, stable
In-Reply-To: <20260126224205.1442196-2-kim.phillips@amd.com>
On 1/27/2026 4:12 AM, Kim Phillips wrote:
> KVM: SEV: IBPB-on-Entry guest support
The subject line should have the prefix "x86/sev" instead
of "KVM: SEV". The below subject line would be more appropriate:
x86/sev: Allow IBPB-on-Entry feature for SNP guests
> The SEV-SNP IBPB-on-Entry feature does not require a guest-side
> implementation. The feature was added in Zen5 h/w, after the first
> SNP Zen implementation, and thus was not accounted for when the
> initial set of SNP features were added to the kernel.
>
> In its abundant precaution, commit 8c29f0165405 ("x86/sev: Add SEV-SNP
> guest feature negotiation support") included SEV_STATUS' IBPB-on-Entry
> bit as a reserved bit, thereby masking guests from using the feature.
>
> Unmask the bit, to allow guests to take advantage of the feature on
> hypervisor kernel versions that support it: Amend the SEV_STATUS MSR
> SNP_RESERVED_MASK to exclude bit 23 (IbpbOnEntry).
>
> Fixes: 8c29f0165405 ("x86/sev: Add SEV-SNP guest feature negotiation support")> Cc: Nikunj A Dadhania <nikunj@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> CC: Borislav Petkov (AMD) <bp@alien8.de>
> CC: Michael Roth <michael.roth@amd.com>
> Cc: stable@kernel.org
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>
Apart from the above comments:
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/boot/compressed/sev.c | 1 +
> arch/x86/coco/sev/core.c | 1 +
> arch/x86/include/asm/msr-index.h | 5 ++++-
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index c8c1464b3a56..2b639703b8dd 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -188,6 +188,7 @@ bool sev_es_check_ghcb_fault(unsigned long address)
> MSR_AMD64_SNP_RESERVED_BIT13 | \
> MSR_AMD64_SNP_RESERVED_BIT15 | \
> MSR_AMD64_SNP_SECURE_AVIC | \
> + MSR_AMD64_SNP_RESERVED_BITS19_22 | \
> MSR_AMD64_SNP_RESERVED_MASK)
>
> #ifdef CONFIG_AMD_SECURE_AVIC
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 9ae3b11754e6..13f608117411 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -122,6 +122,7 @@ static const char * const sev_status_feat_names[] = {
> [MSR_AMD64_SNP_VMSA_REG_PROT_BIT] = "VMSARegProt",
> [MSR_AMD64_SNP_SMT_PROT_BIT] = "SMTProt",
> [MSR_AMD64_SNP_SECURE_AVIC_BIT] = "SecureAVIC",
> + [MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT] = "IBPBOnEntry",
> };
>
> /*
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 4d3566bb1a93..9016a6b00bc7 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -735,7 +735,10 @@
> #define MSR_AMD64_SNP_SMT_PROT BIT_ULL(MSR_AMD64_SNP_SMT_PROT_BIT)
> #define MSR_AMD64_SNP_SECURE_AVIC_BIT 18
> #define MSR_AMD64_SNP_SECURE_AVIC BIT_ULL(MSR_AMD64_SNP_SECURE_AVIC_BIT)
> -#define MSR_AMD64_SNP_RESV_BIT 19
> +#define MSR_AMD64_SNP_RESERVED_BITS19_22 GENMASK_ULL(22, 19)
> +#define MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT 23
> +#define MSR_AMD64_SNP_IBPB_ON_ENTRY BIT_ULL(MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT)
> +#define MSR_AMD64_SNP_RESV_BIT 24
> #define MSR_AMD64_SNP_RESERVED_MASK GENMASK_ULL(63, MSR_AMD64_SNP_RESV_BIT)
> #define MSR_AMD64_SAVIC_CONTROL 0xc0010138
> #define MSR_AMD64_SAVIC_EN_BIT 0
^ permalink raw reply
* Re: [PATCH v3 24/26] x86/virt/seamldr: Extend sigstruct to 16KB
From: Huang, Kai @ 2026-01-27 3:58 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, kas@kernel.org, mingo@redhat.com,
seanjc@google.com, Weiny, Ira, Chatre, Reinette,
tglx@linutronix.de, nik.borisov@suse.com, Verma, Vishal L,
hpa@zytor.com, sagis@google.com, Annapurve, Vishal,
Duan, Zhenzhong, Edgecombe, Rick P, paulmck@kernel.org,
bp@alien8.de, yilun.xu@linux.intel.com, Williams, Dan J
In-Reply-To: <20260123145645.90444-25-chao.gao@intel.com>
On Fri, 2026-01-23 at 06:55 -0800, Gao, Chao wrote:
> Currently, each TDX Module has a 4KB sigstruct that is passed to the
> P-SEAMLDR during module updates to authenticate the TDX Module binary.
>
> Future TDX Module versions will pack additional information into the
> sigstruct, which will exceed the current 4KB size limit.
>
> To accommodate this, the sigstruct is being extended to support up to
> 16KB.
>
[...]
> Update seamldr_params and tdx-blob structures to handle the larger
> sigstruct size.
Nit: there's no update to 'struct tdx_blob' in this patch.
Btw, is there any spec/doc that mentions this publicly?
E.g., we do need some update to the TDX module blob doc, right?
^ permalink raw reply
* Re: [PATCH v1 08/14] x86: make CONFIG_EFI_STUB unconditional
From: H. Peter Anvin @ 2026-01-27 3:21 UTC (permalink / raw)
To: Simon Glass
Cc: ubizjak, linux-kernel, akpm, bp, dave.hansen, kas, kees,
linux-coco, mingo, nathan, peterz, pmladek, rick.p.edgecombe,
tglx, x86
In-Reply-To: <CAFLszTgHvFvB=Be=MwctUfR4ngA=rxxHHP1A1M-XvXbT8yN9JA@mail.gmail.com>
On January 26, 2026 7:14:27 PM PST, Simon Glass <sjg@chromium.org> wrote:
>Hi Peter,
>
>On Tue, 27 Jan 2026 at 15:55, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On January 26, 2026 5:44:43 PM PST, Simon Glass <sjg@chromium.org> wrote:
>> >Hi Peter,
>> >
>> >On Tue, 27 Jan 2026 at 11:21, H. Peter Anvin <hpa@zytor.com> wrote:
>> >>
>> >> On 2026-01-26 13:19, Simon Glass wrote:
>> >> >>
>> >> >> Including the EFI stub doesn't mean using EFI to boot is required.
>> >> >
>> >> > Yes, understood, but it adds bloat. More importantly it will lead to
>> >> > people assuming that the stub is always used and thus unwittingly blur
>> >> > the boundary between the stub and the kernel itself.
>> >> >
>> >> > What is the actual need for this?
>> >> >
>> >>
>> >> I would argue that the opposite is more likely: someone inadvertently builds a
>> >> kernel without the stub, the bootloader goes down a legacy support path and
>> >> things seems to work... except for some platform subtleties.
>> >>
>> >> The bloat is there, but it is small and is only in the on-disk kernel image;
>> >> it is zero at runtime.
>> >>
>> >> As such, I don't think this option is a particularly good idea anymore. If
>> >> necessary, it could be hidden behind an EXPERT option, but I first wanted to
>> >> see who if anyone actually cares in a meaningful way to maintain this option.
>> >> Every option, after all, adds maintenance burden.
>> >>
>> >> Note that the BIOS stub is unconditionally compiled and included, and that has
>> >> not been an issue.
>> >
>> >What is the maintenance burden here? I could potentially take that on,
>> >but I would first want to understand what is involved.
>> >
>> >The use of the word 'legacy' worries me too. Is this patch a step
>> >towards removing the non-EFI path?
>> >
>> >Regards,
>> >Simon
>
>(joining the threads)
>
>> Bypassing the firmware stub (BIOS or EFI) is really only appropriate for special user cases, like kexec, because it removes the ability for the kernel to deal with system issues at an early point.
>
>Does this dealing happen in the EFI stub, or later? Are you referring
>to ACPI fix-ups or something else?
>
>>
>> However, kexec needs it, and it's not going to go away. However, that doesn't mean we should encourage this in cases which doesn't need it (no matter what the Grub maintainers tell you.)
>>
>> Now, if you are using KVM without EFI you are probably doing BIOS boot (regardless of if you know it or not), entering via the BIOS firmware stub.
>
>I am thinking of the 64-bit entry point to the kernel. everything
>being laid out in memory ready to go.
>
>>
>> I just realized you are the U-boot maintainer, so I'm assuming you are thinking of the case where there is no UEFI or BIOS firmware. In that case, just like as for kexec, the current entry point will continue to work, of course.
>>
>> What we don't want is having to suffer being on a BIOS or EFI system but not being able to leverage it for the benefit of the kernel. The kernel image is much easier to upgrade.
>
>More generally I am thinking about a simple and clean API for the
>kernel that doesn't involve having to provide 30K lines of firmware
>code just to boot. The BIOS entry point (if that is what it is called)
>is quite close to this ideal, even though I know it has shortcomings.
>
>Regards,
>Simon
Yes, I understand.
The 32/64-bit entrypoints aren't going away; it would be impossible to do so.
The general rule is: do things as late in the boot process as possible, but no later.
^ permalink raw reply
* Re: [PATCH v3 13/26] x86/virt/seamldr: Allocate and populate a module update request
From: Huang, Kai @ 2026-01-27 3:21 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, kas@kernel.org, seanjc@google.com,
Chatre, Reinette, Weiny, Ira, tglx@linutronix.de, Verma, Vishal L,
nik.borisov@suse.com, mingo@redhat.com, hpa@zytor.com,
sagis@google.com, Chen, Farrah, Duan, Zhenzhong,
Edgecombe, Rick P, paulmck@kernel.org, Annapurve, Vishal,
yilun.xu@linux.intel.com, Williams, Dan J, bp@alien8.de
In-Reply-To: <20260123145645.90444-14-chao.gao@intel.com>
> +/*
> + * Allocate and populate a seamldr_params.
> + * Note that both @module and @sig should be vmalloc'd memory.
> + */
> +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;
> +
> + BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
> + if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> + return ERR_PTR(-EINVAL);
> +
> + if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
> + !IS_ALIGNED((unsigned long)module, SZ_4K) ||
> + !IS_ALIGNED((unsigned long)sig, SZ_4K))
> + return ERR_PTR(-EINVAL);
> +
Based on the the blob format link below, we have
struct tdx_blob
{
...
_u64 sigstruct[256]; // 2KB sigstruct,intel_tdx_module.so.sigstruct
_u64 reserved2[256]; // Reserved space
...
}
So it's clear SIGSTRUCT is just 2KB and the second half 2KB is "reserved
space".
Why is the "reserved space" treated as part of SIGSTRUCT here?
> +
> +/*
> + * Intel TDX Module blob. Its format is defined at:
> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
> + */
> +struct tdx_blob {
> + u16 version;
> + u16 checksum;
> + u32 offset_of_module;
> + u8 signature[8];
> + u32 len;
> + u32 resv1;
> + u64 resv2[509];
Nit: Perhaps s/resv/rsvd ?
"#grep rsvd arch/x86 -Rn" gave me a bunch of results but "#grep resv" gave
me much less (and part of the results were 'resvd' and 'resv_xx' instead of
plain 'resv').
> + u8 data[];
> +} __packed;
For this structure, I need to click the link and open it in a browser to
understand where is the sigstruct and module, and ...
> +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 (blob->version != 0x100) {
> + pr_err("unsupported blob version: %x\n", blob->version);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (blob->resv1 || memchr_inv(blob->resv2, 0, sizeof(blob->resv2))) {
> + pr_err("non-zero reserved fields\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + /* Split the given 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;
> +
... to see whether this code makes sense.
I understand the
...
u64 rsvd[N*512];
u8 module[];
is painful to be declared explicitly in 'struct tdx_blob' because IIUC we
cannot put two flexible array members at the end of the structure.
But I think if we add 'sigstruct' to the 'struct tdx_blob', e.g.,
struct tdx_blob {
u16 version;
...
u64 rsvd2[509];
u64 sigstruct[256];
u64 rsvd3[256];
u64 data;
} __packed;
.. we can just use
sig = blob->sigstruct;
sig_size = 2K (or 4K I don't quite follow);
which is clearer to read IMHO?
> + return alloc_seamldr_params(module, module_size, sig, sig_size);
> +}
> +
^ permalink raw reply
* Re: [PATCH v1 08/14] x86: make CONFIG_EFI_STUB unconditional
From: Simon Glass @ 2026-01-27 3:14 UTC (permalink / raw)
To: H. Peter Anvin
Cc: ubizjak, linux-kernel, akpm, bp, dave.hansen, kas, kees,
linux-coco, mingo, nathan, peterz, pmladek, rick.p.edgecombe,
tglx, x86
In-Reply-To: <E15F74BD-E33C-4184-B694-C152696C4895@zytor.com>
Hi Peter,
On Tue, 27 Jan 2026 at 15:55, H. Peter Anvin <hpa@zytor.com> wrote:
>
> On January 26, 2026 5:44:43 PM PST, Simon Glass <sjg@chromium.org> wrote:
> >Hi Peter,
> >
> >On Tue, 27 Jan 2026 at 11:21, H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >> On 2026-01-26 13:19, Simon Glass wrote:
> >> >>
> >> >> Including the EFI stub doesn't mean using EFI to boot is required.
> >> >
> >> > Yes, understood, but it adds bloat. More importantly it will lead to
> >> > people assuming that the stub is always used and thus unwittingly blur
> >> > the boundary between the stub and the kernel itself.
> >> >
> >> > What is the actual need for this?
> >> >
> >>
> >> I would argue that the opposite is more likely: someone inadvertently builds a
> >> kernel without the stub, the bootloader goes down a legacy support path and
> >> things seems to work... except for some platform subtleties.
> >>
> >> The bloat is there, but it is small and is only in the on-disk kernel image;
> >> it is zero at runtime.
> >>
> >> As such, I don't think this option is a particularly good idea anymore. If
> >> necessary, it could be hidden behind an EXPERT option, but I first wanted to
> >> see who if anyone actually cares in a meaningful way to maintain this option.
> >> Every option, after all, adds maintenance burden.
> >>
> >> Note that the BIOS stub is unconditionally compiled and included, and that has
> >> not been an issue.
> >
> >What is the maintenance burden here? I could potentially take that on,
> >but I would first want to understand what is involved.
> >
> >The use of the word 'legacy' worries me too. Is this patch a step
> >towards removing the non-EFI path?
> >
> >Regards,
> >Simon
(joining the threads)
> Bypassing the firmware stub (BIOS or EFI) is really only appropriate for special user cases, like kexec, because it removes the ability for the kernel to deal with system issues at an early point.
Does this dealing happen in the EFI stub, or later? Are you referring
to ACPI fix-ups or something else?
>
> However, kexec needs it, and it's not going to go away. However, that doesn't mean we should encourage this in cases which doesn't need it (no matter what the Grub maintainers tell you.)
>
> Now, if you are using KVM without EFI you are probably doing BIOS boot (regardless of if you know it or not), entering via the BIOS firmware stub.
I am thinking of the 64-bit entry point to the kernel. everything
being laid out in memory ready to go.
>
> I just realized you are the U-boot maintainer, so I'm assuming you are thinking of the case where there is no UEFI or BIOS firmware. In that case, just like as for kexec, the current entry point will continue to work, of course.
>
> What we don't want is having to suffer being on a BIOS or EFI system but not being able to leverage it for the benefit of the kernel. The kernel image is much easier to upgrade.
More generally I am thinking about a simple and clean API for the
kernel that doesn't involve having to provide 30K lines of firmware
code just to boot. The BIOS entry point (if that is what it is called)
is quite close to this ideal, even though I know it has shortcomings.
Regards,
Simon
^ permalink raw reply
* Re: [PATCH v1 08/14] x86: make CONFIG_EFI_STUB unconditional
From: H. Peter Anvin @ 2026-01-27 2:54 UTC (permalink / raw)
To: Simon Glass
Cc: ubizjak, linux-kernel, akpm, bp, dave.hansen, kas, kees,
linux-coco, mingo, nathan, peterz, pmladek, rick.p.edgecombe,
tglx, x86
In-Reply-To: <CAFLszTjhJkfBrG-QiF6cEz-K1ODHPziJDsTthBUsqoZ=hDxsjA@mail.gmail.com>
On January 26, 2026 5:44:43 PM PST, Simon Glass <sjg@chromium.org> wrote:
>Hi Peter,
>
>On Tue, 27 Jan 2026 at 11:21, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On 2026-01-26 13:19, Simon Glass wrote:
>> >>
>> >> Including the EFI stub doesn't mean using EFI to boot is required.
>> >
>> > Yes, understood, but it adds bloat. More importantly it will lead to
>> > people assuming that the stub is always used and thus unwittingly blur
>> > the boundary between the stub and the kernel itself.
>> >
>> > What is the actual need for this?
>> >
>>
>> I would argue that the opposite is more likely: someone inadvertently builds a
>> kernel without the stub, the bootloader goes down a legacy support path and
>> things seems to work... except for some platform subtleties.
>>
>> The bloat is there, but it is small and is only in the on-disk kernel image;
>> it is zero at runtime.
>>
>> As such, I don't think this option is a particularly good idea anymore. If
>> necessary, it could be hidden behind an EXPERT option, but I first wanted to
>> see who if anyone actually cares in a meaningful way to maintain this option.
>> Every option, after all, adds maintenance burden.
>>
>> Note that the BIOS stub is unconditionally compiled and included, and that has
>> not been an issue.
>
>What is the maintenance burden here? I could potentially take that on,
>but I would first want to understand what is involved.
>
>The use of the word 'legacy' worries me too. Is this patch a step
>towards removing the non-EFI path?
>
>Regards,
>Simon
I just realized you are the U-boot maintainer, so I'm assuming you are thinking of the case where there is no UEFI or BIOS firmware. In that case, just like as for kexec, the current entry point will continue to work, of course.
What we don't want is having to suffer being on a BIOS or EFI system but not being able to leverage it for the benefit of the kernel. The kernel image is much easier to upgrade.
^ permalink raw reply
* Re: [PATCH v2 2/7] KVM: x86: Extract VMXON and EFER.SVME enablement to kernel
From: Binbin Wu @ 2026-01-27 2:46 UTC (permalink / raw)
To: Sean Christopherson, Xu Yilun
Cc: Chao Gao, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Kiryl Shutsemau, Paolo Bonzini, linux-kernel,
linux-coco, kvm, Dan Williams
In-Reply-To: <aUVx20ZRjOzKgKqy@google.com>
On 12/19/2025 11:40 PM, Sean Christopherson wrote:
> On Fri, Dec 19, 2025, Xu Yilun wrote:
>> On Wed, Dec 17, 2025 at 11:01:59AM -0800, Sean Christopherson wrote:
>>> On Wed, Dec 17, 2025, Xu Yilun wrote:
>>>> Is it better we explicitly assert the preemption for x86_virt_get_cpu()
>>>> rather than embed the check in __this_cpu_inc_return()? We are not just
>>>> protecting the racing for the reference counter. We should ensure the
>>>> "counter increase + x86_virt_call(get_cpu)" can't be preempted.
>>>
>>> I don't have a strong preference. Using __this_cpu_inc_return() without any
>>> nearby preemption_{enable,disable}() calls makes it quite clears that preemption
>>> is expected to be disabled by the caller. But I'm also ok being explicit.
>>
>> Looking into __this_cpu_inc_return(), it finally calls
>> check_preemption_disabled() which doesn't strictly requires preemption.
>> It only ensures the context doesn't switch to another CPU. If the caller
>> is in cpuhp context, preemption is possible.
>
> Hmm, right, the cpuhp thread is is_percpu_thread(), and KVM's hooks aren't
> considered atomic and so run with IRQs enabled. In practice, it's "fine", because
> TDX also exclusively does x86_virt_get_cpu() from cpuhp, i.e. the two users are
> mutually exclusive, but relying on that behavior is gross.
>
Side topic:
Isn't the name of check_preemption_disabled() confusing and arguably misleading?
^ permalink raw reply
* Re: [PATCH v1 08/14] x86: make CONFIG_EFI_STUB unconditional
From: H. Peter Anvin @ 2026-01-27 2:39 UTC (permalink / raw)
To: Simon Glass
Cc: ubizjak, linux-kernel, akpm, bp, dave.hansen, kas, kees,
linux-coco, mingo, nathan, peterz, pmladek, rick.p.edgecombe,
tglx, x86
In-Reply-To: <CAFLszTjhJkfBrG-QiF6cEz-K1ODHPziJDsTthBUsqoZ=hDxsjA@mail.gmail.com>
On January 26, 2026 5:44:43 PM PST, Simon Glass <sjg@chromium.org> wrote:
>Hi Peter,
>
>On Tue, 27 Jan 2026 at 11:21, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On 2026-01-26 13:19, Simon Glass wrote:
>> >>
>> >> Including the EFI stub doesn't mean using EFI to boot is required.
>> >
>> > Yes, understood, but it adds bloat. More importantly it will lead to
>> > people assuming that the stub is always used and thus unwittingly blur
>> > the boundary between the stub and the kernel itself.
>> >
>> > What is the actual need for this?
>> >
>>
>> I would argue that the opposite is more likely: someone inadvertently builds a
>> kernel without the stub, the bootloader goes down a legacy support path and
>> things seems to work... except for some platform subtleties.
>>
>> The bloat is there, but it is small and is only in the on-disk kernel image;
>> it is zero at runtime.
>>
>> As such, I don't think this option is a particularly good idea anymore. If
>> necessary, it could be hidden behind an EXPERT option, but I first wanted to
>> see who if anyone actually cares in a meaningful way to maintain this option.
>> Every option, after all, adds maintenance burden.
>>
>> Note that the BIOS stub is unconditionally compiled and included, and that has
>> not been an issue.
>
>What is the maintenance burden here? I could potentially take that on,
>but I would first want to understand what is involved.
>
>The use of the word 'legacy' worries me too. Is this patch a step
>towards removing the non-EFI path?
>
>Regards,
>Simon
Bypassing the firmware stub (BIOS or EFI) is really only appropriate for special user cases, like kexec, because it removes the ability for the kernel to deal with system issues at an early point.
However, kexec needs it, and it's not going to go away. However, that doesn't mean we should encourage this in cases which doesn't need it (no matter what the Grub maintainers tell you.)
Now, if you are using KVM without EFI you are probably doing BIOS boot (regardless of if you know it or not), entering via the BIOS firmware stub.
^ permalink raw reply
* Re: [PATCH v1 08/14] x86: make CONFIG_EFI_STUB unconditional
From: Simon Glass @ 2026-01-27 1:44 UTC (permalink / raw)
To: H. Peter Anvin
Cc: ubizjak, linux-kernel, akpm, bp, dave.hansen, kas, kees,
linux-coco, mingo, nathan, peterz, pmladek, rick.p.edgecombe,
tglx, x86
In-Reply-To: <643bfc24-8cc1-4f70-85e2-f6829c8e03db@zytor.com>
Hi Peter,
On Tue, 27 Jan 2026 at 11:21, H. Peter Anvin <hpa@zytor.com> wrote:
>
> On 2026-01-26 13:19, Simon Glass wrote:
> >>
> >> Including the EFI stub doesn't mean using EFI to boot is required.
> >
> > Yes, understood, but it adds bloat. More importantly it will lead to
> > people assuming that the stub is always used and thus unwittingly blur
> > the boundary between the stub and the kernel itself.
> >
> > What is the actual need for this?
> >
>
> I would argue that the opposite is more likely: someone inadvertently builds a
> kernel without the stub, the bootloader goes down a legacy support path and
> things seems to work... except for some platform subtleties.
>
> The bloat is there, but it is small and is only in the on-disk kernel image;
> it is zero at runtime.
>
> As such, I don't think this option is a particularly good idea anymore. If
> necessary, it could be hidden behind an EXPERT option, but I first wanted to
> see who if anyone actually cares in a meaningful way to maintain this option.
> Every option, after all, adds maintenance burden.
>
> Note that the BIOS stub is unconditionally compiled and included, and that has
> not been an issue.
What is the maintenance burden here? I could potentially take that on,
but I would first want to understand what is involved.
The use of the word 'legacy' worries me too. Is this patch a step
towards removing the non-EFI path?
Regards,
Simon
^ permalink raw reply
* [PATCH 2/2] KVM: SEV: Add support for IBPB-on-Entry
From: Kim Phillips @ 2026-01-26 22:42 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, Kim Phillips
In-Reply-To: <20260126224205.1442196-1-kim.phillips@amd.com>
AMD EPYC 5th generation and above processors support IBPB-on-Entry
for SNP guests. By invoking an Indirect Branch Prediction Barrier
(IBPB) on VMRUN, old indirect branch predictions are prevented
from influencing indirect branches within the guest.
SNP guests may choose to enable IBPB-on-Entry by setting
SEV_FEATURES bit 21 (IbpbOnEntry).
Host support for IBPB on Entry is indicated by CPUID
Fn8000_001F[IbpbOnEntry], bit 31.
If supported, indicate support for IBPB on Entry in
sev_supported_vmsa_features bit 23 (IbpbOnEntry).
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>
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 9 ++++++++-
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index c01fdde465de..3ce5dff36f78 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -459,6 +459,7 @@
#define X86_FEATURE_ALLOWED_SEV_FEATURES (19*32+27) /* Allowed SEV Features */
#define X86_FEATURE_SVSM (19*32+28) /* "svsm" SVSM present */
#define X86_FEATURE_HV_INUSE_WR_ALLOWED (19*32+30) /* Allow Write to in-use hypervisor-owned pages */
+#define X86_FEATURE_IBPB_ON_ENTRY (19*32+31) /* SEV-SNP IBPB on VM Entry */
/* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
#define X86_FEATURE_NO_NESTED_DATA_BP (20*32+ 0) /* No Nested Data Breakpoints */
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index edde36097ddc..eebc65ec948f 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -306,6 +306,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
+#define SVM_SEV_FEAT_IBPB_ON_ENTRY BIT(21)
#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 ea515cf41168..8a6d25db0c00 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3165,8 +3165,15 @@ void __init sev_hardware_setup(void)
cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
- if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
+ if (!sev_snp_enabled)
+ return;
+ /* the following feature bit checks are SNP specific */
+
+ if (tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+
+ if (cpu_feature_enabled(X86_FEATURE_IBPB_ON_ENTRY))
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_IBPB_ON_ENTRY;
}
void sev_hardware_unsetup(void)
--
2.43.0
^ permalink raw reply related
* [PATCH 1/2] KVM: SEV: IBPB-on-Entry guest support
From: Kim Phillips @ 2026-01-26 22:42 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, Kim Phillips, stable
In-Reply-To: <20260126224205.1442196-1-kim.phillips@amd.com>
The SEV-SNP IBPB-on-Entry feature does not require a guest-side
implementation. The feature was added in Zen5 h/w, after the first
SNP Zen implementation, and thus was not accounted for when the
initial set of SNP features were added to the kernel.
In its abundant precaution, commit 8c29f0165405 ("x86/sev: Add SEV-SNP
guest feature negotiation support") included SEV_STATUS' IBPB-on-Entry
bit as a reserved bit, thereby masking guests from using the feature.
Unmask the bit, to allow guests to take advantage of the feature on
hypervisor kernel versions that support it: Amend the SEV_STATUS MSR
SNP_RESERVED_MASK to exclude bit 23 (IbpbOnEntry).
Fixes: 8c29f0165405 ("x86/sev: Add SEV-SNP guest feature negotiation support")
Cc: Nikunj A Dadhania <nikunj@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
CC: Borislav Petkov (AMD) <bp@alien8.de>
CC: Michael Roth <michael.roth@amd.com>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/coco/sev/core.c | 1 +
arch/x86/include/asm/msr-index.h | 5 ++++-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index c8c1464b3a56..2b639703b8dd 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -188,6 +188,7 @@ bool sev_es_check_ghcb_fault(unsigned long address)
MSR_AMD64_SNP_RESERVED_BIT13 | \
MSR_AMD64_SNP_RESERVED_BIT15 | \
MSR_AMD64_SNP_SECURE_AVIC | \
+ MSR_AMD64_SNP_RESERVED_BITS19_22 | \
MSR_AMD64_SNP_RESERVED_MASK)
#ifdef CONFIG_AMD_SECURE_AVIC
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 9ae3b11754e6..13f608117411 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -122,6 +122,7 @@ static const char * const sev_status_feat_names[] = {
[MSR_AMD64_SNP_VMSA_REG_PROT_BIT] = "VMSARegProt",
[MSR_AMD64_SNP_SMT_PROT_BIT] = "SMTProt",
[MSR_AMD64_SNP_SECURE_AVIC_BIT] = "SecureAVIC",
+ [MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT] = "IBPBOnEntry",
};
/*
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 4d3566bb1a93..9016a6b00bc7 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -735,7 +735,10 @@
#define MSR_AMD64_SNP_SMT_PROT BIT_ULL(MSR_AMD64_SNP_SMT_PROT_BIT)
#define MSR_AMD64_SNP_SECURE_AVIC_BIT 18
#define MSR_AMD64_SNP_SECURE_AVIC BIT_ULL(MSR_AMD64_SNP_SECURE_AVIC_BIT)
-#define MSR_AMD64_SNP_RESV_BIT 19
+#define MSR_AMD64_SNP_RESERVED_BITS19_22 GENMASK_ULL(22, 19)
+#define MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT 23
+#define MSR_AMD64_SNP_IBPB_ON_ENTRY BIT_ULL(MSR_AMD64_SNP_IBPB_ON_ENTRY_BIT)
+#define MSR_AMD64_SNP_RESV_BIT 24
#define MSR_AMD64_SNP_RESERVED_MASK GENMASK_ULL(63, MSR_AMD64_SNP_RESV_BIT)
#define MSR_AMD64_SAVIC_CONTROL 0xc0010138
#define MSR_AMD64_SAVIC_EN_BIT 0
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] KVM: SEV: Add support for IBPB-on-Entry
From: Kim Phillips @ 2026-01-26 22:42 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, Kim Phillips
AMD EPYC 5th generation and above processors support IBPB-on-Entry
for SNP guests. By invoking an Indirect Branch Prediction Barrier
(IBPB) on VMRUN, old indirect branch predictions are prevented
from influencing indirect branches within the guest.
The first patch is guest-side support which unmasks the Zen5+ feature
bit to allow kernel guests to set the feature.
The second patch is host-side support that checks the CPUID and
then sets the feature bit in the VMSA supported features mask.
Based on https://github.com/kvm-x86/linux kvm-x86/next
(kvm-x86-next-2026.01.23, e81f7c908e16).
This series also available here:
https://github.com/AMDESE/linux/tree/ibpb-on-entry-latest
Advance qemu bits (to add ibpb-on-entry=on/off switch) available here:
https://github.com/AMDESE/qemu/tree/ibpb-on-entry-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/
Kim Phillips (2):
KVM: SEV: IBPB-on-Entry guest support
KVM: SEV: Add support for IBPB-on-Entry
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/coco/sev/core.c | 1 +
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/msr-index.h | 5 ++++-
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 9 ++++++++-
6 files changed, 16 insertions(+), 2 deletions(-)
base-commit: e81f7c908e1664233974b9f20beead78cde6343a
--
2.43.0
^ permalink raw reply
* Re: [PATCH v1 08/14] x86: make CONFIG_EFI_STUB unconditional
From: H. Peter Anvin @ 2026-01-26 22:20 UTC (permalink / raw)
To: Simon Glass
Cc: ubizjak, linux-kernel, akpm, bp, dave.hansen, kas, kees,
linux-coco, mingo, nathan, peterz, pmladek, rick.p.edgecombe,
tglx, x86
In-Reply-To: <CAFLszTi+N7Yj0yYHKroypmBoFSx+9m5er8pgULsKbPuooFYj-A@mail.gmail.com>
On 2026-01-26 13:19, Simon Glass wrote:
>>
>> Including the EFI stub doesn't mean using EFI to boot is required.
>
> Yes, understood, but it adds bloat. More importantly it will lead to
> people assuming that the stub is always used and thus unwittingly blur
> the boundary between the stub and the kernel itself.
>
> What is the actual need for this?
>
I would argue that the opposite is more likely: someone inadvertently builds a
kernel without the stub, the bootloader goes down a legacy support path and
things seems to work... except for some platform subtleties.
The bloat is there, but it is small and is only in the on-disk kernel image;
it is zero at runtime.
As such, I don't think this option is a particularly good idea anymore. If
necessary, it could be hidden behind an EXPERT option, but I first wanted to
see who if anyone actually cares in a meaningful way to maintain this option.
Every option, after all, adds maintenance burden.
Note that the BIOS stub is unconditionally compiled and included, and that has
not been an issue.
-hpa
^ permalink raw reply
* Re: [PATCH v3 26/26] coco/tdx-host: Set and document TDX Module update expectations
From: dan.j.williams @ 2026-01-26 22:14 UTC (permalink / raw)
To: Chao Gao, linux-coco, linux-kernel, kvm, x86
Cc: reinette.chatre, ira.weiny, kai.huang, dan.j.williams, yilun.xu,
sagis, vannapurve, paulmck, nik.borisov, zhenzhong.duan, seanjc,
rick.p.edgecombe, kas, dave.hansen, vishal.l.verma, Chao Gao
In-Reply-To: <20260123145645.90444-27-chao.gao@intel.com>
Chao Gao wrote:
> In rare cases, TDX Module updates may cause TD management operations to
> fail if they occur during phases of the TD lifecycle that are sensitive
> to update compatibility.
No. The TDX Module wants to be able to claim that some updates are
compatible when they are not. If Linux takes on additional exclusions it
modestly increases the scope of changes that can be included in an
update. It is not possible to claim "rare" if module updates routinely
include that problematic scope.
> But not all combinations of P-SEAMLDR, kernel, and TDX Module have the
> capability to detect and prevent said incompatibilities. Completely
> disabling TDX Module updates on platforms without the capability would
> be overkill, as these incompatibility cases are rare and can be
> addressed by userspace through coordinated scheduling of updates and TD
> management operations.
"Completely disabling" is not the tradeoff. The tradeoff is whether or
not the TDX Module meets Linux compatible update requirements or not.
> To set clear expectations for TDX Module updates, expose the capability
> to detect and prevent these incompatibility cases via sysfs and
> document the compatibility criteria and indications when those criteria
> are violated.
Linux derives no benefit from a "compat_capable" kernel ABI. Yes, the
internals must export the error condition on collision. I am not
debating that nor revisiting the decision of pre-update-fail, vs
post-collision-notify. However, if the module violates the Linux
expectations that is the module's issue to document or preclude. The
fact that the compatibility contract is ambiguous to the kernel is a
feature. It puts the onus squarely on module updates to be documented
(or tools updated to understand) as meeting or violating Linux
compatibility expectations.
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> v3:
> - new, based on a reference patch from Dan Williams
One of the details that is missing is the protocol (module documentation
or tooling) to determine ahead of time if an update is compatible. That
obviates the need for "compat_capable" ABI which serves no long term
purpose. Specifically, the expectation is "run non-compatible updates at
your own operational risk".
So, remove "compat_capable" ABI. Amend the "error" ABI documentation
with the details for avoiding failures and the risk of running updates
on configurations that support update but not collision avoidance.
> ---
> .../ABI/testing/sysfs-devices-faux-tdx-host | 45 +++++++++++++++++++
> drivers/virt/coco/tdx-host/tdx-host.c | 13 ++++++
> 2 files changed, 58 insertions(+)
[..]
>
> +What: /sys/devices/faux/tdx_host/firmware/seamldr_upload/error
> +Contact: linux-coco@lists.linux.dev
> +Description: (RO) See Documentation/ABI/testing/sysfs-class-firmware for
> + baseline expectations for this file. Updates that fail
> + compatibility checks end with the "device-busy" error in the
> + <STATUS>:<ERROR> format of this attribute. When this is
> + signalled current TDs and the current TDX Module stay running.
This wants something like
---
See version_select_and_load.py [1] documentation for how to detect
compatible updates and whether the current platform components catch
errors or let them leak and cause potential TD attestation failures.
[1]: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/version_select_and_load.py
---
...although I do not immediately see any help text or Documentation for
that tool.
^ permalink raw reply
* Re: [PATCH v2 20/21] x86/virt/tdx: Update tdx_sysinfo and check features post-update
From: dan.j.williams @ 2026-01-26 21:22 UTC (permalink / raw)
To: Chao Gao, Binbin Wu
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <aW34Z7CfQORSFZLh@intel.com>
Chao Gao wrote:
[..]
> I think this conservative approach is appropriate for initial support. New
> features can be gradually enabled later as we prove that other components
> (e.g., KVM) are ready for new features introduced via runtime updates.
New features over updates, and revised software contracts (broken
contracts) over updates are not "compatible updates". So it is not a
"conservative approach" to start, it is more a hard barrier that updates
of that nature are out-of-scope via this interface. The Documentation
needs to be clear that the problems caused by update are the Module's
problems, not new kernel problems.
If you look at other platform mutating update mechanisms like ACPI
runtime update and CXL runtime update, the class of updates that expose
new features / contracts require reset. TDX is not special in this
regard.
^ permalink raw reply
* Re: [PATCH v1 08/14] x86: make CONFIG_EFI_STUB unconditional
From: Simon Glass @ 2026-01-26 21:19 UTC (permalink / raw)
To: H. Peter Anvin
Cc: ubizjak, linux-kernel, akpm, bp, dave.hansen, kas, kees,
linux-coco, mingo, nathan, peterz, pmladek, rick.p.edgecombe,
tglx, x86
In-Reply-To: <90DAAB54-CBF0-47BD-981A-FEDD26CDA0FD@zytor.com>
Hi Peter,
On Fri, 23 Jan 2026 at 13:11, H. Peter Anvin <hpa@zytor.com> wrote:
>
> On January 22, 2026 10:57:39 AM PST, Simon Glass <sjg@chromium.org> wrote:
> >Hi Peter,
> >
> >On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >> The EFI stub code is mature, most current x86 systems require EFI to
> >> boot, and as it is exclusively preboot code, it doesn't affect the
> >> runtime memory footprint at all.
> >>
> >> It makes absolutely no sense to omit it anymore, so make it
> >> unconditional.
> >>
> >> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> >> ---
> >> arch/x86/Kconfig | 14 ++------------
> >> arch/x86/boot/compressed/Makefile | 2 --
> >> arch/x86/boot/compressed/error.c | 2 --
> >> arch/x86/boot/header.S | 3 ---
> >> 4 files changed, 2 insertions(+), 19 deletions(-)
> >
> >At least with QEMU the EFI protocol adds quite a lot of overhead.
> >
> >Is there any actual need for this?
> >
> >Regards,
> >Simon
> >
>
> Including the EFI stub doesn't mean using EFI to boot is required.
Yes, understood, but it adds bloat. More importantly it will lead to
people assuming that the stub is always used and thus unwittingly blur
the boundary between the stub and the kernel itself.
What is the actual need for this?
Regards,
Simon
^ 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