[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception
From: |
Luc Michel |
Subject: |
Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception |
Date: |
Fri, 17 Jul 2020 13:01:17 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 |
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~
>