Extend how interrupts are reported to the main execution loop

Some things that were previously hard VM-level errors are now handled by
interrupts. While this is relatively easy to handle, I was wanting a
little more structure for the error types - so, errors that should
invoke an interrupt are passed along in their own structure in a
VmError::Interrupt variant. If an error is raised during the tick()
phase of execution that would cause an interrupt, that interrupt is
intercepted and the VM continues.

The State::interrupt() function also will catch double faults and triple
faults, with a triple fault being its own variant in the VmError
structure (so it cannot be intercepted as an interrupt by accident).

Signed-off-by: Alek Ratzloff <alekratz@gmail.com>

src/libvm/src/error.rs

@@ -3,6 +3,27 @@ use snafu::Snafu;
 
 #[derive(Snafu, Debug, Clone)]
 pub enum VmError {
+    #[snafu(display("object to load spans too much memory"))]
+    ObjectTooLarge { object_size: usize, max_mem: usize },
+
+    #[snafu(display("an interrupt was not caught"))]
+    Interrupt { source: InterruptError },
+
+    #[snafu(display("a triple fault occurred"))]
+    TripleFault,
+}
+
+impl From<InterruptError> for VmError {
+    fn from(other: InterruptError) -> Self {
+        VmError::Interrupt { source: other, }
+    }
+}
+
+/// The error type that is returned by the `State::tick()` function.
+///
+/// This error is meant to be caught and handled specifically, but can be passed up as a `VmError`.
+#[derive(Snafu, Debug, Clone)]
+pub enum InterruptError {
     #[snafu(display("illegal register: 0x{:02x}", reg))]
     IllegalReg { reg: Reg },
 
@@ -10,16 +31,13 @@ pub enum VmError {
     MemOutOfBounds { addr: Addr },
 
     #[snafu(display("illegal instruction opcode: 0x{:04x}", op))]
-    IllegalOp { op: InstOp },
+    IllegalOpcode { op: InstOp },
 
     #[snafu(display("illegal destination specification: 0b{:08b}", spec))]
     IllegalDestSpec { spec: u8 },
 
     #[snafu(display("illegal source specification: 0b{:08b}", spec))]
     IllegalSourceSpec { spec: u8 },
-
-    #[snafu(display("object to load spans too much memory"))]
-    ObjectTooLarge { object_size: usize, max_mem: usize },
 }
 
 pub type Result<T, E = VmError> = std::result::Result<T, E>;

src/libvm/src/interrupt.rs

@@ -2,13 +2,13 @@ use crate::addr::Addr;
 
 macro_rules! interrupts {
     {
-        $($variant:ident = $value:expr),* $(,)?
+        $($const_name:ident = $value:expr),* $(,)?
     } => {
-        pub type IntVec = usize;
+        pub type InterruptIndex = usize;
 
         $(
             #[allow(dead_code)]
-            pub const $variant: IntVec = $value;
+            pub const $const_name: InterruptIndex = $value;
         )*
     };
 }
@@ -17,7 +17,7 @@ interrupts! {
     // Error state exceptions
 
     DOUBLE_FAULT           = 0x00,
-    INVALID_OPCODE         = 0x01,
+    ILLEGAL_INSTRUCTION    = 0x01,
     ILLEGAL_MEMORY_ADDRESS = 0x02,
     DIVIDE_BY_ZERO         = 0x03,
 
@@ -26,15 +26,17 @@ interrupts! {
     //IO_EVENT               = 0x80,
 }
 
+pub const MAX_ERROR_INTERRUPT: usize = 0x7f;
+
 pub const IVT_LENGTH: usize = 512;
 
 #[derive(Debug, Clone, Copy, Default)]
-pub struct Interrupt(u64);
+pub struct InterruptVector(u64);
 
 const ENABLED_MASK: u64 = 0x8000_0000_0000_0000;
 const ADDR_MASK: u64 = 0x1fff_ffff_ffff_ffff;
 
-impl Interrupt {
+impl InterruptVector {
     pub fn enabled(&self) -> bool {
         (self.0 & ENABLED_MASK) != 0
     }

src/libvm/src/mem.rs

@@ -152,7 +152,7 @@ where
             HALT => Ok(Inst::Halt),
             NOP => Ok(Inst::Nop),
             DUMP => Ok(Inst::Dump),
-            _ => Err(VmError::IllegalOp { op }),
+            _ => Err(VmError::from(InterruptError::IllegalOpcode { op, })),
         }?;
         let end = self.position();
         let len = (end - start) as usize;
@@ -189,7 +189,7 @@ where
             DEST_REG_ADDR16 => Ok(Dest::RegAddr16(self.next_reg()?)),
             DEST_REG_ADDR8 => Ok(Dest::RegAddr8(self.next_reg()?)),
             DEST_REG => Ok(Dest::Reg(self.next_reg()?)),
-            _ => Err(VmError::IllegalDestSpec { spec }),
+            _ => Err(InterruptError::IllegalDestSpec { spec }.into()),
         }
     }
 
@@ -208,14 +208,14 @@ where
             SOURCE_IMM32 => Ok(Source::Imm(self.next_u32()? as u64)),
             SOURCE_IMM16 => Ok(Source::Imm(self.next_u16()? as u64)),
             SOURCE_IMM8 => Ok(Source::Imm(self.next_u8()? as u64)),
-            _ => Err(VmError::IllegalSourceSpec { spec }),
+            _ => Err(InterruptError::IllegalSourceSpec { spec }.into()),
         }
     }
 
     fn next_reg(&mut self) -> Result<Reg> {
         let reg = self.next_u8()?;
         if (reg as usize) >= NUM_REGS {
-            Err(VmError::IllegalReg { reg })
+            Err(InterruptError::IllegalReg { reg }.into())
         } else {
             Ok(reg)
         }
@@ -228,7 +228,7 @@ where
 {
     fn check_addr(&self, addr: u64) -> Result<()> {
         if addr >= (self.cursor.get_ref().as_ref().len() as u64) {
-            Err(VmError::MemOutOfBounds { addr: Addr(addr) })
+            Err(InterruptError::MemOutOfBounds { addr: Addr(addr) }.into())
         } else {
             Ok(())
         }

src/libvm/src/obj/assemble.rs

@@ -7,7 +7,7 @@ use self::{error::*, session::AsmSession};
 use crate::{
     addr::Addr,
     inst,
-    interrupt::Interrupt,
+    interrupt::InterruptVector,
     obj::{
         obj::{self, Object},
         syn::ast::*,
@@ -177,7 +177,7 @@ impl Asm for ValueDef {
             }
             ValueDef::Interrupt(e, a) => {
                 use std::convert::TryInto;
-                let mut interrupt = Interrupt::default();
+                let mut interrupt = InterruptVector::default();
                 interrupt.set_enabled(*e);
                 let mut addr_bytes = a.assemble(session)?;
                 addr_bytes.resize(8, 0);

src/libvm/src/obj/syn/ast.rs

@@ -1,4 +1,4 @@
-use crate::{inst, interrupt::Interrupt, reg::Reg};
+use crate::{inst, interrupt::InterruptVector, reg::Reg};
 
 pub type Ast = Vec<Directive>;
 
@@ -152,7 +152,7 @@ impl ValueDef {
             ValueDef::Int(_, s) => s.len(),
             ValueDef::String(s) => 8 + s.as_bytes().len(),
             ValueDef::ZString(s) => s.as_bytes().len() + 1,
-            ValueDef::Interrupt(_, _) => std::mem::size_of::<Interrupt>(),
+            ValueDef::Interrupt(_, _) => std::mem::size_of::<InterruptVector>(),
         }
     }
 }

src/libvm/src/state.rs

@@ -13,6 +13,7 @@ use std::mem;
 pub struct State {
     regs: [u64; NUM_REGS],
     mem: Vec<u8>,
+    interrupt_stack: Vec<InterruptIndex>,
 }
 
 impl State {
@@ -20,6 +21,7 @@ impl State {
         State {
             regs: [0; NUM_REGS],
             mem: Default::default(),
+            interrupt_stack: Default::default(),
         }
     }
 
@@ -106,7 +108,7 @@ impl State {
 
     pub fn get_reg(&self, reg: Reg) -> Result<u64> {
         if (reg as usize) >= NUM_REGS {
-            Err(VmError::IllegalReg { reg })
+            Err(InterruptError::IllegalReg { reg }.into())
         } else {
             Ok(self.get_reg_unchecked(reg))
         }
@@ -118,7 +120,7 @@ impl State {
 
     pub fn set_reg(&mut self, reg: Reg, value: u64) -> Result<()> {
         if (reg as usize) >= NUM_REGS {
-            Err(VmError::IllegalReg { reg })
+            Err(InterruptError::IllegalReg { reg }.into())
         } else {
             Ok(self.set_reg_unchecked(reg, value))
         }
@@ -176,7 +178,7 @@ impl State {
     ////////////////////////////////////////////////////////////////////////////////
     // Interrupts
     ////////////////////////////////////////////////////////////////////////////////
-    pub fn ivt(&self) -> Result<&[Interrupt]> {
+    pub fn ivt(&self) -> Result<&[InterruptVector]> {
         let ivt_addr = self.get_reg_unchecked(IVT);
         if ivt_addr % 64 != 0 {
             panic!(
@@ -186,18 +188,18 @@ impl State {
         }
 
         let start = ivt_addr as usize;
-        let end = start + (IVT_LENGTH * mem::size_of::<Interrupt>());
+        let end = start + (IVT_LENGTH * mem::size_of::<InterruptVector>());
 
         if end > self.mem.len() {
-            return Err(VmError::MemOutOfBounds {
+            return Err(InterruptError::MemOutOfBounds {
                 addr: Addr(ivt_addr),
-            });
+            }.into());
         }
 
         // This is safe because we check the bounds above
         let slice_ptr = (&self.mem[start..]).as_ptr();
         let slice =
-            unsafe { std::slice::from_raw_parts(slice_ptr as *const Interrupt, IVT_LENGTH) };
+            unsafe { std::slice::from_raw_parts(slice_ptr as *const InterruptVector, IVT_LENGTH) };
 
         Ok(slice)
     }
@@ -207,15 +209,33 @@ impl State {
     const INTERRUPT_REG_SPACE: u64 = (R31 - R00 + 4) as u64 * 8;
 
     /// Invoke an interrupt.
-    pub fn interrupt(&mut self, return_ip: u64, index: usize, aux: u64) -> Result<u64> {
+    pub fn interrupt(&mut self, index: InterruptIndex, aux: u64) -> Result<u64> {
         assert!(index < IVT_LENGTH, "invalid interrupt index");
         let interrupt = self.ivt()?[index];
-        if !interrupt.enabled() {
+        if !self.contains_flags(Flags::INTERRUPT_ENABLE) || !interrupt.enabled() {
-            return Ok(return_ip);
+            // get whether this is a double fault or an error interrupt
+            // if a double fault is invoked while it is disabled, immediately triple-fault
+            if index == DOUBLE_FAULT {
+                return Err(VmError::TripleFault);
+            } else if index <= MAX_ERROR_INTERRUPT {
+                return self.interrupt(DOUBLE_FAULT, index as u64);
+            } else {
+                return Ok(self.ip());
+            }
         }
 
+        // If this interrupt is an error, then check if a double or triple fault should be invoked.
+        if index <= MAX_ERROR_INTERRUPT {
+            if self.is_handling_double_fault() {
+                return Err(VmError::TripleFault);
+            } else if self.is_handling_error() {
+                return self.interrupt(DOUBLE_FAULT, index as u64);
+            }
+        }
+        // otherwise, handle it like normal
+
         let fp = self.fp();
-        let ip = return_ip;
+        let ip = self.ip();
         let flags = self.get_reg_unchecked(FLAGS);
         let status = self.status();
 
@@ -233,12 +253,14 @@ impl State {
         self.set_reg_unchecked(FP, sp - Self::INTERRUPT_REG_SPACE);
         self.set_reg_unchecked(R00, index as u64);
         self.set_reg_unchecked(R01, aux);
+        self.interrupt_stack.push(index);
 
         Ok(interrupt.addr().0)
     }
 
     /// Exit/return from the current interrupt.
     pub fn exit_interrupt(&mut self) -> Result<u64> {
+        self.interrupt_stack.pop().expect("mismatched interrupt stack");
         let fp = self.fp();
         let sp = fp + Self::INTERRUPT_REG_SPACE;
 
@@ -256,16 +278,50 @@ impl State {
         Ok(self.ip())
     }
 
+    /// Get whether a double fault interrupt is currently being handled.
+    fn is_handling_double_fault(&self) -> bool {
+        self.interrupt_stack.contains(&DOUBLE_FAULT)
+    }
+
+    /// Get whether an error interrupt is currently being handled.
+    fn is_handling_error(&self) -> bool {
+        self.interrupt_stack.iter()
+            .copied()
+            .any(|i| i <= MAX_ERROR_INTERRUPT)
+    }
+
     ////////////////////////////////////////////////////////////////////////////////
     // Execution
     ////////////////////////////////////////////////////////////////////////////////
+    pub fn halt(&mut self) {
+        self.insert_flags(Flags::HALT);
+    }
+
     pub fn is_halted(&self) -> bool {
         self.contains_flags(Flags::HALT)
     }
 
     pub fn exec(&mut self) -> Result<u64> {
         while !self.is_halted() {
-            self.tick()?;
+            // TODO : better interrupt handling than this. Error-specific interrupts are a good
+            // start, but catching multiple interrupts as they happen may be better.
+            let tick = self.tick();
+            if let Err(VmError::Interrupt { source: interrupt }) = tick {
+                let next_ip = match interrupt {
+                    InterruptError::MemOutOfBounds { addr, } => {
+                        self.interrupt(ILLEGAL_MEMORY_ADDRESS, addr.0)?
+                    }
+                    InterruptError::IllegalReg { .. }
+                    | InterruptError::IllegalOpcode { .. }
+                    | InterruptError::IllegalDestSpec { .. }
+                    | InterruptError::IllegalSourceSpec { .. } => {
+                        self.interrupt(ILLEGAL_INSTRUCTION, self.ip())?
+                    }
+                };
+                self.set_reg_unchecked(IP, next_ip);
+            } else {
+                tick?;
+            }
         }
 
         Ok(self.get_reg_unchecked(STATUS))
@@ -274,7 +330,8 @@ impl State {
     fn tick(&mut self) -> Result<()> {
         let mut cursor = self.mem_cursor(Addr(self.ip()));
         let inst = cursor.next_inst()?;
-        let mut next_ip = self.ip() + (inst.len() as u64);
+        let next_ip = self.ip() + (inst.len() as u64);
+        self.set_reg_unchecked(IP, next_ip);
         match inst {
             Inst::Add(d, s) => {
                 let value = self.load_dest(d)?.wrapping_add(self.load_source(s)?);
@@ -291,7 +348,8 @@ impl State {
             Inst::Div(d, s) => {
                 let src = self.load_source(s)?;
                 if src == 0 {
-                    next_ip = self.interrupt(next_ip, DIVIDE_BY_ZERO, 0)?;
+                    let next_ip = self.interrupt(DIVIDE_BY_ZERO, 0)?;
+                    self.set_reg_unchecked(IP, next_ip);
                 } else {
                     let value = self.load_dest(d)?.wrapping_div(src);
                     self.store_dest(d, value)?;
@@ -301,7 +359,8 @@ impl State {
                 let dest = self.load_dest(d)? as i64;
                 let source = self.load_source(s)? as i64;
                 if source == 0 {
-                    next_ip = self.interrupt(next_ip, DIVIDE_BY_ZERO, 0)?;
+                    let next_ip = self.interrupt(DIVIDE_BY_ZERO, 0)?;
+                    self.set_reg_unchecked(IP, next_ip);
                 } else {
                     let value = dest.wrapping_div(source);
                     self.store_dest(d, value as u64)?;
@@ -360,16 +419,16 @@ impl State {
                 }
             }
             Inst::Jmp(s) => {
-                next_ip = self.load_source(s)?;
+                self.set_reg_unchecked(IP, self.load_source(s)?);
             }
             Inst::Jz(s) => {
                 if !self.contains_flags(Flags::COMPARE) {
-                    next_ip = self.load_source(s)?;
+                    self.set_reg_unchecked(IP, self.load_source(s)?);
                 }
             }
             Inst::Jnz(s) => {
                 if self.contains_flags(Flags::COMPARE) {
-                    next_ip = self.load_source(s)?;
+                    self.set_reg_unchecked(IP, self.load_source(s)?);
                 }
             }
             Inst::Call(s) => {
@@ -384,7 +443,7 @@ impl State {
                     let sp = self.sp();
                     self.set_reg_unchecked(FP, sp - 16);
 
-                    next_ip = self.load_source(s)?;
+                    self.set_reg_unchecked(IP, self.load_source(s)?);
                 }
             }
             Inst::Ret => {
@@ -394,8 +453,6 @@ impl State {
 
                 self.pop(Dest::Reg(IP))?;
                 self.pop(Dest::Reg(FP))?;
-
-                next_ip = self.ip();
             }
             Inst::Push(s) => {
                 self.push(s)?;
@@ -404,26 +461,27 @@ impl State {
                 self.pop(d)?;
             }
             Inst::Int(v, a) => {
-                let vector = self.load_source(v)?;
+                let index = self.load_source(v)?;
                 let aux = self.load_source(a)?;
-                next_ip = self.interrupt(next_ip, vector as usize, aux)?;
+                let next_ip = self.interrupt(index as InterruptIndex, aux)?;
+                self.set_reg_unchecked(IP, next_ip);
             }
             Inst::IRet => {
-                next_ip = self.exit_interrupt()?;
+                let next_ip = self.exit_interrupt()?;
+                self.set_reg_unchecked(IP, next_ip);
             }
             Inst::Mov(d, s) => {
                 let value = self.load_source(s)?;
                 self.store_dest(d, value)?;
             }
             Inst::Halt => {
-                self.insert_flags(Flags::HALT);
+                self.halt();
             }
             Inst::Nop => {}
             Inst::Dump => {
                 // TODO - dump
             }
         }
-        self.set_reg_unchecked(IP, next_ip);
         Ok(())
     }
 

tests/test_interrupts.asm

@@ -1,9 +1,15 @@
 .section code 0x1000 {
     main:
         ; Test that interrupts will not called when enabled flag is not set
-        int 0, 0
+        ; (using 0x80 because anything below it is an error interrupt, and
+        ;  invoking an error interrupt when disabled will cascade to a triple
+        ;  fault)
+        mov %status, 1
+        int 0x80, 0
         cmpeq (count), 0
-        jnz end
+        jz end
+
+        or %flags, 0b100
 
         ; Test divide by zero interrupts
 
@@ -20,6 +26,16 @@
         cmpeq (count), 2
         jz end
 
+        ; Test illegal memory address interrupts
+        add %status, 1
+        ; TODO - fix this bug, this line breaks because it's trying to
+        ; calculate an address that's too large without using wrapping addition
+        ; functions
+        ;mov %status, (0xfffffffffffffff)
+        mov %status, (0xffffffff)
+        cmpeq (count), 3
+        jz end
+
         mov %status, 0
     end:
         halt
@@ -37,7 +53,7 @@
     ivt:
     .interrupt 0, 0
     .interrupt 0, 0
-    .interrupt 0, 0
+    .interrupt 1, generic_handler
     .interrupt 1, generic_handler
     .export ivt
 }

vm.md

@@ -343,12 +343,12 @@ will unconditionally halt the machine.
 
 * Double fault
     * Interrupt vector: 0x00
-    * Auxiliary: 
+    * Auxiliary: The interrupt vector that was being invoked that caused the fault.
     * An error state interrupt occurred while already handling an error state interrupt
-* Invalid opcode
+* Illegal instruction
     * Interrupt vector: 0x01
-    * Auxiliary: N/A
+    * Auxiliary: Memory address where illegal instruction is located
-    * Attempted to invoke an illegal opcode
+    * Attempted to execute a malformed instruction
 * Illegal memory address
     * Interrupt vector: 0x02
     * Auxiliary: Memory address causing the interrupt