[Qemu-devel] [PATCH qom-next 0/5] target-i386: re-factor CPU creation/initialization to QOM

[Qemu-devel] [PATCH qom-next 0/5] target-i386: re-factor CPU creation/initialization to QOM

[Igor Mammedov]

Moving code related to CPU creation and initialization internal parts
from board level into apic and cpu objects will allow X86CPU to better
model QOM object life-cycle.
It will allow to create X86CPU as any other object by creating it with
object_new() then setting properties and then calling x86_cpu_realize()
to make it running. Later x86_cpu_realize() should become realize property.

git tree at: 
  https://github.com/imammedo/qemu/tree/x86-cpu-realize-v2

[Qemu-devel] [PATCH qom-next 1/5] target-i386: move cpu halted decision into x86_cpu_reset

[Igor Mammedov]

From: Igor Mammedov <niallain@gmail.com>

MP initialization protocol differs between cpu families, and for P6 and
onward models it is up to CPU to decide if it will be BSP using this
protocol, so try to model this. However there is no point in implementing
MP initialization protocol in qemu. Thus first CPU is always marked as BSP.

This patch:
 - moves decision to designate BSP from board into cpu, making cpu
self-sufficient in this regard. Later it will allow to cleanup hw/pc.c
and remove cpu_reset and wrappers from there.
 - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior
described in Inted SDM vol 3a part 1 chapter 8.4.1
 - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu is BSP

patch is based on Jan Kiszka's proposal:
    http://thread.gmane.org/gmane.comp.emulators.qemu/100806

Signed-off-by: Igor Mammedov <niallain@gmail.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/apic.h            |    2 +-
 hw/apic_common.c     |   18 ++++++++++++------
 hw/pc.c              |    9 ---------
 target-i386/cpu.c    |    7 +++++++
 target-i386/helper.c |    1 -
 target-i386/kvm.c    |    5 +++--
 6 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/hw/apic.h b/hw/apic.h
index 62179ce..d961ed4 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s);
 void apic_sipi(DeviceState *s);
 void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
                                    TPRAccess access);
+void apic_designate_bsp(DeviceState *d);
 
 /* pc.c */
-int cpu_is_bsp(CPUX86State *env);
 DeviceState *cpu_get_current_apic(void);
 
 #endif
diff --git a/hw/apic_common.c b/hw/apic_common.c
index 60b8259..23d51e8 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d)
         trace_cpu_get_apic_base((uint64_t)s->apicbase);
         return s->apicbase;
     } else {
-        trace_cpu_get_apic_base(0);
-        return 0;
+        trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP);
+        return MSR_IA32_APICBASE_BSP;
     }
 }
 
@@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d)
     s->timer_expiry = -1;
 }
 
+void apic_designate_bsp(DeviceState *d)
+{
+    if (d) {
+        APICCommonState *s = APIC_COMMON(d);
+        s->apicbase |= MSR_IA32_APICBASE_BSP;
+    }
+}
+
 static void apic_reset_common(DeviceState *d)
 {
     APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
-    bool bsp;
 
-    bsp = cpu_is_bsp(s->cpu_env);
     s->apicbase = 0xfee00000 |
-        (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
+        (s->apicbase & MSR_IA32_APICBASE_BSP) | MSR_IA32_APICBASE_ENABLE;
 
     s->vapic_paddr = 0;
     info->vapic_base_update(s);
 
     apic_init_reset(d);
 
-    if (bsp) {
+    if (s->apicbase & MSR_IA32_APICBASE_BSP) {
         /*
          * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
          * time typically by BIOS, so PIC interrupt can be delivered to the
diff --git a/hw/pc.c b/hw/pc.c
index 4167782..6ad1c0d 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -870,12 +870,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
     nb_ne2k++;
 }
 
-int cpu_is_bsp(CPUX86State *env)
-{
-    /* We hard-wire the BSP to the first CPU. */
-    return env->cpu_index == 0;
-}
-
 DeviceState *cpu_get_current_apic(void)
 {
     if (cpu_single_env) {
@@ -935,10 +929,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
 static void pc_cpu_reset(void *opaque)
 {
     X86CPU *cpu = opaque;
-    CPUX86State *env = &cpu->env;
-
     cpu_reset(CPU(cpu));
-    env->halted = !cpu_is_bsp(env);
 }
 
 static X86CPU *pc_new_cpu(const char *cpu_model)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 89b4ac7..c7c23bf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1704,6 +1704,13 @@ static void x86_cpu_reset(CPUState *s)
     env->dr[7] = DR7_FIXED_1;
     cpu_breakpoint_remove_all(env, BP_CPU);
     cpu_watchpoint_remove_all(env, BP_CPU);
+
+    /* We hard-wire the BSP to the first CPU. */
+    if (env->cpu_index == 0) {
+        apic_designate_bsp(env->apic_state);
+    }
+
+    env->halted = !(cpu_get_apic_base(env->apic_state) & MSR_IA32_APICBASE_BSP);
 }
 
 static void mce_init(X86CPU *cpu)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 8df109f..94f95b7 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1197,7 +1197,6 @@ void do_cpu_init(X86CPU *cpu)
     env->interrupt_request = sipi;
     env->pat = pat;
     apic_init_reset(env->apic_state);
-    env->halted = !cpu_is_bsp(env);
 }
 
 void do_cpu_sipi(X86CPU *cpu)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0d0d8f6..09621e5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -583,8 +583,9 @@ void kvm_arch_reset_vcpu(CPUX86State *env)
     env->interrupt_injected = -1;
     env->xcr0 = 1;
     if (kvm_irqchip_in_kernel()) {
-        env->mp_state = cpu_is_bsp(env) ? KVM_MP_STATE_RUNNABLE :
-                                          KVM_MP_STATE_UNINITIALIZED;
+        env->mp_state =
+            cpu_get_apic_base(env->apic_state) & MSR_IA32_APICBASE_BSP ?
+            KVM_MP_STATE_RUNNABLE : KVM_MP_STATE_UNINITIALIZED;
     } else {
         env->mp_state = KVM_MP_STATE_RUNNABLE;
     }
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH qom-next 1/5] target-i386: move cpu halted decision into x86_cpu_reset

[Peter Maydell]

On 22 May 2012 11:35, Igor Mammedov <imammedo@redhat.com> wrote:
> From: Igor Mammedov <niallain@gmail.com>
>
> MP initialization protocol differs between cpu families, and for P6 and
> onward models it is up to CPU to decide if it will be BSP using this
> protocol, so try to model this. However there is no point in implementing
> MP initialization protocol in qemu. Thus first CPU is always marked as BSP.

This breaks compilation of the i386-linux-user target:
target-i386/cpu.c: In function ‘x86_cpu_reset’:
target-i386/cpu.c:1710: error: implicit declaration of function
‘apic_designate_bsp’
target-i386/cpu.c:1710: error: nested extern declaration of ‘apic_designate_bsp’
target-i386/cpu.c:1713: error: implicit declaration of function
‘cpu_get_apic_base’
target-i386/cpu.c:1713: error: nested extern declaration of ‘cpu_get_apic_base’

-- PMM

Re: [Qemu-devel] [PATCH qom-next 1/5] target-i386: move cpu halted decision into x86_cpu_reset

[Igor Mammedov]

On 05/22/2012 12:59 PM, Peter Maydell wrote:
> On 22 May 2012 11:35, Igor Mammedov<imammedo@redhat.com>  wrote:
>> From: Igor Mammedov<niallain@gmail.com>
>>
>> MP initialization protocol differs between cpu families, and for P6 and
>> onward models it is up to CPU to decide if it will be BSP using this
>> protocol, so try to model this. However there is no point in implementing
>> MP initialization protocol in qemu. Thus first CPU is always marked as BSP.
>
> This breaks compilation of the i386-linux-user target:
> target-i386/cpu.c: In function ‘x86_cpu_reset’:
> target-i386/cpu.c:1710: error: implicit declaration of function
> ‘apic_designate_bsp’
> target-i386/cpu.c:1710: error: nested extern declaration of ‘apic_designate_bsp’
> target-i386/cpu.c:1713: error: implicit declaration of function
> ‘cpu_get_apic_base’
> target-i386/cpu.c:1713: error: nested extern declaration of ‘cpu_get_apic_base’
>
> -- PMM
>

I'm sorry for forgetting about testing -user target, I'll fix it and repost.

-- 
-----
Thanks,
  Igor

[Qemu-devel] [PATCH qom-next 2/5] target-i386: add cpu-model property to x86_cpu

[Igor Mammedov]

it's probably intermidiate step till cpu modeled as
sub-classes. After then we probably could drop it.

However it still could be used for overiding default
cpu subclasses definition, and probably renamed to
something like 'features'.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 cpu-defs.h           |    2 +-
 hw/pc.c              |   10 ----------
 target-i386/cpu.c    |   34 ++++++++++++++++++++++++++++++++++
 target-i386/helper.c |   16 ++++++++++++----
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index f49e950..8f4623c 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -221,7 +221,7 @@ typedef struct CPUWatchpoint {
     struct QemuCond *halt_cond;                                         \
     int thread_kicked;                                                  \
     struct qemu_work_item *queued_work_first, *queued_work_last;        \
-    const char *cpu_model_str;                                          \
+    char *cpu_model_str;                                                \
     struct KVMState *kvm_state;                                         \
     struct kvm_run *kvm_run;                                            \
     int kvm_fd;                                                         \
diff --git a/hw/pc.c b/hw/pc.c
index 6ad1c0d..00d738d 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -939,7 +939,6 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
 
     cpu = cpu_x86_init(cpu_model);
     if (cpu == NULL) {
-        fprintf(stderr, "Unable to find x86 CPU definition\n");
         exit(1);
     }
     env = &cpu->env;
@@ -955,15 +954,6 @@ void pc_cpus_init(const char *cpu_model)
 {
     int i;
 
-    /* init CPUs */
-    if (cpu_model == NULL) {
-#ifdef TARGET_X86_64
-        cpu_model = "qemu64";
-#else
-        cpu_model = "qemu32";
-#endif
-    }
-
     for(i = 0; i < smp_cpus; i++) {
         pc_new_cpu(cpu_model);
     }
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c7c23bf..538892d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1729,6 +1729,27 @@ static void mce_init(X86CPU *cpu)
     }
 }
 
+static char *x86_get_cpu_model(Object *obj, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    CPUX86State *env = &cpu->env;
+    return g_strdup(env->cpu_model_str);
+}
+
+static void x86_set_cpu_model(Object *obj, const char *value, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    CPUX86State *env = &cpu->env;
+
+    g_free((gpointer)env->cpu_model_str);
+    env->cpu_model_str = g_strdup(value);
+
+    if (cpu_x86_register(cpu, env->cpu_model_str) < 0) {
+        fprintf(stderr, "Unable to find x86 CPU definition\n");
+        error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+    }
+}
+
 void x86_cpu_realize(Object *obj, Error **errp)
 {
     X86CPU *cpu = X86_CPU(obj);
@@ -1769,7 +1790,20 @@ static void x86_cpu_initfn(Object *obj)
                         x86_cpuid_get_tsc_freq,
                         x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
 
+    object_property_add_str(obj, "cpu-model",
+        x86_get_cpu_model, x86_set_cpu_model, NULL);
+
     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
+    }
 }
 
 static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 94f95b7..6fc67a9 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1160,12 +1160,10 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
 X86CPU *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;
-    env->cpu_model_str = cpu_model;
 
     /* init various static tables used in TCG mode */
     if (tcg_enabled() && !inited) {
@@ -1176,7 +1174,17 @@ X86CPU *cpu_x86_init(const char *cpu_model)
             cpu_set_debug_excp_handler(breakpoint_handler);
 #endif
     }
-    if (cpu_x86_register(cpu, cpu_model) < 0) {
+
+    if (cpu_model) {
+        object_property_set_str(OBJECT(cpu), cpu_model, "cpu-model", &errp);
+    } else {
+#ifdef TARGET_X86_64
+        object_property_set_str(OBJECT(cpu), "qemu64", "cpu-model", &errp);
+#else
+        object_property_set_str(OBJECT(cpu), "qemu32", "cpu-model", &errp);
+#endif
+    }
+    if (errp) {
         object_delete(OBJECT(cpu));
         return NULL;
     }
-- 
1.7.7.6

[Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Igor Mammedov]

Move from apic_init in pc.c  the code that belongs to apic_init_common
and create/init apic in pc_new_cpu instead of separate func.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/apic_common.c |   16 ++++++++++++++++
 hw/msi.h         |    2 ++
 hw/pc.c          |   47 ++++++++---------------------------------------
 3 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/hw/apic_common.c b/hw/apic_common.c
index 23d51e8..703931b 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -21,6 +21,7 @@
 #include "apic_internal.h"
 #include "trace.h"
 #include "kvm.h"
+#include "msi.h"
 
 static int apic_irq_delivered;
 bool apic_report_tpr_access;
@@ -284,6 +285,7 @@ static int apic_init_common(SysBusDevice *dev)
     APICCommonClass *info;
     static DeviceState *vapic;
     static int apic_no;
+    static int apic_mapped;
 
     if (apic_no >= MAX_APICS) {
         return -1;
@@ -295,6 +297,20 @@ static int apic_init_common(SysBusDevice *dev)
 
     sysbus_init_mmio(dev, &s->io_memory);
 
+    /* XXX: mapping more APICs at the same memory location */
+    if (apic_mapped == 0) {
+        /* NOTE: the APIC is directly connected to the CPU - it is not
+           on the global memory bus. */
+        /* XXX: what if the base changes? */
+        sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0, MSI_ADDR_BASE);
+        apic_mapped = 1;
+    }
+
+    /* KVM does not support MSI yet. */
+    if (!kvm_irqchip_in_kernel()) {
+        msi_supported = true;
+    }
+
     if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
         vapic = sysbus_create_simple("kvmvapic", -1, NULL);
     }
diff --git a/hw/msi.h b/hw/msi.h
index 3040bb0..abd52b6 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -40,4 +40,6 @@ static inline bool msi_present(const PCIDevice *dev)
     return dev->cap_present & QEMU_PCI_CAP_MSI;
 }
 
+#define MSI_ADDR_BASE 0xfee00000
+
 #endif /* QEMU_MSI_H */
diff --git a/hw/pc.c b/hw/pc.c
index 00d738d..0eb0b73 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -879,44 +879,6 @@ DeviceState *cpu_get_current_apic(void)
     }
 }
 
-static DeviceState *apic_init(void *env, uint8_t apic_id)
-{
-    DeviceState *dev;
-    static int apic_mapped;
-
-    if (kvm_irqchip_in_kernel()) {
-        dev = qdev_create(NULL, "kvm-apic");
-    } else if (xen_enabled()) {
-        dev = qdev_create(NULL, "xen-apic");
-    } else {
-        dev = qdev_create(NULL, "apic");
-    }
-
-    qdev_prop_set_uint8(dev, "id", apic_id);
-    qdev_prop_set_ptr(dev, "cpu_env", env);
-    qdev_init_nofail(dev);
-
-    /* XXX: mapping more APICs at the same memory location */
-    if (apic_mapped == 0) {
-        /* NOTE: the APIC is directly connected to the CPU - it is not
-           on the global memory bus. */
-        /* XXX: what if the base changes? */
-        sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE);
-        apic_mapped = 1;
-    }
-
-    /* KVM does not support MSI yet. */
-    if (!kvm_irqchip_in_kernel()) {
-        msi_supported = true;
-    }
-
-    if (xen_msi_support()) {
-        msi_supported = true;
-    }
-
-    return dev;
-}
-
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
 {
     CPUX86State *s = opaque;
@@ -943,7 +905,14 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
     }
     env = &cpu->env;
     if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
-        env->apic_state = apic_init(env, env->cpuid_apic_id);
+        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);
     }
     qemu_register_reset(pc_cpu_reset, cpu);
     pc_cpu_reset(cpu);
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Jan Kiszka]

On 2012-05-22 07:35, Igor Mammedov wrote:
> Move from apic_init in pc.c  the code that belongs to apic_init_common
> and create/init apic in pc_new_cpu instead of separate func.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/apic_common.c |   16 ++++++++++++++++
>  hw/msi.h         |    2 ++
>  hw/pc.c          |   47 ++++++++---------------------------------------
>  3 files changed, 26 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index 23d51e8..703931b 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -21,6 +21,7 @@
>  #include "apic_internal.h"
>  #include "trace.h"
>  #include "kvm.h"
> +#include "msi.h"
>  
>  static int apic_irq_delivered;
>  bool apic_report_tpr_access;
> @@ -284,6 +285,7 @@ static int apic_init_common(SysBusDevice *dev)
>      APICCommonClass *info;
>      static DeviceState *vapic;
>      static int apic_no;
> +    static int apic_mapped;
>  
>      if (apic_no >= MAX_APICS) {
>          return -1;
> @@ -295,6 +297,20 @@ static int apic_init_common(SysBusDevice *dev)
>  
>      sysbus_init_mmio(dev, &s->io_memory);
>  
> +    /* XXX: mapping more APICs at the same memory location */
> +    if (apic_mapped == 0) {
> +        /* NOTE: the APIC is directly connected to the CPU - it is not
> +           on the global memory bus. */
> +        /* XXX: what if the base changes? */
> +        sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0, MSI_ADDR_BASE);
> +        apic_mapped = 1;
> +    }
> +
> +    /* KVM does not support MSI yet. */
> +    if (!kvm_irqchip_in_kernel()) {
> +        msi_supported = true;
> +    }
> +
>      if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
>          vapic = sysbus_create_simple("kvmvapic", -1, NULL);
>      }
> diff --git a/hw/msi.h b/hw/msi.h
> index 3040bb0..abd52b6 100644
> --- a/hw/msi.h
> +++ b/hw/msi.h
> @@ -40,4 +40,6 @@ static inline bool msi_present(const PCIDevice *dev)
>      return dev->cap_present & QEMU_PCI_CAP_MSI;
>  }
>  
> +#define MSI_ADDR_BASE 0xfee00000
> +
>  #endif /* QEMU_MSI_H */
> diff --git a/hw/pc.c b/hw/pc.c
> index 00d738d..0eb0b73 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -879,44 +879,6 @@ DeviceState *cpu_get_current_apic(void)
>      }
>  }
>  
> -static DeviceState *apic_init(void *env, uint8_t apic_id)
> -{
> -    DeviceState *dev;
> -    static int apic_mapped;
> -
> -    if (kvm_irqchip_in_kernel()) {
> -        dev = qdev_create(NULL, "kvm-apic");
> -    } else if (xen_enabled()) {
> -        dev = qdev_create(NULL, "xen-apic");
> -    } else {
> -        dev = qdev_create(NULL, "apic");
> -    }
> -
> -    qdev_prop_set_uint8(dev, "id", apic_id);
> -    qdev_prop_set_ptr(dev, "cpu_env", env);
> -    qdev_init_nofail(dev);
> -
> -    /* XXX: mapping more APICs at the same memory location */
> -    if (apic_mapped == 0) {
> -        /* NOTE: the APIC is directly connected to the CPU - it is not
> -           on the global memory bus. */
> -        /* XXX: what if the base changes? */
> -        sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE);

While at it, you should drop MSI_ADDR_BASE definition from pc.c.

> -        apic_mapped = 1;
> -    }
> -
> -    /* KVM does not support MSI yet. */
> -    if (!kvm_irqchip_in_kernel()) {
> -        msi_supported = true;
> -    }
> -
> -    if (xen_msi_support()) {
> -        msi_supported = true;
> -    }
> -
> -    return dev;
> -}
> -

You are loosing some xen bits here. But this will collide with latest
kvm pull request
(http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
You may want to base on uq/master.

>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>  {
>      CPUX86State *s = opaque;
> @@ -943,7 +905,14 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
>      }
>      env = &cpu->env;
>      if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
> -        env->apic_state = apic_init(env, env->cpuid_apic_id);
> +        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);
>      }
>      qemu_register_reset(pc_cpu_reset, cpu);
>      pc_cpu_reset(cpu);

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Igor Mammedov]

On 05/22/2012 12:48 PM, Jan Kiszka wrote:
> On 2012-05-22 07:35, Igor Mammedov wrote:
>> Move from apic_init in pc.c  the code that belongs to apic_init_common
>> and create/init apic in pc_new_cpu instead of separate func.
>>
>> Signed-off-by: Igor Mammedov<imammedo@redhat.com>
>> ---
>>   hw/apic_common.c |   16 ++++++++++++++++
>>   hw/msi.h         |    2 ++
>>   hw/pc.c          |   47 ++++++++---------------------------------------
>>   3 files changed, 26 insertions(+), 39 deletions(-)
>>
>> diff --git a/hw/apic_common.c b/hw/apic_common.c
>> index 23d51e8..703931b 100644
>> --- a/hw/apic_common.c
>> +++ b/hw/apic_common.c
>> @@ -21,6 +21,7 @@
>>   #include "apic_internal.h"
>>   #include "trace.h"
>>   #include "kvm.h"
>> +#include "msi.h"
>>
>>   static int apic_irq_delivered;
>>   bool apic_report_tpr_access;
>> @@ -284,6 +285,7 @@ static int apic_init_common(SysBusDevice *dev)
>>       APICCommonClass *info;
>>       static DeviceState *vapic;
>>       static int apic_no;
>> +    static int apic_mapped;
>>
>>       if (apic_no>= MAX_APICS) {
>>           return -1;
>> @@ -295,6 +297,20 @@ static int apic_init_common(SysBusDevice *dev)
>>
>>       sysbus_init_mmio(dev,&s->io_memory);
>>
>> +    /* XXX: mapping more APICs at the same memory location */
>> +    if (apic_mapped == 0) {
>> +        /* NOTE: the APIC is directly connected to the CPU - it is not
>> +           on the global memory bus. */
>> +        /* XXX: what if the base changes? */
>> +        sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0, MSI_ADDR_BASE);
>> +        apic_mapped = 1;
>> +    }
>> +
>> +    /* KVM does not support MSI yet. */
>> +    if (!kvm_irqchip_in_kernel()) {
>> +        msi_supported = true;
>> +    }
>> +
>>       if (!vapic&&  s->vapic_control&  VAPIC_ENABLE_MASK) {
>>           vapic = sysbus_create_simple("kvmvapic", -1, NULL);
>>       }
>> diff --git a/hw/msi.h b/hw/msi.h
>> index 3040bb0..abd52b6 100644
>> --- a/hw/msi.h
>> +++ b/hw/msi.h
>> @@ -40,4 +40,6 @@ static inline bool msi_present(const PCIDevice *dev)
>>       return dev->cap_present&  QEMU_PCI_CAP_MSI;
>>   }
>>
>> +#define MSI_ADDR_BASE 0xfee00000
>> +
>>   #endif /* QEMU_MSI_H */
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 00d738d..0eb0b73 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -879,44 +879,6 @@ DeviceState *cpu_get_current_apic(void)
>>       }
>>   }
>>
>> -static DeviceState *apic_init(void *env, uint8_t apic_id)
>> -{
>> -    DeviceState *dev;
>> -    static int apic_mapped;
>> -
>> -    if (kvm_irqchip_in_kernel()) {
>> -        dev = qdev_create(NULL, "kvm-apic");
>> -    } else if (xen_enabled()) {
>> -        dev = qdev_create(NULL, "xen-apic");
>> -    } else {
>> -        dev = qdev_create(NULL, "apic");
>> -    }
>> -
>> -    qdev_prop_set_uint8(dev, "id", apic_id);
>> -    qdev_prop_set_ptr(dev, "cpu_env", env);
>> -    qdev_init_nofail(dev);
>> -
>> -    /* XXX: mapping more APICs at the same memory location */
>> -    if (apic_mapped == 0) {
>> -        /* NOTE: the APIC is directly connected to the CPU - it is not
>> -           on the global memory bus. */
>> -        /* XXX: what if the base changes? */
>> -        sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE);
>
> While at it, you should drop MSI_ADDR_BASE definition from pc.c.

Consider it done.

>
>> -        apic_mapped = 1;
>> -    }
>> -
>> -    /* KVM does not support MSI yet. */
>> -    if (!kvm_irqchip_in_kernel()) {
>> -        msi_supported = true;
>> -    }
>> -
>> -    if (xen_msi_support()) {
>> -        msi_supported = true;
>> -    }
>> -
>> -    return dev;
>> -}
>> -
>
> You are loosing some xen bits here. But this will collide with latest
> kvm pull request
> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
> You may want to base on uq/master.
>

This patchset is based on Andreas' qom-next tree. Probably I should wait
till above mentioned kvm pull is pulled in and it aprears in qom-next.

...

-- 
-----
Thanks,
   Igor

Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Andreas Färber]

Am 22.05.2012 14:42, schrieb Igor Mammedov:
> On 05/22/2012 12:48 PM, Jan Kiszka wrote:
>> On 2012-05-22 07:35, Igor Mammedov wrote:
>>> -        apic_mapped = 1;
>>> -    }
>>> -
>>> -    /* KVM does not support MSI yet. */
>>> -    if (!kvm_irqchip_in_kernel()) {
>>> -        msi_supported = true;
>>> -    }
>>> -
>>> -    if (xen_msi_support()) {
>>> -        msi_supported = true;
>>> -    }
>>> -
>>> -    return dev;
>>> -}
>>> -
>>
>> You are loosing some xen bits here. But this will collide with latest
>> kvm pull request
>> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
>> You may want to base on uq/master.
>>
> 
> This patchset is based on Andreas' qom-next tree. Probably I should wait
> till above mentioned kvm pull is pulled in and it aprears in qom-next.

Jan, we currently have a chaos of concurrent, colliding QOM series.
qom-next was intended to resolve this but so far it's a rebasing patch
queue on top of master and not a repository with stable hashes so I
can't do PULLs myself but I could cherry-pick related patches if needed.

Igor, if you put the code movement init -> initfn into its own patch
I'll apply it to qom-next right away. Haven't looked at the series
in-depth yet.

We're not quite there yet with qom-next due to series and counterseries
and lack of input on realize/QBus. My current merge plan is as follows:

* Apply QOM CPUState series part 3 (cpu_state_reset) - aggressively done
last night, prerequisite for part 4.

* Apply the last two remaining ARM cpu_reset followup cleanups - waiting
for one ack by PMM.

* Post QOM CPUState series part 4 (CPU_COMMON) - still fiddling with
bisectability, hope to post today. This will show areas of conflicts wrt
apic and x86 and is quite invasive (qom-cpu branch on GitHub).

* Mix and match patches from Paolo's and Anthony's series for realizefn.
Hope to post a short-term compromise soon, leaving properties aside for
now. Apply it so that we finally have a realizefn.

* Post QOM CPUState series part 5 (CPUState conditionally as device).
WIP (qom-cpu-dev branch on GitHub), needed for hotplug IIUC and this
will enable integration with machine reset. Doesn't depend on part 4 so far.

* Apply PMM's ARM copro series - waiting for acks, still need to
carefully review the final CPUID movements.
* Post realizefn implementation on top - probably to be merged after
PMM's holiday, i.e. to master not to qom-next.

* Align part 4 with Igor's series, possibly rebase on part 5. See how
close to 1.2 we get and how the review of all open series goes.

Whatever progress we make on qom-next, the idea is to have qom-next
merged into master *first*, since it's getting really large. Don't know
what KVM PULL Jan is referring to - if it's for 1.1 then I'll rebase on
it but otherwise I expect series to get rebased onto master w/qom-next
before sending a PULL. That's why I asked target maintainers to queue
the patches from my part 3 in their queues, to avoid merge conflicts
once the 1.2 window opens.

For the current QOM series I'm fine rebasing myself so far.

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Jan Kiszka]

On 2012-05-22 11:24, Andreas Färber wrote:
> Am 22.05.2012 14:42, schrieb Igor Mammedov:
>> On 05/22/2012 12:48 PM, Jan Kiszka wrote:
>>> On 2012-05-22 07:35, Igor Mammedov wrote:
>>>> -        apic_mapped = 1;
>>>> -    }
>>>> -
>>>> -    /* KVM does not support MSI yet. */
>>>> -    if (!kvm_irqchip_in_kernel()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    if (xen_msi_support()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    return dev;
>>>> -}
>>>> -
>>>
>>> You are loosing some xen bits here. But this will collide with latest
>>> kvm pull request
>>> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
>>> You may want to base on uq/master.
>>>
>>
>> This patchset is based on Andreas' qom-next tree. Probably I should wait
>> till above mentioned kvm pull is pulled in and it aprears in qom-next.
> 
> Jan, we currently have a chaos of concurrent, colliding QOM series.
> qom-next was intended to resolve this but so far it's a rebasing patch
> queue on top of master and not a repository with stable hashes so I
> can't do PULLs myself but I could cherry-pick related patches if needed.
> 
> Igor, if you put the code movement init -> initfn into its own patch
> I'll apply it to qom-next right away. Haven't looked at the series
> in-depth yet.
> 
> We're not quite there yet with qom-next due to series and counterseries
> and lack of input on realize/QBus. My current merge plan is as follows:
> 
> * Apply QOM CPUState series part 3 (cpu_state_reset) - aggressively done
> last night, prerequisite for part 4.
> 
> * Apply the last two remaining ARM cpu_reset followup cleanups - waiting
> for one ack by PMM.
> 
> * Post QOM CPUState series part 4 (CPU_COMMON) - still fiddling with
> bisectability, hope to post today. This will show areas of conflicts wrt
> apic and x86 and is quite invasive (qom-cpu branch on GitHub).
> 
> * Mix and match patches from Paolo's and Anthony's series for realizefn.
> Hope to post a short-term compromise soon, leaving properties aside for
> now. Apply it so that we finally have a realizefn.
> 
> * Post QOM CPUState series part 5 (CPUState conditionally as device).
> WIP (qom-cpu-dev branch on GitHub), needed for hotplug IIUC and this
> will enable integration with machine reset. Doesn't depend on part 4 so far.
> 
> * Apply PMM's ARM copro series - waiting for acks, still need to
> carefully review the final CPUID movements.
> * Post realizefn implementation on top - probably to be merged after
> PMM's holiday, i.e. to master not to qom-next.
> 
> * Align part 4 with Igor's series, possibly rebase on part 5. See how
> close to 1.2 we get and how the review of all open series goes.
> 
> Whatever progress we make on qom-next, the idea is to have qom-next
> merged into master *first*, since it's getting really large. Don't know
> what KVM PULL Jan is referring to - if it's for 1.1 then I'll rebase on
> it but otherwise I expect series to get rebased onto master w/qom-next
> before sending a PULL. That's why I asked target maintainers to queue
> the patches from my part 3 in their queues, to avoid merge conflicts
> once the 1.2 window opens.

In this case, conflicts may only be caused between
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91186 (pc: Enable
MSI support at APIC level) and this particular patch (that requires some
more work anyway).

Maybe you can test-merge the KVM pull in your current tree that is
supposed to go in first and ask Avi/Marcelo to the provide uq/master
baseline on top of yours in case of conflicts. I would provide an update
of my patches on top of that afterward.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Paolo Bonzini]

Il 22/05/2012 16:24, Andreas Färber ha scritto:
> * Mix and match patches from Paolo's and Anthony's series for realizefn.
> Hope to post a short-term compromise soon, leaving properties aside for
> now. Apply it so that we finally have a realizefn.

It's still not clear to me what's missing to get the QBus series in,
except perhaps rebasing it... once that is in, I can post again the
realize patches.

Paolo

Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Igor Mammedov]

On 05/22/2012 04:24 PM, Andreas Färber wrote:
> Am 22.05.2012 14:42, schrieb Igor Mammedov:
>> On 05/22/2012 12:48 PM, Jan Kiszka wrote:
>>> On 2012-05-22 07:35, Igor Mammedov wrote:
>>>> -        apic_mapped = 1;
>>>> -    }
>>>> -
>>>> -    /* KVM does not support MSI yet. */
>>>> -    if (!kvm_irqchip_in_kernel()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    if (xen_msi_support()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    return dev;
>>>> -}
>>>> -
>>>
>>> You are loosing some xen bits here. But this will collide with latest
>>> kvm pull request
>>> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
>>> You may want to base on uq/master.
>>>
>>
>> This patchset is based on Andreas' qom-next tree. Probably I should wait
>> till above mentioned kvm pull is pulled in and it aprears in qom-next.
>
> Jan, we currently have a chaos of concurrent, colliding QOM series.
> qom-next was intended to resolve this but so far it's a rebasing patch
> queue on top of master and not a repository with stable hashes so I
> can't do PULLs myself but I could cherry-pick related patches if needed.
>
> Igor, if you put the code movement init ->  initfn into its own patch
> I'll apply it to qom-next right away. Haven't looked at the series
> in-depth yet.
I'll split this patch in apic and cpu parts and repost with fixes for issues
Jan and Peter spotted.

>
> We're not quite there yet with qom-next due to series and counterseries
> and lack of input on realize/QBus. My current merge plan is as follows:
>
> * Apply QOM CPUState series part 3 (cpu_state_reset) - aggressively done
> last night, prerequisite for part 4.
>
> * Apply the last two remaining ARM cpu_reset followup cleanups - waiting
> for one ack by PMM.
>
> * Post QOM CPUState series part 4 (CPU_COMMON) - still fiddling with
> bisectability, hope to post today. This will show areas of conflicts wrt
> apic and x86 and is quite invasive (qom-cpu branch on GitHub).
>
> * Mix and match patches from Paolo's and Anthony's series for realizefn.
> Hope to post a short-term compromise soon, leaving properties aside for
> now. Apply it so that we finally have a realizefn.
>
> * Post QOM CPUState series part 5 (CPUState conditionally as device).
> WIP (qom-cpu-dev branch on GitHub), needed for hotplug IIUC and this
> will enable integration with machine reset. Doesn't depend on part 4 so far.
>
> * Apply PMM's ARM copro series - waiting for acks, still need to
> carefully review the final CPUID movements.
> * Post realizefn implementation on top - probably to be merged after
> PMM's holiday, i.e. to master not to qom-next.
>
> * Align part 4 with Igor's series, possibly rebase on part 5. See how
> close to 1.2 we get and how the review of all open series goes.
>
> Whatever progress we make on qom-next, the idea is to have qom-next
> merged into master *first*, since it's getting really large. Don't know
> what KVM PULL Jan is referring to - if it's for 1.1 then I'll rebase on
> it but otherwise I expect series to get rebased onto master w/qom-next
> before sending a PULL. That's why I asked target maintainers to queue
> the patches from my part 3 in their queues, to avoid merge conflicts
> once the 1.2 window opens.
>
> For the current QOM series I'm fine rebasing myself so far.
>
> Regards,
> Andreas
>

-- 
-----
  Igor

Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Jan Kiszka]

On 2012-05-22 07:35, Igor Mammedov wrote:
> Move from apic_init in pc.c  the code that belongs to apic_init_common
> and create/init apic in pc_new_cpu instead of separate func.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/apic_common.c |   16 ++++++++++++++++
>  hw/msi.h         |    2 ++
>  hw/pc.c          |   47 ++++++++---------------------------------------
>  3 files changed, 26 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index 23d51e8..703931b 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -21,6 +21,7 @@
>  #include "apic_internal.h"
>  #include "trace.h"
>  #include "kvm.h"
> +#include "msi.h"
>  
>  static int apic_irq_delivered;
>  bool apic_report_tpr_access;
> @@ -284,6 +285,7 @@ static int apic_init_common(SysBusDevice *dev)
>      APICCommonClass *info;
>      static DeviceState *vapic;
>      static int apic_no;
> +    static int apic_mapped;
>  
>      if (apic_no >= MAX_APICS) {
>          return -1;
> @@ -295,6 +297,20 @@ static int apic_init_common(SysBusDevice *dev)
>  
>      sysbus_init_mmio(dev, &s->io_memory);
>  
> +    /* XXX: mapping more APICs at the same memory location */
> +    if (apic_mapped == 0) {
> +        /* NOTE: the APIC is directly connected to the CPU - it is not
> +           on the global memory bus. */
> +        /* XXX: what if the base changes? */
> +        sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0, MSI_ADDR_BASE);
> +        apic_mapped = 1;
> +    }
> +
> +    /* KVM does not support MSI yet. */
> +    if (!kvm_irqchip_in_kernel()) {
> +        msi_supported = true;
> +    }
> +
>      if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
>          vapic = sysbus_create_simple("kvmvapic", -1, NULL);
>      }
> diff --git a/hw/msi.h b/hw/msi.h
> index 3040bb0..abd52b6 100644
> --- a/hw/msi.h
> +++ b/hw/msi.h
> @@ -40,4 +40,6 @@ static inline bool msi_present(const PCIDevice *dev)
>      return dev->cap_present & QEMU_PCI_CAP_MSI;
>  }
>  
> +#define MSI_ADDR_BASE 0xfee00000
> +
>  #endif /* QEMU_MSI_H */
> diff --git a/hw/pc.c b/hw/pc.c
> index 00d738d..0eb0b73 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -879,44 +879,6 @@ DeviceState *cpu_get_current_apic(void)
>      }
>  }
>  
> -static DeviceState *apic_init(void *env, uint8_t apic_id)
> -{
> -    DeviceState *dev;
> -    static int apic_mapped;
> -
> -    if (kvm_irqchip_in_kernel()) {
> -        dev = qdev_create(NULL, "kvm-apic");
> -    } else if (xen_enabled()) {
> -        dev = qdev_create(NULL, "xen-apic");
> -    } else {
> -        dev = qdev_create(NULL, "apic");
> -    }
> -
> -    qdev_prop_set_uint8(dev, "id", apic_id);
> -    qdev_prop_set_ptr(dev, "cpu_env", env);
> -    qdev_init_nofail(dev);
> -
> -    /* XXX: mapping more APICs at the same memory location */
> -    if (apic_mapped == 0) {
> -        /* NOTE: the APIC is directly connected to the CPU - it is not
> -           on the global memory bus. */
> -        /* XXX: what if the base changes? */
> -        sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE);

While at it, you should drop MSI_ADDR_BASE definition from pc.c.

> -        apic_mapped = 1;
> -    }
> -
> -    /* KVM does not support MSI yet. */
> -    if (!kvm_irqchip_in_kernel()) {
> -        msi_supported = true;
> -    }
> -
> -    if (xen_msi_support()) {
> -        msi_supported = true;
> -    }
> -
> -    return dev;
> -}
> -

You are loosing some xen bits here. But this will collide with latest
kvm pull request
(http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
You may want to base on uq/master.

>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>  {
>      CPUX86State *s = opaque;
> @@ -943,7 +905,14 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
>      }
>      env = &cpu->env;
>      if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
> -        env->apic_state = apic_init(env, env->cpuid_apic_id);
> +        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);
>      }
>      qemu_register_reset(pc_cpu_reset, cpu);
>      pc_cpu_reset(cpu);

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] [PATCH qom-next 4/5] target-i386: make initialize CPU in QOM way

[Igor Mammedov]

Make CPU creation/initialization consistent with QOM object
behavior in this, by moving tcg and apic initialization from board
level into CPU's initfn/realize calls and cpu_model property setter.

Which makes CPU object self-sufficient in respect of creation/initialization
and matches a typical object creation sequence, i.e.:
  - create CPU instance
  - set properties
  - realize object - (x86_cpu_realize will be converted into realize
      property setter, when it is implemented)

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/pc.c              |   32 +++++---------------------
 target-i386/cpu.c    |   60 +++++++++++++++++++++++++++++++++++++++++++++++++-
 target-i386/helper.c |   39 --------------------------------
 3 files changed, 65 insertions(+), 66 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 0eb0b73..677f9e0 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -894,37 +894,17 @@ static void pc_cpu_reset(void *opaque)
     cpu_reset(CPU(cpu));
 }
 
-static X86CPU *pc_new_cpu(const char *cpu_model)
-{
-    X86CPU *cpu;
-    CPUX86State *env;
-
-    cpu = cpu_x86_init(cpu_model);
-    if (cpu == NULL) {
-        exit(1);
-    }
-    env = &cpu->env;
-    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);
-    }
-    qemu_register_reset(pc_cpu_reset, cpu);
-    pc_cpu_reset(cpu);
-    return cpu;
-}
-
 void pc_cpus_init(const char *cpu_model)
 {
+    X86CPU *cpu;
     int i;
 
     for(i = 0; i < smp_cpus; i++) {
-        pc_new_cpu(cpu_model);
+        cpu = cpu_x86_init(cpu_model);
+        if (cpu == NULL) {
+            exit(1);
+        }
+        qemu_register_reset(pc_cpu_reset, cpu);
     }
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 538892d..0e804ea 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -31,6 +31,9 @@
 
 #include "hyperv.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
  * between feature naming conventions, aliases may be added.
@@ -1747,21 +1750,76 @@ static void x86_set_cpu_model(Object *obj, const char *value, Error **errp)
     if (cpu_x86_register(cpu, env->cpu_model_str) < 0) {
         fprintf(stderr, "Unable to find x86 CPU definition\n");
         error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+        return;
+    }
+
+    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");
+        }
+        object_property_add_child(OBJECT(cpu), "apic",
+            OBJECT(env->apic_state), NULL);
+
+        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
+        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
+    }
+}
+
+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);
     }
 }
 
 void x86_cpu_realize(Object *obj, Error **errp)
 {
     X86CPU *cpu = X86_CPU(obj);
+    CPUX86State *env = &cpu->env;
+
+    if (env->apic_state) {
+        if (qdev_init(env->apic_state) < 0) {
+            error_set(errp, QERR_DEVICE_INIT_FAILED,
+                object_get_typename(OBJECT(env->apic_state)));
+            return;
+        }
+    }
 
     mce_init(cpu);
-    qemu_init_vcpu(&cpu->env);
+    qemu_init_vcpu(env);
+    cpu_reset(CPU(cpu));
 }
 
 static void x86_cpu_initfn(Object *obj)
 {
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
+    static int inited;
 
     cpu_exec_init(env);
 
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 6fc67a9..443092e 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;
@@ -1161,20 +1133,9 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 {
     X86CPU *cpu;
     Error *errp = NULL;
-    static int inited;
 
     cpu = X86_CPU(object_new(TYPE_X86_CPU));
 
-    /* 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);
     } else {
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH qom-next 4/5] target-i386: make initialize CPU in QOM way

[Jan Kiszka]

On 2012-05-22 07:35, Igor Mammedov wrote:
> Make CPU creation/initialization consistent with QOM object
> behavior in this, by moving tcg and apic initialization from board
> level into CPU's initfn/realize calls and cpu_model property setter.
> 
> Which makes CPU object self-sufficient in respect of creation/initialization
> and matches a typical object creation sequence, i.e.:
>   - create CPU instance
>   - set properties
>   - realize object - (x86_cpu_realize will be converted into realize
>       property setter, when it is implemented)
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/pc.c              |   32 +++++---------------------
>  target-i386/cpu.c    |   60 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  target-i386/helper.c |   39 --------------------------------
>  3 files changed, 65 insertions(+), 66 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 0eb0b73..677f9e0 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -894,37 +894,17 @@ static void pc_cpu_reset(void *opaque)
>      cpu_reset(CPU(cpu));
>  }
>  
> -static X86CPU *pc_new_cpu(const char *cpu_model)
> -{
> -    X86CPU *cpu;
> -    CPUX86State *env;
> -
> -    cpu = cpu_x86_init(cpu_model);
> -    if (cpu == NULL) {
> -        exit(1);
> -    }
> -    env = &cpu->env;
> -    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);
> -    }
> -    qemu_register_reset(pc_cpu_reset, cpu);
> -    pc_cpu_reset(cpu);
> -    return cpu;
> -}
> -
>  void pc_cpus_init(const char *cpu_model)
>  {
> +    X86CPU *cpu;
>      int i;
>  
>      for(i = 0; i < smp_cpus; i++) {
> -        pc_new_cpu(cpu_model);
> +        cpu = cpu_x86_init(cpu_model);
> +        if (cpu == NULL) {
> +            exit(1);
> +        }
> +        qemu_register_reset(pc_cpu_reset, cpu);
>      }
>  }
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 538892d..0e804ea 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -31,6 +31,9 @@
>  
>  #include "hyperv.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
>   * between feature naming conventions, aliases may be added.
> @@ -1747,21 +1750,76 @@ static void x86_set_cpu_model(Object *obj, const char *value, Error **errp)
>      if (cpu_x86_register(cpu, env->cpu_model_str) < 0) {
>          fprintf(stderr, "Unable to find x86 CPU definition\n");
>          error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
> +        return;
> +    }
> +
> +    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");
> +        }
> +        object_property_add_child(OBJECT(cpu), "apic",
> +            OBJECT(env->apic_state), NULL);
> +
> +        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
> +        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
> +    }
> +}
> +
> +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);
>      }
>  }
>  
>  void x86_cpu_realize(Object *obj, Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(obj);
> +    CPUX86State *env = &cpu->env;
> +
> +    if (env->apic_state) {
> +        if (qdev_init(env->apic_state) < 0) {
> +            error_set(errp, QERR_DEVICE_INIT_FAILED,
> +                object_get_typename(OBJECT(env->apic_state)));
> +            return;
> +        }
> +    }
>  
>      mce_init(cpu);
> -    qemu_init_vcpu(&cpu->env);
> +    qemu_init_vcpu(env);
> +    cpu_reset(CPU(cpu));
>  }
>  
>  static void x86_cpu_initfn(Object *obj)
>  {
>      X86CPU *cpu = X86_CPU(obj);
>      CPUX86State *env = &cpu->env;
> +    static int inited;
>  
>      cpu_exec_init(env);
>  
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 6fc67a9..443092e 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;
> @@ -1161,20 +1133,9 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>  {
>      X86CPU *cpu;
>      Error *errp = NULL;
> -    static int inited;
>  
>      cpu = X86_CPU(object_new(TYPE_X86_CPU));
>  
> -    /* 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
> -    }
> -

Where does this hunk go to?

Did you test the result against TCG? :)

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH qom-next 4/5] target-i386: make initialize CPU in QOM way

[Igor Mammedov]

On 05/22/2012 12:56 PM, Jan Kiszka wrote:
> On 2012-05-22 07:35, Igor Mammedov wrote:
>> Make CPU creation/initialization consistent with QOM object
>> behavior in this, by moving tcg and apic initialization from board
>> level into CPU's initfn/realize calls and cpu_model property setter.
>>
>> Which makes CPU object self-sufficient in respect of creation/initialization
>> and matches a typical object creation sequence, i.e.:
>>    - create CPU instance
>>    - set properties
>>    - realize object - (x86_cpu_realize will be converted into realize
>>        property setter, when it is implemented)
>>
>> Signed-off-by: Igor Mammedov<imammedo@redhat.com>
>> ---
>>   hw/pc.c              |   32 +++++---------------------
>>   target-i386/cpu.c    |   60 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>   target-i386/helper.c |   39 --------------------------------
>>   3 files changed, 65 insertions(+), 66 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 0eb0b73..677f9e0 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -894,37 +894,17 @@ static void pc_cpu_reset(void *opaque)
>>       cpu_reset(CPU(cpu));
>>   }
>>
>> -static X86CPU *pc_new_cpu(const char *cpu_model)
>> -{
>> -    X86CPU *cpu;
>> -    CPUX86State *env;
>> -
>> -    cpu = cpu_x86_init(cpu_model);
>> -    if (cpu == NULL) {
>> -        exit(1);
>> -    }
>> -    env =&cpu->env;
>> -    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);
>> -    }
>> -    qemu_register_reset(pc_cpu_reset, cpu);
>> -    pc_cpu_reset(cpu);
>> -    return cpu;
>> -}
>> -
>>   void pc_cpus_init(const char *cpu_model)
>>   {
>> +    X86CPU *cpu;
>>       int i;
>>
>>       for(i = 0; i<  smp_cpus; i++) {
>> -        pc_new_cpu(cpu_model);
>> +        cpu = cpu_x86_init(cpu_model);
>> +        if (cpu == NULL) {
>> +            exit(1);
>> +        }
>> +        qemu_register_reset(pc_cpu_reset, cpu);
>>       }
>>   }
>>
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 538892d..0e804ea 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -31,6 +31,9 @@
>>
>>   #include "hyperv.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
>>    * between feature naming conventions, aliases may be added.
>> @@ -1747,21 +1750,76 @@ static void x86_set_cpu_model(Object *obj, const char *value, Error **errp)
>>       if (cpu_x86_register(cpu, env->cpu_model_str)<  0) {
>>           fprintf(stderr, "Unable to find x86 CPU definition\n");
>>           error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
>> +        return;
>> +    }
>> +
>> +    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");
>> +        }
>> +        object_property_add_child(OBJECT(cpu), "apic",
>> +            OBJECT(env->apic_state), NULL);
>> +
>> +        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
>> +        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
>> +    }
>> +}
>> +
>> +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);
>>       }
>>   }
>>
>>   void x86_cpu_realize(Object *obj, Error **errp)
>>   {
>>       X86CPU *cpu = X86_CPU(obj);
>> +    CPUX86State *env =&cpu->env;
>> +
>> +    if (env->apic_state) {
>> +        if (qdev_init(env->apic_state)<  0) {
>> +            error_set(errp, QERR_DEVICE_INIT_FAILED,
>> +                object_get_typename(OBJECT(env->apic_state)));
>> +            return;
>> +        }
>> +    }
>>
>>       mce_init(cpu);
>> -    qemu_init_vcpu(&cpu->env);
>> +    qemu_init_vcpu(env);
>> +    cpu_reset(CPU(cpu));
>>   }
>>
>>   static void x86_cpu_initfn(Object *obj)
>>   {
>>       X86CPU *cpu = X86_CPU(obj);
>>       CPUX86State *env =&cpu->env;
>> +    static int inited;
>>
>>       cpu_exec_init(env);
>>
>> diff --git a/target-i386/helper.c b/target-i386/helper.c
>> index 6fc67a9..443092e 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;
>> @@ -1161,20 +1133,9 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>>   {
>>       X86CPU *cpu;
>>       Error *errp = NULL;
>> -    static int inited;
>>
>>       cpu = X86_CPU(object_new(TYPE_X86_CPU));
>>
>> -    /* 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
>> -    }
>> -
>
> Where does this hunk go to?
>
> Did you test the result against TCG? :)

Yep it works :)
  It's my bad, that due patches reshuffling I've moved this hunk into:
    [PATCH qom-next 2/5] target-i386: add cpu-model property to x86_cpu
  Will move it to this patch.

Thanks,
  Igor

[Qemu-devel] [PATCH qom-next 5/5] target-i386: move reset callback to cpu.c

[Igor Mammedov]

Moving reset callback into cpu object from board level will allow
properly create object during run-time (hotplug).

When reset over QOM hierarchy is implemented, this reset callback
should be removed.

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

diff --git a/hw/pc.c b/hw/pc.c
index 677f9e0..70dd0e6 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -888,12 +888,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));
-}
-
 void pc_cpus_init(const char *cpu_model)
 {
     X86CPU *cpu;
@@ -904,7 +898,6 @@ void pc_cpus_init(const char *cpu_model)
         if (cpu == NULL) {
             exit(1);
         }
-        qemu_register_reset(pc_cpu_reset, cpu);
     }
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e804ea..87f4f5a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1716,6 +1716,13 @@ static void x86_cpu_reset(CPUState *s)
     env->halted = !(cpu_get_apic_base(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));
+}
+
 static void mce_init(X86CPU *cpu)
 {
     CPUX86State *cenv = &cpu->env;
@@ -1812,6 +1819,7 @@ void x86_cpu_realize(Object *obj, Error **errp)
 
     mce_init(cpu);
     qemu_init_vcpu(env);
+    qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
     cpu_reset(CPU(cpu));
 }
 
-- 
1.7.7.6