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.

Sign up for GitHub

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

Jump to bottom

Software-only support for recent esp32 targets #6256

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Software-only support for recent esp32 targets #6256

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1af5ea5
make wolfcrypt use generic esp apis instead of arch specifics
dpalffy Mar 31, 2023
a45684c
clean up wolfssl_test for the generic esp apis
dpalffy Mar 31, 2023
09de0c0
also exclude ssl_bn.c in esp-idf component CMakeLists.txts
dpalffy Mar 31, 2023
73edfc9
also exclude ssl_asn1.c in esp-idf component CMakeLists.txts
dpalffy Mar 31, 2023
2af15f0
fix wolfcrypt/benchmark/benchmark.c for esp-idf v5
dpalffy Mar 31, 2023
80b1b82
add generic frequency reporting to wolfssl_test on esp platforms
dpalffy Apr 3, 2023
969ee0b
wolfcrypt/src/random.c: spaces instead of tabs
dpalffy Apr 4, 2023
fabb44f
clean up ESP32 hw crypto related defines
dpalffy Apr 4, 2023
e1c042b
esp wolfssl_test: add separate reporting for hw accelerated aes, hash…
dpalffy Apr 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions IDE/Espressif/ESP-IDF/examples/wolfssl_benchmark/components/wolfssl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ set(COMPONENT_SRCEXCLUDE
"${WOLFSSL_ROOT}/src/conf.c"
"${WOLFSSL_ROOT}/src/misc.c"
"${WOLFSSL_ROOT}/src/pk.c"
"${WOLFSSL_ROOT}/src/ssl_asn1.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_bn.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_misc.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/x509.c"
"${WOLFSSL_ROOT}/src/x509_str.c"
Expand Down
2 changes: 2 additions & 0 deletions IDE/Espressif/ESP-IDF/examples/wolfssl_client/components/wolfssl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ set(COMPONENT_SRCEXCLUDE
"${WOLFSSL_ROOT}/src/conf.c"
"${WOLFSSL_ROOT}/src/misc.c"
"${WOLFSSL_ROOT}/src/pk.c"
"${WOLFSSL_ROOT}/src/ssl_asn1.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_bn.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_misc.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/x509.c"
"${WOLFSSL_ROOT}/src/x509_str.c"
Expand Down
2 changes: 2 additions & 0 deletions IDE/Espressif/ESP-IDF/examples/wolfssl_server/components/wolfssl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ set(COMPONENT_SRCEXCLUDE
"${WOLFSSL_ROOT}/src/conf.c"
"${WOLFSSL_ROOT}/src/misc.c"
"${WOLFSSL_ROOT}/src/pk.c"
"${WOLFSSL_ROOT}/src/ssl_asn1.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_bn.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_misc.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/x509.c"
"${WOLFSSL_ROOT}/src/x509_str.c"
Expand Down
2 changes: 2 additions & 0 deletions IDE/Espressif/ESP-IDF/examples/wolfssl_test/components/wolfssl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ set(COMPONENT_SRCEXCLUDE
"${WOLFSSL_ROOT}/src/conf.c"
"${WOLFSSL_ROOT}/src/misc.c"
"${WOLFSSL_ROOT}/src/pk.c"
"${WOLFSSL_ROOT}/src/ssl_asn1.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_bn.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/ssl_misc.c" # included by ssl.c
"${WOLFSSL_ROOT}/src/x509.c"
"${WOLFSSL_ROOT}/src/x509_str.c"
Expand Down
49 changes: 29 additions & 20 deletions IDE/Espressif/ESP-IDF/examples/wolfssl_test/main/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,36 +152,45 @@ void app_main(void)


/* some interesting settings are target specific (ESP32, -C3, -S3, etc */
#if defined(CONFIG_IDF_TARGET_ESP32C3)
/* not available for C3 at this time */
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
ESP_LOGI(TAG, "CONFIG_ESP32S3_DEFAULT_CPU_FREQ_MHZ = %u MHz",
CONFIG_ESP32S3_DEFAULT_CPU_FREQ_MHZ
);
ESP_LOGI(TAG, "Xthal_have_ccount = %u", Xthal_have_ccount);
#include <esp_idf_version.h>
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0)
#define CONFIG_IDF_TARGET_NAME ESP
#else
ESP_LOGI(TAG, "CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ = %u MHz",
CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ
#if defined(CONFIG_IDF_TARGET_ESP32)
#define CONFIG_IDF_TARGET_NAME ESP32
#elif defined(CONFIG_IDF_TARGET_ESP32S2)
#define CONFIG_IDF_TARGET_NAME ESP32S2
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
#define CONFIG_IDF_TARGET_NAME ESP32S3
#elif defined(CONFIG_IDF_TARGET_ESP32H2)
#define CONFIG_IDF_TARGET_NAME ESP32H2
#elif defined(CONFIG_IDF_TARGET_ESP32C3)
#define CONFIG_IDF_TARGET_NAME ESP32C3
#else
#error CONFIG_IDF_TARGET " not supported"
#endif
#endif
gojimmypi marked this conversation as resolved.
Show resolved Hide resolved

#define LOG_TARGET_FREQ_INT(target) \
ESP_LOGI(TAG, "CONFIG_" #target "_DEFAULT_CPU_FREQ_MHZ = %u MHz", \
CONFIG_##target##_DEFAULT_CPU_FREQ_MHZ \
);
#define LOG_TARGET_FREQ(target) LOG_TARGET_FREQ_INT(target)

LOG_TARGET_FREQ(CONFIG_IDF_TARGET_NAME);

#if defined(CONFIG_IDF_TARGET_ARCH_XTENSA)
ESP_LOGI(TAG, "Xthal_have_ccount = %u", Xthal_have_ccount);
#endif

/* all platforms: stack high water mark check */
ESP_LOGI(TAG, "Stack HWM: %d\n", uxTaskGetStackHighWaterMark(NULL));

/* check to see if we are using hardware encryption */
#if defined(NO_ESP32WROOM32_CRYPT)
ESP_LOGI(TAG, "NO_ESP32WROOM32_CRYPT defined! HW acceleration DISABLED.");
#if defined(WOLFSSL_ESP32WROOM32_CRYPT)
ESP_LOGI(TAG, "ESP32WROOM32_CRYPT is enabled.");
#else
#if defined(CONFIG_IDF_TARGET_ESP32C3)
#error "ESP32WROOM32_CRYPT not yet supported on ESP32-C3"
#elif defined(CONFIG_IDF_TARGET_ESP32S2)
#error "ESP32WROOM32_CRYPT not yet supported on ESP32-S2"
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
#error "ESP32WROOM32_CRYPT not yet supported on ESP32-S3"
#else
ESP_LOGI(TAG, "ESP32WROOM32_CRYPT is enabled.");
#endif
ESP_LOGI(TAG, "WOLFSSL_ESP32WROOM32_CRYPT not defined! HW acceleration DISABLED.");
#endif


Expand Down
49 changes: 29 additions & 20 deletions IDE/Espressif/ESP-IDF/examples/wolfssl_test_idf/main/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,36 +152,45 @@ void app_main(void)


/* some interesting settings are target specific (ESP32, -C3, -S3, etc */
#if defined(CONFIG_IDF_TARGET_ESP32C3)
/* not available for C3 at this time */
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
ESP_LOGI(TAG, "CONFIG_ESP32S3_DEFAULT_CPU_FREQ_MHZ = %u MHz",
CONFIG_ESP32S3_DEFAULT_CPU_FREQ_MHZ
);
ESP_LOGI(TAG, "Xthal_have_ccount = %u", Xthal_have_ccount);
#include <esp_idf_version.h>
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0)
#define CONFIG_IDF_TARGET_NAME ESP
gojimmypi marked this conversation as resolved.
Show resolved Hide resolved
#else
ESP_LOGI(TAG, "CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ = %u MHz",
CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ
#if defined(CONFIG_IDF_TARGET_ESP32)
#define CONFIG_IDF_TARGET_NAME ESP32
#elif defined(CONFIG_IDF_TARGET_ESP32S2)
#define CONFIG_IDF_TARGET_NAME ESP32S2
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
#define CONFIG_IDF_TARGET_NAME ESP32S3
#elif defined(CONFIG_IDF_TARGET_ESP32H2)
#define CONFIG_IDF_TARGET_NAME ESP32H2
#elif defined(CONFIG_IDF_TARGET_ESP32C3)
#define CONFIG_IDF_TARGET_NAME ESP32C3
#else
#error CONFIG_IDF_TARGET " not supported"
#endif
#endif

#define LOG_TARGET_FREQ_INT(target) \
ESP_LOGI(TAG, "CONFIG_" #target "_DEFAULT_CPU_FREQ_MHZ = %u MHz", \
CONFIG_##target##_DEFAULT_CPU_FREQ_MHZ \
);
#define LOG_TARGET_FREQ(target) LOG_TARGET_FREQ_INT(target)

LOG_TARGET_FREQ(CONFIG_IDF_TARGET_NAME);

#if defined(CONFIG_IDF_TARGET_ARCH_XTENSA)
ESP_LOGI(TAG, "Xthal_have_ccount = %u", Xthal_have_ccount);
#endif

/* all platforms: stack high water mark check */
ESP_LOGI(TAG, "Stack HWM: %d\n", uxTaskGetStackHighWaterMark(NULL));

/* check to see if we are using hardware encryption */
#if defined(NO_ESP32WROOM32_CRYPT)
ESP_LOGI(TAG, "NO_ESP32WROOM32_CRYPT defined! HW acceleration DISABLED.");
#if defined(WOLFSSL_ESP32WROOM32_CRYPT)
ESP_LOGI(TAG, "ESP32WROOM32_CRYPT is enabled.");
#else
#if defined(CONFIG_IDF_TARGET_ESP32C3)
#error "ESP32WROOM32_CRYPT not yet supported on ESP32-C3"
#elif defined(CONFIG_IDF_TARGET_ESP32S2)
#error "ESP32WROOM32_CRYPT not yet supported on ESP32-S2"
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
#error "ESP32WROOM32_CRYPT not yet supported on ESP32-S3"
#else
ESP_LOGI(TAG, "ESP32WROOM32_CRYPT is enabled.");
#endif
ESP_LOGI(TAG, "WOLFSSL_ESP32WROOM32_CRYPT not defined! HW acceleration DISABLED.");
#endif


Expand Down
2 changes: 2 additions & 0 deletions IDE/Espressif/ESP-IDF/libs/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ set(COMPONENT_SRCEXCLUDE
"./src/conf.c"
"./src/misc.c"
"./src/pk.c"
"./src/ssl_asn1.c" # included by ssl.c
"./src/ssl_bn.c" # included by ssl.c
"./src/ssl_misc.c" # included by ssl.c
"./src/x509.c"
"./src/x509_str.c"
Expand Down
8 changes: 3 additions & 5 deletions IDE/Espressif/ESP-IDF/user_settings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,18 @@
#undef WOLFSSL_ESPIDF
#undef WOLFSSL_ESPWROOM32
#undef WOLFSSL_ESPWROOM32SE
#undef WOLFSSL_ESPWROOM32
#undef WOLFSSL_ESP8266

#define WOLFSSL_ESPIDF

/*
* choose ONE of these Espressif chips to define:
* choose ONE of these Espressif chips to define for HW acceleration or
* leave all undefined for a non-accelerated build for other chips:
*
* WOLFSSL_ESPWROOM32
* WOLFSSL_ESPWROOM32SE
* WOLFSSL_ESP8266
*/
gojimmypi marked this conversation as resolved.
Show resolved Hide resolved

#define WOLFSSL_ESPWROOM32
/* #define WOLFSSL_ESPWROOM32 */
Copy link
Contributor

Choose a reason for hiding this comment

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

Although I agree that the ambiguous WOLFSSL_ESPWROOM32 should be refined, it is probably not a good idea to remove it at this time. See wolfssl/wolfcrypt/settings.h. Let's leave HW acceleration enabled by default for the classic ESP32 and disable for architectures that don't support it at this time.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I'll look into adding some #ifdefs to enable it on the appropriate targets.

Copy link
Author

Choose a reason for hiding this comment

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

Please help me clarify a few things here if I read the code correctly:

  • WOLFSSL_ESPWROOM32 has nothing to do with the WROOM module, it's the internal crypto accelerator of the ESP32 chip. and should essentially be CONFIG_IDF_TARGET_ESP32. The other targets have different crypto accelerators.
  • the WROOM32-SE module has the ATECC608A in addition. WOLFSSL_ESPWROOM32SE enables the same features as WOLFSSL_ESPWROOM32 and adds the WOLFSSL_ATECC508A #define (although the code has 608A define, too).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you understand correctly. Unfortunately that's true, and not very intuitive, I know. That was created when there was only the ESP32WROOM chip. At some point that needs to be cleaned up.

I agree the WOLFSSL_ESPWROOM32 and WOLFSSL_ESPWROOM32SE should probably be completely removed and replaced with something more appropriate.

The ESP32-S3 on the Xtensa architecture has very similar crypto accelerators. As you noted: the RISC-V chipsets are very different. I have a WIP for some HW acceleration on the -C3 and -C6.

Oh, and yes: the SE uses the ATECC508A. I'm not certain if it is 100% compatible with the ATECC608A.

Copy link
Author

Choose a reason for hiding this comment

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

According to the datasheet, the SE has a 608A. Do you maybe have the hardware to test if changing the define to 608 still works?

Otherwise how about this:

  • Remove WROOM32 from the following macros, they all refer to the internal crypto in the ESP32:
    • NO_ESP32WROOM32_CRYPT
    • WOLFSSL_ESP32WROOM32_CRYPT
    • NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI
    • NO_WOLFSSL_ESP32WROOM32_CRYPT_HASH
    • NO_WOLFSSL_ESP32WROOM32_CRYPT_AES
    • WOLFSSL_ESP32WROOM32_CRYPT_DEBUG
    • WOLFSSL_USE_ESP32WROOM32_CRYPT_HASH_HW
    • WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI
  • Replace WOLFSSL_ESPWROOM32 with CONFIG_IDF_TARGET_ESP32. It is only used in settings.h/user_settings.h to set other macros. HW crypto can still be disabled from user_settings.h with the NO_ macros.
  • Leave WOLFSSL_ESPWROOM32SE as is (maybe change 508A to 608A) to enable the Atmel chip from user_settings.h, by default disabled.
  • If S3 hardware crypto is merged and shares a significant amount of code with the original ESP32, maybe also set some WOLFSSL_ESP32_XTENSA_CRYPT based on CONFIG_IDF_TARGET in settings.h for both to mark the shared code.

Copy link
Contributor

Choose a reason for hiding this comment

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

My mistake on the SE, I thought it was the '608A. I don't have either one to test.

I agree on others: I like the idea of removing WROOM32. Good to keep WOLFSSL_ESPWROOM32SE as it is a very specific chipset. At some point, I'd like to have 508A/608A support on other chipsets that may have a manually added device.

Copy link
Author

Choose a reason for hiding this comment

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

Done, PTAL.


/* #define DEBUG_WOLFSSL_VERBOSE */

Expand Down
54 changes: 32 additions & 22 deletions wolfcrypt/benchmark/benchmark.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,17 @@
#include <wolfssl/wolfcrypt/wolfmath.h>

#ifdef WOLFSSL_ESPIDF
#include <xtensa/hal.h> /* reminder Espressif RISC-V not yet implemented */
#include <esp_idf_version.h>
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0)
#include <esp_cpu.h>
#elif ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 3, 0)
#include <hal/cpu_hal.h>
#define esp_cpu_get_cycle_count() cpu_hal_get_cycle_count()
dpalffy marked this conversation as resolved.
Show resolved Hide resolved
#else
#include <xtensa/hal.h>
#define esp_cpu_get_cycle_count() xthal_get_ccount()
#endif

#include <esp_log.h>
#endif

Expand Down Expand Up @@ -964,28 +974,28 @@ static const char* bench_desc_words[][15] = {
static THREAD_LS_T word64 total_cycles;

/* the return value */
static THREAD_LS_T word64 _xthal_get_ccount_ex = 0;
static THREAD_LS_T word64 _esp_cpu_get_cycle_count_ex = 0;

/* the last value seen, adjusted for an overflow */
static THREAD_LS_T word64 _xthal_get_ccount_last = 0;
static THREAD_LS_T word64 _esp_cpu_get_cycle_count_last = 0;

/* TAG for ESP_LOGx() */
static const char* TAG = "wolfssl_benchmark";

#define HAVE_GET_CYCLES
#define INIT_CYCLE_COUNTER
static WC_INLINE word64 get_xtensa_cycles(void);
static WC_INLINE word64 get_esp_cpu_cycles(void);

/* WARNING the hal UINT xthal_get_ccount() quietly rolls over. */
#define BEGIN_ESP_CYCLES begin_cycles = (get_xtensa_cycles());
/* WARNING the hal UINT esp_cpu_get_cycle_count() quietly rolls over. */
#define BEGIN_ESP_CYCLES begin_cycles = (get_esp_cpu_cycles());

/* since it rolls over, we have something that will tolerate one */
#define END_ESP_CYCLES \
ESP_LOGV(TAG,"%llu - %llu", \
get_xtensa_cycles(), \
get_esp_cpu_cycles(), \
begin_cycles \
); \
total_cycles = (get_xtensa_cycles() - begin_cycles);
total_cycles = (get_esp_cpu_cycles() - begin_cycles);

#define SHOW_ESP_CYCLES(b, n, s) \
(void)XSNPRINTF(b + XSTRLEN(b), n - XSTRLEN(b), " %s = %6.2f\n", \
Expand All @@ -997,23 +1007,23 @@ static const char* bench_desc_words[][15] = {
(void)XSNPRINTF(b + XSTRLEN(b), n - XSTRLEN(b), "%.6f,\n", \
(float)total_cycles / (count*s))

/* xthal_get_ccount_ex() is a single-overflow-tolerant extension to
** the Espressif `unsigned xthal_get_ccount()` which is known to overflow
** at least once during full benchmark tests.
/* esp_cpu_get_cycle_count_ex() is a single-overflow-tolerant extension to
** the Espressif `unsigned esp_cpu_get_cycle_count()` which is known to
** overflow at least once during full benchmark tests.
*/
word64 xthal_get_ccount_ex()
word64 esp_cpu_get_cycle_count_ex()
{
/* reminder: unsigned long long max = 18,446,744,073,709,551,615 */

/* the currently observed clock counter value */
word64 thisVal = xthal_get_ccount();
word64 thisVal = esp_cpu_get_cycle_count();

/* if the current value is less than the previous value,
** we likely overflowed at least once.
*/
if (thisVal < _xthal_get_ccount_last)
if (thisVal < _esp_cpu_get_cycle_count_last)
{
/* Warning: we assume the return type of xthal_get_ccount()
/* Warning: we assume the return type of esp_cpu_get_cycle_count()
** will always be unsigned int to add UINT_MAX.
**
** NOTE for long duration between calls with multiple overflows:
Expand All @@ -1022,21 +1032,21 @@ static const char* bench_desc_words[][15] = {
**
** At this time no single test overflows. This is currently only a
** concern for cumulative counts over multiple tests. As long
** as well call xthal_get_ccount_ex() with no more than one
** as well call esp_cpu_get_cycle_count_ex() with no more than one
** overflow CPU tick count, all will be well.
*/
ESP_LOGV(TAG, "Alert: Detected xthal_get_ccount overflow, "
ESP_LOGV(TAG, "Alert: Detected esp_cpu_get_cycle_count overflow, "
"adding %ull", UINT_MAX);
thisVal += (word64)UINT_MAX;
}

/* adjust our actual returned value that takes into account overflow */
_xthal_get_ccount_ex += (thisVal - _xthal_get_ccount_last);
_esp_cpu_get_cycle_count_ex += (thisVal - _esp_cpu_get_cycle_count_last);

/* all of this took some time, so reset the "last seen" value */
_xthal_get_ccount_last = xthal_get_ccount();
_esp_cpu_get_cycle_count_last = esp_cpu_get_cycle_count();

return _xthal_get_ccount_ex;
return _esp_cpu_get_cycle_count_ex;
}

/* implement other architecture cycle counters here */
Expand Down Expand Up @@ -8887,9 +8897,9 @@ void bench_sphincsKeySign(byte level, byte optim)
#if defined(HAVE_GET_CYCLES)

#if defined(WOLFSSL_ESPIDF)
static WC_INLINE word64 get_xtensa_cycles(void)
static WC_INLINE word64 get_esp_cpu_cycles(void)
{
return xthal_get_ccount_ex();
return esp_cpu_get_cycle_count_ex();
}

/* implement other architectures here */
Expand Down
Loading