From 5f876d5dbbd1d1be867169f9488f63fc4a03ffc5 Mon Sep 17 00:00:00 2001 From: Roman Kuznetsov Date: Mon, 31 Jul 2017 15:29:41 +0300 Subject: [PATCH] Provide a structure with the debug information to debug_halt() handler Enhance debug box functionality. debug_halt now recieves a struct with the stack frame for better debugging. --- api/inc/debug_exports.h | 3 +- api/inc/halt_exports.h | 29 +++++++++++++++ core/debug/inc/debug.h | 3 +- core/debug/src/debug.c | 45 ++++++++++++++++++++++++ core/debug/src/debug_box.c | 19 +++++++--- core/system/inc/halt.h | 13 ++++--- core/system/src/halt.c | 12 +++---- core/uvisor-config.h | 2 +- core/vmpu/src/mpu_armv7m/vmpu_armv7m.c | 26 ++++++++------ core/vmpu/src/mpu_armv8m/vmpu_armv8m.c | 28 +++++++++------ core/vmpu/src/mpu_kinetis/vmpu_kinetis.c | 28 +++++++++------ tools/uvisor-tests.txt | 2 +- 12 files changed, 158 insertions(+), 52 deletions(-) diff --git a/api/inc/debug_exports.h b/api/inc/debug_exports.h index acbecee1..a269ba32 100644 --- a/api/inc/debug_exports.h +++ b/api/inc/debug_exports.h @@ -17,6 +17,7 @@ #ifndef __UVISOR_API_DEBUG_EXPORTS_H__ #define __UVISOR_API_DEBUG_EXPORTS_H__ +#include "api/inc/halt_exports.h" #include /* Debug box driver -- Version 0 @@ -24,7 +25,7 @@ * code to setup a debug box.*/ typedef struct TUvisorDebugDriver { uint32_t (*get_version)(void); - void (*halt_error)(int); + void (*halt_error)(THaltError, const THaltInfo *); } TUvisorDebugDriver; /* Number of handlers in the debug box driver */ diff --git a/api/inc/halt_exports.h b/api/inc/halt_exports.h index 5ab87d47..b5287a92 100644 --- a/api/inc/halt_exports.h +++ b/api/inc/halt_exports.h @@ -72,4 +72,33 @@ typedef struct { uint32_t retpsr; } UVISOR_PACKED exception_frame_t; +/* A pointer to this structure will be given to halt_error() handler + * of the debug box driver. */ +typedef struct { + /* A basic exception stack frame that is always present with a valid stack. */ + exception_frame_t stack_frame; + + /* A few registers that may be useful for debug. */ + uint32_t lr; + uint32_t control; + uint32_t ipsr; + + /* Fault status registers. */ + uint32_t mmfar; + uint32_t bfar; + uint32_t cfsr; + uint32_t hfsr; + uint32_t dfsr; + uint32_t afsr; + + /* Bitmask telling which of the above regions are valid. */ + uint32_t valid_data; +} UVISOR_PACKED THaltInfo; + +/* Bitmask to specify which HaltInfo regions are valid. */ +typedef enum { + HALT_INFO_STACK_FRAME = 0x1, + HALT_INFO_REGISTERS = 0x2 +} THaltInfoValidMask; + #endif /* __UVISOR_API_HALT_EXPORTS_H__ */ diff --git a/core/debug/inc/debug.h b/core/debug/inc/debug.h index 8ee1693c..4d29a283 100644 --- a/core/debug/inc/debug.h +++ b/core/debug/inc/debug.h @@ -41,7 +41,7 @@ void debug_fault(THaltError reason, uint32_t lr, uint32_t sp); /* Debug box */ void debug_register_driver(const TUvisorDebugDriver * const driver); uint32_t debug_get_version(void); -void debug_halt_error(THaltError reason); +void debug_halt_error(THaltError reason, const THaltInfo *halt_info); void debug_reboot(TResetReason reason); /* Enter the debug box from a privileged mode exception handler. This function @@ -54,6 +54,7 @@ uint32_t debug_box_enter_from_priv(uint32_t lr); void debug_die(void); void debug_deprivilege_and_return(void * debug_handler, void * return_handler, uint32_t a0, uint32_t a1, uint32_t a2, uint32_t a3); +bool debug_collect_halt_info(uint32_t lr, uint32_t sp, THaltInfo *halt_info); #ifdef NDEBUG diff --git a/core/debug/src/debug.c b/core/debug/src/debug.c index 5c0fa296..6368c550 100644 --- a/core/debug/src/debug.c +++ b/core/debug/src/debug.c @@ -432,3 +432,48 @@ void debug_fault(THaltError reason, uint32_t lr, uint32_t sp) #endif /* defined(ARCH_MPU_ARMv8M) */ DEBUG_PRINT_END(); } + +bool debug_collect_halt_info(uint32_t lr, uint32_t sp, THaltInfo *halt_info) +{ + /* For security reasons we don't want to print uVisor faults, since it + * could expose secrets. To debug uVisor faults users will have to use + * a uVisor debug build with semihosting. */ +#if defined(ARCH_MPU_ARMv8M) + bool no_halt_info = EXC_FROM_S(lr); +#else + bool no_halt_info = !EXC_FROM_PSP(lr); +#endif /* defined(ARCH_MPU_ARMv8M) */ + + halt_info->valid_data = 0; + if (no_halt_info) { + return false; + } + + /* CPU registers useful for debug. */ + halt_info->lr = lr; + halt_info->ipsr = __get_IPSR(); + halt_info->control = __get_CONTROL(); + + /* Save the status registers in the halt info. */ + halt_info->mmfar = SCB->MMFAR; + halt_info->bfar = SCB->BFAR; + halt_info->cfsr = SCB->CFSR; + halt_info->hfsr = SCB->HFSR; + halt_info->dfsr = SCB->DFSR; + halt_info->afsr = SCB->AFSR; + halt_info->valid_data |= HALT_INFO_REGISTERS; + + /* Copy the exception stack frame into the halt info. + * Make sure that we're not dealing with stacking error in which case we + * don't have a valid exception stack frame. + * Stacking error may happen in case of Bus Fault, MemManage Fault or + * their escalation to Hard Fault. */ + static const uint32_t stack_error_msk = SCB_CFSR_MSTKERR_Msk | SCB_CFSR_STKERR_Msk; + if (!(halt_info->cfsr & stack_error_msk)) { + memcpy((void *)&halt_info->stack_frame, (void *)sp, + CONTEXT_SWITCH_EXC_SF_BYTES); + halt_info->valid_data |= HALT_INFO_STACK_FRAME; + } + + return true; +} diff --git a/core/debug/src/debug_box.c b/core/debug/src/debug_box.c index 32588cd4..cff31748 100644 --- a/core/debug/src/debug_box.c +++ b/core/debug/src/debug_box.c @@ -27,7 +27,7 @@ TDebugBox g_debug_box; void debug_reboot(TResetReason reason) { if (!g_debug_box.initialized || g_active_box != g_debug_box.box_id || reason >= __TRESETREASON_MAX) { - halt(NOT_ALLOWED); + halt(NOT_ALLOWED, NULL); } /* Note: Currently we do not act differently based on the reset reason. */ @@ -45,9 +45,12 @@ uint32_t debug_get_version(void) return 0; } -void debug_halt_error(THaltError reason) +uint32_t g_debug_interrupt_sp[UVISOR_MAX_BOXES]; + +void debug_halt_error(THaltError reason, const THaltInfo *halt_info) { static int debugged_once_before = 0; + void *info = NULL; /* If the debug box does not exist (or it has not been initialized yet), or * the debug box was already called once, just loop forever. */ @@ -56,16 +59,22 @@ void debug_halt_error(THaltError reason) } else { /* Remember that debug_deprivilege_and_return() has been called once. */ debugged_once_before = 1; + + /* Place the halt info on the interrupt stack. */ + if (halt_info) { + g_debug_interrupt_sp[g_debug_box.box_id] -= sizeof(THaltInfo); + info = (void *)g_debug_interrupt_sp[g_debug_box.box_id]; + memcpy(info, halt_info, sizeof(THaltInfo)); + } /* The following arguments are passed to the destination function: * 1. reason + * 2. halt info * Upon return from the debug handler, the system will die. */ - debug_deprivilege_and_return(g_debug_box.driver->halt_error, debug_die, reason, 0, 0, 0); + debug_deprivilege_and_return(g_debug_box.driver->halt_error, debug_die, reason, (uint32_t)info, 0, 0); } } -uint32_t g_debug_interrupt_sp[UVISOR_MAX_BOXES]; - void debug_register_driver(const TUvisorDebugDriver * const driver) { int i; diff --git a/core/system/inc/halt.h b/core/system/inc/halt.h index 9a2f2bd2..b0bb6cf2 100644 --- a/core/system/inc/halt.h +++ b/core/system/inc/halt.h @@ -20,16 +20,19 @@ #include "api/inc/halt_exports.h" #ifdef NDEBUG -#define HALT_ERROR(reason, ...) halt(reason) +#define HALT_ERROR(reason, ...) halt(reason, NULL) +#define HALT_ERROR_EXTENDED(reason, halt_info, ...) halt(reason, halt_info) #else /*NDEBUG*/ #define HALT_ERROR(reason, ...) \ - halt_line(__FILE__, __LINE__, reason, ##__VA_ARGS__) + halt_line(__FILE__, __LINE__, reason, NULL, ##__VA_ARGS__) +#define HALT_ERROR_EXTENDED(reason, halt_info, ...) \ + halt_line(__FILE__, __LINE__, reason, halt_info, ##__VA_ARGS__) #endif/*NDEBUG*/ extern void halt_user_error(THaltUserError reason); -extern void halt(THaltError reason); -extern void halt_error(THaltError reason, const char *fmt, ...); +extern void halt(THaltError reason, const THaltInfo *halt_info); +extern void halt_error(THaltError reason, const THaltInfo *halt_info, const char *fmt, ...); extern void halt_line(const char *file, uint32_t line, THaltError reason, - const char *fmt, ...); + const THaltInfo *halt_info, const char *fmt, ...); #endif/*__HALT_H__*/ diff --git a/core/system/src/halt.c b/core/system/src/halt.c index 16246bc3..3fa6e0ed 100644 --- a/core/system/src/halt.c +++ b/core/system/src/halt.c @@ -33,13 +33,13 @@ static void halt_printf(const char *fmt, ...) va_end(va); } -void halt(THaltError reason) +void halt(THaltError reason, const THaltInfo *halt_info) { /* Die. */ - debug_halt_error(reason); + debug_halt_error(reason, halt_info); } -void halt_error(THaltError reason, const char *fmt, ...) +void halt_error(THaltError reason, const THaltInfo *halt_info, const char *fmt, ...) { halt_printf("HALT_ERROR: "); @@ -53,11 +53,11 @@ void halt_error(THaltError reason, const char *fmt, ...) default_putc('\n'); /* Die. */ - halt(reason); + halt(reason, halt_info); } void halt_line(const char *file, uint32_t line, THaltError reason, - const char *fmt, ...) + const THaltInfo *halt_info, const char *fmt, ...) { halt_printf("HALT_ERROR(%s#%i): ", file, line); @@ -71,7 +71,7 @@ void halt_line(const char *file, uint32_t line, THaltError reason, default_putc('\n'); /* Die. */ - halt(reason); + halt(reason, halt_info); } void halt_user_error(THaltUserError reason) diff --git a/core/uvisor-config.h b/core/uvisor-config.h index 49c48874..f9ec02f2 100644 --- a/core/uvisor-config.h +++ b/core/uvisor-config.h @@ -32,7 +32,7 @@ #define UVISOR_FLASH_LENGTH_MAX 0x4000 #else /* Debug builds can be up to this big. */ -#define UVISOR_FLASH_LENGTH_MAX 0xC000 +#define UVISOR_FLASH_LENGTH_MAX 0xC0A0 #endif /** Size of the SRAM space protected by uVisor for its own SRAM sections diff --git a/core/vmpu/src/mpu_armv7m/vmpu_armv7m.c b/core/vmpu/src/mpu_armv7m/vmpu_armv7m.c index 6608be81..e7b34bd4 100644 --- a/core/vmpu/src/mpu_armv7m/vmpu_armv7m.c +++ b/core/vmpu/src/mpu_armv7m/vmpu_armv7m.c @@ -130,15 +130,21 @@ uint32_t vmpu_sys_mux_handler(uint32_t lr, uint32_t msp) /* Determine the origin of the exception. */ bool from_psp = EXC_FROM_PSP(lr); uint32_t sp = from_psp ? __get_PSP() : msp; + + /* Collect fault information that will be given to the halt handler in case of halt. */ + THaltInfo info, *halt_info = NULL; + if (debug_collect_halt_info(lr, sp, &info)) { + halt_info = &info; + } switch (ipsr) { case NonMaskableInt_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No NonMaskableInt IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No NonMaskableInt IRQ handler registered."); break; case HardFault_IRQn: DEBUG_FAULT(FAULT_HARD, lr, sp); - HALT_ERROR(FAULT_HARD, "Cannot recover from a hard fault."); + HALT_ERROR_EXTENDED(FAULT_HARD, halt_info, "Cannot recover from a hard fault."); lr = debug_box_enter_from_priv(lr); break; @@ -180,7 +186,7 @@ uint32_t vmpu_sys_mux_handler(uint32_t lr, uint32_t msp) /* If recovery was not successful, throw an error and halt. */ DEBUG_FAULT(FAULT_MEMMANAGE, lr, sp); VMPU_SCB_MMFSR = fault_status; - HALT_ERROR(PERMISSION_DENIED, "Access to restricted resource denied."); + HALT_ERROR_EXTENDED(PERMISSION_DENIED, halt_info, "Access to restricted resource denied."); lr = debug_box_enter_from_priv(lr); break; @@ -211,33 +217,33 @@ uint32_t vmpu_sys_mux_handler(uint32_t lr, uint32_t msp) /* If recovery was not successful, throw an error and halt. */ DEBUG_FAULT(FAULT_BUS, lr, sp); - HALT_ERROR(PERMISSION_DENIED, "Access to restricted resource denied."); + HALT_ERROR_EXTENDED(PERMISSION_DENIED, halt_info, "Access to restricted resource denied."); break; case UsageFault_IRQn: DEBUG_FAULT(FAULT_USAGE, lr, sp); - HALT_ERROR(FAULT_USAGE, "Cannot recover from a usage fault."); + HALT_ERROR_EXTENDED(FAULT_USAGE, halt_info, "Cannot recover from a usage fault."); break; case SVCall_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No SVCall IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No SVCall IRQ handler registered."); break; case DebugMonitor_IRQn: DEBUG_FAULT(FAULT_DEBUG, lr, sp); - HALT_ERROR(FAULT_DEBUG, "Cannot recover from a DebugMonitor fault."); + HALT_ERROR_EXTENDED(FAULT_DEBUG, halt_info, "Cannot recover from a DebugMonitor fault."); break; case PendSV_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No PendSV IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No PendSV IRQ handler registered."); break; case SysTick_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No SysTick IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No SysTick IRQ handler registered."); break; default: - HALT_ERROR(NOT_ALLOWED, "Active IRQn (%i) is not a system interrupt.", ipsr); + HALT_ERROR_EXTENDED(NOT_ALLOWED, halt_info, "Active IRQn (%i) is not a system interrupt.", ipsr); break; } diff --git a/core/vmpu/src/mpu_armv8m/vmpu_armv8m.c b/core/vmpu/src/mpu_armv8m/vmpu_armv8m.c index 42dd41d9..37bba193 100644 --- a/core/vmpu/src/mpu_armv8m/vmpu_armv8m.c +++ b/core/vmpu/src/mpu_armv8m/vmpu_armv8m.c @@ -119,30 +119,36 @@ uint32_t vmpu_sys_mux_handler(uint32_t lr, uint32_t msp_s) bool from_psp = EXC_FROM_PSP(lr); uint32_t sp = from_s ? (from_np ? (from_psp ? __get_PSP() : msp_s) : msp_s) : (from_np ? (from_psp ? __TZ_get_PSP_NS() : __TZ_get_MSP_NS()) : __TZ_get_MSP_NS()); + + /* Collect fault information that will be given to the halt handler in case of halt. */ + THaltInfo info, *halt_info = NULL; + if (debug_collect_halt_info(lr, sp, &info)) { + halt_info = &info; + } switch(ipsr) { case NonMaskableInt_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No NonMaskableInt IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No NonMaskableInt IRQ handler registered."); break; case HardFault_IRQn: DEBUG_FAULT(FAULT_HARD, lr, sp); - HALT_ERROR(FAULT_HARD, "Cannot recover from a hard fault."); + HALT_ERROR_EXTENDED(FAULT_HARD, halt_info, "Cannot recover from a hard fault."); break; case MemoryManagement_IRQn: DEBUG_FAULT(FAULT_MEMMANAGE, lr, sp); - HALT_ERROR(FAULT_MEMMANAGE, "Cannot recover from a memory management fault."); + HALT_ERROR_EXTENDED(FAULT_MEMMANAGE, halt_info, "Cannot recover from a memory management fault."); break; case BusFault_IRQn: DEBUG_FAULT(FAULT_BUS, lr, sp); - HALT_ERROR(FAULT_BUS, "Cannot recover from a bus fault."); + HALT_ERROR_EXTENDED(FAULT_BUS, halt_info, "Cannot recover from a bus fault."); break; case UsageFault_IRQn: DEBUG_FAULT(FAULT_USAGE, lr, sp); - HALT_ERROR(FAULT_USAGE, "Cannot recover from a usage fault."); + HALT_ERROR_EXTENDED(FAULT_USAGE, halt_info, "Cannot recover from a usage fault."); break; case SecureFault_IRQn: @@ -158,28 +164,28 @@ uint32_t vmpu_sys_mux_handler(uint32_t lr, uint32_t msp_s) } } DEBUG_FAULT(FAULT_SECURE, lr, sp); - HALT_ERROR(PERMISSION_DENIED, "Cannot recover from a secure fault."); + HALT_ERROR_EXTENDED(PERMISSION_DENIED, halt_info, "Cannot recover from a secure fault."); break; case SVCall_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No SVCall IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No SVCall IRQ handler registered."); break; case DebugMonitor_IRQn: DEBUG_FAULT(FAULT_DEBUG, lr, sp); - HALT_ERROR(FAULT_DEBUG, "Cannot recover from a DebugMonitor fault."); + HALT_ERROR_EXTENDED(FAULT_DEBUG, halt_info, "Cannot recover from a DebugMonitor fault."); break; case PendSV_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No PendSV IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No PendSV IRQ handler registered."); break; case SysTick_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No SysTick IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No SysTick IRQ handler registered."); break; default: - HALT_ERROR(NOT_ALLOWED, "Active IRQn (%i) is not a system interrupt.", ipsr); + HALT_ERROR_EXTENDED(NOT_ALLOWED, halt_info, "Active IRQn (%i) is not a system interrupt.", ipsr); break; } diff --git a/core/vmpu/src/mpu_kinetis/vmpu_kinetis.c b/core/vmpu/src/mpu_kinetis/vmpu_kinetis.c index 46fc3994..b6b78454 100644 --- a/core/vmpu/src/mpu_kinetis/vmpu_kinetis.c +++ b/core/vmpu/src/mpu_kinetis/vmpu_kinetis.c @@ -65,21 +65,27 @@ uint32_t vmpu_sys_mux_handler(uint32_t lr, uint32_t msp) bool from_np = EXC_FROM_NP(lr); bool from_psp = EXC_FROM_PSP(lr); uint32_t sp = from_np ? (from_psp ? __get_PSP() : msp) : msp; - + + /* Collect fault information that will be given to the halt handler in case of halt. */ + THaltInfo info, *halt_info = NULL; + if (debug_collect_halt_info(lr, sp, &info)) { + halt_info = &info; + } + switch (ipsr) { case NonMaskableInt_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No NonMaskableInt IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No NonMaskableInt IRQ handler registered."); break; case HardFault_IRQn: DEBUG_FAULT(FAULT_HARD, lr, sp); - HALT_ERROR(FAULT_HARD, "Cannot recover from a hard fault."); + HALT_ERROR_EXTENDED(FAULT_HARD, halt_info, "Cannot recover from a hard fault."); lr = debug_box_enter_from_priv(lr); break; case MemoryManagement_IRQn: DEBUG_FAULT(FAULT_MEMMANAGE, lr, sp); - HALT_ERROR(FAULT_MEMMANAGE, "Cannot recover from a memory management fault."); + HALT_ERROR_EXTENDED(FAULT_MEMMANAGE, halt_info, "Cannot recover from a memory management fault."); break; case BusFault_IRQn: @@ -157,34 +163,34 @@ uint32_t vmpu_sys_mux_handler(uint32_t lr, uint32_t msp) /* If recovery was not successful, throw an error and halt. */ DEBUG_FAULT(FAULT_BUS, lr, sp); VMPU_SCB_BFSR = fault_status; - HALT_ERROR(PERMISSION_DENIED, "Access to restricted resource denied."); + HALT_ERROR_EXTENDED(PERMISSION_DENIED, halt_info, "Access to restricted resource denied."); lr = debug_box_enter_from_priv(lr); break; case UsageFault_IRQn: DEBUG_FAULT(FAULT_USAGE, lr, sp); - HALT_ERROR(FAULT_USAGE, "Cannot recover from a usage fault."); + HALT_ERROR_EXTENDED(FAULT_USAGE, halt_info, "Cannot recover from a usage fault."); break; case SVCall_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No SVCall IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No SVCall IRQ handler registered."); break; case DebugMonitor_IRQn: DEBUG_FAULT(FAULT_DEBUG, lr, sp); - HALT_ERROR(FAULT_DEBUG, "Cannot recover from a DebugMonitor fault."); + HALT_ERROR_EXTENDED(FAULT_DEBUG, halt_info, "Cannot recover from a DebugMonitor fault."); break; case PendSV_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No PendSV IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No PendSV IRQ handler registered."); break; case SysTick_IRQn: - HALT_ERROR(NOT_IMPLEMENTED, "No SysTick IRQ handler registered."); + HALT_ERROR_EXTENDED(NOT_IMPLEMENTED, halt_info, "No SysTick IRQ handler registered."); break; default: - HALT_ERROR(NOT_ALLOWED, "Active IRQn (%i) is not a system interrupt.", ipsr); + HALT_ERROR_EXTENDED(NOT_ALLOWED, halt_info, "Active IRQn (%i) is not a system interrupt.", ipsr); break; } diff --git a/tools/uvisor-tests.txt b/tools/uvisor-tests.txt index b6c3fc3e..ed2bf8e0 100644 --- a/tools/uvisor-tests.txt +++ b/tools/uvisor-tests.txt @@ -1 +1 @@ -0fc7c7f55507421463ba46a9692008e87a1ac5f2 +e3b1385c7facc7fdab472440293c4c87ed2b2999