CMSE Secure library creation #5180

deepikabhavnani · 2017-09-22T14:59:45Z

Cortex v8 architecture based devices support secure/non-secure builds. Secure build should generate the object file/library from elf during link process which is used by non-secure binary during linking.
TZ_context functions are part of secure state binary and are used to handle secure stack for thread execution.

NS flag in the core name in Target.json indicated Non Secure build for the core. It is supported for v8M devices only.
Secure library will be built as part of linking process for GCC compiler. Non-Secure project should add this secure object file as part of the linking process.

Secure library is named as cmse_lib.o.

theotherjimmy

I think we can improve the python modifications. Suggestions for how below.

theotherjimmy · 2017-09-22T15:01:55Z

tools/toolchains/gcc.py

Please use

secure_file = join(dirname(output), "cmse_lib.o")

instead.

Team: cmse_lib.o : Is it ok to name secure library as generic secure lib, or should it be core specific like CortexM23-S-lib.o?

For reference, if you are going to name it that way, the python should look like:

secure_file = join(dirname(output), "%s-S-lib.o" % self.target.core)

theotherjimmy · 2017-09-22T15:03:20Z

tools/toolchains/gcc.py

Please use

self.progress("Secure Library Object", secure_file)

instead.

@theotherjimmy - Build for non-secure will fail if secure library file is missing during linking. To make it more user friendly:

Should be scan and report for missing file in advance?

Add it as requirement in app,json and expect it to be at same location?

Hmmmm... We could do that, but what if someone compiles for a pure non-secure CORTEX-M23-NS? Would you be locking them into having an empty object file to trick the build system?

Empty object file won;t help because it will again fail during linking. Core will boot in secure mode always and to switch to non-secure mode, stubs function and switch to non-secure reset handler will be required. One cannot boot with just non-secure project.

You misunderstood my question. Let me rephrase. Can a user build a non-secure Cortex-M23-NS application without any secure component?

Sorry do u mean no secure API's just basic dummy functions. Yes build will be successful and it will be same as current mbed-os without any advantage of security

Okay. If I understand correctly, A user would need to have an object file compiled for secure Cortex-M23 containing these functions, even if they don't care about security and just want to compile there entire application for Cortex-M23-NS.

If that's the case, then I think it's fine to insist that they have an object file matching the naming convention created by the linker.

@theotherjimmy

self.progress("Secure Library Object", secure_file)

Does truncates file path basename(event['file']) and prints just filename. I need path of file to be printed. Didn't saw any args which can be helpful.

Does truncates file path basename(event['file']) and prints just filename.

I'm having trouble understanding that this sentence is supposed to mean. Could you rephrase it? Is this a question? Is this an assertion?

I need path of file to be printed.

Hmmmmmm.... That's not supported by it. Try:

self.info("Secure Library object file %s" % secure_file)

theotherjimmy · 2017-09-22T15:08:51Z

@deepikabhavnani I beat you too it 😉

0xc0170 · 2017-09-26T13:27:35Z

rtos/TARGET_CORTEX/mbed_boot.c

Is this always defined (shouldn't this check first if its defined then its value) ?

Yes it is always defined in

mbed-os/rtos/TARGET_CORTEX/rtx5/core_cm.h

Line 64 in eefb84c

#ifndef __DOMAIN_NS

0xc0170 · 2017-09-26T13:29:13Z

rtos/TARGET_CORTEX/mbed_tz_context.c

4 spaces alignment in this file?

0xc0170 · 2017-09-26T13:30:36Z

rtos/TARGET_CORTEX/mbed_tz_context.c

are these two something that should be in the config ?

Yes we can have them in config

0xc0170 · 2017-09-27T12:25:49Z

Another round of reviews opened!

deepikabhavnani · 2017-09-27T15:32:00Z

Rebased to pick up latest tools fixes

0xc0170 · 2017-09-29T13:20:00Z

Bump for review !

deepikabhavnani · 2017-09-29T19:48:14Z

Application in Secure mode, calls assembly functions which are also toolchain specific. I need to move them to mbed_boot.c after verification.

bulislaw · 2017-10-02T15:25:41Z

rtos/TARGET_CORTEX/mbed_rtx_conf.h

+
+/* Number of process slots (threads may call secure library code) */
+#ifndef MBED_CONF_APP_TZ_PROCESS_STACK_SLOTS
+#define MBED_CONF_APP_TZ_PROCESS_STACK_SLOTS     8U


why 8? Can we document that

Ideally it should be number of threads we allow in mbed-os. (Considering all of them have access to secure library). Or do we want to restrict threads from using secure library?

bulislaw · 2017-10-02T15:27:07Z

rtos/TARGET_CORTEX/mbed_boot.c

    _main_thread_attr.name = "main_thread";
+
+    /* Allow main thread to call secure functions */
+#if (__DOMAIN_NS == 1U)


Don't worry about it for this PR, but this file is becoming unreadable with more ifdefs than anything else, we should find better way of doing this.

bulislaw · 2017-10-02T15:27:28Z

rtos/TARGET_CORTEX/mbed_rtx_conf.h

+#endif
+
+/* Stack size of the secure library code */
+#ifndef MBED_CONF_APP_TZ_PROCESS_STACK_SIZE


What is this stack for exactly?

It is stack for NSC region. i.e Secure library code executed from non-secure process.

Might be worth extending the comment ? Are we using code defaults instead of config - (this could be in the config file set, always present if not overriden) ? All in one place, this can however be fixed, as we already have this mbed_rtx_conf.h file.

@0xc0170 - In case of secure build, we ignore rtos (mbed_rtx_conf.h) hence I am exploring other correct options to update this flag.

bulislaw · 2017-10-02T15:28:11Z

rtos/TARGET_CORTEX/mbed_tz_context.c

+
+
+/// Number of process slots (threads may call secure library code)
+#ifndef TZ_PROCESS_STACK_SLOTS


We do the same in config file. Why do we duplicate that it's confusing

I thought we do not update the flags/code picked up from CMSIS repo, hence i duplicated it. Can remove if required in this case.

bulislaw · 2017-10-02T15:28:45Z

rtos/TARGET_CORTEX/mbed_tz_context.c

@@ -0,0 +1,200 @@
+/* mbed Microcontroller Library


Where does this file coming from?

https://github.com/ARM-software/CMSIS_5/blob/develop/CMSIS/RTOS2/RTX/Examples/TrustZoneV8M/RTOS/CM33_s/tz_context.c

theotherjimmy · 2017-10-05T15:56:27Z

@bulislaw Have your comments been addressed?

mbed-ci · 2017-10-09T22:08:57Z

Build : SUCCESS

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

Triggering tests

/test mbed-os

mbed-ci · 2017-10-09T23:29:20Z

Test : SUCCESS

Build number : 10
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5180/10

0xc0170 · 2017-10-10T10:50:12Z

@tommikas jenkins CI restart here?

mbed-ci · 2017-10-10T20:05:43Z

Build : SUCCESS

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

Triggering tests

/test mbed-os

mbed-ci · 2017-10-10T21:16:57Z

Build : SUCCESS

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

Triggering tests

/test mbed-os

mbed-ci · 2017-10-10T22:50:05Z

Test : SUCCESS

Build number : 22
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5180/22

mbed-ci · 2017-10-11T05:29:19Z

Build : SUCCESS

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

Triggering tests

/test mbed-os

mbed-ci · 2017-10-11T16:45:49Z

Test : SUCCESS

Build number : 35
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5180/35

mbed-ci · 2017-10-11T19:00:51Z

Build : SUCCESS

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

Triggering tests

/test mbed-os

sg- · 2017-10-11T19:04:23Z

tools/toolchains/arm.py

why is this in the tools and not behind a TARGET_M23 folder?

Creation of secure library shouldn;t be part of tools?

I dont know the workflow or modifications needed to that file

@deepikabhavnani As I read this ld flag: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0803h/pge1444636329852.html - this is the output thus located into build dir as cmse_lib.o. Threfore it is part of tools, or is there any other option?

cc @theotherjimmy

@0xc0170 - Generation of Secure library has to be part of tools, ARMC6 compiler allows creation from elf file as well, but GCC/ARM support generation of this object file during linking.

We can rename lib to anything we want (like target specific) and can place in any required build folder (Current: .\BUILD<Target Device><Toolchain>\cmse_lib.o

Cortex v8 architecture based devices support secure/non-secure builds. Secure build should generate the object file/library from elf during link process which is used by non-secure binary during linking. --out-implib=file specifies output library in which symbols are exported --cmse-implib requests libraries mentioned above are secure gateway libraries Creation of secure library is done as part of linking process in GCC/ARMC6/IAR Non-Secure project should add this secure object file as part of the linking process. Secure library is named as cmse_lib.o.

…ions

…d in map Fix by Jimmy

deepikabhavnani · 2017-10-13T15:43:48Z

rtos/TARGET_CORTEX/rtx5/rtx_lib.c

  osPriorityIdle,
-  0U, 0U
+#if (__DOMAIN_NS == 1U)
+  1U,


Actual fix should come in CMSIS, raised an issue ARM-software/CMSIS_5#252.
Need to update when we have proper fix from CMSIS

deepikabhavnani · 2017-10-13T15:45:08Z

platform/mbed_sdk_boot.c

 {
-
+#if defined (__ARM_FEATURE_CMSE) &&  (__ARM_FEATURE_CMSE == 3U)
+    call_non_secure_rtos();


With this change main of secure thread will never be called, any initialization required needs to be done in mbed_sdk_init.

deepikabhavnani · 2017-10-13T15:46:09Z

rtos/TARGET_CORTEX/rtx5/TARGET_M23/TOOLCHAIN_ARM/irq_armv8mbl.S

-                BMI      Sys_ContextSave1       ; Branch if secure
+                MOV      R0,LR                  ; Get EXC_RETURN
+                LSLS     R0,R0,#25              ; Check domain of interrupted thread
+                BPL      Sys_ContextSave1       ; Branch if not secure


Raised issue + added PR in CMSIS
ARM-software/CMSIS_5#260

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. TZ_PROCESS defines are added to config. Stack size is increased to 512 in source file as well, since config file is not picked when compiling mbed-os in secure mode without RTOS.

Non-Secure threads can access secure calls only when tz_module attribute is set as 1(OS_SECURE_CALLABLE_THREAD), while thread creation. Hence adding tz_module as an argument to ctor.

Issues addressed: 1. Switch to secure/nonsecure context save/restore is based on 6th bit in LR register, correct the bug (R7 instead of LR was used for decision) 2. Retain value of R7 3. Branch if non-secure instead of secure

… defined

deepikabhavnani · 2017-10-31T15:01:55Z

@theotherjimmy @adbridge @0xc0170 - This PR is on hold till ARMC6 compiler issue is resolved.
https://jira.arm.com/browse/SDCOMP-49132

deepikabhavnani · 2017-11-22T14:46:09Z

Closing this, will reopen when work on this feature is resumed

deepikabhavnani mentioned this pull request Sep 22, 2017

Add Nuvoton m2351device #4892

Closed

theotherjimmy suggested changes Sep 22, 2017

View reviewed changes

deepikabhavnani requested review from sg- and theotherjimmy September 22, 2017 15:08

theotherjimmy added the needs: review label Sep 25, 2017

0xc0170 reviewed Sep 26, 2017

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Sep 26, 2017

deepikabhavnani force-pushed the secure_support branch from b8123d9 to c4b8138 Compare September 26, 2017 18:38

0xc0170 added needs: review and removed needs: work labels Sep 27, 2017

deepikabhavnani force-pushed the secure_support branch from 2537310 to 4af5926 Compare September 27, 2017 15:31

deepikabhavnani added the needs: work label Sep 29, 2017

theotherjimmy approved these changes Oct 2, 2017

View reviewed changes

bulislaw reviewed Oct 2, 2017

View reviewed changes

theotherjimmy removed the needs: work label Oct 5, 2017

deepikabhavnani force-pushed the secure_support branch from c2a594b to 4d9cd1f Compare October 10, 2017 16:25

sg- reviewed Oct 11, 2017

View reviewed changes

deepikabhavnani added 7 commits October 13, 2017 10:20

Non Secure flag is required for pre-processing of linker file

988b0dd

Allow main thread to call non-secure functions

c4a1697

Using proper lib functions as suggested in comments

87afc6b

Allow Idle and timer thread to access secure functions

6ba2bf9

Update mbed_boot in secure mode to call non-secure reset handler funt…

7b5604e

…ions

Build failed in case of malformed input, code section was not include…

07cbb98

…d in map Fix by Jimmy

deepikabhavnani force-pushed the secure_support branch from 9d8aad2 to f21c1ab Compare October 13, 2017 15:40

deepikabhavnani commented Oct 13, 2017

View reviewed changes

Deepika added 4 commits October 13, 2017 12:07

Access to serial device is allowed when DEVICE_SERIAL is defined

af83953

Trust zone module identifier added to thread class

c459188

Non-Secure threads can access secure calls only when tz_module attribute is set as 1(OS_SECURE_CALLABLE_THREAD), while thread creation. Hence adding tz_module as an argument to ctor.

Systick handler switch to secure/nonsecure

b7358f8

Issues addressed: 1. Switch to secure/nonsecure context save/restore is based on 6th bit in LR register, correct the bug (R7 instead of LR was used for decision) 2. Retain value of R7 3. Branch if non-secure instead of secure

deepikabhavnani force-pushed the secure_support branch 2 times, most recently from 10e2d09 to c52d375 Compare October 16, 2017 19:29

Update files as per CMSIS repo with update to __DOMAIN_NS flag if not…

4d9aeef

… defined

deepikabhavnani force-pushed the secure_support branch from c52d375 to 4d9aeef Compare October 16, 2017 20:01

theotherjimmy added the needs: preceding PR label Oct 23, 2017

theotherjimmy added do not merge and removed needs: review labels Nov 13, 2017

deepikabhavnani closed this Nov 22, 2017

sg- removed the do not merge label Nov 22, 2017

deepikabhavnani removed the needs: preceding PR label Feb 8, 2018



		/// Number of process slots (threads may call secure library code)
		#ifndef TZ_PROCESS_STACK_SLOTS

CMSE Secure library creation #5180

CMSE Secure library creation #5180

Uh oh!

Conversation

deepikabhavnani commented Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepikabhavnani Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepikabhavnani Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Sep 27, 2017

Uh oh!

deepikabhavnani commented Sep 27, 2017

Uh oh!

0xc0170 commented Sep 29, 2017

Uh oh!

deepikabhavnani commented Sep 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepikabhavnani commented Sep 22, 2017 •

edited

Loading

deepikabhavnani Sep 22, 2017 •

edited

Loading

deepikabhavnani Sep 22, 2017 •

edited

Loading

theotherjimmy commented Sep 22, 2017 •

edited

Loading

deepikabhavnani commented Sep 29, 2017 •

edited

Loading