Re: [RFC PATCH 1/3] firmware: smccc: LFA: use smcc 1.2

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

On Wed, 8 Oct 2025 19:09:05 +0000
Vedashree Vidwans <vvidwans@nvidia.com> wrote:

Hi Vedashree,

> Update driver to use SMCCC 1.2+ version as mentioned in the LFA spec.

ah, right, good catch, one call is using x4, so this must be the v1.2
calling convention.

Just one small thing below...

> Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
> ---
>  drivers/firmware/smccc/lfa_fw.c | 80 +++++++++++++++++++++++----------
>  1 file changed, 56 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index 1f333237271d8..49f7feb6a211b 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -117,9 +117,13 @@ static struct kobject *lfa_dir;
>  
>  static int get_nr_lfa_components(void)
>  {
> -	struct arm_smccc_res res = { 0 };
> +	struct arm_smccc_1_2_regs args = { 0 };
> +	struct arm_smccc_1_2_regs res = { 0 };

>  
> -	arm_smccc_1_1_invoke(LFA_1_0_FN_GET_INFO, 0x0, &res);
> +	args.a0 = LFA_1_0_FN_GET_INFO;
> +	args.a1 = 0; /* lfa_info_selector = 0 */
> +
> +	arm_smccc_1_2_invoke(&args, &res);

I wonder if we can share the same struct for both request and reply?
	arm_smccc_1_2_invoke(&args, &args);

Looks like a lot of stack space used for just a few registers.
Same for the other occasions where we just do the smc once.

Cheers,
Andre.

>  	if (res.a0 != LFA_SUCCESS)
>  		return res.a0;
>  
> @@ -129,20 +133,23 @@ static int get_nr_lfa_components(void)
>  static int call_lfa_activate(void *data)
>  {
>  	struct image_props *attrs = data;
> -	struct arm_smccc_res res = { 0 };
> +	struct arm_smccc_1_2_regs args = { 0 };
> +	struct arm_smccc_1_2_regs res = { 0 };
> +
> +	args.a0 = LFA_1_0_FN_ACTIVATE;
> +	args.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 args.a3 and args.a4, holding the entry point and context
> +	 * ID respectively.
> +	 * We want to force CPU rendezvous if either cpu_rendezvous or
> +	 * cpu_rendezvous_forced is set. The flag value is flipped as
> +	 * it is called skip_cpu_rendezvous in the spec.
> +	 */
> +	args.a2 = !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous);
>  
>  	do {
> -		/*
> -		 * As we do not support updates requiring a CPU reset (yet),
> -		 * we pass 0 in x3 and x4, holding the entry point and context
> -		 * ID respectively.
> -		 * We want to force CPU rendezvous if either cpu_rendezvous or
> -		 * cpu_rendezvous_forced is set. The flag value is flipped as
> -		 * it is called skip_cpu_rendezvous in the spec.
> -		 */
> -		arm_smccc_1_1_invoke(LFA_1_0_FN_ACTIVATE, attrs->fw_seq_id,
> -			!(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous),
> -			0, 0, &res);
> +		arm_smccc_1_2_invoke(&args, &res);
>  	} while (res.a0 == 0 && res.a1 == 1);
>  
>  	return res.a0;
> @@ -150,7 +157,8 @@ static int call_lfa_activate(void *data)
>  
>  static int activate_fw_image(struct image_props *attrs)
>  {
> -	struct arm_smccc_res res = { 0 };
> +	struct arm_smccc_1_2_regs args = { 0 };
> +	struct arm_smccc_1_2_regs res = { 0 };
>  	int ret;
>  
>  	/*
> @@ -159,8 +167,10 @@ static int activate_fw_image(struct image_props *attrs)
>  	 * LFA_PRIME/ACTIVATE will need to be called again.
>  	 * res.a1 will become 0 once the prime/activate process completes.
>  	 */
> +	args.a0 = LFA_1_0_FN_PRIME;
> +	args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
>  	do {
> -		arm_smccc_1_1_invoke(LFA_1_0_FN_PRIME, attrs->fw_seq_id, &res);
> +		arm_smccc_1_2_invoke(&args, &res);
>  		if (res.a0 != LFA_SUCCESS) {
>  			pr_err("LFA_PRIME failed: %s\n",
>  				lfa_error_strings[-res.a0]);
> @@ -211,13 +221,16 @@ static ssize_t activation_pending_show(struct kobject *kobj,
>  {
>  	struct image_props *attrs = container_of(attr, struct image_props,
>  					 image_attrs[LFA_ATTR_ACT_PENDING]);
> -	struct arm_smccc_res res = { 0 };
> +	struct arm_smccc_1_2_regs args = { 0 };
> +	struct arm_smccc_1_2_regs res = { 0 };
>  
>  	/*
>  	 * Activation pending status can change anytime thus we need to update
>  	 * and return its current value
>  	 */
> -	arm_smccc_1_1_invoke(LFA_1_0_FN_GET_INVENTORY, attrs->fw_seq_id, &res);
> +	args.a0 = LFA_1_0_FN_GET_INVENTORY;
> +	args.a1 = attrs->fw_seq_id;
> +	arm_smccc_1_2_invoke(&args, &res);
>  	if (res.a0 == LFA_SUCCESS)
>  		attrs->activation_pending = !!(res.a3 & BIT(1));
>  
> @@ -298,9 +311,12 @@ static ssize_t cancel_store(struct kobject *kobj, struct kobj_attribute *attr,
>  {
>  	struct image_props *attrs = container_of(attr, struct image_props,
>  						 image_attrs[LFA_ATTR_CANCEL]);
> -	struct arm_smccc_res res = { 0 };
> +	struct arm_smccc_1_2_regs args = { 0 };
> +	struct arm_smccc_1_2_regs res = { 0 };
>  
> -	arm_smccc_1_1_invoke(LFA_1_0_FN_CANCEL, attrs->fw_seq_id, &res);
> +	args.a0 = LFA_1_0_FN_CANCEL;
> +	args.a1 = attrs->fw_seq_id;
> +	arm_smccc_1_2_invoke(&args, &res);
>  
>  	/*
>  	 * When firmware activation is called with "skip_cpu_rendezvous=1",
> @@ -395,14 +411,17 @@ static int create_fw_inventory(char *fw_uuid, int seq_id, u32 image_flags)
>  
>  static int create_fw_images_tree(void)
>  {
> -	struct arm_smccc_res res = { 0 };
> +	struct arm_smccc_1_2_regs args = { 0 };
> +	struct arm_smccc_1_2_regs res = { 0 };
>  	struct uuid_regs image_uuid;
>  	char image_id_str[40];
>  	int ret, num_of_components;
>  
>  	num_of_components = get_nr_lfa_components();
> +	args.a0 = LFA_1_0_FN_GET_INVENTORY;
>  	for (int i = 0; i < num_of_components; i++) {
> -		arm_smccc_1_1_invoke(LFA_1_0_FN_GET_INVENTORY, i, &res);
> +		args.a1 = i; /* fw_seq_id under consideration */
> +		arm_smccc_1_2_invoke(&args, &res);
>  		if (res.a0 == LFA_SUCCESS) {
>  			image_uuid.uuid_lo = res.a1;
>  			image_uuid.uuid_hi = res.a2;
> @@ -420,10 +439,23 @@ static int create_fw_images_tree(void)
>  
>  static int __init lfa_init(void)
>  {
> -	struct arm_smccc_res res = { 0 };
> +	struct arm_smccc_1_2_regs args = { 0 };
> +	struct arm_smccc_1_2_regs res = { 0 };
>  	int err;
>  
> -	arm_smccc_1_1_invoke(LFA_1_0_FN_GET_VERSION, &res);
> +	/* LFA requires SMCCC version >= 1.2 */
> +	if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2) {
> +		pr_err("Not supported with SMCCC version %u", arm_smccc_get_version());
> +		return -ENODEV;
> +	}
> +
> +	if (arm_smccc_1_1_get_conduit() == SMCCC_CONDUIT_NONE) {
> +		pr_err("Invalid SMCCC conduit");
> +		return -ENODEV;
> +	}
> +
> +	args.a0 = LFA_1_0_FN_GET_VERSION;
> +	arm_smccc_1_2_invoke(&args, &res);
>  	if (res.a0 == -LFA_NOT_SUPPORTED) {
>  		pr_err("Arm Live Firmware activation(LFA): no firmware agent found\n");
>  		return -ENODEV;