|
From: | Stefano Garzarella |
Subject: | Re: [PATCH] KVM: x86: do not fail if software breakpoint has already been removed |
Date: | Tue, 2 Mar 2021 15:52:11 +0100 |
On Mon, Mar 01, 2021 at 02:56:40PM +0200, Maxim Levitsky wrote:
On Mon, 2021-03-01 at 12:17 +0100, Paolo Bonzini wrote:If kvm_arch_remove_sw_breakpoint finds that a software breakpoint does not have an INT3 instruction, it fails. This can happen if one sets a software breakpoint in a kernel module and then reloads it. gdb then thinks the breakpoint cannot be deleted and there is no way to add it back. Suggested-by: Maxim Levitsky <mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/i386/kvm/kvm.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 0b5755e42b..c8d61daf68 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -4352,8 +4352,13 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) { uint8_t int3; - if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0) || int3 != 0xcc || - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) { + if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0)) { + return -EINVAL; + } + if (int3 != 0xcc) { + return 0; + } + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) { return -EINVAL; } return 0;There still remains a philosopical question if kvm_arch_remove_sw_breakpoint should always return 0, since for the usual case of kernel debugging where a breakpoint is in unloaded module, the above will probably still fail as paging for this module is removed as well by the kernel. It is still better though so: Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Note that I managed to make lx-symbols to work in a very stable way with attached WIP patch I hacked on this Sunday. I will send a cleaned up version of it to upstream when I have time. Since I make gdb unload the symbols, it works even without this patch. Added Stefano Garzarella to CC as I know that he tried to make this work as well. https://lkml.org/lkml/2020/10/5/514
Thanks Maxim for CCing me!Just a report when I tried these patches, but I'm not sure they are related.
I found that gdb 10 has some problem with QEMU: $ gdb --version GNU gdb (GDB) Fedora 10.1-2.fc33 (gdb) lx-symbols loading vmlinux scanning for modules in linux/build ../../gdb/dwarf2/frame.c:1085: internal-error: Unknown CFA rule.With gdb 9 'lx-symbols' works well, but I still have some issue when I put a breakpoint to a symbol not yet loaded (vsock_core_register in this example), then I load the module (vsock_loopback in this example) in the guest.
Whit your patch gdb stuck after loading the symbols of the first new module:
(gdb) b vsock_core_register Function "vsock_core_register" not defined. Make breakpoint pending on future shared library load? (y or [n]) y Breakpoint 1 (vsock_core_register) pending. (gdb) c Continuing. loading @0xffffffffc00a1000: linux/build/net/vmw_vsock/vsock.ko Without your patch, gdb loops infinitely reloading the new module: refreshing all symbols to reload module 'vsock' loading vmlinux loading @0xffffffffc00a1000: linux/build/net/vmw_vsock/vsock.ko loading @0xffffffffc00ad000: linux/build/drivers/net/tun.ko loading @0xffffffffc007e000: linux/build/net/bridge/bridge.ko loading @0xffffffffc0077000: linux/build/net/802/stp.ko loading @0xffffffffc0007000: linux/build/net/llc/llc.ko loading @0xffffffffc0013000: linux/build/net/sunrpc/sunrpc.ko loading @0xffffffffc000d000: linux/build/net/ipv4/netfilter/ip_tables.ko loading @0xffffffffc0000000: linux/build/net/netfilter/x_tables.ko refreshing all symbols to reload module 'vsock' loading vmlinux loading @0xffffffffc00a1000: linux/build/net/vmw_vsock/vsock.ko loading @0xffffffffc00ad000: linux/build/drivers/net/tun.ko ... I'll try to get a better look at what's going on.I'm using Linux v5.11 in the guest, and the master of QEMU (commit 51db2d7cf26d05a961ec0ee0eb773594b32cc4a1) with Paolo's patch applied.
Thanks, Stefano
[Prev in Thread] | Current Thread | [Next in Thread] |