All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Pavel Dovgalyuk <dovgaluk@ispras.ru>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Subject: Re: [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index
Date: Wed, 10 Jan 2018 10:24:24 +0000	[thread overview]
Message-ID: <CAFEAcA_RCunjurjJt3rrOp+0hHXCZgK2X9mM857fKtF8v=Kb+Q@mail.gmail.com> (raw)
In-Reply-To: <001401d389e1$495ffa30$dc1fee90$@ru>

On 10 January 2018 at 07:04, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> The failure cause is in incorrect interrupt processing.
> When ARM processes hardware interrupt in arm_cpu_exec_interrupt(),
> it executes cs->exception_index = excp_idx;
>
> This assumes, that the exception will be processed later.
> But it is processed immediately by calling cc->do_interrupt(cs);
> instead of leaving this job to cpu_exec.

That seems fine to me. The code knows it needs to take
an interrupt, it has all the information it needs to do
it, it can just go ahead and call the code that takes
the interrupt. The comment in accel/tcg/cpu-exec.c says:

        /* The target hook has 3 exit conditions:
           False when the interrupt isn't processed,
           True when it is, and we should restart on a new TB,
           and via longjmp via cpu_loop_exit.  */

and here we have processed the interrupt and returned true.
We set exception_index because that's how you tell the
do_interrupt hook which interrupt to deal with.

That is, the pattern that the arm target code assumes for
cs->exception_index is "you don't set this unless you're
about to call do_interrupt; if you do set it then definitely
call do_interrupt and don't do anything much in between".
In that view of the world there's no need to reset it or
check it because nothing is permitted to happen between
"set value" and "call do_interrupt". This is in line with
the way we handle other arm-specific bits of information
associated with the exception, like env->exception.target_el.
Having a long gap between "set value" and "do_interrupt"
is worrying because it means that maybe we might end
up doing something else in that gap that corrupts the
various bits of information associated with the exception,
or something that's not architecturally permitted to
happen at that point.

thanks
-- PMM

  reply	other threads:[~2018-01-10 10:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16 11:59 [Qemu-devel] [PULL 00/11] Miscellaneous patches for QEMU 2.11-rc2 Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 01/11] Enable 8-byte wide MMIO for 16550 serial devices Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 02/11] ioapic/tracing: Remove last DPRINTFs Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 03/11] Makefile: simpler/faster "make help" Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 04/11] thread-posix: fix qemu_rec_mutex_trylock macro Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 05/11] target-i386: adds PV_TLB_FLUSH CPUID feature bit Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 06/11] vhost-user-scsi: add missing virtqueue_size param Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 07/11] cpu-exec: don't overwrite exception_index Paolo Bonzini
2017-11-17 20:07   ` Peter Maydell
2017-11-17 20:26     ` Paolo Bonzini
2017-11-17 20:34       ` Peter Maydell
2017-11-20 10:25         ` Pavel Dovgalyuk
2017-11-20 11:06           ` Peter Maydell
2017-11-20 12:50             ` Peter Maydell
2017-11-20 21:08               ` Paolo Bonzini
2018-01-09 13:21               ` Pavel Dovgalyuk
2018-01-09 13:44                 ` Peter Maydell
2018-01-10  7:04                   ` Pavel Dovgalyuk
2018-01-10 10:24                     ` Peter Maydell [this message]
2018-01-10 10:43                       ` Pavel Dovgalyuk
2017-11-16 11:59 ` [Qemu-devel] [PULL 08/11] cpu-exec: avoid cpu_exec_nocache infinite loop with record/replay Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 09/11] util/stats64: Fix min/max comparisons Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 10/11] exec: Do not resolve subpage in mru_section Paolo Bonzini
2017-11-16 11:59 ` [Qemu-devel] [PULL 11/11] fix scripts/update-linux-headers.sh here document Paolo Bonzini
2017-11-16 16:11 ` [Qemu-devel] [PULL 00/11] Miscellaneous patches for QEMU 2.11-rc2 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='CAFEAcA_RCunjurjJt3rrOp+0hHXCZgK2X9mM857fKtF8v=Kb+Q@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=dovgaluk@ispras.ru \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --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.