linker: generate memory regions from devicetree partitions #31077

JordanYates · 2021-01-03T05:14:26Z

Implements a relatively simple mechanism for automatically generating memory regions based devicetree partitions.
For a partition to result in a memory region, it must add the linker-region property to the appropriate partition.
Only partitions under the chosen/zephyr,flash node result in memory regions, under the assumption that this points at the onboard ROM, and that memory regions for external flash do not make sense.
Regions are named based on the label of the partition.

The primary purpose of this PR is to enable placing constants at absolute addresses in a portable manner.

Naming for FOR_EACH_NESTED and linker-region is provisional pending feedback.
Currently this has only been integrated into the Cortex linker script and enabled on a small subset of nRF hardware.

Implements #31073

Adds a macro that operates on the results of previous evaluations. This is mostly useful for calculating the maximum or minimum of an arbitrary list of integers at compile time. An implementation in a similar style to `Z_FOR_EACH_IDX` was attempted, however macros such as `COND_CODE_1` were not properly being expanded inside nested usage of `UTIL_OBSTRUCT`. Signed-off-by: Jordan Yates <[email protected]>

Add test cases for the `FOR_EACH_NESTED` function. Signed-off-by: Jordan Yates <[email protected]>

Add a new property to the child binding of `fixed-partition` to signify that a memory region in the linker should be generated for this partition. Signed-off-by: Jordan Yates <[email protected]>

Adds two public macros for creating memory regions from devicetree partitions. The `linker-region` property is only considered on partitions under `chosen/zephyr,flash` as this is assumed to be the only flash driver for which memory regions make sense. `DT_PARTITION_REGIONS` declares the memory regions for consumption by ld. While macro limitations mean that region names are quoted and there are no seperators between regions, ld appears to handle this fine. `DT_PARTITION_REGIONS_START` gets the memory address of the region with the lowest origin address. The FLASH region will be constrained to this address. Signed-off-by: Jordan Yates <[email protected]>

Insert macros for generating memory regions from flash partitions that have set the `linker-region` property. Signed-off-by: Jordan Yates <[email protected]>

Demonstrate how to place variables at constants memory addresses defined by devicetree partitions. Signed-off-by: Jordan Yates <[email protected]>

Enable the `linker-region` property on the NVS partition for a subset of nordic boards to validate CI behaviour. Signed-off-by: Jordan Yates <[email protected]>

mbolivar-nordic · 2021-01-11T15:26:22Z

Added dev-review per Slack:

jyates  3:58 PM
I would like to start getting feedback on https://github.com/zephyrproject-rtos/zephyr/pull/31077.
It has been relevant to several discussions here recently and I would like to know if the general method is accepted before putting in the effort of updating non-cortex linker scripts.

JordanYates · 2021-01-14T07:45:47Z

As a cross-link for posterity, this is also an alternative to #29794 for #27471

ioannisg · 2021-01-14T13:26:49Z

As a cross-link for posterity, this is also an alternative to #29794 for #27471

I think this could be treated a complement to #27471, the current PR does not only deal with the sizing and placing of the code partition.

JordanYates · 2021-01-14T13:55:31Z

I think this could be treated a complement to #27471, the current PR does not only deal with the sizing and placing of the code partition.

Could you clarify please? I'm not sure how to read that statement. Did you mean #29794?
Or are you saying that your PR solves problems additional to #27471?

ioannisg · 2021-01-14T14:52:17Z

I think this could be treated a complement to #27471, the current PR does not only deal with the sizing and placing of the code partition.

Could you clarify please? I'm not sure how to read that statement. Did you mean #29794?
Or are you saying that your PR solves problems additional to #27471?

Yeap, sorry, I meant the PR, #29794 (not necessarily the exact solution presented there, but rather its generic idea, i.e. using the existing DT/Kconfig infrastructure to limit the code partition to a certain area)

JordanYates · 2021-01-14T15:00:06Z

i.e. using the existing DT/Kconfig infrastructure to limit the code partition to a certain area)

Sure, this PR makes no statements about the code partition beyond shrinking the default ROM_SIZE so that it doesn't overlay defined linker-region partitions.

galak · 2021-01-14T17:04:53Z

@JordanYates what's the implication if you assumed all partitions had linker-region property set?

ioannisg · 2021-01-14T20:24:36Z

A limitation that I spot in the current implementation is that all the flash area beyond the storage_partition (or, rather, the first partition with the linker-region property) cannot be used by the Zephyr for the application image. This is problematic for all the boards that define the storage partition right after the boot partition. There has to be found a way to by pass this limitation (alternatively all the boards that not define storage_partition at the end will need to re-adjust their existing flash layout)

JordanYates · 2021-01-14T22:19:33Z

@JordanYates what's the implication if you assumed all partitions had linker-region property set?

If by all partitions you mean the entire flash space is covered by linker-region, then the implication is that you have reserved all of your flash for custom uses and have no space left for an application.

JordanYates · 2021-01-14T23:14:37Z

A limitation that I spot in the current implementation is that all the flash area beyond the storage_partition (or, rather, the first partition with the linker-region property) cannot be used by the Zephyr for the application image. This is problematic for all the boards that define the storage partition right after the boot partition. There has to be found a way to by pass this limitation (alternatively all the boards that not define storage_partition at the end will need to re-adjust their existing flash layout)

Without changes to this PR, the resolution is simply to not put linker-region on those storage partitions that exist at the start of ROM. #27471 is not a problem for those boards currently as they must be manually setting CONFIG_FLASH_LOAD_OFFSET to work. Therefore it is already impossible for the partition to be overrun.

The special cases/behavior are starting to pile up however, and the solution no longer seems as clean as it did originally.
Having considered this problem in the light of your PR, I believe the core problem is the implicit behavior of the FLASH region when building standalone applications. Because the user is not explicitly saying where the code should live, we are left to make assumptions (currently "use it all!"), which will always be incorrect in odd cases. IMO the only watertight solution is to force the user to make this information explicit in some way. Once the FLASH region is using a defined memory space, my desired functionality can be implemented entirely by the linker snippet system.

Your single-image partition is one method of doing this. An alternative that wouldn't require touching existing board files would be to define FLASH as the total space occupied by boot_partition,slot0_partition,slot1_partition and scratch_partition, with the caveat that they couldn't have other partitions inserted between them.

ioannisg · 2021-01-15T09:12:07Z

A limitation that I spot in the current implementation is that all the flash area beyond the storage_partition (or, rather, the first partition with the linker-region property) cannot be used by the Zephyr for the application image. This is problematic for all the boards that define the storage partition right after the boot partition. There has to be found a way to by pass this limitation (alternatively all the boards that not define storage_partition at the end will need to re-adjust their existing flash layout)

Without changes to this PR, the resolution is simply to not put linker-region on those storage partitions that exist at the start of ROM.

I believe, this is not ideal, it breaks uniformity across storage partition definitions in the various boards. I would not recommend this way forward; we should be trying to introduce common rules and conventions across definitions.

#27471 is not a problem for those boards currently as they must be manually setting CONFIG_FLASH_LOAD_OFFSET to work. Therefore it is already impossible for the partition to be overrun.

Unfortunately, this does not always hold - I am sure many they are not (so we're just lucky the samples and tests they run fit in the area preceding the storage partition, or the storage partition is not used in in-tree samples).

The special cases/behavior are starting to pile up however, and the solution no longer seems as clean as it did originally.
Having considered this problem in the light of your PR, I believe the core problem is the implicit behavior of the FLASH region when building standalone applications. Because the user is not explicitly saying where the code should live, we are left to make assumptions (currently "use it all!"), which will always be incorrect in odd cases.

IMO the only watertight solution is to force the user to make this information explicit in some way. Once the FLASH region is using a defined memory space, my desired functionality can be implemented entirely by the linker snippet system.

Agreed. And that's why I said initially that #31077 is rather a complement for #29794, not an alternative. We need to make it explicit which area can be used by the image code in all cases - not only when we build with boot loader.

Your single-image partition is one method of doing this.

The implementation #29794, although it does do the job, as you point out, it is not accepted by @galak @mbolivar-nordic, so we have to look for an alternative.

An alternative that wouldn't require touching existing board files would be to define FLASH as the total space occupied by boot_partition,slot0_partition,slot1_partition and scratch_partition, with the caveat that they couldn't have other partitions inserted between them.

So you suggest to "do some math" at Kconfig and/or linker stage :) It could theoretically be an alternative. I don't get why this would be a better solution, though.

To clarify, my "problem" here is that the logic to decide where to place and how to size the code partition (and other partitions) is split in 3 Zephyr main "components":

DeviceTree: where partitions are initially described, in the boards's dts files,
Kconfig: where chosen symbols become enabled, based on Kconfig input not available to DT (IS BOOTLOADER, IS chain-loadable image, etc.) and other Kconfig symbols are initialized (e.g. FLASH_LOAD_OFFSET/SIZE, ROM_START_OFFSET, etc.)
Linker script (where logic is used to further manipulate regions an entries, based, again on DT and/or Kconfig input). Your linker: generate memory regions from devicetree partitions #31077 is actually injecting additional logic at the linker.)

IMHO this is far from ideal. The solution is hard to follow and developers have a hard time to learn what is going on.

My attempt in #29794 was to move as much as we can at the DeviceTree stage, basically, "doing all the math we need" at DeviceTree partition layout descriptions, and then let Kconfig select the right layouts/partitions and initialize the proper symbols, so the Linker would not have to do anything (just accept the incoming configuration without further "internal" logic to derive region boundaries)

If we keep doing "partitioning" calculations at Kconfig/Linker stage, I would argue: why do we, then, need to define partitions in DeviceTree, at all? At Kconfig stage we already know the flash size and start address, so we could even do all the partition layout calculationss there, and remove all stuff from DeviceTree.

JordanYates · 2021-01-15T10:53:50Z

It could theoretically be an alternative. I don't get why this would be a better solution, though.

It's not a better long term solution, but I think it would be a less invasive short term one. It doesn't change the current split of information across across devicetree/kconfig/linker, so it doesn't solve your problem, but it does make the error condition in #27471 impossible. Potentially that makes it a more acceptable solution given the dev-review comments around post-LTS.

The behavior change would be:

For applications that don't specify FLASH_LOAD_OFFSET, the linker script finds the lowest address from the listed partitions to use
For applications that don't specify FLASH_LOAD_SIZE, the linker script finds the highest address from the listed partitions and subtracts the value from the first step.

The main advantage is that user .dts files would not need to change in most cases.

If we keep doing "partitioning" calculations at Kconfig/Linker stage, I would argue: why do we, then, need to define partitions in DeviceTree, at all? At Kconfig stage we already know the flash size and start address, so we could even do all the partition layout calculationss there, and remove all stuff from DeviceTree.

I think the primary motivating reason is the flash_map API. When the partitions are in devicetree, you can iterate over the compatible using the C macros to instantiate each partition as a flash_area. This functionality is lost if partitions are defined in Kconfig.

Laczen · 2021-01-15T14:28:22Z

@ioannisg, @JordanYates, let us set the goal of removing everything from Kconfig, and just set everything in the dts:
a. We can't put different partition definitions in one file, so different overlays are needed.
b. We don't want any DT_MCUBOOT, DT_IS_MCUBOOT, ... this means we need to get rid of the mcuboot header in the compilation: just define a partition for the header (the start of this header partition should be used for imagetool), the imagepartition itself is used for the compilation.

galak · 2021-01-20T18:03:05Z

If by all partitions you mean the entire flash space is covered by linker-region, then the implication is that you have reserved all of your flash for custom uses and have no space left for an application.

I meant if you generated for every partition regardless of linker-region being set. I'm trying to see if we can remove the property, have the generation always occur, and its just based on use/application that a given region ends up coming into play with the linker.

JordanYates · 2021-01-21T08:38:19Z

I meant if you generated for every partition regardless of linker-region being set. I'm trying to see if we can remove the property, have the generation always occur, and its just based on use/application that a given region ends up coming into play with the linker.

So this works, even with overlapping memory regions, as long as you don't have multiple variables attempting to be placed at the same address. For example:

    FLASH                 (rx) : ORIGIN = ROM_ADDR, LENGTH = ROM_SIZE
    BOOTLOADER            (rx) : ORIGIN = 0K, LENGTH = 10K
    IMAGE_0               (rx) : ORIGIN = 10K, LENGTH = 100K
    IMAGE_1               (rx) : ORIGIN = 110K, LENGTH = 100K
    IMAGE_SWAP            (rx) : ORIGIN = 210K, LENGTH = 10K

Trying to place a variable in BOOTLOADER results in an error (overlaps with the application in FLASH)
While placing the variable in IMAGE_SWAP works, even though it overlaps the FLASH region (although not the data in FLASH).

[2/6] Linking C executable zephyr\zephyr_prebuilt.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       81148 B         1 MB      7.74%
      BOOTLOADER:          0 GB        10 KB      0.00%
         IMAGE_0:          0 GB       100 KB      0.00%
         IMAGE_1:          0 GB       100 KB      0.00%
      IMAGE_SWAP:           4 B        10 KB      0.04%
            SRAM:       16016 B       256 KB      6.11%
        IDT_LIST:         104 B         2 KB      5.08%
[6/6] Linking C executable zephyr\zephyr.elf

This would not be a solution to the problem without further changes however. The storage partition for example never has any variables in it, so there would still be no errors as FLASH encroaches into it. The linker output is also rather misleading as it implies that there is more flash available than there actually is. The region that the application is put into needs to accurately reflect the space available to it.

JordanYates · 2021-01-21T09:49:53Z

If we are willing to live with the misleading linker output, we could:

Generate linker regions for all partitions under zephyr,flash
Add an overlapping partition for standalone code as in Zephyr: re-work DT flash partitioning for Nordic nRF boards in the tree #29794
Add a new Kconfig symbol CODE_PARTITION, which would default to 'standalone', with alternate defaults when CONFIG_MCUBOOT or other symbols
Remove the zephyr,code-partition chosen node, DT_USE_CODE_PARTITION, FLASH_LOAD_OFFSET, FLASH_LOAD_SIZE, etc
Add an alias to map FLASH to the region we actually want in the linker file REGION_ALIAS(FLASH CONFIG_CODE_PARTITION);

This approach has much to recommend it IMO. Flash sizes for standalone applications become explicit. A significant chunk of the devicetree Kconfig interaction is reduced to a single symbol with minimal meaning (what partition does the application go into).

Laczen · 2021-01-21T10:53:11Z

@JordanYates, and what if we would generate linker sections only for partitions under the selected code-partition.

Flash sizes would become explicit for standalone and bootloader applications,
Sections can be used in bootloader applications as well,
Add the possibility for the image to access its version (by defining a header section in the partition),
Add the possibility to reserve space for the image trailer (by defining a trailer section in the code partition),
Defining a (sub)partition with label "image" could identify where the image should go. So the REGION_ALIAS() could map FLASH to "image".
No need to have any Kconfig symbols, just use the code-partition.

For standalone applications this would fallback to the partitions defined under flash0 (although IMO it would be better to redefine the flash layout in this case).

JordanYates · 2021-01-21T11:13:34Z

what if we would generate linker sections only for partitions under the selected code-partition

I'm not sure what you mean here, it is memory regions we need to generate, not sections.
Additionally, code-partition points at a partition, not a collection of partitions. I am also proposing to do away with that chosen node altogether.

Defining a (sub)partition with label "image" could identify where the image should go. So the REGION_ALIAS() could map FLASH to "image".

This would break the ability for the same application to compile as standalone or chain-loaded. The purpose of the Kconfig symbol is to allow CONFIG_MCUBOOT to continue 'just working'.

Laczen · 2021-01-21T13:47:27Z

what if we would generate linker sections only for partitions under the selected code-partition

I'm not sure what you mean here, it is memory regions we need to generate, not sections.
Semantics, read as memory regions.
Additionally, code-partition points at a partition, not a collection of partitions. I am also proposing to do away with that chosen node altogether.

If the partition it points to has subpartitions it can also point to a collection of partitions.

Defining a (sub)partition with label "image" could identify where the image should go. So the REGION_ALIAS() could map FLASH to "image".

This would break the ability for the same application to compile as standalone or chain-loaded. The purpose of the Kconfig symbol is to allow CONFIG_MCUBOOT to continue 'just working'.

The proposal would not break this ability. When there are no subpartitions everything would remain as is and CONFIG_MCUBOOT would continue to just work. However it would be possible to use an overlay with a more detailed split up and remove the usage of CONFIG_MCUBOOT (and avoid having to change Kconfig.zephyr for other bootloaders). Depending on whether a subpartition with the label "image" is defined the REGION_ALIAS() could either point to the complete partition or the subpartition with label "image".

A fit for all applications could be defined in one dts-file:
\flash0\image
\flash0\image\bootloader
\flash0\image\bootloader\image
\flash0\image\image0
\flash0\image\image0\image
\flash0\image\image0\image\image-hdr
\flash0\image\image0\image\image
...
\flash0\storage

compiling for code-partition = &flash0 would generate memory regions image and storage and maps FLASH to image,
compiling for code-partition = &bootloader would generate memory region /image/bootloader and map FLASH to image/bootloader/image,
compiling for code-partition = &image0 would generate memory regions /image/image0/image and map FLASH to image/image0/image.
compiling for code-partition = &image0/image would generate ... and map FLASH to image/image0/image/image.

The partitioning could probably be simplified if the is some rule like: "if there is no subpartition labeled "image" then map the flash to the complete partition".

JordanYates · 2021-01-21T14:09:08Z

From what I can tell, you are advocating for the solution where the decision to use a bootloader or not happens before either devicetree or Kconfig have run, either in CMake or from the command line (There is no other way to swap around which partition has the "image" label AFAICT).

That approach allows Kconfig symbols to be dropped at the cost of adding another configuration layer to the build system.
Personally I think a single Kconfig symbol that selects which partition to use is a small price to pay for not complicating the build system further.

Laczen · 2021-01-21T14:23:02Z

From what I can tell, you are advocating for the solution where the decision to use a bootloader or not happens before either devicetree or Kconfig have run, either in CMake or from the command line (There is no other way to swap around which partition has the "image" label AFAICT).

That approach allows Kconfig symbols to be dropped at the cost of adding another configuration layer to the build system.
Personally I think a single Kconfig symbol that selects which partition to use is a small price to pay for not complicating the build system further.

No, no, I am not advocating for another solution. The only selection would happen by the setting of code-partition (as it already is). This would generate a linker region "image" from its subpartitions (not subsubpartitions) and this would be where the image should go.

OK, it will not be so straightforward because you cannot have multiple partitions with the same label.

But by turning the idea around a bit it might even be better: by selecting code-partition the region where the image should live is defined and this generates linker memory regions from the parent partition (if there is a parent).

ioannisg · 2021-01-21T16:14:54Z

That approach allows Kconfig symbols to be dropped at the cost of adding another configuration layer to the build system.
Personally I think a single Kconfig symbol that selects which partition to use is a small price to pay for not complicating the build system further.

I guess that was what was presented in #29794 (if i understood the comment correctly)

Laczen · 2021-01-21T19:10:08Z

@JordanYates, for some reason your name does not easily pop-up on github. I created a new issue #31487 for image generation. This could be extended with linker memory regions.

JordanYates · 2021-01-21T22:43:47Z

I guess that was what was presented in #29794 (if i understood the comment correctly)

Not quite, your proposal adds one symbol, I am proposing deleting all current symbols except for one.
The current FLASH_LOAD_OFFSET, FLASH_LOAD_SIZE and USE_DT_CODE_PARTITION symbols can all be removed if the application must be placed into a partition. If users wish to modify where the image is placed, that becomes a devicetree overlay for that application, instead of overriding Kconfig symbol defaults.

I think I will mock up this solution on a few boards just to see if there are unforeseen problems.

JordanYates · 2021-03-24T10:47:38Z

Superseded by #33656

Jordan Yates added 5 commits January 3, 2021 12:21

tests: util: test FOR_EACH_NESTED

0135ae6

Add test cases for the `FOR_EACH_NESTED` function. Signed-off-by: Jordan Yates <[email protected]>

dts: mtd: partition: add linker-region property

6bf8c55

Add a new property to the child binding of `fixed-partition` to signify that a memory region in the linker should be generated for this partition. Signed-off-by: Jordan Yates <[email protected]>

linker: arm: generate memory regions from dt

6731782

Insert macros for generating memory regions from flash partitions that have set the `linker-region` property. Signed-off-by: Jordan Yates <[email protected]>

JordanYates requested review from MaureenHelm, andrewboie, andyross, carlescufi, galak, ioannisg, lemrey, nashif and stephanosio as code owners January 3, 2021 05:14

Jordan Yates added 2 commits January 3, 2021 16:31

samples: app_dev: linker region example

44e4177

Demonstrate how to place variables at constants memory addresses defined by devicetree partitions. Signed-off-by: Jordan Yates <[email protected]>

boards: enable linker regions on nrf subset

268a36f

Enable the `linker-region` property on the NVS partition for a subset of nordic boards to validate CI behaviour. Signed-off-by: Jordan Yates <[email protected]>

zephyrbot added platform: nRF Nordic nRFx area: Device Tree area: Base OS Base OS Library (lib/os) area: Testsuite Testsuite labels Jan 8, 2021

zephyrbot requested review from carlocaione and mbolivar-nordic January 8, 2021 14:39

mbolivar-nordic added the dev-review To be discussed in dev-review meeting label Jan 11, 2021

galak removed the dev-review To be discussed in dev-review meeting label Mar 11, 2021

JordanYates closed this Mar 24, 2021

carlocaione mentioned this pull request May 19, 2021

Add reserved-memory support #35460

Merged

JordanYates deleted the 210102_linker_sections branch August 15, 2022 11:43

linker: generate memory regions from devicetree partitions #31077

linker: generate memory regions from devicetree partitions #31077

Uh oh!

Conversation

JordanYates commented Jan 3, 2021

Uh oh!

mbolivar-nordic commented Jan 11, 2021

Uh oh!

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

Uh oh!

Uh oh!

ioannisg commented Jan 14, 2021

Uh oh!

JordanYates commented Jan 14, 2021

Uh oh!

ioannisg commented Jan 14, 2021

Uh oh!

JordanYates commented Jan 14, 2021

Uh oh!

galak commented Jan 14, 2021

Uh oh!

ioannisg commented Jan 14, 2021

Uh oh!

JordanYates commented Jan 14, 2021

Uh oh!

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

Uh oh!

Uh oh!

ioannisg commented Jan 15, 2021

Uh oh!

JordanYates commented Jan 15, 2021

Uh oh!

Laczen commented Jan 15, 2021

Uh oh!

galak commented Jan 20, 2021

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Laczen commented Jan 21, 2021

Uh oh!

JordanYates commented Jan 21, 2021

Uh oh!

Laczen commented Jan 21, 2021

Uh oh!

JordanYates commented Jan 21, 2021

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

JordanYates commented Jan 21, 2021

Uh oh!

JordanYates commented Mar 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

JordanYates commented Jan 14, 2021 •

edited

Loading

JordanYates commented Jan 14, 2021 •

edited

Loading

JordanYates commented Jan 21, 2021 •

edited

Loading

JordanYates commented Jan 21, 2021 •

edited

Loading

Laczen commented Jan 21, 2021 •

edited

Loading

ioannisg commented Jan 21, 2021 •

edited

Loading

Laczen commented Jan 21, 2021 •

edited

Loading