Alternative post-instruction early TB termination
cover Alternative post-instruction early TB termination
Originally I was hoping to get my whole debugger patch series on the track, but I'm still stuck with a bug in the x86 debug register support (weird single step race, triggered by dr-usage). So let's start smaller with a friction of that series.
These patches introduce a new single step mode that allows the emulator to generate and execute only a single-instruction TB, but without triggering a debug event afterwards. This is exploited by tb_invalidate_phys_page[_range] and later on by the watchpoint subsystem (patch to be posted). This should also allow to remove cflags from TranslationBlock, as done by the third patch.
Jan
PATCH 1/3 Introduce SSTEP_INTERNAL
Introducing SSTEP_INTERNAL, this patch allows to reuse the (host-injected) single-step infrastructure to let the emulator generate and execute TBs that only include one instruction.
Signed-off-by Jan Kiszka <[email protected]>
--- cpu-all.h | 7 ++++--- cpu-exec.c | 4 +++- exec.c | 2 ++ gdbstub.c | 4 ++-- target-arm/translate.c | 2 +- target-cris/translate.c | 2 +- target-i386/translate.c | 2 +- target-m68k/translate.c | 4 ++-- target-mips/translate.c | 2 +- target-ppc/translate.c | 2 +- target-sh4/translate.c | 6 +++--- vl.c | 7 ++++--- 12 files changed, 25 insertions(+), 19 deletions(-)
cpu-all.h +4 -3
@@ -804,9 +804,10 @@ int cpu_breakpoint_insert(CPUState *env,int cpu_breakpoint_remove (CPUState *env,target_ulong pc);void cpu_breakpoint_remove_all (CPUState *env); -#define SSTEP_ENABLE 0x1/* Enable simulated HW single stepping */ -#define SSTEP_NOIRQ 0x2/* Do not use IRQ while single stepping */ -#define SSTEP_NOTIMER 0x4/* Do not Timers while single stepping */ +#define SSTEP_DEBUG 0x1/* Enable simulated HW single stepping */ +#define SSTEP_NOIRQ 0x2/* Do not use IRQ while single stepping */ +#define SSTEP_NOTIMER 0x4/* Do not Timers while single stepping */ +#define SSTEP_INTERNAL 0x8/* QEMU internal, do not generate breakpoint */ void cpu_single_step (CPUState *env,int enabled);void cpu_reset (CPUState *s);
cpu-exec.c +3 -1
@@ -369,7 +369,8 @@ int cpu_exec(CPUState *env1)for (;;) { interrupt_request = env->interrupt_request;if (__builtin_expect (interrupt_request,0 ) && -likely (!(env->singlestep_enabled &SSTEP_NOIRQ ))) { +likely (!(env->singlestep_enabled & + (SSTEP_NOIRQ |SSTEP_INTERNAL )))) {if (interrupt_request &CPU_INTERRUPT_DEBUG ) { env->interrupt_request &= ~CPU_INTERRUPT_DEBUG ; env->exception_index =EXCP_DEBUG ;
@@ -609,6 +610,7 @@ int cpu_exec(CPUState *env1)#endif next_tb =tcg_qemu_tb_exec (tc_ptr); env->current_tb =NULL ; + env->singlestep_enabled &= ~SSTEP_INTERNAL ;/* reset soft MMU for next block (it can currently only be set by a memory fault) */ #if defined(USE_KQEMU )
exec.c +2 -0
@@ -1292,6 +1292,8 @@ int cpu_breakpoint_remove(CPUState *env,void cpu_single_step (CPUState *env,int enabled) {#if defined(TARGET_HAS_ICE ) + enabled &=SSTEP_DEBUG |SSTEP_NOIRQ |SSTEP_NOTIMER ; + enabled |= env->singlestep_enabled &SSTEP_INTERNAL ;if (env->singlestep_enabled != enabled) { env->singlestep_enabled = enabled;/* must flush all the translated code to avoid inconsistancies */
target-arm/translate.c +1 -1
@@ -8666,7 +8666,7 @@ static inline int gen_intermediate_code_/* 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. */ -if (__builtin_expect (env->singlestep_enabled,0 )) { +if (__builtin_expect (env->singlestep_enabled &SSTEP_DEBUG ,0 )) {/* Make sure the pc is updated, and raise a debug exception. */ if (dc->condjmp) {gen_set_condexec (dc);
target-cris/translate.c +1 -1
@@ -3067,7 +3067,7 @@ gen_intermediate_code_internal(CPUStatecris_evaluate_flags (dc); done: -if (__builtin_expect (env->singlestep_enabled,0 )) { +if (__builtin_expect (env->singlestep_enabled &SSTEP_DEBUG ,0 )) {t_gen_raise_exception (EXCP_DEBUG ); }else {switch (dc->is_jmp) {
target-i386/translate.c +1 -1
@@ -2618,7 +2618,7 @@ static void gen_eob(DisasContext *s)if (s->tb->flags &HF_INHIBIT_IRQ_MASK ) {tcg_gen_helper_0_0 (helper_reset_inhibit_irq); } -if (s->singlestep_enabled) { +if (s->singlestep_enabled &SSTEP_DEBUG ) {tcg_gen_helper_0_0 (helper_debug); }else if (s->tf) {tcg_gen_helper_0_0 (helper_single_step);
target-m68k/translate.c +2 -2
@@ -871,7 +871,7 @@ static void gen_jmp_tb(DisasContext *s,TranslationBlock *tb; tb = s->tb; -if (__builtin_expect (s->singlestep_enabled,0 )) { +if (__builtin_expect (s->singlestep_enabled &SSTEP_DEBUG ,0 )) {gen_exception (s, dest,EXCP_DEBUG ); }else if ((tb->pc &TARGET_PAGE_MASK ) == (dest &TARGET_PAGE_MASK ) || (s->pc &TARGET_PAGE_MASK ) == (dest &TARGET_PAGE_MASK )) {
@@ -2974,7 +2974,7 @@ gen_intermediate_code_internal(CPUState !env->singlestep_enabled && (pc_offset) < (TARGET_PAGE_SIZE - 32)); -if (__builtin_expect (env->singlestep_enabled,0 )) { +if (__builtin_expect (env->singlestep_enabled &SSTEP_DEBUG ,0 )) {/* Make sure the pc is updated, and raise a debug exception. */ if (!dc->is_jmp) {gen_flush_cc_op (dc);
target-mips/translate.c +1 -1
@@ -7259,7 +7259,7 @@ gen_intermediate_code_internal (CPUStatebreak ;#endif } -if (env->singlestep_enabled) { +if (env->singlestep_enabled &SSTEP_DEBUG ) {save_cpu_state (&ctx, ctx.bstate ==BS_NONE );gen_op_debug (); }else {
target-ppc/translate.c +1 -1
@@ -6319,7 +6319,7 @@ static always_inline int gen_intermediatif (ctx.exception ==POWERPC_EXCP_NONE ) {gen_goto_tb (&ctx,0 , ctx.nip); }else if (ctx.exception !=POWERPC_EXCP_BRANCH ) { -if (unlikely (env->singlestep_enabled)) { +if (unlikely (env->singlestep_enabled &SSTEP_DEBUG )) {gen_update_nip (&ctx, ctx.nip);gen_op_debug (); }
target-sh4/translate.c +3 -3
@@ -161,7 +161,7 @@ static void gen_goto_tb(DisasContext * ctcg_gen_exit_tb ((long ) tb + n); }else {gen_op_movl_imm_PC (dest); -if (ctx->singlestep_enabled) +if (ctx->singlestep_enabled &SSTEP_DEBUG )gen_op_debug ();tcg_gen_exit_tb (0 ); }
@@ -173,7 +173,7 @@ static void gen_jump(DisasContext * ctx)/* Target is not statically known, it comes necessarily from a delayed jump as immediate jump are conditinal jumps */ gen_op_movl_delayed_pc_PC (); -if (ctx->singlestep_enabled) +if (ctx->singlestep_enabled &SSTEP_DEBUG )gen_op_debug ();tcg_gen_exit_tb (0 ); }else {
@@ -1251,7 +1251,7 @@ gen_intermediate_code_internal(CPUStatebreak ;#endif } -if (env->singlestep_enabled) { +if (env->singlestep_enabled &SSTEP_DEBUG ) {gen_op_debug (); }else {switch (ctx.bstate) {
vl.c +4 -3
@@ -7032,9 +7032,10 @@ void main_loop_wait(int timeout)qemu_aio_poll ();if (vm_running) { -if (likely (!(cur_cpu->singlestep_enabled &SSTEP_NOTIMER ))) -qemu_run_timers (&active_timers[QEMU_TIMER_VIRTUAL ], -qemu_get_clock (vm_clock)); +if (likely (!(cur_cpu->singlestep_enabled & + (SSTEP_NOTIMER |SSTEP_INTERNAL )))) +qemu_run_timers (&active_timers[QEMU_TIMER_VIRTUAL ], +qemu_get_clock (vm_clock));/* run dma transfers, if any */ DMA_run (); }
gdbstub.c +2 -2
@@ -77,7 +77,7 @@ typedef struct GDBState {/* By default use no IRQs and no timers while single stepping so as to * make single stepping like an ICE HW step. */ -static int sstep_flags =SSTEP_ENABLE |SSTEP_NOIRQ |SSTEP_NOTIMER ; +static int sstep_flags =SSTEP_DEBUG |SSTEP_NOIRQ |SSTEP_NOTIMER ;#ifdef CONFIG_USER_ONLY /* XXX: This is not thread safe. Do we care? */
@@ -1144,7 +1144,7 @@ static int gdb_handle_packet(GDBState *sif (!strcmp (p,"qemu.sstepbits" )) {/* Query Breakpoint bit definitions */ sprintf (buf,"ENABLE=%x,NOIRQ=%x,NOTIMER=%x" , -SSTEP_ENABLE , +SSTEP_DEBUG ,SSTEP_NOIRQ ,SSTEP_NOTIMER );put_packet (s, buf);
PATCH 2/3 Replace CF_SINGLE_INSN with SSTEP_INTERNAL
In reply to Jan KiszkaWith the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and, thus, tb_gen_code.
Signed-off-by Jan Kiszka <[email protected]>
--- exec.c | 43 ++----------------------------------------- 1 file changed, 2 insertions(+), 41 deletions(-)
exec.c +2 -41
@@ -723,43 +723,6 @@ static void build_page_bitmap(PageDesc * } } -#ifdef TARGET_HAS_PRECISE_SMC - -static void tb_gen_code (CPUState *env, -target_ulong pc,target_ulong cs_base,int flags, -int cflags) -{ -TranslationBlock *tb; -uint8_t *tc_ptr; -target_ulong phys_pc, phys_page2, virt_page2; -int code_gen_size; - - phys_pc =get_phys_addr_code (env, pc); - tb =tb_alloc (pc); -if (!tb) { -/* flush must be done */ -tb_flush (env); -/* cannot fail at this point */ - tb =tb_alloc (pc); - } - tc_ptr = code_gen_ptr; - tb->tc_ptr = tc_ptr; - tb->cs_base = cs_base; - tb->flags = flags; - tb->cflags = cflags; -cpu_gen_code (env, tb, &code_gen_size); - code_gen_ptr = (void *)(((unsigned long )code_gen_ptr + code_gen_size +CODE_GEN_ALIGN -1 ) & ~(CODE_GEN_ALIGN -1 )); - -/* check next page if needed */ - virt_page2 = (pc + tb->size -1 ) &TARGET_PAGE_MASK ; - phys_page2 =-1 ; -if ((pc &TARGET_PAGE_MASK ) != virt_page2) { - phys_page2 =get_phys_addr_code (env, virt_page2); - } -tb_link_phys (tb, phys_pc, phys_page2); -} -#endif -/* invalidate all TBs which intersect with the target physical page starting in range [start;end[. NOTE: start and end must refer to the same physical page. 'is_cpu_write_access' should be true if called
@@ -870,8 +833,7 @@ void tb_invalidate_phys_page_range(targemodifying the memory. It will ensure that it cannot modify itself */ env->current_tb =NULL ; -tb_gen_code (env, current_pc, current_cs_base, current_flags, -CF_SINGLE_INSN ); + env->singlestep_enabled |=SSTEP_INTERNAL ; cpu_resume_from_signal(env, NULL); }#endif
@@ -967,8 +929,7 @@ static void tb_invalidate_phys_page(targmodifying the memory. It will ensure that it cannot modify itself */ env->current_tb =NULL ; -tb_gen_code (env, current_pc, current_cs_base, current_flags, -CF_SINGLE_INSN ); + env->singlestep_enabled |=SSTEP_INTERNAL ; cpu_resume_from_signal(env, puc); }#endif
PATCH 3/3 remove unused TB cflags
In reply to Jan KiszkaTranslationBlock.cflags is unused, now that no one is interested in CF_SINGLE_INSN anymore. Also the other CF_* flags have no users, so let's clean this up.
Signed-off-by Jan Kiszka <[email protected]>
--- exec-all.h | 4 ---- exec.c | 7 ++----- target-i386/translate.c | 6 ++---- 3 files changed, 4 insertions(+), 13 deletions(-)
exec-all.h +0 -4
@@ -123,10 +123,6 @@ typedef struct TranslationBlock {uint64_t flags;/* flags defining in which context the code was generated */ uint16_t size;/* size of target code for this block (1 <= size <= TARGET_PAGE_SIZE) */ -uint16_t cflags;/* compile flags */ -#define CF_TB_FP_USED 0x0002/* fp ops are used in the TB */ -#define CF_FP_USED 0x0004/* fp ops are used in the TB or in a chained TB */ -#define CF_SINGLE_INSN 0x0008/* compile only a single instruction */ uint8_t *tc_ptr;/* pointer to the translated code */ /* next matching tb for physical address. */
exec.c +2 -5
@@ -781,8 +781,7 @@ void tb_invalidate_phys_page_range(targe current_tb =tb_find_pc (env->mem_write_pc); } } -if (current_tb == tb && - !(current_tb->cflags &CF_SINGLE_INSN )) { +if (current_tb == tb) {/* If we are modifying the current TB, we must stop its execution. We could be more precise by checking that the modification is after the current PC, but it
@@ -899,8 +898,7 @@ static void tb_invalidate_phys_page(targ n = (long )tb &3 ; tb = (TranslationBlock *)((long )tb & ~3 );#ifdef TARGET_HAS_PRECISE_SMC -if (current_tb == tb && - !(current_tb->cflags &CF_SINGLE_INSN )) { +if (current_tb == tb) {/* If we are modifying the current TB, we must stop its execution. We could be more precise by checking that the modification is after the current PC, but it
@@ -1002,7 +1000,6 @@ TranslationBlock *tb_alloc(target_ulongreturn NULL ; tb = &tbs[nb_tbs++]; tb->pc = pc; - tb->cflags =0 ;return tb; }
target-i386/translate.c +2 -4
@@ -7101,7 +7101,7 @@ static inline int gen_intermediate_code_DisasContext dc1, *dc = &dc1;target_ulong pc_ptr;uint16_t *gen_opc_end; -int j, lj, cflags; +int j, lj;uint64_t flags;target_ulong pc_start;target_ulong cs_base;
@@ -7110,7 +7110,6 @@ static inline int gen_intermediate_code_ pc_start = tb->pc; cs_base = tb->cs_base; flags = tb->flags; - cflags = tb->cflags; dc->pe = (flags >>HF_PE_SHIFT ) &1 ; dc->code32 = (flags >>HF_CS32_SHIFT ) &1 ;
@@ -7206,8 +7205,7 @@ static inline int gen_intermediate_code_the flag and abort the translation to give the irqs a change to be happen */ if (dc->tf || dc->singlestep_enabled || - (flags &HF_INHIBIT_IRQ_MASK ) || - (cflags &CF_SINGLE_INSN )) { + (flags &HF_INHIBIT_IRQ_MASK )) {gen_jmp_im (pc_ptr - dc->cs_base);gen_eob (dc);break ;
[Qemu-devel] Re: [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2
In reply to Jan Kiszka[ Missed even more cleanup possibilities. ]
With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and, thus, tb_gen_code with its setup code.
Signed-off-by Jan Kiszka <[email protected]>
--- exec.c | 71 +++-------------------------------------------------------------- 1 file changed, 4 insertions(+), 67 deletions(-)
exec.c +4 -67
@@ -723,43 +723,6 @@ static void build_page_bitmap(PageDesc * } } -#ifdef TARGET_HAS_PRECISE_SMC - -static void tb_gen_code (CPUState *env, -target_ulong pc,target_ulong cs_base,int flags, -int cflags) -{ -TranslationBlock *tb; -uint8_t *tc_ptr; -target_ulong phys_pc, phys_page2, virt_page2; -int code_gen_size; - - phys_pc =get_phys_addr_code (env, pc); - tb =tb_alloc (pc); -if (!tb) { -/* flush must be done */ -tb_flush (env); -/* cannot fail at this point */ - tb =tb_alloc (pc); - } - tc_ptr = code_gen_ptr; - tb->tc_ptr = tc_ptr; - tb->cs_base = cs_base; - tb->flags = flags; - tb->cflags = cflags; -cpu_gen_code (env, tb, &code_gen_size); - code_gen_ptr = (void *)(((unsigned long )code_gen_ptr + code_gen_size +CODE_GEN_ALIGN -1 ) & ~(CODE_GEN_ALIGN -1 )); - -/* check next page if needed */ - virt_page2 = (pc + tb->size -1 ) &TARGET_PAGE_MASK ; - phys_page2 =-1 ; -if ((pc &TARGET_PAGE_MASK ) != virt_page2) { - phys_page2 =get_phys_addr_code (env, virt_page2); - } -tb_link_phys (tb, phys_pc, phys_page2); -} -#endif -/* invalidate all TBs which intersect with the target physical page starting in range [start;end[. NOTE: start and end must refer to the same physical page. 'is_cpu_write_access' should be true if called
@@ -768,12 +731,11 @@ static void tb_gen_code(CPUState *env,void tb_invalidate_phys_page_range (target_phys_addr_t start,target_phys_addr_t end,int is_cpu_write_access) { -int n, current_tb_modified, current_tb_not_found, current_flags; +int n, current_tb_modified, current_tb_not_found;CPUState *env = cpu_single_env;PageDesc *p;TranslationBlock *tb, *tb_next, *current_tb, *saved_tb;target_ulong tb_start, tb_end; -target_ulong current_pc, current_cs_base; p =page_find (start >>TARGET_PAGE_BITS );if (!p)
@@ -790,9 +752,6 @@ void tb_invalidate_phys_page_range(targe current_tb_not_found = is_cpu_write_access; current_tb_modified =0 ; current_tb =NULL ;/* avoid warning */ - current_pc =0 ;/* avoid warning */ - current_cs_base =0 ;/* avoid warning */ - current_flags =0 ;/* avoid warning */ tb = p->first_tb;while (tb !=NULL ) { n = (long )tb &3 ;
@@ -829,14 +788,6 @@ void tb_invalidate_phys_page_range(targe current_tb_modified =1 ;cpu_restore_state (current_tb, env, env->mem_write_pc, NULL); -#if defined(TARGET_I386 ) - current_flags = env->hflags; - current_flags |= (env->eflags & (IOPL_MASK |TF_MASK |VM_MASK )); - current_cs_base = (target_ulong )env->segs[R_CS ].base; - current_pc = current_cs_base + env->eip; -#else -#error unsupported CPU -#endif }#endif /* TARGET_HAS_PRECISE_SMC */ /* we need to do that to handle the case where a signal
@@ -870,8 +821,7 @@ void tb_invalidate_phys_page_range(targemodifying the memory. It will ensure that it cannot modify itself */ env->current_tb =NULL ; -tb_gen_code (env, current_pc, current_cs_base, current_flags, -CF_SINGLE_INSN ); + env->singlestep_enabled |=SSTEP_INTERNAL ; cpu_resume_from_signal(env, NULL); }#endif
@@ -910,8 +860,7 @@ static inline void tb_invalidate_phys_pastatic void tb_invalidate_phys_page (target_phys_addr_t addr,unsigned long pc,void *puc) { -int n, current_flags, current_tb_modified; -target_ulong current_pc, current_cs_base; +int n, current_tb_modified;PageDesc *p;TranslationBlock *tb, *current_tb;#ifdef TARGET_HAS_PRECISE_SMC
@@ -925,9 +874,6 @@ static void tb_invalidate_phys_page(targ tb = p->first_tb; current_tb_modified =0 ; current_tb =NULL ; - current_pc =0 ;/* avoid warning */ - current_cs_base =0 ;/* avoid warning */ - current_flags =0 ;/* avoid warning */ #ifdef TARGET_HAS_PRECISE_SMC if (tb && pc !=0 ) { current_tb =tb_find_pc (pc);
@@ -947,14 +893,6 @@ static void tb_invalidate_phys_page(targ current_tb_modified =1 ; cpu_restore_state(current_tb, env, pc, puc); -#if defined(TARGET_I386 ) - current_flags = env->hflags; - current_flags |= (env->eflags & (IOPL_MASK |TF_MASK |VM_MASK )); - current_cs_base = (target_ulong )env->segs[R_CS ].base; - current_pc = current_cs_base + env->eip; -#else -#error unsupported CPU -#endif }#endif /* TARGET_HAS_PRECISE_SMC */ tb_phys_invalidate (tb, addr);
@@ -967,8 +905,7 @@ static void tb_invalidate_phys_page(targmodifying the memory. It will ensure that it cannot modify itself */ env->current_tb =NULL ; -tb_gen_code (env, current_pc, current_cs_base, current_flags, -CF_SINGLE_INSN ); + env->singlestep_enabled |=SSTEP_INTERNAL ; cpu_resume_from_signal(env, puc); }#endif
Re: [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL
In reply to Jan KiszkaQuoted 2 lines · Jan Kiszka
With the help of
SSTEP_INTERNAL, we can overcomeCF_SINGLE_INSNand, thus,tb_gen_code.
Does your code avoid looping if an instruction modifies itself ?
Fabrice.
PATCH 2/3 Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2
Quoted 5 lines · Fabrice Bellard
Quoted 2 lines · Jan Kiszka
With the help of
SSTEP_INTERNAL, we can overcomeCF_SINGLE_INSNand, thus,tb_gen_code.Does your code avoid looping if an instruction modifies itself ?
Err, well, of course not. Not completely understanding what I patched, I missed the meaning of those two CF_SINGLE_INSN checks. Version below gets this right. Successfully tested with qemu-x86_64 system&userspace. An updated patch 3 will follow, too. Thanks!
------------
With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and, thus, tb_gen_code with its setup code.
Signed-off-by Jan Kiszka <[email protected]>
--- exec.c | 75 +++++------------------------------------------------------------ 1 file changed, 6 insertions(+), 69 deletions(-)
exec.c +6 -69
@@ -723,43 +723,6 @@ static void build_page_bitmap(PageDesc * } } -#ifdef TARGET_HAS_PRECISE_SMC - -static void tb_gen_code (CPUState *env, -target_ulong pc,target_ulong cs_base,int flags, -int cflags) -{ -TranslationBlock *tb; -uint8_t *tc_ptr; -target_ulong phys_pc, phys_page2, virt_page2; -int code_gen_size; - - phys_pc =get_phys_addr_code (env, pc); - tb =tb_alloc (pc); -if (!tb) { -/* flush must be done */ -tb_flush (env); -/* cannot fail at this point */ - tb =tb_alloc (pc); - } - tc_ptr = code_gen_ptr; - tb->tc_ptr = tc_ptr; - tb->cs_base = cs_base; - tb->flags = flags; - tb->cflags = cflags; -cpu_gen_code (env, tb, &code_gen_size); - code_gen_ptr = (void *)(((unsigned long )code_gen_ptr + code_gen_size +CODE_GEN_ALIGN -1 ) & ~(CODE_GEN_ALIGN -1 )); - -/* check next page if needed */ - virt_page2 = (pc + tb->size -1 ) &TARGET_PAGE_MASK ; - phys_page2 =-1 ; -if ((pc &TARGET_PAGE_MASK ) != virt_page2) { - phys_page2 =get_phys_addr_code (env, virt_page2); - } -tb_link_phys (tb, phys_pc, phys_page2); -} -#endif -/* invalidate all TBs which intersect with the target physical page starting in range [start;end[. NOTE: start and end must refer to the same physical page. 'is_cpu_write_access' should be true if called
@@ -768,12 +731,11 @@ static void tb_gen_code(CPUState *env,void tb_invalidate_phys_page_range (target_phys_addr_t start,target_phys_addr_t end,int is_cpu_write_access) { -int n, current_tb_modified, current_tb_not_found, current_flags; +int n, current_tb_modified, current_tb_not_found;CPUState *env = cpu_single_env;PageDesc *p;TranslationBlock *tb, *tb_next, *current_tb, *saved_tb;target_ulong tb_start, tb_end; -target_ulong current_pc, current_cs_base; p =page_find (start >>TARGET_PAGE_BITS );if (!p)
@@ -790,9 +752,6 @@ void tb_invalidate_phys_page_range(targe current_tb_not_found = is_cpu_write_access; current_tb_modified =0 ; current_tb =NULL ;/* avoid warning */ - current_pc =0 ;/* avoid warning */ - current_cs_base =0 ;/* avoid warning */ - current_flags =0 ;/* avoid warning */ tb = p->first_tb;while (tb !=NULL ) { n = (long )tb &3 ;
@@ -819,7 +778,7 @@ void tb_invalidate_phys_page_range(targe } }if (current_tb == tb && - !(current_tb->cflags &CF_SINGLE_INSN )) { + !(env->singlestep_enabled &SSTEP_INTERNAL )) {/* If we are modifying the current TB, we must stop its execution. We could be more precise by checking that the modification is after the current PC, but it
@@ -829,14 +788,6 @@ void tb_invalidate_phys_page_range(targe current_tb_modified =1 ;cpu_restore_state (current_tb, env, env->mem_write_pc, NULL); -#if defined(TARGET_I386 ) - current_flags = env->hflags; - current_flags |= (env->eflags & (IOPL_MASK |TF_MASK |VM_MASK )); - current_cs_base = (target_ulong )env->segs[R_CS ].base; - current_pc = current_cs_base + env->eip; -#else -#error unsupported CPU -#endif }#endif /* TARGET_HAS_PRECISE_SMC */ /* we need to do that to handle the case where a signal
@@ -870,8 +821,7 @@ void tb_invalidate_phys_page_range(targemodifying the memory. It will ensure that it cannot modify itself */ env->current_tb =NULL ; -tb_gen_code (env, current_pc, current_cs_base, current_flags, -CF_SINGLE_INSN ); + env->singlestep_enabled |=SSTEP_INTERNAL ; cpu_resume_from_signal(env, NULL); }#endif
@@ -910,8 +860,7 @@ static inline void tb_invalidate_phys_pastatic void tb_invalidate_phys_page (target_phys_addr_t addr,unsigned long pc,void *puc) { -int n, current_flags, current_tb_modified; -target_ulong current_pc, current_cs_base; +int n, current_tb_modified;PageDesc *p;TranslationBlock *tb, *current_tb;#ifdef TARGET_HAS_PRECISE_SMC
@@ -925,9 +874,6 @@ static void tb_invalidate_phys_page(targ tb = p->first_tb; current_tb_modified =0 ; current_tb =NULL ; - current_pc =0 ;/* avoid warning */ - current_cs_base =0 ;/* avoid warning */ - current_flags =0 ;/* avoid warning */ #ifdef TARGET_HAS_PRECISE_SMC if (tb && pc !=0 ) { current_tb =tb_find_pc (pc);
@@ -938,7 +884,7 @@ static void tb_invalidate_phys_page(targ tb = (TranslationBlock *)((long )tb & ~3 );#ifdef TARGET_HAS_PRECISE_SMC if (current_tb == tb && - !(current_tb->cflags &CF_SINGLE_INSN )) { + !(env->singlestep_enabled &SSTEP_INTERNAL )) {/* If we are modifying the current TB, we must stop its execution. We could be more precise by checking that the modification is after the current PC, but it
@@ -947,14 +893,6 @@ static void tb_invalidate_phys_page(targ current_tb_modified =1 ; cpu_restore_state(current_tb, env, pc, puc); -#if defined(TARGET_I386 ) - current_flags = env->hflags; - current_flags |= (env->eflags & (IOPL_MASK |TF_MASK |VM_MASK )); - current_cs_base = (target_ulong )env->segs[R_CS ].base; - current_pc = current_cs_base + env->eip; -#else -#error unsupported CPU -#endif }#endif /* TARGET_HAS_PRECISE_SMC */ tb_phys_invalidate (tb, addr);
@@ -967,8 +905,7 @@ static void tb_invalidate_phys_page(targmodifying the memory. It will ensure that it cannot modify itself */ env->current_tb =NULL ; -tb_gen_code (env, current_pc, current_cs_base, current_flags, -CF_SINGLE_INSN ); + env->singlestep_enabled |=SSTEP_INTERNAL ; cpu_resume_from_signal(env, puc); }#endif
PATCH 3/3 remove unused TB cflags - v2
In reply to Jan Kiszka[ updated after fixing patch 2 ]
TranslationBlock.cflags is unused, now that no one is interested in CF_SINGLE_INSN anymore. Also the other CF_* flags have no users, so let's clean this up.
Signed-off-by Jan Kiszka <[email protected]>
--- exec-all.h | 4 ---- exec.c | 1 - target-i386/translate.c | 6 ++---- 3 files changed, 2 insertions(+), 9 deletions(-)
exec-all.h +0 -4
@@ -123,10 +123,6 @@ typedef struct TranslationBlock {uint64_t flags;/* flags defining in which context the code was generated */ uint16_t size;/* size of target code for this block (1 <= size <= TARGET_PAGE_SIZE) */ -uint16_t cflags;/* compile flags */ -#define CF_TB_FP_USED 0x0002/* fp ops are used in the TB */ -#define CF_FP_USED 0x0004/* fp ops are used in the TB or in a chained TB */ -#define CF_SINGLE_INSN 0x0008/* compile only a single instruction */ uint8_t *tc_ptr;/* pointer to the translated code */ /* next matching tb for physical address. */
exec.c +0 -1
@@ -978,7 +978,6 @@ TranslationBlock *tb_alloc(target_ulongreturn NULL ; tb = &tbs[nb_tbs++]; tb->pc = pc; - tb->cflags =0 ;return tb; }
target-i386/translate.c +2 -4
@@ -7101,7 +7101,7 @@ static inline int gen_intermediate_code_DisasContext dc1, *dc = &dc1;target_ulong pc_ptr;uint16_t *gen_opc_end; -int j, lj, cflags; +int j, lj;uint64_t flags;target_ulong pc_start;target_ulong cs_base;
@@ -7110,7 +7110,6 @@ static inline int gen_intermediate_code_ pc_start = tb->pc; cs_base = tb->cs_base; flags = tb->flags; - cflags = tb->cflags; dc->pe = (flags >>HF_PE_SHIFT ) &1 ; dc->code32 = (flags >>HF_CS32_SHIFT ) &1 ;
@@ -7206,8 +7205,7 @@ static inline int gen_intermediate_code_the flag and abort the translation to give the irqs a change to be happen */ if (dc->tf || dc->singlestep_enabled || - (flags &HF_INHIBIT_IRQ_MASK ) || - (cflags &CF_SINGLE_INSN )) { + (flags &HF_INHIBIT_IRQ_MASK )) {gen_jmp_im (pc_ptr - dc->cs_base);gen_eob (dc);break ;