[PATCH -v2 0/13] PCI: allocate pci bus num range for unassigned bridge busn

[PATCH -v2 0/13] PCI: allocate pci bus num range for unassigned bridge busn

[Yinghai Lu]

82b166c: PCI: Seperate child bus scanning to two passes overall
dd174b3: PCI: kill pci_fixup_parent_subordinate_busnr()
6184c18: PCI: Allocate bus range instead of use max blindly
11525b2: PCI: Strict checking of valid range for bridge
05c754e: PCI: Probe safe range that we can use for unassigned bridge.
8bb27f2: PCI: Add pci_bus_extend/shrink_top()
76bb4b4: PCI, parisc: Register busn_res for root buses
36042e6: PCI, powerpc: Register busn_res for root buses
53472fe: PCI, ia64: Register busn_res for root buses
740a84d: PCI, x86: Register busn_res for root buses
a5ddf08: PCI: Add busn_res tracking in core
3810f01: PCI: Add busn_res operation functions
bd3ab8d: PCI: Add iobusn_resource

Set up iobusn_resource tree, and register bus number range to it.
Later when need to find bus range, will try to allocate from the tree

Need to test on arches other than x86. esp for ia64 and powerpc that support
  more than on peer root buses.

could be found at:
	git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-busn-alloc

-v2: according to Jesse, split to more small patches.

Thanks

Yinghai

 arch/ia64/pci/pci.c              |    2 +
 arch/powerpc/kernel/pci-common.c |    7 +-
 arch/x86/include/asm/topology.h  |    3 +-
 arch/x86/pci/acpi.c              |    8 +-
 arch/x86/pci/bus_numa.c          |    8 +-
 arch/x86/pci/common.c            |   11 +-
 drivers/parisc/dino.c            |    2 +
 drivers/parisc/lba_pci.c         |    3 +
 drivers/pci/probe.c              |  427 ++++++++++++++++++++++++++++++--------
 drivers/pci/remove.c             |    1 +
 include/linux/ioport.h           |    1 +
 include/linux/pci.h              |    8 +
 kernel/resource.c                |    8 +
 13 files changed, 395 insertions(+), 94 deletions(-)

[PATCH 01/13] PCI: Add iobusn_resource

[Yinghai Lu]

also add busn_res into struct pci_bus.

will use them to have bus number resource tree.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 include/linux/ioport.h |    1 +
 include/linux/pci.h    |    1 +
 kernel/resource.c      |    8 ++++++++
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index e885ba2..6fe9e19 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -136,6 +136,7 @@ struct resource {
 /* PC/ISA/whatever - the normal PC address spaces: IO and memory */
 extern struct resource ioport_resource;
 extern struct resource iomem_resource;
+extern struct resource iobusn_resource;
 
 extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
 extern int request_resource(struct resource *root, struct resource *new);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f8caaab..94ad468 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -419,6 +419,7 @@ struct pci_bus {
 	struct list_head slots;		/* list of slots on this bus */
 	struct resource *resource[PCI_BRIDGE_RESOURCE_NUM];
 	struct list_head resources;	/* address space routed to this bus */
+	struct resource busn_res;	/* track registered bus num range */
 
 	struct pci_ops	*ops;		/* configuration access functions */
 	void		*sysdata;	/* hook for sys-specific extension */
diff --git a/kernel/resource.c b/kernel/resource.c
index 7640b3a..53b42f0 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -38,6 +38,14 @@ struct resource iomem_resource = {
 };
 EXPORT_SYMBOL(iomem_resource);
 
+struct resource iobusn_resource = {
+	.name	= "PCI busn",
+	.start	= 0,
+	.end	= 0xffffff,
+	.flags	= IORESOURCE_BUS,
+};
+EXPORT_SYMBOL(iobusn_resource);
+
 /* constraints to be met while allocating resources */
 struct resource_constraint {
 	resource_size_t min, max, align;
-- 
1.7.7

[PATCH 02/13] PCI: Add busn_res operation functions

[Yinghai Lu]

will use them insert/update busn res in pci_bus

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a114173..71261da 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1622,6 +1622,50 @@ err_out:
 	return NULL;
 }
 
+void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
+{
+	struct resource *res = &b->busn_res;
+	struct resource *parent_res = &iobusn_resource;
+	int ret;
+
+	res->start = (pci_domain_nr(b) << 8) | bus;
+	res->end = (pci_domain_nr(b) << 8) | bus_max;
+	res->flags = IORESOURCE_BUS;
+
+	if (!pci_is_root_bus(b))
+		parent_res = &b->parent->busn_res;
+
+	ret = insert_resource(parent_res, res);
+
+	dev_printk(KERN_DEBUG, &b->dev,
+	 "busn_res: %06llx-%06llx %s inserted under %06llx-%06llx\n",
+			(unsigned long long)res->start,
+			(unsigned long long)res->end,
+			 ret ? "can not be" : "is",
+			(unsigned long long)parent_res->start,
+			(unsigned long long)parent_res->end);
+}
+
+void pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max)
+{
+	struct resource *res = &b->busn_res;
+	unsigned long long old_end = res->end;
+
+	res->end &= ~0xff;
+	res->end |= bus_max;
+	dev_printk(KERN_DEBUG, &b->dev,
+		 "busn_res: %06llx-%06llx end updated to %06llx-%06llx\n",
+			(unsigned long long)res->start,
+			old_end,
+			(unsigned long long)res->start,
+			(unsigned long long)res->end);
+}
+
+void pci_bus_release_busn_res(struct pci_bus *b)
+{
+	release_resource(&b->busn_res);
+}
+
 struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 		struct pci_ops *ops, void *sysdata, struct list_head *resources)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 94ad468..3da935c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -665,6 +665,9 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
 				    struct list_head *resources);
+void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
+void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
+void pci_bus_release_busn_res(struct pci_bus *b);
 struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 					     struct pci_ops *ops, void *sysdata,
 					     struct list_head *resources);
-- 
1.7.7

Re: [PATCH 02/13] PCI: Add busn_res operation functions

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> will use them insert/update busn res in pci_bus
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 47 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index a114173..71261da 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1622,6 +1622,50 @@ err_out:
>        return NULL;
>  }
>
> +void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
> +{
> +       struct resource *res = &b->busn_res;
> +       struct resource *parent_res = &iobusn_resource;
> +       int ret;
> +
> +       res->start = (pci_domain_nr(b) << 8) | bus;
> +       res->end = (pci_domain_nr(b) << 8) | bus_max;
> +       res->flags = IORESOURCE_BUS;
> +
> +       if (!pci_is_root_bus(b))
> +               parent_res = &b->parent->busn_res;
> +
> +       ret = insert_resource(parent_res, res);
> +
> +       dev_printk(KERN_DEBUG, &b->dev,
> +        "busn_res: %06llx-%06llx %s inserted under %06llx-%06llx\n",
> +                       (unsigned long long)res->start,
> +                       (unsigned long long)res->end,
> +                        ret ? "can not be" : "is",
> +                       (unsigned long long)parent_res->start,
> +                       (unsigned long long)parent_res->end);

Please update the %pR implementation so you can use it instead of
having to fiddle around with start/end separately and casts.

> +}
> +
> +void pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max)
> +{
> +       struct resource *res = &b->busn_res;
> +       unsigned long long old_end = res->end;
> +
> +       res->end &= ~0xff;
> +       res->end |= bus_max;

This updates a resource in place, without removing it from the
resource tree and re-inserting it.  Doesn't this make it possible to
corrupt the resource tree?  For example, let's say you have this tree:

    00-3f : host bridge A
      00-1f : P2P bridge B
    40-7f : host bridge C

Now you use pci_bus_update_busn_res_end() to update the resource for
P2P bridge B,  setting its end to 5f.  I think the resource tree is
now corrupted.

> +       dev_printk(KERN_DEBUG, &b->dev,
> +                "busn_res: %06llx-%06llx end updated to %06llx-%06llx\n",
> +                       (unsigned long long)res->start,
> +                       old_end,
> +                       (unsigned long long)res->start,
> +                       (unsigned long long)res->end);
> +}
> +
> +void pci_bus_release_busn_res(struct pci_bus *b)
> +{
> +       release_resource(&b->busn_res);
> +}
> +
>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>                struct pci_ops *ops, void *sysdata, struct list_head *resources)
>  {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 94ad468..3da935c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -665,6 +665,9 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>                                    struct pci_ops *ops, void *sysdata,
>                                    struct list_head *resources);
> +void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
> +void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
> +void pci_bus_release_busn_res(struct pci_bus *b);
>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>                                             struct pci_ops *ops, void *sysdata,
>                                             struct list_head *resources);
> --
> 1.7.7
>

Re: [PATCH 02/13] PCI: Add busn_res operation functions

[Yinghai Lu]

On Mon, Jan 30, 2012 at 7:28 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> will use them insert/update busn res in pci_bus
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/probe.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/pci.h |    3 +++
>>  2 files changed, 47 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index a114173..71261da 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1622,6 +1622,50 @@ err_out:
>>        return NULL;
>>  }
>>
>> +void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
>> +{
>> +       struct resource *res = &b->busn_res;
>> +       struct resource *parent_res = &iobusn_resource;
>> +       int ret;
>> +
>> +       res->start = (pci_domain_nr(b) << 8) | bus;
>> +       res->end = (pci_domain_nr(b) << 8) | bus_max;
>> +       res->flags = IORESOURCE_BUS;
>> +
>> +       if (!pci_is_root_bus(b))
>> +               parent_res = &b->parent->busn_res;
>> +
>> +       ret = insert_resource(parent_res, res);
>> +
>> +       dev_printk(KERN_DEBUG, &b->dev,
>> +        "busn_res: %06llx-%06llx %s inserted under %06llx-%06llx\n",
>> +                       (unsigned long long)res->start,
>> +                       (unsigned long long)res->end,
>> +                        ret ? "can not be" : "is",
>> +                       (unsigned long long)parent_res->start,
>> +                       (unsigned long long)parent_res->end);
>
> Please update the %pR implementation so you can use it instead of
> having to fiddle around with start/end separately and casts.

ok, will check that. to make it support extra domain number there.

>
>> +}
>> +
>> +void pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max)
>> +{
>> +       struct resource *res = &b->busn_res;
>> +       unsigned long long old_end = res->end;
>> +
>> +       res->end &= ~0xff;
>> +       res->end |= bus_max;
>
> This updates a resource in place, without removing it from the
> resource tree and re-inserting it.  Doesn't this make it possible to
> corrupt the resource tree?  For example, let's say you have this tree:
>
>    00-3f : host bridge A
>      00-1f : P2P bridge B
>    40-7f : host bridge C
>
> Now you use pci_bus_update_busn_res_end() to update the resource for
> P2P bridge B,  setting its end to 5f.  I think the resource tree is
> now corrupted.

for expanding, We will make sure expanded range is safe and verified.

another use case is for unknown peer root buses that can not tell end
before scanning,
will assume we are handing those kind of peer root bus for low to high.

Yinghai

[PATCH 03/13] PCI: Add busn_res tracking in core

[Yinghai Lu]

update pci_scan_root_bus, and pci_scan_bus to insert root bus busn into
iobusn_resource tree.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c  |   30 ++++++++++++++++++++++++++----
 drivers/pci/remove.c |    1 +
 include/linux/pci.h  |    4 ++++
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 71261da..55404c3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1666,8 +1666,9 @@ void pci_bus_release_busn_res(struct pci_bus *b)
 	release_resource(&b->busn_res);
 }
 
-struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
-		struct pci_ops *ops, void *sysdata, struct list_head *resources)
+struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
+		int bus_max, struct pci_ops *ops, void *sysdata,
+		struct list_head *resources)
 {
 	struct pci_bus *b;
 
@@ -1675,10 +1676,26 @@ struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 	if (!b)
 		return NULL;
 
+	pci_bus_insert_busn_res(b, bus, bus_max);
 	b->subordinate = pci_scan_child_bus(b);
 	pci_bus_add_devices(b);
 	return b;
 }
+EXPORT_SYMBOL(pci_scan_root_bus_max);
+
+struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
+		struct pci_ops *ops, void *sysdata,
+		struct list_head *resources)
+{
+	struct pci_bus *b;
+
+	b = pci_scan_root_bus_max(parent, bus, 255, ops, sysdata, resources);
+
+	if (b)
+		pci_bus_update_busn_res_end(b, b->subordinate);
+
+	return b;
+}
 EXPORT_SYMBOL(pci_scan_root_bus);
 
 /* Deprecated; use pci_scan_root_bus() instead */
@@ -1691,9 +1708,11 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
 	pci_add_resource(&resources, &ioport_resource);
 	pci_add_resource(&resources, &iomem_resource);
 	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
-	if (b)
+	if (b) {
+		pci_bus_insert_busn_res(b, bus, 255);
 		b->subordinate = pci_scan_child_bus(b);
-	else
+		pci_bus_update_busn_res_end(b, b->subordinate);
+	} else
 		pci_free_resource_list(&resources);
 	return b;
 }
@@ -1709,7 +1728,10 @@ struct pci_bus * __devinit pci_scan_bus(int bus, struct pci_ops *ops,
 	pci_add_resource(&resources, &iomem_resource);
 	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
 	if (b) {
+		pci_bus_insert_busn_res(b, bus, 255);
 		b->subordinate = pci_scan_child_bus(b);
+		pci_bus_update_busn_res_end(b, b->subordinate);
+
 		pci_bus_add_devices(b);
 	} else {
 		pci_free_resource_list(&resources);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index bd70f23..8056e6a 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -68,6 +68,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 
 	down_write(&pci_bus_sem);
 	list_del(&pci_bus->node);
+	pci_bus_release_busn_res(pci_bus);
 	up_write(&pci_bus_sem);
 	if (!pci_bus->is_added)
 		return;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3da935c..d5b6786 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -668,6 +668,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
 void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
 void pci_bus_release_busn_res(struct pci_bus *b);
+struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
+					     int busmax, struct pci_ops *ops,
+					     void *sysdata,
+					     struct list_head *resources);
 struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 					     struct pci_ops *ops, void *sysdata,
 					     struct list_head *resources);
-- 
1.7.7

Re: [PATCH 03/13] PCI: Add busn_res tracking in core

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> update pci_scan_root_bus, and pci_scan_bus to insert root bus busn into
> iobusn_resource tree.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c  |   30 ++++++++++++++++++++++++++----
>  drivers/pci/remove.c |    1 +
>  include/linux/pci.h  |    4 ++++
>  3 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 71261da..55404c3 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1666,8 +1666,9 @@ void pci_bus_release_busn_res(struct pci_bus *b)
>        release_resource(&b->busn_res);
>  }
>
> -struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> -               struct pci_ops *ops, void *sysdata, struct list_head *resources)
> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
> +               int bus_max, struct pci_ops *ops, void *sysdata,
> +               struct list_head *resources)

We are adding "scan root bus" interfaces at an alarming rate (I'm
responsible for most of them, so I'm not blaming you :)).  Could we
get away without adding this one by just adding a bus number resource
to the "resources" list?  For example, then an arch could do:

    LIST_HEAD(resources);
    pci_add_resource(resources, bus_window);
    pci_add_resource(resources, mem_window);
    pci_add_resource(resources, io_window);
    pci_scan_root_bus(..., resources);

If pci_scan_root_bus() didn't find a bus number resource in the list,
it could fall back to a default max_bus of 255.

>  {
>        struct pci_bus *b;
>
> @@ -1675,10 +1676,26 @@ struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>        if (!b)
>                return NULL;
>
> +       pci_bus_insert_busn_res(b, bus, bus_max);
>        b->subordinate = pci_scan_child_bus(b);
>        pci_bus_add_devices(b);
>        return b;
>  }
> +EXPORT_SYMBOL(pci_scan_root_bus_max);
> +
> +struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> +               struct pci_ops *ops, void *sysdata,
> +               struct list_head *resources)
> +{
> +       struct pci_bus *b;
> +
> +       b = pci_scan_root_bus_max(parent, bus, 255, ops, sysdata, resources);
> +
> +       if (b)
> +               pci_bus_update_busn_res_end(b, b->subordinate);
> +
> +       return b;
> +}
>  EXPORT_SYMBOL(pci_scan_root_bus);
>
>  /* Deprecated; use pci_scan_root_bus() instead */
> @@ -1691,9 +1708,11 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
>        pci_add_resource(&resources, &ioport_resource);
>        pci_add_resource(&resources, &iomem_resource);
>        b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
> -       if (b)
> +       if (b) {
> +               pci_bus_insert_busn_res(b, bus, 255);
>                b->subordinate = pci_scan_child_bus(b);
> -       else
> +               pci_bus_update_busn_res_end(b, b->subordinate);
> +       } else
>                pci_free_resource_list(&resources);
>        return b;
>  }
> @@ -1709,7 +1728,10 @@ struct pci_bus * __devinit pci_scan_bus(int bus, struct pci_ops *ops,
>        pci_add_resource(&resources, &iomem_resource);
>        b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
>        if (b) {
> +               pci_bus_insert_busn_res(b, bus, 255);
>                b->subordinate = pci_scan_child_bus(b);
> +               pci_bus_update_busn_res_end(b, b->subordinate);
> +
>                pci_bus_add_devices(b);
>        } else {
>                pci_free_resource_list(&resources);
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index bd70f23..8056e6a 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -68,6 +68,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
>
>        down_write(&pci_bus_sem);
>        list_del(&pci_bus->node);
> +       pci_bus_release_busn_res(pci_bus);
>        up_write(&pci_bus_sem);
>        if (!pci_bus->is_added)
>                return;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3da935c..d5b6786 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -668,6 +668,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
>  void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
>  void pci_bus_release_busn_res(struct pci_bus *b);
> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
> +                                            int busmax, struct pci_ops *ops,
> +                                            void *sysdata,
> +                                            struct list_head *resources);
>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>                                             struct pci_ops *ops, void *sysdata,
>                                             struct list_head *resources);
> --
> 1.7.7
>

Re: [PATCH 03/13] PCI: Add busn_res tracking in core

[Yinghai Lu]

On Mon, Jan 30, 2012 at 7:35 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> update pci_scan_root_bus, and pci_scan_bus to insert root bus busn into
>> iobusn_resource tree.
>> -struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>> -               struct pci_ops *ops, void *sysdata, struct list_head *resources)
>> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
>> +               int bus_max, struct pci_ops *ops, void *sysdata,
>> +               struct list_head *resources)
>
> We are adding "scan root bus" interfaces at an alarming rate (I'm
> responsible for most of them, so I'm not blaming you :)).  Could we
> get away without adding this one by just adding a bus number resource
> to the "resources" list?  For example, then an arch could do:
>
>    LIST_HEAD(resources);
>    pci_add_resource(resources, bus_window);
>    pci_add_resource(resources, mem_window);
>    pci_add_resource(resources, io_window);
>    pci_scan_root_bus(..., resources);
>
> If pci_scan_root_bus() didn't find a bus number resource in the list,
> it could fall back to a default max_bus of 255.
>

this pci_scan_root_bus_max is only used for x86 code.  and should be
fixed later.

Yinghai

[PATCH 04/13] PCI, x86: Register busn_res for root buses

[Yinghai Lu]

update x86_pci_root_bus_resources() to get bus_max for non-acpi case.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/include/asm/topology.h |    3 ++-
 arch/x86/pci/acpi.c             |    8 +++++---
 arch/x86/pci/bus_numa.c         |    8 +++++++-
 arch/x86/pci/common.c           |   11 +++++++----
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index b9676ae..ad4060e 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -172,7 +172,8 @@ static inline void arch_fix_phys_package_id(int num, u32 slot)
 }
 
 struct pci_bus;
-void x86_pci_root_bus_resources(int bus, struct list_head *resources);
+void x86_pci_root_bus_resources(int bus, int *bus_max,
+				struct list_head *resources);
 
 #ifdef CONFIG_SMP
 #define mc_capable()	((boot_cpu_data.x86_max_cores > 1) && \
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index daa4249..b25b5b1 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -348,6 +348,7 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
 	struct acpi_device *device = root->device;
 	int domain = root->segment;
 	int busnum = root->secondary.start;
+	int busmax = root->secondary.end;
 	LIST_HEAD(resources);
 	struct pci_bus *bus;
 	struct pci_sysdata *sd;
@@ -410,12 +411,13 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
 		 * defaults or native bridge info if we're ignoring _CRS.
 		 */
 		if (!pci_use_crs)
-			x86_pci_root_bus_resources(busnum, &resources);
+			x86_pci_root_bus_resources(busnum, &busmax, &resources);
 		bus = pci_create_root_bus(NULL, busnum, &pci_root_ops, sd,
 					  &resources);
-		if (bus)
+		if (bus) {
+			pci_bus_insert_busn_res(bus, busnum, busmax);
 			bus->subordinate = pci_scan_child_bus(bus);
-		else
+		} else
 			pci_free_resource_list(&resources);
 	}
 
diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
index fd3f655..5213cd8 100644
--- a/arch/x86/pci/bus_numa.c
+++ b/arch/x86/pci/bus_numa.c
@@ -7,7 +7,8 @@
 int pci_root_num;
 struct pci_root_info pci_root_info[PCI_ROOT_NR];
 
-void x86_pci_root_bus_resources(int bus, struct list_head *resources)
+void x86_pci_root_bus_resources(int bus, int *bus_max,
+				struct list_head *resources)
 {
 	int i;
 	int j;
@@ -28,6 +29,7 @@ void x86_pci_root_bus_resources(int bus, struct list_head *resources)
 	       bus);
 
 	info = &pci_root_info[i];
+	*bus_max = info->bus_max;
 	for (j = 0; j < info->res_num; j++) {
 		struct resource *res;
 		struct resource *root;
@@ -51,6 +53,10 @@ default_resources:
 	printk(KERN_DEBUG "PCI: root bus %02x: using default resources\n", bus);
 	pci_add_resource(resources, &ioport_resource);
 	pci_add_resource(resources, &iomem_resource);
+	if (!bus)
+		*bus_max = 0xff;
+	else if (!*bus_max)
+		*bus_max = bus;
 }
 
 void __devinit update_res(struct pci_root_info *info, resource_size_t start,
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 323481e..ce0aefc 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -433,6 +433,7 @@ struct pci_bus * __devinit pcibios_scan_root(int busnum)
 	LIST_HEAD(resources);
 	struct pci_bus *bus = NULL;
 	struct pci_sysdata *sd;
+	int bus_max;
 
 	while ((bus = pci_find_next_bus(bus)) != NULL) {
 		if (bus->number == busnum) {
@@ -454,8 +455,9 @@ struct pci_bus * __devinit pcibios_scan_root(int busnum)
 	sd->node = get_mp_bus_to_node(busnum);
 
 	printk(KERN_DEBUG "PCI: Probing PCI hardware (bus %02x)\n", busnum);
-	x86_pci_root_bus_resources(busnum, &resources);
-	bus = pci_scan_root_bus(NULL, busnum, &pci_root_ops, sd, &resources);
+	x86_pci_root_bus_resources(busnum, &bus_max, &resources);
+	bus = pci_scan_root_bus_max(NULL, busnum, bus_max, &pci_root_ops, sd,
+				&resources);
 	if (!bus) {
 		pci_free_resource_list(&resources);
 		kfree(sd);
@@ -643,6 +645,7 @@ struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 	LIST_HEAD(resources);
 	struct pci_bus *bus = NULL;
 	struct pci_sysdata *sd;
+	int bus_max;
 
 	/*
 	 * Allocate per-root-bus (not per bus) arch-specific data.
@@ -655,8 +658,8 @@ struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 		return NULL;
 	}
 	sd->node = node;
-	x86_pci_root_bus_resources(busno, &resources);
-	bus = pci_scan_root_bus(NULL, busno, ops, sd, &resources);
+	x86_pci_root_bus_resources(busno, &bus_max, &resources);
+	bus = pci_scan_root_bus_max(NULL, busno, bus_max, ops, sd, &resources);
 	if (!bus) {
 		pci_free_resource_list(&resources);
 		kfree(sd);
-- 
1.7.7

[PATCH 05/13] PCI, ia64: Register busn_res for root buses

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
---
 arch/ia64/pci/pci.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index f82f5d4..936b2f1 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -331,6 +331,7 @@ pci_acpi_scan_root(struct acpi_pci_root *root)
 	struct acpi_device *device = root->device;
 	int domain = root->segment;
 	int bus = root->secondary.start;
+	int busmax = root->secondary.end;
 	struct pci_controller *controller;
 	unsigned int windows = 0;
 	struct pci_root_info info;
@@ -384,6 +385,7 @@ pci_acpi_scan_root(struct acpi_pci_root *root)
 		return NULL;
 	}
 
+	pci_bus_insert_busn_res(pbus, busnum, busmax);
 	pbus->subordinate = pci_scan_child_bus(pbus);
 	return pbus;
 
-- 
1.7.7

[PATCH 06/13] PCI, powerpc: Register busn_res for root buses

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/pci-common.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index cce98d7..501f29b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1732,6 +1732,8 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
+	pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
+
 	/* Get probe mode and perform scan */
 	mode = PCI_PROBE_NORMAL;
 	if (node && ppc_md.pci_probe_mode)
@@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 		of_scan_bus(node, bus);
 	}
 
-	if (mode == PCI_PROBE_NORMAL)
+	if (mode == PCI_PROBE_NORMAL) {
+		pci_bus_update_busn_res_end(bus, 255);
 		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
+		pci_bus_update_busn_res_end(bus, bus->subordinate);
+	}
 
 	/* Platform gets a chance to do some global fixups before
 	 * we proceed to resource allocation
-- 
1.7.7

Re: [PATCH 06/13] PCI, powerpc: Register busn_res for root buses

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  arch/powerpc/kernel/pci-common.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index cce98d7..501f29b 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1732,6 +1732,8 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>        bus->secondary = hose->first_busno;
>        hose->bus = bus;
>
> +       pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);

The pci_create_root_bus() call is a few lines above this.  So this is
a case of "create the root bus" followed by "fix something that's
wrong with the bus we just created."  I'm trying to get rid of that
pattern because it's just an opportunity for bugs.  I'd rather create
the root bus with all the information it needs up front.

And pci_bus_insert_busn_res() is not really architecture-specific, so
I'd like this better if that call were done in the PCI core somewhere.

>        /* Get probe mode and perform scan */
>        mode = PCI_PROBE_NORMAL;
>        if (node && ppc_md.pci_probe_mode)
> @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>                of_scan_bus(node, bus);
>        }
>
> -       if (mode == PCI_PROBE_NORMAL)
> +       if (mode == PCI_PROBE_NORMAL) {
> +               pci_bus_update_busn_res_end(bus, 255);
>                hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
> +               pci_bus_update_busn_res_end(bus, bus->subordinate);
> +       }
>
>        /* Platform gets a chance to do some global fixups before
>         * we proceed to resource allocation
> --
> 1.7.7
>

[PATCH 07/13] PCI, parisc: Register busn_res for root buses

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
---
 drivers/parisc/dino.c    |    2 ++
 drivers/parisc/lba_pci.c |    3 +++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c
index 7ff10c1..3f2e203 100644
--- a/drivers/parisc/dino.c
+++ b/drivers/parisc/dino.c
@@ -1014,7 +1014,9 @@ static int __init dino_probe(struct parisc_device *dev)
 		return 0;
 	}
 
+	pci_bus_insert_busn_res(bus, dino_current_bus, 255);
 	bus->subordinate = pci_scan_child_bus(bus);
+	pci_bus_update_busn_res_end(bus, bus->subordinate);
 
 	/* This code *depends* on scanning being single threaded
 	 * if it isn't, this global bus number count will fail
diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index d5f3d75..b58bf8b 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -1531,6 +1531,9 @@ lba_driver_probe(struct parisc_device *dev)
 		return 0;
 	}
 
+	pci_bus_insert_busn_res(lba_bus, lba_dev->hba.bus_num.start,
+				lba_dev->hba.bus_num.end);
+
 	lba_bus->subordinate = pci_scan_child_bus(lba_bus);
 
 	/* This is in lieu of calling pci_assign_unassigned_resources() */
-- 
1.7.7

[PATCH 08/13] PCI: Add pci_bus_extend/shrink_top()

[Yinghai Lu]

extend or shrink bus and parent buses top (subordinate)

extended range is verified safe range, and stop at recorded parent_res.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 55404c3..3e62f45 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -624,6 +624,56 @@ static void pci_fixup_parent_subordinate_busnr(struct pci_bus *child, int max)
 	}
 }
 
+static void __devinit pci_bus_extend_top(struct pci_bus *parent,
+		 resource_size_t size, struct resource *parent_res)
+{
+	struct resource *res;
+
+	if (!size)
+		return;
+
+	while (parent) {
+		res = &parent->busn_res;
+		if (res == parent_res)
+			break;
+		res->end += size;
+		parent->subordinate += size;
+		pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS,
+					 parent->subordinate);
+		dev_printk(KERN_DEBUG, &parent->dev,
+				"busn_res: extended %02llx to %06llx-%06llx\n",
+				(unsigned long long)size,
+				(unsigned long long)res->start,
+				(unsigned long long)res->end);
+		parent = parent->parent;
+	}
+}
+
+static void __devinit pci_bus_shrink_top(struct pci_bus *parent,
+		 resource_size_t size, struct resource *parent_res)
+{
+	struct resource *res;
+
+	if (!size)
+		return;
+
+	while (parent) {
+		res = &parent->busn_res;
+		if (res == parent_res)
+			break;
+		res->end -= size;
+		parent->subordinate -= size;
+		pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS,
+					 parent->subordinate);
+		dev_printk(KERN_DEBUG, &parent->dev,
+				"busn_res: shrunk %02llx to %06llx-%06llx\n",
+				(unsigned long long)size,
+				(unsigned long long)res->start,
+				(unsigned long long)res->end);
+		parent = parent->parent;
+	}
+}
+
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
-- 
1.7.7

Re: [PATCH 08/13] PCI: Add pci_bus_extend/shrink_top()

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> extend or shrink bus and parent buses top (subordinate)
>
> extended range is verified safe range, and stop at recorded parent_res.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 55404c3..3e62f45 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -624,6 +624,56 @@ static void pci_fixup_parent_subordinate_busnr(struct pci_bus *child, int max)
>        }
>  }
>
> +static void __devinit pci_bus_extend_top(struct pci_bus *parent,
> +                resource_size_t size, struct resource *parent_res)
> +{
> +       struct resource *res;
> +
> +       if (!size)
> +               return;
> +
> +       while (parent) {
> +               res = &parent->busn_res;
> +               if (res == parent_res)
> +                       break;
> +               res->end += size;
> +               parent->subordinate += size;

This makes me nervous for the same reason as
pci_bus_update_busn_res_end() -- we're changing res->end for resources
already in a tree.  How do we know the tree remains consistent?

> +               pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS,
> +                                        parent->subordinate);
> +               dev_printk(KERN_DEBUG, &parent->dev,
> +                               "busn_res: extended %02llx to %06llx-%06llx\n",
> +                               (unsigned long long)size,
> +                               (unsigned long long)res->start,
> +                               (unsigned long long)res->end);
> +               parent = parent->parent;
> +       }
> +}
> +
> +static void __devinit pci_bus_shrink_top(struct pci_bus *parent,
> +                resource_size_t size, struct resource *parent_res)
> +{
> +       struct resource *res;
> +
> +       if (!size)
> +               return;
> +
> +       while (parent) {
> +               res = &parent->busn_res;
> +               if (res == parent_res)
> +                       break;
> +               res->end -= size;
> +               parent->subordinate -= size;
> +               pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS,
> +                                        parent->subordinate);
> +               dev_printk(KERN_DEBUG, &parent->dev,
> +                               "busn_res: shrunk %02llx to %06llx-%06llx\n",
> +                               (unsigned long long)size,
> +                               (unsigned long long)res->start,
> +                               (unsigned long long)res->end);
> +               parent = parent->parent;
> +       }
> +}
> +
>  /*
>  * If it's a bridge, configure it and scan the bus behind it.
>  * For CardBus bridges, we don't scan behind as the devices will
> --
> 1.7.7
>

Re: [PATCH 08/13] PCI: Add pci_bus_extend/shrink_top()

[Yinghai Lu]

On Mon, Jan 30, 2012 at 7:50 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> extend or shrink bus and parent buses top (subordinate)
>>
>> extended range is verified safe range, and stop at recorded parent_res.
...
> This makes me nervous for the same reason as
> pci_bus_update_busn_res_end() -- we're changing res->end for resources
> already in a tree.  How do we know the tree remains consistent?
>

Yes, we already try to allocate near the top and later pad the top. So
it is safe to just adjust top.

Yinghai

[PATCH 09/13] PCI: Probe safe range that we can use for unassigned bridge.

[Yinghai Lu]

Try to allocate from parent bus busn_res. if can not find any big enough, will try
to extend parent bus top. even the extending is through allocating, after allocating
will pad the range to parent buses top.

When extending happens, We will record the parent_res, so could use it as stopper
for really extend/shrink top later.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 117 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 3e62f45..83df3fb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -674,6 +674,123 @@ static void __devinit pci_bus_shrink_top(struct pci_bus *parent,
 	}
 }
 
+static resource_size_t __devinit find_res_top_free_size(struct resource *res)
+{
+	int ret = -ENOMEM;
+	resource_size_t n_size;
+	struct resource tmp_res;
+
+	/*
+	 *   find out number below res->end, that we can use at first
+	 *	res->start can not be used.
+	 */
+	n_size = resource_size(res) - 1;
+	memset(&tmp_res, 0, sizeof(struct resource));
+	while (n_size > 0) {
+		ret = allocate_resource(res, &tmp_res, n_size,
+			res->end - n_size + 1, res->end,
+			1, NULL, NULL);
+		if (ret == 0) {
+			/* release busn_res */
+			release_resource(&tmp_res);
+			break;
+		}
+		n_size--;
+	}
+
+	return n_size;
+}
+
+static int __devinit pci_bridge_probe_busn_res(struct pci_bus *bus,
+			 struct pci_dev *dev, struct resource *busn_res,
+			 resource_size_t needed_size, struct resource **p)
+{
+	int ret = -ENOMEM;
+	resource_size_t n_size;
+	struct pci_bus *parent;
+	struct resource *parent_res;
+	resource_size_t tmp = bus->busn_res.end + 1;
+	int free_sz = -1;
+
+	parent_res = NULL;
+
+again:
+	/*
+	 * find bigest range in bus->busn_res that we can use in the middle
+	 *  and we can not use bus->busn_res.start.
+	 */
+	n_size = resource_size(&bus->busn_res) - 1;
+	memset(busn_res, 0, sizeof(struct resource));
+	dev_printk(KERN_DEBUG, &dev->dev,
+			"find free busn in busn_res: %06llx-%06llx\n",
+			(unsigned long long)bus->busn_res.start,
+			(unsigned long long)bus->busn_res.end);
+	while (n_size >= needed_size) {
+		ret = allocate_resource(&bus->busn_res, busn_res, n_size,
+				bus->busn_res.start + 1, bus->busn_res.end,
+				1, NULL, NULL);
+		if (ret == 0) {
+			/* release res */
+			release_resource(busn_res);
+
+			return ret;
+		}
+		n_size--;
+	}
+
+	/* try extend the top of parent buss */
+	if (free_sz < 0) {
+		free_sz = find_res_top_free_size(&bus->busn_res);
+		dev_printk(KERN_DEBUG, &dev->dev,
+			"found free busn %d in busn_res: %06llx-%06llx top\n",
+				free_sz,
+				(unsigned long long)bus->busn_res.start,
+				(unsigned long long)bus->busn_res.end);
+	}
+	n_size = free_sz;
+
+	/* check if extend could cross domain boundary */
+	if ((bus->busn_res.end & 0xff) == 0xff)
+		goto reduce_needed_size;
+	if ((0x100 - (tmp & 0xff)) < (needed_size - n_size))
+		goto reduce_needed_size;
+
+	/* find exteded range */
+	memset(busn_res, 0, sizeof(struct resource));
+	parent = bus->parent;
+	while (parent) {
+		ret = allocate_resource(&parent->busn_res, busn_res,
+			 needed_size - n_size,
+			 tmp, tmp + needed_size - n_size - 1,
+			 1, NULL, NULL);
+		if (ret == 0)
+			break;
+		parent = parent->parent;
+	}
+
+reduce_needed_size:
+	if (ret != 0) {
+		needed_size--;
+		if (!needed_size)
+			return ret;
+
+		goto again;
+	}
+
+	/* save parent_res, we need it as stopper later */
+	parent_res = busn_res->parent;
+	/* release busn_res */
+	release_resource(busn_res);
+	busn_res->start -= n_size;
+
+	/* extend parent bus top*/
+	pci_bus_extend_top(bus, needed_size - n_size, parent_res);
+
+	*p = parent_res;
+
+	return ret;
+}
+
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
-- 
1.7.7

Re: [PATCH 09/13] PCI: Probe safe range that we can use for unassigned bridge.

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Try to allocate from parent bus busn_res. if can not find any big enough, will try
> to extend parent bus top. even the extending is through allocating, after allocating
> will pad the range to parent buses top.
>
> When extending happens, We will record the parent_res, so could use it as stopper
> for really extend/shrink top later.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 117 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 3e62f45..83df3fb 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -674,6 +674,123 @@ static void __devinit pci_bus_shrink_top(struct pci_bus *parent,
>        }
>  }
>
> +static resource_size_t __devinit find_res_top_free_size(struct resource *res)

I think what this does is "find the largest available area in 'res'."
That *sounds* sort of useful (and like something that could go
somewhere more generic than drivers/pci/probe.c), but there's no
locking, so we don't have any assurance that the area we find will
*remain* available.

Since the caller should deal with failure anyway (if the largest
available area is no longer available by the time it gets around to
allocating it), it seems like it'd be better to fold this into the
caller somehow.

> +{
> +       int ret = -ENOMEM;
> +       resource_size_t n_size;
> +       struct resource tmp_res;
> +
> +       /*
> +        *   find out number below res->end, that we can use at first
> +        *      res->start can not be used.
> +        */
> +       n_size = resource_size(res) - 1;
> +       memset(&tmp_res, 0, sizeof(struct resource));
> +       while (n_size > 0) {
> +               ret = allocate_resource(res, &tmp_res, n_size,
> +                       res->end - n_size + 1, res->end,
> +                       1, NULL, NULL);
> +               if (ret == 0) {
> +                       /* release busn_res */

Comments like this that repeat exactly what the next line of code does
without adding any useful information are unnecessary and distracting.

> +                       release_resource(&tmp_res);
> +                       break;
> +               }
> +               n_size--;
> +       }
> +
> +       return n_size;
> +}
> +
> +static int __devinit pci_bridge_probe_busn_res(struct pci_bus *bus,
> +                        struct pci_dev *dev, struct resource *busn_res,
> +                        resource_size_t needed_size, struct resource **p)
> +{
> +       int ret = -ENOMEM;
> +       resource_size_t n_size;
> +       struct pci_bus *parent;
> +       struct resource *parent_res;
> +       resource_size_t tmp = bus->busn_res.end + 1;
> +       int free_sz = -1;
> +
> +       parent_res = NULL;
> +
> +again:
> +       /*
> +        * find bigest range in bus->busn_res that we can use in the middle
> +        *  and we can not use bus->busn_res.start.
> +        */
> +       n_size = resource_size(&bus->busn_res) - 1;
> +       memset(busn_res, 0, sizeof(struct resource));
> +       dev_printk(KERN_DEBUG, &dev->dev,
> +                       "find free busn in busn_res: %06llx-%06llx\n",
> +                       (unsigned long long)bus->busn_res.start,
> +                       (unsigned long long)bus->busn_res.end);
> +       while (n_size >= needed_size) {
> +               ret = allocate_resource(&bus->busn_res, busn_res, n_size,
> +                               bus->busn_res.start + 1, bus->busn_res.end,
> +                               1, NULL, NULL);
> +               if (ret == 0) {
> +                       /* release res */
> +                       release_resource(busn_res);
> +
> +                       return ret;
> +               }
> +               n_size--;
> +       }
> +
> +       /* try extend the top of parent buss */
> +       if (free_sz < 0) {
> +               free_sz = find_res_top_free_size(&bus->busn_res);
> +               dev_printk(KERN_DEBUG, &dev->dev,
> +                       "found free busn %d in busn_res: %06llx-%06llx top\n",
> +                               free_sz,
> +                               (unsigned long long)bus->busn_res.start,
> +                               (unsigned long long)bus->busn_res.end);
> +       }
> +       n_size = free_sz;
> +
> +       /* check if extend could cross domain boundary */
> +       if ((bus->busn_res.end & 0xff) == 0xff)
> +               goto reduce_needed_size;
> +       if ((0x100 - (tmp & 0xff)) < (needed_size - n_size))
> +               goto reduce_needed_size;
> +
> +       /* find exteded range */
> +       memset(busn_res, 0, sizeof(struct resource));
> +       parent = bus->parent;
> +       while (parent) {
> +               ret = allocate_resource(&parent->busn_res, busn_res,
> +                        needed_size - n_size,
> +                        tmp, tmp + needed_size - n_size - 1,
> +                        1, NULL, NULL);
> +               if (ret == 0)
> +                       break;
> +               parent = parent->parent;
> +       }
> +
> +reduce_needed_size:
> +       if (ret != 0) {
> +               needed_size--;
> +               if (!needed_size)
> +                       return ret;
> +
> +               goto again;
> +       }
> +
> +       /* save parent_res, we need it as stopper later */
> +       parent_res = busn_res->parent;
> +       /* release busn_res */
> +       release_resource(busn_res);
> +       busn_res->start -= n_size;
> +
> +       /* extend parent bus top*/
> +       pci_bus_extend_top(bus, needed_size - n_size, parent_res);
> +
> +       *p = parent_res;
> +
> +       return ret;
> +}
> +
>  /*
>  * If it's a bridge, configure it and scan the bus behind it.
>  * For CardBus bridges, we don't scan behind as the devices will
> --
> 1.7.7
>

Re: [PATCH 09/13] PCI: Probe safe range that we can use for unassigned bridge.

[Yinghai Lu]

On Mon, Jan 30, 2012 at 7:59 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Try to allocate from parent bus busn_res. if can not find any big enough, will try
>> to extend parent bus top. even the extending is through allocating, after allocating
>> will pad the range to parent buses top.
>>
>> When extending happens, We will record the parent_res, so could use it as stopper
>> for really extend/shrink top later.
>>
..
>
> I think what this does is "find the largest available area in 'res'."
> That *sounds* sort of useful (and like something that could go
> somewhere more generic than drivers/pci/probe.c), but there's no
> locking, so we don't have any assurance that the area we find will
> *remain* available.

two usages:
1. init booting : should be safe, that is sequential scanning
2. hotplug: we should have overall lock somewhere for it?

>
> Since the caller should deal with failure anyway (if the largest
> available area is no longer available by the time it gets around to
> allocating it), it seems like it'd be better to fold this into the
> caller somehow.



>> +               if (ret == 0) {
>> +                       /* release busn_res */
>
> Comments like this that repeat exactly what the next line of code does
> without adding any useful information are unnecessary and distracting.

ok, will remove them.

Yinghai

[PATCH 10/13] PCI: Strict checking of valid range for bridge

[Yinghai Lu]

children bridges busn range should be able to be allocated from parent bus range.

to avoid overlapping between sibling bridges on same bus.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 83df3fb..e12f65f0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -791,6 +791,33 @@ reduce_needed_size:
 	return ret;
 }
 
+static int __devinit pci_bridge_check_busn_res(struct pci_bus *bus,
+				struct pci_dev *dev,
+				int secondary, int subordinate)
+{
+	int broken = 0;
+
+	struct resource busn_res;
+	int ret;
+
+	memset(&busn_res, 0, sizeof(struct resource));
+	dev_printk(KERN_DEBUG, &dev->dev,
+		 "check if busn %02x-%02x is in busn_res: %06llx-%06llx\n",
+		 secondary, subordinate,
+		 (unsigned long long)bus->busn_res.start,
+		 (unsigned long long)bus->busn_res.end);
+	ret = allocate_resource(&bus->busn_res, &busn_res,
+			 (subordinate - secondary + 1),
+			 (pci_domain_nr(bus)<<8) | secondary,
+			 (pci_domain_nr(bus)<<8) | subordinate,
+			 1, NULL, NULL);
+	if (ret)
+		broken = 1;
+	else
+		release_resource(&busn_res);
+
+	return broken;
+}
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
-- 
1.7.7

Re: [PATCH 10/13] PCI: Strict checking of valid range for bridge

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> children bridges busn range should be able to be allocated from parent bus range.
>
> to avoid overlapping between sibling bridges on same bus.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 83df3fb..e12f65f0 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -791,6 +791,33 @@ reduce_needed_size:
>        return ret;
>  }
>
> +static int __devinit pci_bridge_check_busn_res(struct pci_bus *bus,
> +                               struct pci_dev *dev,
> +                               int secondary, int subordinate)

This function returns a boolean, but the function name doesn't give
any clue about what a true/false return means.  Something like
"busn_valid" would make the callers more readable.

> +{
> +       int broken = 0;
> +
> +       struct resource busn_res;
> +       int ret;
> +
> +       memset(&busn_res, 0, sizeof(struct resource));
> +       dev_printk(KERN_DEBUG, &dev->dev,
> +                "check if busn %02x-%02x is in busn_res: %06llx-%06llx\n",
> +                secondary, subordinate,
> +                (unsigned long long)bus->busn_res.start,
> +                (unsigned long long)bus->busn_res.end);
> +       ret = allocate_resource(&bus->busn_res, &busn_res,
> +                        (subordinate - secondary + 1),
> +                        (pci_domain_nr(bus)<<8) | secondary,
> +                        (pci_domain_nr(bus)<<8) | subordinate,

I think this "(pci_domain_nr(bus)<<8) | secondary" stuff needs to be a
macro or something.

> +                        1, NULL, NULL);
> +       if (ret)
> +               broken = 1;
> +       else
> +               release_resource(&busn_res);
> +
> +       return broken;
> +}
>  /*
>  * If it's a bridge, configure it and scan the bus behind it.
>  * For CardBus bridges, we don't scan behind as the devices will
> --
> 1.7.7
>

Re: [PATCH 10/13] PCI: Strict checking of valid range for bridge

[Yinghai Lu]

On Mon, Jan 30, 2012 at 8:04 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> children bridges busn range should be able to be allocated from parent bus range.
>>
>> to avoid overlapping between sibling bridges on same bus.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/probe.c |   27 +++++++++++++++++++++++++++
>>  1 files changed, 27 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 83df3fb..e12f65f0 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -791,6 +791,33 @@ reduce_needed_size:
>>        return ret;
>>  }
>>
>> +static int __devinit pci_bridge_check_busn_res(struct pci_bus *bus,
>> +                               struct pci_dev *dev,
>> +                               int secondary, int subordinate)
>
> This function returns a boolean, but the function name doesn't give
> any clue about what a true/false return means.  Something like
> "busn_valid" would make the callers more readable.

ok

>
>> +{
>> +       int broken = 0;
>> +
>> +       struct resource busn_res;
>> +       int ret;
>> +
>> +       memset(&busn_res, 0, sizeof(struct resource));
>> +       dev_printk(KERN_DEBUG, &dev->dev,
>> +                "check if busn %02x-%02x is in busn_res: %06llx-%06llx\n",
>> +                secondary, subordinate,
>> +                (unsigned long long)bus->busn_res.start,
>> +                (unsigned long long)bus->busn_res.end);
>> +       ret = allocate_resource(&bus->busn_res, &busn_res,
>> +                        (subordinate - secondary + 1),
>> +                        (pci_domain_nr(bus)<<8) | secondary,
>> +                        (pci_domain_nr(bus)<<8) | subordinate,
>
> I think this "(pci_domain_nr(bus)<<8) | secondary" stuff needs to be a
> macro or something.

maybe.

[PATCH 11/13] PCI: Allocate bus range instead of use max blindly

[Yinghai Lu]

every bus have extra busn_res, and linked them toghter to iobusn_resource.

when need to find usable bus number range, try probe from iobusn_resource tree.

To avoid falling to small hole in the middle, we try from 8 spare bus.
if can not find 8 or more in the middle, will try to append 8 on top later.
then if can not append, will try to find 7 from the middle, then will try to append 7 on top.
then if can not append, will try to find 6 from the middle...

for cardbus will only find 4 spare.

if extend from top, at last will shrink back to really needed range...

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   82 ++++++++++++++++++++++++++++-----------------------
 1 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e12f65f0..23c2f0d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -832,10 +832,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 {
 	struct pci_bus *child;
 	int is_cardbus = (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS);
-	u32 buses, i, j = 0;
+	u32 buses;
 	u16 bctl;
 	u8 primary, secondary, subordinate;
 	int broken = 0;
+	struct resource *parent_res = NULL;
 
 	pci_read_config_dword(dev, PCI_PRIMARY_BUS, &buses);
 	primary = buses & 0xFF;
@@ -847,10 +848,16 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 
 	/* Check if setup is sensible at all */
 	if (!pass &&
-	    (primary != bus->number || secondary <= bus->number)) {
-		dev_dbg(&dev->dev, "bus configuration invalid, reconfiguring\n");
+	    (primary != bus->number || secondary <= bus->number))
 		broken = 1;
-	}
+
+	/* more strict checking */
+	if (!pass && !broken)
+		broken = pci_bridge_check_busn_res(bus, dev,
+						   secondary, subordinate);
+
+	if (broken)
+		dev_dbg(&dev->dev, "bus configuration invalid, reconfiguring\n");
 
 	/* Disable MasterAbortMode during probing to avoid reporting
 	   of bus errors (in some architectures) */ 
@@ -883,6 +890,8 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 			child->primary = primary;
 			child->subordinate = subordinate;
 			child->bridge_ctl = bctl;
+
+			pci_bus_insert_busn_res(child, secondary, subordinate);
 		}
 
 		cmax = pci_scan_child_bus(child);
@@ -895,6 +904,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 		 * We need to assign a number to this bus which we always
 		 * do in the second pass.
 		 */
+		resource_size_t shrink_size;
+		struct resource busn_res;
+		int ret = -ENOMEM;
+		int old_max = max;
+
 		if (!pass) {
 			if (pcibios_assign_all_busses() || broken)
 				/* Temporarily disable forwarding of the
@@ -911,20 +925,28 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 		/* Clear errors */
 		pci_write_config_word(dev, PCI_STATUS, 0xffff);
 
-		/* Prevent assigning a bus number that already exists.
-		 * This can happen when a bridge is hot-plugged, so in
-		 * this case we only re-scan this bus. */
-		child = pci_find_bus(pci_domain_nr(bus), max+1);
-		if (!child) {
-			child = pci_add_new_bus(bus, dev, ++max);
-			if (!child)
-				goto out;
-		}
+		ret = pci_bridge_probe_busn_res(bus, dev, &busn_res,
+				is_cardbus ? (CARDBUS_RESERVE_BUSNR + 1) : 8,
+				&parent_res);
+
+		if (ret != 0)
+			goto out;
+
+		child = pci_add_new_bus(bus, dev, busn_res.start & 0xff);
+		if (!child)
+			goto out;
+
+		child->subordinate = busn_res.end & 0xff;
+		pci_bus_insert_busn_res(child, busn_res.start & 0xff,
+					busn_res.end & 0xff);
+
 		buses = (buses & 0xff000000)
 		      | ((unsigned int)(child->primary)     <<  0)
 		      | ((unsigned int)(child->secondary)   <<  8)
 		      | ((unsigned int)(child->subordinate) << 16);
 
+		max = child->subordinate;
+
 		/*
 		 * yenta.c forces a secondary latency timer of 176.
 		 * Copy that behaviour here.
@@ -955,43 +977,29 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 			 * the real value of max.
 			 */
 			pci_fixup_parent_subordinate_busnr(child, max);
+
 		} else {
 			/*
 			 * For CardBus bridges, we leave 4 bus numbers
 			 * as cards with a PCI-to-PCI bridge can be
 			 * inserted later.
 			 */
-			for (i=0; i<CARDBUS_RESERVE_BUSNR; i++) {
-				struct pci_bus *parent = bus;
-				if (pci_find_bus(pci_domain_nr(bus),
-							max+i+1))
-					break;
-				while (parent->parent) {
-					if ((!pcibios_assign_all_busses()) &&
-					    (parent->subordinate > max) &&
-					    (parent->subordinate <= max+i)) {
-						j = 1;
-					}
-					parent = parent->parent;
-				}
-				if (j) {
-					/*
-					 * Often, there are two cardbus bridges
-					 * -- try to leave one valid bus number
-					 * for each one.
-					 */
-					i /= 2;
-					break;
-				}
-			}
-			max += i;
 			pci_fixup_parent_subordinate_busnr(child, max);
 		}
 		/*
 		 * Set the subordinate bus number to its real value.
 		 */
+		shrink_size = child->subordinate - max;
 		child->subordinate = max;
 		pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
+		pci_bus_update_busn_res_end(child, max);
+
+		/* shrink some back, if we extend top before */
+		if (!is_cardbus &&  (shrink_size > 0) && parent_res)
+			pci_bus_shrink_top(bus, shrink_size, parent_res);
+
+		if (old_max > max)
+			max = old_max;
 	}
 
 	sprintf(child->name,
-- 
1.7.7

[PATCH 12/13] PCI: kill pci_fixup_parent_subordinate_busnr()

[Yinghai Lu]

Now we can safely extend parent top and shrink them according iobusn_resource tree.

Don't need that any more.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   45 +++++++++------------------------------------
 1 files changed, 9 insertions(+), 36 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 23c2f0d..54615a9 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -608,22 +608,6 @@ struct pci_bus *__ref pci_add_new_bus(struct pci_bus *parent, struct pci_dev *de
 	return child;
 }
 
-static void pci_fixup_parent_subordinate_busnr(struct pci_bus *child, int max)
-{
-	struct pci_bus *parent = child->parent;
-
-	/* Attempts to fix that up are really dangerous unless
-	   we're going to re-assign all bus numbers. */
-	if (!pcibios_assign_all_busses())
-		return;
-
-	while (parent->parent && parent->subordinate < max) {
-		parent->subordinate = max;
-		pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS, max);
-		parent = parent->parent;
-	}
-}
-
 static void __devinit pci_bus_extend_top(struct pci_bus *parent,
 		 resource_size_t size, struct resource *parent_res)
 {
@@ -925,6 +909,13 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 		/* Clear errors */
 		pci_write_config_word(dev, PCI_STATUS, 0xffff);
 
+		/*
+		 * For CardBus bridges, we leave 4 bus numbers
+		 * as cards with a PCI-to-PCI bridge can be
+		 * inserted later.
+		 * other just allocate 8 bus to avoid we fall into
+		 *  small hole in the middle.
+		 */
 		ret = pci_bridge_probe_busn_res(bus, dev, &busn_res,
 				is_cardbus ? (CARDBUS_RESERVE_BUSNR + 1) : 8,
 				&parent_res);
@@ -963,29 +954,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 
 		if (!is_cardbus) {
 			child->bridge_ctl = bctl;
-			/*
-			 * Adjust subordinate busnr in parent buses.
-			 * We do this before scanning for children because
-			 * some devices may not be detected if the bios
-			 * was lazy.
-			 */
-			pci_fixup_parent_subordinate_busnr(child, max);
+
 			/* Now we can scan all subordinate buses... */
 			max = pci_scan_child_bus(child);
-			/*
-			 * now fix it up again since we have found
-			 * the real value of max.
-			 */
-			pci_fixup_parent_subordinate_busnr(child, max);
-
-		} else {
-			/*
-			 * For CardBus bridges, we leave 4 bus numbers
-			 * as cards with a PCI-to-PCI bridge can be
-			 * inserted later.
-			 */
-			pci_fixup_parent_subordinate_busnr(child, max);
 		}
+
 		/*
 		 * Set the subordinate bus number to its real value.
 		 */
-- 
1.7.7

Re: [PATCH 12/13] PCI: kill pci_fixup_parent_subordinate_busnr()

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 6:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Now we can safely extend parent top and shrink them according iobusn_resource tree.
>
> Don't need that any more.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c |   45 +++++++++------------------------------------
>  1 files changed, 9 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 23c2f0d..54615a9 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -608,22 +608,6 @@ struct pci_bus *__ref pci_add_new_bus(struct pci_bus *parent, struct pci_dev *de
>        return child;
>  }
>
> -static void pci_fixup_parent_subordinate_busnr(struct pci_bus *child, int max)
> -{
> -       struct pci_bus *parent = child->parent;
> -
> -       /* Attempts to fix that up are really dangerous unless
> -          we're going to re-assign all bus numbers. */
> -       if (!pcibios_assign_all_busses())
> -               return;
> -
> -       while (parent->parent && parent->subordinate < max) {
> -               parent->subordinate = max;
> -               pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS, max);
> -               parent = parent->parent;
> -       }
> -}
> -
>  static void __devinit pci_bus_extend_top(struct pci_bus *parent,
>                 resource_size_t size, struct resource *parent_res)
>  {
> @@ -925,6 +909,13 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
>                /* Clear errors */
>                pci_write_config_word(dev, PCI_STATUS, 0xffff);
>
> +               /*
> +                * For CardBus bridges, we leave 4 bus numbers
> +                * as cards with a PCI-to-PCI bridge can be
> +                * inserted later.
> +                * other just allocate 8 bus to avoid we fall into
> +                *  small hole in the middle.
> +                */

This comment addition is in the wrong patch; it should go in the one
that added the code.

>                ret = pci_bridge_probe_busn_res(bus, dev, &busn_res,
>                                is_cardbus ? (CARDBUS_RESERVE_BUSNR + 1) : 8,
>                                &parent_res);
> @@ -963,29 +954,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
>
>                if (!is_cardbus) {
>                        child->bridge_ctl = bctl;
> -                       /*
> -                        * Adjust subordinate busnr in parent buses.
> -                        * We do this before scanning for children because
> -                        * some devices may not be detected if the bios
> -                        * was lazy.
> -                        */
> -                       pci_fixup_parent_subordinate_busnr(child, max);
> +
>                        /* Now we can scan all subordinate buses... */
>                        max = pci_scan_child_bus(child);
> -                       /*
> -                        * now fix it up again since we have found
> -                        * the real value of max.
> -                        */
> -                       pci_fixup_parent_subordinate_busnr(child, max);
> -
> -               } else {
> -                       /*
> -                        * For CardBus bridges, we leave 4 bus numbers
> -                        * as cards with a PCI-to-PCI bridge can be
> -                        * inserted later.
> -                        */
> -                       pci_fixup_parent_subordinate_busnr(child, max);
>                }
> +
>                /*
>                 * Set the subordinate bus number to its real value.
>                 */
> --
> 1.7.7
>

[PATCH 13/13] PCI: Seperate child bus scanning to two passes overall

[Yinghai Lu]

In extreme case: Two bridges are properly setup.
       bridge A
               bridge AA
               bridge AB
       bridge B
	       bridge BA
	       bridge BB
   but AA, AB are not setup properly.
   bridge A has small range, and bridge AB could need more, when do the
       first pass0 for bridge A, it will do pass0 and pass1 for AA and AB,
	during that process, it will extend range of A for AB blindly.
	because bridge B is not registered yet.
       that could overlap range that is used by bridge B.

Right way should be:
       do pass0 for all good bridges at first.
So we could do pass0 for bridge B before pass1 for bridge AB.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   50 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 54615a9..e2dbfde 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -802,6 +802,9 @@ static int __devinit pci_bridge_check_busn_res(struct pci_bus *bus,
 
 	return broken;
 }
+
+static unsigned int __devinit __pci_scan_child_bus(struct pci_bus *bus,
+						 int pass);
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
@@ -853,11 +856,10 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 	    !is_cardbus && !broken) {
 		unsigned int cmax;
 		/*
-		 * Bus already configured by firmware, process it in the first
-		 * pass and just note the configuration.
+		 * Bus already configured by firmware, still process it in two
+		 * passes in extreme case like two adjaced bridges have children
+		 * bridges that are not setup properly.
 		 */
-		if (pass)
-			goto out;
 
 		/*
 		 * If we already got to this bus through a different bridge,
@@ -878,7 +880,7 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 			pci_bus_insert_busn_res(child, secondary, subordinate);
 		}
 
-		cmax = pci_scan_child_bus(child);
+		cmax = __pci_scan_child_bus(child, pass);
 		if (cmax > max)
 			max = cmax;
 		if (child->subordinate > max)
@@ -1668,12 +1670,13 @@ void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss)
 }
 EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
 
-unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
+static unsigned int __devinit __pci_scan_child_bus(struct pci_bus *bus,
+						 int pass)
 {
-	unsigned int devfn, pass, max = bus->secondary;
+	unsigned int devfn, max = bus->secondary;
 	struct pci_dev *dev;
 
-	dev_dbg(&bus->dev, "scanning bus\n");
+	dev_dbg(&bus->dev, "scanning bus pass %d\n", pass);
 
 	/* Go find them, Rover! */
 	for (devfn = 0; devfn < 0x100; devfn += 8)
@@ -1687,18 +1690,16 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 	 * all PCI-to-PCI bridges on this bus.
 	 */
 	if (!bus->is_added) {
-		dev_dbg(&bus->dev, "fixups for bus\n");
+		dev_dbg(&bus->dev, "fixups for bus pass %d\n", pass);
 		pcibios_fixup_bus(bus);
 		if (pci_is_root_bus(bus))
 			bus->is_added = 1;
 	}
 
-	for (pass=0; pass < 2; pass++)
-		list_for_each_entry(dev, &bus->devices, bus_list) {
-			if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
-			    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-				max = pci_scan_bridge(bus, dev, max, pass);
-		}
+	list_for_each_entry(dev, &bus->devices, bus_list)
+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
+		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
+			max = pci_scan_bridge(bus, dev, max, pass);
 
 	/*
 	 * We've scanned the bus and so we know all about what's on
@@ -1707,7 +1708,24 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 	 *
 	 * Return how far we've got finding sub-buses.
 	 */
-	dev_dbg(&bus->dev, "bus scan returning with max=%02x\n", max);
+	dev_dbg(&bus->dev, "bus scan returning with max=%02x pass %d\n",
+			max, pass);
+
+	return max;
+}
+
+unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
+{
+	int pass;
+	unsigned int max = 0, tmp;
+
+	for (pass = 0; pass < 2;  pass++) {
+		tmp = __pci_scan_child_bus(bus, pass);
+
+		if (tmp > max)
+			max = tmp;
+	}
+
 	return max;
 }
 
-- 
1.7.7