accel/tcg: Improve can_do_io management
We already attempted to set and clear can_do_io before the first
and last insns, but only used the initial value of max_insns and
the call to translator_io_start to find those insns.
Now that we track insn_start in DisasContextBase, and now that
we have emit_before_op, we can wait until we have finished
translation to identify the true first and last insns and emit
the sets of can_do_io at that time.
This fixes the case of a translation block which crossed a page
boundary, and for which the second page turned out to be mmio.
In this case we truncate the block, and the previous logic for
can_do_io could leave a block with a single insn with can_do_io
set to false, which would fail an assertion in cpu_io_recompile.
Reported-by: Jørgen Hansen <Jorgen.Hansen@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Jørgen Hansen <Jorgen.Hansen@wdc.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index ae61c15..9de0bc3 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -18,20 +18,14 @@
static void set_can_do_io(DisasContextBase *db, bool val)
{
- if (db->saved_can_do_io != val) {
- db->saved_can_do_io = val;
-
- QEMU_BUILD_BUG_ON(sizeof_field(CPUState, neg.can_do_io) != 1);
- tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env,
- offsetof(ArchCPU, parent_obj.neg.can_do_io) -
- offsetof(ArchCPU, env));
- }
+ QEMU_BUILD_BUG_ON(sizeof_field(CPUState, neg.can_do_io) != 1);
+ tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env,
+ offsetof(ArchCPU, parent_obj.neg.can_do_io) -
+ offsetof(ArchCPU, env));
}
bool translator_io_start(DisasContextBase *db)
{
- set_can_do_io(db, true);
-
/*
* Ensure that this instruction will be the last in the TB.
* The target may override this to something more forceful.
@@ -84,13 +78,6 @@
- offsetof(ArchCPU, env));
}
- /*
- * cpu->neg.can_do_io is set automatically here at the beginning of
- * each translation block. The cost is minimal, plus it would be
- * very easy to forget doing it in the translator.
- */
- set_can_do_io(db, db->max_insns == 1);
-
return icount_start_insn;
}
@@ -129,6 +116,7 @@
{
uint32_t cflags = tb_cflags(tb);
TCGOp *icount_start_insn;
+ TCGOp *first_insn_start = NULL;
bool plugin_enabled;
/* Initialize DisasContext */
@@ -139,7 +127,6 @@
db->num_insns = 0;
db->max_insns = *max_insns;
db->singlestep_enabled = cflags & CF_SINGLE_STEP;
- db->saved_can_do_io = -1;
db->insn_start = NULL;
db->host_addr[0] = host_pc;
db->host_addr[1] = NULL;
@@ -159,6 +146,9 @@
*max_insns = ++db->num_insns;
ops->insn_start(db, cpu);
db->insn_start = tcg_last_op();
+ if (first_insn_start == NULL) {
+ first_insn_start = db->insn_start;
+ }
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
if (plugin_enabled) {
@@ -171,10 +161,6 @@
* done next -- either exiting this loop or locate the start of
* the next instruction.
*/
- if (db->num_insns == db->max_insns) {
- /* Accept I/O on the last instruction. */
- set_can_do_io(db, true);
- }
ops->translate_insn(db, cpu);
/*
@@ -207,6 +193,21 @@
ops->tb_stop(db, cpu);
gen_tb_end(tb, cflags, icount_start_insn, db->num_insns);
+ /*
+ * Manage can_do_io for the translation block: set to false before
+ * the first insn and set to true before the last insn.
+ */
+ if (db->num_insns == 1) {
+ tcg_debug_assert(first_insn_start == db->insn_start);
+ } else {
+ tcg_debug_assert(first_insn_start != db->insn_start);
+ tcg_ctx->emit_before_op = first_insn_start;
+ set_can_do_io(db, false);
+ }
+ tcg_ctx->emit_before_op = db->insn_start;
+ set_can_do_io(db, true);
+ tcg_ctx->emit_before_op = NULL;
+
if (plugin_enabled) {
plugin_gen_tb_end(cpu, db->num_insns);
}
diff --git a/include/exec/translator.h b/include/exec/translator.h
index ceaeca8..2c4fb81 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -87,7 +87,6 @@
int num_insns;
int max_insns;
bool singlestep_enabled;
- int8_t saved_can_do_io;
bool plugin_enabled;
struct TCGOp *insn_start;
void *host_addr[2];
