Re: [PATCH v2 6/8] firmware: smccc: lfa: Add auto_activate sysfs file

[Nirmoy Das <nirmoyd@nvidia.com>]

Hi Andre,

On 17.03.26 12:33, Andre Przywara wrote:
> The Arm LFA spec places control over the actual activation process in
> the hands of the non-secure host OS. An platform initiated interrupt or
> notification signals the availability of an updateable firmware image,
> but does not necessarily need to trigger it automatically.
>
> Add a sysfs control file that guards such automatic activation. If an
> administrator wants to allow automatic platform initiated updates, they
> can activate that by echoing a "1" into the auto_activate file in the
> respective sysfs directory. Any incoming notification would then result
> in the activation triggered.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>   drivers/firmware/smccc/lfa_fw.c | 34 ++++++++++++++++++++++++++++++---
>   1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index f20ea45cdbd9..5dc531e462eb 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -101,6 +101,7 @@ enum image_attr_names {
>   	LFA_ATTR_FORCE_CPU_RENDEZVOUS,
>   	LFA_ATTR_ACTIVATE,
>   	LFA_ATTR_CANCEL,
> +	LFA_ATTR_AUTO_ACTIVATE,
>   	LFA_ATTR_NR_IMAGES
>   };
>   
> @@ -115,6 +116,7 @@ struct fw_image {
>   	bool may_reset_cpu;
>   	bool cpu_rendezvous;
>   	bool cpu_rendezvous_forced;
> +	bool auto_activate;
>   	struct kobj_attribute image_attrs[LFA_ATTR_NR_IMAGES];
>   };
>   
> @@ -561,6 +563,28 @@ static ssize_t cancel_store(struct kobject *kobj, struct kobj_attribute *attr,
>   	return count;
>   }
>   
> +static ssize_t auto_activate_store(struct kobject *kobj,
> +				   struct kobj_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	struct fw_image *image = kobj_to_fw_image(kobj);
> +	int ret;
> +
> +	ret = kstrtobool(buf, &image->auto_activate);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static ssize_t auto_activate_show(struct kobject *kobj,
> +				  struct kobj_attribute *attr, char *buf)
> +{
> +	struct fw_image *image = kobj_to_fw_image(kobj);
> +
> +	return sysfs_emit(buf, "%d\n", image->auto_activate);
> +}
> +
>   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),
> @@ -571,7 +595,8 @@ static struct kobj_attribute image_attrs_group[LFA_ATTR_NR_IMAGES] = {
>   	[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)
> +	[LFA_ATTR_CANCEL]		= __ATTR_WO(cancel),
> +	[LFA_ATTR_AUTO_ACTIVATE]	= __ATTR_RW(auto_activate),
>   };
>   
>   static void init_image_default_attrs(void)
> @@ -640,6 +665,7 @@ static int update_fw_image_node(char *fw_uuid, int seq_id,
>   	image->kobj.kset = lfa_kset;
>   	image->image_name = image_name;
>   	image->cpu_rendezvous_forced = true;
> +	image->auto_activate = false;


I think sysadmins should be able to automate setting this at boot and
after firmware inventory changes.

A udev rule matching /sys/firmware/lfa/<uuid>/auto_activate did not work
when I tried it, since those entries are plain kobjects under
/sys/firmware. The arm-lfa faux device worked better as the event anchor,
with something like:

   if (lfa_dev)
           kobject_uevent(&lfa_dev->dev.kobj, KOBJ_CHANGE);

Then userspace can use:

   ACTION=="add|change", SUBSYSTEM=="faux", KERNEL=="arm-lfa", \
           RUN+="/usr/sbin/lfa-auto-activate"

What do you think about adding such a notification in v3?

Regards,
Nirmoy


>   	set_image_flags(image, seq_id, image_flags, reg_current_ver,
>   			reg_pending_ver);
>   	if (kobject_init_and_add(&image->kobj, &image_ktype, NULL,
> @@ -709,7 +735,8 @@ static int update_fw_images_tree(void)
>   
>   /*
>    * Go through all FW images in a loop and trigger activation
> - * of all activatible and pending images.
> + * of all activatible and pending images, but only if automatic
> + * activation for that image is allowed.
>    * We have to restart enumeration after every triggered activation,
>    * since the firmware images might have changed during the activation.
>    */
> @@ -728,7 +755,8 @@ static int activate_pending_image(void)
>   			continue; /* Invalid FW component */
>   
>   		update_fw_image_pending(image);
> -		if (image->activation_capable && image->activation_pending) {
> +		if (image->activation_capable && image->activation_pending &&
> +		    image->auto_activate) {
>   			found_pending = true;
>   			break;
>   		}