From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UotRa-0003yB-Ov for qemu-devel@nongnu.org; Tue, 18 Jun 2013 06:47:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UotRX-00014I-Rn for qemu-devel@nongnu.org; Tue, 18 Jun 2013 06:47:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7143) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UotRX-00014E-Je for qemu-devel@nongnu.org; Tue, 18 Jun 2013 06:47:19 -0400 Date: Tue, 18 Jun 2013 13:48:02 +0300 From: "Michael S. Tsirkin" Message-ID: <20130618104802.GD26536@redhat.com> References: <20130618102320.GC26066@redhat.com> <20130618103953.GB26536@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, qemu-devel@nongnu.org, pbonzini@redhat.com, edgar.iglesias@gmail.com, afaerber@suse.de 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 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 wrote: > >> > On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.com wrote: > >> >> From: Peter Crosthwaite > >> >> > >> >> > >> >> 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 > >> > > >