From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44365) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SdfZn-0003Vr-9l for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:40:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SdfZl-0002vc-Ev for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:40:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13514) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SdfZl-0002vL-6x for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:40:53 -0400 Date: Sun, 10 Jun 2012 13:41:22 +0300 From: "Michael S. Tsirkin" Message-ID: <20120610104120.GF6250@redhat.com> References: <20120607143216.GA12638@redhat.com> <4FD0C459.7070104@web.de> <20120607162850.GA12884@redhat.com> <4FD0DAEE.2040405@web.de> <20120610095507.GD6250@redhat.com> <4FD47217.7020909@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FD47217.7020909@web.de> Subject: Re: [Qemu-devel] [PATCH 05/13] pci: Add pci_device_route_intx_to_irq List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Alex Williamson , qemu-devel On Sun, Jun 10, 2012 at 12:08:23PM +0200, Jan Kiszka wrote: > On 2012-06-10 11:55, Michael S. Tsirkin wrote: > > On Thu, Jun 07, 2012 at 06:46:38PM +0200, Jan Kiszka wrote: > >> On 2012-06-07 18:28, Michael S. Tsirkin wrote: > >>> On Thu, Jun 07, 2012 at 05:10:17PM +0200, Jan Kiszka wrote: > >>>> On 2012-06-07 16:32, Michael S. Tsirkin wrote: > >>>>> On Mon, Jun 04, 2012 at 10:52:13AM +0200, Jan Kiszka wrote: > >>>>>> @@ -1089,6 +1093,14 @@ static void pci_set_irq(void *opaque, int irq_num, int level) > >>>>>> pci_change_irq_level(pci_dev, irq_num, change); > >>>>>> } > >>>>>> > >>>>>> +PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin) > >>>>>> +{ > >>>>>> + PCIBus *bus = dev->host_bus; > >>>>>> + > >>>>>> + assert(bus->route_intx_to_irq); > >>>>>> + return bus->route_intx_to_irq(bus->irq_opaque, dev->host_intx_pin[pin]); > >>>>>> +} > >>>>>> + > >>>>>> /***********************************************************/ > >>>>>> /* monitor info on PCI */ > >>>>>> > >>>>> > >>>>> Just an idea: can devices cache this result, bypassing the > >>>>> intx to irq lookup on data path? > >>>> > >>>> That lookup is part of set_irq which we don't bypass so far and where > >>>> this is generally trivial. If we want to cache the effects of set_irq as > >>>> well, I guess things would become pretty complex (e.g. due to vmstate > >>>> compatibility), and I'm unsure if it would buy us much. > >>> > >>> This is less for performance but more for making > >>> everyone use the same infrastructure rather than > >>> assigned devices being the weird case. > >> > >> Device assignment is weird. It bypasses all state updates as it does not > >> have to bother about migratability. > >> > >> Well, of course we could cache the host bridge routing result as well, > >> for every device. It would have to be in addition to host_intx_pin. But > >> the result would look pretty strange to me. > >> > >> In any case, I would prefer to do this, if at all, on top of this > >> series, specifically as it will require to touch all host bridges. > > > > I'd like to ponder this a bit more then. > > > > If the claim is that device assignment is only needed for > > piix anyway, then why not make it depend on piix *explicitly*? > > Yes ugly but this will make it very easy to find and > > address any missing pieces. > > Because it is conceptually independent of the PIIX, we will need it for > successors of that x86 chipset as well, and I won't add the ugly hack of > qemu-kvm upstream So you look at an API and see it requires a route callback. And you ask "why doesn't chipset X implement it"? And the answer is "because it's only used by device assignment". Which you will only know if you read this thread. So it's a hack. And I'd rather have the hacks in device-assignment.c than in pci.c even if the former are nastier. > > > > As it is you are adding APIs that in theory address > > non PIIX issues but in practice don't implement for non > > PIIX. So we never really know. > > I once hacked q35 to make it work with device assignment. This really > requires something like this. Yes I'm aware of this motivation. This does not do much to address the concerns though. > It actually requires something generic, > independent of PCI, but that's too much for this round. > > Jan And this just makes the concerns worse :( -- MST