Re: [PATCH v2 4/8] firmware: smccc: lfa: Add timeout and trigger watchdog

[Nirmoy Das <nirmoyd@nvidia.com>]

Hi Andre, Veda,


On 17.03.26 12:33, Andre Przywara wrote:
> From: Vedashree Vidwans <vvidwans@nvidia.com>
>
> Enhance PRIME/ACTIVATION functions to touch watchdog and implement
> timeout mechanism. This update ensures that any potential hangs are
> detected promptly and that the LFA process is allocated sufficient
> execution time before the watchdog timer expires. These changes improve
> overall system reliability by reducing the risk of undetected process
> stalls and unexpected watchdog resets.
>
> Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>   drivers/firmware/smccc/lfa_fw.c | 43 ++++++++++++++++++++++++++++++---
>   1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index 4831abf2b60e..d1b5cd29b8a0 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -5,11 +5,14 @@
>   
>   #include <linux/arm-smccc.h>
>   #include <linux/array_size.h>
> +#include <linux/delay.h>
>   #include <linux/fs.h>
>   #include <linux/init.h>
>   #include <linux/kobject.h>
> +#include <linux/ktime.h>
>   #include <linux/list.h>
>   #include <linux/module.h>
> +#include <linux/nmi.h>
>   #include <linux/psci.h>
>   #include <linux/stop_machine.h>
>   #include <linux/string.h>
> @@ -38,6 +41,11 @@
>   #define LFA_PRIME_CALL_AGAIN		BIT(0)
>   #define LFA_ACTIVATE_CALL_AGAIN		BIT(0)
>   
> +#define LFA_PRIME_BUDGET_MS		30000		/* 30s cap */
> +#define LFA_PRIME_DELAY_MS		10		/* 10ms between polls */
> +#define LFA_ACTIVATE_BUDGET_MS		10000		/* 10s cap */
> +#define LFA_ACTIVATE_DELAY_MS		10		/* 10ms between polls */
> +
>   /* LFA return values */
>   #define LFA_SUCCESS			0
>   #define LFA_NOT_SUPPORTED		1
> @@ -287,6 +295,7 @@ static int call_lfa_activate(void *data)
>   	struct fw_image *image = data;
>   	struct arm_smccc_1_2_regs reg = { 0 }, res;
>   
> +	touch_nmi_watchdog();
>   	reg.a0 = LFA_1_0_FN_ACTIVATE;
>   	reg.a1 = image->fw_seq_id;
>   	/*
> @@ -310,6 +319,7 @@ static int call_lfa_activate(void *data)
>   
>   static int activate_fw_image(struct fw_image *image)
>   {
> +	ktime_t end = ktime_add_ms(ktime_get(), LFA_ACTIVATE_BUDGET_MS);
>   	int ret;
>   
>   retry:
> @@ -324,8 +334,15 @@ static int activate_fw_image(struct fw_image *image)
>   		return 0;
>   	}
>   
> -	if (ret == -LFA_CALL_AGAIN)
> -		goto retry;
> +	if (ret == -LFA_CALL_AGAIN) {
> +		/* SMC returned with call_again flag set */
> +		if (ktime_before(ktime_get(), end)) {
> +			msleep_interruptible(LFA_ACTIVATE_DELAY_MS);
> +			goto retry;
> +		}
> +
> +		ret = -LFA_TIMED_OUT;
> +	}
>   
>   	lfa_cancel(image);
Correct me if I am wrong here

DEN0147 2.6.6: LFA_ACTIVATE can return LFA_BUSY ("activation postponed 
... prime status not changed"), i.e. retriable like CALL_AGAIN. So we need:
       if (ret == -LFA_BUSY) {
           if (ktime_before(ktime_get(), end)) {
               msleep_interruptible(LFA_ACTIVATE_DELAY_MS);
               goto retry;
           }
           ret = -LFA_TIMED_OUT;

       }

Regards,

Nirmoy


>   
> @@ -338,6 +355,8 @@ static int activate_fw_image(struct fw_image *image)
>   static int prime_fw_image(struct fw_image *image)
>   {
>   	struct arm_smccc_1_2_regs reg = { 0 }, res;
> +	ktime_t end = ktime_add_ms(ktime_get(), LFA_PRIME_BUDGET_MS);
> +	int ret;
>   
>   	if (image->may_reset_cpu) {
>   		pr_err("CPU reset not supported by kernel driver\n");
> @@ -345,6 +364,8 @@ static int prime_fw_image(struct fw_image *image)
>   		return -EINVAL;
>   	}
>   
> +	touch_nmi_watchdog();
> +
>   	reg.a0 = LFA_1_0_FN_PRIME;
>   retry:
>   	/*
> @@ -363,8 +384,22 @@ static int prime_fw_image(struct fw_image *image)
>   		return res.a0;
>   	}
>   
> -	if (res.a1 & LFA_PRIME_CALL_AGAIN)
> -		goto retry;
> +	if (res.a1 & LFA_PRIME_CALL_AGAIN) {
> +		/* SMC returned with call_again flag set */
> +		if (ktime_before(ktime_get(), end)) {
> +			msleep_interruptible(LFA_PRIME_DELAY_MS);
> +			goto retry;
> +		}
> +
> +		pr_err("LFA_PRIME for image %s timed out",
> +		       get_image_name(image));
> +
> +		ret = lfa_cancel(image);
> +		if (ret != 0)
> +			return ret;
> +
> +		return -ETIMEDOUT;
> +	}
>   
>   	return 0;
>   }