[Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>


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(-)

-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [RFC PATCH v1 1/7] target-arm/cpu.c: delete un-needed instance/class sizes

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

QOM automatically inherits class and instance size from the parent
class. No need to redefine as the same value as the parent.

CC: qemu-trivial@nongnu.org

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 target-arm/cpu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 496a59f..47a21ec 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -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,
     };
 
-- 
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

[Andreas Färber]

Am 18.06.2013 11:44, schrieb peter.crosthwaite@xilinx.com:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> 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. :)

> 
> CC: qemu-trivial@nongnu.org
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Anyway,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas

> ---
> 
>  target-arm/cpu.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 496a59f..47a21ec 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -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,
>      };
>  

-- 
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] [RFC PATCH v1 2/7] qom: Add super class accessor

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/qom/object.h | 18 ++++++++++++++++++
 qom/object.c         | 15 +++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 23fc048..034c87c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -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.
  *
diff --git a/qom/object.c b/qom/object.c
index 803b94b..69d4889 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -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);
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [RFC PATCH v1 3/7] qdev-core: Introduce DEVICE super class cast macros

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/hw/qdev-core.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7fbffcb..96647c4 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -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);
-- 
1.8.3.rc1.44.gb387c77.dirty

Re: [Qemu-devel] [RFC PATCH v1 3/7] qdev-core: Introduce DEVICE super class cast macros

[Michael S. Tsirkin]

On Tue, Jun 18, 2013 at 07:45:36PM +1000, peter.crosthwaite@xilinx.com wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> 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).
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> 
>  include/hw/qdev-core.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 7fbffcb..96647c4 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -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.

>  
>  typedef int (*qdev_initfn)(DeviceState *dev);
>  typedef int (*qdev_event)(DeviceState *dev);
> -- 
> 1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [RFC PATCH v1 4/7] qom/cpu: Introduce CPU super class cast macros

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/qom/cpu.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index a5bb515..50d478a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -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;
 
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [RFC PATCH v1 5/7] target-arm: Remove ARMCPUClass

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

ARMCPUClass is only needed for super-class abstract function access.
Just use Super classes for reset and realize access and remove
ARMCPUClass completely.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 target-arm/cpu-qom.h | 20 --------------------
 target-arm/cpu.c     | 14 +++++---------
 2 files changed, 5 insertions(+), 29 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 12fcefe..c937cd7 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 47a21ec..a64de0f 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -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,
 };
 
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [RFC PATCH v1 6/7] target-microblaze: Remove MicroblazeCPUClass

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

MicroblazeCPUClass is only needed for super-class abstract function
access. Just use Super classes for reset and realize access and remove
MicroblazeCPUClass completely.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 target-microblaze/cpu-qom.h | 20 --------------------
 target-microblaze/cpu.c     | 13 ++++---------
 2 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
index ce92a4e..a9ae2da 100644
--- a/target-microblaze/cpu-qom.h
+++ b/target-microblaze/cpu-qom.h
@@ -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:
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 404f82c..02af2af 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -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,
 };
 
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [RFC PATCH v1 7/7] i8254: Remove [KVM]PITClass

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

[KVM]PITClass is only needed for super-class realize function access.
Just use Super classes for realize access and remove [KVM]PITClass
completely.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/i386/kvm/i8254.c | 17 ++---------------
 hw/timer/i8254.c    | 16 ++--------------
 2 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c
index 93a3669..f344a23 100644
--- a/hw/i386/kvm/i8254.c
+++ b/hw/i386/kvm/i8254.c
@@ -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)
diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c
index 16c8dd6..afccb28 100644
--- a/hw/timer/i8254.c
+++ b/hw/timer/i8254.c
@@ -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)
-- 
1.8.3.rc1.44.gb387c77.dirty

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Andreas Färber]

Hi Peter,

Am 18.06.2013 11:43, schrieb peter.crosthwaite@xilinx.com:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> 
> 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.

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.

> 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 ...

>   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

>  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

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Peter Crosthwaite]

Hi Andreas,

On Tue, Jun 18, 2013 at 8:12 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hi Peter,
>
> Am 18.06.2013 11:43, schrieb peter.crosthwaite@xilinx.com:
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>>
>> 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.
>
> 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.
>
>> 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 ...
>

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.

>>   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

> Cheers,
> Andreas
>
>>  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
>

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Michael S. Tsirkin]

On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> 
> 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

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?


> 
> 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(-)
> 
> -- 
> 1.8.3.rc1.44.gb387c77.dirty

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Peter Crosthwaite]

Hi Michael,

On Tue, Jun 18, 2013 at 8:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>>
>> 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
>
> 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.

> - 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.

> 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

>
>>
>> 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(-)
>>
>> --
>> 1.8.3.rc1.44.gb387c77.dirty
>

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Michael S. Tsirkin]

On Tue, Jun 18, 2013 at 08:35:22PM +1000, Peter Crosthwaite wrote:
> Hi Michael,
> 
> On Tue, Jun 18, 2013 at 8:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
> >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >>
> >>
> >> 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
> >
> > 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.
> 
> > - 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.
> 
> > 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.

None of these uses has a chance to be at all performance
sensitive.


> 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?

> Thanks for the review.
> 
> Regards,
> Peter
> 
> >
> >>
> >> 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(-)
> >>
> >> --
> >> 1.8.3.rc1.44.gb387c77.dirty
> >

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Peter Crosthwaite]

HI Michael,

On Tue, Jun 18, 2013 at 8:39 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Jun 18, 2013 at 08:35:22PM +1000, Peter Crosthwaite wrote:
>> Hi Michael,
>>
>> On Tue, Jun 18, 2013 at 8:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
>> >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> >>
>> >>
>> >> 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
>> >
>> > 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.
>>
>> > - 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.
>>
>> > 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.
>
> None of these uses has a chance to be at all performance
> sensitive.
>
>
>> 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

>> Thanks for the review.
>>
>> Regards,
>> Peter
>>
>> >
>> >>
>> >> 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(-)
>> >>
>> >> --
>> >> 1.8.3.rc1.44.gb387c77.dirty
>> >
>

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Michael S. Tsirkin]

On Tue, Jun 18, 2013 at 08:44:23PM +1000, Peter Crosthwaite wrote:
> HI Michael,
> 
> On Tue, Jun 18, 2013 at 8:39 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Jun 18, 2013 at 08:35:22PM +1000, Peter Crosthwaite wrote:
> >> Hi Michael,
> >>
> >> On Tue, Jun 18, 2013 at 8:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
> >> >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >> >>
> >> >>
> >> >> 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
> >> >
> >> > 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.
> >>
> >> > - 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.
> >>
> >> > 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.
> >
> > None of these uses has a chance to be at all performance
> > sensitive.
> >
> >
> >> 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

It seems unlikely hot-path does anything
that needs to walk the hierarchy like that.
Premature optimization and all that.


> >> Thanks for the review.
> >>
> >> Regards,
> >> Peter
> >>
> >> >
> >> >>
> >> >> 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(-)
> >> >>
> >> >> --
> >> >> 1.8.3.rc1.44.gb387c77.dirty
> >> >
> >

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Andreas Färber]

Am 18.06.2013 12:39, schrieb Michael S. Tsirkin:
> On Tue, Jun 18, 2013 at 08:35:22PM +1000, Peter Crosthwaite wrote:
>> Hi Michael,
>>
>> On Tue, Jun 18, 2013 at 8:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
>>>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>
>>>>
>>>> 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
>>>
>>> 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.
>>
>>> - 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.
>>
>>> 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.
> 
> None of these uses has a chance to be at all performance
> sensitive.
> 
> 
>> 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

> 
>> Thanks for the review.
>>
>> Regards,
>> Peter
>>
>>>
>>>>
>>>> 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(-)
>>>>
>>>> --
>>>> 1.8.3.rc1.44.gb387c77.dirty
>>>


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Andreas Färber]

Am 18.06.2013 12:23, schrieb Michael S. Tsirkin:
> On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>>
>> 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
> 
> 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?

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

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Michael S. Tsirkin]

On Tue, Jun 18, 2013 at 12:41:29PM +0200, Andreas Färber wrote:
> Am 18.06.2013 12:23, schrieb Michael S. Tsirkin:
> > On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
> >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >>
> >>
> >> 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
> > 
> > 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?
> 
> 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".

> and then use
> 
> DEVICE_CLASS(ARM_CPU_SUPER_CLASS())
> or
> CPU_CLASS(ARM_CPU_SUPER_CLASS())
> 
> as needed. What do you think?
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Hu Tao]

On Tue, Jun 18, 2013 at 12:41:29PM +0200, Andreas Färber wrote:
> Am 18.06.2013 12:23, schrieb Michael S. Tsirkin:
> > On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote:
> >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >>
> >>
> >> 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
> > 
> > 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?
> 
> 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.

> 
> and then use
> 
> DEVICE_CLASS(ARM_CPU_SUPER_CLASS())
> or
> CPU_CLASS(ARM_CPU_SUPER_CLASS())
> 
> as needed. What do you think?
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg