On 6/24/25 6:21 PM, Jarkko Sakkinen wrote:
This range comes from a similar timeout defined in the crb_wait_for_reg_32 function.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.ba...@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 busynit: 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.
See: https://github.com/torvalds/linux/blob/c4dce0c094a89b1bc8fde1163342bd6fe29c0370/drivers/char/tpm/tpm_crb.c#L153A 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.0BR, 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.ba...@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 busynit: 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.0BR, Jarkko
