From 51f4e57fdd249c29e68bbb5a5920a7fcd1f0cde2 Mon Sep 17 00:00:00 2001 From: Gerson Fernando Budke Date: Tue, 6 Apr 2021 22:24:42 -0300 Subject: [PATCH 1/5] tests: add test for context switch from ISR This try to explore a potential issue that may happen at context switch inside ISR. Signed-off-by: Gerson Fernando Budke --- boards/atxmega-a1u-xpro/include/board.h | 4 + tests/isr_context_switch_by_ext_int/Makefile | 8 ++ tests/isr_context_switch_by_ext_int/main.c | 111 ++++++++++++++++++ .../tests/01-run.py | 20 ++++ 4 files changed, 143 insertions(+) create mode 100644 tests/isr_context_switch_by_ext_int/Makefile create mode 100644 tests/isr_context_switch_by_ext_int/main.c create mode 100755 tests/isr_context_switch_by_ext_int/tests/01-run.py diff --git a/boards/atxmega-a1u-xpro/include/board.h b/boards/atxmega-a1u-xpro/include/board.h index 8ac664712d7f..0e177b539c07 100644 --- a/boards/atxmega-a1u-xpro/include/board.h +++ b/boards/atxmega-a1u-xpro/include/board.h @@ -72,6 +72,10 @@ extern "C" { #define BTN0_PIN GPIO_PIN(PORT_Q, 2) #define BTN0_MODE (GPIO_IN | GPIO_OPC_PU | GPIO_SLEW_RATE) #define BTN0_INT_FLANK (GPIO_ISC_FALLING | GPIO_LVL_LOW) + +#define BTN1_PIN GPIO_PIN(PORT_C, 2) +#define BTN1_MODE (GPIO_IN | GPIO_OPC_PU | GPIO_SLEW_RATE) +#define BTN1_INT_FLANK (GPIO_ISC_FALLING | GPIO_LVL_LOW) /** @} */ /** diff --git a/tests/isr_context_switch_by_ext_int/Makefile b/tests/isr_context_switch_by_ext_int/Makefile new file mode 100644 index 000000000000..a82d8b6725d8 --- /dev/null +++ b/tests/isr_context_switch_by_ext_int/Makefile @@ -0,0 +1,8 @@ +include ../Makefile.tests_common + +FEATURES_REQUIRED += periph_gpio +FEATURES_OPTIONAL += periph_gpio_irq + +include $(RIOTBASE)/Makefile.include + +CFLAGS += -DSCHED_TEST_STACK diff --git a/tests/isr_context_switch_by_ext_int/main.c b/tests/isr_context_switch_by_ext_int/main.c new file mode 100644 index 000000000000..2f7cb7d49716 --- /dev/null +++ b/tests/isr_context_switch_by_ext_int/main.c @@ -0,0 +1,111 @@ +/* + * Copyright (C) 2020 Otto-von-Guericke-Universität Magdeburg + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @ingroup tests + * @{ + * + * @file + * @brief Application for testing context switching triggered from IRQ + * + * @author Marian Buschsieweke + * + * @} + */ + +#include + +#include "mutex.h" +#include "thread.h" +#include "periph/gpio.h" +#include "board.h" + +#ifndef TEST_REPETITIONS +#define TEST_REPETITIONS 500 +#endif + +static mutex_t mut1 = MUTEX_INIT_LOCKED; +static mutex_t mut2 = MUTEX_INIT_LOCKED; +static mutex_t mut3 = MUTEX_INIT_LOCKED; + +static char t2_stack[THREAD_STACKSIZE_DEFAULT]; +static char t3_stack[THREAD_STACKSIZE_DEFAULT]; + +static uint16_t counter = 0; + +static void cb_btn0(void *arg) +{ + (void) arg; + thread_yield_higher(); + if (counter++ == TEST_REPETITIONS) { + mutex_unlock(&mut1); + } + else { + mutex_unlock(&mut2); + gpio_toggle(BTN1_PIN); + gpio_toggle(BTN1_PIN); + } +} + +static void cb_btn1(void *arg) +{ + (void) arg; + thread_yield_higher(); + if (counter++ == TEST_REPETITIONS) { + mutex_unlock(&mut1); + } + else { + mutex_unlock(&mut3); + gpio_toggle(BTN0_PIN); + gpio_toggle(BTN0_PIN); + } +} + +static void *t2_impl(void *unused) +{ + (void)unused; + while (1) { + mutex_lock(&mut2); + } + return NULL; +} + +static void *t3_impl(void *unused) +{ + (void)unused; + while (1) { + mutex_lock(&mut3); + } + return NULL; +} + +int main(void) +{ + printf("Testing %u context switches triggered from ISR\n", (unsigned)TEST_REPETITIONS); + thread_create(t2_stack, sizeof(t2_stack), + THREAD_PRIORITY_MAIN - 1, + THREAD_CREATE_STACKTEST, + t2_impl, NULL, "t2"); + + thread_create(t3_stack, sizeof(t3_stack), + THREAD_PRIORITY_MAIN - 1, + THREAD_CREATE_STACKTEST, + t3_impl, NULL, "t3"); + + gpio_init_int(BTN0_PIN, GPIO_OUT, BTN0_INT_FLANK, cb_btn0, NULL); + gpio_init_int(BTN1_PIN, GPIO_OUT, BTN1_INT_FLANK, cb_btn1, NULL); + + gpio_toggle(BTN0_PIN); + gpio_toggle(BTN0_PIN); + + mutex_lock(&mut1); + + puts("TEST PASSED"); + + return 0; +} diff --git a/tests/isr_context_switch_by_ext_int/tests/01-run.py b/tests/isr_context_switch_by_ext_int/tests/01-run.py new file mode 100755 index 000000000000..a2614823d9e1 --- /dev/null +++ b/tests/isr_context_switch_by_ext_int/tests/01-run.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 + +# Copyright (C) 2021 Otto-von-Guericke-Universität Magdeburg +# +# This file is subject to the terms and conditions of the GNU Lesser +# General Public License v2.1. See the file LICENSE in the top level +# directory for more details. + +# @author Marian Buschsieweke + +import sys +from testrunner import run + + +def testfunc(child): + child.expect("TEST PASSED") + + +if __name__ == "__main__": + sys.exit(run(testfunc)) From 2fc698a7943329bdd35f290a62a51122e64c7a66 Mon Sep 17 00:00:00 2001 From: Gerson Fernando Budke Date: Sun, 14 Mar 2021 16:10:51 -0300 Subject: [PATCH 2/5] cpu/avr8_common: Rework avr-8 interrupts AVR-8 supports nested interrupts but there is no infrastructure to use that. This refactor infraestrocuture and enable the enhancement. It rework the ISR processing to use a shared stack implemeting the isr prolog/epilog/return. Signed-off-by: Gerson Fernando Budke --- cpu/avr8_common/include/cpu.h | 119 ++++++++++-- cpu/avr8_common/include/irq_arch.h | 2 +- cpu/avr8_common/include/states_internal.h | 77 +++++++- cpu/avr8_common/thread_arch.c | 209 ++++++++++++++++++++-- 4 files changed, 372 insertions(+), 35 deletions(-) diff --git a/cpu/avr8_common/include/cpu.h b/cpu/avr8_common/include/cpu.h index 6e183661e74f..364534fb05e0 100644 --- a/cpu/avr8_common/include/cpu.h +++ b/cpu/avr8_common/include/cpu.h @@ -67,18 +67,6 @@ extern "C" #define PERIPH_I2C_NEED_WRITE_REGS /** @} */ -/** - * @brief Run this code on entering interrupt routines - */ -static inline void avr8_enter_isr(void) -{ - /* This flag is only called from IRQ context, and nested IRQs are not - * supported as of now. The flag will be unset before the IRQ context is - * left, so no need to use memory barriers or atomics here - */ - ++avr8_state_irq_count; -} - /** * @brief Compute UART TX channel * @@ -117,10 +105,115 @@ static inline int avr8_is_uart_tx_pending(void) return avr8_state_uart; } +/** + * @brief Enter ISR routine + * + * It saves the register context for process an ISR. + */ +__attribute__((always_inline)) +static inline void avr8_isr_prolog(void) +{ + /* This flag is only called from IRQ context. The value will be handled + * before ISR context is left by @ref avr8_isr_epilog. ATxmega requires a + * cli() as first instruction inside an ISR to create a critical section + * around `avr8_state_irq_count`. + */ + __asm__ volatile ( +#if defined(CPU_ATXMEGA) + "cli \n\t" +#endif + /* Register pair r24/25 are used to update `avr8_state_irq_count` content. + * This registers are used to test conditions related to context switch in + * ISR at @ref avr8_isr_epilog. + */ + "push r24 \n\t" + "push r25 \n\t" +#if (AVR8_STATE_IRQ_USE_SRAM) + "lds r24, %[state] \n\t" + "subi r24, 0xFF \n\t" + "sts %[state], r24 \n\t" +#else + "in r24, %[state] \n\t" + "subi r24, 0xFF \n\t" + "out %[state], r24 \n\t" +#endif +#if defined(CPU_ATXMEGA) + "sei \n\t" +#endif + "push __zero_reg__ \n\t" + "push __tmp_reg__ \n\t" + "in __tmp_reg__, __SREG__ \n\t" + "push __tmp_reg__ \n\t" + "eor __zero_reg__, __zero_reg__ \n\t" +#if __AVR_HAVE_RAMPD__ + "in __tmp_reg__, __RAMPD__ \n\t" + "push __tmp_reg__ \n\t" + "out __RAMPD__, __zero_reg__ \n\t" +#endif +#if __AVR_HAVE_RAMPX__ + "in __tmp_reg__, __RAMPX__ \n\t" + "push __tmp_reg__ \n\t" + "out __RAMPX__, __zero_reg__ \n\t" +#endif +#if __AVR_HAVE_RAMPZ__ + "in __tmp_reg__, __RAMPZ__ \n\t" + "push __tmp_reg__ \n\t" + "out __RAMPZ__, __zero_reg__ \n\t" +#endif + "push r18 \n\t" + "push r19 \n\t" + "push r20 \n\t" + "push r21 \n\t" + "push r22 \n\t" + "push r23 \n\t" + "push r26 \n\t" + "push r27 \n\t" + "push r30 \n\t" + "push r31 \n\t" + : /* no output */ +#if (AVR8_STATE_IRQ_USE_SRAM) + : [state] "" (avr8_state_irq_count) +#else + : [state] "I" (_SFR_IO_ADDR(avr8_state_irq_count)) +#endif + : "memory" + ); +} + +/** + * @brief Restore register context from ISR + */ +__attribute__((naked)) +void avr8_isr_epilog(void); + +/** + * @brief Run this code on entering interrupt routines + * + * Notes: + * - This code must not be shared. + * - This code must not be a call or jmp. + */ +__attribute__((always_inline)) +static inline void avr8_enter_isr(void) +{ + avr8_isr_prolog(); +} + /** * @brief Run this code on exiting interrupt routines */ -void avr8_exit_isr(void); +__attribute__((always_inline)) +static inline void avr8_exit_isr(void) +{ + /* Let's not add more address in stack and save some code sharing + * avr8_isr_epilog. + */ +#if (FLASHEND <= 0x1FFF) + __asm__ volatile ("rjmp avr8_isr_epilog" : : : "memory"); +#else + __asm__ volatile ("jmp avr8_isr_epilog" : : : "memory"); +#endif +} /** * @brief Initialization of the CPU clock diff --git a/cpu/avr8_common/include/irq_arch.h b/cpu/avr8_common/include/irq_arch.h index 5ec04ede5180..eaf1ae08f0e0 100644 --- a/cpu/avr8_common/include/irq_arch.h +++ b/cpu/avr8_common/include/irq_arch.h @@ -146,7 +146,7 @@ __attribute__((always_inline)) static inline bool irq_is_enabled(void) * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ #define AVR8_ISR(vector, function, ...) \ - ISR(vector, ISR_BLOCK) \ + ISR(vector, ISR_NAKED) \ { \ avr8_enter_isr(); \ function(__VA_ARGS__); \ diff --git a/cpu/avr8_common/include/states_internal.h b/cpu/avr8_common/include/states_internal.h index 788c7c20993b..cc403a7ae31e 100644 --- a/cpu/avr8_common/include/states_internal.h +++ b/cpu/avr8_common/include/states_internal.h @@ -84,18 +84,77 @@ extern uint8_t avr8_state_uart_sram; /**< UART state variable. */ /** @} */ /** - * @name Global variable containing the current state of the MCU - * @{ - * - * @note This variable is updated from IRQ context; access to it should - * be wrapped into @ref irq_disable and @ref irq_restore should be - * used. + * @brief MCU ISR State * * Contents: * - * | Label | Description | - * |:-------|:--------------------------------------------------------------| - * | IRQ | This variable is incremented when in IRQ context | + * The `avr8_state_irq_count` is used to indicate that system is processing an + * ISR for AVR-8 devices. It stores how deep system is processing a nested + * interrupt. ATxmega have three selectable interrupt levels for any interrupt: + * low, medium and high and are controlled by PMIC. ATmega requires that users + * re-enable interrupts after executing the `_isr_handle` method to enable + * nested IRQs in low priority interrupts. ATxmega PMIC is not enough to + * determine the current state of MCU ISR in RIOT-OS because scheduler may + * perform a context switch inside any IRQ. + * + * If the system is running outside an interrupt, `avr8_state_irq_count` will + * have a 0 value. When one or more interrupt vectors are activated, + * `avr8_state_irq_count` will be incremented and have a value greater than 0. + * These operations are performed by the pair @ref avr8_enter_isr and + * @ref avr8_exit_isr. + * + * An 3-level nested IRQ illustration can be visualized below: + * + * int-3 + * ↯ + * +----------+ + * | high lvl | + * int-2 +----------+ + * ↯ | | + * +---------+ +---------+ + * | mid lvl | | mid lvl | + * int-1 +---------+ +---------+ + * ↯ | | + * +---------+ +---------+ + * | low lvl | | low lvl | + * +---------+ +---------+ ↯ can switch + * | | context here + * +----------+ +----------+ + * | thread A | | thread B | + * +----------+ +----------+ + * + * At @ref avr8_exit_isr scheduler may require a switch context. That can be + * performed only when `avr8_state_irq_count` is equal to zero. This is + * necessary to avoid stack corruption and since system is processing interrupts + * since priority inheritance happened. + * + * The AVR-8 allow nested IRQ and MCUs cores perform with different behaviour, a + * custom IRQ management was developed. The code is simple and must use the + * following skeleton: + * + * The `_isr_handle` method represents the body of an ISR routine. This is the + * code that handles the IRQ itself. It can be a shared function and don't + * need any special care. + * + * The IRQ method `_isr_handle` should have minimal parameters. It is + * recommended up to 2 and convention is to use the first parameter as the + * instance and the second parameter usually represent the unit inside that + * instance. + * + * Example: UART uses 1 parameter and Timers, usually 2. + * + * static inline void _uart_isr_handler(instance) + * { + * ... + * } + * AVR8_ISR(UART_0_RXC_ISR, _uart_isr_handler, 0); + * + * static inline void _tmr_isr_handler(instance, channel) + * { + * ... + * } + * AVR8_ISR(TIMER_0_ISRA, _tmr_isr_handler, 0, 0); + * AVR8_ISR(TIMER_0_ISRB, _tmr_isr_handler, 0, 1); */ #if (AVR8_STATE_IRQ_USE_SRAM) extern uint8_t avr8_state_irq_count_sram; /**< IRQ state variable. */ diff --git a/cpu/avr8_common/thread_arch.c b/cpu/avr8_common/thread_arch.c index 1806db364b4c..b2c9b88647f3 100644 --- a/cpu/avr8_common/thread_arch.c +++ b/cpu/avr8_common/thread_arch.c @@ -38,7 +38,16 @@ #define __EIND__ 0x3C #endif -static void avr8_context_save(void); +/** + * The AVR-8 Context Save is divided in two parts: Header & Body + * The Header consists of: __tmp_reg__ and SREG + * The Body consists of remaining registers + * + * The Header in Thread + */ +static void avr8_context_save_header_in_thread(void); +static void avr8_context_save_header_in_isr(void); +static void avr8_context_save_body(void); static void avr8_context_restore(void); static void avr8_enter_thread_mode(void); @@ -261,7 +270,8 @@ void NORETURN avr8_enter_thread_mode(void) void thread_yield_higher(void) { if (irq_is_in() == 0) { - avr8_context_save(); + avr8_context_save_header_in_thread(); + avr8_context_save_body(); sched_run(); avr8_context_restore(); } @@ -270,30 +280,204 @@ void thread_yield_higher(void) } } -void avr8_exit_isr(void) +/** + * @brief Restore remaining thread context + * + * The tail part is responsible to restore r1, r25 and r24. Those register + * were used to update `avr8_state_irq_count` and perform tests related to + * switch context. + */ +__attribute__((always_inline)) +static inline void avr8_isr_epilog_tail(void) { - --avr8_state_irq_count; + __asm__ volatile ( + "pop __zero_reg__ \n\t" + "pop r25 \n\t" + "pop r24 \n\t" + : + : + : "memory" + ); +} - /* Force access to avr8_state to take place */ - __asm__ volatile ("" : : : "memory"); +/** + * @brief Restore thread/IRQ context + * + * To allow nested interrupts in RIOT-OS multi thread environment, + * `avr8_state_irq_count` is used as IRQ global state. This means that a + * context switch must happen when `avr8_state_irq_count` is equal to 0. The + * pair r24/r25 is postponed to save/restore on the stack because are used at + * IRQ epilog. Those registers are used to update `avr8_state_irq_count`, SREG + * and execute branch condition tests. All AVR-8 requires a critical section + * between updating the `avr8_state_irq_count` value and branch tests at epilog. + * This avoids that a high priority IRQ read an invalid value of + * `avr8_state_irq_count` and start a context switch. The execution of context + * switch out-of-order will corrupt at ISR epilog. + */ +void avr8_isr_epilog(void) +{ + uint8_t isr_sts; - /* schedule should switch context when returning from a non nested interrupt */ - if (sched_context_switch_request && avr8_state_irq_count == 0) { - avr8_context_save(); + __asm__ volatile ( + /* Force zero_reg to ensure ISR body not changed it, this is be necessary + * to test isr_sts value + */ + "eor __zero_reg__, __zero_reg__ \n\t" + + /* Restore thread context + */ + "pop r31 \n\t" + "pop r30 \n\t" + "pop r27 \n\t" + "pop r26 \n\t" + "pop r23 \n\t" + "pop r22 \n\t" + "pop r21 \n\t" + "pop r20 \n\t" + "pop r19 \n\t" + "pop r18 \n\t" +#if __AVR_HAVE_RAMPZ__ + "pop __tmp_reg__ \n\t" + "out __RAMPZ__, __tmp_reg__ \n\t" +#endif +#if __AVR_HAVE_RAMPX__ + "pop __tmp_reg__ \n\t" + "out __RAMPX__, __tmp_reg__ \n\t" +#endif +#if __AVR_HAVE_RAMPD__ + "pop __tmp_reg__ \n\t" + "out __RAMPD__, __tmp_reg__ \n\t" +#endif + "pop __tmp_reg__ \n\t" + "out __SREG__, __tmp_reg__ \n\t" + "pop __tmp_reg__ \n\t" + + /** + * ISR Epilog for Nested ISR Condition + * + * This critical section may be finished in following situations: + * - Last 'pop' instruction in @ref avr8_context_restore; + * - Last instructions from current avr8_isr_epilog. + * + * Note: + * - ATmega may be affected when IRQ is re-enabled on ISR body by the user. + */ + "cli \n\t" +#if AVR8_STATE_IRQ_USE_SRAM + "lds r24, %[state] \n\t" + "subi r24, 0x01 \n\t" + "sts %[state], r24 \n\t" +#else + "in r24, %[state] \n\t" + "subi r24, 0x01 \n\t" + "out %[state], r24 \n\t" +#endif + "mov %[isr_sts], r24 \n\t" + : [isr_sts] "=r" (isr_sts) +#if (AVR8_STATE_IRQ_USE_SRAM) + : [state] "" (avr8_state_irq_count) +#else + : [state] "I" (_SFR_IO_ADDR(avr8_state_irq_count)) +#endif + : "memory" + ); + + /* Branch Tests - critical section. + * Note: isr_sts was optimized to avoid a second load and perform fastest + * tests for critical tasks. Do not switch position from isr_sts with + * sched_context_switch_request. Care is necessary to test r24 before + * reloaded with sched_context_switch_request value. + */ + if (isr_sts == 0 && sched_context_switch_request) { + /* A context switch was requested in the body of interrupt: + * + * `avr8_state_irq_count` is 0 and; + * `sched_context_switch_request` is greater than 0 + * + * - Restore remaining context register r24/25 + * - Start context save using the current critical section at #ref + * avr8_context_save_header_in_isr + * - For ATxmega, IRQ should be re-enabled because reti instruction + * don't do it! In this case, it should be re-enabled because a + * context switch may occor by @ref thread_yield_higher and + * epilog needs guarantee that IRQ are enabled for that case. + * However, care is required because it can broke the critical + * section. This will be performed at + * `avr8_context_save_header_in_isr`. + * - Context switch + * - At top of stack, the previous context can be found or a another + * one that was just reload from @ref sched_run. + */ + avr8_isr_epilog_tail(); + avr8_context_save_header_in_isr(); + avr8_context_save_body(); sched_run(); avr8_context_restore(); - __asm__ volatile ("reti"); } + else { + /* Don't perform a context switch. Simple finish context restore + * before leave ISR. + */ + avr8_isr_epilog_tail(); + } + + /* ATxmega always runs with IRQ enabled, let's enabled it! For ATmega, + * reti instruction will do it! + */ + __asm__ volatile ( +#if defined(CPU_ATXMEGA) + "sei \n\t" +#endif + "reti \n\t" + : : : "memory" + ); } -__attribute__((always_inline)) static inline void avr8_context_save(void) +/** + * @brief Saves context header in thread context + * + * This saves temporary register and SREG. Keep SREG with their original + * value. The normal procedure should use this context save header. + */ +__attribute__((always_inline)) +static inline void avr8_context_save_header_in_thread(void) { __asm__ volatile ( "push __tmp_reg__ \n\t" "in __tmp_reg__, __SREG__ \n\t" "cli \n\t" + "push __tmp_reg__ \n\t"); +} + +/** + * @brief Saves context header in ISR + * + * This saves temporary register and SREG. The epilog critical section + * disables global IRQ. This code must re-enabled the context IRQ without + * disabling the current critical section. + */ +__attribute__((always_inline)) +static inline void avr8_context_save_header_in_isr(void) +{ + __asm__ volatile ( "push __tmp_reg__ \n\t" + "push r24 \n\t" + "in r24, __SREG__ \n\t" + "sbr r24, 0x80 \n\t" + "mov __tmp_reg__, r24 \n\t" + "pop r24 \n\t" + "push __tmp_reg__ \n\t"); +} +/** + * @brief Saves context body + * + * This saves remaining context registers. + */ +__attribute__((always_inline)) +static inline void avr8_context_save_body(void) +{ + __asm__ volatile ( #if __AVR_HAVE_RAMPZ__ "in __tmp_reg__, __RAMPZ__ \n\t" "push __tmp_reg__ \n\t" @@ -359,7 +543,8 @@ __attribute__((always_inline)) static inline void avr8_context_save(void) "st x+, __tmp_reg__ \n\t"); } -__attribute__((always_inline)) static inline void avr8_context_restore(void) +__attribute__((always_inline)) +static inline void avr8_context_restore(void) { __asm__ volatile ( "lds r26, sched_active_thread \n\t" From 8db0bfc41ad096235effbe7f30e8c0fc745e2a7b Mon Sep 17 00:00:00 2001 From: Gerson Fernando Budke Date: Sun, 2 Jul 2023 16:02:25 +0200 Subject: [PATCH 3/5] cpu/avr8_common: Fix assembly code style Adjust the code style of cpu_get_caller_pc function. Signed-off-by: Gerson Fernando Budke --- cpu/avr8_common/include/cpu.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cpu/avr8_common/include/cpu.h b/cpu/avr8_common/include/cpu.h index 364534fb05e0..3f5174117dde 100644 --- a/cpu/avr8_common/include/cpu.h +++ b/cpu/avr8_common/include/cpu.h @@ -230,18 +230,18 @@ static inline uinttxtptr_t __attribute__((always_inline)) cpu_get_caller_pc(void { uinttxtptr_t addr; __asm__ volatile( - "ldi %D[dest], 0" "\n\t" + "ldi %D[dest], 0 \n\t" #if __AVR_3_BYTE_PC__ - "pop %C[dest] " "\n\t" + "pop %C[dest] \n\t" #else - "ldi %C[dest], 0" "\n\t" + "ldi %C[dest], 0 \n\t" #endif - "pop %B[dest]" "\n\t" - "pop %A[dest]" "\n\t" - "push %A[dest]" "\n\t" - "push %B[dest]" "\n\t" + "pop %B[dest] \n\t" + "pop %A[dest] \n\t" + "push %A[dest] \n\t" + "push %B[dest] \n\t" #if __AVR_3_BYTE_PC__ - "push %C[dest] " "\n\t" + "push %C[dest] \n\t" #endif : [dest] "=r"(addr) : /* no inputs */ From cd5f955e714702a086f707abac183d0f5525bf7b Mon Sep 17 00:00:00 2001 From: Gerson Fernando Budke Date: Wed, 5 Jul 2023 20:55:41 +0200 Subject: [PATCH 4/5] cpu/avr8_common: Optimize context ISR switch The current context switch implementaion don't take in consideration the call-used registers. This means that ISR and context switch can not be optimized. This rework both context switch and ISR save/restore order to optimize the context switch on ISR and reduce code size. This chance should improve the performance of AVR platform on RIOT-OS if is compared with the 2023.04 release. Signed-off-by: Gerson Fernando Budke --- cpu/avr8_common/include/cpu.h | 73 ++++--- cpu/avr8_common/thread_arch.c | 358 +++++++++++++++------------------- 2 files changed, 192 insertions(+), 239 deletions(-) diff --git a/cpu/avr8_common/include/cpu.h b/cpu/avr8_common/include/cpu.h index 3f5174117dde..8aba66bcfa1e 100644 --- a/cpu/avr8_common/include/cpu.h +++ b/cpu/avr8_common/include/cpu.h @@ -119,57 +119,54 @@ static inline void avr8_isr_prolog(void) * around `avr8_state_irq_count`. */ __asm__ volatile ( -#if defined(CPU_ATXMEGA) - "cli \n\t" -#endif /* Register pair r24/25 are used to update `avr8_state_irq_count` content. * This registers are used to test conditions related to context switch in * ISR at @ref avr8_isr_epilog. */ - "push r24 \n\t" - "push r25 \n\t" + "push r30 \n\t" + "in r30, __SREG__ \n\t" + "push r30 \n\t" + "push r31 \n\t" +#if defined(CPU_ATXMEGA) + "cli \n\t" +#endif #if (AVR8_STATE_IRQ_USE_SRAM) - "lds r24, %[state] \n\t" - "subi r24, 0xFF \n\t" - "sts %[state], r24 \n\t" + "lds r30, %[state] \n\t" + "subi r30, 0xFF \n\t" + "sts %[state], r30 \n\t" #else - "in r24, %[state] \n\t" - "subi r24, 0xFF \n\t" - "out %[state], r24 \n\t" + "in r30, %[state] \n\t" + "subi r30, 0xFF \n\t" + "out %[state], r30 \n\t" #endif #if defined(CPU_ATXMEGA) - "sei \n\t" + "sei \n\t" #endif - "push __zero_reg__ \n\t" - "push __tmp_reg__ \n\t" - "in __tmp_reg__, __SREG__ \n\t" - "push __tmp_reg__ \n\t" - "eor __zero_reg__, __zero_reg__ \n\t" -#if __AVR_HAVE_RAMPD__ - "in __tmp_reg__, __RAMPD__ \n\t" - "push __tmp_reg__ \n\t" - "out __RAMPD__, __zero_reg__ \n\t" +#if __AVR_HAVE_RAMPZ__ + "in r30, __RAMPZ__ \n\t" + "push r30 \n\t" #endif #if __AVR_HAVE_RAMPX__ - "in __tmp_reg__, __RAMPX__ \n\t" - "push __tmp_reg__ \n\t" - "out __RAMPX__, __zero_reg__ \n\t" + "in r30, __RAMPX__ \n\t" + "push r30 \n\t" #endif -#if __AVR_HAVE_RAMPZ__ - "in __tmp_reg__, __RAMPZ__ \n\t" - "push __tmp_reg__ \n\t" - "out __RAMPZ__, __zero_reg__ \n\t" +#if __AVR_HAVE_RAMPD__ + "in r30, __RAMPD__ \n\t" + "push r30 \n\t" #endif - "push r18 \n\t" - "push r19 \n\t" - "push r20 \n\t" - "push r21 \n\t" - "push r22 \n\t" - "push r23 \n\t" - "push r26 \n\t" - "push r27 \n\t" - "push r30 \n\t" - "push r31 \n\t" + "push r0 \n\t" + "push r1 \n\t" + "clr r1 \n\t" + "push r18 \n\t" + "push r19 \n\t" + "push r20 \n\t" + "push r21 \n\t" + "push r22 \n\t" + "push r23 \n\t" + "push r24 \n\t" + "push r25 \n\t" + "push r26 \n\t" + "push r27 \n\t" : /* no output */ #if (AVR8_STATE_IRQ_USE_SRAM) : [state] "" (avr8_state_irq_count) diff --git a/cpu/avr8_common/thread_arch.c b/cpu/avr8_common/thread_arch.c index b2c9b88647f3..c591b51fbe6f 100644 --- a/cpu/avr8_common/thread_arch.c +++ b/cpu/avr8_common/thread_arch.c @@ -45,10 +45,14 @@ * * The Header in Thread */ -static void avr8_context_save_header_in_thread(void); -static void avr8_context_save_header_in_isr(void); -static void avr8_context_save_body(void); -static void avr8_context_restore(void); +static inline void avr8_context_save(void); +static inline void avr8_context_save_lower(void); +static inline void avr8_context_save_upper(void); +static inline void avr8_context_save_tcb(void); +static inline void avr8_context_restore_tcb(void); +static inline void avr8_context_restore_upper(void); +static inline void avr8_context_restore_lower(void); +static inline void avr8_context_restore(void); static void avr8_enter_thread_mode(void); /** @@ -88,6 +92,7 @@ char *thread_stack_init(thread_task_func_t task_func, void *arg, { uint16_t tmp_adress; uint8_t *stk; + int i; /* AVR uses 16 Bit or two 8 Bit registers for storing pointers*/ stk = (uint8_t *)((uintptr_t)stack_start + stack_size); @@ -128,22 +133,21 @@ char *thread_stack_init(thread_task_func_t task_func, void *arg, *stk = (uint8_t)0x00; #endif - /* r0 */ + /* r30 */ stk--; - *stk = (uint8_t)0x00; + *stk = (uint8_t)30; /* status register (with interrupts enabled) */ stk--; *stk = (uint8_t)0x80; -#if __AVR_HAVE_RAMPZ__ + /* r31 */ stk--; - *stk = (uint8_t)0x00; /* RAMPZ */ -#endif + *stk = (uint8_t)31; -#if __AVR_HAVE_RAMPY__ +#if __AVR_HAVE_RAMPZ__ stk--; - *stk = (uint8_t)0x00; /* RAMPY */ + *stk = (uint8_t)0x00; /* RAMPZ */ #endif #if __AVR_HAVE_RAMPX__ stk--; @@ -154,23 +158,21 @@ char *thread_stack_init(thread_task_func_t task_func, void *arg, *stk = (uint8_t)0x00; /* RAMPD */ #endif -#if __AVR_3_BYTE_PC__ + /* r0 - scratch register */ stk--; - *stk = (uint8_t)0x00; /* EIND */ -#endif + *stk = (uint8_t)0x00; /* r1 - has always to be 0 */ stk--; *stk = (uint8_t)0x00; + /* - * Space for registers r2 -r23 + * Space for registers r18-r23 * * use loop for better readability, the compiler unrolls anyways */ - int i; - - for (i = 2; i <= 23; i++) { + for (i = 18; i <= 23; i++) { stk--; *stk = (uint8_t)0; } @@ -187,13 +189,30 @@ char *thread_stack_init(thread_task_func_t task_func, void *arg, *stk = (uint8_t)(tmp_adress & (uint16_t)0x00ff); /* - * Space for registers r26-r31 + * Space for registers r26-r29 */ - for (i = 26; i <= 31; i++) { + for (i = 26; i <= 29; i++) { stk--; *stk = (uint8_t)i; } +#if __AVR_HAVE_RAMPY__ + stk--; + *stk = (uint8_t)0x00; /* RAMPY */ +#endif +#if __AVR_3_BYTE_PC__ + stk--; + *stk = (uint8_t)0x00; /* EIND */ +#endif + + /* + * Space for registers r2-r17 + */ + for (i = 2; i <= 17; i++) { + stk--; + *stk = (uint8_t)0; + } + stk--; return (char *)stk; } @@ -246,8 +265,6 @@ extern char *__brkval; */ void NORETURN avr8_enter_thread_mode(void) { - irq_enable(); - /* * Save the current stack pointer to __malloc_heap_end. Since * context_restore is always inline, there is no function call and the @@ -270,8 +287,7 @@ void NORETURN avr8_enter_thread_mode(void) void thread_yield_higher(void) { if (irq_is_in() == 0) { - avr8_context_save_header_in_thread(); - avr8_context_save_body(); + avr8_context_save(); sched_run(); avr8_context_restore(); } @@ -280,26 +296,6 @@ void thread_yield_higher(void) } } -/** - * @brief Restore remaining thread context - * - * The tail part is responsible to restore r1, r25 and r24. Those register - * were used to update `avr8_state_irq_count` and perform tests related to - * switch context. - */ -__attribute__((always_inline)) -static inline void avr8_isr_epilog_tail(void) -{ - __asm__ volatile ( - "pop __zero_reg__ \n\t" - "pop r25 \n\t" - "pop r24 \n\t" - : - : - : "memory" - ); -} - /** * @brief Restore thread/IRQ context * @@ -319,39 +315,6 @@ void avr8_isr_epilog(void) uint8_t isr_sts; __asm__ volatile ( - /* Force zero_reg to ensure ISR body not changed it, this is be necessary - * to test isr_sts value - */ - "eor __zero_reg__, __zero_reg__ \n\t" - - /* Restore thread context - */ - "pop r31 \n\t" - "pop r30 \n\t" - "pop r27 \n\t" - "pop r26 \n\t" - "pop r23 \n\t" - "pop r22 \n\t" - "pop r21 \n\t" - "pop r20 \n\t" - "pop r19 \n\t" - "pop r18 \n\t" -#if __AVR_HAVE_RAMPZ__ - "pop __tmp_reg__ \n\t" - "out __RAMPZ__, __tmp_reg__ \n\t" -#endif -#if __AVR_HAVE_RAMPX__ - "pop __tmp_reg__ \n\t" - "out __RAMPX__, __tmp_reg__ \n\t" -#endif -#if __AVR_HAVE_RAMPD__ - "pop __tmp_reg__ \n\t" - "out __RAMPD__, __tmp_reg__ \n\t" -#endif - "pop __tmp_reg__ \n\t" - "out __SREG__, __tmp_reg__ \n\t" - "pop __tmp_reg__ \n\t" - /** * ISR Epilog for Nested ISR Condition * @@ -364,15 +327,15 @@ void avr8_isr_epilog(void) */ "cli \n\t" #if AVR8_STATE_IRQ_USE_SRAM - "lds r24, %[state] \n\t" - "subi r24, 0x01 \n\t" - "sts %[state], r24 \n\t" + "lds r30, %[state] \n\t" + "subi r30, 0x01 \n\t" + "sts %[state], r30 \n\t" #else - "in r24, %[state] \n\t" - "subi r24, 0x01 \n\t" - "out %[state], r24 \n\t" + "in r30, %[state] \n\t" + "subi r30, 0x01 \n\t" + "out %[state], r30 \n\t" #endif - "mov %[isr_sts], r24 \n\t" + "mov %[isr_sts], r30 \n\t" : [isr_sts] "=r" (isr_sts) #if (AVR8_STATE_IRQ_USE_SRAM) : [state] "" (avr8_state_irq_count) @@ -408,19 +371,15 @@ void avr8_isr_epilog(void) * - At top of stack, the previous context can be found or a another * one that was just reload from @ref sched_run. */ - avr8_isr_epilog_tail(); - avr8_context_save_header_in_isr(); - avr8_context_save_body(); + avr8_context_save_upper(); + avr8_context_save_tcb(); sched_run(); - avr8_context_restore(); - } - else { - /* Don't perform a context switch. Simple finish context restore - * before leave ISR. - */ - avr8_isr_epilog_tail(); + avr8_context_restore_tcb(); + avr8_context_restore_upper(); } + avr8_context_restore_lower(); + /* ATxmega always runs with IRQ enabled, let's enabled it! For ATmega, * reti instruction will do it! */ @@ -434,77 +393,66 @@ void avr8_isr_epilog(void) } /** - * @brief Saves context header in thread context + * @brief Saves context body * - * This saves temporary register and SREG. Keep SREG with their original - * value. The normal procedure should use this context save header. + * This saves remaining context registers. */ __attribute__((always_inline)) -static inline void avr8_context_save_header_in_thread(void) +static inline void avr8_context_save(void) { - __asm__ volatile ( - "push __tmp_reg__ \n\t" - "in __tmp_reg__, __SREG__ \n\t" - "cli \n\t" - "push __tmp_reg__ \n\t"); + avr8_context_save_lower(); + avr8_context_save_upper(); + avr8_context_save_tcb(); } -/** - * @brief Saves context header in ISR - * - * This saves temporary register and SREG. The epilog critical section - * disables global IRQ. This code must re-enabled the context IRQ without - * disabling the current critical section. - */ __attribute__((always_inline)) -static inline void avr8_context_save_header_in_isr(void) +static inline void avr8_context_save_lower(void) { __asm__ volatile ( - "push __tmp_reg__ \n\t" + "push r30 \n\t" + "in r30, __SREG__ \n\t" + "push r30 \n\t" + "push r31 \n\t" +#if __AVR_HAVE_RAMPZ__ + "in r30, __RAMPZ__ \n\t" + "push r30 \n\t" +#endif +#if __AVR_HAVE_RAMPX__ + "in r30, __RAMPX__ \n\t" + "push r30 \n\t" +#endif +#if __AVR_HAVE_RAMPD__ + "in r30, __RAMPD__ \n\t" + "push r30 \n\t" +#endif + "push r0 \n\t" + "push r1 \n\t" + "clr r1 \n\t" + "push r18 \n\t" + "push r19 \n\t" + "push r20 \n\t" + "push r21 \n\t" + "push r22 \n\t" + "push r23 \n\t" "push r24 \n\t" - "in r24, __SREG__ \n\t" - "sbr r24, 0x80 \n\t" - "mov __tmp_reg__, r24 \n\t" - "pop r24 \n\t" - "push __tmp_reg__ \n\t"); + "push r25 \n\t" + "push r26 \n\t" + "push r27 \n\t"); } - -/** - * @brief Saves context body - * - * This saves remaining context registers. - */ __attribute__((always_inline)) -static inline void avr8_context_save_body(void) +static inline void avr8_context_save_upper(void) { __asm__ volatile ( -#if __AVR_HAVE_RAMPZ__ - "in __tmp_reg__, __RAMPZ__ \n\t" - "push __tmp_reg__ \n\t" -#endif - + "push r28 \n\t" + "push r29 \n\t" #if __AVR_HAVE_RAMPY__ - "in __tmp_reg__, __RAMPY__ \n\t" - "push __tmp_reg__ \n\t" -#endif - -#if __AVR_HAVE_RAMPX__ - "in __tmp_reg__, __RAMPX__ \n\t" - "push __tmp_reg__ \n\t" -#endif - -#if __AVR_HAVE_RAMPD__ - "in __tmp_reg__, __RAMPD__ \n\t" - "push __tmp_reg__ \n\t" + "in r30, __RAMPY__ \n\t" + "push r30 \n\t" #endif - #if __AVR_3_BYTE_PC__ - "in __tmp_reg__, " XTSTR(__EIND__) " \n\t" - "push __tmp_reg__ \n\t" + "in r30, " XTSTR(__EIND__) " \n\t" + "push r30 \n\t" #endif - - "push r1 \n\t" - "clr r1 \n\t" "push r2 \n\t" "push r3 \n\t" "push r4 \n\t" @@ -520,53 +468,45 @@ static inline void avr8_context_save_body(void) "push r14 \n\t" "push r15 \n\t" "push r16 \n\t" - "push r17 \n\t" - "push r18 \n\t" - "push r19 \n\t" - "push r20 \n\t" - "push r21 \n\t" - "push r22 \n\t" - "push r23 \n\t" - "push r24 \n\t" - "push r25 \n\t" - "push r26 \n\t" - "push r27 \n\t" - "push r28 \n\t" - "push r29 \n\t" - "push r30 \n\t" - "push r31 \n\t" + "push r17 \n\t"); +} + +__attribute__((always_inline)) +static inline void avr8_context_save_tcb(void) +{ + __asm__ volatile ( "lds r26, sched_active_thread \n\t" "lds r27, sched_active_thread + 1 \n\t" - "in __tmp_reg__, __SP_L__ \n\t" - "st x+, __tmp_reg__ \n\t" - "in __tmp_reg__, __SP_H__ \n\t" - "st x+, __tmp_reg__ \n\t"); + "in r28, __SP_L__ \n\t" + "st x+, r28 \n\t" + "in r29, __SP_H__ \n\t" + "st x+, r29 \n\t"); } __attribute__((always_inline)) static inline void avr8_context_restore(void) { - __asm__ volatile ( + avr8_context_restore_tcb(); + avr8_context_restore_upper(); + avr8_context_restore_lower(); +} + +__attribute__((always_inline)) +static inline void avr8_context_restore_tcb(void) +{ + __asm__ volatile ( "lds r26, sched_active_thread \n\t" "lds r27, sched_active_thread + 1 \n\t" "ld r28, x+ \n\t" "out __SP_L__, r28 \n\t" "ld r29, x+ \n\t" - "out __SP_H__, r29 \n\t" - "pop r31 \n\t" - "pop r30 \n\t" - "pop r29 \n\t" - "pop r28 \n\t" - "pop r27 \n\t" - "pop r26 \n\t" - "pop r25 \n\t" - "pop r24 \n\t" - "pop r23 \n\t" - "pop r22 \n\t" - "pop r21 \n\t" - "pop r20 \n\t" - "pop r19 \n\t" - "pop r18 \n\t" + "out __SP_H__, r29 \n\t"); +} + +__attribute__((always_inline)) +static inline void avr8_context_restore_upper(void) +{ + __asm__ volatile ( "pop r17 \n\t" "pop r16 \n\t" "pop r15 \n\t" @@ -583,32 +523,48 @@ static inline void avr8_context_restore(void) "pop r4 \n\t" "pop r3 \n\t" "pop r2 \n\t" - "pop r1 \n\t" - #if __AVR_3_BYTE_PC__ - "pop __tmp_reg__ \n\t" - "out " XTSTR(__EIND__) ", __tmp_reg__ \n\t" + "pop r30 \n\t" + "out " XTSTR(__EIND__) ", r30 \n\t" #endif +#if __AVR_HAVE_RAMPY__ + "pop r30 \n\t" + "out __RAMPY__, r30 \n\t" +#endif + "pop r29 \n\t" + "pop r28 \n\t"); +} +__attribute__((always_inline)) +static inline void avr8_context_restore_lower(void) +{ + __asm__ volatile ( + "pop r27 \n\t" + "pop r26 \n\t" + "pop r25 \n\t" + "pop r24 \n\t" + "pop r23 \n\t" + "pop r22 \n\t" + "pop r21 \n\t" + "pop r20 \n\t" + "pop r19 \n\t" + "pop r18 \n\t" + "pop r1 \n\t" + "pop r0 \n\t" #if __AVR_HAVE_RAMPD__ - "pop __tmp_reg__ \n\t" - "out __RAMPD__, __tmp_reg__ \n\t" + "pop r30 \n\t" + "out __RAMPD__, r30 \n\t" #endif #if __AVR_HAVE_RAMPX__ - "pop __tmp_reg__ \n\t" - "out __RAMPX__, __tmp_reg__ \n\t" -#endif -#if __AVR_HAVE_RAMPY__ - "pop __tmp_reg__ \n\t" - "out __RAMPY__, __tmp_reg__ \n\t" + "pop r30 \n\t" + "out __RAMPX__, r30 \n\t" #endif - #if __AVR_HAVE_RAMPZ__ - "pop __tmp_reg__ \n\t" - "out __RAMPZ__, __tmp_reg__ \n\t" + "pop r30 \n\t" + "out __RAMPZ__, r30 \n\t" #endif - - "pop __tmp_reg__ \n\t" - "out __SREG__, __tmp_reg__ \n\t" - "pop __tmp_reg__ \n\t"); + "pop r31 \n\t" + "pop r30 \n\t" + "out __SREG__, r30 \n\t" + "pop r30 \n\t"); } From 4503f2e7d4897052f087e46f365354f823cda61e Mon Sep 17 00:00:00 2001 From: Gerson Fernando Budke Date: Thu, 6 Jul 2023 22:53:21 +0200 Subject: [PATCH 5/5] cpu/avr8_common: Add dedicated ISR stack Signed-off-by: Gerson Fernando Budke --- cpu/avr8_common/avr8_cpu.c | 1 + cpu/avr8_common/include/cpu.h | 48 +++++++++++++++++++++++++++++------ cpu/avr8_common/thread_arch.c | 31 ++++++++++++++-------- 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/cpu/avr8_common/avr8_cpu.c b/cpu/avr8_common/avr8_cpu.c index 7bd258bfbe1d..6a7556e124e0 100644 --- a/cpu/avr8_common/avr8_cpu.c +++ b/cpu/avr8_common/avr8_cpu.c @@ -68,6 +68,7 @@ */ uint8_t mcusr_mirror __attribute__((section(".noinit"))); uint8_t soft_rst __attribute__((section(".noinit"))); +uint8_t avr8_isr_stack[ISR_STACKSIZE] __attribute__((section(".data"))); #if (AVR8_STATE_IRQ_USE_SRAM) uint8_t avr8_state_irq_count_sram = 0; #endif diff --git a/cpu/avr8_common/include/cpu.h b/cpu/avr8_common/include/cpu.h index 8aba66bcfa1e..b9b1142f7219 100644 --- a/cpu/avr8_common/include/cpu.h +++ b/cpu/avr8_common/include/cpu.h @@ -48,6 +48,8 @@ extern "C" { #endif +extern uint8_t avr8_isr_stack[]; + /** * @name BOD monitoring when CPU is on sleep * @{ @@ -105,6 +107,13 @@ static inline int avr8_is_uart_tx_pending(void) return avr8_state_uart; } +/* + * The AVR-8 uses a shared stack only for process interrupts and switch context. + */ +#ifndef ISR_STACKSIZE +#define ISR_STACKSIZE (512) +#endif + /** * @brief Enter ISR routine * @@ -131,13 +140,13 @@ static inline void avr8_isr_prolog(void) "cli \n\t" #endif #if (AVR8_STATE_IRQ_USE_SRAM) - "lds r30, %[state] \n\t" - "subi r30, 0xFF \n\t" - "sts %[state], r30 \n\t" + "lds r31, %[state] \n\t" + "subi r31, 0xFF \n\t" + "sts %[state], r31 \n\t" #else - "in r30, %[state] \n\t" - "subi r30, 0xFF \n\t" - "out %[state], r30 \n\t" + "in r31, %[state] \n\t" + "subi r31, 0xFF \n\t" + "out %[state], r31 \n\t" #endif #if defined(CPU_ATXMEGA) "sei \n\t" @@ -167,12 +176,35 @@ static inline void avr8_isr_prolog(void) "push r25 \n\t" "push r26 \n\t" "push r27 \n\t" + /* + * Load irq_stack + */ +#if defined(CPU_ATXMEGA) + "cli \n\t" +#endif + "cpi r31, 1 \n\t" + "brne skip_save_if_nested_isr_%= \n\t" + "ldi r30, lo8(%[irq_stack]) \n\t" + "ldi r31, hi8(%[irq_stack]) \n\t" + "in r24, __SP_L__ \n\t" + "st z, r24 \n\t" + "in r24, __SP_H__ \n\t" + "st -z, r24 \n\t" + "subi r30, 1 \n\t" + "sbci r31, 0 \n\t" + "out __SP_L__, r30 \n\t" + "out __SP_H__, r31 \n\t" + "skip_save_if_nested_isr_%=: \n\t" +#if defined(CPU_ATXMEGA) + "sei \n\t" +#endif : /* no output */ #if (AVR8_STATE_IRQ_USE_SRAM) - : [state] "" (avr8_state_irq_count) + : [state] "" (avr8_state_irq_count), #else - : [state] "I" (_SFR_IO_ADDR(avr8_state_irq_count)) + : [state] "I" (_SFR_IO_ADDR(avr8_state_irq_count)), #endif + [irq_stack] "" (avr8_isr_stack + ISR_STACKSIZE - 1) : "memory" ); } diff --git a/cpu/avr8_common/thread_arch.c b/cpu/avr8_common/thread_arch.c index c591b51fbe6f..a7cb32219a2a 100644 --- a/cpu/avr8_common/thread_arch.c +++ b/cpu/avr8_common/thread_arch.c @@ -325,24 +325,33 @@ void avr8_isr_epilog(void) * Note: * - ATmega may be affected when IRQ is re-enabled on ISR body by the user. */ - "cli \n\t" + "cli \n\t" #if AVR8_STATE_IRQ_USE_SRAM - "lds r30, %[state] \n\t" - "subi r30, 0x01 \n\t" - "sts %[state], r30 \n\t" + "lds %[isr_sts], %[state] \n\t" + "subi %[isr_sts], 0x01 \n\t" + "sts %[state], %[isr_sts] \n\t" #else - "in r30, %[state] \n\t" - "subi r30, 0x01 \n\t" - "out %[state], r30 \n\t" + "in %[isr_sts], %[state] \n\t" + "subi %[isr_sts], 0x01 \n\t" + "out %[state], %[isr_sts] \n\t" #endif - "mov %[isr_sts], r30 \n\t" + /** + * Restore thread stack + */ + "cpse %[isr_sts], r1 \n\t" + "rjmp skip_restore_if_nested_isr \n\t" + "pop r31 \n\t" + "pop r30 \n\t" + "out __SP_L__, r30 \n\t" + "out __SP_H__, r31 \n\t" + "skip_restore_if_nested_isr: \n\t" : [isr_sts] "=r" (isr_sts) #if (AVR8_STATE_IRQ_USE_SRAM) : [state] "" (avr8_state_irq_count) #else : [state] "I" (_SFR_IO_ADDR(avr8_state_irq_count)) #endif - : "memory" + : "r30", "r31", "memory" ); /* Branch Tests - critical section. @@ -385,9 +394,9 @@ void avr8_isr_epilog(void) */ __asm__ volatile ( #if defined(CPU_ATXMEGA) - "sei \n\t" + "sei \n\t" #endif - "reti \n\t" + "reti \n\t" : : : "memory" ); }