[Qemu-devel] [PATCH v2 0/4] QOM'ify x86 CPU, part 1

[Qemu-devel] [PATCH v2 0/4] QOM'ify x86 CPU, part 1

[Andreas Färber]

Hello,

This series strips down x86 CPU QOM'ification to the bare minimum,
leaving out subclasses for builtin or external CPU models.
It is ordered after the s390x conversion but is independent of it, again
due to alphabetical ordering, so that it could be applied right away now.

While I haven't seen any follow-up patches for X86CPU hotplug yet,
patch 2 is the one that allows to either use it as a child<> of a device
or to put TYPE_CPU directly onto some qdev bus for CONFIG_SOFTMMU.

Available from:
git://github.com/afaerber/qemu-cpu.git qom-cpu-x86.v2
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-x86.v2

Regards,
Andreas

Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Liu Jinsong <jinsong.liu@intel.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>

v1 -> v2:
* Move file rename to its own patch.
* Set diff.renames to true to force rename detection for cpu.c.
* Postpone TYPE_X86_CPU subclasses and split off reset and initfn conversion.

Andreas Färber (4):
  target-i386: Rename cpuid.c
  target-i386: QOM'ify CPU
  target-i386: QOM'ify CPU init
  target-i386: QOM'ify CPU reset

 Makefile.target                |    2 +-
 target-i386/cpu-qom.h          |   71 +++++++++++++++++++++
 target-i386/{cpuid.c => cpu.c} |  134 ++++++++++++++++++++++++++++++++++++++++
 target-i386/cpu.h              |    3 +-
 target-i386/helper.c           |  100 ++----------------------------
 5 files changed, 213 insertions(+), 97 deletions(-)
 create mode 100644 target-i386/cpu-qom.h
 rename target-i386/{cpuid.c => cpu.c} (92%)

-- 
1.7.7

[Qemu-devel] [PATCH v2 1/4] target-i386: Rename cpuid.c

[Andreas Färber]

Name it cpu.c to align with other QOM'ified targets.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Makefile.target                |    2 +-
 target-i386/{cpuid.c => cpu.c} |    0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename target-i386/{cpuid.c => cpu.c} (100%)

diff --git a/Makefile.target b/Makefile.target
index 999a968..6c4a77a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -87,7 +87,7 @@ endif
 endif
 libobj-y += helper.o
 ifeq ($(TARGET_BASE_ARCH), i386)
-libobj-y += cpuid.o
+libobj-y += cpu.o
 endif
 libobj-$(TARGET_SPARC64) += vis_helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o
diff --git a/target-i386/cpuid.c b/target-i386/cpu.c
similarity index 100%
rename from target-i386/cpuid.c
rename to target-i386/cpu.c
-- 
1.7.7

[Qemu-devel] [PATCH v2 2/4] target-i386: QOM'ify CPU

[Andreas Färber]

Embed CPUX86State as first member of X86CPU.
Drop cpu_x86_close() in favor of calling object_delete() directly.

For now let CPUClass::reset() call cpu_state_reset().

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu-qom.h |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
 target-i386/cpu.c     |   37 +++++++++++++++++++++++++
 target-i386/cpu.h     |    3 +-
 target-i386/helper.c  |   11 +++-----
 4 files changed, 114 insertions(+), 8 deletions(-)
 create mode 100644 target-i386/cpu-qom.h

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
new file mode 100644
index 0000000..e6ebfb8
--- /dev/null
+++ b/target-i386/cpu-qom.h
@@ -0,0 +1,71 @@
+/*
+ * QEMU x86 CPU
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+#ifndef QEMU_I386_CPU_QOM_H
+#define QEMU_I386_CPU_QOM_H
+
+#include "qemu/cpu.h"
+#include "cpu.h"
+
+#define TYPE_X86_CPU "x86-cpu"
+
+#define X86_CPU_CLASS(klass) \
+    OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU)
+#define X86_CPU(obj) \
+    OBJECT_CHECK(X86CPU, (obj), TYPE_X86_CPU)
+#define X86_CPU_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
+
+/**
+ * X86CPUClass:
+ * @parent_reset: The parent class' reset handler.
+ *
+ * An x86 CPU model or family.
+ */
+typedef struct X86CPUClass {
+    /*< private >*/
+    CPUClass parent_class;
+    /*< public >*/
+
+    void (*parent_reset)(CPUState *cpu);
+} X86CPUClass;
+
+/**
+ * X86CPU:
+ * @env: #CPUX86State
+ *
+ * An x86 CPU.
+ */
+typedef struct X86CPU {
+    /*< private >*/
+    CPUState parent_obj;
+    /*< public >*/
+
+    CPUX86State env;
+} X86CPU;
+
+static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
+{
+    return X86_CPU(container_of(env, X86CPU, env));
+}
+
+#define ENV_GET_CPU(e) CPU(x86_env_get_cpu(e))
+
+
+#endif
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 465ea15..36790da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1367,3 +1367,40 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     }
 }
+
+/* CPUClass::reset() */
+static void x86_cpu_reset(CPUState *s)
+{
+    X86CPU *cpu = X86_CPU(s);
+    X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
+    CPUX86State *env = &cpu->env;
+
+    xcc->parent_reset(s);
+
+    cpu_state_reset(env);
+}
+
+static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    CPUClass *cc = CPU_CLASS(oc);
+
+    xcc->parent_reset = cc->reset;
+    cc->reset = x86_cpu_reset;
+}
+
+static const TypeInfo x86_cpu_type_info = {
+    .name = TYPE_X86_CPU,
+    .parent = TYPE_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(X86CPUClass),
+    .class_init = x86_cpu_common_class_init,
+};
+
+static void x86_cpu_register_types(void)
+{
+    type_register_static(&x86_cpu_type_info);
+}
+
+type_init(x86_cpu_register_types)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index a1ed3e7..4bb4592 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -783,9 +783,10 @@ typedef struct CPUX86State {
     TPRAccess tpr_access_type;
 } CPUX86State;
 
+#include "cpu-qom.h"
+
 CPUX86State *cpu_x86_init(const char *cpu_model);
 int cpu_x86_exec(CPUX86State *s);
-void cpu_x86_close(CPUX86State *s);
 void x86_cpu_list (FILE *f, fprintf_function cpu_fprintf, const char *optarg);
 void x86_cpudef_setup(void);
 int cpu_x86_support_mca_broadcast(CPUX86State *env);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 83122bf..fb87975 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -101,11 +101,6 @@ void cpu_state_reset(CPUX86State *env)
     cpu_watchpoint_remove_all(env, BP_CPU);
 }
 
-void cpu_x86_close(CPUX86State *env)
-{
-    g_free(env);
-}
-
 static void cpu_x86_version(CPUX86State *env, int *family, int *model)
 {
     int cpuver = env->cpuid_version;
@@ -1248,10 +1243,12 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
 
 CPUX86State *cpu_x86_init(const char *cpu_model)
 {
+    X86CPU *cpu;
     CPUX86State *env;
     static int inited;
 
-    env = g_malloc0(sizeof(CPUX86State));
+    cpu = X86_CPU(object_new(TYPE_X86_CPU));
+    env = &cpu->env;
     cpu_exec_init(env);
     env->cpu_model_str = cpu_model;
 
@@ -1265,7 +1262,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
 #endif
     }
     if (cpu_x86_register(env, cpu_model) < 0) {
-        cpu_x86_close(env);
+        object_delete(OBJECT(cpu));
         return NULL;
     }
     env->cpuid_apic_id = env->cpu_index;
-- 
1.7.7

[Qemu-devel] [PATCH v2 3/4] target-i386: QOM'ify CPU init

[Andreas Färber]

Move code from cpu_x86_init() to new QOM x86_cpu_initfn().
Also move mce_init() to cpu.c since it's used nowhere else.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu.c    |   27 +++++++++++++++++++++++++++
 target-i386/helper.c |   18 ------------------
 2 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 36790da..f4463e1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1380,6 +1380,32 @@ static void x86_cpu_reset(CPUState *s)
     cpu_state_reset(env);
 }
 
+static void mce_init(X86CPU *cpu)
+{
+    CPUX86State *cenv = &cpu->env;
+    unsigned int bank;
+
+    if (((cenv->cpuid_version >> 8) & 0xf) >= 6
+        && (cenv->cpuid_features & (CPUID_MCE | CPUID_MCA)) ==
+            (CPUID_MCE | CPUID_MCA)) {
+        cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF;
+        cenv->mcg_ctl = ~(uint64_t)0;
+        for (bank = 0; bank < MCE_BANKS_DEF; bank++) {
+            cenv->mce_banks[bank * 4] = ~(uint64_t)0;
+        }
+    }
+}
+
+static void x86_cpu_initfn(Object *obj)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    CPUX86State *env = &cpu->env;
+
+    cpu_exec_init(env);
+    env->cpuid_apic_id = env->cpu_index;
+    mce_init(cpu);
+}
+
 static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 {
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
@@ -1393,6 +1419,7 @@ static const TypeInfo x86_cpu_type_info = {
     .name = TYPE_X86_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(X86CPU),
+    .instance_init = x86_cpu_initfn,
     .abstract = false,
     .class_size = sizeof(X86CPUClass),
     .class_init = x86_cpu_common_class_init,
diff --git a/target-i386/helper.c b/target-i386/helper.c
index fb87975..d8ceee1 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1197,21 +1197,6 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access)
 }
 #endif /* !CONFIG_USER_ONLY */
 
-static void mce_init(CPUX86State *cenv)
-{
-    unsigned int bank;
-
-    if (((cenv->cpuid_version >> 8) & 0xf) >= 6
-        && (cenv->cpuid_features & (CPUID_MCE | CPUID_MCA)) ==
-            (CPUID_MCE | CPUID_MCA)) {
-        cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF;
-        cenv->mcg_ctl = ~(uint64_t)0;
-        for (bank = 0; bank < MCE_BANKS_DEF; bank++) {
-            cenv->mce_banks[bank * 4] = ~(uint64_t)0;
-        }
-    }
-}
-
 int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
                             target_ulong *base, unsigned int *limit,
                             unsigned int *flags)
@@ -1249,7 +1234,6 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
 
     cpu = X86_CPU(object_new(TYPE_X86_CPU));
     env = &cpu->env;
-    cpu_exec_init(env);
     env->cpu_model_str = cpu_model;
 
     /* init various static tables used in TCG mode */
@@ -1265,8 +1249,6 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
         object_delete(OBJECT(cpu));
         return NULL;
     }
-    env->cpuid_apic_id = env->cpu_index;
-    mce_init(env);
 
     qemu_init_vcpu(env);
 
-- 
1.7.7

[Qemu-devel] [PATCH v2 4/4] target-i386: QOM'ify CPU reset

[Andreas Färber]

Move code from cpu_state_reset() into QOM x86_cpu_reset(),
fixing style issues for FPU init.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu.c    |   72 +++++++++++++++++++++++++++++++++++++++++++++++++-
 target-i386/helper.c |   71 +------------------------------------------------
 2 files changed, 72 insertions(+), 71 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f4463e1..3df53ca 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1374,10 +1374,80 @@ static void x86_cpu_reset(CPUState *s)
     X86CPU *cpu = X86_CPU(s);
     X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
     CPUX86State *env = &cpu->env;
+    int i;
+
+    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
+        qemu_log("CPU Reset (CPU %d)\n", env->cpu_index);
+        log_cpu_state(env, X86_DUMP_FPU | X86_DUMP_CCOP);
+    }
 
     xcc->parent_reset(s);
 
-    cpu_state_reset(env);
+
+    memset(env, 0, offsetof(CPUX86State, breakpoints));
+
+    tlb_flush(env, 1);
+
+    env->old_exception = -1;
+
+    /* init to reset state */
+
+#ifdef CONFIG_SOFTMMU
+    env->hflags |= HF_SOFTMMU_MASK;
+#endif
+    env->hflags2 |= HF2_GIF_MASK;
+
+    cpu_x86_update_cr0(env, 0x60000010);
+    env->a20_mask = ~0x0;
+    env->smbase = 0x30000;
+
+    env->idt.limit = 0xffff;
+    env->gdt.limit = 0xffff;
+    env->ldt.limit = 0xffff;
+    env->ldt.flags = DESC_P_MASK | (2 << DESC_TYPE_SHIFT);
+    env->tr.limit = 0xffff;
+    env->tr.flags = DESC_P_MASK | (11 << DESC_TYPE_SHIFT);
+
+    cpu_x86_load_seg_cache(env, R_CS, 0xf000, 0xffff0000, 0xffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_CS_MASK |
+                           DESC_R_MASK | DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_DS, 0, 0, 0xffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_ES, 0, 0, 0xffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_SS, 0, 0, 0xffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_FS, 0, 0, 0xffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0xffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+
+    env->eip = 0xfff0;
+    env->regs[R_EDX] = env->cpuid_version;
+
+    env->eflags = 0x2;
+
+    /* FPU init */
+    for (i = 0; i < 8; i++) {
+        env->fptags[i] = 1;
+    }
+    env->fpuc = 0x37f;
+
+    env->mxcsr = 0x1f80;
+
+    env->pat = 0x0007040600070406ULL;
+    env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT;
+
+    memset(env->dr, 0, sizeof(env->dr));
+    env->dr[6] = DR6_FIXED_1;
+    env->dr[7] = DR7_FIXED_1;
+    cpu_breakpoint_remove_all(env, BP_CPU);
+    cpu_watchpoint_remove_all(env, BP_CPU);
 }
 
 static void mce_init(X86CPU *cpu)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index d8ceee1..87954f0 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -29,76 +29,7 @@
 /* NOTE: must be called outside the CPU execute loop */
 void cpu_state_reset(CPUX86State *env)
 {
-    int i;
-
-    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
-        qemu_log("CPU Reset (CPU %d)\n", env->cpu_index);
-        log_cpu_state(env, X86_DUMP_FPU | X86_DUMP_CCOP);
-    }
-
-    memset(env, 0, offsetof(CPUX86State, breakpoints));
-
-    tlb_flush(env, 1);
-
-    env->old_exception = -1;
-
-    /* init to reset state */
-
-#ifdef CONFIG_SOFTMMU
-    env->hflags |= HF_SOFTMMU_MASK;
-#endif
-    env->hflags2 |= HF2_GIF_MASK;
-
-    cpu_x86_update_cr0(env, 0x60000010);
-    env->a20_mask = ~0x0;
-    env->smbase = 0x30000;
-
-    env->idt.limit = 0xffff;
-    env->gdt.limit = 0xffff;
-    env->ldt.limit = 0xffff;
-    env->ldt.flags = DESC_P_MASK | (2 << DESC_TYPE_SHIFT);
-    env->tr.limit = 0xffff;
-    env->tr.flags = DESC_P_MASK | (11 << DESC_TYPE_SHIFT);
-
-    cpu_x86_load_seg_cache(env, R_CS, 0xf000, 0xffff0000, 0xffff,
-                           DESC_P_MASK | DESC_S_MASK | DESC_CS_MASK |
-                           DESC_R_MASK | DESC_A_MASK);
-    cpu_x86_load_seg_cache(env, R_DS, 0, 0, 0xffff,
-                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
-                           DESC_A_MASK);
-    cpu_x86_load_seg_cache(env, R_ES, 0, 0, 0xffff,
-                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
-                           DESC_A_MASK);
-    cpu_x86_load_seg_cache(env, R_SS, 0, 0, 0xffff,
-                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
-                           DESC_A_MASK);
-    cpu_x86_load_seg_cache(env, R_FS, 0, 0, 0xffff,
-                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
-                           DESC_A_MASK);
-    cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0xffff,
-                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
-                           DESC_A_MASK);
-
-    env->eip = 0xfff0;
-    env->regs[R_EDX] = env->cpuid_version;
-
-    env->eflags = 0x2;
-
-    /* FPU init */
-    for(i = 0;i < 8; i++)
-        env->fptags[i] = 1;
-    env->fpuc = 0x37f;
-
-    env->mxcsr = 0x1f80;
-
-    env->pat = 0x0007040600070406ULL;
-    env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT;
-
-    memset(env->dr, 0, sizeof(env->dr));
-    env->dr[6] = DR6_FIXED_1;
-    env->dr[7] = DR7_FIXED_1;
-    cpu_breakpoint_remove_all(env, BP_CPU);
-    cpu_watchpoint_remove_all(env, BP_CPU);
+    cpu_reset(ENV_GET_CPU(env));
 }
 
 static void cpu_x86_version(CPUX86State *env, int *family, int *model)
-- 
1.7.7