zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.93k stars 6.65k forks source link

RFC: DFU: Single source of MCUboot application image slots definition file #53712

Open de-nordic opened 1 year ago

de-nordic commented 1 year ago

Introduction

Currently most of sources that somehow deal with DFU directly access application slots with use of FIXED_PARTITION macros, provided by Flash Map API, with direct use of DTS labels. This means that each source address the same image partitions with different definitions, which cases problems when doing changes to DFU configuration. Same happens with MCUboot app that uses own definitions.

Problem description

The problem is that each time some change is required to DFU, all direct references to partitions need to be found and updated. New additions to DFU have to define own way of accessing image partitions. When we use MCUboot bootutil library the library uses MCUboot provided definitions which are again directly referencing partitions by DTS node labels.

Proposed change

The proposed change is to provide zephyr/include/mcuboot_partition.h file that will be single point where all MCUboot slots are given specific identifiers, and where additional C macros/identifiers will be defined to be used instead of direct use FIXED_PARTITION macros and DTS node labels.

Detailed RFC

The mcuboot_partitions.h will provide following macros:

ZEPHYR_MCUBOOT_APP_(0|1)_(PRIMARY|SECONDARY)_SLOT_LABEL -- to get the DTS label, for example for accessing device properties;
ZEPHYR_MCUBOOT_APP_(0|1)_(PRIMARY_SECONDARY)_SLOT_EXISTS -- 0/1 value macro that is used to check whether given application slot exists;
ZEPHYR_MCUBOOT_APP_(0|1)_(PRIMARY_SECONDARY)_SLOT_ID -- Flash Map ID of application slot, if it exists in the first place;
ZEPHYR_MCUBOOT_APP_IMAGE_COUNT -- 1/2 number of supported applications;

where (0|1) if above code means application 0 or 1 and (PRIMARY|SECONDARY) means primary slot or secondary slot of the given application.

Dependencies

MCUboot does not need to be change to work with this but it is worth doing because Zephyr uses bootutil which takes configuration from MCUboot definitions in sysflash.h, which means that some code, for accessing the same partitions, is compiled with macros from Zephyr and some with macros from MCUboot.

Alternatives

Leaving chaos as it is.

2023-01-16 Removed ZEPHYR_MCUBOOT_APP_IS_SINGLE_SLOT from RFC (https://github.com/zephyrproject-rtos/zephyr/issues/53712#issuecomment-1384137860)

Laczen commented 1 year ago

@de-nordic, the single source of application slot defines should be mcuboot or some on chip partitioning info. Nothing to these slot defines can be changed without a mcuboot update.

de-nordic commented 1 year ago

@de-nordic, the single source of application slot defines should be mcuboot or some on chip partitioning info. Nothing to these slot defines can be changed without a mcuboot update.

Yes and no. Generally it would be nice if instead of having additional code writing application images we could just call some MCUboot exported, at run-time via some int vector for example, functions that would upload application images, change configuration etc. But we do not have that now. On the other side we generate or add code, to application, depending on board configuration; for example USB DFU will have additional structures/code compiled in when there is slot1_partition present.

Aim of above enhancement is to move all MCUboot partition definitions, and logic that defines them, into single place in Zephyr. Of course it is not perfect, as sysflash.h within MCUboot, does some additional logic, but with the change, at least, you would no longer have to directly address partitions in code that does something with application images.

Laczen commented 1 year ago

@de-nordic, USB DFU should not need to know the presence of slot1_partition. It should need to know a upload and download location (and multiples if they are used), the upload and download locations could point to the same slot and based upon that knowledge some additional code could be added. This is valid for all upload/download mechanisms: they should be decoupled from the slot definitions, this makes them usable for whatever bootloader.

de-nordic commented 1 year ago

@de-nordic, USB DFU should not need to know the presence of slot1_partition. It should need to know a upload and download location (and multiples if they are used), the upload and download locations could point to the same slot and based upon that knowledge some additional code could be added. This is valid for all upload/download mechanisms: they should be decoupled from the slot definitions, this makes them usable for whatever bootloader.

OK, then maybe USB should not be a good target for the change, nevertheless it is hardcoded for mcuboot for now anyway.

de-nordic commented 1 year ago

Following comments by @nordicjm here https://github.com/zephyrproject-rtos/zephyr/pull/53713#discussion_r1067885585 and here https://github.com/zephyrproject-rtos/zephyr/pull/53713#discussion_r1067888518, I have decided to remove definition of ZEPHYR_MCUBOOT_APP_IS_SINGLE_SLOT from the RFC. For record keeping, the removed definitions looked like this:

ZEPHYR_MCUBOOT_APP_IS_SINGLE_SLOT -- 0/1 value macro that is used to check whether current configuration is for single application slot;