-
Notifications
You must be signed in to change notification settings - Fork 1.4k
nrf_ironside: Move Ironside outside of nrf_security #24862
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
base: main
Are you sure you want to change the base?
Conversation
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 22b31d279b8d4645b5621b2638be530069668b3c more detailssdk-nrf:
zephyr:
Github labels
List of changed files detected by CI (36)
Outputs:ToolchainVersion: f66cf421f3 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
ad3ff8d
to
fb6ad87
Compare
You can find the documentation preview for this PR here. |
fb6ad87
to
ddbf7ac
Compare
bool | ||
prompt "nRF Security" if !PSA_PROMPTLESS | ||
depends on SOC_FAMILY_NORDIC_NRF | ||
depends on !NRF_IRONSIDE_CALL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this break if someone wants to use some software crypto on nrf54h20? You aren't really incompatible with NRF_IRONSIDE_CALL I'd say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
software crypto on nrf54h20 is not supported. So if it does break, then that is intended behaviour :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they want to use it along with the hardware crypto yes. But it should break because at the moment no-one worked on this use case. If it worked by accident before it is better to break so that someone can think how to properly support this use case.
Edit: Sebastians comment didn't show up before I sent this. But we are saying the exact same thing basically :)
23fd200
to
9783e81
Compare
The following west manifest projects have changed revision in this Pull Request:
⛔ DNM label due to: 1 project with PR revision Note: This message is automatically posted and updated by the Manifest GitHub Action. |
54a5725
to
cc45945
Compare
prompt "PSA crypto provided through SSF" | ||
default y | ||
depends on SOC_NRF54H20 || SOC_SERIES_NRF92X | ||
depends on SOC_NRF54H20_CPUAPP || SOC_NRF54H20_CPURAD || SOC_SERIES_NRF92X |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably 92X should also be split up into CPUAPP and CPUCELL?
# | ||
|
||
if(CONFIG_PSA_SSF_CRYPTO_CLIENT) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and missing indent
|
||
zephyr_compile_definitions(MBEDTLS_PSA_CRYPTO_CONFIG_FILE="ironside_config.h") | ||
zephyr_compile_definitions(MBEDTLS_CONFIG_FILE="ironside_config.h") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zephyr_compile_definitions(MBEDTLS_PSA_CRYPTO_CONFIG_FILE="ironside_config.h") | ||
zephyr_compile_definitions(MBEDTLS_CONFIG_FILE="ironside_config.h") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this a Kconfig?
Create a separate subsystem called nrf_ironside instead of having the logic in nrf_security. Ironside is completely separate from nrf_security and it should not be placed there. Make sure that nrf_security cannot be enabled at the same time as nrf_ironside as their configurations might collide. Signed-off-by: Georgios Vasilakis <[email protected]>
The NRF_IRONSIDE is a provider of PSA services (including storage) so it cannot be used along with the truested storage subsystem which provides PSA storage APIs. Signed-off-by: Georgios Vasilakis <[email protected]>
Brings Zephyr with PSA RNG as the default entropy provider for the nRF54h20. Signed-off-by: Georgios Vasilakis <[email protected]>
cc45945
to
22b31d2
Compare
Create a separate subsystem called nrf_ironside instead of having the logic in nrf_security. Ironside is completely separate from nrf_security and it should not be placed there.
Make sure that nrf_security cannot be enabled at the same time as nrf_ironside as their configurations might collide.