From 1a40148be0f5aa8366ddf437931d64bafafddab3 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Sun, 16 May 2021 18:51:28 +0300 Subject: [PATCH 01/10] gcov: added dbg stub capabilites and magic number entry to keep backward compatible --- components/app_trace/gcov/gcov_rtio.c | 6 ++-- components/bt/controller/lib_esp32c3_family | 1 + .../include/esp_private/dbg_stubs.h | 29 +++++++++++++++---- components/esp_common/src/dbg_stubs.c | 12 ++++++++ components/tinyusb/tinyusb | 1 + .../components/esp-cryptoauthlib | 1 + 6 files changed, 42 insertions(+), 8 deletions(-) create mode 160000 components/bt/controller/lib_esp32c3_family create mode 160000 components/tinyusb/tinyusb create mode 160000 examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib diff --git a/components/app_trace/gcov/gcov_rtio.c b/components/app_trace/gcov/gcov_rtio.c index 72c194be4a..6508e68b0e 100644 --- a/components/app_trace/gcov/gcov_rtio.c +++ b/components/app_trace/gcov/gcov_rtio.c @@ -106,7 +106,7 @@ static int esp_dbg_stub_gcov_dump_do(void) } /** - * @brief Triggers gcov info dump. + * @brief Triggers gcov info dump task * This function is to be called by OpenOCD, not by normal user code. * TODO: what about interrupted flash access (when cache disabled)??? * @@ -114,11 +114,13 @@ static int esp_dbg_stub_gcov_dump_do(void) */ static int esp_dbg_stub_gcov_entry(void) { - return esp_dbg_stub_gcov_dump_do(); + s_do_dump = true; + return ESP_OK; } int gcov_rtio_atexit(void (*function)(void) __attribute__ ((unused))) { + uint32_t capabilities = 0; ESP_EARLY_LOGV(TAG, "%s", __FUNCTION__); esp_dbg_stub_entry_set(ESP_DBG_STUB_ENTRY_GCOV, (uint32_t)&esp_dbg_stub_gcov_entry); return 0; diff --git a/components/bt/controller/lib_esp32c3_family b/components/bt/controller/lib_esp32c3_family new file mode 160000 index 0000000000..32f15e826a --- /dev/null +++ b/components/bt/controller/lib_esp32c3_family @@ -0,0 +1 @@ +Subproject commit 32f15e826a102d2d64e612620468122ea2234a2e diff --git a/components/esp_common/include/esp_private/dbg_stubs.h b/components/esp_common/include/esp_private/dbg_stubs.h index 899dfa56ed..454c3418f9 100644 --- a/components/esp_common/include/esp_private/dbg_stubs.h +++ b/components/esp_common/include/esp_private/dbg_stubs.h @@ -20,13 +20,18 @@ * Debug stubs entries IDs */ typedef enum { - ESP_DBG_STUB_CONTROL_DATA, ///< stubs descriptor entry + ESP_DBG_STUB_MAGIC_NUM, + ESP_DBG_STUB_CONTROL_DATA, ///< stubs descriptor entry ESP_DBG_STUB_ENTRY_FIRST, - ESP_DBG_STUB_ENTRY_GCOV ///< GCOV entry - = ESP_DBG_STUB_ENTRY_FIRST, + ESP_DBG_STUB_ENTRY_GCOV ///< GCOV entry + = ESP_DBG_STUB_ENTRY_FIRST, + ESP_DBG_STUB_CAPABILITIES, ESP_DBG_STUB_ENTRY_MAX } esp_dbg_stub_id_t; +#define ESP_DBG_STUB_MAGIC_NUM_VAL 0xFEEDBEEF +#define ESP_DBG_STUB_CAP_GCOV_TASK (1 << 0) + /** * @brief Initializes debug stubs. * @@ -41,10 +46,22 @@ void esp_dbg_stubs_init(void); * * @param id Stub ID. * @param entry Stub entry. Usually it is stub entry function address, - * but can be any value meaningfull for OpenOCD command/code. - * + * but can be any value meaningfull for OpenOCD command/code + * such as capabilities * @return ESP_OK on success, otherwise see esp_err_t */ esp_err_t esp_dbg_stub_entry_set(esp_dbg_stub_id_t id, uint32_t entry); -#endif //ESP_DBG_STUBS_H_ \ No newline at end of file +/** + * @brief Retrives the corresponding stub entry + * + * @param id Stub ID. + * @param entry Stub entry. Usually it is stub entry function address, + * but can be any value meaningfull for OpenOCD command/code + * such as capabilities + * + * @return ESP_OK on success, otherwise see esp_err_t + */ +esp_err_t esp_dbg_stub_entry_get(esp_dbg_stub_id_t id, uint32_t *entry); + +#endif //ESP_DBG_STUBS_H_ diff --git a/components/esp_common/src/dbg_stubs.c b/components/esp_common/src/dbg_stubs.c index 1ee9124897..e4c65c22e2 100644 --- a/components/esp_common/src/dbg_stubs.c +++ b/components/esp_common/src/dbg_stubs.c @@ -76,6 +76,7 @@ void esp_dbg_stubs_init(void) s_dbg_stubs_ctl_data.data_alloc = (uint32_t)esp_dbg_stubs_data_alloc; s_dbg_stubs_ctl_data.data_free = (uint32_t)esp_dbg_stubs_data_free; + s_stub_entry[ESP_DBG_STUB_MAGIC_NUM] = ESP_DBG_STUB_MAGIC_NUM_VAL; s_stub_entry[ESP_DBG_STUB_CONTROL_DATA] = (uint32_t)&s_dbg_stubs_ctl_data; eri_write(ESP_DBG_STUBS_TRAX_REG, (uint32_t)s_stub_entry); ESP_LOGV(TAG, "%s stubs %x", __func__, eri_read(ESP_DBG_STUBS_TRAX_REG)); @@ -92,4 +93,15 @@ esp_err_t esp_dbg_stub_entry_set(esp_dbg_stub_id_t id, uint32_t entry) return ESP_OK; } +esp_err_t esp_dbg_stub_entry_get(esp_dbg_stub_id_t id, uint32_t *entry) +{ + if (id < ESP_DBG_STUB_ENTRY_FIRST || id >= ESP_DBG_STUB_ENTRY_MAX) { + ESP_LOGE(TAG, "Invalid stub id %d!", id); + return ESP_ERR_INVALID_ARG; + } + *entry = s_stub_entry[id]; + + return ESP_OK; +} + #endif diff --git a/components/tinyusb/tinyusb b/components/tinyusb/tinyusb new file mode 160000 index 0000000000..68f6ef6790 --- /dev/null +++ b/components/tinyusb/tinyusb @@ -0,0 +1 @@ +Subproject commit 68f6ef679000f9cd22b20af7473d7f0480d2862e diff --git a/examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib b/examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib new file mode 160000 index 0000000000..36d0642e66 --- /dev/null +++ b/examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib @@ -0,0 +1 @@ +Subproject commit 36d0642e66ff5b1c7a291873f24c498ca6ffedef From 357e938298dd98073ed0c4ad497b2455e71e0de7 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Fri, 23 Apr 2021 19:10:35 +0300 Subject: [PATCH 02/10] gcov: dump in a special thread --- components/app_trace/gcov/gcov_rtio.c | 118 +++++++++++--------------- 1 file changed, 49 insertions(+), 69 deletions(-) diff --git a/components/app_trace/gcov/gcov_rtio.c b/components/app_trace/gcov/gcov_rtio.c index 6508e68b0e..accee8f802 100644 --- a/components/app_trace/gcov/gcov_rtio.c +++ b/components/app_trace/gcov/gcov_rtio.c @@ -22,6 +22,7 @@ #include "soc/cpu.h" #include "soc/timer_periph.h" #include "esp_app_trace.h" +#include "esp_freertos_hooks.h" #include "esp_private/dbg_stubs.h" #include "hal/timer_ll.h" #if CONFIG_IDF_TARGET_ESP32 @@ -37,84 +38,71 @@ #define LOG_LOCAL_LEVEL CONFIG_LOG_DEFAULT_LEVEL #include "esp_log.h" const static char *TAG = "esp_gcov_rtio"; +static volatile bool s_create_gcov_task = false; +static volatile bool s_gcov_task_running = false; extern void __gcov_dump(void); extern void __gcov_reset(void); -static struct syscall_stub_table s_gcov_stub_table; - - -static int gcov_stub_lock_try_acquire_recursive(_lock_t *lock) +void gcov_dump_task(void *pvParameter) { - if (*lock && uxSemaphoreGetCount((xSemaphoreHandle)(*lock)) == 0) { - // we can do nothing here, gcov dump is initiated with some resource locked - // which is also used by gcov functions - ESP_EARLY_LOGE(TAG, "Lock 0x%x is busy during GCOV dump! System state can be inconsistent after dump!", lock); - } - return pdTRUE; -} - -static void gcov_stub_lock_acquire_recursive(_lock_t *lock) -{ - gcov_stub_lock_try_acquire_recursive(lock); -} - -static void gcov_stub_lock_release_recursive(_lock_t *lock) -{ -} - -static int esp_dbg_stub_gcov_dump_do(void) -{ - int ret = ESP_OK; - FILE* old_stderr = stderr; - FILE* old_stdout = stdout; - struct syscall_stub_table* old_table = syscall_table_ptr_pro; + int dump_result = 0; + bool *running = (bool *)pvParameter; ESP_EARLY_LOGV(TAG, "Alloc apptrace down buf %d bytes", ESP_GCOV_DOWN_BUF_SIZE); void *down_buf = malloc(ESP_GCOV_DOWN_BUF_SIZE); if (down_buf == NULL) { ESP_EARLY_LOGE(TAG, "Could not allocate memory for the buffer"); - return ESP_ERR_NO_MEM; + dump_result = ESP_ERR_NO_MEM; + goto gcov_exit; } ESP_EARLY_LOGV(TAG, "Config apptrace down buf"); esp_apptrace_down_buffer_config(down_buf, ESP_GCOV_DOWN_BUF_SIZE); + /* we are directing the std outputs to the fake ones in order to reduce stack usage */ + FILE *old_stderr = stderr; + FILE *old_stdout = stdout; + stderr = (FILE *) &__sf_fake_stderr; + stdout = (FILE *) &__sf_fake_stdout; ESP_EARLY_LOGV(TAG, "Dump data..."); - // incase of dual-core chip APP and PRO CPUs share the same table, so it is safe to save only PRO's table - memcpy(&s_gcov_stub_table, old_table, sizeof(s_gcov_stub_table)); - s_gcov_stub_table._lock_acquire_recursive = &gcov_stub_lock_acquire_recursive; - s_gcov_stub_table._lock_release_recursive = &gcov_stub_lock_release_recursive; - s_gcov_stub_table._lock_try_acquire_recursive = &gcov_stub_lock_try_acquire_recursive, - - syscall_table_ptr_pro = &s_gcov_stub_table; - stderr = (FILE*) &__sf_fake_stderr; - stdout = (FILE*) &__sf_fake_stdout; __gcov_dump(); // reset dump status to allow incremental data accumulation __gcov_reset(); - stdout = old_stdout; - stderr = old_stderr; - syscall_table_ptr_pro = old_table; - - ESP_EARLY_LOGV(TAG, "Free apptrace down buf"); free(down_buf); + stderr = old_stderr; + stdout = old_stdout; ESP_EARLY_LOGV(TAG, "Finish file transfer session"); - ret = esp_apptrace_fstop(ESP_APPTRACE_DEST_TRAX); - if (ret != ESP_OK) { - ESP_EARLY_LOGE(TAG, "Failed to send files transfer stop cmd (%d)!", ret); + dump_result = esp_apptrace_fstop(ESP_APPTRACE_DEST_TRAX); + if (dump_result != ESP_OK) { + ESP_EARLY_LOGE(TAG, "Failed to send files transfer stop cmd (%d)!", dump_result); + } + +gcov_exit: + ESP_EARLY_LOGV(TAG, "dump_result %d", dump_result); + if (running) { + *running = false; + } + vTaskDelete(NULL); +} + +static void gcov_create_task_hook(void) +{ + if (s_create_gcov_task) { + xTaskCreatePinnedToCore(&gcov_dump_task, "gcov_dump_task", 2048, &s_gcov_task_running, configMAX_PRIORITIES - 1, NULL, 0); + s_create_gcov_task = false; } - return ret; } /** * @brief Triggers gcov info dump task * This function is to be called by OpenOCD, not by normal user code. - * TODO: what about interrupted flash access (when cache disabled)??? + * TODO: what about interrupted flash access (when cache disabled) * * @return ESP_OK on success, otherwise see esp_err_t */ static int esp_dbg_stub_gcov_entry(void) { - s_do_dump = true; + /* we are in isr context here */ + s_create_gcov_task = true; return ESP_OK; } @@ -123,33 +111,25 @@ int gcov_rtio_atexit(void (*function)(void) __attribute__ ((unused))) uint32_t capabilities = 0; ESP_EARLY_LOGV(TAG, "%s", __FUNCTION__); esp_dbg_stub_entry_set(ESP_DBG_STUB_ENTRY_GCOV, (uint32_t)&esp_dbg_stub_gcov_entry); - return 0; + if (esp_dbg_stub_entry_get(ESP_DBG_STUB_CAPABILITIES, &capabilities) == ESP_OK) { + esp_dbg_stub_entry_set(ESP_DBG_STUB_CAPABILITIES, capabilities | ESP_DBG_STUB_CAP_GCOV_TASK); + } + esp_register_freertos_tick_hook(gcov_create_task_hook); + return ESP_OK; } void esp_gcov_dump(void) { - // disable IRQs on this CPU, other CPU is halted by OpenOCD - unsigned irq_state = portENTER_CRITICAL_NESTED(); -#if !CONFIG_FREERTOS_UNICORE - int other_core = xPortGetCoreID() ? 0 : 1; - esp_cpu_stall(other_core); -#endif while (!esp_apptrace_host_is_connected(ESP_APPTRACE_DEST_TRAX)) { - // to avoid complains that task watchdog got triggered for other tasks - timer_ll_wdt_set_protect(&TIMERG0, false); - timer_ll_wdt_feed(&TIMERG0); - timer_ll_wdt_set_protect(&TIMERG0, true); - // to avoid reboot on INT_WDT - timer_ll_wdt_set_protect(&TIMERG1, false); - timer_ll_wdt_feed(&TIMERG1); - timer_ll_wdt_set_protect(&TIMERG1, true); + vTaskDelay(pdMS_TO_TICKS(10)); } - esp_dbg_stub_gcov_dump_do(); -#if !CONFIG_FREERTOS_UNICORE - esp_cpu_unstall(other_core); -#endif - portEXIT_CRITICAL_NESTED(irq_state); + /* We are not in isr context here. Waiting for the completion is safe */ + s_create_gcov_task = true; + s_gcov_task_running = true; + while (s_gcov_task_running) { + vTaskDelay(pdMS_TO_TICKS(10)); + } } void *gcov_rtio_fopen(const char *path, const char *mode) @@ -166,7 +146,7 @@ int gcov_rtio_fclose(void *stream) size_t gcov_rtio_fread(void *ptr, size_t size, size_t nmemb, void *stream) { - ESP_EARLY_LOGV(TAG, "%s read %u", __FUNCTION__, size*nmemb); + ESP_EARLY_LOGV(TAG, "%s read %u", __FUNCTION__, size * nmemb); size_t sz = esp_apptrace_fread(ESP_APPTRACE_DEST_TRAX, ptr, size, nmemb, stream); ESP_EARLY_LOGV(TAG, "%s actually read %u", __FUNCTION__, sz); return sz; From 49942a94682d89972508b10ce1e837c27e6a8f61 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Thu, 3 Jun 2021 21:13:57 +0300 Subject: [PATCH 03/10] gcov: readme update for ESP32-S2 --- examples/system/gcov/README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/system/gcov/README.md b/examples/system/gcov/README.md index 5404cb96de..a09aa8bade 100644 --- a/examples/system/gcov/README.md +++ b/examples/system/gcov/README.md @@ -1,3 +1,6 @@ +| Supported Targets | ESP32 | ESP32-S2 | +| ----------------- | ----- | -------- | + # Blink Example With Coverage Info (Gcov) (See the README.md file in the upper level 'examples' directory for more information about examples.) @@ -13,10 +16,10 @@ This example implements a simple blink application but with code coverage enable ### Hardware Required -To run this example, you need an ESP32 dev board connected to a JTAG adapter, which can come in the following forms: +To run this example, you need a supported dev board connected to a JTAG adapter, which can come in the following forms: * [ESP-WROVER-KIT](https://docs.espressif.com/projects/esp-idf/en/latest/hw-reference/modules-and-boards.html#esp-wrover-kit-v4-1) which integrates an on-board JTAG adapter. Ensure that the [required jumpers to enable JTAG are connected](https://docs.espressif.com/projects/esp-idf/en/latest/get-started/get-started-wrover-kit.html#setup-options) on the WROVER-KIT. -* ESP32 core board (e.g. ESP32-DevKitC) can also work as long as you connect it to an external JTAG adapter (e.g. FT2232H, J-LINK). +* ESP32 or ESP32-S2 core board (e.g. ESP32-DevKitC, [ESP32-S2-Saola-1](https://docs.espressif.com/projects/esp-idf/en/latest/esp32s2/hw-reference/esp32s2/user-guide-saola-1-v1.2.html)) can also work as long as you connect it to an external JTAG adapter (e.g. FT2232H, J-LINK). This example will assume that that an ESP-WROVER-KIT is used. From fd8b9436195c8d30ed952fcd198060a05485a915 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Tue, 29 Jun 2021 13:12:01 +0300 Subject: [PATCH 04/10] gcov: add gcov callback into the ipc task --- components/app_trace/CMakeLists.txt | 2 +- components/app_trace/gcov/gcov_rtio.c | 28 ++++++++++++---- components/esp_common/src/dbg_stubs.c | 1 + components/esp_common/src/ipc.c | 48 ++++++++++++++++++++++----- 4 files changed, 63 insertions(+), 16 deletions(-) diff --git a/components/app_trace/CMakeLists.txt b/components/app_trace/CMakeLists.txt index 0861299b2c..7153ffcc9e 100644 --- a/components/app_trace/CMakeLists.txt +++ b/components/app_trace/CMakeLists.txt @@ -30,7 +30,7 @@ endif() idf_component_register(SRCS "${srcs}" INCLUDE_DIRS "${include_dirs}" - PRIV_REQUIRES soc + PRIV_REQUIRES soc esp_ipc LDFRAGMENTS linker.lf) diff --git a/components/app_trace/gcov/gcov_rtio.c b/components/app_trace/gcov/gcov_rtio.c index accee8f802..18d6c988a4 100644 --- a/components/app_trace/gcov/gcov_rtio.c +++ b/components/app_trace/gcov/gcov_rtio.c @@ -24,7 +24,8 @@ #include "esp_app_trace.h" #include "esp_freertos_hooks.h" #include "esp_private/dbg_stubs.h" -#include "hal/timer_ll.h" +#include "esp_ipc.h" +#include "hal/wdt_hal.h" #if CONFIG_IDF_TARGET_ESP32 #include "esp32/rom/libc_stubs.h" #elif CONFIG_IDF_TARGET_ESP32S2BETA @@ -49,6 +50,8 @@ void gcov_dump_task(void *pvParameter) int dump_result = 0; bool *running = (bool *)pvParameter; + ESP_EARLY_LOGV(TAG, "%s stack use in %d", __FUNCTION__, uxTaskGetStackHighWaterMark(NULL)); + ESP_EARLY_LOGV(TAG, "Alloc apptrace down buf %d bytes", ESP_GCOV_DOWN_BUF_SIZE); void *down_buf = malloc(ESP_GCOV_DOWN_BUF_SIZE); if (down_buf == NULL) { @@ -81,14 +84,25 @@ gcov_exit: if (running) { *running = false; } + + ESP_EARLY_LOGV(TAG, "%s stack use out %d", __FUNCTION__, uxTaskGetStackHighWaterMark(NULL)); + vTaskDelete(NULL); } -static void gcov_create_task_hook(void) +void gcov_create_task(void *arg) { + ESP_EARLY_LOGV(TAG, "%s", __FUNCTION__); + xTaskCreatePinnedToCore(&gcov_dump_task, "gcov_dump_task", 2048, (void *)&s_gcov_task_running, configMAX_PRIORITIES - 1, NULL, 0); +} + +void gcov_create_task_tick_hook(void) +{ + extern esp_err_t esp_ipc_start_gcov_from_isr(uint32_t cpu_id, esp_ipc_func_t func, void* arg); if (s_create_gcov_task) { - xTaskCreatePinnedToCore(&gcov_dump_task, "gcov_dump_task", 2048, &s_gcov_task_running, configMAX_PRIORITIES - 1, NULL, 0); - s_create_gcov_task = false; + if (esp_ipc_start_gcov_from_isr(xPortGetCoreID(), &gcov_create_task, NULL) == ESP_OK) { + s_create_gcov_task = false; + } } } @@ -114,19 +128,21 @@ int gcov_rtio_atexit(void (*function)(void) __attribute__ ((unused))) if (esp_dbg_stub_entry_get(ESP_DBG_STUB_CAPABILITIES, &capabilities) == ESP_OK) { esp_dbg_stub_entry_set(ESP_DBG_STUB_CAPABILITIES, capabilities | ESP_DBG_STUB_CAP_GCOV_TASK); } - esp_register_freertos_tick_hook(gcov_create_task_hook); + esp_register_freertos_tick_hook(gcov_create_task_tick_hook); return ESP_OK; } void esp_gcov_dump(void) { + ESP_EARLY_LOGV(TAG, "%s", __FUNCTION__); + while (!esp_apptrace_host_is_connected(ESP_APPTRACE_DEST_TRAX)) { vTaskDelay(pdMS_TO_TICKS(10)); } /* We are not in isr context here. Waiting for the completion is safe */ - s_create_gcov_task = true; s_gcov_task_running = true; + s_create_gcov_task = true; while (s_gcov_task_running) { vTaskDelay(pdMS_TO_TICKS(10)); } diff --git a/components/esp_common/src/dbg_stubs.c b/components/esp_common/src/dbg_stubs.c index e4c65c22e2..a1740c23e6 100644 --- a/components/esp_common/src/dbg_stubs.c +++ b/components/esp_common/src/dbg_stubs.c @@ -82,6 +82,7 @@ void esp_dbg_stubs_init(void) ESP_LOGV(TAG, "%s stubs %x", __func__, eri_read(ESP_DBG_STUBS_TRAX_REG)); } +// TODO: add lock mechanism. Not now but in the future ESP_DBG_STUB_CAPABILITIES can be set from different places. esp_err_t esp_dbg_stub_entry_set(esp_dbg_stub_id_t id, uint32_t entry) { if (id < ESP_DBG_STUB_ENTRY_FIRST || id >= ESP_DBG_STUB_ENTRY_MAX) { diff --git a/components/esp_common/src/ipc.c b/components/esp_common/src/ipc.c index a9a7ec5558..d2d75b8a09 100644 --- a/components/esp_common/src/ipc.c +++ b/components/esp_common/src/ipc.c @@ -28,7 +28,7 @@ static TaskHandle_t s_ipc_task_handle[portNUM_PROCESSORS]; static SemaphoreHandle_t s_ipc_mutex[portNUM_PROCESSORS]; // This mutex is used as a global lock for esp_ipc_* APIs static SemaphoreHandle_t s_ipc_sem[portNUM_PROCESSORS]; // Two semaphores used to wake each of ipc tasks static SemaphoreHandle_t s_ipc_ack[portNUM_PROCESSORS]; // Semaphore used to acknowledge that task was woken up, - // or function has finished running + // or function has finished running static volatile esp_ipc_func_t s_func[portNUM_PROCESSORS]; // Function which should be called by high priority task static void * volatile s_func_arg[portNUM_PROCESSORS]; // Argument to pass into s_func typedef enum { @@ -40,6 +40,11 @@ static volatile esp_ipc_wait_t s_ipc_wait[portNUM_PROCESSORS];// This variable t // s_ipc_ack semaphore: before s_func is called, or // after it returns +#if CONFIG_APPTRACE_GCOV_ENABLE +static volatile esp_ipc_func_t s_gcov_func = NULL; // Gcov dump starter function which should be called by high priority task +static void * volatile s_gcov_func_arg; // Argument to pass into s_gcov_func +#endif + static void IRAM_ATTR ipc_task(void* arg) { const uint32_t cpuid = (uint32_t) arg; @@ -53,16 +58,26 @@ static void IRAM_ATTR ipc_task(void* arg) abort(); } - esp_ipc_func_t func = s_func[cpuid]; - void* arg = s_func_arg[cpuid]; +#if CONFIG_APPTRACE_GCOV_ENABLE + if (s_gcov_func) { + (*s_gcov_func)(s_gcov_func_arg); + s_gcov_func = NULL; + } +#endif + if (s_func[cpuid]) { + esp_ipc_func_t func = s_func[cpuid]; + void* arg = s_func_arg[cpuid]; - if (s_ipc_wait[cpuid] == IPC_WAIT_FOR_START) { - xSemaphoreGive(s_ipc_ack[cpuid]); - } - (*func)(arg); - if (s_ipc_wait[cpuid] == IPC_WAIT_FOR_END) { - xSemaphoreGive(s_ipc_ack[cpuid]); + if (s_ipc_wait[cpuid] == IPC_WAIT_FOR_START) { + xSemaphoreGive(s_ipc_ack[cpuid]); + } + (*func)(arg); + if (s_ipc_wait[cpuid] == IPC_WAIT_FOR_END) { + xSemaphoreGive(s_ipc_ack[cpuid]); + } + s_func[cpuid] = NULL; } + } // TODO: currently this is unreachable code. Introduce esp_ipc_uninit // function which will signal to both tasks that they can shut down. @@ -87,6 +102,7 @@ static void esp_ipc_init(void) __attribute__((constructor)); static void esp_ipc_init(void) { char task_name[15]; + for (int i = 0; i < portNUM_PROCESSORS; ++i) { snprintf(task_name, sizeof(task_name), "ipc%d", i); s_ipc_mutex[i] = xSemaphoreCreateMutex(); @@ -144,3 +160,17 @@ esp_err_t esp_ipc_call_blocking(uint32_t cpu_id, esp_ipc_func_t func, void* arg) return esp_ipc_call_and_wait(cpu_id, func, arg, IPC_WAIT_FOR_END); } +// currently this is only called from gcov component +#if CONFIG_APPTRACE_GCOV_ENABLE +esp_err_t esp_ipc_start_gcov_from_isr(uint32_t cpu_id, esp_ipc_func_t func, void* arg) +{ + if (xTaskGetSchedulerState() != taskSCHEDULER_RUNNING) { + return ESP_ERR_INVALID_STATE; + } + s_gcov_func = func; + s_gcov_func_arg = arg; + xSemaphoreGiveFromISR(s_ipc_sem[cpu_id], NULL); + + return ESP_OK; +} +#endif From 1d987334d300e3cfcc5bf95b90a505d2165839a9 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Fri, 16 Jul 2021 21:04:33 +0300 Subject: [PATCH 05/10] gcov: add stub table size entry --- components/app_trace/gcov/gcov_rtio.c | 4 ++-- components/esp_common/include/esp_private/dbg_stubs.h | 3 ++- components/esp_common/src/dbg_stubs.c | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/components/app_trace/gcov/gcov_rtio.c b/components/app_trace/gcov/gcov_rtio.c index 18d6c988a4..660a70ab2a 100644 --- a/components/app_trace/gcov/gcov_rtio.c +++ b/components/app_trace/gcov/gcov_rtio.c @@ -125,8 +125,8 @@ int gcov_rtio_atexit(void (*function)(void) __attribute__ ((unused))) uint32_t capabilities = 0; ESP_EARLY_LOGV(TAG, "%s", __FUNCTION__); esp_dbg_stub_entry_set(ESP_DBG_STUB_ENTRY_GCOV, (uint32_t)&esp_dbg_stub_gcov_entry); - if (esp_dbg_stub_entry_get(ESP_DBG_STUB_CAPABILITIES, &capabilities) == ESP_OK) { - esp_dbg_stub_entry_set(ESP_DBG_STUB_CAPABILITIES, capabilities | ESP_DBG_STUB_CAP_GCOV_TASK); + if (esp_dbg_stub_entry_get(ESP_DBG_STUB_ENTRY_CAPABILITIES, &capabilities) == ESP_OK) { + esp_dbg_stub_entry_set(ESP_DBG_STUB_ENTRY_CAPABILITIES, capabilities | ESP_DBG_STUB_CAP_GCOV_TASK); } esp_register_freertos_tick_hook(gcov_create_task_tick_hook); return ESP_OK; diff --git a/components/esp_common/include/esp_private/dbg_stubs.h b/components/esp_common/include/esp_private/dbg_stubs.h index 454c3418f9..65fff455cb 100644 --- a/components/esp_common/include/esp_private/dbg_stubs.h +++ b/components/esp_common/include/esp_private/dbg_stubs.h @@ -21,11 +21,12 @@ */ typedef enum { ESP_DBG_STUB_MAGIC_NUM, + ESP_DBG_STUB_TABLE_SIZE, ESP_DBG_STUB_CONTROL_DATA, ///< stubs descriptor entry ESP_DBG_STUB_ENTRY_FIRST, ESP_DBG_STUB_ENTRY_GCOV ///< GCOV entry = ESP_DBG_STUB_ENTRY_FIRST, - ESP_DBG_STUB_CAPABILITIES, + ESP_DBG_STUB_ENTRY_CAPABILITIES, ESP_DBG_STUB_ENTRY_MAX } esp_dbg_stub_id_t; diff --git a/components/esp_common/src/dbg_stubs.c b/components/esp_common/src/dbg_stubs.c index a1740c23e6..04cd11398e 100644 --- a/components/esp_common/src/dbg_stubs.c +++ b/components/esp_common/src/dbg_stubs.c @@ -77,12 +77,13 @@ void esp_dbg_stubs_init(void) s_dbg_stubs_ctl_data.data_free = (uint32_t)esp_dbg_stubs_data_free; s_stub_entry[ESP_DBG_STUB_MAGIC_NUM] = ESP_DBG_STUB_MAGIC_NUM_VAL; + s_stub_entry[ESP_DBG_STUB_TABLE_SIZE] = ESP_DBG_STUB_ENTRY_MAX; s_stub_entry[ESP_DBG_STUB_CONTROL_DATA] = (uint32_t)&s_dbg_stubs_ctl_data; eri_write(ESP_DBG_STUBS_TRAX_REG, (uint32_t)s_stub_entry); ESP_LOGV(TAG, "%s stubs %x", __func__, eri_read(ESP_DBG_STUBS_TRAX_REG)); } -// TODO: add lock mechanism. Not now but in the future ESP_DBG_STUB_CAPABILITIES can be set from different places. +// TODO: add lock mechanism. Not now but in the future ESP_DBG_STUB_ENTRY_CAPABILITIES can be set from different places. esp_err_t esp_dbg_stub_entry_set(esp_dbg_stub_id_t id, uint32_t entry) { if (id < ESP_DBG_STUB_ENTRY_FIRST || id >= ESP_DBG_STUB_ENTRY_MAX) { From 9681fb3d718486b8b9d11b4113ffaddd796710c1 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Thu, 12 Aug 2021 08:52:15 +0300 Subject: [PATCH 06/10] ipc: enable ipc task at single core for gcov dump --- components/esp_common/src/ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/esp_common/src/ipc.c b/components/esp_common/src/ipc.c index d2d75b8a09..fce1462c17 100644 --- a/components/esp_common/src/ipc.c +++ b/components/esp_common/src/ipc.c @@ -173,4 +173,4 @@ esp_err_t esp_ipc_start_gcov_from_isr(uint32_t cpu_id, esp_ipc_func_t func, void return ESP_OK; } -#endif +#endif From 384ca3452b1019b8d33a62b450e06af1d53fd9d6 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Fri, 13 Aug 2021 12:06:12 +0300 Subject: [PATCH 07/10] gcov: enable single core tests --- examples/system/gcov/sdkconfig.ci | 1 + 1 file changed, 1 insertion(+) create mode 100644 examples/system/gcov/sdkconfig.ci diff --git a/examples/system/gcov/sdkconfig.ci b/examples/system/gcov/sdkconfig.ci new file mode 100644 index 0000000000..f0b0b5e03d --- /dev/null +++ b/examples/system/gcov/sdkconfig.ci @@ -0,0 +1 @@ +CONFIG_FREERTOS_UNICORE=y From 3830ac21e26284107b7e684f7beb8b6753264eab Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Mon, 23 Aug 2021 17:17:19 +0300 Subject: [PATCH 08/10] esp_ipc: fix race condition in ipc task --- components/esp_common/src/ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/esp_common/src/ipc.c b/components/esp_common/src/ipc.c index fce1462c17..4f6a4092ee 100644 --- a/components/esp_common/src/ipc.c +++ b/components/esp_common/src/ipc.c @@ -75,7 +75,6 @@ static void IRAM_ATTR ipc_task(void* arg) if (s_ipc_wait[cpuid] == IPC_WAIT_FOR_END) { xSemaphoreGive(s_ipc_ack[cpuid]); } - s_func[cpuid] = NULL; } } @@ -142,6 +141,7 @@ static esp_err_t esp_ipc_call_and_wait(uint32_t cpu_id, esp_ipc_func_t func, voi s_ipc_wait[cpu_id] = wait_for; xSemaphoreGive(s_ipc_sem[cpu_id]); xSemaphoreTake(s_ipc_ack[cpu_id], portMAX_DELAY); + s_func[cpu_id] = NULL; #ifdef CONFIG_ESP_IPC_USES_CALLERS_PRIORITY xSemaphoreGive(s_ipc_mutex[cpu_id]); #else From 3277ff68563bd1748242890088c91c6e0c517e93 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Fri, 12 Nov 2021 00:59:02 +0300 Subject: [PATCH 09/10] gcov: ipc component name changei --- components/app_trace/CMakeLists.txt | 2 +- components/app_trace/gcov/gcov_rtio.c | 8 -------- components/esp_common/src/ipc.c | 27 +++++++++++++++++++++++++-- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/components/app_trace/CMakeLists.txt b/components/app_trace/CMakeLists.txt index 7153ffcc9e..c770a964c2 100644 --- a/components/app_trace/CMakeLists.txt +++ b/components/app_trace/CMakeLists.txt @@ -30,7 +30,7 @@ endif() idf_component_register(SRCS "${srcs}" INCLUDE_DIRS "${include_dirs}" - PRIV_REQUIRES soc esp_ipc + PRIV_REQUIRES soc esp_common LDFRAGMENTS linker.lf) diff --git a/components/app_trace/gcov/gcov_rtio.c b/components/app_trace/gcov/gcov_rtio.c index 660a70ab2a..8953611b12 100644 --- a/components/app_trace/gcov/gcov_rtio.c +++ b/components/app_trace/gcov/gcov_rtio.c @@ -25,7 +25,6 @@ #include "esp_freertos_hooks.h" #include "esp_private/dbg_stubs.h" #include "esp_ipc.h" -#include "hal/wdt_hal.h" #if CONFIG_IDF_TARGET_ESP32 #include "esp32/rom/libc_stubs.h" #elif CONFIG_IDF_TARGET_ESP32S2BETA @@ -61,18 +60,11 @@ void gcov_dump_task(void *pvParameter) } ESP_EARLY_LOGV(TAG, "Config apptrace down buf"); esp_apptrace_down_buffer_config(down_buf, ESP_GCOV_DOWN_BUF_SIZE); - /* we are directing the std outputs to the fake ones in order to reduce stack usage */ - FILE *old_stderr = stderr; - FILE *old_stdout = stdout; - stderr = (FILE *) &__sf_fake_stderr; - stdout = (FILE *) &__sf_fake_stdout; ESP_EARLY_LOGV(TAG, "Dump data..."); __gcov_dump(); // reset dump status to allow incremental data accumulation __gcov_reset(); free(down_buf); - stderr = old_stderr; - stdout = old_stdout; ESP_EARLY_LOGV(TAG, "Finish file transfer session"); dump_result = esp_apptrace_fstop(ESP_APPTRACE_DEST_TRAX); if (dump_result != ESP_OK) { diff --git a/components/esp_common/src/ipc.c b/components/esp_common/src/ipc.c index 4f6a4092ee..3255e307ea 100644 --- a/components/esp_common/src/ipc.c +++ b/components/esp_common/src/ipc.c @@ -62,6 +62,8 @@ static void IRAM_ATTR ipc_task(void* arg) if (s_gcov_func) { (*s_gcov_func)(s_gcov_func_arg); s_gcov_func = NULL; + /* we can not interfer with IPC calls so no need for further processing */ + continue; } #endif if (s_func[cpuid]) { @@ -164,13 +166,34 @@ esp_err_t esp_ipc_call_blocking(uint32_t cpu_id, esp_ipc_func_t func, void* arg) #if CONFIG_APPTRACE_GCOV_ENABLE esp_err_t esp_ipc_start_gcov_from_isr(uint32_t cpu_id, esp_ipc_func_t func, void* arg) { + portBASE_TYPE ret = pdFALSE; + if (xTaskGetSchedulerState() != taskSCHEDULER_RUNNING) { return ESP_ERR_INVALID_STATE; } + + /* Lock IPC to avoid interferring with normal IPC calls, e.g. + avoid situation when esp_ipc_start_gcov_from_isr() is called from IRQ + in the middle of IPC call between `s_func` and `s_func_arg` modification. See esp_ipc_call_and_wait() */ +#ifdef CONFIG_ESP_IPC_USES_CALLERS_PRIORITY + ret = xSemaphoreTakeFromISR(s_ipc_mutex[cpu_id], NULL); +#else + ret = xSemaphoreTakeFromISR(s_ipc_mutex[0], NULL); +#endif + if (ret != pdTRUE) { + return ESP_ERR_TIMEOUT; + } + s_gcov_func = func; s_gcov_func_arg = arg; - xSemaphoreGiveFromISR(s_ipc_sem[cpu_id], NULL); + ret = xSemaphoreGiveFromISR(s_ipc_sem[cpu_id], NULL); - return ESP_OK; +#ifdef CONFIG_ESP_IPC_USES_CALLERS_PRIORITY + xSemaphoreGiveFromISR(s_ipc_mutex[cpu_id], NULL); +#else + xSemaphoreGiveFromISR(s_ipc_mutex[0], NULL); +#endif + + return ret == pdTRUE ? ESP_OK : ESP_FAIL; } #endif From 08ef49e4786619b0cfe537778c3f75ba90706c7e Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Fri, 12 Nov 2021 19:25:41 +0300 Subject: [PATCH 10/10] gcov: enable gcov in single core --- components/bt/controller/lib_esp32c3_family | 1 - components/esp_common/CMakeLists.txt | 2 +- components/esp_common/component.mk | 4 +++- components/tinyusb/tinyusb | 1 - .../atecc608_ecdsa/components/esp-cryptoauthlib | 1 - 5 files changed, 4 insertions(+), 5 deletions(-) delete mode 160000 components/bt/controller/lib_esp32c3_family delete mode 160000 components/tinyusb/tinyusb delete mode 160000 examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib diff --git a/components/bt/controller/lib_esp32c3_family b/components/bt/controller/lib_esp32c3_family deleted file mode 160000 index 32f15e826a..0000000000 --- a/components/bt/controller/lib_esp32c3_family +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 32f15e826a102d2d64e612620468122ea2234a2e diff --git a/components/esp_common/CMakeLists.txt b/components/esp_common/CMakeLists.txt index c2b6586437..3f4ae7442f 100644 --- a/components/esp_common/CMakeLists.txt +++ b/components/esp_common/CMakeLists.txt @@ -17,7 +17,7 @@ else() "src/system_api.c") # IPC framework is not applicable if freertos unicore config is selected - if(NOT CONFIG_FREERTOS_UNICORE) + if(NOT CONFIG_FREERTOS_UNICORE OR CONFIG_APPTRACE_GCOV_ENABLE) list(APPEND srcs "src/ipc.c") endif() diff --git a/components/esp_common/component.mk b/components/esp_common/component.mk index 4d80bc4bf6..f8415111d8 100644 --- a/components/esp_common/component.mk +++ b/components/esp_common/component.mk @@ -7,7 +7,9 @@ COMPONENT_SRCDIRS := src # IPC framework is not applicable if freertos unicore config is selected ifdef CONFIG_FREERTOS_UNICORE -COMPONENT_OBJEXCLUDE := src/ipc.o + ifndef CONFIG_APPTRACE_GCOV_ENABLE + COMPONENT_OBJEXCLUDE := src/ipc.o + endif endif # disable stack protection in files which are involved in initialization of that feature diff --git a/components/tinyusb/tinyusb b/components/tinyusb/tinyusb deleted file mode 160000 index 68f6ef6790..0000000000 --- a/components/tinyusb/tinyusb +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 68f6ef679000f9cd22b20af7473d7f0480d2862e diff --git a/examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib b/examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib deleted file mode 160000 index 36d0642e66..0000000000 --- a/examples/peripherals/secure_element/atecc608_ecdsa/components/esp-cryptoauthlib +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 36d0642e66ff5b1c7a291873f24c498ca6ffedef