fixing ABBA deadlocks yay

Author: adrerl

hwinit/src/process/scheduler.rs

@@ -39,9 +39,10 @@ pub(crate) static mut PROCESS_TABLE: [Option<Process>; MAX_PROCESSES] =
 /// The timer ISR on every core calls scheduler_tick() which must not
 /// race against itself or syscall handlers modifying the same process.
 /// Not reentrant — never acquire from code that already holds it.
-/// IF must be disabled before acquiring (IA32_FMASK handles this for syscalls,
-/// hardware handles it for ISRs).
-pub(crate) static PROCESS_TABLE_LOCK: crate::sync::RawSpinLock = crate::sync::RawSpinLock::new();
+/// Disables IF on acquire, restores on release — safe from both ISR
+/// and non-ISR contexts (BSP idle loop, task manager, etc).
+pub(crate) static PROCESS_TABLE_LOCK: crate::sync::IsrSafeRawSpinLock =
+    crate::sync::IsrSafeRawSpinLock::new();
 
 /// PID of the currently executing process on core 0 (BSP).
 /// For SMP, each core tracks its own current_pid via gs:[0x0C].

hwinit/src/sync.rs

@@ -130,11 +130,6 @@ impl<T> Drop for SpinLockGuard<'_, T> {
 }
 
 // RAW SPINLOCK (no interrupt disable, for when you manage it yourself)
-
-/// Raw spinlock without interrupt management.
-///
-/// Use when you're already in an interrupt-disabled context or
-/// don't need interrupt safety.
 pub struct RawSpinLock {
     locked: AtomicBool,
 }
@@ -167,6 +162,93 @@ impl RawSpinLock {
     }
 }
 
+// ISR-SAFE RAW SPINLOCK — disables interrupts before acquiring, restores on release.
+// Uses per-core saved-IF storage so nested/cross-core usage is correct.
+// Max 64 cores. Core index from gs:[0x00] (per-CPU area set up by ap_boot).
+
+pub struct IsrSafeRawSpinLock {
+    locked: AtomicBool,
+    /// Per-core saved IF state. Index = core index (0..MAX_CORES).
+    /// Only the core that holds the lock reads/writes its own slot.
+    saved_if: [core::cell::UnsafeCell<bool>; 64],
+}
+
+// multiple cores touch disjoint slots — safe by construction.
+unsafe impl Sync for IsrSafeRawSpinLock {}
+unsafe impl Send for IsrSafeRawSpinLock {}
+
+impl IsrSafeRawSpinLock {
+    pub const fn new() -> Self {
+        // const init: can't use array::from_fn in const, unroll via macro
+        const CELL_FALSE: core::cell::UnsafeCell<bool> = core::cell::UnsafeCell::new(false);
+        Self {
+            locked: AtomicBool::new(false),
+            saved_if: [CELL_FALSE; 64],
+        }
+    }
+
+    #[inline(always)]
+    fn core_index() -> usize {
+        // before per-CPU area is set up, gs base is 0 and this reads 0 (BSP)
+        let idx: u32;
+        unsafe {
+            core::arch::asm!(
+                "mov {0:e}, gs:[0x00]",
+                out(reg) idx,
+                options(nostack, readonly, preserves_flags)
+            );
+        }
+        (idx as usize) & 63
+    }
+
+    pub fn lock(&self) {
+        let was_enabled = interrupts_enabled();
+        disable_interrupts();
+
+        while self
+            .locked
+            .compare_exchange_weak(false, true, Ordering::Acquire, Ordering::Relaxed)
+            .is_err()
+        {
+            core::hint::spin_loop();
+        }
+
+        // save per-core IF state AFTER acquiring — only holder touches its slot
+        let ci = Self::core_index();
+        unsafe { *self.saved_if[ci].get() = was_enabled; }
+    }
+
+    pub fn unlock(&self) {
+        let ci = Self::core_index();
+        let was_enabled = unsafe { *self.saved_if[ci].get() };
+        self.locked.store(false, Ordering::Release);
+
+        if was_enabled {
+            enable_interrupts();
+        }
+    }
+
+    pub fn try_lock(&self) -> bool {
+        let was_enabled = interrupts_enabled();
+        disable_interrupts();
+
+        if self
+            .locked
+            .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
+            .is_ok()
+        {
+            let ci = Self::core_index();
+            unsafe { *self.saved_if[ci].get() = was_enabled; }
+            true
+        } else {
+            if was_enabled {
+                enable_interrupts();
+            }
+            false
+        }
+    }
+}
+
 // ONCE (run-once initialization)
 
 /// Run-once initialization primitive.
@@ -218,8 +300,6 @@ impl Once {
     }
 }
 
-// LAZY (lazily initialized value)
-
 /// Lazily initialized value.
 pub struct Lazy<T, F = fn() -> T> {
     once: Once,
@@ -259,8 +339,6 @@ impl<T, F: FnOnce() -> T> Deref for Lazy<T, F> {
     }
 }
 
-// INTERRUPT GUARD
-
 /// RAII guard that disables interrupts and restores on drop.
 pub struct InterruptGuard {
     was_enabled: bool,

hwinit/src/syscall/handler/compositor.rs

@@ -28,14 +28,16 @@ pub struct SurfaceEntry {
 /// Returns EBUSY if another compositor is already registered.
 pub unsafe fn sys_compositor_set() -> u64 {
     use crate::serial::{puts, put_hex32};
+    use core::sync::atomic::Ordering::Relaxed;
     let pid = SCHEDULER.current_pid();
-    if COMPOSITOR_PID != 0 && COMPOSITOR_PID != pid {
+    let cur = COMPOSITOR_PID.load(Relaxed);
+    if cur != 0 && cur != pid {
         puts("[COMP] compositor_set EBUSY — already held by pid ");
-        put_hex32(COMPOSITOR_PID);
+        put_hex32(cur);
         puts("\n");
         return EBUSY;
     }
-    COMPOSITOR_PID = pid;
+    COMPOSITOR_PID.store(pid, Relaxed);
     puts("[COMP] compositor_set: pid ");
     put_hex32(pid);
     puts(" registered\n");
@@ -54,12 +56,14 @@ pub unsafe fn sys_compositor_set() -> u64 {
 /// surfaces written (or total count if buf_ptr is 0).
 pub unsafe fn sys_win_surface_list(buf_ptr: u64, max_count: u64) -> u64 {
     // no compositor registered yet. u64::MAX = "try again later".
-    if COMPOSITOR_PID == 0 {
+    use core::sync::atomic::Ordering::Relaxed;
+    let cpid = COMPOSITOR_PID.load(Relaxed);
+    if cpid == 0 {
         return u64::MAX;
     }
 
     let pid = SCHEDULER.current_pid();
-    if pid != COMPOSITOR_PID {
+    if pid != cpid {
         // compositor is alive but you're not it. return 0 so shelld stops waiting.
         return 0;
     }
@@ -142,7 +146,7 @@ pub unsafe fn sys_win_surface_list(buf_ptr: u64, max_count: u64) -> u64 {
 /// EINVAL if the target has no surface, or EPERM if caller isn't compositor.
 pub unsafe fn sys_win_surface_map(target_pid: u64) -> u64 {
     let pid = SCHEDULER.current_pid();
-    if pid != COMPOSITOR_PID {
+    if pid != COMPOSITOR_PID.load(core::sync::atomic::Ordering::Relaxed) {
         return EPERM;
     }
 
@@ -178,7 +182,7 @@ pub unsafe fn sys_win_surface_map(target_pid: u64) -> u64 {
 /// packed_state: bits [15:0] = dx (i16), [31:16] = dy (i16), [39:32] = buttons.
 pub unsafe fn sys_mouse_forward(target_pid: u64, packed: u64) -> u64 {
     let pid = SCHEDULER.current_pid();
-    if pid != COMPOSITOR_PID {
+    if pid != COMPOSITOR_PID.load(core::sync::atomic::Ordering::Relaxed) {
         return EPERM;
     }
 
@@ -208,7 +212,7 @@ pub unsafe fn sys_mouse_forward(target_pid: u64, packed: u64) -> u64 {
 /// Only callable by the compositor (after it has read the surface).
 pub unsafe fn sys_win_surface_dirty_clear(target_pid: u64) -> u64 {
     let pid = SCHEDULER.current_pid();
-    if pid != COMPOSITOR_PID {
+    if pid != COMPOSITOR_PID.load(core::sync::atomic::Ordering::Relaxed) {
         return EPERM;
     }
 
@@ -257,7 +261,7 @@ pub unsafe fn sys_try_wait(pid: u64) -> u64 {
 pub unsafe fn sys_forward_input(target_pid: u64, ptr: u64, len: u64) -> u64 {
     // Only the compositor gets to play input router.
     let pid = SCHEDULER.current_pid();
-    if pid != COMPOSITOR_PID {
+    if pid != COMPOSITOR_PID.load(core::sync::atomic::Ordering::Relaxed) {
         return EPERM;
     }