Re: [Qemu-devel] [PATCH 2/2] target-i386: move cpu_reset and reset callback to cpu.c

[Andreas Färber]

Am 23.07.2012 15:22, schrieb Igor Mammedov:
> Moving reset callback into cpu object from board level and
> resetting cpu at the end of x86_cpu_realize() will allow properly
> create cpu object during run-time (hotplug) without calling reset externaly.
> 
> When reset over QOM hierarchy is implemented, reset callback
> should be removed.
> 
> v2:
>   - leave cpu_reset in pc_new_cpu() for now, it's to be cleaned up when APIC
>     init is moved in cpu.c
> 
> Signed-off-by: Igor Mammedov <address@hidden>

It's still not clear to me what this buys us. Wouldn't it be sufficient
to add the cpu_reset() call in x86_cpu_realize()? Otherwise this defers
from all other machines, so I'd rather do it completely or not at all.
Especially since it's only an interim solution.

Andreas

> ---
>  hw/pc.c           |    9 +--------
>  target-i386/cpu.c |   14 ++++++++++++++
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index a920686..bd193f3 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -904,12 +904,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
> level)
>      }
>  }
>  
> -static void pc_cpu_reset(void *opaque)
> -{
> -    X86CPU *cpu = opaque;
> -    cpu_reset(CPU(cpu));
> -}
> -
>  static X86CPU *pc_new_cpu(const char *cpu_model)
>  {
>      X86CPU *cpu;
> @@ -924,8 +918,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
>      if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
>          env->apic_state = apic_init(env, env->cpuid_apic_id);
>      }
> -    qemu_register_reset(pc_cpu_reset, cpu);
> -    pc_cpu_reset(cpu);
> +    cpu_reset(CPU(cpu));
>      return cpu;
>  }
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index cf612a8..60b8e13 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -31,6 +31,8 @@
>  
>  #include "hyperv.h"
>  
> +#include "hw/hw.h"
> +
>  /* feature flags taken from "Intel Processor Identification and the CPUID
>   * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
>   * between feature naming conventions, aliases may be added.
> @@ -1702,6 +1704,13 @@ bool cpu_is_bsp(X86CPU *cpu)
>  {
>      return cpu_get_apic_base(cpu->env.apic_state) & MSR_IA32_APICBASE_BSP;
>  }
> +
> +/* TODO: remove me, when reset over QOM tree is implemented */
> +static void x86_cpu_machine_reset_cb(void *opaque)
> +{
> +    X86CPU *cpu = opaque;
> +    cpu_reset(CPU(cpu));
> +}
>  #endif
>  
>  static void mce_init(X86CPU *cpu)
> @@ -1724,8 +1733,13 @@ void x86_cpu_realize(Object *obj, Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(obj);
>  
> +#ifndef CONFIG_USER_ONLY
> +    qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
> +#endif
> +
>      mce_init(cpu);
>      qemu_init_vcpu(&cpu->env);
> +    cpu_reset(CPU(cpu));
>  }
>  
>  static void x86_cpu_initfn(Object *obj)
> 


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