| Message ID | 1436439557-21075-1-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Bjorn, Guenter, On Thu, Jul 09, 2015 at 11:59:16AM +0100, Lorenzo Pieralisi wrote: > When a PCI bus is scanned, upon PCI bridge detection the kernel > has to read the bridge registers to set-up its resources so that > the PCI resource hierarchy can be validated properly. > > Most if not all architectures read PCI bridge registers in the > pcibios_fixup_bus hook, that is called by the PCI generic layer > whenever a PCI bus is scanned. > > Since pci_read_bridge_bases is an arch agnostic operation (and it > is carried out on all architectures) it can be moved to the generic > PCI layer in order to consolidate code and remove the respective > calls from the architectures back-ends. > > The PCI_PROBE_ONLY flag is not checked before calling > pci_read_bridge_buses in the generic layer since reading the bridge > bases is not related to resources assignment; this implies that it > can be carried out safely on PCI_PROBE_ONLY systems too and should > not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY > flag before reading the bridge bases. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Ralf Baechle <ralf@linux-mips.org> > Cc: James E.J. Bottomley <jejb@parisc-linux.org> > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: David Howells <dhowells@redhat.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Tony Luck <tony.luck@intel.com> > Cc: David S. Miller <davem@davemloft.net> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Michal Simek <monstr@monstr.eu> > Cc: Chris Zankel <chris@zankel.net> > --- > v2->v3: > > - Dropped RFC status > - Removed bridge resources claiming from pci_read_bridge_bases() What do you want me to do with this patch ? I removed the resource claiming - ie the only controversial bit, I can only test on arm64, I would like to make some progress on this otherwise I will have to add the bridge bases read to arm64 pcibios_fixup_bus() and be done with this. Thanks, Lorenzo > v2: https://lkml.org/lkml/2015/6/9/187 > > v1->v2: > > - Moved pci_read_bridge_bases call to pci_scan_bridge to read bases before > scanning devices > - Added bridge resources claiming > > v1: https://lkml.org/lkml/2015/5/21/359 > > arch/alpha/kernel/pci.c | 7 +------ > arch/frv/mb93090-mb00/pci-vdk.c | 2 -- > arch/ia64/pci/pci.c | 1 - > arch/microblaze/pci/pci-common.c | 9 +-------- > arch/mips/pci/pci.c | 6 ------ > arch/mn10300/unit-asb2305/pci.c | 1 - > arch/powerpc/kernel/pci-common.c | 8 +------- > arch/x86/pci/common.c | 1 - > arch/xtensa/kernel/pci.c | 4 ---- > drivers/parisc/dino.c | 3 --- > drivers/parisc/lba_pci.c | 1 - > drivers/pci/probe.c | 9 +++++++++ > 12 files changed, 12 insertions(+), 40 deletions(-) > > diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c > index 82f738e..cded02c 100644 > --- a/arch/alpha/kernel/pci.c > +++ b/arch/alpha/kernel/pci.c > @@ -242,12 +242,7 @@ pci_restore_srm_config(void) > > void pcibios_fixup_bus(struct pci_bus *bus) > { > - struct pci_dev *dev = bus->self; > - > - if (pci_has_flag(PCI_PROBE_ONLY) && dev && > - (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) { > - pci_read_bridge_bases(bus); > - } > + struct pci_dev *dev; > > list_for_each_entry(dev, &bus->devices, bus_list) { > pdev_save_srm_config(dev); > diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c > index f211839..f9c86c4 100644 > --- a/arch/frv/mb93090-mb00/pci-vdk.c > +++ b/arch/frv/mb93090-mb00/pci-vdk.c > @@ -294,8 +294,6 @@ void pcibios_fixup_bus(struct pci_bus *bus) > printk("### PCIBIOS_FIXUP_BUS(%d)\n",bus->number); > #endif > > - pci_read_bridge_bases(bus); > - > if (bus->number == 0) { > struct pci_dev *dev; > list_for_each_entry(dev, &bus->devices, bus_list) { > diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c > index 7cc3be9..563228b 100644 > --- a/arch/ia64/pci/pci.c > +++ b/arch/ia64/pci/pci.c > @@ -534,7 +534,6 @@ void pcibios_fixup_bus(struct pci_bus *b) > struct pci_dev *dev; > > if (b->self) { > - pci_read_bridge_bases(b); > pcibios_fixup_bridge_resources(b->self); > } > list_for_each_entry(dev, &b->devices, bus_list) > diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c > index ae838ed..6b8b752 100644 > --- a/arch/microblaze/pci/pci-common.c > +++ b/arch/microblaze/pci/pci-common.c > @@ -863,14 +863,7 @@ void pcibios_setup_bus_devices(struct pci_bus *bus) > > void pcibios_fixup_bus(struct pci_bus *bus) > { > - /* When called from the generic PCI probe, read PCI<->PCI bridge > - * bases. This is -not- called when generating the PCI tree from > - * the OF device-tree. > - */ > - if (bus->self != NULL) > - pci_read_bridge_bases(bus); > - > - /* Now fixup the bus bus */ > + /* Fixup the bus */ > pcibios_setup_bus_self(bus); > > /* Now fixup devices on that bus */ > diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c > index b8a0bf5..c6996cf 100644 > --- a/arch/mips/pci/pci.c > +++ b/arch/mips/pci/pci.c > @@ -311,12 +311,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) > > void pcibios_fixup_bus(struct pci_bus *bus) > { > - struct pci_dev *dev = bus->self; > - > - if (pci_has_flag(PCI_PROBE_ONLY) && dev && > - (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) { > - pci_read_bridge_bases(bus); > - } > } > > EXPORT_SYMBOL(PCIBIOS_MIN_IO); > diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c > index 3dfe2d3..deaa893 100644 > --- a/arch/mn10300/unit-asb2305/pci.c > +++ b/arch/mn10300/unit-asb2305/pci.c > @@ -324,7 +324,6 @@ void pcibios_fixup_bus(struct pci_bus *bus) > struct pci_dev *dev; > > if (bus->self) { > - pci_read_bridge_bases(bus); > pcibios_fixup_bridge_resources(bus->self); > } > > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c > index b9de34d..02c1d5d 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -1044,13 +1044,7 @@ void pcibios_set_master(struct pci_dev *dev) > > void pcibios_fixup_bus(struct pci_bus *bus) > { > - /* When called from the generic PCI probe, read PCI<->PCI bridge > - * bases. This is -not- called when generating the PCI tree from > - * the OF device-tree. > - */ > - pci_read_bridge_bases(bus); > - > - /* Now fixup the bus bus */ > + /* Fixup the bus */ > pcibios_setup_bus_self(bus); > > /* Now fixup devices on that bus */ > diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c > index 8fd6f44..3bff244 100644 > --- a/arch/x86/pci/common.c > +++ b/arch/x86/pci/common.c > @@ -166,7 +166,6 @@ void pcibios_fixup_bus(struct pci_bus *b) > { > struct pci_dev *dev; > > - pci_read_bridge_bases(b); > list_for_each_entry(dev, &b->devices, bus_list) > pcibios_fixup_device_resources(dev); > } > diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c > index b848cc3..d27b4dc 100644 > --- a/arch/xtensa/kernel/pci.c > +++ b/arch/xtensa/kernel/pci.c > @@ -210,10 +210,6 @@ subsys_initcall(pcibios_init); > > void pcibios_fixup_bus(struct pci_bus *bus) > { > - if (bus->parent) { > - /* This is a subordinate bridge */ > - pci_read_bridge_bases(bus); > - } > } > > void pcibios_set_master(struct pci_dev *dev) > diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c > index a0580af..baec33c 100644 > --- a/drivers/parisc/dino.c > +++ b/drivers/parisc/dino.c > @@ -560,9 +560,6 @@ dino_fixup_bus(struct pci_bus *bus) > } else if (bus->parent) { > int i; > > - pci_read_bridge_bases(bus); > - > - > for(i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) { > if((bus->self->resource[i].flags & > (IORESOURCE_IO | IORESOURCE_MEM)) == 0) > diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c > index dceb9dd..901e1a3 100644 > --- a/drivers/parisc/lba_pci.c > +++ b/drivers/parisc/lba_pci.c > @@ -693,7 +693,6 @@ lba_fixup_bus(struct pci_bus *bus) > if (bus->parent) { > int i; > /* PCI-PCI Bridge */ > - pci_read_bridge_bases(bus); > for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) > pci_claim_bridge_resource(bus->self, i); > } else { > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index c80626f..de1c424 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -842,6 +842,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) > child->bridge_ctl = bctl; > } > > + /* > + * Read and initialize bridge resources. > + */ > + pci_read_bridge_bases(child); > + > cmax = pci_scan_child_bus(child); > if (cmax > subordinate) > dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n", > @@ -902,6 +907,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) > > if (!is_cardbus) { > child->bridge_ctl = bctl; > + /* > + * Read and initialize bridge resources. > + */ > + pci_read_bridge_bases(child); > max = pci_scan_child_bus(child); > } else { > /* > -- > 2.2.1 > -- 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 07/22/2015 02:14 AM, Lorenzo Pieralisi wrote: > Bjorn, Guenter, > > On Thu, Jul 09, 2015 at 11:59:16AM +0100, Lorenzo Pieralisi wrote: >> When a PCI bus is scanned, upon PCI bridge detection the kernel >> has to read the bridge registers to set-up its resources so that >> the PCI resource hierarchy can be validated properly. >> >> Most if not all architectures read PCI bridge registers in the >> pcibios_fixup_bus hook, that is called by the PCI generic layer >> whenever a PCI bus is scanned. >> >> Since pci_read_bridge_bases is an arch agnostic operation (and it >> is carried out on all architectures) it can be moved to the generic >> PCI layer in order to consolidate code and remove the respective >> calls from the architectures back-ends. >> >> The PCI_PROBE_ONLY flag is not checked before calling >> pci_read_bridge_buses in the generic layer since reading the bridge >> bases is not related to resources assignment; this implies that it >> can be carried out safely on PCI_PROBE_ONLY systems too and should >> not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY >> flag before reading the bridge bases. >> >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> Cc: Ralf Baechle <ralf@linux-mips.org> >> Cc: James E.J. Bottomley <jejb@parisc-linux.org> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Cc: Bjorn Helgaas <bhelgaas@google.com> >> Cc: Richard Henderson <rth@twiddle.net> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: David Howells <dhowells@redhat.com> >> Cc: Russell King <linux@arm.linux.org.uk> >> Cc: Tony Luck <tony.luck@intel.com> >> Cc: David S. Miller <davem@davemloft.net> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Guenter Roeck <linux@roeck-us.net> >> Cc: Michal Simek <monstr@monstr.eu> >> Cc: Chris Zankel <chris@zankel.net> >> --- >> v2->v3: >> >> - Dropped RFC status >> - Removed bridge resources claiming from pci_read_bridge_bases() > > What do you want me to do with this patch ? I removed the resource > claiming - ie the only controversial bit, I can only test on > arm64, I would like to make some progress on this otherwise > I will have to add the bridge bases read to arm64 pcibios_fixup_bus() > and be done with this. > Lorenzo, please give me a few days to test it. I was out on vacation for the last two weeks, and I am still struggling to get my test systems back to a working state. Thanks, Guenter -- 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
Hi Guenter, On Wed, Jul 22, 2015 at 03:22:57PM +0100, Guenter Roeck wrote: > On 07/22/2015 02:14 AM, Lorenzo Pieralisi wrote: > > Bjorn, Guenter, > > > > On Thu, Jul 09, 2015 at 11:59:16AM +0100, Lorenzo Pieralisi wrote: > >> When a PCI bus is scanned, upon PCI bridge detection the kernel > >> has to read the bridge registers to set-up its resources so that > >> the PCI resource hierarchy can be validated properly. > >> > >> Most if not all architectures read PCI bridge registers in the > >> pcibios_fixup_bus hook, that is called by the PCI generic layer > >> whenever a PCI bus is scanned. > >> > >> Since pci_read_bridge_bases is an arch agnostic operation (and it > >> is carried out on all architectures) it can be moved to the generic > >> PCI layer in order to consolidate code and remove the respective > >> calls from the architectures back-ends. > >> > >> The PCI_PROBE_ONLY flag is not checked before calling > >> pci_read_bridge_buses in the generic layer since reading the bridge > >> bases is not related to resources assignment; this implies that it > >> can be carried out safely on PCI_PROBE_ONLY systems too and should > >> not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY > >> flag before reading the bridge bases. > >> > >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > >> Cc: Ralf Baechle <ralf@linux-mips.org> > >> Cc: James E.J. Bottomley <jejb@parisc-linux.org> > >> Cc: Michael Ellerman <mpe@ellerman.id.au> > >> Cc: Bjorn Helgaas <bhelgaas@google.com> > >> Cc: Richard Henderson <rth@twiddle.net> > >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >> Cc: David Howells <dhowells@redhat.com> > >> Cc: Russell King <linux@arm.linux.org.uk> > >> Cc: Tony Luck <tony.luck@intel.com> > >> Cc: David S. Miller <davem@davemloft.net> > >> Cc: Ingo Molnar <mingo@redhat.com> > >> Cc: Guenter Roeck <linux@roeck-us.net> > >> Cc: Michal Simek <monstr@monstr.eu> > >> Cc: Chris Zankel <chris@zankel.net> > >> --- > >> v2->v3: > >> > >> - Dropped RFC status > >> - Removed bridge resources claiming from pci_read_bridge_bases() > > > > What do you want me to do with this patch ? I removed the resource > > claiming - ie the only controversial bit, I can only test on > > arm64, I would like to make some progress on this otherwise > > I will have to add the bridge bases read to arm64 pcibios_fixup_bus() > > and be done with this. > > > > Lorenzo, > > please give me a few days to test it. I was out on vacation for > the last two weeks, and I am still struggling to get my test systems > back to a working state. No worries, I just wanted to make sure we can make progress on this, I do not think there is anything controversial in this set, it just requires testing the mechanical change on all affected archs, that's the problem. Thanks a lot for your help ! Lorenzo -- 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, Jul 09, 2015 at 11:59:16AM +0100, Lorenzo Pieralisi wrote: > When a PCI bus is scanned, upon PCI bridge detection the kernel > has to read the bridge registers to set-up its resources so that > the PCI resource hierarchy can be validated properly. > > Most if not all architectures read PCI bridge registers in the > pcibios_fixup_bus hook, that is called by the PCI generic layer > whenever a PCI bus is scanned. > > Since pci_read_bridge_bases is an arch agnostic operation (and it > is carried out on all architectures) it can be moved to the generic > PCI layer in order to consolidate code and remove the respective > calls from the architectures back-ends. > > The PCI_PROBE_ONLY flag is not checked before calling > pci_read_bridge_buses in the generic layer since reading the bridge > bases is not related to resources assignment; this implies that it > can be carried out safely on PCI_PROBE_ONLY systems too and should > not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY > flag before reading the bridge bases. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Applied to pci/resource for v4.3, thanks! The PCI_PROBE_ONLY text seems backward to me: previously alpha and mips only called pci_read_bridge_bases() if PCI_PROBE_ONLY was set. After this patch, alpha and mips systems that do not set PCI_PROBE_ONLY will also call pci_read_bridge_bases(). I really don't know why alpha and mips were like that. It seems backwards. It seems like we'd want to know the bridge windows if we were assigning things, but they only read them if they were *not* going to assign things. I'm a little uneasy that we might break some alpha or mips system, since there must have been some reason this was done originally. It'd be ideal if somebody could test a non-PCI_PROBE_ONLY system. But maybe they're all obsolete. I amended the changelog like this: PCI: Call pci_read_bridge_bases() from core instead of arch code When we scan a PCI bus, we read PCI-PCI bridge window registers with pci_read_bridge_bases() so we can validate the resource hierarchy. Most architectures call pci_read_bridge_bases() from pcibios_fixup_bus(), but PCI-PCI bridges are not arch-specific, so this doesn't need to be in arch-specific code. Call pci_read_bridge_bases() directly from the PCI core instead of from arch code. For alpha and mips, we now call pci_read_bridge_bases() always; previously we only called it if PCI_PROBE_ONLY was set. [bhelgaas: changelog] Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> ... -- 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 07/23/2015 09:12 AM, Bjorn Helgaas wrote: > On Thu, Jul 09, 2015 at 11:59:16AM +0100, Lorenzo Pieralisi wrote: >> When a PCI bus is scanned, upon PCI bridge detection the kernel >> has to read the bridge registers to set-up its resources so that >> the PCI resource hierarchy can be validated properly. >> >> Most if not all architectures read PCI bridge registers in the >> pcibios_fixup_bus hook, that is called by the PCI generic layer >> whenever a PCI bus is scanned. >> >> Since pci_read_bridge_bases is an arch agnostic operation (and it >> is carried out on all architectures) it can be moved to the generic >> PCI layer in order to consolidate code and remove the respective >> calls from the architectures back-ends. >> >> The PCI_PROBE_ONLY flag is not checked before calling >> pci_read_bridge_buses in the generic layer since reading the bridge >> bases is not related to resources assignment; this implies that it >> can be carried out safely on PCI_PROBE_ONLY systems too and should >> not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY >> flag before reading the bridge bases. >> >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Applied to pci/resource for v4.3, thanks! > > The PCI_PROBE_ONLY text seems backward to me: previously alpha and mips > only called pci_read_bridge_bases() if PCI_PROBE_ONLY was set. After this > patch, alpha and mips systems that do not set PCI_PROBE_ONLY will also call > pci_read_bridge_bases(). > > I really don't know why alpha and mips were like that. It seems backwards. > It seems like we'd want to know the bridge windows if we were assigning > things, but they only read them if they were *not* going to assign things. > > I'm a little uneasy that we might break some alpha or mips system, since > there must have been some reason this was done originally. It'd be ideal > if somebody could test a non-PCI_PROBE_ONLY system. But maybe they're all > obsolete. > For alpha, PCI_PROBE_ONLY is set for two platforms, marvel and titan, out of ~20. mips sets the flag for 6 platforms out of >25. Unlikely that those are the only relevant ones. I could try to run some qemu tests for both architectures, but I have no idea what to look out for. Ideas, anyone ? Guenter -- 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, Jul 23, 2015 at 05:12:57PM +0100, Bjorn Helgaas wrote: > On Thu, Jul 09, 2015 at 11:59:16AM +0100, Lorenzo Pieralisi wrote: > > When a PCI bus is scanned, upon PCI bridge detection the kernel > > has to read the bridge registers to set-up its resources so that > > the PCI resource hierarchy can be validated properly. > > > > Most if not all architectures read PCI bridge registers in the > > pcibios_fixup_bus hook, that is called by the PCI generic layer > > whenever a PCI bus is scanned. > > > > Since pci_read_bridge_bases is an arch agnostic operation (and it > > is carried out on all architectures) it can be moved to the generic > > PCI layer in order to consolidate code and remove the respective > > calls from the architectures back-ends. > > > > The PCI_PROBE_ONLY flag is not checked before calling > > pci_read_bridge_buses in the generic layer since reading the bridge > > bases is not related to resources assignment; this implies that it > > can be carried out safely on PCI_PROBE_ONLY systems too and should > > not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY > > flag before reading the bridge bases. > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Applied to pci/resource for v4.3, thanks! > > The PCI_PROBE_ONLY text seems backward to me: previously alpha and mips > only called pci_read_bridge_bases() if PCI_PROBE_ONLY was set. After this > patch, alpha and mips systems that do not set PCI_PROBE_ONLY will also call > pci_read_bridge_bases(). Yes you are right, your log is much clearer. > I really don't know why alpha and mips were like that. It seems backwards. > It seems like we'd want to know the bridge windows if we were assigning > things, but they only read them if they were *not* going to assign things. I do not know either I will check again what issues this might trigger (apart from some odd log messages) hopefully they are not reading the bases because they are reassigning them anyway on !PCI_PROBE_ONLY, I hope that's not subtler than that. Actually I noticed on alpha at least, they claim resources only on PCI_PROBE_ONLY and for that to succeed bridge bases must be read before, that may be an explanation but I do not know the reason. > I'm a little uneasy that we might break some alpha or mips system, since > there must have been some reason this was done originally. It'd be ideal > if somebody could test a non-PCI_PROBE_ONLY system. But maybe they're all > obsolete. I have no way to test them, that's the reason behind the RFT on this patch, I am uneasy too I do not if anyone can help us test it (maybe adding it to -next can help with that). Please let me know. Thanks ! Lorenzo > > I amended the changelog like this: > > PCI: Call pci_read_bridge_bases() from core instead of arch code > > When we scan a PCI bus, we read PCI-PCI bridge window registers with > pci_read_bridge_bases() so we can validate the resource hierarchy. Most > architectures call pci_read_bridge_bases() from pcibios_fixup_bus(), but > PCI-PCI bridges are not arch-specific, so this doesn't need to be in > arch-specific code. > > Call pci_read_bridge_bases() directly from the PCI core instead of from > arch code. > > For alpha and mips, we now call pci_read_bridge_bases() always; previously > we only called it if PCI_PROBE_ONLY was set. > > [bhelgaas: changelog] > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > ... > -- 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, Jul 23, 2015 at 05:47:44PM +0100, Guenter Roeck wrote: > On 07/23/2015 09:12 AM, Bjorn Helgaas wrote: > > On Thu, Jul 09, 2015 at 11:59:16AM +0100, Lorenzo Pieralisi wrote: > >> When a PCI bus is scanned, upon PCI bridge detection the kernel > >> has to read the bridge registers to set-up its resources so that > >> the PCI resource hierarchy can be validated properly. > >> > >> Most if not all architectures read PCI bridge registers in the > >> pcibios_fixup_bus hook, that is called by the PCI generic layer > >> whenever a PCI bus is scanned. > >> > >> Since pci_read_bridge_bases is an arch agnostic operation (and it > >> is carried out on all architectures) it can be moved to the generic > >> PCI layer in order to consolidate code and remove the respective > >> calls from the architectures back-ends. > >> > >> The PCI_PROBE_ONLY flag is not checked before calling > >> pci_read_bridge_buses in the generic layer since reading the bridge > >> bases is not related to resources assignment; this implies that it > >> can be carried out safely on PCI_PROBE_ONLY systems too and should > >> not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY > >> flag before reading the bridge bases. > >> > >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > Applied to pci/resource for v4.3, thanks! > > > > The PCI_PROBE_ONLY text seems backward to me: previously alpha and mips > > only called pci_read_bridge_bases() if PCI_PROBE_ONLY was set. After this > > patch, alpha and mips systems that do not set PCI_PROBE_ONLY will also call > > pci_read_bridge_bases(). > > > > I really don't know why alpha and mips were like that. It seems backwards. > > It seems like we'd want to know the bridge windows if we were assigning > > things, but they only read them if they were *not* going to assign things. > > > > I'm a little uneasy that we might break some alpha or mips system, since > > there must have been some reason this was done originally. It'd be ideal > > if somebody could test a non-PCI_PROBE_ONLY system. But maybe they're all > > obsolete. > > > > For alpha, PCI_PROBE_ONLY is set for two platforms, marvel and titan, > out of ~20. mips sets the flag for 6 platforms out of >25. > Unlikely that those are the only relevant ones. > > I could try to run some qemu tests for both architectures, but I have > no idea what to look out for. Ideas, anyone ? As I replied to Bjorn, I would not expect pci_read_bridge_bases() to be a disruptive operation on !PCI_PROBE_ONLY systems, I will check again alpha and mips code but it is really hard to say, I will inspect the code again to try to figure it out. Thanks ! Lorenzo -- 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 07/23/2015 10:52 AM, Lorenzo Pieralisi wrote: >> >> For alpha, PCI_PROBE_ONLY is set for two platforms, marvel and titan, >> out of ~20. mips sets the flag for 6 platforms out of >25. >> Unlikely that those are the only relevant ones. >> >> I could try to run some qemu tests for both architectures, but I have >> no idea what to look out for. Ideas, anyone ? > > As I replied to Bjorn, I would not expect pci_read_bridge_bases() to > be a disruptive operation on !PCI_PROBE_ONLY systems, I will check > again alpha and mips code but it is really hard to say, I will > inspect the code again to try to figure it out. > I finally got around to apply the patch to our system (ppc and x86). I also applied it on top of Linus' tree and ran it through my test farm. No problems found, works fine as far as I can see. Guenter -- 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, Jul 23, 2015 at 06:48:45PM +0100, Lorenzo Pieralisi wrote: > On Thu, Jul 23, 2015 at 05:12:57PM +0100, Bjorn Helgaas wrote: > > I'm a little uneasy that we might break some alpha or mips system, since > > there must have been some reason this was done originally. It'd be ideal > > if somebody could test a non-PCI_PROBE_ONLY system. But maybe they're all > > obsolete. > > I have no way to test them, that's the reason behind the RFT on this > patch, I am uneasy too I do not if anyone can help us test it > (maybe adding it to -next can help with that). I've got some dusty old alpha and mips machines w/pci at home if you want to borrow them? They were last seen running 3.0. Will -- 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 Fri, Jul 24, 2015 at 09:55:42AM +0100, Will Deacon wrote: > On Thu, Jul 23, 2015 at 06:48:45PM +0100, Lorenzo Pieralisi wrote: > > On Thu, Jul 23, 2015 at 05:12:57PM +0100, Bjorn Helgaas wrote: > > > I'm a little uneasy that we might break some alpha or mips system, since > > > there must have been some reason this was done originally. It'd be ideal > > > if somebody could test a non-PCI_PROBE_ONLY system. But maybe they're all > > > obsolete. > > > > I have no way to test them, that's the reason behind the RFT on this > > patch, I am uneasy too I do not if anyone can help us test it > > (maybe adding it to -next can help with that). > > I've got some dusty old alpha and mips machines w/pci at home if you > want to borrow them? They were last seen running 3.0. Yes it would be very useful to check their PCI resources allocation policy and test this patch on them. Thank you ! Lorenzo -- 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/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c index 82f738e..cded02c 100644 --- a/arch/alpha/kernel/pci.c +++ b/arch/alpha/kernel/pci.c @@ -242,12 +242,7 @@ pci_restore_srm_config(void) void pcibios_fixup_bus(struct pci_bus *bus) { - struct pci_dev *dev = bus->self; - - if (pci_has_flag(PCI_PROBE_ONLY) && dev && - (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) { - pci_read_bridge_bases(bus); - } + struct pci_dev *dev; list_for_each_entry(dev, &bus->devices, bus_list) { pdev_save_srm_config(dev); diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c index f211839..f9c86c4 100644 --- a/arch/frv/mb93090-mb00/pci-vdk.c +++ b/arch/frv/mb93090-mb00/pci-vdk.c @@ -294,8 +294,6 @@ void pcibios_fixup_bus(struct pci_bus *bus) printk("### PCIBIOS_FIXUP_BUS(%d)\n",bus->number); #endif - pci_read_bridge_bases(bus); - if (bus->number == 0) { struct pci_dev *dev; list_for_each_entry(dev, &bus->devices, bus_list) { diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c index 7cc3be9..563228b 100644 --- a/arch/ia64/pci/pci.c +++ b/arch/ia64/pci/pci.c @@ -534,7 +534,6 @@ void pcibios_fixup_bus(struct pci_bus *b) struct pci_dev *dev; if (b->self) { - pci_read_bridge_bases(b); pcibios_fixup_bridge_resources(b->self); } list_for_each_entry(dev, &b->devices, bus_list) diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c index ae838ed..6b8b752 100644 --- a/arch/microblaze/pci/pci-common.c +++ b/arch/microblaze/pci/pci-common.c @@ -863,14 +863,7 @@ void pcibios_setup_bus_devices(struct pci_bus *bus) void pcibios_fixup_bus(struct pci_bus *bus) { - /* When called from the generic PCI probe, read PCI<->PCI bridge - * bases. This is -not- called when generating the PCI tree from - * the OF device-tree. - */ - if (bus->self != NULL) - pci_read_bridge_bases(bus); - - /* Now fixup the bus bus */ + /* Fixup the bus */ pcibios_setup_bus_self(bus); /* Now fixup devices on that bus */ diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c index b8a0bf5..c6996cf 100644 --- a/arch/mips/pci/pci.c +++ b/arch/mips/pci/pci.c @@ -311,12 +311,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) void pcibios_fixup_bus(struct pci_bus *bus) { - struct pci_dev *dev = bus->self; - - if (pci_has_flag(PCI_PROBE_ONLY) && dev && - (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) { - pci_read_bridge_bases(bus); - } } EXPORT_SYMBOL(PCIBIOS_MIN_IO); diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c index 3dfe2d3..deaa893 100644 --- a/arch/mn10300/unit-asb2305/pci.c +++ b/arch/mn10300/unit-asb2305/pci.c @@ -324,7 +324,6 @@ void pcibios_fixup_bus(struct pci_bus *bus) struct pci_dev *dev; if (bus->self) { - pci_read_bridge_bases(bus); pcibios_fixup_bridge_resources(bus->self); } diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index b9de34d..02c1d5d 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -1044,13 +1044,7 @@ void pcibios_set_master(struct pci_dev *dev) void pcibios_fixup_bus(struct pci_bus *bus) { - /* When called from the generic PCI probe, read PCI<->PCI bridge - * bases. This is -not- called when generating the PCI tree from - * the OF device-tree. - */ - pci_read_bridge_bases(bus); - - /* Now fixup the bus bus */ + /* Fixup the bus */ pcibios_setup_bus_self(bus); /* Now fixup devices on that bus */ diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c index 8fd6f44..3bff244 100644 --- a/arch/x86/pci/common.c +++ b/arch/x86/pci/common.c @@ -166,7 +166,6 @@ void pcibios_fixup_bus(struct pci_bus *b) { struct pci_dev *dev; - pci_read_bridge_bases(b); list_for_each_entry(dev, &b->devices, bus_list) pcibios_fixup_device_resources(dev); } diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c index b848cc3..d27b4dc 100644 --- a/arch/xtensa/kernel/pci.c +++ b/arch/xtensa/kernel/pci.c @@ -210,10 +210,6 @@ subsys_initcall(pcibios_init); void pcibios_fixup_bus(struct pci_bus *bus) { - if (bus->parent) { - /* This is a subordinate bridge */ - pci_read_bridge_bases(bus); - } } void pcibios_set_master(struct pci_dev *dev) diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c index a0580af..baec33c 100644 --- a/drivers/parisc/dino.c +++ b/drivers/parisc/dino.c @@ -560,9 +560,6 @@ dino_fixup_bus(struct pci_bus *bus) } else if (bus->parent) { int i; - pci_read_bridge_bases(bus); - - for(i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) { if((bus->self->resource[i].flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0) diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c index dceb9dd..901e1a3 100644 --- a/drivers/parisc/lba_pci.c +++ b/drivers/parisc/lba_pci.c @@ -693,7 +693,6 @@ lba_fixup_bus(struct pci_bus *bus) if (bus->parent) { int i; /* PCI-PCI Bridge */ - pci_read_bridge_bases(bus); for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) pci_claim_bridge_resource(bus->self, i); } else { diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index c80626f..de1c424 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -842,6 +842,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) child->bridge_ctl = bctl; } + /* + * Read and initialize bridge resources. + */ + pci_read_bridge_bases(child); + cmax = pci_scan_child_bus(child); if (cmax > subordinate) dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n", @@ -902,6 +907,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) if (!is_cardbus) { child->bridge_ctl = bctl; + /* + * Read and initialize bridge resources. + */ + pci_read_bridge_bases(child); max = pci_scan_child_bus(child); } else { /*
When a PCI bus is scanned, upon PCI bridge detection the kernel has to read the bridge registers to set-up its resources so that the PCI resource hierarchy can be validated properly. Most if not all architectures read PCI bridge registers in the pcibios_fixup_bus hook, that is called by the PCI generic layer whenever a PCI bus is scanned. Since pci_read_bridge_bases is an arch agnostic operation (and it is carried out on all architectures) it can be moved to the generic PCI layer in order to consolidate code and remove the respective calls from the architectures back-ends. The PCI_PROBE_ONLY flag is not checked before calling pci_read_bridge_buses in the generic layer since reading the bridge bases is not related to resources assignment; this implies that it can be carried out safely on PCI_PROBE_ONLY systems too and should not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY flag before reading the bridge bases. Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: James E.J. Bottomley <jejb@parisc-linux.org> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: David Howells <dhowells@redhat.com> Cc: Russell King <linux@arm.linux.org.uk> Cc: Tony Luck <tony.luck@intel.com> Cc: David S. Miller <davem@davemloft.net> Cc: Ingo Molnar <mingo@redhat.com> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Michal Simek <monstr@monstr.eu> Cc: Chris Zankel <chris@zankel.net> --- v2->v3: - Dropped RFC status - Removed bridge resources claiming from pci_read_bridge_bases() v2: https://lkml.org/lkml/2015/6/9/187 v1->v2: - Moved pci_read_bridge_bases call to pci_scan_bridge to read bases before scanning devices - Added bridge resources claiming v1: https://lkml.org/lkml/2015/5/21/359 arch/alpha/kernel/pci.c | 7 +------ arch/frv/mb93090-mb00/pci-vdk.c | 2 -- arch/ia64/pci/pci.c | 1 - arch/microblaze/pci/pci-common.c | 9 +-------- arch/mips/pci/pci.c | 6 ------ arch/mn10300/unit-asb2305/pci.c | 1 - arch/powerpc/kernel/pci-common.c | 8 +------- arch/x86/pci/common.c | 1 - arch/xtensa/kernel/pci.c | 4 ---- drivers/parisc/dino.c | 3 --- drivers/parisc/lba_pci.c | 1 - drivers/pci/probe.c | 9 +++++++++ 12 files changed, 12 insertions(+), 40 deletions(-)