Re: [Qemu-devel] [PATCH] target-moxie: set do_interrupt to a target-specific helper function

[Andreas Färber]

Am 31.03.2013 03:35, schrieb Dunrong Huang:
> The value of "do_interrupt" member of CPUClass shoule be set to a
> target-specific function, or it will lead to a segfault like below:
> 
> $ moxie-softmmu/qemu-system-moxie -M moxiesim
> Segmentation fault
> 
> Cc: Anthony Green <address@hidden>
> Cc: Blue Swirl <address@hidden>
> Cc: Andreas Färber <address@hidden>
> Signed-off-by: Dunrong Huang <address@hidden>
> ---
>  target-moxie/cpu.c    | 1 +
>  target-moxie/cpu.h    | 2 +-
>  target-moxie/helper.c | 7 +++++--
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
> index c17d3f0..c0855f0 100644
> --- a/target-moxie/cpu.c
> +++ b/target-moxie/cpu.c
> @@ -98,6 +98,7 @@ static void moxie_cpu_class_init(ObjectClass *oc, void 
> *data)
>      cc->class_by_name = moxie_cpu_class_by_name;
>  
>      dc->vmsd = &vmstate_moxie_cpu;
> +    cc->do_interrupt = moxie_cpu_do_interrupt;
>  }
>  
>  static void moxielite_initfn(Object *obj)
> diff --git a/target-moxie/cpu.h b/target-moxie/cpu.h
> index b96236f..988729a 100644
> --- a/target-moxie/cpu.h
> +++ b/target-moxie/cpu.h
> @@ -117,7 +117,7 @@ static inline MoxieCPU *moxie_env_get_cpu(CPUMoxieState 
> *env)
>  
>  MoxieCPU *cpu_moxie_init(const char *cpu_model);
>  int cpu_moxie_exec(CPUMoxieState *s);
> -void do_interrupt(CPUMoxieState *env);
> +void moxie_cpu_do_interrupt(CPUState *cs);
>  void moxie_translate_init(void);
>  int cpu_moxie_signal_handler(int host_signum, void *pinfo,
>                               void *puc);
> diff --git a/target-moxie/helper.c b/target-moxie/helper.c
> index 8604ce8..6e0ac2a 100644
> --- a/target-moxie/helper.c
> +++ b/target-moxie/helper.c
> @@ -102,7 +102,7 @@ void helper_debug(CPUMoxieState *env)
>  
>  #if defined(CONFIG_USER_ONLY)
>  
> -void do_interrupt(CPUState *env)
> +void moxie_cpu_do_interrupt(CPUState *env)
>  {
>      env->exception_index = -1;
>  }

Anthony, CPUState should not be named "env" but rather "cs" (to reserve
"cpu" for MoxieCPU). That's unrelated to this patch though.

> @@ -147,8 +147,11 @@ int cpu_moxie_handle_mmu_fault(CPUMoxieState *env, 
> target_ulong address,
>  }
>  
>  
> -void do_interrupt(CPUMoxieState *env)
> +void moxie_cpu_do_interrupt(CPUState *cs)
>  {
> +    MoxieCPU *cpu = MOXIE_CPU(cs);
> +    CPUMoxieState *env = &cpu->env;
> +
>      switch (env->exception_index) {
>      case MOXIE_EX_BREAK:
>          break;

That exception_index is used once from CPUMoxieState and once from
CPUState is telling me something is fishy here...

Are any test images available?

Hooking up cc->do_interrupt is the correct thing to do though, so that
could be sorted out later,

Reviewed-by: Andreas Färber <address@hidden>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg