pci_probe_only and bus_to_resources patches rebased to pci linux-next branch

pci_probe_only and bus_to_resources patches rebased to pci linux-next branch

[Bjorn Helgaas]

Hi Jesse,

I rebased these series:

    v2 generic pci_probe_only flag
    (http://marc.info/?l=linux-pci&m=133002621016744&w=2)

    v2 add PCI bus-to-resource offset support in core
    (http://marc.info/?l=linux-pci&m=132884138928641&w=2)

onto your current linux-next tree (based on commit 823806ff).  Minor
conflict resolutions in include/linux/pci.h, and that's about it.

You can examine the result here:

    https://github.com/bjorn-helgaas/linux/tree/pci-next+probe_only+bus2res-fb127cb

or pull both series with:

    git pull git://github.com/bjorn-helgaas/linux.git
pci-next+probe_only+bus2res-fb127cb

Bjorn

Re: pci_probe_only and bus_to_resources patches rebased to pci linux-next branch

[Yinghai Lu]

On Thu, Feb 23, 2012 at 9:04 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Hi Jesse,
>
> I rebased these series:
>

>    v2 add PCI bus-to-resource offset support in core
>    (http://marc.info/?l=linux-pci&m=132884138928641&w=2)
>

did you solve concerns that i raised?
1. two lists that one for host bridge and one for root buses
2. for dev to get host bridge data is some expensive. Now for every
dev, you need go up to find root bus, and loop the host bridge list to
find the host bridge.

     Yinghai

Re: pci_probe_only and bus_to_resources patches rebased to pci linux-next branch

[Bjorn Helgaas]

On Thu, Feb 23, 2012 at 9:23 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Feb 23, 2012 at 9:04 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> Hi Jesse,
>>
>> I rebased these series:
>
>>    v2 add PCI bus-to-resource offset support in core
>>    (http://marc.info/?l=linux-pci&m=132884138928641&w=2)
>
> did you solve concerns that i raised?
> 1. two lists that one for host bridge and one for root buses
> 2. for dev to get host bridge data is some expensive. Now for every
> dev, you need go up to find root bus, and loop the host bridge list to
> find the host bridge.

No.  I responded to those concerns on the list, but I haven't made any
changes to the patches yet.

I'll look at #1 again; we should do something about that, though I
don't think it's important enough to gate the patches.

I don't think #2 is worth worrying about, given that we do it so
infrequently (once per BAR at boot-time).

Bjorn

Re: pci_probe_only and bus_to_resources patches rebased to pci linux-next branch

[Bjorn Helgaas]

On Fri, Feb 24, 2012 at 10:38 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Feb 23, 2012 at 9:23 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Thu, Feb 23, 2012 at 9:04 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> Hi Jesse,
>>>
>>> I rebased these series:
>>
>>>    v2 add PCI bus-to-resource offset support in core
>>>    (http://marc.info/?l=linux-pci&m=132884138928641&w=2)
>>
>> did you solve concerns that i raised?
>> 1. two lists that one for host bridge and one for root buses

I looked at all the uses of the pci_root_buses list, and I think
almost all of them should be removed by restructuring the code.

alpha, frv, microblaze, mn10300, powerpc, and x86 use pci_root_buses
to find all PCI devices, call pci_claim_resource(), and do some
resource allocation.  x86 is fairly typical:

    pci_subsys_init (subsys_initcall)
      pcibios_init
        pcibios_resource_survey
          pcibios_allocate_bus_resources(&pci_root_buses)
            list_for_each_entry(each root bus):
              for_each_bridge_window:
                pci_claim_resource
              pcibios_allocate_bus_resources(&bus->children)
          pcibios_allocate_resources(0)
          pcibios_allocate_resources(1)

I think this is a broken model.  We only call
pcibios_resource_survey() at boot-time, so hotplug devices are treated
differently, which is a problem.  Some architectures use
pci_claim_resource() in a different path, or not at all.  That's
another problem.  None of this stuff is really architecture-dependent,
so having a half-dozen separate implementations is adding complexity
and inconsistencies without any benefit.

In addition to the uses in pcibios_resource_survey(), pci_root_buses
is also used by some ARM PCI IRQ handlers, a powerpc syscall
(sys_pciconfig_iobase()) and DRM (on alpha only).  I suspect these
uses could be removed by keeping a little more architecture-specific
state.

There are also a few uses in the PCI core, but again, these are mostly
in resource allocation paths that I think are not correctly designed.
PCI resource allocation is not a system-wide problem.  It's at most a
per-host bridge problem.  We should not be iterating over all PCI root
buses; rather, we should be doing a piece of the allocation every time
we scan below a host bridge.

I don't want to trivialize the work here.  Restructuring this is a
large amount of work.  But I think it's good to have some idea of
where things should go.

I *could* trivially remove the pci_root_buses usages by converting
them all to iterators over the pci_host_bridges list.  But I don't
really want to do that because it polishes code that I think should be
removed completely, and it would require exporting pci_host_bridges
outside the PCI core, where I don't think it should go.

Bjorn

Re: pci_probe_only and bus_to_resources patches rebased to pci linux-next branch

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

On Thu, 23 Feb 2012 21:04:04 -0800
Bjorn Helgaas <bhelgaas@google.com> wrote:

> Hi Jesse,
> 
> I rebased these series:
> 
>     v2 generic pci_probe_only flag
>     (http://marc.info/?l=linux-pci&m=133002621016744&w=2)
> 
>     v2 add PCI bus-to-resource offset support in core
>     (http://marc.info/?l=linux-pci&m=132884138928641&w=2)
> 
> onto your current linux-next tree (based on commit 823806ff).  Minor
> conflict resolutions in include/linux/pci.h, and that's about it.
> 
> You can examine the result here:
> 
>     https://github.com/bjorn-helgaas/linux/tree/pci-next+probe_only+bus2res-fb127cb
> 
> or pull both series with:
> 
>     git pull git://github.com/bjorn-helgaas/linux.git
> pci-next+probe_only+bus2res-fb127cb

Nice, I like all the deletions especially.

Pulled into linux-next.

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: pci_probe_only and bus_to_resources patches rebased to pci linux-next branch

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On Fri, Feb 24, 2012 at 2:27 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> or pull both series with:
>>
>>     git pull git://github.com/bjorn-helgaas/linux.git
>> pci-next+probe_only+bus2res-fb127cb
>
> Nice, I like all the deletions especially.
>
> Pulled into linux-next.

That bus2res is some kind of punishment for arch like x86 that does
not need offset resources.

Please put attached two patches into tree to remove the side affects on x86.

Thanks

Yinghai

[-- Attachment #2: fix_bus_offset_x86_1.patch --]
[-- Type: text/x-patch, Size: 6766 bytes --]

Subject: [PATCH] PCI: Separate host_bridge code out from probe.c

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/Makefile      |    2 
 drivers/pci/host-bridge.c |   95 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h         |    2 
 drivers/pci/probe.c       |   81 ---------------------------------------
 4 files changed, 99 insertions(+), 81 deletions(-)

Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/pci/host-bridge.c
@@ -0,0 +1,93 @@
+/*
+ * host_bridge.c - host bridge related code
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <linux/module.h>
+
+#include "pci.h"
+
+static LIST_HEAD(pci_host_bridges);
+
+void add_to_pci_host_bridges(struct list_head *list)
+{
+	list_add_tail(list, &pci_host_bridges);
+}
+
+static struct pci_host_bridge *pci_host_bridge(struct pci_dev *dev)
+{
+	struct pci_bus *bus;
+	struct pci_host_bridge *bridge;
+
+	bus = dev->bus;
+	while (bus->parent)
+		bus = bus->parent;
+
+	list_for_each_entry(bridge, &pci_host_bridges, list) {
+		if (bridge->bus == bus)
+			return bridge;
+	}
+
+	return NULL;
+}
+
+static bool resource_contains(struct resource *res1, struct resource *res2)
+{
+	return res1->start <= res2->start && res1->end >= res2->end;
+}
+
+void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
+			     struct resource *res)
+{
+	struct pci_host_bridge *bridge = pci_host_bridge(dev);
+	struct pci_host_bridge_window *window;
+	resource_size_t offset = 0;
+
+	list_for_each_entry(window, &bridge->windows, list) {
+		if (resource_type(res) != resource_type(window->res))
+			continue;
+
+		if (resource_contains(window->res, res)) {
+			offset = window->offset;
+			break;
+		}
+	}
+
+	region->start = res->start - offset;
+	region->end = res->end - offset;
+}
+EXPORT_SYMBOL(pcibios_resource_to_bus);
+
+static bool region_contains(struct pci_bus_region *region1,
+			    struct pci_bus_region *region2)
+{
+	return region1->start <= region2->start && region1->end >= region2->end;
+}
+
+void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+			     struct pci_bus_region *region)
+{
+	struct pci_host_bridge *bridge = pci_host_bridge(dev);
+	struct pci_host_bridge_window *window;
+	struct pci_bus_region bus_region;
+	resource_size_t offset = 0;
+
+	list_for_each_entry(window, &bridge->windows, list) {
+		if (resource_type(res) != resource_type(window->res))
+			continue;
+
+		bus_region.start = window->res->start - window->offset;
+		bus_region.end = window->res->end - window->offset;
+
+		if (region_contains(&bus_region, region)) {
+			offset = window->offset;
+			break;
+		}
+	}
+
+	res->start = region->start + offset;
+	res->end = region->end + offset;
+}
+EXPORT_SYMBOL(pcibios_bus_to_resource);
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -233,6 +233,8 @@ static inline int pci_ari_enabled(struct
 void pci_reassigndev_resource_alignment(struct pci_dev *dev);
 extern void pci_disable_bridge_window(struct pci_dev *dev);
 
+void add_to_pci_host_bridges(struct list_head *list);
+
 /* Single Root I/O Virtualization */
 struct pci_sriov {
 	int pos;		/* capability position */
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -15,13 +15,10 @@
 #define CARDBUS_LATENCY_TIMER	176	/* secondary latency timer */
 #define CARDBUS_RESERVE_BUSNR	3
 
-static LIST_HEAD(pci_host_bridges);
-
 /* Ugh.  Need to stop exporting this to modules. */
 LIST_HEAD(pci_root_buses);
 EXPORT_SYMBOL(pci_root_buses);
 
-
 static int find_anything(struct device *dev, void *data)
 {
 	return 1;
@@ -44,82 +41,6 @@ int no_pci_devices(void)
 }
 EXPORT_SYMBOL(no_pci_devices);
 
-static struct pci_host_bridge *pci_host_bridge(struct pci_dev *dev)
-{
-	struct pci_bus *bus;
-	struct pci_host_bridge *bridge;
-
-	bus = dev->bus;
-	while (bus->parent)
-		bus = bus->parent;
-
-	list_for_each_entry(bridge, &pci_host_bridges, list) {
-		if (bridge->bus == bus)
-			return bridge;
-	}
-
-	return NULL;
-}
-
-static bool resource_contains(struct resource *res1, struct resource *res2)
-{
-	return res1->start <= res2->start && res1->end >= res2->end;
-}
-
-void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
-			     struct resource *res)
-{
-	struct pci_host_bridge *bridge = pci_host_bridge(dev);
-	struct pci_host_bridge_window *window;
-	resource_size_t offset = 0;
-
-	list_for_each_entry(window, &bridge->windows, list) {
-		if (resource_type(res) != resource_type(window->res))
-			continue;
-
-		if (resource_contains(window->res, res)) {
-			offset = window->offset;
-			break;
-		}
-	}
-
-	region->start = res->start - offset;
-	region->end = res->end - offset;
-}
-EXPORT_SYMBOL(pcibios_resource_to_bus);
-
-static bool region_contains(struct pci_bus_region *region1,
-			    struct pci_bus_region *region2)
-{
-	return region1->start <= region2->start && region1->end >= region2->end;
-}
-
-void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
-			     struct pci_bus_region *region)
-{
-	struct pci_host_bridge *bridge = pci_host_bridge(dev);
-	struct pci_host_bridge_window *window;
-	struct pci_bus_region bus_region;
-	resource_size_t offset = 0;
-
-	list_for_each_entry(window, &bridge->windows, list) {
-		if (resource_type(res) != resource_type(window->res))
-			continue;
-
-		bus_region.start = window->res->start - window->offset;
-		bus_region.end = window->res->end - window->offset;
-
-		if (region_contains(&bus_region, region)) {
-			offset = window->offset;
-			break;
-		}
-	}
-
-	res->start = region->start + offset;
-	res->end = region->end + offset;
-}
-EXPORT_SYMBOL(pcibios_bus_to_resource);
-
 /*
  * PCI Bus Class
  */
@@ -1959,7 +1880,7 @@ struct pci_bus *pci_create_root_bus(stru
 	}
 
 	down_write(&pci_bus_sem);
-	list_add_tail(&bridge->list, &pci_host_bridges);
+	add_to_pci_host_bridges(&bridge->list);
 	list_add_tail(&b->node, &pci_root_buses);
 	up_write(&pci_bus_sem);
 
Index: linux-2.6/drivers/pci/Makefile
===================================================================
--- linux-2.6.orig/drivers/pci/Makefile
+++ linux-2.6/drivers/pci/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the PCI bus specific drivers.
 #
 
-obj-y		+= access.o bus.o probe.o remove.o pci.o \
+obj-y		+= access.o bus.o probe.o host-bridge.o remove.o pci.o \
 			pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
 			irq.o vpd.o
 obj-$(CONFIG_PROC_FS) += proc.o

[-- Attachment #3: fix_bus_offset_x86_2.patch --]
[-- Type: text/x-patch, Size: 2234 bytes --]

Subject: [PATCH] x86, PCI: have own version for pcibios_bus_to_resource

x86 does not need to offset the address. So we can skip that costing offset
searching.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/i386.c       |   14 ++++++++++++++
 drivers/pci/host-bridge.c |    9 +++++----
 2 files changed, 19 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/x86/pci/i386.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/i386.c
+++ linux-2.6/arch/x86/pci/i386.c
@@ -335,6 +335,20 @@ void __init pcibios_resource_survey(void
  */
 fs_initcall(pcibios_assign_resources);
 
+void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
+			     struct resource *res)
+{
+	region->start = res->start;
+	region->end = res->end;
+}
+
+void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+			     struct pci_bus_region *region)
+{
+	res->start = region->start;
+	res->end = region->end;
+}
+
 static const struct vm_operations_struct pci_mmap_ops = {
 	.access = generic_access_phys,
 };
Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================
--- linux-2.6.orig/drivers/pci/host-bridge.c
+++ linux-2.6/drivers/pci/host-bridge.c
@@ -38,8 +38,9 @@ static bool resource_contains(struct res
 	return res1->start <= res2->start && res1->end >= res2->end;
 }
 
-void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
-			     struct resource *res)
+void __weak pcibios_resource_to_bus(struct pci_dev *dev,
+				    struct pci_bus_region *region,
+				    struct resource *res)
 {
 	struct pci_host_bridge *bridge = pci_host_bridge(dev);
 	struct pci_host_bridge_window *window;
@@ -66,8 +67,8 @@ static bool region_contains(struct pci_b
 	return region1->start <= region2->start && region1->end >= region2->end;
 }
 
-void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
-			     struct pci_bus_region *region)
+void __weak pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+				    struct pci_bus_region *region)
 {
 	struct pci_host_bridge *bridge = pci_host_bridge(dev);
 	struct pci_host_bridge_window *window;