Permit non-TrustZone ARMv8 build #10520

kjbracey · 2019-05-02T11:24:55Z

Description

Change the heuristic for selection of CMSE in the tools python, so that a non-TrustZone ARMv8 build can happen.

Ideally we would have more direct flagging in the targets, but this refines the heuristic so the necessary behaviour can be easily achieved.

DOMAIN_NS=1 is based purely on the -NS suffix on the core name.
Enabling CMSE in the compiler and outputting a secure import library is now enabled when the core doesn't have an -NS suffix by either the target label TFM being present or the flag trustzone being set.

This covers the existing ARMv8-M behaviour - TF-M builds have the TFM label, as per its documentation; M2351 secure builds have no explicit flagging, so we ensure that the M2351_NS target has the trustzone flag set, and the out-of-tree secure target inherits that.

Replacement for #10514, hopefully fixes #9460

Marked it as "fix", as I think it's quite reasonable to regard not being able to make non-TrustZone builds for current ARMs as a bug, although arguably it's a functionality change.

Pull request type

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

Reviewers

@jeromecoutant, @alzix, @bridadan, @bulislaw

Release Notes

Non-TrustZone ARMv8 targets now supported (core must be set to Cortex-M33 rather than Cortex-M33-NS).

alzix · 2019-05-02T11:50:07Z

tools/targets/__init__.py

+
+    @property
+    def is_TrustZone_non_secure_target(self):
+        return self.core.endswith('-NS')


Suggested change

return self.core.endswith('-NS')

return not self.is_TrustZone_secure_target

No, because it's not the opposite of secure - there are 3 cases:

TrustZone secure target

TrustZone non-secure target

non-TrustZone target (neither of the above)

(I believe that's the same as for PSA)

kjbracey · 2019-05-02T11:59:55Z

I've (finally) realised, after sufficient prodding by @jeromecoutant, this still doesn't cover the M2351 properly, as we do need to support a secure build for it - it's out of-tree, so I missed it.

https://github.com/OpenNuvoton/NuMaker-mbed-TZ-secure-example

So we just need to do something to get is_TrustZone_secure_target true for that. One of Jerome's suggestions was to look for MBED_TZ_DEFAULT_ACCESS=1 in the macros, and I can't see anything better, if our assumption is "change Python only, no change to targets JSON, in-tree or out".

ciarmcom · 2019-05-02T13:00:46Z

@kjbracey-arm, thank you for your changes.
@bulislaw @bridadan @alzix @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

kjbracey · 2019-05-02T13:04:24Z

tools/toolchains/arm.py

-                    kwargs.get('build_dir', False)):
-                # Create Secure library
+        if target.is_TrustZone_secure_target:
+            if kwargs.get('build_dir', False):


Do we really need this "if"? It isn't there in GCC and IAR versions.

kjbracey · 2019-05-02T13:08:00Z

Revised to now cover M2351, which did mean adding a specific flag in targets.json.

Adjusted TF-M documentation to not say "ARMv8-M" so much - the behaviour it's documenting is now activated by the presence of the "TFM" flag it says to add.

I don't believe we have any specific documentation about making non-TF-M TrustZone images like M2351 is doing. If there is, that would now need to state that it's the trustzone flag that needs to be turned on, rather than it being automatic because it's an ARMv8-M chip.

jeromecoutant · 2019-05-02T13:15:35Z

tools/targets/__init__.py

+    # having trustzone = false (default), no TFM flag, and no -NS suffix.
+    @property
+    def is_TrustZone_secure_target(self):
+        return (self.trustzone or 'TFM' in self.labels) and not self.core.endswith('-NS')


You could add
"trustzone": true,
in SPE_Target, and remove this TFM check here ?

Sadly no, because some SPE_Targets are not using TrustZone. See CY8CKIT_062_WIFI_BT_M0_PSA.

Dual-CPU set-up with the secure and non-secure images running on different cores with message-passing.

I could add trustzone to all the ARMv8-M SPE_Targets individually, but that would need to be added to the documentation of creating a TF-M target, and I'm wary about breaking some out-of-tree target I haven't seen. This ensures that the existing instructions work.

And I'm not going to make it "SPE_Target and core arch >= 8", because there's no reason you can't run the same dual-cpu PSA setup with an M23 and M33 - if those guys want a slight CPU performance upgrade from M0+M4 without a complete system rearchitecture.

I guess I'm also wary of documenting the trustzone flag as part of the TF-M instructions, if we think it might be subject to change. It's part of this being the "quick" fix - avoiding discussions about what the targets.json should look like.

If we're willing to commit to it, I could add it, so that the "detect TFM label" is then clearly legacy support - TFM targets are supposed to also have the trustzone flag set properly.

If you don't add trustzone to true in SPE_Target,
maybe we got a strange situation where trustzone value is kept to the default value, i.e. false?

Yes, and that's strange if it is a TFM build. (Not if it isn't).

But the trustzone value is not directly exposed anywhere. The Target.is_TrustZone_xxx properties will give the right answer - looking at both that flag and the TFM label.

I can't add the flag to SPE_Target because not all SPE Targets are trustzone.

Although maybe I could and set it back to false in the exception cases, as it's TFM which is the "normal" case? So PSOC6 inherits from SPE_Target and flips trustzone back to false?

(I'm kind of playing games here trying to avoid changing any documentation or existing publicly visible behaviour so this can slip into 5.12.y as a "target update".)

kjbracey · 2019-05-02T13:17:10Z

With respect to @bridadan's core attribute proposal, I don't think this necessarily conflicts - the "trustzone" flag here can be viewed as a property of the build ("used"), rather than a property of the core ("present").

In discussions with the CMSIS guys, I was pointing out that for FPU and other things they do distinguish between "present" and "used", but they don't for CMSE/TrustZone. They're not currently planning to change that, but if they did, then any core attribute would be the "present", and this would be the "used".

We don't necessarily need to lock this form of the trustzone flag in yet - it really only exists for that M2351 build and its out-of-tree secure image, as there's no other reasonable way to make that keep working, and I believe we generally only intend to support TF-M anyway?

Change the heuristic for selection of CMSE in the tools python, so that a non-TrustZone ARMv8 build can happen. Ideally we would have more direct flagging in the targets, but this refines the heuristic so the necessary behaviour can be easily achieved. * DOMAIN_NS=1 is based purely on the `-NS` suffix on the core name. * Enabling CMSE in the compiler and outputting a secure import library is now enabled when the core doesn't have an `-NS` suffix by either the target label `TFM` being present or the flag `trustzone` being set. This covers the existing ARMv8-M behaviour - TF-M builds have the TFM label, as per its documentation; M2351 secure builds have no explicit flagging, so we ensure that the M2351_NS target has the trustzone flag set, and the out-of-tree secure target inherits that.

bulislaw

Would be good to review the portign guide and document the different ways (non-)TrustZone targets can be added.

adbridge · 2019-05-16T08:31:42Z

CI started

adbridge · 2019-05-16T14:18:13Z

@kjbracey-arm Could you take a look at the test failures ?

adbridge · 2019-05-16T14:27:32Z

Seems there have been some CI issues so I am going to restart the ci on this one to see if that fixes things.

mbed-ci · 2019-05-16T14:29:07Z

Test run: FAILED

Summary: 3 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_dynamic-memory-usage
jenkins-ci/mbed-os-ci_greentea-test

mbed-ci · 2019-05-16T19:11:36Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

adbridge · 2019-05-17T08:34:09Z

Re-running exporters

kjbracey force-pushed the build_tz_heuristic branch 2 times, most recently from 1219492 to 4b856de Compare May 2, 2019 11:26

alzix reviewed May 2, 2019

View reviewed changes

jeromecoutant approved these changes May 2, 2019

View reviewed changes

ciarmcom requested review from alzix, bridadan, bulislaw and a team May 2, 2019 13:00

ciarmcom added the needs: review label May 2, 2019

kjbracey force-pushed the build_tz_heuristic branch 2 times, most recently from 49b4551 to 579b9a2 Compare May 2, 2019 13:03

kjbracey commented May 2, 2019

View reviewed changes

jeromecoutant reviewed May 2, 2019

View reviewed changes

kjbracey force-pushed the build_tz_heuristic branch from 579b9a2 to f9362d9 Compare May 3, 2019 08:38

kjbracey force-pushed the build_tz_heuristic branch from f9362d9 to 65e0887 Compare May 3, 2019 10:36

jeromecoutant approved these changes May 6, 2019

View reviewed changes

bulislaw approved these changes May 10, 2019

View reviewed changes

adbridge added needs: CI and removed needs: review labels May 16, 2019

adbridge added the release-version: 5.12.4 label May 16, 2019

adbridge added needs: work and removed needs: CI labels May 16, 2019

adbridge added needs: CI ready for merge and removed needs: work needs: CI labels May 17, 2019

adbridge merged commit 9fb4429 into ARMmbed:master May 17, 2019

adbridge removed the ready for merge label May 17, 2019

kjbracey deleted the build_tz_heuristic branch May 17, 2019 10:27

	return self.core.endswith('-NS')
	return not self.is_TrustZone_secure_target

Permit non-TrustZone ARMv8 build #10520

Permit non-TrustZone ARMv8 build #10520

Uh oh!

Conversation

kjbracey commented May 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kjbracey May 2, 2019 • 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!

kjbracey commented May 2, 2019

Uh oh!

ciarmcom commented May 2, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kjbracey commented May 2, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kjbracey May 2, 2019 • 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!

kjbracey commented May 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bulislaw left a comment

Choose a reason for hiding this comment

Uh oh!

adbridge commented May 16, 2019

Uh oh!

adbridge commented May 16, 2019

Uh oh!

adbridge commented May 16, 2019

Uh oh!

mbed-ci commented May 16, 2019

Uh oh!

mbed-ci commented May 16, 2019

Uh oh!

adbridge commented May 17, 2019

Uh oh!

Uh oh!

kjbracey commented May 2, 2019 •

edited

Loading

kjbracey May 2, 2019 •

edited

Loading

kjbracey May 2, 2019 •

edited

Loading

kjbracey commented May 2, 2019 •

edited

Loading