[RFC,5/6] target-i386: move reset callback to cpu.c

From: Igor Mammedov <niallain@gmail.com>

Signed-off-by: Igor Mammedov <niallain@gmail.com>
---
 hw/pc.c           |    8 --------
 target-i386/cpu.c |    8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

On 2012-04-17 01:37, Igor Mammedov wrote:
> From: Igor Mammedov <niallain@gmail.com>
> 
> Signed-off-by: Igor Mammedov <niallain@gmail.com>
> ---
>  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?

>  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

----- Original Message -----
> From: "Jan Kiszka" <jan.kiszka@siemens.com>
> To: "Igor Mammedov" <imammedo@redhat.com>
> Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, afaerber@suse.de, aliguori@us.ibm.com
> 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 <niallain@gmail.com>
> > 
> > Signed-off-by: Igor Mammedov <niallain@gmail.com>
> > ---
> >  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
>

Am 17.04.2012 13:40, schrieb Igor Mammedov:
> ----- Original Message -----
>> From: "Jan Kiszka" <jan.kiszka@siemens.com>
>> To: "Igor Mammedov" <imammedo@redhat.com>
>> Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, afaerber@suse.de, aliguori@us.ibm.com
>> 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 <niallain@gmail.com>
>>>
>>> Signed-off-by: Igor Mammedov <niallain@gmail.com>
>>> ---
>>>  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.

Please don't, especially not with CPU*State, cf. the cpu_state_reset()
commit.

But I'd suggest to rename it to x86_cpu_machine_reset if in cpu.c and to
pass the X86CPU as opaque.

> 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.

Affirmative.

Andreas