Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additions to TF-M source integration #9772

Merged
merged 4 commits into from Feb 21, 2019

Conversation

Projects
None yet
8 participants
@mikisch81
Copy link
Contributor

commented Feb 20, 2019

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

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

@ciarmcom ciarmcom requested review from orenc17 and ARMmbed/mbed-os-maintainers Feb 20, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

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

components/TARGET_PSA/TARGET_TFM/mbed_lib.json Outdated
"value": 20
},
"message_pool_size": {
"help": "maximum number of RoT services allowed",

This comment has been minimized.

Copy link
@0xc0170

0xc0170 Feb 20, 2019

Member

help - typo? same as previous one

This comment has been minimized.

Copy link
@orenc17

orenc17 Feb 20, 2019

Contributor

fixed

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

This comment has been minimized.

Copy link
@bridadan

bridadan Feb 20, 2019

Contributor

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.

This comment has been minimized.

Copy link
@orenc17

orenc17 Feb 20, 2019

Contributor

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

This comment has been minimized.

Copy link
@bridadan

bridadan Feb 20, 2019

Contributor

Good enough for me!

@orenc17 orenc17 force-pushed the kfnta:tfm_extras branch Feb 20, 2019

@orenc17

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

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

@mikisch81

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2019

@orenc17 let's wait for @bridadan approval

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

This comment has been minimized.

Copy link
@bridadan

bridadan Feb 20, 2019

Contributor

Good enough for me!

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

CI started

Please rereview. Changes applied.

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 20, 2019

Test run: SUCCESS

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

@NirSonnenschein

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

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

@orenc17

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

@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

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

This comment has been minimized.

Copy link
@cmonr

cmonr Feb 21, 2019

Contributor

What is this suppose to mean?

This comment has been minimized.

Copy link
@orenc17

orenc17 Feb 21, 2019

Contributor

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

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

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?

@mikisch81 mikisch81 force-pushed the kfnta:tfm_extras branch Feb 21, 2019

@bridadan
Copy link
Contributor

left a comment

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.

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.. "

This comment has been minimized.

Copy link
@bridadan

bridadan Feb 21, 2019

Contributor

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.

This comment has been minimized.

Copy link
@mikisch81

mikisch81 Feb 21, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@cmonr

cmonr Feb 21, 2019

Contributor

@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.

This comment has been minimized.

Copy link
@mikisch81

mikisch81 Feb 21, 2019

Author Contributor

@mikisch81 mikisch81 force-pushed the kfnta:tfm_extras branch to 7016ac7 Feb 21, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2019

CI started

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

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 21, 2019

Test run: SUCCESS

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

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

27 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 10156 cycles
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the needs: CI label Feb 21, 2019

@mikisch81 mikisch81 deleted the kfnta:tfm_extras branch Feb 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.