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

RFC: memory regions from devicetree & explicit code locations #33656

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 27 additions & 0 deletions boards/arm/nrf52840dk_nrf52840/dts/mcuboot_partitions.dtsi
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2021, Commonwealth Scientific and Industrial Research
* Organisation (CSIRO) ABN 41 687 119 230.
*
* SPDX-License-Identifier: Apache-2.0
*/

&flash0 {
partitions {
boot_partition: partition@0 {
label = "mcuboot";
reg = <0x000000000 0x0000C000>;
};
slot0_partition: partition@c000 {
label = "image-0";
reg = <0x0000C000 0x00067000>;
};
slot1_partition: partition@73000 {
label = "image-1";
reg = <0x00073000 0x00067000>;
};
scratch_partition: partition@da000 {
label = "image-scratch";
reg = <0x000da000 0x0001e000>;
};
};
};
14 changes: 14 additions & 0 deletions boards/arm/nrf52840dk_nrf52840/layouts/bootloader.overlay
@@ -0,0 +1,14 @@
/*
* Copyright (c) 2021, Commonwealth Scientific and Industrial Research
* Organisation (CSIRO) ABN 41 687 119 230.
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <mcuboot_partitions.dtsi>

/ {
chosen {
zephyr,code-partition = &boot_partition;
};
};
22 changes: 22 additions & 0 deletions boards/arm/nrf52840dk_nrf52840/layouts/default.overlay
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2021, Commonwealth Scientific and Industrial Research
* Organisation (CSIRO) ABN 41 687 119 230.
*
* SPDX-License-Identifier: Apache-2.0
*/

/ {
chosen {
zephyr,code-partition = &code_partition;
};
};

&flash0 {

partitions {
code_partition: partition@0 {
label = "code";
reg = <0x000000000 0x000f8000>;
};
};
};
1 change: 1 addition & 0 deletions boards/arm/nrf52840dk_nrf52840/layouts/mcuboot.conf
@@ -0,0 +1 @@
CONFIG_BOOTLOADER_MCUBOOT=y
14 changes: 14 additions & 0 deletions boards/arm/nrf52840dk_nrf52840/layouts/mcuboot.overlay
@@ -0,0 +1,14 @@
/*
* Copyright (c) 2021, Commonwealth Scientific and Industrial Research
* Organisation (CSIRO) ABN 41 687 119 230.
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <mcuboot_partitions.dtsi>

/ {
chosen {
zephyr,code-partition = &slot0_partition;
};
};
23 changes: 0 additions & 23 deletions boards/arm/nrf52840dk_nrf52840/nrf52840dk_nrf52840.dts
Expand Up @@ -19,7 +19,6 @@
zephyr,bt-c2h-uart = &uart0;
zephyr,sram = &sram0;
zephyr,flash = &flash0;
zephyr,code-partition = &slot0_partition;
};

leds {
Expand Down Expand Up @@ -246,28 +245,6 @@ arduino_spi: &spi3 {
#address-cells = <1>;
#size-cells = <1>;

boot_partition: partition@0 {
label = "mcuboot";
reg = <0x000000000 0x0000C000>;
};
slot0_partition: partition@c000 {
label = "image-0";
reg = <0x0000C000 0x00067000>;
};
slot1_partition: partition@73000 {
label = "image-1";
reg = <0x00073000 0x00067000>;
};
scratch_partition: partition@da000 {
label = "image-scratch";
reg = <0x000da000 0x0001e000>;
};

/*
* The flash starting at 0x000f8000 and ending at
* 0x000fffff is reserved for use by the application.
*/

/*
* Storage partition will be used by FCB/LittleFS/NVS
* if enabled.
Expand Down
35 changes: 34 additions & 1 deletion cmake/app/boilerplate.cmake
Expand Up @@ -217,8 +217,25 @@ list(APPEND ARCH_ROOT ${ZEPHYR_BASE})
# Check that BOARD has been provided, and that it has not changed.
zephyr_check_cache(BOARD REQUIRED)

# Search for revision and layout separators
string(FIND "${BOARD}" "@" REVISION_SEPARATOR_INDEX)
if(NOT (REVISION_SEPARATOR_INDEX EQUAL -1))
string(FIND "${BOARD}" "%" LAYOUT_SEPARATOR_INDEX)

if((NOT (REVISION_SEPARATOR_INDEX EQUAL -1)) AND (NOT (LAYOUT_SEPARATOR_INDEX EQUAL -1)))
# Both a revision and layout are specified
math(EXPR BOARD_REVISION_INDEX "${REVISION_SEPARATOR_INDEX} + 1")
math(EXPR BOARD_LAYOUT_INDEX "${LAYOUT_SEPARATOR_INDEX} + 1")
math(EXPR BOARD_REVISION_LEN "${LAYOUT_SEPARATOR_INDEX} - ${REVISION_SEPARATOR_INDEX} - 1")
string(SUBSTRING ${BOARD} ${BOARD_LAYOUT_INDEX} -1 BOARD_LAYOUT)
string(SUBSTRING ${BOARD} ${BOARD_REVISION_INDEX} ${BOARD_REVISION_LEN} BOARD_REVISION)
string(SUBSTRING ${BOARD} 0 ${REVISION_SEPARATOR_INDEX} BOARD)
elseif(NOT (LAYOUT_SEPARATOR_INDEX EQUAL -1))
# Only a layout is specified
math(EXPR BOARD_LAYOUT_INDEX "${LAYOUT_SEPARATOR_INDEX} + 1")
string(SUBSTRING ${BOARD} ${BOARD_LAYOUT_INDEX} -1 BOARD_LAYOUT)
string(SUBSTRING ${BOARD} 0 ${LAYOUT_SEPARATOR_INDEX} BOARD)
elseif(NOT (REVISION_SEPARATOR_INDEX EQUAL -1))
# Only a revision is specified
math(EXPR BOARD_REVISION_INDEX "${REVISION_SEPARATOR_INDEX} + 1")
string(SUBSTRING ${BOARD} ${BOARD_REVISION_INDEX} -1 BOARD_REVISION)
string(SUBSTRING ${BOARD} 0 ${REVISION_SEPARATOR_INDEX} BOARD)
Expand Down Expand Up @@ -293,6 +310,22 @@ if(DEFINED BOARD_REVISION)
string(REPLACE "." "_" BOARD_REVISION_STRING ${BOARD_REVISION})
endif()

if(EXISTS ${BOARD_DIR}/layouts)
# Set the default layout if not explicitly set
if (NOT DEFINED BOARD_LAYOUT)
set(BOARD_LAYOUT "default")
endif()

if (NOT EXISTS ${BOARD_DIR}/layouts/${BOARD_LAYOUT}.overlay)
message(FATAL_ERROR "Layout \"${BOARD_LAYOUT}\" doesn't exist \
(${BOARD_DIR}/layouts/${BOARD_LAYOUT}.overlay)")
endif()
set(BOARD_MESSAGE "${BOARD_MESSAGE}, Layout: ${BOARD_LAYOUT}")
elseif(DEFINED BOARD_LAYOUT)
message(FATAL_ERROR "Board layout ${BOARD_LAYOUT} specified for ${BOARD}, \
but board has no layouts.")
endif()

# Check that SHIELD has not changed.
zephyr_check_cache(SHIELD WATCH)

Expand Down
3 changes: 3 additions & 0 deletions cmake/dts.cmake
Expand Up @@ -49,6 +49,9 @@ if(EXISTS ${DTS_SOURCE})
if(BOARD_REVISION AND EXISTS ${BOARD_DIR}/${BOARD}_${BOARD_REVISION_STRING}.overlay)
list(APPEND DTS_SOURCE ${BOARD_DIR}/${BOARD}_${BOARD_REVISION_STRING}.overlay)
endif()
if(BOARD_LAYOUT AND EXISTS ${BOARD_DIR}/layouts/${BOARD_LAYOUT}.overlay)
list(APPEND DTS_SOURCE ${BOARD_DIR}/layouts/${BOARD_LAYOUT}.overlay)
endif()
else()
set(SUPPORTS_DTS 0)
endif()
Expand Down
4 changes: 4 additions & 0 deletions cmake/kconfig.cmake
Expand Up @@ -55,6 +55,10 @@ if(OVERLAY_CONFIG)
string(REPLACE " " ";" OVERLAY_CONFIG_AS_LIST "${OVERLAY_CONFIG}")
endif()

if((DEFINED BOARD_LAYOUT) AND EXISTS ${BOARD_DIR}/layouts/${BOARD_LAYOUT}.conf)
list(INSERT CONF_FILE_AS_LIST 0 ${BOARD_DIR}/layouts/${BOARD_LAYOUT}.conf)
endif()

if((DEFINED BOARD_REVISION) AND EXISTS ${BOARD_DIR}/${BOARD}_${BOARD_REVISION_STRING}.conf)
list(INSERT CONF_FILE_AS_LIST 0 ${BOARD_DIR}/${BOARD}_${BOARD_REVISION_STRING}.conf)
endif()
Expand Down
72 changes: 15 additions & 57 deletions include/arch/arm/aarch32/cortex_m/scripts/linker.ld
Expand Up @@ -13,14 +13,15 @@

#include <autoconf.h>
#include <linker/sections.h>
#include <linker/devicetree_regions.h>
#include <devicetree.h>

#include <linker/linker-defs.h>
#include <linker/linker-tool.h>

/* physical address of RAM */
#ifdef CONFIG_XIP
#define ROMABLE_REGION FLASH
#define ROMABLE_REGION DT_CODE_PARITION()
#define RAMABLE_REGION SRAM
#else
#define ROMABLE_REGION SRAM
Expand All @@ -33,25 +34,6 @@
#define _DATA_IN_ROM
#endif

#if !defined(CONFIG_XIP) && (CONFIG_FLASH_SIZE == 0)
#define ROM_ADDR RAM_ADDR
#else
#define ROM_ADDR (CONFIG_FLASH_BASE_ADDRESS + CONFIG_FLASH_LOAD_OFFSET)
#endif

#ifdef CONFIG_HAS_TI_CCFG
#define CCFG_SIZE 88
#define ROM_SIZE (CONFIG_FLASH_SIZE*1K - CONFIG_FLASH_LOAD_OFFSET - \
CCFG_SIZE)
#define CCFG_ADDR (ROM_ADDR + ROM_SIZE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

CCFG_ADDR is still being used in the code below:

#ifdef CONFIG_HAS_TI_CCFG
    FLASH_CCFG            (rwx): ORIGIN = CCFG_ADDR, LENGTH = CCFG_SIZE
#endif

causing:

invalid origin for memory region FLASH_CCFG
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems to be corrected in: #34185
So assume this is now pending a rebase after #34185 is merged.

#else
#if CONFIG_FLASH_LOAD_SIZE > 0
#define ROM_SIZE CONFIG_FLASH_LOAD_SIZE
#else
#define ROM_SIZE (CONFIG_FLASH_SIZE*1K - CONFIG_FLASH_LOAD_OFFSET)
#endif
#endif

#if defined(CONFIG_XIP)
#if defined(CONFIG_IS_BOOTLOADER)
#define RAM_SIZE (CONFIG_BOOTLOADER_SRAM_SIZE * 1K)
Expand All @@ -66,21 +48,6 @@
#define RAM_ADDR CONFIG_SRAM_BASE_ADDRESS
#endif

#if DT_NODE_HAS_STATUS(DT_CHOSEN(zephyr_ccm), okay)
#define CCM_SIZE DT_REG_SIZE(DT_CHOSEN(zephyr_ccm))
#define CCM_ADDR DT_REG_ADDR(DT_CHOSEN(zephyr_ccm))
#endif

#if DT_NODE_HAS_STATUS(DT_CHOSEN(zephyr_itcm), okay)
#define ITCM_SIZE DT_REG_SIZE(DT_CHOSEN(zephyr_itcm))
#define ITCM_ADDR DT_REG_ADDR(DT_CHOSEN(zephyr_itcm))
#endif

#if DT_NODE_HAS_STATUS(DT_CHOSEN(zephyr_dtcm), okay)
#define DTCM_SIZE DT_REG_SIZE(DT_CHOSEN(zephyr_dtcm))
#define DTCM_ADDR DT_REG_ADDR(DT_CHOSEN(zephyr_dtcm))
#endif

#if defined(CONFIG_CUSTOM_SECTION_ALIGN)
_region_min_align = CONFIG_CUSTOM_SECTION_MIN_ALIGN_SIZE;
#else
Expand All @@ -106,37 +73,28 @@ _region_min_align = 4;

MEMORY
{
FLASH (rx) : ORIGIN = ROM_ADDR, LENGTH = ROM_SIZE
#ifdef CONFIG_HAS_TI_CCFG
FLASH_CCFG (rwx): ORIGIN = CCFG_ADDR, LENGTH = CCFG_SIZE
#endif
#if DT_NODE_HAS_STATUS(DT_CHOSEN(zephyr_ccm), okay)
CCM (rw) : ORIGIN = CCM_ADDR, LENGTH = CCM_SIZE
#endif
#if DT_NODE_HAS_STATUS(DT_CHOSEN(zephyr_itcm), okay)
ITCM (rw) : ORIGIN = ITCM_ADDR, LENGTH = ITCM_SIZE
#endif
#if DT_NODE_HAS_STATUS(DT_CHOSEN(zephyr_dtcm), okay)
DTCM (rw) : ORIGIN = DTCM_ADDR, LENGTH = DTCM_SIZE
#endif
SRAM (wx) : ORIGIN = RAM_ADDR, LENGTH = RAM_SIZE
#ifdef CONFIG_BT_STM32_IPM
SRAM1 (rw) : ORIGIN = RAM1_ADDR, LENGTH = RAM1_SIZE
SRAM2 (rw) : ORIGIN = RAM2_ADDR, LENGTH = RAM2_SIZE
#endif
#ifdef CONFIG_MEMC_STM32_SDRAM
#if DT_NODE_HAS_STATUS(DT_NODELABEL(sdram1), okay)
SDRAM1 (rw) : ORIGIN = DT_REG_ADDR(DT_NODELABEL(sdram1)), LENGTH = DT_REG_SIZE(DT_NODELABEL(sdram1))
#endif
#if DT_NODE_HAS_STATUS(DT_NODELABEL(sdram2), okay)
SDRAM2 (rw) : ORIGIN = DT_REG_ADDR(DT_NODELABEL(sdram2)), LENGTH = DT_REG_SIZE(DT_NODELABEL(sdram2))
#endif
#endif
#ifdef CONFIG_STM32_BACKUP_SRAM
BACKUP_SRAM (rw) : ORIGIN = DT_REG_ADDR(DT_NODELABEL(backup_sram)), LENGTH = DT_REG_SIZE(DT_NODELABEL(backup_sram))
#endif

/* Add custom memory regions specified by devicetree partitions */
DT_REGIONS_FROM_CHOSEN_FLASH(zephyr_flash)

/* Used by and documented in include/linker/intlist.ld */
IDT_LIST (wx) : ORIGIN = (RAM_ADDR + RAM_SIZE), LENGTH = 2K

DT_REGION_FROM_NODE(DT_NODELABEL(sdram1))
DT_REGION_FROM_NODE(DT_NODELABEL(sdram2))
DT_REGION_FROM_NODE(DT_NODELABEL(backup_sram))

DT_REGION_FROM_NODE(DT_CHOSEN(zephyr_ccm))
DT_REGION_FROM_NODE(DT_CHOSEN(zephyr_itcm))
DT_REGION_FROM_NODE(DT_CHOSEN(zephyr_dtcm))
}

ENTRY(CONFIG_KERNEL_ENTRY)
Expand All @@ -162,7 +120,7 @@ SECTIONS

GROUP_START(ROMABLE_REGION)

_image_rom_start = ROM_ADDR;
_image_rom_start = .;
Copy link
Collaborator

@tejlmand tejlmand Apr 12, 2021

Choose a reason for hiding this comment

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

This is changing the value of _image_rom_start from a calculated address to 0x0.

This means that code that refers to _image_rom_start might get a wrong value.

This also results in:

_flash_used = LOADADDR(.last_section) - _image_rom_start;

being calculated wrongly.

As example, building hello world for cc3235sf_launchxl with CONFIG_XIP=n on master gives me:

$ objdump -x zephyr/zephyr.elf |grep _image_rom_start
20000000 g       *ABS*  00000000 _image_rom_start
$ objdump -x build_xip_ref/zephyr/zephyr.elf |grep _flash_used
00004158 g       *ABS*  00000000 _flash_used

but with this PR:

$ objdump -x build_xip/zephyr/zephyr.elf |grep _image_rom_start
00000000 g       rom_start      00000000 _image_rom_start
$ objdump -x build_xip/zephyr/zephyr.elf |grep _flash_used
20004158 g       *ABS*  00000000 _flash_used

so any code relying on those two symbols will fail.


SECTION_PROLOGUE(rom_start,,)
{
Expand Down
43 changes: 43 additions & 0 deletions include/linker/devicetree_regions.h
@@ -0,0 +1,43 @@
/*
* Copyright (c) 2021, Commonwealth Scientific and Industrial Research
* Organisation (CSIRO) ABN 41 687 119 230.
*
* SPDX-License-Identifier: Apache-2.0
*
* Generate memory regions from devicetree partitions.
* Partitions are only considered if they exist under chosen/zephyr,flash.
*/

/* Partitions under chosen/zephyr,flash */
#define _CHOSEN_PARTITIONS(p) UTIL_CAT(DT_CHOSEN(p), _S_partitions)

/* Declare a memory region */
#define _REGION_DECLARE(node) DT_LABEL(node) (rx) : \
ORIGIN = DT_REG_ADDR(node), \
LENGTH = DT_REG_SIZE(node)

/* Apply a macro to a partition list, if it exists */
#define _FLASH_PARITIONS_APPLY(p, f) \
COND_CODE_1(DT_NODE_EXISTS(p), (DT_FOREACH_CHILD(p, f)), ())

/* Apply a macro to a node, if it is 'okay' */
#define _OKAY_NODE_APPLY(n, f) \
COND_CODE_1(DT_NODE_HAS_STATUS(n, okay), (f(n)), ())

/* Execution partition */
#define DT_CODE_PARITION() DT_LABEL(DT_CHOSEN(zephyr_code_partition))

/**
* @brief Generate region definitions for all devicetree linker regions
*/
#define DT_REGIONS_FROM_CHOSEN_FLASH(chosen_flash) \
_FLASH_PARITIONS_APPLY(_CHOSEN_PARTITIONS(chosen_flash), \
_REGION_DECLARE)

/**
* @brief Generate a region definition from an enabled devicetree node
*/
#define DT_REGION_FROM_NODE(node) \
COND_CODE_1(DT_NODE_EXISTS(node), \
(_OKAY_NODE_APPLY(node, _REGION_DECLARE)), \
())