Patchwork [2/2] target-i386: move cpu_reset and reset callback to cpu.c

login
register
mail settings
Submitter Igor Mammedov
Date July 23, 2012, 1:22 p.m.
Message ID <1343049748-11539-3-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/172681/
State New
Headers show

Comments

Igor Mammedov - July 23, 2012, 1:22 p.m.
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 <imammedo@redhat.com>
---
 hw/pc.c           |    9 +--------
 target-i386/cpu.c |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 8 deletions(-)
Andreas Färber - Aug. 1, 2012, 2:09 p.m.
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 <imammedo@redhat.com>

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

Patch

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)