On 16.03.21 11:59, Maxim Levitsky wrote: > On Tue, 2021-03-16 at 10:16 +0100, Jan Kiszka wrote: >> On 16.03.21 00:37, Sean Christopherson wrote: >>> On Tue, Mar 16, 2021, Maxim Levitsky wrote: >>>> This change greatly helps with two issues: >>>> >>>> * Resuming from a breakpoint is much more reliable. >>>> >>>> When resuming execution from a breakpoint, with interrupts enabled, more often >>>> than not, KVM would inject an interrupt and make the CPU jump immediately to >>>> the interrupt handler and eventually return to the breakpoint, to trigger it >>>> again. >>>> >>>> From the user point of view it looks like the CPU never executed a >>>> single instruction and in some cases that can even prevent forward progress, >>>> for example, when the breakpoint is placed by an automated script >>>> (e.g lx-symbols), which does something in response to the breakpoint and then >>>> continues the guest automatically. >>>> If the script execution takes enough time for another interrupt to arrive, >>>> the guest will be stuck on the same breakpoint RIP forever. >>>> >>>> * Normal single stepping is much more predictable, since it won't land the >>>> debugger into an interrupt handler, so it is much more usable. >>>> >>>> (If entry to an interrupt handler is desired, the user can still place a >>>> breakpoint at it and resume the guest, which won't activate this workaround >>>> and let the gdb still stop at the interrupt handler) >>>> >>>> Since this change is only active when guest is debugged, it won't affect >>>> KVM running normal 'production' VMs. >>>> >>>> >>>> Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx> >>>> Tested-by: Stefano Garzarella <sgarzare@xxxxxxxxxx> >>>> --- >>>> arch/x86/kvm/x86.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>>> index a9d95f90a0487..b75d990fcf12b 100644 >>>> --- a/arch/x86/kvm/x86.c >>>> +++ b/arch/x86/kvm/x86.c >>>> @@ -8458,6 +8458,12 @@ static void inject_pending_event(struct kvm_vcpu *vcpu, bool *req_immediate_exit >>>> can_inject = false; >>>> } >>>> >>>> + /* >>>> + * Don't inject interrupts while single stepping to make guest debug easier >>>> + */ >>>> + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) >>>> + return; >>> >>> Is this something userspace can deal with? E.g. disable IRQs and/or set NMI >>> blocking at the start of single-stepping, unwind at the end? Deviating this far >>> from architectural behavior will end in tears at some point. >>> >> >> Does this happen to address this suspicious workaround in the kernel? >> >> /* >> * The kernel doesn't use TF single-step outside of: >> * >> * - Kprobes, consumed through kprobe_debug_handler() >> * - KGDB, consumed through notify_debug() >> * >> * So if we get here with DR_STEP set, something is wonky. >> * >> * A known way to trigger this is through QEMU's GDB stub, >> * which leaks #DB into the guest and causes IST recursion. >> */ >> if (WARN_ON_ONCE(dr6 & DR_STEP)) >> regs->flags &= ~X86_EFLAGS_TF; >> >> (arch/x86/kernel/traps.c, exc_debug_kernel) >> >> I wonder why this got merged while no one fixed QEMU/KVM, for years? Oh, >> yeah, question to myself as well, dancing around broken guest debugging >> for a long time while trying to fix other issues... > > To be honest I didn't see that warning even once, but I can imagine KVM > leaking #DB due to bugs in that code. That area historically didn't receive > much attention since it can only be triggered by > KVM_GET/SET_GUEST_DEBUG which isn't used in production. I've triggered it recently while debugging a guest, that's why I got aware of the code path. Long ago, all this used to work (soft BPs, single-stepping etc.) > > The only issue that I on the other hand did > see which is mostly gdb fault is that it fails to remove a software breakpoint > when resuming over it, if that breakpoint's python handler messes up > with gdb's symbols, which is what lx-symbols does. > > And that despite the fact that lx-symbol doesn't mess with the object > (that is the kernel) where the breakpoint is defined. > > Just adding/removing one symbol file is enough to trigger this issue. > > Since lx-symbols already works this around when it reloads all symbols, > I extended that workaround to happen also when loading/unloading > only a single symbol file. You have no issue with interactive debugging when NOT using gdb scripts / lx-symbol? Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
