| Message ID | 1449523949-21898-3-git-send-email-keith.busch@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Keith, On Mon, Dec 07, 2015 at 02:32:24PM -0700, Keith Busch wrote: > Does not allocate a child bus if the new bus number does not fit in the > parent's bus resource window. > > Signed-off-by: Keith Busch <keith.busch@intel.com> > --- > drivers/pci/probe.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index edb1984..6e29f7a 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -704,6 +704,12 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, > int i; > int ret; > > + if (busnr > parent->busn_res.end) { > + dev_printk(KERN_DEBUG, &parent->dev, > + "can not alloc bus:%d under %pR\n", busnr, > + &parent->busn_res); > + return NULL; Can you take a look at 1820ffdccb9b ("PCI: Make sure bus number resources stay within their parents bounds") and 12d8706963f0 ("Revert "PCI: Make sure bus number resources stay within their parents bounds"")? This is implemented differently, but it seems like it might expose the same problem we found with 1820ffdccb9b. If you could take a look and confirm that "no, this does something differently than 1820ffdccb9b did" or "yes, this might expose that problem again," that would help. Bjorn > + } > /* > * Allocate a new bus, and inherit stuff from the parent.. > */ > -- > 2.6.2.307.g37023ba > > -- > 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 -- 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 Mon, Dec 07, 2015 at 02:32:24PM -0700, Keith Busch wrote: > Does not allocate a child bus if the new bus number does not fit in the > parent's bus resource window. > > Signed-off-by: Keith Busch <keith.busch@intel.com> Other nits: please make this similar to previous changelogs in capitalization, sentence structure, etc. If we figure out that this is safe, and I end up applying it, I can just tweak it myself, but if you post it again, something like this is what I'm looking for: PCI: Don't allocate child bus outside parent bus number range Don't allocate a child bus if the new bus number does not fit in the parent's bus resource window. The next patch is missing a subject line: PCI/MSI: Export MSI and IRQ functions for use by modules Export the following functions for use by modules: msi_desc_to_pci_dev() pci_msi_create_irq_domain() irq_domain_set_info() And the following: x86/PCI: Allow PCI domain-specific DMA ops I know this seems pedantic, but try to use "x86/PCI" consistently because I think it's distracting to have the boiler-plate be different in non-useful ways, e.g., 510fb6779eed x86/pci: Initial commit for new VMD device driver 510fb6779eed x86/pci: Initial commit for new VMD device driver b7a48e92f508 x86-pci: allow pci domain specific dma ops 8affb487d4a4 x86/PCI: Don't alloc pcibios-irq when MSI is enabled 991de2e59090 PCI, x86: Implement pcibios_alloc_irq() and pcibios_free_irq() Bjorn > --- > drivers/pci/probe.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index edb1984..6e29f7a 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -704,6 +704,12 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, > int i; > int ret; > > + if (busnr > parent->busn_res.end) { > + dev_printk(KERN_DEBUG, &parent->dev, > + "can not alloc bus:%d under %pR\n", busnr, > + &parent->busn_res); > + return NULL; > + } > /* > * Allocate a new bus, and inherit stuff from the parent.. > */ > -- > 2.6.2.307.g37023ba > > -- > 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 -- 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 Thu, Dec 17, 2015 at 11:27:18AM -0600, Bjorn Helgaas wrote: > On Mon, Dec 07, 2015 at 02:32:24PM -0700, Keith Busch wrote: > > + if (busnr > parent->busn_res.end) { > > + dev_printk(KERN_DEBUG, &parent->dev, > > + "can not alloc bus:%d under %pR\n", busnr, > > + &parent->busn_res); > > + return NULL; > > Can you take a look at 1820ffdccb9b ("PCI: Make sure bus number resources > stay within their parents bounds") and 12d8706963f0 ("Revert "PCI: Make > sure bus number resources stay within their parents bounds"")? > > This is implemented differently, but it seems like it might expose the same > problem we found with 1820ffdccb9b. > > If you could take a look and confirm that "no, this does something > differently than 1820ffdccb9b did" or "yes, this might expose that problem > again," that would help. Thank you for the references. I think 1820ffdccb9b had the check too late, creating the child bus with resource conflicts. That'd probably make it appear unavailable. But I believe this new proposal will still break the same setup for a different reason. We can live with this issue for now if we want to drop this patch from the series. The only way we encounter a problem is when the config space aperture is artificially constrained, so I can request the feature be put on hold while a better fix is developed. -- 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/drivers/pci/probe.c b/drivers/pci/probe.c index edb1984..6e29f7a 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -704,6 +704,12 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, int i; int ret; + if (busnr > parent->busn_res.end) { + dev_printk(KERN_DEBUG, &parent->dev, + "can not alloc bus:%d under %pR\n", busnr, + &parent->busn_res); + return NULL; + } /* * Allocate a new bus, and inherit stuff from the parent.. */
Does not allocate a child bus if the new bus number does not fit in the parent's bus resource window. Signed-off-by: Keith Busch <keith.busch@intel.com> --- drivers/pci/probe.c | 6 ++++++ 1 file changed, 6 insertions(+)