* [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception @ 2020-07-16 10:04 Luc Michel 2020-07-16 17:57 ` Richard Henderson 2020-07-16 20:12 ` Peter Maydell 0 siblings, 2 replies; 6+ messages in thread From: Luc Michel @ 2020-07-16 10:04 UTC (permalink / raw) To: qemu-devel, Paolo Bonzini, Richard Henderson; +Cc: Luc Michel When single-stepping with a debugger attached to QEMU, and when an exception is raised, the debugger misses the first instruction after the exception: $ qemu-system-aarch64 -M virt -display none -cpu cortex-a53 -s -S $ aarch64-linux-gnu-gdb GNU gdb (GDB) 9.2 [...] (gdb) tar rem :1234 Remote debugging using :1234 warning: No executable has been specified and target does not support determining executable automatically. Try using the "file" command. 0x0000000000000000 in ?? () (gdb) # writing nop insns to 0x200 and 0x204 (gdb) set *0x200 = 0xd503201f (gdb) set *0x204 = 0xd503201f (gdb) # 0x0 address contains 0 which is an invalid opcode. (gdb) # The CPU should raise an exception and jump to 0x200 (gdb) si 0x0000000000000204 in ?? () With this commit, the same run steps correctly on the first instruction of the exception vector: (gdb) si 0x0000000000000200 in ?? () Signed-off-by: Luc Michel <luc.michel@greensocs.com> --- RFC because I'm really not sure this is the good place to do that since EXCP_DEBUG are usually raised in each target translate.c. It could also have implications with record/replay I'm not aware of. --- accel/tcg/cpu-exec.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index d95c4848a4..e85fab5d40 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -502,10 +502,21 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret) CPUClass *cc = CPU_GET_CLASS(cpu); qemu_mutex_lock_iothread(); cc->do_interrupt(cpu); qemu_mutex_unlock_iothread(); cpu->exception_index = -1; + + if (unlikely(cpu->singlestep_enabled)) { + /* + * After processing the exception, ensure an EXCP_DEBUG is + * raised when single-stepping so that GDB doesn't miss the + * next instruction. + */ + cpu->exception_index = EXCP_DEBUG; + return cpu_handle_exception(cpu, ret); + } + } else if (!replay_has_interrupt()) { /* give a chance to iothread in replay mode */ *ret = EXCP_INTERRUPT; return true; } -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception 2020-07-16 10:04 [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception Luc Michel @ 2020-07-16 17:57 ` Richard Henderson 2020-07-16 20:12 ` Peter Maydell 1 sibling, 0 replies; 6+ messages in thread From: Richard Henderson @ 2020-07-16 17:57 UTC (permalink / raw) To: Luc Michel, qemu-devel, Paolo Bonzini, Richard Henderson On 7/16/20 3:04 AM, Luc Michel wrote: > When single-stepping with a debugger attached to QEMU, and when an > exception is raised, the debugger misses the first instruction after the > exception: > > $ qemu-system-aarch64 -M virt -display none -cpu cortex-a53 -s -S > > $ aarch64-linux-gnu-gdb > GNU gdb (GDB) 9.2 > [...] > (gdb) tar rem :1234 > Remote debugging using :1234 > warning: No executable has been specified and target does not support > determining executable automatically. Try using the "file" command. > 0x0000000000000000 in ?? () > (gdb) # writing nop insns to 0x200 and 0x204 > (gdb) set *0x200 = 0xd503201f > (gdb) set *0x204 = 0xd503201f > (gdb) # 0x0 address contains 0 which is an invalid opcode. > (gdb) # The CPU should raise an exception and jump to 0x200 > (gdb) si > 0x0000000000000204 in ?? () > > With this commit, the same run steps correctly on the first instruction > of the exception vector: > > (gdb) si > 0x0000000000000200 in ?? () > > Signed-off-by: Luc Michel <luc.michel@greensocs.com> > --- > > RFC because I'm really not sure this is the good place to do that since > EXCP_DEBUG are usually raised in each target translate.c. It could also > have implications with record/replay I'm not aware of. > > --- > accel/tcg/cpu-exec.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index d95c4848a4..e85fab5d40 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -502,10 +502,21 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret) > CPUClass *cc = CPU_GET_CLASS(cpu); > qemu_mutex_lock_iothread(); > cc->do_interrupt(cpu); > qemu_mutex_unlock_iothread(); > cpu->exception_index = -1; > + > + if (unlikely(cpu->singlestep_enabled)) { > + /* > + * After processing the exception, ensure an EXCP_DEBUG is > + * raised when single-stepping so that GDB doesn't miss the > + * next instruction. > + */ > + cpu->exception_index = EXCP_DEBUG; > + return cpu_handle_exception(cpu, ret); Plausible. Although recursion on an inline function is going to defeat the inline, in general. We could expand the recursion by hand with if (unlikely(cpu->singlestep_enabled)) { *ret = EXCP_DEBUG; cpu_handle_debug_exception(cpu); return true; } which might even be clearer. r~ > + } > + > } else if (!replay_has_interrupt()) { > /* give a chance to iothread in replay mode */ > *ret = EXCP_INTERRUPT; > return true; > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception 2020-07-16 10:04 [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception Luc Michel 2020-07-16 17:57 ` Richard Henderson @ 2020-07-16 20:12 ` Peter Maydell 2020-07-16 21:08 ` Richard Henderson 1 sibling, 1 reply; 6+ messages in thread From: Peter Maydell @ 2020-07-16 20:12 UTC (permalink / raw) To: Luc Michel; +Cc: Paolo Bonzini, QEMU Developers, Richard Henderson On Thu, 16 Jul 2020 at 11:08, Luc Michel <luc.michel@greensocs.com> wrote: > > When single-stepping with a debugger attached to QEMU, and when an > exception is raised, the debugger misses the first instruction after the > exception: This is a long-standing bug; thanks for looking at it. (https://bugs.launchpad.net/qemu/+bug/757702) > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index d95c4848a4..e85fab5d40 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -502,10 +502,21 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret) > CPUClass *cc = CPU_GET_CLASS(cpu); > qemu_mutex_lock_iothread(); > cc->do_interrupt(cpu); > qemu_mutex_unlock_iothread(); > cpu->exception_index = -1; > + > + if (unlikely(cpu->singlestep_enabled)) { > + /* > + * After processing the exception, ensure an EXCP_DEBUG is > + * raised when single-stepping so that GDB doesn't miss the > + * next instruction. > + */ > + cpu->exception_index = EXCP_DEBUG; > + return cpu_handle_exception(cpu, ret); > + } I like the idea of being able to do this generically in the main loop. How about interrupts? If we are single-stepping and we take an interrupt I guess we want to stop before the first insn of the interrupt handler rather than after it, which would imply a similar change to cpu_handle_interrupt(). thanks -- PMM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception 2020-07-16 20:12 ` Peter Maydell @ 2020-07-16 21:08 ` Richard Henderson 2020-07-17 11:01 ` Luc Michel 0 siblings, 1 reply; 6+ messages in thread From: Richard Henderson @ 2020-07-16 21:08 UTC (permalink / raw) To: Peter Maydell, Luc Michel Cc: Paolo Bonzini, QEMU Developers, Richard Henderson On 7/16/20 1:12 PM, Peter Maydell wrote: > On Thu, 16 Jul 2020 at 11:08, Luc Michel <luc.michel@greensocs.com> wrote: >> >> When single-stepping with a debugger attached to QEMU, and when an >> exception is raised, the debugger misses the first instruction after the >> exception: > > This is a long-standing bug; thanks for looking at it. > (https://bugs.launchpad.net/qemu/+bug/757702) > > >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >> index d95c4848a4..e85fab5d40 100644 >> --- a/accel/tcg/cpu-exec.c >> +++ b/accel/tcg/cpu-exec.c >> @@ -502,10 +502,21 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret) >> CPUClass *cc = CPU_GET_CLASS(cpu); >> qemu_mutex_lock_iothread(); >> cc->do_interrupt(cpu); >> qemu_mutex_unlock_iothread(); >> cpu->exception_index = -1; >> + >> + if (unlikely(cpu->singlestep_enabled)) { >> + /* >> + * After processing the exception, ensure an EXCP_DEBUG is >> + * raised when single-stepping so that GDB doesn't miss the >> + * next instruction. >> + */ >> + cpu->exception_index = EXCP_DEBUG; >> + return cpu_handle_exception(cpu, ret); >> + } > > I like the idea of being able to do this generically in > the main loop. > > How about interrupts? If we are single-stepping and we > take an interrupt I guess we want to stop before the first > insn of the interrupt handler rather than after it, which > would imply a similar change to cpu_handle_interrupt(). Fair. I think something like this: if (cc->cpu_exec_interrupt(cpu, interrupt_request)) { replay_interrupt(); - cpu->exception_index = -1; + cpu->exception_index = + (cpu->singlestep_enabled ? EXCP_DEBUG : -1); *last_tb = NULL; } I'm not quite sure how to test this though... Probably best to keep this a separate patch anyway. r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception 2020-07-16 21:08 ` Richard Henderson @ 2020-07-17 11:01 ` Luc Michel 2020-07-17 15:42 ` Richard Henderson 0 siblings, 1 reply; 6+ messages in thread From: Luc Michel @ 2020-07-17 11:01 UTC (permalink / raw) To: Richard Henderson, Peter Maydell Cc: Paolo Bonzini, QEMU Developers, Richard Henderson On 7/16/20 11:08 PM, Richard Henderson wrote: > On 7/16/20 1:12 PM, Peter Maydell wrote: >> On Thu, 16 Jul 2020 at 11:08, Luc Michel <luc.michel@greensocs.com> wrote: >>> >>> When single-stepping with a debugger attached to QEMU, and when an >>> exception is raised, the debugger misses the first instruction after the >>> exception: >> >> This is a long-standing bug; thanks for looking at it. >> (https://bugs.launchpad.net/qemu/+bug/757702) >> >> >>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >>> index d95c4848a4..e85fab5d40 100644 >>> --- a/accel/tcg/cpu-exec.c >>> +++ b/accel/tcg/cpu-exec.c >>> @@ -502,10 +502,21 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret) >>> CPUClass *cc = CPU_GET_CLASS(cpu); >>> qemu_mutex_lock_iothread(); >>> cc->do_interrupt(cpu); >>> qemu_mutex_unlock_iothread(); >>> cpu->exception_index = -1; >>> + >>> + if (unlikely(cpu->singlestep_enabled)) { >>> + /* >>> + * After processing the exception, ensure an EXCP_DEBUG is >>> + * raised when single-stepping so that GDB doesn't miss the >>> + * next instruction. >>> + */ >>> + cpu->exception_index = EXCP_DEBUG; >>> + return cpu_handle_exception(cpu, ret); >>> + } >> >> I like the idea of being able to do this generically in >> the main loop. >> >> How about interrupts? If we are single-stepping and we >> take an interrupt I guess we want to stop before the first >> insn of the interrupt handler rather than after it, which >> would imply a similar change to cpu_handle_interrupt(). > > Fair. I think something like this: > > if (cc->cpu_exec_interrupt(cpu, interrupt_request)) { > replay_interrupt(); > - cpu->exception_index = -1; > + cpu->exception_index = > + (cpu->singlestep_enabled ? EXCP_DEBUG : -1); > *last_tb = NULL; > } > > I'm not quite sure how to test this though... I wrote a small test case for the interrupt side that can be run on the virt board: 8<----------------------------------------------- .global _start .text #define GIC_DIST_BASE 0x08000000 #define GIC_CPU_BASE 0x08010000 #define ARCH_TIMER_NS_EL1_IRQ (16 + 14) .org 0x280 _irq_handler: nop nop .org 0x1000 _start: ldr x1, =GIC_DIST_BASE /* GICD_CTLR */ mov w0, #1 /* enable */ str w0, [x1] ldr x1, =GIC_DIST_BASE + 0x100 /* GICD_ISENABLER0 */ ldr w0, =(1 << ARCH_TIMER_NS_EL1_IRQ) str w0, [x1] ldr x1, =GIC_CPU_BASE /* GICC_CTLR */ mov w0, #1 /* enable */ str w0, [x1] ldr x1, =GIC_CPU_BASE + 0x4 /* GICC_PMR */ mov w0, #255 /* min priority */ str w0, [x1] msr daifclr, 2 /* unmask IRQ line */ mov x0, 10 /* timer will trigger in 10 ticks */ msr cntp_tval_el0, x0 mov x0, 1 /* enable timer */ enable_timer: msr cntp_ctl_el0, x0 1: b 1b 8<----------------------------------------------------- $ aarch64-linux-gnu-gcc -g -nostdinc -nostdlib -Wl,-Ttext=0x0 -o foo foo.S $ qemu-system-aarch64 -display none -M virt -cpu cortex-a53 -kernel foo -s -S $ aarch64-linux-gnu-gdb foo (gdb) tar rem :1234 (gdb) maintenance packet Qqemu.sstep=0x1 (gdb) b enable_timer (gdb) disp /i $pc (gdb) c Continuing. Breakpoint 1, enable_timer () at foo.S:40 1: x/i $pc => 0x1040 <enable_timer>: msr cntp_ctl_el0, x0 (gdb) si 1: x/i $pc => 0x1044 <enable_timer+4>: b 0x1044 <enable_timer+4> (gdb) 1: x/i $pc => 0x280 <_irq_handler>: nop This is with your fix. Without it, the second stepi stops on 0x284. > > Probably best to keep this a separate patch anyway. Do you want me to send it? If yes, how should I give credit to you? Should I put your Signed-off-by: in it? thanks Luc > > > r~ > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception 2020-07-17 11:01 ` Luc Michel @ 2020-07-17 15:42 ` Richard Henderson 0 siblings, 0 replies; 6+ messages in thread From: Richard Henderson @ 2020-07-17 15:42 UTC (permalink / raw) To: Luc Michel, Peter Maydell Cc: Paolo Bonzini, QEMU Developers, Richard Henderson On 7/17/20 4:01 AM, Luc Michel wrote: > I wrote a small test case for the interrupt side that can be run on the > virt board: ... > This is with your fix. Without it, the second stepi stops on 0x284. Awesome, thanks. > Do you want me to send it? If yes, how should I give credit to you? > Should I put your Signed-off-by: in it? I'll put it together. You can then add your Reviewed/Tested-by. r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-17 15:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-16 10:04 [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception Luc Michel 2020-07-16 17:57 ` Richard Henderson 2020-07-16 20:12 ` Peter Maydell 2020-07-16 21:08 ` Richard Henderson 2020-07-17 11:01 ` Luc Michel 2020-07-17 15:42 ` Richard Henderson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.