PSA Secure partition manager and services #8744
Conversation
|
Thanks for the credit! |
|
It looks like I forgot to update the python tests to include the new Config attribute. I'll add that later today. |
Test run: SUCCESSSummary: 4 of 4 test jobs passed |
|
We are waiting for the last dependency to land : #8730 |
TESTS/mbed_hal/spm/fault_functions.h
Outdated
| @@ -0,0 +1,48 @@ | |||
| /* mbed Microcontroller Library | |||
| * Copyright (c) 2017 ARM Limited | |||
There was a problem hiding this comment.
All new files should have 2018 year and using SPDX identifier
| @@ -27,6 +27,8 @@ | |||
| ############################################################################## | |||
| BUILD_DIR = abspath(join(ROOT, "BUILD")) | |||
|
|
|||
| DELIVERY_DIR = abspath(join(ROOT, "DELIVERY")) | |||
There was a problem hiding this comment.
what does this do? Is it documented anywhere ( no comment here, cant find it in the commit msg neither)
hal/spm_api.h
Outdated
| * | ||
| * Wakeup the peer processor waiting on the mailbox driver event. | ||
| * | ||
| * @note The functions below should be implemented by target specific code. |
There was a problem hiding this comment.
quick question: these new docs were already reviewed by docs team or shall be done (within this pull request) ?
There was a problem hiding this comment.
@danny4478 and @AnotherButler should know the aswer
There was a problem hiding this comment.
@0xc0170 Which docs are you referring to?
- The code (c and h files) was not reviewed by docs team. Once the cod is ready for merge, the docs team will generate the Doxygen for them.
- Regarding docs, we have 3 documents (handbook) under review by the docs team.
There was a problem hiding this comment.
Doxygen documentation (functionality docs) - they have to be reviewed by docs team.
Therefore if that is true, it would be helpful here provided what doxygen docs should be reviewed. I identified at least this header file
There was a problem hiding this comment.
Doxygen headers for review are:
[ ] - components/TARGET_PSA/spm/spm_client.h
[ ] - components/TARGET_PSA/spm/COMPONENT_SPE/spm_server.h
[ ] - components/TARGET_PSA/spm/psa_defs.h
[ ] - hal/spm_api.h
There was a problem hiding this comment.
Thankss, this is helpful !
@melwee01 @AnotherButler Please review these
Test run: SUCCESSSummary: 4 of 4 test jobs passed |
|
All dependencies were integrated, please rebase (I reviewed astyle failures, rebase should take care of them as well?) |
|
Branch rebased and ready to go |
| #endif | ||
|
|
||
| #if (defined( __CC_ARM ) || defined(__ARMCC_VERSION) || defined( __ICCARM__ )) | ||
| #error [NOT_SUPPORTED] this test is supported on GCC only |
There was a problem hiding this comment.
What limitations are here that only GCC ARM supported? ) ?
There was a problem hiding this comment.
mainly assembly code included in the test
|
|
||
| #if defined(COMPONENT_SPE) | ||
|
|
||
| void psa_spm_init(void); |
There was a problem hiding this comment.
why these do not have any documentation?
| const uint32_t out_len | ||
| ); | ||
|
|
||
| void channel_state_switch(uint8_t *current_state, uint8_t expected_state, uint8_t new_state); |
There was a problem hiding this comment.
these 2 functions do not have doxygen, the rest of the functions here have?
There was a problem hiding this comment.
This is confusing.
This is an internal API.
AFAIU, it should not be Doxygen documented. Of course it should have normal code comments.
My bottom line is: This file comments should be be Doxygen style.
|
|
||
| #else | ||
|
|
||
| #error "Non dual-chip platforms are not yet supported" |
There was a problem hiding this comment.
Is this error correct ( this is else for COMPONENT_SPM_MAILBOX) ?
There was a problem hiding this comment.
Yes, there should be another main function for secure-side on v8 systems.
as we are not yet supporting v8 with SPM, the compilation should fail
|
@ARMmbed/mbed-os-tools Please review today |
|
Making a note here. This appears to be undergoing email discussion. Will update once resolved. |
|
All the PRs need to be engineering ready (marked as "needs: CI") by the end of the day (Austin time). Otherwise it won't make 5.11 and will need to come in the next release (5.12 for features, 5.11.1 for fixes and new platforms). |
Move function description to header files
Note: it is not in Doxygen format since it is an internal module and
Doxygen should not be generated for it
Make minor copy edits, mostly for punctuation and style.
Make minor edits, mostly for consistent capitalization and style.
Make minor edits, mostly for consistent abbreviations.
Edit file, mostly for minor grammar and style changes.
Co-Authored-By: orenc17 <[email protected]>
|
CI restarted |
|
|
||
| /* -------------------------------- Handle Manager Module ---------------------------- */ | ||
|
|
||
| /* The Handle Manager Module manages handles. |
There was a problem hiding this comment.
Presumably. Do we need to actually state this?
|
|
||
| /* The Handle Manager Module manages handles. | ||
| * | ||
| * It basically generates and exposes a unique handle identifier [handle] per |
There was a problem hiding this comment.
Don't use "basically" or similar.
|
|
||
| /* The Handle Manager Module manages handles. | ||
| * | ||
| * It basically generates and exposes a unique handle identifier [handle] per |
There was a problem hiding this comment.
(handle), (handle_mem), etc.
| * - Remove a handle from the handle manager module [handle_destroy] | ||
| * | ||
| * Note: | ||
| * Handles generation is done exclusively. |
| * | ||
| * Note: | ||
| * Handles generation is done exclusively. | ||
| * Once we got a handle, removing a handle or getting its memory can be |
There was a problem hiding this comment.
we, use you instead
non-exclusively
| @@ -0,0 +1,23 @@ | |||
| # Mbed Secure Partition Manager (SPM) | |||
|
|
|||
| The Platform Security Architecture (PSA) firmware framework specifications contain a logic component called the Secure Partition Manager (SPM). PSA defines a Secure Processing Environment (SPE) for: | |||
There was a problem hiding this comment.
Mbed secure partition manager, unless it's a brand name. Is it?
There was a problem hiding this comment.
the file will be removed as well
| # Mbed Secure Partition Manager (SPM) | ||
|
|
||
| The Platform Security Architecture (PSA) firmware framework specifications contain a logic component called the Secure Partition Manager (SPM). PSA defines a Secure Processing Environment (SPE) for: | ||
|
|
There was a problem hiding this comment.
Same as above comment for SPM and PSA. These are general terms, not brand names, right?
| The Platform Security Architecture (PSA) firmware framework specifications contain a logic component called the Secure Partition Manager (SPM). PSA defines a Secure Processing Environment (SPE) for: | ||
|
|
||
| * Sensitive data, such as keys, credentials and firmware. | ||
| * The code that manages it. |
| * The code that manages it. | ||
| * Its trusted hardware resources. | ||
|
|
||
| The PSA SPM interfaces decouple components, allowing reuse of components in other device platform and helps to reduce an integration effort. |
|
|
||
| * Secures low cost IoT devices, where a full Trusted Execution Environment (TEE) would not be appropriate. | ||
| * Protects sensitive assets (keys, credentials and firmware) by separating these from the application firmware and hardware. | ||
| * Is architecture agnostic and can be implemented on different Arm Cortex®-M architectures, offering variable level of protection, based on platform resources. |
There was a problem hiding this comment.
another character render error
|
Restarted CI , one device failed with serial error (in events test) |
|
@melwee01, thank you for review. |
Test run: SUCCESSSummary: 4 of 4 test jobs passed |
Description
Dependencies
#8667
#8680
#8730
The actual changes of this PR are in commit ad7876e0620531cadaad7502f27cdd5f4d8c814f
Pull request type