From a5d6f62e7ea7247e3d208f3ea38ede8141cecc61 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Wed, 18 Jan 2023 11:19:15 +0100 Subject: [PATCH 1/2] heap: Prevent alloc from IRAM to call dram_alloc_to_iram() on esp32c6 target Since DRAM and IRAM are superposed on esp32c6 it is not necessary to convert a freshly allocated DRAM addr to its IRAM equivalent when MALLOC_CAP_EXEC is passed to heap_caps_malloc(). Instead, proceed with a default allocation since the address returned by multi_heap_malloc() already belongs to the IRAM region. Applies for esp32c6 and every boards with superposed DRAM and IRAM addresses. --- .../include/bootloader_memory_utils.h | 10 ++++++++++ components/esp_hw_support/include/esp_memory_utils.h | 10 ++++++++++ components/heap/heap_caps.c | 4 +++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/components/bootloader_support/include/bootloader_memory_utils.h b/components/bootloader_support/include/bootloader_memory_utils.h index 988749593c..5ee1dbded7 100644 --- a/components/bootloader_support/include/bootloader_memory_utils.h +++ b/components/bootloader_support/include/bootloader_memory_utils.h @@ -20,6 +20,16 @@ extern "C" { /** The content of this file is to be kept in sync with the common section of esp_memory_utils.h **/ +/** + * @brief Check if the IRAM and DRAM are separate or using the same memory space + * + * @return true if the DRAM and IRAM are sharing the same memory space, false otherwise + */ +__attribute__((always_inline)) +inline static bool esp_dram_match_iram(void) { + return (SOC_DRAM_LOW == SOC_IRAM_LOW && SOC_DRAM_HIGH == SOC_IRAM_HIGH); +} + /** * @brief Check if the pointer is in iram * diff --git a/components/esp_hw_support/include/esp_memory_utils.h b/components/esp_hw_support/include/esp_memory_utils.h index 330fc114bf..4326a4f6f4 100644 --- a/components/esp_hw_support/include/esp_memory_utils.h +++ b/components/esp_hw_support/include/esp_memory_utils.h @@ -20,6 +20,16 @@ extern "C" { /** Common functions, to be kept in sync with bootloader_memory_utils.h **/ +/** + * @brief Check if the IRAM and DRAM are separate or using the same memory space + * + * @return true if the DRAM and IRAM are sharing the same memory space, false otherwise + */ +__attribute__((always_inline)) +inline static bool esp_dram_match_iram(void) { + return (SOC_DRAM_LOW == SOC_IRAM_LOW && SOC_DRAM_HIGH == SOC_IRAM_HIGH); +} + /** * @brief Check if the pointer is in iram * diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index e28653f69c..d77eab0e5e 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -131,7 +131,9 @@ IRAM_ATTR static void *heap_caps_malloc_base( size_t size, uint32_t caps) //doesn't cover, see if they're available in other prios. if ((get_all_caps(heap) & caps) == caps) { //This heap can satisfy all the requested capabilities. See if we can grab some memory using it. - if ((caps & MALLOC_CAP_EXEC) && esp_ptr_in_diram_dram((void *)heap->start)) { + // If MALLOC_CAP_EXEC is requested but the DRAM and IRAM are on the same addresses (like on esp32c6) + // proceed as for a default allocation. + if ((caps & MALLOC_CAP_EXEC) && !esp_dram_match_iram() && esp_ptr_in_diram_dram((void *)heap->start)) { //This is special, insofar that what we're going to get back is a DRAM address. If so, //we need to 'invert' it (lowest address in DRAM == highest address in IRAM and vice-versa) and //add a pointer to the DRAM equivalent before the address we're going to return. From 11f99ef649a9bfe4208779c4a3d97b56bc46c414 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Mon, 23 Jan 2023 06:59:58 +0100 Subject: [PATCH 2/2] heap: Add memory protection activation test Check that when trying to allocate in IRAM with the system memory protection enabled, null pointer is returned, or that an address in IRAM is returned if the memory protection is disabled. --- components/heap/.build-test-rules.yml | 5 ----- .../heap/test_apps/main/test_malloc_caps.c | 20 +++++++++++++++++++ components/heap/test_apps/pytest_heap.py | 17 +++++++++++++++- .../heap/test_apps/sdkconfig.ci.mem_prot | 1 + 4 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 components/heap/test_apps/sdkconfig.ci.mem_prot diff --git a/components/heap/.build-test-rules.yml b/components/heap/.build-test-rules.yml index 90a31dbc77..ee02ab85e3 100644 --- a/components/heap/.build-test-rules.yml +++ b/components/heap/.build-test-rules.yml @@ -3,8 +3,3 @@ components/heap/host_test/host_test_linux: enable: - if: IDF_TARGET == "linux" -components/heap/test_apps: - disable_test: - - if: IDF_TARGET == "esp32c6" - temporary: true - reason: target esp32c6 is not supported yet diff --git a/components/heap/test_apps/main/test_malloc_caps.c b/components/heap/test_apps/main/test_malloc_caps.c index 7a925dca49..b4943e3d79 100644 --- a/components/heap/test_apps/main/test_malloc_caps.c +++ b/components/heap/test_apps/main/test_malloc_caps.c @@ -278,3 +278,23 @@ TEST_CASE("RTC memory shoule be lowest priority and its free size should be big free(ptr); } #endif + +TEST_CASE("test memory protection features", "[heap][mem_prot]") +{ + // try to allocate memory in IRAM and check that if memory protection is active, + // no memory is being allocated + uint32_t *iram_ptr = heap_caps_malloc(4, MALLOC_CAP_EXEC); + +#ifndef CONFIG_ESP_SYSTEM_MEMPROT_FEATURE + // System memory protection not active, check that iram_ptr is not null + // Check that iram_ptr is in IRAM + TEST_ASSERT_NOT_NULL(iram_ptr); + TEST_ASSERT(true == esp_ptr_in_iram(iram_ptr)); + + // free the memory + heap_caps_free(iram_ptr); +#else + // System memory protection is active, DIRAM seen as DRAM, iram_ptr should be null + TEST_ASSERT_NULL(iram_ptr); +#endif // CONFIG_ESP_SYSTEM_MEMPROT_FEATURE +} diff --git a/components/heap/test_apps/pytest_heap.py b/components/heap/test_apps/pytest_heap.py index 1ccc46f047..a6f6007461 100644 --- a/components/heap/test_apps/pytest_heap.py +++ b/components/heap/test_apps/pytest_heap.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: CC0-1.0 import pytest @@ -92,3 +92,18 @@ def test_heap_trace_dump(dut: Dut) -> None: dut.expect_exact('Enter next test, or \'enter\' to see menu') dut.write('[heap-trace]') dut.expect_unity_test_output(timeout=100) + + +@pytest.mark.generic +@pytest.mark.supported_targets +@pytest.mark.temp_skip_ci(targets=['esp32c3', 'esp32s3'], reason='test failed') +@pytest.mark.parametrize( + 'config', + [ + 'mem_prot' + ] +) +def test_memory_protection(dut: Dut) -> None: + dut.expect_exact('Press ENTER to see the list of tests') + dut.write('[heap][mem_prot]') + dut.expect_unity_test_output(timeout=300) diff --git a/components/heap/test_apps/sdkconfig.ci.mem_prot b/components/heap/test_apps/sdkconfig.ci.mem_prot new file mode 100644 index 0000000000..b6ef908834 --- /dev/null +++ b/components/heap/test_apps/sdkconfig.ci.mem_prot @@ -0,0 +1 @@ +CONFIG_ESP_SYSTEM_MEMPROT_FEATURE=y