RTX V8M: no TZ when MBED_TZ_DEFAULT_ACCESS not present

[jeromecoutant]

Description

For V8M targets, MBED_TZ_DEFAULT_ACCESS macro is defined when TZ is used.

So if this TZ macro is not defined, target has to be used as default cortex M targets.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@deepikabhavnani
@LMESTM

[ciarmcom]

@jeromecoutant, thank you for your changes.
@deepikabhavnani @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

[deepikabhavnani]

@jeromecoutant - Thanks for the changes these changes belong to CMSIS repo, please add a PR to Cmsis repo as well, if approved we can merge it here as well.

[jeromecoutant]

CMSIS repo will not know MBED_TZ_DEFAULT_ACCESS :-(

[kjbracey]

DOMAIN_NS should not be 1 if TrustZone is not in use. There's no "non-secure" code without TrustZone.

Rest of this comment may be confused nonsense - I wasn't involved in the tools work here.

I don't believe Mbed OS currently supports building non-TrustZone ARMv8 targets? I think the answer may lie in addressing that.

As far as I can see you can specify

• cpu = Cortex-M33 to make a TrustZone secure library
• cpu = Cortex-M33-NS to make the non-secure main TrustZone build, which would use the above.

But there's no way to make a standalone non-TrustZone (effectively "secure") full image?

[jeromecoutant]

For information:
ARM-software/CMSIS_5#523

[deepikabhavnani]

I don't believe Mbed OS currently supports building non-TrustZone ARMv8 targets? I

Yes Mbed OS tools and CMSIS currently assume if it is ARMv8M device, it has trustzone enabled.

--cmse option and cmse_lib.o creation is needed or not for non-trustzone enabled is yet not known to me, I believe it should not be required.

Other things to be considered / updated will will be SAU configuration and bootup sequence which is again part of CMSIS.

If we have mechanism available in CMSIS to disable trustzone completely and not treat it as all secure, we can update tools for the same. I believe additional flags will be added by CMSIS which we should be re-using.

[kjbracey]

Yes Mbed OS tools and CMSIS currently assume if it is ARMv8M device, it has trustzone enabled.

I'm not seeing any huge problems in CMSIS. If you turn off -mcsme on the compiler, I believe that deactivates __ARM_FEATURE_CMSE which deactivates all the "I'm a secure library" code in CMSIS, and having DOMAIN_NS=0 deactivates all the "I'm using a secure library" logic, and what's left then is basically a plain non-TrustZone image.

Target system init code might need to poke a few SAU registers to get them into safe state (disabled), but that's all, I think. There might need to be a separate "security present" flag for the device - or is that __SAUREGION_PRESENT?

[deepikabhavnani]

I'm not seeing any huge problems in CMSIS.

Currently we do -mcmse and DOMAIN_NS selection based on core.
-mcmse is set if core==Cortex-M33 (secure) and DOMAIN_NS is set based if core==Cortex-M33-NS

We do not have option to set both as false ( -mcmse - disabled and DOMAIN_NS=0) from core or any other option. Should we have another core setting? Or some other way to set no trustzone?

[deepikabhavnani]

CMSIS repo will not know MBED_TZ_DEFAULT_ACCESS

MBED_TZ_DEFAULT_ACCESS is not correct flag for this setting.. This flag is to set the default access of Trust zone in threads.

mbed-os/rtos/Thread.h

Line 77 in 48232be

* MBED_TZ_DEFAULT_ACCESS (default:0) flag can be used to change the default access of all user threads in non-secure mode.

[deepikabhavnani]

Target system init code might need to poke a few SAU registers to get them into safe state (disabled), but that's all, I think. There might need to be a separate "security present" flag for the device - or is that __SAUREGION_PRESENT?

__SAUREGION_PRESENT is for SAU present or not, and is set in device specific header file before including "core and syetem" header files.

Another way to achieve this can be to have No -TrustZone as different target with core set as secure, and disable __SAU_PRESENT and __SAUREGION_PRESENT settings in target header file

[kjbracey]

We do not have option to set both as false ( -mcmse - disabled and DOMAIN_NS=0) from core or any other option. Should we have another core setting? Or some other way to set no trustzone?

If sticking with a suffix on core name, then I would do:

• "core-S" to mean "secure library" (-mcmse)
• "core-NS" to mean "non-secure image" (DOMAIN_NS=1)
• "core" to mean plain non-TrustZone image

That seems relatively easily maintainable and doesn't totally change the current system - you can detect and strip the suffix for the special handling before looking at what CPU to pass to the tools. And it removes any explicit architecture version dependency, unless you want to have Python to print a nice user warning for "Cortex-M4-S", rather than letting the compiler barf at the combo.

Alternatively, you might switch to what the PSA stuff does - one extra-label (or component?) to say you're using it at all, and then 2 other labels to designate the secure or non-secure half.

Don't have a strong feeling either way on those two big alternatives. I guess it's what works out best with the tooling.

__SAU_REGION

That a thought for any changes that might be needed in CMSIS - how you might be stuffed if you turn -mcmse off because it deactivates some registers you might still want to poke. Discussion continuing at ARM-software/CMSIS_5#523

[cyliangtw]

MBED_TZ_DEFAULT_ACCESS is for thread tz_module default value in Thread.cpp if creator not assign tz_module, it's useful to run existing mbed samples on V8M.
In rtx_thread.c, rely on caller to assign ThreadAttr in svcRtxThreadNew(...) in NS domain, so it's possible to create some threads for invoking NSC and some threads for pure monitor task in NS application. Thus, the combination of turn on ThreadAttr.tz_module, MBED_TZ_DEFAULT_ACCESS=0 & DOMAIN_NS=1 is possible and it needs tz_context switch.

[kjbracey]

Indeed, MBED_TZ_DEFAULT_ACCESS is a setting specific to NS builds in a TrustZone image, and can be either way then - this PR change is not valid.

The real issue is here #9460, but discussion seems to have continued on this PR.

Maybe close this, and transfer/continue discussion there?

[jeromecoutant]

Thx Kevin