From 3efb57141079c435e01c6a5d72f3629b39cf287d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Magalh=C3=A3es?= <joamag@gmail.com> Date: Thu, 25 Jul 2024 10:58:11 +0100 Subject: [PATCH] chore: support for cpu debug Made read MMU a non mutable operation. Added support for CPU description. Added Display support for CPU. Extracted type definition for `Instruction`. Support for diag_gb!() - uses global unsafe approach, which is not problematic for the purpose. Inline fetch() operation in cpu clock. Moved pedantic validation up in the CPU. Fix simple diag issue. Changed naming of read and write unsafe ops to raw. New pedantic asserts. Fix an issue with HDMA destination address prevent CHIP-8 ROM from working. --- crates/common/src/error.rs | 14 ++-- frontends/libretro/src/lib.rs | 15 ++++- frontends/sdl/README.md | 2 +- frontends/sdl/src/main.rs | 33 ++++++---- frontends/sdl/src/sdl.rs | 2 +- src/apu.rs | 9 +-- src/cpu.rs | 116 ++++++++++++++++++++++++---------- src/devices/printer.rs | 15 ++++- src/diag.rs | 103 ++++++++++++++++++++++++++++++ src/dma.rs | 95 ++++++++++++++++++++++++---- src/gb.rs | 9 +++ src/lib.rs | 1 + src/macros.rs | 2 +- src/mmu.rs | 45 ++++++++----- src/pad.rs | 5 +- src/ppu.rs | 12 ++-- src/rom.rs | 25 ++++++-- src/serial.rs | 5 +- src/state.rs | 2 +- src/timer.rs | 9 +-- 20 files changed, 409 insertions(+), 110 deletions(-) create mode 100644 src/diag.rs diff --git a/crates/common/src/error.rs b/crates/common/src/error.rs index 1c5ae65c..b12da2cf 100644 --- a/crates/common/src/error.rs +++ b/crates/common/src/error.rs @@ -1,3 +1,5 @@ +#![allow(clippy::uninlined_format_args)] + //! Error related data structures to be shared and used. //! //! This module contains the [`Error`] enum, which is used to represent @@ -14,16 +16,18 @@ pub enum Error { InvalidData, RomSize, IncompatibleBootRom, + InvalidParameter(String), CustomError(String), } impl Error { - pub fn description(&self) -> &str { + pub fn description(&self) -> String { match self { - Error::InvalidData => "Invalid data format", - Error::RomSize => "Invalid ROM size", - Error::IncompatibleBootRom => "Incompatible Boot ROM", - Error::CustomError(message) => message, + Error::InvalidData => String::from("Invalid data format"), + Error::RomSize => String::from("Invalid ROM size"), + Error::IncompatibleBootRom => String::from("Incompatible Boot ROM"), + Error::InvalidParameter(message) => format!("Invalid parameter: {}", message), + Error::CustomError(message) => String::from(message), } } } diff --git a/frontends/libretro/src/lib.rs b/frontends/libretro/src/lib.rs index d8003c91..233ba9c3 100644 --- a/frontends/libretro/src/lib.rs +++ b/frontends/libretro/src/lib.rs @@ -453,7 +453,10 @@ pub extern "C" fn retro_serialize(data: *mut c_void, size: usize) -> bool { Ok(state) => state, Err(err) => { warnln!("Failed to save state: {}", err); - return false; + #[allow(unreachable_code)] + { + return false; + } } }; if state.len() > size { @@ -462,7 +465,10 @@ pub extern "C" fn retro_serialize(data: *mut c_void, size: usize) -> bool { state.len(), size ); - return false; + #[allow(unreachable_code)] + { + return false; + } } unsafe { ptr::copy_nonoverlapping(state.as_ptr(), data as *mut u8, state.len()); @@ -477,7 +483,10 @@ pub extern "C" fn retro_unserialize(data: *const c_void, size: usize) -> bool { let state = unsafe { from_raw_parts(data as *const u8, size) }; if let Err(err) = StateManager::load(state, instance, None) { warnln!("Failed to load state: {}", err); - return false; + #[allow(unreachable_code)] + { + return false; + } } true } diff --git a/frontends/sdl/README.md b/frontends/sdl/README.md index fbcc3297..ad9cba6f 100644 --- a/frontends/sdl/README.md +++ b/frontends/sdl/README.md @@ -106,4 +106,4 @@ cargo run -- --rom-path ../../res/roms/test/blargg/cpu/cpu_instrs.gb --cycles 10 | `debug` | Activates the base `debug` feature from Boytacean. | | `pedantic` | Additional safety instructions are executed to make sure the machine does no run "out of tracks", making sure to run many `panic()` calls. | | `slow` | Runs the emulator at a very slow page 60x slower to allow visual debugging. | -| `cpulog` | Prints a log of the CPU instruction executed. | +| `cpulog` | Prints a log of the CPU instruction executed - will fill the stdout quickly. | diff --git a/frontends/sdl/src/main.rs b/frontends/sdl/src/main.rs index 34987e3b..c2312203 100644 --- a/frontends/sdl/src/main.rs +++ b/frontends/sdl/src/main.rs @@ -258,12 +258,12 @@ impl Emulator { } } - #[cfg(not(feature = "slow"))] - pub fn start_base(&mut self) {} - - #[cfg(feature = "slow")] pub fn start_base(&mut self) { - self.logic_frequency = 100; + self.system.set_diag(); + #[cfg(feature = "slow")] + { + self.logic_frequency = 100; + } } pub fn start_graphics(&mut self, sdl: &Sdl, screen_scale: f32) { @@ -420,6 +420,10 @@ impl Emulator { } } + pub fn print_debug(&mut self) { + println!("{}", self.system.description_debug()); + } + pub fn limited(&self) -> bool { !self.unlimited } @@ -504,6 +508,10 @@ impl Emulator { keycode: Some(Keycode::P), .. } => self.toggle_palette(), + Event::KeyDown { + keycode: Some(Keycode::C), + .. + } => self.print_debug(), Event::KeyDown { keycode: Some(Keycode::E), keymod, @@ -1030,7 +1038,7 @@ fn main() { let mode = Cartridge::from_file(&args.rom_path).unwrap().gb_mode(); game_boy.set_mode(mode); } - let device: Box<dyn SerialDevice> = build_device(&args.device); + let device: Box<dyn SerialDevice> = build_device(&args.device).unwrap(); game_boy.set_ppu_enabled(!args.no_ppu); game_boy.set_apu_enabled(!args.no_apu); game_boy.set_dma_enabled(!args.no_dma); @@ -1070,10 +1078,10 @@ fn main() { run(args, &mut emulator); } -fn build_device(device: &str) -> Box<dyn SerialDevice> { +fn build_device(device: &str) -> Result<Box<dyn SerialDevice>, Error> { match device { - "null" => Box::<NullDevice>::default(), - "stdout" => Box::<StdoutDevice>::default(), + "null" => Ok(Box::<NullDevice>::default()), + "stdout" => Ok(Box::<StdoutDevice>::default()), "printer" => { let mut printer = Box::<PrinterDevice>::default(); printer.set_callback(|image_buffer| { @@ -1087,9 +1095,12 @@ fn build_device(device: &str) -> Box<dyn SerialDevice> { ) .unwrap(); }); - printer + Ok(printer) } - _ => panic!("Unsupported device: {}", device), + _ => Err(Error::InvalidParameter(format!( + "Unsupported device: {}", + device + ))), } } diff --git a/frontends/sdl/src/sdl.rs b/frontends/sdl/src/sdl.rs index 3037a607..9df42a78 100644 --- a/frontends/sdl/src/sdl.rs +++ b/frontends/sdl/src/sdl.rs @@ -86,7 +86,7 @@ impl SdlSystem { } /// Creates an SDL2 Surface structure from the provided -/// bytes that represent an image (eg: an PNG image buffer). +/// bytes that represent an image (eg: a PNG image buffer). pub fn surface_from_bytes(bytes: &[u8]) -> Surface { unsafe { let rw_ops = RWops::from_bytes(bytes).unwrap(); diff --git a/src/apu.rs b/src/apu.rs index 0402f31d..810ed09e 100644 --- a/src/apu.rs +++ b/src/apu.rs @@ -349,7 +349,7 @@ impl Apu { } } - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { // 0xFF10 — NR10: Channel 1 sweep 0xff10 => { @@ -449,6 +449,7 @@ impl Apu { _ => { warnln!("Reading from unknown APU location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -658,11 +659,11 @@ impl Apu { } } - pub fn read_unsafe(&mut self, addr: u16) -> u8 { + pub fn read_raw(&mut self, addr: u16) -> u8 { self.read(addr) } - pub fn write_unsafe(&mut self, addr: u16, value: u8) { + pub fn write_raw(&mut self, addr: u16, value: u8) { self.write(addr, value); } @@ -1079,7 +1080,7 @@ impl Apu { } impl BusComponent for Apu { - fn read(&mut self, addr: u16) -> u8 { + fn read(&self, addr: u16) -> u8 { self.read(addr) } diff --git a/src/cpu.rs b/src/cpu.rs index d1049cd7..53aef909 100644 --- a/src/cpu.rs +++ b/src/cpu.rs @@ -5,10 +5,14 @@ //! //! Most of the core CPU logic is implemented in the [`Cpu::clock`] method. -use std::sync::Mutex; +use std::{ + fmt::{self, Display, Formatter}, + sync::Mutex, +}; use crate::{ apu::Apu, + assert_pedantic_gb, consts::LCDC_ADDR, debugln, dma::Dma, @@ -24,6 +28,8 @@ use crate::{ pub const PREFIX: u8 = 0xcb; +pub type Instruction = &'static (fn(&mut Cpu), u8, &'static str); + pub struct Cpu { pub pc: u16, pub sp: u16, @@ -50,6 +56,11 @@ pub struct Cpu { /// taken by the current or last CPU operation. pub cycles: u8, + /// Reference to the PC (Program Counter) of the previous executed + /// instruction, used to provide a reference to the instruction + /// so that it can be logged or used for debugging purposes. + pub ppc: u16, + /// The pointer to the parent configuration of the running /// Game Boy emulator, that can be used to control the behaviour /// of Game Boy emulation. @@ -76,6 +87,7 @@ impl Cpu { halted: false, mmu, cycles: 0, + ppc: 0x0, gbc, } } @@ -131,10 +143,18 @@ impl Cpu { // is going to be used in the fetching phase let pc = self.pc; - #[cfg(feature = "debug")] - if (0x8000..0x9fff).contains(&pc) { - panic!("Invalid PC area at 0x{:04x}", pc); - } + // runs a series of assertions to guarantee CPU execution + // state, only if pedantic mode is set + assert_pedantic_gb!( + !(0x8000..=0x9fff).contains(&pc), + "Invalid PC area at 0x{:04x}", + pc + ); + assert_pedantic_gb!( + !self.mmu.boot_active() || pc <= 0x08ff, + "Invalid boot address: {:04x}", + pc + ); // @TODO this is so bad, need to improve this by an order // of magnitude, to be able to have better performance @@ -265,21 +285,13 @@ impl Cpu { return 4; } - // fetches the current instruction and increments - // the PC (program counter) accordingly - let mut opcode = self.mmu.read(self.pc); - self.pc = self.pc.wrapping_add(1); - - let is_prefix = opcode == PREFIX; - let inst: &(fn(&mut Cpu), u8, &str); - - if is_prefix { - opcode = self.mmu.read(self.pc); - self.pc = self.pc.wrapping_add(1); - inst = &EXTENDED[opcode as usize]; - } else { - inst = &INSTRUCTIONS[opcode as usize]; - } + // fetches the current instruction and updates the PC + // (Program Counter) according to the final value returned + // by the fetch operation (we may need to fetch instruction + // more than one byte of length) + let (inst, pc) = self.fetch(self.pc); + self.ppc = self.pc; + self.pc = pc; #[allow(unused_variables)] let (inst_fn, inst_time, inst_str) = inst; @@ -300,19 +312,7 @@ impl Cpu { #[cfg(feature = "cpulog")] { - let title_str = format!("[0x{:04x}] {}", self.pc - 1, inst_str); - let inst_time_str = format!("({} cycles)", inst_time); - let registers_str = format!("[PC=0x{:04x} SP=0x{:04x}] [A=0x{:02x} B=0x{:02x} C=0x{:02x} D=0x{:02x} E=0x{:02x} H=0x{:02x} L=0x{:02x}]", - self.pc, self.sp, self.a, self.b, self.c, self.d, self.e, self.h, self.l); - println!( - "{0: <24} {1: <11} {2: <10}", - title_str, inst_time_str, registers_str - ); - } - - #[cfg(feature = "pedantic")] - if self.mmu.boot_active() && self.pc - 1 > 0x08ff { - panic!("Invalid boot address: {:04x}", self.pc - 1); + println!("{}", self.description(inst, self.ppc)); } // calls the current instruction and increments the number of @@ -327,6 +327,33 @@ impl Cpu { self.cycles } + #[inline(always)] + fn fetch(&self, pc: u16) -> (Instruction, u16) { + let mut pc = pc; + + // fetches the current instruction and increments + // the PC (program counter) accordingly + let mut opcode = self.mmu.read(pc); + pc = pc.wrapping_add(1); + + // checks if the current instruction is a prefix + // instruction, in case it is, fetches the next + // instruction and increments the PC accordingly + let inst: Instruction; + let is_prefix = opcode == PREFIX; + if is_prefix { + opcode = self.mmu.read(pc); + pc = pc.wrapping_add(1); + inst = &EXTENDED[opcode as usize]; + } else { + inst = &INSTRUCTIONS[opcode as usize]; + } + + // returns both the fetched instruction and the + // updated PC (Program Counter) value + (inst, pc) + } + #[inline(always)] pub fn mmu(&mut self) -> &mut Mmu { &mut self.mmu @@ -617,6 +644,23 @@ impl Cpu { pub fn set_gbc(&mut self, value: SharedThread<GameBoyConfig>) { self.gbc = value; } + + pub fn description(&self, inst: Instruction, inst_pc: u16) -> String { + let (_, inst_time, inst_str) = inst; + let title_str: String = format!("[0x{:04x}] {}", inst_pc, inst_str); + let inst_time_str = format!("({} cycles)", inst_time); + let registers_str = format!("[PC=0x{:04x} SP=0x{:04x}] [A=0x{:02x} B=0x{:02x} C=0x{:02x} D=0x{:02x} E=0x{:02x} H=0x{:02x} L=0x{:02x}]", + self.pc, self.sp, self.a, self.b, self.c, self.d, self.e, self.h, self.l); + format!( + "{0: <24} {1: <11} {2: <10}", + title_str, inst_time_str, registers_str + ) + } + + pub fn description_default(&self) -> String { + let (inst, _) = self.fetch(self.ppc); + self.description(inst, self.ppc) + } } impl Default for Cpu { @@ -626,6 +670,12 @@ impl Default for Cpu { } } +impl Display for Cpu { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.description_default()) + } +} + #[cfg(test)] mod tests { use super::Cpu; diff --git a/src/devices/printer.rs b/src/devices/printer.rs index bdfe76f5..910b24d8 100644 --- a/src/devices/printer.rs +++ b/src/devices/printer.rs @@ -266,7 +266,10 @@ impl SerialDevice for PrinterDevice { PrinterState::MagicBytes1 => { if byte != 0x88 { warnln!("Printer: Invalid magic byte 1: {:02x}", byte); - return; + #[allow(unreachable_code)] + { + return; + } } self.command = PrinterCommand::Other; self.command_length = 0; @@ -277,7 +280,10 @@ impl SerialDevice for PrinterDevice { self.state = PrinterState::MagicBytes1; } warnln!("Printer: Invalid magic byte 2: {:02x}", byte); - return; + #[allow(unreachable_code)] + { + return; + } } } PrinterState::Identification => self.command = PrinterCommand::from_u8(byte), @@ -308,7 +314,10 @@ impl SerialDevice for PrinterDevice { } PrinterState::Other => { warnln!("Printer: Invalid state: {:02x}", self.state as u8); - return; + #[allow(unreachable_code)] + { + return; + } } } diff --git a/src/diag.rs b/src/diag.rs new file mode 100644 index 00000000..b33d9c4a --- /dev/null +++ b/src/diag.rs @@ -0,0 +1,103 @@ +use std::ptr::null; + +use crate::gb::GameBoy; + +/// Static mutable reference to the global instance of the +/// Game Boy emulator, going to be used for global diagnostics. +static mut GLOBAL_INSTANCE: *const GameBoy = null(); + +impl GameBoy { + /// Sets the current instance as the one going to be used + /// in panic diagnostics. + pub fn set_diag(&self) { + self.set_global(); + } + + /// Dumps the diagnostics for the global instance of the + /// Boytacean emulator into stdout. + pub fn dump_diagnostics() { + if let Some(gb) = Self::get_global() { + gb.dump_diagnostics_s(); + } + } + + /// Sets the current instance as the global one going to + /// be used in panic diagnostics. + fn set_global(&self) { + unsafe { + GLOBAL_INSTANCE = self; + } + } + + /// Obtains the global instance of the Game Boy emulator + /// ready to be used in diagnostics. + /// + /// If the global instance is not set, it will return `None`. + fn get_global() -> Option<&'static Self> { + unsafe { + if GLOBAL_INSTANCE.is_null() { + None + } else { + Some(&*GLOBAL_INSTANCE) + } + } + } + + fn dump_diagnostics_s(&self) { + println!("Dumping Boytacean diagnostics:"); + println!("{}", self.description_debug()); + } +} + +#[macro_export] +macro_rules! panic_gb { + ($msg:expr) => { + { + $crate::gb::GameBoy::dump_diagnostics(); + panic!($msg); + } + }; + ($fmt:expr, $($arg:tt)*) => { + { + $crate::gb::GameBoy::dump_diagnostics(); + panic!($fmt, $($arg)*); + } + }; +} + +#[macro_export] +macro_rules! assert_gb { + ($cond:expr, $fmt:expr, $($arg:tt)*) => { + if !$cond { + if !$cond { + $crate::gb::GameBoy::dump_diagnostics(); + panic!($fmt, $($arg)*); + } + } + }; + ($cond:expr) => { + assert_gb!($cond, stringify!($cond)); + }; +} + +#[cfg(feature = "pedantic")] +#[macro_export] +macro_rules! assert_pedantic_gb { + ($cond:expr, $fmt:expr, $($arg:tt)*) => { + $crate::assert_gb!($cond, $fmt, $($arg)*); + }; + ($cond:expr) => { + $crate::assert_gb!($cond); + }; +} + +#[cfg(not(feature = "pedantic"))] +#[macro_export] +macro_rules! assert_pedantic_gb { + ($cond:expr, $fmt:expr, $($arg:tt)*) => { + () + }; + ($cond:expr) => { + () + }; +} diff --git a/src/dma.rs b/src/dma.rs index c0bc385a..ced23565 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -1,9 +1,11 @@ //! DMA (Direct Memory Access) functions and structures. +use std::fmt::{self, Display, Formatter}; + use crate::{ consts::{DMA_ADDR, HDMA1_ADDR, HDMA2_ADDR, HDMA3_ADDR, HDMA4_ADDR, HDMA5_ADDR}, mmu::BusComponent, - warnln, + panic_gb, warnln, }; #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -12,6 +14,35 @@ pub enum DmaMode { HBlank = 0x01, } +impl DmaMode { + pub fn description(&self) -> &'static str { + match self { + DmaMode::General => "General-Purpose DMA", + DmaMode::HBlank => "HBlank DMA", + } + } + + pub fn from_u8(value: u8) -> Self { + match value { + 0x00 => DmaMode::General, + 0x01 => DmaMode::HBlank, + _ => DmaMode::General, + } + } +} + +impl Display for DmaMode { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.description()) + } +} + +impl From<u8> for DmaMode { + fn from(value: u8) -> Self { + Self::from_u8(value) + } +} + pub struct Dma { source: u16, destination: u16, @@ -53,7 +84,7 @@ impl Dma { pub fn clock(&mut self, _cycles: u16) {} - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { // 0xFF46 — DMA: OAM DMA source address & start DMA_ADDR => self.value_dma, @@ -63,6 +94,7 @@ impl Dma { } _ => { warnln!("Reading from unknown DMA location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -86,14 +118,27 @@ impl Dma { HDMA4_ADDR => self.destination = (self.destination & 0xff00) | ((value & 0xf0) as u16), // 0xFF55 — HDMA5: VRAM DMA length/mode/start (CGB only) HDMA5_ADDR => { - self.length = (((value & 0x7f) + 0x1) as u16) << 4; - self.mode = match (value & 80) >> 7 { - 0 => DmaMode::General, - 1 => DmaMode::HBlank, - _ => DmaMode::General, - }; - self.pending = self.length; - self.active_hdma = true; + // in case there's an active HDMA transfer and the + // bit 7 is set to 0, the transfer is stopped + if value & 0x80 == 0x00 && self.active_hdma && self.mode == DmaMode::HBlank { + self.pending = 0; + self.active_hdma = false; + } else { + // ensures destination is set within VRAM range + // required for compatibility with some games (know bug) + self.destination = 0x8000 | (self.destination & 0x1fff); + self.length = (((value & 0x7f) + 0x1) as u16) << 4; + self.mode = ((value & 80) >> 7).into(); + self.pending = self.length; + self.active_hdma = true; + + // @TODO: implement HBlank DMA using the proper timing + // and during the HBlank period as described in the + // https://gbdev.io/pandocs/CGB_Registers.html#lcd-vram-dma-transfers + if self.mode == DmaMode::HBlank { + panic_gb!("HBlank DMA not implemented"); + } + } } _ => warnln!("Writing to unknown DMA location 0x{:04x}", addr), } @@ -174,10 +219,32 @@ impl Dma { pub fn active(&self) -> bool { self.active_dma || self.active_hdma } + + pub fn description(&self) -> String { + format!( + "DMA: {}\nHDMA: {}", + self.description_dma(), + self.description_hdma() + ) + } + + pub fn description_dma(&self) -> String { + format!( + "active: {}, cycles: {}, value: 0x{:02x}", + self.active_dma, self.cycles_dma, self.value_dma + ) + } + + pub fn description_hdma(&self) -> String { + format!( + "active: {}, length: 0x{:04x}, mode: {}, source: 0x{:04x}, destination: 0x{:04x}", + self.active_hdma, self.length, self.mode, self.source, self.destination + ) + } } impl BusComponent for Dma { - fn read(&mut self, addr: u16) -> u8 { + fn read(&self, addr: u16) -> u8 { self.read(addr) } @@ -192,6 +259,12 @@ impl Default for Dma { } } +impl Display for Dma { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.description()) + } +} + #[cfg(test)] mod tests { use super::{Dma, DmaMode}; diff --git a/src/gb.rs b/src/gb.rs index 5f86cf00..5d18d316 100644 --- a/src/gb.rs +++ b/src/gb.rs @@ -1022,6 +1022,15 @@ impl GameBoy { self.serial_i().device().description(), ) } + + pub fn description_debug(&self) -> String { + format!( + "{}\nCPU:\n{}\nDMA:\n{}", + self.description(12), + self.cpu_i().description_default(), + self.dma_i().description() + ) + } } /// Gameboy implementations that are meant with performance diff --git a/src/lib.rs b/src/lib.rs index 318726e5..6538d3c3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,6 +8,7 @@ pub mod consts; pub mod cpu; pub mod data; pub mod devices; +pub mod diag; pub mod dma; pub mod gb; pub mod gen; diff --git a/src/macros.rs b/src/macros.rs index 4fb08076..a9344317 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -32,7 +32,7 @@ macro_rules! infoln { macro_rules! warnln { ($($rest:tt)*) => { { - panic!($($rest)*); + $crate::panic_gb!($($rest)*); } } } diff --git a/src/mmu.rs b/src/mmu.rs index cf8f4173..d3cbd313 100644 --- a/src/mmu.rs +++ b/src/mmu.rs @@ -4,9 +4,11 @@ use std::sync::Mutex; use crate::{ apu::Apu, + assert_pedantic_gb, dma::Dma, gb::{Components, GameBoyConfig, GameBoyMode, GameBoySpeed}, pad::Pad, + panic_gb, ppu::Ppu, rom::Cartridge, serial::Serial, @@ -22,9 +24,9 @@ pub const RAM_SIZE_DMG: usize = 8192; pub const RAM_SIZE_CGB: usize = 32768; pub trait BusComponent { - fn read(&mut self, addr: u16) -> u8; + fn read(&self, addr: u16) -> u8; fn write(&mut self, addr: u16, value: u8); - fn read_many(&mut self, addr: u16, count: usize) -> Vec<u8> { + fn read_many(&self, addr: u16, count: usize) -> Vec<u8> { (0..count) .map(|offset| self.read(addr + offset as u16)) .collect() @@ -196,7 +198,7 @@ impl Mmu { match base_addr { 0xa000 => self.rom.ram_data_mut()[addr as usize] = value, 0xc000 => self.ram[addr as usize] = value, - _ => panic!("Invalid base address for write: 0x{:04x}", base_addr), + _ => panic_gb!("Invalid base address for write: 0x{:04x}", base_addr), } } } @@ -306,6 +308,20 @@ impl Mmu { // // we should also respect General-Purpose DMA vs HBlank DMA if self.dma.active_hdma() { + // runs a series of pre-validation on the HDMA transfer in + // pedantic mode is currently active (performance hit) + assert_pedantic_gb!( + (0x0000..=0x7ff0).contains(&self.dma.source()) + || (0xa000..=0xdff0).contains(&self.dma.source()), + "Invalid HDMA source start memory address 0x{:04x}", + self.dma.source() + ); + assert_pedantic_gb!( + (0x8000..=0x9ff0).contains(&self.dma.destination()), + "Invalid HDMA destination start memory address 0x{:04x}", + self.dma.destination() + ); + // only runs the DMA transfer if the system is in CGB mode // this avoids issues when writing to DMG unmapped registers // that would otherwise cause the system to crash @@ -318,19 +334,13 @@ impl Mmu { } } - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { // 0x0000-0x0FFF - BOOT (256 B) + ROM0 (4 KB/16 KB) 0x0000..=0x0fff => { // in case the boot mode is active and the // address is withing boot memory reads from it - if self.boot_active && addr <= 0x00fe { - // if we're reading from this location we can - // safely assume that we're exiting the boot - // loading sequence and disable boot - if addr == 0x00fe { - self.boot_active = false; - } + if self.boot_active && addr <= 0x00ff { return self.boot[addr as usize]; } if self.boot_active @@ -428,6 +438,7 @@ impl Mmu { addr => { warnln!("Reading from unknown location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -535,9 +546,9 @@ impl Mmu { /// Reads a byte from a certain memory address, without the typical /// Game Boy verifications, allowing deep read of values. - pub fn read_unsafe(&mut self, addr: u16) -> u8 { + pub fn read_raw(&mut self, addr: u16) -> u8 { match addr { - 0xff10..=0xff3f => self.apu.read_unsafe(addr), + 0xff10..=0xff3f => self.apu.read_raw(addr), _ => self.read(addr), } } @@ -546,9 +557,9 @@ impl Mmu { /// Game Boy verification process. This allows for faster memory /// access in registers and other memory areas that are typically /// inaccessible. - pub fn write_unsafe(&mut self, addr: u16, value: u8) { + pub fn write_raw(&mut self, addr: u16, value: u8) { match addr { - 0xff10..=0xff3f => self.apu.write_unsafe(addr, value), + 0xff10..=0xff3f => self.apu.write_raw(addr, value), _ => self.write(addr, value), } } @@ -574,7 +585,7 @@ impl Mmu { let mut data: Vec<u8> = vec![]; for index in 0..count { - let byte = self.read_unsafe(addr + index); + let byte = self.read_raw(addr + index); data.push(byte); } @@ -583,7 +594,7 @@ impl Mmu { pub fn write_many_unsafe(&mut self, addr: u16, data: &[u8]) { for (index, byte) in data.iter().enumerate() { - self.write_unsafe(addr + index as u16, *byte) + self.write_raw(addr + index as u16, *byte) } } diff --git a/src/pad.rs b/src/pad.rs index aba8acc1..62dd6080 100644 --- a/src/pad.rs +++ b/src/pad.rs @@ -75,7 +75,7 @@ impl Pad { } } - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { // 0xFF00 — P1/JOYP: Joypad 0xff00 => { @@ -107,6 +107,7 @@ impl Pad { } _ => { warnln!("Reading from unknown Pad location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -174,7 +175,7 @@ impl Pad { } impl BusComponent for Pad { - fn read(&mut self, addr: u16) -> u8 { + fn read(&self, addr: u16) -> u8 { self.read(addr) } diff --git a/src/ppu.rs b/src/ppu.rs index ac1a73be..ccb1ae61 100644 --- a/src/ppu.rs +++ b/src/ppu.rs @@ -21,6 +21,7 @@ use crate::{ }, gb::{GameBoyConfig, GameBoyMode}, mmu::BusComponent, + panic_gb, util::SharedThread, warnln, }; @@ -780,7 +781,7 @@ impl Ppu { } } - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { 0x8000..=0x9fff => self.vram[(self.vram_offset + (addr & 0x1fff)) as usize], 0xfe00..=0xfe9f => self.oam[(addr & 0x009f) as usize], @@ -837,6 +838,7 @@ impl Ppu { 0xff6c => (if self.obj_priority { 0x01 } else { 0x00 }) | 0xfe, _ => { warnln!("Reading from unknown PPU location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -1777,7 +1779,7 @@ impl Ppu { } else if obj.palette == 1 { (&self.palette_obj_1, 0_u8) } else { - panic!("Invalid object palette: {:02x}", obj.palette); + panic_gb!("Invalid object palette: {:02x}", obj.palette); } } else { (&self.palettes_color_obj[obj.palette_cgb as usize], 0_u8) @@ -1787,7 +1789,7 @@ impl Ppu { } else if obj.palette == 1 { (&self.palette_obj_1, 2_u8) } else { - panic!("Invalid object palette: {:02x}", obj.palette); + panic_gb!("Invalid object palette: {:02x}", obj.palette); }; // obtains the current integer value (raw) for the palette in use @@ -1987,7 +1989,7 @@ impl Ppu { let color_index: usize = (value as usize >> (index * 2)) & 3; match color_index { 0..=3 => *palette_item = palette_colors[color_index], - color_index => panic!("Invalid palette color index {:04x}", color_index), + color_index => panic_gb!("Invalid palette color index {:04x}", color_index), } } } @@ -2045,7 +2047,7 @@ impl Ppu { } impl BusComponent for Ppu { - fn read(&mut self, addr: u16) -> u8 { + fn read(&self, addr: u16) -> u8 { self.read(addr) } diff --git a/src/rom.rs b/src/rom.rs index 572c86ac..bc288768 100644 --- a/src/rom.rs +++ b/src/rom.rs @@ -14,6 +14,7 @@ use crate::{ gb::GameBoyMode, licensee::Licensee, mmu::BusComponent, + panic_gb, util::read_file, warnln, }; @@ -424,7 +425,7 @@ impl Cartridge { Self::from_data(&data) } - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { // 0x0000-0x7FFF: 16 KiB ROM bank 00 & 16 KiB ROM Bank 01–NN 0x0000..=0x7fff => (self.handler.read_rom)(self, addr), @@ -930,7 +931,7 @@ impl Cartridge { } impl BusComponent for Cartridge { - fn read(&mut self, addr: u16) -> u8 { + fn read(&self, addr: u16) -> u8 { self.read(addr) } @@ -968,7 +969,7 @@ pub static NO_MBC: Mbc = Mbc { // to this address for some reason (probably related to // some kind of MBC1 compatibility issue) 0x2000 => (), - _ => panic!("Writing to unknown Cartridge ROM location 0x{:04x}", addr), + _ => panic_gb!("Writing to unknown Cartridge ROM location 0x{:04x}", addr), }; }, read_ram: |rom: &Cartridge, addr: u16| -> u8 { rom.ram_data[(addr - 0xa000) as usize] }, @@ -988,6 +989,7 @@ pub static MBC1: Mbc = Mbc { .unwrap_or(&0x0), _ => { warnln!("Reading from unknown Cartridge ROM location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -1033,7 +1035,10 @@ pub static MBC1: Mbc = Mbc { write_ram: |rom: &mut Cartridge, addr: u16, value: u8| { if !rom.ram_enabled { warnln!("Attempt to write to ERAM while write protect is active"); - return; + #[allow(unreachable_code)] + { + return; + } } rom.ram_data[rom.ram_offset + (addr - 0xa000) as usize] = value; }, @@ -1050,6 +1055,7 @@ pub static MBC3: Mbc = Mbc { .unwrap_or(&0x0), _ => { warnln!("Reading from unknown Cartridge ROM location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -1089,7 +1095,10 @@ pub static MBC3: Mbc = Mbc { write_ram: |rom: &mut Cartridge, addr: u16, value: u8| { if !rom.ram_enabled { warnln!("Attempt to write to ERAM while write protect is active"); - return; + #[allow(unreachable_code)] + { + return; + } } rom.ram_data[rom.ram_offset + (addr - 0xa000) as usize] = value; }, @@ -1106,6 +1115,7 @@ pub static MBC5: Mbc = Mbc { .unwrap_or(&0x0), _ => { warnln!("Reading from unknown Cartridge ROM location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -1159,7 +1169,10 @@ pub static MBC5: Mbc = Mbc { write_ram: |rom: &mut Cartridge, addr: u16, value: u8| { if !rom.ram_enabled { warnln!("Attempt to write to ERAM while write protect is active"); - return; + #[allow(unreachable_code)] + { + return; + } } rom.ram_data[rom.ram_offset + (addr - 0xa000) as usize] = value; }, diff --git a/src/serial.rs b/src/serial.rs index 9c5ff073..ac0db54c 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -90,7 +90,7 @@ impl Serial { } } - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { // 0xFF01 — SB: Serial transfer data SB_ADDR => self.data, @@ -104,6 +104,7 @@ impl Serial { } _ => { warnln!("Reding from unknown Serial location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -209,7 +210,7 @@ impl Serial { } impl BusComponent for Serial { - fn read(&mut self, addr: u16) -> u8 { + fn read(&self, addr: u16) -> u8 { self.read(addr) } diff --git a/src/state.rs b/src/state.rs index 8c4c917d..e4f19120 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1659,7 +1659,7 @@ mod tests { assert_eq!(bess_core.de, 0x0000); assert_eq!(bess_core.hl, 0x0000); assert_eq!(bess_core.sp, 0x0000); - assert_eq!(bess_core.ime, false); + assert!(!bess_core.ime); assert_eq!(bess_core.ie, 0x00); assert_eq!(bess_core.execution_mode, 0); assert_eq!(bess_core.io_registers.len(), 128); diff --git a/src/timer.rs b/src/timer.rs index c70b8ff1..2fe4038f 100644 --- a/src/timer.rs +++ b/src/timer.rs @@ -3,7 +3,7 @@ use crate::{ consts::{DIV_ADDR, TAC_ADDR, TIMA_ADDR, TMA_ADDR}, mmu::BusComponent, - warnln, + panic_gb, warnln, }; pub struct Timer { @@ -73,7 +73,7 @@ impl Timer { } } - pub fn read(&mut self, addr: u16) -> u8 { + pub fn read(&self, addr: u16) -> u8 { match addr { // 0xFF04 — DIV: Divider register DIV_ADDR => self.div, @@ -85,6 +85,7 @@ impl Timer { TAC_ADDR => self.tac | 0xf8, _ => { warnln!("Reding from unknown Timer location 0x{:04x}", addr); + #[allow(unreachable_code)] 0xff } } @@ -106,7 +107,7 @@ impl Timer { 0x01 => self.tima_ratio = 16, 0x02 => self.tima_ratio = 64, 0x03 => self.tima_ratio = 256, - value => panic!("Invalid TAC value 0x{:02x}", value), + value => panic_gb!("Invalid TAC value 0x{:02x}", value), } self.tima_enabled = (value & 0x04) == 0x04; } @@ -151,7 +152,7 @@ impl Timer { } impl BusComponent for Timer { - fn read(&mut self, addr: u16) -> u8 { + fn read(&self, addr: u16) -> u8 { self.read(addr) } -- GitLab