Skip to content

rimage: manifest: Make "cold" section size 0 when not used#10612

Open
dbaluta wants to merge 1 commit intothesofproject:mainfrom
dbaluta:fix_cold_store
Open

rimage: manifest: Make "cold" section size 0 when not used#10612
dbaluta wants to merge 1 commit intothesofproject:mainfrom
dbaluta:fix_cold_store

Conversation

@dbaluta
Copy link
Collaborator

@dbaluta dbaluta commented Mar 9, 2026

There are some non-critical data and code sections that are kept in DRAM to be accessed and executed from there without being copyind to SRAM.

Such sections are marked as detached and linked into a separate "cold.mod" module.

rimage considers zones starting at SOF_MODULE_DRAM_LINK_START to SOF_MODULE_DRAM_LINK_END to be cold and links them separately.

On i.MX8MP for M7 support this overalps with ITCM/DTCM/OCRAM areas which causes boot problems.

So, treat all sections as SRAM when CONFIG_COLD_STORE_EXECUTE_DRAM is not enabled by making DRAM link sections size equal to 0 in this case.

Closes: #10602

Copilot AI review requested due to automatic review settings March 9, 2026 16:04
There are some non-critical data and code sections that are kept in DRAM
to be accessed and executed from there without being copied to SRAM.

Such sections are marked as `detached` and linked into a separate
"cold.mod" module.

rimage considers zones starting at SOF_MODULE_DRAM_LINK_START to
SOF_MODULE_DRAM_LINK_END to be cold and links them separately.

On i.MX8MP for M7 support this overalps with ITCM/DTCM/OCRAM areas which
causes boot problems.

So, treat all sections as SRAM when CONFIG_COLD_STORE_EXECUTE_DRAM is
not enabled by making DRAM link sections size equal to 0 in this case.

Closes: thesofproject#10602
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a boot problem on i.MX8MP (M7) where ITCM/DTCM/OCRAM memory sections were incorrectly treated as "detached" (cold/DRAM) by rimage, causing them to be omitted from the output image. The fix makes SOF_MODULE_DRAM_LINK_END equal to SOF_MODULE_DRAM_LINK_START (both 0) when CONFIG_COLD_STORE_EXECUTE_DRAM is not enabled, effectively collapsing the DRAM link range to empty and ensuring all sections are treated as SRAM.

Changes:

  • When CONFIG_COLD_STORE_EXECUTE_DRAM is disabled, SOF_MODULE_DRAM_LINK_END is defined as SOF_MODULE_DRAM_LINK_START (0), making the DRAM detection range empty.
  • When CONFIG_COLD_STORE_EXECUTE_DRAM is enabled, behavior is unchanged (SOF_MODULE_DRAM_LINK_END = 0x08000000).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +260 to +264
#if CONFIG_COLD_STORE_EXECUTE_DRAM
#define SOF_MODULE_DRAM_LINK_END 0x08000000
#else
#define SOF_MODULE_DRAM_LINK_END SOF_MODULE_DRAM_LINK_START
#endif
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The comment block above SOF_MODULE_DRAM_LINK_START (lines 253–258) only accurately describes the behavior when CONFIG_COLD_STORE_EXECUTE_DRAM is enabled. After this change, when the config is not enabled, SOF_MODULE_DRAM_LINK_END equals SOF_MODULE_DRAM_LINK_START (i.e., 0), making the DRAM link range empty and disabling detached-section treatment entirely. The comment should be updated to reflect this conditional behavior — for example, noting that when CONFIG_COLD_STORE_EXECUTE_DRAM is not set, the range is empty and no sections are treated as detached.

Copilot uses AI. Check for mistakes.
@lyakh
Copy link
Collaborator

lyakh commented Mar 10, 2026

@dbaluta The change looks good, but CI fails, and I think I know why. That header that you're modifying is also included for rimage build and there CONFIG_COLD_STORE_EXECUTE_DRAM isn't set. So you need to modify your condition to something like #if CONFIG_COLD_STORE_EXECUTE_DRAM || !CONFIG_ZEPHYR_SOF_MODULE - you should be able to build test it (including rimage of course) for PTL too
UPDATE: but then it will break rimage for you again... Maybe rimage build should also include autoconf.h but that might be a bit more difficult...
UPDATE 2: this would be a bit controversial because we would prefer to have a single rimage executable for all architectures, but in a way this is already not the case: those DRAM start and end addresses are clearly architecture specific

@dbaluta
Copy link
Collaborator Author

dbaluta commented Mar 10, 2026

@lyakh oh this is so unfortunate. Yes, I would also prefer same rimage for all architectures. Maybe add a runtime parameter to ignore detached sections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

What is the role of detached section?

3 participants