From bff9220fb1c41321cc5093fb133b3b0d831a74e3 Mon Sep 17 00:00:00 2001 From: Alek Ratzloff Date: Thu, 12 Mar 2020 16:56:20 -0400 Subject: [PATCH] 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 --- src/libvm/src/error.rs | 26 ++++++-- src/libvm/src/interrupt.rs | 14 +++-- src/libvm/src/mem.rs | 10 ++-- src/libvm/src/obj/assemble.rs | 4 +- src/libvm/src/obj/syn/ast.rs | 4 +- src/libvm/src/state.rs | 110 ++++++++++++++++++++++++++-------- tests/test_interrupts.asm | 22 ++++++- vm.md | 8 +-- 8 files changed, 146 insertions(+), 52 deletions(-) diff --git a/src/libvm/src/error.rs b/src/libvm/src/error.rs index 4868d33..3eb1ebb 100644 --- a/src/libvm/src/error.rs +++ b/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 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 = std::result::Result; diff --git a/src/libvm/src/interrupt.rs b/src/libvm/src/interrupt.rs index 848dfd4..d92d116 100644 --- a/src/libvm/src/interrupt.rs +++ b/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 } diff --git a/src/libvm/src/mem.rs b/src/libvm/src/mem.rs index 2ccb8c5..c8e6be6 100644 --- a/src/libvm/src/mem.rs +++ b/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 { 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(()) } diff --git a/src/libvm/src/obj/assemble.rs b/src/libvm/src/obj/assemble.rs index d15f108..a69311d 100644 --- a/src/libvm/src/obj/assemble.rs +++ b/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); diff --git a/src/libvm/src/obj/syn/ast.rs b/src/libvm/src/obj/syn/ast.rs index 160714b..8f158a8 100644 --- a/src/libvm/src/obj/syn/ast.rs +++ b/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; @@ -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::(), + ValueDef::Interrupt(_, _) => std::mem::size_of::(), } } } diff --git a/src/libvm/src/state.rs b/src/libvm/src/state.rs index 3938110..b6052af 100644 --- a/src/libvm/src/state.rs +++ b/src/libvm/src/state.rs @@ -13,6 +13,7 @@ use std::mem; pub struct State { regs: [u64; NUM_REGS], mem: Vec, + interrupt_stack: Vec, } 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 { 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::()); + let end = start + (IVT_LENGTH * mem::size_of::()); 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 { + pub fn interrupt(&mut self, index: InterruptIndex, aux: u64) -> Result { assert!(index < IVT_LENGTH, "invalid interrupt index"); let interrupt = self.ivt()?[index]; - if !interrupt.enabled() { - return Ok(return_ip); + if !self.contains_flags(Flags::INTERRUPT_ENABLE) || !interrupt.enabled() { + // 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 { + 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 { 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(()) } diff --git a/tests/test_interrupts.asm b/tests/test_interrupts.asm index 4182423..1b7f530 100644 --- a/tests/test_interrupts.asm +++ b/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 } diff --git a/vm.md b/vm.md index e61a68b..7c83d57 100644 --- a/vm.md +++ b/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 - * Attempted to invoke an illegal opcode + * Auxiliary: Memory address where illegal instruction is located + * Attempted to execute a malformed instruction * Illegal memory address * Interrupt vector: 0x02 * Auxiliary: Memory address causing the interrupt