Hi Nick,
The patch set is working for me. Could you please integrate them into LTS?
Regards,
Brian
From: Nicola Mazzucato <Nicola.Mazzucato@arm.com>
Sent: Tuesday, December 3, 2024 2:04 AM
To: Quach, Brian <brian@ti.com>; tf-m@lists.trustedfirmware.org; nd <nd@arm.com>
Subject: [EXTERNAL] Re: connection-based MMIOVEC
Thanks Brian, the patches I shared have been tested with a on-purpose modified Crypto interface and partition in order to handle stateful connections. The interface would connect and then psa-call more than once, then finally close. From my
ZjQcmQRYFpfptBannerStart
|
ZjQcmQRYFpfptBannerEnd
Thanks Brian,
the patches I shared have been tested with a on-purpose modified Crypto interface and partition in order to handle stateful connections. The interface would connect and then psa-call more than once, then finally close.
From my tests now it should be fixed.
For stateless services the issue won't be noticed since the connection is handled by SPM behind the scenes.
Please note that the explicit unmapping of invecs is done for both Crypto and Attestation.
Hope that helps.
Best regards,
Nick
From: Quach, Brian <brian@ti.com>
Sent: 03 December 2024 00:33
To: Nicola Mazzucato <Nicola.Mazzucato@arm.com>;
tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.org>; nd <nd@arm.com>
Subject: RE: connection-based MMIOVEC
Hi Nick,
The patches for attestation and crypto_init look good so I’ll test those soon. But check the response I just sent. If the underlying code of these services is strictly intended to be state-less only, then the existing code is okay as-is but a compile error should be raised if they are ever built for a connection-based service. But I think it’s preferable to explicitly unmap the input vectors so it could work for both stateless and connection-based.
Regards,
Brian
From: Nicola Mazzucato <Nicola.Mazzucato@arm.com>
Sent: Monday, December 2, 2024 3:31 PM
To: Quach, Brian <brian@ti.com>;
tf-m@lists.trustedfirmware.org; nd <nd@arm.com>
Subject: [EXTERNAL] Re: connection-based MMIOVEC
Hi Brian, please find here a set of patches to address the issue you found: https: //review. trustedfirmware. org/q/topic: %22mm-invec-unmap-crypto-att%22 Would appreciate if you can test those in your environment and let us know the results and/or
ZjQcmQRYFpfptBannerStart
|
ZjQcmQRYFpfptBannerEnd
Hi Brian,
please find here a set of patches to address the issue you found: https://review.trustedfirmware.org/q/topic:%22mm-invec-unmap-crypto-att%22
Would appreciate if you can test those in your environment and let us know the results and/or further suggestions.
In addition, please let us know if you wish to integrate the above fixes in the LTS branch.
Everybody else is, as always, more than welcome to provide suggestions.
Many thanks in advance
Best regards,
Nick
From: Nicola Mazzucato via TF-M <tf-m@lists.trustedfirmware.org>
Sent: 27 November 2024 15:35
To: Quach, Brian <brian@ti.com>;
tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.org>; nd <nd@arm.com>
Subject: [TF-M] Re: connection-based MMIOVEC
Hi Brian,
many thanks for your reply.
I managed to do a quick test and what you noticed makes sense to me.
It looks however that the current crypto service does not support connection-based operations, so my tests were limited to stateless calls so far.
Are you based on a different branch from mainline? Do you use a different crypto interface from the one available upstream?
Best regards,
Thanks
Nick
From: Quach, Brian <brian@ti.com>
Sent: 21 November 2024 19:50
To: Nicola Mazzucato <Nicola.Mazzucato@arm.com>;
tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.org>; nd <nd@arm.com>
Subject: RE: connection-based MMIOVEC
Hi Nick,
I’m using medium profile but with PSA_FRAMEWORK_HAS_MM_IOVEC enabled.
In the partition yaml file:
"model": "IPC",
"services" : [
"connection_based": true,
"mm_iovec": "enable"
]
After psa_connect, the second psa_call using MM IOVECS to the partition should fail.
It’s fairly easy to look at the code snippets I sent and see the input vectors are not being unmapped. Was this done on purpose or was it overlooked? I would think if the outputs are being unmapped, that inputs should be as well.
Regards,
Brian
From: Nicola Mazzucato <Nicola.Mazzucato@arm.com>
Sent: Thursday, November 21, 2024 6:37 AM
To: tf-m@lists.trustedfirmware.org; nd <nd@arm.com>
Cc: Quach, Brian <brian@ti.com>
Subject: [EXTERNAL] Re: connection-based MMIOVEC
Hi Brian, many thanks for reporting this. I shall take a look and attempt to reproduce it and will share my findings. In the meantime, could you please share your build commands? Many thanks Best regards, Nick From: Quach, Brian via TF-M <tf-m@ lists. trustedfirmware. org>
ZjQcmQRYFpfptBannerStart
|
ZjQcmQRYFpfptBannerEnd
Hi Brian,
many thanks for reporting this.
I shall take a look and attempt to reproduce it and will share my findings.
In the meantime, could you please share your build commands?
Many thanks
Best regards,
Nick
From: Quach, Brian via TF-M <tf-m@lists.trustedfirmware.org>
Sent: 20 November 2024 22:49
To: tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.org>
Subject: [TF-M] connection-based MMIOVEC
Hi,
I am trying to use connection-based (instead of SFN) Crypto service with MMIOVEC, but I noticed that iovec_status is not automatically cleared so the second psa_call causes a TFM panic due to detection of a previously mapped IOVEC.
FF spec says:
In tfm_crypto_call_srv(), I see the output vecs are unmapped but not the input vectors. Should the inputs be unmapped here as well? (psa_attest_get_token also does not unmap input vecs)
status = tfm_crypto_init_iovecs(msg, in_vec, in_len, out_vec, out_len);
if (status != PSA_SUCCESS) {
return status;
}
tfm_crypto_set_caller_id(msg->client_id);
/* Call the dispatcher to the functions that implement the PSA Crypto API */
status = tfm_crypto_api_dispatcher(in_vec, in_len, out_vec, out_len);
#if PSA_FRAMEWORK_HAS_MM_IOVEC == 1
for (i = 0; i < out_len; i++) {
if (out_vec[i].base != NULL) {
psa_unmap_outvec(msg->handle, i, out_vec[i].len);
}
}
#else
/* Write into the IPC framework outputs from the scratch */
for (i = 0; i < out_len; i++) {
psa_write(msg->handle, i, out_vec[i].base, out_vec[i].len);
}
Currently, to clear the connection’s iovec_status, I have to call psa_close() after every psa_call() and reconnect with psa_connect() before the next psa_call() which adds overhead. Is this behavior of not automatically clearing iovec_status for connection-based services by design?
As an alternative to calling psa_unmap_invecs(), I was wondering if spm_get_connection()/spm_get_idle_connection() could be modified with the following workaround to avoid needing to close/connect:
#if CONFIG_TFM_CONNECTION_BASED_SERVICE_API == 1
connection = handle_to_connection(handle);
if (!connection) {
return PSA_ERROR_PROGRAMMER_ERROR;
}
if (spm_validate_connection(connection) != PSA_SUCCESS) {
return PSA_ERROR_PROGRAMMER_ERROR;
}
/* WORKAROUND */
#if PSA_FRAMEWORK_HAS_MM_IOVEC
connection->iovec_status &= 0xFFFF0000; // Clear status of input vecs
#endif
/* Validate the caller id in the connection handle equals client_id. */
if (connection->msg.client_id != client_id) {
return PSA_ERROR_PROGRAMMER_ERROR;
}
/*
* It is a PROGRAMMER ERROR if the connection is currently
* handling a request.
*/
if (connection->status != TFM_HANDLE_STATUS_IDLE) {
return PSA_ERROR_PROGRAMMER_ERROR;
}
if (!(connection->service)) {
/* FixMe: Need to implement a mechanism to resolve this failure. */
return PSA_ERROR_PROGRAMMER_ERROR;
}
#else
Regards,
Brian Quach
SimpleLink MCU
Texas Instruments Inc.
12500 TI Blvd, MS F-4000
Dallas, TX 75243
214-479-4076