Re: [Qemu-devel] [PATCH] cpu-exec: Don't mask out external interrupts when single-stepping an invalid instruction.

[Martin Galvan]

On Thu, Sep 11, 2014 at 8:52 PM, Richard Henderson <address@hidden> wrote:
> On 09/11/2014 03:13 PM, Peter Maydell wrote:
> In particular, I'd expect the invalid exception to be recognized, and then the
> cpu loop exited, before the single-step exception could overwrite it.  E.g. 
> how
> things work on alpha:
>
> $ cat z.s
>         .globl _start
> _start:
>         nop
>         .long (1 << 26)
>         nop
> $ alphaev67-linux-as -o z.o z.s
> $ alphaev67-linux-ld -Ttext-segment 0xfffffc0000100000 -o z z.o
> $ ./run/bin/qemu-system-alpha -kernel z -S -gdb tcp::12345 &
> $ alphaev67-linux-gdb ./z

Can't test this myself since I don't have an alpha cross-toolchain at
hand right now.

> Set a breakpoint at _start, continue, swap over to qemu monitor and enable
> logging of int,op,in_asm.  Now single-step a couple of times and we get:
>
> IN:
> 0xfffffc0000100078:  nop
>
> OP:
>  ld_i32 tmp0,env,$0xfffffffffffffffc
>  movi_i32 tmp1,$0x0
>  brcond_i32 tmp0,tmp1,ne,$0x0
>  ---- 0xfffffc0000100078
>  movi_i64 pc,$0xfffffc000010007c
>  movi_i32 tmp0,$0x10002
>  movi_i32 tmp1,$0x0
>  call excp,$0x0,$0,env,tmp0,tmp1
>  set_label $0x0
>  exit_tb $0x7f54d4fee013
>
> Notice here that we end the single-step of the insn with a call to excp with
> 0x10002=EXCP_DEBUG.
>
> IN:
> 0xfffffc000010007c:  .long 0x4000000
>
> OP:
>  ld_i32 tmp0,env,$0xfffffffffffffffc
>  movi_i32 tmp1,$0x0
>  brcond_i32 tmp0,tmp1,ne,$0x0
>  ---- 0xfffffc000010007c
>  movi_i64 pc,$0xfffffc0000100080
>  movi_i32 tmp0,$0x7
>  movi_i32 tmp1,$0x0
>  call excp,$0x0,$0,env,tmp0,tmp1
>  set_label $0x0
>  exit_tb $0x7f54d4fee013
>
> Notice here that we end the single-step of the invalid insn with a call to 
> excp
> with 7=EXCP_OPCDEC.  We never attempt to single-step, because we know that
> single-step isn't going to be reachable.
>
> INT      1: opcdec(0) pc=fffffc0000100080 sp=fffffc0000010000
> IN: Pal_OpcDec
> 0xfffffc0000000380:  hw_mfpr    t7,0
>
> But gdb does in fact stop at the very next insn, which is of course the 
> PALcode
> (bios) OPCDEC exception entry point.

So if I understood right, what you do in Alpha is:

- With your PC pointing to the invalid instruction, single-step once.
- The generated assembly will contain a call to excp with EXCP_OPCDEC.
- On excp, it sets cs->exception_index to EXCP_OPCDEC and then does a
cpu_loop_exit.
- As it advances through the loop again, it'll notice exception_index
is greater than 0, thus calling do_interrupt.
- Inside do_interrupt it sets the PC to point to the exception handler
entry point.
- It sets cpu->exception_index to EXCP_DEBUG somehow, thus returning
control back to gdb.
- The net result is that single-stepping with the PC pointing to an
invalid instruction
  immediately leads us to the exception handler.

That's exactly what I'm trying to achieve. However, in target-arm we
end up calling do_interrupt twice:
the first time in the outer for(;;) to set cpu->interrupt_request to
CPU_INTERRUPT_HARD,
and the second time inside the inner for(;;) to actually set the PC to
point to the exception handler.

Silly me, I thought every architecture did the same.

>>> +#define SSTEP_EXCEPTION 0x8  /* Don't mask out exception-related
>>> IRQs. Set only if
>>> +                              * we have to process an exception while 
>>> single-
>>> +                              * stepping (such as when
>>> single-stepping an invalid
>>> +                              * instruction).
>>> +                              */
>
> I'm really not sure what you're doing with this, or why you'd need it.
> Probably target-arm wants fixing in some way, but I don't have time to look
> into it this evening.
>

Like I said, in the inner for(;;) the call to do_interrupt wasn't
being done because
our CPU_INTERRUPT_HARD was being masked out because of the very fact we have
the SSTEP_NOIRQ flag set in singlestep_enabled. The purpose of my new
flag was to avoid that,
so the PC would be set correctly.

However, like you said this should probably be fixed inside the ARM
code instead of
touching cpu-exec.

Besides, my fix was taking advantage of the fact that we'd get a new
translation block
after translating the first instruction of the exception handler. As
we still have singlestep_enabled,
it would generate a new internal exception that would set
cpu->exception_index to EXCP_DEBUG
and return control back to gdb. However, I just noticed that if we
were to have an invalid instruction inside
the interrupt handler itself, Qemu won't generate a new translation
block and once again
we'll never reach the internal exception, thus never returning control
back to gdb.

How do you set cpu->exception_index in to EXCP_DEBUG after calling
do_interrupt with EXCP_OPCDEC?

-- 

Martín Galván

Software Engineer

Taller Technologies Argentina


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina

Phone: 54 351 4217888 / +54 351 4218211