Message ID | 20131106181558.GA14444@google.com (mailing list archive) |
---|---|
State | New, archived |
Delegated to: | Bjorn Helgaas |
Headers | show |
On Wed, Nov 6, 2013 at 10:15 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > commit dfb66fee4715c747a94abd45c20fbe302b10e49c > 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. 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 <bhelgaas@google.com> > > 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. > + > + 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 <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..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. Maybe you can cache return from pci_upstream_bridge and check that before calling pci_enable_bridge. Thanks Yinghai -- 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 11:56:13AM -0800, Yinghai Lu wrote: > On Wed, Nov 6, 2013 at 10:15 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > > commit dfb66fee4715c747a94abd45c20fbe302b10e49c > > 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. 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 <bhelgaas@google.com> > > > > 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 <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..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 -- 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
>> > commit 4e5415f02e32c85e902cd9692eab18200e14b347 >> > 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..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. how about it is VF and it is on it's own virtual bus ? and it will pass !pci_is_root_bus(dev->bus) checking and get into pci_enable_bridge(pci_upstream_bridge(dev)); Thanks Yinghai -- 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..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; + + 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 <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..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++)