[Qemu-devel] [RFC 06/10] protect TBContext with tb_lock.

[fred . konrad]

From: KONRAD Frederic <address@hidden>

This protects TBContext with tb_lock to make tb_* thread safe.

We can still have issue with tb_flush in case of multithread TCG:
  An other CPU can be executing code during a flush.

This can be fixed later by making all other TCG thread exiting before calling
tb_flush().

Signed-off-by: KONRAD Frederic <address@hidden>
---
 cpu-exec.c      |  16 ++++++++-
 translate-all.c | 100 +++++++++++++++++++++++++++++++++++++-------------------
 2 files changed, 82 insertions(+), 34 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 4d22252..68654e3 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -237,6 +237,7 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
     tb_page_addr_t phys_pc, phys_page1;
     target_ulong virt_page2;
 
+    qemu_mutex_lock(&tb_ctx.tb_lock);
     tb_ctx.tb_invalidated_flag = 0;
 
     /* find translated block using physical mappings */
@@ -268,8 +269,14 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
         ptb1 = &tb->phys_hash_next;
     }
  not_found:
+   /*
+    * FIXME: We need to release this mutex because tb_gen_code needs it.
+    * This can be optimised by adding a flag to tb_gen_code?
+    */
+   qemu_mutex_unlock(&tb_ctx.tb_lock);
    /* if no translated code available, then translate it now */
-    tb = tb_gen_code(cpu, pc, cs_base, flags, 0);
+   tb = tb_gen_code(cpu, pc, cs_base, flags, 0);
+   qemu_mutex_lock(&tb_ctx.tb_lock);
 
  found:
     /* Move the last found TB to the head of the list */
@@ -280,6 +287,7 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
     }
     /* we add the TB in the virtual pc hash table */
     cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)] = tb;
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
     return tb;
 }
 
@@ -460,6 +468,9 @@ int cpu_exec(CPUArchState *env)
                 tb = tb_find_fast(env);
                 /* Note: we do it here to avoid a gcc bug on Mac OS X when
                    doing it in tb_find_slow */
+#if !defined(CONFIG_USER_ONLY)
+                qemu_mutex_lock(&tb_ctx.tb_lock);
+#endif
                 if (tb_ctx.tb_invalidated_flag) {
                     /* as some TB could have been invalidated because
                        of memory exceptions while generating the code, we
@@ -467,6 +478,9 @@ int cpu_exec(CPUArchState *env)
                     next_tb = 0;
                     tb_ctx.tb_invalidated_flag = 0;
                 }
+#if !defined(CONFIG_USER_ONLY)
+                qemu_mutex_unlock(&tb_ctx.tb_lock);
+#endif
                 if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
                     qemu_log("Trace %p [" TARGET_FMT_lx "] %s\n",
                              tb->tc_ptr, tb->pc, lookup_symbol(tb->pc));
diff --git a/translate-all.c b/translate-all.c
index e393d30..68505c0 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -689,6 +689,7 @@ static inline void code_gen_alloc(size_t tb_size)
             CODE_GEN_AVG_BLOCK_SIZE;
     tb_ctx.tbs = g_malloc(tcg_ctx.code_gen_max_blocks
                           * sizeof(TranslationBlock));
+    qemu_mutex_init(&tb_ctx.tb_lock);
 }
 
 /* Must be called before using the QEMU cpus. 'tb_size' is the size
@@ -713,20 +714,23 @@ bool tcg_enabled(void)
     return tcg_ctx.code_gen_buffer != NULL;
 }
 
-/* Allocate a new translation block. Flush the translation buffer if
-   too many translation blocks or too much generated code. */
+/*
+ * Allocate a new translation block. Flush the translation buffer if
+ * too many translation blocks or too much generated code.
+ * tb_alloc is not thread safe but tb_gen_code is protected by a mutex so this
+ * function is called only by one thread.
+ */
 static TranslationBlock *tb_alloc(target_ulong pc)
 {
-    TranslationBlock *tb;
+    TranslationBlock *tb = NULL;
 
-    if (tb_ctx.nb_tbs >= tcg_ctx.code_gen_max_blocks ||
-        (tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer) >=
+    if (tb_ctx.nb_tbs < tcg_ctx.code_gen_max_blocks &&
+        (tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer) <
          tcg_ctx.code_gen_buffer_max_size) {
-        return NULL;
+        tb = &tb_ctx.tbs[tb_ctx.nb_tbs++];
+        tb->pc = pc;
+        tb->cflags = 0;
     }
-    tb = &tb_ctx.tbs[tb_ctx.nb_tbs++];
-    tb->pc = pc;
-    tb->cflags = 0;
     return tb;
 }
 
@@ -735,11 +739,16 @@ void tb_free(TranslationBlock *tb)
     /* In practice this is mostly used for single use temporary TB
        Ignore the hard cases and just back up if this TB happens to
        be the last one generated.  */
+
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
     if (tb_ctx.nb_tbs > 0 &&
             tb == &tb_ctx.tbs[tb_ctx.nb_tbs - 1]) {
         tcg_ctx.code_gen_ptr = tb->tc_ptr;
         tb_ctx.nb_tbs--;
     }
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
 }
 
 static inline void invalidate_page_bitmap(PageDesc *p)
@@ -792,6 +801,8 @@ void tb_flush(CPUArchState *env1)
 {
     CPUState *cpu = ENV_GET_CPU(env1);
 
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
 #if defined(DEBUG_FLUSH)
     printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
            (unsigned long)(tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer),
@@ -816,6 +827,8 @@ void tb_flush(CPUArchState *env1)
     /* XXX: flush processor icache at this point if cache flush is
        expensive */
     tb_ctx.tb_flush_count++;
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
 }
 
 #ifdef DEBUG_TB_CHECK
@@ -825,6 +838,8 @@ static void tb_invalidate_check(target_ulong address)
     TranslationBlock *tb;
     int i;
 
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
     address &= TARGET_PAGE_MASK;
     for (i = 0; i < CODE_GEN_PHYS_HASH_SIZE; i++) {
         for (tb = tb_ctx.tb_phys_hash[i]; tb != NULL; tb = tb->phys_hash_next) 
{
@@ -836,6 +851,8 @@ static void tb_invalidate_check(target_ulong address)
             }
         }
     }
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
 }
 
 /* verify that all the pages have correct rights for code */
@@ -844,6 +861,8 @@ static void tb_page_check(void)
     TranslationBlock *tb;
     int i, flags1, flags2;
 
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
     for (i = 0; i < CODE_GEN_PHYS_HASH_SIZE; i++) {
         for (tb = tb_ctx.tb_phys_hash[i]; tb != NULL;
                 tb = tb->phys_hash_next) {
@@ -855,6 +874,8 @@ static void tb_page_check(void)
             }
         }
     }
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
 }
 
 #endif
@@ -935,6 +956,8 @@ void tb_phys_invalidate(TranslationBlock *tb, 
tb_page_addr_t page_addr)
     tb_page_addr_t phys_pc;
     TranslationBlock *tb1, *tb2;
 
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
     /* remove the TB from the hash list */
     phys_pc = tb->page_addr[0] + (tb->pc & ~TARGET_PAGE_MASK);
     h = tb_phys_hash_func(phys_pc);
@@ -982,6 +1005,8 @@ void tb_phys_invalidate(TranslationBlock *tb, 
tb_page_addr_t page_addr)
     tb->jmp_first = (TranslationBlock *)((uintptr_t)tb | 2); /* fail safe */
 
     tb_ctx.tb_phys_invalidate_count++;
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
 }
 
 static inline void set_bits(uint8_t *tab, int start, int len)
@@ -1050,6 +1075,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
     target_ulong virt_page2;
     int code_gen_size;
 
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
     phys_pc = get_page_addr_code(env, pc);
     if (use_icount) {
         cflags |= CF_USE_ICOUNT;
@@ -1078,6 +1105,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
         phys_page2 = get_page_addr_code(env, virt_page2);
     }
     tb_link_page(tb, phys_pc, phys_page2);
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
     return tb;
 }
 
@@ -1383,7 +1412,7 @@ static inline void tb_alloc_page(TranslationBlock *tb,
 }
 
 /* add a new TB and link it to the physical page tables. phys_page2 is
-   (-1) to indicate that only one page contains the TB. */
+ * (-1) to indicate that only one page contains the TB. */
 static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc,
                          tb_page_addr_t phys_page2)
 {
@@ -1431,31 +1460,32 @@ static TranslationBlock *tb_find_pc(uintptr_t tc_ptr)
 {
     int m_min, m_max, m;
     uintptr_t v;
-    TranslationBlock *tb;
-
-    if (tb_ctx.nb_tbs <= 0) {
-        return NULL;
-    }
-    if (tc_ptr < (uintptr_t)tcg_ctx.code_gen_buffer ||
-        tc_ptr >= (uintptr_t)tcg_ctx.code_gen_ptr) {
-        return NULL;
-    }
-    /* binary search (cf Knuth) */
-    m_min = 0;
-    m_max = tb_ctx.nb_tbs - 1;
-    while (m_min <= m_max) {
-        m = (m_min + m_max) >> 1;
-        tb = &tb_ctx.tbs[m];
-        v = (uintptr_t)tb->tc_ptr;
-        if (v == tc_ptr) {
-            return tb;
-        } else if (tc_ptr < v) {
-            m_max = m - 1;
-        } else {
-            m_min = m + 1;
+    TranslationBlock *tb = NULL;
+
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
+    if ((tb_ctx.nb_tbs > 0) && (tc_ptr >= (uintptr_t)tcg_ctx.code_gen_buffer &&
+        tc_ptr < (uintptr_t)tcg_ctx.code_gen_ptr)) {
+        /* binary search (cf Knuth) */
+        m_min = 0;
+        m_max = tb_ctx.nb_tbs - 1;
+        while (m_min <= m_max) {
+            m = (m_min + m_max) >> 1;
+            tb = &tb_ctx.tbs[m];
+            v = (uintptr_t)tb->tc_ptr;
+            if (v == tc_ptr) {
+                return tb;
+            } else if (tc_ptr < v) {
+                m_max = m - 1;
+            } else {
+                m_min = m + 1;
+            }
         }
+        tb = &tb_ctx.tbs[m_max];
     }
-    return &tb_ctx.tbs[m_max];
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
+    return tb;
 }
 
 #if defined(TARGET_HAS_ICE) && !defined(CONFIG_USER_ONLY)
@@ -1604,6 +1634,8 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
     int direct_jmp_count, direct_jmp2_count, cross_page;
     TranslationBlock *tb;
 
+    qemu_mutex_lock(&tb_ctx.tb_lock);
+
     target_code_size = 0;
     max_target_code_size = 0;
     cross_page = 0;
@@ -1655,6 +1687,8 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
                 tb_ctx.tb_phys_invalidate_count);
     cpu_fprintf(f, "TLB flush count     %d\n", tlb_flush_count);
     tcg_dump_info(f, cpu_fprintf);
+
+    qemu_mutex_unlock(&tb_ctx.tb_lock);
 }
 
 void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
-- 
1.9.0