Minimal interrupts, prevent deadlocks

tetros/src/drivers/serial.rs

@@ -1,9 +1,9 @@
-use core::arch::asm;
-
 use lazy_static::lazy_static;
 use spin::Mutex;
 use uart_16550::SerialPort;
 
+use crate::os::util::without_interrupts;
+
 lazy_static! {
 	pub static ref SERIAL1: Mutex<SerialPort> = {
 		let mut serial_port = unsafe { SerialPort::new(0x3F8) };
@@ -16,22 +16,14 @@ lazy_static! {
 pub fn _print(args: core::fmt::Arguments<'_>) {
 	use core::fmt::Write;
 
-	// TODO: preserve previous interrupt state
-
 	// Disable interrupts to prevent deadlocks
 	// (we might get an interrupt while printing)
-	unsafe {
+	without_interrupts(|| {
-		asm!("cli", options(preserves_flags, nostack));
+		SERIAL1
-	}
+			.lock()
-
+			.write_fmt(args)
-	SERIAL1
+			.expect("Printing to serial failed");
-		.lock()
+	})
-		.write_fmt(args)
-		.expect("Printing to serial failed");
-
-	unsafe {
-		asm!("sti", options(preserves_flags, nostack));
-	}
 }
 
 /// Prints to the host through the serial interface.

tetros/src/idt/stackframe.rs

@@ -1,9 +1,9 @@
+use bitflags::bitflags;
+use core::arch::asm;
 use core::{fmt, ops::Deref};
 
 use super::VirtAddr;
 
-use bitflags::bitflags;
-
 bitflags! {
 	/// The EFLAGS register. All bit patterns are valid representations for this type.
 	///
@@ -82,6 +82,24 @@ bitflags! {
 	}
 }
 
+impl EFlags {
+	#[inline]
+	pub fn read() -> EFlags {
+		EFlags::from_bits_truncate(EFlags::read_raw())
+	}
+
+	#[inline]
+	fn read_raw() -> u32 {
+		let r: u32;
+
+		unsafe {
+			asm!("pushfd; pop {0:e}", out(reg) r, options(nomem, preserves_flags));
+		}
+
+		r
+	}
+}
+
 /// Wrapper type for the interrupt stack frame pushed by the CPU.
 ///
 /// This type derefs to an [`InterruptStackFrameValue`], which allows reading the actual values.

tetros/src/lib.rs

@@ -11,7 +11,10 @@ use spin::Mutex;
 
 use drivers::{pic::PICDriver, vga::Vga13h};
 use idt::{InterruptDescriptorTable, InterruptStackFrame};
-use os::thunk::ThunkData;
+use os::{
+	thunk::ThunkData,
+	util::{sti, without_interrupts},
+};
 use tetrisboard::{FallingTetromino, TetrisBoard};
 
 mod idt;
@@ -28,7 +31,7 @@ pub(crate) static PIC: Mutex<PICDriver> = Mutex::new(PICDriver::new(PIC_OFFSET,
 pub(crate) static TICK_COUNTER: Mutex<u32> = Mutex::new(0);
 pub(crate) static BOARD: Mutex<TetrisBoard> = Mutex::new(TetrisBoard::new());
 pub(crate) static FALLING: Mutex<Option<FallingTetromino>> = Mutex::new(None);
-pub(crate) static RUN_TICKS: Mutex<bool> = Mutex::new(false);
+pub(crate) static LAST_INPUT: Mutex<Option<InputKey>> = Mutex::new(None);
 
 lazy_static! {
 	static ref RNG: Mutex<SmallRng> = Mutex::new(SmallRng::seed_from_u64(1337));
@@ -85,9 +88,7 @@ enum InputKey {
 	Down,
 }
 
-extern "x86-interrupt" fn keyboard_handler(stack_frame: InterruptStackFrame) {
+extern "x86-interrupt" fn keyboard_handler(_stack_frame: InterruptStackFrame) {
-	println!("KEYBOARD {:?}", stack_frame);
-
 	let scancode = unsafe { inb(0x60) };
 	println!("{:x?}", scancode);
 
@@ -104,84 +105,13 @@ extern "x86-interrupt" fn keyboard_handler(stack_frame: InterruptStackFrame) {
 		_ => None,
 	};
 
-	if let Some(fall) = &mut *FALLING.lock() {
+	*LAST_INPUT.lock() = key;
-		let board = BOARD.lock();
-		let mut fall_test = fall.clone();
-
-		match key {
-			Some(InputKey::Up) => {
-				fall_test.rotate_cw();
-				if board.tetromino_valid(&fall_test) {
-					fall.rotate_cw()
-				};
-			}
-
-			Some(InputKey::Down) => {
-				fall_test.rotate_ccw();
-				if board.tetromino_valid(&fall_test) {
-					fall.rotate_ccw()
-				};
-			}
-
-			Some(InputKey::Left) => {
-				fall_test.translate(-1, 0);
-				if board.tetromino_valid(&fall_test) {
-					fall.translate(-1, 0);
-				};
-			}
-
-			Some(InputKey::Right) => {
-				fall_test.translate(1, 0);
-				if board.tetromino_valid(&fall_test) {
-					fall.translate(1, 0);
-				};
-			}
-
-			_ => {}
-		}
-	}
-
 	PIC.lock().send_eoi(InterruptIndex::Keyboard.as_u8());
 }
 
 extern "x86-interrupt" fn timer_handler(_stack_frame: InterruptStackFrame) {
-	if !*RUN_TICKS.lock() {
+	let mut t = TICK_COUNTER.lock();
-		PIC.lock().send_eoi(InterruptIndex::Timer.as_u8());
+	*t = (*t).wrapping_add(1);
-		return;
-	}
-
-	// Step tick counter
-	let t = {
-		let mut t = TICK_COUNTER.lock();
-		*t = (*t).wrapping_add(1);
-		*t
-	};
-
-	let mut v = VGA.lock();
-	let mut board = BOARD.lock();
-	let mut fall = FALLING.lock();
-
-	v.swap();
-	board.draw(&mut v, fall.as_ref());
-
-	if let Some(fall_inner) = fall.as_mut() {
-		if t % 5 == 0 {
-			let mut fall_test = fall_inner.clone();
-			fall_test.translate(0, 1);
-
-			if board.tetromino_valid(&fall_test) {
-				fall_inner.translate(0, 1);
-			} else {
-				let mut x = None;
-				core::mem::swap(&mut x, &mut fall);
-				board.place_tetromino(x.unwrap());
-				board.collapse();
-			}
-		}
-	} else {
-		*fall = Some(FallingTetromino::random(5, 1))
-	}
-
 	PIC.lock().send_eoi(InterruptIndex::Timer.as_u8());
 }
 
@@ -224,11 +154,96 @@ pub unsafe extern "C" fn start(
 		pic.init();
 	}
 
-	*RUN_TICKS.lock() = true;
+	// We're ready for interrupts,
+	// enable them
+	sti();
 
-	// Do-nothing loop,
+	let mut last_t = 0;
-	// frames are driven by interrupts
 	loop {
-		asm!("hlt")
+		// All locks use `without_interrupts`
+		// to prevent deadlocks
+		let t = without_interrupts(|| {
+			let l = TICK_COUNTER.lock();
+			let t = *l;
+			drop(l);
+			return t;
+		});
+
+		if t == last_t {
+			continue;
+		}
+		last_t = t;
+
+		// Handle user input
+		without_interrupts(|| {
+			if let Some(fall) = &mut *FALLING.lock() {
+				let board = BOARD.lock();
+				let mut fall_test = fall.clone();
+				let mut last_input = LAST_INPUT.lock();
+
+				match *last_input {
+					Some(InputKey::Up) => {
+						fall_test.rotate_cw();
+						if board.tetromino_valid(&fall_test) {
+							fall.rotate_cw()
+						};
+					}
+
+					Some(InputKey::Down) => {
+						fall_test.rotate_ccw();
+						if board.tetromino_valid(&fall_test) {
+							fall.rotate_ccw()
+						};
+					}
+
+					Some(InputKey::Left) => {
+						fall_test.translate(-1, 0);
+						if board.tetromino_valid(&fall_test) {
+							fall.translate(-1, 0);
+						};
+					}
+
+					Some(InputKey::Right) => {
+						fall_test.translate(1, 0);
+						if board.tetromino_valid(&fall_test) {
+							fall.translate(1, 0);
+						};
+					}
+
+					_ => {}
+				}
+
+				// Clear last input, it was handled.
+				*last_input = None;
+			}
+		});
+
+		// Update board
+		without_interrupts(|| {
+			let mut v = VGA.lock();
+			let mut board = BOARD.lock();
+			let mut fall = FALLING.lock();
+
+			v.swap();
+			board.draw(&mut v, fall.as_ref());
+
+			if let Some(fall_inner) = fall.as_mut() {
+				if t % 5 == 0 {
+					let mut fall_test = fall_inner.clone();
+					fall_test.translate(0, 1);
+
+					if board.tetromino_valid(&fall_test) {
+						fall_inner.translate(0, 1);
+					} else {
+						let mut x = None;
+						core::mem::swap(&mut x, &mut fall);
+						board.place_tetromino(x.unwrap());
+						board.collapse();
+					}
+				}
+			} else {
+				*fall = Some(FallingTetromino::random(5, 1))
+			}
+		})
 	}
 }

tetros/src/os/mod.rs

@@ -1,4 +1,5 @@
 pub mod thunk;
+pub mod util;
 
 #[macro_use]
 pub mod panic;

tetros/src/os/util.rs

@@ -0,0 +1,47 @@
+use core::arch::asm;
+
+use crate::idt::EFlags;
+
+/// Disable interrupts
+#[inline]
+pub fn cli() {
+	unsafe {
+		asm!("cli", options(preserves_flags, nostack));
+	}
+}
+
+/// Enable interrupts
+#[inline]
+pub fn sti() {
+	unsafe {
+		asm!("sti", options(preserves_flags, nostack));
+	}
+}
+
+/// Run a closure with disabled interrupts.
+///
+/// Run the given closure, disabling interrupts before running it (if they aren't already disabled).
+/// Afterwards, interrupts are enabling again if they were enabled before.
+///
+/// If you have other `enable` and `disable` calls _within_ the closure, things may not work as expected.
+#[inline]
+pub fn without_interrupts<F, R>(f: F) -> R
+where
+	F: FnOnce() -> R,
+{
+	// true if the interrupt flag is set (i.e. interrupts are enabled)
+	let saved_intpt_flag = EFlags::read().contains(EFlags::INTERRUPT_FLAG);
+
+	// if interrupts are enabled, disable them for now
+	if saved_intpt_flag {
+		cli();
+	}
+
+	let ret = f();
+
+	if saved_intpt_flag {
+		sti();
+	}
+
+	ret
+}