Patchwork target-mips: Use EXCP_SC rather than a magic number

login
register
mail settings
Submitter 陳韋任
Date Dec. 20, 2012, 2:14 a.m.
Message ID <20121220021420.GA11565@cs.nctu.edu.tw>
Download mbox | patch
Permalink /patch/207600/
State New
Headers show

Comments

陳韋任 - Dec. 20, 2012, 2:14 a.m.
Hi Li,

> > > Signed-off-by: Chen Wei-Ren <chenwj@iis.sinica.edu.tw>
> > > ---
> > >  target-mips/op_helper.c |    6 +++---
> > >  1 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> > > index f45d494..98a445c 100644
> > > --- a/target-mips/op_helper.c
> > > +++ b/target-mips/op_helper.c
> > > @@ -39,10 +39,10 @@ static inline void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
> > >                                                          uintptr_t pc)
> > >  {
> > >      TranslationBlock *tb;
> > > -#if 1
> > > -    if (exception < 0x100)
> > > +    if (exception < EXCP_SC) {
> > >          qemu_log("%s: %d %d\n", __func__, exception, error_code);
> > > -#endif
> > > +    }
> > > +
> 
> seems original '#if 1, #endif' statement
> is  for temporary debug only,
> so maybe can be concealed out entirely.
> for log purpose, every exception code
> will log when 'do_interrupt' for MIPS
> if log is enabled.

  Thanks for your feedback. You mean the code snippet below?

--- target-mips/helper.c
void do_interrupt (CPUMIPSState *env)
{
    ...

    if (qemu_log_enabled() && env->exception_index != EXCP_EXT_INTERRUPT) {
        if (env->exception_index < 0 || env->exception_index > EXCP_LAST)
            name = "unknown";
        else
            name = excp_names[env->exception_index];

        qemu_log("%s enter: PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx " %s exception\n",
                 __func__, env->active_tc.PC, env->CP0_EPC, name);
    }

    ...
}

  Maybe we can do this way?

---
---

  Aurelien, Johnson, thoughts? :)

Regards,
chenwj
Aurelien Jarno - Jan. 1, 2013, 11:07 a.m.
On Thu, Dec 20, 2012 at 10:14:20AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> Hi Li,
> 
> > > > Signed-off-by: Chen Wei-Ren <chenwj@iis.sinica.edu.tw>
> > > > ---
> > > >  target-mips/op_helper.c |    6 +++---
> > > >  1 files changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> > > > index f45d494..98a445c 100644
> > > > --- a/target-mips/op_helper.c
> > > > +++ b/target-mips/op_helper.c
> > > > @@ -39,10 +39,10 @@ static inline void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
> > > >                                                          uintptr_t pc)
> > > >  {
> > > >      TranslationBlock *tb;
> > > > -#if 1
> > > > -    if (exception < 0x100)
> > > > +    if (exception < EXCP_SC) {
> > > >          qemu_log("%s: %d %d\n", __func__, exception, error_code);
> > > > -#endif
> > > > +    }
> > > > +
> > 
> > seems original '#if 1, #endif' statement
> > is  for temporary debug only,
> > so maybe can be concealed out entirely.
> > for log purpose, every exception code
> > will log when 'do_interrupt' for MIPS
> > if log is enabled.
> 
>   Thanks for your feedback. You mean the code snippet below?
> 
> --- target-mips/helper.c
> void do_interrupt (CPUMIPSState *env)
> {
>     ...
> 
>     if (qemu_log_enabled() && env->exception_index != EXCP_EXT_INTERRUPT) {
>         if (env->exception_index < 0 || env->exception_index > EXCP_LAST)
>             name = "unknown";
>         else
>             name = excp_names[env->exception_index];
> 
>         qemu_log("%s enter: PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx " %s exception\n",
>                  __func__, env->active_tc.PC, env->CP0_EPC, name);
>     }
> 
>     ...
> }
> 

I don't think this is necessary to add a qemu_log_enabled(), this is
already something included in qemu_log().

Patch

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 31602ac..507a213 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -616,9 +616,9 @@  enum {
     EXCP_DSPDIS,
 
     EXCP_LAST = EXCP_DSPDIS,
+    /* Dummy exception for conditional stores. */
+    EXCP_SC = EXCP_LAST,
 };
-/* Dummy exception for conditional stores.  */
-#define EXCP_SC 0x100
 
 /*
  * This is an interrnally generated WAKE request line.