* [PATCH 2/2] dma-direct: Make phys_to_dma() pick encrypted vs unencrypted per device
From: Aneesh Kumar K.V (Arm) @ 2026-01-20 6:42 UTC (permalink / raw)
To: linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, robin.murphy, suzuki.poulose, jgg,
steven.price, Marek Szyprowski, Aneesh Kumar K.V (Arm)
In-Reply-To: <20260120064255.179425-1-aneesh.kumar@kernel.org>
On systems that apply an address encryption tag/mask to DMA addresses, the
choice of encrypted vs unencrypted DMA address is device-dependent (e.g.
TDISP trusted devices vs non-trusted devices).
Teach phys_to_dma() to make this choice based on
force_dma_unencrypted(dev), and convert dma-direct users to call
phys_to_dma() directly. With this in place, drop phys_to_dma_direct() as
redundant.
Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
---
include/linux/dma-direct.h | 18 +++++++++++-------
kernel/dma/direct.c | 20 ++++++--------------
2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index c249912456f9..e2e3a08373a1 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -90,17 +90,21 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
{
return dma_addr_unencrypted(__phys_to_dma(dev, paddr));
}
-/*
- * If memory encryption is supported, phys_to_dma will set the memory encryption
- * bit in the DMA address, and dma_to_phys will clear it.
- * phys_to_dma_unencrypted is for use on special unencrypted memory like swiotlb
- * buffers.
- */
-static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
+
+static inline dma_addr_t phys_to_dma_encrypted(struct device *dev,
+ phys_addr_t paddr)
{
return dma_addr_encrypted(__phys_to_dma(dev, paddr));
}
+static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
+{
+
+ if (force_dma_unencrypted(dev))
+ return phys_to_dma_unencrypted(dev, paddr);
+ return phys_to_dma_encrypted(dev, paddr);
+}
+
static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
{
phys_addr_t paddr;
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index a5639e9415f5..59d7d9e15e17 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -23,14 +23,6 @@
*/
u64 zone_dma_limit __ro_after_init = DMA_BIT_MASK(24);
-static inline dma_addr_t phys_to_dma_direct(struct device *dev,
- phys_addr_t phys)
-{
- if (force_dma_unencrypted(dev))
- return phys_to_dma_unencrypted(dev, phys);
- return phys_to_dma(dev, phys);
-}
-
static inline struct page *dma_direct_to_page(struct device *dev,
dma_addr_t dma_addr)
{
@@ -40,7 +32,7 @@ static inline struct page *dma_direct_to_page(struct device *dev,
u64 dma_direct_get_required_mask(struct device *dev)
{
phys_addr_t phys = (phys_addr_t)(max_pfn - 1) << PAGE_SHIFT;
- u64 max_dma = phys_to_dma_direct(dev, phys);
+ u64 max_dma = phys_to_dma(dev, phys);
return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
}
@@ -69,7 +61,7 @@ static gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 *phys_limit)
bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
{
- dma_addr_t dma_addr = phys_to_dma_direct(dev, phys);
+ dma_addr_t dma_addr = phys_to_dma(dev, phys);
if (dma_addr == DMA_MAPPING_ERROR)
return false;
@@ -178,7 +170,7 @@ static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
if (!page)
return NULL;
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
}
@@ -196,7 +188,7 @@ static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
arch_dma_prep_coherent(page, size);
/* return the page pointer as the opaque cookie */
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return page;
}
@@ -311,7 +303,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
goto out_encrypt_pages;
}
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
out_encrypt_pages:
@@ -392,7 +384,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
if (dma_set_decrypted(dev, ret, size))
goto out_leak_pages;
memset(ret, 0, size);
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return page;
out_leak_pages:
return NULL;
--
2.43.0
^ permalink raw reply related
* [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Aneesh Kumar K.V (Arm) @ 2026-01-20 6:42 UTC (permalink / raw)
To: linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, robin.murphy, suzuki.poulose, jgg,
steven.price, Marek Szyprowski, Aneesh Kumar K.V (Arm)
On systems that apply an address encryption tag or mask to DMA addresses,
DMA mask validation must be performed against the canonical DMA address.
Using a non-canonical (e.g. encrypted or unencrypted) DMA address
can incorrectly fail capability checks, since architecture-specific
encryption bits are not part of the device’s actual DMA addressing
capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
addresses.
Fix this by validating device DMA masks against __phys_to_dma(), ensuring
that the architecture encryption mask does not influence the check.
Fixes: b66e2ee7b6c8 ("dma: Introduce generic dma_addr_*crypted helpers")
Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
---
kernel/dma/direct.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 8e04f72baaa3..a5639e9415f5 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
/*
* This check needs to be against the actual bit mask value, so use
- * phys_to_dma_unencrypted() here so that the SME encryption mask isn't
+ * __phys_to_dma() here so that the arch specific encryption mask isn't
* part of the check.
*/
if (IS_ENABLED(CONFIG_ZONE_DMA))
min_mask = min_t(u64, min_mask, zone_dma_limit);
- return mask >= phys_to_dma_unencrypted(dev, min_mask);
+ return mask >= __phys_to_dma(dev, min_mask);
}
static const struct bus_dma_region *dma_find_range(struct device *dev,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: kernel test robot @ 2026-01-19 21:35 UTC (permalink / raw)
To: Salman Nabi, vvidwans, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: llvm, oe-kbuild-all, ardb, chao.gao, linux-arm-kernel, linux-coco,
linux-kernel, sdonthineni, vsethi, vwadekar
In-Reply-To: <20260119122729.287522-2-salman.nabi@arm.com>
Hi Salman,
kernel test robot noticed the following build errors:
[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.19-rc6 next-20260116]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Salman-Nabi/firmware-smccc-add-support-for-Live-Firmware-Activation-LFA/20260119-203221
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20260119122729.287522-2-salman.nabi%40arm.com
patch subject: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
config: arm-defconfig (https://download.01.org/0day-ci/archive/20260120/202601200543.EKFOBfnW-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601200543.EKFOBfnW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601200543.EKFOBfnW-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/firmware/smccc/lfa_fw.c:179:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:179:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
>> drivers/firmware/smccc/lfa_fw.c:184:2: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
184 | arm_smccc_1_2_invoke(®, ®);
| ^
drivers/firmware/smccc/lfa_fw.c:194:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:194:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:198:2: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
198 | arm_smccc_1_2_invoke(®, ®);
| ^
drivers/firmware/smccc/lfa_fw.c:220:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:220:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:234:3: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
234 | arm_smccc_1_2_invoke(®, ®);
| ^
drivers/firmware/smccc/lfa_fw.c:290:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:290:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:318:3: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
318 | arm_smccc_1_2_invoke(®, ®);
| ^
drivers/firmware/smccc/lfa_fw.c:360:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:360:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:368:2: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
368 | arm_smccc_1_2_invoke(®, ®);
| ^
drivers/firmware/smccc/lfa_fw.c:434:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:434:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:443:2: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
443 | arm_smccc_1_2_invoke(®, ®);
| ^
drivers/firmware/smccc/lfa_fw.c:586:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:586:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:600:3: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
600 | arm_smccc_1_2_invoke(®, ®);
| ^
drivers/firmware/smccc/lfa_fw.c:619:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:619:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:623:2: error: call to undeclared function 'arm_smccc_1_2_invoke'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
623 | arm_smccc_1_2_invoke(®, ®);
| ^
16 errors generated.
vim +179 drivers/firmware/smccc/lfa_fw.c
176
177 static unsigned long get_nr_lfa_components(void)
178 {
> 179 struct arm_smccc_1_2_regs reg = { 0 };
180
181 reg.a0 = LFA_1_0_FN_GET_INFO;
182 reg.a1 = 0; /* lfa_info_selector = 0 */
183
> 184 arm_smccc_1_2_invoke(®, ®);
185 if (reg.a0 != LFA_SUCCESS)
186 return reg.a0;
187
188 return reg.a1;
189 }
190
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [RFC PATCH 5/5] firmware: smccc: lfa: refresh fw details
From: Salman Nabi @ 2026-01-19 19:50 UTC (permalink / raw)
To: Vedashree Vidwans, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <20251208221319.1524888-6-vvidwans@nvidia.com>
Hi Veda,
First of all sorry for the delay in responding back to you. You may have already seen my patch submission, following the RFC, here: https://lore.kernel.org/all/20260119122729.287522-2-salman.nabi@arm.com/
I have incorporated all the bug fixes and code refactor/cleanup suggestions from the RFC. It also includes changes to the LFA spec, mainly current and pending firmware version retrieval. I did not include your work and am hoping you would submit follow-up patches to include the updated ACPI table, IRQ handling, and platform driver registration etc.
I would also like to point a few issues here, this would also act as additional justification to some of the code changes I have introduced, for example, a work_queue() for updating firmware images from the activate_store handler.
On 12/8/25 22:13, Vedashree Vidwans wrote:
> FW image details are querried with a SMC call. Currently, these FW
> details are added as nodes in a linked list. This patch updates the
> FW node creation and update functions. Now the linked list is updated
> based on the current value of num_lfa_components.
> As per spec [1], FW inventory is updated after each activation.
>
> [1] https://developer.arm.com/documentation/den0147/latest/
>
> Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
> ---
> drivers/firmware/smccc/lfa_fw.c | 148 +++++++++++++++++++++-----------
> 1 file changed, 100 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index 24916fc53420..334090708405 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -133,6 +133,7 @@ static const struct fw_image_uuid {
> },
> };
>
> +static int update_fw_images_tree(void);
> static struct kobject *lfa_dir;
> static DEFINE_MUTEX(lfa_lock);
>
> @@ -282,17 +283,6 @@ static int activate_fw_image(struct image_props *attrs)
> return ret;
> }
>
> -static void set_image_flags(struct image_props *attrs, int seq_id,
> - u32 image_flags)
> -{
> - attrs->fw_seq_id = seq_id;
> - 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 ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
> char *buf)
> {
> @@ -395,7 +385,9 @@ static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
>
> pr_info("Firmware activation succeeded\n");
>
> - /* TODO: refresh image flags here*/
> + ret = update_fw_images_tree();
Part of the firmware images update process, there is a potential of a firmware component and its attributes being deleted following activation. The to-be-removed component could very well be the one where the activate call has been triggered from. This results in a deadlock as removing a sysfs attribute from within its _store handler results in kernfs to wait for the in-flight store() operation to finish before it can remove itself.
To cater for this scenario in my patch I am differing the firmware images update process, triggered from the _store handler, to a workqueue().
> + if (ret)
> + pr_err("Failed to update FW images tree");
> mutex_unlock(&lfa_lock);
> return count;
> }
> @@ -425,20 +417,41 @@ static struct kobj_attribute image_attrs_group[LFA_ATTR_NR_IMAGES] = {
> [LFA_ATTR_CANCEL] = __ATTR_WO(cancel)
> };
>
> +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);
> + attrs->image_dir = NULL;
> + list_del(&attrs->image_node);
> + kfree(attrs);
> +}
> +
> static void clean_fw_images_tree(void)
> {
> struct image_props *attrs, *tmp;
>
> - list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node) {
> - kobject_put(attrs->image_dir);
> - list_del(&attrs->image_node);
> - kfree(attrs);
> - }
> + list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node)
> + delete_fw_image_node(attrs);
> +}
> +
> +static void update_fw_image_node(struct image_props *attrs, int seq_id,
> + char *fw_uuid, u32 image_flags)
> +{
> + attrs->fw_seq_id = seq_id;
> + attrs->image_name = fw_uuid;
We introduced a C struct "fw_image_uuid" to provide names for each firmware component in the LFA driver rather than using the UUID for a name which is non-intuitive. If UUIDs are desired for comparison then "attrs->image_dir->name" could be used.
> + 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 int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
> +static int create_fw_image_node(int seq_id, char *fw_uuid, u32 image_flags)
> {
> - const char *image_name = "(unknown)";
> struct image_props *attrs;
> int ret;
>
> @@ -446,21 +459,12 @@ static int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
> 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;
> - else
> - image_name = fw_uuid;
> - }
> -
> 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);
> + /* Update FW attributes */
> + update_fw_image_node(attrs, seq_id, fw_uuid, image_flags);
>
> /*
> * The attributes for each sysfs file are constant (handler functions,
> @@ -485,17 +489,19 @@ static int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
> return ret;
> }
> }
> - list_add(&attrs->image_node, &lfa_fw_images);
> + list_add_tail(&attrs->image_node, &lfa_fw_images);
>
> return ret;
> }
>
> -static int create_fw_images_tree(void)
> +static int update_fw_images_tree(void)
> {
> struct arm_smccc_1_2_regs reg = { 0 };
> struct uuid_regs image_uuid;
> + struct image_props *attrs, *tmp;
> char image_id_str[40];
> int ret, num_of_components;
> + int node_idx = 0;
>
> num_of_components = get_nr_lfa_components();
> if (num_of_components <= 0) {
> @@ -503,22 +509,67 @@ static int create_fw_images_tree(void)
> 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) {
> + /*
> + * Pass 1:
> + * For nodes < num_of_components, update fw_image_node
> + * For nodes >= num_of_components, delete
> + */
> + list_for_each_entry_safe(attrs, tmp, &lfa_fw_images, image_node) {
> + if (attrs->fw_seq_id < num_of_components) {
> + /* Update this FW image node */
> +
> + /* Get FW details */
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = attrs->fw_seq_id;
> + arm_smccc_1_2_invoke(®, ®);
> +
> + if (reg.a0 != LFA_SUCCESS)
> + return -EINVAL;
> +
> + /* Build image name with UUID */
> image_uuid.uuid_lo = reg.a1;
> image_uuid.uuid_hi = reg.a2;
> + snprintf(image_id_str, sizeof(image_id_str), "%pUb", &image_uuid);
>
> - snprintf(image_id_str, sizeof(image_id_str), "%pUb",
> - &image_uuid);
> - ret = create_fw_inventory(image_id_str, i, reg.a3);
> - if (ret)
> - return ret;
> + if (strcmp(attrs->image_name, image_id_str)) {
attrs->image_name is supposed to be the name of the firmware image e.g. "TF-RMM" and not the UUID. If we need to compare the UUIDs I think we should use the specified firmware's directory name instead.
> + /* UUID doesn't match previous UUID for given FW, not expected */
This is an expected behavior per the LFA spec. For example, firmware seq_ids get scrambled following an activation, the firmware image with seq_id 0 before activation has now a seq_id 1 following the activation in the LFA agent. When you attempt to request information for the firmware image using its old seq_id stored in the driver, the UUIDs would not match and thus the driver would bail out. We can't bail out for an expected behavior.
Just to confirm, my recent submission includes work on update of firmware images list following an activation.
Many thanks,
Salman
> + pr_err("FW seq id %u: Previous UUID %s doesn't match current %s",
> + attrs->fw_seq_id, attrs->image_name, image_id_str);
> + return -EINVAL;
> + }
> +
> + /* Update FW attributes */
> + update_fw_image_node(attrs, attrs->fw_seq_id, image_id_str, reg.a3);
> + node_idx++;
> + } else {
> + /* This node is beyond valid FW components list */
> + delete_fw_image_node(attrs);
> }
> }
>
> + /*
> + * Pass 2:
> + * If current FW components number is more than previous list, add new component nodes.
> + */
> + for (node_idx; node_idx < num_of_components; node_idx++) {
> + /* Get FW details */
> + reg.a0 = LFA_1_0_FN_GET_INVENTORY;
> + reg.a1 = node_idx;
> + arm_smccc_1_2_invoke(®, ®);
> +
> + if (reg.a0 != LFA_SUCCESS)
> + return -EINVAL;
> +
> + /* Build image name with UUID */
> + image_uuid.uuid_lo = reg.a1;
> + image_uuid.uuid_hi = reg.a2;
> + snprintf(image_id_str, sizeof(image_id_str), "%pUb", &image_uuid);
> +
> + ret = create_fw_image_node(node_idx, image_id_str, reg.a3);
> + if (ret)
> + return ret;
> + }
> +
> return 0;
> }
>
> @@ -538,8 +589,9 @@ static irqreturn_t lfa_irq_thread(int irq, void *data)
> */
>
> do {
> - /* TODO: refresh image flags here */
> - /* If refresh fails goto exit_unlock */
> + ret = update_fw_images_tree();
> + if (ret)
> + goto exit_unlock;
>
> /* Initialize counters to track list traversal */
> num_of_components = get_nr_lfa_components();
> @@ -561,13 +613,13 @@ static irqreturn_t lfa_irq_thread(int irq, void *data)
> }
>
> pr_info("Firmware %s activation succeeded", attrs->image_name);
> - /* Refresh FW component details */
> break;
> }
> } while (curr_component < num_of_components);
>
> - /* TODO: refresh image flags here */
> - /* If refresh fails goto exit_unlock */
> + ret = update_fw_images_tree();
> + if (ret)
> + goto exit_unlock;
>
> exit_unlock:
> mutex_unlock(&lfa_lock);
> @@ -657,7 +709,7 @@ static int __init lfa_init(void)
> return -ENOMEM;
>
> mutex_lock(&lfa_lock);
> - err = create_fw_images_tree();
> + err = update_fw_images_tree();
> if (err != 0)
> kobject_put(lfa_dir);
>
^ permalink raw reply
* [PATCH] KVM: SEV: Enable SNP AP CPU hotplug
From: Jethro Beekman @ 2026-01-19 19:12 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, kvm,
linux-kernel, linux-coco
[-- Attachment #1: Type: text/plain, Size: 3522 bytes --]
The GHCB protocol states that after AP CREATE (as opposed to CREATE_ON_INIT),
the hypervisor must immediately proceed to VMRUN. Update vCPU state on AP
CREATE so this happens.
vCPUs created after SNP_LAUNCH_FINISH don't go through snp_launch_update_vmsa.
Ensure the vCPU state is updated properly during VMCB initialization.
Signed-off-by: Jethro Beekman <jethro@fortanix.com>
---
arch/x86/kvm/svm/sev.c | 43 ++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index cdaca10b8773..9af1bd5b2071 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -960,6 +960,19 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
return 0;
}
+static void sev_es_finalize_vcpu(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.guest_state_protected = true;
+ /*
+ * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
+ * be _always_ ON. Enable it only after setting
+ * guest_state_protected because KVM_SET_MSRS allows dynamic
+ * toggling of LBRV (for performance reason) on write access to
+ * MSR_IA32_DEBUGCTLMSR when guest_state_protected is not set.
+ */
+ svm_enable_lbrv(vcpu);
+}
+
static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
int *error)
{
@@ -999,15 +1012,9 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
* do xsave/xrstor on it.
*/
fpstate_set_confidential(&vcpu->arch.guest_fpu);
- vcpu->arch.guest_state_protected = true;
- /*
- * SEV-ES guest mandates LBR Virtualization to be _always_ ON. Enable it
- * only after setting guest_state_protected because KVM_SET_MSRS allows
- * dynamic toggling of LBRV (for performance reason) on write access to
- * MSR_IA32_DEBUGCTLMSR when guest_state_protected is not set.
- */
- svm_enable_lbrv(vcpu);
+ sev_es_finalize_vcpu(vcpu);
+
return 0;
}
@@ -2480,15 +2487,7 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
- svm->vcpu.arch.guest_state_protected = true;
- /*
- * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
- * be _always_ ON. Enable it only after setting
- * guest_state_protected because KVM_SET_MSRS allows dynamic
- * toggling of LBRV (for performance reason) on write access to
- * MSR_IA32_DEBUGCTLMSR when guest_state_protected is not set.
- */
- svm_enable_lbrv(vcpu);
+ sev_es_finalize_vcpu(vcpu);
}
return 0;
@@ -4030,6 +4029,10 @@ static void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
/* Use the new VMSA */
svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
+ /* vCPU was added after SNP_LAUNCH_FINISH */
+ if (!vcpu->arch.guest_state_protected)
+ sev_es_finalize_vcpu(vcpu);
+
/* Mark the vCPU as runnable */
kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
@@ -4111,8 +4114,12 @@ static int sev_snp_ap_creation(struct vcpu_svm *svm)
* Unless Creation is deferred until INIT, signal the vCPU to update
* its state.
*/
- if (request != SVM_VMGEXIT_AP_CREATE_ON_INIT)
+ if (request != SVM_VMGEXIT_AP_CREATE_ON_INIT) {
+ if (target_vcpu->arch.mp_state == KVM_MP_STATE_UNINITIALIZED ||
+ target_vcpu->arch.mp_state == KVM_MP_STATE_INIT_RECEIVED)
+ kvm_set_mp_state(target_vcpu, KVM_MP_STATE_RUNNABLE);
kvm_make_request_and_kick(KVM_REQ_UPDATE_PROTECTED_GUEST_STATE, target_vcpu);
+ }
return 0;
}
--
2.43.0
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4839 bytes --]
^ permalink raw reply related
* Re: [PATCH] KVM: SEV: Track SNP launch state and disallow invalid userspace interactions
From: Jethro Beekman @ 2026-01-19 19:12 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, kvm,
linux-kernel, linux-coco
In-Reply-To: <d98692e2-d96b-4c36-8089-4bc1e5cc3d57@fortanix.com>
[-- Attachment #1: Type: text/plain, Size: 5213 bytes --]
On 2026-01-19 20:06, Jethro Beekman wrote:
> Calling any of the SNP_LAUNCH_ ioctls after SNP_LAUNCH_FINISH results in a
> kernel page fault due to RMP violation. Track SNP launch state and exit early.
>
> vCPUs created after SNP_LAUNCH_FINISH won't have a guest VMSA automatically
> created during SNP_LAUNCH_FINISH by converting the kernel-allocated VMSA. Don't
> allocate a VMSA page, so that the vCPU is in a state similar to what it would
> be after SNP AP destroy. This ensures pre_sev_run() prevents the vCPU from
> running even if userspace makes the vCPU runnable.
>
> Signed-off-by: Jethro Beekman <jethro@fortanix.com>
> ---
> arch/x86/kvm/svm/sev.c | 43 ++++++++++++++++++++++++++----------------
> arch/x86/kvm/svm/svm.h | 1 +
> 2 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index f59c65abe3cf..cdaca10b8773 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2205,6 +2205,9 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> if (!sev_snp_guest(kvm))
> return -ENOTTY;
>
> + if (sev->snp_finished)
> + return -EINVAL;
> +
> if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
> return -EFAULT;
>
> @@ -2369,7 +2372,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
> void __user *src;
> int ret = 0;
>
> - if (!sev_snp_guest(kvm) || !sev->snp_context)
> + if (!sev_snp_guest(kvm) || !sev->snp_context || sev->snp_finished)
> return -EINVAL;
>
> if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
> @@ -2502,7 +2505,7 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
> if (!sev_snp_guest(kvm))
> return -ENOTTY;
>
> - if (!sev->snp_context)
> + if (!sev->snp_context || sev->snp_finished)
> return -EINVAL;
>
> if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
> @@ -2548,13 +2551,15 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
> data->gctx_paddr = __psp_pa(sev->snp_context);
> ret = sev_issue_cmd(kvm, SEV_CMD_SNP_LAUNCH_FINISH, data, &argp->error);
>
> - /*
> - * Now that there will be no more SNP_LAUNCH_UPDATE ioctls, private pages
> - * can be given to the guest simply by marking the RMP entry as private.
> - * This can happen on first access and also with KVM_PRE_FAULT_MEMORY.
> - */
> - if (!ret)
> + if (!ret) {
> + sev->snp_finished = true;
> + /*
> + * Now that there will be no more SNP_LAUNCH_UPDATE ioctls, private pages
> + * can be given to the guest simply by marking the RMP entry as private.
> + * This can happen on first access and also with KVM_PRE_FAULT_MEMORY.
> + */
> kvm->arch.pre_fault_allowed = true;
> + }
>
> kfree(id_auth);
>
> @@ -3253,6 +3258,9 @@ void sev_free_vcpu(struct kvm_vcpu *vcpu)
>
> svm = to_svm(vcpu);
>
> + if (!svm->sev_es.vmsa)
> + goto skip_vmsa_free;
> +
> /*
> * If it's an SNP guest, then the VMSA was marked in the RMP table as
> * a guest-owned page. Transition the page to hypervisor state before
> @@ -4653,6 +4661,7 @@ void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
>
> int sev_vcpu_create(struct kvm_vcpu *vcpu)
> {
> + struct kvm_sev_info *sev = to_kvm_sev_info(vcpu->kvm);
> struct vcpu_svm *svm = to_svm(vcpu);
> struct page *vmsa_page;
>
> @@ -4661,15 +4670,17 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
> if (!sev_es_guest(vcpu->kvm))
> return 0;
>
> - /*
> - * SEV-ES guests require a separate (from the VMCB) VMSA page used to
> - * contain the encrypted register state of the guest.
> - */
> - vmsa_page = snp_safe_alloc_page();
> - if (!vmsa_page)
> - return -ENOMEM;
> + if (!sev->snp_finished) {
> + /*
> + * SEV-ES guests require a separate (from the VMCB) VMSA page used to
> + * contain the encrypted register state of the guest.
> + */
> + vmsa_page = snp_safe_alloc_page();
> + if (!vmsa_page)
> + return -ENOMEM;
>
> - svm->sev_es.vmsa = page_address(vmsa_page);
> + svm->sev_es.vmsa = page_address(vmsa_page);
> + }
I think there may be a race between this creation of a vCPU and the kvm_for_each_vcpu() loop in snp_launch_update_vmsa(). What should happen is that every vCPU that wasn't considered in snp_launch_update_vmsa() must not have a VMSA allocated here. If there is a race, I'm not sure what the best way is to prevent it.
>
> vcpu->arch.guest_tsc_protected = snp_is_secure_tsc_enabled(vcpu->kvm);
>
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 01be93a53d07..59c328c13b2a 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -96,6 +96,7 @@ struct kvm_sev_info {
> bool active; /* SEV enabled guest */
> bool es_active; /* SEV-ES enabled guest */
> bool need_init; /* waiting for SEV_INIT2 */
> + bool snp_finished; /* SNP guest measurement has been finalized */
> unsigned int asid; /* ASID used for this guest */
> unsigned int handle; /* SEV firmware handle */
> int fd; /* SEV device fd */
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4839 bytes --]
^ permalink raw reply
* [PATCH] KVM: SEV: Track SNP launch state and disallow invalid userspace interactions
From: Jethro Beekman @ 2026-01-19 19:06 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, kvm,
linux-kernel, linux-coco
[-- Attachment #1: Type: text/plain, Size: 4610 bytes --]
Calling any of the SNP_LAUNCH_ ioctls after SNP_LAUNCH_FINISH results in a
kernel page fault due to RMP violation. Track SNP launch state and exit early.
vCPUs created after SNP_LAUNCH_FINISH won't have a guest VMSA automatically
created during SNP_LAUNCH_FINISH by converting the kernel-allocated VMSA. Don't
allocate a VMSA page, so that the vCPU is in a state similar to what it would
be after SNP AP destroy. This ensures pre_sev_run() prevents the vCPU from
running even if userspace makes the vCPU runnable.
Signed-off-by: Jethro Beekman <jethro@fortanix.com>
---
arch/x86/kvm/svm/sev.c | 43 ++++++++++++++++++++++++++----------------
arch/x86/kvm/svm/svm.h | 1 +
2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f59c65abe3cf..cdaca10b8773 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2205,6 +2205,9 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
if (!sev_snp_guest(kvm))
return -ENOTTY;
+ if (sev->snp_finished)
+ return -EINVAL;
+
if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
return -EFAULT;
@@ -2369,7 +2372,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
void __user *src;
int ret = 0;
- if (!sev_snp_guest(kvm) || !sev->snp_context)
+ if (!sev_snp_guest(kvm) || !sev->snp_context || sev->snp_finished)
return -EINVAL;
if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
@@ -2502,7 +2505,7 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
if (!sev_snp_guest(kvm))
return -ENOTTY;
- if (!sev->snp_context)
+ if (!sev->snp_context || sev->snp_finished)
return -EINVAL;
if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
@@ -2548,13 +2551,15 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
data->gctx_paddr = __psp_pa(sev->snp_context);
ret = sev_issue_cmd(kvm, SEV_CMD_SNP_LAUNCH_FINISH, data, &argp->error);
- /*
- * Now that there will be no more SNP_LAUNCH_UPDATE ioctls, private pages
- * can be given to the guest simply by marking the RMP entry as private.
- * This can happen on first access and also with KVM_PRE_FAULT_MEMORY.
- */
- if (!ret)
+ if (!ret) {
+ sev->snp_finished = true;
+ /*
+ * Now that there will be no more SNP_LAUNCH_UPDATE ioctls, private pages
+ * can be given to the guest simply by marking the RMP entry as private.
+ * This can happen on first access and also with KVM_PRE_FAULT_MEMORY.
+ */
kvm->arch.pre_fault_allowed = true;
+ }
kfree(id_auth);
@@ -3253,6 +3258,9 @@ void sev_free_vcpu(struct kvm_vcpu *vcpu)
svm = to_svm(vcpu);
+ if (!svm->sev_es.vmsa)
+ goto skip_vmsa_free;
+
/*
* If it's an SNP guest, then the VMSA was marked in the RMP table as
* a guest-owned page. Transition the page to hypervisor state before
@@ -4653,6 +4661,7 @@ void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
int sev_vcpu_create(struct kvm_vcpu *vcpu)
{
+ struct kvm_sev_info *sev = to_kvm_sev_info(vcpu->kvm);
struct vcpu_svm *svm = to_svm(vcpu);
struct page *vmsa_page;
@@ -4661,15 +4670,17 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
if (!sev_es_guest(vcpu->kvm))
return 0;
- /*
- * SEV-ES guests require a separate (from the VMCB) VMSA page used to
- * contain the encrypted register state of the guest.
- */
- vmsa_page = snp_safe_alloc_page();
- if (!vmsa_page)
- return -ENOMEM;
+ if (!sev->snp_finished) {
+ /*
+ * SEV-ES guests require a separate (from the VMCB) VMSA page used to
+ * contain the encrypted register state of the guest.
+ */
+ vmsa_page = snp_safe_alloc_page();
+ if (!vmsa_page)
+ return -ENOMEM;
- svm->sev_es.vmsa = page_address(vmsa_page);
+ svm->sev_es.vmsa = page_address(vmsa_page);
+ }
vcpu->arch.guest_tsc_protected = snp_is_secure_tsc_enabled(vcpu->kvm);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 01be93a53d07..59c328c13b2a 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -96,6 +96,7 @@ struct kvm_sev_info {
bool active; /* SEV enabled guest */
bool es_active; /* SEV-ES enabled guest */
bool need_init; /* waiting for SEV_INIT2 */
+ bool snp_finished; /* SNP guest measurement has been finalized */
unsigned int asid; /* ASID used for this guest */
unsigned int handle; /* SEV firmware handle */
int fd; /* SEV device fd */
--
2.43.0
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4839 bytes --]
^ permalink raw reply related
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: kernel test robot @ 2026-01-19 18:41 UTC (permalink / raw)
To: Salman Nabi, vvidwans, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: oe-kbuild-all, ardb, chao.gao, linux-arm-kernel, linux-coco,
linux-kernel, sdonthineni, vsethi, vwadekar
In-Reply-To: <20260119122729.287522-2-salman.nabi@arm.com>
Hi Salman,
kernel test robot noticed the following build errors:
[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.19-rc6 next-20260116]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Salman-Nabi/firmware-smccc-add-support-for-Live-Firmware-Activation-LFA/20260119-203221
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20260119122729.287522-2-salman.nabi%40arm.com
patch subject: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
config: arm-sp7021_defconfig (https://download.01.org/0day-ci/archive/20260120/202601200225.cTYuYwRI-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601200225.cTYuYwRI-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601200225.cTYuYwRI-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/firmware/smccc/lfa_fw.c: In function 'get_nr_lfa_components':
>> drivers/firmware/smccc/lfa_fw.c:179:16: error: variable 'reg' has initializer but incomplete type
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:179:43: warning: excess elements in struct initializer
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:179:43: note: (near initialization for 'reg')
>> drivers/firmware/smccc/lfa_fw.c:179:35: error: storage size of 'reg' isn't known
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
>> drivers/firmware/smccc/lfa_fw.c:184:9: error: implicit declaration of function 'arm_smccc_1_2_invoke'; did you mean 'arm_smccc_1_1_invoke'? [-Wimplicit-function-declaration]
184 | arm_smccc_1_2_invoke(®, ®);
| ^~~~~~~~~~~~~~~~~~~~
| arm_smccc_1_1_invoke
drivers/firmware/smccc/lfa_fw.c:179:35: warning: unused variable 'reg' [-Wunused-variable]
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c: In function 'lfa_cancel':
drivers/firmware/smccc/lfa_fw.c:194:16: error: variable 'reg' has initializer but incomplete type
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:194:43: warning: excess elements in struct initializer
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:194:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:194:35: error: storage size of 'reg' isn't known
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:194:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'call_lfa_activate':
drivers/firmware/smccc/lfa_fw.c:220:16: error: variable 'reg' has initializer but incomplete type
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:220:43: warning: excess elements in struct initializer
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:220:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:220:35: error: storage size of 'reg' isn't known
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:220:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'prime_fw_image':
drivers/firmware/smccc/lfa_fw.c:290:16: error: variable 'reg' has initializer but incomplete type
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:290:43: warning: excess elements in struct initializer
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:290:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:290:35: error: storage size of 'reg' isn't known
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:290:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'activation_pending_show':
drivers/firmware/smccc/lfa_fw.c:360:16: error: variable 'reg' has initializer but incomplete type
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:360:43: warning: excess elements in struct initializer
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:360:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:360:35: error: storage size of 'reg' isn't known
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:360:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'pending_version_show':
drivers/firmware/smccc/lfa_fw.c:434:16: error: variable 'reg' has initializer but incomplete type
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:434:43: warning: excess elements in struct initializer
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:434:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:434:35: error: storage size of 'reg' isn't known
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:434:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'update_fw_images_tree':
drivers/firmware/smccc/lfa_fw.c:586:16: error: variable 'reg' has initializer but incomplete type
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:586:43: warning: excess elements in struct initializer
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:586:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:586:35: error: storage size of 'reg' isn't known
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:586:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'lfa_init':
drivers/firmware/smccc/lfa_fw.c:619:16: error: variable 'reg' has initializer but incomplete type
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:619:43: warning: excess elements in struct initializer
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:619:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:619:35: error: storage size of 'reg' isn't known
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:619:35: warning: unused variable 'reg' [-Wunused-variable]
vim +/reg +179 drivers/firmware/smccc/lfa_fw.c
176
177 static unsigned long get_nr_lfa_components(void)
178 {
> 179 struct arm_smccc_1_2_regs reg = { 0 };
180
181 reg.a0 = LFA_1_0_FN_GET_INFO;
182 reg.a1 = 0; /* lfa_info_selector = 0 */
183
> 184 arm_smccc_1_2_invoke(®, ®);
185 if (reg.a0 != LFA_SUCCESS)
186 return reg.a0;
187
188 return reg.a1;
189 }
190
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Andre Przywara @ 2026-01-19 16:41 UTC (permalink / raw)
To: kernel test robot, Salman Nabi, vvidwans, sudeep.holla,
mark.rutland, lpieralisi
Cc: oe-kbuild-all, ardb, chao.gao, linux-arm-kernel, linux-coco,
linux-kernel, sdonthineni, vsethi, vwadekar
In-Reply-To: <202601200007.GVZIjoCx-lkp@intel.com>
Hi,
On 19/01/2026 16:29, kernel test robot wrote:
> Hi Salman,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on soc/for-next]
> [also build test WARNING on linus/master v6.19-rc6 next-20260116]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Salman-Nabi/firmware-smccc-add-support-for-Live-Firmware-Activation-LFA/20260119-203221
> base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
> patch link: https://lore.kernel.org/r/20260119122729.287522-2-salman.nabi%40arm.com
> patch subject: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
> config: arm-sp7021_defconfig (https://download.01.org/0day-ci/archive/20260120/202601200007.GVZIjoCx-lkp@intel.com/config)
> compiler: arm-linux-gnueabi-gcc (GCC) 15.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601200007.GVZIjoCx-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202601200007.GVZIjoCx-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> drivers/firmware/smccc/lfa_fw.c: In function 'get_nr_lfa_components':
> drivers/firmware/smccc/lfa_fw.c:179:16: error: variable 'reg' has initializer but incomplete type
> 179 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
>>> drivers/firmware/smccc/lfa_fw.c:179:43: warning: excess elements in struct initializer
Ah, yes, arm_smccc_1_2_regs is only defined for arm64. We rely on v1.2,
and the LFA spec actually means that this means AArch64 only right at
the beginning (chapter 2).
So we need an additional dependency on ARM64 in the Kconfig entry.
Cheers,
Andre
> 179 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:179:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:179:35: error: storage size of 'reg' isn't known
> 179 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:184:9: error: implicit declaration of function 'arm_smccc_1_2_invoke'; did you mean 'arm_smccc_1_1_invoke'? [-Wimplicit-function-declaration]
> 184 | arm_smccc_1_2_invoke(®, ®);
> | ^~~~~~~~~~~~~~~~~~~~
> | arm_smccc_1_1_invoke
>>> drivers/firmware/smccc/lfa_fw.c:179:35: warning: unused variable 'reg' [-Wunused-variable]
> 179 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c: In function 'lfa_cancel':
> drivers/firmware/smccc/lfa_fw.c:194:16: error: variable 'reg' has initializer but incomplete type
> 194 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
> drivers/firmware/smccc/lfa_fw.c:194:43: warning: excess elements in struct initializer
> 194 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:194:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:194:35: error: storage size of 'reg' isn't known
> 194 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:194:35: warning: unused variable 'reg' [-Wunused-variable]
> drivers/firmware/smccc/lfa_fw.c: In function 'call_lfa_activate':
> drivers/firmware/smccc/lfa_fw.c:220:16: error: variable 'reg' has initializer but incomplete type
> 220 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
> drivers/firmware/smccc/lfa_fw.c:220:43: warning: excess elements in struct initializer
> 220 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:220:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:220:35: error: storage size of 'reg' isn't known
> 220 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:220:35: warning: unused variable 'reg' [-Wunused-variable]
> drivers/firmware/smccc/lfa_fw.c: In function 'prime_fw_image':
> drivers/firmware/smccc/lfa_fw.c:290:16: error: variable 'reg' has initializer but incomplete type
> 290 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
> drivers/firmware/smccc/lfa_fw.c:290:43: warning: excess elements in struct initializer
> 290 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:290:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:290:35: error: storage size of 'reg' isn't known
> 290 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:290:35: warning: unused variable 'reg' [-Wunused-variable]
> drivers/firmware/smccc/lfa_fw.c: In function 'activation_pending_show':
> drivers/firmware/smccc/lfa_fw.c:360:16: error: variable 'reg' has initializer but incomplete type
> 360 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
> drivers/firmware/smccc/lfa_fw.c:360:43: warning: excess elements in struct initializer
> 360 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:360:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:360:35: error: storage size of 'reg' isn't known
> 360 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:360:35: warning: unused variable 'reg' [-Wunused-variable]
> drivers/firmware/smccc/lfa_fw.c: In function 'pending_version_show':
> drivers/firmware/smccc/lfa_fw.c:434:16: error: variable 'reg' has initializer but incomplete type
> 434 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
> drivers/firmware/smccc/lfa_fw.c:434:43: warning: excess elements in struct initializer
> 434 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:434:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:434:35: error: storage size of 'reg' isn't known
> 434 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:434:35: warning: unused variable 'reg' [-Wunused-variable]
> drivers/firmware/smccc/lfa_fw.c: In function 'update_fw_images_tree':
> drivers/firmware/smccc/lfa_fw.c:586:16: error: variable 'reg' has initializer but incomplete type
> 586 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
> drivers/firmware/smccc/lfa_fw.c:586:43: warning: excess elements in struct initializer
> 586 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:586:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:586:35: error: storage size of 'reg' isn't known
> 586 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:586:35: warning: unused variable 'reg' [-Wunused-variable]
> drivers/firmware/smccc/lfa_fw.c: In function 'lfa_init':
> drivers/firmware/smccc/lfa_fw.c:619:16: error: variable 'reg' has initializer but incomplete type
> 619 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~~~~~~~~~~~~~~~~
> drivers/firmware/smccc/lfa_fw.c:619:43: warning: excess elements in struct initializer
> 619 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^
> drivers/firmware/smccc/lfa_fw.c:619:43: note: (near initialization for 'reg')
> drivers/firmware/smccc/lfa_fw.c:619:35: error: storage size of 'reg' isn't known
> 619 | struct arm_smccc_1_2_regs reg = { 0 };
> | ^~~
> drivers/firmware/smccc/lfa_fw.c:619:35: warning: unused variable 'reg' [-Wunused-variable]
>
>
> vim +179 drivers/firmware/smccc/lfa_fw.c
>
> 176
> 177 static unsigned long get_nr_lfa_components(void)
> 178 {
> > 179 struct arm_smccc_1_2_regs reg = { 0 };
> 180
> 181 reg.a0 = LFA_1_0_FN_GET_INFO;
> 182 reg.a1 = 0; /* lfa_info_selector = 0 */
> 183
> 184 arm_smccc_1_2_invoke(®, ®);
> 185 if (reg.a0 != LFA_SUCCESS)
> 186 return reg.a0;
> 187
> 188 return reg.a1;
> 189 }
> 190
>
^ permalink raw reply
* Re: [PATCH] dma-direct: swiotlb: Skip encryption toggles for swiotlb allocations
From: Robin Murphy @ 2026-01-19 16:37 UTC (permalink / raw)
To: Aneesh Kumar K.V, Marek Szyprowski, iommu, linux-kernel,
linux-coco
Cc: steven.price, Suzuki K Poulose, Claire Chang
In-Reply-To: <yq5a8qdtlht8.fsf@kernel.org>
On 19/01/2026 3:53 pm, Aneesh Kumar K.V wrote:
> Robin Murphy <robin.murphy@arm.com> writes:
>
>> On 19/01/2026 9:52 am, Marek Szyprowski wrote:
>>> On 14.01.2026 10:49, Aneesh Kumar K.V wrote:
>>>> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>> On 2026-01-09 2:51 am, Aneesh Kumar K.V wrote:
>>>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>>>> On 2026-01-02 3:54 pm, Aneesh Kumar K.V (Arm) wrote:
>>>>>>>>> Swiotlb backing pages are already mapped decrypted via
>>>>>>>>> swiotlb_update_mem_attributes(), so dma-direct does not need to call
>>>>>>>>> set_memory_decrypted() during allocation or re-encrypt the memory on
>>>>>>>>> free.
>>>>>>>>>
>>>>>>>>> Handle swiotlb-backed buffers explicitly: obtain the DMA address and
>>>>>>>>> zero the linear mapping for lowmem pages, and bypass the decrypt/encrypt
>>>>>>>>> transitions when allocating/freeing from the swiotlb pool (detected via
>>>>>>>>> swiotlb_find_pool()).
>>>>>>>> swiotlb_update_mem_attributes() only applies to the default SWIOTLB
>>>>>>>> buffer, while the dma_direct_alloc_swiotlb() path is only for private
>>>>>>>> restricted pools (because the whole point is that restricted DMA devices
>>>>>>>> cannot use the regular allocator/default pools). There is no redundancy
>>>>>>>> here AFAICS.
>>>>>>>>
>>>>>>> But rmem_swiotlb_device_init() is also marking the entire pool decrypted
>>>>>>>
>>>>>>> set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>>>>>> rmem->size >> PAGE_SHIFT);
>>>>>> OK, so why doesn't the commit message mention that instead of saying
>>>>>> something which fails to justify the patch at all? ;)
>>>>>>
>>>>>> Furthermore, how much does this actually matter? The "real" restricted
>>>>>> DMA use-case is on systems where dma_set_decrypted() is a no-op anyway.
>>>>>> I know we used restricted DMA as a hack in the early days of CCA
>>>>>> prototyping, but is it intended to actually deploy that as a supported
>>>>>> and recommended mechanism now?
>>>>>>
>>>>>> Note also that the swiotlb_alloc path is essentially an emergency
>>>>>> fallback, which doesn't work for all situations anyway - any restricted
>>>>>> device that actually needs to make significant coherent allocations (or
>>>>>> rather, that firmware cannot assume won't want to do so) should really
>>>>>> have a proper coherent pool alongside its restricted one. The expected
>>>>>> use-case here is for something like a wifi driver that only needs to
>>>>>> allocate one or two small coherent buffers once at startup, then do
>>>>>> everything else with streaming DMA.
>>>>> I was aiming to bring more consistency in how swiotlb buffers are
>>>>> handled, specifically by treating all swiotlb memory as decrypted
>>>>> buffers, which is also how the current code behaves.
>>>>>
>>>>> If we are concluding that restricted DMA is not used in conjunction with
>>>>> memory encryption, then we could, in fact, remove the
>>>>> set_memory_decrypted() call from rmem_swiotlb_device_init() and
>>>>> instead add failure conditions for force_dma_unencrypted(dev) in
>>>>> is_swiotlb_for_alloc(). However, it’s worth noting that the initial
>>>>> commit did take the memory encryption feature into account
>>>>> (0b84e4f8b793eb4045fd64f6f514165a7974cd16).
>>>>>
>>>>> Please let me know if you think this needs to be fixed.
>>>> Something like.
>>>>
>>>> dma-direct: restricted-dma: Do not mark the restricted DMA pool unencrypted
>>>>
>>>> As per commit f4111e39a52a ("swiotlb: Add restricted DMA alloc/free
>>>> support"), the restricted-dma-pool is used in conjunction with the
>>>> shared-dma-pool. Since allocations from the shared-dma-pool are not
>>>> marked unencrypted, skip marking the restricted-dma-pool as unencrypted
>>>> as well. We do not expect systems using the restricted-dma-pool to have
>>>> memory encryption or to run with confidential computing features enabled.
>>>>
>>>> If a device requires unencrypted access (force_dma_unencrypted(dev)),
>>>> the dma-direct allocator will mark the restricted-dma-pool allocation as
>>>> unencrypted.
>>>>
>>>> The only disadvantage is that, when running on a CC guest with a
>>>> different hypervisor page size, restricted-dma-pool allocation sizes
>>>> must now be aligned to the hypervisor page size. This alignment would
>>>> not be required if the entire pool were marked unencrypted. However, the
>>>> new code enables the use of the restricted-dma-pool for trusted devices.
>>>> Previously, because the entire pool was marked unencrypted, trusted
>>>> devices were unable to allocate from it.
>>>>
>>>> There is still an open question regarding allocations from the
>>>> shared-dma-pool. Currently, they are not marked unencrypted.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>>>
>>>> 1 file changed, 2 deletions(-)
>>>> kernel/dma/swiotlb.c | 2 --
>>>>
>>>> modified kernel/dma/swiotlb.c
>>>> @@ -1835,8 +1835,6 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
>>>> return -ENOMEM;
>>>> }
>>>>
>>>> - set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>>> - rmem->size >> PAGE_SHIFT);
>>>> swiotlb_init_io_tlb_pool(pool, rmem->base, nslabs,
>>>> false, nareas);
>>>> mem->force_bounce = true;
>>>
>>> Robin, could You review this? Is it ready for applying?
>>
>> But wouldn't this break the actual intended use of restricted pools for
>> streaming DMA bouncing, which does depend on the buffer being
>> pre-decrypted/shared? (Since streaming DMA mappings definitely need to
>> be supported in nowait contexts)
>>
>
> Only if we are using a restricted pool with encrypted memory.
>
> If we assume that swiotlb bounce buffers are always decrypted, then
> allocations from that pool can safely skip the decrypt/encrypt
> transitions. However, we still need to address coherent allocations via
> the shared-dma-pool, which are explicitly marked as unencrypted.
>
> Given this, I’m wondering whether the best approach is to revisit the
> original patch I posted, which moved swiotlb allocations out of
> __dma_direct_alloc_pages(). With that separation in place, we could then
> fix up dma_alloc_from_dev_coherent() accordingly.
>
> If the conclusion is that systems with encrypted memory will, in
> practice, never use restricted-dma-pool or shared-dma-pool, then we can
> take this patch?
But if the conclusion is that it doesn't matter then that can only mean
we don't need this patch either.
We've identified that the combination of restricted DMA and a
"meaningful" memory encryption API is theoretically slightly broken and
can't ever have worked properly, so how do we benefit from churning it
to just be theoretically more broken in a different way? That makes even
less sense than invasive churn to "fix" the theoretical problem that
hasn't been an issue in practice.
> If you can suggest the approach you would like to see taken with
> restricted-dma-pool/shared-dma-pool, I can work on the final change.
TBH my first choice is "do nothing"; second would be something like the
below, then wait and see if any future CoCo development does justify
changing our expectations.
Thanks,
Robin.
----->8-----
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index a547c7693135..3786a81eac40 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -1784,6 +1784,10 @@ bool swiotlb_free(struct device *dev, struct page *page, size_t size)
swiotlb_release_slots(dev, tlb_addr, pool);
+ /* We really don't expect this combination, and making it work is a pain */
+ dev_WARN_ONCE(dev, cc_platform_has(CC_ATTR_MEM_ENCRYPT)),
+ "Freeing coherent allocation potentially corrupts restricted DMA pool\n");
+
return true;
}
^ permalink raw reply related
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: kernel test robot @ 2026-01-19 16:29 UTC (permalink / raw)
To: Salman Nabi, vvidwans, andre.przywara, sudeep.holla, mark.rutland,
lpieralisi
Cc: oe-kbuild-all, ardb, chao.gao, linux-arm-kernel, linux-coco,
linux-kernel, sdonthineni, vsethi, vwadekar
In-Reply-To: <20260119122729.287522-2-salman.nabi@arm.com>
Hi Salman,
kernel test robot noticed the following build warnings:
[auto build test WARNING on soc/for-next]
[also build test WARNING on linus/master v6.19-rc6 next-20260116]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Salman-Nabi/firmware-smccc-add-support-for-Live-Firmware-Activation-LFA/20260119-203221
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20260119122729.287522-2-salman.nabi%40arm.com
patch subject: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
config: arm-sp7021_defconfig (https://download.01.org/0day-ci/archive/20260120/202601200007.GVZIjoCx-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601200007.GVZIjoCx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601200007.GVZIjoCx-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/firmware/smccc/lfa_fw.c: In function 'get_nr_lfa_components':
drivers/firmware/smccc/lfa_fw.c:179:16: error: variable 'reg' has initializer but incomplete type
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
>> drivers/firmware/smccc/lfa_fw.c:179:43: warning: excess elements in struct initializer
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:179:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:179:35: error: storage size of 'reg' isn't known
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:184:9: error: implicit declaration of function 'arm_smccc_1_2_invoke'; did you mean 'arm_smccc_1_1_invoke'? [-Wimplicit-function-declaration]
184 | arm_smccc_1_2_invoke(®, ®);
| ^~~~~~~~~~~~~~~~~~~~
| arm_smccc_1_1_invoke
>> drivers/firmware/smccc/lfa_fw.c:179:35: warning: unused variable 'reg' [-Wunused-variable]
179 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c: In function 'lfa_cancel':
drivers/firmware/smccc/lfa_fw.c:194:16: error: variable 'reg' has initializer but incomplete type
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:194:43: warning: excess elements in struct initializer
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:194:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:194:35: error: storage size of 'reg' isn't known
194 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:194:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'call_lfa_activate':
drivers/firmware/smccc/lfa_fw.c:220:16: error: variable 'reg' has initializer but incomplete type
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:220:43: warning: excess elements in struct initializer
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:220:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:220:35: error: storage size of 'reg' isn't known
220 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:220:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'prime_fw_image':
drivers/firmware/smccc/lfa_fw.c:290:16: error: variable 'reg' has initializer but incomplete type
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:290:43: warning: excess elements in struct initializer
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:290:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:290:35: error: storage size of 'reg' isn't known
290 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:290:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'activation_pending_show':
drivers/firmware/smccc/lfa_fw.c:360:16: error: variable 'reg' has initializer but incomplete type
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:360:43: warning: excess elements in struct initializer
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:360:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:360:35: error: storage size of 'reg' isn't known
360 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:360:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'pending_version_show':
drivers/firmware/smccc/lfa_fw.c:434:16: error: variable 'reg' has initializer but incomplete type
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:434:43: warning: excess elements in struct initializer
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:434:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:434:35: error: storage size of 'reg' isn't known
434 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:434:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'update_fw_images_tree':
drivers/firmware/smccc/lfa_fw.c:586:16: error: variable 'reg' has initializer but incomplete type
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:586:43: warning: excess elements in struct initializer
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:586:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:586:35: error: storage size of 'reg' isn't known
586 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:586:35: warning: unused variable 'reg' [-Wunused-variable]
drivers/firmware/smccc/lfa_fw.c: In function 'lfa_init':
drivers/firmware/smccc/lfa_fw.c:619:16: error: variable 'reg' has initializer but incomplete type
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~~~~~~~~~~~~~~~~
drivers/firmware/smccc/lfa_fw.c:619:43: warning: excess elements in struct initializer
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^
drivers/firmware/smccc/lfa_fw.c:619:43: note: (near initialization for 'reg')
drivers/firmware/smccc/lfa_fw.c:619:35: error: storage size of 'reg' isn't known
619 | struct arm_smccc_1_2_regs reg = { 0 };
| ^~~
drivers/firmware/smccc/lfa_fw.c:619:35: warning: unused variable 'reg' [-Wunused-variable]
vim +179 drivers/firmware/smccc/lfa_fw.c
176
177 static unsigned long get_nr_lfa_components(void)
178 {
> 179 struct arm_smccc_1_2_regs reg = { 0 };
180
181 reg.a0 = LFA_1_0_FN_GET_INFO;
182 reg.a1 = 0; /* lfa_info_selector = 0 */
183
184 arm_smccc_1_2_invoke(®, ®);
185 if (reg.a0 != LFA_SUCCESS)
186 return reg.a0;
187
188 return reg.a1;
189 }
190
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 16/21] x86/virt/seamldr: Handle TDX Module update failures
From: Dave Hansen @ 2026-01-19 16:24 UTC (permalink / raw)
To: Chao Gao, H. Peter Anvin
Cc: Xu Yilun, linux-coco, linux-kernel, x86, reinette.chatre,
ira.weiny, kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov
In-Reply-To: <aW3Ce05mZKCYYoKo@intel.com>
On 1/18/26 21:34, Chao Gao wrote:
> On Sun, Jan 18, 2026 at 08:55:22PM -0800, H. Peter Anvin wrote:
>> On January 14, 2026 10:24:22 PM PST, Xu Yilun <yilun.xu@linux.intel.com> wrote:
>>> On Tue, Sep 30, 2025 at 07:53:00PM -0700, Chao Gao wrote:
>>>> Failures encountered after a successful module shutdown are unrecoverable,
>>>> e.g., there is no way to restore the old TDX Module.
>>> "e.g." is obscure. To me, the following sentence is explaining the
>>> reason why the failure is not recoverable. Maybe "i.e." or "because"?
>> "e.g." (for example) means the following list is non-exhaustive.
> Yes, 'i.e.' or 'because' would be more appropriate since the next sentence
> explains 'unrecoverable'.
FWIW, I very much prefer plain words. "i.e." and "e.g." are really only
useful if you and all your readers know how they work.
I don't feel the need to be draconian about it, but I frankly *wish*
folks would just remove them from their written vocabulary.
^ permalink raw reply
* Re: [PATCH] dma-direct: swiotlb: Skip encryption toggles for swiotlb allocations
From: Aneesh Kumar K.V @ 2026-01-19 15:53 UTC (permalink / raw)
To: Robin Murphy, Marek Szyprowski, iommu, linux-kernel, linux-coco
Cc: steven.price, Suzuki K Poulose, Claire Chang
In-Reply-To: <3110c452-22d7-453f-bc60-7578b2458089@arm.com>
Robin Murphy <robin.murphy@arm.com> writes:
> On 19/01/2026 9:52 am, Marek Szyprowski wrote:
>> On 14.01.2026 10:49, Aneesh Kumar K.V wrote:
>>> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>> On 2026-01-09 2:51 am, Aneesh Kumar K.V wrote:
>>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>>> On 2026-01-02 3:54 pm, Aneesh Kumar K.V (Arm) wrote:
>>>>>>>> Swiotlb backing pages are already mapped decrypted via
>>>>>>>> swiotlb_update_mem_attributes(), so dma-direct does not need to call
>>>>>>>> set_memory_decrypted() during allocation or re-encrypt the memory on
>>>>>>>> free.
>>>>>>>>
>>>>>>>> Handle swiotlb-backed buffers explicitly: obtain the DMA address and
>>>>>>>> zero the linear mapping for lowmem pages, and bypass the decrypt/encrypt
>>>>>>>> transitions when allocating/freeing from the swiotlb pool (detected via
>>>>>>>> swiotlb_find_pool()).
>>>>>>> swiotlb_update_mem_attributes() only applies to the default SWIOTLB
>>>>>>> buffer, while the dma_direct_alloc_swiotlb() path is only for private
>>>>>>> restricted pools (because the whole point is that restricted DMA devices
>>>>>>> cannot use the regular allocator/default pools). There is no redundancy
>>>>>>> here AFAICS.
>>>>>>>
>>>>>> But rmem_swiotlb_device_init() is also marking the entire pool decrypted
>>>>>>
>>>>>> set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>>>>> rmem->size >> PAGE_SHIFT);
>>>>> OK, so why doesn't the commit message mention that instead of saying
>>>>> something which fails to justify the patch at all? ;)
>>>>>
>>>>> Furthermore, how much does this actually matter? The "real" restricted
>>>>> DMA use-case is on systems where dma_set_decrypted() is a no-op anyway.
>>>>> I know we used restricted DMA as a hack in the early days of CCA
>>>>> prototyping, but is it intended to actually deploy that as a supported
>>>>> and recommended mechanism now?
>>>>>
>>>>> Note also that the swiotlb_alloc path is essentially an emergency
>>>>> fallback, which doesn't work for all situations anyway - any restricted
>>>>> device that actually needs to make significant coherent allocations (or
>>>>> rather, that firmware cannot assume won't want to do so) should really
>>>>> have a proper coherent pool alongside its restricted one. The expected
>>>>> use-case here is for something like a wifi driver that only needs to
>>>>> allocate one or two small coherent buffers once at startup, then do
>>>>> everything else with streaming DMA.
>>>> I was aiming to bring more consistency in how swiotlb buffers are
>>>> handled, specifically by treating all swiotlb memory as decrypted
>>>> buffers, which is also how the current code behaves.
>>>>
>>>> If we are concluding that restricted DMA is not used in conjunction with
>>>> memory encryption, then we could, in fact, remove the
>>>> set_memory_decrypted() call from rmem_swiotlb_device_init() and
>>>> instead add failure conditions for force_dma_unencrypted(dev) in
>>>> is_swiotlb_for_alloc(). However, it’s worth noting that the initial
>>>> commit did take the memory encryption feature into account
>>>> (0b84e4f8b793eb4045fd64f6f514165a7974cd16).
>>>>
>>>> Please let me know if you think this needs to be fixed.
>>> Something like.
>>>
>>> dma-direct: restricted-dma: Do not mark the restricted DMA pool unencrypted
>>>
>>> As per commit f4111e39a52a ("swiotlb: Add restricted DMA alloc/free
>>> support"), the restricted-dma-pool is used in conjunction with the
>>> shared-dma-pool. Since allocations from the shared-dma-pool are not
>>> marked unencrypted, skip marking the restricted-dma-pool as unencrypted
>>> as well. We do not expect systems using the restricted-dma-pool to have
>>> memory encryption or to run with confidential computing features enabled.
>>>
>>> If a device requires unencrypted access (force_dma_unencrypted(dev)),
>>> the dma-direct allocator will mark the restricted-dma-pool allocation as
>>> unencrypted.
>>>
>>> The only disadvantage is that, when running on a CC guest with a
>>> different hypervisor page size, restricted-dma-pool allocation sizes
>>> must now be aligned to the hypervisor page size. This alignment would
>>> not be required if the entire pool were marked unencrypted. However, the
>>> new code enables the use of the restricted-dma-pool for trusted devices.
>>> Previously, because the entire pool was marked unencrypted, trusted
>>> devices were unable to allocate from it.
>>>
>>> There is still an open question regarding allocations from the
>>> shared-dma-pool. Currently, they are not marked unencrypted.
>>>
>>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>>
>>> 1 file changed, 2 deletions(-)
>>> kernel/dma/swiotlb.c | 2 --
>>>
>>> modified kernel/dma/swiotlb.c
>>> @@ -1835,8 +1835,6 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
>>> return -ENOMEM;
>>> }
>>>
>>> - set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>> - rmem->size >> PAGE_SHIFT);
>>> swiotlb_init_io_tlb_pool(pool, rmem->base, nslabs,
>>> false, nareas);
>>> mem->force_bounce = true;
>>
>> Robin, could You review this? Is it ready for applying?
>
> But wouldn't this break the actual intended use of restricted pools for
> streaming DMA bouncing, which does depend on the buffer being
> pre-decrypted/shared? (Since streaming DMA mappings definitely need to
> be supported in nowait contexts)
>
Only if we are using a restricted pool with encrypted memory.
If we assume that swiotlb bounce buffers are always decrypted, then
allocations from that pool can safely skip the decrypt/encrypt
transitions. However, we still need to address coherent allocations via
the shared-dma-pool, which are explicitly marked as unencrypted.
Given this, I’m wondering whether the best approach is to revisit the
original patch I posted, which moved swiotlb allocations out of
__dma_direct_alloc_pages(). With that separation in place, we could then
fix up dma_alloc_from_dev_coherent() accordingly.
If the conclusion is that systems with encrypted memory will, in
practice, never use restricted-dma-pool or shared-dma-pool, then we can
take this patch?
If you can suggest the approach you would like to see taken with
restricted-dma-pool/shared-dma-pool, I can work on the final change.
-aneesh
^ permalink raw reply
* Re: [PATCH] dma-direct: swiotlb: Skip encryption toggles for swiotlb allocations
From: Robin Murphy @ 2026-01-19 14:28 UTC (permalink / raw)
To: Marek Szyprowski, Aneesh Kumar K.V, iommu, linux-kernel,
linux-coco
Cc: steven.price, Suzuki K Poulose, Claire Chang
In-Reply-To: <0ad7a059-8c51-4d26-9d7e-055abb702b66@samsung.com>
On 19/01/2026 9:52 am, Marek Szyprowski wrote:
> On 14.01.2026 10:49, Aneesh Kumar K.V wrote:
>> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>> On 2026-01-09 2:51 am, Aneesh Kumar K.V wrote:
>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>> On 2026-01-02 3:54 pm, Aneesh Kumar K.V (Arm) wrote:
>>>>>>> Swiotlb backing pages are already mapped decrypted via
>>>>>>> swiotlb_update_mem_attributes(), so dma-direct does not need to call
>>>>>>> set_memory_decrypted() during allocation or re-encrypt the memory on
>>>>>>> free.
>>>>>>>
>>>>>>> Handle swiotlb-backed buffers explicitly: obtain the DMA address and
>>>>>>> zero the linear mapping for lowmem pages, and bypass the decrypt/encrypt
>>>>>>> transitions when allocating/freeing from the swiotlb pool (detected via
>>>>>>> swiotlb_find_pool()).
>>>>>> swiotlb_update_mem_attributes() only applies to the default SWIOTLB
>>>>>> buffer, while the dma_direct_alloc_swiotlb() path is only for private
>>>>>> restricted pools (because the whole point is that restricted DMA devices
>>>>>> cannot use the regular allocator/default pools). There is no redundancy
>>>>>> here AFAICS.
>>>>>>
>>>>> But rmem_swiotlb_device_init() is also marking the entire pool decrypted
>>>>>
>>>>> set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>>>> rmem->size >> PAGE_SHIFT);
>>>> OK, so why doesn't the commit message mention that instead of saying
>>>> something which fails to justify the patch at all? ;)
>>>>
>>>> Furthermore, how much does this actually matter? The "real" restricted
>>>> DMA use-case is on systems where dma_set_decrypted() is a no-op anyway.
>>>> I know we used restricted DMA as a hack in the early days of CCA
>>>> prototyping, but is it intended to actually deploy that as a supported
>>>> and recommended mechanism now?
>>>>
>>>> Note also that the swiotlb_alloc path is essentially an emergency
>>>> fallback, which doesn't work for all situations anyway - any restricted
>>>> device that actually needs to make significant coherent allocations (or
>>>> rather, that firmware cannot assume won't want to do so) should really
>>>> have a proper coherent pool alongside its restricted one. The expected
>>>> use-case here is for something like a wifi driver that only needs to
>>>> allocate one or two small coherent buffers once at startup, then do
>>>> everything else with streaming DMA.
>>> I was aiming to bring more consistency in how swiotlb buffers are
>>> handled, specifically by treating all swiotlb memory as decrypted
>>> buffers, which is also how the current code behaves.
>>>
>>> If we are concluding that restricted DMA is not used in conjunction with
>>> memory encryption, then we could, in fact, remove the
>>> set_memory_decrypted() call from rmem_swiotlb_device_init() and
>>> instead add failure conditions for force_dma_unencrypted(dev) in
>>> is_swiotlb_for_alloc(). However, it’s worth noting that the initial
>>> commit did take the memory encryption feature into account
>>> (0b84e4f8b793eb4045fd64f6f514165a7974cd16).
>>>
>>> Please let me know if you think this needs to be fixed.
>> Something like.
>>
>> dma-direct: restricted-dma: Do not mark the restricted DMA pool unencrypted
>>
>> As per commit f4111e39a52a ("swiotlb: Add restricted DMA alloc/free
>> support"), the restricted-dma-pool is used in conjunction with the
>> shared-dma-pool. Since allocations from the shared-dma-pool are not
>> marked unencrypted, skip marking the restricted-dma-pool as unencrypted
>> as well. We do not expect systems using the restricted-dma-pool to have
>> memory encryption or to run with confidential computing features enabled.
>>
>> If a device requires unencrypted access (force_dma_unencrypted(dev)),
>> the dma-direct allocator will mark the restricted-dma-pool allocation as
>> unencrypted.
>>
>> The only disadvantage is that, when running on a CC guest with a
>> different hypervisor page size, restricted-dma-pool allocation sizes
>> must now be aligned to the hypervisor page size. This alignment would
>> not be required if the entire pool were marked unencrypted. However, the
>> new code enables the use of the restricted-dma-pool for trusted devices.
>> Previously, because the entire pool was marked unencrypted, trusted
>> devices were unable to allocate from it.
>>
>> There is still an open question regarding allocations from the
>> shared-dma-pool. Currently, they are not marked unencrypted.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>
>> 1 file changed, 2 deletions(-)
>> kernel/dma/swiotlb.c | 2 --
>>
>> modified kernel/dma/swiotlb.c
>> @@ -1835,8 +1835,6 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
>> return -ENOMEM;
>> }
>>
>> - set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>> - rmem->size >> PAGE_SHIFT);
>> swiotlb_init_io_tlb_pool(pool, rmem->base, nslabs,
>> false, nareas);
>> mem->force_bounce = true;
>
> Robin, could You review this? Is it ready for applying?
But wouldn't this break the actual intended use of restricted pools for
streaming DMA bouncing, which does depend on the buffer being
pre-decrypted/shared? (Since streaming DMA mappings definitely need to
be supported in nowait contexts)
Thanks,
Robin.
^ permalink raw reply
* [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Salman Nabi @ 2026-01-19 12:27 UTC (permalink / raw)
To: vvidwans, 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-1-salman.nabi@arm.com>
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 */
+ }
+
+ 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 */
+ }
+ }
+
+ mutex_unlock(&lfa_lock);
+ return ret;
+}
+
+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;
+ }
+
+ 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");
--
2.25.1
^ permalink raw reply related
* [PATCH 0/1] Arm Live Firmware Activation (LFA) support
From: Salman Nabi @ 2026-01-19 12:27 UTC (permalink / raw)
To: vvidwans, andre.przywara, sudeep.holla, mark.rutland, lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
Hi reviewers,
(This is a follow-up to the Live Firmware Activation work that was
submitted for RFC [1]).
This patch introduces a Linux kernel driver implementing the Arm Live
Firmware Activation (LFA) specification [2]. LFA enables the activation
of updated firmware components without requiring a system reboot,
reducing downtime in environments such as data centers and hyperscale
systems.
Unlike firmware update process (which may use tools like fwupd), LFA
focuses solely on the activation of an already updated firmware
component, that is pending activation, without a system reboot. This
capability helps maintain service availability and minimize operational
disruption.
Key features of the driver:
* Detects LFA support in system firmware (EL3).
* Lists all firmware components that support live activation.
* Exposes component attributes (e.g., activation capability, and
activation pending) via sysfs under /sys/firmware/lfa/.
* Provides interfaces to:
- Trigger activation of an updated firmware component.
- Cancel an ongoing activation if required.
This work is conceptually similar to Intel’s Platform Firmware Runtime
Update and telemetry (PFRUT) [3] and TDX module updates [4], but
targets Arm platforms. The driver has been used to successfully activate
a Realm Management Monitor (RMM) firmware image in a controlled test
environment. RMM is analogous to Intel’s TDX module.
There is effort on similar work from the OCP [5]. Future work may
include integration with utilities like fwupd to automatically select
the appropriate driver, based on platform architecture, for Live/Runtime
firmware updates.
Note: The ACPI tables are described in the spec. The Device Tree
bindings are currently work-in-progress, and a follow up patch will soon
be submitted that will add the DT bindings to the driver.
Summary of changes since rfc:
- Updated SMCCC version 1.1 to 1.2 per the LFA specification requirement.
- Changed "image_props" array to a linked list to support the dynamic
removal and addition of firmware images.
- Added code to refresh firmware images following a successful activation.
- Added a work_queue to handle the removal of firmware image attribute
from it's respective kobject "_store" handle.
- Refactored prime and activate into separate functions.
- Kernel config for LFA now defaults to "y" i.e. included by default.
- Added individual kernel attribute files removal when removing the
respective kobjects using kobject_put().
- mutex_lock added to activate_fw_image() and prime_fw_image() calls.
- Renamed create_fw_inventory to update_fw_image_node.
- Renamed create_fw_images_tree to update_fw_images_tree.
- Added two more attributes due to specs update from bet0 to bet1:
current_version: For retrieval of the current firmware's version info.
pending_version: For retrieval of the pending firmware's version info.
- Minor changes such as, improved firmware image names, and code comments.
- do...while loops refactored to for(;;) loops.
Best regards,
Salman Nabi
[1] https://lore.kernel.org/all/20250625142722.1911172-2-andre.przywara@arm.com/
[2] https://developer.arm.com/documentation/den0147/latest/
[3] https://lore.kernel.org/all/cover.1631025237.git.yu.c.chen@intel.com/
[4] https://lore.kernel.org/all/20250523095322.88774-1-chao.gao@intel.com/
[5] https://www.opencompute.org/documents/hyperscale-cpu-impactless-firmware-updates-requirements-specification-v0-7-9-29-2025-pdf
Salman Nabi (1):
firmware: smccc: add support for Live Firmware Activation (LFA)
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
--
2.25.1
^ permalink raw reply
* Re: [PATCH] dma-direct: swiotlb: Skip encryption toggles for swiotlb allocations
From: Marek Szyprowski @ 2026-01-19 9:52 UTC (permalink / raw)
To: Aneesh Kumar K.V, Robin Murphy, iommu, linux-kernel, linux-coco
Cc: steven.price, Suzuki K Poulose, Claire Chang
In-Reply-To: <yq5aldi0cyms.fsf@kernel.org>
On 14.01.2026 10:49, Aneesh Kumar K.V wrote:
> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>> Robin Murphy <robin.murphy@arm.com> writes:
>>> On 2026-01-09 2:51 am, Aneesh Kumar K.V wrote:
>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>> On 2026-01-02 3:54 pm, Aneesh Kumar K.V (Arm) wrote:
>>>>>> Swiotlb backing pages are already mapped decrypted via
>>>>>> swiotlb_update_mem_attributes(), so dma-direct does not need to call
>>>>>> set_memory_decrypted() during allocation or re-encrypt the memory on
>>>>>> free.
>>>>>>
>>>>>> Handle swiotlb-backed buffers explicitly: obtain the DMA address and
>>>>>> zero the linear mapping for lowmem pages, and bypass the decrypt/encrypt
>>>>>> transitions when allocating/freeing from the swiotlb pool (detected via
>>>>>> swiotlb_find_pool()).
>>>>> swiotlb_update_mem_attributes() only applies to the default SWIOTLB
>>>>> buffer, while the dma_direct_alloc_swiotlb() path is only for private
>>>>> restricted pools (because the whole point is that restricted DMA devices
>>>>> cannot use the regular allocator/default pools). There is no redundancy
>>>>> here AFAICS.
>>>>>
>>>> But rmem_swiotlb_device_init() is also marking the entire pool decrypted
>>>>
>>>> set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>>> rmem->size >> PAGE_SHIFT);
>>> OK, so why doesn't the commit message mention that instead of saying
>>> something which fails to justify the patch at all? ;)
>>>
>>> Furthermore, how much does this actually matter? The "real" restricted
>>> DMA use-case is on systems where dma_set_decrypted() is a no-op anyway.
>>> I know we used restricted DMA as a hack in the early days of CCA
>>> prototyping, but is it intended to actually deploy that as a supported
>>> and recommended mechanism now?
>>>
>>> Note also that the swiotlb_alloc path is essentially an emergency
>>> fallback, which doesn't work for all situations anyway - any restricted
>>> device that actually needs to make significant coherent allocations (or
>>> rather, that firmware cannot assume won't want to do so) should really
>>> have a proper coherent pool alongside its restricted one. The expected
>>> use-case here is for something like a wifi driver that only needs to
>>> allocate one or two small coherent buffers once at startup, then do
>>> everything else with streaming DMA.
>> I was aiming to bring more consistency in how swiotlb buffers are
>> handled, specifically by treating all swiotlb memory as decrypted
>> buffers, which is also how the current code behaves.
>>
>> If we are concluding that restricted DMA is not used in conjunction with
>> memory encryption, then we could, in fact, remove the
>> set_memory_decrypted() call from rmem_swiotlb_device_init() and
>> instead add failure conditions for force_dma_unencrypted(dev) in
>> is_swiotlb_for_alloc(). However, it’s worth noting that the initial
>> commit did take the memory encryption feature into account
>> (0b84e4f8b793eb4045fd64f6f514165a7974cd16).
>>
>> Please let me know if you think this needs to be fixed.
> Something like.
>
> dma-direct: restricted-dma: Do not mark the restricted DMA pool unencrypted
>
> As per commit f4111e39a52a ("swiotlb: Add restricted DMA alloc/free
> support"), the restricted-dma-pool is used in conjunction with the
> shared-dma-pool. Since allocations from the shared-dma-pool are not
> marked unencrypted, skip marking the restricted-dma-pool as unencrypted
> as well. We do not expect systems using the restricted-dma-pool to have
> memory encryption or to run with confidential computing features enabled.
>
> If a device requires unencrypted access (force_dma_unencrypted(dev)),
> the dma-direct allocator will mark the restricted-dma-pool allocation as
> unencrypted.
>
> The only disadvantage is that, when running on a CC guest with a
> different hypervisor page size, restricted-dma-pool allocation sizes
> must now be aligned to the hypervisor page size. This alignment would
> not be required if the entire pool were marked unencrypted. However, the
> new code enables the use of the restricted-dma-pool for trusted devices.
> Previously, because the entire pool was marked unencrypted, trusted
> devices were unable to allocate from it.
>
> There is still an open question regarding allocations from the
> shared-dma-pool. Currently, they are not marked unencrypted.
>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>
> 1 file changed, 2 deletions(-)
> kernel/dma/swiotlb.c | 2 --
>
> modified kernel/dma/swiotlb.c
> @@ -1835,8 +1835,6 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> return -ENOMEM;
> }
>
> - set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
> - rmem->size >> PAGE_SHIFT);
> swiotlb_init_io_tlb_pool(pool, rmem->base, nslabs,
> false, nareas);
> mem->force_bounce = true;
Robin, could You review this? Is it ready for applying?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v2 20/21] x86/virt/tdx: Update tdx_sysinfo and check features post-update
From: Chao Gao @ 2026-01-19 9:24 UTC (permalink / raw)
To: 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: <a648b7bc-11aa-41f0-bf42-ca9b66afe6dc@linux.intel.com>
On Wed, Dec 03, 2025 at 03:41:50PM +0800, Binbin Wu wrote:
>
>
>On 10/1/2025 10:53 AM, Chao Gao wrote:
>
>[...]
>>
>> +/*
>> + * Update tdx_sysinfo and check if any TDX module features changed after
>> + * updates
>> + */
>> +int tdx_module_post_update(struct tdx_sys_info *info)
>> +{
>> + struct tdx_sys_info_version *cur, *new;
>> + int ret;
>> +
>> + /* Shouldn't fail as the update has succeeded */
>> + ret = get_tdx_sys_info(info);
>> + if (ret) {
>> + WARN_ONCE(1, "version retrieval failed after update, replace TDX Module\n");
>
>Nit:
>Could be if (WARN_ONCE(ret, "..."))
ack.
>
>> + return ret;
>> + }
>> +
>> + guard(mutex)(&tdx_module_lock);
>> +
>> + cur = &tdx_sysinfo.version;
>
>Nit:
>After update, the current TDX module is the new TDX module already, may be
>better to use old instead of cur.
Indeed.
>
>> + new = &info->version;
>> + pr_info("version %u.%u.%02u -> %u.%u.%02u\n", cur->major_version,
>> + cur->minor_version,
>> + cur->update_version,
>> + new->major_version,
>> + new->minor_version,
>> + new->update_version);
>> +
>> + /*
>> + * Blindly refreshing the entire tdx_sysinfo could disrupt running
>> + * software, as it may subtly rely on the previous state unless
>> + * proven otherwise.
>> + *
>> + * Only refresh version information (including handoff version)
>> + * that does not affect functionality, and ignore all other
>> + * changes.
>> + */
>> + tdx_sysinfo.version = info->version;
>> + tdx_sysinfo.handoff = info->handoff;
>> +
>> + if (!memcmp(&tdx_sysinfo, info, sizeof(*info)))
>> + return 0;
>> +
>> + pr_info("TDX module features have changed after updates, but might not take effect.\n");
>> + pr_info("Please consider a potential BIOS update.\n");
>
>
>BIOS update?
>I guess it's "TDX module update via BIOS"?
ok. I will update the log message.
>
>Does it mean after a system reboot, the change done by TD preserving update will
>be gone?
Yes. After reboot, the update will be gone. The (old) TDX module will be
reloaded by the BIOS.
>If we want the TDX module upgrade to be permanent, it needs to replace
>the TDX module binary the BIOS will load, right?
Yes.
>
>So the scenario of TD preserving update seems to be limited to security fixes?
>(I guess the security fixes will take effect directly after TD preserving
>update?)
Yes. Fixes (whether security, performance, or functional) will take effect. New
features won't. This series takes a minimalist approach, updating only the
module version and handoff-version while leaving all other TDX metadata
unchanged.
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.
This enabling approach is aligned with current microcode updates.
^ permalink raw reply
* Re: [PATCH v2 19/21] x86/virt/tdx: Establish contexts for the new TDX Module
From: Chao Gao @ 2026-01-19 8:49 UTC (permalink / raw)
To: 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: <0a73607f-fe0b-4bd5-bc01-cd90841a39fa@linux.intel.com>
>> +int tdx_module_run_update(void)
>> +{
>> + struct tdx_module_args args = {};
>> + int ret;> +
>> + ret = seamcall(TDH_SYS_UPDATE, &args);
>
>Since it's a seamcall error, shouldn't it be u64?
Good catch. I will use seamcall_prerr(), which returns an int.
>
>> + if (ret) {
>> + tdx_module_status = TDX_MODULE_ERROR;
>> + pr_info("module update failed: %d\n", ret);
>
>pr_info -> pr_err?
Yes.
^ permalink raw reply
* Re: [PATCH v2 14/21] x86/virt/seamldr: Shut down the current TDX module
From: Chao Gao @ 2026-01-19 8:41 UTC (permalink / raw)
To: 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, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Kirill A. Shutemov,
Paolo Bonzini, Rick Edgecombe
In-Reply-To: <daa72134-387b-467c-8eed-88591287c983@linux.intel.com>
On Wed, Dec 03, 2025 at 10:24:58AM +0800, Binbin Wu wrote:
>
>
>On 10/1/2025 10:52 AM, Chao Gao wrote:
>> TDX Module updates request shutting down the existing TDX module.
>> During this shutdown, the module generates hand-off data, which captures
>> the module's states essential for preserving running TDs. The new TDX
>> Module can utilize this hand-off data to establish its states.
>>
>> Invoke the TDH_SYS_SHUTDOWN SEAMCALL on one CPU to perform the shutdown.
>> This SEAMCALL requires a hand-off module version. Use the module's own
>> hand-off version, as it is the highest version the module can produce and
>> is more likely to be compatible with new modules as new modules likely have
>> higher hand-off version.
>
>According to the TDX module base spec (348549006), each TDX module is built with
>TDX Module Handoff Constants, including No-Downgrade Flag. If the current TDX
>module is built with NO_DOWNGRADE=1, the hand-off module version must be the
>current TDX module's HV.
>
>This patch series doesn't seems to handle No-Downgrade Flag, IIUC it needs
>to use the current TDX module's HV to avoid failures.
Note: this patch always uses the current TDX module's HV. So, it won't fail
regardlss of No-Downgrade flag.
>
>About "hand-off version" and "No-Downgrade Flag", I still have some questions.
>Is it possible that two TDX module versions have the same hand-off version?
Yes.
>If the newer TDX module built with NO_DOWNGRADE=1, is it possible to downgrade
>to the older TDX module when they are using the same hand-off version?
AFAIK, this is possible in TDX architecture as long as the SEAMSVN (TDX
module's SVN) doesn't downgrade.
But for now, there is no plan to support downgrade (or roll-back) in any case
as it may result in lost features and cause compatibility issues. so, the
userspace tool [1] now rejects any downgrade attempts
[1]: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/28a4baabc268b1998ec553ab9009f4fd3efd309d/version_select_and_load.py#L301
^ permalink raw reply
* Re: [PATCH v2 2/2] x86/virt/tdx: Print TDX module version during init
From: Tony Lindgren @ 2026-01-19 8:12 UTC (permalink / raw)
To: Vishal Verma
Cc: linux-kernel, linux-coco, kvm, x86, Chao Gao, Dan Williams,
Kai Huang, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe
In-Reply-To: <20260109-tdx_print_module_version-v2-2-e10e4ca5b450@intel.com>
On Fri, Jan 09, 2026 at 12:14:31PM -0700, Vishal Verma wrote:
> It is useful to print the TDX module version in dmesg logs. This is
> currently the only way to determine the module version from the host. It
> also creates a record for any future problems being investigated. This
> was also requested in [1].
>
> Include the version in the log messages during init, e.g.:
>
> virt/tdx: TDX module version: 1.5.24
> virt/tdx: 1034220 KB allocated for PAMT
> virt/tdx: module initialized
>
> Print the version in get_tdx_sys_info(), right after the version
> metadata is read, which makes it available even if there are subsequent
> initialization failures.
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v2 1/2] x86/virt/tdx: Retrieve TDX module version
From: Tony Lindgren @ 2026-01-19 8:12 UTC (permalink / raw)
To: Vishal Verma
Cc: linux-kernel, linux-coco, kvm, x86, Chao Gao, Dan Williams,
Kai Huang, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe
In-Reply-To: <20260109-tdx_print_module_version-v2-1-e10e4ca5b450@intel.com>
On Fri, Jan 09, 2026 at 12:14:30PM -0700, Vishal Verma wrote:
> From: Chao Gao <chao.gao@intel.com>
>
> Each TDX module has several bits of metadata about which specific TDX
> module it is. The primary bit of info is the version, which has an x.y.z
> format. These represent the major version, minor version, and update
> version respectively. Knowing the running TDX Module version is valuable
> for bug reporting and debugging. Note that the module does expose other
> pieces of version-related metadata, such as build number and date. Those
> aren't retrieved for now, that can be added if needed in the future.
>
> Retrieve the TDX Module version using the existing metadata reading
> interface. Later changes will expose this information. The metadata
> reading interfaces have existed for quite some time, so this will work
> with older versions of the TDX module as well - i.e. this isn't a new
> interface.
This is great to have for debugging, if not too late:
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v2 16/21] x86/virt/seamldr: Handle TDX Module update failures
From: Chao Gao @ 2026-01-19 5:34 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Xu Yilun, linux-coco, linux-kernel, x86, reinette.chatre,
ira.weiny, kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov
In-Reply-To: <CB733CD0-C5B5-464C-AD75-DD5475BCF194@zytor.com>
On Sun, Jan 18, 2026 at 08:55:22PM -0800, H. Peter Anvin wrote:
>On January 14, 2026 10:24:22 PM PST, Xu Yilun <yilun.xu@linux.intel.com> wrote:
>>On Tue, Sep 30, 2025 at 07:53:00PM -0700, Chao Gao wrote:
>>> Failures encountered after a successful module shutdown are unrecoverable,
>>> e.g., there is no way to restore the old TDX Module.
>>
>>"e.g." is obscure. To me, the following sentence is explaining the
>>reason why the failure is not recoverable. Maybe "i.e." or "because"?
>
>"e.g." (for example) means the following list is non-exhaustive.
Yes, 'i.e.' or 'because' would be more appropriate since the next sentence
explains 'unrecoverable'.
^ permalink raw reply
* Re: [PATCH v2 16/21] x86/virt/seamldr: Handle TDX Module update failures
From: H. Peter Anvin @ 2026-01-19 4:55 UTC (permalink / raw)
To: Xu Yilun, Chao Gao
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov
In-Reply-To: <aWiIFlbrGqBjpyvX@yilunxu-OptiPlex-7050>
On January 14, 2026 10:24:22 PM PST, Xu Yilun <yilun.xu@linux.intel.com> wrote:
>On Tue, Sep 30, 2025 at 07:53:00PM -0700, Chao Gao wrote:
>> Failures encountered after a successful module shutdown are unrecoverable,
>> e.g., there is no way to restore the old TDX Module.
>
>"e.g." is obscure. To me, the following sentence is explaining the
>reason why the failure is not recoverable. Maybe "i.e." or "because"?
"e.g." (for example) means the following list is non-exhaustive.
^ permalink raw reply
* Re: [PATCH v2 1/1] PCI/IDE: Fix using wrong VF ID for RID range calculation
From: Xu Yilun @ 2026-01-19 2:21 UTC (permalink / raw)
To: Li Ming; +Cc: helgaas, dan.j.williams, linux-pci, linux-coco, linux-kernel
In-Reply-To: <20260114111455.550984-1-ming.li@zohomail.com>
On Wed, Jan 14, 2026 at 07:14:55PM +0800, Li Ming wrote:
> When allocate a new IDE stream for a PCI device in SR-IOV case, the RID
> range of the new IDE stream should cover all VFs of the device. VF ID
> range of a PCI device is [0, num_VFs - 1], so should use (num_VFs - 1)
> as the last VF's ID.
>
> Fixes: 1e4d2ff3ae45 ("PCI/IDE: Add IDE establishment helpers")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
^ 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