Skip to content

cmake: Handle MPU_ALIGN in CONFIG_CMAKE_LINKER_GENERATOR #89387

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+42 −23
Open

cmake: Handle MPU_ALIGN in CONFIG_CMAKE_LINKER_GENERATOR #89387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aba632f
cmake: linker generator: Add recursive @variables@
bjorniuppsala Mar 3, 2025
684d252
cmake: linker generator: MPU_ALIGN for CONFIG_CMAKE_LINKER_GENERATOR
bjorniuppsala Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions cmake/linker/linker_script_common.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,27 +713,46 @@ foreach(file IN LISTS PREPROCESSOR_FILES )
endforeach()

# To pickup information gathered by the scripts from the previous pass, we use
# the syntax @FOO@ where FOO is a cmake variable name
# the syntax @FOO[,undef:VALUE][,var=VALUE]@
# The expansion is recursive so if FOO contains @BAR@ it @BAR will also be
# expanded
# Where the undef:VALUE gets picked if we dont find FOO
# and foo=bar defines a (local) variable in the coming expansion
# The variable values are kept in cmake varaiables, MOSTLY with the
# AT_VAR_ prefix to avoid name-clashes
function(do_var_replace_in res_ptr src)
string(REGEX MATCHALL "@([^@]*)@" match_res "${src}")
string(REGEX MATCHALL "@[^@]*@" match_res "${src}")
foreach(match IN LISTS match_res)
string(REPLACE "@" "" expr ${match})
# the variable expression is as follows:
# @NAME[,undef:VALUE]@ Where the VALUE gets picked if we dont find NAME
string(REPLACE "," ";" expr ${expr})
list(GET expr 0 var)
#Drop the leading and closing @
string(REGEX MATCH "@([^@]*)@" expr ${match})

# Turn the , into ; to treat it as a list:
string(REPLACE "," ";" expr ${CMAKE_MATCH_1})
unset(undef_value)
list(POP_FRONT expr var)
foreach(e IN LISTS expr)
if(e MATCHES "undef:([^,]*)")
set(undef_value ${CMAKE_MATCH_1})
elseif(e MATCHES "([^=;]+)=([^=;]+)")
set(AT_VAR_${CMAKE_MATCH_1} ${CMAKE_MATCH_2})
endif()
endforeach()

if(DEFINED "AT_VAR_${var}")
set(value "${AT_VAR_${var}}")
elseif(DEFINED ${var}) # set by zephyr_linker_include_generated files
elseif(DEFINED ${var})
set(value "${${var}}")
elseif("${expr}" MATCHES ";undef:([^,]*)")
set(value "${CMAKE_MATCH_1}")
elseif(DEFINED undef_value)
set(value "${undef_value}")
else()
set(value "${match}")
# can't warn here because we can't check for what is relevant in this pass
# message(WARNING "Missing definition for ${match}")
continue()
endif()

#Resolve variables inside variables
do_var_replace_in(value "${value}")

if(CMAKE_VERBOSE_MAKEFILE)
message("Using variable ${match} with value ${value}")
endif()
Expand Down
13 changes: 6 additions & 7 deletions cmake/linker_script/arm/linker.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,15 @@ if((NOT DEFINED CONFIG_CUSTOM_SECTION_ALIGN) AND DEFINED CONFIG_MPU_REQUIRES_PO
# . = ALIGN( 1 << LOG2CEIL(region_size))
# Handling this requires us to handle log2ceil() in iar linker since the size
# isn't known until then.
set(MPU_ALIGN_BYTES ${region_min_align})
zephyr_linker_include_var(VAR MPU_ALIGN VALUE "MAX(${region_min_align} , 1 << LOG2CEIL(@region_size@) )")
Copy link
Contributor

Choose a reason for hiding this comment

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

LOG2CEIL is problematic because it's not universally supported across linkers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True,

This is problematic for iar ilink too, I have a "solution" for ilink that does the calculation of log2ceil between pre1 and final linking phase, via some creative use of the @variables@ and scripts (you can see the details at 4ceaa15 not done, please be polite).

We could of course add something similar for all linkers, but in the end the problem remains, what power we want to allow in the expression language. I think it makes sense to try to let ld be the role-model and as best it is possible cover up the shortcomings of the other linkers.
Also, it is easy, if not optimal, to have conditional things in the main cmake machinery for some of these corner cases (CONFIG_CUSTOM_SECTION_ALIGN).

This PR takes CMAKE_LINKER_GENERATOR && CONFIG_USERSPACE closer to working, at least for ld. If we decide to use my iar workaround for other linkers it makes sense to use it for ld too just to make things more similar. Lets tackle that fight when the time comes, knowing that my workaround is a possible solution if we dont find any thing else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tejlmand Do you consider this a blocker for the PR ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm concerned that if we allow to enter ld functions here then we are on a path where things will be hard to handle.

If we need such functionality, then I think a dedicated CMake function is better, and then the linker generator implementation itself can decide how best to map the requested feature.

Copy link
Contributor Author

@bjorniuppsala bjorniuppsala Jun 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Have you looked at my IAR ILink solution ( 4ceaa15 ) ?

It expands e.g. LOG2CEIL with a special tag (@IAR_EVALUATE@ ), which the generator at each linking pass puts into a json list of required expressions. This becomes an extra output from the generator, and after linking the pass a script looks at the list of expressions and evaluates them, using the linking result to lookup symbol values. In the next linking pass the values can be used, at least if not too much has changed (which it hasn't between pre1 and final, for MPU_ALIGN).

This is reasonably well hidden behind @variables@ evaluation, so the impact on the generator itself is acceptable I think.
The python script it self needs some work to be "industrial strength", but I think the overall setup works.

It is rather cumbersome though, and quite hard to follow, and not what you usually expect to happen when "just linking" a project. But I see no other alternatives really, besides removing the problem altogether and disallow log2ceil for all and just force the user to set a kcnofig variable with the approriate alignment manually. It is easy enoguh to have a link time check that it follows the requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am heading out on vacation the next two weeks (until June 30th). I might check in here, but please be patient ;)

I would very much like input or suggestions on how to do this, and bridge the gap between linkers.
When I did this I thought it would be only ILink that needed the extra help with log2ceil and such...

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you looked at my IAR ILink solution ( 4ceaa15 ) ?

yes I did, and especially I find constructs like this problematic:

zephyr/cmake/linker_script/arm/linker.cmake

Lines 26 to 33 in 4ceaa15

set(MPU_ALIGN "MAX(${region_min_align} , 2 << LOG2CEIL(@region_size@) )")
if(ZEPHYR_TOOLCHAIN_VARIANT STREQUAL "iar")
# Ilink needs help with the align-to-power-of-2-of-region-size thing.
# So tag it with IAR_EVALUATE to be handled by iar_linker_evaluate.py
# Note that we need to escape the enclosing @ to get the evaluation
# order right
set(MPU_ALIGN "@AT@IAR_EVALUATE,undef:0,expr=${MPU_ALIGN}@AT@")
endif()

because if we allow ld functions to be written directly then we cannot expect an author to know all possible linkers out there, not to mention it becomes hard to have an out-of-tree linker generator implementation.

Also it allow code to enter and fail for other linkers, for example for IAR until @bjorniuppsala becomes aware of the extra case.

You may say that relying on a function also requires for someone writing the functionality, like in case above first author will likely add the GNU ld support. An oot linker can implement their own.

And you can implement the IAR variant when you have time.
Also it avoids this if this toolchain testing everywhere, cause the function is just called, and toolchain provides the implementation (and with failure / unsupported error message if not ready yet.).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I agree 100% that the ZEPHYR_TOOLCHAIN_VARIANT check is the wrong thing to have there.

What I meant was what do you think of the basic approach of catching these expressions, and using script to evaluate them and inject the result in the next linking pass ?

With the machinery I have proposed (recursive @variables@ and all that jazz..) we could define sections like

zephyr_linker_include_var(VAR MPU_ALIGN VALUE "@EVAL,undef:4,expr=MAX(${region_min_align} , 2 << LOG2CEIL(@region_size@)" )


zephyr_linker_section( ... ALIGN "@MPU_ALIGN,region_size=foo_end - foo_start@")

(With some trick/escape-syntax to tag foo_end and foo_start as linker symbols)

and then in e.g. linker/foo/target.cmake add a defintion of what EVAL does. (for ld we could just expand to @expr@, and for others make it go through the evaluation scripts).

# linker/ld/target.cmake
# Expand @EVAL,expr=... into the expr itself
zephyt_linker_include_var( VAR EVAL VALUE "@expr@")

# linker/iar/target.cmake
# Use the ruby script to evaluate/emulate ld-expressions for @EVAL,expr=...
zephyt_linker_include_var( VAR EVAL VALUE "@AT@EVAL_USING_SCRIPT,expr=@expr@@AT@")

(Do note that in the above expression ${region_min_align} refers to a mcake variable expanded at cmake-generation time, and @region_size@ refers to an "argument" to the expansion that is evaluated at linker file generation time. The mixing is somewhat confusing, and could easily be turned into linker-file-generation-time-only by having the region_min_align value be visible as a @variable@ too.)

When running the linker-file-generator for a pass, it processes all the EVAL_USING_SCRIPT tagged variable-expansions and stores them into a json blob. After linking the pass a script crunches the json and the linker output to find symbol values, and evaluates all the expressions, the successfully evaluated expressions are then fed into the next linker pass as specially named variables @EVALUATED_@ that are picked up as expansions to the @EVAL_USING_SCRIPT...@ stuff.

Except for the horrible syntax I think the principle is sound.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Come to think about it, maybe all of the posibly-eval-expression-arguments to zephyr_linker_* functions (e.g. ALIGN, SUBALIGN, MIN_SIZE,... ) should have an implicit @EVAL_USING_SCRIPT@ thing on them ? That way the syntax gets somewhat less clumsy.

#message(WARNING "We can not handle . = ALIGN( 1 << LOG2CEIL(region_size)) ")
else()
set(MPU_ALIGN_BYTES ${region_min_align})
zephyr_linker_include_var(VAR MPU_ALIGN VALUE "${region_min_align}")
endif()
# The APP_SHARED_ALIGN and SMEM_PARTITION_ALIGN macros are defined as
# ". = ALIGN(...)" things.
# the cmake generator stuff needs an align-size in bytes so:
zephyr_linker_include_var(VAR APP_SHARED_ALIGN_BYTES VALUE ${region_min_align})
zephyr_linker_include_var(VAR SMEM_PARTITION_ALIGN_BYTES VALUE ${MPU_ALIGN_BYTES})

zephyr_linker_include_var(VAR APP_SHARED_ALIGN VALUE ${region_min_align})
# Note that MPU_ALIGN (may) require an argument (region_size
zephyr_linker_include_var(VAR SMEM_PARTITION_ALIGN VALUE "@MPU_ALIGN@")

# Note, the `+ 0` in formulas below avoids errors in cases where a Kconfig
# variable is undefined and thus expands to nothing.
Expand Down
11 changes: 6 additions & 5 deletions scripts/build/gen_app_partitions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,9 @@ def zephyr_linker_symbol(symbol, expr) :

class CmakeTemplate:
section_name = "@_APP_SMEM{SECTION}_SECTION_NAME@"
smem_partition_align = "@SMEM_PARTITION_ALIGN,region_size=z_data_smem_{partition}_bss_end - z_data_smem_{partition}_part_start@"
data_template = (
zephyr_linker_section_configure(section=section_name, align="@SMEM_PARTITION_ALIGN_BYTES@")+
zephyr_linker_section_configure(section=section_name, align=smem_partition_align)+
zephyr_linker_section_configure(section=section_name, input="data_smem_{partition}_data*", symbols="z_data_smem_{partition}_part_start", keep=True)
)

Expand All @@ -294,20 +295,20 @@ class CmakeTemplate:

footer_template = (
zephyr_linker_section_configure(section=section_name, symbols="z_data_smem_{partition}_bss_end", keep=True) +
zephyr_linker_section_configure(section=section_name, align="@SMEM_PARTITION_ALIGN_BYTES@") +
zephyr_linker_section_configure(section=section_name, align=smem_partition_align) +
zephyr_linker_section_configure(section=section_name, symbols="z_data_smem_{partition}_part_end", keep=True)
)

linker_start_seq = (
zephyr_linker_section(name=section_name, group="APP_SMEM_GROUP", noinput=True, align_with_input=True) +
zephyr_linker_section_configure(section=section_name, align="@APP_SHARED_ALIGN_BYTES@", symbols="_app_smem{section}_start"))
zephyr_linker_section_configure(section=section_name, align="@APP_SHARED_ALIGN@", symbols="_app_smem{section}_start"))

linker_end_seq = (
zephyr_linker_section_configure(section=section_name, align="@APP_SHARED_ALIGN_BYTES@") +
zephyr_linker_section_configure(section=section_name, align="@APP_SHARED_ALIGN@") +
zephyr_linker_section_configure(section=section_name, symbols="_app_smem{section}_end") )

empty_app_smem = (
zephyr_linker_section(name=section_name, group="APP_SMEM_GROUP", align="@APP_SHARED_ALIGN_BYTES@", noinput=True, align_with_input=True) +
zephyr_linker_section(name=section_name, group="APP_SMEM_GROUP", align="@APP_SHARED_ALIGN@", noinput=True, align_with_input=True) +
zephyr_linker_section_configure(section = section_name, symbols="_app_smem{section}_start") +
zephyr_linker_section_configure(section = section_name, symbols="_app_smem{section}_end") )

Expand Down
Loading