Re: [Qemu-devel] [RFC PATCH v4 11/25] cpu-exec: allow temporary disabling icount

[Paolo Bonzini]

On 07/11/2014 11:32, Pavel Dovgalyuk wrote:
> This patch is required for deterministic replay to generate an exception
> by trying executing an instruction without changing icount.
> It adds new flag to TB for disabling icount while translating it.
> 
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> ---
>  cpu-exec.c              |    6 +++---
>  include/exec/exec-all.h |    7 ++++---
>  translate-all.c         |   22 ++++++++++++++++++++--
>  3 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 011f51f..e7206ac 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -198,7 +198,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, 
> uint8_t *tb_ptr)
>  /* Execute the code without caching the generated code. An interpreter
>     could be used if available. */
>  static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
> -                             TranslationBlock *orig_tb)
> +                             TranslationBlock *orig_tb, bool ignore_icount)
>  {
>      CPUState *cpu = ENV_GET_CPU(env);
>      TranslationBlock *tb;
> @@ -214,7 +214,7 @@ static void cpu_exec_nocache(CPUArchState *env, int 
> max_cycles,
>      /* tb_gen_code can flush our orig_tb, invalidate it now */
>      tb_phys_invalidate(orig_tb, -1);
>      tb = tb_gen_code(cpu, pc, cs_base, flags,
> -                     max_cycles);
> +                     max_cycles | (ignore_icount ? CF_IGNORE_ICOUNT : 0));
>      cpu->current_tb = tb;
>      /* execute the generated code */
>      trace_exec_tb_nocache(tb, tb->pc);
> @@ -516,7 +516,7 @@ int cpu_exec(CPUArchState *env)
>                          } else {
>                              if (insns_left > 0) {
>                                  /* Execute remaining instructions.  */
> -                                cpu_exec_nocache(env, insns_left, tb);
> +                                cpu_exec_nocache(env, insns_left, tb, false);
>                                  align_clocks(&sc, cpu);
>                              }
>                              cpu->exception_index = EXCP_INTERRUPT;
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 1d17f75..052e8ef 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -142,9 +142,10 @@ 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_COUNT_MASK  0x7fff
> -#define CF_LAST_IO     0x8000 /* Last insn may be an IO access.  */
> +    uint32_t cflags;    /* compile flags */
> +#define CF_COUNT_MASK    0x7fff
> +#define CF_LAST_IO       0x8000 /* Last insn may be an IO access.  */
> +#define CF_IGNORE_ICOUNT 0x10000 /* Do not generate icount code */
>  
>      void *tc_ptr;    /* pointer to the translated code */
>      /* next matching tb for physical address. */
> diff --git a/translate-all.c b/translate-all.c
> index 7177b71..182ea7d 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -207,6 +207,7 @@ static int cpu_restore_state_from_tb(CPUState *cpu, 
> TranslationBlock *tb,
>      TCGContext *s = &tcg_ctx;
>      int j;
>      uintptr_t tc_ptr;
> +    int old_icount = use_icount;
>  #ifdef CONFIG_PROFILER
>      int64_t ti;
>  #endif
> @@ -218,6 +219,10 @@ static int cpu_restore_state_from_tb(CPUState *cpu, 
> TranslationBlock *tb,
>  
>      gen_intermediate_code_pc(env, tb);
>  
> +    if (tb->cflags & CF_IGNORE_ICOUNT) {

Would it be possible to instead have a CF_USE_ICOUNT tbflag, and check
it when applicable instead of use_icount?  It seems cleaner, because it
would avoid playing with global variables, but perhaps it's hard to do it.

Paolo

> +        use_icount = 0;
> +    }
> +
>      if (use_icount) {
>          /* Reset the cycle counter to the start of the block.  */
>          cpu->icount_decr.u16.low += tb->icount;
> @@ -227,8 +232,10 @@ static int cpu_restore_state_from_tb(CPUState *cpu, 
> TranslationBlock *tb,
>  
>      /* find opc index corresponding to search_pc */
>      tc_ptr = (uintptr_t)tb->tc_ptr;
> -    if (searched_pc < tc_ptr)
> +    if (searched_pc < tc_ptr) {
> +        use_icount = old_icount;
>          return -1;
> +    }
>  
>      s->tb_next_offset = tb->tb_next_offset;
>  #ifdef USE_DIRECT_JUMP
> @@ -240,8 +247,10 @@ static int cpu_restore_state_from_tb(CPUState *cpu, 
> TranslationBlock *tb,
>  #endif
>      j = tcg_gen_code_search_pc(s, (tcg_insn_unit *)tc_ptr,
>                                 searched_pc - tc_ptr);
> -    if (j < 0)
> +    if (j < 0) {
> +        use_icount = old_icount;
>          return -1;
> +    }
>      /* now find start of instruction before */
>      while (s->gen_opc_instr_start[j] == 0) {
>          j--;
> @@ -254,6 +263,8 @@ static int cpu_restore_state_from_tb(CPUState *cpu, 
> TranslationBlock *tb,
>      s->restore_time += profile_getclock() - ti;
>      s->restore_count++;
>  #endif
> +    use_icount = old_icount;
> +
>      return 0;
>  }
>  
> @@ -264,6 +275,8 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr)
>      tb = tb_find_pc(retaddr);
>      if (tb) {
>          cpu_restore_state_from_tb(cpu, tb, retaddr);
> +        /* tb could be temporary, generated by exec nocache */
> +        tb_phys_invalidate(tb, -1);
>          return true;
>      }
>      return false;
> @@ -1045,6 +1058,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>      tb_page_addr_t phys_pc, phys_page2;
>      target_ulong virt_page2;
>      int code_gen_size;
> +    int old_icount = use_icount;
>  
>      phys_pc = get_page_addr_code(env, pc);
>      tb = tb_alloc(pc);
> @@ -1060,9 +1074,13 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>      tb->cs_base = cs_base;
>      tb->flags = flags;
>      tb->cflags = cflags;
> +    if (cflags & CF_IGNORE_ICOUNT) {
> +        use_icount = 0;
> +    }
>      cpu_gen_code(env, tb, &code_gen_size);
>      tcg_ctx.code_gen_ptr = (void *)(((uintptr_t)tcg_ctx.code_gen_ptr +
>              code_gen_size + CODE_GEN_ALIGN - 1) & ~(CODE_GEN_ALIGN - 1));
> +    use_icount = old_icount;
>  
>      /* check next page if needed */
>      virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
>