[PATCH qom-cpu 3/4] target-ppc: Slim conversion of model definitions to QOM subclasses

[PATCH qom-cpu 3/4] target-ppc: Slim conversion of model definitions to QOM subclasses

[Andreas Färber]

Since the model list is highly macrofied, keep ppc_def_t for now and
save a pointer to it in PowerPCCPUClass. This results in a flat list of
subclasses including aliases, to be refined later.

Move cpu_ppc_init() to translate_init.c and drop helper.c.
Long-term the idea is to turn translate_init.c into a standalone cpu.c.

Inline cpu_ppc_usable() into type registration.

Split cpu_ppc_register() in two by code movement into the initfn and
by turning the remaining part into a realizefn.
Move qemu_init_vcpu() call into the new realizefn and adapt
create_ppc_opcodes() to return an Error.

Change ppc_find_by_pvr() -> ppc_cpu_class_by_pvr().
Change ppc_find_by_name() -> ppc_cpu_class_by_name().

Turn -cpu host into its own subclass. This requires to move the
kvm_enabled() check in ppc_cpu_class_by_name() to avoid the class being
found via the normal name lookup in the !kvm_enabled() case.
Turn kvmppc_host_cpu_def() into the class_init and add an initfn that
asserts KVM is in fact enabled.

Implement -cpu ? and the QMP equivalent in terms of subclasses.
This newly exposes -cpu host to the user, ordered last for -cpu ?.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-ppc/Makefile.objs    |    3 +-
 target-ppc/cpu-qom.h        |    5 +
 target-ppc/cpu.h            |    4 -
 target-ppc/helper.c         |   50 -------
 target-ppc/kvm.c            |   37 ++++-
 target-ppc/kvm_ppc.h        |    8 +-
 target-ppc/translate_init.c |  344 +++++++++++++++++++++++++++++--------------
 7 Dateien geändert, 275 Zeilen hinzugefügt(+), 176 Zeilen entfernt(-)
 delete mode 100644 target-ppc/helper.c

diff --git a/target-ppc/Makefile.objs b/target-ppc/Makefile.objs
index 237a0ed..15d2a3c 100644
--- a/target-ppc/Makefile.objs
+++ b/target-ppc/Makefile.objs
@@ -1,7 +1,6 @@
-obj-y += translate.o helper.o
+obj-y += translate.o
 obj-$(CONFIG_SOFTMMU) += machine.o
 obj-$(CONFIG_KVM) += kvm.o kvm_ppc.o
-obj-y += helper.o
 obj-y += excp_helper.o
 obj-y += fpu_helper.o
 obj-y += int_helper.o
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index fef6f95..f5c757f 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -50,6 +50,9 @@ typedef struct PowerPCCPUClass {
     /*< public >*/
 
     void (*parent_reset)(CPUState *cpu);
+
+    /* TODO inline fields here */
+    ppc_def_t *info;
 } PowerPCCPUClass;
 
 /**
@@ -73,5 +76,7 @@ static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
 
 #define ENV_GET_CPU(e) CPU(ppc_env_get_cpu(e))
 
+PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr);
+
 
 #endif
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 742d4f8..556080a 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1156,10 +1156,6 @@ void ppc_store_msr (CPUPPCState *env, target_ulong value);
 
 void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
 
-const ppc_def_t *ppc_find_by_pvr(uint32_t pvr);
-const ppc_def_t *cpu_ppc_find_by_name (const char *name);
-int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def);
-
 /* Time-base and decrementer management */
 #ifndef NO_CPU_IO_DEFS
 uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
deleted file mode 100644
index 48b19a7..0000000
--- a/target-ppc/helper.c
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- *  PowerPC emulation helpers for QEMU.
- *
- *  Copyright (c) 2003-2007 Jocelyn Mayer
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "cpu.h"
-#include "helper_regs.h"
-#include "kvm.h"
-#include "kvm_ppc.h"
-#include "cpus.h"
-
-PowerPCCPU *cpu_ppc_init(const char *cpu_model)
-{
-    PowerPCCPU *cpu;
-    CPUPPCState *env;
-    const ppc_def_t *def;
-
-    def = cpu_ppc_find_by_name(cpu_model);
-    if (!def) {
-        return NULL;
-    }
-
-    cpu = POWERPC_CPU(object_new(TYPE_POWERPC_CPU));
-    env = &cpu->env;
-
-    if (tcg_enabled()) {
-        ppc_translate_init();
-    }
-
-    env->cpu_model_str = cpu_model;
-    cpu_ppc_register_internal(env, def);
-
-    qemu_init_vcpu(env);
-
-    return cpu;
-}
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 3f5df57..f115892 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1184,18 +1184,29 @@ static void alter_insns(uint64_t *word, uint64_t flags, bool on)
     }
 }
 
-const ppc_def_t *kvmppc_host_cpu_def(void)
+static void kvmppc_host_cpu_initfn(Object *obj)
 {
+    assert(kvm_enabled());
+}
+
+static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
+{
+    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
     uint32_t host_pvr = mfpvr();
-    const ppc_def_t *base_spec;
+    PowerPCCPUClass *pvr_pcc;
     ppc_def_t *spec;
     uint32_t vmx = kvmppc_get_vmx();
     uint32_t dfp = kvmppc_get_dfp();
 
-    base_spec = ppc_find_by_pvr(host_pvr);
-
     spec = g_malloc0(sizeof(*spec));
-    memcpy(spec, base_spec, sizeof(*spec));
+
+    pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
+    if (pvr_pcc != NULL) {
+        memcpy(spec, pvr_pcc->info, sizeof(*spec));
+    }
+    pcc->info = spec;
+    /* Override the display name for -cpu ? and QMP */
+    pcc->info->name = "host";
 
     /* Now fix up the spec with information we can query from the host */
 
@@ -1208,8 +1219,6 @@ const ppc_def_t *kvmppc_host_cpu_def(void)
         /* Only override when we know what the host supports */
         alter_insns(&spec->insns_flags2, PPC2_DFP, dfp);
     }
-
-    return spec;
 }
 
 int kvmppc_fixup_cpu(CPUPPCState *env)
@@ -1239,3 +1248,17 @@ int kvm_arch_on_sigbus(int code, void *addr)
 {
     return 1;
 }
+
+static const TypeInfo kvm_host_cpu_type_info = {
+    .name = TYPE_HOST_POWERPC_CPU,
+    .parent = TYPE_POWERPC_CPU,
+    .instance_init = kvmppc_host_cpu_initfn,
+    .class_init = kvmppc_host_cpu_class_init,
+};
+
+static void kvm_ppc_register_types(void)
+{
+    type_register_static(&kvm_host_cpu_type_info);
+}
+
+type_init(kvm_ppc_register_types)
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index baad6eb..33d8cc2 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -11,6 +11,8 @@
 
 #include "memory.h"
 
+#define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
+
 void kvmppc_init(void);
 
 #ifdef CONFIG_KVM
@@ -30,7 +32,6 @@ int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
 int kvmppc_reset_htab(int shift_hint);
 uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);
 #endif /* !CONFIG_USER_ONLY */
-const ppc_def_t *kvmppc_host_cpu_def(void);
 int kvmppc_fixup_cpu(CPUPPCState *env);
 
 #else
@@ -115,11 +116,6 @@ static inline int kvmppc_update_sdr1(CPUPPCState *env)
 
 #endif /* !CONFIG_USER_ONLY */
 
-static inline const ppc_def_t *kvmppc_host_cpu_def(void)
-{
-    return NULL;
-}
-
 static inline int kvmppc_fixup_cpu(CPUPPCState *env)
 {
     return -1;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index e63627c..908a558 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9797,8 +9797,11 @@ static void fix_opcode_tables (opc_handler_t **ppc_opcodes)
 }
 
 /*****************************************************************************/
-static int create_ppc_opcodes (CPUPPCState *env, const ppc_def_t *def)
+static void create_ppc_opcodes(PowerPCCPU *cpu, Error **errp)
 {
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+    CPUPPCState *env = &cpu->env;
+    const ppc_def_t *def = pcc->info;
     opcode_t *opc;
 
     fill_new_table(env->opcodes, 0x40);
@@ -9806,18 +9809,16 @@ static int create_ppc_opcodes (CPUPPCState *env, const ppc_def_t *def)
         if (((opc->handler.type & def->insns_flags) != 0) ||
             ((opc->handler.type2 & def->insns_flags2) != 0)) {
             if (register_insn(env->opcodes, opc) < 0) {
-                printf("*** ERROR initializing PowerPC instruction "
-                       "0x%02x 0x%02x 0x%02x\n", opc->opc1, opc->opc2,
-                       opc->opc3);
-                return -1;
+                error_setg(errp, "ERROR initializing PowerPC instruction "
+                           "0x%02x 0x%02x 0x%02x\n", opc->opc1, opc->opc2,
+                           opc->opc3);
+                return;
             }
         }
     }
     fix_opcode_tables(env->opcodes);
     fflush(stdout);
     fflush(stderr);
-
-    return 0;
 }
 
 #if defined(PPC_DUMP_CPU)
@@ -10031,53 +10032,31 @@ static int ppc_fixup_cpu(CPUPPCState *env)
     return 0;
 }
 
-int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def)
+static void ppc_cpu_realize(Object *obj, Error **errp)
 {
-    env->msr_mask = def->msr_mask;
-    env->mmu_model = def->mmu_model;
-    env->excp_model = def->excp_model;
-    env->bus_model = def->bus_model;
-    env->insns_flags = def->insns_flags;
-    env->insns_flags2 = def->insns_flags2;
-    env->flags = def->flags;
-    env->bfd_mach = def->bfd_mach;
-    env->check_pow = def->check_pow;
-
-#if defined(TARGET_PPC64)
-    if (def->sps)
-        env->sps = *def->sps;
-    else if (env->mmu_model & POWERPC_MMU_64) {
-        /* Use default sets of page sizes */
-        static const struct ppc_segment_page_sizes defsps = {
-            .sps = {
-                { .page_shift = 12, /* 4K */
-                  .slb_enc = 0,
-                  .enc = { { .page_shift = 12, .pte_enc = 0 } }
-                },
-                { .page_shift = 24, /* 16M */
-                  .slb_enc = 0x100,
-                  .enc = { { .page_shift = 24, .pte_enc = 0 } }
-                },
-            },
-        };
-        env->sps = defsps;
-    }
-#endif /* defined(TARGET_PPC64) */
+    PowerPCCPU *cpu = POWERPC_CPU(obj);
+    CPUPPCState *env = &cpu->env;
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+    ppc_def_t *def = pcc->info;
+    Error *local_err = NULL;
 
     if (kvm_enabled()) {
         if (kvmppc_fixup_cpu(env) != 0) {
-            fprintf(stderr, "Unable to virtualize selected CPU with KVM\n");
-            exit(1);
+            error_setg(errp, "Unable to virtualize selected CPU with KVM");
+            return;
         }
     } else {
         if (ppc_fixup_cpu(env) != 0) {
-            fprintf(stderr, "Unable to emulate selected CPU with TCG\n");
-            exit(1);
+            error_setg(errp, "Unable to emulate selected CPU with TCG");
+            return;
         }
     }
 
-    if (create_ppc_opcodes(env, def) < 0)
-        return -1;
+    create_ppc_opcodes(cpu, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
     init_ppc_proc(env, def);
 
     if (def->insns_flags & PPC_FLOAT) {
@@ -10093,6 +10072,8 @@ int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def)
                                  34, "power-spe.xml", 0);
     }
 
+    qemu_init_vcpu(env);
+
 #if defined(PPC_DUMP_CPU)
     {
         const char *mmu_model, *excp_model, *bus_model;
@@ -10254,50 +10235,65 @@ int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def)
     dump_ppc_sprs(env);
     fflush(stdout);
 #endif
-
-    return 0;
 }
 
-static bool ppc_cpu_usable(const ppc_def_t *def)
+static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b)
 {
-#if defined(TARGET_PPCEMB)
-    /* When using the ppcemb target, we only support 440 style cores */
-    if (def->mmu_model != POWERPC_MMU_BOOKE) {
-        return false;
+    ObjectClass *oc = (ObjectClass *)a;
+    uint32_t pvr = *(uint32_t *)b;
+    PowerPCCPUClass *pcc = (PowerPCCPUClass *)a;
+
+    /* -cpu host does a PVR lookup during construction */
+    if (unlikely(strcmp(object_class_get_name(oc),
+                        TYPE_HOST_POWERPC_CPU) == 0)) {
+        return -1;
     }
-#endif
 
-    return true;
+    return pcc->info->pvr == pvr ? 0 : -1;
 }
 
-const ppc_def_t *ppc_find_by_pvr(uint32_t pvr)
+PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr)
 {
-    int i;
+    GSList *list, *item;
+    PowerPCCPUClass *pcc = NULL;
 
-    for (i = 0; i < ARRAY_SIZE(ppc_defs); i++) {
-        if (!ppc_cpu_usable(&ppc_defs[i])) {
-            continue;
-        }
-
-        /* If we have an exact match, we're done */
-        if (pvr == ppc_defs[i].pvr) {
-            return &ppc_defs[i];
-        }
+    list = object_class_get_list(TYPE_POWERPC_CPU, false);
+    item = g_slist_find_custom(list, &pvr, ppc_cpu_compare_class_pvr);
+    if (item != NULL) {
+        pcc = POWERPC_CPU_CLASS(item->data);
     }
+    g_slist_free(list);
+
+    return pcc;
+}
+
+static gint ppc_cpu_compare_class_name(gconstpointer a, gconstpointer b)
+{
+    ObjectClass *oc = (ObjectClass *)a;
+    const char *name = b;
 
-    return NULL;
+    if (strncasecmp(name, object_class_get_name(oc), strlen(name)) == 0 &&
+        strcmp(object_class_get_name(oc) + strlen(name),
+               "-" TYPE_POWERPC_CPU) == 0) {
+        return 0;
+    }
+    return -1;
 }
 
 #include <ctype.h>
 
-const ppc_def_t *cpu_ppc_find_by_name (const char *name)
+static ObjectClass *ppc_cpu_class_by_name(const char *name)
 {
-    const ppc_def_t *ret;
+    GSList *list, *item;
+    ObjectClass *ret = NULL;
     const char *p;
-    int i, max, len;
+    int i, len;
 
-    if (kvm_enabled() && (strcasecmp(name, "host") == 0)) {
-        return kvmppc_host_cpu_def();
+    if (strcasecmp(name, "host") == 0) {
+        if (kvm_enabled()) {
+            ret = object_class_by_name(TYPE_HOST_POWERPC_CPU);
+        }
+        return ret;
     }
 
     /* Check if the given name is a PVR */
@@ -10312,63 +10308,151 @@ const ppc_def_t *cpu_ppc_find_by_name (const char *name)
             if (!qemu_isxdigit(*p++))
                 break;
         }
-        if (i == 8)
-            return ppc_find_by_pvr(strtoul(name, NULL, 16));
-    }
-    ret = NULL;
-    max = ARRAY_SIZE(ppc_defs);
-    for (i = 0; i < max; i++) {
-        if (!ppc_cpu_usable(&ppc_defs[i])) {
-            continue;
+        if (i == 8) {
+            ret = OBJECT_CLASS(ppc_cpu_class_by_pvr(strtoul(name, NULL, 16)));
+            return ret;
         }
+    }
 
-        if (strcasecmp(name, ppc_defs[i].name) == 0) {
-            ret = &ppc_defs[i];
-            break;
-        }
+    list = object_class_get_list(TYPE_POWERPC_CPU, false);
+    item = g_slist_find_custom(list, name, ppc_cpu_compare_class_name);
+    if (item != NULL) {
+        ret = OBJECT_CLASS(item->data);
     }
+    g_slist_free(list);
 
     return ret;
 }
 
-void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf)
+PowerPCCPU *cpu_ppc_init(const char *cpu_model)
 {
-    int i, max;
+    PowerPCCPU *cpu;
+    CPUPPCState *env;
+    ObjectClass *oc;
+    Error *err = NULL;
 
-    max = ARRAY_SIZE(ppc_defs);
-    for (i = 0; i < max; i++) {
-        if (!ppc_cpu_usable(&ppc_defs[i])) {
-            continue;
-        }
+    oc = ppc_cpu_class_by_name(cpu_model);
+    if (oc == NULL) {
+        return NULL;
+    }
 
-        (*cpu_fprintf)(f, "PowerPC %-16s PVR %08x\n",
-                       ppc_defs[i].name, ppc_defs[i].pvr);
+    cpu = POWERPC_CPU(object_new(object_class_get_name(oc)));
+    env = &cpu->env;
+
+    if (tcg_enabled()) {
+        ppc_translate_init();
     }
+
+    env->cpu_model_str = cpu_model;
+
+    ppc_cpu_realize(OBJECT(cpu), &err);
+    if (err != NULL) {
+        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_free(err);
+        object_delete(OBJECT(cpu));
+        return NULL;
+    }
+
+    return cpu;
+}
+
+/* Sort by PVR, ordering special case "host" last. */
+static gint ppc_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+    ObjectClass *oc_a = (ObjectClass *)a;
+    ObjectClass *oc_b = (ObjectClass *)b;
+    PowerPCCPUClass *pcc_a = POWERPC_CPU_CLASS(oc_a);
+    PowerPCCPUClass *pcc_b = POWERPC_CPU_CLASS(oc_b);
+    const char *name_a = object_class_get_name(oc_a);
+    const char *name_b = object_class_get_name(oc_b);
+
+    if (strcmp(name_a, TYPE_HOST_POWERPC_CPU) == 0) {
+        return 1;
+    } else if (strcmp(name_b, TYPE_HOST_POWERPC_CPU) == 0) {
+        return -1;
+    } else {
+        /* Avoid an integer overflow during subtraction */
+        if (pcc_a->info->pvr < pcc_b->info->pvr) {
+            return -1;
+        } else if (pcc_a->info->pvr > pcc_b->info->pvr) {
+            return 1;
+        } else {
+            return 0;
+        }
+    }
+}
+
+static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
+{
+    ObjectClass *oc = data;
+    CPUListState *s = user_data;
+    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+
+    (*s->cpu_fprintf)(s->file, "PowerPC %-16s PVR %08x\n",
+                      pcc->info->name, pcc->info->pvr);
+}
+
+void ppc_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+    CPUListState s = {
+        .file = f,
+        .cpu_fprintf = cpu_fprintf,
+    };
+    GSList *list;
+
+    list = object_class_get_list(TYPE_POWERPC_CPU, false);
+    list = g_slist_sort(list, ppc_cpu_list_compare);
+    g_slist_foreach(list, ppc_cpu_list_entry, &s);
+    g_slist_free(list);
+}
+
+static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
+{
+    ObjectClass *oc = data;
+    CpuDefinitionInfoList **first = user_data;
+    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+    CpuDefinitionInfoList *entry;
+    CpuDefinitionInfo *info;
+
+    info = g_malloc0(sizeof(*info));
+    info->name = g_strdup(pcc->info->name);
+
+    entry = g_malloc0(sizeof(*entry));
+    entry->value = info;
+    entry->next = *first;
+    *first = entry;
 }
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
-    int i;
+    GSList *list;
 
-    for (i = 0; i < ARRAY_SIZE(ppc_defs); i++) {
-        CpuDefinitionInfoList *entry;
-        CpuDefinitionInfo *info;
+    list = object_class_get_list(TYPE_POWERPC_CPU, false);
+    g_slist_foreach(list, ppc_cpu_defs_entry, &cpu_list);
+    g_slist_free(list);
 
-        if (!ppc_cpu_usable(&ppc_defs[i])) {
-            continue;
-        }
+    return cpu_list;
+}
 
-        info = g_malloc0(sizeof(*info));
-        info->name = g_strdup(ppc_defs[i].name);
+static void ppc_cpu_def_class_init(ObjectClass *oc, void *data)
+{
+    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+    ppc_def_t *info = data;
 
-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = cpu_list;
-        cpu_list = entry;
-    }
+    pcc->info = info;
+}
 
-    return cpu_list;
+static void ppc_cpu_register_model(const ppc_def_t *def)
+{
+    TypeInfo type_info = {
+        .name = def->name,
+        .parent = TYPE_POWERPC_CPU,
+        .class_init = ppc_cpu_def_class_init,
+        .class_data = (void *)def,
+    };
+
+    type_register(&type_info);
 }
 
 /* CPUClass::reset() */
@@ -10439,9 +10523,42 @@ static void ppc_cpu_reset(CPUState *s)
 static void ppc_cpu_initfn(Object *obj)
 {
     PowerPCCPU *cpu = POWERPC_CPU(obj);
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
     CPUPPCState *env = &cpu->env;
+    ppc_def_t *def = pcc->info;
 
     cpu_exec_init(env);
+
+    env->msr_mask = def->msr_mask;
+    env->mmu_model = def->mmu_model;
+    env->excp_model = def->excp_model;
+    env->bus_model = def->bus_model;
+    env->insns_flags = def->insns_flags;
+    env->insns_flags2 = def->insns_flags2;
+    env->flags = def->flags;
+    env->bfd_mach = def->bfd_mach;
+    env->check_pow = def->check_pow;
+
+#if defined(TARGET_PPC64)
+    if (def->sps) {
+        env->sps = *def->sps;
+    } else if (env->mmu_model & POWERPC_MMU_64) {
+        /* Use default sets of page sizes */
+        static const struct ppc_segment_page_sizes defsps = {
+            .sps = {
+                { .page_shift = 12, /* 4K */
+                  .slb_enc = 0,
+                  .enc = { { .page_shift = 12, .pte_enc = 0 } }
+                },
+                { .page_shift = 24, /* 16M */
+                  .slb_enc = 0x100,
+                  .enc = { { .page_shift = 24, .pte_enc = 0 } }
+                },
+            },
+        };
+        env->sps = defsps;
+    }
+#endif /* defined(TARGET_PPC64) */
 }
 
 static void ppc_cpu_class_init(ObjectClass *oc, void *data)
@@ -10458,14 +10575,27 @@ static const TypeInfo ppc_cpu_type_info = {
     .parent = TYPE_CPU,
     .instance_size = sizeof(PowerPCCPU),
     .instance_init = ppc_cpu_initfn,
-    .abstract = false,
+    .abstract = true,
     .class_size = sizeof(PowerPCCPUClass),
     .class_init = ppc_cpu_class_init,
 };
 
 static void ppc_cpu_register_types(void)
 {
+    int i;
+
     type_register_static(&ppc_cpu_type_info);
+
+    for (i = 0; i < ARRAY_SIZE(ppc_defs); i++) {
+        const ppc_def_t *def = &ppc_defs[i];
+#if defined(TARGET_PPCEMB)
+        /* When using the ppcemb target, we only support 440 style cores */
+        if (def->mmu_model != POWERPC_MMU_BOOKE) {
+            continue;
+        }
+#endif
+        ppc_cpu_register_model(def);
+    }
 }
 
 type_init(ppc_cpu_register_types)
-- 
1.7.10.4

Re: [PATCH qom-cpu 3/4] target-ppc: Slim conversion of model definitions to QOM subclasses

[Eduardo Habkost]

On Tue, Dec 18, 2012 at 08:53:42AM +0100, Andreas Färber wrote:
> Since the model list is highly macrofied, keep ppc_def_t for now and
> save a pointer to it in PowerPCCPUClass. This results in a flat list of
> subclasses including aliases, to be refined later.
> 
> Move cpu_ppc_init() to translate_init.c and drop helper.c.
> Long-term the idea is to turn translate_init.c into a standalone cpu.c.
> 
> Inline cpu_ppc_usable() into type registration.
> 
> Split cpu_ppc_register() in two by code movement into the initfn and
> by turning the remaining part into a realizefn.
> Move qemu_init_vcpu() call into the new realizefn and adapt
> create_ppc_opcodes() to return an Error.
> 
> Change ppc_find_by_pvr() -> ppc_cpu_class_by_pvr().
> Change ppc_find_by_name() -> ppc_cpu_class_by_name().
> 
> Turn -cpu host into its own subclass. This requires to move the
> kvm_enabled() check in ppc_cpu_class_by_name() to avoid the class being
> found via the normal name lookup in the !kvm_enabled() case.
> Turn kvmppc_host_cpu_def() into the class_init and add an initfn that
> asserts KVM is in fact enabled.
> 
> Implement -cpu ? and the QMP equivalent in terms of subclasses.
> This newly exposes -cpu host to the user, ordered last for -cpu ?.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
[...]
> +static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
> +{
> +    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
>      uint32_t host_pvr = mfpvr();
> -    const ppc_def_t *base_spec;
> +    PowerPCCPUClass *pvr_pcc;
>      ppc_def_t *spec;
>      uint32_t vmx = kvmppc_get_vmx();
>      uint32_t dfp = kvmppc_get_dfp();
>  
> -    base_spec = ppc_find_by_pvr(host_pvr);
> -
>      spec = g_malloc0(sizeof(*spec));
> -    memcpy(spec, base_spec, sizeof(*spec));
> +
> +    pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);

I wonder if it's really safe to do this inside class_init.

ppc_cpu_class_by_pvr() calls object_class_get_list(TYPE_POWERPC_CPU),
and TYPE_HOST_POWERPC_CPU will be included in that list, but hasn't
finished its initialization yet.

I see that you took care of checking for TYPE_HOST_POWERPC_CPU inside
ppc_cpu_compare_class_pvr(). So, it looks like this is OK today, but I
am not 100% sure this doesn't break any assumptions/requirements of QOM.



> +    if (pvr_pcc != NULL) {
> +        memcpy(spec, pvr_pcc->info, sizeof(*spec));
> +    }
> +    pcc->info = spec;
> +    /* Override the display name for -cpu ? and QMP */
> +    pcc->info->name = "host";
>  
>      /* Now fix up the spec with information we can query from the host */
>  
> @@ -1208,8 +1219,6 @@ const ppc_def_t *kvmppc_host_cpu_def(void)
>          /* Only override when we know what the host supports */
>          alter_insns(&spec->insns_flags2, PPC2_DFP, dfp);
>      }
> -
> -    return spec;
>  }
>  
>  int kvmppc_fixup_cpu(CPUPPCState *env)
[...]
> +static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
> +{
> +    ObjectClass *oc = data;
> +    CPUListState *s = user_data;
> +    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> +
> +    (*s->cpu_fprintf)(s->file, "PowerPC %-16s PVR %08x\n",
> +                      pcc->info->name, pcc->info->pvr);
> +}
> +
> +void ppc_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> +{
> +    CPUListState s = {
> +        .file = f,
> +        .cpu_fprintf = cpu_fprintf,
> +    };
> +    GSList *list;
> +
> +    list = object_class_get_list(TYPE_POWERPC_CPU, false);
> +    list = g_slist_sort(list, ppc_cpu_list_compare);
> +    g_slist_foreach(list, ppc_cpu_list_entry, &s);
> +    g_slist_free(list);
> +}
> +
> +static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
> +{
> +    ObjectClass *oc = data;
> +    CpuDefinitionInfoList **first = user_data;
> +    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> +    CpuDefinitionInfoList *entry;
> +    CpuDefinitionInfo *info;
> +
> +    info = g_malloc0(sizeof(*info));
> +    info->name = g_strdup(pcc->info->name);
> +
> +    entry = g_malloc0(sizeof(*entry));
> +    entry->value = info;
> +    entry->next = *first;
> +    *first = entry;
>  }
>  
>  CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
>  {
>      CpuDefinitionInfoList *cpu_list = NULL;
> -    int i;
> +    GSList *list;
>  
> -    for (i = 0; i < ARRAY_SIZE(ppc_defs); i++) {
> -        CpuDefinitionInfoList *entry;
> -        CpuDefinitionInfo *info;
> +    list = object_class_get_list(TYPE_POWERPC_CPU, false);
> +    g_slist_foreach(list, ppc_cpu_defs_entry, &cpu_list);
> +    g_slist_free(list);
>  
> -        if (!ppc_cpu_usable(&ppc_defs[i])) {
> -            continue;
> -        }
> +    return cpu_list;
> +}

I still think we could reuse arch_query_cpu_definitions() for both
"-cpu ?" and "query-cpu-definitions". But, anyway: we can unify that one
step at a time. This at least matches the same pattern used in other
architectures.

[...]

I didn't review the ppc-specific code that got moved around. I will try
to take a deeper look today, but I hope this gets reviewed by somebody
with more experience dealing with the target-ppc code.

-- 
Eduardo

Re: [Qemu-devel] [PATCH qom-cpu 3/4] target-ppc: Slim conversion of model definitions to QOM subclasses

[Andreas Färber]

Am 18.12.2012 08:53, schrieb Andreas Färber:
> +static gint ppc_cpu_compare_class_name(gconstpointer a, gconstpointer b)
> +{
> +    ObjectClass *oc = (ObjectClass *)a;
> +    const char *name = b;
>  
> -    return NULL;
> +    if (strncasecmp(name, object_class_get_name(oc), strlen(name)) == 0 &&
> +        strcmp(object_class_get_name(oc) + strlen(name),
> +               "-" TYPE_POWERPC_CPU) == 0) {
> +        return 0;
> +    }
> +    return -1;
>  }
>  
>  #include <ctype.h>
>  
> -const ppc_def_t *cpu_ppc_find_by_name (const char *name)
> +static ObjectClass *ppc_cpu_class_by_name(const char *name)
[...]
> +    list = object_class_get_list(TYPE_POWERPC_CPU, false);
> +    item = g_slist_find_custom(list, name, ppc_cpu_compare_class_name);
> +    if (item != NULL) {
> +        ret = OBJECT_CLASS(item->data);
>      }
> +    g_slist_free(list);
>  
>      return ret;
>  }
[...]
> +static void ppc_cpu_register_model(const ppc_def_t *def)
> +{
> +    TypeInfo type_info = {
> +        .name = def->name,
> +        .parent = TYPE_POWERPC_CPU,
> +        .class_init = ppc_cpu_def_class_init,
> +        .class_data = (void *)def,
> +    };
> +
> +    type_register(&type_info);
>  }

Err, needs the following fix:

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 908a558..ad0763e 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10446,13 +10446,14 @@ static void ppc_cpu_def_class_init(ObjectClass
*oc, void *data)
 static void ppc_cpu_register_model(const ppc_def_t *def)
 {
     TypeInfo type_info = {
-        .name = def->name,
         .parent = TYPE_POWERPC_CPU,
         .class_init = ppc_cpu_def_class_init,
         .class_data = (void *)def,
     };

+    type_info.name = g_strdup_printf("%s-" TYPE_POWERPC_CPU, def->name),
     type_register(&type_info);
+    g_free((gpointer)type_info.name);
 }

 /* CPUClass::reset() */

Not sure how I managed to screw this up so badly, seems I tested corner
cases and -cpu host too much...

Andreas

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

[PATCH qom-cpu 4/4] target-ppc: Error out for -cpu host on unknown PVR

[Andreas Färber]

Previously we silently exited, with subclasses we got an opcode warning.
Instead explicitly tell the user what's wrong.

An indication for this is -cpu ? showing "host" with an all-zero PVR.

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

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index f115892..8998d0f 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1186,7 +1186,14 @@ static void alter_insns(uint64_t *word, uint64_t flags, bool on)
 
 static void kvmppc_host_cpu_initfn(Object *obj)
 {
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(obj);
+
     assert(kvm_enabled());
+
+    if (pcc->info->pvr != mfpvr()) {
+        fprintf(stderr, "Host PVR unsupported.\n");
+        exit(1);
+    }
 }
 
 static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
-- 
1.7.10.4

Re: [PATCH qom-cpu 4/4] target-ppc: Error out for -cpu host on unknown PVR

[Alexander Graf]

On 18.12.2012, at 08:53, Andreas Färber wrote:

> Previously we silently exited, with subclasses we got an opcode warning.
> Instead explicitly tell the user what's wrong.
> 
> An indication for this is -cpu ? showing "host" with an all-zero PVR.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> target-ppc/kvm.c |    7 +++++++
> 1 Datei geändert, 7 Zeilen hinzugefügt(+)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index f115892..8998d0f 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1186,7 +1186,14 @@ static void alter_insns(uint64_t *word, uint64_t flags, bool on)
> 
> static void kvmppc_host_cpu_initfn(Object *obj)
> {
> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(obj);
> +
>     assert(kvm_enabled());
> +
> +    if (pcc->info->pvr != mfpvr()) {
> +        fprintf(stderr, "Host PVR unsupported.\n");

This should probably rather say "Host CPU unsupported for -cpu host" or so :). Not everyone who invokes qemu-system-ppc knows what a PVR is.


Alex