From ed0a1f7b52a649f040ca12127cbe2562c47fecd4 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 20 Feb 2023 15:41:48 +0530 Subject: [PATCH] esp32c6: Fix incorrect PMP configuration - Enable pytest memprot tests for C6 --- .../port/esp32c6/cpu_region_protect.c | 72 +++++++++++-------- components/riscv/include/riscv/csr.h | 5 ++ components/soc/esp32c6/include/soc/soc.h | 3 + tools/test_apps/.build-test-rules.yml | 2 +- tools/test_apps/system/panic/pytest_panic.py | 26 ++++--- .../system/panic/sdkconfig.ci.memprot_esp32c6 | 8 +++ .../test_apps/system/panic/sdkconfig.defaults | 3 + 7 files changed, 81 insertions(+), 38 deletions(-) create mode 100644 tools/test_apps/system/panic/sdkconfig.ci.memprot_esp32c6 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 e293280cd3..b44d6422db 100644 --- a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c @@ -18,18 +18,18 @@ #define CONDITIONAL_RWX PMP_R | PMP_W | PMP_X #else // With L bit set -#define CONDITIONAL_NONE PMP_NONE -#define CONDITIONAL_RX PMP_RX -#define CONDITIONAL_RW PMP_RW -#define CONDITIONAL_RWX PMP_RWX +#define CONDITIONAL_NONE NONE +#define CONDITIONAL_RX RX +#define CONDITIONAL_RW RW +#define CONDITIONAL_RWX RWX #endif static void esp_cpu_configure_invalid_regions(void) { - const unsigned PMA_NONE = PMA_EN; - __attribute__((unused)) const unsigned PMA_RW = PMA_EN | PMA_R | PMA_W; - __attribute__((unused)) const unsigned PMA_RX = PMA_EN | PMA_R | PMA_X; - __attribute__((unused)) const unsigned PMA_RWX = PMA_EN | PMA_R | PMA_W | PMA_X; + const unsigned PMA_NONE = PMA_L | PMA_EN; + __attribute__((unused)) const unsigned PMA_RW = PMA_L | PMA_EN | PMA_R | PMA_W; + __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; // 1. Gap at bottom of address space PMA_ENTRY_SET_TOR(0, SOC_DEBUG_LOW, PMA_TOR | PMA_NONE); @@ -96,10 +96,11 @@ void esp_cpu_configure_region_protection(void) * We set PMP to cover entire valid IRAM and DRAM region. * We also lock these entries so the R/W/X permissions are enforced even for machine mode */ - const unsigned PMP_NONE = PMP_L; - const unsigned PMP_RW = PMP_L | PMP_R | PMP_W; - const unsigned PMP_RX = PMP_L | PMP_R | PMP_X; - const unsigned PMP_RWX = PMP_L | PMP_R | PMP_W | PMP_X; + const unsigned NONE = PMP_L; + 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; // // Configure all the invalid address regions using PMA @@ -112,17 +113,17 @@ void esp_cpu_configure_region_protection(void) // 1. Debug region const uint32_t pmpaddr0 = PMPADDR_NAPOT(SOC_DEBUG_LOW, SOC_DEBUG_HIGH); - PMP_ENTRY_SET(0, pmpaddr0, PMP_NAPOT | PMP_RWX); + PMP_ENTRY_SET(0, pmpaddr0, PMP_NAPOT | RWX); _Static_assert(SOC_DEBUG_LOW < SOC_DEBUG_HIGH, "Invalid CPU debug region"); // 2.1 I-ROM - PMP_ENTRY_SET(1, SOC_IROM_MASK_LOW, PMP_NONE); - PMP_ENTRY_SET(2, SOC_IROM_MASK_HIGH, PMP_TOR | PMP_RX); + 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, PMP_NONE); - PMP_ENTRY_SET(4, SOC_DROM_MASK_HIGH, PMP_TOR | PMP_R); + 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"); if (esp_cpu_dbgr_is_attached()) { @@ -131,15 +132,21 @@ void esp_cpu_configure_region_protection(void) // 5. IRAM and DRAM const uint32_t pmpaddr5 = PMPADDR_NAPOT(SOC_IRAM_LOW, SOC_IRAM_HIGH); - PMP_ENTRY_SET(5, pmpaddr5, PMP_NAPOT | PMP_RWX); + PMP_ENTRY_SET(5, pmpaddr5, PMP_NAPOT | RWX); _Static_assert(SOC_IRAM_LOW < SOC_IRAM_HIGH, "Invalid RAM region"); } else { #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD extern int _iram_end; // 5. IRAM and DRAM - PMP_ENTRY_SET(5, SOC_IRAM_LOW, PMP_NONE); - PMP_ENTRY_SET(6, (int)&_iram_end, PMP_TOR | PMP_RX); - PMP_ENTRY_SET(7, SOC_DRAM_HIGH, PMP_TOR | PMP_RW); + /* 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 + */ + PMP_ENTRY_CFG_RESET(5); + PMP_ENTRY_CFG_RESET(6); + PMP_ENTRY_CFG_RESET(7); + PMP_ENTRY_SET(5, SOC_IRAM_LOW, NONE); + PMP_ENTRY_SET(6, (int)&_iram_end, PMP_TOR | RX); + PMP_ENTRY_SET(7, SOC_DRAM_HIGH, PMP_TOR | RW); #else // 5. IRAM and DRAM const uint32_t pmpaddr5 = PMPADDR_NAPOT(SOC_IRAM_LOW, SOC_IRAM_HIGH); @@ -150,24 +157,33 @@ void esp_cpu_configure_region_protection(void) // 4. I_Cache (flash) const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH); - PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | PMP_RX); + 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_SET(9, pmpaddr9, PMP_NAPOT | PMP_R); + PMP_ENTRY_SET(9, pmpaddr9, PMP_NAPOT | R); _Static_assert(SOC_DROM_LOW < SOC_DROM_HIGH, "Invalid D_Cache region"); // 6. LP memory #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD extern int _rtc_text_end; - PMP_ENTRY_SET(10, SOC_RTC_IRAM_LOW, PMP_NONE); + /* 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 + */ + PMP_ENTRY_CFG_RESET(10); + PMP_ENTRY_CFG_RESET(11); + PMP_ENTRY_CFG_RESET(12); + PMP_ENTRY_CFG_RESET(13); + PMP_ENTRY_SET(10, SOC_RTC_IRAM_LOW, NONE); #if CONFIG_ULP_COPROC_RESERVE_MEM // First part of LP mem is reserved for coprocessor - PMP_ENTRY_SET(11, SOC_RTC_IRAM_LOW + CONFIG_ULP_COPROC_RESERVE_MEM, PMP_TOR | PMP_RW); + PMP_ENTRY_SET(11, SOC_RTC_IRAM_LOW + CONFIG_ULP_COPROC_RESERVE_MEM, PMP_TOR | RW); + PMP_ENTRY_SET(12, (int)&_rtc_text_end, PMP_TOR | RX); + PMP_ENTRY_SET(13, SOC_RTC_IRAM_HIGH, PMP_TOR | RW); #endif //CONFIG_ULP_COPROC_RESERVE_MEM - PMP_ENTRY_SET(12, (int)&_rtc_text_end, PMP_TOR | PMP_RX); - PMP_ENTRY_SET(13, SOC_RTC_IRAM_HIGH, PMP_TOR | PMP_RW); + PMP_ENTRY_SET(11, (int)&_rtc_text_end, PMP_TOR | RX); + PMP_ENTRY_SET(12, SOC_RTC_IRAM_HIGH, PMP_TOR | RW); #else const uint32_t pmpaddr10 = PMPADDR_NAPOT(SOC_RTC_IRAM_LOW, SOC_RTC_IRAM_HIGH); PMP_ENTRY_SET(10, pmpaddr10, PMP_NAPOT | CONDITIONAL_RWX); @@ -177,6 +193,6 @@ void esp_cpu_configure_region_protection(void) // 7. Peripheral addresses const uint32_t pmpaddr14 = PMPADDR_NAPOT(SOC_PERIPHERAL_LOW, SOC_PERIPHERAL_HIGH); - PMP_ENTRY_SET(14, pmpaddr14, PMP_NAPOT | PMP_RW); + PMP_ENTRY_SET(14, pmpaddr14, PMP_NAPOT | RW); _Static_assert(SOC_PERIPHERAL_LOW < SOC_PERIPHERAL_HIGH, "Invalid peripheral region"); } diff --git a/components/riscv/include/riscv/csr.h b/components/riscv/include/riscv/csr.h index 32a93bb634..2d889d1a20 100644 --- a/components/riscv/include/riscv/csr.h +++ b/components/riscv/include/riscv/csr.h @@ -128,6 +128,11 @@ extern "C" { RV_SET_CSR((CSR_PMPCFG0) + (ENTRY)/4, ((CFG)&0xFF) << (ENTRY%4)*8); \ } while(0) +/*Reset all permissions of a particular PMPCFG entry*/ +#define PMP_ENTRY_CFG_RESET(ENTRY) do {\ + RV_CLEAR_CSR((CSR_PMPCFG0) + (ENTRY)/4, (0xFF) << (ENTRY%4)*8); \ + } while(0) + /******************************************************** Trigger Module register fields (Debug specification) ********************************************************/ diff --git a/components/soc/esp32c6/include/soc/soc.h b/components/soc/esp32c6/include/soc/soc.h index 44096ea86d..70e3a947f4 100644 --- a/components/soc/esp32c6/include/soc/soc.h +++ b/components/soc/esp32c6/include/soc/soc.h @@ -179,6 +179,9 @@ #define SOC_DIRAM_DRAM_LOW 0x40800000 #define SOC_DIRAM_DRAM_HIGH 0x40880000 +#define MAP_DRAM_TO_IRAM(addr) (addr) +#define MAP_IRAM_TO_DRAM(addr) (addr) + // Region of memory accessible via DMA. See esp_ptr_dma_capable(). #define SOC_DMA_LOW 0x40800000 #define SOC_DMA_HIGH 0x40880000 diff --git a/tools/test_apps/.build-test-rules.yml b/tools/test_apps/.build-test-rules.yml index 6a975689d0..4f90e9521a 100644 --- a/tools/test_apps/.build-test-rules.yml +++ b/tools/test_apps/.build-test-rules.yml @@ -161,7 +161,7 @@ tools/test_apps/system/panic: enable: - if: INCLUDE_DEFAULT == 1 or IDF_TARGET == "esp32h4" disable_test: - - if: IDF_TARGET not in ["esp32", "esp32s2", "esp32c3", "esp32s3", "esp32c2"] + - if: IDF_TARGET not in ["esp32", "esp32s2", "esp32c3", "esp32s3", "esp32c2", "esp32c6"] temporary: true reason: test app not ported to this target yet diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py index a0900c0c72..005b96e69e 100644 --- a/tools/test_apps/system/panic/pytest_panic.py +++ b/tools/test_apps/system/panic/pytest_panic.py @@ -480,11 +480,12 @@ def test_panic_delay(dut: PanicTestDut) -> None: ######################### # Memprot-related tests are supported only on targets with PMS/PMA peripheral; -# currently ESP32-S2, ESP32-C3 and ESP32-C2 are supported +# currently ESP32-S2, ESP32-C3, ESP32-C2, and ESP32-C6 are supported CONFIGS_MEMPROT_IDRAM = [ pytest.param('memprot_esp32s2', marks=[pytest.mark.esp32s2]), pytest.param('memprot_esp32c3', marks=[pytest.mark.esp32c3]), - pytest.param('memprot_esp32c2', marks=[pytest.mark.esp32c2]) + pytest.param('memprot_esp32c2', marks=[pytest.mark.esp32c2]), + pytest.param('memprot_esp32c6', marks=[pytest.mark.esp32c6]) ] CONFIGS_MEMPROT_DCACHE = [ @@ -494,6 +495,7 @@ CONFIGS_MEMPROT_DCACHE = [ CONFIGS_MEMPROT_RTC_FAST_MEM = [ pytest.param('memprot_esp32s2', marks=[pytest.mark.esp32s2]), pytest.param('memprot_esp32c3', marks=[pytest.mark.esp32c3]), + pytest.param('memprot_esp32c6', marks=[pytest.mark.esp32c6]) ] CONFIGS_MEMPROT_RTC_SLOW_MEM = [ @@ -532,7 +534,7 @@ def test_iram_reg1_write_violation(dut: PanicTestDut, test_func_name: str) -> No dut.expect_backtrace() elif dut.target == 'esp32c3': dut.expect_exact(r'Test error: Test function has returned') - elif dut.target == 'esp32c2': + elif dut.target in ['esp32c2', 'esp32c6']: dut.expect_gme('Store access fault') dut.expect_reg_dump(0) dut.expect_stack_dump() @@ -555,7 +557,7 @@ def test_iram_reg2_write_violation(dut: PanicTestDut, test_func_name: str) -> No dut.expect(r' operation type: (\S+)') dut.expect_reg_dump(0) dut.expect_stack_dump() - elif dut.target == 'esp32c2': + elif dut.target in ['esp32c2', 'esp32c6']: dut.expect_gme('Store access fault') dut.expect_reg_dump(0) dut.expect_stack_dump() @@ -578,7 +580,7 @@ def test_iram_reg3_write_violation(dut: PanicTestDut, test_func_name: str) -> No dut.expect(r' operation type: (\S+)') dut.expect_reg_dump(0) dut.expect_stack_dump() - elif dut.target == 'esp32c2': + elif dut.target in ['esp32c2', 'esp32c6']: dut.expect_gme('Store access fault') dut.expect_reg_dump(0) dut.expect_stack_dump() @@ -603,7 +605,7 @@ def test_iram_reg4_write_violation(dut: PanicTestDut, test_func_name: str) -> No dut.expect(r' operation type: (\S+)') dut.expect_reg_dump(0) dut.expect_stack_dump() - elif dut.target == 'esp32c2': + elif dut.target in ['esp32c2', 'esp32c6']: dut.expect_gme('Store access fault') dut.expect_reg_dump(0) dut.expect_stack_dump() @@ -621,7 +623,7 @@ def test_dram_reg1_execute_violation(dut: PanicTestDut, test_func_name: str) -> dut.expect(r'Unknown operation at address [0-9xa-f]+ not permitted \((\S+)\)') dut.expect_reg_dump(0) dut.expect_corrupted_backtrace() - elif dut.target in ['esp32c3', 'esp32c2']: + elif dut.target in ['esp32c3', 'esp32c2', 'esp32c6']: dut.expect_gme('Instruction access fault') dut.expect_reg_dump(0) dut.expect_stack_dump() @@ -636,7 +638,7 @@ def test_dram_reg2_execute_violation(dut: PanicTestDut, test_func_name: str) -> dut.expect_gme('InstructionFetchError') dut.expect_reg_dump(0) dut.expect_corrupted_backtrace() - elif dut.target in ['esp32c3', 'esp32c2']: + elif dut.target in ['esp32c3', 'esp32c2', 'esp32c6']: dut.expect_gme('Instruction access fault') dut.expect_reg_dump(0) dut.expect_stack_dump() @@ -651,6 +653,7 @@ def test_rtc_fast_reg1_execute_violation(dut: PanicTestDut, test_func_name: str) @pytest.mark.parametrize('config', CONFIGS_MEMPROT_RTC_FAST_MEM, indirect=True) @pytest.mark.generic +@pytest.mark.skipif('config.getvalue("target") == "esp32c6"', reason='Not a violation condition because it does not have PMS peripheral') def test_rtc_fast_reg2_execute_violation(dut: PanicTestDut, test_func_name: str) -> None: dut.run_test_func(test_func_name) dut.expect_gme('Memory protection fault') @@ -673,18 +676,23 @@ def test_rtc_fast_reg2_execute_violation(dut: PanicTestDut, test_func_name: str) @pytest.mark.xfail('config.getvalue("target") == "esp32s2"', reason='Multiple panic reasons for the same test may surface', run=False) def test_rtc_fast_reg3_execute_violation(dut: PanicTestDut, test_func_name: str) -> None: dut.run_test_func(test_func_name) - dut.expect_gme('Memory protection fault') if dut.target == 'esp32s2': + dut.expect_gme('Memory protection fault') dut.expect(r'Unknown operation at address [0-9xa-f]+ not permitted \((\S+)\)') dut.expect_reg_dump(0) dut.expect_backtrace() elif dut.target == 'esp32c3': + dut.expect_gme('Memory protection fault') dut.expect(r' memory type: (\S+)') dut.expect(r' faulting address: [0-9xa-f]+') dut.expect(r' operation type: (\S+)') dut.expect_reg_dump(0) dut.expect_stack_dump() + elif dut.target == 'esp32c6': + dut.expect_gme('Instruction access fault') + dut.expect_reg_dump(0) + dut.expect_stack_dump() @pytest.mark.parametrize('config', CONFIGS_MEMPROT_RTC_SLOW_MEM, indirect=True) diff --git a/tools/test_apps/system/panic/sdkconfig.ci.memprot_esp32c6 b/tools/test_apps/system/panic/sdkconfig.ci.memprot_esp32c6 new file mode 100644 index 0000000000..e4daa56b55 --- /dev/null +++ b/tools/test_apps/system/panic/sdkconfig.ci.memprot_esp32c6 @@ -0,0 +1,8 @@ +# Restricting to ESP32C6 +CONFIG_IDF_TARGET="esp32c6" + +# Enabling memory protection +CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT=y + +# Enable memprot test +CONFIG_TEST_MEMPROT=y diff --git a/tools/test_apps/system/panic/sdkconfig.defaults b/tools/test_apps/system/panic/sdkconfig.defaults index 6f41f062fa..40cf08e07c 100644 --- a/tools/test_apps/system/panic/sdkconfig.defaults +++ b/tools/test_apps/system/panic/sdkconfig.defaults @@ -16,3 +16,6 @@ CONFIG_FREERTOS_USE_TRACE_FACILITY=y # Reduce IRAM size CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y + +# Increase main task stack size +CONFIG_ESP_MAIN_TASK_STACK_SIZE=4096