From 0868604664c7f926a1fe655a9f912a93d61f6ebb Mon Sep 17 00:00:00 2001 From: "harshal.patil" Date: Fri, 31 May 2024 22:26:59 +0530 Subject: [PATCH] fix(esp_hw_support): Fix incorrect PMA configuration for ESP32-P4 - As the PMA entry that made some memory regions cacheable was assigned the highest priority, some intermediate inaccessible memory regions bypassed protection. - Added tests for the same - Verified that even after changing the priority of the PMA entry, a write operation at SOC_IRAM_LOW + 0x40 (a random RAM cached address) still needs the same number (29) of CPU cycles. --- .../port/esp32p4/cpu_region_protect.c | 63 +++++++++---------- .../system/panic/main/include/test_memprot.h | 4 ++ .../system/panic/main/test_app_main.c | 4 ++ .../system/panic/main/test_memprot.c | 18 ++++++ tools/test_apps/system/panic/pytest_panic.py | 24 +++++++ 5 files changed, 81 insertions(+), 32 deletions(-) diff --git a/components/esp_hw_support/port/esp32p4/cpu_region_protect.c b/components/esp_hw_support/port/esp32p4/cpu_region_protect.c index cfed9a0f0c..acf88eae27 100644 --- a/components/esp_hw_support/port/esp32p4/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32p4/cpu_region_protect.c @@ -38,39 +38,38 @@ static void esp_cpu_configure_invalid_regions(void) __attribute__((unused)) const unsigned PMA_RX = PMA_L | PMA_EN | PMA_R | PMA_X; __attribute__((unused)) const unsigned PMA_RWX = PMA_L | PMA_EN | PMA_R | PMA_W | PMA_X; - // 0. Special case - This whitelists the External flash/RAM, HP ROM and HP L2MEM regions and make them cacheable. + // 0. Gap at bottom of address space + PMA_ENTRY_SET_NAPOT(0, 0, SOC_CPU_SUBSYSTEM_LOW, PMA_NAPOT | PMA_NONE); + + // 1. Gap between CPU subsystem region & HP TCM + PMA_ENTRY_SET_TOR(1, SOC_CPU_SUBSYSTEM_HIGH, PMA_NONE); + PMA_ENTRY_SET_TOR(2, SOC_TCM_LOW, PMA_TOR | PMA_NONE); + + // 2. Gap between HP TCM and CPU Peripherals + PMA_ENTRY_SET_TOR(3, SOC_TCM_HIGH, PMA_NONE); + PMA_ENTRY_SET_TOR(4, CPU_PERIPH_LOW, PMA_TOR | PMA_NONE); + + // 3. Gap between CPU Peripherals and I_Cache + PMA_ENTRY_SET_TOR(5, CPU_PERIPH_HIGH, PMA_NONE); + PMA_ENTRY_SET_TOR(6, SOC_IROM_LOW, PMA_TOR | PMA_NONE); + + // 4. Gap between I_Cache and external memory range + PMA_ENTRY_SET_NAPOT(7, SOC_DROM_HIGH, SOC_EXTRAM_LOW - SOC_DROM_HIGH, PMA_NAPOT | PMA_NONE); + + // 5. Gap between external memory and ROM + PMA_ENTRY_SET_TOR(8, SOC_EXTRAM_HIGH, PMA_NONE); + PMA_ENTRY_SET_TOR(9, SOC_IROM_MASK_LOW, PMA_TOR | PMA_NONE); + + // 6. Gap between ROM and internal memory + PMA_ENTRY_SET_TOR(10, SOC_IROM_MASK_HIGH, PMA_NONE); + PMA_ENTRY_SET_TOR(11, SOC_IRAM_LOW, PMA_TOR | PMA_NONE); + + // 7. Gap between internal memory and HP peripherals + PMA_ENTRY_SET_NAPOT(12, SOC_DRAM_HIGH, SOC_PERIPHERAL_LOW - SOC_DRAM_HIGH, PMA_NAPOT | PMA_NONE); + + // 8. Special case - This whitelists the External flash/RAM, HP ROM and HP L2MEM regions and make them cacheable. // At the startup, this is done using PMA entry 15 by the ROM code. - // We are reconfiguring and making it the highest priority entry here. - PMA_ENTRY_SET_NAPOT(0, SOC_IROM_LOW, SOC_PERIPHERAL_LOW - SOC_IROM_LOW, PMA_NAPOT | PMA_RWX); - - // 1. Gap at bottom of address space - PMA_ENTRY_SET_NAPOT(1, 0, SOC_CPU_SUBSYSTEM_LOW, PMA_NAPOT | PMA_NONE); - - // 2. Gap between CPU subsystem region & HP TCM - PMA_ENTRY_SET_TOR(2, SOC_CPU_SUBSYSTEM_HIGH, PMA_NONE); - PMA_ENTRY_SET_TOR(3, SOC_TCM_LOW, PMA_TOR | PMA_NONE); - - // 3. Gap between HP TCM and CPU Peripherals - PMA_ENTRY_SET_TOR(4, SOC_TCM_HIGH, PMA_NONE); - PMA_ENTRY_SET_TOR(5, CPU_PERIPH_LOW, PMA_TOR | PMA_NONE); - - // 4. Gap between CPU Peripherals and I_Cache - PMA_ENTRY_SET_TOR(6, CPU_PERIPH_HIGH, PMA_NONE); - PMA_ENTRY_SET_TOR(7, SOC_IROM_LOW, PMA_TOR | PMA_NONE); - - // 5. Gap between I_Cache and external memory range - PMA_ENTRY_SET_NAPOT(8, SOC_DROM_HIGH, SOC_EXTRAM_LOW - SOC_DROM_HIGH, PMA_NAPOT | PMA_NONE); - - // 6. Gap between external memory and ROM - PMA_ENTRY_SET_TOR(9, SOC_EXTRAM_HIGH, PMA_NONE); - PMA_ENTRY_SET_TOR(10, SOC_IROM_MASK_LOW, PMA_TOR | PMA_NONE); - - // 7. Gap between ROM and internal memory - PMA_ENTRY_SET_TOR(11, SOC_IROM_MASK_HIGH, PMA_NONE); - PMA_ENTRY_SET_TOR(12, SOC_IRAM_LOW, PMA_TOR | PMA_NONE); - - // 8. Gap between internal memory and HP peripherals - PMA_ENTRY_SET_NAPOT(13, SOC_DRAM_HIGH, SOC_PERIPHERAL_LOW - SOC_DRAM_HIGH, PMA_NAPOT | PMA_NONE); + PMA_ENTRY_SET_NAPOT(13, SOC_IROM_LOW, SOC_PERIPHERAL_LOW - SOC_IROM_LOW, PMA_NAPOT | PMA_RWX); // 9. Gap between Uncacheable L2 Mem and end of address space PMA_ENTRY_SET_TOR(14, CACHE_LL_L2MEM_NON_CACHE_ADDR(SOC_DRAM_HIGH), PMA_NONE); diff --git a/tools/test_apps/system/panic/main/include/test_memprot.h b/tools/test_apps/system/panic/main/include/test_memprot.h index d21d795f60..6b8cfde2df 100644 --- a/tools/test_apps/system/panic/main/include/test_memprot.h +++ b/tools/test_apps/system/panic/main/include/test_memprot.h @@ -48,6 +48,10 @@ void test_drom_reg_write_violation(void); void test_drom_reg_execute_violation(void); +void test_invalid_memory_region_write_violation(void); + +void test_invalid_memory_region_execute_violation(void); + #ifdef __cplusplus } #endif diff --git a/tools/test_apps/system/panic/main/test_app_main.c b/tools/test_apps/system/panic/main/test_app_main.c index 1a387f0018..7fb99a273d 100644 --- a/tools/test_apps/system/panic/main/test_app_main.c +++ b/tools/test_apps/system/panic/main/test_app_main.c @@ -155,6 +155,10 @@ void app_main(void) HANDLE_TEST(test_name, test_drom_reg_execute_violation); #endif +#ifdef CONFIG_SOC_CPU_HAS_PMA + HANDLE_TEST(test_name, test_invalid_memory_region_write_violation); + HANDLE_TEST(test_name, test_invalid_memory_region_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 index cfefa415e6..73885f1e0c 100644 --- a/tools/test_apps/system/panic/main/test_memprot.c +++ b/tools/test_apps/system/panic/main/test_memprot.c @@ -246,3 +246,21 @@ void test_drom_reg_execute_violation(void) func_ptr(); } #endif + +#ifdef CONFIG_SOC_CPU_HAS_PMA +void test_invalid_memory_region_write_violation(void) +{ + uint32_t *test_addr = (uint32_t *)((uint32_t)(SOC_DRAM_HIGH + 0x40)); + printf("Write operation | Address: %p\n", test_addr); + *test_addr = RND_VAL; + printf("%ld\n", *test_addr); +} + +void test_invalid_memory_region_execute_violation(void) +{ + void (*func_ptr)(void); + func_ptr = (void(*)(void))(SOC_DRAM_HIGH + 0x40); + printf("Execute operation | Address: %p\n", func_ptr); + func_ptr(); +} +#endif diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py index 90154a40d2..e3193670ad 100644 --- a/tools/test_apps/system/panic/pytest_panic.py +++ b/tools/test_apps/system/panic/pytest_panic.py @@ -588,6 +588,12 @@ CONFIGS_MEMPROT_FLASH_IDROM = [ pytest.param('memprot_esp32p4', marks=[pytest.mark.esp32p4]) ] +CONFIGS_MEMPROT_INVALID_REGION_PROTECTION_USING_PMA = [ + pytest.param('memprot_esp32c6', marks=[pytest.mark.esp32c6]), + pytest.param('memprot_esp32h2', marks=[pytest.mark.esp32h2]), + pytest.param('memprot_esp32p4', marks=[pytest.mark.esp32p4]) +] + @pytest.mark.parametrize('config', CONFIGS_MEMPROT_DCACHE, indirect=True) @pytest.mark.generic @@ -851,6 +857,24 @@ def test_drom_reg_execute_violation(dut: PanicTestDut, test_func_name: str) -> N dut.expect_cpu_reset() +@pytest.mark.parametrize('config', CONFIGS_MEMPROT_INVALID_REGION_PROTECTION_USING_PMA, indirect=True) +@pytest.mark.generic +def test_invalid_memory_region_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) + dut.expect_cpu_reset() + + +@pytest.mark.parametrize('config', CONFIGS_MEMPROT_INVALID_REGION_PROTECTION_USING_PMA, indirect=True) +@pytest.mark.generic +def test_invalid_memory_region_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) + dut.expect_cpu_reset() + + @pytest.mark.esp32 @pytest.mark.generic @pytest.mark.parametrize('config', ['gdbstub_coredump'], indirect=True)