From patchwork Fri Jun 3 20:40:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 630011 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rLx2d4sQfz9t89 for ; Sat, 4 Jun 2016 06:46:17 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=hMAGht83; dkim-atps=neutral Received: from localhost ([::1]:57653 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8vyx-0005Kx-Ny for incoming@patchwork.ozlabs.org; Fri, 03 Jun 2016 16:46:15 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37404) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8vtW-0008QL-G2 for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:40:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8vtV-0000SG-0D for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:40:38 -0400 Received: from mail-wm0-x231.google.com ([2a00:1450:400c:c09::231]:36637) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8vtU-0000R9-Ev for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:40:36 -0400 Received: by mail-wm0-x231.google.com with SMTP id n184so11943978wmn.1 for ; Fri, 03 Jun 2016 13:40:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=n3edezlDAA9LDQe8zQBO4z/vB47QfxruTfpMPwRyqtk=; b=hMAGht830chuh7e+npbGGovrF0kHMrhxGbOuCyNzn0Jo81O+oCI9QqmyOfGTdj9D0q Hv1H4GF4INrcxfrcAyTlO2FgwCrV4qAq7KTwJuNzg9iIVObeGfSuGZkgtRPeoOaeh77H yIfNvWAM1G6p3/4048vrDy2q4Flsad19z5wSU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=n3edezlDAA9LDQe8zQBO4z/vB47QfxruTfpMPwRyqtk=; b=D2ZnNQU5AeAuR8ER+myFcXfwTVNPb+N3NH5Gbyap+AjRF8z233CzD5Jw7epg+dBjOE TwV4SqDjsPnH5D1CyyT6mtyesScvPZmSdyvaUcjrlwipk2uUK25U6kn9bl9QPnd1Sflb vFK2QcbHKQEsyX5Xhr2a5CWsvBm9ouM7JQYf1fu9ayYL8Uvj2FXSgfEpW+8Yz/Vx/OrU 7n5K6eqtRp2fFzgDvxfIy61Sx/y0W8Ea5FAhDgCw781d/KJ5HjuEPqkuDdds6UPwrbYp JkPurpKcXjgOI0RdI1TJi6Th//sIKPyBckEg7xIlNZcnPKaK2QHcublB/H22HHHfprI3 2fFg== X-Gm-Message-State: ALyK8tKYsqod9tNu6CAp1WMNJZSjV8ImbGfbQi0VIVjrkZGdlFtpilzXLpvxLSoPRUigs2IY X-Received: by 10.28.183.8 with SMTP id h8mr1033662wmf.79.1464986435641; Fri, 03 Jun 2016 13:40:35 -0700 (PDT) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id on2sm7359170wjc.32.2016.06.03.13.40.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Jun 2016 13:40:31 -0700 (PDT) Received: from zen.linaroharston (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTP id B21043E2D71; Fri, 3 Jun 2016 21:40:39 +0100 (BST) From: =?UTF-8?q?Alex=20Benn=C3=A9e?= To: mttcg@listserver.greensocs.com, qemu-devel@nongnu.org, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, serge.fdrv@gmail.com, cota@braap.org, bobby.prani@gmail.com Date: Fri, 3 Jun 2016 21:40:15 +0100 Message-Id: <1464986428-6739-7-git-send-email-alex.bennee@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1464986428-6739-1-git-send-email-alex.bennee@linaro.org> References: <1464986428-6739-1-git-send-email-alex.bennee@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::231 Subject: [Qemu-devel] [RFC v3 06/19] tcg: comment on which functions have to be called with tb_lock held X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.maydell@linaro.org, Peter Crosthwaite , claudio.fontana@huawei.com, mark.burton@greensocs.com, jan.kiszka@siemens.com, pbonzini@redhat.com, =?UTF-8?q?Alex=20Benn=C3=A9e?= , rth@twiddle.net Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 | 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;