Kernel/aarch64+LibDebug: Add basic debugger functions support #25975
Conversation
github-actions
bot
added
the
👀 pr-needs-review
There was a problem hiding this comment.
Thanks for adding basic aarch64 debugging support!
LibDebug still seems to peek/poke debug registers and expect x86 values to be there. But I guess it's fine for now. Just returning ENOTUSP in {peek,poke}_debug seems to be good enough to make this work. Although it's a bit hacky.
Kernel/Syscalls/ptrace.cpp
Outdated
| // to PSTATE.SS. To enable single stepping, the MDSCR_EL1.SS must also be set to 1. | ||
| constexpr u32 SS_FLAG = 0x200000; | ||
| regs.spsr_el1 |= SS_FLAG; | ||
| peer->debug_register_state().mdscr_el1 |= 0x1; |
There was a problem hiding this comment.
We set mdscr_el1.ss to 1 after the first PT_SINGLESTEP and never set it to zero again.
Should we clear it somewhere again?
There was a problem hiding this comment.
I cleared it in exception_common(). Originally, I cleared current_thread->debug_register_state().mdscr_el1, but now we probably need to use clear_debug_registers() to clear the hardware register values directly.
There was a problem hiding this comment.
Oh clearing it in exception_common seems fine. Sorry I missed that as well.
There was a problem hiding this comment.
But you should directly write to the actual register, not our debug register state struct. That struct is only used to save/restore the debug register contents during context switches.
There was a problem hiding this comment.
This approach also works: calling clear_debug_registers() in exception_common() to clear mdscr_el1.SS, and using ptrace to first read and then write DebugRegisterState to set mdscr_el1.SS.
But I’d like to ask why is this restriction necessary? I thought DebugRegisterState was similar to RegisterState, which can be modified inside the kernel and then applied to userspace.
There was a problem hiding this comment.
DebugRegisterState is more like FPUState. FPUState is also only saved/restored on a context switch.
We should avoid saving/restoring registers if it isn't necessary and instead do it more lazily. We don't need FPU or debug registers in the kernel, so we don't need to save the userspace values on each trap entry.
There was a problem hiding this comment.
Oh, Sorry for missing something. When the debugger invokes PT_SINGLESTEP via a syscall, we write directly to MDSCR_EL1. After returning to userspace, if the CPU is no longer running the debuggee (which is very likely—it will be running the debugger itself), then you end up with MDSCR_EL1.SS = 1 while SPSR_EL1.SS = 0. In that situation, a single‐step exception fires immediately and kills the current process with SIGTRAP. Therefore, I think in this case we still need to use DebugRegisterState so that the CPU can set MDSCR_EL1.SS properly.
There was a problem hiding this comment.
Yeah that's true. You need to set MDSCR_EL1.SS via accessing the debug register state of the debugee. Sorry for causing confusion.
NoobZang
force-pushed
the
aarch64-sdb
branch
2 times, most recently
from
0b5cd11 to
aca03be
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
`ptrace` need to be able to set the SS bit in the spsr_el1 register to control the software step exception. When copying user-supplied PtraceRegisters' spsr_el1 into the kernel’s RegisterState, only the ARMv8 condition flags (N, Z, C, V) should be accepted. Other fields in spsr_el1 must remain under kernel control.
The least significant bit of mdscr_el1 (the SS bit) is used to enable or disable software step.
Add a PT_SINGLESTEP request to the ptrace API and implement its handing logic in the kernel. Update the debugger to call ptrace(PT_SINGLESTEP) instead of directly poking registers. Ensure MDSCR_EL1.SS is properly cleared and then reloaded when a software step occurs.
spholz
added
✅ pr-maintainer-approved-but-awaiting-ci
github-actions
bot
removed
the
✅ pr-maintainer-approved-but-awaiting-ci
This patch adds the following basic debugger capabilities for AArch64: