| Message ID | 1458222382-6498-6-git-send-email-sergey.fedorov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
sergey.fedorov@linaro.org writes: > From: Paolo Bonzini <pbonzini@redhat.com> > > This is a baby step towards making tb_flush thread safe. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org> > --- > cpu-exec.c | 11 +++++------ > include/exec/exec-all.h | 2 -- > include/qom/cpu.h | 2 ++ > translate-all.c | 3 +-- > 4 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/cpu-exec.c b/cpu-exec.c > index f90482eff778..07545aa91082 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -195,10 +195,11 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles, > if (max_cycles > CF_COUNT_MASK) > max_cycles = CF_COUNT_MASK; > > + cpu->tb_invalidated_flag = 0; We've declared as bool so lets use true/false instead of 1/0's > tb = tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base, orig_tb->flags, > max_cycles | CF_NOCACHE > | (ignore_icount ? CF_IGNORE_ICOUNT : 0)); > - tb->orig_tb = tcg_ctx.tb_ctx.tb_invalidated_flag ? NULL : orig_tb; > + tb->orig_tb = cpu->tb_invalidated_flag ? NULL : orig_tb; > cpu->current_tb = tb; > /* execute the generated code */ > trace_exec_tb_nocache(tb, tb->pc); > @@ -219,8 +220,6 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, > tb_page_addr_t phys_pc, phys_page1; > target_ulong virt_page2; > > - tcg_ctx.tb_ctx.tb_invalidated_flag = 0; > - > /* find translated block using physical mappings */ > phys_pc = get_page_addr_code(env, pc); > phys_page1 = phys_pc & TARGET_PAGE_MASK; > @@ -288,6 +287,7 @@ static TranslationBlock *tb_find_slow(CPUState *cpu, > #endif > > /* if no translated code available, then translate it now */ > + cpu->tb_invalidated_flag = 0; > tb = tb_gen_code(cpu, pc, cs_base, flags, 0); > > #ifdef CONFIG_USER_ONLY > @@ -493,12 +493,11 @@ int cpu_exec(CPUState *cpu) > tb = tb_find_fast(cpu); > /* Note: we do it here to avoid a gcc bug on Mac OS X when > doing it in tb_find_slow */ > - if (tcg_ctx.tb_ctx.tb_invalidated_flag) { > + if (cpu->tb_invalidated_flag) { > /* as some TB could have been invalidated because > - of memory exceptions while generating the code, we > + of a tb_flush while generating the code, we > must recompute the hash index here */ > next_tb = 0; > - tcg_ctx.tb_ctx.tb_invalidated_flag = 0; > } > if (qemu_loglevel_mask(CPU_LOG_EXEC)) { > qemu_log("Trace %p [" TARGET_FMT_lx "] %s\n", > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 05a151da4a54..0ef6ea5cf6dc 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -288,8 +288,6 @@ struct TBContext { > /* statistics */ > int tb_flush_count; > int tb_phys_invalidate_count; > - > - int tb_invalidated_flag; > }; > > void tb_free(TranslationBlock *tb); > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 7052eee7b78a..9538f9cc2af3 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -240,6 +240,7 @@ struct kvm_run; > * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU > * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this > * CPU and return to its top level loop. > + * @tb_invalidated_flag: Set to tell TCG that tb_flush has been called. > * @singlestep_enabled: Flags for single-stepping. > * @icount_extra: Instructions until next timer event. > * @icount_decr: Number of cycles left, with interrupt flag in high bit. > @@ -291,6 +292,7 @@ struct CPUState { > bool stopped; > bool crash_occurred; > bool exit_request; > + bool tb_invalidated_flag; s/_flag// would save a few characters given it should be obvious it is a flag from the setting of true and false? > uint32_t interrupt_request; > int singlestep_enabled; > int64_t icount_extra; > diff --git a/translate-all.c b/translate-all.c > index 1db5a914d9a3..8e1edd6bb633 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -843,6 +843,7 @@ void tb_flush(CPUState *cpu) > tcg_ctx.tb_ctx.nb_tbs = 0; > > CPU_FOREACH(cpu) { > + cpu->tb_invalidated_flag = 1; > memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache)); > } > > @@ -1079,8 +1080,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu, > /* cannot fail at this point */ > tb = tb_alloc(pc); > assert(tb != NULL); > - /* Don't forget to invalidate previous TB info. */ > - tcg_ctx.tb_ctx.tb_invalidated_flag = 1; > } > > gen_code_buf = tcg_ctx.code_gen_ptr; I also note there is some code motion about where these flags are set and cleared which should probably be mentioned in the commit message. -- Alex Bennée
On 22/03/16 18:07, Alex Bennée wrote: > sergey.fedorov@linaro.org writes: (snip) >> diff --git a/cpu-exec.c b/cpu-exec.c >> index f90482eff778..07545aa91082 100644 >> --- a/cpu-exec.c >> +++ b/cpu-exec.c >> @@ -195,10 +195,11 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles, >> if (max_cycles > CF_COUNT_MASK) >> max_cycles = CF_COUNT_MASK; >> >> + cpu->tb_invalidated_flag = 0; > We've declared as bool so lets use true/false instead of 1/0's > (snip) >> diff --git a/include/qom/cpu.h b/include/qom/cpu.h >> index 7052eee7b78a..9538f9cc2af3 100644 >> --- a/include/qom/cpu.h >> +++ b/include/qom/cpu.h >> @@ -240,6 +240,7 @@ struct kvm_run; >> * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU >> * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this >> * CPU and return to its top level loop. >> + * @tb_invalidated_flag: Set to tell TCG that tb_flush has been called. >> * @singlestep_enabled: Flags for single-stepping. >> * @icount_extra: Instructions until next timer event. >> * @icount_decr: Number of cycles left, with interrupt flag in high bit. >> @@ -291,6 +292,7 @@ struct CPUState { >> bool stopped; >> bool crash_occurred; >> bool exit_request; >> + bool tb_invalidated_flag; > s/_flag// would save a few characters given it should be obvious it is a > flag from the setting of true and false? > >> (snip) >> diff --git a/translate-all.c b/translate-all.c >> index 1db5a914d9a3..8e1edd6bb633 100644 >> --- a/translate-all.c >> +++ b/translate-all.c >> @@ -843,6 +843,7 @@ void tb_flush(CPUState *cpu) >> tcg_ctx.tb_ctx.nb_tbs = 0; >> >> CPU_FOREACH(cpu) { >> + cpu->tb_invalidated_flag = 1; >> memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache)); >> } >> >> @@ -1079,8 +1080,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu, >> /* cannot fail at this point */ >> tb = tb_alloc(pc); >> assert(tb != NULL); >> - /* Don't forget to invalidate previous TB info. */ >> - tcg_ctx.tb_ctx.tb_invalidated_flag = 1; >> } >> >> gen_code_buf = tcg_ctx.code_gen_ptr; > I also note there is some code motion about where these flags are set > and cleared which should probably be mentioned in the commit message. > Agree with all the comments. Kind regards, Sergey
diff --git a/cpu-exec.c b/cpu-exec.c index f90482eff778..07545aa91082 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -195,10 +195,11 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles, if (max_cycles > CF_COUNT_MASK) max_cycles = CF_COUNT_MASK; + cpu->tb_invalidated_flag = 0; tb = tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base, orig_tb->flags, max_cycles | CF_NOCACHE | (ignore_icount ? CF_IGNORE_ICOUNT : 0)); - tb->orig_tb = tcg_ctx.tb_ctx.tb_invalidated_flag ? NULL : orig_tb; + tb->orig_tb = cpu->tb_invalidated_flag ? NULL : orig_tb; cpu->current_tb = tb; /* execute the generated code */ trace_exec_tb_nocache(tb, tb->pc); @@ -219,8 +220,6 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, tb_page_addr_t phys_pc, phys_page1; target_ulong virt_page2; - tcg_ctx.tb_ctx.tb_invalidated_flag = 0; - /* find translated block using physical mappings */ phys_pc = get_page_addr_code(env, pc); phys_page1 = phys_pc & TARGET_PAGE_MASK; @@ -288,6 +287,7 @@ static TranslationBlock *tb_find_slow(CPUState *cpu, #endif /* if no translated code available, then translate it now */ + cpu->tb_invalidated_flag = 0; tb = tb_gen_code(cpu, pc, cs_base, flags, 0); #ifdef CONFIG_USER_ONLY @@ -493,12 +493,11 @@ int cpu_exec(CPUState *cpu) tb = tb_find_fast(cpu); /* Note: we do it here to avoid a gcc bug on Mac OS X when doing it in tb_find_slow */ - if (tcg_ctx.tb_ctx.tb_invalidated_flag) { + if (cpu->tb_invalidated_flag) { /* as some TB could have been invalidated because - of memory exceptions while generating the code, we + of a tb_flush while generating the code, we must recompute the hash index here */ next_tb = 0; - tcg_ctx.tb_ctx.tb_invalidated_flag = 0; } if (qemu_loglevel_mask(CPU_LOG_EXEC)) { qemu_log("Trace %p [" TARGET_FMT_lx "] %s\n", diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 05a151da4a54..0ef6ea5cf6dc 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -288,8 +288,6 @@ struct TBContext { /* statistics */ int tb_flush_count; int tb_phys_invalidate_count; - - int tb_invalidated_flag; }; void tb_free(TranslationBlock *tb); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 7052eee7b78a..9538f9cc2af3 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -240,6 +240,7 @@ struct kvm_run; * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this * CPU and return to its top level loop. + * @tb_invalidated_flag: Set to tell TCG that tb_flush has been called. * @singlestep_enabled: Flags for single-stepping. * @icount_extra: Instructions until next timer event. * @icount_decr: Number of cycles left, with interrupt flag in high bit. @@ -291,6 +292,7 @@ struct CPUState { bool stopped; bool crash_occurred; bool exit_request; + bool tb_invalidated_flag; uint32_t interrupt_request; int singlestep_enabled; int64_t icount_extra; diff --git a/translate-all.c b/translate-all.c index 1db5a914d9a3..8e1edd6bb633 100644 --- a/translate-all.c +++ b/translate-all.c @@ -843,6 +843,7 @@ void tb_flush(CPUState *cpu) tcg_ctx.tb_ctx.nb_tbs = 0; CPU_FOREACH(cpu) { + cpu->tb_invalidated_flag = 1; memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache)); } @@ -1079,8 +1080,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu, /* cannot fail at this point */ tb = tb_alloc(pc); assert(tb != NULL); - /* Don't forget to invalidate previous TB info. */ - tcg_ctx.tb_ctx.tb_invalidated_flag = 1; } gen_code_buf = tcg_ctx.code_gen_ptr;