From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57104) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6ium-0005fj-6S for qemu-devel@nongnu.org; Tue, 06 Aug 2013 11:11:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V6iue-00061D-QM for qemu-devel@nongnu.org; Tue, 06 Aug 2013 11:11:12 -0400 Message-ID: <52011202.3000202@suse.de> Date: Tue, 06 Aug 2013 17:10:58 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1375777673-20274-1-git-send-email-aik@ozlabs.ru> <1375777673-20274-7-git-send-email-aik@ozlabs.ru> <5200CC0D.5000908@suse.de> <5200E6E0.5030403@ozlabs.ru> In-Reply-To: <5200E6E0.5030403@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 6/6] xics-kvm: Support for in-kernel XICS interrupt controller List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Anthony Liguori , Alexander Graf , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Paul Mackerras , David Gibson Am 06.08.2013 14:06, schrieb Alexey Kardashevskiy: > On 08/06/2013 08:12 PM, Andreas F=C3=A4rber wrote: >>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >>> index 1066f69..e5889e9 100644 >>> --- a/include/hw/ppc/xics.h >>> +++ b/include/hw/ppc/xics.h >>> @@ -35,6 +35,9 @@ >>> #define TYPE_XICS "xics" >>> #define XICS(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS) >>> =20 >>> +#define TYPE_KVM_XICS "xics-kvm" >>> +#define KVM_XICS(obj) OBJECT_CHECK(KVMXICSState, (obj), TYPE_KVM_XIC= S) >>> + >>> #define XICS_COMMON_CLASS(klass) \ >>> OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_COMMON) >>> #define XICS_CLASS(klass) \ >>> @@ -44,6 +47,9 @@ >>> #define XICS_GET_CLASS(obj) \ >>> OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS) >>> =20 >>> +#define XICS_GET_PARENT_CLASS(obj) \ >>> + (XICSStateClass *) object_class_get_parent(object_get_class(obj)= ) >> >> This is dangerous in that it takes the parent class of whatever type t= he >> obj argument turns out to have. This has been discussed in lengths in >> the context of Peter C.'s proposal for ISA/ARM realize cleanup. >=20 > How is it dangerous? I perfectly know what I call it with. And simple r= un > will crash QEMU if something is wrong. I did not say it was wrong or an error, I said it's dangerous. The reason is that unlike C++ we do not have VTables in QOM: KVM_XICS(obj) =3D=3D=3D XICS(obj) =3D=3D=3D obj. I.e., object_get_class() always returns the actual, most specific type rather than a type corresponding to the variable you are using it through= . So if someone derives TYPE_MY_KVM_XICS from your TYPE_KVM_XICS then the function will silently do something different than it was doing before. Same if someone inserts TYPE_VIRTUAL_XICS between TYPE_XICS and TYPE_KVM_XICS. The thought of both my macro (and Peter C.'s last version) and my comments here is to keep the type information in a central place and to make, say, a kvm_xics_realize C function behave like a KVMXICS::realize C++ function would. >> This should therefore be a macro per type specifying the TYPE_*, eithe= r >> for object_class_by_name() or to my proposed macro which abstracts the >> implementation to core QOM code. >=20 > Please, be more specific. What type should be used in macro here - > XICS_COMMON or KVM_XICS? I asked already in the other thread but someho= w > you missed it. Answered it now! ;) > Neither really makes sense for me as I believe that > get_parent is exactly for the cases (and this is the case) when I do no= t > want to know exact parent type and I already know the current type. Tha= nks. Correct. You do not know, you are making implicit assumptions that I would like to avoid for safety reasons in favor of explicit parent-of-my-type. That allows both to derive further types and to insert intermediate types when using object_class_get_parent() the way I = do. >> XICS_CLASS() would be nicer than open-coding, but why cast here? >> DeviceClass can be needed just as well. >=20 > I do not need DeviceClass, at all, anywhere. I need a pointer to a my > specific cpu_setup callback, this is all about it. You might not do so now - you might if there is common stuff to do in DeviceClass::realize(). But that is something I suggest to discuss on my macro patch instead, as it is a general design question for our parent-related macros. I also suggest to search the archives for Peter's last two proposals that got some discussion of corner cases and intentions as well. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg