| Message ID | 20131106204603.GA17046@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Wed, Nov 6, 2013 at 12:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > Oh, I see. The problem is when VF is on a virtual bus and the > corresponding PF is on a root bus. Then we have: > > pci_is_root_bus(VF) == false > pci_upstream_bridge(VF) == pci_upstream_bridge(PF) > pci_uptream_bridge(PF) == NULL > > I guess that's what your suggestion about "caching the return from > pci_upstream_bridge()" was about. Like this: Yes. For the two patches, Acked-by: Yinghai Lu <yinghai@kernel.org> > > > commit 21ea57bf1311f7a8d1b755d355322a1f077fccb7 > Author: Bjorn Helgaas <bhelgaas@google.com> > 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. This is typically just "dev->bus->self", but in > the case of a VF on a virtual bus, we have to start from the corresponding > PF. Returns NULL if there is no upstream PCI bridge, i.e., if the device > is on a root bus. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index d3a888a..835ec7b 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -480,6 +480,15 @@ 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) > +{ > + dev = pci_physfn(dev); > + if (pci_is_root_bus(dev->bus)) > + return NULL; > + > + return dev->bus->self; > +} > + > #ifdef CONFIG_PCI_MSI > static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) > { > commit bca690ab580d5c0c4a0c7201f3c42057288add8b > Author: Bjorn Helgaas <bhelgaas@google.com> > 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 <bhelgaas@google.com> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index ac40f90..d3ed931 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1148,12 +1148,12 @@ int pci_reenable_device(struct pci_dev *dev) > > static void pci_enable_bridge(struct pci_dev *dev) > { > + struct pci_dev *bridge; > int retval; > > - if (!dev) > - return; > - > - pci_enable_bridge(dev->bus->self); > + bridge = pci_upstream_bridge(dev); > + if (bridge) > + pci_enable_bridge(bridge); > > if (pci_is_enabled(dev)) { > if (!dev->is_busmaster) > @@ -1170,6 +1170,7 @@ static void pci_enable_bridge(struct pci_dev *dev) > > static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) > { > + struct pci_dev *bridge; > int err; > int i, bars = 0; > > @@ -1188,7 +1189,9 @@ 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); > + bridge = pci_upstream_bridge(dev); > + if (bridge) > + pci_enable_bridge(bridge); > > /* only skip sriov related */ > for (i = 0; i <= PCI_ROM_RESOURCE; i++) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 06, 2013 at 04:28:48PM -0800, Yinghai Lu wrote: > On Wed, Nov 6, 2013 at 12:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > > Oh, I see. The problem is when VF is on a virtual bus and the > > corresponding PF is on a root bus. Then we have: > > > > pci_is_root_bus(VF) == false > > pci_upstream_bridge(VF) == pci_upstream_bridge(PF) > > pci_uptream_bridge(PF) == NULL > > > > I guess that's what your suggestion about "caching the return from > > pci_upstream_bridge()" was about. Like this: > > Yes. > > For the two patches, > > Acked-by: Yinghai Lu <yinghai@kernel.org> Thanks, I applied these two with your acks to my pci/misc branch for v3.13. Bjorn > > > > commit 21ea57bf1311f7a8d1b755d355322a1f077fccb7 > > Author: Bjorn Helgaas <bhelgaas@google.com> > > 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. This is typically just "dev->bus->self", but in > > the case of a VF on a virtual bus, we have to start from the corresponding > > PF. Returns NULL if there is no upstream PCI bridge, i.e., if the device > > is on a root bus. > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index d3a888a..835ec7b 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -480,6 +480,15 @@ 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) > > +{ > > + dev = pci_physfn(dev); > > + if (pci_is_root_bus(dev->bus)) > > + return NULL; > > + > > + return dev->bus->self; > > +} > > + > > #ifdef CONFIG_PCI_MSI > > static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) > > { > > commit bca690ab580d5c0c4a0c7201f3c42057288add8b > > Author: Bjorn Helgaas <bhelgaas@google.com> > > 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 <bhelgaas@google.com> > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index ac40f90..d3ed931 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -1148,12 +1148,12 @@ int pci_reenable_device(struct pci_dev *dev) > > > > static void pci_enable_bridge(struct pci_dev *dev) > > { > > + struct pci_dev *bridge; > > int retval; > > > > - if (!dev) > > - return; > > - > > - pci_enable_bridge(dev->bus->self); > > + bridge = pci_upstream_bridge(dev); > > + if (bridge) > > + pci_enable_bridge(bridge); > > > > if (pci_is_enabled(dev)) { > > if (!dev->is_busmaster) > > @@ -1170,6 +1170,7 @@ static void pci_enable_bridge(struct pci_dev *dev) > > > > static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) > > { > > + struct pci_dev *bridge; > > int err; > > int i, bars = 0; > > > > @@ -1188,7 +1189,9 @@ 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); > > + bridge = pci_upstream_bridge(dev); > > + if (bridge) > > + pci_enable_bridge(bridge); > > > > /* only skip sriov related */ > > for (i = 0; i <= PCI_ROM_RESOURCE; i++) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/pci.h b/include/linux/pci.h index d3a888a..835ec7b 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -480,6 +480,15 @@ 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) +{ + dev = pci_physfn(dev); + if (pci_is_root_bus(dev->bus)) + return NULL; + + return dev->bus->self; +} + #ifdef CONFIG_PCI_MSI static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) { commit bca690ab580d5c0c4a0c7201f3c42057288add8b Author: Bjorn Helgaas <bhelgaas@google.com> 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 <bhelgaas@google.com> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index ac40f90..d3ed931 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1148,12 +1148,12 @@ int pci_reenable_device(struct pci_dev *dev) static void pci_enable_bridge(struct pci_dev *dev) { + struct pci_dev *bridge; int retval; - if (!dev) - return; - - pci_enable_bridge(dev->bus->self); + bridge = pci_upstream_bridge(dev); + if (bridge) + pci_enable_bridge(bridge); if (pci_is_enabled(dev)) { if (!dev->is_busmaster) @@ -1170,6 +1170,7 @@ static void pci_enable_bridge(struct pci_dev *dev) static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) { + struct pci_dev *bridge; int err; int i, bars = 0; @@ -1188,7 +1189,9 @@ 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); + bridge = pci_upstream_bridge(dev); + if (bridge) + pci_enable_bridge(bridge); /* only skip sriov related */ for (i = 0; i <= PCI_ROM_RESOURCE; i++)