Re: [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function

[Fabrice Bellard <fabrice@bellard.org>]

This is not the right solution for two reasons:
- This part of the code is speed critical, so the function must be at 
least static inline.
- The related code is never used.

So just removing the piece of code is the right solution :-)

Fabrice.

Glauber Costa wrote:
> Simplify cpu_exec by removing a TARGET_I386 block, responsible
> for conditionally resetting the mmu, and enclosing it inside
> a function.
> ---
>  cpu-exec.c            |   17 +----------------
>  exec-all.h            |    1 +
>  target-alpha/helper.c |    1 +
>  target-arm/helper.c   |    1 +
>  target-cris/helper.c  |    1 +
>  target-i386/helper.c  |   19 +++++++++++++++++++
>  target-m68k/helper.c  |    2 ++
>  target-mips/helper.c  |    1 +
>  target-ppc/helper.c   |    1 +
>  target-sh4/helper.c   |    1 +
>  target-sparc/helper.c |    2 ++
>  11 files changed, 31 insertions(+), 16 deletions(-)
> 
> Index: qemu/cpu-exec.c
> ===================================================================
> --- qemu.orig/cpu-exec.c
> +++ qemu/cpu-exec.c
> @@ -432,15 +432,7 @@ int cpu_exec(CPUState *env1)
>  #endif
>                  next_tb = tcg_qemu_tb_exec(tc_ptr);
>                  env->current_tb = NULL;
> -                /* reset soft MMU for next block (it can currently
> -                   only be set by a memory fault) */
> -#if defined(TARGET_I386) && !defined(CONFIG_SOFTMMU)
> -                if (env->hflags & HF_SOFTMMU_MASK) {
> -                    env->hflags &= ~HF_SOFTMMU_MASK;
> -                    /* do not allow linking to another block */
> -                    next_tb = 0;
> -                }
> -#endif
> +                cpu_reset_mmu(env);
>  #if defined(USE_KQEMU)
>  #define MIN_CYCLE_BEFORE_SWITCH (100 * 1000)
>                  if (kqemu_is_ok(env) &&
> @@ -448,6 +440,7 @@ int cpu_exec(CPUState *env1)
>                      cpu_loop_exit();
>                  }
>  #endif
> +
>              } /* for(;;) */
>          } else {
>              env_to_regs();
> Index: qemu/exec-all.h
> ===================================================================
> --- qemu.orig/exec-all.h
> +++ qemu/exec-all.h
> @@ -93,6 +93,7 @@ void reset_tb(void);
>  
>  void cpu_load_flags(CPUState *env);
>  void cpu_put_flags(CPUState *env);
> +void cpu_reset_mmu(CPUState *env);
>  
>  int page_unprotect(target_ulong address, unsigned long pc, void *puc);
>  void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end,
> Index: qemu/target-alpha/helper.c
> ===================================================================
> --- qemu.orig/target-alpha/helper.c
> +++ qemu/target-alpha/helper.c
> @@ -456,6 +456,7 @@ void cpu_dump_EA (target_ulong EA)
>  int cpu_info_ip(CPUState *env, char *buf) { return 0; }
>  void cpu_load_flags(CPUState *env) {}
>  void cpu_put_flags(CPUState *env) {}
> +void cpu_reset_mmu(CPUState *env) {}
>  
>  void arch_handle_interrupt_request(CPUState *env)
>  {
> Index: qemu/target-arm/helper.c
> ===================================================================
> --- qemu.orig/target-arm/helper.c
> +++ qemu/target-arm/helper.c
> @@ -2527,6 +2527,7 @@ int cpu_info_ip(CPUState *env, char *buf
>  void cpu_load_flags(CPUState *env) { }
>  /* XXX: Save/restore host fpu exception state?.  */
>  void cpu_put_flags(CPUState *env) { }
> +void cpu_reset_mmu(CPUState *env) {}
>  
>  void arch_handle_interrupt_request(CPUState *env)
>  {
> Index: qemu/target-cris/helper.c
> ===================================================================
> --- qemu.orig/target-cris/helper.c
> +++ qemu/target-cris/helper.c
> @@ -187,6 +187,7 @@ target_phys_addr_t cpu_get_phys_page_deb
>  int  cpu_info_ip(CPUState *env, char *buf) { return 0; }
>  void cpu_load_flags(CPUState *env) {}
>  void cpu_put_flags(CPUState *env) {}
> +void cpu_reset_mmu(CPUState *env) {}
>  
>  void arch_handle_interrupt_request(CPUState *env)
>  {
> Index: qemu/target-i386/helper.c
> ===================================================================
> --- qemu.orig/target-i386/helper.c
> +++ qemu/target-i386/helper.c
> @@ -4755,6 +4755,18 @@ void cpu_put_flags(CPUState *env)
>      env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK);
>  }
>  
> +void cpu_reset_mmu(CPUState *env)
> +{
> +    /* reset soft MMU for next block (it can currently
> +       only be set by a memory fault) */
> +#if !defined(CONFIG_SOFTMMU)
> +    if (env->hflags & HF_SOFTMMU_MASK) {
> +        env->hflags &= ~HF_SOFTMMU_MASK;
> +        /* do not allow linking to another block */
> +        reset_tb();
> +    }
> +#endif
> +}
>  #if defined(CONFIG_USER_ONLY)
>  
>  void helper_vmrun(void) 
> Index: qemu/target-m68k/helper.c
> ===================================================================
> --- qemu.orig/target-m68k/helper.c
> +++ qemu/target-m68k/helper.c
> @@ -355,6 +355,8 @@ void cpu_put_flags(CPUState *env)
>                | env->cc_dest | (env->cc_x << 4);
>  }
>  
> +void cpu_reset_mmu(CPUState *env) {}
> +
>  void arch_handle_interrupt_request(CPUState *env)
>  {
>      if (env->interrupt_request & CPU_INTERRUPT_HARD
> Index: qemu/target-mips/helper.c
> ===================================================================
> --- qemu.orig/target-mips/helper.c
> +++ qemu/target-mips/helper.c
> @@ -645,6 +645,7 @@ int cpu_info_ip(CPUState *env, char *buf
>  
>  void cpu_put_flags(CPUState *env) { }
>  void cpu_load_flags(CPUState *env) { }
> +void cpu_reset_mmu(CPUState *env) { }
>  
>  void arch_handle_interrupt_request(CPUState *env)
>  {
> Index: qemu/target-ppc/helper.c
> ===================================================================
> --- qemu.orig/target-ppc/helper.c
> +++ qemu/target-ppc/helper.c
> @@ -2997,6 +2997,7 @@ int cpu_info_ip(CPUState *env, char *buf
>  
>  void cpu_load_flags(CPUState *env) { }
>  void cpu_put_flags(CPUState *env) { }
> +void cpu_reset_mmu(CPUState *env) { }
>  
>  void arch_handle_interrupt_request(CPUState *env)
>  {
> Index: qemu/target-sh4/helper.c
> ===================================================================
> --- qemu.orig/target-sh4/helper.c
> +++ qemu/target-sh4/helper.c
> @@ -538,6 +538,7 @@ void cpu_load_tlb(CPUState * env)
>  int  cpu_info_ip(CPUState *env, char *buf) { return 0; }
>  void cpu_load_flags(CPUState *env) {}
>  void cpu_put_flags(CPUState *env) {}
> +void cpu_reset_mmu(CPUState *env) {}
>  
>  void arch_handle_interrupt_request(CPUState *env)
>  {
> Index: qemu/target-sparc/helper.c
> ===================================================================
> --- qemu.orig/target-sparc/helper.c
> +++ qemu/target-sparc/helper.c
> @@ -1340,6 +1340,8 @@ void cpu_put_flags(CPUState *env)
>  #endif
>  }
>  
> +void cpu_reset_mmu(CPUState *env) {}
> +
>  #ifdef TARGET_SPARC64
>  #if !defined(CONFIG_USER_ONLY)
>  #include "qemu-common.h"
> 
> 
>