From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ie0-f178.google.com ([209.85.223.178]:36490 "EHLO mail-ie0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752017Ab3KFUMo (ORCPT ); Wed, 6 Nov 2013 15:12:44 -0500 Received: by mail-ie0-f178.google.com with SMTP id x13so817ief.37 for ; Wed, 06 Nov 2013 12:12:42 -0800 (PST) Date: Wed, 6 Nov 2013 13:12:39 -0700 From: Bjorn Helgaas To: Yinghai Lu Cc: "linux-pci@vger.kernel.org" , Wei Yang , Nishank Trivedi Subject: Re: [PATCH] PCI: Use pci_is_root_bus() to check for root bus Message-ID: <20131106201239.GA8971@google.com> References: <20131105232903.3790.8738.stgit@bhelgaas-glaptop.roam.corp.google.com> <20131106181558.GA14444@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: Sender: linux-pci-owner@vger.kernel.org List-ID: On Wed, Nov 06, 2013 at 11:56:13AM -0800, Yinghai Lu wrote: > On Wed, Nov 6, 2013 at 10:15 AM, Bjorn Helgaas wrote: > > > commit dfb66fee4715c747a94abd45c20fbe302b10e49c > > Author: Bjorn Helgaas > > Date: Wed Nov 6 10:11:48 2013 -0700 > > > > PCI: Add pci_upstream_bridge() > > > > This adds a pci_upstream_bridge() interface to find the PCI-to-PCI bridge > > upstream from a device. If the device is on a root bus, i.e., the upstream > > bridge is a host bridge instead of a PCI bridge, this returns NULL. If the > > device is a VF, this returns the bridge upstream from the PF corresponding > > to the VF. This is important for VFs on virtual buses, where > > "dev->bus->self == NULL". > > > > Signed-off-by: Bjorn Helgaas > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index d3a888a..e09d19a 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -480,6 +480,18 @@ static inline bool pci_is_root_bus(struct pci_bus *pbus) > > return !(pbus->parent); > > } > > > > +static inline struct pci_dev *pci_upstream_bridge(struct pci_dev *dev) > > +{ > > + if (pci_is_root_bus(dev->bus)) > > + return NULL; > > + > > + dev = pci_physfn(dev); > > + if (pci_is_root_bus(dev->bus)) > > + return NULL; > > Maybe we can drop the first pci_is_root_bus() checking. > for physfn, pci_physfn will return it's self. Yep, that makes sense, thanks. I incorporated that change. > > + > > + return dev->bus->self; > > +} > > + > > #ifdef CONFIG_PCI_MSI > > static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) > > { > > commit 4e5415f02e32c85e902cd9692eab18200e14b347 > > Author: Bjorn Helgaas > > Date: Wed Nov 6 10:00:51 2013 -0700 > > > > PCI: Enable upstream bridges even for VFs on virtual buses > > > > Previously we enabled the upstream PCI-to-PCI bridge only when > > "dev->bus->self != NULL". In the case of a VF on a virtual bus, where > > "bus->self == NULL", we didn't enable the upstream bridge. > > > > This fixes that by enabling the upstream bridge of the PF corresponding to > > the VF. > > > > Signed-off-by: Bjorn Helgaas > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index ac40f90..744dc26 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -1150,10 +1150,8 @@ static void pci_enable_bridge(struct pci_dev *dev) > > { > > int retval; > > > > - if (!dev) > > - return; > > - > > - pci_enable_bridge(dev->bus->self); > > + if (!pci_is_root_bus(dev->bus)) > > + pci_enable_bridge(pci_upstream_bridge(dev)); > > > > if (pci_is_enabled(dev)) { > > if (!dev->is_busmaster) > > @@ -1188,7 +1186,8 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) > > if (atomic_inc_return(&dev->enable_cnt) > 1) > > return 0; /* already enabled */ > > > > - pci_enable_bridge(dev->bus->self); > > + if (!pci_is_root_bus(dev->bus)) > > + pci_enable_bridge(pci_upstream_bridge(dev)); > > > > /* only skip sriov related */ > > for (i = 0; i <= PCI_ROM_RESOURCE; i++) > > still have problem. > > pci_upstream_bridge() could return NULL. so later pci_enable_bridge() > referring dev->bus could panic, as you remove !dev checking. pci_upstream_bridge(dev) can only return NULL if dev is on a root bus, and we never call pci_enable_bridge() in that case. So I don't see the problem. Bjorn