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