Override ROM/RAM start/size for TrustZone targets #7133

ccli8 · 2018-06-06T02:35:13Z

Description

On TrustZone targets, ROM/RAM start/size are not so fixed and can be changed by external tool or other means. This PR introduces new target configuration options for user to override ROM/RAM start/size dependent on its real case.

Introduce new target configuration options (mbed_rom_start/mbed_rom_size)
1. Override rom_start/rom_size pulled from CMSIS pack or no CMSIS pack
2. Pass overridden flash information (MBED_ROM_START/MBED_ROM_SIZE) to compiler
3. Pass overridden flash information (MBED_ROM_START/MBED_ROM_SIZE) to linker
Introduce new target configuration options (mbed_ram_start/mbed_ram_size)
1. Override ram_start/ram_size pulled from CMSIS pack or no CMSIS pack
2. Pass overridden SRAM information (MBED_RAM_START/MBED_RAM_SIZE) to compiler
3. Pass overridden SRAM information (MBED_RAM_START/MBED_RAM_SIZE) to linker

Take our on-going NUMAKER_PFM_M2351 as an example. We have partitioned 512 KB flash/96KB SRAM to:

Secure flash: 64KB
Non-secure flash: 448KB
Secure SRAM: 8KB
Non-secure SRAM: 88KB

For build for NUMAKER_PFM_M2351 secure target, we must have mbed_app.json:

"target_overrides": {

    "NUMAKER_PFM_M2351_S": {
        
        "target.mbed_rom_start": "0x0",
        "target.mbed_rom_size": "0x10000",
        "target.mbed_ram_start": "0x20000000",
        "target.mbed_ram_size": "0x2000",
        
    }
},

For build for NUMAKER_PFM_M2351 non-secure target, we must have mbed_app.json:

"target_overrides": {

    "NUMAKER_PFM_M2351_NS": {
        
        "target.mbed_rom_start": "0x10010000",
        "target.mbed_rom_size": "0x70000",
        "target.mbed_ram_start": "0x30002000",
        "target.mbed_ram_size": "0x16000",
        
    }
},

Related PR

#6890

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[X] Feature
[ ] Breaking change

This fix relies on target configuration options (mbed_rom_start/mbed_rom_size) defined to override CMSIS pack or no CMSIS pack. This is useful for a target which: 1. Doesn't support CMSIS pack, or 2. Supports TrustZone and user needs to change its flash partition

…er/linker This fix relies on target configuration options (mbed_rom_start/mbed_rom_size) defined.

…r/linker This fix relies on target configuration options (mbed_ram_start/mbed_ram_size) defined.

theotherjimmy

Please don't modify macros in the toolchains. If you need the MBED_ROM_START and other macros outside of boot loader mode, we should do that in config, is it's a configuration decision.

theotherjimmy · 2018-06-06T14:53:27Z

@ccli8 I like this proposal. I take it that the macros are needed even when you're not building a bootloader?

ccli8 · 2018-06-07T01:55:50Z

the macros are needed even when you're not building a bootloader?

Yes. The MBED_ROM_START and like macros are not bound to bootloader.
Per my experience, they have the following uses:

Override flash start/size for flash algorithm (flash_api.c)
Still take NUMAKER_PFM_M2351 as an example:

/* Non-secure flash */
static const flash_target_config_t flash_target_config_ns = {
    .page_size  = 4,

#if defined(MBED_ROM_START)
    .flash_start = MBED_ROM_START,
#else
    .flash_start = 0x10040000,
#endif

#if defined(MBED_ROM_SIZE)
    .flash_size = MBED_ROM_SIZE,
#else
    .flash_size = 0x40000,
#endif

    .sectors = sectors_info_ns,
    .sector_info_count = sizeof(sectors_info_ns) / sizeof(sector_info_t)
};

Override flash start/size and SRAM start/size for linker script (*.sct, *.ld, *.icf)

Then about how to define the MBED_ROM_START and like macros. If we define them in config, we can just pass them to compiler and cannot pass to linker (need make_ld_define?). Actually, target.mbed_rom_start and MBED_ROM_START are for the same purpose. User would feel confused if it needs to define them multiple places. So I generate MBED_ROM_START from target.mbed_rom_start to centralize the configuration.

@theotherjimmy

theotherjimmy · 2018-06-07T13:51:08Z

@ccli8 My suggestion is to make this behave like a bootloader build. That way a user also gets visual verification. I can make that modification if you would like.

ccli8 · 2018-06-08T02:07:34Z

@theotherjimmy OK. Please modify it.

…IZE don't pass to linker

ccli8 · 2018-06-08T10:20:51Z

@theotherjimmy I fix the MBED_ROM_START macros don't pass to linker in ae16aa2.

theotherjimmy · 2018-06-08T15:02:35Z

@ccli8 I upgraded the implementation to treat the ROM + RAM specifiers as part of the bootloader mode.

This should let you override them as you would any other configuration variable, and starts a generic "RAM reservation" framework using regions, much like the bootloader builds. @marcuschangarm RAM reservation.

ccli8 · 2018-06-11T08:58:14Z

@theotherjimmy The modification doesn't meet my requirement. Compiling mbed-cloud-clent-example on NUMAKER_PFM_M2351_NS target with mbed_app.json:

"NUMAKER_PFM_M2351_NS": {
        "target.mbed_rom_start":    "0x10010000",
        "target.mbed_rom_size":     "0x70000",
        "target.mbed_ram_start":    "0x30002000",
        "target.mbed_ram_size":     "0x16000",
   
        "target.mbed_app_start"             : "0x10024000",
        "target.mbed_app_size"              : "0x5C000",

Checking .profile-c/.profile-cxx, ROM start/size are overridden by mbed_app_start/mbed_app_size and don't correctly pass to compiler.

'-DAPPLICATION_ADDR=0x10024000', '-DAPPLICATION_SIZE=0x5c000', '-DAPPLICATION_RAM_ADDR=0x30002000', '-DAPPLICATION_RAM_SIZE=0x16000'

Checking .profile-ld, same as above, ROM start/size don't correctly pass to linker.

'--predefine="-DMBED_APP_START=0x10024000"', '--predefine="-DMBED_APP_SIZE=0x5c000"', '--predefine="-DMBED_RAM_START=0x30002000"', '--predefine="-DMBED_RAM_SIZE=0x16000"'

mbed_rom_start/mbed_rom_size and mbed_ram_start/mbed_ram_size mean physical ROM/RAM start/size (even though configurable here) and shouldn't change by application (mbed_app_start/mbed_app_size).

theotherjimmy · 2018-06-11T14:35:42Z

@ccli8 Cloud-client-example uses an unmanaged bootloader project. The defines in here will not take priority over them. The numbers are correct.

ccli8 · 2018-06-12T02:13:32Z

@theotherjimmy The modification meets the initial goal: use mbed_rom_start/mbed_rom_size to override rom_start/rom_size pulled from CMSIS pack for correct bootloader/application build. But besides passed to build tool, mbed_rom_start/mbed_rom_size must also pass to compiler (for flash algorithm to know actual flash start/size) and linker (for .sct/.ld/.icf to know actual flash start/size and SRAM start/size) unchangeable.

As I know, mbed_app_start defines the start where application is located and mbed_rom_start added here defines the start of physical flash. They shouldn't be mixed. Hopefully, I would like to get ROM start/size = 0x10010000/0x70000 (defined in mbed_app.json) irrespective of bootloader/application build.

theotherjimmy · 2018-06-12T15:26:20Z

They shouldn't be mixed.

This is where we disagree. If this patch is to be useful to those without cmsis-packs, then we need them to be mixable.

ccli8 · 2018-06-13T01:56:54Z

@theotherjimmy It seems the disagree lies in that you use mbed_rom_start/mbed_rom_size only for bootloader/application build, but I expect more: they can also pass to compiler/linker irrespective of bootloader/application build. At the start, this PR has three goals with mbed_rom_start/mbed_rom_size added:

Override rom_start/rom_size pulled from CMSIS pack or no CMSIS pack
Pass overridden flash information (MBED_ROM_START/MBED_ROM_SIZE) to compiler
Pass overridden flash information (MBED_ROM_START/MBED_ROM_SIZE) to linker

Currently, the modification meets G1 and doesn't meet G2/G3. In my opinion, now that mbed_rom_start/mbed_rom_size have been passed to build tool, why not also pass along to compiler/linker to meet G2/G3? I'm also focusing on G2/G3 because all G1/G2/G3 are necessary for development of my on-going TrustZone target. Is it possible to add support for G2/G3 based on the modification?

theotherjimmy · 2018-06-13T14:16:08Z

@ccli8 That's where this patch gets clever: The presence of mbed_rom_start, etc. enable bootloader mode. Therefore G2/G3 is the same as with BL mode: MBED_APP_START and MBED_APP_SIZE. This should simplify thing for the linker script.

Why do you need the direct ROM information?

theotherjimmy · 2018-06-13T14:31:27Z

tools/config/__init__.py

+        for memory in memories:
+            try:
+                size = cmsis_part['memory']['IRAM1']['size']
+                start = cmsis_part['memory']['IRAM1']['start']


Oh, that's wrong. Just a sec.

ccli8 · 2018-06-14T02:44:12Z

@theotherjimmy

Why do you need the direct ROM information?

Like G2, flash algorithm needs ROM information to be passed in. Take flash_api.c on NUMAKER_PFM_M2351 as an example: FMC_SECURE_ROM_SIZE currently is hard-coded to 0x40000 (256Kib), but actually it must be configurable dependent on actual secure/non-secure flash partition.

/* Secure flash */
static const flash_target_config_t flash_target_config = {
    .page_size  = 4,
    .flash_start = 0x0,
#if defined(FMC_SECURE_ROM_SIZE)
    .flash_size = FMC_SECURE_ROM_SIZE,
#else
    .flash_size = 0x80000,
#endif
    .sectors = sectors_info,
    .sector_info_count = sizeof(sectors_info) / sizeof(sector_info_t)
};

/* Non-secure flash */
static const flash_target_config_t flash_target_config_ns = {
    .page_size  = 4,
    .flash_start = NS_OFFSET + FMC_SECURE_ROM_SIZE,
#if defined(FMC_SECURE_ROM_SIZE)
    .flash_size = 0x80000 - FMC_SECURE_ROM_SIZE,
#else
    .flash_size = 0,
#endif
    .sectors = sectors_info_ns,
    .sector_info_count = sizeof(sectors_info_ns) / sizeof(sector_info_t)
};

ccli8 · 2018-06-14T07:05:12Z

@theotherjimmy Continuing my above comment. For G3 (linker script), MBED_APP_START/MBED_APP_SIZE have been enough after checking NUMAKER_PFM_M2351's current linker script. But for G2 (flash algorithm), MBED_APP_START/MBED_APP_SIZE cannot replace MBED_ROM_START/MBED_ROM_SIZE. Physical ROM information is still necessary for flash algorithm as exemplified above. Another approach to G2 is to manually add macros MBED_ROM_START/MBED_ROM_SIZE into target configuration option macros, but it may be seen as duplicate to target.mbed_rom_stat/taget.mbed_rom_size and get confusing.

theotherjimmy · 2018-06-14T15:05:02Z

@ccli8 Thanks for the detail. I'll make MBED_ROM_START + MBED_ROM_SIZE available to C, C++.

0xc0170 · 2018-06-20T12:35:55Z

@ccli8 Thanks for the detail. I'll make MBED_ROM_START + MBED_ROM_SIZE available to C, C++.

Are we waiting for another commit here to resolve this? I added a label needs: work

theotherjimmy · 2018-06-20T15:21:59Z

@0xc0170 Yes. We're waiting on me. Just a moment.

* MBED_ROM_START = start of current rom (independent of BL modes) * MBED_ROM_SIZE = size of current rom (independent of BL modes)

theotherjimmy · 2018-06-20T15:50:46Z

@ccli8 That should do it. I'm liking the way the configuration is getting a "richer" set of features and the toolchains are adding the appropriate stuff as defines.

ccli8 · 2018-06-21T07:13:15Z

@theotherjimmy This modification can meet my requirement of configuring TrustZone target. Many thanks for your support.

cmonr · 2018-06-21T15:12:33Z

/morph build

mbed-ci · 2018-06-21T16:37:28Z

Build : SUCCESS

Build number : 2421
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7133/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-06-21T18:31:07Z

Exporter Build : SUCCESS

Build number : 2051
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7133/

mbed-ci · 2018-06-21T22:38:39Z

Test : SUCCESS

Build number : 2202
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7133/2202

ccli8 added 3 commits June 6, 2018 09:45

Pass fixed flash information (MBED_ROM_START/MBED_ROM_SIZE) to compil…

f48f30c

…er/linker This fix relies on target configuration options (mbed_rom_start/mbed_rom_size) defined.

Pass fixed SRAM information (MBED_RAM_START/MBED_RAM_SIZE) to compile…

58f1521

…r/linker This fix relies on target configuration options (mbed_ram_start/mbed_ram_size) defined.

ccli8 mentioned this pull request Jun 6, 2018

Fix build error with secure/non-secure ROM address on Cortex-M23/M33 #6890

Closed

0xc0170 requested a review from a team June 6, 2018 06:43

0xc0170 added the needs: review label Jun 6, 2018

theotherjimmy suggested changes Jun 6, 2018

View reviewed changes

Fix overridden MBED_ROM_START/MBED_ROM_SIZE/MBED_RAM_START/MBED_RAM_S…

ae16aa2

…IZE don't pass to linker

Move from tc constructor to config vars

09cdc06

theotherjimmy previously approved these changes Jun 8, 2018

View reviewed changes

theotherjimmy reviewed Jun 13, 2018

View reviewed changes

Use memories passed in when getting memory specs

92abbb7

theotherjimmy dismissed their stale review via 92abbb7 June 13, 2018 14:34

0xc0170 added needs: work and removed needs: review labels Jun 20, 2018

Add Rom config info getter and rom defines

52e640f

* MBED_ROM_START = start of current rom (independent of BL modes) * MBED_ROM_SIZE = size of current rom (independent of BL modes)

theotherjimmy approved these changes Jun 21, 2018

View reviewed changes

cmonr added the release-version: 5.10.0-rc1 label Jun 21, 2018

cmonr approved these changes Jun 21, 2018

View reviewed changes

cmonr added needs: CI and removed needs: work labels Jun 21, 2018

cmonr merged commit 95d2b3d into ARMmbed:master Jun 21, 2018

0xc0170 removed the needs: CI label Jun 21, 2018

ccli8 mentioned this pull request Jul 9, 2018

Support Nuvoton's NUMAKER_PFM_M2351 target #7302

Merged

cmonr mentioned this pull request Jul 28, 2018

Tools: Extract resources object and implement incremental scan #7183

Merged

3 tasks

ccli8 mentioned this pull request Sep 3, 2018

M2351: Cannot partition flash/SRAM through mbed_app.json #7942

Closed

ccli8 deleted the nuvoton_support_tz_partition branch December 20, 2018 06:43

Override ROM/RAM start/size for TrustZone targets #7133

Override ROM/RAM start/size for TrustZone targets #7133

Uh oh!

Conversation

ccli8 commented Jun 6, 2018 • edited by theotherjimmy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related PR

Pull request type

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Jun 6, 2018

Uh oh!

ccli8 commented Jun 7, 2018

Uh oh!

theotherjimmy commented Jun 7, 2018

Uh oh!

ccli8 commented Jun 8, 2018

Uh oh!

ccli8 commented Jun 8, 2018

Uh oh!

theotherjimmy commented Jun 8, 2018

Uh oh!

ccli8 commented Jun 11, 2018

Uh oh!

theotherjimmy commented Jun 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccli8 commented Jun 12, 2018

Uh oh!

theotherjimmy commented Jun 12, 2018

Uh oh!

ccli8 commented Jun 13, 2018

Uh oh!

theotherjimmy commented Jun 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy Jun 13, 2018

Choose a reason for hiding this comment

Uh oh!

ccli8 commented Jun 14, 2018

Uh oh!

ccli8 commented Jun 14, 2018

Uh oh!

theotherjimmy commented Jun 14, 2018

Uh oh!

0xc0170 commented Jun 20, 2018

Uh oh!

theotherjimmy commented Jun 20, 2018

Uh oh!

theotherjimmy commented Jun 20, 2018

Uh oh!

ccli8 commented Jun 21, 2018

Uh oh!

cmonr commented Jun 21, 2018

Uh oh!

mbed-ci commented Jun 21, 2018

Build : SUCCESS

Triggering tests

Uh oh!

mbed-ci commented Jun 21, 2018

Exporter Build : SUCCESS

Uh oh!

mbed-ci commented Jun 21, 2018

Test : SUCCESS

Uh oh!

Uh oh!

ccli8 commented Jun 6, 2018 •

edited by theotherjimmy

Loading

theotherjimmy commented Jun 11, 2018 •

edited

Loading

theotherjimmy commented Jun 13, 2018 •

edited

Loading