Support for slots to handles crypto API changes under SPM #16
Conversation
There was a problem hiding this comment.
The code comment added seems better suited as a commit message. Consider moving it into the commit message description, or having it in both places, whichever you think is best. We do need something in the commit message description, though, to justify why this change is necessary and give some context to it.
library/psa_crypto_slot_management.c
Outdated
| * In case MBEDTLS_PSA_CRYPTO_SPM is defined the code is built for SPM (Secure | ||
| * Partition Manager) integration which separate the code into two parts | ||
| * NSPE (Non-Secure Process Environment) and SPE (Secure Process Environment). | ||
| * In this mode an additional header file should be included. |
There was a problem hiding this comment.
Could we change this to the following?
When MBEDTLS_PSA_CRYPTO_SPM is defined, the code is being build for SPM (Secure Partition Manager) integration which separates the code into two parts: NSPE (Non-Secure Processing Element) and SPE (Secure Processing Element). When building for the SPE, an additional header file should be included.
There was a problem hiding this comment.
I see this still says "Environment" instead of "Element".
There was a problem hiding this comment.
I don't see where “environment” should be changed to “element”. (N)SPE is (Non-)Secure Processing Environment.
There was a problem hiding this comment.
The docs I've seen refer to SPE as Secure Processing Environment. As Gilles approved this I'm leaving it as is for now, please let me know if you still want the change.
There was a problem hiding this comment.
OK, I've been confusing two different terms. PE is processing element. SPE is not secure processing element, but whatever PSA says it is, which is Environment as you've written. In ARMv8-M PE's with the security extension, the "secure world" is called the "secure state".
library/psa_crypto_slot_management.c
Outdated
| */ | ||
| #if defined(MBEDTLS_PSA_CRYPTO_SPM) | ||
| /* | ||
| * PSA_CRYPTO_SECURE means that this file is compiled to the SPE side. |
There was a problem hiding this comment.
- compiled to the SPE side.
+ compiled for the SPE.
library/psa_crypto_slot_management.c
Outdated
| #if defined(MBEDTLS_PSA_CRYPTO_SPM) | ||
| /* | ||
| * PSA_CRYPTO_SECURE means that this file is compiled to the SPE side. | ||
| * some headers will be affected by this flag. |
There was a problem hiding this comment.
- some headers will be affected by this flag.
+ Some headers will be affected by this flag.
|
@Patater I'll change the commit message and code comment, no problem. But what about https://github.com/ARMmbed/mbed-crypto/blob/development/library/psa_crypto.c#L29-L42, I copy pasted from there... Would you like me to update it as well? |
|
If you wouldn't mind, it'd help to have another commit to update the other comment block. Thanks. |
When MBEDTLS_PSA_CRYPTO_SPM is defined, the code is being built for SPM (Secure Partition Manager) integration which separates the code into two parts: NSPE (Non-Secure Processing Environment) and SPE (Secure Processing Environment). When building for the SPE, an additional header file should be included.
itayzafrir
force-pushed
the
spm-support-crypto-handles-api
branch
from
f21f8e2 to
14e7678
Compare
|
@Patater I've updated the the code comments, commit message title and description, I also added a new commit to update existing SPM documentation in the code. |
itayzafrir
added
the
DO NOT MERGE
itayzafrir
removed
the
DO NOT MERGE
…Mmbed/mbed-crypto#16 This should be part of the mbed-crypto update into mbed-os!!!
Support for the API changes from slots to handles under SPM (code separation between SPE and NSPE).
Same idea as in https://github.com/ARMmbed/mbed-crypto/blob/development/library/psa_crypto.c#L29-L42