From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50244) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QQszM-0007sX-8b for qemu-devel@nongnu.org; Sun, 29 May 2011 23:17:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QQszI-0006M0-Om for qemu-devel@nongnu.org; Sun, 29 May 2011 23:17:56 -0400 Received: from ozlabs.org ([203.10.76.45]:59279) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QQszI-0006LG-Bz for qemu-devel@nongnu.org; Sun, 29 May 2011 23:17:52 -0400 Date: Mon, 30 May 2011 13:16:29 +1000 From: David Gibson Message-ID: <20110530031629.GA5926@yookeroo.fritz.box> References: <1306237507-19189-1-git-send-email-pbonzini@redhat.com> <1306237507-19189-2-git-send-email-pbonzini@redhat.com> <20110524221222.GA11809@yookeroo.fritz.box> <4DDCAFD6.1060900@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4DDCAFD6.1060900@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/3] spapr: allow creating devices with -device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, agraf@suse.de On Wed, May 25, 2011 at 09:29:26AM +0200, Paolo Bonzini wrote: > On 05/25/2011 12:12 AM, David Gibson wrote: > >>@@ -602,6 +604,8 @@ static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo) > >> } > >> > >> dev->qdev.id = id; > >>+ dev->vio_irq_num = bus->irq++; > >>+ dev->qirq = xics_find_qirq(spapr->icp, dev->vio_irq_num); > > > >I'd prefer to see an spapr_allocate_irq() function, rather than open > >coding this multiple times. > > I don't understand. This is the only call to xics_find_qirq, unlike > before this patch. It's not open coded multiple times. Uh, sorry, I wasn't thinking it through when I assumed it was duplicated. Actually, in any case I wouldn't mind multiple calls to xics_find_qirq(), it's the actual allocation - the irq++ - being potentially duplicated which bothers me. It's not now, but with this approach it would need to be when we add non VIO devices to the system which is coming (PCI, virtio, ..). > I can surely add a spapr_allocate_irq() function that would keep the > code independent of the particular interrupt controller. Would you > prefer something that gives back the virtual IRQ number as well: > > qemu_irq *spapr_allocate_irq(uint32_t *p_vio_irq_num) > > or should I keep that in the VIOsPAPRBus, like this: > > qemu_irq *spapr_allocate_irq(uint32_t p_vio_irq_num) In fact just returning the xics irq num and using xics_find_qirq() on that would be ok by me. The point is that I don't want the policy for irq allocation on the global interrupt controller to be open coded here at the bus level. > ? Should I pass a sPAPREnvironment too, or should it just use the global? Passing it would be preferable. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson