From b2311a043a9ea2171c94c88becc73048988161a4 Mon Sep 17 00:00:00 2001 From: Milan Malich Date: Thu, 20 Aug 2026 10:54:34 +0200 Subject: [PATCH 1/6] mips_exec: stop breakpoints from aliasing adjacent instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check_breakpoint() compared both addresses masked with PHYS_MASK = 0x1FFF_FFF8, which clears bit 2 — the comment above it says "mask bottom 2 for word alignment", i.e. & !3, but the constant is & !7. Two consequences: 1. Two adjacent instructions in the same doubleword compare equal. 0x1004c758 and 0x1004c75c both mask to 0x0004c758. GDB single-steps MIPS by planting a breakpoint at PC+4 and continuing (its MIPS backend always uses software single-step — no s/vCont;s packet is ever sent), so that breakpoint matched at PC itself and execution broke *before* the instruction ran. The PC never advanced: single-stepping a guest program silently did nothing, forever, with no error reported anywhere. 2. Only the low 29 bits survived, so a breakpoint also matched any address sharing them — a different segment, or the same virtual address in an unrelated process. Replace the flat mask with bp_match_key(): word-align, and fold *only* the unmapped 32-bit compatibility windows (kseg0 0x8000_0000..0x9FFF_FFFF and kseg1 0xA000_0000..0xBFFF_FFFF, in either sign-extended or plain form) onto the physical address they alias. That keeps the documented and intended behaviour — a bp on physical 0x1fbb0010 is hit through 0x9fbb0010 or 0xbfbb0010 — while every other address keeps all of its bits. Reproducible from the monitor console alone, no GDB needed: > status stopped pc=000000001004c758 > bp add 0x1004c75c Breakpoint 1 added at 000000001004c75c (Pc) > cont PC=000000001004c758: Breakpoint 1 hit <- wrong address > status stopped pc=000000001004c758 <- nothing executed After the fix, with PC at 0xffffffff8812a124 and a breakpoint at 0xffffffff8812a128, `cont` reports the hit at 0xffffffff8812a128 and status shows the PC advanced by 4. Co-Authored-By: Claude Opus 5 (1M context) --- src/mips_exec.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/mips_exec.rs b/src/mips_exec.rs index 93d1238..235710e 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. @@ -2365,12 +2380,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(); From f02027fab09f1147fa2c31fc5e483b36ce369111 Mon Sep 17 00:00:00 2001 From: Milan Malich Date: Thu, 20 Aug 2026 10:55:44 +0200 Subject: [PATCH 2/6] mips_exec: retry EXEC_RETRY in both debugger paths instead of stopping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EXEC_RETRY (bus busy) means the instruction did *not* retire: nothing changed architecturally and PC is unmoved, so the caller must attempt the same PC again. validate.rs's reference pass (~lines 204-233) already documents exactly that contract, and caps the attempts at MAX_RETRIES_PER_INSTRUCTION = 100_000 so a permanently-busy device can't spin forever holding the executor lock. Neither debugger path honoured it: - run_debug_loop() (monitor run/step, and the GDB stub's continue via run_blocking()) broke out of the loop and printed "Retry (Bus Busy)". For a GDB client that turns every transient GIO bus-busy — frequent during ordinary desktop/graphics activity, logged as "MC: GIO Timeout" — into a stop with PC unchanged. Because the PC is still sitting on the user's breakpoint, GDB re-announces "Breakpoint N, main (...) at file.c:LINE" over and over while the program makes no progress. - step_one() silently ignored EXEC_RETRY and reported a completed step, so a single-step became a no-op with nothing surfaced to the client. Both now retry, bounded: - step_one(): retry while status == EXEC_RETRY up to MAX_STEP_RETRIES, with a spin_loop() hint; if still busy after the cap, report a real stop (StopReason::Interrupted) rather than a phantom "step completed". - run_debug_loop(): continue the loop instead of breaking, bounded by MAX_RUN_RETRIES per instruction attempt (the counter is reset whenever something retires, so a long run can't accumulate its way into a false give-up), refunding the `count` budget so `step N` still retires N architectural instructions. The original diagnostic is kept for the genuinely-stuck case, now with the attempt count. The bounded spin in step_one() holds cpu::executor, but the device that answered "busy" runs on its own thread and does not need that lock to make progress, so it cannot deadlock against it. Verified: an 18-second GDB `continue` produced no SIGTRAP and no breakpoint re-announcements, with the monitor console independently confirming the CPU ran throughout. Before, the same operation stopped almost immediately and repeatedly. Co-Authored-By: Claude Opus 5 (1M context) --- src/mips_exec.rs | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/mips_exec.rs b/src/mips_exec.rs index 235710e..f2423a4 100644 --- a/src/mips_exec.rs +++ b/src/mips_exec.rs @@ -7831,6 +7831,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) { @@ -7914,8 +7918,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; @@ -7946,6 +7961,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; @@ -10776,11 +10795,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 @@ -10789,7 +10813,20 @@ 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; + } //eprintln!("GDB: step_one: after step status={:#010x} PC={:#018x}", status, exec.core.pc); let reason = if status == EXEC_BREAKPOINT { drop(exec); From e3d3dacd5acddd5a2147337411133541fb51f660 Mon Sep 17 00:00:00 2001 From: Milan Malich Date: Thu, 20 Aug 2026 10:56:05 +0200 Subject: [PATCH 3/6] mips_exec: clear stale last_bp_hit at the start of run_debug_loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A GDB client issuing `continue` regularly got Program received signal SIGTRAP, Trace/breakpoint trap. 0x00000000103f8fa4 in ?? () repeatedly, at slowly-incrementing addresses, with no resolvable symbol and no breakpoint anywhere near them. last_bp_hit was only ever cleared in step_one(). run_debug_loop() never reset it, and run_blocking() inspects it after the loop to decide what to report to the client. So any loop exit for a reason other than a real breakpoint — EXEC_RETRY, an exception_mask match, an interrupt — inherited whatever id was left from an earlier stop (including the initial stopAtConnect pause) and was misclassified as StopReason::SwBreakpoint. StopReason::to_gdb() maps that to SwBreak(()), which GDB prints as SIGTRAP/Trace-breakpoint-trap; with no real breakpoint at the reported PC there is no symbol to resolve, hence "?? ()". Clear it once at the start of the task closure so a non-breakpoint exit falls through to StopReason::DoneStep. Co-Authored-By: Claude Opus 5 (1M context) --- src/mips_exec.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mips_exec.rs b/src/mips_exec.rs index f2423a4..b6f2477 100644 --- a/src/mips_exec.rs +++ b/src/mips_exec.rs @@ -7810,6 +7810,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 From d849792b6ddbf3877f88d583921dc224e4a988bb Mon Sep 17 00:00:00 2001 From: Milan Malich Date: Thu, 20 Aug 2026 10:56:59 +0200 Subject: [PATCH 4/6] gdb_stub: return an RSP error for unreadable memory instead of zeros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_addrs() answered a failed read with all zeros and an OK reply. The monitor console, for the very same address, correctly refuses it: > status stopped pc=000000001004c758 > dis 0x1004c758 4 0x000000001004c758: Could not fetch (x4) > translate 0x1004c758 Exception(0x38000008) (exc code 2 = TLBL) but over RSP: (gdb) x/4xw 0x1004c758 0x1004c758: 0x00000000 0x00000000 0x00000000 0x00000000 The real instructions there are `lw t9,-26820(gp)` / `jalr t9` / `move a0,zero`. 0x00000000 decodes as a perfectly valid `nop`, and GDB's MIPS backend always software-single-steps (read the instruction at PC, compute the next PC, plant a breakpoint there, continue — the s/vCont;s packet is never sent), so it planned the step from that fiction: wrong next PC, step breakpoint at an address the real code never reaches, and stepping quietly stopped working with no diagnostic in GDB, VS Code or the emulator log. Zeros are also indistinguishable from genuinely zeroed memory, so inspecting a data structure on a non-resident page yields plausible-looking garbage rather than a fault. Propagate the failure as TargetError::NonFatal (an "E xx" reply) so GDB says "Cannot access memory at address 0x..." and declines the step. This does not make the memory readable — a debug-path fallback to walking IRIX's page tables on a TLB miss would, and is a separate change — but it turns a silent wrong answer into an obvious one. Co-Authored-By: Claude Opus 5 (1M context) --- src/gdb_stub.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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()) } From 4fb5bf47a111cd09b48339fb2306ecfc800c80c3 Mon Sep 17 00:00:00 2001 From: Milan Malich Date: Thu, 20 Aug 2026 10:57:19 +0200 Subject: [PATCH 5/6] mips_exec: defer a PC breakpoint until the instruction is fetchable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit step() tests check_breakpoint::(pc) *before* the fetch. MIPS TLB refill is software-managed, so a userland PC whose page isn't resident right now is unreadable — and a breakpoint at such an address therefore fires before the demand-paging fault that would map the page. That is the normal case, not a corner case: observed with K:U, EXL=0, EPC=0x1004c758 and no pending interrupts, i.e. the kernel had just eret'd back into user code whose TLB entries had been recycled while other processes ran, and the stop was taken on the very first, not-yet-faulted instruction. Reporting a stop there strands the debugger on code it cannot read. GDB's MIPS backend always software-single-steps (read the instruction at PC, compute the next PC, plant a breakpoint there, continue), so every step becomes a no-op and the user sees the debugger stuck on one source line with no error anywhere. Probe with debug_translate() — the non-faulting path, so the guest isn't perturbed — and only report the breakpoint if the instruction can actually be fetched. Otherwise let execution proceed: the fetch takes the TLB exception, the kernel maps the page and eret's back to this same PC, and the breakpoint matches again with the code now readable. That is also the more accurate architectural semantics, since an instruction whose fetch faults never executed. The last_bp_hit reset on the not-taken path is required, or the id check_breakpoint() just recorded would make the *next* stop get misreported as this breakpoint. Observed at the address above: translate went from Exception(0x38000008) (TLBL) to Translated { phys_addr: 0x170ce758 } by the time the stop was reported, and `dis` showed real instructions instead of "Could not fetch". Co-Authored-By: Claude Opus 5 (1M context) --- src/mips_exec.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/mips_exec.rs b/src/mips_exec.rs index b6f2477..eac4d24 100644 --- a/src/mips_exec.rs +++ b/src/mips_exec.rs @@ -2199,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 From f1444225fa4a8c15d7ebfccf4226e002348eaeac Mon Sep 17 00:00:00 2001 From: Milan Malich Date: Thu, 20 Aug 2026 10:57:52 +0200 Subject: [PATCH 6/6] mips_exec: mirror run_debug_loop's step-off-breakpoint skip into step_one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit step() checks for a breakpoint match before dispatching the instruction, so when the CPU sits exactly on an active breakpoint's address — as it always does right after that breakpoint was hit — step_one() returned StopReason::SwBreakpoint immediately and the instruction was never executed. Every subsequent single-step would re-detect the same breakpoint. run_debug_loop() already handles this with a one-shot skip_breakpoints before retrying exec.step() on its first iteration; step_one() was simply missing the equivalent. step() clears the flag itself after every call (it is a genuine one-shot), so no manual reset is needed. Note this is a parity fix with no reproduced user-visible failure: measurement showed stepi does advance the PC even while stopped on a breakpoint, and since GDB's MIPS backend always software-single-steps (breakpoint + continue, never the s/vCont;s packet), step_one() may not be reachable from a GDB client at all. The inconsistency between the two execution paths is real; its impact is unconfirmed. Drop this commit if you'd rather not carry an unproven change. Co-Authored-By: Claude Opus 5 (1M context) --- src/mips_exec.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/mips_exec.rs b/src/mips_exec.rs index eac4d24..8d5e60e 100644 --- a/src/mips_exec.rs +++ b/src/mips_exec.rs @@ -10840,6 +10840,13 @@ impl CpuDebug 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);