Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TF-M: provide option for non-secure targets to request TF-M to issue system resets #33510

Merged
merged 6 commits into from
Mar 26, 2021
Merged

TF-M: provide option for non-secure targets to request TF-M to issue system resets #33510

merged 6 commits into from
Mar 26, 2021

Conversation

ioannisg
Copy link
Member

@ioannisg ioannisg commented Mar 19, 2021 •
edited
Loading

Add the infrastructure to issue system resets for Non-Secure Cortex-M builds with TF-M.

TF-M does not allow Non-Secure system reset by default. This needs to be done with a non-secure callable API.
This PR:

  • provides this functionality for Non-Secure builds to request system resets, using tfm_platform_system_reset
  • introduces functionality to initialize the TFM NS API at boot time
  • fixes dependencies for BUILD_WITH_TFM
  • introduces an option that signifies that the default NS interface is used (native), and sets up the required dependencies.
  • implements the TF-M NS interface, natively, in Zephyr, using directly the required mutex APIs

@ioannisg ioannisg requested a review from microbuilder as a code owner March 19, 2021 16:08
@ioannisg ioannisg self-assigned this Mar 19, 2021
@ioannisg ioannisg force-pushed the tfm_override_arch_system_reset branch from 6e4edf7 to 4c9f427 Compare March 19, 2021 16:24
@ioannisg ioannisg changed the title TF-M: provide option to request TF-M to reset the core TF-M: provide option for non-secure targets to request TF-M to issue system resets Mar 19, 2021
@ioannisg ioannisg added area: Trusted Execution Trusted Execution area: TF-M ARM Trusted Firmware-M (TF-M) labels Mar 19, 2021
@ioannisg ioannisg requested a review from hakonfam March 19, 2021 16:28
@ioannisg ioannisg force-pushed the tfm_override_arch_system_reset branch 2 times, most recently from b4467f8 to 8b9a931 Compare March 19, 2021 16:43
@ioannisg ioannisg requested a review from agansari March 19, 2021 16:50
@oyvindronningstad
Copy link
Collaborator

oyvindronningstad commented Mar 22, 2021 •
edited
Loading

@ioannisg It turns out you don't need the native interface or the CMSIS support. The samples had their own workarounds, and I have pulled it out into its own file in #33553. See if it solves your problem.

@ioannisg
Copy link
Member Author

@ioannisg It turns out you don't need the native interface or the CMSIS support. The samples had their own workarounds, and I have pulled it out into its own file in #33553. See if it solves your problem.

Thanks for the proposal @oyvindronningstad. I had also seen this myself while browsing the samples. However, I think that Zephyr should have an option to support the native TF-M interface, which based on CMSIS RTOS V2, as community might prefer that one instead of whatever we would like to define ourselves.

The interface you present in #33553 should be the foundation of Zephyr's TF-M NS interface; currently it is minimal, so we need to do enhancements there.

@oyvindronningstad
Copy link
Collaborator

However, I think that Zephyr should have an option to support the native TF-M interface, which based on CMSIS RTOS V2, as community might prefer that one instead of whatever we would like to define ourselves.

@ioannisg I don't see much reason though, since it's not really native. The osMutexAcquire eventually leads to a k_mutex_lock, so with the zephyr version we just cut out the indirection and call k_mutex_lock directly. I might be missing something though.

@ioannisg ioannisg force-pushed the tfm_override_arch_system_reset branch from 8b9a931 to 3b57fef Compare March 23, 2021 14:21
@github-actions github-actions bot added the area: Samples Samples label Mar 23, 2021
@ioannisg ioannisg force-pushed the tfm_override_arch_system_reset branch 2 times, most recently from df1c165 to be01fd8 Compare March 23, 2021 16:36
@ioannisg
Copy link
Member Author

@oyvindronningstad i dragged your commit into this PR, which now introduces the Zephyr, (native) NS interface.

@oyvindronningstad
Copy link
Collaborator

be01fd8 can be merged into a14eb15 since it is where the actual file is removed.

@oyvindronningstad oyvindronningstad mentioned this pull request Mar 24, 2021
Merged
oyvindronningstad
oyvindronningstad approved these changes Mar 25, 2021
View reviewed changes
Copy link
Collaborator

@oyvindronningstad oyvindronningstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No comments except the merging of commits

@ioannisg
Copy link
Member Author

LGTM. No comments except the merging of commits

The reason that i pulled this out was to ease reviewing, as it does not have anything to do with moving the NS interface, it is a minor cleanup patch. But, since it is part of your original commit, actually, I will pull this back, if you insist @oyvindronningstad :) :)

@ioannisg
Copy link
Member Author

@oyvindronningstad thanks for cooperating on this PR, btw.

@ioannisg ioannisg force-pushed the tfm_override_arch_system_reset branch from 5fdcf05 to dac56d3 Compare March 25, 2021 15:15
@ioannisg
Copy link
Member Author

Merge the two commits in one, as suggested above.

ioannisg and others added 5 commits March 26, 2021 09:42
@ioannisg
trusted-firmware-m: include interface sources to the build
9a77d02 
Instruct CMake to include interface libraries when
building a Non-Secure ARM target with TF-M. In
particular, include the reboot.c source file, which
overrides the sys_arch_reboot implementation.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg
trusted-firmware-m: override sys_arch_reboot for non-secure builds
e345334 
In ARM Non-Secure builds with TF-M it is not, generally,
possible to issue system reset requests from Non-Secure
domain. When the Platform SPM Partition is enabled, the
tfm_platform_system_reset(.) API can be used to request
system resets from TF-M. This commit overrides the weak
sys_arch_reboot() implementation in scb.c so Non-Secure
code is able to issue system resets.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg
modules: trusted-firmware-m: initialize the TF-M NS interface
31054f1 
The TF-M NS interface needs to be initialized before
it is used. We add an initialization function that
calls tfm_ns_interface_init(), at boot time, so then
we can use TF-M interface calls (veneers).

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@oyvindronningstad @ioannisg
modules: tf-m: Add zephyr native NS interface and logging function
aecbd9b 
To allow using TFM NS interface without enabling
CMSIS_RTOS V2 support. And to allow using TFM NS
code that uses logging.

Signed-off-by: Øyvind Rønningstad <[email protected]>
Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg
modules: trusted-firmware-m: lock scheduler around secure calls
3a3693c 
Prevent a thread from being preempted, while executing a Secure
function. This is required to prevent system crashes that could
occur if a thead context switch is triggered in the middle of a
Secure call.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg ioannisg force-pushed the tfm_override_arch_system_reset branch from dac56d3 to 3a3693c Compare March 26, 2021 08:42
@ioannisg
Copy link
Member Author

Rebased, to get TF-M v1.3.0 -RC1

@ioannisg
Copy link
Member Author

@oyvindronningstad @microbuilder I added a patch to disable Audit Log for profile:medium builds, due to the upstream bug reported here: https://developer.trustedfirmware.org/T907

@ioannisg
modules: tf-m: disable audit log partition with medium profile
6da3d70 
When compiling TF-M with profile_medium, disable the support
for Audit Log due to an upstream bug.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg ioannisg force-pushed the tfm_override_arch_system_reset branch from d0ca663 to 6da3d70 Compare March 26, 2021 15:34
@ioannisg ioannisg merged commit edd4ab5 into zephyrproject-rtos:master Mar 26, 2021
@oyvindronningstad oyvindronningstad mentioned this pull request Mar 26, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oyvindronningstad oyvindronningstad oyvindronningstad approved these changes

@microbuilder microbuilder microbuilder approved these changes

@tejlmand tejlmand Awaiting requested review from tejlmand

@erwango erwango Awaiting requested review from erwango

@hakonfam hakonfam Awaiting requested review from hakonfam

@agansari agansari Awaiting requested review from agansari

Assignees

@microbuilder microbuilder

@ioannisg ioannisg

Labels
area: Modules area: Samples Samples area: TF-M ARM Trusted Firmware-M (TF-M) area: Trusted Execution Trusted Execution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ioannisg @oyvindronningstad @microbuilder