diff mbox

[RFC,4/6] target-i386: convert cpu to 2-stage initialization

Message ID 1334619423-13012-5-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov April 16, 2012, 11:37 p.m. UTC 
From: Igor Mammedov <niallain@gmail.com>

Use realize to start a cpu prepared by x86_cpu_initfn.

v2:
Create apic for cpu only once.

Signed-off-by: Igor Mammedov <niallain@gmail.com>
---
 hw/pc.c              |   35 ++++---------------------
 target-i386/cpu.c    |   69 +++++++++++++++++++++++++++++++++++++++++++++++++-
 target-i386/helper.c |   46 +++-----------------------------
 3 files changed, 79 insertions(+), 71 deletions(-)

Comments

Paolo Bonzini April 17, 2012, 7:20 a.m. UTC | #1 
Il 17/04/2012 01:37, Igor Mammedov ha scritto:
> +    if (((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) && !env->apic_state) {
> +        if (kvm_irqchip_in_kernel()) {
> +            env->apic_state = qdev_create(NULL, "kvm-apic");
> +        } else {
> +            env->apic_state = qdev_create(NULL, "apic");
> +        }
> +        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
> +        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
> +	object_property_add_child(OBJECT(cpu), "apic", OBJECT(env->apic_state), NULL);

Ah, nevermind my previous comment. :)

Paolo
Igor Mammedov April 18, 2012, 4:03 p.m. UTC | #2 
On 04/17/2012 01:37 AM, Igor Mammedov wrote:
> From: Igor Mammedov<niallain@gmail.com>
>
> Use realize to start a cpu prepared by x86_cpu_initfn.
>
> v2:
> Create apic for cpu only once.
>
> Signed-off-by: Igor Mammedov<niallain@gmail.com>
> ---
>   hw/pc.c              |   35 ++++---------------------
>   target-i386/cpu.c    |   69 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   target-i386/helper.c |   46 +++-----------------------------
>   3 files changed, 79 insertions(+), 71 deletions(-)
>
[...]
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 30ae0c2..e4dcf52 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -30,6 +30,8 @@
>   #include "hyperv.h"
>
>   #include "qerror.h"
> +#include "hw/qdev.h"
> +#include "sysemu.h"
>
>   /* feature flags taken from "Intel Processor Identification and the CPUID
>    * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
> @@ -1489,16 +1491,73 @@ static void x86_set_cpu_model(Object *obj, const char *value, Error **errp)
>           fprintf(stderr, "Unable to find x86 CPU definition\n");
>           error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
>       }
> +
> +    mce_init(cpu);
> +
> +    if (((env->cpuid_features&  CPUID_APIC) || smp_cpus>  1)&&  !env->apic_state) {
I wonder if it is ok to drop "smp_cpus>  1" here?
Booted linux guest with
   -smp 2 -cpu host,-apic
and it sees/initializes only one cpu. So I guess that CPUID_APIC must be
present for smp machine and that makes "smp_cpus>  1" redundant.

> +        if (kvm_irqchip_in_kernel()) {
> +            env->apic_state = qdev_create(NULL, "kvm-apic");
> +        } else {
> +            env->apic_state = qdev_create(NULL, "apic");
> +        }
> +        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
> +        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
> +	object_property_add_child(OBJECT(cpu), "apic", OBJECT(env->apic_state), NULL);
> +
> +        /* We hard-wire the BSP to the first CPU. */
> +        if (env->cpu_index == 0) {
> +            apic_designate_bsp(env->apic_state);
> +        }
> +    }
> +}
[...]

-----
  Igor
diff mbox

Patch

diff --git a/hw/pc.c b/hw/pc.c
index d00ca0f..db381ab 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -894,40 +894,17 @@  static void pc_cpu_reset(void *opaque)
     cpu_state_reset(env);
 }
 
-static CPUX86State *pc_new_cpu(const char *cpu_model)
-{
-    CPUX86State *env;
-
-    env = cpu_init(cpu_model);
-    if (!env) {
-        exit(1);
-    }
-    if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
-        if (kvm_irqchip_in_kernel()) {
-            env->apic_state = qdev_create(NULL, "kvm-apic");
-        } else {
-            env->apic_state = qdev_create(NULL, "apic");
-        }
-        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
-        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
-        qdev_init_nofail(env->apic_state);
-
-        /* We hard-wire the BSP to the first CPU. */
-        if (env->cpu_index == 0) {
-            apic_designate_bsp(env->apic_state);
-        }
-    }
-    qemu_register_reset(pc_cpu_reset, env);
-    pc_cpu_reset(env);
-    return env;
-}
-
 void pc_cpus_init(const char *cpu_model)
 {
+    CPUX86State *env;
     int i;
 
     for(i = 0; i < smp_cpus; i++) {
-        pc_new_cpu(cpu_model);
+        env = cpu_init(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 30ae0c2..e4dcf52 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -30,6 +30,8 @@ 
 #include "hyperv.h"
 
 #include "qerror.h"
+#include "hw/qdev.h"
+#include "sysemu.h"
 
 /* feature flags taken from "Intel Processor Identification and the CPUID
  * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
@@ -1489,16 +1491,73 @@  static void x86_set_cpu_model(Object *obj, const char *value, Error **errp)
         fprintf(stderr, "Unable to find x86 CPU definition\n");
         error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
     }
+
+    mce_init(cpu);
+
+    if (((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) && !env->apic_state) {
+        if (kvm_irqchip_in_kernel()) {
+            env->apic_state = qdev_create(NULL, "kvm-apic");
+        } else {
+            env->apic_state = qdev_create(NULL, "apic");
+        }
+        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
+        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
+	object_property_add_child(OBJECT(cpu), "apic", OBJECT(env->apic_state), NULL);
+
+        /* We hard-wire the BSP to the first CPU. */
+        if (env->cpu_index == 0) {
+            apic_designate_bsp(env->apic_state);
+        }
+    }
+}
+
+static CPUDebugExcpHandler *prev_debug_excp_handler;
+
+static void breakpoint_handler(CPUX86State *env)
+{
+    CPUBreakpoint *bp;
+
+    if (env->watchpoint_hit) {
+        if (env->watchpoint_hit->flags & BP_CPU) {
+            env->watchpoint_hit = NULL;
+            if (check_hw_breakpoints(env, 0))
+                raise_exception_env(EXCP01_DB, env);
+            else
+                cpu_resume_from_signal(env, NULL);
+        }
+    } else {
+        QTAILQ_FOREACH(bp, &env->breakpoints, entry)
+            if (bp->pc == env->eip) {
+                if (bp->flags & BP_CPU) {
+                    check_hw_breakpoints(env, 1);
+                    raise_exception_env(EXCP01_DB, env);
+                }
+                break;
+            }
+    }
+    if (prev_debug_excp_handler)
+        prev_debug_excp_handler(env);
 }
 
 static void x86_cpu_initfn(Object *obj)
 {
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
+    static int inited;
 
     cpu_exec_init(env);
     env->cpuid_apic_id = env->cpu_index;
 
+    /* init various static tables used in TCG mode */
+    if (tcg_enabled() && !inited) {
+        inited = 1;
+        optimize_flags_init();
+#ifndef CONFIG_USER_ONLY
+        prev_debug_excp_handler =
+            cpu_set_debug_excp_handler(breakpoint_handler);
+#endif
+    }
+
     object_property_add_str(obj, "cpu-model",
         x86_get_cpu_model, x86_set_cpu_model, NULL);
 
@@ -1507,8 +1566,15 @@  static void x86_cpu_initfn(Object *obj)
 #else
     object_property_set_str(OBJECT(cpu), "qemu32", "cpu-model", NULL);
 #endif
+}
 
-    mce_init(cpu);
+static void x86_cpu_realize(Object *obj, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    CPUX86State *env = &cpu->env;
+
+    qemu_init_vcpu(env);
+    cpu_reset(CPU(cpu));
 }
 
 static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
@@ -1518,6 +1584,7 @@  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 
     xcc->parent_reset = cc->reset;
     cc->reset = x86_cpu_reset;
+    oc->realize = x86_cpu_realize;
 }
 
 static const TypeInfo x86_cpu_type_info = {
diff --git a/target-i386/helper.c b/target-i386/helper.c
index df33d83..de7637c 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -947,34 +947,6 @@  int check_hw_breakpoints(CPUX86State *env, int force_dr6_update)
     return hit_enabled;
 }
 
-static CPUDebugExcpHandler *prev_debug_excp_handler;
-
-static void breakpoint_handler(CPUX86State *env)
-{
-    CPUBreakpoint *bp;
-
-    if (env->watchpoint_hit) {
-        if (env->watchpoint_hit->flags & BP_CPU) {
-            env->watchpoint_hit = NULL;
-            if (check_hw_breakpoints(env, 0))
-                raise_exception_env(EXCP01_DB, env);
-            else
-                cpu_resume_from_signal(env, NULL);
-        }
-    } else {
-        QTAILQ_FOREACH(bp, &env->breakpoints, entry)
-            if (bp->pc == env->eip) {
-                if (bp->flags & BP_CPU) {
-                    check_hw_breakpoints(env, 1);
-                    raise_exception_env(EXCP01_DB, env);
-                }
-                break;
-            }
-    }
-    if (prev_debug_excp_handler)
-        prev_debug_excp_handler(env);
-}
-
 typedef struct MCEInjectionParams {
     Monitor *mon;
     CPUX86State *env;
@@ -1162,21 +1134,10 @@  CPUX86State *cpu_x86_init(const char *cpu_model)
     X86CPU *cpu;
     CPUX86State *env;
     Error *errp = NULL;
-    static int inited;
 
     cpu = X86_CPU(object_new(TYPE_X86_CPU));
     env = &cpu->env;
 
-    /* init various static tables used in TCG mode */
-    if (tcg_enabled() && !inited) {
-        inited = 1;
-        optimize_flags_init();
-#ifndef CONFIG_USER_ONLY
-        prev_debug_excp_handler =
-            cpu_set_debug_excp_handler(breakpoint_handler);
-#endif
-    }
-
     if (cpu_model) {
         object_property_set_str(OBJECT(cpu), cpu_model, "cpu-model", &errp);
         if (errp) {
@@ -1185,8 +1146,11 @@  CPUX86State *cpu_x86_init(const char *cpu_model)
         }
     }
 
-    qemu_init_vcpu(env);
-
+    object_property_set_bool(OBJECT(cpu), true, "realized", &errp);
+    if (errp) {
+        object_delete(OBJECT(cpu));
+        return NULL;
+    }
     return env;
 }