Additions to TF-M source integration #9772

mikisch81 · 2019-02-20T10:14:30Z

Description

Add common TF-M mbed_lib.json in addition to the secure-side mbed_lib.json.
Add TF-M defines to mbed_app.json.
Update list of SHAs in TF-M importer json.
Add image signing scripts from TF-M bl2 library, which is used by targets ported to TF-M
- These scripts are used by Musca_a1 (and probably) more targets which will use TF-M as their Secure Kernel.

Pull request type

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

Reviewers

@orenc17

Release Notes

ciarmcom · 2019-02-20T12:00:40Z

@mikisch81, thank you for your changes.
@orenc17 @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2019-02-20T13:29:54Z

components/TARGET_PSA/TARGET_TFM/mbed_lib.json

+            "value": 20
+        },
+        "message_pool_size": {
+            "help": "maximum number of RoT services allowed",


help - typo? same as previous one

bridadan · 2019-02-20T14:41:17Z

components/TARGET_PSA/TARGET_TFM/COMPONENT_SPE/mbed_lib.json

-        }
-    }
+    "name": "tfm-s",
+    "macros": ["MBED_FAULT_HANDLER_DISABLED", "BYPASS_NVSTORE_CHECK=1"]


Is it appropriate to disable the fault handler in a library? I looked for a justification for this change in the commit message but I didn't find one.

when building TFM we are not using parts of mbed-os
Some of those parts are the fault handlers, and this is because TFM implements their own

Good enough for me!

orenc17 · 2019-02-20T18:41:37Z

@ARMmbed/mbed-os-maintainers can this start CI please ?

mikisch81 · 2019-02-20T18:43:35Z

@orenc17 let's wait for @bridadan approval

bridadan · 2019-02-20T18:48:17Z

components/TARGET_PSA/TARGET_TFM/COMPONENT_SPE/mbed_lib.json

-        }
-    }
+    "name": "tfm-s",
+    "macros": ["MBED_FAULT_HANDLER_DISABLED", "BYPASS_NVSTORE_CHECK=1"]


Good enough for me!

cmonr · 2019-02-20T19:16:59Z

CI started

Please rereview. Changes applied.

mbed-ci · 2019-02-20T21:03:00Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 1
Build artifacts

NirSonnenschein · 2019-02-20T21:06:12Z

@0xc0170 , this has now passed CI, please re-review the fix for your comments so this can be "ready"

orenc17 · 2019-02-20T21:22:34Z

@0xc0170 , this has now passed CI, please re-review the fix for your comments so this can be "ready"

@cmonr i think you got competition for the MOTR award

cmonr · 2019-02-21T00:39:41Z

components/TARGET_PSA/TARGET_TFM/mbed_lib.json

+    ],
+    "config": {
+        "level": {
+            "help": "TFM security level",


What is this suppose to mean?

This is one TFM properties
security level is corresponding with PSA memory separation level desired

This is an internal TFM config, and a prep for the future when we will have a greater level of memory separation.

mikisch81 · 2019-02-21T13:27:15Z

Added another commit which was cherry-picked from #9221.
It adds image signing scripts from TF-M bl2 library which are used by Musca_a1 post-build hook.
Because more targets which will use TF-M as their Secure Kernel may use these scripts I moved the commit here in order to remove dependency on the Musca PR.
@bridadan can you re-review?

bridadan

It's generally ok since I've already reviewed these files. However, my understanding is these scripts are scraping addresses from a flash_layout.h file. We do not scrape info from source files in Mbed OS. We use the configuration system for this and then generate the macros that can be used in the code base. If the intention is to reuse these scripts across targets, I think this needs to be revisited in the future.

bridadan · 2019-02-21T14:43:03Z

tools/psa/tfm/bin_utils/imgtool.py

+            if m is not None:
+                ramLoadAddress = int(m.group(1), 0)
+                print("**[INFO]** Writing load address from the macro in "
+                      "flash_layout.h to the image header.. "


In the future, if this script is going to be reused by other targets, we should be moving "configuration" into the targets.json/mbed_lib.json, not scraping it from the flash_layout.h file.

I absolutely agree, this is something we are aware of and will try to use this in the Musca PR

@mikisch81 @orenc17 Can y'all make and link a Jira issue so that we don't lose this conversation?

We'll start CI in the meanwhile.

@cmonr @bridadan @orenc17 @alzix Created IOTSEC-1104 for that.

- Link to bug tracking: https://developer.trustedfirmware.org/T224

cmonr · 2019-02-21T16:28:17Z

CI started

mbed-ci · 2019-02-21T20:46:07Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 2
Build artifacts

mikisch81 changed the title ~~Tfm extras~~ Additions to TF-M source integration Feb 20, 2019

orenc17 approved these changes Feb 20, 2019

View reviewed changes

ciarmcom requested review from orenc17 and a team February 20, 2019 12:00

ciarmcom added the needs: review label Feb 20, 2019

NirSonnenschein added the release-version: 5.12.0-rc1 label Feb 20, 2019

mikisch81 mentioned this pull request Feb 20, 2019

Add ARM_MUSCA_A1 target #9221

Merged

0xc0170 previously requested changes Feb 20, 2019

View reviewed changes

bridadan reviewed Feb 20, 2019

View reviewed changes

orenc17 approved these changes Feb 20, 2019

View reviewed changes

bridadan approved these changes Feb 20, 2019

View reviewed changes

cmonr reviewed Feb 21, 2019

View reviewed changes

bridadan approved these changes Feb 21, 2019

View reviewed changes

Michael Schwarcz and others added 4 commits February 21, 2019 17:38

TF-M patch: More fixes to tfm_memory_check issue (TF-M issue #224)

6354b9a

- Link to bug tracking: https://developer.trustedfirmware.org/T224

TFM: Add defines to mbed_lib.json

8800ad5

TFM Importer: Update SHAa of TFM patches

d2c433c

Add image signing scripts from TF-M bl2 library

7016ac7

cmonr added needs: CI and removed needs: review labels Feb 21, 2019

cmonr merged commit 77591fb into ARMmbed:master Feb 21, 2019

cmonr removed the needs: CI label Feb 21, 2019

mikisch81 deleted the tfm_extras branch February 25, 2019 15:38

Additions to TF-M source integration #9772

Additions to TF-M source integration #9772

Uh oh!

Conversation

mikisch81 commented Feb 20, 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!

ciarmcom commented Feb 20, 2019

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!

orenc17 commented Feb 20, 2019

Uh oh!

mikisch81 commented Feb 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmonr commented Feb 20, 2019

Uh oh!

mbed-ci commented Feb 20, 2019

Uh oh!

NirSonnenschein commented Feb 20, 2019

Uh oh!

orenc17 commented Feb 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikisch81 commented Feb 21, 2019

Uh oh!

bridadan 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!

cmonr commented Feb 21, 2019

Uh oh!

mbed-ci commented Feb 21, 2019

Uh oh!

Uh oh!

mikisch81 commented Feb 20, 2019 •

edited

Loading