[RFC,v4,02/20] target-arm: Introduce QOM ARMCPUClass

Create a CPU subclass, and register classes matching all CPU models
except for "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 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 <anthony@codemonkey.ws>
Cc: Paul Brook <paul@codesourcery.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 Makefile.target      |    1 +
 target-arm/cpu-qom.h |   64 +++++++++++++++++
 target-arm/cpu.c     |  193 ++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu.h     |    1 -
 target-arm/helper.c  |  108 +++++++++++++---------------
 5 files changed, 308 insertions(+), 59 deletions(-)
 create mode 100644 target-arm/cpu-qom.h
 create mode 100644 target-arm/cpu.c

On 03/10/2012 08:53 PM, Andreas Färber wrote:
> Create a CPU subclass, and register classes matching all CPU models
> except for "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 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<anthony@codemonkey.ws>
> Cc: Paul Brook<paul@codesourcery.com>
> Cc: Peter Maydell<peter.maydell@linaro.org>
> ---
>   Makefile.target      |    1 +
>   target-arm/cpu-qom.h |   64 +++++++++++++++++
>   target-arm/cpu.c     |  193 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   target-arm/cpu.h     |    1 -
>   target-arm/helper.c  |  108 +++++++++++++---------------
>   5 files changed, 308 insertions(+), 59 deletions(-)
>   create mode 100644 target-arm/cpu-qom.h
>   create mode 100644 target-arm/cpu.c
>
> diff --git a/Makefile.target b/Makefile.target
> index cb1532a..c2c4bca 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -87,6 +87,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.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-qom.h b/target-arm/cpu-qom.h
> new file mode 100644
> index 0000000..b2917ea
> --- /dev/null
> +++ b/target-arm/cpu-qom.h
> @@ -0,0 +1,64 @@
> +/*
> + * QEMU ARM CPU
> + *
> + * Copyright (c) 2012 SUSE LINUX Products GmbH
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see
> + *<http://www.gnu.org/licenses/gpl-2.0.html>
> + */
> +#ifndef QEMU_ARM_CPU_QOM_H
> +#define QEMU_ARM_CPU_QOM_H
> +
> +#include "qemu/cpu.h"
> +
> +#define TYPE_ARM_CPU "arm-cpu"
> +
> +#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)
> +
> +/**
> + * ARMCPUClass:
> + * @parent_reset: The parent class' reset handler.
> + *
> + * An ARM CPU model.
> + */
> +typedef struct ARMCPUClass {
> +    /*<  private>*/
> +    CPUClass parent_class;
> +    /*<  public>*/
> +
> +    void (*parent_reset)(CPUState *cpu);
> +
> +    struct {
> +        uint32_t c0_cpuid;
> +    } cp15;
> +} ARMCPUClass;
> +
> +/**
> + * ARMCPU:
> + *
> + * An ARM CPU core.
> + */
> +typedef struct ARMCPU {
> +    /*<  private>*/
> +    CPUState parent_obj;
> +    /*<  public>*/
> +} ARMCPU;
> +
> +
> +#endif
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> new file mode 100644
> index 0000000..dabc094
> --- /dev/null
> +++ b/target-arm/cpu.c
> @@ -0,0 +1,193 @@
> +/*
> + * QEMU ARM CPU
> + *
> + * Copyright (c) 2012 SUSE LINUX Products GmbH
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see
> + *<http://www.gnu.org/licenses/gpl-2.0.html>
> + */
> +
> +#include "cpu-qom.h"
> +#include "qemu-common.h"
> +
> +static void arm_cpu_reset(CPUState *c)
> +{
> +    ARMCPUClass *klass = ARM_CPU_GET_CLASS(c);
> +
> +    klass->parent_reset(c);
> +}
> +
> +/* CPU models */
> +
> +typedef struct ARMCPUInfo {
> +    const char *name;
> +    uint32_t id;
> +} ARMCPUInfo;
> +
> +static const ARMCPUInfo arm_cpus[] = {
> +    {
> +        .name = "arm926",
> +        .id = 0x41069265,
> +    },
> +    {
> +        .name = "arm946",
> +        .id = 0x41059461,
> +    },
> +    {
> +        .name = "arm1026",
> +        .id = 0x4106a262,
> +    },
> +    /* 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.
> +     */
> +    {
> +        .name = "arm1136-r2",
> +        .id = 0x4107b362,
> +    },
> +    {
> +        .name = "arm1136",
> +        .id = 0x4117b363,
> +    },
> +    {
> +        .name = "arm1176",
> +        .id = 0x410fb767,
> +    },
> +    {
> +        .name = "arm11mpcore",
> +        .id = 0x410fb022,
> +    },
> +    {
> +        .name = "cortex-m3",
> +        .id = 0x410fc231,
> +    },
> +    {
> +        .name = "cortex-a8",
> +        .id = 0x410fc080,
> +    },
> +    {
> +        .name = "cortex-a9",
> +        .id = 0x410fc090,
> +    },
> +    {
> +        .name = "cortex-a15",
> +        .id = 0x412fc0f1,
> +    },
> +    {
> +        .name = "ti925t",
> +        .id = 0x54029252,
> +    },
> +    {
> +        .name = "sa1100",
> +        .id = 0x4401A11B,
> +    },
> +    {
> +        .name = "sa1110",
> +        .id = 0x6901B119,
> +    },
> +    {
> +        .name = "pxa250",
> +        .id = 0x69052100,
> +    },
> +    {
> +        .name = "pxa255",
> +        .id = 0x69052d00,
> +    },
> +    {
> +        .name = "pxa260",
> +        .id = 0x69052903,
> +    },
> +    {
> +        .name = "pxa261",
> +        .id = 0x69052d05,
> +    },
> +    {
> +        .name = "pxa262",
> +        .id = 0x69052d06,
> +    },
> +    {
> +        .name = "pxa270-a0",
> +        .id = 0x69054110,
> +    },
> +    {
> +        .name = "pxa270-a1",
> +        .id = 0x69054111,
> +    },
> +    {
> +        .name = "pxa270-b0",
> +        .id = 0x69054112,
> +    },
> +    {
> +        .name = "pxa270-b1",
> +        .id = 0x69054113,
> +    },
> +    {
> +        .name = "pxa270-c0",
> +        .id = 0x69054114,
> +    },
> +    {
> +        .name = "pxa270-c5",
> +        .id = 0x69054117,
> +    },
> +    {
> +        .name = "any",
> +        .id = 0xffffffff,
> +    },
> +};
> +
> +static void arm_cpu_class_init(ObjectClass *klass, void *data)
> +{
> +    ARMCPUClass *k = ARM_CPU_CLASS(klass);
> +    CPUClass *cpu_class = CPU_CLASS(klass);
> +    const ARMCPUInfo *info = data;
> +
> +    k->parent_reset = cpu_class->reset;
> +    cpu_class->reset = arm_cpu_reset;
> +
> +    k->cp15.c0_cpuid = info->id;
> +}
> +
> +static void cpu_register(const ARMCPUInfo *info)
> +{
> +    TypeInfo type = {
> +        .name = info->name,
> +        .parent = TYPE_ARM_CPU,
> +        .instance_size = sizeof(ARMCPU),
> +        .class_size = sizeof(ARMCPUClass),
> +        .class_init = arm_cpu_class_init,
> +        .class_data = (void *)info,
> +    };

Are non-initialized members guaranteed to be zero here?

> +    type_register_static(&type);
> +}
> +

Probably should be type_register() here in case these two will actually 
differ in the future.


If this information is of any help, we've got no problems when emulating 
ARM-based Exynos boards in QEMU with this whole patchset applied.

Am 13.03.2012 13:31, schrieb Igor Mitsyanko:
> On 03/10/2012 08:53 PM, Andreas Färber wrote:
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> new file mode 100644
>> index 0000000..dabc094
>> --- /dev/null
>> +++ b/target-arm/cpu.c
[...]
>> +static void cpu_register(const ARMCPUInfo *info)
>> +{
>> +    TypeInfo type = {
>> +        .name = info->name,
>> +        .parent = TYPE_ARM_CPU,
>> +        .instance_size = sizeof(ARMCPU),
>> +        .class_size = sizeof(ARMCPUClass),
>> +        .class_init = arm_cpu_class_init,
>> +        .class_data = (void *)info,
>> +    };
> 
> Are non-initialized members guaranteed to be zero here?

I thought so for the C99-style struct initialization... I never ran into
crashes while testing. Do we need static to be safe?

>> +    type_register_static(&type);
>> +}
>> +
> 
> Probably should be type_register() here in case these two will actually
> differ in the future.

My thinking was we don't need it here because the data (esp. strings)
are not dynamically allocated. By comparison, I used type_register() for
-cpudef in target-i386, I believe.

But I really guess it's a bug that they're just an alias right now! ;)

> If this information is of any help, we've got no problems when emulating
> ARM-based Exynos boards in QEMU with this whole patchset applied.

Thanks a lot for testing!

Have you thought about how to QOM'ify your boards? Mid-term I'd like to
see an "exynos4210" object with the CPUs on it - maybe "cpu[0]" and
"cpu[1]" child properties? Or "core[x]"? I had played with the sh7750 a
bit on my branch but like the arm926 it's a single-core.

Andreas

On 03/13/2012 11:58 AM, Andreas Färber wrote:

>>> +static void cpu_register(const ARMCPUInfo *info)
>>> +{
>>> +    TypeInfo type = {
>>> +        .name = info->name,

>>
>> Are non-initialized members guaranteed to be zero here?
> 
> I thought so for the C99-style struct initialization... I never ran into
> crashes while testing. Do we need static to be safe?

Yes, C99 guarantees in 6.7.8 p19:

The initialization shall occur in initializer list order, each
initializer provided for a particular subobject overriding any
previously listed initializer for the same subobject; all subobjects
that are not initialized explicitly shall be initialized implicitly the
same as objects that have static storage duration.

Il 13/03/2012 18:58, Andreas Färber ha scritto:
>> > Are non-initialized members guaranteed to be zero here?
> I thought so for the C99-style struct initialization...

Yes.

> I never ran into
> crashes while testing. Do we need static to be safe?

No, I don't think so.  It would raise other questions (static seems to
imply that the lifetime needs to extend across multiple calls, one could
wonder if it is safe to reuse TypeInfos that way; with stack-allocated
storage it's more obvious).

Paolo

On 03/13/2012 12:04 PM, Eric Blake wrote:
> On 03/13/2012 11:58 AM, Andreas Färber wrote:
> 
>>>> +static void cpu_register(const ARMCPUInfo *info)
>>>> +{
>>>> +    TypeInfo type = {
>>>> +        .name = info->name,
> 
>>>
>>> Are non-initialized members guaranteed to be zero here?
>>
>> I thought so for the C99-style struct initialization... I never ran into
>> crashes while testing. Do we need static to be safe?
> 
> Yes, C99 guarantees in 6.7.8 p19:

Apologies for my ambiguous answer.  Yes, C99 guarantees that
non-mentioned members are 0-initialized if you have at least one
explicit initialization.  Therefore, no, you don't need static to be safe.

On 13 March 2012 17:58, Andreas Färber <afaerber@suse.de> wrote:
> Have you thought about how to QOM'ify your boards? Mid-term I'd like to
> see an "exynos4210" object with the CPUs on it - maybe "cpu[0]" and
> "cpu[1]" child properties? Or "core[x]"? I had played with the sh7750 a
> bit on my branch but like the arm926 it's a single-core.

I think the exynos4210 object should have a single child object
cortex-a9 which in turn has child objects for core[0], core[1],
gic, scu, and the private timers and watchdogs.

In hardware the A9 is a single block which includes all the
cores and all the onboard peripherals; we should model it
the same way.

One step in this direction might be to disentangle the GIC
so that a9mpcore_priv and friends instantiate a GIC object
as a child object rather than subclassing the GIC. I think
now that Avi has made sub-page-size memory regions work
we can get the M profile NVIC code out of arm_gic.c and
have a standalone GIC object everybody instantiates.

-- PMM

On 03/13/2012 09:58 PM, Andreas Färber wrote:
> Am 13.03.2012 13:31, schrieb Igor Mitsyanko:
>> On 03/10/2012 08:53 PM, Andreas Färber wrote:
>>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>>> new file mode 100644
>>> index 0000000..dabc094
>>> --- /dev/null
>>> +++ b/target-arm/cpu.c
> [...]
>>> +static void cpu_register(const ARMCPUInfo *info)
>>> +{
>>> +    TypeInfo type = {
>>> +        .name = info->name,
>>> +        .parent = TYPE_ARM_CPU,
>>> +        .instance_size = sizeof(ARMCPU),
>>> +        .class_size = sizeof(ARMCPUClass),
>>> +        .class_init = arm_cpu_class_init,
>>> +        .class_data = (void *)info,
>>> +    };
>> Are non-initialized members guaranteed to be zero here?
> I thought so for the C99-style struct initialization... I never ran into
> crashes while testing. Do we need static to be safe?
>
>>> +    type_register_static(&type);
>>> +}
>>> +
>> Probably should be type_register() here in case these two will actually
>> differ in the future.
> My thinking was we don't need it here because the data (esp. strings)
> are not dynamically allocated. By comparison, I used type_register() for
> -cpudef in target-i386, I believe.
>
> But I really guess it's a bug that they're just an alias right now! ;)
>

Object.h states we need to use type_register_static() only when TypeInfo 
exists for the lifetime of
Type. As I understand this, TypeInfo must have *static const* storage 
qualifier when used with type_register_static() and can have automatic 
storage qualifier when used with type_register().
Maybe in the future type_register_static() will just copy a pointer to 
string variables of passed
TypeInfo instead of g_strdup() them, then your code wouldn't work as 
expected.

>> If this information is of any help, we've got no problems when emulating
>> ARM-based Exynos boards in QEMU with this whole patchset applied.
> Thanks a lot for testing!
>
> Have you thought about how to QOM'ify your boards? Mid-term I'd like to
> see an "exynos4210" object with the CPUs on it - maybe "cpu[0]" and
> "cpu[1]" child properties? Or "core[x]"? I had played with the sh7750 a
> bit on my branch but like the arm926 it's a single-core.
>
> Andreas
>
Yes, we've done some work on this, but we are waiting to see how you 
want to proceed with SoC
QOM type, and if you're going to introduce it at all. Personally I have 
no idea what members and
methods a general SoC type could have.
And another question, should we somehow exploit a fact that majority of 
peripheral devices on
SoCs share i/o ports with each other and only a handful of them are 
actually active at the same
time. Currently we instantiate all SoC devices even if half of them are 
not used in emulated configuration. Maybe we should dynamically 
create/destroy devices depending on runtime GPIO
configuration, or make all devices a board's child instead of SoC's 
child. Not sure that it's worth an effort though.

As for CPU's object, Peter's idea with CortexA9 and core1, core2 childs 
looks good to me.