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

suit: nRF9280 SUIT support #18496

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

mparpala
Copy link

@mparpala mparpala commented Nov 4, 2024

Add SUIT support for nRF9280 EngB product.

@mparpala mparpala added the DNM label Nov 4, 2024
@mparpala mparpala requested review from a team as code owners November 4, 2024 10:30
@CLAassistant
Copy link

CLAassistant commented Nov 4, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 4, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 4, 2024

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 32

Inputs:

Sources:

sdk-nrf: PR head: 2e091634fba8fd8c5955ede8e90e96eea753a508
zephyr: PR head: 2c7f6efa7220ef2fbdd70e8597aedf1a07e86067

more details

sdk-nrf:

PR head: 2e091634fba8fd8c5955ede8e90e96eea753a508
merge base: 24f8ed39996138636281c702a226664ce1a5e312
target head (main): 296fa39d3256542decafb1cd53d0f50beda2b5d9
Diff

zephyr:

PR head: 2c7f6efa7220ef2fbdd70e8597aedf1a07e86067
merge base: 9761d261943a916443cf6c748ee3daa2915e3546
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (43)
cmake
│  ├── sysbuild
│  │  ├── suit.cmake
│  │  ├── suit_provisioning.cmake
│  │  │ suit_provisioning_nrf9280.cmake
config
│  ├── suit
│  │  ├── templates
│  │  │  ├── nrf9280
│  │  │  │  ├── default
│  │  │  │  │  ├── v1
│  │  │  │  │  │  ├── app_envelope.yaml.jinja2
│  │  │  │  │  │  ├── app_recovery_envelope_app_local.yaml.jinja2
│  │  │  │  │  │  ├── app_recovery_envelope_direct.yaml.jinja2
│  │  │  │  │  │  ├── app_recovery_local_envelope.yaml.jinja2
│  │  │  │  │  │  ├── rad_envelope.yaml.jinja2
│  │  │  │  │  │  ├── rad_recovery_envelope.yaml.jinja2
│  │  │  │  │  │  │ root_with_binary_nordic_top.yaml.jinja2
subsys
│  ├── nrf_security
│  │  ├── Kconfig
│  │  ├── src
│  │  │  ├── drivers
│  │  │  │  ├── cracen
│  │  │  │  │  ├── Kconfig
│  │  │  │  │  │ psa_driver.Kconfig
│  ├── suit
│  │  ├── mci
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── src
│  │  │  │  │ suit_mci_nrf9280.c
│  │  ├── memory_layout
│  │  │  │ Kconfig
│  │  ├── metadata
│  │  │  ├── include
│  │  │  │  │ suit_metadata.h
│  │  ├── platform
│  │  │  ├── Kconfig
│  │  │  ├── sdfw
│  │  │  │  ├── src
│  │  │  │  │  │ suit_plat_check_image_match_sdfw_specific.c
│  │  ├── storage
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── src
│  │  │  │  │ suit_storage_nrf9280.c
│  │  ├── stream
│  │  │  │ Kconfig
│  │  ├── validator
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── src
│  │  │  │  │ suit_validator_nrf9280.c
sysbuild
│  ├── Kconfig.suit_provisioning
│  ├── suit_provisioning
│  │  ├── Kconfig.nrf9280
│  │  │ nrf9280.cmake
west.yml
zephyr
│  ├── boards
│  │  ├── nordic
│  │  │  ├── nrf54h20dk
│  │  │  │  ├── support
│  │  │  │  │  ├── nrf54h20_cpuapp.JLinkScript
│  │  │  │  │  │ nrf54h20_cpurad.JLinkScript
│  │  │  ├── nrf9280pdk
│  │  │  │  ├── support
│  │  │  │  │  ├── nrf9280_cpuapp.JLinkScript
│  │  │  │  │  │ nrf9280_cpurad.JLinkScript
│  ├── drivers
│  │  ├── pinctrl
│  │  │  │ pinctrl_nrf.c
│  ├── include
│  │  ├── zephyr
│  │  │  ├── drivers
│  │  │  │  ├── clock_control
│  │  │  │  │  │ nrf_clock_control.h
│  │  │  ├── dt-bindings
│  │  │  │  ├── pinctrl
│  │  │  │  │  │ nrf-pinctrl.h
│  ├── tests
│  │  ├── drivers
│  │  │  ├── mspi
│  │  │  │  ├── api
│  │  │  │  │  ├── boards
│  │  │  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.overlay
│  │  │  │  │  ├── src
│  │  │  │  │  │  │ main.c
│  │  │  ├── uart
│  │  │  │  ├── uart_async_api
│  │  │  │  │  ├── boards
│  │  │  │  │  │  │ nrf54h20dk_nrf54h20_cpuppr.overlay
│  │  │  │  │  ├── sysbuild
│  │  │  │  │  │  ├── vpr_launcher
│  │  │  │  │  │  │  ├── boards
│  │  │  │  │  │  │  │  │ nrf54h20dk_nrf54h20_cpuapp.overlay
│  │  │  │  │  │  │  │ prj.conf

Outputs:

Toolchain

Version: 11349092be
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:11349092be_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
  • ❌ Integration tests
    • ✅ test-sdk-audio
    • ✅ desktop52_verification
    • ❌ test-fw-nrfconnect-boot
    • ✅ test-fw-nrfconnect-apps
    • ❌ test_ble_nrf_config
    • ✅ test-fw-nrfconnect-ble_mesh
    • ✅ test-fw-nrfconnect-ble_samples
    • ✅ test-fw-nrfconnect-chip
    • ✅ test-fw-nrfconnect-nfc
    • ✅ test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • ✅ test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • ✅ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • ✅ test-fw-nrfconnect-nrf-iot_samples
    • ✅ doc-internal
    • ✅ test-fw-nrfconnect-nrf-iot_thingy91
    • ✅ test-fw-nrfconnect-nrf_crypto
    • ✅ test-fw-nrfconnect-rpc
    • ✅ test-fw-nrfconnect-rs
    • ✅ test-fw-nrfconnect-fem
    • ✅ test-fw-nrfconnect-tfm
    • ✅ test-fw-nrfconnect-thread
    • ✅ test-fw-nrfconnect-zigbee
    • ✅ test-sdk-find-my
    • ✅ test-fw-nrfconnect-nrf-iot_mosh
    • ✅ test-fw-nrfconnect-nrf-iot_positioning
    • ✅ test-sdk-sidewalk
    • ✅ test-sdk-wifi
    • ✅ test-low-level
    • ✅ test-fw-nrfconnect-nrf-iot_nrf_provisioning
    • ✅ test-sdk-pmic-samples
    • ✅ test-sdk-mcuboot
    • ✅ test-sdk-dfu
    • ✅ test-fw-nrfconnect-ps
    • ✅ test-secdom-samples-public
    • ⚠️ test-fw-nrfconnect-nrf-iot_cloud
    • ⚠️ test-sdk-dfu

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

Copy link
Contributor

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

2 comments are optional, rest are required

@@ -20,6 +20,11 @@ config SUIT_MCI_IMPL_NRF54H20_SDFW
bool "nRF54H20: Secure domain"
depends on SUIT_PLATFORM_VARIANT_SDFW
Copy link
Contributor

Choose a reason for hiding this comment

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

needs nrf54h20 guard

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd not agree here. We have cases, where we use the 54 implementation on POSIX to run tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

depends on <nrf54h20> || <posix>?

Copy link
Contributor

Choose a reason for hiding this comment

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

And actually tests should not need this because they should force config e.g. https://github.com/nrfconnect/sdk-nrf/blob/main/tests/lib/modem_info/CMakeLists.txt there is a similar thing for network tests

subsys/suit/memory_layout/Kconfig Outdated Show resolved Hide resolved
subsys/suit/provisioning/CMakeLists.txt Outdated Show resolved Hide resolved
subsys/suit/provisioning/CMakeLists.txt Outdated Show resolved Hide resolved
subsys/suit/provisioning/CMakeLists.txt Outdated Show resolved Hide resolved
subsys/suit/provisioning/CMakeLists.txt Outdated Show resolved Hide resolved
subsys/suit/provisioning/Kconfig Outdated Show resolved Hide resolved
subsys/suit/provisioning/soc/Kconfig.nrf9280 Outdated Show resolved Hide resolved
Comment on lines 8 to 13
# WARNING:
# All addresses and sizes in this file must be sunchronized with the SUIT storage address
# defined in the Device Tree.
# Any modification of those values require alignment in the SDFW.
# Contents of the MPI (vendor name, class name, policies) may be modified by the
# product vendor.
Copy link
Contributor

Choose a reason for hiding this comment

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

get the values from devicetree then? Why are there kconfigs duplicating what already exists

Copy link
Contributor

Choose a reason for hiding this comment

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

The storage address is read from devicetree, so you may cross-out the addresses from the warning message (it used to be hard-coded here).
There is a minor duplication, but this is solely due to the fact that SDFW-specific device-tree files are not accessible from NCS applications (you cannot even build for cpusec).

The current state is:

  • the SUIT storage address is encoded as reserved area in sdk-nrf
  • the reserved area is compared with SDFW definitions though static asserts
  • the storage partitioning/layout is documented and only at the domain-level visible in SDFW dts files (so other SDFW components can implement per-domain firewalls).
  • all offsets visible in this file are necessary to generate the output artifacts. They are also hard-coded in other places (storage implementation, suit generator), but since changing those break the compatibility, they should be constant for a given board for a lifetime (see engb/engc migration to realize how hard that is). There is no point to make it configurable at user level. There is also no point to make all offset values depending on the Kconfigs, because: SDFW does not even generate MPI, suit-generator may be used in other contexts than sdk-nrf app build.

@NordicBuilder
Copy link
Contributor

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.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 4, 2024

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
zephyr nrfconnect/sdk-zephyr@9761d26 nrfconnect/sdk-zephyr@2c7f6ef (main) nrfconnect/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

subsys/suit/provisioning/soc/Kconfig.nrf9280 Outdated Show resolved Hide resolved
subsys/suit/provisioning/soc/Kconfig.nrf9280 Outdated Show resolved Hide resolved
subsys/suit/stream/Kconfig Outdated Show resolved Hide resolved
subsys/suit/stream/Kconfig Outdated Show resolved Hide resolved
subsys/suit/mci/src/suit_mci_nrf9280.c Show resolved Hide resolved
@anttik-nordic anttik-nordic requested a review from a team as a code owner November 18, 2024 14:12
@parttimaa parttimaa force-pushed the nrf9280_suit branch 2 times, most recently from ce5733a to 5625d67 Compare November 27, 2024 11:05
@NordicBuilder
Copy link
Contributor

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 Publish GitHub Action.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Dec 10, 2024

Memory footprint analysis revealed the following potential issues

sample.matter.template.debug[nrf7002dk/nrf5340/cpuapp]: High ROM usage: 911938[B] - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.matter.template.release[nrf7002dk/nrf5340/cpuapp]: High ROM usage: 820770[B] - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)

Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-18496/32)

@parttimaa parttimaa force-pushed the nrf9280_suit branch 3 times, most recently from 57e182a to 13bb933 Compare December 16, 2024 12:58
@NordicBuilder NordicBuilder removed the DNM label Dec 20, 2024
ahasztag

This comment was marked as resolved.

west.yml Outdated Show resolved Hide resolved
Add SUIT support for nRF9280 EngB product.

Signed-off-by: Tuomas Parttimaa <[email protected]>
Update to latest sdk-zephyr.
Revert Kconfig file due to compliancy fail.

Signed-off-by: Tuomas Parttimaa <[email protected]>
Add SUIT dependency to platform keys config in order
to keep support for no SUIT build variant of nRF9280.

Signed-off-by: Tuomas Parttimaa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. manifest manifest-zephyr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants