> From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> On 2015-06-18 16:28, Pavel Dovgalyuk wrote:
> > This patch improves 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.
> >
> > v3 changes:
> > This patch stops translation when instruction which always generates
> > exception
> > is translated. This improves the performance of the patched version compared
> > to original one.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovga...@ispras.ru>
> > ---
> > 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 | 379
> > ++++++++++++++++++++++------------------------
> > 5 files changed, 302 insertions(+), 294 deletions(-)
> >
> > diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> > index f9d2b4c..70ba39a 100644
> > --- a/target-mips/cpu.h
> > +++ b/target-mips/cpu.h
> > @@ -1015,4 +1015,32 @@ static inline void cpu_mips_store_cause(CPUMIPSState
> > *env,
> target_ulong val)
> > }
> > #endif
> >
> > +static inline void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
> > + uint32_t exception,
> > + int error_code,
> > + uintptr_t pc)
> > +{
> > + 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);
>
> What about creating a cpu_loop_exit_restore(cs, pc) (maybe with a better
> name?) in the common code, if we now have to repeat this pattern for
> every target?
Ok.
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index fd063a2..f87d5ac 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -1677,15 +1677,21 @@ generate_exception_err (DisasContext *ctx, int
> > excp, int err)
> > gen_helper_raise_exception_err(cpu_env, texcp, terr);
> > tcg_temp_free_i32(terr);
> > tcg_temp_free_i32(texcp);
> > + ctx->bstate = BS_STOP;
> > }
> >
> > static inline void
> > generate_exception (DisasContext *ctx, int excp)
> > {
> > - save_cpu_state(ctx, 1);
> > gen_helper_0e0i(raise_exception, excp);
> > }
> >
> > +static inline void
> > +generate_exception_end(DisasContext *ctx, int excp)
> > +{
> > + generate_exception_err(ctx, excp, 0);
> > +}
> > +
>
> This sets error_code to 0, which is different than leaving it unchanged.
> This might be ok, but have you checked there is no side effect?
Previous version called do_raise_exception, which passes 0 as error_code.
>
> > /* Addresses computation */
> > static inline void gen_op_addr_add (DisasContext *ctx, TCGv ret, TCGv
> > arg0, TCGv arg1)
> > {
> > @@ -1731,7 +1737,7 @@ static inline void check_cp1_enabled(DisasContext
> > *ctx)
> > static inline void check_cop1x(DisasContext *ctx)
> > {
> > if (unlikely(!(ctx->hflags & MIPS_HFLAG_COP1X)))
> > - generate_exception(ctx, EXCP_RI);
> > + generate_exception_end(ctx, EXCP_RI);
>
> I don't think it is correct. Before triggering such an exception, we
> were saving the CPU state, and not going through retranslation. With
> this change, we don't save the CPU state, but we don't go through
> retranslation either.
>
> The rule is to either go through retranslation, or to save the CPU state
> before a possible exception.
generate_exception_end saves CPU state and stops the translation
through calling of generate_exception_err function.
Pavel Dovgalyuk
