nrf_security: cracen: Exclude functions that are not needed #21033
Conversation
github-actions
bot
added
the
changelog-entry-required
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: bc6380fd62bab491c4920fc0e7b758e24a5d01c2 more detailssdk-nrf:
Github labels
List of changed files detected by CI (1)Outputs:ToolchainVersion: Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
degjorva
force-pushed
the
simple-rom-optimizations
branch
3 times, most recently
from
664a10d to
76db5d7
Compare
| if (psa_status != PSA_SUCCESS) { | ||
| return psa_status; | ||
| } | ||
| if (psa_curve != PSA_ECC_FAMILY_TWISTED_EDWARDS) { |
There was a problem hiding this comment.
I see that in the handle_curve_family only the ed25519 is handled there, not the ed448. So could we use this check for both?
Not very strong opinion but I have a suggestion, it would be nice if the function handle_cracen_key_location does the same thing for every curve and not handle special cases. So it would be more readable for me if you place the logic for the ed25519 in this function and not inside the handle_curve_family. But as I said, this is a suggestion.
There was a problem hiding this comment.
Talked about this offline, but just a summary: ed448 does currently not work/is not supported for cracen, but a lot of the code for it exists. Removing that code sees counter productive as we might want to support it in the future. This is why I left it in, but it does not actually work.
The reason for the split between handle_curve_family and handle_identity_key is just that you need the attributes to separate between them, and am already at seven inputs to the handle_curve_family. Can make it work, but when I tried it I found it to be less readable than having a separate function.
Once sicrypto is fully removed this function will look a lot nicer as we don't need the entire last if statement for si_task creation, but at least this way of doing it makes it easier to #ifdef it out when not used, which I have done in a later commit
degjorva
force-pushed
the
simple-rom-optimizations
branch
from
76db5d7 to
47148a4
Compare
|
subsys/nrf_security/src/drivers/cracen/cracenpsa/src/key_management.c
Outdated
Show resolved
Hide resolved
subsys/nrf_security/src/drivers/cracen/cracenpsa/src/key_management.c
Outdated
Show resolved
Hide resolved
degjorva
force-pushed
the
simple-rom-optimizations
branch
from
47148a4 to
9d9f90e
Compare
| static psa_status_t handle_identity_key(const uint8_t *key_buffer, size_t key_buffer_size, | ||
| const struct sx_pk_ecurve *sx_curve, uint8_t *data, | ||
| struct si_sig_privkey *priv_key, | ||
| struct si_sig_pubkey *pub_key) |
Add IS_ENABLED checks to ensure a given key type is needed This improves rom usage for many standard use cases Signed-off-by: Dag Erik Gjørvad <dag.erik.gjorvad@nordicsemi.no>
Refactor export_ecc_public_key_from_keypair. Split support functionality into separate functions Signed-off-by: Dag Erik Gjørvad <dag.erik.gjorvad@nordicsemi.no>
degjorva
force-pushed
the
simple-rom-optimizations
branch
from
9d9f90e to
bc6380f
Compare
|
Rebase for compliance |
Add IS_ENABLED checks to ensure a given key type is needed. This improves rom usage for many standard use cases.
To make the sonar cloud pass I had to refactor export_ecc_public_key so added that as a separate commit to make it easy to see what was the intended functional change and what is refactoring.