From patchwork Fri Mar 18 16:18:42 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: 599552 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 3qRVps6JSGz9sCj for ; Sat, 19 Mar 2016 03:21:41 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=Pjv26y1G; dkim-atps=neutral Received: from localhost ([::1]:44804 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agx9f-0005Tc-Mm for incoming@patchwork.ozlabs.org; Fri, 18 Mar 2016 12:21:39 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agx79-0000R8-Ig for qemu-devel@nongnu.org; Fri, 18 Mar 2016 12:19:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agx74-0002vL-Vp for qemu-devel@nongnu.org; Fri, 18 Mar 2016 12:19:03 -0400 Received: from mail-wm0-x229.google.com ([2a00:1450:400c:c09::229]:37456) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agx74-0002ut-M2 for qemu-devel@nongnu.org; Fri, 18 Mar 2016 12:18:58 -0400 Received: by mail-wm0-x229.google.com with SMTP id p65so44376325wmp.0 for ; Fri, 18 Mar 2016 09:18:58 -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=ibHrZQ641ZvWLOfcqzgfWrhOzPHt5Fqxl66lmpU7nXI=; b=Pjv26y1Gf+MVHKyjfBz7TzoNFpsvXXGCUtSZkpVYH+zg255cFIYokB2jmW3m5e6rXH IcUYwNXiDCN1/SW3Kmrvzw8k2GUH7NfI47Krmq+AVL9AYUSn5rzlCf9X+rsR/Wj0Hw0B ocbEfq/GVnRH6XgrdaczK6StBS48jDAVNN1LM= 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=ibHrZQ641ZvWLOfcqzgfWrhOzPHt5Fqxl66lmpU7nXI=; b=UTigwHMohz9EhJTeob9yTB5DVNlV6s2JSJ7k7ZaewxwHd6Je+gZblAV3P19djB9J/i IcAc2quHZmlX9/A0seP2QqyF0VSBUH5TEhNh0I8WcD5xXq/+2oBXVAyp++wbNihLlyGY uvx+0jYK+YBMgB6rbEhSrObJ2NMcxfPurqgKsUFhaqmSgcIi4vPcYFuhp7JmXVWE5sfL eAo/bVpjF3pAiQTCa/VM7VhdXO9hr3drvUmc+Kg8he34cRYNY3Q2bLSCMX5fyq/00v4F e8sB8CNAU9Ue5SpDXXgiQOyFcUzp8cyUJ6jcnicTfxwYsFwlXayd8RxFN80LWyuQngbM Wi4g== X-Gm-Message-State: AD7BkJKyukcRlPuUQKLVxmLiB2boGCKiSKQRx3nOnFqikPWMICuiLNSqPQoLqewss6Vy6sCE X-Received: by 10.28.30.215 with SMTP id e206mr188721wme.25.1458317938110; Fri, 18 Mar 2016 09:18:58 -0700 (PDT) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id up6sm12876529wjc.6.2016.03.18.09.18.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Mar 2016 09:18:56 -0700 (PDT) Received: from zen.linaroharston (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTP id B82B63E015F; Fri, 18 Mar 2016 16:18:55 +0000 (GMT) From: =?UTF-8?q?Alex=20Benn=C3=A9e?= To: mttcg@listserver.greensocs.com, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, serge.fdrv@gmail.com, cota@braap.org Date: Fri, 18 Mar 2016 16:18:42 +0000 Message-Id: <1458317932-1875-2-git-send-email-alex.bennee@linaro.org> X-Mailer: git-send-email 2.7.3 In-Reply-To: <1458317932-1875-1-git-send-email-alex.bennee@linaro.org> References: <1458317932-1875-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::229 Cc: Peter Crosthwaite , mark.burton@greensocs.com, qemu-devel@nongnu.org, pbonzini@redhat.com, =?UTF-8?q?Alex=20Benn=C3=A9e?= , =?UTF-8?q?Andreas=20F=C3=A4rber?= , Richard Henderson Subject: [Qemu-devel] [RFC v1 01/11] tcg: move tb_find_fast outside the tb_lock critical section X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: KONRAD Frederic Signed-off-by: KONRAD Frederic Signed-off-by: Paolo Bonzini [AJB: minor checkpatch fixes] Signed-off-by: Alex Bennée --- v1(ajb) - checkpatch fixes --- cpu-exec.c | 74 +++++++++++++++++++++++++++++++++---------------------- include/qom/cpu.h | 2 ++ tcg/tcg.h | 1 + translate-all.c | 23 ++++++++++++++++- 4 files changed, 70 insertions(+), 30 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 07545aa..52f25de 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -225,8 +225,9 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, phys_page1 = phys_pc & TARGET_PAGE_MASK; h = tb_phys_hash_func(phys_pc); for (ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h]; - (tb = *ptb1) != NULL; + (tb = atomic_read(ptb1)) != NULL; ptb1 = &tb->phys_hash_next) { + smp_read_barrier_depends(); if (tb->pc != pc || tb->page_addr[0] != phys_page1 || tb->cs_base != cs_base || @@ -254,7 +255,18 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, *ptb1 = tb->phys_hash_next; tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h]; tcg_ctx.tb_ctx.tb_phys_hash[h] = tb; + } else { + return NULL; } + + /* If tb_flush was called since the last time we released the lock, + * forget about this TB. + */ + smp_rmb(); + if (atomic_read(&cpu->tb_invalidated_flag)) { + return NULL; + } + return tb; } @@ -265,36 +277,31 @@ static TranslationBlock *tb_find_slow(CPUState *cpu, { TranslationBlock *tb; - tb = tb_find_physical(cpu, pc, cs_base, flags); - if (tb) { - goto found; - } - -#ifdef CONFIG_USER_ONLY - /* mmap_lock is needed by tb_gen_code, and mmap_lock must be - * taken outside tb_lock. Since we're momentarily dropping - * tb_lock, there's a chance that our desired tb has been - * translated. + /* First try to get the tb. If we don't find it we need to lock and + * compile it. */ - tb_unlock(); - mmap_lock(); - tb_lock(); tb = tb_find_physical(cpu, pc, cs_base, flags); - if (tb) { - mmap_unlock(); - goto found; - } -#endif - - /* if no translated code available, then translate it now */ - cpu->tb_invalidated_flag = 0; - tb = tb_gen_code(cpu, pc, cs_base, flags, 0); - + if (!tb) { #ifdef CONFIG_USER_ONLY - mmap_unlock(); + /* mmap_lock is needed by tb_gen_code, and mmap_lock must be + * taken outside tb_lock. tb_lock is released later in + * cpu_exec. + */ + mmap_lock(); + tb_lock(); + + /* Retry to get the TB in case a CPU just translate it to avoid having + * duplicated TB in the pool. + */ + tb = tb_find_physical(cpu, pc, cs_base, flags); #endif + if (!tb) { + /* if no translated code available, then translate it now */ + tb = tb_gen_code(cpu, pc, cs_base, flags, 0); + } + mmap_unlock(); + } -found: /* we add the TB in the virtual pc hash table */ cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)] = tb; return tb; @@ -312,6 +319,8 @@ static inline TranslationBlock *tb_find_fast(CPUState *cpu) is executed. */ cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags); tb = cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)]; + /* Read tb_jmp_cache before tb->pc. */ + smp_read_barrier_depends(); if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base || tb->flags != flags)) { tb = tb_find_slow(cpu, pc, cs_base, flags); @@ -489,15 +498,18 @@ int cpu_exec(CPUState *cpu) cpu->exception_index = EXCP_INTERRUPT; cpu_loop_exit(cpu); } - tb_lock(); tb = tb_find_fast(cpu); /* Note: we do it here to avoid a gcc bug on Mac OS X when doing it in tb_find_slow */ - if (cpu->tb_invalidated_flag) { + if (atomic_read(&cpu->tb_invalidated_flag)) { /* as some TB could have been invalidated because of a tb_flush while generating the code, we must recompute the hash index here */ next_tb = 0; + + /* Clear the flag, we've now observed the flush. */ + tb_lock_recursive(); + cpu->tb_invalidated_flag = 0; } if (qemu_loglevel_mask(CPU_LOG_EXEC)) { qemu_log("Trace %p [" TARGET_FMT_lx "] %s\n", @@ -508,10 +520,14 @@ int cpu_exec(CPUState *cpu) jump. */ if (next_tb != 0 && tb->page_addr[1] == -1 && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { + tb_lock_recursive(); tb_add_jump((TranslationBlock *)(next_tb & ~TB_EXIT_MASK), next_tb & TB_EXIT_MASK, tb); } - tb_unlock(); + /* The lock may not be taken if we went through the + * fast lookup path and did not have to do any patching. + */ + tb_lock_reset(); if (likely(!cpu->exit_request)) { trace_exec_tb(tb, tb->pc); tc_ptr = tb->tc_ptr; diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 9538f9c..4132108 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -241,6 +241,8 @@ struct kvm_run; * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this * CPU and return to its top level loop. * @tb_invalidated_flag: Set to tell TCG that tb_flush has been called. + * It is only cleared while holding the tb_lock, so that no tb_flush can + * happen concurrently. * @singlestep_enabled: Flags for single-stepping. * @icount_extra: Instructions until next timer event. * @icount_decr: Number of cycles left, with interrupt flag in high bit. diff --git a/tcg/tcg.h b/tcg/tcg.h index b83f763..aa4e123 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -615,6 +615,7 @@ void tcg_pool_delete(TCGContext *s); void tb_lock(void); void tb_unlock(void); +bool tb_lock_recursive(void); void tb_lock_reset(void); static inline void *tcg_malloc(int size) diff --git a/translate-all.c b/translate-all.c index 8e1edd6..f68dcbc 100644 --- a/translate-all.c +++ b/translate-all.c @@ -143,6 +143,17 @@ void tb_unlock(void) #endif } +bool tb_lock_recursive(void) +{ +#ifdef CONFIG_USER_ONLY + if (have_tb_lock) { + return false; + } + tb_lock(); +#endif + return true; +} + void tb_lock_reset(void) { #ifdef CONFIG_USER_ONLY @@ -843,7 +854,8 @@ void tb_flush(CPUState *cpu) tcg_ctx.tb_ctx.nb_tbs = 0; CPU_FOREACH(cpu) { - cpu->tb_invalidated_flag = 1; + atomic_set(&cpu->tb_invalidated_flag, 1); + smp_wmb(); memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache)); } @@ -979,6 +991,9 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) pc = tb->pc; tb->pc = -1; + /* Pairs with smp_read_barrier_depends() in tb_find_fast. */ + smp_wmb(); + /* Then suppress this TB from the two jump lists. CPUs will not jump * anymore into this translation block. */ @@ -1478,7 +1493,13 @@ static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, /* add in the physical hash table */ h = tb_phys_hash_func(phys_pc); ptb = &tcg_ctx.tb_ctx.tb_phys_hash[h]; + + /* Both write barriers pair with tb_find_physical's + * smp_read_barrier_depends. + */ + smp_wmb(); tb->phys_hash_next = *ptb; + smp_wmb(); *ptb = tb; /* add in the page list */