M2351: Enhance secure/non-secure image build flow for non-PSA target #11288

ccli8 · 2019-08-22T10:13:53Z

Description

This PR is to enhance secure/non-secure image build flow for non-PSA target. Its update is abstracted from #10959 just for the non-PSA part to improve build UX. It is expected that this PR and #10959 are compatible for the build flow. It has the following modifications:

Rename non-PSA target name to NUMAKER_PFM_M2351_NOPSA_S/NS
Support non-PSA secure/non-secure combined build

The above modifications enhance build flow but at the same time cause incompatibility with current one:

The target name NUMAKER_PFM_M2351 is recycled. User needs to change to NUMAKER_PFM_M2351_NOPSA_S/NS to build non-PSA secure/non-secure targets.
Just flash combined secure/non-secure image altogether instead of flash secure image first and then non-secure image.
User can also drop pre-built secure image saved in TARGET_NU_PREBUILD_SECURE directory and provide its own by adding the line below in mbed_app.json instead of removing it through .mbedignore:

    "target.extra_labels_remove": ["NU_PREBUILD_SECURE"]

Pull request type

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

Release Notes

Changes for NuMaker-PFM-M2351:

Change target name to NU_PFM_M2351_NPSA_S for non-PSA secure target and and NU_PFM_M2351_NPSA_NS for non-PSA non-secure target. Original target name NUMAKER_PFM_M2351 is recycled.
Combine secure image and non-secure image into one in non-secure target post-build. With this, user just needs to flash the combined image instead of flash secure image and then non-secure image separately.
To drop pre-built secure image in mbed-os tree and provide custom one, add the line '"target.extra_labels_remove": ["NU_PREBUILD_SECURE"]' into 'mbed_app.json' instead of via .mbedignnore.

ciarmcom · 2019-08-22T13:00:43Z

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

ccli8 · 2019-08-26T07:43:06Z

Any update?

0xc0170 · 2019-08-26T08:07:07Z

Rename non-PSA target name to NUMAKER_PFM_M2351_NOPSA_S/NS

Can this be non breaking change or this target has not been released ?

Please add release notes section above.

ccli8 · 2019-08-26T08:35:37Z

Add Release Notes section.

0xc0170 · 2019-08-26T09:33:07Z

Thanks, what is the impact of this change ? What are we breaking ? My previous question still stand: "Can this be non breaking change or this target has not been released ?"

We should be aware of the scope of breakages. Are there any migration steps needed?

ccli8 · 2019-08-26T09:56:22Z

Can this be non breaking change or this target has not been released ?

This target has been released with target name NUMAKER_PFM_M2351. But to support combined build and further TF-M #10959, target name must be changed. So this PR must be breaking change. The related changes user needs to follow are as Release Notes section describes.

bulislaw · 2019-08-27T15:31:18Z

I'm not big fan of the breaking changes, even if it's only for 1 target. Why do we add nopsa to the target name? That would make me think there's a PSA version. Also what does nu in NU_PREBUILD_SECURE stands for?

@Devran01 @Patater please review

ccli8 · 2019-08-28T01:51:28Z

I'm not big fan of the breaking changes, even if it's only for 1 target. Why do we add nopsa to the target name? That would make me think there's a PSA version.

Yes, there are also PSA targets. #10959 is on-going for it. Actually, there will be four target names for the NuMaker-PFM-M2351 board:

NUMAKER_PFM_M2351_S: Target name for building M2351 TFM secure code
NUMAKER_PFM_M2351_NS: Target name for building M2351 TFM non-secure code
NUMAKER_PFM_M2351_NOPSA_S: Target name for building M2351 non-TFM secure code
NUMAKER_PFM_M2351_NOPSA_NS: Target name for building M2351 non-TFM non-secure code

#10959 is blocked with TF-M integration. This PR is an excerpt of #10959 to support combined build on non-PSA target in advance.

Also what does nu in NU_PREBUILD_SECURE stands for?

For TZ target, prebuilt secure image will place in mbed-os tree. In normal cases, user just needs to build non-secure code. But if user doesn't want the prebuilt secure image and wants to provide his own for e.g. ROM/RAM re-partition on NuMaker-PFM-M2351, he needs to remove the prebuilt one in mbed-os tree. NU_PREBUILD_SECURE is the means for it.

mark-edgeworth · 2019-08-28T08:01:14Z

These target names may be too long for the database that stores them (there is a 20 character limit I believe)

ccli8 · 2019-08-28T08:25:12Z

These target names may be too long for the database that stores them (there is a 20 character limit I believe)

@mark-edgeworth Which database? Can this be released?

madchutney · 2019-08-28T08:41:13Z

targets/targets.json

@@ -8353,8 +8353,7 @@
        "macros_add": ["CMSDK_CM7"],
        "device_has_add": ["MPU"]
    },
-    "NUMAKER_PFM_M2351": {
-        "core": "Cortex-M23-NS",
+    "NUMAKER_PFM_M2351_CM": {


This change needs to be replicated into the online database (please ask you TAM representative) and the platform database in https://github.com/ARMmbed/mbed-os-tools

This change needs to be replicated into the online database (please ask you TAM representative)

OK

the platform database in https://github.com/ARMmbed/mbed-os-tools

The platform name NUMAKER_PFM_M2351 in mbed-os-tools database is kept unchanged for compatibility. Don't want to meet mismatch issue with mbed-os-tools and mbed-os.

I'm not sure what your comments mean, can you explain? The names need the same in all places or the entire build system, testing and reporting will not work.

mbed-os-tools defines the board type when the product/detect code from the board is detected. This means that some systems will attempts to build based on this board type, which will not be present in mbed-os.

@madchutney Originally, I want to keep the M2351 platform name in mbed-os-tools unchanged, so users don't need to care about version match between mbed-os and mbed-os-tools. Now that the name must be consistent across different tools, I will send another PR to mbed-os-tools repo to change platform name to NU_PFM_M2351 from NUMAKER_PFM_M2351 after this PR is confirmed. bcf8cc6 is addressing the target name/platform name conversion for M2351 like other PSA targets.

madchutney · 2019-08-28T08:42:26Z

targets/targets.json

-        "mbed_ram_size"     : "0x10000"
+        "public": false
+    },
+    "NUMAKER_PFM_M2351_NOPSA_NS": {


This name is too long as it also needs to be in the online database (max 20 characters)

madchutney · 2019-08-28T08:42:31Z

targets/targets.json

+        "mbed_ram_start"        : "0x30008000",
+        "mbed_ram_size"         : "0x10000"
+    },
+    "NUMAKER_PFM_M2351_NOPSA_S": {


This name is too long as it also needs to be in the online database (max 20 characters)

mark-edgeworth · 2019-08-28T08:42:58Z

These target names may be too long for the database that stores them (there is a 20 character limit I believe)

@mark-edgeworth Which database? Can this be released?

I'm sorry, no. See the inline review comments for why (@madchutney is responding now).

madchutney · 2019-08-28T08:53:10Z

tools/targets/__init__.py

+        assert os.path.isfile(ns_hex), "Non-secure image %s must be regular file" % s_hex
+
+        # Keep original non-secure before merge with secure
+        ns_nosecure_hex = _ + "_no-secure-merge" + ext


Please don't use _ as variable name, convention says this can be used when a return value is not required but then to actually use it is wrong.

madchutney · 2019-08-28T08:56:37Z

tools/targets/__init__.py

+        t_self.notify.info("Found secure image %s" % s_hex)
+
+        _, ext = os.path.splitext(s_hex)
+        assert ext == ".hex", "Secure image %s must be in Intel HEX format" % s_hex


This assert does not seem to be checking for a programming assertion but rather a test for the file retrieved.

ccli8 · 2019-08-28T09:15:36Z

For target name exceeding 20 characters limited by on-line, I will change to:

NU_PFM_M2351_P_S: Target name for building M2351 TFM secure code
NU_PFM_M2351_P_NS: Target name for building M2351 TFM non-secure code
NU_PFM_M2351_NP_S: Target name for building M2351 non-TFM secure code
NU_PFM_M2351_NP_NS: Target name for building M2351 non-TFM non-secure code

Their base name NUMAKER_PFM_M2351_CM will rename to NU_PFM_M2351_CM.

ccli8 · 2019-08-29T09:41:23Z

Make modifications:

Do rebase
Rename target to NU_PFM_M2351+SUFFIX rather than NUMAKER_PFM_M2351+SUFFIX due to max 20 chars limit in online database
Translate target name NU_PFM_M2351+SUFFIX to platform name NUMAKER_PFM_M2351 (tools/test_api.py), which has registered in mbed-os-tools and keeps unchanged for compatibility.

ccli8 · 2019-09-02T01:33:02Z

Any update?

This will add back immediately after target renaming is done.

1. Create a private target name NU_PFM_M2351_CM which stands for the NuMaker-PFM-M2351 board and is to be extended. 2. NU_PFM_M2351_NPSA_S/NS target names for non-PSA secure/non-secure targets respectively. 3. The original target name NUMAKER_PFM_M2351 is recycled and cannot be used. Use NU_PFM_M2351_S/NS for non-PSA secure/non-secure targets instead. NOTE: Target name doesn't follow the rule below because online database has limit of max 20 chars: NUMAKER_PFM_M2351_PSA/NOPSA_S/NS Instead, it has the rule: NU_PFM_M2351_[NPSA_]S/NS NU_PFM_M2351_S/NS for PSA targets. This is to be consistent with current PSA target naming. So the resolved target names are: NU_PFM_M2351_S : PSA secure target NU_PFM_M2351_NS : PSA non-secure target NU_PFM_M2351_NPSA_S : Non-PSA secure target NU_PFM_M2351_NPSA_NS : Non-PSA non-secure target

Support secure/non-secure combined build for non-PSA target: 1. In secure post-build, deliver built secure image to TARGET_NU_PREBUILD_SECURE directory which is to combine later. 2. In non-secure post-build, merge non-secure image with secure image saved in TARGET_NU_PREBUILD_SECURE directory. 3. In non-secure post-build, user can also drop pre-built secure image saved in TARGET_NU_PREBUILD_SECURE directory and provide its own by adding the line below in mbed_app.json: "target.extra_labels_remove": ["NU_PREBUILD_SECURE"]

NOTE1: USB UART is partitioned for non-secure world. Secure world still can share it with limit that its interrupt cannot use in secure world. NOTE2: In secure world, USB UART is only for Greentea and STDIO. Developers shouldn't use it for other purposes.

This is necessary for exporting M2351 uvision project.

ccli8 · 2019-09-16T05:36:55Z

Make modifications:

Do rebase
Rename targets for M2351: Enhance secure/non-secure image build flow for non-PSA target #11288 (comment). This change requires Convert TZ target name 'NPSA' to test spec platform name #11487 @Devran01

NU_PFM_M2351_S: Target name for building M2351 TFM secure code
NU_PFM_M2351_NS: Target name for building M2351 TFM non-secure code
NU_PFM_M2351_NPSA_S: Target name for building M2351 non-TFM secure code
NU_PFM_M2351_NPSA_NS: Target name for building M2351 non-TFM non-secure code

urutva · 2019-09-16T11:51:32Z

@ccli8 Thanks for the modifications.

0xc0170 · 2019-09-16T12:02:57Z

Rename targets for #11288 (comment). This change requires #11487 @Devran01

As soon as dependency is in, we can restart CI here !

0xc0170 · 2019-09-17T08:04:39Z

CI started

dependency is in!

mbed-ci · 2019-09-17T08:28:44Z

Test run: FAILED

Summary: 2 of 4 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2019-09-17T08:46:12Z

CI restarted

mbed-ci · 2019-09-17T09:35:16Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 5
Build artifacts

0xc0170 · 2019-09-17T09:53:23Z

This is now ready for integration

ciarmcom requested review from Ronny-Liu and a team August 22, 2019 13:00

ciarmcom added the needs: review label Aug 22, 2019

This was referenced Aug 23, 2019

M2351: Integrate ESP8266 power on/off logic into ESP8266 driver main… #11299

Closed

ESP8266: Add configurations for reset control #11331

Closed

ccli8 force-pushed the nuvoton_m2351_comb-sec-nonsec branch from 8919493 to a2d47e7 Compare August 26, 2019 07:24

0xc0170 added the BREAKING-CHANGE label Aug 26, 2019

0xc0170 requested a review from bulislaw August 26, 2019 08:07

0xc0170 requested review from Patater and urutva August 27, 2019 18:54

madchutney requested changes Aug 28, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Aug 28, 2019

ccli8 force-pushed the nuvoton_m2351_comb-sec-nonsec branch from a2d47e7 to 5a87dbb Compare August 29, 2019 09:29

adbridge added needs: work release-version: 5.14.0-rc3 and removed release-version: 5.14.0-rc2 labels Sep 12, 2019

ccli8 added 6 commits September 16, 2019 10:20

M2351: Remove pre-built non-PSA secure image temporarily

2471c9e

This will add back immediately after target renaming is done.

M2351: Add pre-built secure image for non-PSA

85bb65c

M2351: Add post-binary hook into uvision whitelist

0ed126b

This is necessary for exporting M2351 uvision project.

ccli8 mentioned this pull request Sep 16, 2019

Convert TZ target name 'NPSA' to test spec platform name #11487

Merged

ccli8 force-pushed the nuvoton_m2351_comb-sec-nonsec branch from b1ca3e1 to 0ed126b Compare September 16, 2019 05:23

0xc0170 added needs: review and removed needs: work labels Sep 16, 2019

urutva approved these changes Sep 16, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Sep 17, 2019

0xc0170 added ready for merge and removed needs: CI labels Sep 17, 2019

0xc0170 merged commit ffbd92c into ARMmbed:master Sep 17, 2019

0xc0170 removed the ready for merge label Sep 17, 2019

ccli8 mentioned this pull request Sep 20, 2019

M2351: Support TFM level 1 #10959

Merged

cyliangtw deleted the nuvoton_m2351_comb-sec-nonsec branch March 9, 2023 05:28

M2351: Enhance secure/non-secure image build flow for non-PSA target #11288

M2351: Enhance secure/non-secure image build flow for non-PSA target #11288

Uh oh!

Conversation

ccli8 commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Release Notes

Uh oh!

ciarmcom commented Aug 22, 2019

Uh oh!

ccli8 commented Aug 26, 2019

Uh oh!

0xc0170 commented Aug 26, 2019

Uh oh!

ccli8 commented Aug 26, 2019

Uh oh!

0xc0170 commented Aug 26, 2019

Uh oh!

ccli8 commented Aug 26, 2019

Uh oh!

bulislaw commented Aug 27, 2019

Uh oh!

ccli8 commented Aug 28, 2019

Uh oh!

mark-edgeworth commented Aug 28, 2019

Uh oh!

ccli8 commented Aug 28, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madchutney Sep 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-edgeworth commented Aug 28, 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!

ccli8 commented Aug 28, 2019

Uh oh!

ccli8 commented Aug 29, 2019

Uh oh!

ccli8 commented Sep 2, 2019

Uh oh!

ccli8 commented Sep 16, 2019

Uh oh!

urutva commented Sep 16, 2019

Uh oh!

0xc0170 commented Sep 16, 2019

Uh oh!

0xc0170 commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbed-ci commented Sep 17, 2019

Uh oh!

0xc0170 commented Sep 17, 2019

Uh oh!

mbed-ci commented Sep 17, 2019

Uh oh!

0xc0170 commented Sep 17, 2019

ccli8 commented Aug 22, 2019 •

edited

Loading

madchutney Sep 2, 2019 •

edited

Loading

0xc0170 commented Sep 17, 2019 •

edited

Loading