diff --git a/kernel/src/arch/x86_64/gdb/mod.rs b/kernel/src/arch/x86_64/gdb/mod.rs index 5b6745291..075684e48 100644 --- a/kernel/src/arch/x86_64/gdb/mod.rs +++ b/kernel/src/arch/x86_64/gdb/mod.rs @@ -22,6 +22,7 @@ use super::debug::GDB_REMOTE_PORT; use crate::error::KError; mod breakpoints; +mod multi_thread_ops; mod section_offsets; mod serial; mod single_register; @@ -99,7 +100,6 @@ pub(crate) fn event_loop(reason: KCoreStopReason) -> Result<(), KError> { gdb_stm = match gdb_stm { GdbStubStateMachine::Idle(mut gdb_stm_inner) => { //trace!("GdbStubStateMachine::Idle"); - // This means we expect stuff on the serial line (from GDB) // Let's read and react to it: let conn = gdb_stm_inner.borrow_conn(); @@ -119,7 +119,18 @@ pub(crate) fn event_loop(reason: KCoreStopReason) -> Result<(), KError> { } } GdbStubStateMachine::CtrlCInterrupt(gdb_stm_inner) => { - match gdb_stm_inner.interrupt_handled(&mut target, Some(ThreadStopReason::DoneStep)) + trace!("GdbStubStateMachine::CtrlCInterrupt"); + let _reason = stop_reason.take(); + // Ideally, this would hold: assert_eq!(reason, + // Some(ThreadStopReason::Signal(Signal::SIGINT))); + // + // But, turns out `reason` can be `StepDone` too if we are + // single-stepping through things and the ctrl+c packet arrives + // while we're in this file. + // + // So we just consume `reason` and report SIGINT unconditionally. + match gdb_stm_inner + .interrupt_handled(&mut target, Some(ThreadStopReason::Signal(Signal::SIGINT))) { Ok(gdb) => gdb, Err(e) => { @@ -129,11 +140,11 @@ pub(crate) fn event_loop(reason: KCoreStopReason) -> Result<(), KError> { } } GdbStubStateMachine::Disconnected(_gdb_stm_inner) => { - error!("GdbStubStateMachine::Disconnected byebye"); + error!("GdbStubStateMachine::Disconnected"); break; } GdbStubStateMachine::Running(mut gdb_stm_inner) => { - //trace!("GdbStubStateMachine::DeferredStopReason"); + trace!("GdbStubStateMachine::Running"); // If we're here we were running but have stopped now (either // because we hit Ctrl+c in gdb and hence got a serial interrupt @@ -143,6 +154,8 @@ pub(crate) fn event_loop(reason: KCoreStopReason) -> Result<(), KError> { let data_to_read = conn.peek().unwrap().is_some(); if data_to_read { + trace!("GdbStubStateMachine::Running data_to_read"); + let byte = gdb_stm_inner.borrow_conn().read().unwrap(); match gdb_stm_inner.incoming_data(&mut target, byte) { Ok(pumped_stm) => pumped_stm, @@ -156,6 +169,8 @@ pub(crate) fn event_loop(reason: KCoreStopReason) -> Result<(), KError> { } } } else if let Some(reason) = stop_reason.take() { + trace!("GdbStubStateMachine::Running stop_reason"); + match gdb_stm_inner.report_stop(&mut target, reason) { Ok(gdb_stm_new) => gdb_stm_new, Err(GdbStubError::TargetError(e)) => { @@ -168,6 +183,8 @@ pub(crate) fn event_loop(reason: KCoreStopReason) -> Result<(), KError> { } } } else if target.resume_with.is_some() { + trace!("GdbStubStateMachine::Running target.resume_with.is_some"); + // We don't have a `stop_reason` and we don't have something // to read on the line. This probably means we're done and // we should run again. @@ -245,7 +262,7 @@ impl KernelDebugger { // Also does some additional stuff like re-enabling the breakpoints. fn determine_stop_reason(&mut self, reason: KCoreStopReason) -> Option> { match reason { - KCoreStopReason::ConnectionInterrupt => Some(ThreadStopReason::Signal(Signal::SIGTRAP)), + KCoreStopReason::ConnectionInterrupt => Some(ThreadStopReason::Signal(Signal::SIGINT)), KCoreStopReason::BreakpointInterrupt => { unimplemented!("Breakpoint interrupt not implemented"); //Some(ThreadStopReason::SwBreak(NonZeroUsize::new(1).unwrap())) @@ -338,7 +355,11 @@ impl Target for KernelDebugger { type Arch = X86_64_SSE; fn base_ops(&mut self) -> BaseOps { - BaseOps::SingleThread(self) + if cfg!(feature = "bsp-only") { + BaseOps::SingleThread(self) + } else { + BaseOps::MultiThread(self) + } } fn support_section_offsets(&mut self) -> Option> { diff --git a/kernel/src/arch/x86_64/gdb/multi_thread_ops.rs b/kernel/src/arch/x86_64/gdb/multi_thread_ops.rs new file mode 100644 index 000000000..c7bcb417c --- /dev/null +++ b/kernel/src/arch/x86_64/gdb/multi_thread_ops.rs @@ -0,0 +1,163 @@ +#![allow(warnings)] + +use core::convert::TryInto; + +use log::{debug, error, info, trace, warn}; + +use gdbstub::common::{Signal, Tid}; +use gdbstub::target; +use gdbstub::target::ext::base::multithread::{MultiThreadOps, MultiThreadSingleStep}; +use gdbstub::target::ext::breakpoints::WatchKind; +use gdbstub::target::{Target, TargetError, TargetResult}; + +use super::{ExecMode, KernelDebugger}; +use crate::error::KError; +use crate::memory::vspace::AddressSpace; +use crate::memory::{VAddr, BASE_PAGE_SIZE}; + +impl MultiThreadOps for KernelDebugger { + fn resume(&mut self) -> Result<(), Self::Error> { + // Upon returning from the `resume` method, the target being debugged should be + // configured to run according to whatever resume actions the GDB client has + // specified (as specified by `set_resume_action`, `set_resume_range_step`, + // `set_reverse_{step, continue}`, etc...) + // + // In this basic `armv4t_multicore` example, the `resume` method is actually a + // no-op, as the execution mode of the emulator's interpreter loop has already + // been modified via the various `set_X` methods. + // + // In more complex implementations, it's likely that the target being debugged + // will be running in another thread / process, and will require some kind of + // external "orchestration" to set it's execution mode (e.g: modifying the + // target's process state via platform specific debugging syscalls). + + Ok(()) + } + + fn clear_resume_actions(&mut self) -> Result<(), Self::Error> { + //self.exec_mode.clear(); + Ok(()) + } + + #[inline(always)] + fn support_single_step( + &mut self, + ) -> Option> { + Some(self) + } + + fn set_resume_action_continue( + &mut self, + tid: Tid, + signal: Option, + ) -> Result<(), Self::Error> { + /*if signal.is_some() { + return Err(KError::Unknown); + } + + self.exec_mode + .insert(tid_to_cpuid(tid)?, ExecMode::Continue);*/ + + Ok(()) + } + + fn read_registers( + &mut self, + regs: &mut gdbstub_arch::x86::reg::X86_64CoreRegs, + tid: Tid, + ) -> TargetResult<(), Self> { + /* + let cpu = match tid_to_cpuid(tid).map_err(TargetError::Fatal)? { + CpuId::Cpu => &mut self.cpu, + CpuId::Cop => &mut self.cop, + }; + + let mode = cpu.mode(); + + for i in 0..13 { + regs.r[i] = cpu.reg_get(mode, i as u8); + } + regs.sp = cpu.reg_get(mode, reg::SP); + regs.lr = cpu.reg_get(mode, reg::LR); + regs.pc = cpu.reg_get(mode, reg::PC); + regs.cpsr = cpu.reg_get(mode, reg::CPSR);*/ + + Ok(()) + } + + fn write_registers( + &mut self, + regs: &gdbstub_arch::x86::reg::X86_64CoreRegs, + tid: Tid, + ) -> TargetResult<(), Self> { + /*let cpu = match tid_to_cpuid(tid).map_err(TargetError::Fatal)? { + CpuId::Cpu => &mut self.cpu, + CpuId::Cop => &mut self.cop, + }; + + let mode = cpu.mode(); + + for i in 0..13 { + cpu.reg_set(mode, i, regs.r[i as usize]); + } + cpu.reg_set(mode, reg::SP, regs.sp); + cpu.reg_set(mode, reg::LR, regs.lr); + cpu.reg_set(mode, reg::PC, regs.pc); + cpu.reg_set(mode, reg::CPSR, regs.cpsr); + */ + Ok(()) + } + + fn read_addrs( + &mut self, + start_addr: u64, + data: &mut [u8], + _tid: Tid, // same address space for each core + ) -> TargetResult<(), Self> { + /*for (addr, val) in (start_addr..).zip(data.iter_mut()) { + *val = self.mem.r8(addr) + }*/ + Ok(()) + } + + fn write_addrs( + &mut self, + start_addr: u64, + data: &[u8], + _tid: Tid, // same address space for each core + ) -> TargetResult<(), Self> { + /*for (addr, val) in (start_addr..).zip(data.iter().copied()) { + self.mem.w8(addr, val) + }*/ + Ok(()) + } + + fn list_active_threads( + &mut self, + register_thread: &mut dyn FnMut(Tid), + ) -> Result<(), Self::Error> { + //register_thread(cpuid_to_tid(CpuId::Cpu)); + //register_thread(cpuid_to_tid(CpuId::Cop)); + Ok(()) + } +} + +/// Adds `gdbstub` support for single-stepping. +impl MultiThreadSingleStep for KernelDebugger { + fn set_resume_action_step( + &mut self, + _tid: Tid, + signal: Option, + ) -> Result<(), Self::Error> { + assert!(signal.is_none(), "Not supported at the moment."); + + self._signal = signal; + self.resume_with = Some(ExecMode::SingleStep); + info!( + "SingleThreadSingleStep::step: set signal = {:?} resume_with = {:?}", + signal, self.resume_with + ); + + Ok(()) + } +} diff --git a/kernel/src/arch/x86_64/gdb/single_thread_ops.rs b/kernel/src/arch/x86_64/gdb/single_thread_ops.rs index 4bf5aa1dc..cbe5877b4 100644 --- a/kernel/src/arch/x86_64/gdb/single_thread_ops.rs +++ b/kernel/src/arch/x86_64/gdb/single_thread_ops.rs @@ -24,10 +24,14 @@ impl SingleThreadOps for KernelDebugger { self._signal = signal; self.resume_with = Some(ExecMode::Continue); trace!( - "resume: signal = {:?} resume_with = {:?}", + "SingleThreadOps::resume: signal = {:?} resume_with = {:?}", signal, self.resume_with ); + info!( + "SingleThreadOps::resume: signal = {:?} resume_with = {:?}", + signal, self.resume_with + ); // If the target is running under the more advanced GdbStubStateMachine // API, it is possible to "defer" reporting a stop reason to some point @@ -39,6 +43,7 @@ impl SingleThreadOps for KernelDebugger { &mut self, regs: &mut gdbstub_arch::x86::reg::X86_64CoreRegs, ) -> TargetResult<(), Self> { + trace!("read_registers"); let kcb = super::super::kcb::get_kcb(); if let Some(saved) = &kcb.save_area { // RAX, RBX, RCX, RDX, RSI, RDI, RBP, RSP, r8-r15 @@ -269,13 +274,11 @@ impl SingleThreadOps for KernelDebugger { } fn support_single_register_access(&mut self) -> Option> { - //Some(self) - None + Some(self) } fn support_single_step(&mut self) -> Option> { Some(self) - //None } } @@ -287,7 +290,7 @@ impl SingleThreadSingleStep for KernelDebugger { self._signal = signal; self.resume_with = Some(ExecMode::SingleStep); info!( - "set signal = {:?} resume_with = {:?}", + "SingleThreadSingleStep::step: set signal = {:?} resume_with = {:?}", signal, self.resume_with ); diff --git a/kernel/src/integration_tests.rs b/kernel/src/integration_tests.rs index 49eb59ffa..52fedfc42 100644 --- a/kernel/src/integration_tests.rs +++ b/kernel/src/integration_tests.rs @@ -579,23 +579,25 @@ fn vspace_debug() { fn gdb() { use log::info; - //arch::irq::ioapic_establish_route(0x0, 0x0); - // watchpoint test: let mut watchpoint_trigger: usize = 0; info!("watchpoint_trigger is {}", watchpoint_trigger); watchpoint_trigger = 0xdeadbeef; info!("watchpoint_trigger is {}", watchpoint_trigger); - // step through all of info: + // step through all of info: info!("step"); info!("step"); - //arch::irq::enable(); - //let mut cond = true; - //while cond {} - //cond = false; - //info!("cond is {}", cond); + // Test ctrl+c + crate::arch::irq::ioapic_establish_route(0x0, 0x0); // TODO(api): enables serial irq + crate::arch::irq::enable(); + let mut cond = true; + while cond {} + cond = false; // make sure cond is mut + // (I figured having cond mut is a little less shady + // if we overwrite it with gdb) + info!("cond is {}", cond); // continue until exit: shutdown(ExitReason::Ok); diff --git a/kernel/tests/integration-test.rs b/kernel/tests/integration-test.rs index a2979899f..eae86cc2b 100644 --- a/kernel/tests/integration-test.rs +++ b/kernel/tests/integration-test.rs @@ -1246,7 +1246,7 @@ fn s02_gdb() { /// Spawn the gdb debugger pub fn spawn_gdb(binary: &str) -> Result { // The `-nx` ignores any potential .gdbinit files that may mess with the test - spawn(format!("gdb -nx {}", binary).as_str(), Some(3_000)).and_then(|p| { + spawn(format!("gdb -nx {}", binary).as_str(), Some(5_000)).and_then(|p| { Ok(PtyReplSession { prompt: "(gdb) ".to_string(), pty_session: p, @@ -1327,6 +1327,22 @@ fn s02_gdb() { // Test `continue` output += gdb.wait_for_prompt()?.as_str(); gdb.send_line("continue")?; + // We're in the "infinite" loop now, we won't get a prompt... + std::thread::sleep(std::time::Duration::from_millis(300)); + + // Interrupt execution with a Ctrl+C + gdb.send_control('c')?; + output += gdb + .exp_string("Program received signal SIGINT, Interrupt.")? + .as_str(); + output += gdb.wait_for_prompt()?.as_str(); + + // Get out of the loop by overwriting var (test mem/register write) + gdb.send_line("set cond = false")?; + output += gdb.wait_for_prompt()?.as_str(); + + // Should go to the end now... + gdb.send_line("continue")?; output += gdb.exp_string("Remote connection closed")?.as_str(); output += p.exp_eof()?.as_str();