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

mgmt: mcumgr: grp: os: Add active b0 slot bootloader info query #19714

Merged
merged 4 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion cmake/sysbuild/b0_mcuboot_signing.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ if(SB_CONFIG_BOOTLOADER_MCUBOOT)

if(SB_CONFIG_SECURE_BOOT_BUILD_S1_VARIANT_IMAGE)
ncs_secure_boot_mcuboot_sign(s1_image "${bin_files}" "${signed_targets}" "")
set(extra_bin_data "signed_by_mcuboot_and_b0_s1_image.binload_address=$<TARGET_PROPERTY:partition_manager,PM_S1_ADDRESS>")
set(extra_bin_data "signed_by_mcuboot_and_b0_s1_image.binload_address=$<TARGET_PROPERTY:partition_manager,PM_S1_ADDRESS>;signed_by_mcuboot_and_b0_s1_image.binslot=1")
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit.

Very long line before, and even much longer now 😞

endif()

if(bin_files)
Expand All @@ -156,6 +156,7 @@ if(SB_CONFIG_BOOTLOADER_MCUBOOT)
${extra_bin_data}
"version_MCUBOOT=${SB_CONFIG_SECURE_BOOT_MCUBOOT_VERSION}"
"version_B0=${mcuboot_fw_info_firmware_version}"
"signed_by_mcuboot_and_b0_mcuboot.binslot=0"
DEPENDS ${signed_targets}
)
endif()
Expand Down
1 change: 1 addition & 0 deletions subsys/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,4 @@ add_subdirectory(suit)
add_subdirectory_ifdef(CONFIG_MGMT_SUITFU mgmt/suitfu)
add_subdirectory_ifdef(CONFIG_DULT dult)
add_subdirectory_ifdef(CONFIG_NRF_COMPRESS nrf_compress)
add_subdirectory(mgmt/mcumgr)
1 change: 1 addition & 0 deletions subsys/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rsource "nrf_rpc/Kconfig"
rsource "zigbee/Kconfig"
rsource "mgmt/fmfu/Kconfig"
rsource "mgmt/suitfu/Kconfig"
rsource "mgmt/mcumgr/Kconfig"
rsource "caf/Kconfig"
rsource "ieee802154/Kconfig"
rsource "dm/Kconfig"
Expand Down
7 changes: 7 additions & 0 deletions subsys/mgmt/mcumgr/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#
# Copyright (c) 2024 Nordic Semiconductor
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

add_subdirectory(grp)
11 changes: 11 additions & 0 deletions subsys/mgmt/mcumgr/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#
# Copyright (c) 2024 Nordic Semiconductor
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

menu "Additional MCUmgr configuration"

rsource "grp/Kconfig"

endmenu
7 changes: 7 additions & 0 deletions subsys/mgmt/mcumgr/grp/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#
# Copyright (c) 2024 Nordic Semiconductor
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

add_subdirectory_ifdef(CONFIG_MCUMGR_GRP_OS os_mgmt)
11 changes: 11 additions & 0 deletions subsys/mgmt/mcumgr/grp/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#
# Copyright (c) 2024 Nordic Semiconductor
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

menu "Additional MCUmgr group configuration"

rsource "os_mgmt/Kconfig"

endmenu
10 changes: 10 additions & 0 deletions subsys/mgmt/mcumgr/grp/os_mgmt/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#
# Copyright (c) 2024 Nordic Semiconductor ASA
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

if(CONFIG_MCUMGR_GRP_OS_BOOTLOADER_INFO_B0_ACTIVE_SLOT)
zephyr_library_amend()
zephyr_library_sources(src/os_mgmt_b0_active_slot.c)
endif()
22 changes: 22 additions & 0 deletions subsys/mgmt/mcumgr/grp/os_mgmt/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#
# Copyright (c) 2024 Nordic Semiconductor ASA
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

menu "Additional MCUmgr OS group management functionality"

config MCUMGR_GRP_OS_BOOTLOADER_INFO_B0_ACTIVE_SLOT
bool "Bootloader info query for active b0 slot"
depends on MCUMGR_GRP_OS_BOOTLOADER_INFO
depends on BOOTLOADER_MCUBOOT
depends on MCUBOOT_MCUBOOT_IMAGE_NUMBER != -1
depends on FW_INFO
depends on FW_INFO_API
depends on MCUMGR_MGMT_NOTIFICATION_HOOKS
depends on MCUMGR_GRP_OS_BOOTLOADER_INFO_HOOK
help
Enables a bootloader info command for `active_b0_slot` which will return the active b0
image slot number, and can be used to determine which update image should be loaded.

endmenu
70 changes: 70 additions & 0 deletions subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt_b0_active_slot.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright (c) 2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

#include <zcbor_common.h>
#include <zcbor_encode.h>
#include <pm_config.h>
#include <fw_info.h>
#include <zephyr/mgmt/mcumgr/mgmt/callbacks.h>
#include <zephyr/mgmt/mcumgr/grp/os_mgmt/os_mgmt.h>

static enum mgmt_cb_return bootloader_info_hook(uint32_t event, enum mgmt_cb_return prev_status,
int32_t *rc, uint16_t *group, bool *abort_more,
void *data, size_t data_size)
{
struct os_mgmt_bootloader_info_data *bootloader_info_data =
(struct os_mgmt_bootloader_info_data *)data;

if (event != MGMT_EVT_OP_OS_MGMT_BOOTLOADER_INFO || data_size !=
sizeof(*bootloader_info_data)) {
*rc = MGMT_ERR_EUNKNOWN;
return MGMT_CB_ERROR_RC;
}

if (*(bootloader_info_data->decoded) >= 1 && (sizeof("active_b0_slot") - 1) ==
bootloader_info_data->query->len && memcmp("active_b0_slot",
bootloader_info_data->query->value,
bootloader_info_data->query->len) == 0) {
const struct fw_info *s0_info = fw_info_find(PM_S0_ADDRESS);
const struct fw_info *s1_info = fw_info_find(PM_S1_ADDRESS);

if (s0_info || s1_info) {
uint32_t active_slot;
bool ok;

if (!s1_info || (s0_info && s0_info->version >= s1_info->version)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is the !s1_info needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's possible that there is no s1 image loaded in which case s1 would be a null pointer so the check is needed.

active_slot = 0;
} else if (!s0_info || (s1_info && s1_info->version > s0_info->version)) {
active_slot = 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming that at this point we can have both s0_info and s1_info and the above else and else if have not been triggered, with the !s0_info and !s1_info, we have no processing of situation where both s0_info && s1_info, in which case active_slot is left uninitialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there must be an s0_info or s1_info for b0 to be able to boot the image so one of the conditions must run, if there is no s0_info and no s1_info and b0 has booted the image then something is very wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, my bad.


ok = zcbor_tstr_put_lit(bootloader_info_data->zse, "active") &&
zcbor_uint32_put(bootloader_info_data->zse, active_slot);
*rc = (ok ? MGMT_ERR_EOK : MGMT_ERR_EMSGSIZE);
return MGMT_CB_ERROR_RC;
}

/* Return response not valid error when active slot cannot be determined */
*group = MGMT_GROUP_ID_OS;
*rc = OS_MGMT_ERR_QUERY_RESPONSE_VALUE_NOT_VALID;
return MGMT_CB_ERROR_ERR;
}

return MGMT_CB_OK;
}

static struct mgmt_callback cmd_bootloader_info_cb = {
.callback = bootloader_info_hook,
.event_id = MGMT_EVT_OP_OS_MGMT_BOOTLOADER_INFO,
};

static int os_mgmt_register_bootloader_info_hook_b0_active_slot(void)
{
mgmt_callback_register(&cmd_bootloader_info_cb);
return 0;
}

SYS_INIT(os_mgmt_register_bootloader_info_hook_b0_active_slot, APPLICATION, 0);
1 change: 1 addition & 0 deletions sysbuild/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ function(${SYSBUILD_CURRENT_MODULE_NAME}_pre_cmake)
set_property(TARGET ${DEFAULT_IMAGE} APPEND_STRING PROPERTY CONFIG "CONFIG_MCUBOOT_NETWORK_CORE_IMAGE_NUMBER=${SB_CONFIG_MCUBOOT_NETWORK_CORE_IMAGE_NUMBER}\n")
set_property(TARGET ${DEFAULT_IMAGE} APPEND_STRING PROPERTY CONFIG "CONFIG_MCUBOOT_WIFI_PATCHES_IMAGE_NUMBER=${SB_CONFIG_MCUBOOT_WIFI_PATCHES_IMAGE_NUMBER}\n")
set_property(TARGET ${DEFAULT_IMAGE} APPEND_STRING PROPERTY CONFIG "CONFIG_MCUBOOT_QSPI_XIP_IMAGE_NUMBER=${SB_CONFIG_MCUBOOT_QSPI_XIP_IMAGE_NUMBER}\n")
set_property(TARGET ${DEFAULT_IMAGE} APPEND_STRING PROPERTY CONFIG "CONFIG_MCUBOOT_MCUBOOT_IMAGE_NUMBER=${SB_CONFIG_MCUBOOT_MCUBOOT_IMAGE_NUMBER}\n")
endif()

if(SB_CONFIG_MCUBOOT_HARDWARE_DOWNGRADE_PREVENTION)
Expand Down
2 changes: 1 addition & 1 deletion west.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ manifest:
# https://developer.nordicsemi.com/nRF_Connect_SDK/doc/latest/zephyr/guides/modules.html
- name: zephyr
repo-path: sdk-zephyr
revision: 76a46cb2baf310a06bbdb0b1d7d99394ec04a671
revision: 763e6a8ab82bd1a346a9b14a4b8fc7509c8441b6
import:
# In addition to the zephyr repository itself, NCS also
# imports the contents of zephyr/west.yml at the above
Expand Down
Loading