diff --git a/src/gdb_stub.rs b/src/gdb_stub.rs index 1643b7c..0f71ff6 100644 --- a/src/gdb_stub.rs +++ b/src/gdb_stub.rs @@ -356,9 +356,12 @@ impl SingleThreadBase for IrisTarget { } fn read_addrs(&mut self, start_addr: u64, data: &mut [u8]) -> TargetResult { - if self.cpu.read_mem(mips_sign_extend(start_addr), data).is_err() { - data.fill(0); - } + // Fail the read instead of returning zeros with an OK: GDB decodes + // zeroed bytes as a valid `nop` and would plan its software + // single-step from that fabricated code instead of reporting a fault. + self.cpu + .read_mem(mips_sign_extend(start_addr), data) + .map_err(|_| TargetError::NonFatal)?; Ok(data.len()) } diff --git a/src/mips_exec.rs b/src/mips_exec.rs index 93d1238..8d5e60e 100644 --- a/src/mips_exec.rs +++ b/src/mips_exec.rs @@ -1085,6 +1085,21 @@ pub(crate) const HW_READ_FIXUP_ADDRS: &[u64] = &[ 0x1fbd98bc, 0x1fbd98bd, 0x1fbd98be, 0x1fbd98bf, // IOC_TIMER_CTL ]; +/// Comparison key for breakpoint addresses (see `check_breakpoint`): word-aligns, +/// and folds the unmapped kseg0/kseg1 windows onto the physical address they alias +/// so either window hits. Every other address keeps all of its bits. +#[inline] +fn bp_match_key(addr: u64) -> u64 { + let hi = addr >> 32; + if hi == 0 || hi == 0xFFFF_FFFF { + let lo = addr & 0xFFFF_FFFF; + if (0x8000_0000..0xC000_0000).contains(&lo) { + return (lo & 0x1FFF_FFFF) & !3; + } + } + addr & !3 +} + // ---- translate_fn slow-path wrappers (one per privilege × addressing-mode combination) ------ // These are free functions so they can be stored as bare fn pointers in MipsExecutor. // They are only called on a nanotlb miss — the nanotlb probe happens before the fn-pointer call. @@ -2184,7 +2199,15 @@ impl MipsExecutor { #[cfg(not(feature = "lightning"))] if self.bp_enabled() && self.check_breakpoint::<{ BpType::Pc as u8 }>(pc) { - return EXEC_BREAKPOINT; + // This runs before the fetch, so only honor the breakpoint once the + // instruction is fetchable: an instruction whose fetch takes a TLB + // miss never executes, and a debugger can't read that page either. + if !self.debug_translate(pc).is_exception() { + return EXEC_BREAKPOINT; + } + // Not taken: drop the id check_breakpoint() just recorded, or the + // next stop is misreported as this breakpoint. + self.last_bp_hit = None; } // No per-instruction CP0 Count work: Count is virtual (materialized @@ -2365,12 +2388,10 @@ impl MipsExecutor { let mut hit = false; for bp in &self.breakpoints { if bp.enabled && bp.kind as u8 == KIND { - // Normalize to physical: strip sign-extension and kseg bits (top 3 of - // low 32), then mask bottom 2 for word alignment. This makes a bp set - // on physical 0x1fbb0010 hit whether the CPU access came through kseg0 - // (0x9fbb0010) or kseg1 (0xbfbb0010). - const PHYS_MASK: u64 = 0x1FFF_FFF8; - if (bp.addr & PHYS_MASK) == (addr & PHYS_MASK) { + // Normalize kseg0/kseg1 to physical so a bp on physical + // 0x1fbb0010 hits through kseg0 (0x9fbb0010) or kseg1 + // (0xbfbb0010), then word-align. Never mask below bit 2. + if bp_match_key(bp.addr) == bp_match_key(addr) { // Check optional register condition if let Some(expr) = &bp.condition { let symbols = self.symbols.lock(); @@ -7797,6 +7818,11 @@ impl MipsCpu { let task = move || { let mut exec = executor.lock(); + // Clear any breakpoint id left from a previous run/step, or a stop + // caused by something else (retry, exception_mask) is reported by + // run_blocking() — and to GDB — as a phantom breakpoint hit. + exec.last_bp_hit = None; + // Same reasoning as MipsCpu::start(): this closure runs on its own // fresh OS thread ("MIPS-Debug"), so the host FPU rounding mode // needs to be re-synced from the guest's tracked FCSR.RM rather @@ -7818,6 +7844,10 @@ impl MipsCpu { let mut first_step = true; let mut steps_since_yield = 0; + // Consecutive EXEC_RETRY count for the instruction currently being + // attempted (see the EXEC_RETRY arm); reset once anything retires. + let mut retry_count: u32 = 0; + const MAX_RUN_RETRIES: u32 = 100_000; loop { if !running.load(Ordering::Relaxed) { @@ -7901,8 +7931,19 @@ impl MipsCpu { let pc = exec.core.pc; match status { EXEC_RETRY => { - writeln!(writer, "PC={:016x}: Retry (Bus Busy)", pc).unwrap(); - break; + // Nothing retired, so the same PC must be attempted + // again (validate.rs documents the same contract); + // breaking out made a transient bus-busy a real stop. + retry_count += 1; + if retry_count > MAX_RUN_RETRIES { + writeln!(writer, "PC={:016x}: Retry (Bus Busy) — still busy after {} attempts, giving up", pc, MAX_RUN_RETRIES).unwrap(); + break; + } + // Refund the step budget: a retry retires no + // instruction, so `step N` must still retire N. + if let Some(c) = count { count = Some(c + 1); } + std::hint::spin_loop(); + continue; } s if s & EXEC_IS_EXCEPTION != 0 && s & EXEC_IS_TLB_REFILL == 0 => { let code = (s >> crate::mips_core::CAUSE_EXCCODE_SHIFT) & 0x1F; @@ -7933,6 +7974,10 @@ impl MipsCpu { _ => {} } + // Something retired: the retry budget is per instruction + // attempt, not cumulative over a long run. + retry_count = 0; + steps_since_yield += 1; if steps_since_yield >= 500000 { steps_since_yield = 0; @@ -10763,11 +10808,16 @@ impl CpuDebug } fn step_one(&self) -> StopReason { - use crate::mips_exec::EXEC_BREAKPOINT; + use crate::mips_exec::{EXEC_BREAKPOINT, EXEC_RETRY}; self.cpu.stop(); // ensure no thread is running let mut exec = self.cpu.executor.lock(); exec.last_bp_hit = None; + // EXEC_RETRY means the instruction did not retire and PC is unmoved, so + // the same PC must be stepped again — bounded like validate.rs's + // MAX_RETRIES_PER_INSTRUCTION so a stuck device can't spin here. + const MAX_STEP_RETRIES: u32 = 100_000; + // Execute one instruction. If it's a branch, also execute the delay // slot, so a single GDB-visible step lands past it rather than // stopping mid-delay-slot. `core.in_delay_slot` (set by @@ -10776,7 +10826,27 @@ impl CpuDebug // yet — status codes carry no PC-related information of their own // (every handler sets core.pc itself before returning). //eprintln!("GDB: step_one: PC={:#018x}", exec.core.pc); - let status = exec.step(); + let mut status = exec.step(); + let mut retries = 0u32; + while status == EXEC_RETRY && retries < MAX_STEP_RETRIES { + retries += 1; + std::hint::spin_loop(); + status = exec.step(); + } + if status == EXEC_RETRY { + // Still busy after the cap: a device is stuck, not transiently + // busy. Report a stop rather than a phantom "step completed". + drop(exec); + self.stop_state.set(StopReason::Interrupted); + return StopReason::Interrupted; + } + if status == EXEC_BREAKPOINT { + // Sitting on an active breakpoint: step() matches before it + // dispatches, so without a one-shot skip every later single-step + // re-detects it (as run_debug_loop's first_step does for `cont`). + exec.skip_breakpoints = true; + status = exec.step(); + } //eprintln!("GDB: step_one: after step status={:#010x} PC={:#018x}", status, exec.core.pc); let reason = if status == EXEC_BREAKPOINT { drop(exec);