From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35228) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5D2b-0001jU-Nw for qemu-devel@nongnu.org; Wed, 17 Jun 2015 09:06:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5D2V-00065p-Mg for qemu-devel@nongnu.org; Wed, 17 Jun 2015 09:06:05 -0400 Received: from hall.aurel32.net ([2001:bc8:30d7:101::1]:58790) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5D2U-0005t4-PO for qemu-devel@nongnu.org; Wed, 17 Jun 2015 09:05:59 -0400 Date: Wed, 17 Jun 2015 15:05:48 +0200 From: Aurelien Jarno Message-ID: <20150617130548.GH931@aurel32.net> References: <20150617124158.3316.54954.stgit@PASHA-ISP> <20150617124210.3316.94921.stgit@PASHA-ISP> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150617124210.3316.94921.stgit@PASHA-ISP> Subject: Re: [Qemu-devel] [PATCH v2 2/3] target-mips: exceptions handling in icount mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgalyuk Cc: pbonzini@redhat.com, rth7680@gmail.com, leon.alrae@imgtec.com, qemu-devel@nongnu.org On 2015-06-17 15:42, Pavel Dovgalyuk wrote: > This patch fixes exception handling in MIPS. > Instructions generate several types of exceptions. > When exception is generated, it breaks the execution of the current translation > block. Implementation of the exceptions handling does not correctly > restore icount for the instruction which caused the exception. In most cases > icount will be decreased by the value equal to the size of TB. > This patch passes pointer to the translation block internals to the exception > handler. It allows correct restoring of the icount value. > > Signed-off-by: Pavel Dovgalyuk > --- > target-mips/cpu.h | 28 +++++++ > target-mips/helper.h | 1 > target-mips/msa_helper.c | 5 + > target-mips/op_helper.c | 183 ++++++++++++++++++++++------------------------ > target-mips/translate.c | 46 ++++++------ > 5 files changed, 141 insertions(+), 122 deletions(-) [ snip ] > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c > index 73a8e45..2815c60 100644 > --- a/target-mips/op_helper.c > +++ b/target-mips/op_helper.c > @@ -30,41 +30,23 @@ static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global); > /*****************************************************************************/ > /* Exceptions processing helpers */ > > -static inline void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env, > - uint32_t exception, > - int error_code, > - uintptr_t pc) > +void helper_raise_exception_err(CPUMIPSState *env, uint32_t exception, > + int error_code) > { > - CPUState *cs = CPU(mips_env_get_cpu(env)); > - > - if (exception < EXCP_SC) { > - qemu_log("%s: %d %d\n", __func__, exception, error_code); > - } > - cs->exception_index = exception; > - env->error_code = error_code; > - > - if (pc) { > - /* now we have a real cpu fault */ > - cpu_restore_state(cs, pc); > - } > - > - cpu_loop_exit(cs); > + do_raise_exception_err(env, exception, error_code, GETPC()); > } > > -static inline void QEMU_NORETURN do_raise_exception(CPUMIPSState *env, > - uint32_t exception, > - uintptr_t pc) > +void helper_raise_exception(CPUMIPSState *env, uint32_t exception) > { > - do_raise_exception_err(env, exception, 0, pc); > + do_raise_exception(env, exception, GETPC()); > } raise_exception is used to implement the SYSCALL instruction on MIPS. With this change, this mean that each TB containing a syscall will have to be translated at least twice, probably more. That's not something acceptable performance wise. > diff --git a/target-mips/translate.c b/target-mips/translate.c > index fd063a2..0de9244 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -1673,7 +1673,7 @@ generate_exception_err (DisasContext *ctx, int excp, int err) > { > TCGv_i32 texcp = tcg_const_i32(excp); > TCGv_i32 terr = tcg_const_i32(err); > - save_cpu_state(ctx, 1); > + save_cpu_state(ctx, 0); If retranslation is used, you don't even need to save the branch status, restore_state_to_opc can restore it. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net