From 301e40ed8005306c009978be295ed9a4b725178b Mon Sep 17 00:00:00 2001 From: Alex Bennée Date: Thu, 27 Oct 2016 16:10:00 +0100 Subject: translate-all: add DEBUG_LOCKING asserts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds asserts to check the locking on the various translation engines structures. There are two sets of structures that are protected by locks. The first the l1map and PageDesc structures used to track which translation blocks are associated with which physical addresses. In user-mode this is covered by the mmap_lock. The second case are TB context related structures which are protected by tb_lock which is also user-mode only. Currently the asserts do nothing in SoftMMU mode but this will change for MTTCG. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20161027151030.20863-4-alex.bennee@linaro.org> Signed-off-by: Paolo Bonzini --- include/exec/exec-all.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/exec/exec-all.h') diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index cb624e4acc..4d36ee38f7 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -369,6 +369,7 @@ void tlb_fill(CPUState *cpu, target_ulong addr, MMUAccessType access_type, #if defined(CONFIG_USER_ONLY) void mmap_lock(void); void mmap_unlock(void); +bool have_mmap_lock(void); static inline tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) { -- cgit 1.4.1 From 7d7500d99895f888f97397ef32bb536bb0df3b74 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 27 Oct 2016 16:10:03 +0100 Subject: tcg: comment on which functions have to be called with tb_lock held MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20161027151030.20863-7-alex.bennee@linaro.org> Signed-off-by: Paolo Bonzini --- include/exec/exec-all.h | 1 + include/qom/cpu.h | 3 +++ tcg/tcg.h | 2 ++ translate-all.c | 28 +++++++++++++++++++++++----- 4 files changed, 29 insertions(+), 5 deletions(-) (limited to 'include/exec/exec-all.h') diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 4d36ee38f7..a8c13cee66 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -316,6 +316,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 633c3fc124..9f597bb0c0 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -319,7 +319,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 b34b5fbc2f..dc1281fb4e 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -726,6 +726,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); @@ -733,6 +734,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 5aded3d3bd..fad2646ddd 100644 --- a/translate-all.c +++ b/translate-all.c @@ -308,7 +308,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) { @@ -462,6 +464,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) @@ -826,8 +829,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; @@ -842,6 +849,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 @@ -966,6 +974,10 @@ do_tb_invalidate_check(struct qht *ht, void *p, uint32_t hash, void *userp) } } +/* verify that all the pages have correct rights for code + * + * Called with tb_lock held. + */ static void tb_invalidate_check(target_ulong address) { address &= TARGET_PAGE_MASK; @@ -1070,7 +1082,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; @@ -1504,7 +1519,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) { @@ -1645,6 +1662,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; -- cgit 1.4.1