From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=50339 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PgETf-0001Zl-Ki for qemu-devel@nongnu.org; Fri, 21 Jan 2011 05:44:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PgETd-0000oH-Qk for qemu-devel@nongnu.org; Fri, 21 Jan 2011 05:44:23 -0500 Received: from mail.valinux.co.jp ([210.128.90.3]:34060) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PgETd-0000nA-Aj for qemu-devel@nongnu.org; Fri, 21 Jan 2011 05:44:21 -0500 Date: Fri, 21 Jan 2011 19:44:16 +0900 From: Isaku Yamahata Message-ID: <20110121104416.GE20801@valinux.co.jp> References: <20110120141548.GC15426@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110120141548.GC15426@redhat.com> Subject: [Qemu-devel] Re: [PATCH] pci/pcie: make pci_find_device() ARI aware. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org On Thu, Jan 20, 2011 at 04:15:48PM +0200, Michael S. Tsirkin wrote: > On Thu, Jan 20, 2011 at 03:57:39PM +0900, Isaku Yamahata wrote: > > make pci_find_device() ARI aware. > > > > Signed-off-by: Isaku Yamahata > > --- > > hw/pci.c | 7 +++++++ > > 1 files changed, 7 insertions(+), 0 deletions(-) > > > > diff --git a/hw/pci.c b/hw/pci.c > > index 8d0e3df..851f350 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -1596,11 +1596,18 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > > > > PCIDevice *pci_find_device(PCIBus *bus, int bus_num, int slot, int function) > > { > > + PCIDevice *d; > > bus = pci_find_bus(bus, bus_num); > > > > if (!bus) > > return NULL; > > > > + d = bus->parent_dev; > > + if (d && pci_is_express(d) && > > + pcie_cap_get_type(d) == PCI_EXP_TYPE_DOWNSTREAM && > > + !pcie_cap_is_ari_enabled(d) && slot > 0) { > > + return NULL; > > + } > > return bus->devices[PCI_DEVFN(slot, function)]; > > } > > I'd like to split this express-specific code out in some way. > Further, the downstream port always has a single slot. > Maybe it shouldn't be a bus at all, but this needs some thought. Yes, it needs consideration. > How about we put flag in PCIBus that says that a single > slot is supported? Downstream ports would just set it. So such a flag must be set/clear by something like pcie_cap_ari_write_config() depending on ARI bit at runtime. pcie device can have 256 functions instead of 8. Maybe we'd like to emulate how p2p bridge transfers pci transaction to child pci bus somehow. So how about introducing new method? something like the following. Which is your preference? new flag or new method? PCIBus PCIDevice (*get_device)(bus, slot, function) pci_get_device_default() return bus->devices[PCI_DEVFN(slot, function)]; pcie_downstream_get_device() d = bus->parent_dev; if (!pcie_cap_is_ari_enabled(d) && slot > 0) { return NULL } return bus->devices[PCI_DEVFN(slot, function)]; pci_find_device() ... return bus->get_device(bus, slot, function) -- yamahata