[RFC,v3,03/21] qom: Introduce CPU class

It's abstract and derived directly from TYPE_OBJECT.
Prepare a virtual reset method.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/cpu.h |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qom/Makefile       |    1 +
 qom/cpu.c          |   50 +++++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+), 0 deletions(-)
 create mode 100644 include/qemu/cpu.h
 create mode 100644 qom/cpu.c

On 02/02/2012 08:59 PM, Andreas Färber wrote:
> It's abstract and derived directly from TYPE_OBJECT.
> Prepare a virtual reset method.
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> Cc: Anthony Liguori<anthony@codemonkey.ws>
> Cc: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/qemu/cpu.h |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   qom/Makefile       |    1 +
>   qom/cpu.c          |   50 +++++++++++++++++++++++++++++++++++
>   3 files changed, 124 insertions(+), 0 deletions(-)
>   create mode 100644 include/qemu/cpu.h
>   create mode 100644 qom/cpu.c
>
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> new file mode 100644
> index 0000000..22c7404
> --- /dev/null
> +++ b/include/qemu/cpu.h
> @@ -0,0 +1,73 @@
> +/*
> + * QEMU CPU model
> + *
> + * 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/old-licenses/gpl-2.0>
> + */
> +#ifndef QEMU_CPU_H
> +#define QEMU_CPU_H
> +
> +#include "qemu/object.h"
> +
> +#define TYPE_CPU "cpu"
> +
> +#define CPU(obj) OBJECT_CHECK(CPU, (obj), TYPE_CPU)
> +#define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU)
> +#define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU)
> +
> +typedef struct CPU CPU;

If this doesn't result in a build error, then C is a worse language than I 
thought it was :-/

But we definitely shouldn't have a typename and macro name of the same thing and 
expect it to work...

> +
> +/**
> + * CPUClass:
> + * @reset: Callback to reset the #CPU to its initial state.
> + *
> + * Represents a CPU family or model.
> + */
> +typedef struct CPUClass {
> +    ObjectClass parent_class;
> +
> +    void (*reset)(CPU *cpu);
> +} CPUClass;
> +
> +/**
> + * CPU:
> + *
> + * State of one CPU core or thread.
> + */
> +struct CPU {
> +    Object parent_obj;
> +
> +    /* TODO Move common fields from CPUState here. */
> +};
> +
> +
> +/* TODO Rename to cpu_reset once all CPUState is converted to QOM. */
> +/**
> + * cpu_do_reset:
> + * @cpu: The CPU whose state is to be reset.
> + */
> +void cpu_do_reset(CPU *cpu);

Better to sed-away the current cpu_reset() callers to cpu_reset_legacy() and 
then make this cpu_reset().  Otherwise, you're introducing new code that has the 
wrong usage which is never a good strategy.

> +
> +/**
> + * cpu_common_reset:
> + * @cpu: The CPU whose common state is to be reset.
> + *
> + * To be used by derived classes.
> + */
> +void cpu_common_reset(CPU *cpu);

Make this static, initialize reset = cpu_common_reset in cpu_class_initfn, then 
in the derived class initfn, save the pointer to the parent reset function so it 
can be called later.

At some point, I had an:

ObjectClass *object_get_super_class(Object *obj);

Such that you could do:

CPUClass *super = CPU_CLASS(object_get_super_class(OBJECT(s)));

super->reset(CPU(s));

But my fear was that the subtle semantic of the object representing the class of 
your super class (which is different than a casted form of your class) would 
lead to confusion.

Regards,

Anthony Liguori

> +
> +
> +#endif
> diff --git a/qom/Makefile b/qom/Makefile
> index f33f0be..0a1c8e0 100644
> --- a/qom/Makefile
> +++ b/qom/Makefile
> @@ -1 +1,2 @@
>   qom-y = object.o container.o
> +qom-y += cpu.o
> diff --git a/qom/cpu.c b/qom/cpu.c
> new file mode 100644
> index 0000000..a60823f
> --- /dev/null
> +++ b/qom/cpu.c
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU CPU model
> + *
> + * 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/old-licenses/gpl-2.0>
> + */
> +
> +#include "qemu/cpu.h"
> +#include "qemu-common.h"
> +
> +void cpu_do_reset(CPU *cpu)
> +{
> +    CPUClass *klass = CPU_GET_CLASS(cpu);
> +
> +    if (klass->reset != NULL) {
> +        (*klass->reset)(cpu);
> +    }
> +}
> +
> +void cpu_common_reset(CPU *cpu)
> +{
> +}
> +
> +static TypeInfo cpu_type_info = {
> +    .name = TYPE_CPU,
> +    .parent = TYPE_OBJECT,
> +    .instance_size = sizeof(CPU),
> +    .abstract = true,
> +    .class_size = sizeof(CPUClass),
> +};
> +
> +static void cpu_register_types(void)
> +{
> +    type_register_static(&cpu_type_info);
> +}
> +
> +type_init(cpu_register_types)

On 6 February 2012 19:24, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 02/02/2012 08:59 PM, Andreas Färber wrote:

>> +#define CPU(obj) OBJECT_CHECK(CPU, (obj), TYPE_CPU)
>> +#define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU)
>> +#define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU)
>> +
>> +typedef struct CPU CPU;
>
> If this doesn't result in a build error, then C is a worse language than I
> thought it was :-/
>
> But we definitely shouldn't have a typename and macro name of the same thing
> and expect it to work...

The C standard says that function-like macros are only applied when
the function-like
macro name is followed by a '(' as the next preprocessing token, so this isn't
relying on any kind of gcc-specific behaviour.

Whether we *want* to do it is largely a style issue :-)

-- PMM

Am 06.02.2012 20:24, schrieb Anthony Liguori:
> On 02/02/2012 08:59 PM, Andreas Färber wrote:
>> +/**
>> + * cpu_common_reset:
>> + * @cpu: The CPU whose common state is to be reset.
>> + *
>> + * To be used by derived classes.
>> + */
>> +void cpu_common_reset(CPU *cpu);
> 
> Make this static, initialize reset = cpu_common_reset in
> cpu_class_initfn, then in the derived class initfn, save the pointer to
> the parent reset function so it can be called later.

I don't see how that would work. To initialize, e.g., the ARMCPUClass
with additional class fields I'm overriding the .class_init. So in order
to let CPUClass initialize the reset callback to its static one I'd need
to make CPU's class_init function non-static so that I can call that
from my derived class' class_init function, no?

Andreas

On 02/06/2012 02:14 PM, Andreas Färber wrote:
> Am 06.02.2012 20:24, schrieb Anthony Liguori:
>> On 02/02/2012 08:59 PM, Andreas Färber wrote:
>>> +/**
>>> + * cpu_common_reset:
>>> + * @cpu: The CPU whose common state is to be reset.
>>> + *
>>> + * To be used by derived classes.
>>> + */
>>> +void cpu_common_reset(CPU *cpu);
>>
>> Make this static, initialize reset = cpu_common_reset in
>> cpu_class_initfn, then in the derived class initfn, save the pointer to
>> the parent reset function so it can be called later.
>
> I don't see how that would work. To initialize, e.g., the ARMCPUClass
> with additional class fields I'm overriding the .class_init.

You're not overriding the class_init.  The class init for a type is called when 
that class is initialized for the first time.  See the documentation in object.h.

When class_init is called, the parent class type's class_init has already been 
called and the default values are set.  So:

static void arm_cpu_reset(CPUCommon *cpu)
{
    ARMCPU *s = ARM_CPU(cpu);
    ARMCPUClass *ac = ARM_CPU_GET_CLASS(s);

    // do arm specific reset

    // call super class reset
    ac->super_reset(cpu);
}

static void arm_cpu_class_initfn(ObjectClass *klass, void *data)
{
    CPUCommonClass *cc = CPU_COMMON_CLASS(klass);
    ARMCPUClass *ac = ARM_CPU_CLASS(klass);

    // cc->reset was set in CPUCommonClass's class_init
    ac->super_reset = cc->reset;
    cc->reset = arm_cpu_reset;
}

It's admittedly a little funky but this is the common idiom in gobject.  I'm not 
sure there's a better way.

Regards,

Anthony Liguori

  So in order
> to let CPUClass initialize the reset callback to its static one I'd need
> to make CPU's class_init function non-static so that I can call that
> from my derived class' class_init function, no?
>
> Andreas
>