Re: [Qemu-devel] [PATCH RFC 5/6] target-i386: move reset callback to cpu.c

[Igor Mammedov]

----- Original Message -----
> From: "Jan Kiszka" <address@hidden>
> To: "Igor Mammedov" <address@hidden>
> Cc: address@hidden, address@hidden, address@hidden, address@hidden
> Sent: Tuesday, April 17, 2012 1:28:45 PM
> Subject: Re: [PATCH RFC 5/6] target-i386: move reset callback to cpu.c
> 
> On 2012-04-17 01:37, Igor Mammedov wrote:
> > From: Igor Mammedov <address@hidden>
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/pc.c           |    8 --------
> >  target-i386/cpu.c |    8 ++++++++
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/pc.c b/hw/pc.c
> > index db381ab..157327a 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -887,13 +887,6 @@ void pc_acpi_smi_interrupt(void *opaque, int
> > irq, int level)
> >      }
> >  }
> >  
> > -static void pc_cpu_reset(void *opaque)
> > -{
> > -    CPUX86State *env = opaque;
> > -
> > -    cpu_state_reset(env);
> > -}
> > -
> >  void pc_cpus_init(const char *cpu_model)
> >  {
> >      CPUX86State *env;
> > @@ -904,7 +897,6 @@ void pc_cpus_init(const char *cpu_model)
> >          if (!env) {
> >              exit(1);
> >          }
> > -        qemu_register_reset(pc_cpu_reset, env);
> >      }
> >  }
> >  
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index e4dcf52..d92ec58 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1456,6 +1456,13 @@ static void x86_cpu_reset(CPUState *s)
> >      env->halted = !(cpu_get_apic_base(env->apic_state) &
> >      MSR_IA32_APICBASE_BSP);
> >  }
> >  
> > +static void pc_cpu_reset(void *opaque)
> > +{
> > +    CPUX86State *env = opaque;
> > +
> > +    cpu_reset(ENV_GET_CPU(env));
> > +}
> > +
> 
> I'm not fully familiar with QOM for CPUs, but can't we avoid this
> wrapper completely and just register cpu_reset directly?

I'd like to do so, but qemu_register_reset requires QEMUResetHandler * as first
arg. I could just cast cpu_reset to it and be done with it, but not I'm
sure people will like it.

We probably could get rid of it in the future when/if reset is implemented on
QOM hierarchy. Then we just reset /machine and it propagates to children.
and there won't be need in qemu_register_reset hack here.

> 
> >  static void mce_init(X86CPU *cpu)
> >  {
> >      CPUX86State *cenv = &cpu->env;
> > @@ -1574,6 +1581,7 @@ static void x86_cpu_realize(Object *obj,
> > Error **errp)
> >      CPUX86State *env = &cpu->env;
> >  
> >      qemu_init_vcpu(env);
> > +    qemu_register_reset(pc_cpu_reset, env);
> >      cpu_reset(CPU(cpu));
> >  }
> >  
> 
> Jan
> 
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
>