Re: [Qemu-devel] [4352] Make sure we flush cached blocks from the tb-jmp-cache when we replace valid tlb entries .

[Fabrice Bellard <fabrice@bellard.org>]

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).

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 {
> 
> 
> 
> 
>