[Qemu-devel] [PATCH 0/7] target-alpha: More CPU QOM'ifications

[Qemu-devel] [PATCH 0/7] target-alpha: More CPU QOM'ifications

[Andreas Färber]

Hello Richard,

This series introduces CPU subclasses and implements -cpu ? support and CPU reset.
The PATCHes are intended for v1.3; the reset RFC can well be postponed if more
work is needed, it demonstrates what the preceding patch is about.

Regards,
Andreas

Cc: Richard Henderson <rth@twiddle.net>

Andreas Färber (7):
  target-alpha: Use consistent include paths
  target-alpha: Turn CPU definitions into subclasses
  target-alpha: Add support for -cpu ?
  target-alpha: Let cpu_alpha_init() return AlphaCPU
  alpha: Pass AlphaCPU array to Typhoon
  target-alpha: Avoid leaking the alarm timer over reset
  target-alpha: Implement CPU reset

 hw/alpha_dp264.c          |   18 ++--
 hw/alpha_sys.h            |    2 +-
 hw/alpha_typhoon.c        |   30 +++---
 target-alpha/cpu-qom.h    |    3 +
 target-alpha/cpu.c        |  228 ++++++++++++++++++++++++++++++++++++++++++++-
 target-alpha/cpu.h        |   16 +++-
 target-alpha/sys_helper.c |    6 +-
 target-alpha/translate.c  |   51 ++--------
 8 Dateien geändert, 280 Zeilen hinzugefügt(+), 74 Zeilen entfernt(-)

-- 
1.7.10.4

[Qemu-devel] [FYI 1/7] target-alpha: Use consistent include paths

[Andreas Färber]

Part of "cpus: Pass CPUState to [qemu_]cpu_has_work()" in pull request
for master.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-alpha/cpu.c |    2 +-
 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)

diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 62d2a66..11a19eb 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -19,7 +19,7 @@
  * <http://www.gnu.org/licenses/lgpl-2.1.html>
  */
 
-#include "cpu-qom.h"
+#include "cpu.h"
 #include "qemu-common.h"
 
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/7] target-alpha: Turn CPU definitions into subclasses

[Andreas Färber]

Make TYPE_ALPHA_CPU abstract and default to creating type "ev67".

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-alpha/cpu.c       |  157 +++++++++++++++++++++++++++++++++++++++++++++-
 target-alpha/translate.c |   49 +++------------
 2 Dateien geändert, 163 Zeilen hinzugefügt(+), 43 Zeilen entfernt(-)

diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 11a19eb..e1a5739 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -23,6 +23,145 @@
 #include "qemu-common.h"
 
 
+/* Models */
+
+static void ev4_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_2106x;
+}
+
+static void ev5_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21164;
+}
+
+static void ev56_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21164;
+    env->amask = AMASK_BWX;
+}
+
+static void pca56_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21164;
+    env->amask = AMASK_BWX | AMASK_MVI;
+}
+
+static void ev6_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21264;
+    env->amask = AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP;
+}
+
+static void ev67_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21264;
+    env->amask = AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI | AMASK_TRAP |
+                 AMASK_PREFETCH;
+}
+
+static void ev68_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21264;
+    env->amask = AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI | AMASK_TRAP |
+                 AMASK_PREFETCH;
+}
+
+static void alpha_21064_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_2106x;
+}
+
+static void alpha_21164_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21164;
+}
+
+static void alpha_21164a_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21164;
+    env->amask = AMASK_BWX;
+}
+
+static void alpha_21164pc_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21164;
+    env->amask = AMASK_BWX | AMASK_MVI;
+}
+
+static void alpha_21264_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21264;
+    env->amask = AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP;
+}
+
+static void alpha_21264a_cpu_initfn(Object *obj)
+{
+    AlphaCPU *cpu = ALPHA_CPU(obj);
+    CPUAlphaState *env = &cpu->env;
+
+    env->implver = IMPLVER_21264;
+    env->amask = AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI | AMASK_TRAP |
+                 AMASK_PREFETCH;
+}
+
+typedef struct AlphaCPUInfo {
+    const char *name;
+    void (*initfn)(Object *obj);
+} AlphaCPUInfo;
+
+static const AlphaCPUInfo alpha_cpus[] = {
+    { .name = "ev4",     .initfn = ev4_cpu_initfn },
+    { .name = "ev5",     .initfn = ev5_cpu_initfn },
+    { .name = "ev56",    .initfn = ev56_cpu_initfn },
+    { .name = "pca56",   .initfn = pca56_cpu_initfn },
+    { .name = "ev6",     .initfn = ev6_cpu_initfn },
+    { .name = "ev67",    .initfn = ev67_cpu_initfn },
+    { .name = "ev68",    .initfn = ev68_cpu_initfn },
+    { .name = "21064",   .initfn = alpha_21064_cpu_initfn },
+    { .name = "21164",   .initfn = alpha_21164_cpu_initfn },
+    { .name = "21164a",  .initfn = alpha_21164a_cpu_initfn },
+    { .name = "21164pc", .initfn = alpha_21164pc_cpu_initfn },
+    { .name = "21264",   .initfn = alpha_21264_cpu_initfn },
+    { .name = "21264a",  .initfn = alpha_21264a_cpu_initfn },
+};
+
 static void alpha_cpu_initfn(Object *obj)
 {
     AlphaCPU *cpu = ALPHA_CPU(obj);
@@ -41,18 +180,34 @@ static void alpha_cpu_initfn(Object *obj)
     env->fen = 1;
 }
 
+static void alpha_cpu_register(const AlphaCPUInfo *info)
+{
+    TypeInfo type_info = {
+        .name = info->name,
+        .parent = TYPE_ALPHA_CPU,
+        .instance_init = info->initfn,
+    };
+
+    type_register_static(&type_info);
+}
+
 static const TypeInfo alpha_cpu_type_info = {
     .name = TYPE_ALPHA_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(AlphaCPU),
     .instance_init = alpha_cpu_initfn,
-    .abstract = false,
+    .abstract = true,
     .class_size = sizeof(AlphaCPUClass),
 };
 
 static void alpha_cpu_register_types(void)
 {
+    int i;
+
     type_register_static(&alpha_cpu_type_info);
+    for (i = 0; i < ARRAY_SIZE(alpha_cpus); i++) {
+        alpha_cpu_register(&alpha_cpus[i]);
+    }
 }
 
 type_init(alpha_cpu_register_types)
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index f707d8d..6ee031d 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -3493,56 +3493,21 @@ void gen_intermediate_code_pc (CPUAlphaState *env, struct TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-struct cpu_def_t {
-    const char *name;
-    int implver, amask;
-};
-
-static const struct cpu_def_t cpu_defs[] = {
-    { "ev4",   IMPLVER_2106x, 0 },
-    { "ev5",   IMPLVER_21164, 0 },
-    { "ev56",  IMPLVER_21164, AMASK_BWX },
-    { "pca56", IMPLVER_21164, AMASK_BWX | AMASK_MVI },
-    { "ev6",   IMPLVER_21264, AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP },
-    { "ev67",  IMPLVER_21264, (AMASK_BWX | AMASK_FIX | AMASK_CIX
-			       | AMASK_MVI | AMASK_TRAP | AMASK_PREFETCH), },
-    { "ev68",  IMPLVER_21264, (AMASK_BWX | AMASK_FIX | AMASK_CIX
-			       | AMASK_MVI | AMASK_TRAP | AMASK_PREFETCH), },
-    { "21064", IMPLVER_2106x, 0 },
-    { "21164", IMPLVER_21164, 0 },
-    { "21164a", IMPLVER_21164, AMASK_BWX },
-    { "21164pc", IMPLVER_21164, AMASK_BWX | AMASK_MVI },
-    { "21264", IMPLVER_21264, AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP },
-    { "21264a", IMPLVER_21264, (AMASK_BWX | AMASK_FIX | AMASK_CIX
-				| AMASK_MVI | AMASK_TRAP | AMASK_PREFETCH), }
-};
-
-CPUAlphaState * cpu_alpha_init (const char *cpu_model)
+CPUAlphaState *cpu_alpha_init(const char *cpu_model)
 {
     AlphaCPU *cpu;
     CPUAlphaState *env;
-    int implver, amask, i, max;
+    const char *typename = cpu_model;
 
-    cpu = ALPHA_CPU(object_new(TYPE_ALPHA_CPU));
+    if (object_class_by_name(typename) == NULL) {
+        /* Default to ev67; no reason not to emulate insns by default.  */
+        typename = "ev67";
+    }
+    cpu = ALPHA_CPU(object_new(typename));
     env = &cpu->env;
 
     alpha_translate_init();
 
-    /* Default to ev67; no reason not to emulate insns by default.  */
-    implver = IMPLVER_21264;
-    amask = (AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI
-	     | AMASK_TRAP | AMASK_PREFETCH);
-
-    max = ARRAY_SIZE(cpu_defs);
-    for (i = 0; i < max; i++) {
-        if (strcmp (cpu_model, cpu_defs[i].name) == 0) {
-            implver = cpu_defs[i].implver;
-            amask = cpu_defs[i].amask;
-            break;
-        }
-    }
-    env->implver = implver;
-    env->amask = amask;
     env->cpu_model_str = cpu_model;
 
     qemu_init_vcpu(env);
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/7] target-alpha: Add support for -cpu ?

[Andreas Färber]

Implement alphabetical listing of CPU subclasses.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-alpha/cpu.c |   41 +++++++++++++++++++++++++++++++++++++++++
 target-alpha/cpu.h |    4 +++-
 2 Dateien geändert, 44 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)

diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index e1a5739..ab25c44 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -23,6 +23,47 @@
 #include "qemu-common.h"
 
 
+typedef struct AlphaCPUListState {
+    fprintf_function cpu_fprintf;
+    FILE *file;
+} AlphaCPUListState;
+
+/* Sort alphabetically by type name. */
+static gint alpha_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+    ObjectClass *class_a = (ObjectClass *)a;
+    ObjectClass *class_b = (ObjectClass *)b;
+    const char *name_a, *name_b;
+
+    name_a = object_class_get_name(class_a);
+    name_b = object_class_get_name(class_b);
+    return strcmp(name_a, name_b);
+}
+
+static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
+{
+    ObjectClass *oc = data;
+    AlphaCPUListState *s = user_data;
+
+    (*s->cpu_fprintf)(s->file, "  %s\n",
+                      object_class_get_name(oc));
+}
+
+void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+    AlphaCPUListState s = {
+        .file = f,
+        .cpu_fprintf = cpu_fprintf,
+    };
+    GSList *list;
+
+    list = object_class_get_list(TYPE_ALPHA_CPU, false);
+    list = g_slist_sort(list, alpha_cpu_list_compare);
+    (*cpu_fprintf)(f, "Available CPUs:\n");
+    g_slist_foreach(list, alpha_cpu_list_entry, &s);
+    g_slist_free(list);
+}
+
 /* Models */
 
 static void ev4_cpu_initfn(Object *obj)
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 8f131b7..28999ab 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -298,6 +298,7 @@ struct CPUAlphaState {
 };
 
 #define cpu_init cpu_alpha_init
+#define cpu_list alpha_cpu_list
 #define cpu_exec cpu_alpha_exec
 #define cpu_gen_code cpu_alpha_gen_code
 #define cpu_signal_handler cpu_alpha_signal_handler
@@ -434,7 +435,8 @@ enum {
     IR_ZERO = 31,
 };
 
-CPUAlphaState * cpu_alpha_init (const char *cpu_model);
+CPUAlphaState *cpu_alpha_init(const char *cpu_model);
+void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 int cpu_alpha_exec(CPUAlphaState *s);
 /* you can call this signal handler from your SIGBUS and SIGSEGV
    signal handlers to inform the virtual CPU of exceptions. non zero
-- 
1.7.10.4

[Qemu-devel] [PATCH 4/7] target-alpha: Let cpu_alpha_init() return AlphaCPU

[Andreas Färber]

Replace cpu_init() macro with inline function for backwards
compatibility.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-alpha/cpu.h       |   13 +++++++++++--
 target-alpha/translate.c |    4 ++--
 2 Dateien geändert, 13 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-)

diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 28999ab..2bbde9c 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -297,7 +297,6 @@ struct CPUAlphaState {
     int implver;
 };
 
-#define cpu_init cpu_alpha_init
 #define cpu_list alpha_cpu_list
 #define cpu_exec cpu_alpha_exec
 #define cpu_gen_code cpu_alpha_gen_code
@@ -435,7 +434,17 @@ enum {
     IR_ZERO = 31,
 };
 
-CPUAlphaState *cpu_alpha_init(const char *cpu_model);
+AlphaCPU *cpu_alpha_init(const char *cpu_model);
+
+static inline CPUAlphaState *cpu_init(const char *cpu_model)
+{
+    AlphaCPU *cpu = cpu_alpha_init(cpu_model);
+    if (cpu == NULL) {
+        return NULL;
+    }
+    return &cpu->env;
+}
+
 void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 int cpu_alpha_exec(CPUAlphaState *s);
 /* you can call this signal handler from your SIGBUS and SIGSEGV
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 6ee031d..5419df1 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -3493,7 +3493,7 @@ void gen_intermediate_code_pc (CPUAlphaState *env, struct TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-CPUAlphaState *cpu_alpha_init(const char *cpu_model)
+AlphaCPU *cpu_alpha_init(const char *cpu_model)
 {
     AlphaCPU *cpu;
     CPUAlphaState *env;
@@ -3511,7 +3511,7 @@ CPUAlphaState *cpu_alpha_init(const char *cpu_model)
     env->cpu_model_str = cpu_model;
 
     qemu_init_vcpu(env);
-    return env;
+    return cpu;
 }
 
 void restore_state_to_opc(CPUAlphaState *env, TranslationBlock *tb, int pc_pos)
-- 
1.7.10.4

[Qemu-devel] [PATCH 5/7] alpha: Pass AlphaCPU array to Typhoon

[Andreas Färber]

Also store it in TyphoonCchip.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/alpha_dp264.c   |   18 +++++++++---------
 hw/alpha_sys.h     |    2 +-
 hw/alpha_typhoon.c |   29 ++++++++++++++++-------------
 3 Dateien geändert, 26 Zeilen hinzugefügt(+), 23 Zeilen entfernt(-)

diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
index 76d8ae8..af24d1e 100644
--- a/hw/alpha_dp264.c
+++ b/hw/alpha_dp264.c
@@ -50,7 +50,7 @@ static void clipper_init(QEMUMachineInitArgs *args)
     const char *kernel_filename = args->kernel_filename;
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
-    CPUAlphaState *cpus[4];
+    AlphaCPU *cpus[4];
     PCIBus *pci_bus;
     ISABus *isa_bus;
     qemu_irq rtc_irq;
@@ -62,12 +62,12 @@ static void clipper_init(QEMUMachineInitArgs *args)
     /* Create up to 4 cpus.  */
     memset(cpus, 0, sizeof(cpus));
     for (i = 0; i < smp_cpus; ++i) {
-        cpus[i] = cpu_init(cpu_model ? cpu_model : "ev67");
+        cpus[i] = cpu_alpha_init(cpu_model ? cpu_model : "ev67");
     }
 
-    cpus[0]->trap_arg0 = ram_size;
-    cpus[0]->trap_arg1 = 0;
-    cpus[0]->trap_arg2 = smp_cpus;
+    cpus[0]->env.trap_arg0 = ram_size;
+    cpus[0]->env.trap_arg1 = 0;
+    cpus[0]->env.trap_arg2 = smp_cpus;
 
     /* Init the chipset.  */
     pci_bus = typhoon_init(ram_size, &isa_bus, &rtc_irq, cpus,
@@ -119,9 +119,9 @@ static void clipper_init(QEMUMachineInitArgs *args)
 
     /* Start all cpus at the PALcode RESET entry point.  */
     for (i = 0; i < smp_cpus; ++i) {
-        cpus[i]->pal_mode = 1;
-        cpus[i]->pc = palcode_entry;
-        cpus[i]->palbr = palcode_entry;
+        cpus[i]->env.pal_mode = 1;
+        cpus[i]->env.pc = palcode_entry;
+        cpus[i]->env.palbr = palcode_entry;
     }
 
     /* Load a kernel.  */
@@ -136,7 +136,7 @@ static void clipper_init(QEMUMachineInitArgs *args)
             exit(1);
         }
 
-        cpus[0]->trap_arg1 = kernel_entry;
+        cpus[0]->env.trap_arg1 = kernel_entry;
 
         param_offset = kernel_low - 0x6000;
 
diff --git a/hw/alpha_sys.h b/hw/alpha_sys.h
index 7604d09..69929ea 100644
--- a/hw/alpha_sys.h
+++ b/hw/alpha_sys.h
@@ -11,7 +11,7 @@
 #include "irq.h"
 
 
-PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, CPUAlphaState *[4],
+PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, AlphaCPU *[4],
                      pci_map_irq_fn);
 
 /* alpha_pci.c.  */
diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
index 9b16d96..4cc810f 100644
--- a/hw/alpha_typhoon.c
+++ b/hw/alpha_typhoon.c
@@ -23,7 +23,7 @@ typedef struct TyphoonCchip {
     uint64_t drir;
     uint64_t dim[4];
     uint32_t iic[4];
-    CPUAlphaState *cpu[4];
+    AlphaCPU *cpu[4];
 } TyphoonCchip;
 
 typedef struct TyphoonWindow {
@@ -58,10 +58,11 @@ typedef struct TyphoonState {
 } TyphoonState;
 
 /* Called when one of DRIR or DIM changes.  */
-static void cpu_irq_change(CPUAlphaState *env, uint64_t req)
+static void cpu_irq_change(AlphaCPU *cpu, uint64_t req)
 {
     /* If there are any non-masked interrupts, tell the cpu.  */
-    if (env) {
+    if (cpu != NULL) {
+        CPUAlphaState *env = &cpu->env;
         if (req) {
             cpu_interrupt(env, CPU_INTERRUPT_HARD);
         } else {
@@ -353,8 +354,9 @@ static void cchip_write(void *opaque, hwaddr addr,
         if ((newval ^ oldval) & 0xff0) {
             int i;
             for (i = 0; i < 4; ++i) {
-                CPUAlphaState *env = s->cchip.cpu[i];
-                if (env) {
+                AlphaCPU *cpu = s->cchip.cpu[i];
+                if (cpu != NULL) {
+                    CPUAlphaState *env = &cpu->env;
                     /* IPI can be either cleared or set by the write.  */
                     if (newval & (1 << (i + 8))) {
                         cpu_interrupt(env, CPU_INTERRUPT_SMP);
@@ -661,8 +663,8 @@ static void typhoon_set_timer_irq(void *opaque, int irq, int level)
 
     /* Deliver the interrupt to each CPU, considering each CPU's IIC.  */
     for (i = 0; i < 4; ++i) {
-        CPUAlphaState *env = s->cchip.cpu[i];
-        if (env) {
+        AlphaCPU *cpu = s->cchip.cpu[i];
+        if (cpu != NULL) {
             uint32_t iic = s->cchip.iic[i];
 
             /* ??? The verbage in Section 10.2.2.10 isn't 100% clear.
@@ -681,7 +683,7 @@ static void typhoon_set_timer_irq(void *opaque, int irq, int level)
                 /* Set the ITI bit for this cpu.  */
                 s->cchip.misc |= 1 << (i + 4);
                 /* And signal the interrupt.  */
-                cpu_interrupt(env, CPU_INTERRUPT_TIMER);
+                cpu_interrupt(&cpu->env, CPU_INTERRUPT_TIMER);
             }
         }
     }
@@ -694,12 +696,12 @@ static void typhoon_alarm_timer(void *opaque)
 
     /* Set the ITI bit for this cpu.  */
     s->cchip.misc |= 1 << (cpu + 4);
-    cpu_interrupt(s->cchip.cpu[cpu], CPU_INTERRUPT_TIMER);
+    cpu_interrupt(&s->cchip.cpu[cpu]->env, CPU_INTERRUPT_TIMER);
 }
 
 PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
                      qemu_irq *p_rtc_irq,
-                     CPUAlphaState *cpus[4], pci_map_irq_fn sys_map_irq)
+                     AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq)
 {
     const uint64_t MB = 1024 * 1024;
     const uint64_t GB = 1024 * MB;
@@ -719,9 +721,10 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
 
     /* Remember the CPUs so that we can deliver interrupts to them.  */
     for (i = 0; i < 4; i++) {
-        CPUAlphaState *env = cpus[i];
-        s->cchip.cpu[i] = env;
-        if (env) {
+        AlphaCPU *cpu = cpus[i];
+        s->cchip.cpu[i] = cpu;
+        if (cpu != NULL) {
+            CPUAlphaState *env = &cpu->env;
             env->alarm_timer = qemu_new_timer_ns(rtc_clock,
                                                  typhoon_alarm_timer,
                                                  (void *)((uintptr_t)s + i));
-- 
1.7.10.4

[Qemu-devel] [PATCH 6/7] target-alpha: Avoid leaking the alarm timer over reset

[Andreas Färber]

Move the timer from CPUAlphaState to AlphaCPU to avoid the pointer being
zero'ed once we implement reset. Would cause a segfault in
sys_helper.c:helper_set_alarm().

This also simplifies timer initialization in Typhoon.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/alpha_typhoon.c        |    3 +--
 target-alpha/cpu-qom.h    |    3 +++
 target-alpha/cpu.h        |    1 -
 target-alpha/sys_helper.c |    6 ++++--
 4 Dateien geändert, 8 Zeilen hinzugefügt(+), 5 Zeilen entfernt(-)

diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
index 4cc810f..40b3a47 100644
--- a/hw/alpha_typhoon.c
+++ b/hw/alpha_typhoon.c
@@ -724,8 +724,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
         AlphaCPU *cpu = cpus[i];
         s->cchip.cpu[i] = cpu;
         if (cpu != NULL) {
-            CPUAlphaState *env = &cpu->env;
-            env->alarm_timer = qemu_new_timer_ns(rtc_clock,
+            cpu->alarm_timer = qemu_new_timer_ns(rtc_clock,
                                                  typhoon_alarm_timer,
                                                  (void *)((uintptr_t)s + i));
         }
diff --git a/target-alpha/cpu-qom.h b/target-alpha/cpu-qom.h
index 6b4ca6d..98585d5 100644
--- a/target-alpha/cpu-qom.h
+++ b/target-alpha/cpu-qom.h
@@ -58,6 +58,9 @@ typedef struct AlphaCPU {
     /*< public >*/
 
     CPUAlphaState env;
+
+    /* This alarm doesn't exist in real hardware; we wish it did.  */
+    struct QEMUTimer *alarm_timer;
 } AlphaCPU;
 
 static inline AlphaCPU *alpha_env_get_cpu(CPUAlphaState *env)
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 2bbde9c..fbfafe6 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -277,7 +277,6 @@ struct CPUAlphaState {
 #endif
 
     /* This alarm doesn't exist in real hardware; we wish it did.  */
-    struct QEMUTimer *alarm_timer;
     uint64_t alarm_expire;
 
 #if TARGET_LONG_BITS > HOST_LONG_BITS
diff --git a/target-alpha/sys_helper.c b/target-alpha/sys_helper.c
index 40ca49c..d4f14ef 100644
--- a/target-alpha/sys_helper.c
+++ b/target-alpha/sys_helper.c
@@ -77,11 +77,13 @@ uint64_t helper_get_time(void)
 
 void helper_set_alarm(CPUAlphaState *env, uint64_t expire)
 {
+    AlphaCPU *cpu = alpha_env_get_cpu(env);
+
     if (expire) {
         env->alarm_expire = expire;
-        qemu_mod_timer(env->alarm_timer, expire);
+        qemu_mod_timer(cpu->alarm_timer, expire);
     } else {
-        qemu_del_timer(env->alarm_timer);
+        qemu_del_timer(cpu->alarm_timer);
     }
 }
 #endif /* CONFIG_USER_ONLY */
-- 
1.7.10.4

[Qemu-devel] [RFC 7/7] target-alpha: Implement CPU reset

[Andreas Färber]

The parent_reset class field was already prepared but unused so far.

No guarantees that this actually does The Right Thing, more fields
may need to be moved within CPUAlphaState or to AlphaCPU.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-alpha/cpu.c |   28 ++++++++++++++++++++++++++++
 1 Datei geändert, 28 Zeilen hinzugefügt(+)

diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index ab25c44..d798390 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -23,6 +23,24 @@
 #include "qemu-common.h"
 
 
+/* CPUClass::reset() */
+static void alpha_cpu_reset(CPUState *s)
+{
+    AlphaCPU *cpu = ALPHA_CPU(s);
+    AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(cpu);
+    CPUAlphaState *env = &cpu->env;
+
+    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
+        qemu_log("CPU Reset (CPU %d)\n", env->cpu_index);
+        log_cpu_state(env, 0);
+    }
+
+    acc->parent_reset(s);
+
+    memset(env, 0, offsetof(CPUAlphaState, breakpoints));
+    tlb_flush(env, 1);
+}
+
 typedef struct AlphaCPUListState {
     fprintf_function cpu_fprintf;
     FILE *file;
@@ -221,6 +239,15 @@ static void alpha_cpu_initfn(Object *obj)
     env->fen = 1;
 }
 
+static void alpha_cpu_class_init(ObjectClass *oc, void *data)
+{
+    CPUClass *cc = CPU_CLASS(oc);
+    AlphaCPUClass *acc = ALPHA_CPU_CLASS(oc);
+
+    acc->parent_reset = cc->reset;
+    cc->reset = alpha_cpu_reset;
+}
+
 static void alpha_cpu_register(const AlphaCPUInfo *info)
 {
     TypeInfo type_info = {
@@ -239,6 +266,7 @@ static const TypeInfo alpha_cpu_type_info = {
     .instance_init = alpha_cpu_initfn,
     .abstract = true,
     .class_size = sizeof(AlphaCPUClass),
+    .class_init = alpha_cpu_class_init,
 };
 
 static void alpha_cpu_register_types(void)
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 2/7] target-alpha: Turn CPU definitions into subclasses

[Richard Henderson]

On 2012-10-31 14:03, Andreas Färber wrote:
> +static const AlphaCPUInfo alpha_cpus[] = {
> +    { .name = "ev4",     .initfn = ev4_cpu_initfn },
> +    { .name = "ev5",     .initfn = ev5_cpu_initfn },
> +    { .name = "ev56",    .initfn = ev56_cpu_initfn },
> +    { .name = "pca56",   .initfn = pca56_cpu_initfn },
> +    { .name = "ev6",     .initfn = ev6_cpu_initfn },
> +    { .name = "ev67",    .initfn = ev67_cpu_initfn },
> +    { .name = "ev68",    .initfn = ev68_cpu_initfn },
> +    { .name = "21064",   .initfn = alpha_21064_cpu_initfn },
> +    { .name = "21164",   .initfn = alpha_21164_cpu_initfn },
> +    { .name = "21164a",  .initfn = alpha_21164a_cpu_initfn },
> +    { .name = "21164pc", .initfn = alpha_21164pc_cpu_initfn },
> +    { .name = "21264",   .initfn = alpha_21264_cpu_initfn },
> +    { .name = "21264a",  .initfn = alpha_21264a_cpu_initfn },
> +};

The "2*" names are aliases of the "ev*" names.  There's no need for so
much duplication.  And for that matter, "ev68" is no different from "ev67"
at the level for which we emulate.  In hw, it was more cache and a faster
multiply implementation.


r~

Re: [Qemu-devel] [PATCH 3/7] target-alpha: Add support for -cpu ?

[Richard Henderson]

On 2012-10-31 14:04, Andreas Färber wrote:
> Implement alphabetical listing of CPU subclasses.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

There doesn't seem to be anything alpha-specific about this.
Does it really need to be replicated?  That said,

Acked-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 4/7] target-alpha: Let cpu_alpha_init() return AlphaCPU

[Richard Henderson]

On 2012-10-31 14:04, Andreas Färber wrote:
> Replace cpu_init() macro with inline function for backwards
> compatibility.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-alpha/cpu.h       |   13 +++++++++++--
>  target-alpha/translate.c |    4 ++--

Acked-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 5/7] alpha: Pass AlphaCPU array to Typhoon

[Richard Henderson]

On 2012-10-31 14:04, Andreas Färber wrote:
> Also store it in TyphoonCchip.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/alpha_dp264.c   |   18 +++++++++---------
>  hw/alpha_sys.h     |    2 +-
>  hw/alpha_typhoon.c |   29 ++++++++++++++++-------------

Acked-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 6/7] target-alpha: Avoid leaking the alarm timer over reset

[Richard Henderson]

On 2012-10-31 14:04, Andreas Färber wrote:
> Move the timer from CPUAlphaState to AlphaCPU to avoid the pointer being
> zero'ed once we implement reset. Would cause a segfault in
> sys_helper.c:helper_set_alarm().
> 
> This also simplifies timer initialization in Typhoon.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Acked-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [RFC 7/7] target-alpha: Implement CPU reset

[Richard Henderson]

On 2012-10-31 14:04, Andreas Färber wrote:
> +/* CPUClass::reset() */
> +static void alpha_cpu_reset(CPUState *s)
> +{
> +    AlphaCPU *cpu = ALPHA_CPU(s);
> +    AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(cpu);
> +    CPUAlphaState *env = &cpu->env;
> +
> +    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
> +        qemu_log("CPU Reset (CPU %d)\n", env->cpu_index);
> +        log_cpu_state(env, 0);
> +    }
> +
> +    acc->parent_reset(s);
> +
> +    memset(env, 0, offsetof(CPUAlphaState, breakpoints));
> +    tlb_flush(env, 1);
> +}

Does this somehow reset the alarm_timer as well?  I'd sort of known about its
unfortunate position and resetting problems before, but I'd been ignoring it.

As for whether this works... I'd be surprised if the bios I wrote needs nothing
else for it to handle reset itself...


r~

Re: [Qemu-devel] [PATCH 2/7] target-alpha: Turn CPU definitions into subclasses

[Andreas Färber]

Am 31.10.2012 05:57, schrieb Richard Henderson:
> On 2012-10-31 14:03, Andreas Färber wrote:
>> +static const AlphaCPUInfo alpha_cpus[] = {
>> +    { .name = "ev4",     .initfn = ev4_cpu_initfn },
>> +    { .name = "ev5",     .initfn = ev5_cpu_initfn },
>> +    { .name = "ev56",    .initfn = ev56_cpu_initfn },
>> +    { .name = "pca56",   .initfn = pca56_cpu_initfn },
>> +    { .name = "ev6",     .initfn = ev6_cpu_initfn },
>> +    { .name = "ev67",    .initfn = ev67_cpu_initfn },
>> +    { .name = "ev68",    .initfn = ev68_cpu_initfn },
>> +    { .name = "21064",   .initfn = alpha_21064_cpu_initfn },
>> +    { .name = "21164",   .initfn = alpha_21164_cpu_initfn },
>> +    { .name = "21164a",  .initfn = alpha_21164a_cpu_initfn },
>> +    { .name = "21164pc", .initfn = alpha_21164pc_cpu_initfn },
>> +    { .name = "21264",   .initfn = alpha_21264_cpu_initfn },
>> +    { .name = "21264a",  .initfn = alpha_21264a_cpu_initfn },
>> +};
> 
> The "2*" names are aliases of the "ev*" names.  There's no need for so
> much duplication.  And for that matter, "ev68" is no different from "ev67"
> at the level for which we emulate.  In hw, it was more cache and a faster
> multiply implementation.

Clearly I know little to nothing about Alpha CPU models. :)
Regarding ev68, we'll need to carry it for backwards compatibility; can
we assume that the Alpha ISA is dead? Then I could drop this shrinking
array and make, e.g., ev68 a trivial subclass of ev67.

The name scheme we are heading towards now looks like <name>-alpha-cpu.
Did I understand you correctly that we would want, e.g., ev4-alpha-cpu
as type and have "21064" map to it? Or the other way around?

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 2/7] target-alpha: Turn CPU definitions into subclasses

[Eduardo Habkost]

On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote:
[...]
> +static void alpha_cpu_register(const AlphaCPUInfo *info)
> +{
> +    TypeInfo type_info = {
> +        .name = info->name,
> +        .parent = TYPE_ALPHA_CPU,
> +        .instance_init = info->initfn,
> +    };
> +
> +    type_register_static(&type_info);

You should use type_register() instead of type_register_static(), here.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/7] target-alpha: Add support for -cpu ?

[Eduardo Habkost]

On Wed, Oct 31, 2012 at 04:04:00AM +0100, Andreas Färber wrote:
> Implement alphabetical listing of CPU subclasses.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-alpha/cpu.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  target-alpha/cpu.h |    4 +++-
>  2 Dateien geändert, 44 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
> 
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index e1a5739..ab25c44 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -23,6 +23,47 @@
>  #include "qemu-common.h"
>  
>  
> +typedef struct AlphaCPUListState {
> +    fprintf_function cpu_fprintf;
> +    FILE *file;
> +} AlphaCPUListState;
> +
> +/* Sort alphabetically by type name. */
> +static gint alpha_cpu_list_compare(gconstpointer a, gconstpointer b)
> +{
> +    ObjectClass *class_a = (ObjectClass *)a;
> +    ObjectClass *class_b = (ObjectClass *)b;
> +    const char *name_a, *name_b;
> +
> +    name_a = object_class_get_name(class_a);
> +    name_b = object_class_get_name(class_b);
> +    return strcmp(name_a, name_b);
> +}
> +
> +static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
> +{
> +    ObjectClass *oc = data;
> +    AlphaCPUListState *s = user_data;
> +
> +    (*s->cpu_fprintf)(s->file, "  %s\n",
> +                      object_class_get_name(oc));
> +}
> +
> +void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> +{
> +    AlphaCPUListState s = {
> +        .file = f,
> +        .cpu_fprintf = cpu_fprintf,
> +    };
> +    GSList *list;
> +
> +    list = object_class_get_list(TYPE_ALPHA_CPU, false);
> +    list = g_slist_sort(list, alpha_cpu_list_compare);
> +    (*cpu_fprintf)(f, "Available CPUs:\n");
> +    g_slist_foreach(list, alpha_cpu_list_entry, &s);
> +    g_slist_free(list);
> +}

target-arm has very similar code. Isn't it better to first write a
common reusable function to list CPU models using the list of
subclasses, instead of adding very similar functions to all
architectures?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/7] target-alpha: Add support for -cpu ?

[Andreas Färber]

Am 06.12.2012 16:37, schrieb Eduardo Habkost:
> On Wed, Oct 31, 2012 at 04:04:00AM +0100, Andreas Färber wrote:
>> Implement alphabetical listing of CPU subclasses.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  target-alpha/cpu.c |   41 +++++++++++++++++++++++++++++++++++++++++
>>  target-alpha/cpu.h |    4 +++-
>>  2 Dateien geändert, 44 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
>>
>> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
>> index e1a5739..ab25c44 100644
>> --- a/target-alpha/cpu.c
>> +++ b/target-alpha/cpu.c
>> @@ -23,6 +23,47 @@
>>  #include "qemu-common.h"
>>  
>>  
>> +typedef struct AlphaCPUListState {
>> +    fprintf_function cpu_fprintf;
>> +    FILE *file;
>> +} AlphaCPUListState;
>> +
>> +/* Sort alphabetically by type name. */
>> +static gint alpha_cpu_list_compare(gconstpointer a, gconstpointer b)
>> +{
>> +    ObjectClass *class_a = (ObjectClass *)a;
>> +    ObjectClass *class_b = (ObjectClass *)b;
>> +    const char *name_a, *name_b;
>> +
>> +    name_a = object_class_get_name(class_a);
>> +    name_b = object_class_get_name(class_b);
>> +    return strcmp(name_a, name_b);
>> +}
>> +
>> +static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
>> +{
>> +    ObjectClass *oc = data;
>> +    AlphaCPUListState *s = user_data;
>> +
>> +    (*s->cpu_fprintf)(s->file, "  %s\n",
>> +                      object_class_get_name(oc));
>> +}
>> +
>> +void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>> +{
>> +    AlphaCPUListState s = {
>> +        .file = f,
>> +        .cpu_fprintf = cpu_fprintf,
>> +    };
>> +    GSList *list;
>> +
>> +    list = object_class_get_list(TYPE_ALPHA_CPU, false);
>> +    list = g_slist_sort(list, alpha_cpu_list_compare);
>> +    (*cpu_fprintf)(f, "Available CPUs:\n");
>> +    g_slist_foreach(list, alpha_cpu_list_entry, &s);
>> +    g_slist_free(list);
>> +}
> 
> target-arm has very similar code. Isn't it better to first write a
> common reusable function to list CPU models using the list of
> subclasses, instead of adding very similar functions to all
> architectures?

Most ordering functions vary slightly (target-arm for "any"). It would
be possible to generalize the struct and provide a wrapper with type and
callback arguments, but then again some functions add a header line like
here, some don't, and some even hardcode some options like "host". For
the targets that already had -cpu ? support before QOM I tried to keep
output identical apart from possibly the order.

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 2/7] target-alpha: Turn CPU definitions into subclasses

[Andreas Färber]

Am 06.12.2012 16:29, schrieb Eduardo Habkost:
> On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote:
> [...]
>> +static void alpha_cpu_register(const AlphaCPUInfo *info)
>> +{
>> +    TypeInfo type_info = {
>> +        .name = info->name,
>> +        .parent = TYPE_ALPHA_CPU,
>> +        .instance_init = info->initfn,
>> +    };
>> +
>> +    type_register_static(&type_info);
> 
> You should use type_register() instead of type_register_static(), here.

I still don't understand why. (CC'ing Anthony, Paolo, Peter)

The TypeInfo argument is in no way retained inside
qom/object.c:type_register_internal().
Therefore the lifetime of TypeInfo should be completely irrelevant for
the static/non-static decision and the documentation should be fixed IMO.
Is there a reason to do it differently? What would we want to do with
TypeInfo after transfer of its field values to TypeImpl?

FWIW if, as suggested earlier, we don't loop over Alpha CPU models in
favor of a handful of trivial static TypeInfos, it becomes irrelevant
for this patch but I'd still like to understand for the remaining
architectures.

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 3/7] target-alpha: Add support for -cpu ?

[Peter Maydell]

On 6 December 2012 15:37, Eduardo Habkost <ehabkost@redhat.com> wrote:
> target-arm has very similar code. Isn't it better to first write a
> common reusable function to list CPU models using the list of
> subclasses, instead of adding very similar functions to all
> architectures?

What would be particularly useful to have as a common
utility routine is support for x86-style +feature,-feature
syntax. At the moment we don't implement that on most
other targets but it would be good to have that on ARM
at some point.

-- PMM

Re: [Qemu-devel] [PATCH 3/7] target-alpha: Add support for -cpu ?

[Andreas Färber]

Am 06.12.2012 16:42, schrieb Andreas Färber:
> Am 06.12.2012 16:37, schrieb Eduardo Habkost:
>> On Wed, Oct 31, 2012 at 04:04:00AM +0100, Andreas Färber wrote:
>>> Implement alphabetical listing of CPU subclasses.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>>  target-alpha/cpu.c |   41 +++++++++++++++++++++++++++++++++++++++++
>>>  target-alpha/cpu.h |    4 +++-
>>>  2 Dateien geändert, 44 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
>>>
>>> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
>>> index e1a5739..ab25c44 100644
>>> --- a/target-alpha/cpu.c
>>> +++ b/target-alpha/cpu.c
>>> @@ -23,6 +23,47 @@
>>>  #include "qemu-common.h"
>>>  
>>>  
>>> +typedef struct AlphaCPUListState {
>>> +    fprintf_function cpu_fprintf;
>>> +    FILE *file;
>>> +} AlphaCPUListState;
>>> +
>>> +/* Sort alphabetically by type name. */
>>> +static gint alpha_cpu_list_compare(gconstpointer a, gconstpointer b)
>>> +{
>>> +    ObjectClass *class_a = (ObjectClass *)a;
>>> +    ObjectClass *class_b = (ObjectClass *)b;
>>> +    const char *name_a, *name_b;
>>> +
>>> +    name_a = object_class_get_name(class_a);
>>> +    name_b = object_class_get_name(class_b);
>>> +    return strcmp(name_a, name_b);
>>> +}
>>> +
>>> +static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
>>> +{
>>> +    ObjectClass *oc = data;
>>> +    AlphaCPUListState *s = user_data;
>>> +
>>> +    (*s->cpu_fprintf)(s->file, "  %s\n",
>>> +                      object_class_get_name(oc));
>>> +}
>>> +
>>> +void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>>> +{
>>> +    AlphaCPUListState s = {
>>> +        .file = f,
>>> +        .cpu_fprintf = cpu_fprintf,
>>> +    };
>>> +    GSList *list;
>>> +
>>> +    list = object_class_get_list(TYPE_ALPHA_CPU, false);
>>> +    list = g_slist_sort(list, alpha_cpu_list_compare);
>>> +    (*cpu_fprintf)(f, "Available CPUs:\n");
>>> +    g_slist_foreach(list, alpha_cpu_list_entry, &s);
>>> +    g_slist_free(list);
>>> +}
>>
>> target-arm has very similar code. Isn't it better to first write a
>> common reusable function to list CPU models using the list of
>> subclasses, instead of adding very similar functions to all
>> architectures?
> 
> Most ordering functions vary slightly (target-arm for "any"). It would
> be possible to generalize the struct and provide a wrapper with type and
> callback arguments,

Just remembered Anthony being against callbacks in this context:

http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02944.html

The RFC was for specifically for implementing the CPU lists. So I used
g_slist_* instead as suggested, which duplicates a few lines FWIW.
If someone has suggestions how else to share more code, I'm all ears.

Andreas

> but then again some functions add a header line like
> here, some don't, and some even hardcode some options like "host". For
> the targets that already had -cpu ? support before QOM I tried to keep
> output identical apart from possibly the order.
> 
> 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 2/7] target-alpha: Turn CPU definitions into subclasses

[Eduardo Habkost]

On Thu, Dec 06, 2012 at 04:51:31PM +0100, Andreas Färber wrote:
> Am 06.12.2012 16:29, schrieb Eduardo Habkost:
> > On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote:
> > [...]
> >> +static void alpha_cpu_register(const AlphaCPUInfo *info)
> >> +{
> >> +    TypeInfo type_info = {
> >> +        .name = info->name,
> >> +        .parent = TYPE_ALPHA_CPU,
> >> +        .instance_init = info->initfn,
> >> +    };
> >> +
> >> +    type_register_static(&type_info);
> > 
> > You should use type_register() instead of type_register_static(), here.
> 
> I still don't understand why. (CC'ing Anthony, Paolo, Peter)
> 
> The TypeInfo argument is in no way retained inside
> qom/object.c:type_register_internal().
> Therefore the lifetime of TypeInfo should be completely irrelevant for
> the static/non-static decision and the documentation should be fixed IMO.
> Is there a reason to do it differently? What would we want to do with
> TypeInfo after transfer of its field values to TypeImpl?

The current implementation doesn't matter. It can change at any minute. The
interface, on the other hand, is documented as:

  type_register_static:
  @info: The #TypeInfo of the new type.

  @info and all of the strings it points to should exist for the life time
  that the type is registered.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/7] target-alpha: Add support for -cpu ?

[Eduardo Habkost]

On Thu, Dec 06, 2012 at 03:59:46PM +0000, Peter Maydell wrote:
> On 6 December 2012 15:37, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > target-arm has very similar code. Isn't it better to first write a
> > common reusable function to list CPU models using the list of
> > subclasses, instead of adding very similar functions to all
> > architectures?
> 
> What would be particularly useful to have as a common
> utility routine is support for x86-style +feature,-feature
> syntax. At the moment we don't implement that on most
> other targets but it would be good to have that on ARM
> at some point.

Once we finish the work on x86, I plan to make the code reusable by
other architectures.

That would include the +feature,-feature parsing and the CPU model ->
CPU class lookup (this one may be more complicated to make reusable, but
I think it's doable).

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/7] target-alpha: Turn CPU definitions into subclasses

[Andreas Färber]

Am 06.12.2012 17:09, schrieb Eduardo Habkost:
> On Thu, Dec 06, 2012 at 04:51:31PM +0100, Andreas Färber wrote:
>> Am 06.12.2012 16:29, schrieb Eduardo Habkost:
>>> On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote:
>>> [...]
>>>> +static void alpha_cpu_register(const AlphaCPUInfo *info)
>>>> +{
>>>> +    TypeInfo type_info = {
>>>> +        .name = info->name,
>>>> +        .parent = TYPE_ALPHA_CPU,
>>>> +        .instance_init = info->initfn,
>>>> +    };
>>>> +
>>>> +    type_register_static(&type_info);
>>>
>>> You should use type_register() instead of type_register_static(), here.
>>
>> I still don't understand why. (CC'ing Anthony, Paolo, Peter)
>>
>> The TypeInfo argument is in no way retained inside
>> qom/object.c:type_register_internal().
>> Therefore the lifetime of TypeInfo should be completely irrelevant for
>> the static/non-static decision and the documentation should be fixed IMO.
>> Is there a reason to do it differently? What would we want to do with
>> TypeInfo after transfer of its field values to TypeImpl?
> 
> The current implementation doesn't matter. It can change at any minute. The
> interface, on the other hand, is documented as:
> 
>   type_register_static:
>   @info: The #TypeInfo of the new type.
> 
>   @info and all of the strings it points to should exist for the life time
>   that the type is registered.

Both implementation and documentation can be changed. My question is,
why does the documentation say this and where does Anthony (or Paolo)
want to go with the current implementation that makes this necessary.

Like, if we switched to C++, we would drop both registration functions
completely.

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 3/7] target-alpha: Add support for -cpu ?

[Eduardo Habkost]

On Thu, Dec 06, 2012 at 05:02:52PM +0100, Andreas Färber wrote:
> Am 06.12.2012 16:42, schrieb Andreas Färber:
> > Am 06.12.2012 16:37, schrieb Eduardo Habkost:
> >> On Wed, Oct 31, 2012 at 04:04:00AM +0100, Andreas Färber wrote:
> >>> Implement alphabetical listing of CPU subclasses.
> >>>
> >>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >>> ---
> >>>  target-alpha/cpu.c |   41 +++++++++++++++++++++++++++++++++++++++++
> >>>  target-alpha/cpu.h |    4 +++-
> >>>  2 Dateien geändert, 44 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
> >>>
> >>> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> >>> index e1a5739..ab25c44 100644
> >>> --- a/target-alpha/cpu.c
> >>> +++ b/target-alpha/cpu.c
> >>> @@ -23,6 +23,47 @@
> >>>  #include "qemu-common.h"
> >>>  
> >>>  
> >>> +typedef struct AlphaCPUListState {
> >>> +    fprintf_function cpu_fprintf;
> >>> +    FILE *file;
> >>> +} AlphaCPUListState;
> >>> +
> >>> +/* Sort alphabetically by type name. */
> >>> +static gint alpha_cpu_list_compare(gconstpointer a, gconstpointer b)
> >>> +{
> >>> +    ObjectClass *class_a = (ObjectClass *)a;
> >>> +    ObjectClass *class_b = (ObjectClass *)b;
> >>> +    const char *name_a, *name_b;
> >>> +
> >>> +    name_a = object_class_get_name(class_a);
> >>> +    name_b = object_class_get_name(class_b);
> >>> +    return strcmp(name_a, name_b);
> >>> +}
> >>> +
> >>> +static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
> >>> +{
> >>> +    ObjectClass *oc = data;
> >>> +    AlphaCPUListState *s = user_data;
> >>> +
> >>> +    (*s->cpu_fprintf)(s->file, "  %s\n",
> >>> +                      object_class_get_name(oc));
> >>> +}
> >>> +
> >>> +void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> >>> +{
> >>> +    AlphaCPUListState s = {
> >>> +        .file = f,
> >>> +        .cpu_fprintf = cpu_fprintf,
> >>> +    };
> >>> +    GSList *list;
> >>> +
> >>> +    list = object_class_get_list(TYPE_ALPHA_CPU, false);
> >>> +    list = g_slist_sort(list, alpha_cpu_list_compare);
> >>> +    (*cpu_fprintf)(f, "Available CPUs:\n");
> >>> +    g_slist_foreach(list, alpha_cpu_list_entry, &s);
> >>> +    g_slist_free(list);
> >>> +}
> >>
> >> target-arm has very similar code. Isn't it better to first write a
> >> common reusable function to list CPU models using the list of
> >> subclasses, instead of adding very similar functions to all
> >> architectures?
> > 
> > Most ordering functions vary slightly (target-arm for "any"). It would
> > be possible to generalize the struct and provide a wrapper with type and
> > callback arguments,
> 
> Just remembered Anthony being against callbacks in this context:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02944.html
> 
> The RFC was for specifically for implementing the CPU lists. So I used
> g_slist_* instead as suggested, which duplicates a few lines FWIW.
> If someone has suggestions how else to share more code, I'm all ears.

We could simply reuse the existing arch_query_cpu_definitions() interface to
implement cpu_list(), and the target-specific arch_query_cpu_definitions()
could reorder the list any way it wants. The list could then be used for both
cpu_list() and the the QMP query-cpu-definitions command.

If necessary, we can add a "description" field to CpuDefinitionInfo, so targets
can optionally return a description of each CPU model, too (that's the case for
the current x86 cpu_list() output).

 
> Andreas
> 
> > but then again some functions add a header line like
> > here, some don't, and some even hardcode some options like "host". For
> > the targets that already had -cpu ? support before QOM I tried to keep
> > output identical apart from possibly the order.
> > 
> > Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/7] target-alpha: Turn CPU definitions into subclasses

[Richard Henderson]

On 2012-12-06 04:11, Andreas Färber wrote:
>> The "2*" names are aliases of the "ev*" names.  There's no need for so
>> much duplication.  And for that matter, "ev68" is no different from "ev67"
>> at the level for which we emulate.  In hw, it was more cache and a faster
>> multiply implementation.
> 
> Clearly I know little to nothing about Alpha CPU models. :)
> Regarding ev68, we'll need to carry it for backwards compatibility; can
> we assume that the Alpha ISA is dead? Then I could drop this shrinking
> array and make, e.g., ev68 a trivial subclass of ev67.

Yes, the ISA is dead.  We're also nearing the end of "extended" hardware
support for Alpha systems from HP.

> The name scheme we are heading towards now looks like <name>-alpha-cpu.
> Did I understand you correctly that we would want, e.g., ev4-alpha-cpu
> as type and have "21064" map to it?

Correct.  That corresponds more closely to how the compiler tools are
configured (e.g. alphaev67-linux, not alpha21164-linux).


r~