QOM Super class access
cover QOM Super class access
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the save->override->call via FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.
Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the save->override->call issue. ARMCPU I've done a brief test on and seems to work.
ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (TYPE_ARM_CPU) need to super class itself (to TYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.
The main future work to the series is to apply the change pattern to the reset of the tree
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
i8254: Remove [KVM]PITClass
hw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
RFC PATCH 1/7 target-arm/cpu.c: delete un-needed instance/class sizes
From: Peter Crosthwaite <[email protected]>
QOM automatically inherits class and instance size from the parent class. No need to redefine as the same value as the parent.
---
target-arm/cpu.c | 2 -- 1 file changed, 2 deletions(-)
target-arm/cpu.c +0 -2
@@ -821,9 +821,7 @@ static void cpu_register(const ARMCPUInfo *info) {TypeInfo type_info = { .parent =TYPE_ARM_CPU , - .instance_size =sizeof (ARMCPU ), .instance_init = info->initfn, - .class_size =sizeof (ARMCPUClass), .class_init = info->class_init, };
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Re: [Qemu-devel] [RFC PATCH v1 1/7] target-arm/cpu.c: delete un-needed instance/class sizes
Am 18.06.2013 11:44, schrieb [email protected]:
Quoted 4 lines
From: Peter Crosthwaite <[email protected]>
QOM automatically inherits class and instance size from the parent class. No need to redefine as the same value as the parent.
It would be fair to mention since which commit because that was originally not the case. :)
Quoted 4 lines
Signed-off-by Peter Crosthwaite <[email protected]>
Anyway,
Andreas
Quoted 19 lines
---
target-arm/cpu.c | 2 -- 1 file changed, 2 deletions(-)target-arm/cpu.c +0
-2@@ -821,9 +821,7 @@ static void cpu_register(const ARMCPUInfo *info) {TypeInfo type_info = { .parent =TYPE_ARM_CPU , - .instance_size =sizeof (ARMCPU ), .instance_init = info->initfn, - .class_size =sizeof (ARMCPUClass), .class_init = info->class_init, };
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
RFC PATCH 2/7 qom: Add super class accessor
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
Add a function that allows obtaining a super class implementation of a given class. The super classing is done relative to a specified level.
See added documentation comment for details.
---
include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ 2 files changed, 33 insertions(+)
include/qom/object.h +18 -0
@@ -706,6 +706,24 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *klass,ObjectClass *object_class_get_parent (ObjectClass *klass);/** +* object_class_get_super: +* @klass: The class to obtain the super class for. +* @typename: The typename of the class level to super +* +* Returns: The super for @klass or %NULL if none. +* +* An example - suppose you have a inheritance heirachy: +* (parent) TYPE_A <- TYPE_B <- TYPE_C (concrete) +* object_class_get_parent(klass, TYPE_B) will return +* the ObjectClass for TYPE_A, if klass is either of TYPE_B +* or TYPE_C (or any other child class of TYPE_B). +* If typename is null, the typename is inferred as the concrete +* class level (functionaly identical to get_parent()). +*/ + +ObjectClass *object_class_get_super (ObjectClass *klass, +const char *typename); +/** * object_class_get_name: * @klass: The class to obtain the QOM typename for. *
qom/object.c +15 -0
@@ -608,6 +608,21 @@ ObjectClass *object_class_get_parent(ObjectClass *class)return type->class; } +/* FIXME: If this starts getting used in critical paths +* a cast cache should be added to save on the strcmp()s +*/ + +ObjectClass *object_class_get_super (ObjectClass *class, +const char *typename) +{ +/* Move up to the specified typelevel */ +while (class && typename &&strcmp (class->type->name, typename)) { + class =object_class_get_parent (class); + } +/* and go one more for the super class */ +return class ?object_class_get_parent (class) :NULL ; +} + typedef struct OCFData { void (*fn)(ObjectClass *klass, void *opaque);
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
RFC PATCH 3/7 qdev-core: Introduce DEVICE super class cast macros
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
Add macros DEVICE_SUPER_CLASS and DEVICE_GET_SUPER_CLASS. These are the similar to their respective non SUPER versions, except instead of returning the class object for the concrete class, they return their parent classes implementation (usually some form of abstract class).
---
include/hw/qdev-core.h | 4 ++++ 1 file changed, 4 insertions(+)
include/hw/qdev-core.h +4 -0
@@ -16,6 +16,10 @@ enum {#define DEVICE (obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)#define DEVICE_CLASS (klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)#define DEVICE_GET_CLASS (obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE) +#define DEVICE_SUPER_CLASS (klass, typename) \ + DEVICE_CLASS(object_class_get_super(OBJECT_CLASS(klass), typename)) +#define DEVICE_GET_SUPER_CLASS (obj, typename) \ + DEVICE_SUPER_CLASS(DEVICE_GET_CLASS(obj), typename)typedef int (*qdev_initfn )(DeviceState *dev);typedef int (*qdev_event )(DeviceState *dev);
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Re: [Qemu-devel] [RFC PATCH v1 3/7] qdev-core: Introduce DEVICE super class cast macros
Quoted 18 lines · On Tue, Jun 18, 2013 at 07:45:36PM +1000, [email protected] wrote:
From: Peter Crosthwaite <[email protected]>
Add macros
DEVICE_SUPER_CLASSandDEVICE_GET_SUPER_CLASS. These are the similar to their respective non SUPER versions, except instead of returning the class object for the concrete class, they return their parent classes implementation (usually some form of abstract class).Signed-off-by Peter Crosthwaite <[email protected]>---
include/hw/qdev-core.h | 4 ++++ 1 file changed, 4 insertions(+)include/hw/qdev-core.h +0
-0
include/hw/qdev-core.h enum { · line 22
Quoted 7 lines
include/hw/qdev-core.h +4
-0@@ -16,6 +16,10 @@ enum {#define DEVICE (obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)#define DEVICE_CLASS (klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)#define DEVICE_GET_CLASS (obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE) +#define DEVICE_SUPER_CLASS (klass, typename) \ + DEVICE_CLASS(object_class_get_super(OBJECT_CLASS(klass), typename)) +#define DEVICE_GET_SUPER_CLASS (obj, typename) \ + DEVICE_SUPER_CLASS(DEVICE_GET_CLASS(obj), typename)
DEVICE_GET_SUPER_CLASS is not a great name. It actually gets you a device not a class.
I know this mirrors DEVICE_GET_CLASS that's not
a great name too.
Generally in cases where we change the type of object but in fact return same pointer, it would be better to rename _GET to _CAST I think.
Quoted 5 lines
include/hw/qdev-core.h +0
-0typedef int (*qdev_initfn )(DeviceState *dev);typedef int (*qdev_event )(DeviceState *dev);Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
RFC PATCH 4/7 qom/cpu: Introduce CPU super class cast macros
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
Add macros CPU_SUPER_CLASS and CPU_GET_SUPER_CLASS. These are the similar to their respective non SUPER versions, except instead of returning the class object for the concrete class, they return their parent classes implementation (usually some form of abstract class).
---
include/qom/cpu.h | 4 ++++ 1 file changed, 4 insertions(+)
include/qom/cpu.h +4 -0
@@ -39,6 +39,10 @@ typedef int (*WriteCoreDumpFunction)(void *buf, size_t size, void *opaque);#define CPU (obj) OBJECT_CHECK(CPUState, (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) +#define CPU_SUPER_CLASS (klass, typename) \ + CPU_CLASS(object_class_get_super(OBJECT_CLASS(klass), typename)) +#define CPU_GET_SUPER_CLASS (obj, typename) \ + CPU_SUPER_CLASS(CPU_GET_CLASS(obj), typename)typedef struct CPUState CPUState ;
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
RFC PATCH 5/7 target-arm: Remove ARMCPUClass
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
ARMCPUClass is only needed for super-class abstract function access. Just use Super classes for reset and realize access and remove ARMCPUClass completely.
---
target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 14 +++++--------- 2 files changed, 5 insertions(+), 29 deletions(-)
target-arm/cpu-qom.h +0 -20
@@ -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:
target-arm/cpu.c +5 -9
@@ -60,7 +60,7 @@ 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_super =CPU_GET_SUPER_CLASS (cpu,TYPE_ARM_CPU );CPUARMState *env = &cpu->env;if (qemu_loglevel_mask (CPU_LOG_RESET )) {
@@ -68,7 +68,7 @@ static void arm_cpu_reset(CPUState *s)log_cpu_state (env,0 ); } - acc->parent_reset (s); + cc_super->reset (s);memset (env,0 , offsetof(CPUARMState , breakpoints));g_hash_table_foreach (cpu->cp_regs, cp_reg_reset, cpu);
@@ -158,7 +158,7 @@ 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_super =DEVICE_GET_SUPER_CLASS (dev,TYPE_ARM_CPU );CPUARMState *env = &cpu->env;/* Some features automatically imply others: */
@@ -207,7 +207,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)cpu_reset (CPU (cpu));qemu_init_vcpu (env); - acc->parent_realize (dev, errp); + dc_super->realize (dev, errp); }/* CPU models */
@@ -802,14 +802,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;
@@ -837,7 +834,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, };
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
RFC PATCH 6/7 target-microblaze: Remove MicroblazeCPUClass
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
MicroblazeCPUClass is only needed for super-class abstract function access. Just use Super classes for reset and realize access and remove MicroblazeCPUClass completely.
---
target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 2 files changed, 4 insertions(+), 29 deletions(-)
target-microblaze/cpu-qom.h +0 -20
@@ -24,28 +24,8 @@#define TYPE_MICROBLAZE_CPU "microblaze-cpu" -#define MICROBLAZE_CPU_CLASS (klass) \ - OBJECT_CLASS_CHECK(MicroBlazeCPUClass, (klass), TYPE_MICROBLAZE_CPU)#define MICROBLAZE_CPU (obj) \ OBJECT_CHECK(MicroBlazeCPU, (obj), TYPE_MICROBLAZE_CPU) -#define MICROBLAZE_CPU_GET_CLASS (obj) \ - OBJECT_GET_CLASS(MicroBlazeCPUClass, (obj), TYPE_MICROBLAZE_CPU) - -/** -* MicroBlazeCPUClass: -* @parent_realize: The parent class' realize handler. -* @parent_reset: The parent class' reset handler. -* -* A MicroBlaze CPU model. -*/ -typedef struct MicroBlazeCPUClass { -/*< private >*/ -CPUClass parent_class; -/*< public >*/ - -DeviceRealize parent_realize; -void (*parent_reset)(CPUState *cpu); -}MicroBlazeCPUClass ;/** * MicroBlazeCPU:
target-microblaze/cpu.c +4 -9
@@ -25,12 +25,11 @@#include "hw/qdev-properties.h" #include "migration/vmstate.h" -/* CPUClass::reset() */ static void mb_cpu_reset (CPUState *s) {MicroBlazeCPU *cpu =MICROBLAZE_CPU (s); -MicroBlazeCPUClass *mcc =MICROBLAZE_CPU_GET_CLASS (cpu); +CPUClass *cc_super =CPU_GET_SUPER_CLASS (cpu,TYPE_MICROBLAZE_CPU );CPUMBState *env = &cpu->env;if (qemu_loglevel_mask (CPU_LOG_RESET )) {
@@ -38,7 +37,7 @@ static void mb_cpu_reset(CPUState *s)log_cpu_state (env,0 ); } - mcc->parent_reset (s); + cc_super->reset (s);memset (env,0 , offsetof(CPUMBState , breakpoints)); env->res_addr =RES_ADDR_NONE ;
@@ -89,12 +88,12 @@ static void mb_cpu_reset(CPUState *s)static void mb_cpu_realizefn (DeviceState *dev,Error **errp) {MicroBlazeCPU *cpu =MICROBLAZE_CPU (dev); -MicroBlazeCPUClass *mcc =MICROBLAZE_CPU_GET_CLASS (dev); +DeviceClass *dc_super =DEVICE_GET_SUPER_CLASS (dev,TYPE_MICROBLAZE_CPU );cpu_reset (CPU (cpu));qemu_init_vcpu (&cpu->env); - mcc->parent_realize (dev, errp); + dc_super->realize (dev, errp); }static void mb_cpu_initfn (Object *obj)
@@ -129,12 +128,9 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data) {DeviceClass *dc =DEVICE_CLASS (oc);CPUClass *cc =CPU_CLASS (oc); -MicroBlazeCPUClass *mcc =MICROBLAZE_CPU_CLASS (oc); - mcc->parent_realize = dc->realize; dc->realize = mb_cpu_realizefn; - mcc->parent_reset = cc->reset; cc->reset = mb_cpu_reset; cc->do_interrupt = mb_cpu_do_interrupt;
@@ -148,7 +144,6 @@ static const TypeInfo mb_cpu_type_info = { .parent = TYPE_CPU, .instance_size = sizeof(MicroBlazeCPU), .instance_init = mb_cpu_initfn, - .class_size = sizeof(MicroBlazeCPUClass), .class_init = mb_cpu_class_init, };
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
RFC PATCH 7/7 i8254: Remove [KVM]PITClass
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
[KVM]PITClass is only needed for super-class realize function access. Just use Super classes for realize access and remove [KVM]PITClass completely.
---
hw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- 2 files changed, 4 insertions(+), 29 deletions(-)
hw/i386/kvm/i8254.c +2 -15
@@ -33,10 +33,6 @@#define CALIBRATION_ROUNDS 3#define KVM_PIT (obj) OBJECT_CHECK(KVMPITState, (obj), TYPE_KVM_I8254) -#define KVM_PIT_CLASS (class) \ - OBJECT_CLASS_CHECK(KVMPITClass, (class), TYPE_KVM_I8254) -#define KVM_PIT_GET_CLASS (obj) \ - OBJECT_GET_CLASS(KVMPITClass, (obj), TYPE_KVM_I8254) typedef struct KVMPITState { PITCommonState parent_obj;
@@ -46,12 +42,6 @@ typedef struct KVMPITState {int64_t kernel_clock_offset; } KVMPITState; -typedef struct KVMPITClass { -PITCommonClass parent_class; - -DeviceRealize parent_realize; -}KVMPITClass ; -static int64_t abs64 (int64_t v) {return v <0 ? -v : v;
@@ -250,7 +240,7 @@ static void kvm_pit_vm_state_change(void *opaque, int running,static void kvm_pit_realizefn (DeviceState *dev,Error **errp) {PITCommonState *pit =PIT_COMMON (dev); -KVMPITClass *kpc =KVM_PIT_GET_CLASS (dev); +DeviceClass *dc_super =DEVICE_GET_SUPER_CLASS (dev,TYPE_KVM_I8254 );KVMPITState *s =KVM_PIT (pit);struct kvm_pit_config config = { .flags =0 ,
@@ -294,7 +284,7 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp)qemu_add_vm_change_state_handler (kvm_pit_vm_state_change, s); - kpc->parent_realize (dev, errp); + dc_super->realize (dev, errp); }static Property kvm_pit_properties[] = {
@@ -306,11 +296,9 @@ static Property kvm_pit_properties[] = {static void kvm_pit_class_init (ObjectClass *klass,void *data) { -KVMPITClass *kpc =KVM_PIT_CLASS (klass);PITCommonClass *k =PIT_COMMON_CLASS (klass);DeviceClass *dc =DEVICE_CLASS (klass); - kpc->parent_realize = dc->realize; dc->realize = kvm_pit_realizefn; k->set_channel_gate = kvm_pit_set_gate; k->get_channel_info = kvm_pit_get_channel_info;
@@ -325,7 +313,6 @@ static const TypeInfo kvm_pit_info = { .parent = TYPE_PIT_COMMON, .instance_size = sizeof(KVMPITState), .class_init = kvm_pit_class_init, - .class_size = sizeof(KVMPITClass), };static void kvm_pit_register (void )
hw/timer/i8254.c +2 -14
@@ -35,15 +35,6 @@#define RW_STATE_WORD0 3#define RW_STATE_WORD1 4 -#define PIT_CLASS (class) OBJECT_CLASS_CHECK(PITClass, (class), TYPE_I8254) -#define PIT_GET_CLASS (obj) OBJECT_GET_CLASS(PITClass, (obj), TYPE_I8254) - -typedef struct PITClass { -PITCommonClass parent_class; - -DeviceRealize parent_realize; -}PITClass ; -static void pit_irq_timer_update (PITChannelState *s,int64_t current_time);static int pit_get_count (PITChannelState *s)
@@ -325,7 +316,7 @@ static void pit_post_load(PITCommonState *s)static void pit_realizefn (DeviceState *dev,Error **err) {PITCommonState *pit =PIT_COMMON (dev); -PITClass *pc =PIT_GET_CLASS (dev); +DeviceClass *dc_super =DEVICE_GET_SUPER_CLASS (dev,TYPE_I8254 );PITChannelState *s; s = &pit->channels[0 ];
@@ -337,7 +328,7 @@ static void pit_realizefn(DeviceState *dev, Error **err)qdev_init_gpio_in (dev, pit_irq_control,1 ); - pc->parent_realize (dev, err); + dc_super->realize (dev, err); }static Property pit_properties[] = {
@@ -347,11 +338,9 @@ static Property pit_properties[] = {static void pit_class_initfn (ObjectClass *klass,void *data) { -PITClass *pc =PIT_CLASS (klass);PITCommonClass *k =PIT_COMMON_CLASS (klass);DeviceClass *dc =DEVICE_CLASS (klass); - pc->parent_realize = dc->realize; dc->realize = pit_realizefn; k->set_channel_gate = pit_set_channel_gate; k->get_channel_info = pit_get_channel_info_common;
@@ -365,7 +354,6 @@ static const TypeInfo pit_info = { .parent = TYPE_PIT_COMMON, .instance_size = sizeof(PITCommonState), .class_init = pit_class_initfn, - .class_size = sizeof(PITClass), };static void pit_register_types (void )
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Hi Peter,
Am 18.06.2013 11:43, schrieb [email protected]:
Quoted 7 lines
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.
Quoted 8 lines
Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.
Thanks for experimenting with this. Anthony had asked to give him more review time on my virtio series before choosing a path to pursue there.
Quoted 11 lines
The main future work to the series is to apply the change pattern to the reset of the tree
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
Still need to review the new macros in-depth, but I'm skeptical about removing *CPUClass'es while CPUState conversions are still ongoing ...
Quoted 1 line
i8254: Remove [KVM]PITClass
... whereas this one was purely introduced for QOM realize, so OK.
Also the subjects are a bit misleading, suggest something like "...: Use super class macro".
Cheers,
Andreas
Quoted 11 lines
hw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Hi Andreas,
Quoted 34 lines · Andreas Färber
Hi Peter,
Am 18.06.2013 11:43, schrieb [email protected]:
Quoted 7 lines
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Quoted 8 lines
Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.Thanks for experimenting with this. Anthony had asked to give him more review time on my virtio series before choosing a path to pursue there.
Quoted 11 lines
The main future work to the series is to apply the change pattern to the reset of the tree
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
Quoted 3 lines
Still need to review the new macros in-depth, but I'm skeptical about removing *CPUClass'es while CPUState conversions are still ongoing ...
Sure thing,
Its likely that ARMCPUClass (being an abstraction in its own right) will pick up a few abstractions in full QOMificiation so that one is very likely to come back to life. Microblaze I doubt will every become an abstraction due to its very dynamic nature - our plan is to use static properties to differentiate Microblaze variants rather than (ARM style) sub-classes.
Quoted 7 lines
Quoted 1 line
i8254: Remove [KVM]PITClass
... whereas this one was purely introduced for QOM realize, so OK.
Also the subjects are a bit misleading, suggest something like "...: Use super class macro".
Ok,
I'll wait on Anthonys comments before a respin anyways.
Regards,
Peter
Quoted 19 lines
Cheers,
AndreasQuoted 11 lines
hw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 11 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.
Quoted 7 lines
ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.
So, why do we need the new APIs with _SUPER?
What's wrong with simple
object_class_by_name()
and casting to that?
Quoted 24 lines
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
i8254: Remove [KVM]PITClasshw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Hi Michael,
Quoted 17 lines · Michael S. Tsirkin
Quoted 16 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.
Quoted 6 lines
Quoted 2 lines
The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.
s/Super/parent might be the go. But it is designed to work like py/java. Its the immediate parent of the specified level, and it is analogous to the java super.
Quoted 5 lines
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.
So, why do we need the new APIs with
_SUPER?
The SUPER APIs have a nice consistent appearance with the GET_CLASS and FOO_CLASS APIs and they are likely to live alongside each other in QOM fns.
Quoted 4 lines
What's wrong with simple
object_class_by_name()
and casting to that?
There a performance consequence - object_class_by_name is a lookup, whereas this approach is able to just walk the pointer through the inheritance heirachy till it hits. Tying it to an object also brings into play the possibility of a cast cache should that be needed.
Thanks for the review.
Regards,
Peter
Quoted 26 lines
Quoted 25 lines
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
i8254: Remove [KVM]PITClasshw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)--
1.8.3.rc1.44.gb387c77.dirty
Quoted 41 lines · Peter Crosthwaite
Hi Michael,
Quoted 17 lines · Michael S. Tsirkin
Quoted 16 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.Quoted 6 lines
Quoted 2 lines
The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.s/Super/parent might be the go. But it is designed to work like py/java. Its the immediate parent of the specified level, and it is analogous to the java super.
Quoted 5 lines
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.
So, why do we need the new APIs with
_SUPER?The SUPER APIs have a nice consistent appearance with the
GET_CLASSandFOO_CLASSAPIs and they are likely to live alongside each other in QOM fns.
Quoted 8 lines
Quoted 4 lines
What's wrong with simple
object_class_by_name()
and casting to that?There a performance consequence -
object_class_by_nameis a lookup, whereas this approach is able to just walk the pointer through the inheritance heirachy till it hits.
None of these uses has a chance to be at all performance sensitive.
Quoted 2 lines
Tying it to an object also brings
into play the possibility of a cast cache should that be needed.
Doesn't make sense to me. This is object construction, how many classes do you expect there to be so this would affect qemu startup speed noticeably? 1000000?
Quoted 31 lines
Thanks for the review.
Regards,
PeterQuoted 26 lines
Quoted 25 lines
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
i8254: Remove [KVM]PITClasshw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)--
1.8.3.rc1.44.gb387c77.dirty
HI Michael,
Quoted 55 lines · Michael S. Tsirkin
Quoted 41 lines · Peter Crosthwaite
Hi Michael,
Quoted 17 lines · Michael S. Tsirkin
Quoted 16 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.Quoted 6 lines
Quoted 2 lines
The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.s/Super/parent might be the go. But it is designed to work like py/java. Its the immediate parent of the specified level, and it is analogous to the java super.
Quoted 5 lines
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.
So, why do we need the new APIs with
_SUPER?The SUPER APIs have a nice consistent appearance with the
GET_CLASSandFOO_CLASSAPIs and they are likely to live alongside each other in QOM fns.Quoted 8 lines
Quoted 4 lines
What's wrong with simple
object_class_by_name()
and casting to that?There a performance consequence -
object_class_by_nameis a lookup, whereas this approach is able to just walk the pointer through the inheritance heirachy till it hits.None of these uses has a chance to be at all performance sensitive.
Quoted 8 lines
Quoted 2 lines
Tying it to an object also brings
into play the possibility of a cast cache should that be needed.Doesn't make sense to me. This is object construction, how many classes do you expect there to be so this would affect qemu startup speed noticeably? 1000000?
The idea is this is usable in any overridden QOM fn. True, our only current need for such a system is realize, reset and unrealize, but that could change in the future with more sophistaicated QOM setups where hot-path run time functionality has QOM heirachy as well.
Regards,
Peter
Quoted 32 lines
Quoted 31 lines
Thanks for the review.
Regards,
PeterQuoted 26 lines
Quoted 25 lines
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
i8254: Remove [KVM]PITClasshw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)--
1.8.3.rc1.44.gb387c77.dirty
Quoted 67 lines · Peter Crosthwaite
HI Michael,
Quoted 55 lines · Michael S. Tsirkin
Quoted 41 lines · Peter Crosthwaite
Hi Michael,
Quoted 17 lines · Michael S. Tsirkin
Quoted 16 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.Quoted 6 lines
Quoted 2 lines
The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.s/Super/parent might be the go. But it is designed to work like py/java. Its the immediate parent of the specified level, and it is analogous to the java super.
Quoted 5 lines
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.
So, why do we need the new APIs with
_SUPER?The SUPER APIs have a nice consistent appearance with the
GET_CLASSandFOO_CLASSAPIs and they are likely to live alongside each other in QOM fns.Quoted 8 lines
Quoted 4 lines
What's wrong with simple
object_class_by_name()
and casting to that?There a performance consequence -
object_class_by_nameis a lookup, whereas this approach is able to just walk the pointer through the inheritance heirachy till it hits.None of these uses has a chance to be at all performance sensitive.
Quoted 8 lines
Quoted 2 lines
Tying it to an object also brings
into play the possibility of a cast cache should that be needed.Doesn't make sense to me. This is object construction, how many classes do you expect there to be so this would affect qemu startup speed noticeably? 1000000?
Quoted 7 lines
The idea is this is usable in any overridden QOM fn. True, our only current need for such a system is realize, reset and unrealize, but that could change in the future with more sophistaicated QOM setups where hot-path run time functionality has QOM heirachy as well.
Regards,
Peter
It seems unlikely hot-path does anything
that needs to walk the hierarchy like that.
Premature optimization and all that.
Quoted 32 lines
Quoted 32 lines
Quoted 31 lines
Thanks for the review.
Regards,
PeterQuoted 26 lines
Quoted 25 lines
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
i8254: Remove [KVM]PITClasshw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)--
1.8.3.rc1.44.gb387c77.dirty
Am 18.06.2013 12:39, schrieb Michael S. Tsirkin:
Quoted 55 lines · Peter Crosthwaite
Quoted 41 lines
Hi Michael,
Quoted 17 lines · Michael S. Tsirkin
Quoted 16 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.Quoted 6 lines
Quoted 2 lines
The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.s/Super/parent might be the go. But it is designed to work like py/java. Its the immediate parent of the specified level, and it is analogous to the java super.
Quoted 5 lines
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.
So, why do we need the new APIs with
_SUPER?The SUPER APIs have a nice consistent appearance with the
GET_CLASSandFOO_CLASSAPIs and they are likely to live alongside each other in QOM fns.Quoted 8 lines
Quoted 4 lines
What's wrong with simple
object_class_by_name()
and casting to that?There a performance consequence -
object_class_by_nameis a lookup, whereas this approach is able to just walk the pointer through the inheritance heirachy till it hits.None of these uses has a chance to be at all performance sensitive.
Quoted 7 lines
Quoted 2 lines
Tying it to an object also brings
into play the possibility of a cast cache should that be needed.Doesn't make sense to me. This is object construction, how many classes do you expect there to be so this would affect qemu startup speed noticeably? 1000000?
It's not just about startup, also about CPU reset for instance. But still not that performance-sensitive.
If we want to performance-optimize it, sharing code with object_class_by_name() or any other class-comparing function would be nice.
Andreas
Quoted 32 lines
Quoted 32 lines
Thanks for the review.
Regards,
PeterQuoted 26 lines
Quoted 25 lines
Peter Crosthwaite (7):
target-arm/cpu.c: delete un-needed instance/class sizes
qom: Add super class accessor
qdev-core: Introduce DEVICE super class cast macros qom/cpu: Introduce CPU super class cast macros
target-arm: Remove ARMCPUClass
target-microblaze: Remove MicroblazeCPUClass
i8254: Remove [KVM]PITClasshw/i386/kvm/i8254.c | 17 ++--------------- hw/timer/i8254.c | 16 ++-------------- include/hw/qdev-core.h | 4 ++++ include/qom/cpu.h | 4 ++++ include/qom/object.h | 18 ++++++++++++++++++ qom/object.c | 15 +++++++++++++++ target-arm/cpu-qom.h | 20 -------------------- target-arm/cpu.c | 16 +++++----------- target-microblaze/cpu-qom.h | 20 -------------------- target-microblaze/cpu.c | 13 ++++--------- 10 files changed, 54 insertions(+), 89 deletions(-)--
1.8.3.rc1.44.gb387c77.dirty
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Am 18.06.2013 12:23, schrieb Michael S. Tsirkin:
Quoted 27 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
Quoted 11 lines
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.Quoted 7 lines
ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.
Quoted 4 lines
So, why do we need the new APIs with
_SUPER?
What's wrong with simple
object_class_by_name()
and casting to that?
I guess the idea was to avoid open-coding that multiple times, but I think I would then prefer something more local like
#define ARM_CPU_SUPER_CLASS() \
object_class_get_parent(object_class_by_name(TYPE_ARM_CPU))
and then use
DEVICE_CLASS(ARM_CPU_SUPER_CLASS())
or
CPU_CLASS(ARM_CPU_SUPER_CLASS())
as needed. What do you think?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 33 lines · Andreas Färber
Am 18.06.2013 12:23, schrieb Michael S. Tsirkin:
Quoted 27 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
Quoted 11 lines
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.Quoted 7 lines
ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.Quoted 4 lines
So, why do we need the new APIs with
_SUPER?
What's wrong with simple
object_class_by_name()
and casting to that?
Quoted 5 lines
I guess the idea was to avoid open-coding that multiple times, but I think I would then prefer something more local like
#define
ARM_CPU_SUPER_CLASS()\
object_class_get_parent(object_class_by_name(TYPE_ARM_CPU))
s/SUPER_CLASS/PARENT_CLASS/
Super class also confusingly means "very good".
Quoted 13 lines
and then use
DEVICE_CLASS(ARM_CPU_SUPER_CLASS())
or
CPU_CLASS(ARM_CPU_SUPER_CLASS())as needed. What do you think?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 33 lines · Andreas Färber
Am 18.06.2013 12:23, schrieb Michael S. Tsirkin:
Quoted 27 lines · On Tue, Jun 18, 2013 at 07:43:11PM +1000, [email protected] wrote:
Quoted 11 lines
From: Peter Crosthwaite <[email protected]>
This series enables QOM super class access and demostrates some usages. Replaces the
save->override->callvia FooClass technique, to reduce some of the boiler plate in recently fully QOMified devices.Applied the change to ARM CPU, MB CPU and some of Andreas's recently QOMified i386 devices, all which have the
save->override->callissue. ARMCPU I've done a brief test on and seems to work.Quoted 7 lines
ARM CPU was particularly difficult, as it has 3 layers of heirachy, where a non-concrete class (
TYPE_ARM_CPU) need to super class itself (toTYPE_CPU). This sees the need for super-classers to specify their expected base class level. See patches for illustration.The main future work to the series is to apply the change pattern to the reset of the tree
Looks good to me overall.
Some nits:
- Super is an immediate parent in java and python.
- One of the design points of QOM is that it let you ignore which class is a parent and which is a child. All casts look the same.Quoted 4 lines
So, why do we need the new APIs with
_SUPER?
What's wrong with simple
object_class_by_name()
and casting to that?
Quoted 5 lines
I guess the idea was to avoid open-coding that multiple times, but I think I would then prefer something more local like
#define
ARM_CPU_SUPER_CLASS()\
object_class_get_parent(object_class_by_name(TYPE_ARM_CPU))
This can simply be object_class_get_parent_by_type(), then we don't have to define a macro for it everytime.
Quoted 14 lines
and then use
DEVICE_CLASS(ARM_CPU_SUPER_CLASS())
or
CPU_CLASS(ARM_CPU_SUPER_CLASS())as needed. What do you think?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg