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. 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. Best regards, Maxim Levitsky > > Jan > > > > + > > > /* > > > * Finally, inject interrupt events. If an event cannot be injected > > > * due to architectural conditions (e.g. IF=0) a window-open exit > > > -- > > > 2.26.2 > > >
