From 24d44e16961e0e6d4e3b489fda4613fc08f585a9 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Mon, 16 Dec 2024 10:31:22 +0530 Subject: [PATCH] fix(ulp): Added unit tests for ulp binary embed with prefix and misc fixes This commit: - Removes the link time symbol name clash detection. - Extracts symbols of type NOTYPE for global identifiers defined in assembly files. - Makes the prefix argument optional for ulp_add_build_binary_targets(). - Adds a unit test for the ulp binary embed with a prefix feature. --- components/ulp/cmake/CMakeLists.txt | 2 +- components/ulp/cmake/IDFULPProject.cmake | 9 ++- components/ulp/esp32ulp_mapgen.py | 37 ++++++++---- .../lp_core_basic_tests/main/CMakeLists.txt | 5 ++ .../main/lp_core/test_main_prefix1.c | 18 ++++++ .../main/lp_core/test_main_prefix2.c | 25 ++++++++ .../main/test_lp_core_prefix.c | 58 +++++++++++++++++++ docs/en/api-reference/system/ulp-lp-core.rst | 2 +- docs/en/api-reference/system/ulp-risc-v.rst | 2 +- docs/en/api-reference/system/ulp.rst | 2 +- 10 files changed, 142 insertions(+), 18 deletions(-) create mode 100644 components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix1.c create mode 100644 components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix2.c create mode 100644 components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_prefix.c diff --git a/components/ulp/cmake/CMakeLists.txt b/components/ulp/cmake/CMakeLists.txt index 816a4006ab..5bacedede2 100644 --- a/components/ulp/cmake/CMakeLists.txt +++ b/components/ulp/cmake/CMakeLists.txt @@ -8,4 +8,4 @@ include(IDFULPProject) ulp_apply_default_options(${ULP_APP_NAME}) ulp_apply_default_sources(${ULP_APP_NAME}) -ulp_add_build_binary_targets(${ULP_APP_NAME} ${ULP_VAR_PREFIX}) +ulp_add_build_binary_targets(${ULP_APP_NAME} PREFIX ${ULP_VAR_PREFIX}) diff --git a/components/ulp/cmake/IDFULPProject.cmake b/components/ulp/cmake/IDFULPProject.cmake index 28f68dcc5c..12d4a3aa53 100644 --- a/components/ulp/cmake/IDFULPProject.cmake +++ b/components/ulp/cmake/IDFULPProject.cmake @@ -80,6 +80,7 @@ function(ulp_apply_default_sources ulp_app_name) # To avoid warning "Manually-specified variables were not used by the project" set(bypassWarning "${IDF_TARGET}") + set(bypassWarning "${ULP_VAR_PREFIX}") if(CONFIG_ULP_COPROC_TYPE_RISCV) #risc-v ulp uses extra files for building: @@ -178,7 +179,11 @@ function(ulp_apply_default_sources ulp_app_name) endif() endfunction() -function(ulp_add_build_binary_targets ulp_app_name prefix) +function(ulp_add_build_binary_targets ulp_app_name) + cmake_parse_arguments(ULP "" "PREFIX" "" ${ARGN}) + if(NOT ULP_PREFIX) + set(ULP_PREFIX "ulp_") + endif() if(ADD_PICOLIBC_SPECS) target_compile_options(${ulp_app_name} PRIVATE $<$:-specs=picolibc.specs>) @@ -207,7 +212,7 @@ function(ulp_add_build_binary_targets ulp_app_name prefix) add_custom_command(OUTPUT ${ulp_app_name}.ld ${ulp_app_name}.h COMMAND ${ULP_MAP_GEN} -s ${ulp_app_name}.sym -o ${ulp_app_name} - --base ${ULP_BASE_ADDR} --prefix ${prefix} + --base ${ULP_BASE_ADDR} --prefix ${ULP_PREFIX} DEPENDS ${ulp_app_name}.sym WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) diff --git a/components/ulp/esp32ulp_mapgen.py b/components/ulp/esp32ulp_mapgen.py index b0ab767a33..972d8fd3e2 100755 --- a/components/ulp/esp32ulp_mapgen.py +++ b/components/ulp/esp32ulp_mapgen.py @@ -43,7 +43,7 @@ def gen_ld_h_from_sym(f_sym: typing.TextIO, f_ld: typing.TextIO, f_h: typing.Tex tmp = prefix.split('::') namespace = tmp[0] var_prefix = '_'.join(tmp[1:]) # Limit to a single namespace here to avoid complex mangling rules - f_h.write('namespace {0} {{\n'.format(namespace)) + f_h.write(f'namespace {namespace} {{\n') cpp_mode = True else: f_h.write(textwrap.dedent( @@ -54,25 +54,38 @@ def gen_ld_h_from_sym(f_sym: typing.TextIO, f_ld: typing.TextIO, f_h: typing.Tex #pragma once #ifdef __cplusplus extern "C" {{ - #endif + #endif\n """ # noqa: E222 )) - expr = re.compile('^\\s*\\d+: ([a-f0-9]{8})\\s+(\\d+) OBJECT\\s+GLOBAL\\s+DEFAULT\\s+[^ ]+ (.*)$') - already_defined = 'this_symbol_is_already_defined_please_use_prefix_in_ulp_embed_binary' + # Format the regular expression to match the readelf output + expr = re.compile(r'^.*(?P
[a-f0-9]{8})\s+(?P\d+) (OBJECT|NOTYPE)\s+GLOBAL\s+DEFAULT\s+[^ ]+ (?P.*)$') for line in f_sym: # readelf format output has the following structure: - # index: addr_hex size TYPE SCOPE DEFAULT junk symbol_name + # Num: Value Size Type Bind Vis Ndx Name # So match the line with a regular expression to parse it first groups = expr.match(line) - if groups is None: # Ignore non global or non object + + # Ignore lines that do not match the expected format such as non global symbols + if groups is None: continue - addr = int(groups.group(1), 16) + base_addr - size = int(groups.group(2)) - name = var_prefix + groups.group(3) - f_h.write('extern uint32_t {0}{1};\n'.format(name, '[{0}]'.format(int(size / 4)) if size > 4 else '')) - f_ld.write('{0} = DEFINED({0}) ? {2} : 0x{1:08x};\n'.format( - name_mangling(namespace + '::' + name) if cpp_mode else name, addr, already_defined)) + + # Extract the symbol information + addr = int(groups.group('address'), 16) + base_addr + size = int(groups.group('size')) + sym_name = groups.group('name') + + # Add the variable prefix if provided + var_name = var_prefix + sym_name + + # Get the dimension of the variable if it is an array + var_dimension = f'[{size // 4}]' if size > 4 else '' # Use // to automatically perform an integer division + + # Write the variable definition to the header file and the address to the linker script + f_h.write(f'extern uint32_t {var_name}{var_dimension};\n') + + name_in_ld = name_mangling(namespace + '::' + var_name) if cpp_mode else var_name + f_ld.write(f'{name_in_ld} = 0x{addr:08x};\n') if cpp_mode: f_h.write('}\n') diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt index 239b78cf33..6c1a9d331d 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt @@ -24,6 +24,8 @@ if(CONFIG_SOC_LP_VAD_SUPPORTED) list(APPEND app_sources "test_lp_core_vad.c") endif() +list(APPEND app_sources "test_lp_core_prefix.c") + set(lp_core_sources "lp_core/test_main.c") set(lp_core_sources_counter "lp_core/test_main_counter.c") @@ -92,3 +94,6 @@ endif() if(CONFIG_SOC_LP_VAD_SUPPORTED) ulp_embed_binary(lp_core_test_app_vad "${lp_core_sources_vad}" "${lp_core_exp_dep_srcs}") endif() + +ulp_embed_binary(lp_core_test_app_prefix1 "lp_core/test_main_prefix1.c" "${lp_core_exp_dep_srcs}" PREFIX "ulp1_") +ulp_embed_binary(lp_core_test_app_prefix2 "lp_core/test_main_prefix2.c" "${lp_core_exp_dep_srcs}" PREFIX "ulp2_") diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix1.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix1.c new file mode 100644 index 0000000000..d6bf5669f8 --- /dev/null +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix1.c @@ -0,0 +1,18 @@ +/* + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/* Global variable with the same name across different ULP binaries */ +volatile int g_var; + +int main(void) +{ + g_var = 1; + + while (1) + ; + + return 0; +} diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix2.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix2.c new file mode 100644 index 0000000000..80b219841b --- /dev/null +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_prefix2.c @@ -0,0 +1,25 @@ +/* + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/* A global variable to make sure that g_var doesn't have the same address across different ULP binaries */ +volatile int g_array[10]; + +/* Global variable with the same name across different ULP binaries */ +volatile int g_var; + +int main(void) +{ + int i; + for (i = 0; i < 10; i++) { + g_array[i] = i; + } + g_var = 2; + + while (1) + ; + + return 0; +} diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_prefix.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_prefix.c new file mode 100644 index 0000000000..2e089dc4e9 --- /dev/null +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_prefix.c @@ -0,0 +1,58 @@ +/* + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include "lp_core_test_app_prefix1.h" +#include "lp_core_test_app_prefix2.h" +#include "ulp_lp_core.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "test_shared.h" +#include "unity.h" +#include "test_utils.h" + +extern const uint8_t lp_core_main_prefix1_bin_start[] asm("_binary_lp_core_test_app_prefix1_bin_start"); +extern const uint8_t lp_core_main_prefix1_bin_end[] asm("_binary_lp_core_test_app_prefix1_bin_end"); +extern const uint8_t lp_core_main_prefix2_bin_start[] asm("_binary_lp_core_test_app_prefix2_bin_start"); +extern const uint8_t lp_core_main_prefix2_bin_end[] asm("_binary_lp_core_test_app_prefix2_bin_end"); + +static void load_and_start_lp_core_firmware(ulp_lp_core_cfg_t* cfg, const uint8_t* firmware_start, const uint8_t* firmware_end) +{ + TEST_ASSERT(ulp_lp_core_load_binary(firmware_start, + (firmware_end - firmware_start)) == ESP_OK); + + TEST_ASSERT(ulp_lp_core_run(cfg) == ESP_OK); +} + +TEST_CASE("LP-Core header prefix test", "[lp_core]") +{ + /* Load ULP firmware and start the coprocessor */ + ulp_lp_core_cfg_t cfg = { + .wakeup_source = ULP_LP_CORE_WAKEUP_SOURCE_HP_CPU, +#if ESP_ROM_HAS_LP_ROM + .skip_lp_rom_boot = true, +#endif //ESP_ROM_HAS_LP_ROM + }; + + /* Load and run the first LP core firmware */ + load_and_start_lp_core_firmware(&cfg, lp_core_main_prefix1_bin_start, lp_core_main_prefix1_bin_end); + vTaskDelay(10); + + /* Verify the shared variable value */ + TEST_ASSERT_EQUAL(1, ulp1_g_var); + + /* Load and run the second LP core firmware */ + load_and_start_lp_core_firmware(&cfg, lp_core_main_prefix2_bin_start, lp_core_main_prefix2_bin_end); + vTaskDelay(10); + + /* Verify that a global array can be accessed as an array on the HP CPU */ + for (int i = 0; i < 10; i++) { + TEST_ASSERT_EQUAL(i, ulp2_g_array[i]); + } + + /* Verify that the shared variable with the same name is updated once the second LP core binary runs */ + TEST_ASSERT_EQUAL(2, ulp2_g_var); +} diff --git a/docs/en/api-reference/system/ulp-lp-core.rst b/docs/en/api-reference/system/ulp-lp-core.rst index ca1d9c6a0c..7f53a09fd8 100644 --- a/docs/en/api-reference/system/ulp-lp-core.rst +++ b/docs/en/api-reference/system/ulp-lp-core.rst @@ -50,7 +50,7 @@ If you need to embed multiple ULP programs, you may add a custom prefix in order ulp_embed_binary(${ulp_app_name} "${ulp_sources}" "${ulp_exp_dep_srcs}" PREFIX "ULP::") -The additional argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``). +The additional PREFIX argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``). Using a Custom CMake Project ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/en/api-reference/system/ulp-risc-v.rst b/docs/en/api-reference/system/ulp-risc-v.rst index 059e6dc1a6..b37e4f6070 100644 --- a/docs/en/api-reference/system/ulp-risc-v.rst +++ b/docs/en/api-reference/system/ulp-risc-v.rst @@ -53,7 +53,7 @@ If you need to embed multiple ULP programs, you may add a custom prefix in order ulp_embed_binary(${ulp_app_name} "${ulp_sources}" "${ulp_exp_dep_srcs}" PREFIX "ULP::") -The additional argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``). +The additional PREFIX argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``). Using a Custom CMake Project ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/docs/en/api-reference/system/ulp.rst b/docs/en/api-reference/system/ulp.rst index 5ca31c84d0..a468b72928 100644 --- a/docs/en/api-reference/system/ulp.rst +++ b/docs/en/api-reference/system/ulp.rst @@ -64,7 +64,7 @@ If you need to embed multiple ULP programs, you may add a custom prefix in order ulp_embed_binary(${ulp_app_name} "${ulp_sources}" "${ulp_exp_dep_srcs}" PREFIX "ULP::") -The additional argument can be a C style prefix (like ``ulp2_``). +The additional PREFIX argument can be a C style prefix (like ``ulp2_``) or a C++ style prefix (like ``ULP::``). 3. Build the application as usual (e.g., ``idf.py app``).