From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47791) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V5Rqh-0007zQ-3d for qemu-devel@nongnu.org; Fri, 02 Aug 2013 22:45:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V5RqY-00044n-N4 for qemu-devel@nongnu.org; Fri, 02 Aug 2013 22:45:43 -0400 Received: from mail-pb0-f54.google.com ([209.85.160.54]:56765) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V5RqY-00044H-F9 for qemu-devel@nongnu.org; Fri, 02 Aug 2013 22:45:34 -0400 Received: by mail-pb0-f54.google.com with SMTP id ro12so1321135pbb.13 for ; Fri, 02 Aug 2013 19:45:32 -0700 (PDT) Message-ID: <51FC6EC4.7020400@ozlabs.ru> Date: Sat, 03 Aug 2013 12:45:24 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1374043057-27208-1-git-send-email-aik@ozlabs.ru> <1374043057-27208-5-git-send-email-aik@ozlabs.ru> <51F96B13.1060201@suse.de> <51F9A855.9000702@ozlabs.ru> <51F9BA0E.6050003@suse.de> <51FBC8D0.8050603@ozlabs.ru> In-Reply-To: <51FBC8D0.8050603@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 4/4] xics: Support for in-kernel XICS interrupt controller List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: Peter Maydell , Anthony Liguori , qemu-devel@nongnu.org, Alexander Graf , Paul Mackerras , qemu-ppc@nongnu.org, David Gibson On 08/03/2013 12:57 AM, Alexey Kardashevskiy wrote: > On 08/01/2013 11:29 AM, Andreas Färber wrote: >> Am 01.08.2013 02:14, schrieb Alexey Kardashevskiy: >>> On 08/01/2013 05:52 AM, Andreas Färber wrote: >>>> Am 17.07.2013 08:37, schrieb Alexey Kardashevskiy: > >>>>> + >>>>> + icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState)); >>>>> + for (i = 0; i < icp->nr_servers; i++) { >>>>> + char buffer[32]; >>>>> + object_initialize(&icp->ss[i], TYPE_ICP_KVM); >>>>> + snprintf(buffer, sizeof(buffer), "icp[%d]", i); >>>>> + object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]), NULL); >>>>> + qdev_init_nofail(DEVICE(&icp->ss[i])); >>>> >>>> object_property_set_bool() >>> >>> >>> ? Anthony did XICS refactoring recently and that has qdev_init_nofail(). >> >> Nobody is perfect. ;) >> >> The point is, this is an object, and in realize you shouldn't abort but >> set errp and leave error printing and handling to your caller. The QOM >> API as opposed to qdev works with an Error object that you can >> error_propagate() to your caller. >> >> (Also using qdev_* for something that is new-style QOM is ugly IMO.) >> >>>> Where does icp->nr_servers come from? >>> >>> Via properties in try_create_xics() (hw/ppc/spapr.c). >> >> Sounds tricky... Peter introduced static array properties for a similar >> purpose, I believe. Don't know if that would help here. > > Peter who? For what purpose? Could you please point to it? Cannot find it > anywhere. > > >>> >>>> Is there no way to split this into >>>> instance_init and realize? >>> >>> >>> Why would we want to split? >> >> Because realize is too late to create new devices: With our targetted >> late, recursive realization model it will not be possible to see and >> modify such objects from management interface - only before realize. > > > Oh. I lost you here. I need to create XICS with some child ICP devices (now > they are devices). The number of child devices comes from spapr.c via > properties. Now you are saying it is too late to create devices (even so > lame ones) in realize(). So I have to put the creation in instance_init(). > Could you please point now I can create device-and-pass-propetries in one > shot? Thanks! No, I am still missing some bits. In spapr we try to initialize xics-kvm and if we cannot have it, we switch to emulated xics. I.e.: dev = qdev_create(NULL, type); qdev_prop_set_uint32(dev, "nr_servers", nr_servers); qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); if (qdev_init(dev) < 0) { return NULL; } qdev_create() calls instance_init() (which cannot fail) and qdev_init() calls realize() (which can fail and this is how I know that xics-kvm cannot work). instance_init() and realize() are called at once, right here. But as I understand, realize() is not supposed to be called this way, it should be called by QOM, no? And about properties - before I create child devices, I need both properties set. So it looks like I need a third property such as "realized", call it "real_instance_init" and assign a callback on it as it is done for realize(). Correct? -- Alexey