From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXrqU-0003dj-SS for qemu-devel@nongnu.org; Thu, 11 Aug 2016 11:24:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bXrqQ-0006Zj-Iw for qemu-devel@nongnu.org; Thu, 11 Aug 2016 11:24:34 -0400 Received: from mail-wm0-x236.google.com ([2a00:1450:400c:c09::236]:37244) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXrqQ-0006Zf-8D for qemu-devel@nongnu.org; Thu, 11 Aug 2016 11:24:30 -0400 Received: by mail-wm0-x236.google.com with SMTP id i5so3237458wmg.0 for ; Thu, 11 Aug 2016 08:24:30 -0700 (PDT) From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 11 Aug 2016 16:24:02 +0100 Message-Id: <1470929064-4092-7-git-send-email-alex.bennee@linaro.org> In-Reply-To: <1470929064-4092-1-git-send-email-alex.bennee@linaro.org> References: <1470929064-4092-1-git-send-email-alex.bennee@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: [Qemu-devel] [RFC v4 06/28] tcg: comment on which functions have to be called with tb_lock held List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mttcg@listserver.greensocs.com, qemu-devel@nongnu.org, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org, bobby.prani@gmail.com, nikunj@linux.vnet.ibm.com Cc: mark.burton@greensocs.com, pbonzini@redhat.com, jan.kiszka@siemens.com, serge.fdrv@gmail.com, rth@twiddle.net, peter.maydell@linaro.org, claudio.fontana@huawei.com, =?UTF-8?q?Alex=20Benn=C3=A9e?= , Peter Crosthwaite From: Paolo Bonzini 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 --- 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 | 30 ++++++++++++++++++++++++------ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index d5f29bd..d73b0e6 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -315,6 +315,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 060a473..8ee770a 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -324,7 +324,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 6046dcd..a7f34db 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -712,6 +712,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); @@ -720,6 +721,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 7ab1fd6..a3448e1 100644 --- a/translate-all.c +++ b/translate-all.c @@ -288,7 +288,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) { @@ -438,6 +440,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) @@ -802,8 +805,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; @@ -818,6 +825,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 @@ -927,7 +935,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); @@ -1031,7 +1043,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; @@ -1465,7 +1480,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) { @@ -1606,6 +1623,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