boards: arm: thingy91: Secure boot #12052

gregersrygg · 2023-08-16T13:17:30Z

This PR fixes some problems that appears when trying to build for thingy:91 with CONFIG_SECURE_BOOT enabled:

ADP536X needed to be excluded from the secure bootloader. Created a new kconfig symbol IS_BOOTLOADER to make it easier to check for both mcuboot and the secure bootloader at the same time.
The thingy:91 board CMakeLists.txt enforced a static partition layout that did not include partitions for the secure bootloader and an extra slot for updating mcuboot. Added new kconfig options to allow the user to select what partition layout to use.
Not directly related, but also added a kconfig option (CONFIG_THINGY91_PARTITION_MANAGER) to not inject a thingy:91 board static configuration for those that want to use dynamic or an application specific static partition layout.

This does not solve every problem for using secure boot on thingy:91, but is a couple of steps in the right direction. Further steps are needed to get it working properly, but I want to reduce the scope of this PR and delegate some of the remaining work to other teams. Marked the new partitions layouts as experimental to make it clear that this is not production ready and under development, as described in software maturity levels.

Known issues:

After updating mcuboot using nRF Programmer, the boot process halts after the immutable bootloader.
Compiling with CONFIG_THINGY91_PARTITION_MANAGER=y and CONFIG_SECURE_BOOT=n will fail because the dynamic app partition is not aligned according to SPU flash region size.

CIA-604

NordicBuilder · 2023-08-16T13:20:15Z

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-ci-nrfconnect-boot-fw-update	X
test-fw-nrfconnect-chip	X
test-fw-nrfconnect-tfm	X
test-fw-nrfconnect-zigbee	X
test-sdk-find-my	X
test-sdk-sidewalk	X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2023-08-16T14:02:24Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

tejlmand

some minor nits.

tejlmand · 2023-08-24T08:14:41Z

boards/arm/thingy91_nrf9160/CMakeLists.txt

- set(PM_STATIC_YML_FILE ${CMAKE_CURRENT_LIST_DIR}/thingy91_pm_static.yml CACHE INTERNAL "")
- endif()
+if(CONFIG_THINGY91_STATIC_PARTITIONS_FACTORY)
+ set(PM_STATIC_YML_FILE ${CMAKE_CURRENT_LIST_DIR}/thingy91_pm_static.yml CACHE INTERNAL "")


we don't use tabs in CMake files, please convert to two spaces.

Suggested change

set(PM_STATIC_YML_FILE ${CMAKE_CURRENT_LIST_DIR}/thingy91_pm_static.yml CACHE INTERNAL "")

set(PM_STATIC_YML_FILE ${CMAKE_CURRENT_LIST_DIR}/thingy91_pm_static.yml CACHE INTERNAL "")

Perhaps we should add an .editorconfig to our repos and recommend that developers use a plugin for it?

boards/arm/thingy91_nrf9160/CMakeLists.txt

boards/arm/thingy91_nrf9160/Kconfig.board

tejlmand · 2023-08-24T08:23:37Z

boards/arm/thingy91_nrf9160/Kconfig.board

@@ -40,6 +40,15 @@ config THINGY91_STATIC_PARTITIONS_LWM2M_CARRIER
 help
 Use a partition layout including a storage partition needed for the lwm2m carrier library.

+config THINGY91_PARTITION_MANAGER
+ bool "Default partition manager logic [EXPERIMENTAL]"


Not sure default is the best wording here.

Maybe the word dynamic is better, as that is what is used in the PM guide: https://developer.nordicsemi.com/nRF_Connect_SDK/doc/latest/nrf/scripts/partition_manager/partition_manager.html

Suggested change

bool "Default partition manager logic [EXPERIMENTAL]"

bool "Dynamic partition manager logic [EXPERIMENTAL]"

I struggled how to best formulate this. Dynamic is not precise, because it's still possible to define static partitions from the application if they want to. The PM_STATIC_YML_FILE define we use in the CMakeLists.txt has the top level priority in partition manager logic - which makes it impossible to define an application static partition. There seems to be no way to make a board static partition which is possible to override from the application. The THINGY91_PARTITION_MANAGER config is used to so it's possible to not force one of the board static partitions.

I see what you mean, and upon second look I agree that Dynamic is not the best word either.

I still dislike the use default in this context.
As a user, it gives me the impression that I can go lookup somewhere to see what the default layout will be, which is not the case.
Here default doesn't refer to a given layout (as opposed to the choice options above).

When looking at this again, and the choice title:
https://github.com/nrfconnect/sdk-nrf/blob/154f3c80854f4416a0c8cef8e1d9307d9799c51b/boards/arm/thingy91_nrf9160/Kconfig.board#L18

How about changing that title to:

Fixed Thingy:91 partition layout

or

Pre-defined Thingy:91 partition layout

Then the final choice could then be: None, and look as:

config THINGY91_PARTITION_MANAGER bool "None [EXPERIMENTAL]" select EXPERIMENTAL help Don't use a pre-defined static partition layout, but use the default Partition Manager logic to handle the partition layout. This allows the application to use dynamic layout or define a custom static partition layout for the application. A debugger is needed to flash Thingy:91 with a different partition layout.

And the menu, seen by the user will look:

Pre-defined Thingy:91 partition layout [X] "Factory Thingy:91 partition layout" [ ] "Secure boot Thingy:91 partition layout [EXPERIMENTAL]" [ ] "LWM2M Carrier partition layout" [ ] "None [EXPERIMENTAL]"

an alternative to None could be the word auto.

Thanks! That sounds like a good way to solve it. I also renamed the kconfig option itself from THINGY91_PARTITION_MANAGER to THINGY91_NO_PREDEFINED_LAYOUT.

tejlmand · 2023-08-24T08:24:05Z

boards/arm/thingy91_nrf9160/Kconfig.board

+ bool "Default partition manager logic [EXPERIMENTAL]"
+ select EXPERIMENTAL
+ help
+ Don't enforce static partition layout, but use the default Partition Manager logic to


Suggested change

Don't enforce static partition layout, but use the default Partition Manager logic to

Don't enforce static partition layout, but use the dynamic Partition Manager logic to

When looking at this again now, I don't think there's any point in mentioning Partition Manager here at all. So have updated the help text to: «Disable pre-defined static partition layout. This allows the application to use dynamic layout or define a custom static partition layout for the application...»

gregersrygg · 2023-08-29T07:49:16Z

@tejlmand can you please have another look?

The ADP536X integrated circuit interfaces with two distinct drivers: the Zephyr regulator driver, responsible for power regulation aspects of the ADP536X, and the sdk-nrf ADP536X driver, which manages other functionalities. Both drivers are now reserved exclusively for inclusion within the application image. Introduced a new kconfig symbol IS_BOOTLOADER_IMG, to simplify checking for both MCUboot and secure bootloaders. In addition, changed how the driver is automatically enabled. Now it is enabled if the devicetree has an "adi,adp5360" compatible node enabled. CIA-604 Signed-off-by: Gregers Gram Rygg <gregers.gram.rygg@nordicsemi.no>

Move static partition logic for thingy:91 out to separate Kconfig options: CONFIG_THINGY91_STATIC_PARTITIONS_FACTORY for the original factory partition layout. This is the default. CONFIG_THINGY91_STATIC_PARTITIONS_SECURE_BOOT for a partition layout similar to the factory layout, but it has the immutable bootloader and partitions for uptadable bootloader. This is still experimental. CONFIG_THINGY91_STATIC_PARTITIONS_LWM2M_CARRIER for a partition layout that has the partitions needed to use the lwm2m carrier library. CIA-604 Signed-off-by: Gregers Gram Rygg <gregers.gram.rygg@nordicsemi.no>

doc/nrf/device_guides/working_with_nrf/nrf91/thingy91.rst

Allow using none of the pre-defined static partition layouts for thingy:91. This allows the application to use a dynamic layout or define a custom static partition layout for the application. Signed-off-by: Gregers Gram Rygg <gregers.gram.rygg@nordicsemi.no>

plskeggs · 2023-12-12T22:00:10Z

Great solution @gregersrygg.

gregersrygg requested review from hakonfam, sigvartmh, jtguggedal, carlescufi, tejlmand and anangl as code owners August 16, 2023 13:17

gregersrygg requested a review from plskeggs August 16, 2023 13:17

github-actions bot added the doc-required PR must not be merged without tech writer approval. label Aug 16, 2023

gregersrygg requested a review from glarsennordic August 16, 2023 13:34

gregersrygg force-pushed the fix-thingy91-secure-boot branch 4 times, most recently from 86fac76 to 63c08d0 Compare August 21, 2023 13:09

tejlmand requested changes Aug 24, 2023

View reviewed changes

gregersrygg force-pushed the fix-thingy91-secure-boot branch from 63c08d0 to 806e38c Compare August 24, 2023 10:55

plskeggs approved these changes Aug 28, 2023

View reviewed changes

gregersrygg requested a review from divipillai August 29, 2023 07:49

divipillai force-pushed the fix-thingy91-secure-boot branch from 1c0ed0d to 41946bd Compare August 31, 2023 10:49

gregersrygg force-pushed the fix-thingy91-secure-boot branch from 41946bd to 154f3c8 Compare August 31, 2023 13:04

divipillai approved these changes Aug 31, 2023

View reviewed changes

carlescufi requested a review from tejlmand October 20, 2023 10:22

gregersrygg added 2 commits December 6, 2023 09:10

gregersrygg force-pushed the fix-thingy91-secure-boot branch from 154f3c8 to d79c32f Compare December 6, 2023 10:21

gregersrygg requested a review from a team as a code owner December 6, 2023 10:21

gregersrygg requested review from balaji-nordic and removed request for a team December 6, 2023 10:21

divipillai reviewed Dec 7, 2023

View reviewed changes

doc/nrf/device_guides/working_with_nrf/nrf91/thingy91.rst Outdated Show resolved Hide resolved

gregersrygg force-pushed the fix-thingy91-secure-boot branch from d79c32f to 75ac6db Compare December 7, 2023 09:10

balaji-nordic removed their request for review December 8, 2023 08:05

tejlmand approved these changes Dec 11, 2023

View reviewed changes

divipillai approved these changes Dec 12, 2023

View reviewed changes

rlubos merged commit d3365b5 into nrfconnect:main Dec 12, 2023
19 checks passed

gregersrygg deleted the fix-thingy91-secure-boot branch December 12, 2023 13:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boards: arm: thingy91: Secure boot #12052

boards: arm: thingy91: Secure boot #12052

gregersrygg commented Aug 16, 2023

NordicBuilder commented Aug 16, 2023 •

edited

Loading

NordicBuilder commented Aug 16, 2023

tejlmand left a comment

tejlmand Aug 24, 2023

gregersrygg Aug 24, 2023

tejlmand Aug 24, 2023

gregersrygg Aug 24, 2023

tejlmand Sep 11, 2023

gregersrygg Dec 6, 2023

tejlmand Aug 24, 2023

gregersrygg Dec 6, 2023

gregersrygg commented Aug 29, 2023

plskeggs commented Dec 12, 2023

	set(PM_STATIC_YML_FILE ${CMAKE_CURRENT_LIST_DIR}/thingy91_pm_static.yml CACHE INTERNAL "")
	set(PM_STATIC_YML_FILE ${CMAKE_CURRENT_LIST_DIR}/thingy91_pm_static.yml CACHE INTERNAL "")

	bool "Default partition manager logic [EXPERIMENTAL]"
	bool "Dynamic partition manager logic [EXPERIMENTAL]"

	Don't enforce static partition layout, but use the default Partition Manager logic to
	Don't enforce static partition layout, but use the dynamic Partition Manager logic to

boards: arm: thingy91: Secure boot #12052

boards: arm: thingy91: Secure boot #12052

Conversation

gregersrygg commented Aug 16, 2023

NordicBuilder commented Aug 16, 2023 • edited Loading

Test specification

NordicBuilder commented Aug 16, 2023

tejlmand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregersrygg commented Aug 29, 2023

plskeggs commented Dec 12, 2023

NordicBuilder commented Aug 16, 2023 •

edited

Loading