Fix panic by disabling exceptions and more BIOS bug fixes

.github/workflows/rust.yml

@@ -48,7 +48,7 @@ jobs:
         TEST_EXE=$(CARGO_TARGET_MIPSEL_SONY_PSX_RUNNER=echo cargo psx test)
         xvfb-run mednafen -psx.dbg_level 2 -psx.bios_jp scph7001.bin $TEST_EXE \
             | tee bios_stdout &
-        sleep 15
+        sleep 30
         grep "test result: ok. [0-9]* passed; 0 failed" bios_stdout
         popd
     - name: Build examples

examples/bios/src/allocator.rs

@@ -4,7 +4,7 @@ use core::mem::size_of;
 use core::ptr;
 use core::ptr::NonNull;
 use linked_list_allocator::Heap;
-use psx::sys;
+use psx::hw::{cop0, Register};
 
 #[global_allocator]
 pub static HEAP: Global<Heap> = Global::new(Heap::empty());
@@ -75,7 +75,8 @@ trait CAlloc: GlobalAlloc {
 
 unsafe impl GlobalAlloc for Global<Heap> {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
-        let ptr = sys::critical_section(|cs| self.borrow(cs).allocate_first_fit(layout));
+        let ptr =
+            cop0::Status::new().critical_section(|cs| self.borrow(cs).allocate_first_fit(layout));
         match ptr {
             Ok(nonnull) => nonnull.as_ptr(),
             Err(_) => ptr::null_mut(),
@@ -87,14 +88,14 @@ unsafe impl GlobalAlloc for Global<Heap> {
             Some(ptr) => ptr,
             None => return,
         };
-        sys::critical_section(|cs| self.borrow(cs).deallocate(ptr, layout))
+        cop0::Status::new().critical_section(|cs| self.borrow(cs).deallocate(ptr, layout))
     }
 }
 
 impl CAlloc for Global<Heap> {}
 
 pub fn init_heap(addr: *mut u8, len: usize) -> u32 {
-    sys::critical_section(|cs| {
+    cop0::Status::new().critical_section(|cs| {
         let heap = HEAP.borrow(cs);
         // SAFETY: Let's hope the user passed an unused region of memory
         unsafe { heap.init(addr, len) }

examples/bios/src/exceptions.rs

@@ -162,8 +162,10 @@ extern "C" fn call_handlers(
     let new_tcb = match cause.excode() {
         Excode::Interrupt => call_irq_handlers(tcb, cs),
         Excode::Syscall | Excode::Breakpoint => {
-            unsafe {
-                asm!("addiu $k1, 4");
+            if cause.branch_delay_slot() {
+                unsafe {
+                    asm!("addiu $k1, 4");
+                }
             }
             if cause.excode() == Excode::Syscall {
                 syscall_handler(cs, r4, r5)
@@ -172,7 +174,7 @@ extern "C" fn call_handlers(
                 ptr::null_mut()
             }
         },
-        _ => unsafe { core::hint::unreachable_unchecked() },
+        _ => unreachable!(""),
     };
     new_tcb
 }
@@ -266,7 +268,7 @@ fn syscall_handler(cs: &mut CriticalSection, r4: u32, r5: u32) -> *mut ThreadCon
         *CURRENT_THREAD.borrow(cs) = new_tcb;
         return new_tcb
     } else {
-        unsafe { core::hint::unreachable_unchecked() }
+        unreachable!("")
     }
     ptr::null_mut()
 }

examples/bios/src/global.rs

@@ -24,6 +24,6 @@ impl<T> Global<T> {
     pub fn borrow(&self, _: &mut CriticalSection) -> &mut T {
         let ptr = self.as_ptr();
         let opt_ref = unsafe { ptr.as_mut() };
-        unsafe { opt_ref.unwrap_unchecked() }
+        opt_ref.unwrap()
     }
 }

examples/bios/src/main.rs

@@ -1,5 +1,6 @@
 #![no_std]
 #![no_main]
+#![allow(internal_features)]
 #![feature(linked_list_remove)]
 #![feature(asm_experimental_arch)]
 #![feature(asm_const)]

examples/bios/src/thread.rs

@@ -15,7 +15,6 @@ use core::ptr;
 use core::slice;
 use psx::constants::KB;
 use psx::hw::{cop0, Register};
-use psx::sys;
 use psx::sys::kernel::psx_change_thread_sub_fn;
 use psx::CriticalSection;
 
@@ -73,10 +72,10 @@ pub fn resume_main() {
 }
 
 pub fn park() {
-    sys::critical_section(|cs| {
+    cop0::Status::new().critical_section(|cs| {
         let threads = THREADS.borrow(cs);
         let idx = threads.iter().position(|tcb| tcb.running).unwrap();
-        let mut current_tcb = &mut threads[idx];
+        let current_tcb = &mut threads[idx];
         current_tcb.parked = true;
     });
 }
@@ -154,15 +153,15 @@ impl<A: Send, R: Send> Thread<A, R> {
     }
 
     pub fn unpark(&mut self) {
-        sys::critical_section(|cs| {
+        cop0::Status::new().critical_section(|cs| {
             let tcb = &mut THREADS.borrow(cs)[self.handle.get_idx()];
             tcb.parked = false;
         });
     }
 
     pub fn join(mut self) -> R {
         self.resume();
-        let regs = sys::critical_section(|cs| {
+        let regs = cop0::Status::new().critical_section(|cs| {
             // SAFETY: static mut access in a critical section
             let threads = THREADS.borrow(cs);
             let tcb = &threads[self.handle.get_idx()];
@@ -323,13 +322,11 @@ pub fn reschedule_threads(
             tcb.running = false;
         }
     }
-    let next_tcb = unsafe {
-        threads
-            .iter_mut()
-            .filter(unparked)
-            .nth(next_thread)
-            .unwrap_unchecked()
-    };
+    let next_tcb = threads
+        .iter_mut()
+        .filter(unparked)
+        .nth(next_thread)
+        .unwrap();
     // Mark the next TCB as running
     next_tcb.running = true;
     *CURRENT_THREAD.borrow(cs) = next_tcb;
@@ -362,9 +359,10 @@ static THREADS: Global<[ThreadControlBlock; 4]> = {
 pub fn open_thread(
     pc: *const u32, sp: *mut u32, gp: *mut u32, args: [u32; 4], stack: *mut u32,
 ) -> ThreadHandle {
-    let old_sr = cop0::Status::new().to_bits();
+    let mut sr = cop0::Status::new();
+    let old_sr = sr.to_bits();
     let old_cause = cop0::Cause::new().to_bits();
-    sys::critical_section(|cs| {
+    sr.critical_section(|cs| {
         let threads = THREADS.borrow(cs);
         for (i, tcb) in threads.iter_mut().enumerate() {
             if !tcb.in_use {
@@ -398,7 +396,7 @@ pub fn open_thread(
 
 pub fn change_thread(handle: ThreadHandle, set_ra: bool) -> u32 {
     let new = handle.get_idx();
-    sys::critical_section(|cs| {
+    cop0::Status::new().critical_section(|cs| {
         let threads = THREADS.borrow(cs);
         if threads[new].in_use {
             let old = threads.iter().position(|tcb| tcb.running).unwrap();
@@ -421,8 +419,8 @@ pub fn change_thread(handle: ThreadHandle, set_ra: bool) -> u32 {
 
 pub fn close_thread(handle: ThreadHandle) -> u32 {
     let idx = handle.get_idx();
-    sys::critical_section(|cs| {
-        let mut thread = &mut THREADS.borrow(cs)[idx];
+    cop0::Status::new().critical_section(|cs| {
+        let thread = &mut THREADS.borrow(cs)[idx];
         thread.in_use = false;
         thread.running = false;
         thread.parked = true;

psx/src/gpu/mod.rs

@@ -194,9 +194,9 @@ impl DispEnv {
         let size = Vertex::new(size);
         let offset = PackedVertex::try_from(offset)?;
         let (center, range) = if video_mode == VideoMode::NTSC {
-            (0x88, 0x240)
+            (0x88, 240)
         } else {
-            (0xA3, 0x256)
+            (0xA3, 256)
         };
         let ntsc_vrange = Vertex(center - (range / 2), center + (range / 2));
         let hrange = Vertex(0x260, 0x260 + (size.0 * 8));

psx/src/hw/cop0/status.rs

@@ -208,7 +208,7 @@ impl Status {
     /// Run a closure in an interrupt-free context
     pub fn critical_section<F: FnMut(&mut CriticalSection) -> R, R>(&mut self, mut f: F) -> R {
         let in_critical_section =
-            self.interrupt_masked(IntSrc::Hardware) && self.interrupts_disabled();
+            self.interrupt_masked(IntSrc::Hardware) || self.interrupts_disabled();
 
         if !in_critical_section {
             self.mask_interrupt(IntSrc::Hardware)

psx/src/panic.rs

@@ -1,60 +1,80 @@
+use crate::hw::{cop0, Register};
 use crate::{dprintln, println, Framebuffer};
 
 #[panic_handler]
 fn panic(info: &core::panic::PanicInfo) -> ! {
+    cop0::Status::new().critical_section(|_| {
+        // SAFETY: We're in a critical section so no other threads can get a
+        // mutable reference. The previous (nested) call to panic, if any,
+        // that accesses IN_PANIC won't hold onto a reference since we drop
+        // them before calling display_panic.
+        static mut IN_PANIC: bool = false;
+        unsafe {
+            if IN_PANIC {
+                println!("Panicked in panic!");
+            }
+            IN_PANIC = true;
+        }
+        display_panic(info);
+        loop {}
+    })
+}
+
+fn display_panic(info: &core::panic::PanicInfo) {
     // Print to stdout unless no_panic is set. This includes the default case since
     // printing to the screen during a panic is not always reliable.
     #[cfg(not(feature = "no_panic"))]
-    {
+    min_panic(info);
+    // In the default case print the panic message to the screen
+    #[cfg(not(any(feature = "min_panic", feature = "no_panic")))]
+    normal_panic(info);
+}
+
+fn min_panic(info: &core::panic::PanicInfo) {
+    match info.location() {
+        Some(location) => {
+            println!(
+                "Panicked at {}:{}:{}",
+                location.file(),
+                location.line(),
+                location.column()
+            )
+        },
+        None => {
+            println!("Panicked at unknown location")
+        },
+    }
+    if let Some(msg) = info.message() {
+        println!("{}", msg)
+    }
+}
+
+fn normal_panic(info: &core::panic::PanicInfo) {
+    // We have no idea what state the GPU was in when the panic happened, so reset
+    // it to a known state and reload the font into VRAM.
+    let mut fb = Framebuffer::default();
+    let mut txt = fb.load_default_font().new_text_box((0, 8), (320, 240));
+    loop {
+        txt.reset();
         match info.location() {
             Some(location) => {
-                println!(
+                dprintln!(
+                    txt,
                     "Panicked at {}:{}:{}",
                     location.file(),
                     location.line(),
                     location.column()
-                )
+                );
             },
             None => {
-                println!("Panicked at unknown location")
+                dprintln!(txt, "Panicked at unknown location");
             },
         }
         if let Some(msg) = info.message() {
-            println!("{}", msg)
-        }
-    };
-    // In the default case print the panic message to the screen
-    #[cfg(not(any(feature = "min_panic", feature = "no_panic")))]
-    {
-        // We have no idea what state the GPU was in when the panic happened, so reset
-        // it to a known state and reload the font into VRAM.
-        let mut fb = Framebuffer::default();
-        let mut txt = fb.load_default_font().new_text_box((0, 8), (320, 240));
-        loop {
-            txt.reset();
-            match info.location() {
-                Some(location) => {
-                    dprintln!(
-                        txt,
-                        "Panicked at {}:{}:{}",
-                        location.file(),
-                        location.line(),
-                        location.column()
-                    );
-                },
-                None => {
-                    dprintln!(txt, "Panicked at unknown location");
-                },
-            }
-            if let Some(msg) = info.message() {
-                dprintln!(txt, "{}", msg);
-            }
-            fb.draw_sync();
-            fb.wait_vblank();
-            fb.swap();
+            dprintln!(txt, "{}", msg);
         }
-    };
-    // Both min_panic and no_panic end execution here
-    #[cfg(any(feature = "min_panic", feature = "no_panic"))]
-    loop {}
+        fb.draw_sync();
+        fb.wait_vblank();
+        fb.swap();
+    }
 }

psx/src/std.rs

@@ -11,26 +11,26 @@ pub trait AsCStr: AsRef<[u8]> {
 impl<T: AsRef<[u8]>> AsCStr for T {
     /// Calls a function `f` with `Self` as a null-terminated C-style string.
     /// This panics if the string is not null-terminated and requires more than
-    /// 64 bytes of stack space.
+    /// 128 bytes of stack space.
     fn as_cstr<F: FnOnce(&CStr) -> R, R>(&self, f: F) -> R {
         let slice = self.as_ref();
         match CStr::from_bytes_with_nul(slice) {
             Ok(cstr) => f(cstr),
             Err(_) => {
-                // Panic if we need more than 64 bytes for the CStr. This is an arbitrary
+                // Panic if we need more than 128 bytes for the CStr. This is an arbitrary
                 // limit. Since the executables are statically linked, the
                 // compiler should avoid allocating the maximum stack space a
                 // lot of the time. When printing strings with unknown length at
                 // compile-time such as strings from the BIOS or user input, the
                 // compiler will allocate the maximum space on the stack which is why
                 // this limit is not as high as it could be.
-                const MAX_LEN: usize = 64;
+                const MAX_LEN: usize = 128;
                 if slice.len() >= MAX_LEN {
                     panic!(
-                        "Attempted to allocate more than 64 bytes on the stack for a `CStr`\n\0"
+                        "Attempted to allocate more than 128 bytes on the stack for a `CStr`\n\0"
                     );
                 }
-                // Create an uninitialized array of up to 64 bytes on the stack
+                // Create an uninitialized array of up to 128 bytes on the stack
                 let mut uninitialized = MaybeUninit::uninit_array::<MAX_LEN>();
                 // Initialize the CStr with the input string
                 let initialized_part = &mut uninitialized[0..slice.len() + 1];

psx/src/sys/mod.rs

@@ -14,7 +14,12 @@ pub mod tty;
 
 /// Calls the given function in an interrupt-free critical section using BIOS
 /// syscalls.
-pub fn critical_section<F: FnMut(&mut CriticalSection) -> R, R>(mut f: F) -> R {
+///
+/// # Safety
+///
+/// Exception handlers might not support nested exceptions so make sure to not
+/// call this from a critical section.
+pub unsafe fn critical_section<F: FnMut(&mut CriticalSection) -> R, R>(mut f: F) -> R {
     let changed_state = unsafe { kernel::psx_enter_critical_section() };
     // SAFETY: We are in a critical section so we can create this
     let mut cs = unsafe { CriticalSection::new() };