[PATCH v6 0/2] tpm_crb_ffa: handle tpm busy return code

[PATCH v6 0/2] tpm_crb_ffa: handle tpm busy return code

[Prachotan Bathi]

Platforms that support FF-A direct message request v2 can implement
Secure Partitions (SPs) that host multiple services. When the TPM
service shares its SP with other services, message requests from the
driver may fail with a BUSY response if another service is currently
active.

To improve robustness in such scenarios, we need to introduce retry
logic in the driver. When a BUSY error is received, the driver will
re-attempt the TPM request until it succeeds or a run-time configurable 
timeout(default: 2000 ms) is reached. This ensures reliable TPM access 
under shared-SP conditions.

Add a module parameter, `busy_timeout_ms`, which specifies the
maximum amount of time (in milliseconds) to retry on BUSY before giving
up.

This change builds on top of commit a85b55ee64a5, which introduced
support for TPM service communication using the FF-A direct message v2
path, in accordance with section 3.3 of the TPM Service Command
Response Buffer Interface specification.
https://developer.arm.com/documentation/den0138/latest/

This was tested with an FF-A based fTPM currently not publicly available. 
There are plans to open source the fTPM.

Changes in v6:
- Typo fixes in function name.
- Introduce __tpm_crb_ffa_try_send_receive.
- Modify tpm_crb_ffa_send_receive to use the new function with retry logic.
- Use memzero() macro instead of memset() for clearing buffers.

Prachotan Bathi (2):
  tpm_crb_ffa: Fix typos in function name
  tpm_crb_ffa: handle tpm busy return code

 drivers/char/tpm/tpm_crb_ffa.c | 70 ++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH v6 1/2] tpm_crb_ffa: Fix typos in function name

[Prachotan Bathi]

s/recieve/receive in __tpm_crb_ffa_send_receive

Signed-off-by: Prachotan Bathi <prachotan.bathi@arm.com>
---
 drivers/char/tpm/tpm_crb_ffa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 4ead61f01299..089d1e54bb46 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -178,7 +178,7 @@ int tpm_crb_ffa_init(void)
 }
 EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
 
-static int __tpm_crb_ffa_send_recieve(unsigned long func_id,
+static int __tpm_crb_ffa_send_receive(unsigned long func_id,
 				      unsigned long a0,
 				      unsigned long a1,
 				      unsigned long a2)
@@ -251,7 +251,7 @@ int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor)
 
 	guard(mutex)(&tpm_crb_ffa->msg_data_lock);
 
-	rc = __tpm_crb_ffa_send_recieve(CRB_FFA_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);
+	rc = __tpm_crb_ffa_send_receive(CRB_FFA_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);
 	if (!rc) {
 		if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
 			*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data2.data[1]);
@@ -289,7 +289,7 @@ int tpm_crb_ffa_start(int request_type, int locality)
 
 	guard(mutex)(&tpm_crb_ffa->msg_data_lock);
 
-	return __tpm_crb_ffa_send_recieve(CRB_FFA_START, request_type, locality, 0x00);
+	return __tpm_crb_ffa_send_receive(CRB_FFA_START, request_type, locality, 0x00);
 }
 EXPORT_SYMBOL_GPL(tpm_crb_ffa_start);
 
-- 
2.43.0

[PATCH v6 2/2] tpm_crb_ffa: handle tpm busy return code

[Prachotan Bathi]

Platforms supporting direct message request v2 [1] can support secure
partitions that support multiple services. For CRB over FF-A interface,
if the firmware TPM or TPM service [1] shares its Secure Partition (SP)
with another service, message requests may fail with a -EBUSY error.

To handle this, replace the single check and call with a retry loop
that attempts the TPM message send operation until it succeeds or a
configurable timeout is reached. Implement a _try_send_receive function
to do a single send/receive and modify the existing send_receive to
add this retry loop.
The retry mechanism introduces a module parameter (`busy_timeout_ms`,
default: 2000ms) to control how long to keep retrying on -EBUSY
responses. Between retries, the code waits briefly (50-100 microseconds)
to avoid busy-waiting and handling TPM BUSY conditions more gracefully.

The parameter can be modified at run-time as such:
echo 3000 | tee /sys/module/tpm_crb_ffa/parameters/busy_timeout_ms
This changes the timeout from the default 2000ms to 3000ms.

[1] TPM Service Command Response Buffer Interface Over FF-A
https://developer.arm.com/documentation/den0138/latest/

Signed-off-by: Prachotan Bathi <prachotan.bathi@arm.com>
---
 drivers/char/tpm/tpm_crb_ffa.c | 66 +++++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 13 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 089d1e54bb46..4615347795fa 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -10,8 +10,12 @@
 #define pr_fmt(fmt) "CRB_FFA: " fmt
 
 #include <linux/arm_ffa.h>
+#include <linux/delay.h>
+#include <linux/moduleparam.h>
 #include "tpm_crb_ffa.h"
 
+#define memzero(s, n) memset((s), 0, (n))
+
 /* TPM service function status codes */
 #define CRB_FFA_OK			0x05000001
 #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
@@ -178,22 +182,18 @@ int tpm_crb_ffa_init(void)
 }
 EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
 
-static int __tpm_crb_ffa_send_receive(unsigned long func_id,
-				      unsigned long a0,
-				      unsigned long a1,
-				      unsigned long a2)
+static int __tpm_crb_ffa_try_send_receive(unsigned long func_id,
+					  unsigned long a0, unsigned long a1,
+					  unsigned long a2)
 {
 	const struct ffa_msg_ops *msg_ops;
 	int ret;
 
-	if (!tpm_crb_ffa)
-		return -ENOENT;
-
 	msg_ops = tpm_crb_ffa->ffa_dev->ops->msg_ops;
 
 	if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
-		memset(&tpm_crb_ffa->direct_msg_data2, 0x00,
-		       sizeof(struct ffa_send_direct_data2));
+		memzero(&tpm_crb_ffa->direct_msg_data2,
+			sizeof(struct ffa_send_direct_data2));
 
 		tpm_crb_ffa->direct_msg_data2.data[0] = func_id;
 		tpm_crb_ffa->direct_msg_data2.data[1] = a0;
@@ -201,12 +201,12 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
 		tpm_crb_ffa->direct_msg_data2.data[3] = a2;
 
 		ret = msg_ops->sync_send_receive2(tpm_crb_ffa->ffa_dev,
-				&tpm_crb_ffa->direct_msg_data2);
+						&tpm_crb_ffa->direct_msg_data2);
 		if (!ret)
 			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data2.data[0]);
 	} else {
-		memset(&tpm_crb_ffa->direct_msg_data, 0x00,
-		       sizeof(struct ffa_send_direct_data));
+		memzero(&tpm_crb_ffa->direct_msg_data,
+			sizeof(struct ffa_send_direct_data));
 
 		tpm_crb_ffa->direct_msg_data.data1 = func_id;
 		tpm_crb_ffa->direct_msg_data.data2 = a0;
@@ -214,11 +214,51 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
 		tpm_crb_ffa->direct_msg_data.data4 = a2;
 
 		ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
-				&tpm_crb_ffa->direct_msg_data);
+						 &tpm_crb_ffa->direct_msg_data);
 		if (!ret)
 			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
 	}
 
+	return ret;
+}
+
+static unsigned int busy_timeout_ms = 2000;
+/**
+ * busy_timeout_ms - Maximum time to retry before giving up on busy
+ *
+ * This parameter defines the maximum time in milliseconds to retry
+ * sending a message to the TPM service before giving up.
+ */
+module_param(busy_timeout_ms, uint, 0644);
+MODULE_PARM_DESC(busy_timeout_ms,
+		 "Maximum time(in ms) to retry before giving up on busy");
+
+static int __tpm_crb_ffa_send_receive(unsigned long func_id, unsigned long a0,
+				      unsigned long a1, unsigned long a2)
+{
+	ktime_t start, stop;
+	int ret;
+
+	if (!tpm_crb_ffa)
+		return -ENOENT;
+
+	start = ktime_get();
+	stop = ktime_add(start, ms_to_ktime(busy_timeout_ms));
+
+	for (;;) {
+		ret = __tpm_crb_ffa_try_send_receive(func_id, a0, a1, a2);
+
+		if (ret == -EBUSY) {
+			usleep_range(50, 100);
+			if (ktime_after(ktime_get(), stop)) {
+				dev_warn(&tpm_crb_ffa->ffa_dev->dev,
+					 "Busy retry timed out\n");
+				break;
+			}
+		} else {
+			break;
+		}
+	}
 
 	return ret;
 }
-- 
2.43.0

Re: [PATCH v6 1/2] tpm_crb_ffa: Fix typos in function name

[Jarkko Sakkinen]

On Tue, Jun 17, 2025 at 11:05:43AM -0500, Prachotan Bathi wrote:
> s/recieve/receive in __tpm_crb_ffa_send_receive
> 
> Signed-off-by: Prachotan Bathi <prachotan.bathi@arm.com>
> ---
>  drivers/char/tpm/tpm_crb_ffa.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> index 4ead61f01299..089d1e54bb46 100644
> --- a/drivers/char/tpm/tpm_crb_ffa.c
> +++ b/drivers/char/tpm/tpm_crb_ffa.c
> @@ -178,7 +178,7 @@ int tpm_crb_ffa_init(void)
>  }
>  EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
>  
> -static int __tpm_crb_ffa_send_recieve(unsigned long func_id,
> +static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>  				      unsigned long a0,
>  				      unsigned long a1,
>  				      unsigned long a2)
> @@ -251,7 +251,7 @@ int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor)
>  
>  	guard(mutex)(&tpm_crb_ffa->msg_data_lock);
>  
> -	rc = __tpm_crb_ffa_send_recieve(CRB_FFA_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);
> +	rc = __tpm_crb_ffa_send_receive(CRB_FFA_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);
>  	if (!rc) {
>  		if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
>  			*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data2.data[1]);
> @@ -289,7 +289,7 @@ int tpm_crb_ffa_start(int request_type, int locality)
>  
>  	guard(mutex)(&tpm_crb_ffa->msg_data_lock);
>  
> -	return __tpm_crb_ffa_send_recieve(CRB_FFA_START, request_type, locality, 0x00);
> +	return __tpm_crb_ffa_send_receive(CRB_FFA_START, request_type, locality, 0x00);
>  }
>  EXPORT_SYMBOL_GPL(tpm_crb_ffa_start);
>  
> -- 
> 2.43.0
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v6 2/2] tpm_crb_ffa: handle tpm busy return code

[Jarkko Sakkinen]

On Tue, Jun 17, 2025 at 11:05:44AM -0500, Prachotan Bathi wrote:
> Platforms supporting direct message request v2 [1] can support secure
> partitions that support multiple services. For CRB over FF-A interface,
> if the firmware TPM or TPM service [1] shares its Secure Partition (SP)
> with another service, message requests may fail with a -EBUSY error.
> 
> To handle this, replace the single check and call with a retry loop
> that attempts the TPM message send operation until it succeeds or a
> configurable timeout is reached. Implement a _try_send_receive function
> to do a single send/receive and modify the existing send_receive to
> add this retry loop.
> The retry mechanism introduces a module parameter (`busy_timeout_ms`,
> default: 2000ms) to control how long to keep retrying on -EBUSY
> responses. Between retries, the code waits briefly (50-100 microseconds)
> to avoid busy-waiting and handling TPM BUSY conditions more gracefully.
> 
> The parameter can be modified at run-time as such:
> echo 3000 | tee /sys/module/tpm_crb_ffa/parameters/busy_timeout_ms
> This changes the timeout from the default 2000ms to 3000ms.
> 
> [1] TPM Service Command Response Buffer Interface Over FF-A
> https://developer.arm.com/documentation/den0138/latest/
> 
> Signed-off-by: Prachotan Bathi <prachotan.bathi@arm.com>
> ---
>  drivers/char/tpm/tpm_crb_ffa.c | 66 +++++++++++++++++++++++++++-------
>  1 file changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> index 089d1e54bb46..4615347795fa 100644
> --- a/drivers/char/tpm/tpm_crb_ffa.c
> +++ b/drivers/char/tpm/tpm_crb_ffa.c
> @@ -10,8 +10,12 @@
>  #define pr_fmt(fmt) "CRB_FFA: " fmt
>  
>  #include <linux/arm_ffa.h>
> +#include <linux/delay.h>
> +#include <linux/moduleparam.h>
>  #include "tpm_crb_ffa.h"
>  
> +#define memzero(s, n) memset((s), 0, (n))
> +
>  /* TPM service function status codes */
>  #define CRB_FFA_OK			0x05000001
>  #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
> @@ -178,22 +182,18 @@ int tpm_crb_ffa_init(void)
>  }
>  EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
>  
> -static int __tpm_crb_ffa_send_receive(unsigned long func_id,
> -				      unsigned long a0,
> -				      unsigned long a1,
> -				      unsigned long a2)
> +static int __tpm_crb_ffa_try_send_receive(unsigned long func_id,
> +					  unsigned long a0, unsigned long a1,
> +					  unsigned long a2)
>  {
>  	const struct ffa_msg_ops *msg_ops;
>  	int ret;
>  
> -	if (!tpm_crb_ffa)
> -		return -ENOENT;
> -
>  	msg_ops = tpm_crb_ffa->ffa_dev->ops->msg_ops;
>  
>  	if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
> -		memset(&tpm_crb_ffa->direct_msg_data2, 0x00,
> -		       sizeof(struct ffa_send_direct_data2));
> +		memzero(&tpm_crb_ffa->direct_msg_data2,
> +			sizeof(struct ffa_send_direct_data2));
>  
>  		tpm_crb_ffa->direct_msg_data2.data[0] = func_id;
>  		tpm_crb_ffa->direct_msg_data2.data[1] = a0;
> @@ -201,12 +201,12 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>  		tpm_crb_ffa->direct_msg_data2.data[3] = a2;
>  
>  		ret = msg_ops->sync_send_receive2(tpm_crb_ffa->ffa_dev,
> -				&tpm_crb_ffa->direct_msg_data2);
> +						&tpm_crb_ffa->direct_msg_data2);
>  		if (!ret)
>  			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data2.data[0]);
>  	} else {
> -		memset(&tpm_crb_ffa->direct_msg_data, 0x00,
> -		       sizeof(struct ffa_send_direct_data));
> +		memzero(&tpm_crb_ffa->direct_msg_data,
> +			sizeof(struct ffa_send_direct_data));
>  
>  		tpm_crb_ffa->direct_msg_data.data1 = func_id;
>  		tpm_crb_ffa->direct_msg_data.data2 = a0;
> @@ -214,11 +214,51 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>  		tpm_crb_ffa->direct_msg_data.data4 = a2;
>  
>  		ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
> -				&tpm_crb_ffa->direct_msg_data);
> +						 &tpm_crb_ffa->direct_msg_data);
>  		if (!ret)
>  			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
>  	}
>  
> +	return ret;
> +}
> +
> +static unsigned int busy_timeout_ms = 2000;
> +/**
> + * busy_timeout_ms - Maximum time to retry before giving up on busy

nit: s/busy_timeout_ms/busy_timeout_ms()/

> + *
> + * This parameter defines the maximum time in milliseconds to retry
> + * sending a message to the TPM service before giving up.
> + */
> +module_param(busy_timeout_ms, uint, 0644);
> +MODULE_PARM_DESC(busy_timeout_ms,
> +		 "Maximum time(in ms) to retry before giving up on busy");

Patch lacks update to Documentation/admin-guide/kernel-parameters.rst
(and also document that the default value is two seconds).

> +
> +static int __tpm_crb_ffa_send_receive(unsigned long func_id, unsigned long a0,
> +				      unsigned long a1, unsigned long a2)
> +{
> +	ktime_t start, stop;
> +	int ret;
> +
> +	if (!tpm_crb_ffa)
> +		return -ENOENT;
> +
> +	start = ktime_get();
> +	stop = ktime_add(start, ms_to_ktime(busy_timeout_ms));
> +
> +	for (;;) {
> +		ret = __tpm_crb_ffa_try_send_receive(func_id, a0, a1, a2);
> +
> +		if (ret == -EBUSY) {

This loop would be less convoluted if you instead:

	if (ret != -EBUSY)
		break;

	/* ... */

> +			usleep_range(50, 100);

I wonder where this range comes from.

> +			if (ktime_after(ktime_get(), stop)) {
> +				dev_warn(&tpm_crb_ffa->ffa_dev->dev,
> +					 "Busy retry timed out\n");
> +				break;
> +			}
> +		} else {
> +			break;
> +		}
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.43.0
> 

BR, Jarkko

Re: [PATCH v6 2/2] tpm_crb_ffa: handle tpm busy return code

[Prachotan Bathi]

On 6/24/25 6:21 PM, Jarkko Sakkinen wrote:

> On Tue, Jun 17, 2025 at 11:05:44AM -0500, Prachotan Bathi wrote:
>> Platforms supporting direct message request v2 [1] can support secure
>> partitions that support multiple services. For CRB over FF-A interface,
>> if the firmware TPM or TPM service [1] shares its Secure Partition (SP)
>> with another service, message requests may fail with a -EBUSY error.
>>
>> To handle this, replace the single check and call with a retry loop
>> that attempts the TPM message send operation until it succeeds or a
>> configurable timeout is reached. Implement a _try_send_receive function
>> to do a single send/receive and modify the existing send_receive to
>> add this retry loop.
>> The retry mechanism introduces a module parameter (`busy_timeout_ms`,
>> default: 2000ms) to control how long to keep retrying on -EBUSY
>> responses. Between retries, the code waits briefly (50-100 microseconds)
>> to avoid busy-waiting and handling TPM BUSY conditions more gracefully.
>>
>> The parameter can be modified at run-time as such:
>> echo 3000 | tee /sys/module/tpm_crb_ffa/parameters/busy_timeout_ms
>> This changes the timeout from the default 2000ms to 3000ms.
>>
>> [1] TPM Service Command Response Buffer Interface Over FF-A
>> https://developer.arm.com/documentation/den0138/latest/
>>
>> Signed-off-by: Prachotan Bathi <prachotan.bathi@arm.com>
>> ---
>>   drivers/char/tpm/tpm_crb_ffa.c | 66 +++++++++++++++++++++++++++-------
>>   1 file changed, 53 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
>> index 089d1e54bb46..4615347795fa 100644
>> --- a/drivers/char/tpm/tpm_crb_ffa.c
>> +++ b/drivers/char/tpm/tpm_crb_ffa.c
>> @@ -10,8 +10,12 @@
>>   #define pr_fmt(fmt) "CRB_FFA: " fmt
>>   
>>   #include <linux/arm_ffa.h>
>> +#include <linux/delay.h>
>> +#include <linux/moduleparam.h>
>>   #include "tpm_crb_ffa.h"
>>   
>> +#define memzero(s, n) memset((s), 0, (n))
>> +
>>   /* TPM service function status codes */
>>   #define CRB_FFA_OK			0x05000001
>>   #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
>> @@ -178,22 +182,18 @@ int tpm_crb_ffa_init(void)
>>   }
>>   EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
>>   
>> -static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>> -				      unsigned long a0,
>> -				      unsigned long a1,
>> -				      unsigned long a2)
>> +static int __tpm_crb_ffa_try_send_receive(unsigned long func_id,
>> +					  unsigned long a0, unsigned long a1,
>> +					  unsigned long a2)
>>   {
>>   	const struct ffa_msg_ops *msg_ops;
>>   	int ret;
>>   
>> -	if (!tpm_crb_ffa)
>> -		return -ENOENT;
>> -
>>   	msg_ops = tpm_crb_ffa->ffa_dev->ops->msg_ops;
>>   
>>   	if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
>> -		memset(&tpm_crb_ffa->direct_msg_data2, 0x00,
>> -		       sizeof(struct ffa_send_direct_data2));
>> +		memzero(&tpm_crb_ffa->direct_msg_data2,
>> +			sizeof(struct ffa_send_direct_data2));
>>   
>>   		tpm_crb_ffa->direct_msg_data2.data[0] = func_id;
>>   		tpm_crb_ffa->direct_msg_data2.data[1] = a0;
>> @@ -201,12 +201,12 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>>   		tpm_crb_ffa->direct_msg_data2.data[3] = a2;
>>   
>>   		ret = msg_ops->sync_send_receive2(tpm_crb_ffa->ffa_dev,
>> -				&tpm_crb_ffa->direct_msg_data2);
>> +						&tpm_crb_ffa->direct_msg_data2);
>>   		if (!ret)
>>   			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data2.data[0]);
>>   	} else {
>> -		memset(&tpm_crb_ffa->direct_msg_data, 0x00,
>> -		       sizeof(struct ffa_send_direct_data));
>> +		memzero(&tpm_crb_ffa->direct_msg_data,
>> +			sizeof(struct ffa_send_direct_data));
>>   
>>   		tpm_crb_ffa->direct_msg_data.data1 = func_id;
>>   		tpm_crb_ffa->direct_msg_data.data2 = a0;
>> @@ -214,11 +214,51 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>>   		tpm_crb_ffa->direct_msg_data.data4 = a2;
>>   
>>   		ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
>> -				&tpm_crb_ffa->direct_msg_data);
>> +						 &tpm_crb_ffa->direct_msg_data);
>>   		if (!ret)
>>   			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
>>   	}
>>   
>> +	return ret;
>> +}
>> +
>> +static unsigned int busy_timeout_ms = 2000;
>> +/**
>> + * busy_timeout_ms - Maximum time to retry before giving up on busy
> nit: s/busy_timeout_ms/busy_timeout_ms()/
>
>> + *
>> + * This parameter defines the maximum time in milliseconds to retry
>> + * sending a message to the TPM service before giving up.
>> + */
>> +module_param(busy_timeout_ms, uint, 0644);
>> +MODULE_PARM_DESC(busy_timeout_ms,
>> +		 "Maximum time(in ms) to retry before giving up on busy");
> Patch lacks update to Documentation/admin-guide/kernel-parameters.rst
> (and also document that the default value is two seconds).
>
>> +
>> +static int __tpm_crb_ffa_send_receive(unsigned long func_id, unsigned long a0,
>> +				      unsigned long a1, unsigned long a2)
>> +{
>> +	ktime_t start, stop;
>> +	int ret;
>> +
>> +	if (!tpm_crb_ffa)
>> +		return -ENOENT;
>> +
>> +	start = ktime_get();
>> +	stop = ktime_add(start, ms_to_ktime(busy_timeout_ms));
>> +
>> +	for (;;) {
>> +		ret = __tpm_crb_ffa_try_send_receive(func_id, a0, a1, a2);
>> +
>> +		if (ret == -EBUSY) {
> This loop would be less convoluted if you instead:
>
> 	if (ret != -EBUSY)
> 		break;
>
> 	/* ... */
>
>> +			usleep_range(50, 100);
> I wonder where this range comes from.
This range comes from a similar timeout defined in the 
crb_wait_for_reg_32 function.
See:

https://github.com/torvalds/linux/blob/c4dce0c094a89b1bc8fde1163342bd6fe29c0370/drivers/char/tpm/tpm_crb.c#L153

A TPM Service might have a discrete TPM (dtpm) that it communicates 
with, a busy SP might become available again and be able to proxy 
commands to a dtpm within a similar retry window. This window works well 
with current internal implementations and can be changed as future 
implementations and specifications evolve and define a more 
sophisticated retry window.

>> +			if (ktime_after(ktime_get(), stop)) {
>> +				dev_warn(&tpm_crb_ffa->ffa_dev->dev,
>> +					 "Busy retry timed out\n");
>> +				break;
>> +			}
>> +		} else {
>> +			break;
>> +		}
>> +	}
>>   
>>   	return ret;
>>   }
>> -- 
>> 2.43.0
>>
> BR, Jarkko


On 6/24/25 6:21 PM, Jarkko Sakkinen wrote:
> On Tue, Jun 17, 2025 at 11:05:44AM -0500, Prachotan Bathi wrote:
>> Platforms supporting direct message request v2 [1] can support secure
>> partitions that support multiple services. For CRB over FF-A interface,
>> if the firmware TPM or TPM service [1] shares its Secure Partition (SP)
>> with another service, message requests may fail with a -EBUSY error.
>>
>> To handle this, replace the single check and call with a retry loop
>> that attempts the TPM message send operation until it succeeds or a
>> configurable timeout is reached. Implement a _try_send_receive function
>> to do a single send/receive and modify the existing send_receive to
>> add this retry loop.
>> The retry mechanism introduces a module parameter (`busy_timeout_ms`,
>> default: 2000ms) to control how long to keep retrying on -EBUSY
>> responses. Between retries, the code waits briefly (50-100 microseconds)
>> to avoid busy-waiting and handling TPM BUSY conditions more gracefully.
>>
>> The parameter can be modified at run-time as such:
>> echo 3000 | tee /sys/module/tpm_crb_ffa/parameters/busy_timeout_ms
>> This changes the timeout from the default 2000ms to 3000ms.
>>
>> [1] TPM Service Command Response Buffer Interface Over FF-A
>> https://developer.arm.com/documentation/den0138/latest/
>>
>> Signed-off-by: Prachotan Bathi <prachotan.bathi@arm.com>
>> ---
>>   drivers/char/tpm/tpm_crb_ffa.c | 66 +++++++++++++++++++++++++++-------
>>   1 file changed, 53 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
>> index 089d1e54bb46..4615347795fa 100644
>> --- a/drivers/char/tpm/tpm_crb_ffa.c
>> +++ b/drivers/char/tpm/tpm_crb_ffa.c
>> @@ -10,8 +10,12 @@
>>   #define pr_fmt(fmt) "CRB_FFA: " fmt
>>   
>>   #include <linux/arm_ffa.h>
>> +#include <linux/delay.h>
>> +#include <linux/moduleparam.h>
>>   #include "tpm_crb_ffa.h"
>>   
>> +#define memzero(s, n) memset((s), 0, (n))
>> +
>>   /* TPM service function status codes */
>>   #define CRB_FFA_OK			0x05000001
>>   #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
>> @@ -178,22 +182,18 @@ int tpm_crb_ffa_init(void)
>>   }
>>   EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
>>   
>> -static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>> -				      unsigned long a0,
>> -				      unsigned long a1,
>> -				      unsigned long a2)
>> +static int __tpm_crb_ffa_try_send_receive(unsigned long func_id,
>> +					  unsigned long a0, unsigned long a1,
>> +					  unsigned long a2)
>>   {
>>   	const struct ffa_msg_ops *msg_ops;
>>   	int ret;
>>   
>> -	if (!tpm_crb_ffa)
>> -		return -ENOENT;
>> -
>>   	msg_ops = tpm_crb_ffa->ffa_dev->ops->msg_ops;
>>   
>>   	if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
>> -		memset(&tpm_crb_ffa->direct_msg_data2, 0x00,
>> -		       sizeof(struct ffa_send_direct_data2));
>> +		memzero(&tpm_crb_ffa->direct_msg_data2,
>> +			sizeof(struct ffa_send_direct_data2));
>>   
>>   		tpm_crb_ffa->direct_msg_data2.data[0] = func_id;
>>   		tpm_crb_ffa->direct_msg_data2.data[1] = a0;
>> @@ -201,12 +201,12 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>>   		tpm_crb_ffa->direct_msg_data2.data[3] = a2;
>>   
>>   		ret = msg_ops->sync_send_receive2(tpm_crb_ffa->ffa_dev,
>> -				&tpm_crb_ffa->direct_msg_data2);
>> +						&tpm_crb_ffa->direct_msg_data2);
>>   		if (!ret)
>>   			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data2.data[0]);
>>   	} else {
>> -		memset(&tpm_crb_ffa->direct_msg_data, 0x00,
>> -		       sizeof(struct ffa_send_direct_data));
>> +		memzero(&tpm_crb_ffa->direct_msg_data,
>> +			sizeof(struct ffa_send_direct_data));
>>   
>>   		tpm_crb_ffa->direct_msg_data.data1 = func_id;
>>   		tpm_crb_ffa->direct_msg_data.data2 = a0;
>> @@ -214,11 +214,51 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>>   		tpm_crb_ffa->direct_msg_data.data4 = a2;
>>   
>>   		ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
>> -				&tpm_crb_ffa->direct_msg_data);
>> +						 &tpm_crb_ffa->direct_msg_data);
>>   		if (!ret)
>>   			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
>>   	}
>>   
>> +	return ret;
>> +}
>> +
>> +static unsigned int busy_timeout_ms = 2000;
>> +/**
>> + * busy_timeout_ms - Maximum time to retry before giving up on busy
> nit: s/busy_timeout_ms/busy_timeout_ms()/
>
>> + *
>> + * This parameter defines the maximum time in milliseconds to retry
>> + * sending a message to the TPM service before giving up.
>> + */
>> +module_param(busy_timeout_ms, uint, 0644);
>> +MODULE_PARM_DESC(busy_timeout_ms,
>> +		 "Maximum time(in ms) to retry before giving up on busy");
> Patch lacks update to Documentation/admin-guide/kernel-parameters.rst
> (and also document that the default value is two seconds).
>
>> +
>> +static int __tpm_crb_ffa_send_receive(unsigned long func_id, unsigned long a0,
>> +				      unsigned long a1, unsigned long a2)
>> +{
>> +	ktime_t start, stop;
>> +	int ret;
>> +
>> +	if (!tpm_crb_ffa)
>> +		return -ENOENT;
>> +
>> +	start = ktime_get();
>> +	stop = ktime_add(start, ms_to_ktime(busy_timeout_ms));
>> +
>> +	for (;;) {
>> +		ret = __tpm_crb_ffa_try_send_receive(func_id, a0, a1, a2);
>> +
>> +		if (ret == -EBUSY) {
> This loop would be less convoluted if you instead:
>
> 	if (ret != -EBUSY)
> 		break;
>
> 	/* ... */
>
>> +			usleep_range(50, 100);
> I wonder where this range comes from.
>
>> +			if (ktime_after(ktime_get(), stop)) {
>> +				dev_warn(&tpm_crb_ffa->ffa_dev->dev,
>> +					 "Busy retry timed out\n");
>> +				break;
>> +			}
>> +		} else {
>> +			break;
>> +		}
>> +	}
>>   
>>   	return ret;
>>   }
>> -- 
>> 2.43.0
>>
> BR, Jarkko