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.

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

CMSIS: Add TrustZone functions #6483

Merged
merged 4 commits into from Apr 18, 2018

Conversation

Projects
None yet
5 participants
@deepikabhavnani
Contributor

deepikabhavnani commented Mar 27, 2018

Description

Add TZ functions to mbed-os build from CMSIS repo. These functions are required to be part of secure library when RTOS support is required in non-secure mbed-os.

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[X] Feature
[ ] Breaking change

CC @bulislaw

Add TZ_context function
TZ_context functions are part of secure state binary and are used to handle
secure stack for thread execution.Template implementation from CMSIS is used,
can be enhanced in future.
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 27, 2018

@tkaman Please review

@@ -149,7 +149,8 @@
"a019acaf8d6fb1f0512414d072f667cc2749b1d9",
"a884fdc0639ae4e17299838ec9de4fddd83cf93c",
"6c827cb5879bc096e45efd992dfadcb96c1d50bc",
"919282322e106b82fea50878f41b6c75a7eb356b"
"919282322e106b82fea50878f41b6c75a7eb356b",

This comment has been minimized.

@bulislaw

bulislaw Mar 27, 2018

Member

Is there a way forward without addingn extra commits here?

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Mar 27, 2018

Contributor

@bulislaw - Can work on that if you have any suggestions. In this case the code is picked from sample implementation and wont even compile as is.
https://github.com/ARM-software/CMSIS_5/blob/develop/CMSIS/RTOS2/RTX/Examples/TrustZoneV8M/RTOS/CM33_s/tz_context.c#L29

I have 3 changes listed below of which I have solution for two, but still for header file we will need a commit, unless you want to track it some other way.

  1. Update Stack size to 512: Can add PR to CMSIS repo
  2. Config options : Can remove completely (Cannot add to config files which reside inside RTOS, as the entire folder is skipped during secure lib build)
  3. Add correct header file.

This comment has been minimized.

@bulislaw

bulislaw Mar 27, 2018

Member

Could you do this: f072380

This comment has been minimized.

@bulislaw

bulislaw Mar 27, 2018

Member

You probably won't need the _RTE_ for current version of CMSIS

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Mar 27, 2018

Contributor

@bulislaw - b93aca2 Will have to add this change in json file.

This comment has been minimized.

@bulislaw

bulislaw Mar 28, 2018

Member

Why? Use the #include "RTE_Components.h" and #include CMSIS_device_header the same way my commit does and configure it in config file. Is there something i'm missing?

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Mar 28, 2018

Contributor

@bulislaw - Sorry I missed the header part, but the change I wanted to address was conditional compilation. b93aca2#diff-ae477765d86108e79533ea6617f50737R28

File should be compiled only in case of secure build and not for non-secure build, will check with @theotherjimmy if we can have some option from tools

This comment has been minimized.

@theotherjimmy

theotherjimmy Mar 28, 2018

Contributor

Yep. We can have that support in the tools. We currently don't have something like that. Would you like that as part of this PR? or another PR?

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:add_tz_functions branch from a289285 to 5425e82 Mar 27, 2018

Trustzone stack requirement for Mbed-OS is 512 bytes
Added config parameter for TZ stack size and update code to add correct header
file.

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:add_tz_functions branch from 5425e82 to ecf2f86 Mar 28, 2018

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Mar 28, 2018

Would you like that as part of this PR? or another PR?

I will leave it to you.

Commit ecf2f86 is reminder for me, Will remove it once we have support in tools

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 10, 2018

Commit ecf2f86 is reminder for me, Will remove it once we have support in tools

@deepikabhavnani @theotherjimmy Are we expecting the tools support anytime soon, is this PR blocked because of it?

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Apr 10, 2018

@0xc0170 - Yes PR is blocked for tools support

deepikabhavnani added some commits Mar 28, 2018

CMSIS/RTX: Patch to conditionally compile
tz_context.c should be compiled only for secure world,
definition of API's in tz_context.h should be part of secure
binary/bootloader when building mbed-os as non-secure

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:add_tz_functions branch from ecf2f86 to 40e2737 Apr 11, 2018

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Apr 11, 2018

@bulislaw - Please review

@bulislaw

Approved, we'll need to revisit that when the secure world story is a bit more matured. I wouldn't like people to assume Mbed is supposed to live in TrustZone or that the mbed_tz_context.c is a proper production solution. @SenRamakri

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 17, 2018

/morph build

@0xc0170

0xc0170 requested changes Apr 17, 2018 edited

Before I approve, I would like to understand how is this used (the plans) ? As this brings some functions but how to use them, can I? should I?

cmsis/TARGET_CORTEX_M/mbed_tz_context.c - this is our file? From the name it seems, but implementation does not conform to the style we do?

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 17, 2018

Build : SUCCESS

Build number : 1769
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6483/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Apr 17, 2018

@0xc0170 - cmsis/TARGET_CORTEX_M/mbed_tz_context.c is CMSIS file, part of application hence we renamed it to mbed_ - for mbed OS.
It will be used only for Cortex-M23 / M33 targets, while building the secure library. Default Mbed OS target should be NS and user will never use this source file, rather will link to secure library containing symbols of these functions.

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Apr 17, 2018

We'll need to revisit that when the secure world story is a bit more matured

👍

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 merged commit 099e54b into ARMmbed:master Apr 18, 2018

12 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9707 cycles (-56 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@0xc0170 0xc0170 changed the title from Add tz functions to CMSIS: Add TrustZone functions Apr 18, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 19, 2018

@deepikabhavnani deepikabhavnani deleted the deepikabhavnani:add_tz_functions branch Apr 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment