panic on RISC-V: Take into account Merge Request comments

This commit is contained in:
Omar Chebib
2020-12-28 11:39:25 +08:00
parent 5962b1dc56
commit c218f669ba
8 changed files with 38 additions and 41 deletions

View File

@@ -18,8 +18,8 @@
to panic the CPU, which from a debugging perspective is better than grabbing bad to panic the CPU, which from a debugging perspective is better than grabbing bad
data from the bus. data from the bus.
*/ */
#include "esp32c3/rom/ets_sys.h"
#include "freertos/FreeRTOS.h" #include "esp_attr.h"
#include "esp_intr_alloc.h" #include "esp_intr_alloc.h"
#include "soc/extmem_reg.h" #include "soc/extmem_reg.h"
#include "soc/periph_defs.h" #include "soc/periph_defs.h"

View File

@@ -184,11 +184,13 @@ void panic_soc_fill_info(void *f, panic_info_t *info)
RvExcFrame *frame = (RvExcFrame *) f; RvExcFrame *frame = (RvExcFrame *) f;
/* Please keep in sync with PANIC_RSN_* defines */ /* Please keep in sync with PANIC_RSN_* defines */
static const char *pseudo_reason[PANIC_RSN_MAX + 1] = { static const char *pseudo_reason[PANIC_RSN_COUNT] = {
"Unknown reason", "Unknown reason",
"Interrupt wdt timeout on CPU0", "Interrupt wdt timeout on CPU0",
#if SOC_CPU_NUM > 1
"Interrupt wdt timeout on CPU1", "Interrupt wdt timeout on CPU1",
"Cache exception", #endif
"Cache error"
}; };
info->reason = pseudo_reason[0]; info->reason = pseudo_reason[0];
@@ -213,8 +215,10 @@ void panic_soc_fill_info(void *f, panic_info_t *info)
info->core = core; info->core = core;
info->exception = PANIC_EXCEPTION_TWDT; info->exception = PANIC_EXCEPTION_TWDT;
#if SOC_CPU_NUM > 1
_Static_assert(PANIC_RSN_INTWDT_CPU0 + 1 == PANIC_RSN_INTWDT_CPU1, _Static_assert(PANIC_RSN_INTWDT_CPU0 + 1 == PANIC_RSN_INTWDT_CPU1,
"PANIC_RSN_INTWDT_CPU1 must be equal to PANIC_RSN_INTWDT_CPU0 + 1"); "PANIC_RSN_INTWDT_CPU1 must be equal to PANIC_RSN_INTWDT_CPU0 + 1");
#endif
info->reason = pseudo_reason[PANIC_RSN_INTWDT_CPU0 + core]; info->reason = pseudo_reason[PANIC_RSN_INTWDT_CPU0 + core];
} }
} }

View File

@@ -411,7 +411,7 @@ void panic_soc_fill_info(void *f, panic_info_t *info)
#if CONFIG_IDF_TARGET_ESP32 #if CONFIG_IDF_TARGET_ESP32
"Cache disabled but cached memory region accessed", "Cache disabled but cached memory region accessed",
#elif CONFIG_IDF_TARGET_ESP32S2 #elif CONFIG_IDF_TARGET_ESP32S2
"Cache exception", "Cache error",
#endif #endif
}; };

View File

@@ -179,8 +179,12 @@ static void panic_handler(void *frame, bool pseudo_excause)
panic_set_address(frame, (uint32_t)&_invalid_pc_placeholder); panic_set_address(frame, (uint32_t)&_invalid_pc_placeholder);
} }
#endif #endif
if (panic_get_cause(frame) == PANIC_RSN_INTWDT_CPU0 || if (panic_get_cause(frame) == PANIC_RSN_INTWDT_CPU0
panic_get_cause(frame) == PANIC_RSN_INTWDT_CPU1) { #if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE
|| panic_get_cause(frame) == PANIC_RSN_INTWDT_CPU1
#endif
)
{
wdt_hal_write_protect_disable(&wdt0_context); wdt_hal_write_protect_disable(&wdt0_context);
wdt_hal_handle_intr(&wdt0_context); wdt_hal_handle_intr(&wdt0_context);
wdt_hal_write_protect_enable(&wdt0_context); wdt_hal_write_protect_enable(&wdt0_context);

View File

@@ -55,7 +55,7 @@ __attribute__((pure)) int interrupt_controller_hal_desc_level(int interrupt_num
*/ */
__attribute__((pure)) int_desc_flag_t interrupt_controller_hal_desc_flags(int interrupt_number, int cpu_number); __attribute__((pure)) int_desc_flag_t interrupt_controller_hal_desc_flags(int interrupt_number, int cpu_number);
#if CONFIG_IDF_TARGET_ARCH_RISCV #if SOC_INTERRUPT_LEVEL_CAN_SET
/** /**
* @brief Set the interrupt level given an interrupt number. * @brief Set the interrupt level given an interrupt number.
* *

View File

@@ -13,8 +13,12 @@
// limitations under the License. // limitations under the License.
#pragma once #pragma once
#define PANIC_RSN_NONE 0 enum _panic_reasons {
#define PANIC_RSN_INTWDT_CPU0 1 PANIC_RSN_NONE = 0,
#define PANIC_RSN_INTWDT_CPU1 2 PANIC_RSN_INTWDT_CPU0,
#define PANIC_RSN_CACHEERR 3 #if SOC_CPU_NUM > 1
#define PANIC_RSN_MAX 3 PANIC_RSN_INTWDT_CPU1,
#endif
PANIC_RSN_CACHEERR,
PANIC_RSN_COUNT
} panic_reasons;

View File

@@ -18,8 +18,8 @@
.equ SAVE_REGS, 32 .equ SAVE_REGS, 32
.equ CONTEXT_SIZE, (SAVE_REGS * 4) .equ CONTEXT_SIZE, (SAVE_REGS * 4)
.equ exception_from_panic, xt_unhandled_exception .equ panic_from_exception, xt_unhandled_exception
.equ exception_from_isr, panicHandler .equ panic_from_isr, panicHandler
.macro save_regs .macro save_regs
addi sp, sp, -CONTEXT_SIZE addi sp, sp, -CONTEXT_SIZE
@@ -182,20 +182,20 @@ _panic_handler:
csrr t0, mhartid csrr t0, mhartid
sw t0, RV_STK_MHARTID(sp) sw t0, RV_STK_MHARTID(sp)
/* Call exception_from_panic(sp) or exception_from_isr(sp) /* Call panic_from_exception(sp) or panic_from_isr(sp)
* depending on whether we have a pseudo excause or not. * depending on whether we have a pseudo excause or not.
* If mcause's highest bit is 1, then an interrupt called this routine, * If mcause's highest bit is 1, then an interrupt called this routine,
* so we have a pseudo excause. Else, it is due to a panic, we don't have * so we have a pseudo excause. Else, it is due to a exception, we don't
* an excause */ * have an pseudo excause */
mv a0, sp mv a0, sp
csrr a1, mcause csrr a1, mcause
/* Branches instructions don't accept immediates values, so use t1 to store /* Branches instructions don't accept immediates values, so use t1 to
* our comparator */ * store our comparator */
li t0, 0x80000000 li t0, 0x80000000
bgeu a1, t0, _call_panic_handler bgeu a1, t0, _call_panic_handler
sw a1, RV_STK_MCAUSE(sp) sw a1, RV_STK_MCAUSE(sp)
/* exception_from_panic never returns */ /* exception_from_panic never returns */
j exception_from_panic j panic_from_exception
_call_panic_handler: _call_panic_handler:
/* Remove highest bit from mcause (a1) register and save it in the /* Remove highest bit from mcause (a1) register and save it in the
* structure */ * structure */
@@ -203,8 +203,8 @@ _call_panic_handler:
and a1, a1, t0 and a1, a1, t0
sw a1, RV_STK_MCAUSE(sp) sw a1, RV_STK_MCAUSE(sp)
/* exception_from_isr never returns */ /* exception_from_isr never returns */
j exception_from_isr j panic_from_isr
.size exception_from_isr, .-exception_from_isr .size panic_from_isr, .-panic_from_isr
/* This is the interrupt handler. /* This is the interrupt handler.
* It saves the registers on the stack, * It saves the registers on the stack,

View File

@@ -51,19 +51,10 @@ TEST_CASE("spi_flash_cache_enabled() works on both CPUs", "[spi_flash][esp_flash
vQueueDelete(result_queue); vQueueDelete(result_queue);
} }
/** static const uint32_t s_in_rodata[] = { 0x12345678, 0xfedcba98 };
* On ESP32-C3 boards, constant data with a size less or equal to 8 bytes
* (64 bits) are placed in the DRAM.
* Let's add a third unused element to this array to force it to the DROM.
*/
static const uint32_t s_in_rodata[] = { 0x12345678, 0xfedcba98, 0x42 };
static void IRAM_ATTR cache_access_test_func(void* arg) static void IRAM_ATTR cache_access_test_func(void* arg)
{ {
/* Assert that the array s_in_rodata is in DROM. If not, this test is
* invalid as disabling the cache wouldn't have any effect. */
TEST_ASSERT(esp_ptr_in_drom(s_in_rodata));
spi_flash_disable_interrupts_caches_and_other_cpu(); spi_flash_disable_interrupts_caches_and_other_cpu();
volatile uint32_t* src = (volatile uint32_t*) s_in_rodata; volatile uint32_t* src = (volatile uint32_t*) s_in_rodata;
uint32_t v1 = src[0]; uint32_t v1 = src[0];
@@ -74,15 +65,9 @@ static void IRAM_ATTR cache_access_test_func(void* arg)
vTaskDelete(NULL); vTaskDelete(NULL);
} }
#ifdef CONFIG_IDF_TARGET_ESP32C3
#define CACHE_ERROR_REASON "Cache exception,RTC_SW_CPU_RST"
#else
#define CACHE_ERROR_REASON "Cache disabled,SW_RESET"
#endif
// These tests works properly if they resets the chip with the // These tests works properly if they resets the chip with the
// "Cache disabled but cached memory region accessed" reason and the correct CPU is logged. // "Cache disabled but cached memory region accessed" reason and the correct CPU is logged.
TEST_CASE("invalid access to cache raises panic (PRO CPU)", "[spi_flash][reset="CACHE_ERROR_REASON"]") TEST_CASE("invalid access to cache raises panic (PRO CPU)", "[spi_flash][ignore]")
{ {
xTaskCreatePinnedToCore(&cache_access_test_func, "ia", 2048, NULL, 5, NULL, 0); xTaskCreatePinnedToCore(&cache_access_test_func, "ia", 2048, NULL, 5, NULL, 0);
vTaskDelay(1000/portTICK_PERIOD_MS); vTaskDelay(1000/portTICK_PERIOD_MS);
@@ -90,7 +75,7 @@ TEST_CASE("invalid access to cache raises panic (PRO CPU)", "[spi_flash][reset="
#ifndef CONFIG_FREERTOS_UNICORE #ifndef CONFIG_FREERTOS_UNICORE
TEST_CASE("invalid access to cache raises panic (APP CPU)", "[spi_flash][reset=TG1WDT_SYS_RESET]") TEST_CASE("invalid access to cache raises panic (APP CPU)", "[spi_flash][ignore]")
{ {
xTaskCreatePinnedToCore(&cache_access_test_func, "ia", 2048, NULL, 5, NULL, 1); xTaskCreatePinnedToCore(&cache_access_test_func, "ia", 2048, NULL, 5, NULL, 1);
vTaskDelay(1000/portTICK_PERIOD_MS); vTaskDelay(1000/portTICK_PERIOD_MS);