All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <rth@twiddle.net>
To: Peter Maydell <peter.maydell@linaro.org>,
	Martin Galvan <martin.galvan@tallertechnologies.com>
Cc: "QEMU Developers" <qemu-devel@nongnu.org>,
	"Anthony Liguori" <aliguori@amazon.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH] cpu-exec: Don't mask out external interrupts when single-stepping an invalid instruction.
Date: Thu, 11 Sep 2014 16:52:06 -0700	[thread overview]
Message-ID: <541235A6.5050006@twiddle.net> (raw)
In-Reply-To: <CAFEAcA-bkDxmWeK8LPTZFZgXkwySppi3m5yjwa1qkdGk+qL37w@mail.gmail.com>

On 09/11/2014 03:13 PM, Peter Maydell wrote:
>> The long story: Qemu generates an exception_with_syndrome instruction
>> when it realizes the instruction it's trying to translate is invalid.
>> That instruction in turn modifies cs->exception_index and calls cpu_loop_exit.
>> Normally, the value in cs->exception_index would cause do_interrupt to set
>> the PC to point to the corresponding exception handler.
>> However, since we're masking out IRQs, the PC will never be set correctly.
>> Even worse, since Qemu will have generated an internal exception
>> to return control back to the remote Gdb *after* it generated the syndrome one,
>> its code will appear after the call to cpu_loop_exit.
>> Since the PC won't have changed, we'll try to excecute
>> the same translation block as before, thus calling cpu_loop_exit
>> again, and so on.

This suggests that target-arm (64 or 32-bit?) isn't implementing illegal
instruction traping in the way that I'd expect.

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

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.

>> +#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.


r~

  reply	other threads:[~2014-09-11 23:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-11 21:02 [Qemu-devel] [PATCH] cpu-exec: Don't mask out external interrupts when single-stepping an invalid instruction Martin Galvan
2014-09-11 22:13 ` Peter Maydell
2014-09-11 23:52   ` Richard Henderson [this message]
2014-09-12 14:33     ` Martin Galvan
2014-09-12 15:37       ` Richard Henderson
2014-09-12 15:50         ` Martin Galvan
2014-09-15 16:02           ` Martin Galvan
2014-09-15 16:10             ` Peter Maydell
2014-09-15 16:17               ` Andreas Färber
2014-09-15 16:22                 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=541235A6.5050006@twiddle.net \
    --to=rth@twiddle.net \
    --cc=afaerber@suse.de \
    --cc=aliguori@amazon.com \
    --cc=martin.galvan@tallertechnologies.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.