[Qemu-devel] [PATCH RFC 0/7] Introduce QOM CPU and use for target-arm

[Qemu-devel] [PATCH RFC 0/7] Introduce QOM CPU and use for target-arm

[Andreas Färber]

Hello,

Here's a series against master, pushing QOM beyond what I've seen on Anthony's
qom-upstream and qom-rebase branches.
It depends on the object_class_foreach() fix posted separately.

Patch 1 is included here to show its use case in patch 5.

Patch 2 suggests a way to start using QOM beyond "devices" in system emulation.
Patch 3 suggests a way to integrate QOM into the user emulators as well.

Patches 4-5 introduce a CPU class and prepare its use in system and user mode.

Patches 6-7 build upon this infrastructure and start using it for ARM.

This series is surely not yet the final goal as discussed on IRC. It does help
with my and Peter's quest to further clean up the constantly growing mess
surrounding ARM reset vs. one-time initialization though.

The plan there is to get rid of the huge CPUID switch by moving the knowledge
of reset values to CPU classes and by dumb copying of values from class to
instance on reset. The latter for now requires to be able to obtain the
ObjectClass matching a CPUState - in target-specific code this works by simple
pointer arithmetic, encapsulated in a macro.

So, in a spirit similar to Anthony's i440FX rework I'm posting this to check if
I'm on the right track here before I start messing around with other targets.

Regards,
Andreas

Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Paul Brook <paul@codesourcery.com>

Andreas Färber (7):
  qom: Introduce object_class_is_abstract()
  qom: Register QOM infrastructure early
  qom: Add QOM support to user emulators
  qom: Introduce CPU class
  cpu: Introduce cpu_class_foreach()
  target-arm: Introduce QOM CPU and use for it CPUID lookup
  target-arm: Embed CPUARMState in QOM ARMCPU

 Makefile.objs         |    1 +
 Makefile.target       |   16 +++-
 Makefile.user         |    1 +
 arch_init.c           |    1 +
 bsd-user/main.c       |    3 +
 darwin-user/main.c    |    4 +
 hw/cpu.c              |   52 +++++++++
 include/qemu/cpu.h    |   36 +++++++
 include/qemu/object.h |    8 ++
 linux-user/main.c     |    3 +
 module.h              |    4 +
 qom/object.c          |    7 +-
 target-arm/cpu-core.c |  281 +++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu-core.h |   40 +++++++
 target-arm/helper.c   |   87 +++++-----------
 vl.c                  |    2 +
 16 files changed, 480 insertions(+), 66 deletions(-)
 create mode 100644 hw/cpu.c
 create mode 100644 include/qemu/cpu.h
 create mode 100644 target-arm/cpu-core.c
 create mode 100644 target-arm/cpu-core.h

-- 
1.7.7

[Qemu-devel] [PATCH 1/7][RESEND] qom: Introduce object_class_is_abstract()

[Andreas Färber]

Since struct TypeImpl is not public, this is useful for enumerating
available types.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
---
 include/qemu/object.h |    8 ++++++++
 qom/object.c          |    5 +++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index ba37850..8ec45f2 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -428,6 +428,14 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *klass,
  */
 const char *object_class_get_name(ObjectClass *klass);
 
+/**
+ * object_class_is_abstract:
+ * @klass: The class to obtain the abstractness for.
+ *
+ * Returns: Whether @klass is an abstract class or not.
+ */
+bool object_class_is_abstract(ObjectClass *klass);
+
 ObjectClass *object_class_by_name(const char *typename);
 
 void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
diff --git a/qom/object.c b/qom/object.c
index 57cc592..1821959 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -451,6 +451,11 @@ const char *object_class_get_name(ObjectClass *klass)
     return klass->type->name;
 }
 
+bool object_class_is_abstract(ObjectClass *klass)
+{
+    return klass->type->abstract;
+}
+
 ObjectClass *object_class_by_name(const char *typename)
 {
     TypeImpl *type = type_get_by_name(typename);
-- 
1.7.7

[Qemu-devel] [PATCH RFC 2/7] qom: Register QOM infrastructure early

[Andreas Färber]

QOM TYPE_INTERFACE was registered with device_init(), whose
constructors are executed rather late in vl.c's main().

Introduce a new module init type and register it very early so that QOM
can safely be used for machines and CPUs.

Note that *_init() defines an attributed function, so no semicolon is
needed after the brace.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
---
 module.h     |    2 ++
 qom/object.c |    2 +-
 vl.c         |    2 ++
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/module.h b/module.h
index ef66730..567ff3a 100644
--- a/module.h
+++ b/module.h
@@ -21,6 +21,7 @@ static void __attribute__((constructor)) do_qemu_init_ ## function(void) {  \
 }
 
 typedef enum {
+    MODULE_INIT_EARLY,
     MODULE_INIT_BLOCK,
     MODULE_INIT_DEVICE,
     MODULE_INIT_MACHINE,
@@ -28,6 +29,7 @@ typedef enum {
     MODULE_INIT_MAX
 } module_init_type;
 
+#define early_init(function) module_init(function, MODULE_INIT_EARLY)
 #define block_init(function) module_init(function, MODULE_INIT_BLOCK)
 #define device_init(function) module_init(function, MODULE_INIT_DEVICE)
 #define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
diff --git a/qom/object.c b/qom/object.c
index 1821959..3c79e1d 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -388,7 +388,7 @@ static void register_interface(void)
     type_register_static(&interface_info);
 }
 
-device_init(register_interface);
+early_init(register_interface)
 
 Object *object_dynamic_cast_assert(Object *obj, const char *typename)
 {
diff --git a/vl.c b/vl.c
index d88a18c..379ca4e 100644
--- a/vl.c
+++ b/vl.c
@@ -2208,6 +2208,8 @@ int main(int argc, char **argv, char **envp)
 #endif
     }
 
+    module_call_init(MODULE_INIT_EARLY);
+
     runstate_init();
 
     init_clocks();
-- 
1.7.7

[Qemu-devel] [PATCH RFC 3/7] qom: Add QOM support to user emulators

[Andreas Färber]

Link the Object base class and the module infrastructure for class
registration. Call early module init.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.target    |    6 ++++++
 Makefile.user      |    1 +
 bsd-user/main.c    |    2 ++
 darwin-user/main.c |    3 +++
 linux-user/main.c  |    2 ++
 5 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 68481a3..d1b7867 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -129,6 +129,8 @@ obj-m68k-y += m68k-sim.o m68k-semi.o
 
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
+obj-y += module.o
+obj-y += $(addprefix ../qom/, $(qom-y))
 obj-y += $(addprefix ../libuser/, $(user-obj-y))
 obj-y += $(addprefix ../libdis-user/, $(libdis-y))
 obj-y += $(libobj-y)
@@ -156,6 +158,8 @@ obj-i386-y += ioport-user.o
 
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
+obj-y += module.o
+obj-y += $(addprefix ../qom/, $(qom-y))
 obj-y += $(addprefix ../libuser/, $(user-obj-y))
 obj-y += $(addprefix ../libdis-user/, $(libdis-y))
 obj-y += $(libobj-y)
@@ -178,6 +182,8 @@ obj-i386-y += ioport-user.o
 
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
+obj-y += module.o
+obj-y += $(addprefix ../qom/, $(qom-y))
 obj-y += $(addprefix ../libuser/, $(user-obj-y))
 obj-y += $(addprefix ../libdis-user/, $(libdis-y))
 obj-y += $(libobj-y)
diff --git a/Makefile.user b/Makefile.user
index 2b1e4d1..72d01c1 100644
--- a/Makefile.user
+++ b/Makefile.user
@@ -9,6 +9,7 @@ include $(SRC_PATH)/rules.mak
 $(call set-vpath, $(SRC_PATH))
 
 QEMU_CFLAGS+=-I..
+QEMU_CFLAGS+=-I$(SRC_PATH)/include
 
 include $(SRC_PATH)/Makefile.objs
 
diff --git a/bsd-user/main.c b/bsd-user/main.c
index cc7d4a3..2ff0361 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -748,6 +748,8 @@ int main(int argc, char **argv)
     if (argc <= 1)
         usage();
 
+    module_call_init(MODULE_INIT_EARLY);
+
     if ((envlist = envlist_create()) == NULL) {
         (void) fprintf(stderr, "Unable to allocate envlist\n");
         exit(1);
diff --git a/darwin-user/main.c b/darwin-user/main.c
index 9b57c20..a4c630d 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -28,6 +28,7 @@
 #include <sys/mman.h>
 
 #include "qemu.h"
+#include "qemu-common.h"
 
 #define DEBUG_LOGFILE "/tmp/qemu.log"
 
@@ -749,6 +750,8 @@ int main(int argc, char **argv)
     if (argc <= 1)
         usage();
 
+    module_call_init(MODULE_INIT_EARLY);
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
diff --git a/linux-user/main.c b/linux-user/main.c
index 64d2208..d4368b6 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3280,6 +3280,8 @@ int main(int argc, char **argv, char **envp)
 
     qemu_cache_utils_init(envp);
 
+    module_call_init(MODULE_INIT_EARLY);
+
     if ((envlist = envlist_create()) == NULL) {
         (void) fprintf(stderr, "Unable to allocate envlist\n");
         exit(1);
-- 
1.7.7

[Qemu-devel] [PATCH RFC 4/7] qom: Introduce CPU class

[Andreas Färber]

It's abstract, derived directly from TYPE_OBJECT (to avoid dependency
on MODULE_INIT_DEVICE) and for now is empty.

Place it in hw/. Have user emulators pick it up via VPATH, building it
per target since they didn't use any qdev/QOM devices so far.

Introduce processor_init() for registering, and call module init as
needed.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.objs      |    1 +
 Makefile.target    |    9 ++++++---
 arch_init.c        |    1 +
 bsd-user/main.c    |    1 +
 darwin-user/main.c |    1 +
 hw/cpu.c           |   27 +++++++++++++++++++++++++++
 include/qemu/cpu.h |   27 +++++++++++++++++++++++++++
 linux-user/main.c  |    1 +
 module.h           |    2 ++
 9 files changed, 67 insertions(+), 3 deletions(-)
 create mode 100644 hw/cpu.c
 create mode 100644 include/qemu/cpu.h

diff --git a/Makefile.objs b/Makefile.objs
index b942625..a4b20fa 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -189,6 +189,7 @@ user-obj-y += $(trace-obj-y)
 
 hw-obj-y =
 hw-obj-y += vl.o loader.o
+hw-obj-y += cpu.o
 hw-obj-$(CONFIG_VIRTIO) += virtio-console.o
 hw-obj-y += usb-libhw.o
 hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
diff --git a/Makefile.target b/Makefile.target
index d1b7867..5d3470e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -107,7 +107,7 @@ signal.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 
 ifdef CONFIG_LINUX_USER
 
-$(call set-vpath, $(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR))
+$(call set-vpath, $(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR):$(SRC_PATH)/hw)
 
 QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR) -I$(SRC_PATH)/linux-user
 obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
@@ -130,6 +130,7 @@ obj-m68k-y += m68k-sim.o m68k-semi.o
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
 obj-y += module.o
+obj-y += cpu.o
 obj-y += $(addprefix ../qom/, $(qom-y))
 obj-y += $(addprefix ../libuser/, $(user-obj-y))
 obj-y += $(addprefix ../libdis-user/, $(libdis-y))
@@ -142,7 +143,7 @@ endif #CONFIG_LINUX_USER
 
 ifdef CONFIG_DARWIN_USER
 
-$(call set-vpath, $(SRC_PATH)/darwin-user)
+$(call set-vpath, $(SRC_PATH)/darwin-user:$(SRC_PATH)/hw)
 
 QEMU_CFLAGS+=-I$(SRC_PATH)/darwin-user -I$(SRC_PATH)/darwin-user/$(TARGET_ARCH)
 
@@ -159,6 +160,7 @@ obj-i386-y += ioport-user.o
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
 obj-y += module.o
+obj-y += cpu.o
 obj-y += $(addprefix ../qom/, $(qom-y))
 obj-y += $(addprefix ../libuser/, $(user-obj-y))
 obj-y += $(addprefix ../libdis-user/, $(libdis-y))
@@ -171,7 +173,7 @@ endif #CONFIG_DARWIN_USER
 
 ifdef CONFIG_BSD_USER
 
-$(call set-vpath, $(SRC_PATH)/bsd-user)
+$(call set-vpath, $(SRC_PATH)/bsd-user:$(SRC_PATH)/hw)
 
 QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ARCH)
 
@@ -183,6 +185,7 @@ obj-i386-y += ioport-user.o
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
 obj-y += module.o
+obj-y += cpu.o
 obj-y += $(addprefix ../qom/, $(qom-y))
 obj-y += $(addprefix ../libuser/, $(user-obj-y))
 obj-y += $(addprefix ../libdis-user/, $(libdis-y))
diff --git a/arch_init.c b/arch_init.c
index 2366511..c0d5f4f 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -692,6 +692,7 @@ void do_smbios_option(const char *optarg)
 
 void cpudef_init(void)
 {
+    module_call_init(MODULE_INIT_CPU);
 #if defined(cpudef_setup)
     cpudef_setup(); /* parse cpu definitions in target config file */
 #endif
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 2ff0361..70e1146 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -761,6 +761,7 @@ int main(int argc, char **argv)
     }
 
     cpu_model = NULL;
+    module_call_init(MODULE_INIT_CPU);
 #if defined(cpudef_setup)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
diff --git a/darwin-user/main.c b/darwin-user/main.c
index a4c630d..d065f00 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -751,6 +751,7 @@ int main(int argc, char **argv)
         usage();
 
     module_call_init(MODULE_INIT_EARLY);
+    module_call_init(MODULE_INIT_CPU);
 
     optind = 1;
     for(;;) {
diff --git a/hw/cpu.c b/hw/cpu.c
new file mode 100644
index 0000000..c0e9cfa
--- /dev/null
+++ b/hw/cpu.c
@@ -0,0 +1,27 @@
+/*
+ * QEMU CPU model
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * Licensed under the terms of the GNU GPL version 2
+ * or (at your option) any later version.
+ */
+
+#include "qemu/object.h"
+#include "qemu/cpu.h"
+#include "qemu-common.h"
+
+static TypeInfo cpu_type_info = {
+    .name = TYPE_CPU,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(CPU),
+    .abstract = true,
+    .class_size = sizeof(CPUClass),
+};
+
+static void cpu_register_types(void)
+{
+    type_register_static(&cpu_type_info);
+}
+
+processor_init(cpu_register_types)
diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
new file mode 100644
index 0000000..4b81f3b
--- /dev/null
+++ b/include/qemu/cpu.h
@@ -0,0 +1,27 @@
+/*
+ * QEMU CPU model
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * Licensed under the terms of the GNU GPL version 2
+ * or (at your option) any later version.
+ */
+#ifndef QEMU_CPU_H
+#define QEMU_CPU_H
+
+#include "qemu/object.h"
+
+#define TYPE_CPU "cpu"
+
+typedef struct CPUClass {
+    ObjectClass parent_class;
+} CPUClass;
+
+typedef struct CPU {
+    Object parent_obj;
+
+    /* TODO Move common CPUState here */
+} CPU;
+
+
+#endif
diff --git a/linux-user/main.c b/linux-user/main.c
index d4368b6..e727e8d 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3304,6 +3304,7 @@ int main(int argc, char **argv, char **envp)
     }
 
     cpu_model = NULL;
+    module_call_init(MODULE_INIT_CPU);
 #if defined(cpudef_setup)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
diff --git a/module.h b/module.h
index 567ff3a..512ba6c 100644
--- a/module.h
+++ b/module.h
@@ -26,6 +26,7 @@ typedef enum {
     MODULE_INIT_DEVICE,
     MODULE_INIT_MACHINE,
     MODULE_INIT_QAPI,
+    MODULE_INIT_CPU,
     MODULE_INIT_MAX
 } module_init_type;
 
@@ -34,6 +35,7 @@ typedef enum {
 #define device_init(function) module_init(function, MODULE_INIT_DEVICE)
 #define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
 #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
+#define processor_init(function) module_init(function, MODULE_INIT_CPU)
 
 void register_module_init(void (*fn)(void), module_init_type type);
 
-- 
1.7.7

[Qemu-devel] [PATCH RFC 5/7] cpu: Introduce cpu_class_foreach()

[Andreas Färber]

Provides an easy way to loop over all non-abstract CPU classes.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/cpu.c           |   25 +++++++++++++++++++++++++
 include/qemu/cpu.h |    9 +++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/hw/cpu.c b/hw/cpu.c
index c0e9cfa..ac0add7 100644
--- a/hw/cpu.c
+++ b/hw/cpu.c
@@ -11,6 +11,31 @@
 #include "qemu/cpu.h"
 #include "qemu-common.h"
 
+struct CPUListData {
+    void (*fn)(ObjectClass *klass, void *opaque);
+    void *opaque;
+};
+
+static void cpu_class_foreach_tramp(ObjectClass *klass, void *opaque)
+{
+    struct CPUListData *s = opaque;
+
+    if (!object_class_is_abstract(klass) &&
+        object_class_dynamic_cast(klass, TYPE_CPU) != NULL) {
+        s->fn(klass, s->opaque);
+    }
+}
+
+void cpu_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
+                       void *opaque)
+{
+    struct CPUListData s = {
+        .fn = fn,
+        .opaque = opaque,
+    };
+    object_class_foreach(cpu_class_foreach_tramp, &s);
+}
+
 static TypeInfo cpu_type_info = {
     .name = TYPE_CPU,
     .parent = TYPE_OBJECT,
diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
index 4b81f3b..d06c87e 100644
--- a/include/qemu/cpu.h
+++ b/include/qemu/cpu.h
@@ -24,4 +24,13 @@ typedef struct CPU {
 } CPU;
 
 
+/**
+ * cpu_class_foreach:
+ * @fn: Callback function called for each non-abstract CPU type.
+ * @opaque: Opaque data passed through to the callback function.
+ */
+void cpu_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
+                       void *opaque);
+
+
 #endif
-- 
1.7.7

[Qemu-devel] [PATCH RFC 6/7] target-arm: Introduce QOM CPU and use for it CPUID lookup

[Andreas Färber]

Create a CPU subclass, and register classes matching all CPU models.
Don't name the file target-arm/cpu.c so that the user emulators can
still easily pick up the base class in hw/cpu.c via VPATH.

Make arm_cpu_list() enumerate CPU subclasses.

Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
CPUID in the class.
NB: CPUIDs were first introduced by Paul Brook in r1765 (2006).

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Paul Brook <paul@codesourcery.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 Makefile.target       |    1 +
 target-arm/cpu-core.c |  268 +++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu-core.h |   33 ++++++
 target-arm/helper.c   |   80 ++++-----------
 4 files changed, 324 insertions(+), 58 deletions(-)
 create mode 100644 target-arm/cpu-core.c
 create mode 100644 target-arm/cpu-core.h

diff --git a/Makefile.target b/Makefile.target
index 5d3470e..96043c4 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -80,6 +80,7 @@ endif
 libobj-$(TARGET_SPARC64) += vis_helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o
 libobj-$(TARGET_ARM) += neon_helper.o iwmmxt_helper.o
+libobj-$(TARGET_ARM) += cpu-core.o
 ifeq ($(TARGET_BASE_ARCH), sparc)
 libobj-y += fop_helper.o cc_helper.o win_helper.o mmu_helper.o ldst_helper.o
 libobj-y += cpu_init.o
diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c
new file mode 100644
index 0000000..9761d8e
--- /dev/null
+++ b/target-arm/cpu-core.c
@@ -0,0 +1,268 @@
+/*
+ * QEMU ARM CPU core
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * Licensed under the terms of the GNU GPL version 2
+ * or (at your option) any later version.
+ */
+
+#include "cpu-core.h"
+#include "qemu-common.h"
+
+/* CPU models */
+
+static void arm926_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x41069265;
+}
+
+static void arm946_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x41059461;
+}
+
+static void arm1026_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x4106a262;
+}
+
+static void arm1136_r0_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x4107b362;
+}
+
+static void arm1136_r1_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x4117b363;
+}
+
+static void arm1176_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x410fb767;
+}
+
+static void arm11mpcore_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x410fb022;
+}
+
+static void cortex_m3_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x410fc231;
+}
+
+static void cortex_a8_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x410fc080;
+}
+
+static void cortex_a9_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x410fc090;
+}
+
+static void cortex_a15_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x412fc0f1;
+}
+
+static void ti925t_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x54029252;
+}
+
+static void sa1100_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x4401A11B;
+}
+
+static void sa1110_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x6901B119;
+}
+
+static void pxa250_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69052100;
+}
+
+static void pxa255_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69052d00;
+}
+
+static void pxa260_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69052903;
+}
+
+static void pxa261_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69052d05;
+}
+
+static void pxa262_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69052d06;
+}
+
+static void pxa270_a0_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69054110;
+}
+
+static void pxa270_a1_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69054111;
+}
+
+static void pxa270_b0_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69054112;
+}
+
+static void pxa270_b1_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69054113;
+}
+
+static void pxa270_c0_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69054114;
+}
+
+static void pxa270_c5_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0x69054117;
+}
+
+static void arm_any_cpu_class_init(ObjectClass *klass, void *data)
+{
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
+
+    k->id = 0xffffffff;
+}
+
+struct ARMCPUDef {
+    const char *name;
+    void (*class_init)(ObjectClass *klass, void *data);
+};
+
+static const struct ARMCPUDef arm_cpu_models[] = {
+    { "arm926", arm926_class_init },
+    { "arm946", arm946_class_init },
+    { "arm1026", arm1026_class_init },
+    /* What QEMU calls "arm1136-r2" is actually the 1136 r0p2, i.e. an
+     * older core than plain "arm1136". In particular this does not
+     * have the v6K features.
+     */
+    { "arm1136-r2", arm1136_r0_class_init },
+    { "arm1136",    arm1136_r1_class_init },
+    { "arm1176", arm1176_class_init },
+    { "arm11mpcore", arm11mpcore_class_init },
+    { "cortex-m3", cortex_m3_class_init },
+    { "cortex-a8", cortex_a8_class_init },
+    { "cortex-a9", cortex_a9_class_init },
+    { "cortex-a15", cortex_a15_class_init },
+    { "ti925t", ti925t_class_init },
+    { "sa1100", sa1100_class_init },
+    { "sa1110", sa1110_class_init },
+    { "pxa250", pxa250_class_init },
+    { "pxa255", pxa255_class_init },
+    { "pxa260", pxa260_class_init },
+    { "pxa261", pxa261_class_init },
+    { "pxa262", pxa262_class_init },
+    { "pxa270",    pxa270_a0_class_init }, /* XXX just an alias */
+    { "pxa270-a0", pxa270_a0_class_init },
+    { "pxa270-a1", pxa270_a1_class_init },
+    { "pxa270-b0", pxa270_b0_class_init },
+    { "pxa270-b1", pxa270_b1_class_init },
+    { "pxa270-c0", pxa270_c0_class_init },
+    { "pxa270-c5", pxa270_c5_class_init },
+    { "any", arm_any_cpu_class_init },
+    { }
+};
+
+static void cpu_register(const struct ARMCPUDef *def)
+{
+    TypeInfo type = {
+        .name = def->name,
+        .parent = TYPE_ARM_CPU,
+        .instance_size = sizeof(ARMCPU),
+        .class_size = sizeof(ARMCPUClass),
+        .class_init = def->class_init,
+    };
+
+    type_register_static(&type);
+}
+
+static TypeInfo arm_cpu_type_info = {
+    .name = TYPE_ARM_CPU,
+    .parent = TYPE_CPU,
+    .instance_size = sizeof(ARMCPU),
+    .abstract = true,
+    .class_size = sizeof(ARMCPUClass),
+};
+
+static void arm_cpu_types_init(void)
+{
+    const struct ARMCPUDef *def;
+
+    type_register_static(&arm_cpu_type_info);
+    for (def = arm_cpu_models; def->name != NULL; def++) {
+        cpu_register(def);
+    }
+}
+
+processor_init(arm_cpu_types_init)
diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h
new file mode 100644
index 0000000..be4bbc3
--- /dev/null
+++ b/target-arm/cpu-core.h
@@ -0,0 +1,33 @@
+/*
+ * QEMU ARM CPU core
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * Licensed under the terms of the GNU GPL version 2
+ * or (at your option) any later version.
+ */
+#ifndef QEMU_ARM_CPU_CORE_H
+#define QEMU_ARM_CPU_CORE_H
+
+#include "qemu/cpu.h"
+
+#define TYPE_ARM_CPU "arm-cpu-core"
+#define ARM_CPU_CLASS(klass) \
+    OBJECT_CLASS_CHECK(ARMCPUClass, (klass), TYPE_ARM_CPU)
+#define ARM_CPU(obj) \
+    OBJECT_CHECK(ARMCPU, (obj), TYPE_ARM_CPU)
+#define ARM_CPU_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(ARMCPUClass, (obj), TYPE_ARM_CPU)
+
+typedef struct ARMCPUClass {
+    CPUClass parent_class;
+
+    uint32_t id;
+} ARMCPUClass;
+
+typedef struct ARMCPU {
+    CPU parent_obj;
+} ARMCPU;
+
+
+#endif
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ea4f35f..ece9635 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -11,6 +11,7 @@
 #include "hw/loader.h"
 #endif
 #include "sysemu.h"
+#include "cpu-core.h"
 
 static uint32_t cortexa15_cp15_c0_c1[8] = {
     0x00001131, 0x00011011, 0x02010555, 0x00000000,
@@ -51,8 +52,6 @@ static uint32_t arm1176_cp15_c0_c1[8] =
 static uint32_t arm1176_cp15_c0_c2[8] =
 { 0x0140011, 0x12002111, 0x11231121, 0x01102131, 0x01141, 0, 0, 0 };
 
-static uint32_t cpu_arm_find_by_name(const char *name);
-
 static inline void set_feature(CPUARMState *env, int feature)
 {
     env->features |= 1u << feature;
@@ -400,13 +399,16 @@ static int vfp_gdb_set_reg(CPUState *env, uint8_t *buf, int reg)
 
 CPUARMState *cpu_arm_init(const char *cpu_model)
 {
+    ObjectClass *klass;
+    ARMCPUClass *cpu_class;
     CPUARMState *env;
-    uint32_t id;
     static int inited = 0;
 
-    id = cpu_arm_find_by_name(cpu_model);
-    if (id == 0)
+    klass = object_class_by_name(cpu_model);
+    if (klass == NULL) {
         return NULL;
+    }
+    cpu_class = ARM_CPU_CLASS(klass);
     env = g_malloc0(sizeof(CPUARMState));
     cpu_exec_init(env);
     if (tcg_enabled() && !inited) {
@@ -415,7 +417,7 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
     }
 
     env->cpu_model_str = cpu_model;
-    env->cp15.c0_cpuid = id;
+    env->cp15.c0_cpuid = cpu_class->id;
     cpu_reset(env);
     if (arm_feature(env, ARM_FEATURE_NEON)) {
         gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
@@ -431,66 +433,28 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
     return env;
 }
 
-struct arm_cpu_t {
-    uint32_t id;
-    const char *name;
+struct ARMCPUListState {
+    fprintf_function cpu_fprintf;
+    FILE *file;
 };
 
-static const struct arm_cpu_t arm_cpu_names[] = {
-    { ARM_CPUID_ARM926, "arm926"},
-    { ARM_CPUID_ARM946, "arm946"},
-    { ARM_CPUID_ARM1026, "arm1026"},
-    { ARM_CPUID_ARM1136, "arm1136"},
-    { ARM_CPUID_ARM1136_R2, "arm1136-r2"},
-    { ARM_CPUID_ARM1176, "arm1176"},
-    { ARM_CPUID_ARM11MPCORE, "arm11mpcore"},
-    { ARM_CPUID_CORTEXM3, "cortex-m3"},
-    { ARM_CPUID_CORTEXA8, "cortex-a8"},
-    { ARM_CPUID_CORTEXA9, "cortex-a9"},
-    { ARM_CPUID_CORTEXA15, "cortex-a15" },
-    { ARM_CPUID_TI925T, "ti925t" },
-    { ARM_CPUID_PXA250, "pxa250" },
-    { ARM_CPUID_SA1100,    "sa1100" },
-    { ARM_CPUID_SA1110,    "sa1110" },
-    { ARM_CPUID_PXA255, "pxa255" },
-    { ARM_CPUID_PXA260, "pxa260" },
-    { ARM_CPUID_PXA261, "pxa261" },
-    { ARM_CPUID_PXA262, "pxa262" },
-    { ARM_CPUID_PXA270, "pxa270" },
-    { ARM_CPUID_PXA270_A0, "pxa270-a0" },
-    { ARM_CPUID_PXA270_A1, "pxa270-a1" },
-    { ARM_CPUID_PXA270_B0, "pxa270-b0" },
-    { ARM_CPUID_PXA270_B1, "pxa270-b1" },
-    { ARM_CPUID_PXA270_C0, "pxa270-c0" },
-    { ARM_CPUID_PXA270_C5, "pxa270-c5" },
-    { ARM_CPUID_ANY, "any"},
-    { 0, NULL}
-};
-
-void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+static void arm_cpu_list_entry(ObjectClass *klass, void *opaque)
 {
-    int i;
+    struct ARMCPUListState *s = opaque;
 
-    (*cpu_fprintf)(f, "Available CPUs:\n");
-    for (i = 0; arm_cpu_names[i].name; i++) {
-        (*cpu_fprintf)(f, "  %s\n", arm_cpu_names[i].name);
-    }
+    (*s->cpu_fprintf)(s->file, "  %s\n",
+                      object_class_get_name(klass));
 }
 
-/* return 0 if not found */
-static uint32_t cpu_arm_find_by_name(const char *name)
+void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
-    int i;
-    uint32_t id;
+    struct ARMCPUListState s = {
+        .cpu_fprintf = cpu_fprintf,
+        .file = f,
+    };
 
-    id = 0;
-    for (i = 0; arm_cpu_names[i].name; i++) {
-        if (strcmp(name, arm_cpu_names[i].name) == 0) {
-            id = arm_cpu_names[i].id;
-            break;
-        }
-    }
-    return id;
+    (*cpu_fprintf)(f, "Available CPUs:\n");
+    cpu_class_foreach(arm_cpu_list_entry, &s);
 }
 
 void cpu_arm_close(CPUARMState *env)
-- 
1.7.7

[Qemu-devel] [PATCH RFC 7/7] target-arm: Embed CPUARMState in QOM ARMCPU

[Andreas Färber]

We g_malloc0()'ed CPUARMState ourself, and exec.c's cpu_copy() runs
through cpu_init() as well, so we are at liberty to supply the CPUState
any way we see fit. Having CPUARMState as field in the QOM CPU allows
both to access env from an ARMCPU object and to access the QOM Object
and its ObjectClass from an env pointer, in ARM code for now.

The goal is to convert all CPUs to QOM and to use CPU objects in central
places, especially once we have property support for Object.
This will then allow to have TCG AREG0 point to target-specific fields
where small immediate offsets are desired (as pointed out by rth) while
allowing for common fields at known offsets from the base class.

Having the CPUID in ARMCPUClass, we can set it from the realize function.
Same for cpu_model_str, which is now the QOM class name.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Paul Brook <paul@codesourcery.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Richard Henderson <rth@twiddle.net>
---
 target-arm/cpu-core.c |   13 +++++++++++++
 target-arm/cpu-core.h |    7 +++++++
 target-arm/helper.c   |   13 ++++++-------
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c
index 9761d8e..b1ac22c 100644
--- a/target-arm/cpu-core.c
+++ b/target-arm/cpu-core.c
@@ -234,12 +234,25 @@ static const struct ARMCPUDef arm_cpu_models[] = {
     { }
 };
 
+static void arm_cpu_realize(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    ARMCPUClass *cpu_class = ARM_CPU_GET_CLASS(obj);
+
+    memset(&cpu->env, 0, sizeof(CPUARMState));
+    cpu_exec_init(&cpu->env);
+
+    cpu->env.cpu_model_str = object_get_typename(obj);
+    cpu->env.cp15.c0_cpuid = cpu_class->id;
+}
+
 static void cpu_register(const struct ARMCPUDef *def)
 {
     TypeInfo type = {
         .name = def->name,
         .parent = TYPE_ARM_CPU,
         .instance_size = sizeof(ARMCPU),
+        .instance_init = arm_cpu_realize,
         .class_size = sizeof(ARMCPUClass),
         .class_init = def->class_init,
     };
diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h
index be4bbc3..08b6b2b 100644
--- a/target-arm/cpu-core.h
+++ b/target-arm/cpu-core.h
@@ -10,6 +10,7 @@
 #define QEMU_ARM_CPU_CORE_H
 
 #include "qemu/cpu.h"
+#include "cpu.h"
 
 #define TYPE_ARM_CPU "arm-cpu-core"
 #define ARM_CPU_CLASS(klass) \
@@ -27,7 +28,13 @@ typedef struct ARMCPUClass {
 
 typedef struct ARMCPU {
     CPU parent_obj;
+
+    /* TODO Inline this and split off common state */
+    CPUARMState env;
 } ARMCPU;
 
+#define ENV_GET_OBJECT(e) \
+    (Object *)((void *)(e) - offsetof(ARMCPU, env))
+
 
 #endif
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ece9635..1ffd7ba 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -400,7 +400,7 @@ static int vfp_gdb_set_reg(CPUState *env, uint8_t *buf, int reg)
 CPUARMState *cpu_arm_init(const char *cpu_model)
 {
     ObjectClass *klass;
-    ARMCPUClass *cpu_class;
+    ARMCPU *cpu;
     CPUARMState *env;
     static int inited = 0;
 
@@ -408,16 +408,14 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
     if (klass == NULL) {
         return NULL;
     }
-    cpu_class = ARM_CPU_CLASS(klass);
-    env = g_malloc0(sizeof(CPUARMState));
-    cpu_exec_init(env);
+    cpu = ARM_CPU(object_new_with_type(klass->type));
+    env = &cpu->env;
+
     if (tcg_enabled() && !inited) {
         inited = 1;
         arm_translate_init();
     }
 
-    env->cpu_model_str = cpu_model;
-    env->cp15.c0_cpuid = cpu_class->id;
     cpu_reset(env);
     if (arm_feature(env, ARM_FEATURE_NEON)) {
         gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
@@ -459,7 +457,8 @@ void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 
 void cpu_arm_close(CPUARMState *env)
 {
-    g_free(env);
+    Object *obj = ENV_GET_OBJECT(env);
+    object_delete(obj);
 }
 
 static int bad_mode_switch(CPUState *env, int mode)
-- 
1.7.7

[Qemu-devel] [PATCH RFC 8/7] target-arm: Use IoC for CPU init

[Andreas Färber]

Instead of having each CPU's class_init function call its parent's
function, indirect this through arm_cpu_class_init().

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-arm/cpu-core.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c
index b1ac22c..cdd049e 100644
--- a/target-arm/cpu-core.c
+++ b/target-arm/cpu-core.c
@@ -246,6 +246,13 @@ static void arm_cpu_realize(Object *obj)
     cpu->env.cp15.c0_cpuid = cpu_class->id;
 }
 
+static void arm_cpu_class_init(ObjectClass *klass, void *data)
+{
+    const struct ARMCPUDef *d = data;
+
+    (*d->class_init)(klass, NULL);
+}
+
 static void cpu_register(const struct ARMCPUDef *def)
 {
     TypeInfo type = {
@@ -254,7 +261,8 @@ static void cpu_register(const struct ARMCPUDef *def)
         .instance_size = sizeof(ARMCPU),
         .instance_init = arm_cpu_realize,
         .class_size = sizeof(ARMCPUClass),
-        .class_init = def->class_init,
+        .class_init = arm_cpu_class_init,
+        .class_data = (void *)def,
     };
 
     type_register_static(&type);
-- 
1.7.7

[Qemu-devel] [PATCH RFC 9/7] target-arm: Move CPU feature flags to class

[Andreas Färber]

The internal CPU feature flags were only ever set in
cpu_reset_model_id(). Therefore move them into ARMCPUClass.

The feature flags were saved and loaded as part of CPUState. Keep
writing them so that they can still be loaded in older versions, but
ignore when loading.

Since cpu.h defines ARMCPUState, which has been incorporated into
ARMCPU, and tries to use arm_feature() in cpu_get_tb_cpu_state(),
move arm_feature() to cpu-core.h and add a forward declaration.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu-core.c |  135 +++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu-core.h |    9 +++
 target-arm/cpu.h      |    9 +---
 target-arm/helper.c   |   95 ----------------------------------
 target-arm/machine.c  |    6 ++-
 5 files changed, 150 insertions(+), 104 deletions(-)

diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c
index cdd049e..15b710b 100644
--- a/target-arm/cpu-core.c
+++ b/target-arm/cpu-core.c
@@ -10,6 +10,16 @@
 #include "cpu-core.h"
 #include "qemu-common.h"
 
+static inline void set_feature(ARMCPUClass *klass, int feature)
+{
+    klass->features |= 1u << feature;
+}
+
+static inline int has_feature(ARMCPUClass *klass, int feature)
+{
+    return (klass->features & (1u << feature)) != 0;
+}
+
 /* CPU models */
 
 static void arm926_class_init(ObjectClass *klass, void *data)
@@ -17,6 +27,8 @@ static void arm926_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x41069265;
+    set_feature(k, ARM_FEATURE_V5);
+    set_feature(k, ARM_FEATURE_VFP);
 }
 
 static void arm946_class_init(ObjectClass *klass, void *data)
@@ -24,6 +36,8 @@ static void arm946_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x41059461;
+    set_feature(k, ARM_FEATURE_V5);
+    set_feature(k, ARM_FEATURE_MPU);
 }
 
 static void arm1026_class_init(ObjectClass *klass, void *data)
@@ -31,6 +45,9 @@ static void arm1026_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x4106a262;
+    set_feature(k, ARM_FEATURE_V5);
+    set_feature(k, ARM_FEATURE_VFP);
+    set_feature(k, ARM_FEATURE_AUXCR);
 }
 
 static void arm1136_r0_class_init(ObjectClass *klass, void *data)
@@ -38,13 +55,18 @@ static void arm1136_r0_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x4107b362;
+    set_feature(k, ARM_FEATURE_V6);
+    set_feature(k, ARM_FEATURE_VFP);
 }
 
 static void arm1136_r1_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    arm1136_r0_class_init(klass, data);
+
     k->id = 0x4117b363;
+    set_feature(k, ARM_FEATURE_V6K);
 }
 
 static void arm1176_class_init(ObjectClass *klass, void *data)
@@ -52,6 +74,9 @@ static void arm1176_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x410fb767;
+    set_feature(k, ARM_FEATURE_V6K);
+    set_feature(k, ARM_FEATURE_VFP);
+    set_feature(k, ARM_FEATURE_VAPA);
 }
 
 static void arm11mpcore_class_init(ObjectClass *klass, void *data)
@@ -59,6 +84,9 @@ static void arm11mpcore_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x410fb022;
+    set_feature(k, ARM_FEATURE_V6K);
+    set_feature(k, ARM_FEATURE_VFP);
+    set_feature(k, ARM_FEATURE_VAPA);
 }
 
 static void cortex_m3_class_init(ObjectClass *klass, void *data)
@@ -66,6 +94,8 @@ static void cortex_m3_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x410fc231;
+    set_feature(k, ARM_FEATURE_V7);
+    set_feature(k, ARM_FEATURE_M);
 }
 
 static void cortex_a8_class_init(ObjectClass *klass, void *data)
@@ -73,6 +103,10 @@ static void cortex_a8_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x410fc080;
+    set_feature(k, ARM_FEATURE_V7);
+    set_feature(k, ARM_FEATURE_VFP3);
+    set_feature(k, ARM_FEATURE_NEON);
+    set_feature(k, ARM_FEATURE_THUMB2EE);
 }
 
 static void cortex_a9_class_init(ObjectClass *klass, void *data)
@@ -80,6 +114,16 @@ static void cortex_a9_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x410fc090;
+    set_feature(k, ARM_FEATURE_V7);
+    set_feature(k, ARM_FEATURE_VFP3);
+    set_feature(k, ARM_FEATURE_VFP_FP16);
+    set_feature(k, ARM_FEATURE_NEON);
+    set_feature(k, ARM_FEATURE_THUMB2EE);
+    /* Note that A9 supports the MP extensions even for
+     * A9UP and single-core A9MP (which are both different
+     * and valid configurations; we don't model A9UP).
+     */
+    set_feature(k, ARM_FEATURE_V7MP);
 }
 
 static void cortex_a15_class_init(ObjectClass *klass, void *data)
@@ -87,6 +131,14 @@ static void cortex_a15_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x412fc0f1;
+    set_feature(k, ARM_FEATURE_V7);
+    set_feature(k, ARM_FEATURE_VFP4);
+    set_feature(k, ARM_FEATURE_VFP_FP16);
+    set_feature(k, ARM_FEATURE_NEON);
+    set_feature(k, ARM_FEATURE_THUMB2EE);
+    set_feature(k, ARM_FEATURE_ARM_DIV);
+    set_feature(k, ARM_FEATURE_V7MP);
+    set_feature(k, ARM_FEATURE_GENERIC_TIMER);
 }
 
 static void ti925t_class_init(ObjectClass *klass, void *data)
@@ -94,12 +146,21 @@ static void ti925t_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x54029252;
+    set_feature(k, ARM_FEATURE_V4T);
+    set_feature(k, ARM_FEATURE_OMAPCP);
+}
+
+static void sa11xx_common_class_init(ARMCPUClass *k, void *data)
+{
+    set_feature(k, ARM_FEATURE_STRONGARM);
 }
 
 static void sa1100_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    sa11xx_common_class_init(k, data);
+
     k->id = 0x4401A11B;
 }
 
@@ -107,6 +168,8 @@ static void sa1110_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    sa11xx_common_class_init(k, data);
+
     k->id = 0x6901B119;
 }
 
@@ -115,12 +178,16 @@ static void pxa250_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0x69052100;
+    set_feature(k, ARM_FEATURE_V5);
+    set_feature(k, ARM_FEATURE_XSCALE);
 }
 
 static void pxa255_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa250_class_init(klass, data);
+
     k->id = 0x69052d00;
 }
 
@@ -128,6 +195,8 @@ static void pxa260_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa250_class_init(klass, data);
+
     k->id = 0x69052903;
 }
 
@@ -135,6 +204,8 @@ static void pxa261_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa250_class_init(klass, data);
+
     k->id = 0x69052d05;
 }
 
@@ -142,13 +213,24 @@ static void pxa262_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa250_class_init(klass, data);
+
     k->id = 0x69052d06;
 }
 
+static void pxa270_common_class_init(ARMCPUClass *k, void *data)
+{
+    set_feature(k, ARM_FEATURE_V5);
+    set_feature(k, ARM_FEATURE_XSCALE);
+    set_feature(k, ARM_FEATURE_IWMMXT);
+}
+
 static void pxa270_a0_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa270_common_class_init(k, data);
+
     k->id = 0x69054110;
 }
 
@@ -156,6 +238,8 @@ static void pxa270_a1_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa270_common_class_init(k, data);
+
     k->id = 0x69054111;
 }
 
@@ -163,6 +247,8 @@ static void pxa270_b0_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa270_common_class_init(k, data);
+
     k->id = 0x69054112;
 }
 
@@ -170,6 +256,8 @@ static void pxa270_b1_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa270_common_class_init(k, data);
+
     k->id = 0x69054113;
 }
 
@@ -177,6 +265,8 @@ static void pxa270_c0_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa270_common_class_init(k, data);
+
     k->id = 0x69054114;
 }
 
@@ -184,6 +274,8 @@ static void pxa270_c5_class_init(ObjectClass *klass, void *data)
 {
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
+    pxa270_common_class_init(k, data);
+
     k->id = 0x69054117;
 }
 
@@ -192,6 +284,13 @@ static void arm_any_cpu_class_init(ObjectClass *klass, void *data)
     ARMCPUClass *k = ARM_CPU_CLASS(klass);
 
     k->id = 0xffffffff;
+    set_feature(k, ARM_FEATURE_V7);
+    set_feature(k, ARM_FEATURE_VFP4);
+    set_feature(k, ARM_FEATURE_VFP_FP16);
+    set_feature(k, ARM_FEATURE_NEON);
+    set_feature(k, ARM_FEATURE_THUMB2EE);
+    set_feature(k, ARM_FEATURE_ARM_DIV);
+    set_feature(k, ARM_FEATURE_V7MP);
 }
 
 struct ARMCPUDef {
@@ -248,9 +347,45 @@ static void arm_cpu_realize(Object *obj)
 
 static void arm_cpu_class_init(ObjectClass *klass, void *data)
 {
+    ARMCPUClass *k = ARM_CPU_CLASS(klass);
     const struct ARMCPUDef *d = data;
 
     (*d->class_init)(klass, NULL);
+
+    /* Some features automatically imply others: */
+    if (has_feature(k, ARM_FEATURE_V7)) {
+        set_feature(k, ARM_FEATURE_VAPA);
+        set_feature(k, ARM_FEATURE_THUMB2);
+        if (!has_feature(k, ARM_FEATURE_M)) {
+            set_feature(k, ARM_FEATURE_V6K);
+        } else {
+            set_feature(k, ARM_FEATURE_V6);
+        }
+    }
+    if (has_feature(k, ARM_FEATURE_V6K)) {
+        set_feature(k, ARM_FEATURE_V6);
+    }
+    if (has_feature(k, ARM_FEATURE_V6)) {
+        set_feature(k, ARM_FEATURE_V5);
+        if (!has_feature(k, ARM_FEATURE_M)) {
+            set_feature(k, ARM_FEATURE_AUXCR);
+        }
+    }
+    if (has_feature(k, ARM_FEATURE_V5)) {
+        set_feature(k, ARM_FEATURE_V4T);
+    }
+    if (has_feature(k, ARM_FEATURE_M)) {
+        set_feature(k, ARM_FEATURE_THUMB_DIV);
+    }
+    if (has_feature(k, ARM_FEATURE_ARM_DIV)) {
+        set_feature(k, ARM_FEATURE_THUMB_DIV);
+    }
+    if (has_feature(k, ARM_FEATURE_VFP4)) {
+        set_feature(k, ARM_FEATURE_VFP3);
+    }
+    if (has_feature(k, ARM_FEATURE_VFP3)) {
+        set_feature(k, ARM_FEATURE_VFP);
+    }
 }
 
 static void cpu_register(const struct ARMCPUDef *def)
diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h
index 08b6b2b..80141a1 100644
--- a/target-arm/cpu-core.h
+++ b/target-arm/cpu-core.h
@@ -24,6 +24,9 @@ typedef struct ARMCPUClass {
     CPUClass parent_class;
 
     uint32_t id;
+
+    /* Internal CPU feature flags. */
+    uint32_t features;
 } ARMCPUClass;
 
 typedef struct ARMCPU {
@@ -36,5 +39,11 @@ typedef struct ARMCPU {
 #define ENV_GET_OBJECT(e) \
     (Object *)((void *)(e) - offsetof(ARMCPU, env))
 
+static inline int arm_feature(CPUARMState *env, int feature)
+{
+    ARMCPUClass *k = ARM_CPU_GET_CLASS(ENV_GET_OBJECT(env));
+    return (k->features & (1u << feature)) != 0;
+}
+
 
 #endif
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 0d9b39c..b595e48 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -170,9 +170,6 @@ typedef struct CPUARMState {
     uint32_t teecr;
     uint32_t teehbr;
 
-    /* Internal CPU feature flags.  */
-    uint32_t features;
-
     /* VFP coprocessor state.  */
     struct {
         float64 regs[32];
@@ -385,10 +382,7 @@ enum arm_features {
     ARM_FEATURE_GENERIC_TIMER,
 };
 
-static inline int arm_feature(CPUARMState *env, int feature)
-{
-    return (env->features & (1u << feature)) != 0;
-}
+static inline int arm_feature(CPUARMState *env, int feature);
 
 void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 
@@ -476,6 +470,7 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
+#include "cpu-core.h"
 
 /* Bit usage in the TB flags field: */
 #define ARM_TBFLAG_THUMB_SHIFT      0
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1ffd7ba..535e49d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -52,47 +52,32 @@ static uint32_t arm1176_cp15_c0_c1[8] =
 static uint32_t arm1176_cp15_c0_c2[8] =
 { 0x0140011, 0x12002111, 0x11231121, 0x01102131, 0x01141, 0, 0, 0 };
 
-static inline void set_feature(CPUARMState *env, int feature)
-{
-    env->features |= 1u << feature;
-}
-
 static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 {
     env->cp15.c0_cpuid = id;
     switch (id) {
     case ARM_CPUID_ARM926:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_VFP);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x41011090;
         env->cp15.c0_cachetype = 0x1dd20d2;
         env->cp15.c1_sys = 0x00090078;
         break;
     case ARM_CPUID_ARM946:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_MPU);
         env->cp15.c0_cachetype = 0x0f004006;
         env->cp15.c1_sys = 0x00000078;
         break;
     case ARM_CPUID_ARM1026:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_VFP);
-        set_feature(env, ARM_FEATURE_AUXCR);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410110a0;
         env->cp15.c0_cachetype = 0x1dd20d2;
         env->cp15.c1_sys = 0x00090078;
         break;
     case ARM_CPUID_ARM1136:
         /* This is the 1136 r1, which is a v6K core */
-        set_feature(env, ARM_FEATURE_V6K);
         /* Fall through */
     case ARM_CPUID_ARM1136_R2:
         /* What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
          * older core than plain "arm1136". In particular this does not
          * have the v6K features.
          */
-        set_feature(env, ARM_FEATURE_V6);
-        set_feature(env, ARM_FEATURE_VFP);
         /* These ID register values are correct for 1136 but may be wrong
          * for 1136_r2 (in particular r0p2 does not actually implement most
          * of the ID registers).
@@ -106,9 +91,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM1176:
-        set_feature(env, ARM_FEATURE_V6K);
-        set_feature(env, ARM_FEATURE_VFP);
-        set_feature(env, ARM_FEATURE_VAPA);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410120b5;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11111111;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00000000;
@@ -118,9 +100,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM11MPCORE:
-        set_feature(env, ARM_FEATURE_V6K);
-        set_feature(env, ARM_FEATURE_VFP);
-        set_feature(env, ARM_FEATURE_VAPA);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410120b4;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11111111;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00000000;
@@ -129,10 +108,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c0_cachetype = 0x1dd20d2;
         break;
     case ARM_CPUID_CORTEXA8:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP3);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410330c0;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00011100;
@@ -146,16 +121,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXA9:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP3);
-        set_feature(env, ARM_FEATURE_VFP_FP16);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
-        /* Note that A9 supports the MP extensions even for
-         * A9UP and single-core A9MP (which are both different
-         * and valid configurations; we don't model A9UP).
-         */
-        set_feature(env, ARM_FEATURE_V7MP);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x41034000; /* Guess */
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x01111111;
@@ -168,14 +133,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXA15:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP4);
-        set_feature(env, ARM_FEATURE_VFP_FP16);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
-        set_feature(env, ARM_FEATURE_ARM_DIV);
-        set_feature(env, ARM_FEATURE_V7MP);
-        set_feature(env, ARM_FEATURE_GENERIC_TIMER);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410430f0;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x10110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x11111111;
@@ -189,22 +146,11 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXM3:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_M);
         break;
     case ARM_CPUID_ANY: /* For userspace emulation.  */
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP4);
-        set_feature(env, ARM_FEATURE_VFP_FP16);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
-        set_feature(env, ARM_FEATURE_ARM_DIV);
-        set_feature(env, ARM_FEATURE_V7MP);
         break;
     case ARM_CPUID_TI915T:
     case ARM_CPUID_TI925T:
-        set_feature(env, ARM_FEATURE_V4T);
-        set_feature(env, ARM_FEATURE_OMAPCP);
         env->cp15.c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring.  */
         env->cp15.c0_cachetype = 0x5109149;
         env->cp15.c1_sys = 0x00000070;
@@ -216,8 +162,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
     case ARM_CPUID_PXA260:
     case ARM_CPUID_PXA261:
     case ARM_CPUID_PXA262:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_XSCALE);
         /* JTAG_ID is ((id << 28) | 0x09265013) */
         env->cp15.c0_cachetype = 0xd172172;
         env->cp15.c1_sys = 0x00000078;
@@ -228,58 +172,19 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
     case ARM_CPUID_PXA270_B1:
     case ARM_CPUID_PXA270_C0:
     case ARM_CPUID_PXA270_C5:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_XSCALE);
         /* JTAG_ID is ((id << 28) | 0x09265013) */
-        set_feature(env, ARM_FEATURE_IWMMXT);
         env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
         env->cp15.c0_cachetype = 0xd172172;
         env->cp15.c1_sys = 0x00000078;
         break;
     case ARM_CPUID_SA1100:
     case ARM_CPUID_SA1110:
-        set_feature(env, ARM_FEATURE_STRONGARM);
         env->cp15.c1_sys = 0x00000070;
         break;
     default:
         cpu_abort(env, "Bad CPU ID: %x\n", id);
         break;
     }
-
-    /* Some features automatically imply others: */
-    if (arm_feature(env, ARM_FEATURE_V7)) {
-        set_feature(env, ARM_FEATURE_VAPA);
-        set_feature(env, ARM_FEATURE_THUMB2);
-        if (!arm_feature(env, ARM_FEATURE_M)) {
-            set_feature(env, ARM_FEATURE_V6K);
-        } else {
-            set_feature(env, ARM_FEATURE_V6);
-        }
-    }
-    if (arm_feature(env, ARM_FEATURE_V6K)) {
-        set_feature(env, ARM_FEATURE_V6);
-    }
-    if (arm_feature(env, ARM_FEATURE_V6)) {
-        set_feature(env, ARM_FEATURE_V5);
-        if (!arm_feature(env, ARM_FEATURE_M)) {
-            set_feature(env, ARM_FEATURE_AUXCR);
-        }
-    }
-    if (arm_feature(env, ARM_FEATURE_V5)) {
-        set_feature(env, ARM_FEATURE_V4T);
-    }
-    if (arm_feature(env, ARM_FEATURE_M)) {
-        set_feature(env, ARM_FEATURE_THUMB_DIV);
-    }
-    if (arm_feature(env, ARM_FEATURE_ARM_DIV)) {
-        set_feature(env, ARM_FEATURE_THUMB_DIV);
-    }
-    if (arm_feature(env, ARM_FEATURE_VFP4)) {
-        set_feature(env, ARM_FEATURE_VFP3);
-    }
-    if (arm_feature(env, ARM_FEATURE_VFP3)) {
-        set_feature(env, ARM_FEATURE_VFP);
-    }
 }
 
 void cpu_reset(CPUARMState *env)
diff --git a/target-arm/machine.c b/target-arm/machine.c
index f66b8df..8d2fbd8 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -61,7 +61,8 @@ void cpu_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, env->cp15.c15_diagnostic);
     qemu_put_be32(f, env->cp15.c15_power_diagnostic);
 
-    qemu_put_be32(f, env->features);
+    /* Write internal CPU feature flags for backward compatibility. */
+    qemu_put_be32(f, ARM_CPU_GET_CLASS(ENV_GET_OBJECT(env))->features);
 
     if (arm_feature(env, ARM_FEATURE_VFP)) {
         for (i = 0;  i < 16; i++) {
@@ -179,7 +180,8 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     env->cp15.c15_diagnostic = qemu_get_be32(f);
     env->cp15.c15_power_diagnostic = qemu_get_be32(f);
 
-    env->features = qemu_get_be32(f);
+    /* Ignore internal CPU feature flags (moved to ARMCPUClass). */
+    qemu_get_be32(f);
 
     if (arm_feature(env, ARM_FEATURE_VFP)) {
         for (i = 0;  i < 16; i++) {
-- 
1.7.7

Re: [Qemu-devel] [PATCH RFC 4/7] qom: Introduce CPU class

[Anthony Liguori]

On 01/29/2012 07:25 AM, Andreas Färber wrote:
> It's abstract, derived directly from TYPE_OBJECT (to avoid dependency
> on MODULE_INIT_DEVICE) and for now is empty.
>
> Place it in hw/. Have user emulators pick it up via VPATH, building it
> per target since they didn't use any qdev/QOM devices so far.
>
> Introduce processor_init() for registering, and call module init as
> needed.
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> Cc: Anthony Liguori<aliguori@us.ibm.com>
> ---
>   Makefile.objs      |    1 +
>   Makefile.target    |    9 ++++++---
>   arch_init.c        |    1 +
>   bsd-user/main.c    |    1 +
>   darwin-user/main.c |    1 +
>   hw/cpu.c           |   27 +++++++++++++++++++++++++++
>   include/qemu/cpu.h |   27 +++++++++++++++++++++++++++
>   linux-user/main.c  |    1 +
>   module.h           |    2 ++
>   9 files changed, 67 insertions(+), 3 deletions(-)
>   create mode 100644 hw/cpu.c
>   create mode 100644 include/qemu/cpu.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index b942625..a4b20fa 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -189,6 +189,7 @@ user-obj-y += $(trace-obj-y)
>
>   hw-obj-y =
>   hw-obj-y += vl.o loader.o
> +hw-obj-y += cpu.o
>   hw-obj-$(CONFIG_VIRTIO) += virtio-console.o
>   hw-obj-y += usb-libhw.o
>   hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
> diff --git a/Makefile.target b/Makefile.target
> index d1b7867..5d3470e 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -107,7 +107,7 @@ signal.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
>
>   ifdef CONFIG_LINUX_USER
>
> -$(call set-vpath, $(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR))
> +$(call set-vpath, $(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR):$(SRC_PATH)/hw)
>
>   QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR) -I$(SRC_PATH)/linux-user
>   obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
> @@ -130,6 +130,7 @@ obj-m68k-y += m68k-sim.o m68k-semi.o
>   $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
>
>   obj-y += module.o
> +obj-y += cpu.o
>   obj-y += $(addprefix ../qom/, $(qom-y))
>   obj-y += $(addprefix ../libuser/, $(user-obj-y))
>   obj-y += $(addprefix ../libdis-user/, $(libdis-y))
> @@ -142,7 +143,7 @@ endif #CONFIG_LINUX_USER
>
>   ifdef CONFIG_DARWIN_USER
>
> -$(call set-vpath, $(SRC_PATH)/darwin-user)
> +$(call set-vpath, $(SRC_PATH)/darwin-user:$(SRC_PATH)/hw)
>
>   QEMU_CFLAGS+=-I$(SRC_PATH)/darwin-user -I$(SRC_PATH)/darwin-user/$(TARGET_ARCH)
>
> @@ -159,6 +160,7 @@ obj-i386-y += ioport-user.o
>   $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
>
>   obj-y += module.o
> +obj-y += cpu.o
>   obj-y += $(addprefix ../qom/, $(qom-y))
>   obj-y += $(addprefix ../libuser/, $(user-obj-y))
>   obj-y += $(addprefix ../libdis-user/, $(libdis-y))
> @@ -171,7 +173,7 @@ endif #CONFIG_DARWIN_USER
>
>   ifdef CONFIG_BSD_USER
>
> -$(call set-vpath, $(SRC_PATH)/bsd-user)
> +$(call set-vpath, $(SRC_PATH)/bsd-user:$(SRC_PATH)/hw)
>
>   QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ARCH)
>
> @@ -183,6 +185,7 @@ obj-i386-y += ioport-user.o
>   $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
>
>   obj-y += module.o
> +obj-y += cpu.o
>   obj-y += $(addprefix ../qom/, $(qom-y))
>   obj-y += $(addprefix ../libuser/, $(user-obj-y))
>   obj-y += $(addprefix ../libdis-user/, $(libdis-y))
> diff --git a/arch_init.c b/arch_init.c
> index 2366511..c0d5f4f 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -692,6 +692,7 @@ void do_smbios_option(const char *optarg)
>
>   void cpudef_init(void)
>   {
> +    module_call_init(MODULE_INIT_CPU);
>   #if defined(cpudef_setup)
>       cpudef_setup(); /* parse cpu definitions in target config file */
>   #endif
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 2ff0361..70e1146 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -761,6 +761,7 @@ int main(int argc, char **argv)
>       }
>
>       cpu_model = NULL;
> +    module_call_init(MODULE_INIT_CPU);
>   #if defined(cpudef_setup)
>       cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
>   #endif
> diff --git a/darwin-user/main.c b/darwin-user/main.c
> index a4c630d..d065f00 100644
> --- a/darwin-user/main.c
> +++ b/darwin-user/main.c
> @@ -751,6 +751,7 @@ int main(int argc, char **argv)
>           usage();
>
>       module_call_init(MODULE_INIT_EARLY);
> +    module_call_init(MODULE_INIT_CPU);
>
>       optind = 1;
>       for(;;) {
> diff --git a/hw/cpu.c b/hw/cpu.c
> new file mode 100644
> index 0000000..c0e9cfa
> --- /dev/null
> +++ b/hw/cpu.c
> @@ -0,0 +1,27 @@
> +/*
> + * QEMU CPU model
> + *
> + * Copyright (c) 2012 SUSE LINUX Products GmbH
> + *
> + * Licensed under the terms of the GNU GPL version 2
> + * or (at your option) any later version.
> + */
> +
> +#include "qemu/object.h"
> +#include "qemu/cpu.h"
> +#include "qemu-common.h"
> +
> +static TypeInfo cpu_type_info = {
> +    .name = TYPE_CPU,
> +    .parent = TYPE_OBJECT,
> +    .instance_size = sizeof(CPU),


Probably want to do CPUState or something of that nature so that you can use 
CPU() as a dynamic_cast macro.

BTW, if the class_size == parent.class_size you don't need to define or specific 
the class.

Regards,

Anthony Liguori

> +    .abstract = true,
> +    .class_size = sizeof(CPUClass),
> +};
> +
> +static void cpu_register_types(void)
> +{
> +    type_register_static(&cpu_type_info);
> +}
> +
> +processor_init(cpu_register_types)
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> new file mode 100644
> index 0000000..4b81f3b
> --- /dev/null
> +++ b/include/qemu/cpu.h
> @@ -0,0 +1,27 @@
> +/*
> + * QEMU CPU model
> + *
> + * Copyright (c) 2012 SUSE LINUX Products GmbH
> + *
> + * Licensed under the terms of the GNU GPL version 2
> + * or (at your option) any later version.
> + */
> +#ifndef QEMU_CPU_H
> +#define QEMU_CPU_H
> +
> +#include "qemu/object.h"
> +
> +#define TYPE_CPU "cpu"
> +
> +typedef struct CPUClass {
> +    ObjectClass parent_class;
> +} CPUClass;
> +
> +typedef struct CPU {
> +    Object parent_obj;
> +
> +    /* TODO Move common CPUState here */
> +} CPU;
> +
> +
> +#endif
> diff --git a/linux-user/main.c b/linux-user/main.c
> index d4368b6..e727e8d 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3304,6 +3304,7 @@ int main(int argc, char **argv, char **envp)
>       }
>
>       cpu_model = NULL;
> +    module_call_init(MODULE_INIT_CPU);
>   #if defined(cpudef_setup)
>       cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
>   #endif
> diff --git a/module.h b/module.h
> index 567ff3a..512ba6c 100644
> --- a/module.h
> +++ b/module.h
> @@ -26,6 +26,7 @@ typedef enum {
>       MODULE_INIT_DEVICE,
>       MODULE_INIT_MACHINE,
>       MODULE_INIT_QAPI,
> +    MODULE_INIT_CPU,
>       MODULE_INIT_MAX
>   } module_init_type;
>
> @@ -34,6 +35,7 @@ typedef enum {
>   #define device_init(function) module_init(function, MODULE_INIT_DEVICE)
>   #define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
>   #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
> +#define processor_init(function) module_init(function, MODULE_INIT_CPU)
>
>   void register_module_init(void (*fn)(void), module_init_type type);
>

Re: [Qemu-devel] [PATCH RFC 5/7] cpu: Introduce cpu_class_foreach()

[Anthony Liguori]

On 01/29/2012 07:25 AM, Andreas Färber wrote:
> Provides an easy way to loop over all non-abstract CPU classes.
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>

I have a patch in my next series which provides an interface for this.

https://github.com/aliguori/qemu/commit/42ab3f78477307438591c77816f06f12cf9d9fc0

I'll post a proper version tomorrow.

Regards,

Anthony Liguori

> ---
>   hw/cpu.c           |   25 +++++++++++++++++++++++++
>   include/qemu/cpu.h |    9 +++++++++
>   2 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/hw/cpu.c b/hw/cpu.c
> index c0e9cfa..ac0add7 100644
> --- a/hw/cpu.c
> +++ b/hw/cpu.c
> @@ -11,6 +11,31 @@
>   #include "qemu/cpu.h"
>   #include "qemu-common.h"
>
> +struct CPUListData {
> +    void (*fn)(ObjectClass *klass, void *opaque);
> +    void *opaque;
> +};
> +
> +static void cpu_class_foreach_tramp(ObjectClass *klass, void *opaque)
> +{
> +    struct CPUListData *s = opaque;
> +
> +    if (!object_class_is_abstract(klass)&&
> +        object_class_dynamic_cast(klass, TYPE_CPU) != NULL) {
> +        s->fn(klass, s->opaque);
> +    }
> +}
> +
> +void cpu_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
> +                       void *opaque)
> +{
> +    struct CPUListData s = {
> +        .fn = fn,
> +        .opaque = opaque,
> +    };
> +    object_class_foreach(cpu_class_foreach_tramp,&s);
> +}
> +
>   static TypeInfo cpu_type_info = {
>       .name = TYPE_CPU,
>       .parent = TYPE_OBJECT,
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> index 4b81f3b..d06c87e 100644
> --- a/include/qemu/cpu.h
> +++ b/include/qemu/cpu.h
> @@ -24,4 +24,13 @@ typedef struct CPU {
>   } CPU;
>
>
> +/**
> + * cpu_class_foreach:
> + * @fn: Callback function called for each non-abstract CPU type.
> + * @opaque: Opaque data passed through to the callback function.
> + */
> +void cpu_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
> +                       void *opaque);
> +
> +
>   #endif

Re: [Qemu-devel] [PATCH RFC 6/7] target-arm: Introduce QOM CPU and use for it CPUID lookup

[Anthony Liguori]

On 01/29/2012 07:25 AM, Andreas Färber wrote:
> Create a CPU subclass, and register classes matching all CPU models.
> Don't name the file target-arm/cpu.c so that the user emulators can
> still easily pick up the base class in hw/cpu.c via VPATH.
>
> Make arm_cpu_list() enumerate CPU subclasses.
>
> Replace cpu_arm_find_by_name()'s string ->  CPUID lookup by storing the
> CPUID in the class.
> NB: CPUIDs were first introduced by Paul Brook in r1765 (2006).
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> Cc: Anthony Liguori<aliguori@us.ibm.com>
> Cc: Paul Brook<paul@codesourcery.com>
> Cc: Peter Maydell<peter.maydell@linaro.org>
> ---
>   Makefile.target       |    1 +
>   target-arm/cpu-core.c |  268 +++++++++++++++++++++++++++++++++++++++++++++++++
>   target-arm/cpu-core.h |   33 ++++++
>   target-arm/helper.c   |   80 ++++-----------
>   4 files changed, 324 insertions(+), 58 deletions(-)
>   create mode 100644 target-arm/cpu-core.c
>   create mode 100644 target-arm/cpu-core.h
>
> diff --git a/Makefile.target b/Makefile.target
> index 5d3470e..96043c4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -80,6 +80,7 @@ endif
>   libobj-$(TARGET_SPARC64) += vis_helper.o
>   libobj-$(CONFIG_NEED_MMU) += mmu.o
>   libobj-$(TARGET_ARM) += neon_helper.o iwmmxt_helper.o
> +libobj-$(TARGET_ARM) += cpu-core.o
>   ifeq ($(TARGET_BASE_ARCH), sparc)
>   libobj-y += fop_helper.o cc_helper.o win_helper.o mmu_helper.o ldst_helper.o
>   libobj-y += cpu_init.o
> diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c
> new file mode 100644
> index 0000000..9761d8e
> --- /dev/null
> +++ b/target-arm/cpu-core.c
> @@ -0,0 +1,268 @@
> +/*
> + * QEMU ARM CPU core
> + *
> + * Copyright (c) 2012 SUSE LINUX Products GmbH
> + *
> + * Licensed under the terms of the GNU GPL version 2
> + * or (at your option) any later version.
> + */
> +
> +#include "cpu-core.h"
> +#include "qemu-common.h"
> +
> +/* CPU models */
> +
> +static void arm926_class_init(ObjectClass *klass, void *data)
> +{
> +    ARMCPUClass *k = ARM_CPU_CLASS(klass);
> +
> +    k->id = 0x41069265;
> +}
> +
> +static void arm946_class_init(ObjectClass *klass, void *data)
> +{
> +    ARMCPUClass *k = ARM_CPU_CLASS(klass);
> +
> +    k->id = 0x41059461;
> +}

In a situation like this, you probably want to make use of the class_data field 
in TypeInfo.  You can use that to create a bunch of types based on a table.

Take a look at hw/eepro100.c for an example of this (although read the comment 
for the reference to class_data and why we can't use it until after the next 
series).

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH RFC 7/7] target-arm: Embed CPUARMState in QOM ARMCPU

[Anthony Liguori]

On 01/29/2012 07:25 AM, Andreas Färber wrote:
> We g_malloc0()'ed CPUARMState ourself, and exec.c's cpu_copy() runs
> through cpu_init() as well, so we are at liberty to supply the CPUState
> any way we see fit. Having CPUARMState as field in the QOM CPU allows
> both to access env from an ARMCPU object and to access the QOM Object
> and its ObjectClass from an env pointer, in ARM code for now.
>
> The goal is to convert all CPUs to QOM and to use CPU objects in central
> places, especially once we have property support for Object.
> This will then allow to have TCG AREG0 point to target-specific fields
> where small immediate offsets are desired (as pointed out by rth) while
> allowing for common fields at known offsets from the base class.
>
> Having the CPUID in ARMCPUClass, we can set it from the realize function.
> Same for cpu_model_str, which is now the QOM class name.
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> Cc: Anthony Liguori<aliguori@us.ibm.com>
> Cc: Paul Brook<paul@codesourcery.com>
> Cc: Peter Maydell<peter.maydell@linaro.org>
> Cc: Richard Henderson<rth@twiddle.net>
> ---
>   target-arm/cpu-core.c |   13 +++++++++++++
>   target-arm/cpu-core.h |    7 +++++++
>   target-arm/helper.c   |   13 ++++++-------
>   3 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c
> index 9761d8e..b1ac22c 100644
> --- a/target-arm/cpu-core.c
> +++ b/target-arm/cpu-core.c
> @@ -234,12 +234,25 @@ static const struct ARMCPUDef arm_cpu_models[] = {
>       { }
>   };
>
> +static void arm_cpu_realize(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +    ARMCPUClass *cpu_class = ARM_CPU_GET_CLASS(obj);
> +
> +    memset(&cpu->env, 0, sizeof(CPUARMState));
> +    cpu_exec_init(&cpu->env);
> +
> +    cpu->env.cpu_model_str = object_get_typename(obj);
> +    cpu->env.cp15.c0_cpuid = cpu_class->id;
> +}
> +
>   static void cpu_register(const struct ARMCPUDef *def)
>   {
>       TypeInfo type = {
>           .name = def->name,
>           .parent = TYPE_ARM_CPU,
>           .instance_size = sizeof(ARMCPU),
> +        .instance_init = arm_cpu_realize,


The convention I'm using is: instance_init => type_name_initfn.

DeviceState::init => type_name_realize,

Eventually, realized will become a property and there will be a realize and 
unrealize method.

>           .class_size = sizeof(ARMCPUClass),
>           .class_init = def->class_init,
>       };
> diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h
> index be4bbc3..08b6b2b 100644
> --- a/target-arm/cpu-core.h
> +++ b/target-arm/cpu-core.h
> @@ -10,6 +10,7 @@
>   #define QEMU_ARM_CPU_CORE_H
>
>   #include "qemu/cpu.h"
> +#include "cpu.h"
>
>   #define TYPE_ARM_CPU "arm-cpu-core"
>   #define ARM_CPU_CLASS(klass) \
> @@ -27,7 +28,13 @@ typedef struct ARMCPUClass {
>
>   typedef struct ARMCPU {
>       CPU parent_obj;
> +
> +    /* TODO Inline this and split off common state */
> +    CPUARMState env;

This is an interesting (and good) idea.  I think this could make it fairly easy 
to qomify things.

>   } ARMCPU;
>
> +#define ENV_GET_OBJECT(e) \
> +    (Object *)((void *)(e) - offsetof(ARMCPU, env))

sizeof(CPU) should be sizeof(void *).  Presumably it's okay to add 8 bytes to 
the beginning of CPUState?

If so, that would make things much nicer from a QOM perspective.

Regards,

Anthony Liguori

>
>   #endif
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ece9635..1ffd7ba 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -400,7 +400,7 @@ static int vfp_gdb_set_reg(CPUState *env, uint8_t *buf, int reg)
>   CPUARMState *cpu_arm_init(const char *cpu_model)
>   {
>       ObjectClass *klass;
> -    ARMCPUClass *cpu_class;
> +    ARMCPU *cpu;
>       CPUARMState *env;
>       static int inited = 0;
>
> @@ -408,16 +408,14 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
>       if (klass == NULL) {
>           return NULL;
>       }
> -    cpu_class = ARM_CPU_CLASS(klass);
> -    env = g_malloc0(sizeof(CPUARMState));
> -    cpu_exec_init(env);
> +    cpu = ARM_CPU(object_new_with_type(klass->type));
> +    env =&cpu->env;
> +
>       if (tcg_enabled()&&  !inited) {
>           inited = 1;
>           arm_translate_init();
>       }
>
> -    env->cpu_model_str = cpu_model;
> -    env->cp15.c0_cpuid = cpu_class->id;
>       cpu_reset(env);
>       if (arm_feature(env, ARM_FEATURE_NEON)) {
>           gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
> @@ -459,7 +457,8 @@ void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>
>   void cpu_arm_close(CPUARMState *env)
>   {
> -    g_free(env);
> +    Object *obj = ENV_GET_OBJECT(env);
> +    object_delete(obj);
>   }
>
>   static int bad_mode_switch(CPUState *env, int mode)

Re: [Qemu-devel] [PATCH RFC 8/7] target-arm: Use IoC for CPU init

[Anthony Liguori]

On 01/29/2012 05:50 PM, Andreas Färber wrote:
> Instead of having each CPU's class_init function call its parent's
> function, indirect this through arm_cpu_class_init().
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> ---
>   target-arm/cpu-core.c |   10 +++++++++-
>   1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c
> index b1ac22c..cdd049e 100644
> --- a/target-arm/cpu-core.c
> +++ b/target-arm/cpu-core.c
> @@ -246,6 +246,13 @@ static void arm_cpu_realize(Object *obj)
>       cpu->env.cp15.c0_cpuid = cpu_class->id;
>   }
>
> +static void arm_cpu_class_init(ObjectClass *klass, void *data)
> +{
> +    const struct ARMCPUDef *d = data;
> +
> +    (*d->class_init)(klass, NULL);
> +}
> +
>   static void cpu_register(const struct ARMCPUDef *def)
>   {
>       TypeInfo type = {
> @@ -254,7 +261,8 @@ static void cpu_register(const struct ARMCPUDef *def)
>           .instance_size = sizeof(ARMCPU),
>           .instance_init = arm_cpu_realize,
>           .class_size = sizeof(ARMCPUClass),
> -        .class_init = def->class_init,
> +        .class_init = arm_cpu_class_init,
> +        .class_data = (void *)def,

Would be better IMHO to embed the data needed by class_init in class_data.  That 
way we're not going through multiple method tables.

Regards,

Anthony Liguori

>       };
>
>       type_register_static(&type);

Re: [Qemu-devel] [PATCH RFC 4/7] qom: Introduce CPU class

[Andreas Färber]

Am 30.01.2012 03:14, schrieb Anthony Liguori:
> On 01/29/2012 07:25 AM, Andreas Färber wrote:
>> +static TypeInfo cpu_type_info = {
>> +    .name = TYPE_CPU,
>> +    .parent = TYPE_OBJECT,
>> +    .instance_size = sizeof(CPU),
> 
> 
> Probably want to do CPUState or something of that nature so that you can
> use CPU() as a dynamic_cast macro.

Yeah, I stumbled upon that but didn't need it so far. CPUState is
already taken by the old concept.
In an earlier version I had used CPUCore (CPU_CORE()) but then I thought
that doesn't sound right for HyperThreading (CPU -> cores -> threads).
Since QEMU so far uses the term "CPU" for all of those I went with that.
Any better suggestions?

> BTW, if the class_size == parent.class_size you don't need to define or
> specific the class.

I'm sorry? You mean we don't need CPUClass yet because it doesn't add
anything to ObjectClass yet? I was planning on having a reset function
defined there, to be called in place of current aliased cpu_reset(). For
that to work I am emptying cpu_arm_reset() step by step (cf. 9/7) until
it can defer to:
(*CPU_GET_CLASS(ENV_GET_OBJECT(env))->reset)(ENV_GET_OBJECT(env));

I've been thinking along the lines of: if something doesn't change at
runtime then it goes into a class.

Or did you mean something else?

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 RFC 5/7] cpu: Introduce cpu_class_foreach()

[Andreas Färber]

Am 30.01.2012 03:16, schrieb Anthony Liguori:
> On 01/29/2012 07:25 AM, Andreas Färber wrote:
>> Provides an easy way to loop over all non-abstract CPU classes.
>>
>> Signed-off-by: Andreas Färber<afaerber@suse.de>
> 
> I have a patch in my next series which provides an interface for this.
> 
> https://github.com/aliguori/qemu/commit/42ab3f78477307438591c77816f06f12cf9d9fc0

Looks good, sorry I missed it.

You might want to document it in object.h while you're at it.

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 RFC 6/7] target-arm: Introduce QOM CPU and use for it CPUID lookup

[Andreas Färber]

Am 30.01.2012 03:19, schrieb Anthony Liguori:
> On 01/29/2012 07:25 AM, Andreas Färber wrote:
>> +/* CPU models */
>> +
>> +static void arm926_class_init(ObjectClass *klass, void *data)
>> +{
>> +    ARMCPUClass *k = ARM_CPU_CLASS(klass);
>> +
>> +    k->id = 0x41069265;
>> +}
>> +
>> +static void arm946_class_init(ObjectClass *klass, void *data)
>> +{
>> +    ARMCPUClass *k = ARM_CPU_CLASS(klass);
>> +
>> +    k->id = 0x41059461;
>> +}
> 
> In a situation like this, you probably want to make use of the
> class_data field in TypeInfo.  You can use that to create a bunch of
> types based on a table.

That would work for this first trivial (which is why I picked it)
example, but a declarative table approach would not work well for things
that don't apply to all models.
Not to mention that I thought you wanted to have everything declarative
and might even be opposed to my name -> class_init table. ;)

A table approach for features would mean introducing a non-imperative
FEATURE() macro.

It might work to still include an optional class_init for those models
that need it (re 8/7). Will give it a try.

> Take a look at hw/eepro100.c for an example of this (although read the
> comment for the reference to class_data and why we can't use it until
> after the next series).

Thanks for the pointer, will check. (Hm, for 9/7 it appeared to work...)

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 RFC 7/7] target-arm: Embed CPUARMState in QOM ARMCPU

[Andreas Färber]

Am 30.01.2012 03:22, schrieb Anthony Liguori:
> On 01/29/2012 07:25 AM, Andreas Färber wrote:
>> +#define ENV_GET_OBJECT(e) \
>> +    (Object *)((void *)(e) - offsetof(ARMCPU, env))
> 
> sizeof(CPU) should be sizeof(void *).

Not following... CPU is a struct, so:
sizeof(ARMCPU) > sizeof(CPU) >= sizeof(Object) == 2x sizeof(void *)

The above formula is calculating the address of the ARMCPU object the
CPUState *env pointer belongs to. Maybe I should make it an inline
function for readability, I just followed the QOM naming conventions.

> Presumably it's okay to add 8
> bytes to the beginning of CPUState?

The whole problem with today's CPUState is that CPU_COMMON fields are at
the back or in the middle, with target-specific offset from the pointer.
We can of course add an Object *obj field to the start of each
CPU*State, but rth's (and Peter's?) worry was that adding things to the
front may push fields in a (TCG) hot path out of range for
immediate-offset load/stores. Thus if we just do it in the back for ARM
I don't see a benefit over the above solution. Per target we know the
offset.

Fields in the front of CPUState are usually
memset(env, 0, offsetof(CPUState, breakpoints));
on reset - one issue this series is working towards cleaning up. :) If
we add an Object* to the front we also have to save and restore it on
reset per target.

Again, the idea here is an interim solution to get moving in the right
direction. Things that are target-specific should not be accessed from
generic code but indirected through CPUClass methods, dispatching to
target code (e.g., reset), at some point obsoleting the aliased
cpu_reset() functions with one real function in cpu.c.
If it turns out that the TLB is common code modulo target_ulong or so
then it may be moved from CPU*State to CPU, making common code work with
CPU rather than CPUState. Long-term, today's CPUState would remain
what's needed for TCG and it's AREG0 + offset(CPUState, x) calculations.

Regards,
Andreas

> If so, that would make things much nicer from a QOM perspective.
> 
> Regards,
> 
> Anthony Liguori

-- 
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 RFC 7/7] target-arm: Embed CPUARMState in QOM ARMCPU

[Andreas Färber]

Am 30.01.2012 03:22, schrieb Anthony Liguori:
> On 01/29/2012 07:25 AM, Andreas Färber wrote:
>> +static void arm_cpu_realize(Object *obj)
>> +{
>> +    ARMCPU *cpu = ARM_CPU(obj);
>> +    ARMCPUClass *cpu_class = ARM_CPU_GET_CLASS(obj);
>> +
>> +    memset(&cpu->env, 0, sizeof(CPUARMState));
>> +    cpu_exec_init(&cpu->env);
>> +
>> +    cpu->env.cpu_model_str = object_get_typename(obj);
>> +    cpu->env.cp15.c0_cpuid = cpu_class->id;
>> +}
>> +
>>   static void cpu_register(const struct ARMCPUDef *def)
>>   {
>>       TypeInfo type = {
>>           .name = def->name,
>>           .parent = TYPE_ARM_CPU,
>>           .instance_size = sizeof(ARMCPU),
>> +        .instance_init = arm_cpu_realize,
> 
> 
> The convention I'm using is: instance_init => type_name_initfn.
> 
> DeviceState::init => type_name_realize,

Fixed, thanks.

Andreas

> 
> Eventually, realized will become a property and there will be a realize
> and unrealize method.

-- 
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 RFC 9/7] target-arm: Move CPU feature flags to class

[Andreas Färber]

Am 30.01.2012 00:50, schrieb Andreas Färber:
> The internal CPU feature flags were only ever set in
> cpu_reset_model_id(). Therefore move them into ARMCPUClass.

Peter made me aware that this will not work for -cpu cortex-a9,-neon.
The initial feature flags can be stored in ARMCPUClass but the actual
ones need to be stored and checked in ARMCPU.

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 RFC 4/7] qom: Introduce CPU class

[Andreas Färber]

Am 30.01.2012 03:14, schrieb Anthony Liguori:
> On 01/29/2012 07:25 AM, Andreas Färber wrote:
>> +static TypeInfo cpu_type_info = {
>> +    .name = TYPE_CPU,
>> +    .parent = TYPE_OBJECT,
>> +    .instance_size = sizeof(CPU),
> 
> 
> Probably want to do CPUState or something of that nature so that you can
> use CPU() as a dynamic_cast macro.

My testing seems to indicate CPU and CPU(obj) can co-exist (GCC 4.6.2).

Considered name alternatives:

CPUState - already taken as #define for legacy CPU${arch}State
CPUCore - fits ARM but there's socket vs. core vs. thread on x86
CPUCommon - puts the emphasis on common, which is not a noun
CommonCPU - no other base class uses such a naming scheme
CPUModel - it's not just a model but the state of one instance
CPUFamily - same as for CPUModel

CPUState would be nicest IMO, but refactoring CPUState into something
else just to refactor it back to QOM CPUState feels like churn.

Andreas

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