| Message ID | 1412000971-9242-11-git-send-email-Liviu.Dudau@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 29, 2014 at 7:29 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > If the firmware has not assigned all the bus resources and we are not just > probing the PCI buses, it makes sense to assign the unassigned resources > in pci_scan_root_bus(). > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > CC: Arnd Bergmann <arnd@arndb.de> > CC: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > CC: Rob Herring <robh+dt@kernel.org> > --- > drivers/pci/probe.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 636d1c9..d2ebd49 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1951,6 +1951,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus, > if (!found) > pci_bus_update_busn_res_end(b, max); > > + if (!pci_has_flag(PCI_PROBE_ONLY)) > + pci_assign_unassigned_bus_resources(b); > + > pci_bus_add_devices(b); > return b; > } No, you can not do it that early. On x86, we need to call pcibios_resource_survey_bus at first. Thanks Yinghai
On Mon, Sep 29, 2014 at 11:25 AM, Yinghai Lu <yinghai@kernel.org> wrote: > On Mon, Sep 29, 2014 at 7:29 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: >> If the firmware has not assigned all the bus resources and we are not just >> probing the PCI buses, it makes sense to assign the unassigned resources >> in pci_scan_root_bus(). >> >> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> >> CC: Arnd Bergmann <arnd@arndb.de> >> CC: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> CC: Rob Herring <robh+dt@kernel.org> >> --- >> drivers/pci/probe.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 636d1c9..d2ebd49 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -1951,6 +1951,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus, >> if (!found) >> pci_bus_update_busn_res_end(b, max); >> >> + if (!pci_has_flag(PCI_PROBE_ONLY)) >> + pci_assign_unassigned_bus_resources(b); >> + >> pci_bus_add_devices(b); >> return b; >> } > > No, you can not do it that early. > > On x86, we need to call > pcibios_resource_survey_bus at first. on x86: pcibios_init pcibios_resource_survey() pcibios_assign_resources() via fs_initcall
On Mon, 2014-09-29 at 12:06 -0700, Yinghai Lu wrote: > On Mon, Sep 29, 2014 at 11:25 AM, Yinghai Lu <yinghai@kernel.org> wrote: > > On Mon, Sep 29, 2014 at 7:29 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > >> If the firmware has not assigned all the bus resources and we are not just > >> probing the PCI buses, it makes sense to assign the unassigned resources > >> in pci_scan_root_bus(). > >> > >> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > >> CC: Arnd Bergmann <arnd@arndb.de> > >> CC: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > >> CC: Rob Herring <robh+dt@kernel.org> > >> --- > >> drivers/pci/probe.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > >> index 636d1c9..d2ebd49 100644 > >> --- a/drivers/pci/probe.c > >> +++ b/drivers/pci/probe.c > >> @@ -1951,6 +1951,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus, > >> if (!found) > >> pci_bus_update_busn_res_end(b, max); > >> > >> + if (!pci_has_flag(PCI_PROBE_ONLY)) > >> + pci_assign_unassigned_bus_resources(b); > >> + > >> pci_bus_add_devices(b); > >> return b; > >> } > > > > No, you can not do it that early. > > > > On x86, we need to call > > pcibios_resource_survey_bus at first. > > on x86: > pcibios_init > pcibios_resource_survey() > pcibios_assign_resources() via fs_initcall Right and on powerpc and others as well. We need to survey existing resources. We also have a number of platform things that might need to happen before we do the final re-assignment pass. Ben.
On Mon, Sep 29, 2014 at 3:02 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2014-09-29 at 12:06 -0700, Yinghai Lu wrote: >> On Mon, Sep 29, 2014 at 11:25 AM, Yinghai Lu <yinghai@kernel.org> wrote: >> > On Mon, Sep 29, 2014 at 7:29 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: >> >> If the firmware has not assigned all the bus resources and we are not just >> >> probing the PCI buses, it makes sense to assign the unassigned resources >> >> in pci_scan_root_bus(). >> >> >> >> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> >> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> >> >> CC: Arnd Bergmann <arnd@arndb.de> >> >> CC: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> >> CC: Rob Herring <robh+dt@kernel.org> >> >> --- >> >> drivers/pci/probe.c | 3 +++ >> >> 1 file changed, 3 insertions(+) >> >> >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> >> index 636d1c9..d2ebd49 100644 >> >> --- a/drivers/pci/probe.c >> >> +++ b/drivers/pci/probe.c >> >> @@ -1951,6 +1951,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus, >> >> if (!found) >> >> pci_bus_update_busn_res_end(b, max); >> >> >> >> + if (!pci_has_flag(PCI_PROBE_ONLY)) >> >> + pci_assign_unassigned_bus_resources(b); >> >> + >> >> pci_bus_add_devices(b); >> >> return b; >> >> } >> > >> > No, you can not do it that early. >> > >> > On x86, we need to call >> > pcibios_resource_survey_bus at first. >> >> on x86: >> pcibios_init >> pcibios_resource_survey() >> pcibios_assign_resources() via fs_initcall > > Right and on powerpc and others as well. We need to survey existing > resources. We also have a number of platform things that might need > to happen before we do the final re-assignment pass. That's true today. But I don't know whether it *has* to be this way forever. On x86, pcibios_resource_survey() throws in E820 information (which we know long before we do any PCI enumeration) and some IO-APIC resources (it looks like we also know these before PCI enumeration). Powerpc has pcibios_reserve_legacy_regions(), but that looks like mostly stuff that could be done when we find the host bridge, before we enumerate PCI devices below it. Bjorn
On Mon, 2014-09-29 at 15:33 -0600, Bjorn Helgaas wrote: > > Right and on powerpc and others as well. We need to survey existing > > resources. We also have a number of platform things that might need > > to happen before we do the final re-assignment pass. > > That's true today. But I don't know whether it *has* to be this way > forever. On x86, pcibios_resource_survey() throws in E820 information > (which we know long before we do any PCI enumeration) and some IO-APIC > resources (it looks like we also know these before PCI enumeration). > Powerpc has pcibios_reserve_legacy_regions(), but that looks like > mostly stuff that could be done when we find the host bridge, before > we enumerate PCI devices below it. Oh we can probably change that but it's going to be much more work than just moving the assignment into pci_scan_root_bus(). There are also a number of subtle and not-so-subtle bits of code that rely on side effects of the current code such as anything that tests for bus->added or dev->added. Again, nothing we can't sort out eventually but the transition might be a bit painful. We might need to introduce a new flag for platforms converted to the "new style" generalized resource assignment which we can deprecate once everybody has moved over. Cheers, Ben.
On Mon, Sep 29, 2014 at 4:31 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2014-09-29 at 15:33 -0600, Bjorn Helgaas wrote: >> > Right and on powerpc and others as well. We need to survey existing >> > resources. We also have a number of platform things that might need >> > to happen before we do the final re-assignment pass. >> >> That's true today. But I don't know whether it *has* to be this way >> forever. On x86, pcibios_resource_survey() throws in E820 information >> (which we know long before we do any PCI enumeration) and some IO-APIC >> resources (it looks like we also know these before PCI enumeration). >> Powerpc has pcibios_reserve_legacy_regions(), but that looks like >> mostly stuff that could be done when we find the host bridge, before >> we enumerate PCI devices below it. > > Oh we can probably change that but it's going to be much more work than > just moving the assignment into pci_scan_root_bus(). Agreed. It's obvious that we can't apply Liviu's patch as-is. > There are also a > number of subtle and not-so-subtle bits of code that rely on side > effects of the current code such as anything that tests for bus->added > or dev->added. Again, nothing we can't sort out eventually but the > transition might be a bit painful. > > We might need to introduce a new flag for platforms converted to the > "new style" generalized resource assignment which we can deprecate once > everybody has moved over. There's definitely a lot of work here if we ever want to make this happen. Bjorn
On Tue, Sep 30, 2014 at 12:08:49AM +0100, Bjorn Helgaas wrote: > On Mon, Sep 29, 2014 at 4:31 PM, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > On Mon, 2014-09-29 at 15:33 -0600, Bjorn Helgaas wrote: > >> > Right and on powerpc and others as well. We need to survey existing > >> > resources. We also have a number of platform things that might need > >> > to happen before we do the final re-assignment pass. > >> > >> That's true today. But I don't know whether it *has* to be this way > >> forever. On x86, pcibios_resource_survey() throws in E820 information > >> (which we know long before we do any PCI enumeration) and some IO-APIC > >> resources (it looks like we also know these before PCI enumeration). > >> Powerpc has pcibios_reserve_legacy_regions(), but that looks like > >> mostly stuff that could be done when we find the host bridge, before > >> we enumerate PCI devices below it. > > > > Oh we can probably change that but it's going to be much more work than > > just moving the assignment into pci_scan_root_bus(). > > Agreed. It's obvious that we can't apply Liviu's patch as-is. Thanks everyone for the feedback! Bjorn, you can drop this patch from the series if you want to add it to your tree, and the host bridge drivers will have to reimplement pci_scan_root_bus() for now (most do it already as they need to add msi information as well). Benjamin, I still have on my ToDo list to convert powerpc. I might not be able to do it successfully on my own (as I don't understand a lot of the corner cases) but Bjorn is correct here in that pcibios_reserve_legacy_regions() will move out of the scanning of the root bus. > > > There are also a > > number of subtle and not-so-subtle bits of code that rely on side > > effects of the current code such as anything that tests for bus->added > > or dev->added. Again, nothing we can't sort out eventually but the > > transition might be a bit painful. > > > > We might need to introduce a new flag for platforms converted to the > > "new style" generalized resource assignment which we can deprecate once > > everybody has moved over. > > There's definitely a lot of work here if we ever want to make this happen. I know :) Best regards, Liviu > > Bjorn >
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 636d1c9..d2ebd49 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1951,6 +1951,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus, if (!found) pci_bus_update_busn_res_end(b, max); + if (!pci_has_flag(PCI_PROBE_ONLY)) + pci_assign_unassigned_bus_resources(b); + pci_bus_add_devices(b); return b; }