qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: [PULL 6/7] target/arm: drop CF_LAST_IO/dc->condjump check
Date: Sat, 17 Apr 2021 20:42:04 +0100	[thread overview]
Message-ID: <20210417194205.17057-7-peter.maydell@linaro.org> (raw)
In-Reply-To: <20210417194205.17057-1-peter.maydell@linaro.org>

From: Alex Bennée <alex.bennee@linaro.org>

This is a left over erroneous check from the days front-ends handled
io start/end themselves. Regardless just because IO could be performed
on the last instruction doesn't obligate the front end to do so.

This fixes an abort faced by the aspeed execute-in-place support which
will necessarily trigger this state (even before the one-shot
CF_LAST_IO fix). The test still seems to hang once it attempts to boot
the Linux kernel but I suspect this is an unrelated issue with icount
and the timer handling code.

The original intention of the cpu_abort (added in commit 2e70f6efa8b9
when the icount stuff was first added) seems to have been to act as
an assert() to catch an unhandled corner case where the generated code
would be something like:
    conditional branch to condlabel if its cc failed
    implementation of the insn (a conditional branch or trap)
    code emitted by gen_io_end()
 condlabel:
    gen_goto_tb or equivalent thing to go to next insn

At runtime the cc-failed case would skip over the code emitted by
gen_io_end(), leaving the can_do_io flag incorrectly set.

In commit ba3e7926691ed33 we switched to an implementation which
always clears can_do_io at the start of the following TB instead
of trying to clear it at the end of a TB that did IO. So the corner
case that this cpu_abort() was trying to flag is no longer possible,
because the gen_io_end() call has been deleted. We can therefore
safely remove the no-longer-valid assertion.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20210416170207.12504-1-alex.bennee@linaro.org
Cc: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/translate.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 62b1c2081b6..7103da2d7ab 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9199,11 +9199,6 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
-    if (tb_cflags(dc->base.tb) & CF_LAST_IO && dc->condjmp) {
-        /* FIXME: This can theoretically happen with self-modifying code. */
-        cpu_abort(cpu, "IO on conditional branch instruction");
-    }
-
     /* At this stage dc->condjmp will only be set when the skipped
        instruction was a conditional branch or trap, and the PC has
        already been written.  */
-- 
2.20.1



  parent reply	other threads:[~2021-04-17 19:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-17 19:41 [PULL 0/7] queue of proposed rc4 fixes Peter Maydell
2021-04-17 19:41 ` [PULL 1/7] osdep: include glib-compat.h before other QEMU headers Peter Maydell
2021-04-17 19:42 ` [PULL 2/7] osdep: protect qemu/osdep.h with extern "C" Peter Maydell
2021-04-17 19:42 ` [PULL 3/7] include/qemu/osdep.h: Move system includes to top Peter Maydell
2021-04-17 19:42 ` [PULL 4/7] hw/arm/armsse: Give SSE-300 its own Property array Peter Maydell
2021-04-17 19:42 ` [PULL 5/7] hw/arm/armsse: Make SSE-300 use Cortex-M55 Peter Maydell
2021-04-17 19:42 ` Peter Maydell [this message]
2021-04-17 19:42 ` [PULL 7/7] accel/tcg: avoid re-translating one-shot instructions Peter Maydell
2021-04-17 19:51 ` [PULL 0/7] queue of proposed rc4 fixes no-reply
2021-04-18  5:17 ` Philippe Mathieu-Daudé
2021-04-19  9:54   ` Philippe Mathieu-Daudé
2021-04-18 16:31 ` Alex Bennée
2021-04-19  9:18   ` Peter Maydell
2021-04-19 10:53 ` Thomas Huth
2021-04-19 14:48 ` 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=20210417194205.17057-7-peter.maydell@linaro.org \
    --to=peter.maydell@linaro.org \
    --cc=alex.bennee@linaro.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).