From c9700af2bd5648ba0d0de87f029c4a1ed8a3d7da Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Fri, 23 Feb 2018 23:26:53 -0500 Subject: [PATCH] tcg: Clean up from 'next_tb' The value returned from tcg_qemu_tb_exec() is the value passed to the corresponding tcg_gen_exit_tb() at translation time of the last TB attempted to execute. It is a little confusing to store it in a variable named 'next_tb'. In fact, it is a combination of 4-byte aligned pointer and additional information in its two least significant bits. Break it down right away into two variables named 'last_tb' and 'tb_exit' which are a pointer to the last TB attempted to execute and the TB exit reason, correspondingly. This simplifies the code and improves its readability. Correct a misleading documentation comment for tcg_qemu_tb_exec() and fix logging in cpu_tb_exec(). Also rename a misleading 'next_tb' in another couple of places. Backports commit 819af24b9c1e95e6576f1cefd32f4d6bf56dfa56 from qemu --- qemu/cpu-exec.c | 60 +++++++++++++++++++++++++++---------------------- qemu/tcg/tcg.h | 19 ++++++++-------- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/qemu/cpu-exec.c b/qemu/cpu-exec.c index 556d27d6..cbc09d05 100644 --- a/qemu/cpu-exec.c +++ b/qemu/cpu-exec.c @@ -44,11 +44,10 @@ int cpu_exec(struct uc_struct *uc, CPUState *cpu) X86CPU *x86_cpu = X86_CPU(uc, cpu); #endif int ret, interrupt_request; - TranslationBlock *tb; - uintptr_t next_tb; + TranslationBlock *tb, *last_tb; + int tb_exit = 0; struct hook *hook; - if (cpu->halted) { if (!cpu_has_work(cpu)) { return EXCP_HALTED; @@ -130,7 +129,7 @@ int cpu_exec(struct uc_struct *uc, CPUState *cpu) } } - next_tb = 0; /* force lookup of first TB */ + last_tb = NULL; /* forget the last executed TB after exception */ for(;;) { interrupt_request = cpu->interrupt_request; @@ -167,7 +166,7 @@ int cpu_exec(struct uc_struct *uc, CPUState *cpu) True when it is, and we should restart on a new TB, and via longjmp via cpu_loop_exit. */ if (cc->cpu_exec_interrupt(cpu, interrupt_request)) { - next_tb = 0; + last_tb = NULL; } /* Don't use the cached interrupt_request value, do_interrupt may have updated the EXITTB flag. */ @@ -175,7 +174,7 @@ int cpu_exec(struct uc_struct *uc, CPUState *cpu) cpu->interrupt_request &= ~CPU_INTERRUPT_EXITTB; /* ensure that no TB jump will be modified as the program flow was changed */ - next_tb = 0; + last_tb = NULL; } } if (unlikely(cpu->exit_request)) { @@ -195,22 +194,23 @@ int cpu_exec(struct uc_struct *uc, CPUState *cpu) /* as some TB could have been invalidated because of memory exceptions while generating the code, we must recompute the hash index here */ - next_tb = 0; + last_tb = NULL; tcg_ctx->tb_ctx.tb_invalidated_flag = 0; } /* See if we can patch the calling TB. */ - if (next_tb != 0 && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { - tb_add_jump((TranslationBlock *)(next_tb & ~TB_EXIT_MASK), - next_tb & TB_EXIT_MASK, tb); + if (last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { + tb_add_jump(last_tb, tb_exit, tb); } if (likely(!cpu->exit_request)) { + uintptr_t ret; cpu->current_tb = tb; /* execute the generated code */ - next_tb = cpu_tb_exec(cpu, tb); + ret = cpu_tb_exec(cpu, tb); cpu->current_tb = NULL; - - switch (next_tb & TB_EXIT_MASK) { + last_tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK); + tb_exit = ret & TB_EXIT_MASK; + switch (tb_exit) { case TB_EXIT_REQUESTED: /* Something asked us to stop executing * chained TBs; just continue round the main @@ -223,8 +223,7 @@ int cpu_exec(struct uc_struct *uc, CPUState *cpu) * or cpu->interrupt_request. */ smp_rmb(); - tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK); - next_tb = 0; + last_tb = NULL; break; default: break; @@ -277,46 +276,53 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb) { CPUArchState *env = cpu->env_ptr; TCGContext *tcg_ctx = env->uc->tcg_ctx; - uintptr_t next_tb; + uintptr_t ret; + TranslationBlock *last_tb; + int tb_exit; uint8_t *tb_ptr = itb->tc_ptr; // Unicorn: commented out //qemu_log_mask_and_addr(CPU_LOG_EXEC, itb->pc, // "Trace %p [" TARGET_FMT_lx "] %s\n", // itb->tc_ptr, itb->pc, lookup_symbol(itb->pc)); - next_tb = tcg_qemu_tb_exec(env, tb_ptr); + ret = tcg_qemu_tb_exec(env, tb_ptr); + last_tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK); + tb_exit = ret & TB_EXIT_MASK; + //trace_exec_tb_exit(last_tb, tb_exit); - if ((next_tb & TB_EXIT_MASK) > TB_EXIT_IDX1) { + if (tb_exit > TB_EXIT_IDX1) { /* We didn't start executing this TB (eg because the instruction * counter hit zero); we must restore the guest PC to the address * of the start of the TB. */ CPUClass *cc = CPU_GET_CLASS(env->uc, cpu); - TranslationBlock *tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK); // Unicorn: commented out - //qemu_log_mask_and_addr(CPU_LOG_EXEC, itb->pc, + //qemu_log_mask_and_addr(CPU_LOG_EXEC, last_tb->pc, // "Stopped execution of TB chain before %p [" // TARGET_FMT_lx "] %s\n", - // itb->tc_ptr, itb->pc, lookup_symbol(itb->pc)); + // last_tb->tc_ptr, last_tb->pc, + // lookup_symbol(last_tb->pc)); if (cc->synchronize_from_tb) { // avoid sync twice when helper_uc_tracecode() already did this. if (env->uc->emu_counter <= env->uc->emu_count && - !env->uc->stop_request && !env->uc->quit_request) - cc->synchronize_from_tb(cpu, tb); + !env->uc->stop_request && !env->uc->quit_request) { + cc->synchronize_from_tb(cpu, last_tb); + } } else { assert(cc->set_pc); // avoid sync twice when helper_uc_tracecode() already did this. - if (env->uc->emu_counter <= env->uc->emu_count && !env->uc->quit_request) - cc->set_pc(cpu, tb->pc); + if (env->uc->emu_counter <= env->uc->emu_count && !env->uc->quit_request) { + cc->set_pc(cpu, last_tb->pc); + } } } - if ((next_tb & TB_EXIT_MASK) == TB_EXIT_REQUESTED) { + if (tb_exit == TB_EXIT_REQUESTED) { /* We were asked to stop executing TBs (probably a pending * interrupt. We've now stopped, so clear the flag. */ cpu->tcg_exit_req = 0; } - return next_tb; + return ret; } static TranslationBlock *tb_find_slow(CPUState *cpu, diff --git a/qemu/tcg/tcg.h b/qemu/tcg/tcg.h index e8cdbefa..f12eed91 100644 --- a/qemu/tcg/tcg.h +++ b/qemu/tcg/tcg.h @@ -1062,7 +1062,7 @@ static inline unsigned get_mmuidx(TCGMemOpIdx oi) /** * tcg_qemu_tb_exec: - * @env: CPUArchState * for the CPU + * @env: pointer to CPUArchState for the CPU * @tb_ptr: address of generated code for the TB to execute * * Start executing code from a given translation block. @@ -1073,22 +1073,23 @@ static inline unsigned get_mmuidx(TCGMemOpIdx oi) * which has not yet been directly linked, or an asynchronous * event such as an interrupt needs handling. * - * The return value is a pointer to the next TB to execute - * (if known; otherwise zero). This pointer is assumed to be - * 4-aligned, and the bottom two bits are used to return further - * information: + * Return: The return value is the value passed to the corresponding + * tcg_gen_exit_tb() at translation time of the last TB attempted to execute. + * The value is either zero or a 4-byte aligned pointer to that TB combined + * with additional information in its two least significant bits. The + * additional information is encoded as follows: * 0, 1: the link between this TB and the next is via the specified * TB index (0 or 1). That is, we left the TB via (the equivalent * of) "goto_tb ". The main loop uses this to determine * how to link the TB just executed to the next. * 2: we are using instruction counting code generation, and we * did not start executing this TB because the instruction counter - * would hit zero midway through it. In this case the next-TB pointer + * would hit zero midway through it. In this case the pointer * returned is the TB we were about to execute, and the caller must * arrange to execute the remaining count of instructions. * 3: we stopped because the CPU's exit_request flag was set - * (usually meaning that there is an interrupt that needs to be - * handled). The next-TB pointer returned is the TB we were + * handled). The pointer returned is the TB we were about to execute + * when we noticed the pending exit request. * about to execute when we noticed the pending exit request. * * If the bottom two bits indicate an exit-via-index then the CPU @@ -1096,7 +1097,7 @@ static inline unsigned get_mmuidx(TCGMemOpIdx oi) * TB (and in particular the guest PC is the address to execute next). * Otherwise, we gave up on execution of this TB before it started, and * the caller must fix up the CPU state by calling the CPU's - * synchronize_from_tb() method with the next-TB pointer we return (falling + * synchronize_from_tb() method with the TB pointer we return (falling * back to calling the CPU's set_pc method with tb->pb if no * synchronize_from_tb() method exists). *