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

M2354: Support PSA Firmware Update #14972

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Aug 2, 2021

Summary of changes

This PR tries to support PSA Firmware Update for M2354. It has the following major modifications:

  1. Change from single image boot to multiple image boot
  2. SDH is configured to Secure for placing update firmware in SD card. It becomes inaccessible to Mbed.
  3. Post-build script supports both multiple image boot and single image boot
  4. Update readme to reflect above change
  5. Increase forced_reset_timeout (targets.json) due to longer booting time for Greentea test

Impact of changes

  1. Due to change from single image boot to multiple image boot, the number of signing keys changes from one to two, if signing key was changed before.
  2. SDH (SD card) gets inaccessible to Mbed application.

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)

@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Aug 2, 2021
@ciarmcom
Copy link
Member

ciarmcom commented Aug 2, 2021

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

@ccli8
Copy link
Contributor Author

ccli8 commented Aug 3, 2021

@LDong-Arm Same as #14441, this PR changes frozen tools/targets to update post-build script for M2354 for Mbed CLI 1.

1.  Change from single image boot to multiple image boot
2.  SDH is configured to Secure for placing update firmware. It becomes inaccessible to Mbed.
3.  Post-build script supports both multiple image boot and single image boot
4.  Update readme to reflect above change
5.  Increase forced_reset_timeout due to longer booting time for Greentea test
@ccli8 ccli8 force-pushed the nuvoton_m2354_tfm_psa_fwu branch from e37d471 to 5114e4c Compare August 5, 2021 06:14
@LDong-Arm LDong-Arm requested a review from a team August 11, 2021 09:05
@LDong-Arm
Copy link
Contributor

@ARMmbed/mbed-os-core Hopefully we get this in before TF-M v1.4 upgrade.

Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccli8 Thanks for the contribution, please see my comments.

Also, the Pull request type in the PR description should be "Feature update" as it adds a new feature.

```sh
git clone https://github.com/OpenNuvoton/trusted-firmware-m -b nuvoton_mbed_m2354_tfm-1.3
```
$ git clone https://github.com/OpenNuvoton/trusted-firmware-m -b nuvoton_mbed_m2354_tfm-1.3
Copy link
Contributor

@LDong-Arm LDong-Arm Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fork and branch doesn't seem to contain the Firmware Update changes in this PR? Do they come from somewhere else?

We will upgrade TF-M in Mbed OS to v1.4.0 soon, so it would be good to have all patches on one branch, making it easy to rebase them to TF-M v1.4.0 when we need to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I can see your fork of mbed-os-tf-m-regression-tests points to your fork of trusted-firmware-m, which has branches for both TF-M v1.3 and v1.4 with all the Firmware Update patches. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OpenNuvoton TF-M fork is the officially public one for users who want to re-build TF-M for M2354. As this PR lands, the nuvoton_mbed_m2354_tfm-1.3 branch will update accordingly. When TF-M 1.4 is supported in the near future, there will be nuvoton_mbed_m2354_tfm-1.4 created in OpenNuvoton TF-M fork also.

Comment on lines +13 to +16
set(mcuboot_image_number 2)
set(region_defs_h_path "${CMAKE_CURRENT_SOURCE_DIR}/partition/region_defs.h")
set(update_stage_sdh true)
set(update_stage_flash false)
Copy link
Contributor

@LDong-Arm LDong-Arm Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean we support both single and multi image boots, and updating from SD card and flash? Have all of them been tested with Mbed OS? And how will users select which one to use (with both Mbed CLI 1 and 2)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both from SD and from flash are supported and they are both verified with Mbed OS. But due to limited flash size 1MiB on M2354, from flash is not practical. From SD is the default and for public. From flash is for internal development/test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle, we can keep only the default/supported cased here, rather than having code for all cases. But I get the idea of making it flexible for internal development, so I'm personally fine with this.

@ccli8
Copy link
Contributor Author

ccli8 commented Aug 11, 2021

Also, the Pull request type in the PR description should be "Feature update" as it adds a new feature.

@LDong-Arm Updated to "Feature update"

@LDong-Arm
Copy link
Contributor

@ccli8 We will start upgrading TF-M in Mbed OS to v1.4 soon. It would be really helpful if you could raise a PR to "upstream" your mbed-os-tf-m-regression-tests fork to our main repository please? It has just one commit on top of our main repo, so it would be a rather small PR. We can help with selecting different trusted-firmware-m repositories for different targets (i.e. upstream for Musca, Nuvoton fork for M2354) in python scripts during code review.
Because the common TF-M code base imported into mbed-os will move from v1.3 to v1.4 altogether, updating different targets at different times will not be very feasible, so having their support in one place will make it much easier.

Many thanks in advance!

@Patater FYI

Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving tools changes, as the changes are at a low-risk of breaking other targets in the online compiler

@Patater
Copy link
Contributor

Patater commented Aug 11, 2021

CI Started

@mbed-ci
Copy link

mbed-ci commented Aug 11, 2021

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

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

@Patater Patater merged commit 4125fd0 into ARMmbed:master Aug 11, 2021
@mergify mergify bot removed the ready for merge label Aug 11, 2021
@ccli8 ccli8 deleted the nuvoton_m2354_tfm_psa_fwu branch August 12, 2021 00:36
@ccli8
Copy link
Contributor Author

ccli8 commented Aug 12, 2021

@LDong-Arm I will evaluate upstream of M2354 to mbed-os-tf-m-regression-tests and update you soon.

@LDong-Arm
Copy link
Contributor

@LDong-Arm I will evaluate upstream of M2354 to mbed-os-tf-m-regression-tests and update you soon.

Thanks in advance!

@ccli8
Copy link
Contributor Author

ccli8 commented Aug 13, 2021

@mbedmain mbedmain added release-version: 6.14.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants