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>
This commit is contained in:
parent
b338970f8c
commit
dcd092a063
@ -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 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
|
||||
- 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 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
|
||||
{
|
||||
uint32_t cflags = tb_cflags(tb);
|
||||
TCGOp *icount_start_insn;
|
||||
TCGOp *first_insn_start = NULL;
|
||||
bool plugin_enabled;
|
||||
|
||||
/* Initialize DisasContext */
|
||||
@ -139,7 +127,6 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
|
||||
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 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
|
||||
*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 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
|
||||
* 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 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
|
||||
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);
|
||||
}
|
||||
|
@ -87,7 +87,6 @@ typedef struct DisasContextBase {
|
||||
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];
|
||||
|
Loading…
Reference in New Issue
Block a user