From 1f3d3380f1c47b02c3356fec95ea3162cd45e0a2 Mon Sep 17 00:00:00 2001 From: Corwin Date: Mon, 21 Mar 2022 21:52:39 +0000 Subject: [PATCH 1/4] switch interrupts to be in boxes --- agb/src/interrupt.rs | 192 ++++++++++++++++++++----------------------- 1 file changed, 90 insertions(+), 102 deletions(-) diff --git a/agb/src/interrupt.rs b/agb/src/interrupt.rs index 0c81656e..71f72f5d 100644 --- a/agb/src/interrupt.rs +++ b/agb/src/interrupt.rs @@ -4,6 +4,7 @@ use core::{ pin::Pin, }; +use alloc::boxed::Box; use bare_metal::CriticalSection; use crate::{display::DISPLAY_STATUS, memory_mapped::MemoryMapped}; @@ -94,7 +95,7 @@ fn disable_interrupts() { } struct InterruptRoot { - next: Cell<*const InterruptClosure>, + next: Cell<*const InterruptInner>, count: Cell, interrupt: Interrupt, } @@ -153,16 +154,60 @@ extern "C" fn __RUST_INTERRUPT_HANDLER(interrupt: u16) -> u16 { interrupt } -pub struct InterruptClosureBounded<'a> { - c: InterruptClosure, - _phantom: PhantomData<&'a ()>, - _unpin: PhantomPinned, +struct InterruptInner { + next: Cell<*const InterruptInner>, + root: *const InterruptRoot, + closure: *const dyn Fn(&CriticalSection), + _pin: PhantomPinned, } -struct InterruptClosure { - closure: *const (dyn Fn(&CriticalSection)), - next: Cell<*const InterruptClosure>, +unsafe fn create_interrupt_inner( + c: impl Fn(&CriticalSection), root: *const InterruptRoot, +) -> Pin> { + let c = Box::new(c); + let c: &dyn Fn(&CriticalSection) = Box::leak(c); + let c: &dyn Fn(&CriticalSection) = core::mem::transmute(c); + Box::pin(InterruptInner { + next: Cell::new(core::ptr::null()), + root, + closure: c, + _pin: PhantomPinned, + }) +} + +impl Drop for InterruptInner { + fn drop(&mut self) { + inner_drop(unsafe { Pin::new_unchecked(self) }); + fn inner_drop(this: Pin<&mut InterruptInner>) { + // drop the closure allocation safely + let _closure_box = + unsafe { Box::from_raw(this.closure as *mut dyn Fn(&CriticalSection)) }; + + // perform the rest of the drop sequence + let root = unsafe { &*this.root }; + root.reduce(); + let mut c = root.next.get(); + let own_pointer = &*this as *const _; + if c == own_pointer { + unsafe { &*this.root }.next.set(this.next.get()); + return; + } + loop { + let p = unsafe { &*c }.next.get(); + if p == own_pointer { + unsafe { &*c }.next.set(this.next.get()); + return; + } + c = p; + } + } + } +} + +pub struct InterruptHandler<'a> { + _inner: Pin>, + _lifetime: PhantomData<&'a ()>, } impl InterruptRoot { @@ -177,101 +222,46 @@ impl InterruptRoot { } } -impl Drop for InterruptClosure { - fn drop(&mut self) { - let root = unsafe { &*self.root }; - root.reduce(); - let mut c = root.next.get(); - let own_pointer = self as *const _; - if c == own_pointer { - unsafe { &*self.root }.next.set(self.next.get()); - return; - } - loop { - let p = unsafe { &*c }.next.get(); - if p == own_pointer { - unsafe { &*c }.next.set(self.next.get()); - return; - } - c = p; - } - } -} - fn interrupt_to_root(interrupt: Interrupt) -> &'static InterruptRoot { unsafe { &INTERRUPT_TABLE[interrupt as usize] } } -fn get_interrupt_handle_root<'a>( - f: &'a dyn Fn(&CriticalSection), - root: &InterruptRoot, -) -> InterruptClosureBounded<'a> { - InterruptClosureBounded { - c: InterruptClosure { - closure: unsafe { core::mem::transmute(f as *const _) }, - next: Cell::new(core::ptr::null()), - root: root as *const _, - }, - _phantom: PhantomData, - _unpin: PhantomPinned, - } -} - -/// The [add_interrupt_handler!] macro should be used instead of this function. -/// Creates an interrupt handler from a closure. -pub fn get_interrupt_handle( - f: &(dyn Fn(&CriticalSection) + Send + Sync), +#[must_use] +pub fn add_interrupt_handler<'a>( interrupt: Interrupt, -) -> InterruptClosureBounded { - let root = interrupt_to_root(interrupt); - - get_interrupt_handle_root(f, root) -} - -/// The [add_interrupt_handler!] macro should be used instead of this. -/// Adds an interrupt handler to the interrupt table such that when that -/// interrupt is triggered the closure is called. -pub fn add_interrupt<'a>(interrupt: Pin<&'a InterruptClosureBounded<'a>>) { - free(|_| { - let root = unsafe { &*interrupt.c.root }; - root.add(); - let mut c = root.next.get(); - if c.is_null() { - root.next.set((&interrupt.c) as *const _); - return; - } - loop { - let p = unsafe { &*c }.next.get(); - if p.is_null() { - unsafe { &*c }.next.set((&interrupt.c) as *const _); + handler: impl Fn(&CriticalSection) + 'a, +) -> InterruptHandler<'a> { + fn do_with_inner<'a>( + interrupt: Interrupt, + inner: Pin>, + ) -> InterruptHandler<'a> { + free(|_| { + let root = interrupt_to_root(interrupt); + root.add(); + let mut c = root.next.get(); + if c.is_null() { + root.next.set((&*inner) as *const _); return; } + loop { + let p = unsafe { &*c }.next.get(); + if p.is_null() { + unsafe { &*c }.next.set((&*inner) as *const _); + return; + } - c = p; + c = p; + } + }); + + InterruptHandler { + _inner: inner, + _lifetime: PhantomData, } - }) -} - -#[macro_export] -/// Creates a new interrupt handler in the current scope, when this scope drops -/// the interrupt handler is removed. Note that this returns nothing, but some -/// stack space is used. The interrupt handler is of the form `Fn(Key) + Send + -/// Sync` where Key can be used to unlock a mutex without checking whether -/// interrupts need to be disabled, as during an interrupt interrupts are -/// disabled. -/// -/// # Usage -/// ``` -/// add_interrupt_handler!(Interrupt::VBlank, |key| agb::println!("hello world!")); -/// ``` -/// -macro_rules! add_interrupt_handler { - ($interrupt: expr, $handler: expr) => { - let a = $handler; - let a = $crate::interrupt::get_interrupt_handle(&a, $interrupt); - let a = unsafe { core::pin::Pin::new_unchecked(&a) }; - $crate::interrupt::add_interrupt(a); - }; + } + let root = interrupt_to_root(interrupt) as *const _; + let inner = unsafe { create_interrupt_inner(handler, root) }; + do_with_inner(interrupt, inner) } pub fn free(f: F) -> R @@ -322,14 +312,12 @@ mod tests { { let counter = Mutex::new(RefCell::new(0)); let counter_2 = Mutex::new(RefCell::new(0)); - add_interrupt_handler!(Interrupt::VBlank, |key: &CriticalSection| *counter - .borrow(*key) - .borrow_mut() += - 1); - add_interrupt_handler!(Interrupt::VBlank, |key: &CriticalSection| *counter_2 - .borrow(*key) - .borrow_mut() += - 1); + let _a = add_interrupt_handler(Interrupt::VBlank, |key: &CriticalSection| { + *counter.borrow(*key).borrow_mut() += 1 + }); + let _b = add_interrupt_handler(Interrupt::VBlank, |key: &CriticalSection| { + *counter_2.borrow(*key).borrow_mut() += 1 + }); let vblank = VBlank::get(); From c74707b1a6a88922ab363f363e0812f136841bab Mon Sep 17 00:00:00 2001 From: Corwin Date: Mon, 21 Mar 2022 21:52:46 +0000 Subject: [PATCH 2/4] update examples --- agb/examples/output.rs | 4 ++-- agb/examples/wave.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/agb/examples/output.rs b/agb/examples/output.rs index e28558f6..6101438e 100644 --- a/agb/examples/output.rs +++ b/agb/examples/output.rs @@ -8,13 +8,13 @@ use bare_metal::{CriticalSection, Mutex}; #[agb::entry] fn main(_gba: agb::Gba) -> ! { let count = Mutex::new(RefCell::new(0)); - agb::add_interrupt_handler!( + let _a = agb::interrupt::add_interrupt_handler( agb::interrupt::Interrupt::VBlank, |key: &CriticalSection| { let mut count = count.borrow(*key).borrow_mut(); agb::println!("Hello, world, frame = {}", *count); *count += 1; - } + }, ); loop {} } diff --git a/agb/examples/wave.rs b/agb/examples/wave.rs index fd513367..8fffaddc 100644 --- a/agb/examples/wave.rs +++ b/agb/examples/wave.rs @@ -28,7 +28,7 @@ fn main(mut gba: agb::Gba) -> ! { let back = Mutex::new(RefCell::new(BackCosines { cosines, row: 0 })); - agb::add_interrupt_handler!(Interrupt::HBlank, |key: &CriticalSection| { + let _a = agb::interrupt::add_interrupt_handler(Interrupt::HBlank, |key: &CriticalSection| { let mut backc = back.borrow(*key).borrow_mut(); let deflection = backc.cosines[backc.row % 32]; unsafe { ((0x0400_0010) as *mut u16).write_volatile(deflection) } From f312ff7df8bdd0d9bf8434999a0e14bc35fdfd39 Mon Sep 17 00:00:00 2001 From: Corwin Date: Mon, 21 Mar 2022 21:52:56 +0000 Subject: [PATCH 3/4] enable interrupts at initialisation --- agb/crt0.s | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/agb/crt0.s b/agb/crt0.s index 3bf46691..d3039ae2 100644 --- a/agb/crt0.s +++ b/agb/crt0.s @@ -55,6 +55,11 @@ b .Initialise_mb ldr r2, =__iwram_rom_length_halfwords swi 0x000B0000 + @ enable interrupts + ldr r0, =0x04000208 + ldr r1, =1 + str r1, [r0] + @ put zero in both r0 and r1 @ This corresponds to zero for argc and argv (which would technically be required for a c runtime) ldr r0, =0 From aa0337941ad0243484ba7fce8b08c5c5f9e7f6c3 Mon Sep 17 00:00:00 2001 From: Corwin Date: Mon, 21 Mar 2022 22:19:07 +0000 Subject: [PATCH 4/4] add docs --- agb/src/interrupt.rs | 16 ++++++++++++++++ agb/src/lib.rs | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/agb/src/interrupt.rs b/agb/src/interrupt.rs index 71f72f5d..701e6d89 100644 --- a/agb/src/interrupt.rs +++ b/agb/src/interrupt.rs @@ -227,6 +227,18 @@ fn interrupt_to_root(interrupt: Interrupt) -> &'static InterruptRoot { } #[must_use] +/// Adds an interrupt handler as long as the returned value is alive. The +/// closure takes a [`CriticalSection`] which can be used for mutexes. +/// +/// [`CriticalSection`]: bare_metal::CriticalSection +/// +/// # Examples +/// +/// ``` +/// let _a = add_interrupt_handler(Interrupt::VBlank, |_: &CriticalSection| { +/// println!("Woah there! There's been a vblank!"); +/// }); +/// ``` pub fn add_interrupt_handler<'a>( interrupt: Interrupt, handler: impl Fn(&CriticalSection) + 'a, @@ -264,6 +276,10 @@ pub fn add_interrupt_handler<'a>( do_with_inner(interrupt, inner) } +/// How you can access mutexes outside of interrupts by being given a +/// [`CriticalSection`] +/// +/// [`CriticalSection`]: bare_metal::CriticalSection pub fn free(f: F) -> R where F: FnOnce(&CriticalSection) -> R, diff --git a/agb/src/lib.rs b/agb/src/lib.rs index efc06cbf..0f50a1cd 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -145,7 +145,7 @@ pub mod display; mod dma; /// Button inputs to the system. pub mod input; -#[doc(hidden)] // hide for now as the implementation in here is unsound +/// Interacting with the GBA interrupts pub mod interrupt; mod memory_mapped; /// Implements logging to the mgba emulator.