Merge pull request #311 from Liamolucko/multicore-no-alloc

Remove the `alloc` requirement for `Core::spawn`

rp2040-hal/Cargo.toml

@@ -42,7 +42,6 @@ dht-sensor = "0.2.1"
 
 [features]
 rt = ["rp2040-pac/rt"]
-alloc = []
 rom-func-cache = []
 disable-intrinsics = []
 rom-v2-intrinsics = []

rp2040-hal/examples/multicore_fifo_blink.rs

@@ -57,7 +57,7 @@ const CORE1_TASK_COMPLETE: u32 = 0xEE;
 /// the stack guard to take up the least amount of usable RAM.
 static mut CORE1_STACK: Stack<4096> = Stack::new();
 
-fn core1_task() -> ! {
+fn core1_task(sys_freq: u32) -> ! {
     let mut pac = unsafe { pac::Peripherals::steal() };
     let core = unsafe { pac::CorePeripherals::steal() };
 
@@ -70,10 +70,6 @@ fn core1_task() -> ! {
     );
 
     let mut led_pin = pins.gpio25.into_push_pull_output();
-    // The first thing core0 sends us is the system bus frequency.
-    // The systick is based on this frequency, so we need that to
-    // be accurate when sleeping via cortex_m::delay::Delay
-    let sys_freq = sio.fifo.read_blocking();
     let mut delay = cortex_m::delay::Delay::new(core.SYST, sys_freq);
     loop {
         let input = sio.fifo.read();
@@ -114,17 +110,18 @@ fn main() -> ! {
     .ok()
     .unwrap();
 
+    let sys_freq = clocks.system_clock.freq().integer();
+
     // The single-cycle I/O block controls our GPIO pins
     let mut sio = hal::sio::Sio::new(pac.SIO);
 
-    let mut mc = Multicore::new(&mut pac.PSM, &mut pac.PPB, &mut sio);
+    let mut mc = Multicore::new(&mut pac.PSM, &mut pac.PPB, &mut sio.fifo);
     let cores = mc.cores();
     let core1 = &mut cores[1];
-    let _test = core1.spawn(core1_task, unsafe { &mut CORE1_STACK.mem });
+    let _test = core1.spawn(unsafe { &mut CORE1_STACK.mem }, move || {
+        core1_task(sys_freq)
+    });
 
-    // Let core1 know how fast the system clock is running
-    let sys_freq = clocks.system_clock.freq().integer();
-    sio.fifo.write_blocking(sys_freq);
     /// How much we adjust the LED period every cycle
     const LED_PERIOD_INCREMENT: i32 = 2;
 

rp2040-hal/examples/multicore_polyblink.rs

@@ -0,0 +1,133 @@
+//! # Multicore Blinking Example
+//!
+//! This application blinks two LEDs on GPIOs 2 and 3 at different rates (3Hz
+//! and 4Hz respectively.)
+//!
+//! See the `Cargo.toml` file for Copyright and licence details.
+
+#![no_std]
+#![no_main]
+
+use cortex_m::delay::Delay;
+// The macro for our start-up function
+use cortex_m_rt::entry;
+
+use embedded_time::fixed_point::FixedPoint;
+use hal::clocks::Clock;
+use hal::gpio::Pins;
+use hal::multicore::{Multicore, Stack};
+use hal::sio::Sio;
+// Ensure we halt the program on panic (if we don't mention this crate it won't
+// be linked)
+use panic_halt as _;
+
+// Alias for our HAL crate
+use rp2040_hal as hal;
+
+// A shorter alias for the Peripheral Access Crate, which provides low-level
+// register access
+use hal::pac;
+
+// Some traits we need
+use embedded_hal::digital::v2::ToggleableOutputPin;
+
+/// The linker will place this boot block at the start of our program image. We
+/// need this to help the ROM bootloader get our code up and running.
+#[link_section = ".boot2"]
+#[used]
+pub static BOOT2: [u8; 256] = rp2040_boot2::BOOT_LOADER_W25Q080;
+
+/// External high-speed crystal on the Raspberry Pi Pico board is 12 MHz. Adjust
+/// if your board has a different frequency
+const XTAL_FREQ_HZ: u32 = 12_000_000u32;
+
+/// The frequency at which core 0 will blink its LED (Hz).
+const CORE0_FREQ: u32 = 3;
+/// The frequency at which core 1 will blink its LED (Hz).
+const CORE1_FREQ: u32 = 4;
+/// The delay between each toggle of core 0's LED (us).
+const CORE0_DELAY: u32 = 1_000_000 / CORE0_FREQ;
+/// The delay between each toggle of core 1's LED (us).
+const CORE1_DELAY: u32 = 1_000_000 / CORE1_FREQ;
+
+/// Stack for core 1
+///
+/// Core 0 gets its stack via the normal route - any memory not used by static
+/// values is reserved for stack and initialised by cortex-m-rt.
+/// To get the same for Core 1, we would need to compile everything seperately
+/// and modify the linker file for both programs, and that's quite annoying.
+/// So instead, core1.spawn takes a [usize] which gets used for the stack.
+/// NOTE: We use the `Stack` struct here to ensure that it has 32-byte
+/// alignment, which allows the stack guard to take up the least amount of
+/// usable RAM.
+static mut CORE1_STACK: Stack<4096> = Stack::new();
+
+/// Entry point to our bare-metal application.
+///
+/// The `#[entry]` macro ensures the Cortex-M start-up code calls this function
+/// as soon as all global variables are initialised.
+#[entry]
+fn main() -> ! {
+    // Grab our singleton objects
+    let mut pac = pac::Peripherals::take().unwrap();
+    let core = pac::CorePeripherals::take().unwrap();
+
+    // Set up the watchdog driver - needed by the clock setup code
+    let mut watchdog = hal::watchdog::Watchdog::new(pac.WATCHDOG);
+
+    // Configure the clocks
+    let clocks = hal::clocks::init_clocks_and_plls(
+        XTAL_FREQ_HZ,
+        pac.XOSC,
+        pac.CLOCKS,
+        pac.PLL_SYS,
+        pac.PLL_USB,
+        &mut pac.RESETS,
+        &mut watchdog,
+    )
+    .ok()
+    .unwrap();
+
+    // Set up the GPIO pins
+    let mut sio = Sio::new(pac.SIO);
+    let pins = Pins::new(
+        pac.IO_BANK0,
+        pac.PADS_BANK0,
+        sio.gpio_bank0,
+        &mut pac.RESETS,
+    );
+    let mut led1 = pins.gpio2.into_push_pull_output();
+    let mut led2 = pins.gpio3.into_push_pull_output();
+
+    // Set up the delay for the first core.
+    let sys_freq = clocks.system_clock.freq().integer();
+    let mut delay = Delay::new(core.SYST, sys_freq);
+
+    // Start up the second core to blink the second LED
+    let mut mc = Multicore::new(&mut pac.PSM, &mut pac.PPB, &mut sio.fifo);
+    let cores = mc.cores();
+    let core1 = &mut cores[1];
+    core1
+        .spawn(unsafe { &mut CORE1_STACK.mem }, move || {
+            // Get the second core's copy of the `CorePeripherals`, which are per-core.
+            // Unfortunately, `cortex-m` doesn't support this properly right now,
+            // so we have to use `steal`.
+            let core = unsafe { pac::CorePeripherals::steal() };
+            // Set up the delay for the second core.
+            let mut delay = Delay::new(core.SYST, sys_freq);
+            // Blink the second LED.
+            loop {
+                led2.toggle().unwrap();
+                delay.delay_us(CORE1_DELAY)
+            }
+        })
+        .unwrap();
+
+    // Blink the first LED.
+    loop {
+        led1.toggle().unwrap();
+        delay.delay_us(CORE0_DELAY)
+    }
+}
+
+// End of file

rp2040-hal/src/multicore.rs

@@ -3,29 +3,30 @@
 //! This module handles setup of the 2nd cpu core on the rp2040, which we refer to as core1.
 //! It provides functionality for setting up the stack, and starting core1.
 //!
-//! The options for an entrypoint for core1 are
+//! The entrypoint for core1 can be any function that never returns, including closures.
-//! - a function that never returns - eg
-//! `fn core1_task() -> ! { loop{} }; `
-//! - a lambda (note: This requires a global allocator which requires a nightly compiler. Not recommended for beginners)
 //!
 //! # Usage
 //!
 //! ```no_run
+//! use rp2040_hal::{pac, gpio::Pins, sio::Sio, multicore::{Multicore, Stack}};
+//!
 //! static mut CORE1_STACK: Stack<4096> = Stack::new();
+//!
 //! fn core1_task() -> ! {
 //!     loop {}
 //! }
-//! // fn main() -> ! {
+//!
-//!     use rp2040_hal::{pac, gpio::Pins, sio::Sio, multicore::{Multicore, Stack}};
+//! fn main() -> ! {
 //!     let mut pac = pac::Peripherals::take().unwrap();
 //!     let mut sio = Sio::new(pac.SIO);
 //!     // Other init code above this line
-//!     let mut mc = Multicore::new(&mut pac.PSM, &mut pac.PPB, &mut sio);
+//!     let mut mc = Multicore::new(&mut pac.PSM, &mut pac.PPB, &mut sio.fifo);
 //!     let cores = mc.cores();
 //!     let core1 = &mut cores[1];
-//!     let _test = core1.spawn(core1_task, unsafe { &mut CORE1_STACK.mem });
+//!     let _test = core1.spawn(unsafe { &mut CORE1_STACK.mem }, core1_task);
 //!     // The rest of your application below this line
-//! //}
+//!     # loop {}
+//! }
 //!
 //! ```
 //!
@@ -33,10 +34,12 @@
 //!
 //! For a detailed example, see [examples/multicore_fifo_blink.rs](https://github.com/rp-rs/rp-hal/tree/main/rp2040-hal/examples/multicore_fifo_blink.rs)
 
-use crate::pac;
+use core::mem::ManuallyDrop;
+use core::sync::atomic::compiler_fence;
+use core::sync::atomic::Ordering;
 
-#[cfg(feature = "alloc")]
+use crate::pac;
-extern crate alloc;
+use crate::Sio;
 
 /// Errors for multicore operations.
 #[derive(Debug)]
@@ -47,15 +50,6 @@ pub enum Error {
     Unresponsive,
 }
 
-// We pass data to cores via the stack, so we read
-// the data off the stack and into parameters that
-// rust can read here. Ideally this would be a
-// #[naked] function but that is not stable yet.
-static MULTICORE_TRAMPOLINE: [u16; 2] = [
-    0xbd03, // pop {r0, r1, pc} - call wrapper (pc) with r0 and r1
-    0x46c0, // nop - pad this out to 32 bits long
-];
-
 #[inline(always)]
 fn install_stack_guard(stack_bottom: *mut usize) {
     let core = unsafe { pac::CorePeripherals::steal() };
@@ -109,7 +103,11 @@ impl<const SIZE: usize> Stack<SIZE> {
 
 impl<'p> Multicore<'p> {
     /// Create a new |Multicore| instance.
-    pub fn new(psm: &'p mut pac::PSM, ppb: &'p mut pac::PPB, sio: &'p mut crate::Sio) -> Self {
+    pub fn new(
+        psm: &'p mut pac::PSM,
+        ppb: &'p mut pac::PPB,
+        sio: &'p mut crate::sio::SioFifo,
+    ) -> Self {
         Self {
             cores: [
                 Core { inner: None },
@@ -128,7 +126,11 @@ impl<'p> Multicore<'p> {
 
 /// A handle for controlling a logical core.
 pub struct Core<'p> {
-    inner: Option<(&'p mut pac::PSM, &'p mut pac::PPB, &'p mut crate::Sio)>,
+    inner: Option<(
+        &'p mut pac::PSM,
+        &'p mut pac::PPB,
+        &'p mut crate::sio::SioFifo,
+    )>,
 }
 
 impl<'p> Core<'p> {
@@ -140,13 +142,36 @@ impl<'p> Core<'p> {
         }
     }
 
-    fn inner_spawn(
+    /// Spawn a function on this core.
-        &mut self,
+    pub fn spawn<F>(&mut self, stack: &'static mut [usize], entry: F) -> Result<(), Error>
-        wrapper: *mut (),
+    where
-        entry: *mut (),
+        F: FnOnce() -> bad::Never + Send + 'static,
-        stack: &'static mut [usize],
+    {
-    ) -> Result<(), Error> {
+        if let Some((psm, ppb, fifo)) = self.inner.as_mut() {
-        if let Some((psm, ppb, sio)) = self.inner.as_mut() {
+            // The first two ignored `u64` parameters are there to take up all of the registers,
+            // which means that the rest of the arguments are taken from the stack,
+            // where we're able to put them from core 0.
+            extern "C" fn core1_startup<F: FnOnce() -> bad::Never>(
+                _: u64,
+                _: u64,
+                entry: &mut ManuallyDrop<F>,
+                stack_bottom: *mut usize,
+            ) -> ! {
+                core1_setup(stack_bottom);
+
+                let entry = unsafe { ManuallyDrop::take(entry) };
+
+                // Signal that it's safe for core 0 to get rid of the original value now.
+                //
+                // We don't have any way to get at core 1's SIO without using `Peripherals::steal` right now,
+                // since svd2rust doesn't really support multiple cores properly.
+                let peripherals = unsafe { pac::Peripherals::steal() };
+                let mut sio = Sio::new(peripherals.SIO);
+                sio.fifo.write_blocking(1);
+
+                entry()
+            }
+
             // Reset the core
             psm.frce_off.modify(|_, w| w.proc1().set_bit());
             while !psm.frce_off.read().proc1().bit_is_set() {
@@ -157,14 +182,28 @@ impl<'p> Core<'p> {
             // Set up the stack
             let mut stack_ptr = unsafe { stack.as_mut_ptr().add(stack.len()) };
 
-            let mut push = |v: usize| unsafe {
+            // We don't want to drop this, since it's getting moved to the other core.
-                stack_ptr = stack_ptr.sub(1);
+            let mut entry = ManuallyDrop::new(entry);
-                stack_ptr.write(v);
-            };
 
-            push(wrapper as usize);
+            // Push the arguments to `core1_startup` onto the stack.
-            push(stack.as_mut_ptr() as usize);
+            unsafe {
-            push(entry as usize);
+                // Push `stack_bottom`.
+                stack_ptr = stack_ptr.sub(1);
+                stack_ptr.cast::<*mut usize>().write(stack.as_mut_ptr());
+
+                // Push `entry`.
+                stack_ptr = stack_ptr.sub(1);
+                stack_ptr.cast::<&mut ManuallyDrop<F>>().write(&mut entry);
+            }
+
+            // Make sure the compiler does not reorder the stack writes after to after the
+            // below FIFO writes, which would result in them not being seen by the second
+            // core.
+            //
+            // From the compiler perspective, this doesn't guarantee that the second core
+            // actually sees those writes. However, we know that the RP2040 doesn't have
+            // memory caches, and writes happen in-order.
+            compiler_fence(Ordering::Release);
 
             let vector_table = ppb.vtor.read().bits();
 
@@ -176,7 +215,7 @@ impl<'p> Core<'p> {
                 1,
                 vector_table as usize,
                 stack_ptr as usize,
-                MULTICORE_TRAMPOLINE.as_ptr() as usize + 1,
+                core1_startup::<F> as usize,
             ];
 
             let mut seq = 0;
@@ -184,17 +223,20 @@ impl<'p> Core<'p> {
             loop {
                 let cmd = cmd_seq[seq] as u32;
                 if cmd == 0 {
-                    sio.fifo.drain();
+                    fifo.drain();
                     cortex_m::asm::sev();
                 }
-                sio.fifo.write_blocking(cmd);
+                fifo.write_blocking(cmd);
-                let response = sio.fifo.read_blocking();
+                let response = fifo.read_blocking();
                 if cmd == response {
                     seq += 1;
                 } else {
                     seq = 0;
                     fails += 1;
                     if fails > 16 {
+                        // The second core isn't responding, and isn't going to take the entrypoint,
+                        // so we have to drop it ourselves.
+                        drop(ManuallyDrop::into_inner(entry));
                         return Err(Error::Unresponsive);
                     }
                 }
@@ -203,48 +245,17 @@ impl<'p> Core<'p> {
                 }
             }
 
+            // Wait until the other core has copied `entry` before returning.
+            fifo.read_blocking();
+
             Ok(())
         } else {
             Err(Error::InvalidCore)
         }
     }
-
-    /// Spawn a function on this core.
-    #[cfg(not(feature = "alloc"))]
-    pub fn spawn(&mut self, entry: fn() -> !, stack: &'static mut [usize]) -> Result<(), Error> {
-        #[allow(improper_ctypes_definitions)]
-        extern "C" fn core1_no_alloc(entry: fn() -> !, stack_bottom: *mut usize) -> ! {
-            core1_setup(stack_bottom);
-            entry();
-        }
-
-        self.inner_spawn(core1_no_alloc as _, entry as _, stack)
-    }
-
-    /// Spawn a function on this core.
-    #[cfg(feature = "alloc")]
-    pub fn spawn<F>(&mut self, entry: F, stack: &'static mut [usize]) -> Result<(), Error>
-    where
-        F: FnOnce() -> bad::Never,
-        F: Send + 'static,
-    {
-        use alloc::boxed::Box;
-
-        let main: Box<dyn FnOnce() -> bad::Never> = Box::new(move || entry());
-        let p = Box::into_raw(Box::new(main));
-
-        extern "C" fn core1_alloc(entry: *mut (), stack_bottom: *mut usize) -> ! {
-            core1_setup(stack_bottom);
-            let main = unsafe { Box::from_raw(entry as *mut Box<dyn FnOnce() -> bad::Never>) };
-            main();
-        }
-
-        self.inner_spawn(core1_alloc as _, p as _, stack)
-    }
 }
 
 // https://github.com/nvzqz/bad-rs/blob/master/src/never.rs
-#[cfg(feature = "alloc")]
 mod bad {
     pub(crate) type Never = <F as HasOutput>::Output;