Merge branch 'fix/pmp_idcache_reg_prot_v5.1' into 'release/v5.1'

fix(esp_hw_support): Fix the I/DCACHE region PMP protection (v5.1) See merge request espressif/esp-idf!29357
espressif · Mar 8, 2024 · 590c650 · 590c650
2 parents 50a2d34 + 9ed7c8b
commit 590c650
Show file tree

Hide file tree

Showing 6 changed files with 180 additions and 28 deletions.
diff --git a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c
@@ -13,17 +13,45 @@
 #ifdef BOOTLOADER_BUILD
 // Without L bit set
 #define CONDITIONAL_NONE        0x0
+#define CONDITIONAL_R           PMP_R
 #define CONDITIONAL_RX          PMP_R | PMP_X
 #define CONDITIONAL_RW          PMP_R | PMP_W
 #define CONDITIONAL_RWX         PMP_R | PMP_W | PMP_X
 #else
 // With L bit set
 #define CONDITIONAL_NONE        NONE
+#define CONDITIONAL_R           R
 #define CONDITIONAL_RX          RX
 #define CONDITIONAL_RW          RW
 #define CONDITIONAL_RWX         RWX
 #endif
 
+#define ALIGN_UP_TO_MMU_PAGE_SIZE(addr) (((addr) + (SOC_MMU_PAGE_SIZE) - 1) & ~((SOC_MMU_PAGE_SIZE) - 1))
+#define ALIGN_DOWN_TO_MMU_PAGE_SIZE(addr)  ((addr) & ~((SOC_MMU_PAGE_SIZE) - 1))
+
+/**
+ * @brief Generate the PMP address field value for PMPCFG.A == NAPOT
+ *
+ * NOTE: Here, (end-start) must be a power of 2 size and start must
+ * be aligned to this size. This API returns UINT32_MAX on failing
+ * these conditions, which when plugged into the PMP entry registers
+ * does nothing. This skips the corresponding region's protection.
+ *
+ * @param start      Region starting address
+ * @param end        Region ending address
+ *
+ * @return uint32_t PMP address field value
+ */
+static inline uint32_t pmpaddr_napot(uint32_t start, uint32_t end)
+{
+    uint32_t size = end - start;
+    if ((size & (size - 1)) || (start % size)) {
+        return UINT32_MAX;
+    }
+
+    return start | ((size - 1) >> 1);
+}
+
 static void esp_cpu_configure_invalid_regions(void)
 {
     const unsigned PMA_NONE                            = PMA_L | PMA_EN;
@@ -97,7 +125,7 @@ void esp_cpu_configure_region_protection(void)
      *      We also lock these entries so the R/W/X permissions are enforced even for machine mode
      */
     const unsigned NONE    = PMP_L;
-    const unsigned R       = PMP_L | PMP_R;
+    __attribute__((unused)) const unsigned R       = PMP_L | PMP_R;
     const unsigned RW      = PMP_L | PMP_R | PMP_W;
     const unsigned RX      = PMP_L | PMP_R | PMP_X;
     const unsigned RWX     = PMP_L | PMP_R | PMP_W | PMP_X;
@@ -116,15 +144,10 @@ void esp_cpu_configure_region_protection(void)
     PMP_ENTRY_SET(0, pmpaddr0, PMP_NAPOT | RWX);
     _Static_assert(SOC_CPU_SUBSYSTEM_LOW < SOC_CPU_SUBSYSTEM_HIGH, "Invalid CPU subsystem region");
 
-    // 2.1 I-ROM
+    // 2.1 I/D-ROM
     PMP_ENTRY_SET(1, SOC_IROM_MASK_LOW, NONE);
     PMP_ENTRY_SET(2, SOC_IROM_MASK_HIGH, PMP_TOR | RX);
-    _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I-ROM region");
-
-    // 2.2 D-ROM
-    PMP_ENTRY_SET(3, SOC_DROM_MASK_LOW, NONE);
-    PMP_ENTRY_SET(4, SOC_DROM_MASK_HIGH, PMP_TOR | R);
-    _Static_assert(SOC_DROM_MASK_LOW < SOC_DROM_MASK_HIGH, "Invalid D-ROM region");
+    _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I/D-ROM region");
 
     if (esp_cpu_dbgr_is_attached()) {
         // Anti-FI check that cpu is really in ocd mode
@@ -155,15 +178,30 @@ void esp_cpu_configure_region_protection(void)
 #endif
     }
 
+#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD
+    extern int _instruction_reserved_end;
+    extern int _rodata_reserved_start;
+    extern int _rodata_reserved_end;
+
+    const uint32_t irom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_instruction_reserved_end));
+    const uint32_t drom_resv_start = ALIGN_DOWN_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_start));
+    const uint32_t drom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_end));
+
     // 4. I_Cache (flash)
-    const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH);
+    PMP_ENTRY_CFG_RESET(8);
+    const uint32_t pmpaddr8 = pmpaddr_napot(SOC_IROM_LOW, irom_resv_end);
     PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | RX);
-    _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I_Cache region");
 
     // 5. D_Cache (flash)
-    const uint32_t pmpaddr9 = PMPADDR_NAPOT(SOC_DROM_LOW, SOC_DROM_HIGH);
+    PMP_ENTRY_CFG_RESET(9);
+    const uint32_t pmpaddr9 = pmpaddr_napot(drom_resv_start, drom_resv_end);
     PMP_ENTRY_SET(9, pmpaddr9, PMP_NAPOT | R);
-    _Static_assert(SOC_DROM_LOW < SOC_DROM_HIGH, "Invalid D_Cache region");
+#else
+    // 4. I_Cache / D_Cache (flash)
+    const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH);
+    PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | CONDITIONAL_RX);
+    _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I/D_Cache region");
+#endif
 
     // 6. LP memory
 #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD

diff --git a/components/esp_hw_support/port/esp32h2/cpu_region_protect.c b/components/esp_hw_support/port/esp32h2/cpu_region_protect.c
@@ -13,17 +13,45 @@
 #ifdef BOOTLOADER_BUILD
 // Without L bit set
 #define CONDITIONAL_NONE        0x0
+#define CONDITIONAL_R           PMP_R
 #define CONDITIONAL_RX          PMP_R | PMP_X
 #define CONDITIONAL_RW          PMP_R | PMP_W
 #define CONDITIONAL_RWX         PMP_R | PMP_W | PMP_X
 #else
 // With L bit set
 #define CONDITIONAL_NONE        NONE
+#define CONDITIONAL_R           R
 #define CONDITIONAL_RX          RX
 #define CONDITIONAL_RW          RW
 #define CONDITIONAL_RWX         RWX
 #endif
 
+#define ALIGN_UP_TO_MMU_PAGE_SIZE(addr) (((addr) + (SOC_MMU_PAGE_SIZE) - 1) & ~((SOC_MMU_PAGE_SIZE) - 1))
+#define ALIGN_DOWN_TO_MMU_PAGE_SIZE(addr)  ((addr) & ~((SOC_MMU_PAGE_SIZE) - 1))
+
+/**
+ * @brief Generate the PMP address field value for PMPCFG.A == NAPOT
+ *
+ * NOTE: Here, (end-start) must be a power of 2 size and start must
+ * be aligned to this size. This API returns UINT32_MAX on failing
+ * these conditions, which when plugged into the PMP entry registers
+ * does nothing. This skips the corresponding region's protection.
+ *
+ * @param start      Region starting address
+ * @param end        Region ending address
+ *
+ * @return uint32_t PMP address field value
+ */
+static inline uint32_t pmpaddr_napot(uint32_t start, uint32_t end)
+{
+    uint32_t size = end - start;
+    if ((size & (size - 1)) || (start % size)) {
+        return UINT32_MAX;
+    }
+
+    return start | ((size - 1) >> 1);
+}
+
 static void esp_cpu_configure_invalid_regions(void)
 {
     const unsigned PMA_NONE                            = PMA_L | PMA_EN;
@@ -97,7 +125,7 @@ void esp_cpu_configure_region_protection(void)
      *      We also lock these entries so the R/W/X permissions are enforced even for machine mode
      */
     const unsigned NONE    = PMP_L;
-    const unsigned R       = PMP_L | PMP_R;
+    __attribute__((unused)) const unsigned R       = PMP_L | PMP_R;
     const unsigned RW      = PMP_L | PMP_R | PMP_W;
     const unsigned RX      = PMP_L | PMP_R | PMP_X;
     const unsigned RWX     = PMP_L | PMP_R | PMP_W | PMP_X;
@@ -116,15 +144,10 @@ void esp_cpu_configure_region_protection(void)
     PMP_ENTRY_SET(0, pmpaddr0, PMP_NAPOT | RWX);
     _Static_assert(SOC_CPU_SUBSYSTEM_LOW < SOC_CPU_SUBSYSTEM_HIGH, "Invalid CPU subsystem region");
 
-    // 2.1 I-ROM
+    // 2.1 I/D-ROM
     PMP_ENTRY_SET(1, SOC_IROM_MASK_LOW, NONE);
     PMP_ENTRY_SET(2, SOC_IROM_MASK_HIGH, PMP_TOR | RX);
-    _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I-ROM region");
-
-    // 2.2 D-ROM
-    PMP_ENTRY_SET(3, SOC_DROM_MASK_LOW, NONE);
-    PMP_ENTRY_SET(4, SOC_DROM_MASK_HIGH, PMP_TOR | R);
-    _Static_assert(SOC_DROM_MASK_LOW < SOC_DROM_MASK_HIGH, "Invalid D-ROM region");
+    _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I/D-ROM region");
 
     if (esp_cpu_dbgr_is_attached()) {
         // Anti-FI check that cpu is really in ocd mode
@@ -155,15 +178,30 @@ void esp_cpu_configure_region_protection(void)
 #endif
     }
 
+#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD
+    extern int _instruction_reserved_end;
+    extern int _rodata_reserved_start;
+    extern int _rodata_reserved_end;
+
+    const uint32_t irom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_instruction_reserved_end));
+    const uint32_t drom_resv_start = ALIGN_DOWN_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_start));
+    const uint32_t drom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_end));
+
     // 4. I_Cache (flash)
-    const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH);
+    PMP_ENTRY_CFG_RESET(8);
+    const uint32_t pmpaddr8 = pmpaddr_napot(SOC_IROM_LOW, irom_resv_end);
     PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | RX);
-    _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I_Cache region");
 
     // 5. D_Cache (flash)
-    const uint32_t pmpaddr9 = PMPADDR_NAPOT(SOC_DROM_LOW, SOC_DROM_HIGH);
+    PMP_ENTRY_CFG_RESET(9);
+    const uint32_t pmpaddr9 = pmpaddr_napot(drom_resv_start, drom_resv_end);
     PMP_ENTRY_SET(9, pmpaddr9, PMP_NAPOT | R);
-    _Static_assert(SOC_DROM_LOW < SOC_DROM_HIGH, "Invalid D_Cache region");
+#else
+    // 4. I_Cache / D_Cache (flash)
+    const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH);
+    PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | CONDITIONAL_RX);
+    _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I/D_Cache region");
+#endif
 
     // 6. LP memory
 #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD

diff --git a/tools/test_apps/system/panic/main/include/test_memprot.h b/tools/test_apps/system/panic/main/include/test_memprot.h
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -42,6 +42,11 @@ void test_rtc_slow_reg1_execute_violation(void);
 
 void test_rtc_slow_reg2_execute_violation(void);
 
+void test_irom_reg_write_violation(void);
+
+void test_drom_reg_write_violation(void);
+
+void test_drom_reg_execute_violation(void);
 
 #ifdef __cplusplus
 }

diff --git a/tools/test_apps/system/panic/main/test_app_main.c b/tools/test_apps/system/panic/main/test_app_main.c
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -134,6 +134,12 @@ void app_main(void)
     HANDLE_TEST(test_name, test_rtc_slow_reg2_execute_violation);
 #endif
 
+#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT
+    HANDLE_TEST(test_name, test_irom_reg_write_violation);
+    HANDLE_TEST(test_name, test_drom_reg_write_violation);
+    HANDLE_TEST(test_name, test_drom_reg_execute_violation);
+#endif
+
 #endif
 
     die("Unknown test name");

diff --git a/tools/test_apps/system/panic/main/test_memprot.c b/tools/test_apps/system/panic/main/test_memprot.c
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -15,7 +15,7 @@
 #include "soc/soc.h"
 
 #include "test_memprot.h"
-
+#include "sdkconfig.h"
 
 #define RND_VAL     (0xA5A5A5A5)
 #define SPIN_ITER   (16)
@@ -213,3 +213,36 @@ static void __attribute__((constructor)) test_print_rtc_var_func(void)
     printf("foo_s: %p | var_s: %p\n", &foo_s, &var_s);
 #endif
 }
+
+
+/* ---------------------------------------------------- I/D Cache (Flash) Violation Checks ---------------------------------------------------- */
+
+#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT
+static const uint16_t foo_buf[8] = {
+    0x0001, 0x0001, 0x0001, 0x0001,
+    0x0001, 0x0001, 0x0001, 0x0001,
+};
+
+void test_irom_reg_write_violation(void)
+{
+    extern int _instruction_reserved_end;
+    uint32_t *test_addr = (uint32_t *)((uint32_t)(&_instruction_reserved_end - 0x100));
+    printf("Flash (IROM): Write operation | Address: %p\n", test_addr);
+    *test_addr = RND_VAL;
+}
+
+void test_drom_reg_write_violation(void)
+{
+    uint32_t *test_addr = (uint32_t *)((uint32_t)(foo_buf));
+    printf("Flash (DROM): Write operation | Address: %p\n", test_addr);
+    *test_addr = RND_VAL;
+}
+
+void test_drom_reg_execute_violation(void)
+{
+    printf("Flash (DROM): Execute operation | Address: %p\n", foo_buf);
+    void (*func_ptr)(void);
+    func_ptr = (void(*)(void))foo_buf;
+    func_ptr();
+}
+#endif
diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py
@@ -537,6 +537,11 @@ def test_panic_delay(dut: PanicTestDut) -> None:
     pytest.param('memprot_esp32s2', marks=[pytest.mark.esp32s2]),
 ]
 
+CONFIGS_MEMPROT_FLASH_IDROM = [
+    pytest.param('memprot_esp32c6', marks=[pytest.mark.esp32c6]),
+    pytest.param('memprot_esp32h2', marks=[pytest.mark.esp32h2])
+]
+
 
 @pytest.mark.parametrize('config', CONFIGS_MEMPROT_DCACHE, indirect=True)
 @pytest.mark.generic
@@ -752,6 +757,33 @@ def test_rtc_slow_reg2_execute_violation(dut: PanicTestDut, test_func_name: str)
     dut.expect_corrupted_backtrace()
 
 
+@pytest.mark.parametrize('config', CONFIGS_MEMPROT_FLASH_IDROM, indirect=True)
+@pytest.mark.generic
+def test_irom_reg_write_violation(dut: PanicTestDut, test_func_name: str) -> None:
+    dut.run_test_func(test_func_name)
+    if dut.target == 'esp32c6':
+        dut.expect_gme('Store access fault')
+    elif dut.target == 'esp32h2':
+        dut.expect_gme('Cache error')
+    dut.expect_reg_dump(0)
+
+
+@pytest.mark.parametrize('config', CONFIGS_MEMPROT_FLASH_IDROM, indirect=True)
+@pytest.mark.generic
+def test_drom_reg_write_violation(dut: PanicTestDut, test_func_name: str) -> None:
+    dut.run_test_func(test_func_name)
+    dut.expect_gme('Store access fault')
+    dut.expect_reg_dump(0)
+
+
+@pytest.mark.parametrize('config', CONFIGS_MEMPROT_FLASH_IDROM, indirect=True)
+@pytest.mark.generic
+def test_drom_reg_execute_violation(dut: PanicTestDut, test_func_name: str) -> None:
+    dut.run_test_func(test_func_name)
+    dut.expect_gme('Instruction access fault')
+    dut.expect_reg_dump(0)
+
+
 @pytest.mark.esp32
 @pytest.mark.parametrize('config', ['gdbstub_coredump'], indirect=True)
 def test_gdbstub_coredump(dut: PanicTestDut) -> None: