Re: [Qemu-devel] [PATCH] [SPARC] Fix segfault in Leon3

[Blue Swirl]

On Thu, Sep 22, 2011 at 3:42 PM, Fabien Chouteau <address@hidden> wrote:
> Remove faulty access to global "env" in op_helper.c, replaced by function 
> argument.
>
> Signed-off-by: Fabien Chouteau <address@hidden>
> ---
>  target-sparc/cpu.h       |   10 ++++++----
>  target-sparc/helper.c    |    2 +-
>  target-sparc/op_helper.c |    6 +++---
>  3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 19de5ba..8245258 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -356,7 +356,9 @@ struct QEMUFile;
>  void cpu_put_timer(struct QEMUFile *f, CPUTimer *s);
>  void cpu_get_timer(struct QEMUFile *f, CPUTimer *s);
>
> -typedef struct CPUSPARCState {
> +typedef struct CPUSPARCState CPUSPARCState;

Please remove these changes. Using CPUState instead of CPUSPARCState
makes the code more easily reusable across targets.

> +
> +struct CPUSPARCState {
>     target_ulong gregs[8]; /* general registers */
>     target_ulong *regwptr; /* pointer to current register window */
>     target_ulong pc;       /* program counter */
> @@ -478,11 +480,11 @@ typedef struct CPUSPARCState {
>     sparc_def_t *def;
>
>     void *irq_manager;
> -    void (*qemu_irq_ack) (void *irq_manager, int intno);
> +    void (*qemu_irq_ack) (CPUSPARCState *env, void *irq_manager, int intno);
>
>     /* Leon3 cache control */
>     uint32_t cache_control;
> -} CPUSPARCState;
> +};
>
>  #ifndef NO_CPU_IO_DEFS
>  /* helper.c */
> @@ -521,7 +523,7 @@ void cpu_change_pstate(CPUState *env1, uint32_t 
> new_pstate);
>  int cpu_cwp_inc(CPUState *env1, int cwp);
>  int cpu_cwp_dec(CPUState *env1, int cwp);
>  void cpu_set_cwp(CPUState *env1, int new_cwp);
> -void leon3_irq_manager(void *irq_manager, int intno);
> +void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno);
>
>  /* sun4m.c, sun4u.c */
>  void cpu_check_irqs(CPUSPARCState *env);
> diff --git a/target-sparc/helper.c b/target-sparc/helper.c
> index c80531a..a84796c 100644
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -1119,7 +1119,7 @@ void do_interrupt(CPUState *env)
>  #if !defined(CONFIG_USER_ONLY)
>     /* IRQ acknowledgment */
>     if ((intno & ~15) == TT_EXTINT && env->qemu_irq_ack != NULL) {
> -        env->qemu_irq_ack(env->irq_manager, intno);
> +        env->qemu_irq_ack(env, env->irq_manager, intno);
>     }
>  #endif
>  }
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index 48e1db8..ee90268 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -1667,7 +1667,7 @@ static void dump_asi(const char *txt, target_ulong 
> addr, int asi, int size,
>
>  /* Leon3 cache control */
>
> -static void leon3_cache_control_int(void)
> +static void leon3_cache_control_int(CPUSPARCState *env)
>  {
>     uint32_t state = 0;
>
> @@ -1760,10 +1760,10 @@ static uint64_t leon3_cache_control_ld(target_ulong 
> addr, int size)
>     return ret;
>  }
>
> -void leon3_irq_manager(void *irq_manager, int intno)
> +void leon3_irq_manager(CPUSPARCState *env, void *irq_manager, int intno)

There is actually no need for this code to remain in op_helper.c after
this, it can be moved to helper.c. My AREG0 patches also make the same
changes (unfortunately I didn't realize that it also fixes a bug) but
also move the code to int_helper.c which is not compiled with
HELPER_CFLAGS. I can rebase if you'd like this fix applied before
that.

>  {
>     leon3_irq_ack(irq_manager, intno);
> -    leon3_cache_control_int();
> +    leon3_cache_control_int(env);
>  }
>
>  uint64_t helper_ld_asi(target_ulong addr, int asi, int size, int sign)
> --
> 1.7.4.1
>
>