From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40740) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UotQB-00021y-GM for qemu-devel@nongnu.org; Tue, 18 Jun 2013 06:46:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UotQ4-0000Uo-D8 for qemu-devel@nongnu.org; Tue, 18 Jun 2013 06:45:55 -0400 Received: from cantor2.suse.de ([195.135.220.15]:60130 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UotQ3-0000Ue-Vr for qemu-devel@nongnu.org; Tue, 18 Jun 2013 06:45:48 -0400 Message-ID: <51C03A59.4010004@suse.de> Date: Tue, 18 Jun 2013 12:45:45 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20130618102320.GC26066@redhat.com> <20130618103953.GB26536@redhat.com> In-Reply-To: <20130618103953.GB26536@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: "Michael S. Tsirkin" Cc: peter.maydell@linaro.org, Peter Crosthwaite , aliguori@us.ibm.com, qemu-devel@nongnu.org, edgar.iglesias@gmail.com, pbonzini@redhat.com 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 w= rote: >>> On Tue, Jun 18, 2013 at 07:43:11PM +1000, peter.crosthwaite@xilinx.co= m wrote: >>>> From: Peter Crosthwaite >>>> >>>> >>>> This series enables QOM super class access and demostrates some usag= es. >>>> 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 thei= r >>>> 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. >=20 > None of these uses has a chance to be at all performance > sensitive. >=20 >=20 >> Tying it to an object also brings >> into play the possibility of a cast cache should that be needed. >=20 > 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 nic= e. Andreas >=20 >> 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 >>> --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg