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

[nrf noup] boot: zephyr: Do not lock PCD region with TF-M #330

Merged

Conversation

MarkusLassila
Copy link
Contributor

@MarkusLassila MarkusLassila commented Aug 30, 2024

Previously PCD memory was locked as read-only, non-secure in MCUboot. Given that TF-M also needs write to PCD to communicate with b0n, the memory is left unlocked and locked to read-only, non-secure in TF-M.

@MarkusLassila
Copy link
Contributor Author

@nvlsianpu: Could you take a look at this?

@MarkusLassila MarkusLassila requested a review from a team October 18, 2024 09:59
Previously PCD memory was locked as read-only, non-secure in
MCUboot. Given that TF-M also needs write to PCD to
communicate with b0n, the memory is left unlocked and
locked to read-only, non-secure in TF-M.

Signed-off-by: Markus Lassila <[email protected]>
@MarkusLassila MarkusLassila force-pushed the mcuboot-do-not-lock-pcd-with-tfm branch from 365d973 to 396120f Compare October 21, 2024 07:13
@MarkusLassila
Copy link
Contributor Author

@nordicjm: Could you review this?

The manifest PR is green and is only pending our discussion about: nrfconnect/sdk-nrf#17093 (comment)

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.

Can you details your test case for the issue mentioned in the second commit?

@MarkusLassila
Copy link
Contributor Author

MarkusLassila commented Oct 21, 2024

Can you details your test case for the issue mentioned in the second commit?

@nordicjm: Sure, this was manually tested following the guidance in https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/samples/tfm/tfm_psa_template/README.html.

First, set the keys for the sample according to: https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/samples/tfm/tfm_psa_template/README.html#firmware_update.

Then flash the provisioning sample and tfm_psa_template according to: https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/samples/tfm/tfm_psa_template/README.html#building_and_running.

Then perform the following steps from the https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/samples/tfm/tfm_psa_template/README.html#bootloader_firmware_update:

west build -b nrf5340dk/nrf5340/cpuapp/ns nrf/samples/tfm/tfm_psa_template
-Dmcuboot_CONFIG_FW_INFO_FIRMWARE_VERSION=2

mcumgr --conntype serial --connstring dev=/dev/ttyACM1,baud=115200,mtu=512 image list
mcumgr --conntype serial --connstring dev=/dev/ttyACM1,baud=115200,mtu=512 image upload
build/signed_by_mcuboot_and_b0_s1_image.bin

Now, instead of flagging the image to be tested/confirmed on the next reboot, reset the device.

Now, if you had the CONFIG_NRF53_MULTI_IMAGE_UPDATE set, the device no longer boots.

@MarkusLassila
Copy link
Contributor Author

The reason for the failure was that the boot_validate_slot failure did not clear the values that were set.

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.

Using #338 and the steps provided I cannot reproduce a lockup:

*** Booting nRF Connect SDK v2.7.99-1e3a4e8a6bd1 ***
*** Using Zephyr OS v3.7.99-774c0c31d7e4 ***
Attempting to boot slot 0.

Attempting to boot from address 0x8200.

I: Verifying signature against key 0.
I: Hash: 0xb3...0f
I: Firmware signature verified.
Firmware version 1

Booting (0x8200).

*** Booting My Application v2.1.0-dev-818f77bddf21 ***
*** Using nRF Connect SDK v2.7.99-1e3a4e8a6bd1 ***
*** Using Zephyr OS v3.7.99-774c0c31d7e4 ***
I: Starting bootloader
I: Image index: 1, Swap type: none
I: Bootloader chainload address offset: 0x20000
I: Jumping to the first image slot
�*** Booting nRF Connect SDK v2.7.99-1e3a4e8a6bd1 ***
*** Using Zephyr OS v3.7.99-774c0c31d7e4 ***
build time: Oct 22 2024 07:52:37

FW info S0:
Magic: 0x281ee6de8fcebb4c00003502
Total Size: 140
Size: 0x0000773c
Version: 1
Address: 0x00008200
Boot address: 0x00008200
Valid: 0x9102ffff (CONFIG_FW_INFO_VALID_VAL=0x9102ffff)

FW info S1:
Magic: 0x281ee6de8fcebb4c00003502
Total Size: 140
Size: 0x0000773c
Version: 1
Address: 0x00014200
Boot address: 0x00014200
Valid: 0x9102ffff (CONFIG_FW_INFO_VALID_VAL=0x9102ffff)

Active slot: S0

Requesting initial attestation token with 64 byte challenge.
Received initial attestation token of 307 bytes.

0  1  2  3  4  5  6  7   8  9  A  B  C  D  E  F
D2 84 43 A1 01 26 A0 58  E8 AA 3A 00 01 24 FF 58  |  ..C..&.X..:..$.X
40 00 11 22 33 44 55 66  77 88 99 AA BB CC DD EE  |  @.."3DUfw.......

You mention network core as well, but network core is not even enabled in those steps? If you are building a network core image as well with b0n please provide the steps for this, and test using the above linked PR

@MarkusLassila
Copy link
Contributor Author

@nordicjm: This is the result when you perform the steps using my PR's, without the commit for fixing the issue:

*** Booting nRF Connect SDK v2.7.99-15677cea39aa ***
*** Using Zephyr OS v3.7.99-8d6fc59bf40a ***
Attempting to boot slot 0.
Attempting to boot from address 0x8200.
I: Verifying signature against key 0.
I: Hash: 0x18...fc
I: Firmware signature verified.
Firmware version 1
*** Booting My Application v2.1.0-dev-0a58adb35ccb ***
*** Using nRF Connect SDK v2.7.99-15677cea39aa ***
*** Using Zephyr OS v3.7.99-8d6fc59bf40a ***
I: Starting bootloader
I: Image index: 0, Swap type: none
I: Image index: 1, Swap type: none
I: Bootloader chainload address offset: 0x14000
�ERROR: Cannot fulfill EXT_API request.

Note the bootloader chainload address offset which points to s1_image, instead of application. With the changes in PR's the network core image is build automatically, but we are not trying to update it here.

I am not sure how you expect me to reproduce this using #338. The PR does not compile against main. And it does not set the CONFIG_NRF53_MULTI_IMAGE_UPDATE, which is not set by the sample, before the changes in my PR's.

If you want to reproduce this with my PR's:

After which you run the steps that I gave previously.

@nordicjm
Copy link
Contributor

The PR does not compile against main

Pasted one of linked PRs but not the ncs one, use nrfconnect/sdk-nrf#17567

@MarkusLassila
Copy link
Contributor Author

The PR does not compile against main

Pasted one of linked PRs but not the ncs one, use nrfconnect/sdk-nrf#17567

@nordicjm: I was not able to reproduce the failure with nrfconnect/sdk-nrf#17567. I think that the changes that you have done for checking the netcore image area prevent this from happening.

Furthermore, I cherry-picked my changes on top of you PR's and I was able to simultaneously update network core and application core.

Is the nrfconnect/sdk-nrf#17567 coming to 2.8? If it is, then I should remove the fix from this PR.

@nordicjm
Copy link
Contributor

Is the nrfconnect/sdk-nrf#17567 coming to 2.8? If it is, then I should remove the fix from this PR.

Yes, it is urgently needed to fix some issues with sysbuild image IDs. @nvlsianpu @de-nordic please review nrfconnect/sdk-nrf#17567 and #338

@nordicjm nordicjm merged commit 68b96b8 into nrfconnect:main Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants