search

ARM-software / CMSIS_5

CMSIS Version 5 Development Repository
http://arm-software.github.io/CMSIS_5/index.html
Apache License 2.0
1.34k stars 1.08k forks source link

Cortex-M33 without Trust Zone feature #523

Closed jeromecoutant closed 5 years ago

jeromecoutant commented 5 years ago

Hi

I would like to use a M33 target in the same way as "legacy" V7 targets, i.e. without Trust Zone enabled.

It seems it is not yet possible: https://github.com/ARM-software/CMSIS_5/blob/2d37fc49bcfbca9c5bd274dcac78ad9bedbb4d11/CMSIS/RTOS2/RTX/Source/rtx_kernel.c#L85

Is it possible to add something like:

- #if (DOMAIN_NS == 1)
+ #if (DOMAIN_NS == 1) && (TZ_Enabled == 1)

Regards,

JonatanAntoni commented 5 years ago

Hi @jeromecoutant,

Fair point. Thanks for raising this. Please give me some time to discuss this issue.

Cheers, Jonatan

jeromecoutant commented 5 years ago

@deepikabhavnani

kjbracey commented 5 years ago

DOMAIN_NS is surely 0 in a non-TrustZone build. The entire image is secure.

jeromecoutant commented 5 years ago

OK, so issue is on MBED side, which always set DOMAIN_NS in case of M33 ?

DOMAIN_NS should be set to 0 or 1 in case of M33 and TZ enabled

kjbracey commented 5 years ago

DOMAIN_NS should be 1 if you are the non-secure code in a TrustZone image - if you don't have full control of the device and are relying on the secure code.

If you are the secure code, or you don't have TrustZone, so you have full access, it should be 0.

deepikabhavnani commented 5 years ago

DOMAIN_NS is surely 0 in a non-TrustZone build. The entire image is secure.

DOMAIN_NS is 1 for non-secure build, else it will be zero. But we do not have any flag / option to disable security extension of v8M (Trustzone). @jeromecoutant is looking for a way to disable security extension.

kjbracey commented 5 years ago

You don't need to disable the security extension - you just need to not use it. Then the image works either on a chip that doesn't have TrustZone, or it works by just running everything in Secure state.

I'm raising a new issue with the requirement and my suggested approach for the build.

JonatanAntoni commented 5 years ago

@RobertRostohar please review.

kjbracey commented 5 years ago

Looking through the code, I think it all nearly falls out if:

So the three modes of operation are

What may not quite work is that some of the register map stuff in core_xxx.h is switched on __ARM_FEATURE_CMSE, rather than some sort of __SECURITY_PRESENT. That's basically the same distinction you already have between __FPU_PRESENT and __FPU_USED. (I suppose most security-related registers and APIs could actually be __SECURITY_PRESENT && DOMAIN_NS == 0 if known to be inaccessible from NS state).

The non-TrustZone image by definition should be steering clear of any security stuff, as it's supposed to work on a chip without security extensions, but startup code might want to safe certain security registers that have implementation-defined init values if they're present, so might need the definitions.

jeromecoutant commented 5 years ago

Hi Any updates ? Thx

JonatanAntoni commented 5 years ago

Hi @jeromecoutant,

I think @kjbracey-arm already pointed out that what we have in place should be enough to solve your issue.

Simply keep DOMAIN_NS=0 and compile your code as usual. That should work. Please let us know if you face any issues with that solution.

Cheers, Jonatan

kjbracey commented 5 years ago

I think you do still have a minor issue:

What may not quite work is that some of the register map stuff in core_xxx.h is switched on ARM_FEATURE_CMSE, rather than some sort of SECURITY_PRESENT.

But I don't think that affects Jerome immediately - his problem is getting Mbed OS to do that DOMAIN_NS=0 build. It may be that Mbed OS will hit the above in some init code.

Mbed OS issue being tracked here: https://github.com/ARMmbed/mbed-os/issues/9460

JonatanAntoni commented 5 years ago

Hi @kjbracey-arm,

The core_cm33.h provides access to certain secure TrustZone registers only with __ARM_FEATURE_CMSE==3 such as SAU and some NS bankend registers. These registers should not be needed when running in legacy compliant mode.

If you need access to them, why not compile with --cmse?

Cheers, Jonatan

kjbracey commented 5 years ago

why not compile with --cmse?

To stop the compiler generating any CMSE code, and to fault any attempt of the code to use the security extensions, as we're supposed to be generating a non-TrustZone image.

It seems reasonable to me to treat this like DSP or FPU - whether a chip has the registers is an invariant feature of the chip, even if you've told the compiler not to use CMSE instructions. We may still want to look at the registers in init code or fault handling code.

So I'd suggest follow the exact model you have for FPU - the chip definition has to state __CMSE_PRESENT, you detect __CMSE_USED from the __ARM_FEATURE_CMSE compiler predefine, and you fault if you detect __CMSE_USED && !_CMSE_PRESENT.

Forcing CMSIS users to use -mcmse as a proxy for __CMSE_PRESENT - forcing it on for every ARMv8 build - rules out them using it themselves to determine __CMSE_USED. And rather defeats the point of having it be a separate switch in the first place.

jeromecoutant commented 5 years ago

Is there still some internal discussion in CMSIS team about this issue ?

JonatanAntoni commented 5 years ago

Hi @jeromecoutant,

yes, we discuss about adding __CMSE_PRESENT and _CMSE_USED like we have it for other processor features (like FPU and DSP).

To be honest I am yet unsure what that might be really good for. Using an Armv8-M with Security Extensions (e.g. CM33) without actually using the TrustZone segregation (i.e. legacy compliance mode) sounds valid. But why would one need to access Security Extension registers (e.g. SAU or NS-banked) in such a scenario?

To be fair, one could argue the same for the other CPU features.

A possible drawback might be that all device header files for v8-M devices would need to be updated, i.e. the new define needs to be added.

Cheers, Jonatan

kjbracey commented 5 years ago

But why would one need to access Security Extension registers (e.g. SAU or NS-banked) in such a scenario?

Maybe some sort of initialisation, eg resetting registers to defaults in a main image, just in case a bootloader has been messing? Or possibly crash dump code?

But those are just hypotheticals - I don't currently have a real need to access those registers in a non-TZ image.

To be fair, one could argue the same for the other CPU features.

Indeed. I think consistency with the way you're handling the other CPU features is a large part of why I'm pushing it - I do quite like the way they're working, and it just feels like the SAU is being inconsistent.

kjbracey commented 5 years ago

I guess there's also the minor point about a cross-check now being possible - #include "cmsis.h" could detect "CMSE enabled in compiler but chip does not support CMSE" like it does for FPU.

Although whether that's a likely error we need to be pointing out to people I don't know - seems less likely than accidentally compiling FP code.

jeromecoutant commented 5 years ago

Using an Armv8-M with Security Extensions (e.g. CM33) without actually using the TrustZone segregation (i.e. legacy compliance mode) sounds valid

Yes, new STM32 MCU will provide a the latest development for performance, power and security. Some customers could be only interested in 2, excluding security.

A possible drawback might be that all device header files for v8-M devices would need to be updated, i.e. the new define needs to be added.

Using security or not should not be part of CMSIS configuration... it should be part of user application configuration.

JonatanAntoni commented 5 years ago

@jeromecoutant,

Using TrustZone or not is always up to the user by giving --cmse on the Compiler command line or not. This discussion is about introducing __CMSE_PRESENT and __CMSE_USED to have similar compile time checks like we already have for MPU and FPU.

@kjbracey-arm, I think __CMSE_PRESENT would in fact give us nothing more than a sanity check. You need to actually make use of CMSE (i.e. --cmse) if you intend to access Security Extension registers. If we would enable the access functions not only for __CMSE_USED but globally for __CMSE_PRESENT even a non-secure application gets visibility of them. But using them from non-secure mode would fail at run-time.

My conclusion: The defines we have today gives you all the options a user might need. It lacks the sanity check, i.e. compiling with --cmse for a target device without security extensions. But the access to TrustZone features would always be limited to the case where CMSE is actually used (i.e. its present and activated).

Cheers, Jonatan

kjbracey commented 5 years ago

You need to actually make use of CMSE (i.e. --cmse) if you intend to access Security Extension registers.

--cmse activates the language extensions in the compiler. I don't need to activate those language extensions to access any chip config, just as I don't need to activate any FPU/SIMD stuff in the compiler to manipulate the FPU. If all I want to do is reset registers to default state, or dump them, I don't need --cmse on.

And turning --cmse on could cause a whole bunch of other effects in the build (from code looking at __ARM_FEATURE_CMSE, and using it to say assume that we're building a secure library).

Still, this need to reset/dump registers is only theoretical, so I'm fine with leaving this for now.

globally for __CMSE_PRESENT even a non-secure application gets visibility of them

You could gate on __CMSE_PRESENT && !DOMAIN_NS to force the compile-time check.

JonatanAntoni commented 5 years ago

@kjbracey-arm,

You're right, strictly speaking. Please be aware that DOMAIN_NS is not a CMSIS configuration macro but an RTX5 specific one. I am not aware that there is any automatic way of deducing the target security domain.

We do the same for FPU functions, i.e. if FPU is not used the calls to __get_FPSCR and __set_FPSCR are removed. The drawback is that one cannot access these registers without using the FPU. We could leave these functions in if FPU is available.

For CMSE stuff we would need to create yet another user config macro that tells us the target security domain, i.e. __CMSE_DOMAIN make DOMAIN_NS a CMSIS standard define. But this one needs to be set manually by the user because we cannot automatically deduce its value.

Cheers, Jonatan

jeromecoutant commented 5 years ago

So do you consider we should close this issue? or do you think few updates are needed at CMSIS level ?

JonatanAntoni commented 5 years ago

@jeromecoutant,

Actually I am a bit puzzled. We could of course add new configuration macros to CMSIS. But this is only half the way through. All the device header files for Armv8-M devices needs to reflect this change.

What do like to see in CMSIS from device vendor point of view?

Cheers, Jonatan

jeromecoutant commented 5 years ago

Hi all

After discussion with @JonatanAntoni and @kjbracey-arm it has been concluded that current CMSIS should be OK to build a M33 target without TZ.

Thx