From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1K11Ju-0003wU-RO for qemu-devel@nongnu.org; Tue, 27 May 2008 11:42:38 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1K11Jt-0003vE-Am for qemu-devel@nongnu.org; Tue, 27 May 2008 11:42:37 -0400 Received: from [199.232.76.173] (port=60915 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1K11Js-0003v0-RW for qemu-devel@nongnu.org; Tue, 27 May 2008 11:42:36 -0400 Received: from yw-out-1718.google.com ([74.125.46.158]:63406) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1K11Js-0002PT-LZ for qemu-devel@nongnu.org; Tue, 27 May 2008 11:42:37 -0400 Received: by yw-out-1718.google.com with SMTP id 6so1384164ywa.82 for ; Tue, 27 May 2008 08:42:17 -0700 (PDT) Message-ID: <5d6222a80805270842i47dbd77eh22d7f8867e41ba34@mail.gmail.com> Date: Tue, 27 May 2008 12:42:16 -0300 From: "Glauber Costa" Subject: Re: [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function In-Reply-To: <483C291C.2040809@bellard.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <1211901505-30519-1-git-send-email-gcosta@redhat.com> <1211901505-30519-2-git-send-email-gcosta@redhat.com> <1211901505-30519-3-git-send-email-gcosta@redhat.com> <1211901505-30519-4-git-send-email-gcosta@redhat.com> <1211901505-30519-5-git-send-email-gcosta@redhat.com> <1211901505-30519-6-git-send-email-gcosta@redhat.com> <483C291C.2040809@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: qemu-devel@nongnu.org On Tue, May 27, 2008 at 12:30 PM, Fabrice Bellard wrote: > 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 :-) If it can really be safely removed, this sounds like the best solution to me by far. > 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" >> >> >> > > > > -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act."