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:
>          cpu_restore_state_from_tb(cpu, tb, retaddr);
> +        /* tb could be temporary, generated by exec nocache */
> +        tb_phys_invalidate(tb, -1);

Would you need to do the same sequence as cpu_exec_nocache in this case?

    cpu->current_tb = NULL;
    tb_phys_invalidate(tb, -1);
    tb_free(tb);

In this case, perhaps something like this would be better:

diff --git a/cpu-exec.c b/cpu-exec.c
index 10c0f42..faf8041 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -209,7 +209,7 @@ static void cpu_exec_nocache(CPUArchState *env, int 
max_cycles,
         max_cycles = CF_COUNT_MASK;
 
     tb = tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
-                     max_cycles);
+                     max_cycles | CF_NOCACHE);
     cpu->current_tb = tb;
     /* execute the generated code */
     trace_exec_tb_nocache(tb, tb->pc);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 9b67d16..41464f3 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -148,6 +148,7 @@ struct TranslationBlock {
 #define CF_COUNT_MASK  0x7fff
 #define CF_LAST_IO     0x8000 /* Last insn may be an IO access.  */
 #define CF_LAST_IO     0x8000 /* Last insn may be an IO access.  */
+#define CF_NOCACHE     0x40000 /* To be freed after execution */
 
     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 1a27df7..6ea4da4 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -264,6 +264,12 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr)
     tb = tb_find_pc(retaddr);
     if (tb) {
         cpu_restore_state_from_tb(cpu, tb, retaddr);
+        if (tb->cflags & CF_NOCACHE) {
+            /* one-shot translation, invalidate it immediately */
+            cpu->current_tb = NULL;
+            tb_phys_invalidate(tb, -1);
+            tb_free(tb);
+        }
         return true;
     }
     return false;


I pushed it to an icount branch on my github repository, together with
other patches that introduce CF_USE_ICOUNT instead of touching the
use_icount global variable.  Can you please take a look and, if it
works, incorporate it in your patch series?

Thanks,

Paolo