linux-kernel - Re: [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4113b3de-918c-4140-9f0d-d1699d2efc32@arm.com>
Date: Sun, 20 Jul 2025 18:05:59 -0500
From: Prachotan Bathi <prachotan.bathi@....com>
To: Jarkko Sakkinen <jarkko@...nel.org>
Cc: Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>,
 Stuart Yoder <stuart.yoder@....com>, linux-integrity@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 3/3] tpm_crb_ffa: handle tpm busy return code

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