[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qom-next v2 3/5] target-arm: Use parent classes
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH qom-next v2 3/5] target-arm: Use parent classes for reset + realize |
Date: |
Thu, 11 Jul 2013 12:36:42 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 |
Am 11.07.2013 12:31, schrieb Michael S. Tsirkin:
> On Thu, Jul 11, 2013 at 11:47:59AM +0200, Igor Mammedov wrote:
>> On Thu, 11 Jul 2013 11:47:16 +1000
>> address@hidden wrote:
>>
>>> From: Peter Crosthwaite <address@hidden>
>>>
>>> ARMCPUClass is only needed for parent-class abstract function access.
>>> Just use parent classes for reset and realize access and remove
>>> ARMCPUClass completely.
>>>
>>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>> ---
>>>
>>> target-arm/cpu-qom.h | 20 --------------------
>>> target-arm/cpu.c | 16 +++++++---------
>>> 2 files changed, 7 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>>> index ef6261f..bdad93a 100644
>>> --- a/target-arm/cpu-qom.h
>>> +++ b/target-arm/cpu-qom.h
>>> @@ -24,28 +24,8 @@
>>>
>>> #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_realize: The parent class' realize handler.
>>> - * @parent_reset: The parent class' reset handler.
>>> - *
>>> - * An ARM CPU model.
>>> - */
>>> -typedef struct ARMCPUClass {
>>> - /*< private >*/
>>> - CPUClass parent_class;
>>> - /*< public >*/
>>> -
>>> - DeviceRealize parent_realize;
>>> - void (*parent_reset)(CPUState *cpu);
>>> -} ARMCPUClass;
>>>
>>> /**
>>> * ARMCPU:
>>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>>> index ed53df8..ad5ec7b 100644
>>> --- a/target-arm/cpu.c
>>> +++ b/target-arm/cpu.c
>>> @@ -60,7 +60,8 @@ static void cp_reg_reset(gpointer key, gpointer value,
>>> gpointer opaque)
>>> static void arm_cpu_reset(CPUState *s)
>>> {
>>> ARMCPU *cpu = ARM_CPU(s);
>>> - ARMCPUClass *acc = ARM_CPU_GET_CLASS(cpu);
>>> + CPUClass *cc_parent =
>>> + CPU_CLASS(object_class_get_parent_by_name(TYPE_ARM_CPU));
>> Maybe object_class_get_parent_of_type() would be less confusing?
>>
>> This usage assumes that parent of TYPE_ARM_CPU is TYPE_CPU and if
>> another TYPE_X added between them, it might break if TYPE_X doesn't
>> re-implement this logic in its reset.
>
> If what's needed is TYPE_CPU, you can just look up the class by name.
My suggestion was to hide the implementation details behind an
ARM_CPU_GET_PARENT_CLASS(obj) macro.
That would at the same time allow to later switch to an iterative
approach as seen in v1 without a whole lot of refactoring.
Andreas
>
>> Could reset be modeled like DEVICE.instance_init() instead? Then
>> no explicit access to parent from child would be needed and it
>> still leaves possibility to override resets if parent->child
>> propagation order is not desirable for a particular device.
>>
>>> CPUARMState *env = &cpu->env;
>>>
>>> if (qemu_loglevel_mask(CPU_LOG_RESET)) {
>>> @@ -68,7 +69,7 @@ static void arm_cpu_reset(CPUState *s)
>>> log_cpu_state(env, 0);
>>> }
>>>
>>> - acc->parent_reset(s);
>>> + cc_parent->reset(s);
>>>
>>> memset(env, 0, offsetof(CPUARMState, breakpoints));
>>> g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu);
>>> @@ -158,7 +159,8 @@ static void arm_cpu_finalizefn(Object *obj)
>>> static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>>> {
>>> ARMCPU *cpu = ARM_CPU(dev);
>>> - ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
>>> + DeviceClass *dc_parent =
>>> + DEVICE_CLASS(object_class_get_parent_by_name(TYPE_ARM_CPU));
>>> CPUARMState *env = &cpu->env;
>>>
>>> /* Some features automatically imply others: */
>>> @@ -209,7 +211,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
>>> **errp)
>>>
>>> cpu_reset(CPU(cpu));
>>>
>>> - acc->parent_realize(dev, errp);
>>> + dc_parent->realize(dev, errp);
>>> }
>>>
>>> /* CPU models */
>>> @@ -803,14 +805,11 @@ static const ARMCPUInfo arm_cpus[] = {
>>>
>>> static void arm_cpu_class_init(ObjectClass *oc, void *data)
>>> {
>>> - ARMCPUClass *acc = ARM_CPU_CLASS(oc);
>>> - CPUClass *cc = CPU_CLASS(acc);
>>> + CPUClass *cc = CPU_CLASS(oc);
>>> DeviceClass *dc = DEVICE_CLASS(oc);
>>>
>>> - acc->parent_realize = dc->realize;
>>> dc->realize = arm_cpu_realizefn;
>>>
>>> - acc->parent_reset = cc->reset;
>>> cc->reset = arm_cpu_reset;
>>>
>>> cc->class_by_name = arm_cpu_class_by_name;
>>> @@ -839,7 +838,6 @@ static const TypeInfo arm_cpu_type_info = {
>>> .instance_init = arm_cpu_initfn,
>>> .instance_finalize = arm_cpu_finalizefn,
>>> .abstract = true,
>>> - .class_size = sizeof(ARMCPUClass),
>>> .class_init = arm_cpu_class_init,
>>> };
>>>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH qom-next v2 4/5] target-microblaze: Use parent class for reset + realize, peter . crosthwaite, 2013/07/10
[Qemu-devel] [PATCH qom-next v2 5/5] i8254: Use parent class for realize, peter . crosthwaite, 2013/07/10