TF-M v1.2 integration #14187

jainvikas8 · 2021-01-22T11:32:49Z

Summary of changes

PR related to upgrading TF-M v1.1 --> v1.2

The following have been changed:

Switch from TFM-v1.1 to v1.2
Upgrade platform/FEATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_TFM/ related files
Update post binary hook scripts for MUSCA_B1 and S1 targets
Update partition and region_def for MUSCA_B1 and S1 targets
Checkin new secure binaries for MUSCA_B1 and S1 targets
Refactor code
Remove MUSCA_A1 target support.

Impact of changes

Migration actions required

Documentation

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@Patater @urutva @evedon @LDong-Arm @0xc0170

ciarmcom · 2021-01-22T12:00:23Z

@jainvikas8, thank you for your changes.
@evedon @Patater @0xc0170 @LDong-Arm @urutva @ARMmbed/mbed-os-maintainers please review.

ciarmcom · 2021-01-22T13:00:23Z

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

LDong-Arm · 2021-01-22T17:33:12Z

Preceding PR: #14160

LDong-Arm

Looks good to me. Will approve once the license headers of the imported scripted are resolved and it's rebased on #14160.

...m/FEATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_TFM/TARGET_TFM_V1_2/include/psa/crypto_extra.h

targets/TARGET_ARM_SSG/TARGET_MUSCA_B1/partition/region_defs.h

jeromecoutant · 2021-01-25T16:04:41Z

Hi

platform/FEATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_TFM/TARGET_TFM_V1_2

Do you need to keep such path...?

platform/FEATURE_PSA/TARGET_TFM should be enough....?

LDong-Arm · 2021-01-25T16:14:30Z

Hi

platform/FEATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_TFM/TARGET_TFM_V1_2

Do you need to keep such path...?

platform/FEATURE_PSA/TARGET_TFM should be enough....?

@evedon @Patater Is PSA still experimental in Mbed OS? Or shall we take the suggestion (not in this PR though)?

jainvikas8 · 2021-01-26T11:04:36Z

The frozen files don't allow the Travis to continue after its failure...

These are the results when they are not part of https://travis-ci.com/github/ARMmbed/mbed-os/builds/214540183

Failure: Frozen files were modified
tools/targets/ARM_MUSCA.py
tools/targets/__init__.py
tools/targets/musca_b1-root-rsa-3072.md
tools/targets/musca_s1-root-rsa-3072.md
Please see https://os.mbed.com/blog/entry/Introducing-the-new-Mbed-Tools/ 
for why we've frozen the legacy tools.

Are been modified, the Travis tracks them as frozen files as they are part of Mbed CLI 1

jainvikas8 · 2021-01-26T12:57:01Z

Forced pushed:

Rebased on new MbedTLS changes - PR - Update Mbed TLS to 2.24.0 and Mbed PSA service #14160

LDong-Arm

The rebase and license headers look good since my previous review.

LDong-Arm · 2021-01-26T15:59:42Z

#14160 has been merged, feel free to rebase again onto feature-tf-m-1.2-integration

Pull request has been modified.

jainvikas8 · 2021-01-27T13:56:31Z

Addressed comments from @urutva regarding error handling and using TARGET_TFM_LATEST

LDong-Arm

LGTM

urutva · 2021-01-27T15:13:52Z

@jainvikas8 Changes look good, following commits can be squashed together,

14612e474dc55a9a14f86a32c975fdafc7dd71b3 and 7dcc2726a878a05d879702c952fd2d210a604762
6d5a4653b6d37636700658a5a807ce3a87ce8d12 and 6a08b1731e175f0cf4ab6467f6c288f5c26431d5

jainvikas8 · 2021-01-27T15:38:04Z

14612e4 and 7dcc272

Squashing the above is going to create an issue, as there are lot of files added here in the following commits abe35bb and 9e13d99

Though I have moved 7dcc272 up the order, if that needs more work, then please let me know

6d5a465 and 6a08b17

I have squashed 6a08b17 with refactoring code commit

Rather than maintaining a specific `TARGET_TFM_V1_x`, its better to use more generic name `TARGET_TFM_LATEST` to avoid confusion on the latest TFM version supported by Mbed OS * Rename the folder from `TARGET_TFM_V1_1` to `TARGET_TFM_LATEST` * Update the CmakeLists.txt * Change the name of the MUSCA targets to maintain uniformity with TF-M v1.2 * Update target.json for PSA_V8_M to use `TFM_LATEST`

These files have been imported/copied from: * ARMmbed/trusted-firmware-m * ARMmbed/tf-m-tests These are generic files, which are required for TF-M v1.2 integration with Mbed OS for PSA_V8M and PSA_DUAL_CORE targets.

The PSA headers imported from TF-M does not contain a declaration of mbedtls_ecc_group_to_psa(), which is expected by pk.c from Mbed TLS. This leads to an "undefined symbol" error when using the ARM toolchain to compile an application for a TF-M target.

* Partition files are synced with TF-M v1.2 * To have uniformity with TF-M v1.2, rename the following: ** image_macros_preprocessed_ns.c to `signing_layout_ns.c` ** image_macros_preprocessed_s.c to `signing_layout_s.c` * `MCUBOOT_IMAGE_NUMBER` is set to 2 by default for TF-M v1.2, therefore it is necessary that Mbed OS compiles the right macros for when linking and using the partition files ** Workaround ** The `region_defs.h` has an explicit definition of `BL2`, even though it is already defined in target.json for `ARM_MUSCA_B1`. This is because of Mbed CLI 1, as it can't seem to use the right macro when linking the files for Mbed OS application when using the ARMCLANG toolchain.

* Remove the old `mcuboot.bin` * Add `bl2.bin` * Update TF-M secure binaries GCC_ARM toolchain used.

The script changes are required with respect to TF-M v1.2 integration for this target. The imgtool.py is been replaced with `wrapper.py` which uses click command to run the signing algorithm. The version `-v` and dependencies `-d` have been updated to resolve upgrade issues from TF-M v1.1 --> v1.2

* Partition files are synced with TF-M v1.2 * To have uniformity with TF-M v1.2, rename the following: ** image_macros_preprocessed_ns.c to `signing_layout_ns.c` ** image_macros_preprocessed_s.c to `signing_layout_s.c` * `MCUBOOT_IMAGE_NUMBER` is set to 2 by default for TF-M v1.2, therefore it is necessary that Mbed OS compiles the right macros for when linking and using the partition files.

* Remove the old `mcuboot.bin` * Add `bl2.bin` * Update TF-M secure binaries GCC_ARM toolchain used.

The script changes are required with respect to TF-M v1.2 integration for this target. The imgtool.py is been replaced with `wrapper.py` which uses click command to run the signing algorithm. The version `-v` and dependencies `-d` have been updated to resolve upgrade issues from TF-M v1.1 --> v1.2

ARM_MUSCA_A1 is not supported since Mbed OS 6.0 Refer: ARMmbed#13165 Therefore remove files from kv_config and TF-M post binary hook script.

Raise an exception if there was an issue with handling any command when signing the binaries for MUSCA targets.

This files are imported and part of: mcu-tools/mcuboot@a8e12da Issue has been raised mcu-tools/mcuboot#930 As these scripts are going to be part of release its important to have licensing information.

jainvikas8 · 2021-01-27T16:22:25Z

Forced pushed to address review comments, please ignore - #14187 (comment)

6d5a465 and 6a08b17

I have squashed 6a08b17 with refactoring code commit

LDong-Arm

LGTM. GitHub shows the final result of the force-push is unchanged, so it should be good.

0xc0170 · 2021-01-27T16:36:35Z

CI restarted

mbed-ci · 2021-01-27T17:08:06Z

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	❌
jenkins-ci/mbed-os-ci_cmake-example-ARM	❌
jenkins-ci/mbed-os-ci_build-greentea-ARM	❌
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	❌
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	❌
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	❌

jainvikas8 · 2021-01-27T17:21:01Z

Excepted CI failures:

mbed-os-example-lorawan CMake build for K64F: recently fixed by 5616537 on master branch, but our feature branch lags behind. This should be automatically resolved once we merge back to master
Examples and Greentea fail to build for CYTFM_064B0S2_4343W targets - Mbed TLS 2.24.0 only supports the PSA version that comes with TF-M 1.2, but this target has TF-M 1.0 and is maintained by Cypress
Examples and Greentea fail to build for Cypress targets, due to a new Mbed TLS API mbedtls_ecp_write_key() missing from Cypress's driver - it needs an update.

Can be merged.

adbridge · 2021-01-29T15:53:22Z

@evedon when merging a PR to a feature branch, if there are outstanding 'needs: ' labels, please remove them.

donatieng · 2021-02-08T11:38:39Z

@evedon @Patater Is PSA still experimental in Mbed OS?

So what is the status ? :-)

We need to review internally if PSA can be moved to "Stable" API. @donatieng

Good question @jeromecoutant, we believe that we still need more developer feedback on the use of this API, so would like to keep it experimental for the time being. There are probably improvements around documentation, tooling and ease of use we'll need to make based on this.

jainvikas8 changed the title ~~Tfm integration v1.2 mbedtls s1 b1~~ TF-M v1.2 integration Jan 22, 2021

mergify bot added release-version: feature branch needs: work labels Jan 22, 2021

ciarmcom requested review from 0xc0170, evedon, LDong-Arm, Patater, urutva and a team January 22, 2021 12:00

ciarmcom added needs: review and removed needs: review needs: work labels Jan 22, 2021

ciarmcom added the needs: review label Jan 22, 2021

LDong-Arm mentioned this pull request Jan 25, 2021

Update Mbed TLS to 2.24.0 and Mbed PSA service #14160

Merged

LDong-Arm reviewed Jan 25, 2021

View reviewed changes

mergify bot added needs: work and removed needs: review labels Jan 25, 2021

evedon reviewed Jan 25, 2021

View reviewed changes

...m/FEATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_TFM/TARGET_TFM_V1_2/include/psa/crypto_extra.h Outdated Show resolved Hide resolved

targets/TARGET_ARM_SSG/TARGET_MUSCA_B1/partition/region_defs.h Show resolved Hide resolved

jainvikas8 mentioned this pull request Jan 25, 2021

Remove workaround for region_defs.h #14193

Open

jainvikas8 force-pushed the tfm-integration-v1.2-Mbedtls-S1-B1 branch from b14e245 to b97d92f Compare January 26, 2021 12:54

LDong-Arm approved these changes Jan 26, 2021

View reviewed changes

jainvikas8 force-pushed the tfm-integration-v1.2-Mbedtls-S1-B1 branch from b97d92f to c5e5487 Compare January 26, 2021 16:17

LDong-Arm approved these changes Jan 27, 2021

View reviewed changes

jainvikas8 force-pushed the tfm-integration-v1.2-Mbedtls-S1-B1 branch from 7dcc272 to d1d28aa Compare January 27, 2021 15:38

jainvikas8 added 13 commits January 27, 2021 16:02

core: Upgrade TFM v1.2 related files

93438fe

These files have been imported/copied from: * ARMmbed/trusted-firmware-m * ARMmbed/tf-m-tests These are generic files, which are required for TF-M v1.2 integration with Mbed OS for PSA_V8M and PSA_DUAL_CORE targets.

Update target ARM_MUSCA_B1 TF-M V1.2 binaries

1a22f71

* Remove the old `mcuboot.bin` * Add `bl2.bin` * Update TF-M secure binaries GCC_ARM toolchain used.

Update target ARM_MUSCA_S1 TF-M V1.2 binaries

b37b08f

* Remove the old `mcuboot.bin` * Add `bl2.bin` * Update TF-M secure binaries GCC_ARM toolchain used.

Remove: ARM_MUSCA_A1 support

df66e25

ARM_MUSCA_A1 is not supported since Mbed OS 6.0 Refer: ARMmbed#13165 Therefore remove files from kv_config and TF-M post binary hook script.

Remove old binary signing scripts of TF-M 1.1

92676b7

Refactor: MUSCA targets post binary hook scripts

e084cf6

Raise an exception if there was an issue with handling any command when signing the binaries for MUSCA targets.

Add missing license header and SPDX identifier

f675ec5

This files are imported and part of: mcu-tools/mcuboot@a8e12da Issue has been raised mcu-tools/mcuboot#930 As these scripts are going to be part of release its important to have licensing information.

jainvikas8 force-pushed the tfm-integration-v1.2-Mbedtls-S1-B1 branch from d1d28aa to f675ec5 Compare January 27, 2021 16:18

LDong-Arm approved these changes Jan 27, 2021

View reviewed changes

urutva approved these changes Jan 27, 2021

View reviewed changes

evedon merged commit 45d9a22 into ARMmbed:feature-tf-m-1.2-integration Jan 28, 2021

evedon removed the needs: work label Jan 29, 2021

TF-M v1.2 integration #14187

TF-M v1.2 integration #14187

Uh oh!

Conversation

jainvikas8 commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

@Patater @urutva @evedon @LDong-Arm @0xc0170

Uh oh!

ciarmcom commented Jan 22, 2021

Uh oh!

ciarmcom commented Jan 22, 2021

Uh oh!

LDong-Arm commented Jan 22, 2021

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeromecoutant commented Jan 25, 2021

Uh oh!

LDong-Arm commented Jan 25, 2021

Uh oh!

jainvikas8 commented Jan 26, 2021

Uh oh!

jainvikas8 commented Jan 26, 2021

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

Uh oh!

LDong-Arm commented Jan 26, 2021

Uh oh!

jainvikas8 commented Jan 27, 2021

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

Uh oh!

urutva commented Jan 27, 2021

Uh oh!

jainvikas8 commented Jan 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jainvikas8 commented Jan 27, 2021

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jan 27, 2021

Uh oh!

mbed-ci commented Jan 27, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

jainvikas8 commented Jan 27, 2021

Uh oh!

adbridge commented Jan 29, 2021

Uh oh!

donatieng commented Feb 8, 2021

Uh oh!

Uh oh!

jainvikas8 commented Jan 22, 2021 •

edited

Loading

jainvikas8 commented Jan 27, 2021 •

edited

Loading