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

nRF54L15: Add initial TF-M support #14463

Merged
merged 19 commits into from
Mar 22, 2024
Merged

Conversation

Vge0rge
Copy link
Contributor

@Vge0rge Vge0rge commented Mar 8, 2024

Add initial support for TF-M in nRF54L15.

This has the DTS files here in this repository until we have more mature support, then we will add upstream TF-M support and we will relocate the files.

Adds support for the tfm_hello_world sample and the following crypto samples:

  • aes_cbc
  • aes_ccm
  • aes_ctr
  • aes_gcm
  • chachapoly
  • ecdh
  • ecdsa
  • eddsa
  • hkdf
  • hmac
  • rng
  • rsa
  • sha256

test_crypto: PR-614
test_tfm: PR-155

@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Mar 8, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Mar 8, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
trusted-firmware-m nrfconnect/sdk-trusted-firmware-m@d824e94 nrfconnect/sdk-trusted-firmware-m@a22fef3 (main) nrfconnect/[email protected]
zephyr nrfconnect/sdk-zephyr@38cf979 nrfconnect/sdk-zephyr@05ff843 (main) nrfconnect/[email protected]

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

@Vge0rge Vge0rge force-pushed the nrf54l_initial_tfm branch 4 times, most recently from ea651f9 to d9ece98 Compare March 15, 2024 20:58
@Vge0rge Vge0rge changed the title Not for review - CI only nRF54L15: Add initial TF-M support Mar 15, 2024
@Vge0rge Vge0rge marked this pull request as ready for review March 15, 2024 21:00
@Vge0rge Vge0rge force-pushed the nrf54l_initial_tfm branch from d9ece98 to 1de4864 Compare March 15, 2024 21:05
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Mar 15, 2024

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite

Detailed information of selected test modules

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

RAM regions must be aligned to this value due to SPU HW
limitations.

endif #BOARD_NRF54L15PDK_NRF54L15_CPUAPP_NS
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newling

sw1 = &button1;
sw2 = &button2;
sw3 = &button3;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

The leds, buttons, and aliases, is there any way of sharing this code with the secure board?

I believe that for nrf53 this code is shared in a file called *_common.dtsi.

This DT code changes quite a lot and will be expensive to maintain, as it is also not well tested, resulting in user bug reports.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that should be in a common file to make our future us more happy, but I'm also fine with creating a follow-up Jira task on this instead of fixing it in this big PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fully agree that this is the correct approach but I decided not to put that in scope of this PR.
I created a follow up task: NCSDK-26632 which covers that exact part.

CONFIG_HW_STACK_PROTECTION=y

# MPU-based null-pointer dereferencing detection cannot
# be applied as the (0x0 - 0x400) is unmapped for this target.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't true, is it?

When I look at the datasheet it looks like RRAM starts at address 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point, this was actually copy pasted by the defconfig file in Zephyr. I also don't see how this can apply for this device so I will remove.

reg = <0x10e000 0x1000>;
interrupts = <270 NRF_DEFAULT_IRQ_PRIORITY>;
status = "disabled";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -1737,7 +1737,7 @@ config PSA_NEED_CRACEN_KEY_DERIVATION_DRIVER

config PSA_NEED_CRACEN_KMU_DRIVER
bool
default y
default y if !BUILD_WITH_TFM
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a link to the jira for KMU support so the developer doing that jira can easily find all places that need patching.

(and also make it clear that this is a temporary patch)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I added it.

@@ -1235,6 +1235,7 @@ psa_status_t cracen_get_builtin_key(psa_drv_slot_number_t slot_number,
}
}

#if !CONFIG_BUILD_WITH_TFM
Copy link
Contributor

Choose a reason for hiding this comment

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

port = <0>;
gpiote-instance = <&gpiote30>;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that both watchdog30 and 31 have been removed with the rationale that they are secure-only, but it is only WDT30 that is secure-only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I will add the 31 watchdog.

#ifdef CONFIG_HAS_HW_NRF_CC3XX
int err = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

When we add support for CONFIG_HW_UNIQUE_KEY_RANDOM on 54L we will get an error with this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right, I will try to refactor a bit.

@Vge0rge Vge0rge removed DNM changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Mar 22, 2024
@Vge0rge Vge0rge force-pushed the nrf54l_initial_tfm branch from 04b097d to 249eb51 Compare March 22, 2024 15:17
@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 Mar 22, 2024
Vge0rge and others added 19 commits March 22, 2024 16:20
Update TF-M with required changes for nRF54L15 support.

Signed-off-by: Georgios Vasilakis <[email protected]>
Update Zephyr with required changes for nRF54L15 support.

Signed-off-by: Georgios Vasilakis <[email protected]>
Add TF-M support for nRF54L15.

This includes the DTS files, Kconfig configurations,
source files and CMake logic.

These are added out of tree (not upstream) until
we have more mature support.

Ref: NCSDK-26277

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Georgios Vasilakis <[email protected]>
BUILD_FROM_TFM doesn't exist anymore, it is renamed to
BUILD_INSIDE_TFM. Adapt the CRACEN code for this change.

Signed-off-by: Georgios Vasilakis <[email protected]>
Through the correct MBEDTLS macro.

Signed-off-by: Georgios Vasilakis <[email protected]>
which uses mbedtls_svc_key_id and not the psa one

Signed-off-by: Georgios Vasilakis <[email protected]>
KMU is not supported for TF-M yet.

Signed-off-by: Georgios Vasilakis <[email protected]>
We don't use any loggin in this file.

Signed-off-by: Georgios Vasilakis <[email protected]>
Don't include the builtin function in non-secure builds
since this collides with the implementation inside TF-M.

Builtin keys are not supported in TF-M with CRACEN at
the moment.

Ref: NCSDK-26277

Signed-off-by: Georgios Vasilakis <[email protected]>
Add dtsi files for nRF54L15:

nrf54l15_cpuapp_ns.dtsi
1) Removes the UICR which is not accesible from the
   non-secure application
2) Updates the bus address of the peripherals to 0x4xxxxx
   as they should be for the non-secure app

nrf54l15_cpuapp_peripherals_ns.dtsi
1) Removes the watchdog timer 30 which is mapped as secure

These filles will placed in Zephyr when we add support
for upstream TF-M.

Signed-off-by: Georgios Vasilakis <[email protected]>
In order to use the LFXO in nRF5340 we need to setup
the PINS XL1 and XL2 but this already done in TF-M
so there no reason to do it in this sample.

Signed-off-by: Georgios Vasilakis <[email protected]>
Make tfm_hello_world compatible with nRF54L15

1) Choose an NS timer based on the available options
2) Use CPUID for demonstrating a secure memory read
   when FICR is not secure

Ref: NCSDK-26277

Signed-off-by: Georgios Vasilakis <[email protected]>
For devices which don't have CryptoCell the err
variable will be unused. This defines it only for CryptoCell
devices.

Signed-off-by: Georgios Vasilakis <[email protected]>
Allow the output buffer to be NULL when there is
no input to process in the PSA cipher driver.

This is used by TF-M but in general this restriction
doesn't make sense since we will return from this function
before we use this buffer.

Ref: NCSDK-26277

Signed-off-by: Georgios Vasilakis <[email protected]>
Enable the required PSA_WANT_KEY_TYPE for the HKDF and
HMAC samples.

Signed-off-by: Georgios Vasilakis <[email protected]>
It is not used by the Nordic platforms.

Signed-off-by: Georgios Vasilakis <[email protected]>
This adds the required overlay configuration and the sample.yaml
additions for all the crypto samples which are supported by the
nRF54L15 with TF-M.

Ref: NCSDK-26277

Signed-off-by: Georgios Vasilakis <[email protected]>
Set the CONFIG_MCUBOOT_HARDWARE_DOWNGRADE_PREVENTION
option for the bootladers test case.

This used to be enabled in the CI and we prefer to have
it in the yaml file to be more visible.

Signed-off-by: Georgios Vasilakis <[email protected]>
Increase the stack size of the crypto partition when RSA
is enabled with keys >= 3072 bits.

Signed-off-by: Georgios Vasilakis <[email protected]>
@Vge0rge Vge0rge force-pushed the nrf54l_initial_tfm branch from 249eb51 to 50bb0ce Compare March 22, 2024 15:20
@NordicBuilder NordicBuilder removed the DNM label Mar 22, 2024
@Vge0rge Vge0rge removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Mar 22, 2024
@rlubos rlubos merged commit 87d76a9 into nrfconnect:main Mar 22, 2024
14 of 15 checks passed
@SebastianBoe SebastianBoe mentioned this pull request Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants