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.

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

[WIP] Multi image building #430

Closed
wants to merge 2 commits into from
Closed

[WIP] Multi image building #430

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
083b331
cmake: Migrate to support multi-image building
SebastianBoe Dec 12, 2018
49d5af7
Prefix dtc overlay with image
hakonfam May 15, 2019
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
32 changes: 25 additions & 7 deletions boot/zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ set(BOARD qemu_x86)
# and fits inside, the boot partition. (If the user specified a
# DTC_OVERLAY_FILE on the CMake command line, we need to append onto
# the list).
if(DTC_OVERLAY_FILE)
set(DTC_OVERLAY_FILE
"${DTC_OVERLAY_FILE} ${CMAKE_CURRENT_LIST_DIR}/dts.overlay")
if(${IMAGE}DTC_OVERLAY_FILE)
set(${IMAGE}DTC_OVERLAY_FILE
"${${IMAGE}DTC_OVERLAY_FILE} ${CMAKE_CURRENT_LIST_DIR}/dts.overlay")
else()
set(DTC_OVERLAY_FILE ${CMAKE_CURRENT_LIST_DIR}/dts.overlay)
set(${IMAGE}DTC_OVERLAY_FILE ${CMAKE_CURRENT_LIST_DIR}/dts.overlay)
endif()

if (EXISTS ${CMAKE_CURRENT_LIST_DIR}/boards/${BOARD}.overlay)
set(DTC_OVERLAY_FILE
"${DTC_OVERLAY_FILE} ${CMAKE_CURRENT_LIST_DIR}/boards/${BOARD}.overlay")
set(${IMAGE}DTC_OVERLAY_FILE
"${${IMAGE}DTC_OVERLAY_FILE} ${CMAKE_CURRENT_LIST_DIR}/boards/${BOARD}.overlay")
endif()

# Enable Zephyr runner options which request mass erase if so
Expand Down Expand Up @@ -134,7 +134,7 @@ if(CONFIG_MCUBOOT_SERIAL)

zephyr_link_libraries_ifdef(
CONFIG_TINYCBOR
TINYCBOR
${IMAGE}TINYCBOR
)

zephyr_include_directories_ifdef(
Expand All @@ -149,6 +149,14 @@ if(NOT CONFIG_BOOT_SIGNATURE_KEY_FILE STREQUAL "")
else()
set(KEY_FILE ${MCUBOOT_DIR}/${CONFIG_BOOT_SIGNATURE_KEY_FILE})
endif()

set_property(
GLOBAL
PROPERTY
KEY_FILE
Copy link

Choose a reason for hiding this comment

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

Where is this used?

${KEY_FILE}
)

set(GENERATED_PUBKEY ${ZEPHYR_BINARY_DIR}/autogen-pubkey.c)
add_custom_command(
OUTPUT ${GENERATED_PUBKEY}
Expand All @@ -163,3 +171,13 @@ if(NOT CONFIG_BOOT_SIGNATURE_KEY_FILE STREQUAL "")
)
zephyr_library_sources(${GENERATED_PUBKEY})
endif()

# TODO: Configurable?
set_property(GLOBAL APPEND PROPERTY
HEX_FILES_TO_MERGE
${PROJECT_BINARY_DIR}/zephyr/${KERNEL_HEX_NAME}
Copy link

Choose a reason for hiding this comment

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

Why isn't zephyr_add_executable() taking care of these?

)
set_property(GLOBAL APPEND PROPERTY
HEX_FILES_TO_MERGE_TARGET
${logical_target_for_zephyr_elf}
)
8 changes: 0 additions & 8 deletions boot/zephyr/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ config BOOT_SIGNATURE_TYPE_ECDSA_P256

endchoice

config BOOT_SIGNATURE_KEY_FILE
Copy link

Choose a reason for hiding this comment

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

Does this break the build when MCUboot is built as a single image?

string "PEM key file"
default ""
help
The key file will be parsed by imgtool's getpub command and a .c source
with the public key information will be written in a format expected by
MCUboot.

config MBEDTLS_CFG_FILE
default "mcuboot-mbedtls-cfg.h"

Expand Down
39 changes: 39 additions & 0 deletions zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
if(CONFIG_BOOTLOADER_MCUBOOT)
Copy link

Choose a reason for hiding this comment

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

Might want to drop a README in this directory or add a comment to this file saying that the zephyr directory is for use of mcuboot as a zephyr module (https://docs.zephyrproject.org/latest/application/index.html#modules-external-projects), and that boot/zephyr is where the Zephyr-specific application code is, to avoid confusing newcomers who don't know to look for boot/zephyr yet.

I've had to clear up confusion that CONFIG_BOOTLOADER_MCUBOOT should not be enabled when building mcuboot itself, but is rather an option that's enabled by applications using MCUboot; this file muddies that distinction a bit, so a comment would be nice to clear up that what's going on here is kind of complex:

  1. Application CMakeLists.txt
  2. Application boilerplate.cmake
  3. Application zephyr_modules.cmake
  4. This file
  5. MCUboot CMakeLists.txt (boot/zephyr/CMakeLists.txt)
  6. MCUboot boilerplate.cmake
    ...

# Build a second bootloader image

set(MCUBOOT_BASE ${CMAKE_CURRENT_LIST_DIR}/..)

zephyr_add_executable(mcuboot require_build)

if (${require_build})
add_subdirectory(${MCUBOOT_BASE}/boot/zephyr ${CMAKE_CURRENT_BINARY_DIR}/mcuboot)

# TODO: Assert that the bootloader and image use the same key.
Copy link

Choose a reason for hiding this comment

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

Tricky: multiple keys are supported simultaneously.


set(SIGNED_IMAGE signed.hex)
Copy link

Choose a reason for hiding this comment

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

Would you mind zephyr.signed.hex, to match the default from west sign?


set_property(GLOBAL APPEND PROPERTY
extra_post_build_commands
COMMAND
${PYTHON_EXECUTABLE}
${MCUBOOT_BASE}/scripts/imgtool.py
sign
--key ${MCUBOOT_BASE}/${CONFIG_BOOT_SIGNATURE_KEY_FILE}
--header-size ${CONFIG_TEXT_SECTION_OFFSET}
--align ${DT_FLASH_WRITE_BLOCK_SIZE}
--version 0.1 # TODO: Configurable?
Copy link

Choose a reason for hiding this comment

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

TODO: Configurable?

I think so. One could set it at CMake time; maybe even let the environment override that. That'd be pretty convenient on macOS and Linux at least.

Copy link
Contributor

@hakonfam hakonfam May 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Could you elaborate the benefits of not simply having this as a Kconfig option?

Copy link

Choose a reason for hiding this comment

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

I'm thinking of things like developer workflows where you want to update this on a build-by-build basis every few minutes, without having to go through the overhead of running menuconfig. An environment variable makes that pretty easy -- not sure if it's possible to have a kconfig option fall back to an env var dynamically, but I believe not. OTOH from a reproducible builds perspective I take your point. Tradeoffs here, your call.

--slot-size 0x32000 # TODO: Configurable?
Copy link

Choose a reason for hiding this comment

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

Not configurable; this is from DT (DT_FLASH_AREA_IMAGE_0_SIZE).

Copy link
Contributor

@hakonfam hakonfam May 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Will fix

${KERNEL_HEX_NAME} # TODO: Enforce that this will be present through Kconfig
${SIGNED_IMAGE}
)

set_property(GLOBAL APPEND PROPERTY
HEX_FILES_TO_MERGE
${SIGNED_IMAGE}
)
set_property(GLOBAL APPEND PROPERTY
HEX_FILES_TO_MERGE_TARGET
${logical_target_for_zephyr_elf}
)
endif() # ${require_build}
endif()
46 changes: 46 additions & 0 deletions zephyr/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
if BOOTLOADER_MCUBOOT

config MCUBOOT_CMAKELISTS_DIR
string "Path to the directory of the MCUBoot CMakeLists.txt file"
default "$MCUBOOT_BASE/boot/zephyr/"


choice
Copy link

Choose a reason for hiding this comment

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

These options are boilerplate needed by any child image and should be put into a boilerplate file, like zephyr/subsys/logging/Kconfig.template.log_config, to be included here after setting the name to MCUBOOT or so.

prompt "MCUBoot build strategy"
default MCUBOOT_BUILD_STRATEGY_FROM_SOURCE

config MCUBOOT_BUILD_STRATEGY_USE_HEX_FILE
# Mandatory option when being built through 'zephyr_add_executable'
bool "Use hex file instead of building MCUBoot"

if MCUBOOT_BUILD_STRATEGY_USE_HEX_FILE

config MCUBOOT_HEX_FILE
Copy link

Choose a reason for hiding this comment

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

I don't think you meant this to be a choice option; did you? Shouldn't it be moved after the endchoice?

# Mandatory option when being built through 'zephyr_add_executable'
string "MCUBoot hex file"

endif # MCUBOOT_USE_HEX_FILE

config MCUBOOT_BUILD_STRATEGY_SKIP_BUILD
# Mandatory option when being built through 'zephyr_add_executable'
bool "Skip building MCUBoot"

config MCUBOOT_BUILD_STRATEGY_FROM_SOURCE
# Mandatory option when being built through 'zephyr_add_executable'
bool "Build from source"

endchoice

endif # BOOTLOADER_MCUBOOT

if MCUBOOT || BOOTLOADER_MCUBOOT
# TODO: Support sharing Kconfig configuration between images
Copy link

Choose a reason for hiding this comment

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

That is a really interesting idea!

config BOOT_SIGNATURE_KEY_FILE
string "PEM key file"
default "root-rsa-2048.pem"
help
The key file will be parsed by imgtool's getpub command and a .c source
with the public key information will be written in a format expected by
MCUboot.

endif # MCUBOOT || BOOTLOADER_MCUBOOT