linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/2] tpm_crb_ffa: handle tpm busy return code
@ 2025-06-25 19:19 Prachotan Bathi
  2025-06-25 19:19 ` [PATCH v7 1/2] tpm_crb_ffa: Fix typos in function name Prachotan Bathi
  2025-06-25 19:19 ` [PATCH v7 2/2] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
  0 siblings, 2 replies; 4+ messages in thread
From: Prachotan Bathi @ 2025-06-25 19:19 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 v7:
- Updated Documentation/admin-guide/kernel-parameters with busy_timeout_ms.
- Used a less convoluted break condition to exit the retry loop.

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

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

-- 
2.43.0


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

* [PATCH v7 1/2] tpm_crb_ffa: Fix typos in function name
  2025-06-25 19:19 [PATCH v7 0/2] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
@ 2025-06-25 19:19 ` Prachotan Bathi
  2025-06-25 19:19 ` [PATCH v7 2/2] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
  1 sibling, 0 replies; 4+ messages in thread
From: Prachotan Bathi @ 2025-06-25 19:19 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] 4+ messages in thread

* [PATCH v7 2/2] tpm_crb_ffa: handle tpm busy return code
  2025-06-25 19:19 [PATCH v7 0/2] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
  2025-06-25 19:19 ` [PATCH v7 1/2] tpm_crb_ffa: Fix typos in function name Prachotan Bathi
@ 2025-06-25 19:19 ` Prachotan Bathi
  2025-06-26 10:57   ` Jarkko Sakkinen
  1 sibling, 1 reply; 4+ messages in thread
From: Prachotan Bathi @ 2025-06-25 19:19 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                | 65 +++++++++++++++----
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f1f2c0874da9..5cb5dbfd761f 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 089d1e54bb46..1a51c966e42c 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,50 @@ 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)
+			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] 4+ messages in thread

* Re: [PATCH v7 2/2] tpm_crb_ffa: handle tpm busy return code
  2025-06-25 19:19 ` [PATCH v7 2/2] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
@ 2025-06-26 10:57   ` Jarkko Sakkinen
  0 siblings, 0 replies; 4+ messages in thread
From: Jarkko Sakkinen @ 2025-06-26 10:57 UTC (permalink / raw)
  To: Prachotan Bathi
  Cc: Peter Huewe, Jason Gunthorpe, Stuart Yoder, linux-integrity,
	linux-kernel

On Wed, Jun 25, 2025 at 02:19:43PM -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                | 65 +++++++++++++++----
>  2 files changed, 60 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f1f2c0874da9..5cb5dbfd761f 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 089d1e54bb46..1a51c966e42c 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))

This is a spurious change i.e., out of scope of the topic.

If you wanted to do this, you'd need to have a separate patch for it.

Otherwise, stick to memset(). This is the "split to logical changes"
part of SubmittingPatches document.

> +
>  /* 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,50 @@ 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.

I'd keep this single line and save the effort to kernel-parameters.rst.
Otherwise, this will be pain to maintain.

> + */
> +module_param(busy_timeout_ms, uint, 0644);
> +MODULE_PARM_DESC(busy_timeout_ms,
> +		 "Maximum time(in ms) to retry before giving up on busy");

"Maximum time (in ms)", or rather "Maximum time in ms" because you
really need to know the unit every single time you use the parameter.

> +
> +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
> 

BR, Jarkko

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

end of thread, other threads:[~2025-06-26 10:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-25 19:19 [PATCH v7 0/2] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
2025-06-25 19:19 ` [PATCH v7 1/2] tpm_crb_ffa: Fix typos in function name Prachotan Bathi
2025-06-25 19:19 ` [PATCH v7 2/2] tpm_crb_ffa: handle tpm busy return code Prachotan Bathi
2025-06-26 10:57   ` Jarkko Sakkinen

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).