From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JzwBr-0005sf-K8 for qemu-devel@nongnu.org; Sat, 24 May 2008 12:01:51 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JzwBq-0005rr-1m for qemu-devel@nongnu.org; Sat, 24 May 2008 12:01:50 -0400 Received: from [199.232.76.173] (port=42453 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JzwBp-0005ra-No for qemu-devel@nongnu.org; Sat, 24 May 2008 12:01:49 -0400 Received: from miranda.se.axis.com ([193.13.178.8]:43130) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JzwBp-0005tA-3U for qemu-devel@nongnu.org; Sat, 24 May 2008 12:01:49 -0400 Received: from axis.com (edgar.se.axis.com [10.93.151.1]) by miranda.se.axis.com (8.13.4/8.13.4/Debian-3sarge3) with ESMTP id m4OG1jMx031907 for ; Sat, 24 May 2008 18:01:46 +0200 Date: Sat, 24 May 2008 20:01:49 +0200 From: "Edgar E. Iglesias" Subject: Re: [Qemu-devel] [4352] Make sure we flush cached blocks from the tb-jmp-cache when we replace valid tlb entries . Message-ID: <20080524180149.GA29484@edgar.se.axis.com> References: <483753C9.2030009@bellard.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <483753C9.2030009@bellard.org> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fabrice Bellard Cc: edgar.iglesias@gmail.com, qemu-devel@nongnu.org On Sat, May 24, 2008 at 01:31:21AM +0200, Fabrice Bellard wrote: > Please revert this patch: it has a major performance hit because the > tb_jmp_cache is flushed too often. Flushing the tb_jmp_cache when > overriding a tlb_cache entry is not necessary, provided a given virtual > address is always remapped at the same physical address with the same > rights, which is the assumed API here. Detecting possible > inconsistencies is useful, but I am sure there is a solution without > such a performance hit. > > Please avoid doing such modifications without doing regressions tests on > performance (e.g. a compilation with gcc). My misstake, unfortunately I only ran performance tests on CRIS. That flush was needed at the time in order to avoid a complete tlb_flush() every time the guest touched the TLB. IIRC those changes improved performance and got rid of bugs were TBs got executed even if their addresses were not in the TLB. The flush might not be needed once the tb jump cache hash function got fixed, but I havent had time to verify that. Thanks for clarifying the API. Best regards > > Regards, > > Fabrice. > > Edgar E. Iglesias wrote: > > Revision: 4352 > > http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=4352 > > Author: edgar_igl > > Date: 2008-05-06 08:44:21 +0000 (Tue, 06 May 2008) > > > > Log Message: > > ----------- > > Make sure we flush cached blocks from the tb-jmp-cache when we replace valid tlb entries. > > > > Modified Paths: > > -------------- > > trunk/exec.c > > > > Modified: trunk/exec.c > > =================================================================== > > --- trunk/exec.c 2008-05-06 08:38:22 UTC (rev 4351) > > +++ trunk/exec.c 2008-05-06 08:44:21 UTC (rev 4352) > > @@ -1366,6 +1366,21 @@ > > > > #if !defined(CONFIG_USER_ONLY) > > > > +static inline void tlb_flush_jmp_cache(CPUState *env, target_ulong addr) > > +{ > > + unsigned int i; > > + > > + /* Discard jump cache entries for any tb which might potentially > > + overlap the flushed page. */ > > + i = tb_jmp_cache_hash_page(addr - TARGET_PAGE_SIZE); > > + memset (&env->tb_jmp_cache[i], 0, > > + TB_JMP_PAGE_SIZE * sizeof(TranslationBlock *)); > > + > > + i = tb_jmp_cache_hash_page(addr); > > + memset (&env->tb_jmp_cache[i], 0, > > + TB_JMP_PAGE_SIZE * sizeof(TranslationBlock *)); > > +} > > + > > /* NOTE: if flush_global is true, also flush global entries (not > > implemented yet) */ > > void tlb_flush(CPUState *env, int flush_global) > > @@ -1428,7 +1443,6 @@ > > void tlb_flush_page(CPUState *env, target_ulong addr) > > { > > int i; > > - TranslationBlock *tb; > > > > #if defined(DEBUG_TLB) > > printf("tlb_flush_page: " TARGET_FMT_lx "\n", addr); > > @@ -1448,14 +1462,8 @@ > > #endif > > #endif > > > > - /* Discard jump cache entries for any tb which might potentially > > - overlap the flushed page. */ > > - i = tb_jmp_cache_hash_page(addr - TARGET_PAGE_SIZE); > > - memset (&env->tb_jmp_cache[i], 0, TB_JMP_PAGE_SIZE * sizeof(tb)); > > + tlb_flush_jmp_cache(env, addr); > > > > - i = tb_jmp_cache_hash_page(addr); > > - memset (&env->tb_jmp_cache[i], 0, TB_JMP_PAGE_SIZE * sizeof(tb)); > > - > > #if !defined(CONFIG_SOFTMMU) > > if (addr < MMAP_AREA_END) > > munmap((void *)addr, TARGET_PAGE_SIZE); > > @@ -1706,6 +1714,10 @@ > > } else { > > te->addr_read = -1; > > } > > + > > + if (te->addr_code != -1) { > > + tlb_flush_jmp_cache(env, te->addr_code); > > + } > > if (prot & PAGE_EXEC) { > > te->addr_code = address; > > } else { > > > > > > > > > > > > -- Edgar E. Iglesias Axis Communications AB