From 26fa7109f357f36ecfb56218464624338fa65abc Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Thu, 6 Feb 2025 17:33:51 +0530 Subject: [PATCH 1/3] fix(esp_tee): Protect the AES/SHA clock registers from REE access --- components/esp_system/port/soc/esp32c6/clk.c | 3 ++ .../scripts/esp32c6/sec_srv_tbl_default.yml | 4 +++ .../esp_tee/src/esp_secure_service_wrapper.c | 5 +++ .../main/core/esp_secure_services.c | 6 ++++ .../main/soc/esp32c6/esp_tee_apm_prot_cfg.c | 31 ++++++++++------ .../main/soc/esp32c6/esp_tee_secure_sys_cfg.c | 7 +++- .../port/sha/core/include/esp_sha_internal.h | 7 ++++ components/mbedtls/port/sha/core/sha.c | 36 ++++++++----------- components/mbedtls/port/sha/esp_sha.c | 24 +++++++++++++ 9 files changed, 90 insertions(+), 33 deletions(-) diff --git a/components/esp_system/port/soc/esp32c6/clk.c b/components/esp_system/port/soc/esp32c6/clk.c index 15c8dfd70b..6b8355392a 100644 --- a/components/esp_system/port/soc/esp32c6/clk.c +++ b/components/esp_system/port/soc/esp32c6/clk.c @@ -291,8 +291,11 @@ __attribute__((weak)) void esp_perip_clk_init(void) periph_ll_disable_clk_set_rst(PERIPH_ASSIST_DEBUG_MODULE); #endif periph_ll_disable_clk_set_rst(PERIPH_RSA_MODULE); +#if !CONFIG_SECURE_ENABLE_TEE + // NOTE: [ESP-TEE] The TEE is responsible for the AES and SHA peripherals periph_ll_disable_clk_set_rst(PERIPH_AES_MODULE); periph_ll_disable_clk_set_rst(PERIPH_SHA_MODULE); +#endif periph_ll_disable_clk_set_rst(PERIPH_ECC_MODULE); periph_ll_disable_clk_set_rst(PERIPH_HMAC_MODULE); periph_ll_disable_clk_set_rst(PERIPH_DS_MODULE); diff --git a/components/esp_tee/scripts/esp32c6/sec_srv_tbl_default.yml b/components/esp_tee/scripts/esp32c6/sec_srv_tbl_default.yml index 45a0837722..53349f4d0e 100644 --- a/components/esp_tee/scripts/esp32c6/sec_srv_tbl_default.yml +++ b/components/esp_tee/scripts/esp32c6/sec_srv_tbl_default.yml @@ -208,6 +208,10 @@ secure_services: type: IDF function: esp_sha_write_digest_state args: 2 + - id: 132 + type: IDF + function: esp_sha_enable_periph_clk + args: 1 # ID: 134-149 (16) - eFuse - family: efuse entries: diff --git a/components/esp_tee/src/esp_secure_service_wrapper.c b/components/esp_tee/src/esp_secure_service_wrapper.c index 5c3e3232f8..692bceb92c 100644 --- a/components/esp_tee/src/esp_secure_service_wrapper.c +++ b/components/esp_tee/src/esp_secure_service_wrapper.c @@ -228,6 +228,11 @@ void __wrap_esp_sha_write_digest_state(esp_sha_type sha_type, void *digest_state esp_tee_service_call(3, SS_ESP_SHA_WRITE_DIGEST_STATE, sha_type, digest_state); } +void __wrap_esp_sha_enable_periph_clk(bool enable) +{ + esp_tee_service_call(2, SS_ESP_SHA_ENABLE_PERIPH_CLK, enable); +} + /* ---------------------------------------------- MMU HAL ------------------------------------------------- */ void IRAM_ATTR __wrap_mmu_hal_map_region(uint32_t mmu_id, mmu_target_t mem_type, uint32_t vaddr, uint32_t paddr, uint32_t len, uint32_t *out_len) diff --git a/components/esp_tee/subproject/main/core/esp_secure_services.c b/components/esp_tee/subproject/main/core/esp_secure_services.c index 6c5fb5dc9b..0961a304a3 100644 --- a/components/esp_tee/subproject/main/core/esp_secure_services.c +++ b/components/esp_tee/subproject/main/core/esp_secure_services.c @@ -26,6 +26,7 @@ #include "soc/soc_caps.h" #include "aes/esp_aes.h" #include "sha/sha_core.h" +#include "esp_sha_internal.h" #include "esp_tee.h" #include "esp_tee_memory_utils.h" @@ -325,6 +326,11 @@ void _ss_esp_sha_block(esp_sha_type sha_type, const void *data_block, bool is_fi esp_sha_block(sha_type, data_block, is_first_block); } +void _ss_esp_sha_enable_periph_clk(bool enable) +{ + esp_sha_enable_periph_clk(enable); +} + /* ---------------------------------------------- OTA ------------------------------------------------- */ int _ss_esp_tee_ota_begin(void) diff --git a/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_apm_prot_cfg.c b/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_apm_prot_cfg.c index 3c027c0693..da17b38773 100644 --- a/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_apm_prot_cfg.c +++ b/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_apm_prot_cfg.c @@ -11,6 +11,7 @@ #include "soc/soc.h" #include "soc/spi_mem_reg.h" #include "soc/efuse_reg.h" +#include "soc/pcr_reg.h" extern void tee_apm_violation_isr(void *arg); @@ -91,34 +92,41 @@ apm_ctrl_region_config_data_t hp_apm_pms_data[] = { .regn_pms = 0x6, .filter_enable = 1, }, - /* Region 5: Peripherals [RSA - TEE Controller & APM] (RW) */ - /* Protected: APM, TEE Controller */ + /* Region 5/6: Peripherals [RSA - TEE Controller & APM] (RW) */ + /* Protected: AES + SHA PCR, APM, TEE Controller */ { .regn_num = 5, .regn_start_addr = DR_REG_RSA_BASE, + .regn_end_addr = (PCR_AES_CONF_REG - 0x4), + .regn_pms = 0x6, + .filter_enable = 1, + }, + { + .regn_num = 6, + .regn_start_addr = PCR_RSA_CONF_REG, .regn_end_addr = (DR_REG_TEE_BASE - 0x4), .regn_pms = 0x6, .filter_enable = 1, }, - /* Region 6: Peripherals [Miscellaneous - PMU] (RW) */ + /* Region 7: Peripherals [Miscellaneous - PMU] (RW) */ { - .regn_num = 6, + .regn_num = 7, .regn_start_addr = DR_REG_MISC_BASE, .regn_end_addr = (DR_REG_PMU_BASE - 0x04), .regn_pms = 0x6, .filter_enable = 1, }, - /* Region 7: Peripherals [DEBUG - PWDET] (RW) */ + /* Region 8: Peripherals [DEBUG - PWDET] (RW) */ { - .regn_num = 7, + .regn_num = 8, .regn_start_addr = DR_REG_OPT_DEBUG_BASE, .regn_end_addr = 0x600D0000, .regn_pms = 0x6, .filter_enable = 1, }, - /* Region 8: REE SRAM region (RW) */ + /* Region 9: REE SRAM region (RW) */ { - .regn_num = 8, + .regn_num = 9, .regn_start_addr = SOC_NS_IRAM_START, .regn_end_addr = SOC_IRAM_HIGH, .regn_pms = 0x6, @@ -164,9 +172,9 @@ apm_ctrl_secure_mode_config_t hp_apm_sec_mode_data = { /* HP_APM: TEE mode accessible regions */ apm_ctrl_region_config_data_t hp_apm_pms_data_tee[] = { - /* Region 9: Entire memory region (RWX)*/ + /* Region 10: Entire memory region (RWX)*/ { - .regn_num = 9, + .regn_num = 10, .regn_start_addr = 0x0, .regn_end_addr = ~0x0, .regn_pms = 0x7, @@ -303,6 +311,9 @@ void esp_tee_configure_apm_protection(void) /* Disable all control filter first to have full access of address rage. */ apm_hal_apm_ctrl_filter_enable_all(false); + /* Switch HP_CPU to TEE mode */ + apm_tee_hal_set_master_secure_mode(HP_APM_CTRL, APM_LL_MASTER_HPCORE, APM_LL_SECURE_MODE_TEE); + /* LP APM0 configuration. */ lp_apm0_sec_mode_data.regn_count = sizeof(lp_apm0_pms_data) / sizeof(apm_ctrl_region_config_data_t); apm_hal_apm_ctrl_master_sec_mode_config(&lp_apm0_sec_mode_data); diff --git a/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c b/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c index 9b558054cc..f02e17a48d 100644 --- a/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c +++ b/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -12,6 +12,7 @@ #include "esp_cpu.h" #include "esp_log.h" #include "hal/apm_hal.h" +#include "hal/clk_gate_ll.h" #include "esp_tee.h" #include "esp_tee_intr.h" @@ -91,6 +92,10 @@ void esp_tee_soc_secure_sys_init(void) esp_tee_protect_intr_src(ETS_EFUSE_INTR_SOURCE); // eFuse esp_tee_protect_intr_src(ETS_AES_INTR_SOURCE); // AES esp_tee_protect_intr_src(ETS_SHA_INTR_SOURCE); // SHA + + /* Disable AES/SHA peripheral clocks; they will be toggled as needed when the peripheral is in use */ + periph_ll_disable_clk_set_rst(PERIPH_AES_MODULE); + periph_ll_disable_clk_set_rst(PERIPH_SHA_MODULE); } IRAM_ATTR inline void esp_tee_switch_to_ree(uint32_t ree_entry_addr) diff --git a/components/mbedtls/port/sha/core/include/esp_sha_internal.h b/components/mbedtls/port/sha/core/include/esp_sha_internal.h index 55e667774e..76fcf5f7db 100644 --- a/components/mbedtls/port/sha/core/include/esp_sha_internal.h +++ b/components/mbedtls/port/sha/core/include/esp_sha_internal.h @@ -56,6 +56,13 @@ static inline esp_sha_mode sha_operation_mode(size_t length) return SHA_BLOCK_MODE; } +/** + * @brief Enable or disable the SHA peripheral clock + * + * @param enable true to enable, false to disable + */ +void esp_sha_enable_periph_clk(bool enable); + #ifdef __cplusplus } #endif diff --git a/components/mbedtls/port/sha/core/sha.c b/components/mbedtls/port/sha/core/sha.c index 84e11396af..fe3731196a 100644 --- a/components/mbedtls/port/sha/core/sha.c +++ b/components/mbedtls/port/sha/core/sha.c @@ -16,6 +16,7 @@ #include "esp_private/esp_crypto_lock_internal.h" #include "esp_log.h" #include "sha/sha_core.h" +#include "esp_sha_internal.h" #include "hal/sha_hal.h" #include "hal/sha_ll.h" #include "soc/soc_caps.h" @@ -51,6 +52,15 @@ #endif #endif /* SOC_SHA_SUPPORT_DMA */ +#if !ESP_TEE_BUILD +#define SHA_LOCK() esp_crypto_sha_aes_lock_acquire() +#define SHA_RELEASE() esp_crypto_sha_aes_lock_release() +#else +#define SHA_RCC_ATOMIC() +#define SHA_LOCK() +#define SHA_RELEASE() +#endif + void esp_sha_write_digest_state(esp_sha_type sha_type, void *digest_state) { sha_hal_write_digest(sha_type, digest_state); @@ -89,34 +99,16 @@ inline static size_t block_length(esp_sha_type type) /* Enable SHA peripheral and then lock it */ void esp_sha_acquire_hardware(void) { -#if !ESP_TEE_BUILD /* Released when releasing hw with esp_sha_release_hardware() */ - esp_crypto_sha_aes_lock_acquire(); -#endif - - SHA_RCC_ATOMIC() { - sha_ll_enable_bus_clock(true); - sha_ll_reset_register(); -#if SOC_AES_CRYPTO_DMA - crypto_dma_ll_enable_bus_clock(true); - crypto_dma_ll_reset_register(); -#endif - } + SHA_LOCK(); + esp_sha_enable_periph_clk(true); } /* Disable SHA peripheral block and then release it */ void esp_sha_release_hardware(void) { - SHA_RCC_ATOMIC() { - sha_ll_enable_bus_clock(false); -#if SOC_AES_CRYPTO_DMA - crypto_dma_ll_enable_bus_clock(false); -#endif - } - -#if !ESP_TEE_BUILD - esp_crypto_sha_aes_lock_release(); -#endif + esp_sha_enable_periph_clk(false); + SHA_RELEASE(); } void esp_sha_block(esp_sha_type sha_type, const void *data_block, bool is_first_block) diff --git a/components/mbedtls/port/sha/esp_sha.c b/components/mbedtls/port/sha/esp_sha.c index e0882987d5..30185ff620 100644 --- a/components/mbedtls/port/sha/esp_sha.c +++ b/components/mbedtls/port/sha/esp_sha.c @@ -7,6 +7,7 @@ #include #include #include +#include "hal/sha_ll.h" #include "hal/sha_hal.h" #include "hal/sha_types.h" #include "soc/soc_caps.h" @@ -20,10 +21,33 @@ #include "sha/sha_parallel_engine.h" #else #include "sha/sha_core.h" +#include "esp_sha_internal.h" +#include "esp_private/esp_crypto_lock_internal.h" +#if SOC_SHA_CRYPTO_DMA +#include "hal/crypto_dma_ll.h" +#endif #endif static const char *TAG = "esp_sha"; +#if !SOC_SHA_SUPPORT_PARALLEL_ENG +void esp_sha_enable_periph_clk(bool enable) +{ + SHA_RCC_ATOMIC() { + sha_ll_enable_bus_clock(enable); + if (enable) { + sha_ll_reset_register(); + } +#if SOC_SHA_CRYPTO_DMA + crypto_dma_ll_enable_bus_clock(enable); + if (enable) { + crypto_dma_ll_reset_register(); + } +#endif + } +} +#endif + void esp_sha(esp_sha_type sha_type, const unsigned char *input, size_t ilen, unsigned char *output) { union { From 5c4a527750294a38515c7d612fb62528a7ba3e63 Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Thu, 6 Feb 2025 17:32:32 +0530 Subject: [PATCH 2/3] refactor(esp_tee): Remove explicit setting of the `HP_CPU` APM/TEE security mode --- .../main/arch/riscv/esp_tee_secure_entry.S | 11 +------- .../main/arch/riscv/esp_tee_vectors.S | 28 +++++++++---------- .../main/soc/esp32c6/esp_tee_secure_sys_cfg.c | 3 -- 3 files changed, 15 insertions(+), 27 deletions(-) diff --git a/components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S b/components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S index dc5487d48d..333952a247 100644 --- a/components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S +++ b/components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -15,21 +15,12 @@ .global _sec_world_entry .type _sec_world_entry, @function _sec_world_entry: - /* Setup the APM for HP CPU in TEE mode */ - li t0, TEE_M0_MODE_CTRL_REG - sw zero, 0(t0) /* APM_LL_SECURE_MODE_TEE = 0 */ - /* Disable the U-mode delegation of all interrupts */ csrwi mideleg, 0 /* Jump to the secure service dispatcher */ jal esp_tee_service_dispatcher - /* Setup the APM for HP CPU in REE mode */ - li t0, TEE_M0_MODE_CTRL_REG - li t1, 0x1 /* APM_LL_SECURE_MODE_REE = 1 */ - sw t1, 0(t0) - /* Enable the U-mode delegation of all interrupts (except the TEE secure interrupt) */ li t0, 0xffffbfff csrw mideleg, t0 diff --git a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S index aded4c17a5..92a3a80938 100644 --- a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S +++ b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -177,15 +177,16 @@ _panic_handler: addi sp, sp, -16 sw t0, 0(sp) - /* Check whether the exception is an M-mode ecall */ + /* Read mcause */ csrr t0, mcause - xori t0, t0, ECALL_M_MODE - beqz t0, _machine_ecall + + /* Check whether the exception is an M-mode ecall */ + li t1, ECALL_M_MODE + beq t0, t1, _machine_ecall /* Check whether the exception is an U-mode ecall */ - csrr t0, mcause - xori t0, t0, ECALL_U_MODE - beqz t0, _user_ecall + li t1, ECALL_U_MODE + beq t0, t1, _user_ecall /* Restore t0 from the stack */ lw t0, 0(sp) @@ -250,6 +251,10 @@ _return_from_exception: _ecall_handler: /* M-mode ecall handler */ _machine_ecall: + /* Set the privilege mode to transition to after mret to U-mode */ + li t0, MSTATUS_MPP + csrc mstatus, t0 + /* Check whether this is the first M-mode ecall (see esp_tee_init) and skip context restoration */ lui t0, ESP_TEE_M2U_SWITCH_MAGIC beq a1, t0, _skip_ctx_restore @@ -267,15 +272,10 @@ _machine_ecall: restore_general_regs RV_STK_FRMSZ csrrw a0, mscratch, zero - /* This point is reached only after the first M-mode ecall, never again (see esp_tee_init) */ _skip_ctx_restore: /* Copy the ra register to mepc which contains the user app entry point (i.e. call_start_cpu0) */ csrw mepc, ra - /* Set the privilege mode to transition to after mret to U-mode */ - li t3, MSTATUS_MPP - csrc mstatus, t3 - /* Jump to the REE */ mret @@ -291,8 +291,8 @@ _user_ecall: lw t0, 0(sp) addi sp, sp, 16 - /* This point is reached after a secure service call is issued from the REE */ - /* Save register context and the mepc */ + /* This point is reached when a service call is issued from the REE */ + /* Save register context and mepc */ save_general_regs RV_STK_FRMSZ save_mepc diff --git a/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c b/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c index f02e17a48d..24f3b3c085 100644 --- a/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c +++ b/components/esp_tee/subproject/main/soc/esp32c6/esp_tee_secure_sys_cfg.c @@ -100,9 +100,6 @@ void esp_tee_soc_secure_sys_init(void) IRAM_ATTR inline void esp_tee_switch_to_ree(uint32_t ree_entry_addr) { - /* Switch HP_CPU to REE0 mode. */ - apm_tee_hal_set_master_secure_mode(HP_APM_CTRL, APM_LL_MASTER_HPCORE, APM_LL_SECURE_MODE_REE0); - /* 2nd argument is used as magic value to detect very first M2U switch */ /* TBD: clean this up and use proper temporary register instead of a1 */ /* Switch to non-secure world and launch App. */ From 873409da6b141f1876bcb22c4592ecd9840258cc Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Fri, 7 Feb 2025 16:59:14 +0530 Subject: [PATCH 3/3] refactor(esp_tee): Simplify service call ASM routine - Remove `mret` for jumping to the service call dispatcher; instead, enable interrupts and execute directly - Fix potential corruption of the `t3` register when returning from a service call - Simplify the secure service dispatcher function --- components/esp_tee/include/esp_tee.h | 5 +- components/esp_tee/src/esp_tee_config.c | 8 +- .../esp_tee/subproject/main/CMakeLists.txt | 7 +- .../main/arch/riscv/esp_tee_secure_entry.S | 33 ------- .../main/arch/riscv/esp_tee_vectors.S | 54 +++++++----- .../main/core/esp_secure_dispatcher.c | 85 +++++++------------ .../subproject/main/core/esp_tee_init.c | 1 - .../test_sec_srv/include/esp_tee_test.h | 13 +-- .../test_sec_srv/sec_srv_tbl_test.yml | 2 +- .../test_sec_srv/src/test_dummy_srv.c | 4 +- .../test_sec_srv/src/test_dummy_srv_wrapper.c | 4 +- .../test_sec_srv/src/test_sec_srv.c | 12 +-- .../main/test_esp_tee_ctx_switch.c | 6 +- 13 files changed, 96 insertions(+), 138 deletions(-) delete mode 100644 components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S diff --git a/components/esp_tee/include/esp_tee.h b/components/esp_tee/include/esp_tee.h index c22d887b49..a529928897 100644 --- a/components/esp_tee/include/esp_tee.h +++ b/components/esp_tee/include/esp_tee.h @@ -43,9 +43,8 @@ typedef struct { uint32_t magic_word; uint32_t api_major_version; uint32_t api_minor_version; - uint32_t reserved[2]; + uint32_t reserved[3]; /* TEE-related fields */ - void *s_entry_addr; void *s_int_handler; /* REE-related fields */ void *ns_entry_addr; @@ -85,14 +84,12 @@ uint32_t esp_tee_service_call_with_noniram_intr_disabled(int argc, ...); #if !(__DOXYGEN__) /* Offsets of some values in esp_tee_config_t that are used by assembly code */ -#define ESP_TEE_CFG_OFFS_S_ENTRY_ADDR 0x14 #define ESP_TEE_CFG_OFFS_S_INTR_HANDLER 0x18 #define ESP_TEE_CFG_OFFS_NS_ENTRY_ADDR 0x1C #define ESP_TEE_CFG_OFFS_NS_INTR_HANDLER 0x20 #if !defined(__ASSEMBLER__) /* Check the offsets are correct using the C compiler */ -ESP_STATIC_ASSERT(offsetof(esp_tee_config_t, s_entry_addr) == ESP_TEE_CFG_OFFS_S_ENTRY_ADDR, "offset macro is wrong"); ESP_STATIC_ASSERT(offsetof(esp_tee_config_t, s_int_handler) == ESP_TEE_CFG_OFFS_S_INTR_HANDLER, "offset macro is wrong"); ESP_STATIC_ASSERT(offsetof(esp_tee_config_t, ns_entry_addr) == ESP_TEE_CFG_OFFS_NS_ENTRY_ADDR, "offset macro is wrong"); ESP_STATIC_ASSERT(offsetof(esp_tee_config_t, ns_int_handler) == ESP_TEE_CFG_OFFS_NS_INTR_HANDLER, "offset macro is wrong"); diff --git a/components/esp_tee/src/esp_tee_config.c b/components/esp_tee/src/esp_tee_config.c index eec0210f9e..62f5051b31 100644 --- a/components/esp_tee/src/esp_tee_config.c +++ b/components/esp_tee/src/esp_tee_config.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -25,9 +25,9 @@ esp_tee_config_t esp_tee_app_config __attribute__((section(".esp_tee_app_cfg"))) .api_major_version = ESP_TEE_API_MAJOR_VER, .api_minor_version = ESP_TEE_API_MINOR_VER, - /* .s_entry_addr and .s_intr_handler are NULL in the - app binary, but will be written by the TEE before it loads the binary - */ + /* s_intr_handler is NULL in the REE image, but will be written by + * the TEE before it loads the binary + */ .ns_int_handler = &_tee_interrupt_handler, .ns_entry_addr = &_u2m_switch, diff --git a/components/esp_tee/subproject/main/CMakeLists.txt b/components/esp_tee/subproject/main/CMakeLists.txt index 4be567ca54..9dfeb7e527 100644 --- a/components/esp_tee/subproject/main/CMakeLists.txt +++ b/components/esp_tee/subproject/main/CMakeLists.txt @@ -20,8 +20,7 @@ set(srcs "core/esp_tee_init.c" # Arch specific implementation for TEE list(APPEND srcs "arch/${arch}/esp_tee_vectors.S" - "arch/${arch}/esp_tee_vector_table.S" - "arch/${arch}/esp_tee_secure_entry.S") + "arch/${arch}/esp_tee_vector_table.S") # SoC specific implementation for TEE list(APPEND srcs "soc/${target}/esp_tee_secure_sys_cfg.c" @@ -78,7 +77,9 @@ list(APPEND srcs "common/esp_app_desc_tee.c") idf_component_register(SRCS ${srcs} INCLUDE_DIRS ${include}) -set_source_files_properties("core/esp_secure_services.c" PROPERTIES COMPILE_FLAGS -Wno-deprecated) +# TODO: Currently only -Og optimization level works correctly at runtime +set_source_files_properties("core/esp_secure_dispatcher.c" PROPERTIES COMPILE_FLAGS "-Og") + include(${CMAKE_CURRENT_LIST_DIR}/ld/esp_tee_ld.cmake) # esp_app_desc_t configuration structure for TEE: Linking symbol and trimming project version and name diff --git a/components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S b/components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S deleted file mode 100644 index 333952a247..0000000000 --- a/components/esp_tee/subproject/main/arch/riscv/esp_tee_secure_entry.S +++ /dev/null @@ -1,33 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD - * - * SPDX-License-Identifier: Apache-2.0 - */ -#include "soc/tee_reg.h" -#include "soc/plic_reg.h" - - .global esp_tee_service_dispatcher - -/* Entry point to the secure world (i.e. M-mode) - responsible for - * setting up the execution environment for the secure world */ - .section .text - .align 4 - .global _sec_world_entry - .type _sec_world_entry, @function -_sec_world_entry: - /* Disable the U-mode delegation of all interrupts */ - csrwi mideleg, 0 - - /* Jump to the secure service dispatcher */ - jal esp_tee_service_dispatcher - - /* Enable the U-mode delegation of all interrupts (except the TEE secure interrupt) */ - li t0, 0xffffbfff - csrw mideleg, t0 - - /* Fire an M-ecall */ - mv a1, zero - ecall - fence - - ret diff --git a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S index 92a3a80938..2028bec395 100644 --- a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S +++ b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors.S @@ -12,6 +12,7 @@ #include "riscv/encoding.h" #include "riscv/rvruntime-frames.h" +#include "esp_private/vectors_const.h" #include "esp_tee.h" #include "sdkconfig.h" @@ -25,9 +26,12 @@ .equ ECALL_U_MODE, 0x8 .equ ECALL_M_MODE, 0xb .equ TEE_APM_INTR_MASK_0, 0x00300000 - .equ TEE_APM_INTR_MASK_1, 0x000000F8 + .equ TEE_APM_INTR_MASK_1, 0x000000f8 + .equ TEE_INTR_DELEG_MASK, 0xffffbfff .global esp_tee_global_interrupt_handler + .global esp_tee_service_dispatcher + .section .data .align 4 @@ -179,6 +183,8 @@ _panic_handler: /* Read mcause */ csrr t0, mcause + li t1, VECTORS_MCAUSE_INTBIT_MASK | VECTORS_MCAUSE_REASON_MASK + and t0, t0, t1 /* Check whether the exception is an M-mode ecall */ li t1, ECALL_M_MODE @@ -291,28 +297,34 @@ _user_ecall: lw t0, 0(sp) addi sp, sp, 16 - /* This point is reached when a service call is issued from the REE */ + /* This point is reached when a secure service call is issued from the REE */ /* Save register context and mepc */ save_general_regs RV_STK_FRMSZ save_mepc - /* Saving the U-mode (i.e. REE) stack pointer */ + /* Save the U-mode (i.e. REE) stack pointer */ la t0, _ns_sp sw sp, 0(t0) - /* Switching to the M-mode (i.e. TEE) stack */ + /* Switch to the M-mode (i.e. TEE) stack */ la sp, _tee_stack - /* Load the TEE entry point (see sec_world_entry) in the mepc */ - la t2, esp_tee_app_config - lw t2, ESP_TEE_CFG_OFFS_S_ENTRY_ADDR(t2) - csrw mepc, t2 + /* Disable the U-mode delegation of all interrupts */ + csrwi mideleg, 0 - /* Set the privilege mode to transition to after mret to M-mode */ - li t3, MSTATUS_MPP - csrs mstatus, t3 + /* Enable interrupts */ + csrsi mstatus, MSTATUS_MIE - mret + /* Jump to the secure service dispatcher */ + jal esp_tee_service_dispatcher + + /* Enable the U-mode delegation of all interrupts (except the TEE secure interrupt) */ + li t0, TEE_INTR_DELEG_MASK + csrs mideleg, t0 + + /* Fire an M-ecall */ + mv a1, zero + ecall /* This point is reached after servicing a U-mode interrupt occurred * while executing a secure service */ @@ -333,7 +345,7 @@ _rtn_from_ns_int: /* Restore register context and resume the secure service */ restore_mepc - restore_general_regs + restore_general_regs RV_STK_FRMSZ mret @@ -347,7 +359,7 @@ _rtn_from_ns_int: _tee_ns_intr_handler: /* Start by saving the general purpose registers and the PC value before * the interrupt happened. */ - save_general_regs + save_general_regs RV_STK_FRMSZ save_mepc /* Though it is not necessary we save GP and SP here. @@ -357,7 +369,7 @@ _tee_ns_intr_handler: /* As gp register is not saved by the macro, save it here */ sw gp, RV_STK_GP(sp) /* Same goes for the SP value before trapping */ - addi t0, sp, CONTEXT_SIZE /* restore sp with the value when interrupt happened */ + addi t0, sp, RV_STK_FRMSZ /* restore sp with the value when interrupt happened */ /* Save SP */ sw t0, RV_STK_SP(sp) @@ -395,8 +407,8 @@ _tee_ns_intr_handler: csrw mscratch, t0 /* Enable the U-mode interrupt delegation (except for the TEE secure interrupt) */ - li t0, 0xffffbfff - csrw mideleg, t0 + li t0, TEE_INTR_DELEG_MASK + csrs mideleg, t0 /* Place magic bytes in all the general registers */ store_magic_general_regs @@ -413,7 +425,7 @@ _tee_ns_intr_handler: _tee_s_intr_handler: /* Start by saving the general purpose registers and the PC value before * the interrupt happened. */ - save_general_regs + save_general_regs RV_STK_FRMSZ save_mepc /* Though it is not necessary we save GP and SP here. @@ -423,7 +435,7 @@ _tee_s_intr_handler: /* As gp register is not saved by the macro, save it here */ sw gp, RV_STK_GP(sp) /* Same goes for the SP value before trapping */ - addi t0, sp, CONTEXT_SIZE /* restore sp with the value when interrupt happened */ + addi t0, sp, RV_STK_FRMSZ /* restore sp with the value when interrupt happened */ /* Save SP */ sw t0, RV_STK_SP(sp) @@ -457,7 +469,7 @@ _save_reg_ctx: _continue: /* Before doing anything preserve the stack pointer */ mv s11, sp - /* Switching to the TEE interrupt stack */ + /* Switch to the TEE interrupt stack */ la sp, _tee_intr_stack /* If this is a non-nested interrupt, SP now points to the interrupt stack */ @@ -527,7 +539,7 @@ _intr_hdlr_exec: mv sp, s11 restore_mepc - restore_general_regs + restore_general_regs RV_STK_FRMSZ /* exit, this will also re-enable the interrupts */ mret diff --git a/components/esp_tee/subproject/main/core/esp_secure_dispatcher.c b/components/esp_tee/subproject/main/core/esp_secure_dispatcher.c index 323c715164..2b77e1fa7f 100644 --- a/components/esp_tee/subproject/main/core/esp_secure_dispatcher.c +++ b/components/esp_tee/subproject/main/core/esp_secure_dispatcher.c @@ -3,7 +3,7 @@ * * SPDX-License-Identifier: Apache-2.0 */ - +#include #include #include "esp_log.h" #include "esp_tee.h" @@ -34,13 +34,10 @@ static const secure_service_entry_t *find_service_by_id(uint32_t id) } /** - * @brief Entry point to the TEE binary during secure service call. It decipher the call and dispatch it - * to corresponding Secure Service API in secure world. - * TODO: Fix the assembly routine here for compatibility with all levels of compiler optimizations + * @brief Handles incoming secure service requests to the TEE. + * Validates and routes each request to the appropriate + * secure service implementation. */ -#pragma GCC push_options -#pragma GCC optimize ("Og") - int esp_tee_service_dispatcher(int argc, va_list ap) { if (argc > ESP_TEE_MAX_INPUT_ARG) { @@ -50,7 +47,7 @@ int esp_tee_service_dispatcher(int argc, va_list ap) } int ret = -1; - uint32_t argv[ESP_TEE_MAX_INPUT_ARG], *argp; + uint32_t argv[ESP_TEE_MAX_INPUT_ARG] = {0}; uint32_t sid = va_arg(ap, uint32_t); argc--; @@ -58,13 +55,11 @@ int esp_tee_service_dispatcher(int argc, va_list ap) const secure_service_entry_t *service = find_service_by_id(sid); if (service == NULL) { ESP_LOGE(TAG, "Invalid service ID!"); - va_end(ap); return ret; } if (argc != service->nargs) { ESP_LOGE(TAG, "Invalid number of arguments for service %d!", sid); - va_end(ap); return ret; } @@ -73,65 +68,47 @@ int esp_tee_service_dispatcher(int argc, va_list ap) for (int i = 0; i < argc; i++) { argv[i] = va_arg(ap, uint32_t); } - argp = &argv[0]; - va_end(ap); + uint32_t *argp = &argv[0]; asm volatile( - "mv t0, %1 \n" - "beqz t0, service_call \n" + "mv t0, %1 \n" // t0 = argc + "mv t1, %3 \n" // t1 = argp + "li t2, 8 \n" // t2 = 8 (max register args) + "ble t0, t2, load_regs \n" // If argc <= 8 (a0-a7), skip stack routine + + // Store extra args (argc > 8) on stack + "mv t3, sp \n" + "addi t1, t1, 32 \n" + + "stack_loop: \n" + "lw t4, 0(t1) \n" + "sw t4, 0(t3) \n" + "addi t1, t1, 4 \n" + "addi t3, t3, 4 \n" + "addi t0, t0, -1 \n" + "bge t0, t2, stack_loop \n" + + // Load the first 8 arguments into a0-a7 + "load_regs: \n" "lw a0, 0(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - "lw a1, 4(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - "lw a2, 8(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - "lw a3, 12(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - "lw a4, 16(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - "lw a5, 20(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - "lw a6, 24(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - "lw a7, 28(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" + "fence \n" - "addi %3, %3, 32 \n" - "mv t2, sp \n" - "loop: \n" - "lw t1, 0(%3) \n" - "sw t1, 0(t2) \n" - "addi t0, t0, -1 \n" - "addi t2, t2, 4 \n" - "addi %3, %3, 4 \n" - "bnez t0, loop \n" - - "service_call: \n" - "mv t1, %2 \n" - "jalr 0(t1) \n" - "mv %0, a0 \n" + "mv t1, %2 \n" // Load function pointer + "jalr 0(t1) \n" // Call function + "mv %0, a0 \n" // Store return value : "=r"(ret) : "r"(argc), "r"(fp_secure_service), "r"(argp) - : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", "t0", "t1", "t2" + : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", + "t0", "t1", "t2", "t3", "t4" ); return ret; } - -#pragma GCC pop_options diff --git a/components/esp_tee/subproject/main/core/esp_tee_init.c b/components/esp_tee/subproject/main/core/esp_tee_init.c index 6976aa561b..995f282769 100644 --- a/components/esp_tee/subproject/main/core/esp_tee_init.c +++ b/components/esp_tee/subproject/main/core/esp_tee_init.c @@ -59,7 +59,6 @@ static void tee_init_app_config(void) esp_tee_app_config.api_minor_version = ESP_TEE_API_MINOR_VER; /* Set the TEE-related fields (from the TEE binary) that the REE will use to interface with TEE */ - esp_tee_app_config.s_entry_addr = &_sec_world_entry; esp_tee_app_config.s_int_handler = &_tee_s_intr_handler; } diff --git a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/include/esp_tee_test.h b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/include/esp_tee_test.h index 0830bfb8be..705383e5fc 100644 --- a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/include/esp_tee_test.h +++ b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/include/esp_tee_test.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -10,16 +10,17 @@ extern "C" { #endif #include +#include "esp_attr.h" #define TEE_TEST_INT_COUNT 3 -uint32_t __attribute__((__noinline__)) esp_tee_service_add(uint32_t a, uint32_t b); +uint32_t NOINLINE_ATTR esp_tee_service_add(uint32_t a, uint32_t b); -uint32_t __attribute__((__noinline__)) esp_tee_service_sub(uint32_t a, uint32_t b); +uint32_t NOINLINE_ATTR esp_tee_service_sub(uint32_t a, uint32_t b); -uint32_t __attribute__((__noinline__)) esp_tee_service_mul(uint32_t a, uint32_t b); +uint32_t NOINLINE_ATTR esp_tee_service_mul(uint32_t a, uint32_t b); -uint32_t __attribute__((__noinline__)) esp_tee_service_div(uint32_t a, uint32_t b); +uint32_t NOINLINE_ATTR esp_tee_service_div(uint32_t a, uint32_t b); int esp_tee_secure_int_test(void); @@ -33,7 +34,7 @@ int esp_tee_test_illegal_instr(void); int esp_tee_test_instr_fetch_prohibited(uint32_t type); -void dummy_secure_service(void); +void NOINLINE_ATTR dummy_secure_service(int a, int b, int c, int d, int e, int f, int g, int h, int *i); uint32_t add_in_loop(uint32_t a, uint32_t b, uint32_t iter); diff --git a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/sec_srv_tbl_test.yml b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/sec_srv_tbl_test.yml index 76ba15c605..5945326e7a 100644 --- a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/sec_srv_tbl_test.yml +++ b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/sec_srv_tbl_test.yml @@ -64,7 +64,7 @@ secure_services: - id: 215 type: custom function: dummy_secure_service - args: 0 + args: 9 - id: 216 type: custom function: add_in_loop diff --git a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv.c b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv.c index 24faeb5e41..7562516a77 100644 --- a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv.c +++ b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv.c @@ -7,8 +7,10 @@ #include "esp_tee.h" #include "esp_err.h" #include "esp_rom_sys.h" +#include "esp_attr.h" -void _ss_dummy_secure_service(void) +void NOINLINE_ATTR _ss_dummy_secure_service(int a, int b, int c, int d, int e, int f, int g, int h, int *i) { esp_rom_printf("Dummy secure service\n"); + *i = a + b + c + d + e + f + g + h; } diff --git a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv_wrapper.c b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv_wrapper.c index ae40a94193..d31b968ce7 100644 --- a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv_wrapper.c +++ b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_dummy_srv_wrapper.c @@ -8,7 +8,7 @@ #include "esp_tee.h" #include "esp_err.h" -void dummy_secure_service(void) +void dummy_secure_service(int a, int b, int c, int d, int e, int f, int g, int h, int *i) { - esp_tee_service_call(1, SS_DUMMY_SECURE_SERVICE); + esp_tee_service_call(10, SS_DUMMY_SECURE_SERVICE, a, b, c, d, e, f, g, h, i); } diff --git a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_sec_srv.c b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_sec_srv.c index 8cf63bf4e1..4fc14500cf 100644 --- a/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_sec_srv.c +++ b/components/esp_tee/test_apps/tee_test_fw/components/test_sec_srv/src/test_sec_srv.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,30 +7,30 @@ #include "esp_log.h" #include "esp_tee.h" #include "esp_tee_test.h" - +#include "esp_attr.h" static const char *TAG = "test_sec_srv"; /* Sample Trusted App */ -uint32_t __attribute__((__noinline__)) _ss_esp_tee_service_add(uint32_t a, uint32_t b) +uint32_t NOINLINE_ATTR _ss_esp_tee_service_add(uint32_t a, uint32_t b) { ESP_LOGD(TAG, "SS: %s", __func__); return (a + b); } -uint32_t __attribute__((__noinline__)) _ss_esp_tee_service_sub(uint32_t a, uint32_t b) +uint32_t NOINLINE_ATTR _ss_esp_tee_service_sub(uint32_t a, uint32_t b) { ESP_LOGD(TAG, "SS: %s", __func__); return (a - b); } -uint32_t __attribute__((__noinline__)) _ss_esp_tee_service_mul(uint32_t a, uint32_t b) +uint32_t NOINLINE_ATTR _ss_esp_tee_service_mul(uint32_t a, uint32_t b) { ESP_LOGD(TAG, "SS: %s", __func__); return (a * b); } -uint32_t __attribute__((__noinline__)) _ss_esp_tee_service_div(uint32_t a, uint32_t b) +uint32_t NOINLINE_ATTR _ss_esp_tee_service_div(uint32_t a, uint32_t b) { ESP_LOGD(TAG, "SS: %s", __func__); return (a / b); diff --git a/components/esp_tee/test_apps/tee_test_fw/main/test_esp_tee_ctx_switch.c b/components/esp_tee/test_apps/tee_test_fw/main/test_esp_tee_ctx_switch.c index 298d3ade3e..474e2c3430 100644 --- a/components/esp_tee/test_apps/tee_test_fw/main/test_esp_tee_ctx_switch.c +++ b/components/esp_tee/test_apps/tee_test_fw/main/test_esp_tee_ctx_switch.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -68,7 +68,9 @@ TEST_CASE("Test multiple calls to sample app (basic services)", "[basic]") TEST_CASE("Custom secure service call", "[basic]") { - dummy_secure_service(); + int res = -1; + dummy_secure_service(1, 2, 3, 4, 5, 6, 7, 8, &res); + TEST_ASSERT_EQUAL_UINT32(36, res); } void test_task(void *pvParameters)