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

Jump to bottom

applications: nrf_desktop: nRF54L - Mcuboot using HW crypto and KMU #19035

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

applications: nrf_desktop: nRF54L - Mcuboot using HW crypto and KMU #19035

wants to merge 5 commits into from
+25 −43

Conversation

MarekPieta
Copy link
Contributor

Draft of MCUboot using HW crypto and KMU. MCUboot XiP no revert. Applied some memory footprint optimizations for CRACEN PSA driver.

@MarekPieta MarekPieta requested a review from a team as a code owner November 22, 2024 14:38
@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 22, 2024
@MarekPieta MarekPieta removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 22, 2024
@MarekPieta
Copy link
Contributor Author

MCUboot memory footprint estimate (using LTO and applied my CRACEN PSA driver optimizations):

Memory region         Used Size  Region Size  %age Used
           FLASH:       21154 B        28 KB     73.78%
             RAM:       16096 B       188 KB      8.36%
        IDT_LIST:          0 GB        32 KB      0.00%

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 22, 2024 •
edited
Loading

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: e05a2609f3539ce62d8959eb75a8376495d42e87

more details

sdk-nrf:

PR head: e05a2609f3539ce62d8959eb75a8376495d42e87
merge base: 5a1659005cf0b3477818b150bd9e3e14da1e700a
target head (main): f7eb8c80df81927d72fb9606b8ff736f66ee91a9
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 (4) 
applications
│  ├── nrf_desktop
│  │  ├── configuration
│  │  │  ├── nrf54l15dk_nrf54l15_cpuapp
│  │  │  │  ├── images
│  │  │  │  │  ├── mcuboot
│  │  │  │  │  │  ├── mcuboot_private.pem
│  │  │  │  │  │  │ prj.conf
│  │  │  │  ├── pm_static.yml
│  │  │  │  │ sysbuild.conf

Outputs:

Toolchain

Version: b77d8c1312
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:b77d8c1312_912848a074

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

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

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

@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.

kapi-no
kapi-no reviewed Nov 27, 2024
View reviewed changes
applications/nrf_desktop/configuration/nrf54l15dk_nrf54l15_cpuapp/pm_static.yml Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we align all build types for the nRF54L15 board target with this new configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually we should. The PR is a draft for @nvlsianpu to let him see our MCUboot configuration with lower memory footprint

de-nordic
de-nordic reviewed Nov 28, 2024
View reviewed changes
applications/nrf_desktop/configuration/nrf54l15dk_nrf54l15_cpuapp/images/mcuboot/prj.conf
@@ -30,6 +27,12 @@ CONFIG_UART_CONSOLE=n
CONFIG_PRINTK=n
CONFIG_USE_SEGGER_RTT=n

CONFIG_PSA_USE_CRACEN_CIPHER_DRIVER=n
CONFIG_PSA_USE_CRACEN_AEAD_DRIVER=n
CONFIG_PSA_USE_CRACEN_HASH_DRIVER=n
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK the driver only supports sha256, so it is useless with sha512 requirements for ed25519

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the SHA512 needed for pure ed25519?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so the comment around the HASH driver is misleading because it does not list the sha512.
The SHA512 is needed internally for pure, so it may not require driver.

Copy link
Contributor

Choose a reason for hiding this comment

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

A am now building app with mcuboot gradually removing the kconfigs listed above and checking whether it still works.

Copy link
Contributor

Choose a reason for hiding this comment

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

So OK, HASH needs to stay with non-pure

@MarekPieta
applications: nrf_dektop: Increase MCUboot's partition size
7c36e1a 
Change increases partition size of MCUboot bootloader. Hardware crypto
increases memory footprint of the bootloader.

Jira: NCSDK-26885

Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
@MarekPieta
applications: nrf_desktop: Use hardware crypto for nRF54L's MCUboot
2ef73dc 
Change switches to using pure ED25519 signature and hardware crypto.

Jira: NCSDK-26885

Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
@MarekPieta
applications: nrf_desktop: Keep MCUboot key in KMU for nRF54L
51fd5e8 
Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
@MarekPieta
applications: nrf_desktop: optimize bootloader
f5243a2 
Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
@MarekPieta
Copy link
Contributor Author

Rebased onto main

@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 28, 2024
@MarekPieta MarekPieta removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 28, 2024
@de-nordic
Copy link
Contributor

I have implemented some of the Kconfig cut-outs here: #19148

@MarekPieta
Size optimizations
e05a260 
Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
@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 28, 2024
@MarekPieta MarekPieta removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kapi-no kapi-no kapi-no left review comments

@de-nordic de-nordic de-nordic approved these changes

@nvlsianpu nvlsianpu Awaiting requested review from nvlsianpu

@pdunaj pdunaj Awaiting requested review from pdunaj

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MarekPieta @NordicBuilder @de-nordic @kapi-no