From 4c58af8eaec61183e8ada49e029cb1df55465353 Mon Sep 17 00:00:00 2001 From: wuzhenghui Date: Thu, 30 Nov 2023 15:57:16 +0800 Subject: [PATCH] fix(riscv): supports 1 byte and larger than 64byte range watchpoint setting --- components/esp_hw_support/cpu_util.c | 14 ++++++++ components/hal/esp32c3/include/hal/cpu_ll.h | 33 +++++++++++++++---- components/riscv/include/esp_debug_helpers.h | 26 ++++++++------- components/xtensa/include/esp_debug_helpers.h | 21 ++++++------ 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/components/esp_hw_support/cpu_util.c b/components/esp_hw_support/cpu_util.c index 2840b17db1..0889bc8375 100644 --- a/components/esp_hw_support/cpu_util.c +++ b/components/esp_hw_support/cpu_util.c @@ -51,6 +51,20 @@ esp_err_t IRAM_ATTR esp_set_watchpoint(int no, void *adr, int size, int flags) { watchpoint_trigger_t trigger; + if (no < 0 || no >= SOC_CPU_WATCHPOINTS_NUM) { + return ESP_ERR_INVALID_ARG; + } + + // Check that the watched region's start address is naturally aligned to the size of the region + if ((uint32_t)adr % (uint32_t)size) { + return ESP_ERR_INVALID_ARG; + } + + // Check if size is 2^n, and size is in the range of [1 ... SOC_CPU_WATCHPOINT_SIZE] + if (size < 1 || size > SOC_CPU_WATCHPOINT_SIZE || (size & (size - 1)) != 0) { + return ESP_ERR_INVALID_ARG; + } + switch (flags) { case ESP_WATCHPOINT_LOAD: diff --git a/components/hal/esp32c3/include/hal/cpu_ll.h b/components/hal/esp32c3/include/hal/cpu_ll.h index ab3ab17685..959d5f6361 100644 --- a/components/hal/esp32c3/include/hal/cpu_ll.h +++ b/components/hal/esp32c3/include/hal/cpu_ll.h @@ -137,8 +137,6 @@ static inline void cpu_ll_set_watchpoint(int id, bool on_read, bool on_write) { - uint32_t addr_napot; - if (cpu_ll_is_debugger_attached()) { /* see description in cpu_ll_set_breakpoint() */ long args[] = {true, id, (long)addr, (long)size, @@ -152,17 +150,38 @@ static inline void cpu_ll_set_watchpoint(int id, RV_WRITE_CSR(tselect,id); RV_SET_CSR(CSR_TCONTROL, TCONTROL_MPTE | TCONTROL_MTE); RV_SET_CSR(CSR_TDATA1, TDATA1_USER|TDATA1_MACHINE); - RV_SET_CSR_FIELD(CSR_TDATA1, TDATA1_MATCH, 1); - // add 0 in napot encoding - addr_napot = ((uint32_t) addr) | ((size >> 1) - 1); + RV_SET_CSR_FIELD(CSR_TDATA1, TDATA1_MATCH, (size == 1) ? 0 : 1); + if (on_read) { RV_SET_CSR(CSR_TDATA1, TDATA1_LOAD); } if (on_write) { RV_SET_CSR(CSR_TDATA1, TDATA1_STORE); } - RV_WRITE_CSR(tdata2,addr_napot); - return; + + /* From RISC-V Debug Specification: + * tdata1(mcontrol) match = 0 : Exact byte match + * + * tdata1(mcontrol) match = 1 : NAPOT (Naturally Aligned Power-Of-Two): + * Matches when the top M bits of any compare value match the top M bits of tdata2. + * M is XLEN − 1 minus the index of the least-significant bit containing 0 in tdata2. + * Note: Expecting that size is number power of 2 (numbers should be in the range of 1 ~ 31) + * + * Examples for understanding how to calculate match pattern to tdata2: + * + * nnnn...nnnnn 1-byte Exact byte match + * nnnn...nnnn0 2-byte NAPOT range + * nnnn...nnn01 4-byte NAPOT range + * nnnn...nn011 8-byte NAPOT range + * nnnn...n0111 16-byte NAPOT range + * nnnn...01111 32-byte NAPOT range + * ... + * n011...11111 2^31 byte NAPOT range + * * where n are bits from original address + */ + uint32_t match_pattern = ((uint32_t)addr & ~(size-1)) | ((size-1) >> 1); + + RV_WRITE_CSR(tdata2, match_pattern); } static inline void cpu_ll_clear_watchpoint(int id) diff --git a/components/riscv/include/esp_debug_helpers.h b/components/riscv/include/esp_debug_helpers.h index d088ddeef1..6da4bccf77 100644 --- a/components/riscv/include/esp_debug_helpers.h +++ b/components/riscv/include/esp_debug_helpers.h @@ -54,20 +54,24 @@ typedef struct { void esp_set_breakpoint_if_jtag(void *fn); /** - * @brief Set a watchpoint to break/panic when a certain memory range is accessed. + * @brief Set and enable a hardware watchpoint on the current CPU * - * @param no Watchpoint number. On the ESP32, this can be 0 or 1. - * @param adr Base address to watch - * @param size Size of the region, starting at the base address, to watch. Must - * be one of 2^n, with n in [0..6]. + * Set and enable a hardware watchpoint on the current CPU, specifying the + * memory range and trigger operation. Watchpoints will break/panic the CPU when + * the CPU accesses (according to the trigger type) on a certain memory range. + * + * @note Overwrites previously set watchpoint with same watchpoint number. + * On RISC-V chips, this API uses method0(Exact matching) and method1(NAPOT matching) according to the + * riscv-debug-spec-0.13 specification for address matching. + * If the watch region size is 1byte, it uses exact matching (method 0). + * If the watch region size is larger than 1byte, it uses NAPOT matching (method 1). This mode requires + * the watching region start address to be aligned to the watching region size. + * + * @param no Hardware watchpoint number [0..SOC_CPU_WATCHPOINTS_NUM - 1] + * @param adr Watchpoint's base address, must be naturally aligned to the size of the region + * @param size Size of the region to watch. Must be one of 2^n and in the range of [1 ... SOC_CPU_WATCHPOINT_SIZE] * @param flags One of ESP_WATCHPOINT_* flags - * * @return ESP_ERR_INVALID_ARG on invalid arg, ESP_OK otherwise - * - * @warning The ESP32 watchpoint hardware watches a region of bytes by effectively - * masking away the lower n bits for a region with size 2^n. If adr does - * not have zero for these lower n bits, you may not be watching the - * region you intended. */ esp_err_t esp_set_watchpoint(int no, void *adr, int size, int flags); diff --git a/components/xtensa/include/esp_debug_helpers.h b/components/xtensa/include/esp_debug_helpers.h index 5bdc028c61..0928d75515 100644 --- a/components/xtensa/include/esp_debug_helpers.h +++ b/components/xtensa/include/esp_debug_helpers.h @@ -54,20 +54,19 @@ typedef struct { void esp_set_breakpoint_if_jtag(void *fn); /** - * @brief Set a watchpoint to break/panic when a certain memory range is accessed. + * @brief Set and enable a hardware watchpoint on the current CPU * - * @param no Watchpoint number. On the ESP32, this can be 0 or 1. - * @param adr Base address to watch - * @param size Size of the region, starting at the base address, to watch. Must - * be one of 2^n, with n in [0..6]. + * Set and enable a hardware watchpoint on the current CPU, specifying the + * memory range and trigger operation. Watchpoints will break/panic the CPU when + * the CPU accesses (according to the trigger type) on a certain memory range. + * + * @note Overwrites previously set watchpoint with same watchpoint number. + * + * @param no Hardware watchpoint number [0..SOC_CPU_WATCHPOINTS_NUM - 1] + * @param adr Watchpoint's base address, must be naturally aligned to the size of the region + * @param size Size of the region to watch. Must be one of 2^n and in the range of [1 ... SOC_CPU_WATCHPOINT_SIZE] * @param flags One of ESP_WATCHPOINT_* flags - * * @return ESP_ERR_INVALID_ARG on invalid arg, ESP_OK otherwise - * - * @warning The ESP32 watchpoint hardware watches a region of bytes by effectively - * masking away the lower n bits for a region with size 2^n. If adr does - * not have zero for these lower n bits, you may not be watching the - * region you intended. */ esp_err_t esp_set_watchpoint(int no, void *adr, int size, int flags);