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
darthcloud · Jul 4, 2024 · 41d39a4 · 41d39a4
1 parent 7696f0f
commit 41d39a4
Show file tree

Hide file tree

Showing 14 changed files with 62 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -4,7 +4,7 @@
 from pytest_embedded import Dut
 
 
-# @pytest.mark.esp32c5  # TODO: [ESP32C5] IDF-10336
+@pytest.mark.esp32c5
 @pytest.mark.esp32c6
 @pytest.mark.esp32p4
 @pytest.mark.generic