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

[m5k8]

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.

[nordicjm]

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]

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.

[nordicjm]

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]

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.

[nordicjm]

I had to go remind myself of what the intention of this was, as the Kconfig has:

config BOOT_SHARE_BACKEND_EXTERNAL
        bool "External (user-provided code)"
        select BOOT_SHARE_BACKEND_AVAILABLE
        help
          Use a custom user-specified storage.

The code part here is actually important, this feature is used by TF-M and the changes here completely break it, the feature is currently working and fully usable. To use it, you just need to define the addresses as compile definitions, generally you would do this in a zephyr module, as an example you could use:

if(CONFIG_MCUBOOT AND CONFIG_BOOT_SHARE_BACKEND_EXTERNAL)
  zephyr_compile_definitions(
    MCUBOOT_SHARED_DATA_BASE=0x20002000
    MCUBOOT_SHARED_DATA_SIZE=0x200
  )
endif()

You can get those values how you like, you can read them from Kconfigs if you want or can read them from devicetree

[m5k8]

Thank you for your ongoing interest in this minor case.

My point of view was like that: when I select some variant in Kconfig and this variant needs more additional settings, then these settings should also be applied via Kconfig, it makes the configuration coherent. For newbie like me, when I see this way of doing things in 100 places, I expect 101th place have the same logic.

But if there exists some prior practice of doing it the other way (your TF-M case), that's fine with me. Maybe TF-M encountered the same problem that I did and they solved it omitting the KConfig way, and now these two solutions slash. Maybe TF-M could also use the above Kconfig settings, but I agree to stop here, I already have to keep my own fork of mcuboot out of NCS tree, so one private patch more or less is a non-issue now.

Maybe somewhat longer comment for BOOT_SHARE_BACKEND_EXTERNAL could be useful for new users, because currrent "user-provided code" is not very descriptive.

Thanks!

[nordicjm]

My point of view was like that: when I select some variant in Kconfig and this variant needs more additional settings, then these settings should also be applied via Kconfig, it makes the configuration coherent. For newbie like me, when I see this way of doing things in 100 places, I expect 101th place have the same logic.

Not the case at all, if you enable hooks, you need to supply a .c file with said hooks, again using cmake, that's what cmake's purpose is, to provide the required information to generate a build including defines and files.

But if there exists some prior practice of doing it the other way (your TF-M case), that's fine with me. Maybe TF-M encountered the same problem that I did and they solved it omitting the KConfig way, and now these two solutions slash. Maybe TF-M could also use the above Kconfig settings, but I agree to stop here, I already have to keep my own fork of mcuboot out of NCS tree, so one private patch more or less is a non-issue now.

TFM is it's own project, it does not use Kconfig.

Maybe somewhat longer comment for BOOT_SHARE_BACKEND_EXTERNAL could be useful for new users, because currrent "user-provided code" is not very descriptive.

If you have any suggestions for better text please post or submit a PR