linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/3] tpm_crb_ffa: handle tpm busy return code
@ 2025-07-08 22:51 Prachotan Bathi
  2025-07-08 22:51 ` [PATCH v9 1/3] tpm_crb_ffa: Fix typos in function name Prachotan Bathi
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Prachotan Bathi @ 2025-07-08 22:51 UTC (permalink / raw)
  To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, Stuart Yoder
  Cc: linux-integrity, linux-kernel, 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 v9:
- Removed memset usage, introduced designated initialization for
  `tpm_crb_ffa_data` structure.
- Code formatting changes for consistency.

Prachotan Bathi (3):
  tpm_crb_ffa: Fix typos in function name
  tpm_crb_ffa:Remove memset usage
  tpm_crb_ffa: handle tpm busy return code

 .../admin-guide/kernel-parameters.txt         |  8 +++
 drivers/char/tpm/tpm_crb_ffa.c                | 72 +++++++++++++------
 2 files changed, 57 insertions(+), 23 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v9 1/3] tpm_crb_ffa: Fix typos in function name
  2025-07-08 22:51 [PATCH v9 0/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
@ 2025-07-08 22:51 ` Prachotan Bathi
  2025-07-08 22:51 ` [PATCH v9 2/3] tpm_crb_ffa:Remove memset usage Prachotan Bathi
  2025-07-08 22:51 ` [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
  2 siblings, 0 replies; 6+ messages in thread
From: Prachotan Bathi @ 2025-07-08 22:51 UTC (permalink / raw)
  To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, Stuart Yoder
  Cc: linux-integrity, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v9 2/3] tpm_crb_ffa:Remove memset usage
  2025-07-08 22:51 [PATCH v9 0/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
  2025-07-08 22:51 ` [PATCH v9 1/3] tpm_crb_ffa: Fix typos in function name Prachotan Bathi
@ 2025-07-08 22:51 ` Prachotan Bathi
  2025-07-08 22:51 ` [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
  2 siblings, 0 replies; 6+ messages in thread
From: Prachotan Bathi @ 2025-07-08 22:51 UTC (permalink / raw)
  To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, Stuart Yoder
  Cc: linux-integrity, linux-kernel, Prachotan Bathi

Simplify initialization of `ffa_send_direct_data2` and
`ffa_send_direct_data` structures by using designated
initializers instead of `memset()` followed by field
assignments, reducing code size and improving readability.

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

diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 089d1e54bb46..7faed6f3bf66 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -192,26 +192,21 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
 	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));
-
-		tpm_crb_ffa->direct_msg_data2.data[0] = func_id;
-		tpm_crb_ffa->direct_msg_data2.data[1] = a0;
-		tpm_crb_ffa->direct_msg_data2.data[2] = a1;
-		tpm_crb_ffa->direct_msg_data2.data[3] = a2;
+		tpm_crb_ffa->direct_msg_data2 = (struct ffa_send_direct_data2){
+			.data = { func_id, a0, a1, a2 },
+		};
 
 		ret = msg_ops->sync_send_receive2(tpm_crb_ffa->ffa_dev,
 				&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));
-
-		tpm_crb_ffa->direct_msg_data.data1 = func_id;
-		tpm_crb_ffa->direct_msg_data.data2 = a0;
-		tpm_crb_ffa->direct_msg_data.data3 = a1;
-		tpm_crb_ffa->direct_msg_data.data4 = a2;
+		tpm_crb_ffa->direct_msg_data = (struct ffa_send_direct_data){
+			.data1 = func_id,
+			.data2 = a0,
+			.data3 = a1,
+			.data4 = a2,
+		};
 
 		ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
 				&tpm_crb_ffa->direct_msg_data);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code
  2025-07-08 22:51 [PATCH v9 0/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
  2025-07-08 22:51 ` [PATCH v9 1/3] tpm_crb_ffa: Fix typos in function name Prachotan Bathi
  2025-07-08 22:51 ` [PATCH v9 2/3] tpm_crb_ffa:Remove memset usage Prachotan Bathi
@ 2025-07-08 22:51 ` Prachotan Bathi
  2025-07-19 13:58   ` Jarkko Sakkinen
  2 siblings, 1 reply; 6+ messages in thread
From: Prachotan Bathi @ 2025-07-08 22:51 UTC (permalink / raw)
  To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, Stuart Yoder
  Cc: linux-integrity, linux-kernel, 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>
---
 .../admin-guide/kernel-parameters.txt         |  8 ++++
 drivers/char/tpm/tpm_crb_ffa.c                | 45 ++++++++++++++++---
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 07e22ba5bfe3..343377538fe9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -7214,6 +7214,14 @@
 			causing a major performance hit, and the space where
 			machines are deployed is by other means guarded.
 
+	tpm_crb_ffa.busy_timeout_ms= [ARM64,TPM]
+			Maximum time in milliseconds to retry sending a message
+			to the TPM service before giving up. This parameter controls
+			how long the system will continue retrying when the TPM
+			service is busy.
+			Format: <unsigned int>
+			Default: 2000 (2 seconds)
+
 	tpm_suspend_pcr=[HW,TPM]
 			Format: integer pcr id
 			Specify that at suspend time, the tpm driver
diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 7faed6f3bf66..06599c07b746 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -10,8 +10,16 @@
 #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"
 
+static unsigned int busy_timeout_ms = 2000;
+
+module_param(busy_timeout_ms, uint, 0644);
+MODULE_PARM_DESC(busy_timeout_ms,
+		 "Maximum time in ms to retry before giving up on busy");
+
 /* TPM service function status codes */
 #define CRB_FFA_OK			0x05000001
 #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
@@ -178,17 +186,13 @@ 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)) {
@@ -214,6 +218,33 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
 			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
 	}
 
+	return ret;
+}
+
+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)
+			break;
+
+		usleep_range(50, 100);
+		if (ktime_after(ktime_get(), stop)) {
+			dev_warn(&tpm_crb_ffa->ffa_dev->dev,
+				 "Busy retry timed out\n");
+			break;
+		}
+	}
 
 	return ret;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code
  2025-07-08 22:51 ` [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
@ 2025-07-19 13:58   ` Jarkko Sakkinen
  2025-07-20 23:05     ` Prachotan Bathi
  0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-07-19 13:58 UTC (permalink / raw)
  To: Prachotan Bathi
  Cc: Peter Huewe, Jason Gunthorpe, Stuart Yoder, linux-integrity,
	linux-kernel

On Tue, Jul 08, 2025 at 05:51:51PM -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>
> ---
>  .../admin-guide/kernel-parameters.txt         |  8 ++++
>  drivers/char/tpm/tpm_crb_ffa.c                | 45 ++++++++++++++++---
>  2 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 07e22ba5bfe3..343377538fe9 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7214,6 +7214,14 @@
>  			causing a major performance hit, and the space where
>  			machines are deployed is by other means guarded.
>  
> +	tpm_crb_ffa.busy_timeout_ms= [ARM64,TPM]
> +			Maximum time in milliseconds to retry sending a message
> +			to the TPM service before giving up. This parameter controls
> +			how long the system will continue retrying when the TPM
> +			service is busy.
> +			Format: <unsigned int>
> +			Default: 2000 (2 seconds)
> +
>  	tpm_suspend_pcr=[HW,TPM]
>  			Format: integer pcr id
>  			Specify that at suspend time, the tpm driver
> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> index 7faed6f3bf66..06599c07b746 100644
> --- a/drivers/char/tpm/tpm_crb_ffa.c
> +++ b/drivers/char/tpm/tpm_crb_ffa.c
> @@ -10,8 +10,16 @@
>  #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"
>  
> +static unsigned int busy_timeout_ms = 2000;
> +
> +module_param(busy_timeout_ms, uint, 0644);
> +MODULE_PARM_DESC(busy_timeout_ms,
> +		 "Maximum time in ms to retry before giving up on busy");
> +
>  /* TPM service function status codes */
>  #define CRB_FFA_OK			0x05000001
>  #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
> @@ -178,17 +186,13 @@ 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)) {
> @@ -214,6 +218,33 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>  			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
>  	}
>  
> +	return ret;
> +}
> +
> +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)
> +			break;
> +
> +		usleep_range(50, 100);
> +		if (ktime_after(ktime_get(), stop)) {
> +			dev_warn(&tpm_crb_ffa->ffa_dev->dev,
> +				 "Busy retry timed out\n");
> +			break;
> +		}
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.43.0
> 

I think these patches look totally fine now. If you don't mind I'll add
suggested-by to 2/3?

Anyhow, I can pick these to my tree. Just coming from holiday but
they'll end up the 6.17 PR.

BR, Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code
  2025-07-19 13:58   ` Jarkko Sakkinen
@ 2025-07-20 23:05     ` Prachotan Bathi
  0 siblings, 0 replies; 6+ messages in thread
From: Prachotan Bathi @ 2025-07-20 23:05 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Jason Gunthorpe, Stuart Yoder, linux-integrity,
	linux-kernel

On 7/19/25 8:58 AM, Jarkko Sakkinen wrote:

> On Tue, Jul 08, 2025 at 05:51:51PM -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>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  8 ++++
>>   drivers/char/tpm/tpm_crb_ffa.c                | 45 ++++++++++++++++---
>>   2 files changed, 46 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 07e22ba5bfe3..343377538fe9 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -7214,6 +7214,14 @@
>>   			causing a major performance hit, and the space where
>>   			machines are deployed is by other means guarded.
>>   
>> +	tpm_crb_ffa.busy_timeout_ms= [ARM64,TPM]
>> +			Maximum time in milliseconds to retry sending a message
>> +			to the TPM service before giving up. This parameter controls
>> +			how long the system will continue retrying when the TPM
>> +			service is busy.
>> +			Format: <unsigned int>
>> +			Default: 2000 (2 seconds)
>> +
>>   	tpm_suspend_pcr=[HW,TPM]
>>   			Format: integer pcr id
>>   			Specify that at suspend time, the tpm driver
>> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
>> index 7faed6f3bf66..06599c07b746 100644
>> --- a/drivers/char/tpm/tpm_crb_ffa.c
>> +++ b/drivers/char/tpm/tpm_crb_ffa.c
>> @@ -10,8 +10,16 @@
>>   #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"
>>   
>> +static unsigned int busy_timeout_ms = 2000;
>> +
>> +module_param(busy_timeout_ms, uint, 0644);
>> +MODULE_PARM_DESC(busy_timeout_ms,
>> +		 "Maximum time in ms to retry before giving up on busy");
>> +
>>   /* TPM service function status codes */
>>   #define CRB_FFA_OK			0x05000001
>>   #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
>> @@ -178,17 +186,13 @@ 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)) {
>> @@ -214,6 +218,33 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>>   			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
>>   	}
>>   
>> +	return ret;
>> +}
>> +
>> +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)
>> +			break;
>> +
>> +		usleep_range(50, 100);
>> +		if (ktime_after(ktime_get(), stop)) {
>> +			dev_warn(&tpm_crb_ffa->ffa_dev->dev,
>> +				 "Busy retry timed out\n");
>> +			break;
>> +		}
>> +	}
>>   
>>   	return ret;
>>   }
>> -- 
>> 2.43.0
>>
> I think these patches look totally fine now. If you don't mind I'll add
> suggested-by to 2/3?
>
> Anyhow, I can pick these to my tree. Just coming from holiday but
> they'll end up the 6.17 PR.
>
> BR, Jarkko

Yes, of course, thanks for catching these and suggesting the needed changes.

Best

Prachotan.

On 7/19/25 8:58 AM, Jarkko Sakkinen wrote:
> On Tue, Jul 08, 2025 at 05:51:51PM -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>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  8 ++++
>>   drivers/char/tpm/tpm_crb_ffa.c                | 45 ++++++++++++++++---
>>   2 files changed, 46 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 07e22ba5bfe3..343377538fe9 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -7214,6 +7214,14 @@
>>   			causing a major performance hit, and the space where
>>   			machines are deployed is by other means guarded.
>>   
>> +	tpm_crb_ffa.busy_timeout_ms= [ARM64,TPM]
>> +			Maximum time in milliseconds to retry sending a message
>> +			to the TPM service before giving up. This parameter controls
>> +			how long the system will continue retrying when the TPM
>> +			service is busy.
>> +			Format: <unsigned int>
>> +			Default: 2000 (2 seconds)
>> +
>>   	tpm_suspend_pcr=[HW,TPM]
>>   			Format: integer pcr id
>>   			Specify that at suspend time, the tpm driver
>> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
>> index 7faed6f3bf66..06599c07b746 100644
>> --- a/drivers/char/tpm/tpm_crb_ffa.c
>> +++ b/drivers/char/tpm/tpm_crb_ffa.c
>> @@ -10,8 +10,16 @@
>>   #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"
>>   
>> +static unsigned int busy_timeout_ms = 2000;
>> +
>> +module_param(busy_timeout_ms, uint, 0644);
>> +MODULE_PARM_DESC(busy_timeout_ms,
>> +		 "Maximum time in ms to retry before giving up on busy");
>> +
>>   /* TPM service function status codes */
>>   #define CRB_FFA_OK			0x05000001
>>   #define CRB_FFA_OK_RESULTS_RETURNED	0x05000002
>> @@ -178,17 +186,13 @@ 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)) {
>> @@ -214,6 +218,33 @@ static int __tpm_crb_ffa_send_receive(unsigned long func_id,
>>   			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
>>   	}
>>   
>> +	return ret;
>> +}
>> +
>> +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)
>> +			break;
>> +
>> +		usleep_range(50, 100);
>> +		if (ktime_after(ktime_get(), stop)) {
>> +			dev_warn(&tpm_crb_ffa->ffa_dev->dev,
>> +				 "Busy retry timed out\n");
>> +			break;
>> +		}
>> +	}
>>   
>>   	return ret;
>>   }
>> -- 
>> 2.43.0
>>
> I think these patches look totally fine now. If you don't mind I'll add
> suggested-by to 2/3?
>
> Anyhow, I can pick these to my tree. Just coming from holiday but
> they'll end up the 6.17 PR.
>
> BR, Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-20 23:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-08 22:51 [PATCH v9 0/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
2025-07-08 22:51 ` [PATCH v9 1/3] tpm_crb_ffa: Fix typos in function name Prachotan Bathi
2025-07-08 22:51 ` [PATCH v9 2/3] tpm_crb_ffa:Remove memset usage Prachotan Bathi
2025-07-08 22:51 ` [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
2025-07-19 13:58   ` Jarkko Sakkinen
2025-07-20 23:05     ` Prachotan Bathi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).