[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/3] target-mips: improve exceptions handling
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/3] target-mips: improve exceptions handling |
Date: |
Wed, 1 Jul 2015 18:37:22 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On 2015-06-29 09:57, Pavel Dovgaluk wrote:
> > From: Aurelien Jarno [mailto:address@hidden
> > 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 <address@hidden>
> > > ---
> > > 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.
Ok, it's all fine then.
> >
> > > /* 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.
I missed that. Thanks for pointing me to that. That looks fine then.
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
address@hidden http://www.aurel32.net
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v3 2/3] target-mips: improve exceptions handling,
Aurelien Jarno <=