M2351: Support TFM level 1 #10959
Merged
M2351: Support TFM level 1 #10959
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ccli8, thank you for your changes. |
Greentea test on TFM target |
Greentea test on non-TFM target |
0xc0170
previously requested changes
Jul 9, 2019
| * include extra header file like RTE_Components.h. To avoid inclusion error | ||
| * or inclusion of more irrelevant header files, we expand osRtxMutexId() | ||
| * in-place to get mutex lock count straight. */ | ||
| //os_mutex_t *mutex = osRtxMutexId(ns_lock.id); |
There was a problem hiding this comment.
There's an on-going change Interface: Refactor the NS interface in TFM repo. The change to TF-M NS interface and related code in this PR are temporary. Just ignore it and wait for TF-M NS interface to be confirmed.
| SYS_UnlockReg_S(); | ||
| CLK_Idle_S(); | ||
| SYS_LockReg_S(); | ||
| nu_idle_s(); |
There was a problem hiding this comment.
how is this related to TFM support? Shouldn't this be separate PR addressing sleep issue?
There was a problem hiding this comment.
Seeing other HAL files, this is related to the TFM ?
There was a problem hiding this comment.
With TF-M integrated in, secure call overhead gets higher. Combine 3 secure calls into one for TFM build and also for non-TFM build.
| @@ -0,0 +1,26 @@ | |||
| /* | |||
| * Copyright (c) 2018-2020, Nuvoton Technology Corporation | |||
There was a problem hiding this comment.
2019 instead of 2020 (some files have this one as well).
There was a problem hiding this comment.
Not familiar with copyright claim. Which one is better, 2018-2020, 2019-2020, or 2019?
There was a problem hiding this comment.
just use the year this was created/edit. we are not yet in 2020
There was a problem hiding this comment.
2018-2019 would be fine here
|
@Patater please review |
|
The Mbed OS wide changes are ok, I don't know enough about PSA to review that part. |
|
A TFM change 1460 which supports overriding NS interface API has merged. Would you integrate it into mbed-os? So that M2351 can override |
0xc0170
requested review from
0xc0170 and
a team
and removed request for
a team
|
@ARMmbed/mbed-os-crypto Please review @ccli8 please rebase |
ccli8
force-pushed
the
nuvoton_m2351_psa
branch
from
de5dbbe to
a0bfb96
Compare
|
Do rebase to fix conflicts |
After the latest 5.14 changes, this needs rebase again. I assume the naming should be fixed as well. |
ccli8
force-pushed
the
nuvoton_m2351_psa
branch
from
a0bfb96 to
d478de3
Compare
This idea comes from TFM.
uVisor is deprecated since mbed-os 5.10, so remove related code with it.
On M2351, some spaces like SYS/CLK are hard-wired to secure and cannot change. To access these spaces from non-secure world, we must provide platform-specific NSC functions. With TFM introduced, we must synchronize NSC calls into TFM to keep TFM in sync instead of straight NSC calls. To achieve this goal, we go with the following approach: 1. Like PSA APIs, enforce locked entry through tfm_ns_lock_dispatch(). 2. Run platform-specific secure functions in default secure partition, in which SYS/CLK spaces have been configured to be accessible.
Lock kernel scheduler rather than mutex to guarantee serialization of NS secure calls
Merge SYS_UnlockReg_S()/CLK_Idle_S() or CLK_PowerDown_S()/SYS_LockReg_S() into nu_idle_s() or nu_powerdown_s() when they are available.
Support secure/non-secure combined build for PSA target:
1. In secure post-build, deliver built secure image to TARGET_NU_PREBUILD_SECURE
directory.
2. In non-secure post-build, merge non-secure image with secure image saved in
TARGET_NU_PREBUILD_SECURE directory.
3. In non-secure post-build, user can also drop secure image saved in
TARGET_NU_PREBUILD_SECURE directory and provide its own by adding the line below
in mbed_app.json:
"target.extra_labels_remove": ["NU_PREBUILD_SECURE"]
1. Change secure/non-secure ROM to 240KiB/272KiB 2. Change secure/non-secure RAM to 64KiB/32KiB
madchutney
previously requested changes
Jan 6, 2020
|
@madchutney For the 20-char limit with target name, I believe you see the outdated version. It has been addressed in #11288 (comment). |
ccli8
force-pushed
the
nuvoton_m2351_psa
branch
from
f55efa8 to
22e4f93
Compare
|
This PR has been on-going for a long time. It relies on mbed-os re-integration with TF-M for refactoring NS interface (#10959 (comment)). However, the re-integration is not scheduled yet? Here, I propose a solution: I separate out 85dfc1a which changes NS lock functions to weak so that M2351 platform can override them to provide another mechanism. This is also what https://review.trustedfirmware.org/c/trusted-firmware-m/+/1460 is adopting. |
|
Hi @ccli8, my apologies. I was basing my comment on the description of the PR. I see the code is now fixed. Thanks. |
👍 |
OK for now. We'll need to revisit/re-integrate when we bring in the new TF-M integration work from |
|
@ccli8 Once updated as suggested previously, please let us know. |
@0xc0170 I've updated as #10959 (comment) suggested. Please go ahead. |
|
CI restarted |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds TFM level 1 support on NUMAKER_PFM_M2351 target. It has the following major changes compared to no TFM integration:
NUMAKER_PFM_M2351_S: Target name for building M2351 TFM secure codeNUMAKER_PFM_M2351_NS: Target name for building M2351 TFM non-secure codeNUMAKER_PFM_M2351_NOPSA_S: Target name for building M2351 non-TFM secure codeNUMAKER_PFM_M2351_NOPSA_NS: Target name for building M2351 non-TFM non-secure codeNU_PREBUILD_SECUREfromtarget.extra_labelsin mbed_app.json like:NUMAKER_PFM_M2351is used for building non-TFM both secure/non-secure code via Nuvoton samples NuMaker-mbed-TZ-secure-example and NuMaker-mbed-TZ-nonsecure-example. Committed to keeping supporting non-TFM target, new target namesNUMAKER_PFM_M2351_NOPSA_S/NUMAKER_PFM_M2351_NOPSA_NSare created to substitute forNUMAKER_PFM_M2351to build non-TFM secure/non-secure code respectively. Their naming can be negotiated. After this PR is merged, the Nuvoton samples mentioned above will also update to reflect this change.Pull request type