qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access
Date: Tue, 18 Jun 2013 20:44:23 +1000

HI Michael,

On Tue, Jun 18, 2013 at 8:39 PM, Michael S. Tsirkin <address@hidden> 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 <address@hidden> wrote:
>> > On Tue, Jun 18, 2013 at 07:43:11PM +1000, address@hidden wrote:
>> >> From: Peter Crosthwaite <address@hidden>
>> >>
>> >>
>> >> 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
>> >
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]