From: Paolo Bonzini <pbonz...@redhat.com> softmmu requires more functions to be thread-safe, because translation blocks can be invalidated from e.g. notdirty callbacks. Probably the same holds for user-mode emulation, it's just that no one has ever tried to produce a coherent locking there.
This patch will guide the introduction of more tb_lock and tb_unlock calls for system emulation. Note that after this patch some (most) of the mentioned functions are still called outside tb_lock/tb_unlock. The next one will rectify this. Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v1(ajb): - just s-o-b v2 - clarify write lock on tb_jump_cache v3 - drop RCU comment for debug stuff (separate commit now) --- include/exec/exec-all.h | 1 + include/qom/cpu.h | 3 +++ tcg/tcg.h | 2 ++ translate-all.c | 32 ++++++++++++++++++++++++++------ 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index ea1c925..e30f02b 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -313,6 +313,7 @@ static inline void tb_set_jmp_target(TranslationBlock *tb, #endif +/* Called with tb_lock held. */ static inline void tb_add_jump(TranslationBlock *tb, int n, TranslationBlock *tb_next) { diff --git a/include/qom/cpu.h b/include/qom/cpu.h index c9ba16c..b82625f 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -313,7 +313,10 @@ struct CPUState { MemoryRegion *memory; void *env_ptr; /* CPUArchState */ + + /* Writes protected by tb_lock, reads not thread-safe */ struct TranslationBlock *tb_jmp_cache[TB_JMP_CACHE_SIZE]; + struct GDBRegisterState *gdb_regs; int gdb_num_regs; int gdb_num_g_regs; diff --git a/tcg/tcg.h b/tcg/tcg.h index 909db3f..db6a062 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -638,6 +638,7 @@ static inline bool tcg_op_buf_full(void) /* pool based memory allocation */ +/* tb_lock must be held for tcg_malloc_internal. */ void *tcg_malloc_internal(TCGContext *s, int size); void tcg_pool_reset(TCGContext *s); void tcg_pool_delete(TCGContext *s); @@ -646,6 +647,7 @@ void tb_lock(void); void tb_unlock(void); void tb_lock_reset(void); +/* Called with tb_lock held. */ static inline void *tcg_malloc(int size) { TCGContext *s = &tcg_ctx; diff --git a/translate-all.c b/translate-all.c index e3f44d9..8b162ff 100644 --- a/translate-all.c +++ b/translate-all.c @@ -290,7 +290,9 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) return p - block; } -/* The cpu state corresponding to 'searched_pc' is restored. */ +/* The cpu state corresponding to 'searched_pc' is restored. + * Called with tb_lock held. + */ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, uintptr_t searched_pc) { @@ -347,8 +349,10 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr) cpu_restore_state_from_tb(cpu, tb, retaddr); if (tb->cflags & CF_NOCACHE) { /* one-shot translation, invalidate it immediately */ + tb_lock(); tb_phys_invalidate(tb, -1); tb_free(tb); + tb_unlock(); } return true; } @@ -440,6 +444,7 @@ static void page_init(void) } /* If alloc=1: + * Called with tb_lock held for system emulation. * Called with mmap_lock held for user-mode emulation. */ static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc) @@ -804,8 +809,12 @@ 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. + * + * Called with tb_lock held. + */ static TranslationBlock *tb_alloc(target_ulong pc) { TranslationBlock *tb; @@ -819,6 +828,7 @@ static TranslationBlock *tb_alloc(target_ulong pc) return tb; } +/* Called with tb_lock held. */ void tb_free(TranslationBlock *tb) { /* In practice this is mostly used for single use temporary TB @@ -918,7 +928,11 @@ do_tb_invalidate_check(struct qht *ht, void *p, uint32_t hash, void *userp) } } -static void tb_invalidate_check(target_ulong address) +/* verify that all the pages have correct rights for code + * + * Called with tb_lock held. + */ +static void tb_page_check(void) { address &= TARGET_PAGE_MASK; qht_iter(&tcg_ctx.tb_ctx.htable, do_tb_invalidate_check, &address); @@ -1022,7 +1036,10 @@ static inline void tb_jmp_unlink(TranslationBlock *tb) } } -/* invalidate one TB */ +/* invalidate one TB + * + * Called with tb_lock held. + */ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) { CPUState *cpu; @@ -1453,7 +1470,9 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len) } if (!p->code_bitmap && ++p->code_write_count >= SMC_BITMAP_USE_THRESHOLD) { - /* build code bitmap */ + /* build code bitmap. FIXME: writes should be protected by + * tb_lock, reads by tb_lock or RCU. + */ build_page_bitmap(p); } if (p->code_bitmap) { @@ -1593,6 +1612,7 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr) } #endif /* !defined(CONFIG_USER_ONLY) */ +/* Called with tb_lock held. */ void tb_check_watchpoint(CPUState *cpu) { TranslationBlock *tb; -- 2.7.4