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

[Fabien Chouteau <chouteau@adacore.com>]

On 24/09/2011 10:29, Blue Swirl wrote:
> On Thu, Sep 22, 2011 at 3:42 PM, Fabien Chouteau <chouteau@adacore.com> wrote:
>> Remove faulty access to global "env" in op_helper.c, replaced by function argument.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>>  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.

I'll wait for your patches to be applied.

Thanks,

-- 
Fabien Chouteau