aes/sha/mpi: Bugfix a use of shared registers.

This commit resolves a blocking in esp_aes_block function.

Introduce:
The problem was in the fact that AES is switched off at the moment when he should give out the processed data. But because of the disabled, the operation can not be completed successfully, there is an infinite hang. The reason for this behavior is that the registers for controlling the inclusion of AES, SHA, MPI have shared registers and they were not protected from sharing.

Fix some related issue with shared using of AES SHA RSA accelerators.

Closes: https://github.com/espressif/esp-idf/issues/2295#issuecomment-432898137

diff --git a/components/driver/include/driver/periph_ctrl.h b/components/driver/include/driver/periph_ctrl.h
index b418100902e3b76e854f8a4c81397155cdd45de0..f93909c1d0fbebbda9a9776d2a645e6d4c1d27ab 100644 (file)
--- a/components/driver/include/driver/periph_ctrl.h
+++ b/components/driver/include/driver/periph_ctrl.h
@@ -53,6 +53,9 @@ typedef enum {
     PERIPH_WIFI_MODULE,
     PERIPH_BT_MODULE,
     PERIPH_WIFI_BT_COMMON_MODULE,
+    PERIPH_AES_MODULE,
+    PERIPH_SHA_MODULE,
+    PERIPH_RSA_MODULE,
 } periph_module_t;
 
 /**

diff --git a/components/driver/periph_ctrl.c b/components/driver/periph_ctrl.c
index 67431a4276cdb55851a845a68da679e9f19798b1..653d2973e4b4baf86c0feea3af27d7559af8f7bf 100644 (file)
--- a/components/driver/periph_ctrl.c
+++ b/components/driver/periph_ctrl.c
@@ -23,7 +23,7 @@ static portMUX_TYPE periph_spinlock = portMUX_INITIALIZER_UNLOCKED;
 
 /* Static functions to return register address & mask for clk_en / rst of each peripheral */
 static uint32_t get_clk_en_mask(periph_module_t periph);
-static uint32_t get_rst_en_mask(periph_module_t periph);
+static uint32_t get_rst_en_mask(periph_module_t periph, bool enable);
 static uint32_t get_clk_en_reg(periph_module_t periph);
 static uint32_t get_rst_en_reg(periph_module_t periph);
 
@@ -31,7 +31,7 @@ void periph_module_enable(periph_module_t periph)
 {
     portENTER_CRITICAL(&periph_spinlock);
     DPORT_SET_PERI_REG_MASK(get_clk_en_reg(periph), get_clk_en_mask(periph));
-    DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph));
+    DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, true));
     portEXIT_CRITICAL(&periph_spinlock);
 }
 
@@ -39,15 +39,15 @@ void periph_module_disable(periph_module_t periph)
 {
     portENTER_CRITICAL(&periph_spinlock);
     DPORT_CLEAR_PERI_REG_MASK(get_clk_en_reg(periph), get_clk_en_mask(periph));
-    DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph));
+    DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false));
     portEXIT_CRITICAL(&periph_spinlock);
 }
 
 void periph_module_reset(periph_module_t periph)
 {
     portENTER_CRITICAL(&periph_spinlock);
-    DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph));
-    DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph));
+    DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false));
+    DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false));
     portEXIT_CRITICAL(&periph_spinlock);
 }
 
@@ -114,12 +114,18 @@ static uint32_t get_clk_en_mask(periph_module_t periph)
             return DPORT_WIFI_CLK_BT_EN_M;
         case PERIPH_WIFI_BT_COMMON_MODULE:
             return DPORT_WIFI_CLK_WIFI_BT_COMMON_M;
+        case PERIPH_AES_MODULE:
+            return DPORT_PERI_EN_AES;
+        case PERIPH_SHA_MODULE:
+            return DPORT_PERI_EN_SHA;
+        case PERIPH_RSA_MODULE:
+            return DPORT_PERI_EN_RSA;
         default:
             return 0;
     }
 }
 
-static uint32_t get_rst_en_mask(periph_module_t periph)
+static uint32_t get_rst_en_mask(periph_module_t periph, bool enable)
 {
     switch(periph) {
         case PERIPH_RMT_MODULE:
@@ -174,6 +180,30 @@ static uint32_t get_rst_en_mask(periph_module_t periph)
             return DPORT_CAN_RST;
         case PERIPH_EMAC_MODULE:
             return DPORT_EMAC_RST;
+        case PERIPH_AES_MODULE:
+            if (enable == true) {
+                // Clear reset on digital signature & secure boot units, otherwise AES unit is held in reset also.
+                return (DPORT_PERI_EN_AES | DPORT_PERI_EN_DIGITAL_SIGNATURE | DPORT_PERI_EN_SECUREBOOT);
+            } else {
+                //Don't return other units to reset, as this pulls reset on RSA & SHA units, respectively.
+                return DPORT_PERI_EN_AES;
+            }
+        case PERIPH_SHA_MODULE:
+            if (enable == true) {
+                // Clear reset on secure boot, otherwise SHA is held in reset
+                return (DPORT_PERI_EN_SHA | DPORT_PERI_EN_SECUREBOOT);
+            } else {
+                // Don't assert reset on secure boot, otherwise AES is held in reset
+                return DPORT_PERI_EN_SHA;
+            }
+        case PERIPH_RSA_MODULE:
+            if (enable == true) {
+                // Also clear reset on digital signature, otherwise RSA is held in reset
+                return (DPORT_PERI_EN_RSA | DPORT_PERI_EN_DIGITAL_SIGNATURE);
+            } else {
+                // Don't reset digital signature unit, as this resets AES also
+                return DPORT_PERI_EN_RSA;
+            }
         case PERIPH_WIFI_MODULE:
         case PERIPH_BT_MODULE:
         case PERIPH_WIFI_BT_COMMON_MODULE:
@@ -203,12 +233,20 @@ static bool is_wifi_clk_peripheral(periph_module_t periph)
 
 static uint32_t get_clk_en_reg(periph_module_t periph)
 {
-    return is_wifi_clk_peripheral(periph) ? DPORT_WIFI_CLK_EN_REG : DPORT_PERIP_CLK_EN_REG;
+    if (periph == PERIPH_AES_MODULE || periph == PERIPH_SHA_MODULE || periph == PERIPH_RSA_MODULE) {
+        return DPORT_PERI_CLK_EN_REG;
+    } else {
+        return is_wifi_clk_peripheral(periph) ? DPORT_WIFI_CLK_EN_REG : DPORT_PERIP_CLK_EN_REG;
+    }
 }
 
 static uint32_t get_rst_en_reg(periph_module_t periph)
 {
-    return is_wifi_clk_peripheral(periph) ? DPORT_CORE_RST_EN_REG : DPORT_PERIP_RST_EN_REG;
+    if (periph == PERIPH_AES_MODULE || periph == PERIPH_SHA_MODULE || periph == PERIPH_RSA_MODULE) {
+        return DPORT_PERI_RST_EN_REG;
+    } else {
+        return is_wifi_clk_peripheral(periph) ? DPORT_CORE_RST_EN_REG : DPORT_PERIP_RST_EN_REG;
+    }
 }
 
 

diff --git a/components/esp32/hwcrypto/aes.c b/components/esp32/hwcrypto/aes.c
index 125f543371d650f3c12072a073f3db72dd4a6401..9bc665e321e3cea8124fd71c042cfd595cbd1a76 100644 (file)
--- a/components/esp32/hwcrypto/aes.c
+++ b/components/esp32/hwcrypto/aes.c
@@ -36,6 +36,7 @@
 
 #include "soc/cpu.h"
 #include <stdio.h>
+#include "driver/periph_ctrl.h"
 
 
 /* AES uses a spinlock mux not a lock as the underlying block operation
@@ -50,9 +51,9 @@ static portMUX_TYPE aes_spinlock = portMUX_INITIALIZER_UNLOCKED;
 
 void esp_aes_acquire_hardware( void )
 {
-    /* newlib locks lazy initialize on ESP-IDF */
     portENTER_CRITICAL(&aes_spinlock);
 
+#if defined(DPORT_PROTECT_STALL_OTHER_CPU_USE)
     DPORT_STALL_OTHER_CPU_START();
     {
         /* Enable AES hardware */
@@ -65,10 +66,15 @@ void esp_aes_acquire_hardware( void )
                            | DPORT_PERI_EN_SECUREBOOT);
     }
     DPORT_STALL_OTHER_CPU_END();
+#else
+    /* Enable AES hardware */
+    periph_module_enable(PERIPH_AES_MODULE);
+#endif
 }
 
 void esp_aes_release_hardware( void )
 {
+#if defined(DPORT_PROTECT_STALL_OTHER_CPU_USE)
     DPORT_STALL_OTHER_CPU_START();
     {
         /* Disable AES hardware */
@@ -78,7 +84,10 @@ void esp_aes_release_hardware( void )
         _DPORT_REG_CLR_BIT(DPORT_PERI_CLK_EN_REG, DPORT_PERI_EN_AES);
     }
     DPORT_STALL_OTHER_CPU_END();
-
+#else
+    /* Disable AES hardware */
+    periph_module_disable(PERIPH_AES_MODULE);
+#endif
     portEXIT_CRITICAL(&aes_spinlock);
 }
 

diff --git a/components/esp32/hwcrypto/sha.c b/components/esp32/hwcrypto/sha.c
index c235804611939f85ec71ebd76c9254e354b30845..1d951023a7764def5f18f1c90fca28ac6d27a093 100644 (file)
--- a/components/esp32/hwcrypto/sha.c
+++ b/components/esp32/hwcrypto/sha.c
@@ -35,6 +35,7 @@
 #include "rom/ets_sys.h"
 #include "soc/dport_reg.h"
 #include "soc/hwcrypto_reg.h"
+#include "driver/periph_ctrl.h"
 
 inline static uint32_t SHA_LOAD_REG(esp_sha_type sha_type) {
     return SHA_1_LOAD_REG + sha_type * 0x10;
@@ -159,6 +160,7 @@ static void esp_sha_lock_engine_inner(sha_engine_state *engine)
     _lock_acquire(&state_change_lock);
 
     if (sha_engines_all_idle()) {
+#if defined(DPORT_PROTECT_STALL_OTHER_CPU_USE)
         DPORT_STALL_OTHER_CPU_START();
         {
             /* Enable SHA hardware */
@@ -170,6 +172,11 @@ static void esp_sha_lock_engine_inner(sha_engine_state *engine)
             ets_sha_enable();
         }
         DPORT_STALL_OTHER_CPU_END();
+#else
+        /* Enable SHA hardware */
+        periph_module_enable(PERIPH_SHA_MODULE);
+        ets_sha_enable();
+#endif
     }
 
     assert( !engine->in_use && "in_use flag should be cleared" );
@@ -189,6 +196,7 @@ void esp_sha_unlock_engine(esp_sha_type sha_type)
     engine->in_use = false;
 
     if (sha_engines_all_idle()) {
+#if defined(DPORT_PROTECT_STALL_OTHER_CPU_USE)
         /* Disable SHA hardware */
         /* Don't assert reset on secure boot, otherwise AES is held in reset */
         DPORT_STALL_OTHER_CPU_START();
@@ -197,6 +205,9 @@ void esp_sha_unlock_engine(esp_sha_type sha_type)
             _DPORT_REG_CLR_BIT(DPORT_PERI_CLK_EN_REG, DPORT_PERI_EN_SHA);
         }
         DPORT_STALL_OTHER_CPU_END();
+#else
+    periph_module_disable(PERIPH_SHA_MODULE);
+#endif
     }
 
     _lock_release(&state_change_lock);

diff --git a/components/esp32/include/esp_dport_access.h b/components/esp32/include/esp_dport_access.h
index 3acf806888be94530779c44335186e9b51b9b4b6..4d97b496fb610b7622cc7757e7f895e17ade24a8 100644 (file)
--- a/components/esp32/include/esp_dport_access.h
+++ b/components/esp32/include/esp_dport_access.h
@@ -35,6 +35,7 @@ void esp_dport_access_int_abort(void);
 #define DPORT_STALL_OTHER_CPU_START()
 #define DPORT_STALL_OTHER_CPU_END()
 #else
+#define DPORT_PROTECT_STALL_OTHER_CPU_USE
 #define DPORT_STALL_OTHER_CPU_START()   esp_dport_access_stall_other_cpu_start()
 #define DPORT_STALL_OTHER_CPU_END()     esp_dport_access_stall_other_cpu_end()
 #endif

diff --git a/components/mbedtls/port/esp_bignum.c b/components/mbedtls/port/esp_bignum.c
index f18652c7bb5a456c43c865369a1677b167596bc2..e6581cbb55df902174bff7907d07313770675373 100644 (file)
--- a/components/mbedtls/port/esp_bignum.c
+++ b/components/mbedtls/port/esp_bignum.c
@@ -40,6 +40,7 @@
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "freertos/semphr.h"
+#include "driver/periph_ctrl.h"
 
 static const __attribute__((unused)) char *TAG = "bignum";
 
@@ -75,7 +76,7 @@ void esp_mpi_acquire_hardware( void )
 {
     /* newlib locks lazy initialize on ESP-IDF */
     _lock_acquire(&mpi_lock);
-
+#if defined(DPORT_PROTECT_STALL_OTHER_CPU_USE)
     DPORT_STALL_OTHER_CPU_START();
     {
         _DPORT_REG_SET_BIT(DPORT_PERI_CLK_EN_REG, DPORT_PERI_EN_RSA);
@@ -87,7 +88,10 @@ void esp_mpi_acquire_hardware( void )
         _DPORT_REG_CLR_BIT(DPORT_RSA_PD_CTRL_REG, DPORT_RSA_PD);
     }
     DPORT_STALL_OTHER_CPU_END();
-
+#else
+    periph_module_enable(PERIPH_RSA_MODULE);
+    DPORT_REG_CLR_BIT(DPORT_RSA_PD_CTRL_REG, DPORT_RSA_PD);
+#endif
     while(DPORT_REG_READ(RSA_CLEAN_REG) != 1);
     // Note: from enabling RSA clock to here takes about 1.3us
 
@@ -98,6 +102,7 @@ void esp_mpi_acquire_hardware( void )
 
 void esp_mpi_release_hardware( void )
 {
+#if defined(DPORT_PROTECT_STALL_OTHER_CPU_USE)
     DPORT_STALL_OTHER_CPU_START();
     {
         _DPORT_REG_SET_BIT(DPORT_RSA_PD_CTRL_REG, DPORT_RSA_PD);
@@ -107,7 +112,11 @@ void esp_mpi_release_hardware( void )
         _DPORT_REG_CLR_BIT(DPORT_PERI_CLK_EN_REG, DPORT_PERI_EN_RSA);
     }
     DPORT_STALL_OTHER_CPU_END();
-
+#else
+    /* Disable RSA hardware */
+    DPORT_REG_SET_BIT(DPORT_RSA_PD_CTRL_REG, DPORT_RSA_PD);
+    periph_module_disable(PERIPH_RSA_MODULE);
+#endif
     _lock_release(&mpi_lock);
 }