From c5462e412deae39f681c8017722eaae0372bc6d7 Mon Sep 17 00:00:00 2001 From: Daniele Cattaneo <daniele3.cattaneo@mail.polimi.it> Date: Thu, 30 Jan 2025 18:05:01 +0100 Subject: [PATCH] Add SMP-safe interrupt disable spinlocks Signed-off-by: Terraneo Federico <fede.tft@miosix.org> --- .../interfaces-impl/cpu_const_smp_impl.h | 39 +++++++++++++++ .../interfaces-impl/interrupts_smp_impl.h | 50 +++++++++++++++++++ .../common/rp2040_boot.cpp | 5 ++ .../rp2040_raspberry_pi_pico/board_settings.h | 4 ++ miosix/interfaces/cpu_const.h | 16 ++++++ miosix/interfaces/interrupts.h | 29 ++++++++++- miosix/kernel/kernel.cpp | 31 +++++++++--- 7 files changed, 167 insertions(+), 7 deletions(-) create mode 100644 miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/cpu_const_smp_impl.h create mode 100644 miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/interrupts_smp_impl.h diff --git a/miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/cpu_const_smp_impl.h b/miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/cpu_const_smp_impl.h new file mode 100644 index 00000000..136792db --- /dev/null +++ b/miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/cpu_const_smp_impl.h @@ -0,0 +1,39 @@ +/*************************************************************************** + * Copyright (C) 2025 by Federico Terraneo, Daniele Cattaneo * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * As a special exception, if other files instantiate templates or use * + * macros or inline functions from this file, or you compile this file * + * and link it with other works to produce a work based on this file, * + * this file does not by itself cause the resulting work to be covered * + * by the GNU General Public License. However the source code for this * + * file must still be made available in accordance with the GNU General * + * Public License. This exception does not invalidate any other reasons * + * why a work based on this file might be covered by the GNU General * + * Public License. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, see <http://www.gnu.org/licenses/> * + ***************************************************************************/ + +#pragma once + +#include "interfaces/arch_registers.h" + +namespace miosix { + +inline unsigned char getCurrentCoreId() +{ + return get_core_num(); +} + +} diff --git a/miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/interrupts_smp_impl.h b/miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/interrupts_smp_impl.h new file mode 100644 index 00000000..d5592d2a --- /dev/null +++ b/miosix/arch/cortexM0plus_rp2040/common/interfaces-impl/interrupts_smp_impl.h @@ -0,0 +1,50 @@ +/*************************************************************************** + * Copyright (C) 2025 by Federico Terraneo, Daniele Cattaneo * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * As a special exception, if other files instantiate templates or use * + * macros or inline functions from this file, or you compile this file * + * and link it with other works to produce a work based on this file, * + * this file does not by itself cause the resulting work to be covered * + * by the GNU General Public License. However the source code for this * + * file must still be made available in accordance with the GNU General * + * Public License. This exception does not invalidate any other reasons * + * why a work based on this file might be covered by the GNU General * + * Public License. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, see <http://www.gnu.org/licenses/> * + ***************************************************************************/ + +#include "interfaces/arch_registers.h" + +namespace miosix { + +inline void globalInterruptLock() noexcept +{ + for(;;) + { + unsigned long lock=sio_hw->spinlock[0]; + if(lock) break; + __WFE(); + } + __DSB(); +} + +inline void globalInterruptUnlock() noexcept +{ + __DSB(); + sio_hw->spinlock[0]=0; + __SEV(); +} + +} // namespace miosix diff --git a/miosix/arch/cortexM0plus_rp2040/common/rp2040_boot.cpp b/miosix/arch/cortexM0plus_rp2040/common/rp2040_boot.cpp index 7cc3902f..bcc4ea22 100644 --- a/miosix/arch/cortexM0plus_rp2040/common/rp2040_boot.cpp +++ b/miosix/arch/cortexM0plus_rp2040/common/rp2040_boot.cpp @@ -269,6 +269,11 @@ void IRQmemoryAndClockInit() //bad hardware design. for(unsigned int i=0; i<NUM_BANK0_GPIOS; i++) padsbank0_hw->io[i]=toUint(Mode::DISABLED); + + //QUIRK: You would expect the hardware spinlocks to be reset on a CPU + //reset but they are not. To release a spinlock you have to write something + //non-zero! + for(unsigned int i=0; i<32; i++) sio_hw->spinlock[i]=1; // Setup clock generation clockTreeSetup(); diff --git a/miosix/config/arch/cortexM0plus_rp2040/rp2040_raspberry_pi_pico/board_settings.h b/miosix/config/arch/cortexM0plus_rp2040/rp2040_raspberry_pi_pico/board_settings.h index 95acc845..7a9a195a 100644 --- a/miosix/config/arch/cortexM0plus_rp2040/rp2040_raspberry_pi_pico/board_settings.h +++ b/miosix/config/arch/cortexM0plus_rp2040/rp2040_raspberry_pi_pico/board_settings.h @@ -42,6 +42,10 @@ namespace miosix { * \{ */ +/// \def WITH_SMP +/// Enables multicore support +#define WITH_SMP + /// Size of stack for main(). /// The C standard library is stack-heavy (iprintf requires 1KB) but the /// RP2040 has 264KB of RAM so there is room for a big 4K stack. diff --git a/miosix/interfaces/cpu_const.h b/miosix/interfaces/cpu_const.h index 2b6eb2d1..84f3ef2a 100644 --- a/miosix/interfaces/cpu_const.h +++ b/miosix/interfaces/cpu_const.h @@ -27,6 +27,8 @@ #pragma once +#include "config/miosix_settings.h" + /** * \addtogroup Interfaces * \{ @@ -52,8 +54,22 @@ * const unsigned int STACK_OFFSET_IN_CTXSAVE=...; */ +namespace miosix { + +inline unsigned char getCurrentCoreId(); + +#ifndef WITH_SMP +inline unsigned char getCurrentCoreId() { return 0; } +#endif + +} // namespace miosix + /** * \} */ #include "interfaces-impl/cpu_const_impl.h" + +#ifdef WITH_SMP +#include "interfaces-impl/cpu_const_smp_impl.h" +#endif diff --git a/miosix/interfaces/interrupts.h b/miosix/interfaces/interrupts.h index a33a4929..b3d3596d 100644 --- a/miosix/interfaces/interrupts.h +++ b/miosix/interfaces/interrupts.h @@ -1,5 +1,5 @@ /*************************************************************************** - * Copyright (C) 2023-2024 by Terraneo Federico * + * Copyright (C) 2023-2025 by Terraneo Federico * * * * This program is free software; you can redistribute it and/or modify * * it under the terms of the GNU General Public License as published by * @@ -28,6 +28,7 @@ #pragma once #include "e20/unmember.h" +#include "config/miosix_settings.h" /** * \addtogroup Interfaces @@ -229,6 +230,28 @@ inline void fastEnableInterrupts() noexcept; */ bool areInterruptsEnabled() noexcept; +void globalInterruptLock() noexcept; +void globalInterruptUnlock() noexcept; + +#ifndef WITH_SMP + +void globalInterruptLock() {} +void globalInterruptUnlock() {} + +#endif + +inline void globalInterruptDisableLock() noexcept +{ + fastDisableInterrupts(); + globalInterruptLock(); +} + +inline void globalInterruptEnableUnlock() noexcept +{ + globalInterruptUnlock(); + fastEnableInterrupts(); +} + } //namespace miosix /** @@ -236,3 +259,7 @@ bool areInterruptsEnabled() noexcept; */ #include "interfaces-impl/interrupts_impl.h" + +#ifdef WITH_SMP +#include "interfaces-impl/interrupts_smp_impl.h" +#endif diff --git a/miosix/kernel/kernel.cpp b/miosix/kernel/kernel.cpp index 235b241d..32b0e2e9 100755 --- a/miosix/kernel/kernel.cpp +++ b/miosix/kernel/kernel.cpp @@ -1,5 +1,5 @@ /*************************************************************************** - * Copyright (C) 2008-2023 by Terraneo Federico * + * Copyright (C) 2008-2025 by Terraneo Federico * * * * This program is free software; you can redistribute it and/or modify * * it under the terms of the GNU General Public License as published by * @@ -84,6 +84,9 @@ static bool kernelStarted=false;///<\internal becomes true after startKernel. /// This is used by disableInterrupts() and enableInterrupts() to allow nested /// calls to these functions. static unsigned char interruptDisableNesting=0; +#ifdef WITH_SMP +static unsigned char globalIntrNestLockHoldingCore=0xFF; +#endif #ifdef WITH_DEEP_SLEEP @@ -144,11 +147,23 @@ void *idleThread(void *argv) void disableInterrupts() { - //Before the kernel is started interrupts are disabled, - //so disabling them again won't hurt - fastDisableInterrupts(); +#ifdef WITH_SMP + unsigned char state=globalIntrNestLockHoldingCore; + if(state==getCurrentCoreId()) + { + if(interruptDisableNesting==0xff) errorHandler(NESTING_OVERFLOW); + interruptDisableNesting++; + } else { + globalInterruptDisableLock(); + globalIntrNestLockHoldingCore=getCurrentCoreId(); + if(interruptDisableNesting!=0) errorHandler(DISABLE_INTERRUPTS_NESTING); + interruptDisableNesting=1; + } +#else + globalInterruptDisableLock(); if(interruptDisableNesting==0xff) errorHandler(NESTING_OVERFLOW); interruptDisableNesting++; +#endif } void enableInterrupts() @@ -159,9 +174,13 @@ void enableInterrupts() errorHandler(DISABLE_INTERRUPTS_NESTING); } interruptDisableNesting--; - if(interruptDisableNesting==0 && kernelStarted==true) + if(interruptDisableNesting==0) { - fastEnableInterrupts(); + #ifdef WITH_SMP + globalIntrNestLockHoldingCore=0xFF; + #endif + if(kernelStarted==true) globalInterruptEnableUnlock(); + else globalInterruptUnlock(); } } -- GitLab