Re: [Qemu-devel] [PATCH qom-next 42/59] xtensa_pic: Pass XtensaCPU to xtensa_ccompare_cb()

[Andreas Färber]

Am 10.10.2012 17:35, schrieb Max Filippov:
> On Wed, Oct 10, 2012 at 7:15 PM, Andreas Färber <address@hidden> wrote:
>> Am 23.05.2012 05:08, schrieb Andreas Färber:
>>> Needed for cpu_has_work().
>>>
>>> Signed-off-by: Andreas Färber <address@hidden>
>>
>> Max, could you ack this trivial patch please? It still applies.
> 
> Well, it does but why do you want to add a level of indirection here?
> Does that mean that cpu->env may change during cpu lifetime?
> Commit message is not very helpful here.

Patch 43/59 in that series updates cpu_has_work() argument to CPUState*,
thus this patch prepares xtensa_pic as one of the callers.

This is the only xtensa patch I have in my queue, so I wanted to avoid
hitting you with a large resend.

I see now that xtensa_set_irq could get an XtensaCPU opaque as well, but
so far there was no need apparently, so that can be changed later.

For target-specific code my general rule of thumb is, use FooCPU rather
than CPUFooState arguments and opaques wherever possible since the need
is growing. Also, any new fields that are not accessed by TCG should be
placed into FooCPU rather than CPUFooState.

Andreas

>>> @@ -139,12 +140,14 @@ static void xtensa_ccompare_cb(void *opaque)
>>>
>>>  void xtensa_irq_init(CPUXtensaState *env)
>>>  {
>>> +    XtensaCPU *cpu = xtensa_env_get_cpu(env);
>>> +
>>>      env->irq_inputs = (void **)qemu_allocate_irqs(
>>>              xtensa_set_irq, env, env->config->ninterrupt);
>>>      if (xtensa_option_enabled(env->config, XTENSA_OPTION_TIMER_INTERRUPT) 
>>> &&
>>>              env->config->nccompare > 0) {
>>>          env->ccompare_timer =
>>> -            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, env);
>>> +            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, cpu);
>>>      }
>>>  }

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