On 24.08.2021 14:41, Andrew Cooper wrote: > On 24/08/2021 13:16, Jan Beulich wrote: >> On 18.08.2021 22:29, Bobby Eshleman wrote: >>> Unlike debugger_trap_fatal() and debugger_trap_immediate(), >>> debugger_trap_entry() is specific to guest debugging and *NOT* the >>> debugging of Xen itself. That is, it is part of gdbsx functionality and >>> not the Xen gdstub. This is evidenced by debugger_trap_fatal()'s usage >>> of domain_pause_for_debugger(). Because of this, debugger_trap_entry() >>> does not belong alongside the generic Xen debugger functionality. >> I'm not convinced this is what the original intentions were. Instead I >> think this was meant to be a generic hook function which initially >> only cared to deal with the gdbsx needs. > > It doesn't exactly matter what the original intentions where - what we > currently have is unused and and a clear source of confusion between two > unrelated subsystems. > > It is unclear that the gdbstub is even usable, given at least a decade > of bitrot. > > Keeping an empty static inline in the enabled case is nonsense, because > at the point you need to edit Xen to insert some real debugging, there > are better ways to do it in something which isn't even a catch-all > despite appearing to be one.
Perhaps I should have said explicitly that my remark isn't an objection to the code change, but a suggestion to weaken the claims made in the description. Jan
