From 45df5b56d5a5050cf2b292ec5aef85fda7c6df9d Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Fri, 27 Jan 2023 18:05:33 +0530 Subject: [PATCH 1/2] esp32/mpi: Added alternate workaround for MPI data corruption issue - Use DPORT_WRITE_REG (volatile writes) wrappers to write to the MPI peripheral - Updated the previous workaround added for the same issue as it was failing in some long runs and with `COMPILER_OPTIMIZATION_PERF` enabled. - The test performance numbers had to be updated due to the performance penalty introduced by this fix. Closes https://github.com/espressif/esp-idf/issues/10403 --- .../include/esp32/idf_performance_target.h | 4 ++-- components/mbedtls/port/esp32/bignum.c | 21 +++++++------------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/components/idf_test/include/esp32/idf_performance_target.h b/components/idf_test/include/esp32/idf_performance_target.h index 2ca60b40e96..87152aef235 100644 --- a/components/idf_test/include/esp32/idf_performance_target.h +++ b/components/idf_test/include/esp32/idf_performance_target.h @@ -22,11 +22,11 @@ #define IDF_PERFORMANCE_MAX_TIME_SHA512_32KB 4500 #define IDF_PERFORMANCE_MAX_RSA_2048KEY_PUBLIC_OP 19000 -#define IDF_PERFORMANCE_MAX_RSA_2048KEY_PRIVATE_OP 420000 +#define IDF_PERFORMANCE_MAX_RSA_2048KEY_PRIVATE_OP 450000 #define IDF_PERFORMANCE_MAX_RSA_3072KEY_PUBLIC_OP 33000 #define IDF_PERFORMANCE_MAX_RSA_3072KEY_PRIVATE_OP 950000 #define IDF_PERFORMANCE_MAX_RSA_4096KEY_PUBLIC_OP 90000 -#define IDF_PERFORMANCE_MAX_RSA_4096KEY_PRIVATE_OP 1700000 +#define IDF_PERFORMANCE_MAX_RSA_4096KEY_PRIVATE_OP 1900000 #define IDF_PERFORMANCE_MAX_SPI_PER_TRANS_POLLING 15 #define IDF_PERFORMANCE_MAX_SPI_PER_TRANS_POLLING_NO_DMA 15 diff --git a/components/mbedtls/port/esp32/bignum.c b/components/mbedtls/port/esp32/bignum.c index 2b55bc4c7df..ce6b1ca41d5 100644 --- a/components/mbedtls/port/esp32/bignum.c +++ b/components/mbedtls/port/esp32/bignum.c @@ -69,22 +69,22 @@ void esp_mpi_interrupt_clear( void ) */ /* Please see detailed note inside the function body below. - * Relevant: https://github.com/espressif/esp-idf/issues/8710 and IDF-6029 + * Relevant: IDF-6029 + https://github.com/espressif/esp-idf/issues/8710 + https://github.com/espressif/esp-idf/issues/10403 */ -static inline void __attribute__((optimize("-fno-tree-loop-distribute-patterns"))) - mpi_to_mem_block(uint32_t mem_base, const mbedtls_mpi *mpi, size_t hw_words) +static inline void mpi_to_mem_block(uint32_t mem_base, const mbedtls_mpi *mpi, size_t hw_words) { - uint32_t *pbase = (uint32_t *)mem_base; uint32_t copy_words = MIN(hw_words, mpi->MBEDTLS_PRIVATE(n)); /* Copy MPI data to memory block registers */ for (uint32_t i = 0; i < copy_words; i++) { - pbase[i] = mpi->MBEDTLS_PRIVATE(p[i]); + DPORT_REG_WRITE(mem_base + i * 4, mpi->MBEDTLS_PRIVATE(p[i])); } /* Zero any remaining memory block data */ for (uint32_t i = copy_words; i < hw_words; i++) { - pbase[i] = 0; + DPORT_REG_WRITE(mem_base + i * 4, 0); } #if _INTERNAL_DEBUG_PURPOSE @@ -94,14 +94,9 @@ static inline void __attribute__((optimize("-fno-tree-loop-distribute-patterns") * specific write (store) operation to the MPI peripheral block was getting lost erroneously. * Following data re-verify loop could catch it during runtime. * - * As a workaround, we are disabling loop distribute patterns for this function and hence - * compiler does not enforce usage of `memset` (or `memcpy`) calls for this routine. It - * appears that `-ftree-loop-distribute-patterns` was enabled with O2/Os starting from - * GCC-10.x. It is quite possible that there is some issue with DPORT write with sequence of - * store instructions as generated by `memset` call, but for now this should serve as good - * interim workaround without any impact on the performance. + * As a workaround, we are using DPORT_WRITE_REG (volatile writes) wrappers to write to + * the MPI peripheral. * - * Please see IDF-6029 for more details. */ //for (uint32_t i = copy_words; i < hw_words; i++) { assert(pbase[i] == 0); } From 9f0435faa45c7cb5b7c861038f2f1529000425cc Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Wed, 4 Jan 2023 10:52:12 +0530 Subject: [PATCH 2/2] mbedtls: Add test config with `CONFIG_COMPILER_OPTIMIZATION_PERF` --- .../mbedtls/test_apps/pytest_mbedtls_ut.py | 29 +++++++++++-------- .../mbedtls/test_apps/sdkconfig.ci.perf_esp32 | 1 + 2 files changed, 18 insertions(+), 12 deletions(-) create mode 100644 components/mbedtls/test_apps/sdkconfig.ci.perf_esp32 diff --git a/components/mbedtls/test_apps/pytest_mbedtls_ut.py b/components/mbedtls/test_apps/pytest_mbedtls_ut.py index bc3a22263cf..beb6cc7fcc2 100644 --- a/components/mbedtls/test_apps/pytest_mbedtls_ut.py +++ b/components/mbedtls/test_apps/pytest_mbedtls_ut.py @@ -8,9 +8,20 @@ @pytest.mark.supported_targets @pytest.mark.generic def test_mbedtls(dut: Dut) -> None: - dut.expect_exact('Press ENTER to see the list of tests') - dut.write('*') - dut.expect_unity_test_output(timeout=120) + dut.run_all_single_board_cases() + + +@pytest.mark.esp32 +@pytest.mark.generic +@pytest.mark.parametrize( + 'config', + [ + 'perf_esp32', + ], + indirect=True, +) +def test_mbedtls_esp32_compiler_perf_opt(dut: Dut) -> None: + dut.run_all_single_board_cases() @pytest.mark.esp32 @@ -26,9 +37,7 @@ def test_mbedtls(dut: Dut) -> None: indirect=True, ) def test_mbedtls_aes_no_hw(dut: Dut) -> None: - dut.expect_exact('Press ENTER to see the list of tests') - dut.write('*') - dut.expect_unity_test_output(timeout=120) + dut.run_all_single_board_cases() @pytest.mark.esp32 @@ -43,9 +52,7 @@ def test_mbedtls_aes_no_hw(dut: Dut) -> None: indirect=True, ) def test_mbedtls_psram(dut: Dut) -> None: - dut.expect_exact('Press ENTER to see the list of tests') - dut.write('*') - dut.expect_unity_test_output(timeout=120) + dut.run_all_single_board_cases() @pytest.mark.esp32 @@ -59,6 +66,4 @@ def test_mbedtls_psram(dut: Dut) -> None: indirect=True, ) def test_mbedtls_psram_esp32(dut: Dut) -> None: - dut.expect_exact('Press ENTER to see the list of tests') - dut.write('*') - dut.expect_unity_test_output(timeout=120) + dut.run_all_single_board_cases() diff --git a/components/mbedtls/test_apps/sdkconfig.ci.perf_esp32 b/components/mbedtls/test_apps/sdkconfig.ci.perf_esp32 new file mode 100644 index 00000000000..3c5a0fabea2 --- /dev/null +++ b/components/mbedtls/test_apps/sdkconfig.ci.perf_esp32 @@ -0,0 +1 @@ +CONFIG_COMPILER_OPTIMIZATION_PERF=y