-
Notifications
You must be signed in to change notification settings - Fork 833
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
Espressif ESP32-C3 ESP32-C6 ESP32-S2 Hardware Acceleration #6990
Espressif ESP32-C3 ESP32-C6 ESP32-S2 Hardware Acceleration #6990
Conversation
|
||
/* MP_HW_FALLBACK: signal to caller to fall back to SW for math: | ||
* algorithm not supported in SW | ||
* known state needing only SW, (e.g. ctx copy) | ||
* any other reason to force SW */ | ||
#define MP_HW_FALLBACK (-108) | ||
#define MP_HW_FALLBACK ESP_MP_HW_FALLBACK /* was -108 */ |
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.
Having comments in macro definitions can have unusual unintended consequences. Recommend instead putting it into the comment block above.
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.
Good point on the macro comments. I've updated those and adjusted some line lengths.
Jenkins retest this please. |
ca41df5
to
7e69030
Compare
Jenkins retest this please. |
|
||
* Uncomment out `#define WOLFSSL_ESPIDF` in `/path/to/wolfssl/wolfssl/wolfcrypt/settings.h` | ||
* Uncomment out `#define WOLFSSL_ESP32` in `/path/to/wolfssl/wolfssl/wolfcrypt/settings.h` | ||
Hardware acceleration is enabled by default. |
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.
Add note to where the build macro detection for HW acceleration is done.
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.
Ah yes, I missed that README file. I've corrected that and added more detail.
wolfcrypt/src/sha.c
Outdated
#if defined(WOLFSSL_USE_ESP32_CRYPT_HASH_HW) | ||
if (sha->ctx.mode == ESP32_SHA_INIT) { | ||
ESP_LOGV(TAG, "wc_ShaUpdate try hardware"); | ||
#if defined(DEBUG_WOLFSSL_SHA_MUTEX) |
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.
I don't like adding another new macro just for this. Can we use an existing macro or use a more generic name?
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.
Agreed. I could not find a similar "mutex tracking" macro, so I renamed this one to a generic WOLFSSL_DEBUG_MUTEX
.
wolfcrypt/src/sha.c
Outdated
#if (defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6)) && \ | ||
defined(WOLFSSL_ESP32_CRYPT) && !defined(NO_WOLFSSL_ESP32_CRYPT_HASH) | ||
if (sha->ctx.mode == ESP32_SHA_HW) { | ||
/* TODO is this the proper way to reverse endianness for the 64bit Espressif value? |
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.
Can you use ByteReverseWord64
and fix this?
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.
Good question! I dug a bit more on that & no: in the ByteReverseWord64
the "64" is meant for 64-bit architectures, not so much to just byte reverse a 64 bit word. The implementation fall back to the regular ByteReverseWords
(a 32 bit-optimized function, perfect for the 32-bit Xtensa / RISC-V).
I removed the comment.
wolfcrypt/src/sha256.c
Outdated
*/ | ||
static int set_default_digest256(wc_Sha256* sha256) | ||
{ | ||
return 0; /* TODO not used? */ |
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.
Red flag... This is all dead code below. Please cleanup/fix.
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.
Oops. My bad, That's code meant for future optimizations. Removed.
#error "WOLFSSL_SP_RISCV32 can only be used on RISC-V architecture" | ||
#endif | ||
#endif | ||
#if defined(WOLFSSL_SM2) || defined(WOLFSSL_SM3) || defined(WOLFSSL_SM4) |
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.
@SparkiDev is there value in doing this globally in settings.h? Seems like this could be refined a bit as some of the options are just SM4 specific.
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.
Looks good to me.
Jenkins retest this please. |
Assigning to @gojimmypi for merge conflict and had one question. Looks good otherwise, thanks! |
2da8811
Back to you @JacobBarthelmeh :) merge conflict resolved. Thank you |
See #6844 for Espressif example updates to fully utilize this PR. |
Description
Add Hardware Encryption Acceleration for ESP32-C3, ESP32-C6, and ESP32-S2.
Feature parity is with original ESP32. There are additional hardware accleration features for some targets to be addressed in a separate PR.
See Chip Series Comparison
This PR builds upon the prior work in #6624. Included is support for all of the current Espressif ESP-IDF v5.1 targets:
See
idf.py --list-targets
.With the 5 examples and 7 different targets, there are now 35 builds to check. At some point we may consider moving some (all?) of the examples to wolfssl-examples.
In the meantime, I've been testing with a revised compileAllExamples.sh to test all the builds. I need to parallelize it before including it in a PR.
Not included in this PR:
Although this PR will work with all Espressif targets, not all wolfSSL examples work without some minor target-specific attention. Rather than muddy up this PR with changes to examples, I'll be submitting a separate PR soon for the minor changes needed. See also pending TLS1.3 examples update in #6844.
See #6234 for a roadmap of Espressif updates.
Fixes zd# n/a/
Testing
How did you test?
For testing wolfSSL from Linux or WSL:
See espressif/esp-idf #10423 for ESP32-C6 development status.
See #6234 for Espressif Roadmap and related wolfSSL updates.
Checklist