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

[ccli8]

Description

This PR tries to fix secure/non-secure ROM address error in bootloader-related build on Cortex-M23/M33. Take our on-going NUMAKER_PFM_M2351 as an example. Its ROM address is 0x0 from CMSIS pack. By introducing tz_offset/tz_offset_ns, we can correct secure/non-secure ROM address to 0x0/0x10000000 in build tool.

"NUMAKER_PFM_M2351": {
        "core": "Cortex-M23-NS",
        "default_toolchain": "GCC_ARM",
        "extra_labels": ["NUVOTON", "M2351", "M2351KIAAEES", "FLASH_CMSIS_ALGO"],
        "OUTPUT_EXT": "hex",
        "macros": ["MBED_FAULT_HANDLER_DISABLED", "MBED_TZ_DEFAULT_ACCESS=1"],
        "is_disk_virtual": true,
        "supported_toolchains": ["GCC_ARM", "IAR", "ARMC6"],
        "config": {
            "gpio-irq-debounce-enable": {
                "help": "Enable GPIO IRQ debounce",
                "value": 0
            },
            "gpio-irq-debounce-enable-list": {
                "help": "Comma separated pin list to enable GPIO IRQ debounce",
                "value": "NC"
            },
            "gpio-irq-debounce-clock-source": {
                "help": "Select GPIO IRQ debounce clock source: GPIO_DBCTL_DBCLKSRC_HCLK or GPIO_DBCTL_DBCLKSRC_LIRC",
                "value": "GPIO_DBCTL_DBCLKSRC_LIRC"
            },
            "gpio-irq-debounce-sample-rate": {
                "help": "Select GPIO IRQ debounce sample rate: GPIO_DBCTL_DBCLKSEL_1, GPIO_DBCTL_DBCLKSEL_2, GPIO_DBCTL_DBCLKSEL_4, ..., or GPIO_DBCTL_DBCLKSEL_32768",
                "value": "GPIO_DBCTL_DBCLKSEL_16"
            }
        },
        "inherits": ["Target"],
        "device_has": ["ANALOGIN", "I2C", "I2CSLAVE", "I2C_ASYNCH", "LOWPOWERTIMER", "RTC", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "SERIAL", "SERIAL_ASYNCH", "SERIAL_FC", "STDIO_MESSAGES", "SLEEP", "SPI", "SPISLAVE", "SPI_ASYNCH", "TRNG", "FLASH"],
        "detect_code": ["1305"],
        "release_versions": ["5"],
        "device_name": "M2351KIAAEES",
        "bootloader_supported": true,
        
        "tz_offset": "0x0",
        "tz_offset_ns": "0x10000000"
        
    },
    "NUMAKER_PFM_M2351_S": {
        "core": "Cortex-M23",
        "inherits": ["NUMAKER_PFM_M2351"],
        "device_has_remove": ["TRNG"]
    },
    "NUMAKER_PFM_M2351_NS": {
        "core": "Cortex-M23-NS",
        "inherits": ["NUMAKER_PFM_M2351"]
    }

Pull request type

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

[0xc0170]

What about the documentation? These 2 should be documented (what how they differ and what is expected to have there - value ranges, and when we should add these to).

[ccli8]

@0xc0170 Where can I add documentation for the new configuration options, tz_offset and tz_offset_ns?

[0xc0170]

@0xc0170 Where can I add documentation for the new configuration options, tz_offset and tz_offset_ns?

@AnotherButler Can you help? or @deepikabhavnani might know - where we have secure target documentation, this can be mentioned there?

[deepikabhavnani]

We do not have any specific document for secure/non-secure. Should it be part of tools?

[bulislaw]

I would be careful about the docs, we don't want to give an impression that current state is here to stay and something we will support. We are not going to use Mbed OS in secure mode, we will be relying on secure firmware running in TZ. If the changes only touch/address normal world lets document it, if it's for the temporary secure part, lets not add it to the docs.

[deepikabhavnani]

Document part suggested is addresses only, which will be needed later as well. I would recommend to rename them to SECURE_START_ADDR and MBED_START_ADDR to avoid confusion later

[ccli8]

@deepikabhavnani Do you mean SECURE_START_ADDR for start address of secure flash and MBED_START_ADDR for start address of non-secure flash? Then I have some further questions:

1. New configuration option for size of secure/non-secure flash?
2. New configuration option for start address/size of secure/non-secure SRAM?
3. Because user can partition flash/SRAM to secure/non-secure to meet its requirement, all the above configuration options also must be changed through e.g.mbed_app.json.

[deepikabhavnani]

@ccli8 - I believe start address option was needed to have support in build tools, same as bootloader but I am not fully convinced with other config options for user application. We have them in target specific files.
During porting of cloud and other application did you felt the need to change SRAM/flash partitions?

[ccli8]

@deepikabhavnani I think it is necessary to change flash/SRAM partition by application developer to meet different application needs. But for this case, lets focus on flash and leave configuable for future support.

According to the file change, I just fix rom_start and rom_size is not fixed. So if SECURE_START_ADDR/MBED_START_ADDR are used to fix secure/non-secure rom_start, other secure/non-secure size configuration options e.g. SECURE_FLASH_SIZE/MBED_FLASH_SIZE are also needed to fix rom_size.

# Fix TrustZone secure/non-secure ROM address
if (self.target.core == "Cortex-M23" or self.target.core == "Cortex-M33"):
    rom_start = rom_start + int(getattr(self.target, "tz_offset", "0x0"), 0)
elif (self.target.core == "Cortex-M23-NS" or self.target.core == "Cortex-M33-NS"):
    rom_start = rom_start + int(getattr(self.target, "tz_offset_ns", "0x0"), 0)

if self.target.bootloader_img or self.target.restrict_size:
    return self._generate_bootloader_build(rom_start, rom_size)
elif self.target.mbed_app_start or self.target.mbed_app_size:

[deepikabhavnani]

Looks good to me 👍 . But should wait for @theotherjimmy approval since it is more of tools change

[theotherjimmy]

I remain unconvinced. How does this much better from providing the secure partition binary as a boot loader to managed boot loader mode?

[ccli8]

How does this much better from providing the secure partition binary as a boot loader to managed boot loader mode?

@theotherjimmy Would you comment more? I don't catch your point.

[theotherjimmy]

@ccli8 You could provide the secure partition to managed bootloader mode through the use of target.bootloader_img. It would reserve room for the secure partition. I was asking what features this solution offers over that one.

[AnotherButler]

Would it make sense for this documentation to go in our configuration section?
https://os-doc-builder.test.mbed.com/docs/development/reference/configuration.html

[ccli8]

@theotherjimmy To my knowledge, there would be two boot flows as TZ is introduced in:

1. secure bootloader > non-secure bootloader > non-secure application
2. secure bootloader > non-secure application

They would have the differences:

1. F/W update logic is in non-secure bootloader in B1 and in secure bootloader in B2. So secure bootloader in B1 is simpler than in B2.
2. Because uses of flash/SRAM are exclusive among secure world and non-secure world, B1 has more SRAM space and B2 has more flash space for non-secure application development.

I think B1 and B2 are both needed for different application development requirements.

[theotherjimmy]

@ccli8 I'm still convinced that the target.bootloader_img configuration parameter could be used for both flows. That's the crux of this PR: adding more configuration parameters. I'm not convinced that this is needed or an ergonomics improvement (a nice convenience for many applications).

[ccli8]

@theotherjimmy No. This PR is to fix flash address with TZ and is not bound to the two flows. Take my on-going NUMAKER_PFM_M2351 target for example. It has 512KB flash and if I partition it to 64KB secure flash/448KB non-secure flash. Its flash address space will be:
Secure flash
0x00000000 - 0x0000FFFF
Non-secure flash
0x10010000 - 0x1007FFFF

Then look at rom_start in PR code.

try:
    rom_size = int(cmsis_part['memory']['IROM1']['size'], 0)
    rom_start = int(cmsis_part['memory']['IROM1']['start'], 0)
except KeyError:
    try:
        rom_size = int(cmsis_part['memory']['PROGRAM_FLASH']['size'], 0)
        rom_start = int(cmsis_part['memory']['PROGRAM_FLASH']['start'], 0)
    except KeyError:
        raise ConfigException("Not enough information in CMSIS packs to "
                              "build a bootloader project")

rom_start is fetched from CMSIS pack and it would be 0x00000000 for NUMAKER_PFM_M2351 target. When I build non-secure code for NUMAKER_PFM_M2351, rom_start should be corrected to 0x10000000 (or more exactly, 0x10010000) rather than 0x00000000 to pass to below code:

if self.target.bootloader_img or self.target.restrict_size:
    return self._generate_bootloader_build(rom_start, rom_size)
elif self.target.mbed_app_start or self.target.mbed_app_size:
    return self._generate_linker_overrides(rom_start, rom_size)
else:
    raise ConfigException(
        "Bootloader build requested but no bootlader configuration")

[theotherjimmy]

@ccli8 So, if I get this correct, the non-secure partition is mapped at a different offset to the secure partition? Does the user select the size of these partitions?.

---

Be aware that I take additional configuration parameters very seriously. Any parameters must be supported for the foreseeable future, so any complications in parameters are very serious indeed.

[theotherjimmy]

@ccli8 Is this information described in CMSIS packs?

[ccli8]

@theotherjimmy

So, if I get this correct, the non-secure partition is mapped at a different offset to the secure partition?

Yes. For NUMAKER_PFM_M2351, secure/non-secure flash starts at 0x00000000/0x10000000. But contrary for Cortex-M23/M33 forMPS2+, secure/non-secure flash starts at 0x10000000/0x00000000.

Does the user select the size of these partitions?.

Yes. Partition size must be configurable by developer. This PR doesn't address it and just fixes secure/non-secure address offset.

Is this information described in CMSIS packs?

Checking our tool team, NUMAKER_PFM_M2351 CMSIS pack just exposes IROM1 start/size (0x0, 512KB).

[theotherjimmy]

@ccli8 I'm really hesitant to merge this PR, as I will have to support this tz_offset configuration parameter going forward if I do. Ideally, we would pull this information from the CMSIS Pack, and I would have to do a release of cmsis-pack-manager to include Secure permission parsing and we would use that info to select the correct ROM region when in Managed Bootolader Mode.

That's probably not going to happen in the next few days though.

[ccli8]

@theotherjimmy This is 2-level issue:

1. Secure/non-secue flash address offset
2. Configurable partition size information

This PR addresses L1, and it would get of no use if this information can be pulled from CMSIS pack. For L2, partition size is configurable by user through another tool provided by chip manufacturer, so this information is dynamic and cannot be pulled from CMSIS pack. This PR doesn't address it.

I have another idea to address L1 and L2 simultaneously. Drop tz_offset which can just address L1. Rather, introduce e.g. mbed_rom_start/mbed_rom_size to override IROM1 size/start pulled from CMSIS pack if they are defined. Still taking NUMAKER_PFM_M2351 for example, mbed_rom_start/mbed_rom_size are 0x00000000/0x00010000 for secure code and 0x10010000/0x00070000 for non-secure code.

[theotherjimmy]

@ccli8 If I read that correctly, there is some tool outside of mbed-os that manages the secure/non-secure partition table, and It's not something we could set every time we flash a binary. If that's the case, we're in quite a pickle.

---

mbed_rom_start and mbed_rom_size are a good suggestion. It's something that's not specific to trustzone, and could even be useful for targets that don't currently have CMSIS Pack support.

I don't follow the example though. If you're suggesting that a user can select the offset, and not the manufacturer, then why would the definitions for NUMAKER_PFM_M2351 be fixed like that?

[ccli8]

If I read that correctly, there is some tool outside of mbed-os that manages the secure/non-secure partition table, and It's not something we could set every time we flash a binary. If that's the case, we're in quite a pickle.

Yes.

mbed_rom_start and mbed_rom_size are a good suggestion. It's something that's not specific to trustzone, and could even be useful for targets that don't currently have CMSIS Pack support.

Yes.

I don't follow the example though. If you're suggesting that a user can select the offset, and not the manufacturer, then why would the definitions for NUMAKER_PFM_M2351 be fixed like that?

I'm confused. Dependent on actual partition through external tool, user could override mbed_rom_start/mbed_rom_size in mbed_app.json to pass flash information to mbed build system for secure or non-secure target:

    "NUMAKER_PFM_M2351_NS": {
        
        "target.mbed_rom_start": "0x10010000",
        "target.mbed_rom_size": "0x70000"
        
    },

    "NUMAKER_PFM_M2351_S": {
        
        "target.mbed_rom_start": "0x0",
        "target.mbed_rom_size": "0x10000"
        
    }

@theotherjimmy

[theotherjimmy]

Dependent on actual partition through external tool, user could override mbed_rom_start/mbed_rom_size in mbed_app.json to pass flash information to mbed build system for secure or non-secure target:

That's what confused me: if a user always picks this, why do we have a default?

[ccli8]

That's what confused me: if a user always picks this, why do we have a default?

Default is useful. For example, in most applications, secure code is small and non-secure code is large. We can give a workable partition suggestion in target.json. User just follows this suggestion and partitions its flash through external tool for its initial development.

Then there's a similar issue for SRAM. On TZ target, like flash, SRAM also needs to be partitioned. It would be helpful to also provide mbed_ram_start/mbed_ram_size.

@theotherjimmy

[theotherjimmy]

To keep discussion moving in a more constructive direction, @ccli8 Could you implement mbed_rom_start and mbed_rom_size so that we can proceed with that solution?

[ccli8]

@0xc0170 @theotherjimmy
I would close this PR. #7133 is to replace this one.