From e3bdce6065b393d63616bb74b6ad16684ba4872f Mon Sep 17 00:00:00 2001
From: Mark <mark@betalupi.com>
Date: Sun, 2 Mar 2025 10:03:16 -0800
Subject: [PATCH] Minimal interrupts, prevent deadlocks
---
tetros/src/drivers/serial.rs | 24 ++---
tetros/src/idt/stackframe.rs | 22 ++++-
tetros/src/lib.rs | 181 +++++++++++++++++++----------------
tetros/src/os/mod.rs | 1 +
tetros/src/os/util.rs | 47 +++++++++
5 files changed, 174 insertions(+), 101 deletions(-)
create mode 100644 tetros/src/os/util.rs
diff --git a/tetros/src/drivers/serial.rs b/tetros/src/drivers/serial.rs
index 6e771ed..cb5bad5 100644
--- a/tetros/src/drivers/serial.rs
+++ b/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 {
- asm!("cli", options(preserves_flags, nostack));
- }
-
- SERIAL1
- .lock()
- .write_fmt(args)
- .expect("Printing to serial failed");
-
- unsafe {
- asm!("sti", options(preserves_flags, nostack));
- }
+ without_interrupts(|| {
+ SERIAL1
+ .lock()
+ .write_fmt(args)
+ .expect("Printing to serial failed");
+ })
}
/// Prints to the host through the serial interface.
diff --git a/tetros/src/idt/stackframe.rs b/tetros/src/idt/stackframe.rs
index e0c82e1..fb207b5 100644
--- a/tetros/src/idt/stackframe.rs
+++ b/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.
diff --git a/tetros/src/lib.rs b/tetros/src/lib.rs
index 2017f8c..660421b 100644
--- a/tetros/src/lib.rs
+++ b/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) {
- println!("KEYBOARD {:?}", stack_frame);
-
+extern "x86-interrupt" fn keyboard_handler(_stack_frame: InterruptStackFrame) {
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() {
- 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);
- };
- }
-
- _ => {}
- }
- }
-
+ *LAST_INPUT.lock() = key;
PIC.lock().send_eoi(InterruptIndex::Keyboard.as_u8());
}
extern "x86-interrupt" fn timer_handler(_stack_frame: InterruptStackFrame) {
- if !*RUN_TICKS.lock() {
- PIC.lock().send_eoi(InterruptIndex::Timer.as_u8());
- 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))
- }
-
+ let mut t = TICK_COUNTER.lock();
+ *t = (*t).wrapping_add(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,
- // frames are driven by interrupts
+ let mut last_t = 0;
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))
+ }
+ })
}
}
diff --git a/tetros/src/os/mod.rs b/tetros/src/os/mod.rs
index 99795d2..5b1be8e 100644
--- a/tetros/src/os/mod.rs
+++ b/tetros/src/os/mod.rs
@@ -1,4 +1,5 @@
pub mod thunk;
+pub mod util;
#[macro_use]
pub mod panic;
diff --git a/tetros/src/os/util.rs b/tetros/src/os/util.rs
new file mode 100644
index 0000000..56cd62a
--- /dev/null
+++ b/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
+}