On 21/04/2022 14:02, Jan Beulich wrote: > On 20.04.2022 16:13, Andrew Cooper wrote: >> From: Bobby Eshleman <bobby.eshle...@gmail.com> >> >> debugger_trap_entry() is unrelated to the other contents of debugger.h. It >> is >> a no-op for everything other than #DB/#BP, and for those it invokes guest >> debugging (CONFIG_GDBSX) not host debugging (CONFIG_CRASH_DEBUG). >> >> Furthermore, the description of how to use debugger_trap_entry() is at best, >> stale. It is not called from all exception paths, > But on almost all (before this change) - the exception looks to be > #NM. > >> and because the developer >> is forced to modify Xen to perform debugging, editing debugger_trap_entry() >> is >> not the way one would efficiently go about diagnosing the problem. > Shouldn't it be the remote end to request which exceptions it wants > to be notified of? If so, removing the hook invocation isn't very > helpful.
That's not part of the gdb remote protocol. In normal conditions, gdb gets to see see anything which manifests as a signal. It does not get to see anything which is resolved by the kernel behind the scenes. #NM you've already identified, and most #PF's would count too. Back in the 32bit days, Xen-induced #GP/#SS's for non-4G segments would count too. But in addition to filtering Xen's idea of "fixing up behind the scenes", you also need to Xen to understand when to skip notifications based on what a PV guest kernel can fix up, and this is getting even further out of gdb's comfort zone. debugger_trap_entry() is empty (WRT gdbstub) specifically because it's description is nonsense in any practical debugging scenario. And lets not start on the fact that the lack of ability to invoke pv_event_inject() means that any fault from userspace will livelock under debugging. Deleting it is absolutely the right way forward, because a theoretical future with someone wiring this up would have to start again from scratch. Not that qualifies as a good reason in isolation, do_trap() contains unreachable logic because the compiler can't figure out that #DB/#BP are handled via alternative paths, and the gdbsx logic is dead. ~Andrew
