From 41d39a419f12f0bfb9ee2929f607afe1d6c1d0fc Mon Sep 17 00:00:00 2001 From: Marius Vikhammer Date: Thu, 4 Jul 2024 15:25:29 +0800 Subject: [PATCH] fix(pmp): fixed alignment of PMP addr for RTC mem on C5 Also refactored it for C6/H2/C61 to keep the approach consistent between targets --- .../port/esp32c5/cpu_region_protect.c | 12 +++++------- .../port/esp32c6/cpu_region_protect.c | 12 +++++------- components/esp_system/ld/esp32c5/sections.ld.in | 8 ++++++-- components/esp_system/ld/esp32c6/sections.ld.in | 13 ++++++++++--- components/esp_system/ld/esp32c61/sections.ld.in | 11 +++++++++-- components/esp_system/ld/esp32h2/sections.ld.in | 13 ++++++++++--- .../soc/esp32c6/include/soc/Kconfig.soc_caps.in | 4 ++++ components/soc/esp32c6/include/soc/soc_caps.h | 1 + .../soc/esp32c61/include/soc/Kconfig.soc_caps.in | 4 ++++ components/soc/esp32c61/include/soc/soc_caps.h | 2 ++ .../soc/esp32h2/include/soc/Kconfig.soc_caps.in | 4 ++++ components/soc/esp32h2/include/soc/soc_caps.h | 1 + components/ulp/test_apps/.build-test-rules.yml | 4 ---- components/ulp/test_apps/lp_core/pytest_lp_core.py | 2 +- 14 files changed, 62 insertions(+), 29 deletions(-) diff --git a/components/esp_hw_support/port/esp32c5/cpu_region_protect.c b/components/esp_hw_support/port/esp32c5/cpu_region_protect.c index e0a9873588..d4d0b42e72 100644 --- a/components/esp_hw_support/port/esp32c5/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32c5/cpu_region_protect.c @@ -182,6 +182,7 @@ void esp_cpu_configure_region_protection(void) // 5. LP memory #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD + extern int _rtc_text_start; extern int _rtc_text_end; /* Reset the corresponding PMP config because PMP_ENTRY_SET only sets the given bits * Bootloader might have given extra permissions and those won't be cleared @@ -191,13 +192,10 @@ void esp_cpu_configure_region_protection(void) PMP_ENTRY_CFG_RESET(13); PMP_ENTRY_CFG_RESET(14); PMP_ENTRY_SET(11, SOC_RTC_IRAM_LOW, NONE); -#if CONFIG_ULP_COPROC_RESERVE_MEM - // First part of LP mem is reserved for coprocessor - PMP_ENTRY_SET(12, SOC_RTC_IRAM_LOW + CONFIG_ULP_COPROC_RESERVE_MEM, PMP_TOR | RW); -#else // CONFIG_ULP_COPROC_RESERVE_MEM - // Repeat same previous entry, to ensure next entry has correct base address (TOR) - PMP_ENTRY_SET(12, SOC_RTC_IRAM_LOW, NONE); -#endif // !CONFIG_ULP_COPROC_RESERVE_MEM + + // First part of LP mem is reserved for ULP coprocessor + PMP_ENTRY_SET(12, (int)&_rtc_text_start, PMP_TOR | RW); + PMP_ENTRY_SET(13, (int)&_rtc_text_end, PMP_TOR | RX); PMP_ENTRY_SET(14, SOC_RTC_IRAM_HIGH, PMP_TOR | RW); #else diff --git a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c index c11435fb84..71cee55e40 100644 --- a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c @@ -178,6 +178,7 @@ void esp_cpu_configure_region_protection(void) // 6. LP memory #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD + extern int _rtc_text_start; extern int _rtc_text_end; /* Reset the corresponding PMP config because PMP_ENTRY_SET only sets the given bits * Bootloader might have given extra permissions and those won't be cleared @@ -187,13 +188,10 @@ void esp_cpu_configure_region_protection(void) PMP_ENTRY_CFG_RESET(13); PMP_ENTRY_CFG_RESET(14); PMP_ENTRY_SET(11, SOC_RTC_IRAM_LOW, NONE); -#if CONFIG_ULP_COPROC_RESERVE_MEM - // First part of LP mem is reserved for coprocessor - PMP_ENTRY_SET(12, SOC_RTC_IRAM_LOW + CONFIG_ULP_COPROC_RESERVE_MEM, PMP_TOR | RW); -#else // CONFIG_ULP_COPROC_RESERVE_MEM - // Repeat same previous entry, to ensure next entry has correct base address (TOR) - PMP_ENTRY_SET(12, SOC_RTC_IRAM_LOW, NONE); -#endif // !CONFIG_ULP_COPROC_RESERVE_MEM + + // First part of LP mem is reserved for ULP coprocessor + PMP_ENTRY_SET(12, (int)&_rtc_text_start, PMP_TOR | RW); + PMP_ENTRY_SET(13, (int)&_rtc_text_end, PMP_TOR | RX); PMP_ENTRY_SET(14, SOC_RTC_IRAM_HIGH, PMP_TOR | RW); #else diff --git a/components/esp_system/ld/esp32c5/sections.ld.in b/components/esp_system/ld/esp32c5/sections.ld.in index 4059d97d36..12540e4ac9 100644 --- a/components/esp_system/ld/esp32c5/sections.ld.in +++ b/components/esp_system/ld/esp32c5/sections.ld.in @@ -17,8 +17,12 @@ SECTIONS */ .rtc.text : { - ALIGNED_SYMBOL(4, _rtc_fast_start) - ALIGNED_SYMBOL(4, _rtc_text_start) + /* Align the start of RTC code region as per PMP granularity + * this ensures we do not overwrite the permissions for the previous + * region (ULP mem) regardless of its end alignment + */ + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_fast_start) + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_text_start) *(.rtc.entry.text) diff --git a/components/esp_system/ld/esp32c6/sections.ld.in b/components/esp_system/ld/esp32c6/sections.ld.in index ff44c831b6..c9be5aae30 100644 --- a/components/esp_system/ld/esp32c6/sections.ld.in +++ b/components/esp_system/ld/esp32c6/sections.ld.in @@ -17,8 +17,12 @@ SECTIONS */ .rtc.text : { - ALIGNED_SYMBOL(4, _rtc_fast_start) - ALIGNED_SYMBOL(4, _rtc_text_start) + /* Align the start of RTC code region as per PMP granularity + * this ensures we do not overwrite the permissions for the previous + * region (ULP mem) regardless of its end alignment + */ + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_fast_start) + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_text_start) *(.rtc.entry.text) @@ -27,7 +31,10 @@ SECTIONS *rtc_wake_stub*.*(.text .text.*) *(.rtc_text_end_test) - ALIGNED_SYMBOL(4, _rtc_text_end) + /* Align the end of RTC code region as per PMP granularity */ + . = ALIGN(_esp_pmp_align_size); + + _rtc_text_end = ABSOLUTE(.); } > lp_ram_seg /** diff --git a/components/esp_system/ld/esp32c61/sections.ld.in b/components/esp_system/ld/esp32c61/sections.ld.in index 54673f7a82..de877d27b7 100644 --- a/components/esp_system/ld/esp32c61/sections.ld.in +++ b/components/esp_system/ld/esp32c61/sections.ld.in @@ -17,8 +17,12 @@ SECTIONS */ .rtc.text : { - ALIGNED_SYMBOL(4, _rtc_fast_start) - ALIGNED_SYMBOL(4, _rtc_text_start) + /* Align the start of RTC code region as per PMP granularity + * this ensures we do not overwrite the permissions for any potential previous + * region regardless of its end alignment + */ + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_fast_start) + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_text_start) *(.rtc.entry.text) @@ -27,6 +31,9 @@ SECTIONS *rtc_wake_stub*.*(.text .text.*) *(.rtc_text_end_test) + /* Align the end of RTC code region as per PMP granularity */ + . = ALIGN(_esp_pmp_align_size); + _rtc_text_end = ABSOLUTE(.); } > lp_ram_seg diff --git a/components/esp_system/ld/esp32h2/sections.ld.in b/components/esp_system/ld/esp32h2/sections.ld.in index ff44c831b6..57e8a7c513 100644 --- a/components/esp_system/ld/esp32h2/sections.ld.in +++ b/components/esp_system/ld/esp32h2/sections.ld.in @@ -17,8 +17,12 @@ SECTIONS */ .rtc.text : { - ALIGNED_SYMBOL(4, _rtc_fast_start) - ALIGNED_SYMBOL(4, _rtc_text_start) + /* Align the start of RTC code region as per PMP granularity + * this ensures we do not overwrite the permissions for any potential previous + * region regardless of its end alignment + */ + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_fast_start) + ALIGNED_SYMBOL(_esp_pmp_align_size, _rtc_text_start) *(.rtc.entry.text) @@ -27,7 +31,10 @@ SECTIONS *rtc_wake_stub*.*(.text .text.*) *(.rtc_text_end_test) - ALIGNED_SYMBOL(4, _rtc_text_end) + /* Align the end of RTC code region as per PMP granularity */ + . = ALIGN(_esp_pmp_align_size); + + _rtc_text_end = ABSOLUTE(.); } > lp_ram_seg /** diff --git a/components/soc/esp32c6/include/soc/Kconfig.soc_caps.in b/components/soc/esp32c6/include/soc/Kconfig.soc_caps.in index 14916f9cab..aea0417790 100644 --- a/components/soc/esp32c6/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32c6/include/soc/Kconfig.soc_caps.in @@ -419,6 +419,10 @@ config SOC_CPU_IDRAM_SPLIT_USING_PMP bool default y +config SOC_CPU_PMP_REGION_GRANULARITY + int + default 4 + config SOC_DS_SIGNATURE_MAX_BIT_LEN int default 3072 diff --git a/components/soc/esp32c6/include/soc/soc_caps.h b/components/soc/esp32c6/include/soc/soc_caps.h index 6f32679bf4..31f0ceefe5 100644 --- a/components/soc/esp32c6/include/soc/soc_caps.h +++ b/components/soc/esp32c6/include/soc/soc_caps.h @@ -155,6 +155,7 @@ #define SOC_CPU_HAS_PMA 1 #define SOC_CPU_IDRAM_SPLIT_USING_PMP 1 +#define SOC_CPU_PMP_REGION_GRANULARITY 4 // TODO: IDF-5360 (Copy from esp32c3, need check) /*-------------------------- DIGITAL SIGNATURE CAPS ----------------------------------------*/ diff --git a/components/soc/esp32c61/include/soc/Kconfig.soc_caps.in b/components/soc/esp32c61/include/soc/Kconfig.soc_caps.in index 45fc5a8cb9..bf1ef06560 100644 --- a/components/soc/esp32c61/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32c61/include/soc/Kconfig.soc_caps.in @@ -135,6 +135,10 @@ config SOC_CPU_IDRAM_SPLIT_USING_PMP bool default y +config SOC_CPU_PMP_REGION_GRANULARITY + int + default 128 + config SOC_DS_SIGNATURE_MAX_BIT_LEN int default 3072 diff --git a/components/soc/esp32c61/include/soc/soc_caps.h b/components/soc/esp32c61/include/soc/soc_caps.h index d0dbd4d01e..2846054586 100644 --- a/components/soc/esp32c61/include/soc/soc_caps.h +++ b/components/soc/esp32c61/include/soc/soc_caps.h @@ -153,6 +153,8 @@ #define SOC_CPU_HAS_PMA 1 #define SOC_CPU_IDRAM_SPLIT_USING_PMP 1 +#define SOC_CPU_PMP_REGION_GRANULARITY 128 // TODO IDF-9580 check when doing PMP bringup + /*-------------------------- DIGITAL SIGNATURE CAPS ----------------------------------------*/ //TODO: [ESP32C61] IDF-9325 (Copy from esp32c6, need check) diff --git a/components/soc/esp32h2/include/soc/Kconfig.soc_caps.in b/components/soc/esp32h2/include/soc/Kconfig.soc_caps.in index de63925546..6d730810e2 100644 --- a/components/soc/esp32h2/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32h2/include/soc/Kconfig.soc_caps.in @@ -407,6 +407,10 @@ config SOC_CPU_IDRAM_SPLIT_USING_PMP bool default y +config SOC_CPU_PMP_REGION_GRANULARITY + int + default 4 + config SOC_MMU_PAGE_SIZE_CONFIGURABLE bool default y diff --git a/components/soc/esp32h2/include/soc/soc_caps.h b/components/soc/esp32h2/include/soc/soc_caps.h index 79b8323a75..d8d3c4bc1a 100644 --- a/components/soc/esp32h2/include/soc/soc_caps.h +++ b/components/soc/esp32h2/include/soc/soc_caps.h @@ -152,6 +152,7 @@ #define SOC_CPU_HAS_PMA 1 #define SOC_CPU_IDRAM_SPLIT_USING_PMP 1 +#define SOC_CPU_PMP_REGION_GRANULARITY 4 /*-------------------------- MMU CAPS ----------------------------------------*/ #define SOC_MMU_PAGE_SIZE_CONFIGURABLE (1) diff --git a/components/ulp/test_apps/.build-test-rules.yml b/components/ulp/test_apps/.build-test-rules.yml index 50e056d903..b601d66f14 100644 --- a/components/ulp/test_apps/.build-test-rules.yml +++ b/components/ulp/test_apps/.build-test-rules.yml @@ -3,10 +3,6 @@ components/ulp/test_apps/lp_core: disable: - if: SOC_LP_CORE_SUPPORTED != 1 - disable_test: - - if: IDF_TARGET == "esp32c5" - temporary: true - reason: test not pass, should be re-enable # TODO: [ESP32C5] IDF-10336 depends_components: - ulp diff --git a/components/ulp/test_apps/lp_core/pytest_lp_core.py b/components/ulp/test_apps/lp_core/pytest_lp_core.py index 370632de83..0c8a36668c 100644 --- a/components/ulp/test_apps/lp_core/pytest_lp_core.py +++ b/components/ulp/test_apps/lp_core/pytest_lp_core.py @@ -4,7 +4,7 @@ import pytest from pytest_embedded import Dut -# @pytest.mark.esp32c5 # TODO: [ESP32C5] IDF-10336 +@pytest.mark.esp32c5 @pytest.mark.esp32c6 @pytest.mark.esp32p4 @pytest.mark.generic