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

zephyr: added address/size of bootloader/firmware shared memory area #2174

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

m5k8
Copy link
Contributor

@m5k8 m5k8 commented Jan 13, 2025

Added definitions of base address and size of user-defined shared memory area between bootloader and runtime firmware, when using BOOT_SHARE_BACKEND_EXTERNAL.

It's possible to select Kconfig BOOT_SHARE_BACKEND_EXTERNAL, but then mcuboot expects the following symbols:
MCUBOOT_SHARED_DATA_BASE and MCUBOOT_SHARED_DATA_SIZE. There was no way to set them via KConfig and compilation fails.

This patch adds KConfig symbols that give the possibility to set them.

Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

This is not how things are done in zephyr where there is devicetree, this is for basic bare metal applications where such a feature is not available

@m5k8
Copy link
Contributor Author

m5k8 commented Jan 14, 2025

Yes, I've seen that there is support for shared data via retention module. Feature is available, but it adds bytes to both bootloader and firmware, and I'm trying to strip unnecessary things off, to save bytes.

Retention module is really unnecessary for passing few static constant bytes. It's more appropriate for storing really variable runtime retention data, often in battery backed SRAM, to keep them across power offs or resets.

On the side note, this shared data is static constant export form bootloader, so more compact way would be to share only pointer to some const byte array located in bootloader, instead of rewriting const data to RAM, which is wasteful - measured in code and RAM area.

In my case I'm reserving some space in RAM (telling device tree that RAM starts with some offset and having few bytes less) and than accessing that area directly. Bare metal? Yes, but why not, it's easy. It becomes some signature area located somewhere in memory at known address.

There already is possibility to select CONFIG_BOOT_SHARE_BACKEND_EXTERNAL config option in Mcuboot Zephyr Kconfig, and that leads to compilation failure. This patch completes support for this option. Elegant solution is either this patch or complete removal of CONFIG_BOOT_SHARE_BACKEND_EXTERNAL option - that would send a signal that's not supported intentionally, and at the moment it's a bummer - I can select it, but there's no way to supply the following required options.

In my opinion, it's better to have complete support for various mcuboot options, someone may need it sometime. Like me at the moment. Cost is zero. I'm using it and would prefer to not keep my local private patches, to ease upgrades in the future.

Please reconsider.

Added definitions of base address and size of user-defined
shared memory area between bootloader and runtime firmware, when using
BOOT_SHARE_BACKEND_EXTERNAL.

It's possible to select Kconfig BOOT_SHARE_BACKEND_EXTERNAL, but then
mcuboot expects the following symbols:
MCUBOOT_SHARED_DATA_BASE and MCUBOOT_SHARED_DATA_SIZE.
There was no way to set them via KConfig and compilation fails.

This patch adds KConfig symbols that give the possibility to set them.

Signed-off-by: Michael Konieczny <[email protected]>
@m5k8 m5k8 force-pushed the add-shared-data-params branch from 6c8c217 to 8f9956f Compare January 14, 2025 09:44
@nordicjm
Copy link
Collaborator

What I mean is that, these symbols should come from device tree, so it's fine to have it but you need to get them from that, with a chosen node

@m5k8
Copy link
Contributor Author

m5k8 commented Jan 24, 2025

As of now, there's no way to get these symbols from device tree. mcuboot_config.h doesn't have required logic. And it doesn't make sense, in my opinion. Please bear with me.

These symbols are used only when BOOT_SHARE_BACKEND_EXTERNAL is selected.
You are right that it's not "the kosher proper Zephyr way". The Zephyr way is to define retention RAM area in device tree, but then CONFIG_BOOT_SHARE_BACKEND_RETENTION is selected, not BOOT_SHARE_BACKEND_EXTERNAL, and MCUBOOT_SHARED_DATA_BASE / MCUBOOT_SHARED_DATA_SIZE are not used at all and it compiles fine.
BOOT_SHARE_BACKEND_EXTERNAL is used only when we want this shared area to be outside of OS governance, hence the _EXTERNAL name.
It's not typical nor popular use case with Zephyr, but I went this way, due to required FLASH space savings.

As of now, menuconfig allows selecting BOOT_SHARE_BACKEND_EXTERNAL, and doesn't offer any way to define missing symbols, resulting in compilation failure. My patch corrects that corner case, and it's intended to be used without device tree RAM retention area. It makes this _EXTERNAL option usable. Without that, we get errors. If you insist that user shall not have such option with Zephyr, removal of BOOT_SHARE_BACKEND_EXTERNAL would make it clear that it's not supported. So either my patch, or removal of BOOT_SHARE_BACKEND_EXTERNAL.

I suggest to make this option complete and usable, instead of limiting user options.

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.

2 participants