Re: [Qemu-ppc] [PATCH ppc-next v2 42/52] target-ppc: Convert CPU definitions

[Alexander Graf]

On 18.02.2013, at 10:16, Andreas Färber wrote:

> Turn the array of model definitions into a set of self-registering QOM
> types with their own class_init. Unique identifiers are obtained from
> the combination of PVR, SVR and family identifiers; this requires all
> alias #defines to be removed from the list. Possibly there are some more
> left after this commit that are not currently being compiled.
> 
> Prepares for introducing abstract intermediate CPU types for families.
> 
> Keep the right-aligned macro line breaks within 78 chars to aid
> three-way merges.
> 
> Signed-off-by: Andreas Färber <address@hidden>
> ---
> target-ppc/cpu-qom.h        |   17 ++++-
> target-ppc/cpu.h            |   20 ------
> target-ppc/kvm.c            |   32 +++++----
> target-ppc/translate_init.c |  163 +++++++++++++++++++++----------------------
> 4 Dateien geändert, 115 Zeilen hinzugefügt(+), 117 Zeilen entfernt(-)
> 
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index b338f8f..7220908 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -51,8 +51,21 @@ typedef struct PowerPCCPUClass {
> 
>     void (*parent_reset)(CPUState *cpu);
> 
> -    /* TODO inline fields here */
> -    ppc_def_t *info;
> +    uint32_t pvr;
> +    uint32_t svr;
> +    uint64_t insns_flags;
> +    uint64_t insns_flags2;
> +    uint64_t msr_mask;
> +    powerpc_mmu_t   mmu_model;
> +    powerpc_excp_t  excp_model;
> +    powerpc_input_t bus_model;
> +    uint32_t flags;
> +    int bfd_mach;
> +#if defined(TARGET_PPC64)
> +    const struct ppc_segment_page_sizes *sps;
> +#endif
> +    void (*init_proc)(CPUPPCState *env);
> +    int  (*check_pow)(CPUPPCState *env);
> } PowerPCCPUClass;
> 
> /**
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 8c081db..86ebd3a 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -307,7 +307,6 @@ enum powerpc_input_t {
> #define PPC_INPUT(env) (env->bus_model)
> 
> /*****************************************************************************/
> -typedef struct ppc_def_t ppc_def_t;
> typedef struct opc_handler_t opc_handler_t;
> 
> /*****************************************************************************/
> @@ -902,25 +901,6 @@ struct ppc_segment_page_sizes {
> /* The whole PowerPC CPU context */
> #define NB_MMU_MODES 3
> 
> -struct ppc_def_t {
> -    const char *name;
> -    uint32_t pvr;
> -    uint32_t svr;
> -    uint64_t insns_flags;
> -    uint64_t insns_flags2;
> -    uint64_t msr_mask;
> -    powerpc_mmu_t   mmu_model;
> -    powerpc_excp_t  excp_model;
> -    powerpc_input_t bus_model;
> -    uint32_t flags;
> -    int bfd_mach;
> -#if defined(TARGET_PPC64)
> -    const struct ppc_segment_page_sizes *sps;
> -#endif
> -    void (*init_proc)(CPUPPCState *env);
> -    int  (*check_pow)(CPUPPCState *env);
> -};
> -
> struct CPUPPCState {
>     /* First are the most commonly used resources
>      * during translated code execution
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 2c64c63..e601059 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1263,7 +1263,7 @@ static void kvmppc_host_cpu_initfn(Object *obj)
> 
>     assert(kvm_enabled());
> 
> -    if (pcc->info->pvr != mfpvr()) {
> +    if (pcc->pvr != mfpvr()) {
>         fprintf(stderr, "Your host CPU is unsupported.\n"
>                 "Please choose a supported model instead, see -cpu ?.\n");
>         exit(1);
> @@ -1275,30 +1275,38 @@ static void kvmppc_host_cpu_class_init(ObjectClass 
> *oc, void *data)
>     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
>     uint32_t host_pvr = mfpvr();
>     PowerPCCPUClass *pvr_pcc;
> -    ppc_def_t *spec;
>     uint32_t vmx = kvmppc_get_vmx();
>     uint32_t dfp = kvmppc_get_dfp();
> 
> -    spec = g_malloc0(sizeof(*spec));
> -
>     pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
>     if (pvr_pcc != NULL) {
> -        memcpy(spec, pvr_pcc->info, sizeof(*spec));
> +        pcc->pvr          = pvr_pcc->pvr;
> +        pcc->svr          = pvr_pcc->svr;
> +        pcc->insns_flags  = pvr_pcc->insns_flags;
> +        pcc->insns_flags2 = pvr_pcc->insns_flags2;
> +        pcc->msr_mask     = pvr_pcc->msr_mask;
> +        pcc->mmu_model    = pvr_pcc->mmu_model;
> +        pcc->excp_model   = pvr_pcc->excp_model;
> +        pcc->bus_model    = pvr_pcc->bus_model;
> +        pcc->flags        = pvr_pcc->flags;
> +        pcc->bfd_mach     = pvr_pcc->bfd_mach;
> +#ifdef TARGET_PPC64
> +        pcc->sps          = pvr_pcc->sps;
> +#endif
> +        pcc->init_proc    = pvr_pcc->init_proc;
> +        pcc->check_pow    = pvr_pcc->check_pow;

It would be nice to have field copying more streamlined. This way, whoever adds 
a new field to the class needs to know that he also has to change this piece of 
code, which is non-obvious.

Speaking of which, why aren't you copying parent_reset for example? Or asked 
differently: Why can't we do a memcpy? We're really trying to do a subclass of 
the parent class here, no?


Alex