Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Eduardo Habkost]

Sorry for replying to a patch 7 months later, but I just have a question
related to how we will handle CPU model classes on all targets:

On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
> Register subclasses for each ARM CPU implementation (with the
> exception of "pxa270", which is an alias for "pxa270-a0").
> 
> Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
> except for special value "any".
> 
> Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
> CPUID (aka MIDR, Main ID Register) value in the class.
> 
> Signed-off-by: Andreas Färber <address@hidden>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  target-arm/cpu-qom.h |   12 +++
>  target-arm/cpu.c     |  226 
> +++++++++++++++++++++++++++++++++++++++++++++++++-
>  target-arm/helper.c  |  109 ++++++++++--------------
>  3 files changed, 282 insertions(+), 65 deletions(-)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 42d2a6b..a4bcb31 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -58,6 +58,18 @@ typedef struct ARMCPU {
[...]
> +typedef struct ARMCPUInfo {
> +    const char *name;
> +    void (*initfn)(Object *obj);
> +} ARMCPUInfo;
> +
> +static const ARMCPUInfo arm_cpus[] = {
[...]
> +    { .name = "any",         .initfn = arm_any_initfn },
> +};
> +

Do we really want to use "any" as the class name? Maybe we should use
"cpu-<model>" as the namespace for the CPU model class names? Or maybe
try "cpu-<model>" first, and then "<model>" as a fallback (making sure
that the class we found is a subclass of TYPE_<arch>_CPU).

I guess we will want address this before qdevifying the CPU class, as
the qdevification will make the CPU class names visible through the
monitor.


>  static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      ARMCPUClass *acc = ARM_CPU_CLASS(oc);
> @@ -43,18 +248,37 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
> *data)
>      cc->reset = arm_cpu_reset;
>  }
>  
> +static void cpu_register(const ARMCPUInfo *info)
> +{
> +    TypeInfo type_info = {
> +        .name = info->name,
> +        .parent = TYPE_ARM_CPU,
> +        .instance_size = sizeof(ARMCPU),
> +        .instance_init = info->initfn,
> +        .class_size = sizeof(ARMCPUClass),
> +    };
> +
> +    type_register_static(&type_info);
> +}
> +
>  static const TypeInfo arm_cpu_type_info = {
>      .name = TYPE_ARM_CPU,
>      .parent = TYPE_CPU,
>      .instance_size = sizeof(ARMCPU),
> -    .abstract = false,
> +    .instance_init = arm_cpu_initfn,
> +    .abstract = true,
>      .class_size = sizeof(ARMCPUClass),
>      .class_init = arm_cpu_class_init,
>  };
>  
>  static void arm_cpu_register_types(void)
>  {
> +    int i;
> +
>      type_register_static(&arm_cpu_type_info);
> +    for (i = 0; i < ARRAY_SIZE(arm_cpus); i++) {
> +        cpu_register(&arm_cpus[i]);
> +    }
>  }
>  
>  type_init(arm_cpu_register_types)

-- 
Eduardo