From 0203dea024af68b4b0dbfbc5d7a98ee29839f02b Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Tue, 12 Jan 2021 13:51:26 -0800 Subject: [PATCH] Added replacement for the Boot ROM `_xtos_set_exception_handler` (#7820) Added replacement for the Boot ROM `_xtos_set_exception_handler` to handle installing our replacement `_xtos_c_wrapper_handler`. Simplified install in the non 32-bit exception module to make use of the improved `_xtos_set_exception_handler` Reorganized and improved comments. --- cores/esp8266/core_esp8266_non32xfer.cpp | 41 +------- cores/esp8266/exc-c-wrapper-handler.S | 13 ++- cores/esp8266/exc-sethandler.cpp | 113 +++++++++++++++++++++++ 3 files changed, 126 insertions(+), 41 deletions(-) create mode 100644 cores/esp8266/exc-sethandler.cpp diff --git a/cores/esp8266/core_esp8266_non32xfer.cpp b/cores/esp8266/core_esp8266_non32xfer.cpp index 0d7bb1919..4eb0e3a50 100644 --- a/cores/esp8266/core_esp8266_non32xfer.cpp +++ b/cores/esp8266/core_esp8266_non32xfer.cpp @@ -200,39 +200,12 @@ IRAM_ATTR void non32xfer_exception_handler(struct __exception_frame *ef, int cau } /* - An issue, the Boot ROM "C" wrapper for exception handlers, - _xtos_c_wrapper_handler, turns interrupts back on. To address this issue we - use our replacement in file `exc-c-wrapper-handler.S`. - - An overview, of an exception at entry: New interrupts are blocked by EXCM - being set. Once cleared, interrupts above the current INTLEVEL and exceptions - (w/o creating a DoubleException) can occur. - - Using our replacement for _xtos_c_wrapper_handler, INTLEVEL is raised to 15 - with EXCM cleared. - - The original Boot ROM `_xtos_c_wrapper_handler` would set INTLEVEL to 1 with - EXCM cleared, saved registers, then do a `rsil 0`, and called the registered - "C" Exception handler with interrupts fully enabled! Our replacement keeps - INTLEVEL at 15. This is needed to support the Arduino model of interrupts - disabled while an ISR runs. - - And we also need it for umm_malloc to work safely with an IRAM heap from an - ISR call. While malloc() will supply DRAM for all allocation from an ISR, - we want free() to safely operate from an ISR to avoid a leak potential. - - This replacement "C" Wrapper is only applied to this exception handler. + To operate reliably, this module requires the new + `_xtos_set_exception_handler` from `exc-sethandler.cpp` and + `_xtos_c_wrapper_handler` from `exc-c-wrapper-handler.S`. See comment block in + `exc-sethandler.cpp` for details on issues with interrupts being enabled by + "C" wrapper. */ - -#define ROM_xtos_c_wrapper_handler (reinterpret_cast<_xtos_handler>(0x40000598)) - -static void _set_exception_handler_wrapper(int cause) { - _xtos_handler old_wrapper = _xtos_exc_handler_table[cause]; - if (old_wrapper == ROM_xtos_c_wrapper_handler) { - _xtos_exc_handler_table[cause] = _xtos_c_wrapper_handler; - } -} - void install_non32xfer_exception_handler(void) __attribute__((weak)); void install_non32xfer_exception_handler(void) { if (NULL == old_c_handler) { @@ -240,10 +213,6 @@ void install_non32xfer_exception_handler(void) { old_c_handler = _xtos_set_exception_handler(EXCCAUSE_LOAD_STORE_ERROR, non32xfer_exception_handler); - - // Set the replacement ASM based exception "C" wrapper function which will - // be calling `non32xfer_exception_handler`. - _set_exception_handler_wrapper(EXCCAUSE_LOAD_STORE_ERROR); } } diff --git a/cores/esp8266/exc-c-wrapper-handler.S b/cores/esp8266/exc-c-wrapper-handler.S index 872702aa2..e1c3f1e15 100644 --- a/cores/esp8266/exc-c-wrapper-handler.S +++ b/cores/esp8266/exc-c-wrapper-handler.S @@ -168,9 +168,12 @@ _xtos_c_wrapper_handler: rsync // wait for WSR to PS to complete rsr a12, SAR -// @mhightower83 - I think after the next instruction we have the potential of -// loosing UEXC_excvaddr Which the earlier comment said we need to preserve for -// the exception handler. +// @mhightower83 - I think, after the next instruction, we have the potential of +// losing UEXC_excvaddr. Which the earlier comment said we need to preserve for +// the exception handler. We keep interrupts off when calling the "C" exception +// handler. For the use cases that I am looking at, this is a must. If there are +// future use cases that need interrupts enabled, those "C" exception handlers +// can turn them on. // // rsil a13, 0 @@ -192,8 +195,8 @@ _xtos_c_wrapper_handler: // load early - saves two cycles - @mhightower83 movi a0, _xtos_return_from_exc -// For compatibility with Arduino interrupt architecture, we keep interrupts 100% -// disabled. +// @mhightower83 - For compatibility with Arduino interrupt architecture, we +// keep interrupts 100% disabled. // /* // * Disable interrupts while returning from the pseudo-CALL setup above, // * for the same reason they were disabled while doing the pseudo-CALL: diff --git a/cores/esp8266/exc-sethandler.cpp b/cores/esp8266/exc-sethandler.cpp new file mode 100644 index 000000000..82f829394 --- /dev/null +++ b/cores/esp8266/exc-sethandler.cpp @@ -0,0 +1,113 @@ +/* + * Adaptation of _xtos_set_exception_handler for Arduino ESP8266 core + * + * This replacement for the Boot ROM `_xtos_set_exception_handler` is used to + * install our replacement `_xtos_c_wrapper_handler`. This change protects the + * value of `excvaddr` from corruption. + * + * + * Details + * + * The issue, the Boot ROM "C" wrapper for exception handlers, + * `_xtos_c_wrapper_handler`, turns interrupts back on. This leaves `excvaddr` + * exposed to possible overwrite before it is read. For example, if an interrupt + * is taken during the exception handler processing and the ISR handler + * generates a new exception, the original value of `excvaddr` is lost. To + * address this issue we have a replacement `_xtos_c_wrapper_handler` in file + * `exc-c-wrapper-handler.S`. + * + * An overview, of an exception at entry: New interrupts are blocked by EXCM + * being set. Once cleared, interrupts above the current INTLEVEL and exceptions + * (w/o creating a DoubleException) can occur. + * + * Using our replacement for `_xtos_c_wrapper_handler`, INTLEVEL is raised to 15 + * with EXCM cleared. + * + * The original Boot ROM `_xtos_c_wrapper_handler` at entry would set INTLEVEL + * to 3 with EXCM cleared, save registers, then do a `rsil 0` (interrupts fully + * enabled!) just before calling the registered "C" Exception handler. Our + * replacement keeps INTLEVEL at 15. This is needed to support the Arduino model + * of interrupts disabled while an ISR runs. + * + * And we also need it for umm_malloc to work safely with an IRAM heap from an + * ISR call. While malloc() will supply DRAM for all allocation from an ISR, we + * want free() to safely operate from an ISR to avoid a leak potential. + * + * If an exception handler needs interrupts enabled, it would be done after it + * has consumed the value of `excvaddr`. Whether such action is safe is left to + * the exception handler writer to determine. However, with our current + * architecture, I am not convinced it can be done safely. + * +*/ + +#if defined(NON32XFER_HANDLER) || defined(MMU_IRAM_HEAP) || defined(NEW_EXC_C_WRAPPER) + +/* + * The original module source code came from: + * https://github.com/qca/open-ath9k-htc-firmware/blob/master/sboot/magpie_1_1/sboot/athos/src/xtos/exc-sethandler.c + * + * It has been revised to use Arduino ESP8266 core includes, types, and + * formating. +*/ + +/* exc-sethandler.c - register an exception handler in XTOS */ + +/* + * Copyright (c) 1999-2006 Tensilica Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#include +#include "esp8266_undocumented.h" + +extern "C" { + +/* + * Register a C handler for the specified general exception + * (specified EXCCAUSE value). + */ +fn_c_exception_handler_t _xtos_set_exception_handler(int cause, fn_c_exception_handler_t fn) +{ + fn_c_exception_handler_t ret; + + if( (unsigned) cause >= XCHAL_EXCCAUSE_NUM ) + return 0; + + if( fn == 0 ) + fn = &_xtos_p_none; + + ret = _xtos_c_handler_table[cause]; + + _xtos_exc_handler_table[cause] = ( (fn == &_xtos_p_none) + ? &_xtos_unhandled_exception + : &_xtos_c_wrapper_handler ); + + _xtos_c_handler_table[cause] = fn; + + if( ret == &_xtos_p_none ) + ret = 0; + + return ret; +} + +}; + +#endif