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

[Andre Przywara <andre.przywara@arm.com>]

Hi Nirmoy,

thanks for your replies and comments, I think I incorporated every one 
in the v3 post. However this one here is a bit tricky ...

On 6/24/26 21:45, Nirmoy Das wrote:
> 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,

I like that idea, however with the change from faux device to the SMCCC 
bus this doesn't work as easily anymore. There would be still a device 
somewhere, but I don't know how this is mapped to udev.
Since you seem to know more about it, can you please check how the 
changed patch set behaves in this respect? And ideally let us know what 
it would take to adapt the uevent code the new version?

Cheers,
Andre


> 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;
>>           }