2009-10-29 09:53:01

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v5


move out bus_size_bridges and assign resources out of pciehp_add_bridge()
and at last do them all together one time including slot bridge, to avoid to
call assign resources several times, when there are several bridges under the
slot bridge.

need to introduce pci_bridge_assign_resources there.

handle the case the slot bridge that doesn't get pre-allocated big enough res
from FW.
for example pcie devices need 256M, but the bridge only get preallocated 2M...

pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
update res is not updated to bridge BAR. that is bridge is enabled already for
port service.

-v2: address Alex's concern about pci remove/rescan feature about pci_setup_bridge changes.
-v3: Kenji pointed out that pci_config_slot need to be called before pci_bus_add_devices()
-v4: move out pci_is_enabled checkout of pci_setup_bridge()
-v5: change the applying sequence.

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/hotplug/pciehp_pci.c | 31 +++++++++++++---
drivers/pci/setup-bus.c | 73 ++++++++++++++++++++++++++++++++++++---
include/linux/pci.h | 1
3 files changed, 95 insertions(+), 10 deletions(-)

Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
@@ -53,19 +53,18 @@ static int __ref pciehp_add_bridge(struc
busnr = pci_scan_bridge(parent, dev, busnr, pass);
if (!dev->subordinate)
return -1;
- pci_bus_size_bridges(dev->subordinate);
- pci_bus_assign_resources(parent);
- pci_enable_bridges(parent);
- pci_bus_add_devices(parent);
+
return 0;
}

int pciehp_configure_device(struct slot *p_slot)
{
struct pci_dev *dev;
- struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
+ struct pci_dev *bridge = p_slot->ctrl->pcie->port;
+ struct pci_bus *parent = bridge->subordinate;
int num, fn;
struct controller *ctrl = p_slot->ctrl;
+ int retval;

dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
if (dev) {
@@ -96,12 +95,30 @@ int pciehp_configure_device(struct slot
(dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
pciehp_add_bridge(dev);
}
+ pci_dev_put(dev);
+ }
+
+ pci_bus_size_bridges(parent);
+ pci_clear_master(bridge);
+ pci_bridge_assign_resources(bridge);
+ retval = pci_reenable_device(bridge);
+ pci_set_master(bridge);
+ pci_enable_bridges(parent);
+
+ for (fn = 0; fn < 8; fn++) {
+ dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
+ if (!dev)
+ continue;
+ if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+ pci_dev_put(dev);
+ continue;
+ }
pci_configure_slot(dev);
pci_dev_put(dev);
}

- pci_bus_assign_resources(parent);
pci_bus_add_devices(parent);
+
return 0;
}

@@ -155,5 +172,7 @@ int pciehp_unconfigure_device(struct slo
pci_dev_put(temp);
}

+ pci_bus_release_bridges_not_used_res(parent);
+
return rc;
}
Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -27,6 +27,44 @@
#include <linux/slab.h>
#include "pci.h"

+static void pdev_assign_resources_sorted(struct pci_dev *dev)
+{
+ struct resource *res;
+ struct resource_list head, *list, *tmp;
+ int idx;
+ u16 class = dev->class >> 8;
+
+ head.next = NULL;
+
+ /* Don't touch classless devices or host bridges or ioapics. */
+ if (class == PCI_CLASS_NOT_DEFINED ||
+ class == PCI_CLASS_BRIDGE_HOST)
+ return;
+
+ /* Don't touch ioapic devices already enabled by firmware */
+ if (class == PCI_CLASS_SYSTEM_PIC) {
+ u16 command;
+ pci_read_config_word(dev, PCI_COMMAND, &command);
+ if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
+ return;
+ }
+
+ pdev_sort_resources(dev, &head);
+
+ for (list = head.next; list;) {
+ res = list->res;
+ idx = res - &list->dev->resource[0];
+ if (pci_assign_resource(list->dev, idx)) {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+}
+
static void pbus_assign_resources_sorted(const struct pci_bus *bus)
{
struct pci_dev *dev;
@@ -144,9 +182,6 @@ static void pci_setup_bridge(struct pci_
u32 l, bu, lu, io_upper16;
int pref_mem64;

- if (pci_is_enabled(bridge))
- return;
-
dev_info(&bridge->dev, "PCI bridge, secondary bus %04x:%02x\n",
pci_domain_nr(bus), bus->number);

@@ -541,6 +576,35 @@ void __ref pci_bus_size_bridges(struct p
}
EXPORT_SYMBOL(pci_bus_size_bridges);

+void __ref pci_bridge_assign_resources(const struct pci_dev *bridge)
+{
+ struct pci_bus *b;
+
+ pdev_assign_resources_sorted((struct pci_dev *)bridge);
+
+ b = bridge->subordinate;
+ if (!b)
+ return;
+
+ pci_bus_assign_resources(b);
+
+ switch (bridge->class >> 8) {
+ case PCI_CLASS_BRIDGE_PCI:
+ pci_setup_bridge(b);
+ break;
+
+ case PCI_CLASS_BRIDGE_CARDBUS:
+ pci_setup_cardbus(b);
+ break;
+
+ default:
+ dev_info(&bridge->dev, "not setting up bridge for bus "
+ "%04x:%02x\n", pci_domain_nr(b), b->number);
+ break;
+ }
+}
+EXPORT_SYMBOL(pci_bridge_assign_resources);
+
void __ref pci_bus_assign_resources(const struct pci_bus *bus)
{
struct pci_bus *b;
@@ -557,7 +621,8 @@ void __ref pci_bus_assign_resources(cons

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
- pci_setup_bridge(b);
+ if (!pci_is_enabled(dev))
+ pci_setup_bridge(b);
break;

case PCI_CLASS_BRIDGE_CARDBUS:
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -756,6 +756,7 @@ ssize_t pci_write_vpd(struct pci_dev *de
int pci_vpd_truncate(struct pci_dev *dev, size_t size);

/* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
+void pci_bridge_assign_resources(const struct pci_dev *bridge);
void pci_bus_assign_resources(const struct pci_bus *bus);
void pci_bus_size_bridges(struct pci_bus *bus);
void pci_bus_release_bridges_not_used_res(struct pci_bus *bus);


2009-11-04 17:30:44

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v5

On Thu, 29 Oct 2009 02:52:25 -0700
Yinghai Lu <[email protected]> wrote:

>
> move out bus_size_bridges and assign resources out of
> pciehp_add_bridge() and at last do them all together one time
> including slot bridge, to avoid to call assign resources several
> times, when there are several bridges under the slot bridge.
>
> need to introduce pci_bridge_assign_resources there.
>
> handle the case the slot bridge that doesn't get pre-allocated big
> enough res from FW.
> for example pcie devices need 256M, but the bridge only get
> preallocated 2M...
>
> pci_setup_bridge() will take extra check_enabled for the slot bridge,
> otherwise update res is not updated to bridge BAR. that is bridge is
> enabled already for port service.
>
> -v2: address Alex's concern about pci remove/rescan feature about
> pci_setup_bridge changes. -v3: Kenji pointed out that pci_config_slot
> need to be called before pci_bus_add_devices() -v4: move out
> pci_is_enabled checkout of pci_setup_bridge() -v5: change the
> applying sequence.
>
> Signed-off-by: Yinghai Lu <[email protected]>

Where are we with this patchset? Given the nature of the changes I'll
definitely want to get a few Tested-bys in addition to acks from people
involved in this thread.

Thanks,
--
Jesse Barnes, Intel Open Source Technology Center

2009-11-04 18:53:32

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v5

Jesse Barnes wrote:
> On Thu, 29 Oct 2009 02:52:25 -0700
> Yinghai Lu <[email protected]> wrote:
>
>> move out bus_size_bridges and assign resources out of
>> pciehp_add_bridge() and at last do them all together one time
>> including slot bridge, to avoid to call assign resources several
>> times, when there are several bridges under the slot bridge.
>>
>> need to introduce pci_bridge_assign_resources there.
>>
>> handle the case the slot bridge that doesn't get pre-allocated big
>> enough res from FW.
>> for example pcie devices need 256M, but the bridge only get
>> preallocated 2M...
>>
>> pci_setup_bridge() will take extra check_enabled for the slot bridge,
>> otherwise update res is not updated to bridge BAR. that is bridge is
>> enabled already for port service.
>>
>> -v2: address Alex's concern about pci remove/rescan feature about
>> pci_setup_bridge changes. -v3: Kenji pointed out that pci_config_slot
>> need to be called before pci_bus_add_devices() -v4: move out
>> pci_is_enabled checkout of pci_setup_bridge() -v5: change the
>> applying sequence.
>>
>> Signed-off-by: Yinghai Lu <[email protected]>
>
> Where are we with this patchset? Given the nature of the changes I'll
> definitely want to get a few Tested-bys in addition to acks from people
> involved in this thread.

already have -v8.

but will refresh them after you push out your tree with several patches from Bjorn.

YH

2009-11-05 01:40:54

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v9


only for index < 3

v2: Jesse doesn't like it is in find_free_bus_resource...
try to move out of pci_bus_size_bridges loop.
v3: add pci_setup_bridge calling after pci_bridge_release_not_used_res.
only clear release those res for x86.
v4: Bjorn want to release use dev instead of bus.
v5: Kenji pointed out it will have problem with several level bridge.
so let only handle leaf bridge.
v6: address Kenji's request (new pci_bus_release...). and change applying order
move back release to pci_assign_unassigned_resource
v7: change functions name pci_bus_release_unused_bridge_res according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not big
enough need to do it in two steps, and first step recore the failed res,
and don't touch bridge res that programmed by firmware. second step will
try to release bridge resource that is too small at first.
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/setup-bus.c | 253 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 234 insertions(+), 19 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -27,7 +27,49 @@
#include <linux/slab.h>
#include "pci.h"

-static void pbus_assign_resources_sorted(const struct pci_bus *bus)
+
+static void add_to_failed_list(struct resource_list *head, struct pci_dev *dev,
+ struct resource *res)
+{
+ struct resource_list *list = head;
+ struct resource_list *ln = list->next;
+ struct resource_list *tmp;
+
+ tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp) {
+ pr_warning("add_to_failed_list: kmalloc() failed!\n");
+ return;
+ }
+
+ tmp->next = ln;
+ tmp->res = res;
+ tmp->dev = dev;
+ list->next = tmp;
+}
+
+static void free_failed_list(struct resource_list *head)
+{
+ struct resource_list *list, *tmp;
+ struct resource *res;
+ /*
+ * Try to release leaf bridge's resources that there is no child
+ * under it
+ */
+ for (list = head->next; list;) {
+ res = list->res;
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ head->next = NULL;
+}
+
+static void pbus_assign_resources_sorted(const struct pci_bus *bus,
+ struct resource_list *fail_head)
{
struct pci_dev *dev;
struct resource *res;
@@ -58,9 +100,18 @@ static void pbus_assign_resources_sorted
res = list->res;
idx = res - &list->dev->resource[0];
if (pci_assign_resource(list->dev, idx)) {
- res->start = 0;
- res->end = 0;
- res->flags = 0;
+ if (fail_head && !list->dev->subordinate &&
+ !pci_is_root_bus(list->dev->bus)) {
+ /*
+ * device need to keep flags and size
+ * for second try
+ */
+ add_to_failed_list(fail_head, list->dev, res);
+ } else {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
}
tmp = list;
list = list->next;
@@ -134,19 +185,12 @@ EXPORT_SYMBOL(pci_setup_cardbus);
config space writes, so it's quite possible that an I/O window of
the bridge will have some undesirable address (e.g. 0) after the
first write. Ditto 64-bit prefetchable MMIO. */
-static void pci_setup_bridge(struct pci_bus *bus)
+static void pci_setup_bridge_io(struct pci_bus *bus)
{
struct pci_dev *bridge = bus->self;
struct resource *res;
struct pci_bus_region region;
- u32 l, bu, lu, io_upper16;
- int pref_mem64;
-
- if (pci_is_enabled(bridge))
- return;
-
- dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
- bus->secondary, bus->subordinate);
+ u32 l, io_upper16;

/* Set up the top and bottom of the PCI I/O segment for this bus. */
res = bus->resource[0];
@@ -172,7 +216,13 @@ static void pci_setup_bridge(struct pci_
pci_write_config_dword(bridge, PCI_IO_BASE, l);
/* Update upper 16 bits of I/O base/limit. */
pci_write_config_dword(bridge, PCI_IO_BASE_UPPER16, io_upper16);
-
+}
+static void pci_setup_bridge_mmio(struct pci_bus *bus)
+{
+ struct pci_dev *bridge = bus->self;
+ struct resource *res;
+ struct pci_bus_region region;
+ u32 l;
/* Set up the top and bottom of the PCI Memory segment
for this bus. */
res = bus->resource[1];
@@ -187,6 +237,14 @@ static void pci_setup_bridge(struct pci_
dev_info(&bridge->dev, " bridge window [mem disabled]\n");
}
pci_write_config_dword(bridge, PCI_MEMORY_BASE, l);
+}
+static void pci_setup_bridge_mmio_pref(struct pci_bus *bus)
+{
+ struct pci_dev *bridge = bus->self;
+ struct resource *res;
+ struct pci_bus_region region;
+ u32 l, bu, lu;
+ int pref_mem64;

/* Clear out the upper 32 bits of PREF limit.
If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
@@ -219,10 +277,37 @@ static void pci_setup_bridge(struct pci_
pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
}
+}
+static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type)
+{
+ struct pci_dev *bridge = bus->self;
+
+ if (pci_is_enabled(bridge))
+ return;
+
+ dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
+ bus->secondary, bus->subordinate);
+
+ if (type & IORESOURCE_IO)
+ pci_setup_bridge_io(bus);
+
+ if (type & IORESOURCE_MEM)
+ pci_setup_bridge_mmio(bus);
+
+ if (type & IORESOURCE_PREFETCH)
+ pci_setup_bridge_mmio_pref(bus);

pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
}

+static void pci_setup_bridge(struct pci_bus *bus)
+{
+ unsigned long type = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+ __pci_setup_bridge(bus, type);
+}
+
/* Check whether the bridge supports optional I/O and
prefetchable memory ranges. If not, the respective
base/limit registers must be read-only and read as 0. */
@@ -543,19 +628,20 @@ void __ref pci_bus_size_bridges(struct p
}
EXPORT_SYMBOL(pci_bus_size_bridges);

-void __ref pci_bus_assign_resources(const struct pci_bus *bus)
+static void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
+ struct resource_list *fail_head)
{
struct pci_bus *b;
struct pci_dev *dev;

- pbus_assign_resources_sorted(bus);
+ pbus_assign_resources_sorted(bus, fail_head);

list_for_each_entry(dev, &bus->devices, bus_list) {
b = dev->subordinate;
if (!b)
continue;

- pci_bus_assign_resources(b);
+ __pci_bus_assign_resources(b, fail_head);

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
@@ -573,15 +659,105 @@ void __ref pci_bus_assign_resources(cons
}
}
}
+
+void __ref pci_bus_assign_resources(const struct pci_bus *bus)
+{
+ __pci_bus_assign_resources(bus, NULL);
+}
EXPORT_SYMBOL(pci_bus_assign_resources);

+static void release_children_resource(struct resource *r)
+{
+ struct resource *p;
+ resource_size_t size;
+
+ p = r->child;
+ while (p) {
+ release_children_resource(p);
+ release_resource(p);
+ printk(KERN_DEBUG "PCI: release child resource %pRt\n", p);
+ /* need to restore size, and keep flags */
+ size = resource_size(p);
+ p->start = 0;
+ p->end = size - 1;
+ p = r->child;
+ }
+}
+
+static void pci_bridge_release_unused_res(struct pci_bus *bus,
+ unsigned long type)
+{
+ int idx;
+ bool changed = false;
+ struct pci_dev *dev;
+ struct resource *r;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+ /* for pci bridges res only */
+ dev = bus->self;
+ for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_BRIDGE_RESOURCES + 3;
+ idx++) {
+ r = &dev->resource[idx];
+ if ((r->flags & type_mask) != type)
+ continue;
+ if (!r->parent)
+ continue;
+ /*
+ * if there are children under that, we should release them
+ * all
+ */
+ release_children_resource(r);
+ if (!release_resource(r)) {
+ dev_printk(KERN_DEBUG, &dev->dev,
+ "resource %d %pRt released\n", idx, r);
+ r->flags = 0;
+ changed = true;
+ }
+ }
+
+ if (changed) {
+ if (type & IORESOURCE_PREFETCH) {
+ /* avoiding touch the one without PREF */
+ type = IORESOURCE_PREFETCH;
+ }
+ __pci_setup_bridge(bus, type);
+ }
+}
+
+/*
+ * try to release pci bridge resources that is from leaf bridge,
+ * so we can allocate big new one later
+ */
+static void __ref pci_bus_release_unused_bridge_res(struct pci_bus *bus,
+ unsigned long type)
+{
+ struct pci_bus *b;
+
+ /* If the bus is cardbus, do nothing */
+ if (bus->self && (bus->self->class >> 8) == PCI_CLASS_BRIDGE_CARDBUS)
+ return;
+
+ /* We only release the resources under the leaf bridge */
+ if (list_empty(&bus->children)) {
+ if (!pci_is_root_bus(bus))
+ pci_bridge_release_unused_res(bus, type);
+ return;
+ }
+
+ /* Scan child buses if the bus is not leaf */
+ list_for_each_entry(b, &bus->children, node)
+ pci_bus_release_unused_bridge_res(b, type);
+}
+
static void pci_bus_dump_res(struct pci_bus *bus)
{
int i;

for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
struct resource *res = bus->resource[i];
- if (!res || !res->end)
+
+ if (!res || !res->end || !res->flags)
continue;

dev_printk(KERN_DEBUG, &bus->dev, "resource %d %pR\n", i, res);
@@ -609,6 +785,12 @@ void __init
pci_assign_unassigned_resources(void)
{
struct pci_bus *bus;
+ bool second_tried = false;
+ struct resource_list head, *list, *tmp;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+again:
+ head.next = NULL;

/* Depth first, calculate sizes and alignments of all
subordinate buses. */
@@ -617,7 +799,40 @@ pci_assign_unassigned_resources(void)
}
/* Depth last, allocate resources and update the hardware. */
list_for_each_entry(bus, &pci_root_buses, node) {
- pci_bus_assign_resources(bus);
+ __pci_bus_assign_resources(bus, &head);
+ }
+
+ /* any device complain? */
+ if (!head.next)
+ goto enable_and_dump;
+
+ if (second_tried) {
+ /* still fail, don't want to try more */
+ free_failed_list(&head);
+ goto enable_and_dump;
+ }
+
+ second_tried = true;
+ printk(KERN_DEBUG "PCI: second try to assign unassigned res\n");
+
+ /*
+ * Try to release leaf bridge's resources that doesn't fit resource of
+ * child device under that bridge
+ */
+ for (list = head.next; list;) {
+ bus = list->dev->bus;
+ pci_bus_release_unused_bridge_res(bus,
+ list->res->flags & type_mask);
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ goto again;
+
+enable_and_dump:
+ /* Depth last, update the hardware. */
+ list_for_each_entry(bus, &pci_root_buses, node) {
pci_enable_bridges(bus);
}

2009-11-05 01:41:51

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v9


move out bus_size_bridges and assign resources out of pciehp_add_bridge()
and at last do them all together one time including slot bridge, to avoid to
call assign resources several times, when there are several bridges under the
slot bridge.

need to introduce pci_bridge_assign_resources there.

handle the case the slot bridge that doesn't get pre-allocated big enough res
from FW.
for example pcie devices need 256M, but the bridge only get preallocated 2M...

pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
update res is not updated to bridge BAR. that is bridge is enabled already for
port service.

v2: address Alex's concern about pci remove/rescan feature about
pci_setup_bridge changes.
v3: Kenji pointed out that pci_config_slot need to be called before
pci_bus_add_devices()
v4: move out pci_is_enabled checkout of pci_setup_bridge()
v5: change the applying sequence.
v6: change the functions name according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not
big enough
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/hotplug/pciehp_pci.c | 23 +++++-
drivers/pci/setup-bus.c | 129 +++++++++++++++++++++++++++++++++++++--
include/linux/pci.h | 1
3 files changed, 143 insertions(+), 10 deletions(-)

Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
@@ -53,17 +53,15 @@ static int __ref pciehp_add_bridge(struc
busnr = pci_scan_bridge(parent, dev, busnr, pass);
if (!dev->subordinate)
return -1;
- pci_bus_size_bridges(dev->subordinate);
- pci_bus_assign_resources(parent);
- pci_enable_bridges(parent);
- pci_bus_add_devices(parent);
+
return 0;
}

int pciehp_configure_device(struct slot *p_slot)
{
struct pci_dev *dev;
- struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
+ struct pci_dev *bridge = p_slot->ctrl->pcie->port;
+ struct pci_bus *parent = bridge->subordinate;
int num, fn;
struct controller *ctrl = p_slot->ctrl;

@@ -96,12 +94,25 @@ int pciehp_configure_device(struct slot
(dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
pciehp_add_bridge(dev);
}
+ pci_dev_put(dev);
+ }
+
+ pci_assign_unassigned_bridge_resources(bridge);
+
+ for (fn = 0; fn < 8; fn++) {
+ dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
+ if (!dev)
+ continue;
+ if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+ pci_dev_put(dev);
+ continue;
+ }
pci_configure_slot(dev);
pci_dev_put(dev);
}

- pci_bus_assign_resources(parent);
pci_bus_add_devices(parent);
+
return 0;
}

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -68,6 +68,52 @@ static void free_failed_list(struct reso
head->next = NULL;
}

+static void pdev_assign_resources_sorted(struct pci_dev *dev,
+ struct resource_list *fail_head)
+{
+ struct resource *res;
+ struct resource_list head, *list, *tmp;
+ int idx;
+ u16 class = dev->class >> 8;
+
+ head.next = NULL;
+
+ /* Don't touch classless devices or host bridges or ioapics. */
+ if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
+ return;
+
+ /* Don't touch ioapic devices already enabled by firmware */
+ if (class == PCI_CLASS_SYSTEM_PIC) {
+ u16 command;
+ pci_read_config_word(dev, PCI_COMMAND, &command);
+ if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
+ return;
+ }
+
+ pdev_sort_resources(dev, &head);
+
+ for (list = head.next; list;) {
+ res = list->res;
+ idx = res - &list->dev->resource[0];
+ if (pci_assign_resource(list->dev, idx)) {
+ if (fail_head && !list->dev->subordinate &&
+ !pci_is_root_bus(list->dev->bus)) {
+ /*
+ * device need to keep flags and size
+ * for second try
+ */
+ add_to_failed_list(fail_head, list->dev, res);
+ } else {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
+ }
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+}
static void pbus_assign_resources_sorted(const struct pci_bus *bus,
struct resource_list *fail_head)
{
@@ -282,9 +328,6 @@ static void __pci_setup_bridge(struct pc
{
struct pci_dev *bridge = bus->self;

- if (pci_is_enabled(bridge))
- return;
-
dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
bus->secondary, bus->subordinate);

@@ -645,7 +688,8 @@ static void __ref __pci_bus_assign_resou

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
- pci_setup_bridge(b);
+ if (!pci_is_enabled(dev))
+ pci_setup_bridge(b);
break;

case PCI_CLASS_BRIDGE_CARDBUS:
@@ -666,6 +710,34 @@ void __ref pci_bus_assign_resources(cons
}
EXPORT_SYMBOL(pci_bus_assign_resources);

+static void __ref __pci_bridge_assign_resources(const struct pci_dev *bridge,
+ struct resource_list *fail_head)
+{
+ struct pci_bus *b;
+
+ pdev_assign_resources_sorted((struct pci_dev *)bridge, fail_head);
+
+ b = bridge->subordinate;
+ if (!b)
+ return;
+
+ __pci_bus_assign_resources(b, fail_head);
+
+ switch (bridge->class >> 8) {
+ case PCI_CLASS_BRIDGE_PCI:
+ pci_setup_bridge(b);
+ break;
+
+ case PCI_CLASS_BRIDGE_CARDBUS:
+ pci_setup_cardbus(b);
+ break;
+
+ default:
+ dev_info(&bridge->dev, "not setting up bridge for bus "
+ "%04x:%02x\n", pci_domain_nr(b), b->number);
+ break;
+ }
+}
static void release_children_resource(struct resource *r)
{
struct resource *p;
@@ -841,3 +913,52 @@ enable_and_dump:
pci_bus_dump_resources(bus);
}
}
+
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
+{
+ struct pci_bus *bus;
+ struct pci_bus *parent = bridge->subordinate;
+ bool second_tried = false;
+ struct resource_list head, *list, *tmp;
+ int retval;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+again:
+ head.next = NULL;
+
+ pci_bus_size_bridges(parent);
+ pci_clear_master(bridge);
+ __pci_bridge_assign_resources(bridge, &head);
+ retval = pci_reenable_device(bridge);
+ pci_set_master(bridge);
+ pci_enable_bridges(parent);
+
+ /* any device complain? */
+ if (!head.next)
+ return;
+
+ if (second_tried) {
+ /* still fail, don't want to try more */
+ free_failed_list(&head);
+ return;
+ }
+
+ second_tried = true;
+ printk(KERN_DEBUG "PCI: second try to assign unassigned res\n");
+
+ /*
+ * Try to release leaf bridge's resources that doesn't fit resource of
+ * child device under that bridge
+ */
+ for (list = head.next; list;) {
+ bus = list->dev->bus;
+ pci_bus_release_unused_bridge_res(bus,
+ list->res->flags & type_mask);
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ goto again;
+}
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -767,6 +767,7 @@ void pci_bus_assign_resources(const stru
void pci_bus_size_bridges(struct pci_bus *bus);
int pci_claim_resource(struct pci_dev *, int);
void pci_assign_unassigned_resources(void);
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
void pdev_enable_device(struct pci_dev *);
void pdev_sort_resources(struct pci_dev *, struct resource_list *);
int pci_enable_resources(struct pci_dev *, int mask);

2009-11-05 20:47:56

by Alex Chiang

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v9

* Yinghai Lu <[email protected]>:
>
> move out bus_size_bridges and assign resources out of pciehp_add_bridge()
> and at last do them all together one time including slot bridge, to avoid to
> call assign resources several times, when there are several bridges under the
> slot bridge.
>
> need to introduce pci_bridge_assign_resources there.
>
> handle the case the slot bridge that doesn't get pre-allocated big enough res
> from FW.
> for example pcie devices need 256M, but the bridge only get preallocated 2M...
>
> pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
> update res is not updated to bridge BAR. that is bridge is enabled already for
> port service.
>
> v2: address Alex's concern about pci remove/rescan feature about
> pci_setup_bridge changes.
> v3: Kenji pointed out that pci_config_slot need to be called before
> pci_bus_add_devices()
> v4: move out pci_is_enabled checkout of pci_setup_bridge()
> v5: change the applying sequence.
> v6: change the functions name according to Jesse
> v8: address Eric's concern, only overwrite leaf bridge resource that is not
> big enough
> v9: refresh to be applied after bjorn's patch, and remove trick about save
> size and restore resource second try.
>
> Signed-off-by: Yinghai Lu <[email protected]>

How did you build and test this? I got this during building:

ERROR: "pci_assign_unassigned_bridge_resources" [drivers/pci/hotplug/pciehp.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....

This patch fixed it for me:
---
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 95fe3f4..2f15473 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -962,3 +962,4 @@ again:

goto again;
}
+EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);

2009-11-05 21:07:43

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v9

Alex Chiang wrote:
> * Yinghai Lu <[email protected]>:
>> move out bus_size_bridges and assign resources out of pciehp_add_bridge()
>> and at last do them all together one time including slot bridge, to avoid to
>> call assign resources several times, when there are several bridges under the
>> slot bridge.
>>
>> need to introduce pci_bridge_assign_resources there.
>>
>> handle the case the slot bridge that doesn't get pre-allocated big enough res
>> from FW.
>> for example pcie devices need 256M, but the bridge only get preallocated 2M...
>>
>> pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
>> update res is not updated to bridge BAR. that is bridge is enabled already for
>> port service.
>>
>> v2: address Alex's concern about pci remove/rescan feature about
>> pci_setup_bridge changes.
>> v3: Kenji pointed out that pci_config_slot need to be called before
>> pci_bus_add_devices()
>> v4: move out pci_is_enabled checkout of pci_setup_bridge()
>> v5: change the applying sequence.
>> v6: change the functions name according to Jesse
>> v8: address Eric's concern, only overwrite leaf bridge resource that is not
>> big enough
>> v9: refresh to be applied after bjorn's patch, and remove trick about save
>> size and restore resource second try.
>>
>> Signed-off-by: Yinghai Lu <[email protected]>
>
> How did you build and test this? I got this during building:
>
> ERROR: "pci_assign_unassigned_bridge_resources" [drivers/pci/hotplug/pciehp.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> make: *** Waiting for unfinished jobs....
>
> This patch fixed it for me:
> ---
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 95fe3f4..2f15473 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -962,3 +962,4 @@ again:
>
> goto again;
> }
> +EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);

thanks.

i built that in

YH

2009-11-07 05:42:12

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v10


move out bus_size_bridges and assign resources out of pciehp_add_bridge()
and at last do them all together one time including slot bridge, to avoid to
call assign resources several times, when there are several bridges under the
slot bridge.

need to introduce pci_bridge_assign_resources there.

handle the case the slot bridge that doesn't get pre-allocated big enough res
from FW.
for example pcie devices need 256M, but the bridge only get preallocated 2M...

pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
update res is not updated to bridge BAR. that is bridge is enabled already for
port service.

v2: address Alex's concern about pci remove/rescan feature about
pci_setup_bridge changes.
v3: Kenji pointed out that pci_config_slot need to be Yinghai Lu wrote:
> move out bus_size_bridges and assign resources out of pciehp_add_bridge()
> and at last do them all together one time including slot bridge, to avoid to
> call assign resources several times, when there are several bridges under the
> slot bridge.
>
> need to introduce pci_bridge_assign_resources there.
>
> handle the case the slot bridge that doesn't get pre-allocated big enough res
> from FW.
> for example pcie devices need 256M, but the bridge only get preallocated 2M...
>
> pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
> update res is not updated to bridge BAR. that is bridge is enabled already for
> port service.
>
> v2: address Alex's concern about pci remove/rescan feature about
> pci_setup_bridge changes.
> v3: Kenji pointed out that pci_config_slot need to be called before
> pci_bus_add_devices()
> v4: move out pci_is_enabled checkout of pci_setup_bridge()
> v5: change the applying sequence.
> v6: change the functions name according to Jesse
> v8: address Eric's concern, only overwrite leaf bridge resource that is not
> big enough
> v9: refresh to be applied after bjorn's patch, and remove trick about save
> size and restore resource second try.
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> ---
> drivers/pci/hotplug/pciehp_pci.c | 23 +++++-
> drivers/pci/setup-bus.c | 129 +++++++++++++++++++++++++++++++++++++--
> include/linux/pci.h | 1
> 3 files changed, 143 insertions(+), 10 deletions(-)
>
> Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
> +++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
> @@ -53,17 +53,15 @@ static int __ref pciehp_add_bridge(struc
> busnr = pci_scan_bridge(parent, dev, busnr, pass);
> if (!dev->subordinate)
> return -1;
> - pci_bus_size_bridges(dev->subordinate);
> - pci_bus_assign_resources(parent);
> - pci_enable_bridges(parent);
> - pci_bus_add_devices(parent);
> +
> return 0;
> }
>
> int pciehp_configure_device(struct slot *p_slot)
> {
> struct pci_dev *dev;
> - struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
> + struct pci_dev *bridge = p_slot->ctrl->pcie->port;
> + struct pci_bus *parent = bridge->subordinate;
> int num, fn;
> struct controller *ctrl = p_slot->ctrl;
>
> @@ -96,12 +94,25 @@ int pciehp_configure_device(struct slot
> (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
> pciehp_add_bridge(dev);
> }
> + pci_dev_put(dev);
> + }
> +
> + pci_assign_unassigned_bridge_resources(bridge);
> +
> + for (fn = 0; fn < 8; fn++) {
> + dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
> + if (!dev)
> + continue;
> + if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
> + pci_dev_put(dev);
> + continue;
> + }
> pci_configure_slot(dev);
> pci_dev_put(dev);
> }
>
> - pci_bus_assign_resources(parent);
> pci_bus_add_devices(parent);
> +
> return 0;
> }
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -68,6 +68,52 @@ static void free_failed_list(struct reso
> head->next = NULL;
> }
>
> +static void pdev_assign_resources_sorted(struct pci_dev *dev,
> + struct resource_list *fail_head)
> +{
> + struct resource *res;
> + struct resource_list head, *list, *tmp;
> + int idx;
> + u16 class = dev->class >> 8;
> +
> + head.next = NULL;
> +
> + /* Don't touch classless devices or host bridges or ioapics. */
> + if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
> + return;
> +
> + /* Don't touch ioapic devices already enabled by firmware */
> + if (class == PCI_CLASS_SYSTEM_PIC) {
> + u16 command;
> + pci_read_config_word(dev, PCI_COMMAND, &command);
> + if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
> + return;
> + }
> +
> + pdev_sort_resources(dev, &head);
> +
> + for (list = head.next; list;) {
> + res = list->res;
> + idx = res - &list->dev->resource[0];
> + if (pci_assign_resource(list->dev, idx)) {
> + if (fail_head && !list->dev->subordinate &&
> + !pci_is_root_bus(list->dev->bus)) {
> + /*
> + * device need to keep flags and size
> + * for second try
> + */
> + add_to_failed_list(fail_head, list->dev, res);
> + } else {
> + res->start = 0;
> + res->end = 0;
> + res->flags = 0;
> + }
> + }
> + tmp = list;
> + list = list->next;
> + kfree(tmp);
> + }
> +}
> static void pbus_assign_resources_sorted(const struct pci_bus *bus,
> struct resource_list *fail_head)
> {
> @@ -282,9 +328,6 @@ static void __pci_setup_bridge(struct pc
> {
> struct pci_dev *bridge = bus->self;
>
> - if (pci_is_enabled(bridge))
> - return;
> -
> dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
> bus->secondary, bus->subordinate);
>
> @@ -645,7 +688,8 @@ static void __ref __pci_bus_assign_resou
>
> switch (dev->class >> 8) {
> case PCI_CLASS_BRIDGE_PCI:
> - pci_setup_bridge(b);
> + if (!pci_is_enabled(dev))
> + pci_setup_bridge(b);
> break;
>
> case PCI_CLASS_BRIDGE_CARDBUS:
> @@ -666,6 +710,34 @@ void __ref pci_bus_assign_resources(cons
> }
> EXPORT_SYMBOL(pci_bus_assign_resources);
>
> +static void __ref __pci_bridge_assign_resources(const struct pci_dev *bridge,
> + struct resource_list *fail_head)
> +{
> + struct pci_bus *b;
> +
> + pdev_assign_resources_sorted((struct pci_dev *)bridge, fail_head);
> +
> + b = bridge->subordinate;
> + if (!b)
> + return;
> +
> + __pci_bus_assign_resources(b, fail_head);
> +
> + switch (bridge->class >> 8) {
> + case PCI_CLASS_BRIDGE_PCI:
> + pci_setup_bridge(b);
> + break;
> +
> + case PCI_CLASS_BRIDGE_CARDBUS:
> + pci_setup_cardbus(b);
> + break;
> +
> + default:
> + dev_info(&bridge->dev, "not setting up bridge for bus "
> + "%04x:%02x\n", pci_domain_nr(b), b->number);
> + break;
> + }
> +}
> static void release_children_resource(struct resource *r)
> {
> struct resource *p;
> @@ -841,3 +913,52 @@ enable_and_dump:
> pci_bus_dump_resources(bus);
> }
> }
> +
> +void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
> +{
> + struct pci_bus *bus;
> + struct pci_bus *parent = bridge->subordinate;
> + bool second_tried = false;
> + struct resource_list head, *list, *tmp;
> + int retval;
> + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH;
> +
> +again:
> + head.next = NULL;
> +
> + pci_bus_size_bridges(parent);
> + pci_clear_master(bridge);
> + __pci_bridge_assign_resources(bridge, &head);
> + retval = pci_reenable_device(bridge);
> + pci_set_master(bridge);
> + pci_enable_bridges(parent);
> +
> + /* any device complain? */
> + if (!head.next)
> + return;
> +
> + if (second_tried) {
> + /* still fail, don't want to try more */
> + free_failed_list(&head);
> + return;
> + }
> +
> + second_tried = true;
> + printk(KERN_DEBUG "PCI: second try to assign unassigned res\n");
> +
> + /*
> + * Try to release leaf bridge's resources that doesn't fit resource of
> + * child device under that bridge
> + */
> + for (list = head.next; list;) {
> + bus = list->dev->bus;
> + pci_bus_release_unused_bridge_res(bus,
> + list->res->flags & type_mask);
> + tmp = list;
> + list = list->next;
> + kfree(tmp);
> + }
> +
> + goto again;
> +}
> Index: linux-2.6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pci.h
> +++ linux-2.6/include/linux/pci.h
> @@ -767,6 +767,7 @@ void pci_bus_assign_resources(const stru
> void pci_bus_size_bridges(struct pci_bus *bus);
> int pci_claim_resource(struct pci_dev *, int);
> void pci_assign_unassigned_resources(void);
> +void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
> void pdev_enable_device(struct pci_dev *);
> void pdev_sort_resources(struct pci_dev *, struct resource_list *);
> int pci_enable_resources(struct pci_dev *, int mask);
>

called before
pci_bus_add_devices()
v4: move out pci_is_enabled checkout of pci_setup_bridge()
v5: change the applying sequence.
v6: change the functions name according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not
big enough
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v10: alex found need to have export for pci_assign_unassigned_bridge_resources

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/hotplug/pciehp_pci.c | 23 +++++-
drivers/pci/setup-bus.c | 130 +++++++++++++++++++++++++++++++++++++--
include/linux/pci.h | 1
3 files changed, 144 insertions(+), 10 deletions(-)

Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
@@ -53,17 +53,15 @@ static int __ref pciehp_add_bridge(struc
busnr = pci_scan_bridge(parent, dev, busnr, pass);
if (!dev->subordinate)
return -1;
- pci_bus_size_bridges(dev->subordinate);
- pci_bus_assign_resources(parent);
- pci_enable_bridges(parent);
- pci_bus_add_devices(parent);
+
return 0;
}

int pciehp_configure_device(struct slot *p_slot)
{
struct pci_dev *dev;
- struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
+ struct pci_dev *bridge = p_slot->ctrl->pcie->port;
+ struct pci_bus *parent = bridge->subordinate;
int num, fn;
struct controller *ctrl = p_slot->ctrl;

@@ -96,12 +94,25 @@ int pciehp_configure_device(struct slot
(dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
pciehp_add_bridge(dev);
}
+ pci_dev_put(dev);
+ }
+
+ pci_assign_unassigned_bridge_resources(bridge);
+
+ for (fn = 0; fn < 8; fn++) {
+ dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
+ if (!dev)
+ continue;
+ if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+ pci_dev_put(dev);
+ continue;
+ }
pci_configure_slot(dev);
pci_dev_put(dev);
}

- pci_bus_assign_resources(parent);
pci_bus_add_devices(parent);
+
return 0;
}

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -68,6 +68,52 @@ static void free_failed_list(struct reso
head->next = NULL;
}

+static void pdev_assign_resources_sorted(struct pci_dev *dev,
+ struct resource_list *fail_head)
+{
+ struct resource *res;
+ struct resource_list head, *list, *tmp;
+ int idx;
+ u16 class = dev->class >> 8;
+
+ head.next = NULL;
+
+ /* Don't touch classless devices or host bridges or ioapics. */
+ if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
+ return;
+
+ /* Don't touch ioapic devices already enabled by firmware */
+ if (class == PCI_CLASS_SYSTEM_PIC) {
+ u16 command;
+ pci_read_config_word(dev, PCI_COMMAND, &command);
+ if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
+ return;
+ }
+
+ pdev_sort_resources(dev, &head);
+
+ for (list = head.next; list;) {
+ res = list->res;
+ idx = res - &list->dev->resource[0];
+ if (pci_assign_resource(list->dev, idx)) {
+ if (fail_head && !list->dev->subordinate &&
+ !pci_is_root_bus(list->dev->bus)) {
+ /*
+ * device need to keep flags and size
+ * for second try
+ */
+ add_to_failed_list(fail_head, list->dev, res);
+ } else {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
+ }
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+}
static void pbus_assign_resources_sorted(const struct pci_bus *bus,
struct resource_list *fail_head)
{
@@ -282,9 +328,6 @@ static void __pci_setup_bridge(struct pc
{
struct pci_dev *bridge = bus->self;

- if (pci_is_enabled(bridge))
- return;
-
dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
bus->secondary, bus->subordinate);

@@ -645,7 +688,8 @@ static void __ref __pci_bus_assign_resou

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
- pci_setup_bridge(b);
+ if (!pci_is_enabled(dev))
+ pci_setup_bridge(b);
break;

case PCI_CLASS_BRIDGE_CARDBUS:
@@ -666,6 +710,34 @@ void __ref pci_bus_assign_resources(cons
}
EXPORT_SYMBOL(pci_bus_assign_resources);

+static void __ref __pci_bridge_assign_resources(const struct pci_dev *bridge,
+ struct resource_list *fail_head)
+{
+ struct pci_bus *b;
+
+ pdev_assign_resources_sorted((struct pci_dev *)bridge, fail_head);
+
+ b = bridge->subordinate;
+ if (!b)
+ return;
+
+ __pci_bus_assign_resources(b, fail_head);
+
+ switch (bridge->class >> 8) {
+ case PCI_CLASS_BRIDGE_PCI:
+ pci_setup_bridge(b);
+ break;
+
+ case PCI_CLASS_BRIDGE_CARDBUS:
+ pci_setup_cardbus(b);
+ break;
+
+ default:
+ dev_info(&bridge->dev, "not setting up bridge for bus "
+ "%04x:%02x\n", pci_domain_nr(b), b->number);
+ break;
+ }
+}
static void release_children_resource(struct resource *r)
{
struct resource *p;
@@ -841,3 +913,53 @@ enable_and_dump:
pci_bus_dump_resources(bus);
}
}
+
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
+{
+ struct pci_bus *bus;
+ struct pci_bus *parent = bridge->subordinate;
+ bool second_tried = false;
+ struct resource_list head, *list, *tmp;
+ int retval;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+again:
+ head.next = NULL;
+
+ pci_bus_size_bridges(parent);
+ pci_clear_master(bridge);
+ __pci_bridge_assign_resources(bridge, &head);
+ retval = pci_reenable_device(bridge);
+ pci_set_master(bridge);
+ pci_enable_bridges(parent);
+
+ /* any device complain? */
+ if (!head.next)
+ return;
+
+ if (second_tried) {
+ /* still fail, don't want to try more */
+ free_failed_list(&head);
+ return;
+ }
+
+ second_tried = true;
+ printk(KERN_DEBUG "PCI: second try to assign unassigned res\n");
+
+ /*
+ * Try to release leaf bridge's resources that doesn't fit resource of
+ * child device under that bridge
+ */
+ for (list = head.next; list;) {
+ bus = list->dev->bus;
+ pci_bus_release_unused_bridge_res(bus,
+ list->res->flags & type_mask);
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ goto again;
+}
+EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -768,6 +768,7 @@ void pci_bus_assign_resources(const stru
void pci_bus_size_bridges(struct pci_bus *bus);
int pci_claim_resource(struct pci_dev *, int);
void pci_assign_unassigned_resources(void);
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
void pdev_enable_device(struct pci_dev *);
void pdev_sort_resources(struct pci_dev *, struct resource_list *);
int pci_enable_resources(struct pci_dev *, int mask);

2009-11-07 05:44:04

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v10


move out bus_size_bridges and assign resources out of pciehp_add_bridge()
and at last do them all together one time including slot bridge, to avoid to
call assign resources several times, when there are several bridges under the
slot bridge.

need to introduce pci_bridge_assign_resources there.

handle the case the slot bridge that doesn't get pre-allocated big enough res
from FW.
for example pcie devices need 256M, but the bridge only get preallocated 2M...

pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
update res is not updated to bridge BAR. that is bridge is enabled already for
port service.

v2: address Alex's concern about pci remove/rescan feature about
pci_setup_bridge changes.
v3: Kenji pointed out that pci_config_slot need to be called before
pci_bus_add_devices()
v4: move out pci_is_enabled checkout of pci_setup_bridge()
v5: change the applying sequence.
v6: change the functions name according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not
big enough
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v10: alex found need to have export for pci_assign_unassigned_bridge_resources

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/hotplug/pciehp_pci.c | 23 +++++-
drivers/pci/setup-bus.c | 130 +++++++++++++++++++++++++++++++++++++--
include/linux/pci.h | 1
3 files changed, 144 insertions(+), 10 deletions(-)

Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
@@ -53,17 +53,15 @@ static int __ref pciehp_add_bridge(struc
busnr = pci_scan_bridge(parent, dev, busnr, pass);
if (!dev->subordinate)
return -1;
- pci_bus_size_bridges(dev->subordinate);
- pci_bus_assign_resources(parent);
- pci_enable_bridges(parent);
- pci_bus_add_devices(parent);
+
return 0;
}

int pciehp_configure_device(struct slot *p_slot)
{
struct pci_dev *dev;
- struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
+ struct pci_dev *bridge = p_slot->ctrl->pcie->port;
+ struct pci_bus *parent = bridge->subordinate;
int num, fn;
struct controller *ctrl = p_slot->ctrl;

@@ -96,12 +94,25 @@ int pciehp_configure_device(struct slot
(dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
pciehp_add_bridge(dev);
}
+ pci_dev_put(dev);
+ }
+
+ pci_assign_unassigned_bridge_resources(bridge);
+
+ for (fn = 0; fn < 8; fn++) {
+ dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
+ if (!dev)
+ continue;
+ if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+ pci_dev_put(dev);
+ continue;
+ }
pci_configure_slot(dev);
pci_dev_put(dev);
}

- pci_bus_assign_resources(parent);
pci_bus_add_devices(parent);
+
return 0;
}

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -68,6 +68,52 @@ static void free_failed_list(struct reso
head->next = NULL;
}

+static void pdev_assign_resources_sorted(struct pci_dev *dev,
+ struct resource_list *fail_head)
+{
+ struct resource *res;
+ struct resource_list head, *list, *tmp;
+ int idx;
+ u16 class = dev->class >> 8;
+
+ head.next = NULL;
+
+ /* Don't touch classless devices or host bridges or ioapics. */
+ if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
+ return;
+
+ /* Don't touch ioapic devices already enabled by firmware */
+ if (class == PCI_CLASS_SYSTEM_PIC) {
+ u16 command;
+ pci_read_config_word(dev, PCI_COMMAND, &command);
+ if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
+ return;
+ }
+
+ pdev_sort_resources(dev, &head);
+
+ for (list = head.next; list;) {
+ res = list->res;
+ idx = res - &list->dev->resource[0];
+ if (pci_assign_resource(list->dev, idx)) {
+ if (fail_head && !list->dev->subordinate &&
+ !pci_is_root_bus(list->dev->bus)) {
+ /*
+ * device need to keep flags and size
+ * for second try
+ */
+ add_to_failed_list(fail_head, list->dev, res);
+ } else {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
+ }
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+}
static void pbus_assign_resources_sorted(const struct pci_bus *bus,
struct resource_list *fail_head)
{
@@ -282,9 +328,6 @@ static void __pci_setup_bridge(struct pc
{
struct pci_dev *bridge = bus->self;

- if (pci_is_enabled(bridge))
- return;
-
dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
bus->secondary, bus->subordinate);

@@ -645,7 +688,8 @@ static void __ref __pci_bus_assign_resou

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
- pci_setup_bridge(b);
+ if (!pci_is_enabled(dev))
+ pci_setup_bridge(b);
break;

case PCI_CLASS_BRIDGE_CARDBUS:
@@ -666,6 +710,34 @@ void __ref pci_bus_assign_resources(cons
}
EXPORT_SYMBOL(pci_bus_assign_resources);

+static void __ref __pci_bridge_assign_resources(const struct pci_dev *bridge,
+ struct resource_list *fail_head)
+{
+ struct pci_bus *b;
+
+ pdev_assign_resources_sorted((struct pci_dev *)bridge, fail_head);
+
+ b = bridge->subordinate;
+ if (!b)
+ return;
+
+ __pci_bus_assign_resources(b, fail_head);
+
+ switch (bridge->class >> 8) {
+ case PCI_CLASS_BRIDGE_PCI:
+ pci_setup_bridge(b);
+ break;
+
+ case PCI_CLASS_BRIDGE_CARDBUS:
+ pci_setup_cardbus(b);
+ break;
+
+ default:
+ dev_info(&bridge->dev, "not setting up bridge for bus "
+ "%04x:%02x\n", pci_domain_nr(b), b->number);
+ break;
+ }
+}
static void release_children_resource(struct resource *r)
{
struct resource *p;
@@ -841,3 +913,53 @@ enable_and_dump:
pci_bus_dump_resources(bus);
}
}
+
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
+{
+ struct pci_bus *bus;
+ struct pci_bus *parent = bridge->subordinate;
+ bool second_tried = false;
+ struct resource_list head, *list, *tmp;
+ int retval;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+again:
+ head.next = NULL;
+
+ pci_bus_size_bridges(parent);
+ pci_clear_master(bridge);
+ __pci_bridge_assign_resources(bridge, &head);
+ retval = pci_reenable_device(bridge);
+ pci_set_master(bridge);
+ pci_enable_bridges(parent);
+
+ /* any device complain? */
+ if (!head.next)
+ return;
+
+ if (second_tried) {
+ /* still fail, don't want to try more */
+ free_failed_list(&head);
+ return;
+ }
+
+ second_tried = true;
+ printk(KERN_DEBUG "PCI: second try to assign unassigned res\n");
+
+ /*
+ * Try to release leaf bridge's resources that doesn't fit resource of
+ * child device under that bridge
+ */
+ for (list = head.next; list;) {
+ bus = list->dev->bus;
+ pci_bus_release_unused_bridge_res(bus,
+ list->res->flags & type_mask);
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ goto again;
+}
+EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -768,6 +768,7 @@ void pci_bus_assign_resources(const stru
void pci_bus_size_bridges(struct pci_bus *bus);
int pci_claim_resource(struct pci_dev *, int);
void pci_assign_unassigned_resources(void);
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
void pdev_enable_device(struct pci_dev *);
void pdev_sort_resources(struct pci_dev *, struct resource_list *);
int pci_enable_resources(struct pci_dev *, int mask);

2009-11-10 08:08:13

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v10

Can I ask which is the latest version?

I think -v10 is the latest. But I could not find -v10 for patch 1/2.

Thanks,
Kenji Kaneshige


Yinghai Lu wrote:
> move out bus_size_bridges and assign resources out of pciehp_add_bridge()
> and at last do them all together one time including slot bridge, to avoid to
> call assign resources several times, when there are several bridges under the
> slot bridge.
>
> need to introduce pci_bridge_assign_resources there.
>
> handle the case the slot bridge that doesn't get pre-allocated big enough res
> from FW.
> for example pcie devices need 256M, but the bridge only get preallocated 2M...
>
> pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
> update res is not updated to bridge BAR. that is bridge is enabled already for
> port service.
>
> v2: address Alex's concern about pci remove/rescan feature about
> pci_setup_bridge changes.
> v3: Kenji pointed out that pci_config_slot need to be called before
> pci_bus_add_devices()
> v4: move out pci_is_enabled checkout of pci_setup_bridge()
> v5: change the applying sequence.
> v6: change the functions name according to Jesse
> v8: address Eric's concern, only overwrite leaf bridge resource that is not
> big enough
> v9: refresh to be applied after bjorn's patch, and remove trick about save
> size and restore resource second try.
> v10: alex found need to have export for pci_assign_unassigned_bridge_resources
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> ---
> drivers/pci/hotplug/pciehp_pci.c | 23 +++++-
> drivers/pci/setup-bus.c | 130 +++++++++++++++++++++++++++++++++++++--
> include/linux/pci.h | 1
> 3 files changed, 144 insertions(+), 10 deletions(-)
>
> Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
> +++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
> @@ -53,17 +53,15 @@ static int __ref pciehp_add_bridge(struc
> busnr = pci_scan_bridge(parent, dev, busnr, pass);
> if (!dev->subordinate)
> return -1;
> - pci_bus_size_bridges(dev->subordinate);
> - pci_bus_assign_resources(parent);
> - pci_enable_bridges(parent);
> - pci_bus_add_devices(parent);
> +
> return 0;
> }
>
> int pciehp_configure_device(struct slot *p_slot)
> {
> struct pci_dev *dev;
> - struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
> + struct pci_dev *bridge = p_slot->ctrl->pcie->port;
> + struct pci_bus *parent = bridge->subordinate;
> int num, fn;
> struct controller *ctrl = p_slot->ctrl;
>
> @@ -96,12 +94,25 @@ int pciehp_configure_device(struct slot
> (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
> pciehp_add_bridge(dev);
> }
> + pci_dev_put(dev);
> + }
> +
> + pci_assign_unassigned_bridge_resources(bridge);
> +
> + for (fn = 0; fn < 8; fn++) {
> + dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
> + if (!dev)
> + continue;
> + if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
> + pci_dev_put(dev);
> + continue;
> + }
> pci_configure_slot(dev);
> pci_dev_put(dev);
> }
>
> - pci_bus_assign_resources(parent);
> pci_bus_add_devices(parent);
> +
> return 0;
> }
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -68,6 +68,52 @@ static void free_failed_list(struct reso
> head->next = NULL;
> }
>
> +static void pdev_assign_resources_sorted(struct pci_dev *dev,
> + struct resource_list *fail_head)
> +{
> + struct resource *res;
> + struct resource_list head, *list, *tmp;
> + int idx;
> + u16 class = dev->class >> 8;
> +
> + head.next = NULL;
> +
> + /* Don't touch classless devices or host bridges or ioapics. */
> + if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
> + return;
> +
> + /* Don't touch ioapic devices already enabled by firmware */
> + if (class == PCI_CLASS_SYSTEM_PIC) {
> + u16 command;
> + pci_read_config_word(dev, PCI_COMMAND, &command);
> + if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
> + return;
> + }
> +
> + pdev_sort_resources(dev, &head);
> +
> + for (list = head.next; list;) {
> + res = list->res;
> + idx = res - &list->dev->resource[0];
> + if (pci_assign_resource(list->dev, idx)) {
> + if (fail_head && !list->dev->subordinate &&
> + !pci_is_root_bus(list->dev->bus)) {
> + /*
> + * device need to keep flags and size
> + * for second try
> + */
> + add_to_failed_list(fail_head, list->dev, res);
> + } else {
> + res->start = 0;
> + res->end = 0;
> + res->flags = 0;
> + }
> + }
> + tmp = list;
> + list = list->next;
> + kfree(tmp);
> + }
> +}
> static void pbus_assign_resources_sorted(const struct pci_bus *bus,
> struct resource_list *fail_head)
> {
> @@ -282,9 +328,6 @@ static void __pci_setup_bridge(struct pc
> {
> struct pci_dev *bridge = bus->self;
>
> - if (pci_is_enabled(bridge))
> - return;
> -
> dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
> bus->secondary, bus->subordinate);
>
> @@ -645,7 +688,8 @@ static void __ref __pci_bus_assign_resou
>
> switch (dev->class >> 8) {
> case PCI_CLASS_BRIDGE_PCI:
> - pci_setup_bridge(b);
> + if (!pci_is_enabled(dev))
> + pci_setup_bridge(b);
> break;
>
> case PCI_CLASS_BRIDGE_CARDBUS:
> @@ -666,6 +710,34 @@ void __ref pci_bus_assign_resources(cons
> }
> EXPORT_SYMBOL(pci_bus_assign_resources);
>
> +static void __ref __pci_bridge_assign_resources(const struct pci_dev *bridge,
> + struct resource_list *fail_head)
> +{
> + struct pci_bus *b;
> +
> + pdev_assign_resources_sorted((struct pci_dev *)bridge, fail_head);
> +
> + b = bridge->subordinate;
> + if (!b)
> + return;
> +
> + __pci_bus_assign_resources(b, fail_head);
> +
> + switch (bridge->class >> 8) {
> + case PCI_CLASS_BRIDGE_PCI:
> + pci_setup_bridge(b);
> + break;
> +
> + case PCI_CLASS_BRIDGE_CARDBUS:
> + pci_setup_cardbus(b);
> + break;
> +
> + default:
> + dev_info(&bridge->dev, "not setting up bridge for bus "
> + "%04x:%02x\n", pci_domain_nr(b), b->number);
> + break;
> + }
> +}
> static void release_children_resource(struct resource *r)
> {
> struct resource *p;
> @@ -841,3 +913,53 @@ enable_and_dump:
> pci_bus_dump_resources(bus);
> }
> }
> +
> +void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
> +{
> + struct pci_bus *bus;
> + struct pci_bus *parent = bridge->subordinate;
> + bool second_tried = false;
> + struct resource_list head, *list, *tmp;
> + int retval;
> + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH;
> +
> +again:
> + head.next = NULL;
> +
> + pci_bus_size_bridges(parent);
> + pci_clear_master(bridge);
> + __pci_bridge_assign_resources(bridge, &head);
> + retval = pci_reenable_device(bridge);
> + pci_set_master(bridge);
> + pci_enable_bridges(parent);
> +
> + /* any device complain? */
> + if (!head.next)
> + return;
> +
> + if (second_tried) {
> + /* still fail, don't want to try more */
> + free_failed_list(&head);
> + return;
> + }
> +
> + second_tried = true;
> + printk(KERN_DEBUG "PCI: second try to assign unassigned res\n");
> +
> + /*
> + * Try to release leaf bridge's resources that doesn't fit resource of
> + * child device under that bridge
> + */
> + for (list = head.next; list;) {
> + bus = list->dev->bus;
> + pci_bus_release_unused_bridge_res(bus,
> + list->res->flags & type_mask);
> + tmp = list;
> + list = list->next;
> + kfree(tmp);
> + }
> +
> + goto again;
> +}
> +EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
> Index: linux-2.6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pci.h
> +++ linux-2.6/include/linux/pci.h
> @@ -768,6 +768,7 @@ void pci_bus_assign_resources(const stru
> void pci_bus_size_bridges(struct pci_bus *bus);
> int pci_claim_resource(struct pci_dev *, int);
> void pci_assign_unassigned_resources(void);
> +void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
> void pdev_enable_device(struct pci_dev *);
> void pdev_sort_resources(struct pci_dev *, struct resource_list *);
> int pci_enable_resources(struct pci_dev *, int mask);
>
>

2009-11-10 09:49:35

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v10

Kenji Kaneshige wrote:
> Can I ask which is the latest version?
>
> I think -v10 is the latest. But I could not find -v10 for patch 1/2.

please use

http://patchwork.kernel.org/patch/57814/ 1/2 -v9
http://patchwork.kernel.org/patch/58294/ 2/2 -v10

Thanks

Yinghai

2009-11-13 06:08:51

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v10

00000000-0009efff : System RAM
0009f000-0009ffff : ACPI Non-volatile Storage
00100000-79190fff : System RAM
01000000-0141e142 : Kernel code
0141e143-0183639f : Kernel data
01acb000-027006f7 : Kernel bss
79191000-791c0fff : reserved
791c1000-79207fff : System RAM
79208000-79214fff : ACPI Tables
79215000-79321fff : System RAM
79322000-79322fff : reserved
79323000-7934afff : System RAM
7934b000-7934bfff : ACPI Tables
7934c000-793dffff : System RAM
793e0000-793e1fff : ACPI Tables
793e2000-79401fff : System RAM
79402000-79404fff : ACPI Tables
79405000-79450fff : System RAM
79451000-79451fff : ACPI Tables
79452000-79452fff : ACPI Non-volatile Storage
79453000-799e6fff : System RAM
799e7000-799e9fff : reserved
799ea000-799eefff : System RAM
799ef000-799f1fff : reserved
799f2000-79a30fff : System RAM
79a31000-79a34fff : reserved
79a35000-79a3cfff : System RAM
79a3d000-79a5ffff : reserved
79a60000-79aa1fff : System RAM
79aa2000-7a2b2fff : ACPI Non-volatile Storage
7a2b3000-7a302fff : System RAM
7a303000-7a303fff : reserved
7a304000-7a31dfff : System RAM
7a31e000-7a31efff : ACPI Tables
7a31f000-7a320fff : ACPI Non-volatile Storage
7a321000-7a3a4fff : System RAM
7a3a5000-7a3a7fff : reserved
7a3a8000-7a3a8fff : ACPI Tables
7a3a9000-7a3b4fff : System RAM
7a3b5000-7a3d4fff : reserved
7a3d5000-7a43dfff : System RAM
7a43e000-7a443fff : reserved
7a444000-7a4a1fff : System RAM
7a4a2000-7a4a5fff : reserved
7a4a6000-7a4f3fff : System RAM
7a4f4000-7a4fbfff : reserved
7a4fc000-7a514fff : System RAM
7a515000-7a515fff : ACPI Tables
7a516000-7a553fff : System RAM
7a554000-7a557fff : reserved
7a558000-7a9a9fff : System RAM
7a9aa000-7a9aafff : reserved
7a9ab000-7a9e3fff : System RAM
7a9e4000-7a9e7fff : reserved
7a9e8000-7aad4fff : System RAM
7aad5000-7aad5fff : reserved
7aad6000-7af3efff : System RAM
7af3f000-7af3ffff : reserved
7af40000-7af44fff : System RAM
7af45000-7af45fff : reserved
7af46000-7effffff : System RAM
7f000000-7fffffff : RAM buffer
80000000-8fffffff : PCI MMCONFIG 0 [00-ff]
80000000-8fffffff : reserved
90000000-fbffffff : PCI Bus #00
90000000-90ffffff : PCI Bus 0000:19
90000000-90ffffff : 0000:19:00.0
90000000-907fffff : efifb
91000000-917fffff : PCI Bus 0000:03
91000000-917fffff : PCI Bus 0000:04
91000000-917fffff : PCI Bus 0000:0c
91000000-917fffff : PCI Bus 0000:0d
91000000-911fffff : PCI Bus 0000:11
91000000-9103ffff : 0000:11:00.0
91200000-913fffff : PCI Bus 0000:10
91400000-915fffff : PCI Bus 0000:0f
91400000-9141ffff : 0000:0f:00.0
91420000-9143ffff : 0000:0f:00.1
91600000-917fffff : PCI Bus 0000:0e
c6000000-c68fffff : PCI Bus 0000:19
c6000000-c67fffff : 0000:19:00.0
c6800000-c6803fff : 0000:19:00.0
c6810000-c681ffff : 0000:19:00.0
c6a00000-c79fffff : PCI Bus 0000:03
c6a00000-c79fffff : PCI Bus 0000:04
c6a00000-c75fffff : PCI Bus 0000:0c
c6a00000-c75fffff : PCI Bus 0000:0d
c6a00000-c6cfffff : PCI Bus 0000:11
c6c00000-c6c03fff : 0000:11:00.0
c6c00000-c6c03fff : lpfc
c6c04000-c6c04fff : 0000:11:00.0
c6c04000-c6c04fff : lpfc
c6e00000-c6ffffff : PCI Bus 0000:10
c7000000-c72fffff : PCI Bus 0000:0f
c7200000-c721ffff : 0000:0f:00.1
c7200000-c721ffff : e1000e
c7220000-c723ffff : 0000:0f:00.1
c7220000-c723ffff : e1000e
c7240000-c725ffff : 0000:0f:00.0
c7240000-c725ffff : e1000e
c7260000-c727ffff : 0000:0f:00.0
c7260000-c727ffff : e1000e
c7400000-c75fffff : PCI Bus 0000:0e
c7600000-c79fffff : PCI Bus 0000:05
c7600000-c79fffff : PCI Bus 0000:06
c7600000-c76fffff : PCI Bus 0000:0a
c7600000-c760ffff : 0000:0a:00.0
c7600000-c760ffff : mpt
c7610000-c7613fff : 0000:0a:00.0
c7610000-c7613fff : mpt
c7700000-c77fffff : PCI Bus 0000:09
c7700000-c770ffff : 0000:09:00.0
c7700000-c770ffff : mpt
c7710000-c7713fff : 0000:09:00.0
c7710000-c7713fff : mpt
c7800000-c79fffff : PCI Bus 0000:09
c7800000-c79fffff : 0000:09:00.0
c7a00000-c7a1ffff : 0000:00:19.0
c7a00000-c7a1ffff : e1000e
c7a20000-c7a23fff : 0000:00:16.0
c7a20000-c7a23fff : ioatdma
c7a24000-c7a27fff : 0000:00:16.1
c7a24000-c7a27fff : ioatdma
c7a28000-c7a2bfff : 0000:00:16.2
c7a28000-c7a2bfff : ioatdma
c7a2c000-c7a2ffff : 0000:00:16.3
c7a2c000-c7a2ffff : ioatdma
c7a30000-c7a33fff : 0000:00:16.4
c7a30000-c7a33fff : ioatdma
c7a34000-c7a37fff : 0000:00:16.5
c7a34000-c7a37fff : ioatdma
c7a38000-c7a3bfff : 0000:00:16.6
c7a38000-c7a3bfff : ioatdma
c7a3c000-c7a3ffff : 0000:00:16.7
c7a3c000-c7a3ffff : ioatdma
c7a40000-c7a40fff : 0000:00:1f.6
c7a41000-c7a41fff : 0000:00:19.0
c7a41000-c7a41fff : e1000e
c7a42000-c7a42fff : 0000:00:13.0
c7a43000-c7a433ff : 0000:00:1d.7
c7a43000-c7a433ff : ehci_hcd
c7a43400-c7a437ff : 0000:00:1a.7
c7a43400-c7a437ff : ehci_hcd
c7a43800-c7a438ff : 0000:00:1f.3
fec00000-fec00fff : IOAPIC 0
fee00000-fee00fff : Local APIC
100000000-47fffffff : System RAM
fc000000000-fc07fffffff : PCI Bus #00


Attachments:
iomem-default.txt (6.21 kB)
iomem-yinghai.txt (5.12 kB)
Download all attachments

2009-11-13 06:27:45

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v10

Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> Can I ask which is the latest version?
>>>
>>> I think -v10 is the latest. But I could not find -v10 for patch 1/2.
>>
>> please use
>>
>> http://patchwork.kernel.org/patch/57814/ 1/2 -v9
>> http://patchwork.kernel.org/patch/58294/ 2/2 -v10
>>
>
> Regardless of PCIe hotplug, some of PCIe devices (not on hotplug
> slots) doesn't work with your patches by the following error.
>
> Copyright (c) 2007-2009 Intel Corporation.
> igb 0000:07:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
> igb 0000:07:00.0: setting latency timer to 64
> igb 0000:07:00.0: PCI INT A disabled
> igb: probe of 0000:07:00.0 failed with error -5
> igb 0000:07:00.1: PCI INT B -> GSI 17 (level, low) -> IRQ 17
> igb 0000:07:00.1: setting latency timer to 64
> igb 0000:07:00.1: PCI INT B disabled
> igb: probe of 0000:07:00.1 failed with error -5
> igb 0000:08:00.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
> igb 0000:08:00.0: setting latency timer to 64
> igb 0000:08:00.0: PCI INT A disabled
> igb: probe of 0000:08:00.0 failed with error -5
> igb 0000:08:00.1: PCI INT B -> GSI 18 (level, low) -> IRQ 18
> igb 0000:08:00.1: setting latency timer to 64
> igb 0000:08:00.1: PCI INT B disabled
> igb: probe of 0000:08:00.1 failed with error -5
>
> I'm using Jesse's latest linux-next.
> I'm attaching the /proc/iomem outputs. The iomem-default.txt is
> /proc/iomem output without your patches. The iomem-yinghai.txt
> is /proc/iomem output with your patches.

can you post whole bootlog with pci debug enabled?

Thanks

Yinghai

2009-11-13 08:33:53

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v10

Yinghai Lu wrote:
> Kenji Kaneshige wrote:
>> Yinghai Lu wrote:
>>> Kenji Kaneshige wrote:
>>>> Can I ask which is the latest version?
>>>>
>>>> I think -v10 is the latest. But I could not find -v10 for patch 1/2.
>>> please use
>>>
>>> http://patchwork.kernel.org/patch/57814/ 1/2 -v9
>>> http://patchwork.kernel.org/patch/58294/ 2/2 -v10
>>>
>> Regardless of PCIe hotplug, some of PCIe devices (not on hotplug
>> slots) doesn't work with your patches by the following error.
>>
>> Copyright (c) 2007-2009 Intel Corporation.
>> igb 0000:07:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
>> igb 0000:07:00.0: setting latency timer to 64
>> igb 0000:07:00.0: PCI INT A disabled
>> igb: probe of 0000:07:00.0 failed with error -5
>> igb 0000:07:00.1: PCI INT B -> GSI 17 (level, low) -> IRQ 17
>> igb 0000:07:00.1: setting latency timer to 64
>> igb 0000:07:00.1: PCI INT B disabled
>> igb: probe of 0000:07:00.1 failed with error -5
>> igb 0000:08:00.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
>> igb 0000:08:00.0: setting latency timer to 64
>> igb 0000:08:00.0: PCI INT A disabled
>> igb: probe of 0000:08:00.0 failed with error -5
>> igb 0000:08:00.1: PCI INT B -> GSI 18 (level, low) -> IRQ 18
>> igb 0000:08:00.1: setting latency timer to 64
>> igb 0000:08:00.1: PCI INT B disabled
>> igb: probe of 0000:08:00.1 failed with error -5
>>
>> I'm using Jesse's latest linux-next.
>> I'm attaching the /proc/iomem outputs. The iomem-default.txt is
>> /proc/iomem output without your patches. The iomem-yinghai.txt
>> is /proc/iomem output with your patches.
>
> can you post whole bootlog with pci debug enabled?
>

I'll send it privately.

Thanks,
Kenji Kaneshige

2009-11-14 08:51:42

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11


v2: Jesse doesn't like it is in find_free_bus_resource...
try to move out of pci_bus_size_bridges loop.
v3: add pci_setup_bridge calling after pci_bridge_release_not_used_res.
only clear release those res for x86.
v4: Bjorn want to release use dev instead of bus.
v5: Kenji pointed out it will have problem with several level bridge.
so let only handle leaf bridge.
v6: address Kenji's request (new pci_bus_release...). and change applying order
move back release to pci_assign_unassigned_resource
v7: change functions name pci_bus_release_unused_bridge_res according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not big
enough need to do it in two steps, and first step recore the failed res,
and don't touch bridge res that programmed by firmware. second step will
try to release bridge resource that is too small at first.
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v11:add pci=try=5, about more try to change more bridge

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/pci.c | 5
drivers/pci/pci.h | 2
drivers/pci/setup-bus.c | 304 +++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 292 insertions(+), 19 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -27,7 +27,49 @@
#include <linux/slab.h>
#include "pci.h"

-static void pbus_assign_resources_sorted(const struct pci_bus *bus)
+
+static void add_to_failed_list(struct resource_list *head, struct pci_dev *dev,
+ struct resource *res)
+{
+ struct resource_list *list = head;
+ struct resource_list *ln = list->next;
+ struct resource_list *tmp;
+
+ tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp) {
+ pr_warning("add_to_failed_list: kmalloc() failed!\n");
+ return;
+ }
+
+ tmp->next = ln;
+ tmp->res = res;
+ tmp->dev = dev;
+ list->next = tmp;
+}
+
+static void free_failed_list(struct resource_list *head)
+{
+ struct resource_list *list, *tmp;
+ struct resource *res;
+ /*
+ * Try to release leaf bridge's resources that there is no child
+ * under it
+ */
+ for (list = head->next; list;) {
+ res = list->res;
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ head->next = NULL;
+}
+
+static void pbus_assign_resources_sorted(const struct pci_bus *bus,
+ struct resource_list *fail_head)
{
struct pci_dev *dev;
struct resource *res;
@@ -58,9 +100,17 @@ static void pbus_assign_resources_sorted
res = list->res;
idx = res - &list->dev->resource[0];
if (pci_assign_resource(list->dev, idx)) {
- res->start = 0;
- res->end = 0;
- res->flags = 0;
+ if (fail_head && !pci_is_root_bus(list->dev->bus)) {
+ /*
+ * device need to keep flags and size
+ * for next try
+ */
+ add_to_failed_list(fail_head, list->dev, res);
+ } else {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
}
tmp = list;
list = list->next;
@@ -134,19 +184,12 @@ EXPORT_SYMBOL(pci_setup_cardbus);
config space writes, so it's quite possible that an I/O window of
the bridge will have some undesirable address (e.g. 0) after the
first write. Ditto 64-bit prefetchable MMIO. */
-static void pci_setup_bridge(struct pci_bus *bus)
+static void pci_setup_bridge_io(struct pci_bus *bus)
{
struct pci_dev *bridge = bus->self;
struct resource *res;
struct pci_bus_region region;
- u32 l, bu, lu, io_upper16;
- int pref_mem64;
-
- if (pci_is_enabled(bridge))
- return;
-
- dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
- bus->secondary, bus->subordinate);
+ u32 l, io_upper16;

/* Set up the top and bottom of the PCI I/O segment for this bus. */
res = bus->resource[0];
@@ -172,7 +215,13 @@ static void pci_setup_bridge(struct pci_
pci_write_config_dword(bridge, PCI_IO_BASE, l);
/* Update upper 16 bits of I/O base/limit. */
pci_write_config_dword(bridge, PCI_IO_BASE_UPPER16, io_upper16);
-
+}
+static void pci_setup_bridge_mmio(struct pci_bus *bus)
+{
+ struct pci_dev *bridge = bus->self;
+ struct resource *res;
+ struct pci_bus_region region;
+ u32 l;
/* Set up the top and bottom of the PCI Memory segment
for this bus. */
res = bus->resource[1];
@@ -187,6 +236,14 @@ static void pci_setup_bridge(struct pci_
dev_info(&bridge->dev, " bridge window [mem disabled]\n");
}
pci_write_config_dword(bridge, PCI_MEMORY_BASE, l);
+}
+static void pci_setup_bridge_mmio_pref(struct pci_bus *bus)
+{
+ struct pci_dev *bridge = bus->self;
+ struct resource *res;
+ struct pci_bus_region region;
+ u32 l, bu, lu;
+ int pref_mem64;

/* Clear out the upper 32 bits of PREF limit.
If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
@@ -219,10 +276,37 @@ static void pci_setup_bridge(struct pci_
pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
}
+}
+static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type)
+{
+ struct pci_dev *bridge = bus->self;
+
+ if (pci_is_enabled(bridge))
+ return;
+
+ dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
+ bus->secondary, bus->subordinate);
+
+ if (type & IORESOURCE_IO)
+ pci_setup_bridge_io(bus);
+
+ if (type & IORESOURCE_MEM)
+ pci_setup_bridge_mmio(bus);
+
+ if (type & IORESOURCE_PREFETCH)
+ pci_setup_bridge_mmio_pref(bus);

pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
}

+static void pci_setup_bridge(struct pci_bus *bus)
+{
+ unsigned long type = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+ __pci_setup_bridge(bus, type);
+}
+
/* Check whether the bridge supports optional I/O and
prefetchable memory ranges. If not, the respective
base/limit registers must be read-only and read as 0. */
@@ -543,19 +627,20 @@ void __ref pci_bus_size_bridges(struct p
}
EXPORT_SYMBOL(pci_bus_size_bridges);

-void __ref pci_bus_assign_resources(const struct pci_bus *bus)
+static void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
+ struct resource_list *fail_head)
{
struct pci_bus *b;
struct pci_dev *dev;

- pbus_assign_resources_sorted(bus);
+ pbus_assign_resources_sorted(bus, fail_head);

list_for_each_entry(dev, &bus->devices, bus_list) {
b = dev->subordinate;
if (!b)
continue;

- pci_bus_assign_resources(b);
+ __pci_bus_assign_resources(b, fail_head);

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
@@ -573,15 +658,130 @@ void __ref pci_bus_assign_resources(cons
}
}
}
+
+void __ref pci_bus_assign_resources(const struct pci_bus *bus)
+{
+ __pci_bus_assign_resources(bus, NULL);
+}
EXPORT_SYMBOL(pci_bus_assign_resources);

+static void release_children_resource(struct resource *r)
+{
+ struct resource *p;
+ resource_size_t size;
+
+ p = r->child;
+ while (p) {
+ release_children_resource(p);
+ release_resource(p);
+ printk(KERN_DEBUG "PCI: release child resource %pRt\n", p);
+ /* need to restore size, and keep flags */
+ size = resource_size(p);
+ p->start = 0;
+ p->end = size - 1;
+ p = r->child;
+ }
+}
+
+static void pci_bridge_release_unused_res(struct pci_bus *bus,
+ unsigned long type)
+{
+ int idx;
+ bool changed = false;
+ struct pci_dev *dev;
+ struct resource *r;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+ /* for pci bridges res only */
+ dev = bus->self;
+ for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_BRIDGE_RESOURCES + 3;
+ idx++) {
+ r = &dev->resource[idx];
+ if ((r->flags & type_mask) != type)
+ continue;
+ if (!r->parent)
+ continue;
+ /*
+ * if there are children under that, we should release them
+ * all
+ */
+ release_children_resource(r);
+ if (!release_resource(r)) {
+ dev_printk(KERN_DEBUG, &dev->dev,
+ "resource %d %pRt released\n", idx, r);
+ r->flags = 0;
+ changed = true;
+ }
+ }
+
+ if (changed) {
+ if (type & IORESOURCE_PREFETCH) {
+ /* avoiding touch the one without PREF */
+ type = IORESOURCE_PREFETCH;
+ }
+ __pci_setup_bridge(bus, type);
+ }
+}
+
+/*
+ * try to release pci bridge resources that is from leaf bridge,
+ * so we can allocate big new one later
+ * check:
+ * 0: only release the bridge and only the bridge is leaf
+ * 1: release all down side bridge for third shoot
+ */
+static void __ref pci_bus_release_unused_bridge_res(struct pci_bus *bus,
+ unsigned long type,
+ int check_leaf)
+{
+ struct pci_dev *dev;
+ bool is_leaf_bridge = true;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct pci_bus *b = dev->subordinate;
+ if (!b)
+ continue;
+
+ switch (dev->class >> 8) {
+ case PCI_CLASS_BRIDGE_CARDBUS:
+ is_leaf_bridge = false;
+ break;
+
+ case PCI_CLASS_BRIDGE_PCI:
+ default:
+ is_leaf_bridge = false;
+ if (!check_leaf)
+ pci_bus_release_unused_bridge_res(b, type,
+ check_leaf);
+ break;
+ }
+ }
+
+ /* The root bus? */
+ if (!bus->self)
+ return;
+
+ switch (bus->self->class >> 8) {
+ case PCI_CLASS_BRIDGE_CARDBUS:
+ break;
+
+ case PCI_CLASS_BRIDGE_PCI:
+ default:
+ if ((check_leaf && is_leaf_bridge) || !check_leaf)
+ pci_bridge_release_unused_res(bus, type);
+ break;
+ }
+}
+
static void pci_bus_dump_res(struct pci_bus *bus)
{
int i;

for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
struct resource *res = bus->resource[i];
- if (!res || !res->end)
+
+ if (!res || !res->end || !res->flags)
continue;

dev_printk(KERN_DEBUG, &bus->dev, "resource %d %pR\n", i, res);
@@ -605,10 +805,25 @@ static void pci_bus_dump_resources(struc
}
}

+/*
+ * first try will not touch pci bridge res
+ * second try will clear small leaf bridge res
+ * third try will clear related bridge: some aggressive
+ */
+/* assume we only have 4 level bridges, so only try 5 times */
+int pci_try_num = 5;
void __init
pci_assign_unassigned_resources(void)
{
struct pci_bus *bus;
+ int tried_times = 0;
+ int check_leaf = 1;
+ struct resource_list head, *list, *tmp;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+ unsigned long failed_type;
+again:
+ head.next = NULL;

/* Depth first, calculate sizes and alignments of all
subordinate buses. */
@@ -617,7 +832,58 @@ pci_assign_unassigned_resources(void)
}
/* Depth last, allocate resources and update the hardware. */
list_for_each_entry(bus, &pci_root_buses, node) {
- pci_bus_assign_resources(bus);
+ __pci_bus_assign_resources(bus, &head);
+ }
+ tried_times++;
+
+ /* any device complain? */
+ if (!head.next)
+ goto enable_and_dump;
+ failed_type = 0;
+ for (list = head.next; list;) {
+ unsigned long flags = list->res->flags;
+
+ failed_type |= flags;
+ list = list->next;
+ }
+ /*
+ * io port are tight, don't try extra
+ * or if reach the limit, don't want to try more
+ */
+ failed_type &= type_mask;
+ if ((failed_type == IORESOURCE_IO) || (tried_times >= pci_try_num)) {
+ free_failed_list(&head);
+ goto enable_and_dump;
+ }
+
+ printk(KERN_DEBUG "PCI: No. %d try to assign unassigned res\n",
+ tried_times + 1);
+
+ /*
+ * Try to release leaf bridge's resources that doesn't fit resource of
+ * child device under that bridge
+ */
+ /* third times and later will not check if it is leaf */
+ if ((tried_times + 1) > 2)
+ check_leaf = 0;
+ for (list = head.next; list;) {
+ unsigned long flags = list->res->flags;
+
+ bus = list->dev->bus;
+ if (list->dev->subordinate)
+ list->res->flags = 0;
+ pci_bus_release_unused_bridge_res(bus, flags & type_mask,
+ check_leaf);
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ goto again;
+
+enable_and_dump:
+ /* Depth last, update the hardware. */
+ list_for_each_entry(bus, &pci_root_buses, node) {
pci_enable_bridges(bus);
}

Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -2779,6 +2779,11 @@ static int __init pci_setup(char *str)
pci_no_aer();
} else if (!strcmp(str, "nodomains")) {
pci_no_domains();
+ } else if (!strncmp(str, "try=", 4)) {
+ int try_num = memparse(str + 4, &str);
+
+ if (try_num > 0 && try_num < 10)
+ pci_try_num = try_num;
} else if (!strncmp(str, "cbiosize=", 9)) {
pci_cardbus_io_size = memparse(str + 9, &str);
} else if (!strncmp(str, "cbmemsize=", 10)) {
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -203,6 +203,8 @@ static inline int pci_ari_enabled(struct
return bus->self && bus->self->ari_enabled;
}

+extern int pci_try_num;
+
#ifdef CONFIG_PCI_QUIRKS
extern int pci_is_reassigndev(struct pci_dev *dev);
resource_size_t pci_specified_resource_alignment(struct pci_dev *dev);

2009-11-14 08:52:20

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 2/2] pci: pciehp update the slot bridge res to get big range for pcie devices - v11


move out bus_size_bridges and assign resources out of pciehp_add_bridge()
and at last do them all together one time including slot bridge, to avoid to
call assign resources several times, when there are several bridges under the
slot bridge.

need to introduce pci_bridge_assign_resources there.

handle the case the slot bridge that doesn't get pre-allocated big enough res
from FW.
for example pcie devices need 256M, but the bridge only get preallocated 2M...

pci_setup_bridge() will take extra check_enabled for the slot bridge, otherwise
update res is not updated to bridge BAR. that is bridge is enabled already for
port service.

v2: address Alex's concern about pci remove/rescan feature about
pci_setup_bridge changes.
v3: Kenji pointed out that pci_config_slot need to be called before
pci_bus_add_devices()
v4: move out pci_is_enabled checkout of pci_setup_bridge()
v5: change the applying sequence.
v6: change the functions name according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not
big enough
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v10:alex found need to have export for pci_assign_unassigned_bridge_resources
v11: pass check_leaf with pci_bus_release_unused_bridge_res

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/hotplug/pciehp_pci.c | 23 ++++--
drivers/pci/setup-bus.c | 133 +++++++++++++++++++++++++++++++++++++--
include/linux/pci.h | 1
3 files changed, 147 insertions(+), 10 deletions(-)

Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
@@ -53,17 +53,15 @@ static int __ref pciehp_add_bridge(struc
busnr = pci_scan_bridge(parent, dev, busnr, pass);
if (!dev->subordinate)
return -1;
- pci_bus_size_bridges(dev->subordinate);
- pci_bus_assign_resources(parent);
- pci_enable_bridges(parent);
- pci_bus_add_devices(parent);
+
return 0;
}

int pciehp_configure_device(struct slot *p_slot)
{
struct pci_dev *dev;
- struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
+ struct pci_dev *bridge = p_slot->ctrl->pcie->port;
+ struct pci_bus *parent = bridge->subordinate;
int num, fn;
struct controller *ctrl = p_slot->ctrl;

@@ -96,12 +94,25 @@ int pciehp_configure_device(struct slot
(dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
pciehp_add_bridge(dev);
}
+ pci_dev_put(dev);
+ }
+
+ pci_assign_unassigned_bridge_resources(bridge);
+
+ for (fn = 0; fn < 8; fn++) {
+ dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
+ if (!dev)
+ continue;
+ if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+ pci_dev_put(dev);
+ continue;
+ }
pci_configure_slot(dev);
pci_dev_put(dev);
}

- pci_bus_assign_resources(parent);
pci_bus_add_devices(parent);
+
return 0;
}

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -68,6 +68,51 @@ static void free_failed_list(struct reso
head->next = NULL;
}

+static void pdev_assign_resources_sorted(struct pci_dev *dev,
+ struct resource_list *fail_head)
+{
+ struct resource *res;
+ struct resource_list head, *list, *tmp;
+ int idx;
+ u16 class = dev->class >> 8;
+
+ head.next = NULL;
+
+ /* Don't touch classless devices or host bridges or ioapics. */
+ if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
+ return;
+
+ /* Don't touch ioapic devices already enabled by firmware */
+ if (class == PCI_CLASS_SYSTEM_PIC) {
+ u16 command;
+ pci_read_config_word(dev, PCI_COMMAND, &command);
+ if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
+ return;
+ }
+
+ pdev_sort_resources(dev, &head);
+
+ for (list = head.next; list;) {
+ res = list->res;
+ idx = res - &list->dev->resource[0];
+ if (pci_assign_resource(list->dev, idx)) {
+ if (fail_head && !pci_is_root_bus(list->dev->bus)) {
+ /*
+ * device need to keep flags and size
+ * for second try
+ */
+ add_to_failed_list(fail_head, list->dev, res);
+ } else {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
+ }
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+}
static void pbus_assign_resources_sorted(const struct pci_bus *bus,
struct resource_list *fail_head)
{
@@ -281,9 +326,6 @@ static void __pci_setup_bridge(struct pc
{
struct pci_dev *bridge = bus->self;

- if (pci_is_enabled(bridge))
- return;
-
dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
bus->secondary, bus->subordinate);

@@ -644,7 +686,8 @@ static void __ref __pci_bus_assign_resou

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
- pci_setup_bridge(b);
+ if (!pci_is_enabled(dev))
+ pci_setup_bridge(b);
break;

case PCI_CLASS_BRIDGE_CARDBUS:
@@ -665,6 +708,34 @@ void __ref pci_bus_assign_resources(cons
}
EXPORT_SYMBOL(pci_bus_assign_resources);

+static void __ref __pci_bridge_assign_resources(const struct pci_dev *bridge,
+ struct resource_list *fail_head)
+{
+ struct pci_bus *b;
+
+ pdev_assign_resources_sorted((struct pci_dev *)bridge, fail_head);
+
+ b = bridge->subordinate;
+ if (!b)
+ return;
+
+ __pci_bus_assign_resources(b, fail_head);
+
+ switch (bridge->class >> 8) {
+ case PCI_CLASS_BRIDGE_PCI:
+ pci_setup_bridge(b);
+ break;
+
+ case PCI_CLASS_BRIDGE_CARDBUS:
+ pci_setup_cardbus(b);
+ break;
+
+ default:
+ dev_info(&bridge->dev, "not setting up bridge for bus "
+ "%04x:%02x\n", pci_domain_nr(b), b->number);
+ break;
+ }
+}
static void release_children_resource(struct resource *r)
{
struct resource *p;
@@ -892,3 +963,57 @@ enable_and_dump:
pci_bus_dump_resources(bus);
}
}
+
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
+{
+ struct pci_bus *bus;
+ struct pci_bus *parent = bridge->subordinate;
+ bool second_tried = false;
+ struct resource_list head, *list, *tmp;
+ int retval;
+ unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH;
+
+again:
+ head.next = NULL;
+
+ pci_bus_size_bridges(parent);
+ pci_clear_master(bridge);
+ __pci_bridge_assign_resources(bridge, &head);
+ retval = pci_reenable_device(bridge);
+ pci_set_master(bridge);
+ pci_enable_bridges(parent);
+
+ /* any device complain? */
+ if (!head.next)
+ return;
+
+ if (second_tried) {
+ /* still fail, don't want to try more */
+ free_failed_list(&head);
+ return;
+ }
+
+ second_tried = true;
+ printk(KERN_DEBUG "PCI: second try to assign unassigned res\n");
+
+ /*
+ * Try to release leaf bridge's resources that doesn't fit resource of
+ * child device under that bridge
+ */
+ for (list = head.next; list;) {
+ unsigned long flags = list->res->flags;
+
+ bus = list->dev->bus;
+ if (list->dev->subordinate)
+ list->res->flags = 0;
+ pci_bus_release_unused_bridge_res(bus, flags & type_mask, 0);
+
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ goto again;
+}
+EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -768,6 +768,7 @@ void pci_bus_assign_resources(const stru
void pci_bus_size_bridges(struct pci_bus *bus);
int pci_claim_resource(struct pci_dev *, int);
void pci_assign_unassigned_resources(void);
+void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
void pdev_enable_device(struct pci_dev *);
void pdev_sort_resources(struct pci_dev *, struct resource_list *);
int pci_enable_resources(struct pci_dev *, int mask);

2009-11-24 01:09:17

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Hi,

I tried v11 patches. This version seems to fix the problem I
reported against previous version.

I have no objection against the idea of resource allocation
changes for PCI express hotplug slots.

But I still have concern about changing resource allocation for
other than PCI express hotplug slots. For example, some hotplug
controller other than PCI express can have multiple slots under
the same bus. If some hotplug slots are occupied and the others
are empty at the boot time, I think your code try to shrink the
bus resources for hotplug slots allocated by BIOS. It would break
the hot-add on the empty slots due to the resource allocation
failure.

Thanks,
Kenji Kaneshige



Yinghai Lu wrote:
> v2: Jesse doesn't like it is in find_free_bus_resource...
> try to move out of pci_bus_size_bridges loop.
> v3: add pci_setup_bridge calling after pci_bridge_release_not_used_res.
> only clear release those res for x86.
> v4: Bjorn want to release use dev instead of bus.
> v5: Kenji pointed out it will have problem with several level bridge.
> so let only handle leaf bridge.
> v6: address Kenji's request (new pci_bus_release...). and change applying order
> move back release to pci_assign_unassigned_resource
> v7: change functions name pci_bus_release_unused_bridge_res according to Jesse
> v8: address Eric's concern, only overwrite leaf bridge resource that is not big
> enough need to do it in two steps, and first step recore the failed res,
> and don't touch bridge res that programmed by firmware. second step will
> try to release bridge resource that is too small at first.
> v9: refresh to be applied after bjorn's patch, and remove trick about save
> size and restore resource second try.
> v11:add pci=try=5, about more try to change more bridge
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> ---
> drivers/pci/pci.c | 5
> drivers/pci/pci.h | 2
> drivers/pci/setup-bus.c | 304 +++++++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 292 insertions(+), 19 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -27,7 +27,49 @@
> #include <linux/slab.h>
> #include "pci.h"
>
> -static void pbus_assign_resources_sorted(const struct pci_bus *bus)
> +
> +static void add_to_failed_list(struct resource_list *head, struct pci_dev *dev,
> + struct resource *res)
> +{
> + struct resource_list *list = head;
> + struct resource_list *ln = list->next;
> + struct resource_list *tmp;
> +
> + tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
> + if (!tmp) {
> + pr_warning("add_to_failed_list: kmalloc() failed!\n");
> + return;
> + }
> +
> + tmp->next = ln;
> + tmp->res = res;
> + tmp->dev = dev;
> + list->next = tmp;
> +}
> +
> +static void free_failed_list(struct resource_list *head)
> +{
> + struct resource_list *list, *tmp;
> + struct resource *res;
> + /*
> + * Try to release leaf bridge's resources that there is no child
> + * under it
> + */
> + for (list = head->next; list;) {
> + res = list->res;
> + res->start = 0;
> + res->end = 0;
> + res->flags = 0;
> + tmp = list;
> + list = list->next;
> + kfree(tmp);
> + }
> +
> + head->next = NULL;
> +}
> +
> +static void pbus_assign_resources_sorted(const struct pci_bus *bus,
> + struct resource_list *fail_head)
> {
> struct pci_dev *dev;
> struct resource *res;
> @@ -58,9 +100,17 @@ static void pbus_assign_resources_sorted
> res = list->res;
> idx = res - &list->dev->resource[0];
> if (pci_assign_resource(list->dev, idx)) {
> - res->start = 0;
> - res->end = 0;
> - res->flags = 0;
> + if (fail_head && !pci_is_root_bus(list->dev->bus)) {
> + /*
> + * device need to keep flags and size
> + * for next try
> + */
> + add_to_failed_list(fail_head, list->dev, res);
> + } else {
> + res->start = 0;
> + res->end = 0;
> + res->flags = 0;
> + }
> }
> tmp = list;
> list = list->next;
> @@ -134,19 +184,12 @@ EXPORT_SYMBOL(pci_setup_cardbus);
> config space writes, so it's quite possible that an I/O window of
> the bridge will have some undesirable address (e.g. 0) after the
> first write. Ditto 64-bit prefetchable MMIO. */
> -static void pci_setup_bridge(struct pci_bus *bus)
> +static void pci_setup_bridge_io(struct pci_bus *bus)
> {
> struct pci_dev *bridge = bus->self;
> struct resource *res;
> struct pci_bus_region region;
> - u32 l, bu, lu, io_upper16;
> - int pref_mem64;
> -
> - if (pci_is_enabled(bridge))
> - return;
> -
> - dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
> - bus->secondary, bus->subordinate);
> + u32 l, io_upper16;
>
> /* Set up the top and bottom of the PCI I/O segment for this bus. */
> res = bus->resource[0];
> @@ -172,7 +215,13 @@ static void pci_setup_bridge(struct pci_
> pci_write_config_dword(bridge, PCI_IO_BASE, l);
> /* Update upper 16 bits of I/O base/limit. */
> pci_write_config_dword(bridge, PCI_IO_BASE_UPPER16, io_upper16);
> -
> +}
> +static void pci_setup_bridge_mmio(struct pci_bus *bus)
> +{
> + struct pci_dev *bridge = bus->self;
> + struct resource *res;
> + struct pci_bus_region region;
> + u32 l;
> /* Set up the top and bottom of the PCI Memory segment
> for this bus. */
> res = bus->resource[1];
> @@ -187,6 +236,14 @@ static void pci_setup_bridge(struct pci_
> dev_info(&bridge->dev, " bridge window [mem disabled]\n");
> }
> pci_write_config_dword(bridge, PCI_MEMORY_BASE, l);
> +}
> +static void pci_setup_bridge_mmio_pref(struct pci_bus *bus)
> +{
> + struct pci_dev *bridge = bus->self;
> + struct resource *res;
> + struct pci_bus_region region;
> + u32 l, bu, lu;
> + int pref_mem64;
>
> /* Clear out the upper 32 bits of PREF limit.
> If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
> @@ -219,10 +276,37 @@ static void pci_setup_bridge(struct pci_
> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
> pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
> }
> +}
> +static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type)
> +{
> + struct pci_dev *bridge = bus->self;
> +
> + if (pci_is_enabled(bridge))
> + return;
> +
> + dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
> + bus->secondary, bus->subordinate);
> +
> + if (type & IORESOURCE_IO)
> + pci_setup_bridge_io(bus);
> +
> + if (type & IORESOURCE_MEM)
> + pci_setup_bridge_mmio(bus);
> +
> + if (type & IORESOURCE_PREFETCH)
> + pci_setup_bridge_mmio_pref(bus);
>
> pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
> }
>
> +static void pci_setup_bridge(struct pci_bus *bus)
> +{
> + unsigned long type = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH;
> +
> + __pci_setup_bridge(bus, type);
> +}
> +
> /* Check whether the bridge supports optional I/O and
> prefetchable memory ranges. If not, the respective
> base/limit registers must be read-only and read as 0. */
> @@ -543,19 +627,20 @@ void __ref pci_bus_size_bridges(struct p
> }
> EXPORT_SYMBOL(pci_bus_size_bridges);
>
> -void __ref pci_bus_assign_resources(const struct pci_bus *bus)
> +static void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
> + struct resource_list *fail_head)
> {
> struct pci_bus *b;
> struct pci_dev *dev;
>
> - pbus_assign_resources_sorted(bus);
> + pbus_assign_resources_sorted(bus, fail_head);
>
> list_for_each_entry(dev, &bus->devices, bus_list) {
> b = dev->subordinate;
> if (!b)
> continue;
>
> - pci_bus_assign_resources(b);
> + __pci_bus_assign_resources(b, fail_head);
>
> switch (dev->class >> 8) {
> case PCI_CLASS_BRIDGE_PCI:
> @@ -573,15 +658,130 @@ void __ref pci_bus_assign_resources(cons
> }
> }
> }
> +
> +void __ref pci_bus_assign_resources(const struct pci_bus *bus)
> +{
> + __pci_bus_assign_resources(bus, NULL);
> +}
> EXPORT_SYMBOL(pci_bus_assign_resources);
>
> +static void release_children_resource(struct resource *r)
> +{
> + struct resource *p;
> + resource_size_t size;
> +
> + p = r->child;
> + while (p) {
> + release_children_resource(p);
> + release_resource(p);
> + printk(KERN_DEBUG "PCI: release child resource %pRt\n", p);
> + /* need to restore size, and keep flags */
> + size = resource_size(p);
> + p->start = 0;
> + p->end = size - 1;
> + p = r->child;
> + }
> +}
> +
> +static void pci_bridge_release_unused_res(struct pci_bus *bus,
> + unsigned long type)
> +{
> + int idx;
> + bool changed = false;
> + struct pci_dev *dev;
> + struct resource *r;
> + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH;
> +
> + /* for pci bridges res only */
> + dev = bus->self;
> + for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_BRIDGE_RESOURCES + 3;
> + idx++) {
> + r = &dev->resource[idx];
> + if ((r->flags & type_mask) != type)
> + continue;
> + if (!r->parent)
> + continue;
> + /*
> + * if there are children under that, we should release them
> + * all
> + */
> + release_children_resource(r);
> + if (!release_resource(r)) {
> + dev_printk(KERN_DEBUG, &dev->dev,
> + "resource %d %pRt released\n", idx, r);
> + r->flags = 0;
> + changed = true;
> + }
> + }
> +
> + if (changed) {
> + if (type & IORESOURCE_PREFETCH) {
> + /* avoiding touch the one without PREF */
> + type = IORESOURCE_PREFETCH;
> + }
> + __pci_setup_bridge(bus, type);
> + }
> +}
> +
> +/*
> + * try to release pci bridge resources that is from leaf bridge,
> + * so we can allocate big new one later
> + * check:
> + * 0: only release the bridge and only the bridge is leaf
> + * 1: release all down side bridge for third shoot
> + */
> +static void __ref pci_bus_release_unused_bridge_res(struct pci_bus *bus,
> + unsigned long type,
> + int check_leaf)
> +{
> + struct pci_dev *dev;
> + bool is_leaf_bridge = true;
> +
> + list_for_each_entry(dev, &bus->devices, bus_list) {
> + struct pci_bus *b = dev->subordinate;
> + if (!b)
> + continue;
> +
> + switch (dev->class >> 8) {
> + case PCI_CLASS_BRIDGE_CARDBUS:
> + is_leaf_bridge = false;
> + break;
> +
> + case PCI_CLASS_BRIDGE_PCI:
> + default:
> + is_leaf_bridge = false;
> + if (!check_leaf)
> + pci_bus_release_unused_bridge_res(b, type,
> + check_leaf);
> + break;
> + }
> + }
> +
> + /* The root bus? */
> + if (!bus->self)
> + return;
> +
> + switch (bus->self->class >> 8) {
> + case PCI_CLASS_BRIDGE_CARDBUS:
> + break;
> +
> + case PCI_CLASS_BRIDGE_PCI:
> + default:
> + if ((check_leaf && is_leaf_bridge) || !check_leaf)
> + pci_bridge_release_unused_res(bus, type);
> + break;
> + }
> +}
> +
> static void pci_bus_dump_res(struct pci_bus *bus)
> {
> int i;
>
> for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
> struct resource *res = bus->resource[i];
> - if (!res || !res->end)
> +
> + if (!res || !res->end || !res->flags)
> continue;
>
> dev_printk(KERN_DEBUG, &bus->dev, "resource %d %pR\n", i, res);
> @@ -605,10 +805,25 @@ static void pci_bus_dump_resources(struc
> }
> }
>
> +/*
> + * first try will not touch pci bridge res
> + * second try will clear small leaf bridge res
> + * third try will clear related bridge: some aggressive
> + */
> +/* assume we only have 4 level bridges, so only try 5 times */
> +int pci_try_num = 5;
> void __init
> pci_assign_unassigned_resources(void)
> {
> struct pci_bus *bus;
> + int tried_times = 0;
> + int check_leaf = 1;
> + struct resource_list head, *list, *tmp;
> + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH;
> + unsigned long failed_type;
> +again:
> + head.next = NULL;
>
> /* Depth first, calculate sizes and alignments of all
> subordinate buses. */
> @@ -617,7 +832,58 @@ pci_assign_unassigned_resources(void)
> }
> /* Depth last, allocate resources and update the hardware. */
> list_for_each_entry(bus, &pci_root_buses, node) {
> - pci_bus_assign_resources(bus);
> + __pci_bus_assign_resources(bus, &head);
> + }
> + tried_times++;
> +
> + /* any device complain? */
> + if (!head.next)
> + goto enable_and_dump;
> + failed_type = 0;
> + for (list = head.next; list;) {
> + unsigned long flags = list->res->flags;
> +
> + failed_type |= flags;
> + list = list->next;
> + }
> + /*
> + * io port are tight, don't try extra
> + * or if reach the limit, don't want to try more
> + */
> + failed_type &= type_mask;
> + if ((failed_type == IORESOURCE_IO) || (tried_times >= pci_try_num)) {
> + free_failed_list(&head);
> + goto enable_and_dump;
> + }
> +
> + printk(KERN_DEBUG "PCI: No. %d try to assign unassigned res\n",
> + tried_times + 1);
> +
> + /*
> + * Try to release leaf bridge's resources that doesn't fit resource of
> + * child device under that bridge
> + */
> + /* third times and later will not check if it is leaf */
> + if ((tried_times + 1) > 2)
> + check_leaf = 0;
> + for (list = head.next; list;) {
> + unsigned long flags = list->res->flags;
> +
> + bus = list->dev->bus;
> + if (list->dev->subordinate)
> + list->res->flags = 0;
> + pci_bus_release_unused_bridge_res(bus, flags & type_mask,
> + check_leaf);
> + tmp = list;
> + list = list->next;
> + kfree(tmp);
> + }
> +
> + goto again;
> +
> +enable_and_dump:
> + /* Depth last, update the hardware. */
> + list_for_each_entry(bus, &pci_root_buses, node) {
> pci_enable_bridges(bus);
> }
>
> Index: linux-2.6/drivers/pci/pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -2779,6 +2779,11 @@ static int __init pci_setup(char *str)
> pci_no_aer();
> } else if (!strcmp(str, "nodomains")) {
> pci_no_domains();
> + } else if (!strncmp(str, "try=", 4)) {
> + int try_num = memparse(str + 4, &str);
> +
> + if (try_num > 0 && try_num < 10)
> + pci_try_num = try_num;
> } else if (!strncmp(str, "cbiosize=", 9)) {
> pci_cardbus_io_size = memparse(str + 9, &str);
> } else if (!strncmp(str, "cbmemsize=", 10)) {
> Index: linux-2.6/drivers/pci/pci.h
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.h
> +++ linux-2.6/drivers/pci/pci.h
> @@ -203,6 +203,8 @@ static inline int pci_ari_enabled(struct
> return bus->self && bus->self->ari_enabled;
> }
>
> +extern int pci_try_num;
> +
> #ifdef CONFIG_PCI_QUIRKS
> extern int pci_is_reassigndev(struct pci_dev *dev);
> resource_size_t pci_specified_resource_alignment(struct pci_dev *dev);
>
>

2009-11-24 01:15:32

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Kenji Kaneshige wrote:
> Hi,
>
> I tried v11 patches. This version seems to fix the problem I
> reported against previous version.
>
> I have no objection against the idea of resource allocation
> changes for PCI express hotplug slots.

thanks

>
> But I still have concern about changing resource allocation for
> other than PCI express hotplug slots. For example, some hotplug
> controller other than PCI express can have multiple slots under
> the same bus. If some hotplug slots are occupied and the others
> are empty at the boot time, I think your code try to shrink the
> bus resources for hotplug slots allocated by BIOS. It would break
> the hot-add on the empty slots due to the resource allocation
> failure.

no,
it will not touch bridge resources that already assigned by BIOS except
some bridge resource is not big enough. and get big one for them.

YH

2009-11-24 01:51:39

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Yinghai Lu wrote:
> Kenji Kaneshige wrote:
>> Hi,
>>
>> I tried v11 patches. This version seems to fix the problem I
>> reported against previous version.
>>
>> I have no objection against the idea of resource allocation
>> changes for PCI express hotplug slots.
>
> thanks
>
>> But I still have concern about changing resource allocation for
>> other than PCI express hotplug slots. For example, some hotplug
>> controller other than PCI express can have multiple slots under
>> the same bus. If some hotplug slots are occupied and the others
>> are empty at the boot time, I think your code try to shrink the
>> bus resources for hotplug slots allocated by BIOS. It would break
>> the hot-add on the empty slots due to the resource allocation
>> failure.
>
> no,
> it will not touch bridge resources that already assigned by BIOS except
> some bridge resource is not big enough. and get big one for them.
>

Ok, I understood that if the BIOS assigns enough resources to the
bridge, it has no impact.

One question. I thought your patch shrinks the bridge resource to
allocate enough resource for sibling bridge. But it actually doesn't.
Right?

It would be appreciated if you update the patch description about
the problem and how to fix/improbe it.

Thanks,
Kenji Kaneshige

2009-11-24 02:33:56

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> Hi,
>>>
>>> I tried v11 patches. This version seems to fix the problem I
>>> reported against previous version.
>>>
>>> I have no objection against the idea of resource allocation
>>> changes for PCI express hotplug slots.
>>
>> thanks
>>
>>> But I still have concern about changing resource allocation for
>>> other than PCI express hotplug slots. For example, some hotplug
>>> controller other than PCI express can have multiple slots under
>>> the same bus. If some hotplug slots are occupied and the others
>>> are empty at the boot time, I think your code try to shrink the
>>> bus resources for hotplug slots allocated by BIOS. It would break
>>> the hot-add on the empty slots due to the resource allocation
>>> failure.
>>
>> no,
>> it will not touch bridge resources that already assigned by BIOS except
>> some bridge resource is not big enough. and get big one for them.
>>
>
> Ok, I understood that if the BIOS assigns enough resources to the
> bridge, it has no impact.
>
> One question. I thought your patch shrinks the bridge resource to
> allocate enough resource for sibling bridge. But it actually doesn't.
> Right?

ok, i got it. will change pci_try_num default to 1.

>
> It would be appreciated if you update the patch description about
> the problem and how to fix/improbe it.

sure.

YH

2009-11-24 23:19:53

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> Hi,
>>>
>>> I tried v11 patches. This version seems to fix the problem I
>>> reported against previous version.
>>>
>>> I have no objection against the idea of resource allocation
>>> changes for PCI express hotplug slots.
>>
>> thanks
>>
>>> But I still have concern about changing resource allocation for
>>> other than PCI express hotplug slots. For example, some hotplug
>>> controller other than PCI express can have multiple slots under
>>> the same bus. If some hotplug slots are occupied and the others
>>> are empty at the boot time, I think your code try to shrink the
>>> bus resources for hotplug slots allocated by BIOS. It would break
>>> the hot-add on the empty slots due to the resource allocation
>>> failure.
>>
>> no,
>> it will not touch bridge resources that already assigned by BIOS except
>> some bridge resource is not big enough. and get big one for them.
>>
>
> Ok, I understood that if the BIOS assigns enough resources to the
> bridge, it has no impact.
>
> One question. I thought your patch shrinks the bridge resource to
> allocate enough resource for sibling bridge. But it actually doesn't.
> Right?

please check if this one could fix the shrinking bridge resource problem...

[PATCH] pci: don't shrink bridge resources

when we are clearing leaf bridge resource and try to get big one, we could shrink the bridge if
there resource under it.

let check with old resource size and make sure we are trying to get big one.

Signed-off-by: Yinghai Lu <[email protected]>

---
drivers/pci/setup-bus.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -432,7 +432,7 @@ static void pbus_size_io(struct pci_bus
{
struct pci_dev *dev;
struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
- unsigned long size = 0, size1 = 0;
+ unsigned long size = 0, size1 = 0, old_size;

if (!b_res)
return;
@@ -457,17 +457,18 @@ static void pbus_size_io(struct pci_bus
}
if (size < min_size)
size = min_size;
+ old_size = resource_size(b_res);
+ if (old_size == 1)
+ old_size = 0;
/* To be fixed in 2.5: we should have sort of HAVE_ISA
flag in the struct pci_bus. */
#if defined(CONFIG_ISA) || defined(CONFIG_EISA)
size = (size & 0xff) + ((size & ~0xffUL) << 2);
#endif
size = ALIGN(size + size1, 4096);
+ if (size < old_size)
+ size = old_size;
if (!size) {
- if (b_res->start || b_res->end)
- dev_info(&bus->self->dev, "disabling bridge window "
- "%pR to [bus %02x-%02x] (unused)\n", b_res,
- bus->secondary, bus->subordinate);
b_res->flags = 0;
return;
}
@@ -483,7 +484,7 @@ static int pbus_size_mem(struct pci_bus
unsigned long type, resource_size_t min_size)
{
struct pci_dev *dev;
- resource_size_t min_align, align, size;
+ resource_size_t min_align, align, size, old_size;
resource_size_t aligns[12]; /* Alignments from 1Mb to 2Gb */
int order, max_order;
struct resource *b_res = find_free_bus_resource(bus, type);
@@ -533,6 +534,11 @@ static int pbus_size_mem(struct pci_bus
}
if (size < min_size)
size = min_size;
+ old_size = resource_size(b_res);
+ if (old_size == 1)
+ old_size = 0;
+ if (size < old_size)
+ size = old_size;

align = 0;
min_align = 0;
@@ -549,10 +555,6 @@ static int pbus_size_mem(struct pci_bus
}
size = ALIGN(size, min_align);
if (!size) {
- if (b_res->start || b_res->end)
- dev_info(&bus->self->dev, "disabling bridge window "
- "%pR to [bus %02x-%02x] (unused)\n", b_res,
- bus->secondary, bus->subordinate);
b_res->flags = 0;
return 1;
}

2009-11-25 11:24:54

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Hi Yinghai,

I would like to reconfirm what is the problem you're trying to solve
before reviewing and testing the additional patch. To be honest, your
patch looks more and more complicated and it is becoming difficult
for me to review and test it.

By the way, if your problem is that BIOS doesn't assign the resource
to the parent bridge (root port or switch downstream port) of PCIe
hotplug slots, I guess we can improve it with simpler way. I made a
sample patches (no enough testing). Please take a look. Patches are

- [PATCH 1/2] pciehp: remove redundancy in bridge resource allocation
- [PATCH 2/2] pciehp: add support for bridge resource reallocation

Thanks,
Kenji Kaneshige


Yinghai Lu wrote:
> Kenji Kaneshige wrote:
>> Yinghai Lu wrote:
>>> Kenji Kaneshige wrote:
>>>> Hi,
>>>>
>>>> I tried v11 patches. This version seems to fix the problem I
>>>> reported against previous version.
>>>>
>>>> I have no objection against the idea of resource allocation
>>>> changes for PCI express hotplug slots.
>>> thanks
>>>
>>>> But I still have concern about changing resource allocation for
>>>> other than PCI express hotplug slots. For example, some hotplug
>>>> controller other than PCI express can have multiple slots under
>>>> the same bus. If some hotplug slots are occupied and the others
>>>> are empty at the boot time, I think your code try to shrink the
>>>> bus resources for hotplug slots allocated by BIOS. It would break
>>>> the hot-add on the empty slots due to the resource allocation
>>>> failure.
>>> no,
>>> it will not touch bridge resources that already assigned by BIOS except
>>> some bridge resource is not big enough. and get big one for them.
>>>
>> Ok, I understood that if the BIOS assigns enough resources to the
>> bridge, it has no impact.
>>
>> One question. I thought your patch shrinks the bridge resource to
>> allocate enough resource for sibling bridge. But it actually doesn't.
>> Right?
>
> please check if this one could fix the shrinking bridge resource problem...
>
> [PATCH] pci: don't shrink bridge resources
>
> when we are clearing leaf bridge resource and try to get big one, we could shrink the bridge if
> there resource under it.
>
> let check with old resource size and make sure we are trying to get big one.
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> ---
> drivers/pci/setup-bus.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -432,7 +432,7 @@ static void pbus_size_io(struct pci_bus
> {
> struct pci_dev *dev;
> struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
> - unsigned long size = 0, size1 = 0;
> + unsigned long size = 0, size1 = 0, old_size;
>
> if (!b_res)
> return;
> @@ -457,17 +457,18 @@ static void pbus_size_io(struct pci_bus
> }
> if (size < min_size)
> size = min_size;
> + old_size = resource_size(b_res);
> + if (old_size == 1)
> + old_size = 0;
> /* To be fixed in 2.5: we should have sort of HAVE_ISA
> flag in the struct pci_bus. */
> #if defined(CONFIG_ISA) || defined(CONFIG_EISA)
> size = (size & 0xff) + ((size & ~0xffUL) << 2);
> #endif
> size = ALIGN(size + size1, 4096);
> + if (size < old_size)
> + size = old_size;
> if (!size) {
> - if (b_res->start || b_res->end)
> - dev_info(&bus->self->dev, "disabling bridge window "
> - "%pR to [bus %02x-%02x] (unused)\n", b_res,
> - bus->secondary, bus->subordinate);
> b_res->flags = 0;
> return;
> }
> @@ -483,7 +484,7 @@ static int pbus_size_mem(struct pci_bus
> unsigned long type, resource_size_t min_size)
> {
> struct pci_dev *dev;
> - resource_size_t min_align, align, size;
> + resource_size_t min_align, align, size, old_size;
> resource_size_t aligns[12]; /* Alignments from 1Mb to 2Gb */
> int order, max_order;
> struct resource *b_res = find_free_bus_resource(bus, type);
> @@ -533,6 +534,11 @@ static int pbus_size_mem(struct pci_bus
> }
> if (size < min_size)
> size = min_size;
> + old_size = resource_size(b_res);
> + if (old_size == 1)
> + old_size = 0;
> + if (size < old_size)
> + size = old_size;
>
> align = 0;
> min_align = 0;
> @@ -549,10 +555,6 @@ static int pbus_size_mem(struct pci_bus
> }
> size = ALIGN(size, min_align);
> if (!size) {
> - if (b_res->start || b_res->end)
> - dev_info(&bus->self->dev, "disabling bridge window "
> - "%pR to [bus %02x-%02x] (unused)\n", b_res,
> - bus->secondary, bus->subordinate);
> b_res->flags = 0;
> return 1;
> }
>
>

2009-11-25 11:26:13

by Kenji Kaneshige

[permalink] [raw]
Subject: [PATCH 1/2] pciehp: remove redundancy in bridge resource allocation

Current pciehp driver configures bridge's child bus each time the
bridge is found on the hot-added PCIe adapter card. It is redundant
and strange because pciehp tryies to add devices on the child bus
before adding its parent bridge. It should be done at one time on the
slot's parent bus.

Signed-off-by: Kenji Kaneshige <[email protected]>

---
drivers/pci/hotplug/pciehp_pci.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)

Index: 20091125/drivers/pci/hotplug/pciehp_pci.c
===================================================================
--- 20091125.orig/drivers/pci/hotplug/pciehp_pci.c
+++ 20091125/drivers/pci/hotplug/pciehp_pci.c
@@ -34,30 +34,26 @@
#include "../pci.h"
#include "pciehp.h"

-static int __ref pciehp_add_bridge(struct pci_dev *dev)
+static void __ref pciehp_scan_bridge(struct pci_dev *dev)
{
struct pci_bus *parent = dev->bus;
- int pass, busnr, start = parent->secondary;
- int end = parent->subordinate;
+ int pass, busnr, start, end;

+ start = parent->secondary;
+ end = parent->subordinate;
for (busnr = start; busnr <= end; busnr++) {
if (!pci_find_bus(pci_domain_nr(parent), busnr))
break;
}
+
if (busnr-- > end) {
err("No bus number available for hot-added bridge %s\n",
- pci_name(dev));
- return -1;
+ pci_name(dev));
+ return;
}
+
for (pass = 0; pass < 2; pass++)
busnr = pci_scan_bridge(parent, dev, busnr, pass);
- if (!dev->subordinate)
- return -1;
- pci_bus_size_bridges(dev->subordinate);
- pci_bus_assign_resources(parent);
- pci_enable_bridges(parent);
- pci_bus_add_devices(parent);
- return 0;
}

int pciehp_configure_device(struct slot *p_slot)
@@ -93,14 +89,16 @@ int pciehp_configure_device(struct slot
continue;
}
if ((dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
- (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
- pciehp_add_bridge(dev);
- }
+ (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS))
+ pciehp_scan_bridge(dev);
+
pci_configure_slot(dev);
pci_dev_put(dev);
}

+ pci_bus_size_bridges(parent);
pci_bus_assign_resources(parent);
+ pci_enable_bridges(parent);
pci_bus_add_devices(parent);
return 0;
}

2009-11-25 11:27:34

by Kenji Kaneshige

[permalink] [raw]
Subject: [PATCH 2/2] pciehp: add support for bridge resource reallocation

Some platform BIOS doesn't assign PCI bridge resources (IO base/limit,
Mem base/limit and Prefechable Mem base/limit) to root port or switch
downstream port with hotplug slots. On such platform, PCIe hotplug
doesn't work due to the resource allocation failure. This patch is to
improbe this situation.

With this patch, pciehp driver try to assign PCI bridge resources to
parent bridge (root port or switch downstream port) of the slot in
addition to assign resources to the hot-added device.

With this patch, pciehp driver try to release PCI bridge resources so
that we can reassign those resources to another root port on the same
PCI bus segment, or to another downstream port on the same swith.

This feature is enabled when 'pciehp_realloc' option is specified.

Signed-off-by: Kenji Kaneshige <[email protected]>

---
drivers/pci/hotplug/pciehp.h | 1
drivers/pci/hotplug/pciehp_core.c | 9 ++++
drivers/pci/hotplug/pciehp_pci.c | 9 +++-
drivers/pci/setup-bus.c | 84 +++++++++++++++++++++++++++++---------
include/linux/pci.h | 2
5 files changed, 85 insertions(+), 20 deletions(-)

Index: 20091125/drivers/pci/setup-bus.c
===================================================================
--- 20091125.orig/drivers/pci/setup-bus.c
+++ 20091125/drivers/pci/setup-bus.c
@@ -27,13 +27,31 @@
#include <linux/slab.h>
#include "pci.h"

-static void pbus_assign_resources_sorted(const struct pci_bus *bus)
+static void pbus_assign_resources_list(struct resource_list *head)
{
- struct pci_dev *dev;
struct resource *res;
- struct resource_list head, *list, *tmp;
+ struct resource_list *list, *tmp;
int idx;

+ for (list = head->next; list;) {
+ res = list->res;
+ idx = res - &list->dev->resource[0];
+ if (pci_assign_resource(list->dev, idx)) {
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+}
+
+static void pbus_assign_resources_sorted(const struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+ struct resource_list head;
+
head.next = NULL;
list_for_each_entry(dev, &bus->devices, bus_list) {
u16 class = dev->class >> 8;
@@ -54,18 +72,7 @@ static void pbus_assign_resources_sorted
pdev_sort_resources(dev, &head);
}

- for (list = head.next; list;) {
- res = list->res;
- idx = res - &list->dev->resource[0];
- if (pci_assign_resource(list->dev, idx)) {
- res->start = 0;
- res->end = 0;
- res->flags = 0;
- }
- tmp = list;
- list = list->next;
- kfree(tmp);
- }
+ pbus_assign_resources_list(&head);
}

void pci_setup_cardbus(struct pci_bus *bus)
@@ -142,9 +149,6 @@ static void pci_setup_bridge(struct pci_
u32 l, bu, lu, io_upper16;
int pref_mem64;

- if (pci_is_enabled(bridge))
- return;
-
dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
bus->secondary, bus->subordinate);

@@ -559,7 +563,8 @@ void __ref pci_bus_assign_resources(cons

switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
- pci_setup_bridge(b);
+ if (!pci_is_enabled(dev))
+ pci_setup_bridge(b);
break;

case PCI_CLASS_BRIDGE_CARDBUS:
@@ -575,6 +580,47 @@ void __ref pci_bus_assign_resources(cons
}
EXPORT_SYMBOL(pci_bus_assign_resources);

+void __ref pci_bridge_assign_resources(struct pci_bus *bus)
+{
+ struct pci_dev *bridge = bus->self;
+ struct resource_list head;
+
+ if (pci_is_root_bus(bus) ||
+ (bridge->class >> 8) != PCI_CLASS_BRIDGE_PCI)
+ return;
+
+ head.next = NULL;
+ pdev_sort_resources(bridge, &head);
+ pbus_assign_resources_list(&head);
+ pci_bus_assign_resources(bus);
+ pci_setup_bridge(bus);
+}
+EXPORT_SYMBOL(pci_bridge_assign_resources);
+
+void __ref pci_bridge_release_window(struct pci_bus *bus)
+{
+ struct pci_dev *bridge = bus->self;
+ int i;
+
+ if (pci_is_root_bus(bus) ||
+ (bridge->class >> 8) != PCI_CLASS_BRIDGE_PCI)
+ return;
+
+ for (i = 0; i < 3; i++)
+ if (bus->resource[i]->child)
+ return;
+
+ for (i = 0; i < 3; i++)
+ bus->resource[i]->flags = 0;
+
+ pci_setup_bridge(bus);
+
+ for (i = 0; i < 3; i++)
+ if (bus->resource[i]->parent)
+ release_resource(bus->resource[i]);
+}
+EXPORT_SYMBOL(pci_bridge_release_window);
+
static void pci_bus_dump_res(struct pci_bus *bus)
{
int i;
Index: 20091125/drivers/pci/hotplug/pciehp_core.c
===================================================================
--- 20091125.orig/drivers/pci/hotplug/pciehp_core.c
+++ 20091125/drivers/pci/hotplug/pciehp_core.c
@@ -41,6 +41,7 @@ int pciehp_debug;
int pciehp_poll_mode;
int pciehp_poll_time;
int pciehp_force;
+int pciehp_realloc;
struct workqueue_struct *pciehp_wq;

#define DRIVER_VERSION "0.4"
@@ -55,10 +56,13 @@ module_param(pciehp_debug, bool, 0644);
module_param(pciehp_poll_mode, bool, 0644);
module_param(pciehp_poll_time, int, 0644);
module_param(pciehp_force, bool, 0644);
+module_param(pciehp_realloc, bool, 0644);
+
MODULE_PARM_DESC(pciehp_debug, "Debugging mode enabled or not");
MODULE_PARM_DESC(pciehp_poll_mode, "Using polling mechanism for hot-plug events or not");
MODULE_PARM_DESC(pciehp_poll_time, "Polling mechanism frequency, in seconds");
MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if _OSC and OSHP are missing");
+MODULE_PARM_DESC(pciehp_realloc, "Realloc resources for slot's parent bridge");

#define PCIE_MODULE_NAME "pciehp"

@@ -293,10 +297,15 @@ static int pciehp_probe(struct pcie_devi
pciehp_get_power_status(slot, &poweron);
if (occupied && pciehp_force)
pciehp_enable_slot(slot);
+
/* If empty slot's power status is on, turn power off */
if (!occupied && poweron && POWER_CTRL(ctrl))
pciehp_power_off_slot(slot);

+ /* If no device is running on the slot, release bridge's I/O window */
+ if (pciehp_realloc && !poweron)
+ pci_bridge_release_window(dev->port->subordinate);
+
return 0;

err_out_free_ctrl_slot:
Index: 20091125/drivers/pci/hotplug/pciehp_pci.c
===================================================================
--- 20091125.orig/drivers/pci/hotplug/pciehp_pci.c
+++ 20091125/drivers/pci/hotplug/pciehp_pci.c
@@ -97,7 +97,10 @@ int pciehp_configure_device(struct slot
}

pci_bus_size_bridges(parent);
- pci_bus_assign_resources(parent);
+ if (pciehp_realloc)
+ pci_bridge_assign_resources(parent);
+ else
+ pci_bus_assign_resources(parent);
pci_enable_bridges(parent);
pci_bus_add_devices(parent);
return 0;
@@ -153,5 +156,9 @@ int pciehp_unconfigure_device(struct slo
pci_dev_put(temp);
}

+ /* Release I/O window of the slots's parent bridge */
+ if (pciehp_realloc)
+ pci_bridge_release_window(parent);
+
return rc;
}
Index: 20091125/include/linux/pci.h
===================================================================
--- 20091125.orig/include/linux/pci.h
+++ 20091125/include/linux/pci.h
@@ -764,6 +764,8 @@ int pci_vpd_truncate(struct pci_dev *dev

/* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
void pci_bus_assign_resources(const struct pci_bus *bus);
+void pci_bridge_assign_resources(struct pci_bus *bus);
+void pci_bridge_release_window(struct pci_bus *bus);
void pci_bus_size_bridges(struct pci_bus *bus);
int pci_claim_resource(struct pci_dev *, int);
void pci_assign_unassigned_resources(void);
Index: 20091125/drivers/pci/hotplug/pciehp.h
===================================================================
--- 20091125.orig/drivers/pci/hotplug/pciehp.h
+++ 20091125/drivers/pci/hotplug/pciehp.h
@@ -43,6 +43,7 @@ extern int pciehp_poll_mode;
extern int pciehp_poll_time;
extern int pciehp_debug;
extern int pciehp_force;
+extern int pciehp_realloc;
extern struct workqueue_struct *pciehp_wq;

#define dbg(format, arg...) \

2009-11-25 17:38:52

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pciehp: remove redundancy in bridge resource allocation

Kenji Kaneshige wrote:
> Current pciehp driver configures bridge's child bus each time the
> bridge is found on the hot-added PCIe adapter card. It is redundant
> and strange because pciehp tryies to add devices on the child bus
> before adding its parent bridge. It should be done at one time on the
> slot's parent bus.
>
> Signed-off-by: Kenji Kaneshige <[email protected]>
>
> ---
> drivers/pci/hotplug/pciehp_pci.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> Index: 20091125/drivers/pci/hotplug/pciehp_pci.c
> ===================================================================
> --- 20091125.orig/drivers/pci/hotplug/pciehp_pci.c
> +++ 20091125/drivers/pci/hotplug/pciehp_pci.c
> @@ -34,30 +34,26 @@
> #include "../pci.h"
> #include "pciehp.h"
>
> -static int __ref pciehp_add_bridge(struct pci_dev *dev)
> +static void __ref pciehp_scan_bridge(struct pci_dev *dev)
> {
> struct pci_bus *parent = dev->bus;
> - int pass, busnr, start = parent->secondary;
> - int end = parent->subordinate;
> + int pass, busnr, start, end;
>
> + start = parent->secondary;
> + end = parent->subordinate;
> for (busnr = start; busnr <= end; busnr++) {
> if (!pci_find_bus(pci_domain_nr(parent), busnr))
> break;
> }
> +
> if (busnr-- > end) {
> err("No bus number available for hot-added bridge %s\n",
> - pci_name(dev));
> - return -1;
> + pci_name(dev));
> + return;
> }
> +
> for (pass = 0; pass < 2; pass++)
> busnr = pci_scan_bridge(parent, dev, busnr, pass);
> - if (!dev->subordinate)
> - return -1;
> - pci_bus_size_bridges(dev->subordinate);
> - pci_bus_assign_resources(parent);
> - pci_enable_bridges(parent);
> - pci_bus_add_devices(parent);
> - return 0;
> }
>
> int pciehp_configure_device(struct slot *p_slot)
> @@ -93,14 +89,16 @@ int pciehp_configure_device(struct slot
> continue;
> }
> if ((dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
> - (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
> - pciehp_add_bridge(dev);
> - }
> + (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS))
> + pciehp_scan_bridge(dev);
> +
> pci_configure_slot(dev);
> pci_dev_put(dev);
> }
>
> + pci_bus_size_bridges(parent);
> pci_bus_assign_resources(parent);
> + pci_enable_bridges(parent);
> pci_bus_add_devices(parent);
> return 0;
> }

you comment on my patch2, that we should move pci_configure_slot later?


YH

2009-11-25 17:45:35

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Kenji Kaneshige wrote:
> Hi Yinghai,
>
> I would like to reconfirm what is the problem you're trying to solve
> before reviewing and testing the additional patch. To be honest, your
> patch looks more and more complicated and it is becoming difficult
> for me to review and test it.

the real problem:
1. boot time:

BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to the bridge, but stop
assigning resource to the device under that bridge, because the device need big resource.

so patch1 is trying to
a. pci assign unassign and record the failed device resource.
b. clear the BIOS assigned resource of the parent bridge of fail device
c. go back and call pci assign unsigned
d. if it still fail, will go up more bridges. and clear and try again.

2. hotplug:
BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to every bridge. (8M)
but when insert one card that big resource, the card can not get resource. because kernel will not touch the
bridge resource.

so patch2 is trying to
a. assign resource to devices with that slot. and record fail devices
b. if there is some failed, will clear sepcifically io port of bridge, or mmio of bridge, or mmio pref of bridge.
c. try to assign the parent bridge of the slot.

so it will keep original assigned resource by BIOS if possible.

and you have tested patch1 and patch2 in V11, but said patch1 may have shrinking resource problem.
the patch3 is addressing the patch1 that could shrinking resource for non-pcie hotplug bridge...


>
> By the way, if your problem is that BIOS doesn't assign the resource
> to the parent bridge (root port or switch downstream port) of PCIe
> hotplug slots, I guess we can improve it with simpler way. I made a
> sample patches (no enough testing). Please take a look. Patches are
>
> - [PATCH 1/2] pciehp: remove redundancy in bridge resource allocation
> - [PATCH 2/2] pciehp: add support for bridge resource reallocation

like some earlier version of patch2 (release it them all at first) but have pciehp_realloc parameter.
could be useful in some case when current patch2 (try and increase) could use up all space.
i like to have that after patch2.

YH

2009-11-25 20:00:00

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v12

split the two patches in to 9 for easy review...

please check

1. boot time:

BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to the bridge, but stop
assigning resource to the device under that bridge, because the device need big resource.

so patches (first 6) are trying to
a. pci assign unassign and record the failed device resource.
b. clear the BIOS assigned resource of the parent bridge of fail device
c. go back and call pci assign unsigned
d. if it still fail, will go up more bridges. and clear and try again.

v2: Jesse doesn't like it is in find_free_bus_resource...
try to move out of pci_bus_size_bridges loop.
v3: add pci_setup_bridge calling after pci_bridge_release_not_used_res.
only clear release those res for x86.
v4: Bjorn want to release use dev instead of bus.
v5: Kenji pointed out it will have problem with several level bridge.
so let only handle leaf bridge.
v6: address Kenji's request (new pci_bus_release...). and change applying order
move back release to pci_assign_unassigned_resource
v7: change functions name pci_bus_release_unused_bridge_res according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not big
enough need to do it in two steps, and first step recore the failed res,
and don't touch bridge res that programmed by firmware. second step will
try to release bridge resource that is too small at first.
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v11:add pci=try=5, about more try to change more bridge
v12:not shrink pci bridge resource

2. hotplug:
BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to every bridge. (8M)
but when insert one card that big resource, the card can not get resource. because kernel will not touch the
bridge resource.

so patches (last 3) are trying to
a. assign resource to devices with that slot. and record fail devices
b. if there is some failed, will clear sepcifically io port of bridge, or mmio of bridge, or mmio pref of bridge.
c. try to assign the parent bridge of the slot.

v2: address Alex's concern about pci remove/rescan feature about
pci_setup_bridge changes.
v3: Kenji pointed out that pci_config_slot need to be called before
pci_bus_add_devices()
v4: move out pci_is_enabled checkout of pci_setup_bridge()
v5: change the applying sequence.
v6: change the functions name according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not
big enough
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v10:alex found need to have export for pci_assign_unassigned_bridge_resources
v11: pass check_leaf with pci_bus_release_unused_bridge_res


Thanks

Yinghai

2009-11-26 06:43:49

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Yinghai Lu wrote:
> Kenji Kaneshige wrote:
>> Hi Yinghai,
>>
>> I would like to reconfirm what is the problem you're trying to solve
>> before reviewing and testing the additional patch. To be honest, your
>> patch looks more and more complicated and it is becoming difficult
>> for me to review and test it.
>
> the real problem:
> 1. boot time:
>
> BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to the bridge, but stop
> assigning resource to the device under that bridge, because the device need big resource.
>
> so patch1 is trying to
> a. pci assign unassign and record the failed device resource.
> b. clear the BIOS assigned resource of the parent bridge of fail device
> c. go back and call pci assign unsigned
> d. if it still fail, will go up more bridges. and clear and try again.
>
> 2. hotplug:
> BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to every bridge. (8M)
> but when insert one card that big resource, the card can not get resource. because kernel will not touch the
> bridge resource.
>
> so patch2 is trying to
> a. assign resource to devices with that slot. and record fail devices
> b. if there is some failed, will clear sepcifically io port of bridge, or mmio of bridge, or mmio pref of bridge.
> c. try to assign the parent bridge of the slot.
>
> so it will keep original assigned resource by BIOS if possible.
>
> and you have tested patch1 and patch2 in V11, but said patch1 may have shrinking resource problem.
> the patch3 is addressing the patch1 that could shrinking resource for non-pcie hotplug bridge...
>

Thank you for the explanation. The patch3 seems to solve my concern.

Your patch only touch the leaf bridge at the 2nd try of resource
assignment. IIRC, this behavior is to prevent from shrinking bridge
resources. Am I correct? I'm not sure but I think we don't need this
behavior because now that we have another mechanism to prevent
from shrinking bridge resource.

Thanks,
Kenji Kaneshige



>
>> By the way, if your problem is that BIOS doesn't assign the resource
>> to the parent bridge (root port or switch downstream port) of PCIe
>> hotplug slots, I guess we can improve it with simpler way. I made a
>> sample patches (no enough testing). Please take a look. Patches are
>>
>> - [PATCH 1/2] pciehp: remove redundancy in bridge resource allocation
>> - [PATCH 2/2] pciehp: add support for bridge resource reallocation
>
> like some earlier version of patch2 (release it them all at first) but have pciehp_realloc parameter.
> could be useful in some case when current patch2 (try and increase) could use up all space.
> i like to have that after patch2.
>
> YH
>
>

2009-11-26 07:30:44

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11





On Nov 25, 2009, at 10:43 PM, Kenji Kaneshige <[email protected]
> wrote:

> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> Hi Yinghai,
>>>
>>> I would like to reconfirm what is the problem you're trying to solve
>>> before reviewing and testing the additional patch. To be honest,
>>> your
>>> patch looks more and more complicated and it is becoming difficult
>>> for me to review and test it.
>> the real problem:
>> 1. boot time:
>> BIOS separate IO range between several IOHs, and on some slots,
>> BIOS assign the resource to the bridge, but stop
>> assigning resource to the device under that bridge, because the
>> device need big resource.
>> so patch1 is trying to a. pci assign unassign and record the failed
>> device resource.
>> b. clear the BIOS assigned resource of the parent bridge of fail
>> device
>> c. go back and call pci assign unsigned
>> d. if it still fail, will go up more bridges. and clear and try
>> again.
>> 2. hotplug:
>> BIOS separate IO range between several IOHs, and on some slots,
>> BIOS assign the resource to every bridge. (8M)
>> but when insert one card that big resource, the card can not get
>> resource. because kernel will not touch the bridge resource.
>> so patch2 is trying to
>> a. assign resource to devices with that slot. and record fail devices
>> b. if there is some failed, will clear sepcifically io port of
>> bridge, or mmio of bridge, or mmio pref of bridge.
>> c. try to assign the parent bridge of the slot.
>> so it will keep original assigned resource by BIOS if possible.
>> and you have tested patch1 and patch2 in V11, but said patch1 may
>> have shrinking resource problem.
>> the patch3 is addressing the patch1 that could shrinking resource
>> for non-pcie hotplug bridge...
>>
>
> Thank you for the explanation. The patch3 seems to solve my concern.
>
> Your patch only touch the leaf bridge at the 2nd try of resource
> assignment. IIRC, this behavior is to prevent from shrinking bridge
> resources. Am I correct? I'm not sure but I think we don't need this
> behavior because now that we have another mechanism to prevent
> from shrinking bridge resource.
>>>

Third patch will only try to increase the bridge res

Try num still default to 5 , it could help other case

Can you send me whole bootlog ?

Thanks

Yinghai

2009-11-27 07:12:39

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Yinghai wrote:
>
>
>
>
> On Nov 25, 2009, at 10:43 PM, Kenji Kaneshige
> <[email protected]> wrote:
>
>> Yinghai Lu wrote:
>>> Kenji Kaneshige wrote:
>>>> Hi Yinghai,
>>>>
>>>> I would like to reconfirm what is the problem you're trying to solve
>>>> before reviewing and testing the additional patch. To be honest, your
>>>> patch looks more and more complicated and it is becoming difficult
>>>> for me to review and test it.
>>> the real problem:
>>> 1. boot time:
>>> BIOS separate IO range between several IOHs, and on some slots, BIOS
>>> assign the resource to the bridge, but stop
>>> assigning resource to the device under that bridge, because the
>>> device need big resource.
>>> so patch1 is trying to a. pci assign unassign and record the failed
>>> device resource.
>>> b. clear the BIOS assigned resource of the parent bridge of fail device
>>> c. go back and call pci assign unsigned
>>> d. if it still fail, will go up more bridges. and clear and try again.
>>> 2. hotplug:
>>> BIOS separate IO range between several IOHs, and on some slots, BIOS
>>> assign the resource to every bridge. (8M)
>>> but when insert one card that big resource, the card can not get
>>> resource. because kernel will not touch the bridge resource.
>>> so patch2 is trying to
>>> a. assign resource to devices with that slot. and record fail devices
>>> b. if there is some failed, will clear sepcifically io port of
>>> bridge, or mmio of bridge, or mmio pref of bridge.
>>> c. try to assign the parent bridge of the slot.
>>> so it will keep original assigned resource by BIOS if possible.
>>> and you have tested patch1 and patch2 in V11, but said patch1 may
>>> have shrinking resource problem.
>>> the patch3 is addressing the patch1 that could shrinking resource for
>>> non-pcie hotplug bridge...
>>>
>>
>> Thank you for the explanation. The patch3 seems to solve my concern.
>>
>> Your patch only touch the leaf bridge at the 2nd try of resource
>> assignment. IIRC, this behavior is to prevent from shrinking bridge
>> resources. Am I correct? I'm not sure but I think we don't need this
>> behavior because now that we have another mechanism to prevent
>> from shrinking bridge resource.
>>>>
>
> Third patch will only try to increase the bridge res
>
> Try num still default to 5 , it could help other case
>
> Can you send me whole bootlog ?
>

Bad news...

My system doesn't boot (hangup) with your latest set of patches.
Fusion MPT SAS driver initialization failed on some devices.
Please see below

...
Fusion MPT SAS Host driver 3.04.12
mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
mptsas 0000:09:00.0: BAR 1: can't reserve [mem 0x00510000-0x00513fff 64bit]
mptbase: ioc0: ERROR - pci_request_selected_regions() with MEM failed
mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
mptsas 0000:0a:00.0: BAR 1: can't reserve [mem 0x00610000-0x00613fff 64bit]
mptbase: ioc1: ERROR - pci_request_selected_regions() with MEM failed
...


This problem disappear when I revert the patch 6/9, and my system
can boot. But I found igb driver initialization failed on some
devices even in this case. Please see below

...
igb 0000:08:00.0: BAR 0: can't reserve [mem 0x00100000-0x0011ffff]
igb 0000:08:00.0: PCI INT A disabled
igb: probe of 0000:08:00.0 failed with error -16
igb 0000:08:00.1: PCI INT B -> GSI 18 (level, low) -> IRQ 18
igb 0000:08:00.1: BAR 0: can't reserve [mem 0x00140000-0x0015ffff]
igb 0000:08:00.1: PCI INT B disabled
igb: probe of 0000:08:00.1 failed with error -16
...

I'll send the whole boot log in both cases privately later.

Thanks,
Kenji Kaneshige

2009-11-27 07:54:09

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

>>
>
> Bad news...
>
> My system doesn't boot (hangup) with your latest set of patches.
> Fusion MPT SAS driver initialization failed on some devices.
> Please see below
>
> ...
> Fusion MPT SAS Host driver 3.04.12
> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
> mptsas 0000:09:00.0: BAR 1: can't reserve [mem 0x00510000-0x00513fff 64bit]
> mptbase: ioc0: ERROR - pci_request_selected_regions() with MEM failed
> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
> mptsas 0000:0a:00.0: BAR 1: can't reserve [mem 0x00610000-0x00613fff 64bit]
> mptbase: ioc1: ERROR - pci_request_selected_regions() with MEM failed
> ...
>
>
> This problem disappear when I revert the patch 6/9, and my system
> can boot. But I found igb driver initialization failed on some
> devices even in this case. Please see below
>
> ...
> igb 0000:08:00.0: BAR 0: can't reserve [mem 0x00100000-0x0011ffff]
> igb 0000:08:00.0: PCI INT A disabled
> igb: probe of 0000:08:00.0 failed with error -16
> igb 0000:08:00.1: PCI INT B -> GSI 18 (level, low) -> IRQ 18
> igb 0000:08:00.1: BAR 0: can't reserve [mem 0x00140000-0x0015ffff]
> igb 0000:08:00.1: PCI INT B disabled
> igb: probe of 0000:08:00.1 failed with error -16
> ...

that looks werid, without patch 6/9 should only have pci bridge res shrink problem with your last test?

maybe we need keep that feature default to be disabled.

please try

---
drivers/pci/setup-bus.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -918,8 +918,7 @@ static void pci_bus_dump_resources(struc
* second try will clear small leaf bridge res
* third try will clear related bridge: some aggressive
*/
-/* assume we only have 4 level bridges, so only try 5 times */
-int pci_try_num = 5;
+int pci_try_num = 1;
void __init
pci_assign_unassigned_resources(void)
{

2009-11-27 08:27:25

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Yinghai Lu wrote:
>> Bad news...
>>
>> My system doesn't boot (hangup) with your latest set of patches.
>> Fusion MPT SAS driver initialization failed on some devices.
>> Please see below
>>
>> ...
>> Fusion MPT SAS Host driver 3.04.12
>> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
>> mptsas 0000:09:00.0: BAR 1: can't reserve [mem 0x00510000-0x00513fff 64bit]
>> mptbase: ioc0: ERROR - pci_request_selected_regions() with MEM failed
>> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
>> mptsas 0000:0a:00.0: BAR 1: can't reserve [mem 0x00610000-0x00613fff 64bit]
>> mptbase: ioc1: ERROR - pci_request_selected_regions() with MEM failed
>> ...
>>
>>
>> This problem disappear when I revert the patch 6/9, and my system
>> can boot. But I found igb driver initialization failed on some
>> devices even in this case. Please see below
>>
>> ...
>> igb 0000:08:00.0: BAR 0: can't reserve [mem 0x00100000-0x0011ffff]
>> igb 0000:08:00.0: PCI INT A disabled
>> igb: probe of 0000:08:00.0 failed with error -16
>> igb 0000:08:00.1: PCI INT B -> GSI 18 (level, low) -> IRQ 18
>> igb 0000:08:00.1: BAR 0: can't reserve [mem 0x00140000-0x0015ffff]
>> igb 0000:08:00.1: PCI INT B disabled
>> igb: probe of 0000:08:00.1 failed with error -16
>> ...
>
> that looks werid, without patch 6/9 should only have pci bridge res shrink problem with your last test?
>

I don't know why. I might have overlooked it at the previous
test.

> maybe we need keep that feature default to be disabled.

Will do it next week.

Thanks,
Kenji Kaneshige


>
> please try
>
> ---
> drivers/pci/setup-bus.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -918,8 +918,7 @@ static void pci_bus_dump_resources(struc
> * second try will clear small leaf bridge res
> * third try will clear related bridge: some aggressive
> */
> -/* assume we only have 4 level bridges, so only try 5 times */
> -int pci_try_num = 5;
> +int pci_try_num = 1;
> void __init
> pci_assign_unassigned_resources(void)
> {
>
>

2009-11-27 23:14:59

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11

Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>>> Bad news...
>>>
>>> My system doesn't boot (hangup) with your latest set of patches.
>>> Fusion MPT SAS driver initialization failed on some devices.
>>> Please see below
>>>
>>> ...
>>> Fusion MPT SAS Host driver 3.04.12
>>> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
>>> mptsas 0000:09:00.0: BAR 1: can't reserve [mem 0x00510000-0x00513fff
>>> 64bit]
>>> mptbase: ioc0: ERROR - pci_request_selected_regions() with MEM failed
>>> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
>>> mptsas 0000:0a:00.0: BAR 1: can't reserve [mem 0x00610000-0x00613fff
>>> 64bit]
>>> mptbase: ioc1: ERROR - pci_request_selected_regions() with MEM failed
>>> ...
>>>
>>>
>>> This problem disappear when I revert the patch 6/9, and my system
>>> can boot. But I found igb driver initialization failed on some
>>> devices even in this case. Please see below
>>>
>>> ...
>>> igb 0000:08:00.0: BAR 0: can't reserve [mem 0x00100000-0x0011ffff]
>>> igb 0000:08:00.0: PCI INT A disabled
>>> igb: probe of 0000:08:00.0 failed with error -16
>>> igb 0000:08:00.1: PCI INT B -> GSI 18 (level, low) -> IRQ 18
>>> igb 0000:08:00.1: BAR 0: can't reserve [mem 0x00140000-0x0015ffff]
>>> igb 0000:08:00.1: PCI INT B disabled
>>> igb: probe of 0000:08:00.1 failed with error -16
>>> ...
>>
>> that looks werid, without patch 6/9 should only have pci bridge res
>> shrink problem with your last test?
>>
>
> I don't know why. I might have overlooked it at the previous
> test.
>
>> maybe we need keep that feature default to be disabled.
>
> Will do it next week.


>
> Thanks,
> Kenji Kaneshige
>
>
>>
>> please try
>>
>> ---
>> drivers/pci/setup-bus.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> Index: linux-2.6/drivers/pci/setup-bus.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/pci/setup-bus.c
>> +++ linux-2.6/drivers/pci/setup-bus.c
>> @@ -918,8 +918,7 @@ static void pci_bus_dump_resources(struc
>> * second try will clear small leaf bridge res
>> * third try will clear related bridge: some aggressive
>> */
>> -/* assume we only have 4 level bridges, so only try 5 times */
>> -int pci_try_num = 5;
>> +int pci_try_num = 1;
>> void __init
>> pci_assign_unassigned_resources(void)
>> {
>>
>>
>

don't try it. found the cause.

even change pci_try_num = 1 will not help.

...

will have another patch.

Thanks

Yinghai

2009-11-28 07:36:42

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

split the two patches in to 9 for easy review...

please check

1. boot time:

BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to the bridge, but stop
assigning resource to the device under that bridge, because the device need big resource.

so patches (first 6) are trying to
a. pci assign unassign and record the failed device resource.
b. clear the BIOS assigned resource of the parent bridge of fail device
c. go back and call pci assign unsigned
d. if it still fail, will go up more bridges. and clear and try again.

v2: Jesse doesn't like it is in find_free_bus_resource...
try to move out of pci_bus_size_bridges loop.
v3: add pci_setup_bridge calling after pci_bridge_release_not_used_res.
only clear release those res for x86.
v4: Bjorn want to release use dev instead of bus.
v5: Kenji pointed out it will have problem with several level bridge.
so let only handle leaf bridge.
v6: address Kenji's request (new pci_bus_release...). and change applying order
move back release to pci_assign_unassigned_resource
v7: change functions name pci_bus_release_unused_bridge_res according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not big
enough need to do it in two steps, and first step recore the failed res,
and don't touch bridge res that programmed by firmware. second step will
try to release bridge resource that is too small at first.
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v11:add pci=try=5, about more try to change more bridge
v12:not shrink pci bridge resource

2. hotplug:
BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to every bridge. (8M)
but when insert one card that big resource, the card can not get resource. because kernel will not touch the
bridge resource.

so patches (last 3) are trying to
a. assign resource to devices with that slot. and record fail devices
b. if there is some failed, will clear sepcifically io port of bridge, or mmio of bridge, or mmio pref of bridge.
c. try to assign the parent bridge of the slot.

v2: address Alex's concern about pci remove/rescan feature about
pci_setup_bridge changes.
v3: Kenji pointed out that pci_config_slot need to be called before
pci_bus_add_devices()
v4: move out pci_is_enabled checkout of pci_setup_bridge()
v5: change the applying sequence.
v6: change the functions name according to Jesse
v8: address Eric's concern, only overwrite leaf bridge resource that is not
big enough
v9: refresh to be applied after bjorn's patch, and remove trick about save
size and restore resource second try.
v10:alex found need to have export for pci_assign_unassigned_bridge_resources
v11: pass check_leaf with pci_bus_release_unused_bridge_res

-v13: change resource_list to resource_list_x, to save size and flags aside, otherwise grandchild res will
get confused with son's res as could be used


Thanks

Yinghai


2009-11-28 08:17:45

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Kenji, Jesse:

or you can check it from

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-2.6-yinghai.git for-pci

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-2.6-yinghai.git;a=shortlog;h=refs/heads/for-pci

Thanks

Yinghai

2009-11-30 07:10:52

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Yinghai Lu wrote:
> split the two patches in to 9 for easy review...
>
> please check
>
> 1. boot time:
>
> BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to the bridge, but stop
> assigning resource to the device under that bridge, because the device need big resource.
>
> so patches (first 6) are trying to
> a. pci assign unassign and record the failed device resource.
> b. clear the BIOS assigned resource of the parent bridge of fail device
> c. go back and call pci assign unsigned
> d. if it still fail, will go up more bridges. and clear and try again.
>
> v2: Jesse doesn't like it is in find_free_bus_resource...
> try to move out of pci_bus_size_bridges loop.
> v3: add pci_setup_bridge calling after pci_bridge_release_not_used_res.
> only clear release those res for x86.
> v4: Bjorn want to release use dev instead of bus.
> v5: Kenji pointed out it will have problem with several level bridge.
> so let only handle leaf bridge.
> v6: address Kenji's request (new pci_bus_release...). and change applying order
> move back release to pci_assign_unassigned_resource
> v7: change functions name pci_bus_release_unused_bridge_res according to Jesse
> v8: address Eric's concern, only overwrite leaf bridge resource that is not big
> enough need to do it in two steps, and first step recore the failed res,
> and don't touch bridge res that programmed by firmware. second step will
> try to release bridge resource that is too small at first.
> v9: refresh to be applied after bjorn's patch, and remove trick about save
> size and restore resource second try.
> v11:add pci=try=5, about more try to change more bridge
> v12:not shrink pci bridge resource
>
> 2. hotplug:
> BIOS separate IO range between several IOHs, and on some slots, BIOS assign the resource to every bridge. (8M)
> but when insert one card that big resource, the card can not get resource. because kernel will not touch the
> bridge resource.
>
> so patches (last 3) are trying to
> a. assign resource to devices with that slot. and record fail devices
> b. if there is some failed, will clear sepcifically io port of bridge, or mmio of bridge, or mmio pref of bridge.
> c. try to assign the parent bridge of the slot.
>
> v2: address Alex's concern about pci remove/rescan feature about
> pci_setup_bridge changes.
> v3: Kenji pointed out that pci_config_slot need to be called before
> pci_bus_add_devices()
> v4: move out pci_is_enabled checkout of pci_setup_bridge()
> v5: change the applying sequence.
> v6: change the functions name according to Jesse
> v8: address Eric's concern, only overwrite leaf bridge resource that is not
> big enough
> v9: refresh to be applied after bjorn's patch, and remove trick about save
> size and restore resource second try.
> v10:alex found need to have export for pci_assign_unassigned_bridge_resources
> v11: pass check_leaf with pci_bus_release_unused_bridge_res
>
> -v13: change resource_list to resource_list_x, to save size and flags aside, otherwise grandchild res will
> get confused with son's res as could be used
>

My system doesn't boot (hangup) with the following error messages
from Fusion MPT SAS driver. Those are different from the one
captured with the previous version of your patches.


Fusion MPT SAS Host driver 3.04.12
mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
mptbase: ioc0: Initiating bringup
mptbase: ioc0: WARNING - Unexpected doorbell active!
mptbase: ioc0: ERROR - Failed to come READY after reset! IocState=f0000000
mptbase: ioc0: WARNING - ResetHistory bit failed to clear!
mptbase: ioc0: ERROR - Diagnostic reset FAILED! (ffffffffh)
mptbase: ioc0: WARNING - NOT READY WARNING!
mptbase: ioc0: ERROR - didn't initialize properly! (-1)
mptsas: probe of 0000:09:00.0 failed with error -1
mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
mptbase: ioc1: Initiating bringup
mptbase: ioc1: WARNING - Unexpected doorbell active!
mptbase: ioc1: ERROR - Failed to come READY after reset! IocState=f0000000
mptbase: ioc1: WARNING - ResetHistory bit failed to clear!
mptbase: ioc1: ERROR - Diagnostic reset FAILED! (ffffffffh)
mptbase: ioc1: WARNING - NOT READY WARNING!
mptbase: ioc1: ERROR - didn't initialize properly! (-1)
mptsas: probe of 0000:0a:00.0 failed with error -1


I'll send the whole boot log privately.

Thanks,
Kenji Kaneshige

2009-11-30 07:15:52

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Kenji Kaneshige wrote:
> My system doesn't boot (hangup) with the following error messages
> from Fusion MPT SAS driver. Those are different from the one
> captured with the previous version of your patches.
>
>
> Fusion MPT SAS Host driver 3.04.12
> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
> mptbase: ioc0: Initiating bringup
> mptbase: ioc0: WARNING - Unexpected doorbell active!
> mptbase: ioc0: ERROR - Failed to come READY after reset! IocState=f0000000
> mptbase: ioc0: WARNING - ResetHistory bit failed to clear!
> mptbase: ioc0: ERROR - Diagnostic reset FAILED! (ffffffffh)
> mptbase: ioc0: WARNING - NOT READY WARNING!
> mptbase: ioc0: ERROR - didn't initialize properly! (-1)
> mptsas: probe of 0000:09:00.0 failed with error -1
> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
> mptbase: ioc1: Initiating bringup
> mptbase: ioc1: WARNING - Unexpected doorbell active!
> mptbase: ioc1: ERROR - Failed to come READY after reset! IocState=f0000000
> mptbase: ioc1: WARNING - ResetHistory bit failed to clear!
> mptbase: ioc1: ERROR - Diagnostic reset FAILED! (ffffffffh)
> mptbase: ioc1: WARNING - NOT READY WARNING!
> mptbase: ioc1: ERROR - didn't initialize properly! (-1)
> mptsas: probe of 0000:0a:00.0 failed with error -1
>
>
> I'll send the whole boot log privately.

thanks, at the same time please try "debug pci=try=1"

Yinghai

2009-11-30 07:26:18

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Yinghai Lu wrote:
> Kenji Kaneshige wrote:
>> My system doesn't boot (hangup) with the following error messages
>> from Fusion MPT SAS driver. Those are different from the one
>> captured with the previous version of your patches.
>>
>>
>> Fusion MPT SAS Host driver 3.04.12
>> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
>> mptbase: ioc0: Initiating bringup
>> mptbase: ioc0: WARNING - Unexpected doorbell active!
>> mptbase: ioc0: ERROR - Failed to come READY after reset! IocState=f0000000
>> mptbase: ioc0: WARNING - ResetHistory bit failed to clear!
>> mptbase: ioc0: ERROR - Diagnostic reset FAILED! (ffffffffh)
>> mptbase: ioc0: WARNING - NOT READY WARNING!
>> mptbase: ioc0: ERROR - didn't initialize properly! (-1)
>> mptsas: probe of 0000:09:00.0 failed with error -1
>> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
>> mptbase: ioc1: Initiating bringup
>> mptbase: ioc1: WARNING - Unexpected doorbell active!
>> mptbase: ioc1: ERROR - Failed to come READY after reset! IocState=f0000000
>> mptbase: ioc1: WARNING - ResetHistory bit failed to clear!
>> mptbase: ioc1: ERROR - Diagnostic reset FAILED! (ffffffffh)
>> mptbase: ioc1: WARNING - NOT READY WARNING!
>> mptbase: ioc1: ERROR - didn't initialize properly! (-1)
>> mptsas: probe of 0000:0a:00.0 failed with error -1
>>
>>
>> I'll send the whole boot log privately.
>
> thanks, at the same time please try "debug pci=try=1"
>

I confirmed system boot up without errors by "pci=try=1".
I'll capture the bootlog with "debug" option and send it
to you soon.

Thanks,
Kenji Kaneshige

2009-11-30 07:44:53

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> My system doesn't boot (hangup) with the following error messages
>>> from Fusion MPT SAS driver. Those are different from the one
>>> captured with the previous version of your patches.
>>>
>>>
>>> Fusion MPT SAS Host driver 3.04.12
>>> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
>>> mptbase: ioc0: Initiating bringup
>>> mptbase: ioc0: WARNING - Unexpected doorbell active!
>>> mptbase: ioc0: ERROR - Failed to come READY after reset!
>>> IocState=f0000000
>>> mptbase: ioc0: WARNING - ResetHistory bit failed to clear!
>>> mptbase: ioc0: ERROR - Diagnostic reset FAILED! (ffffffffh)
>>> mptbase: ioc0: WARNING - NOT READY WARNING!
>>> mptbase: ioc0: ERROR - didn't initialize properly! (-1)
>>> mptsas: probe of 0000:09:00.0 failed with error -1
>>> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
>>> mptbase: ioc1: Initiating bringup
>>> mptbase: ioc1: WARNING - Unexpected doorbell active!
>>> mptbase: ioc1: ERROR - Failed to come READY after reset!
>>> IocState=f0000000
>>> mptbase: ioc1: WARNING - ResetHistory bit failed to clear!
>>> mptbase: ioc1: ERROR - Diagnostic reset FAILED! (ffffffffh)
>>> mptbase: ioc1: WARNING - NOT READY WARNING!
>>> mptbase: ioc1: ERROR - didn't initialize properly! (-1)
>>> mptsas: probe of 0000:0a:00.0 failed with error -1
>>>
>>>
>>> I'll send the whole boot log privately.
>>
>> thanks, at the same time please try "debug pci=try=1"
>>
>
> I confirmed system boot up without errors by "pci=try=1".
> I'll capture the bootlog with "debug" option and send it
> to you soon.

thanks.

please send one only have "debug" only.

Yinghai

2009-11-30 08:21:14

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> My system doesn't boot (hangup) with the following error messages
>>> from Fusion MPT SAS driver. Those are different from the one
>>> captured with the previous version of your patches.
>>>
>>>
>>> Fusion MPT SAS Host driver 3.04.12
>>> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
>>> mptbase: ioc0: Initiating bringup
>>> mptbase: ioc0: WARNING - Unexpected doorbell active!
>>> mptbase: ioc0: ERROR - Failed to come READY after reset!
>>> IocState=f0000000
>>> mptbase: ioc0: WARNING - ResetHistory bit failed to clear!
>>> mptbase: ioc0: ERROR - Diagnostic reset FAILED! (ffffffffh)
>>> mptbase: ioc0: WARNING - NOT READY WARNING!
>>> mptbase: ioc0: ERROR - didn't initialize properly! (-1)
>>> mptsas: probe of 0000:09:00.0 failed with error -1
>>> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
>>> mptbase: ioc1: Initiating bringup
>>> mptbase: ioc1: WARNING - Unexpected doorbell active!
>>> mptbase: ioc1: ERROR - Failed to come READY after reset!
>>> IocState=f0000000
>>> mptbase: ioc1: WARNING - ResetHistory bit failed to clear!
>>> mptbase: ioc1: ERROR - Diagnostic reset FAILED! (ffffffffh)
>>> mptbase: ioc1: WARNING - NOT READY WARNING!
>>> mptbase: ioc1: ERROR - didn't initialize properly! (-1)
>>> mptsas: probe of 0000:0a:00.0 failed with error -1
>>>
>>>
>>> I'll send the whole boot log privately.
>>
>> thanks, at the same time please try "debug pci=try=1"
>>
>
> I confirmed system boot up without errors by "pci=try=1".
> I'll capture the bootlog with "debug" option and send it
> to you soon.

Please also try "debug pci=try=7"
it should work too.

Thanks

Yinghai

2009-11-30 08:44:46

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Yinghai Lu wrote:
> Kenji Kaneshige wrote:
>> Yinghai Lu wrote:
>>> Kenji Kaneshige wrote:
>>>> My system doesn't boot (hangup) with the following error messages
>>>> from Fusion MPT SAS driver. Those are different from the one
>>>> captured with the previous version of your patches.
>>>>
>>>>
>>>> Fusion MPT SAS Host driver 3.04.12
>>>> mptsas 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
>>>> mptbase: ioc0: Initiating bringup
>>>> mptbase: ioc0: WARNING - Unexpected doorbell active!
>>>> mptbase: ioc0: ERROR - Failed to come READY after reset!
>>>> IocState=f0000000
>>>> mptbase: ioc0: WARNING - ResetHistory bit failed to clear!
>>>> mptbase: ioc0: ERROR - Diagnostic reset FAILED! (ffffffffh)
>>>> mptbase: ioc0: WARNING - NOT READY WARNING!
>>>> mptbase: ioc0: ERROR - didn't initialize properly! (-1)
>>>> mptsas: probe of 0000:09:00.0 failed with error -1
>>>> mptsas 0000:0a:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
>>>> mptbase: ioc1: Initiating bringup
>>>> mptbase: ioc1: WARNING - Unexpected doorbell active!
>>>> mptbase: ioc1: ERROR - Failed to come READY after reset!
>>>> IocState=f0000000
>>>> mptbase: ioc1: WARNING - ResetHistory bit failed to clear!
>>>> mptbase: ioc1: ERROR - Diagnostic reset FAILED! (ffffffffh)
>>>> mptbase: ioc1: WARNING - NOT READY WARNING!
>>>> mptbase: ioc1: ERROR - didn't initialize properly! (-1)
>>>> mptsas: probe of 0000:0a:00.0 failed with error -1
>>>>
>>>>
>>>> I'll send the whole boot log privately.
>>> thanks, at the same time please try "debug pci=try=1"
>>>
>> I confirmed system boot up without errors by "pci=try=1".
>> I'll capture the bootlog with "debug" option and send it
>> to you soon.
>
> Please also try "debug pci=try=7"
> it should work too.
>

I confirmed it works.
I will send the whole boot log soon.

Thanks,
Kenji Kaneshige

2009-12-16 20:55:23

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

On Fri, 27 Nov 2009 23:34:30 -0800
Yinghai Lu <[email protected]> wrote:

> split the two patches in to 9 for easy review...
>
> please check
>
> 1. boot time:
>
> BIOS separate IO range between several IOHs, and on some slots, BIOS
> assign the resource to the bridge, but stop assigning resource to the
> device under that bridge, because the device need big resource.
>
> so patches (first 6) are trying to
> a. pci assign unassign and record the failed device resource.
> b. clear the BIOS assigned resource of the parent bridge of fail
> device c. go back and call pci assign unsigned
> d. if it still fail, will go up more bridges. and clear and try again.
>
> v2: Jesse doesn't like it is in find_free_bus_resource...
> try to move out of pci_bus_size_bridges loop.
> v3: add pci_setup_bridge calling after
> pci_bridge_release_not_used_res. only clear release those res for x86.
> v4: Bjorn want to release use dev instead of bus.
> v5: Kenji pointed out it will have problem with several level bridge.
> so let only handle leaf bridge.
> v6: address Kenji's request (new pci_bus_release...). and change
> applying order move back release to pci_assign_unassigned_resource
> v7: change functions name pci_bus_release_unused_bridge_res according
> to Jesse v8: address Eric's concern, only overwrite leaf bridge
> resource that is not big enough need to do it in two steps, and first
> step recore the failed res, and don't touch bridge res that
> programmed by firmware. second step will try to release bridge
> resource that is too small at first. v9: refresh to be applied after
> bjorn's patch, and remove trick about save size and restore resource
> second try. v11:add pci=try=5, about more try to change more bridge
> v12:not shrink pci bridge resource
>
> 2. hotplug:
> BIOS separate IO range between several IOHs, and on some slots, BIOS
> assign the resource to every bridge. (8M) but when insert one card
> that big resource, the card can not get resource. because kernel will
> not touch the bridge resource.
>
> so patches (last 3) are trying to
> a. assign resource to devices with that slot. and record fail devices
> b. if there is some failed, will clear sepcifically io port of
> bridge, or mmio of bridge, or mmio pref of bridge. c. try to assign
> the parent bridge of the slot.
>
> v2: address Alex's concern about pci remove/rescan feature about
> pci_setup_bridge changes.
> v3: Kenji pointed out that pci_config_slot need to be called before
> pci_bus_add_devices()
> v4: move out pci_is_enabled checkout of pci_setup_bridge()
> v5: change the applying sequence.
> v6: change the functions name according to Jesse
> v8: address Eric's concern, only overwrite leaf bridge resource that
> is not big enough
> v9: refresh to be applied after bjorn's patch, and remove trick about
> save size and restore resource second try.
> v10:alex found need to have export for
> pci_assign_unassigned_bridge_resources v11: pass check_leaf with
> pci_bus_release_unused_bridge_res
>
> -v13: change resource_list to resource_list_x, to save size and flags
> aside, otherwise grandchild res will get confused with son's res as
> could be used

Patches 4-6 make me the most nervous, since they have the potential of
really changing our resource allocation (hopefully for the better) on
many platforms.

Linus, can you check them out and see if you're ok with the direction?

Patches 7-9 seem like they've recieved some review from Alex and
Kenji-san, but I don't see acks or reviewed-bys on them.

Alex and Kenji-san, are you ok with them assuming the previous patches
or something like them go upstream?

Thanks,
--
Jesse Barnes, Intel Open Source Technology Center

2009-12-16 21:12:05

by Alex Chiang

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Hi Yinghai,

* Jesse Barnes <[email protected]>:
> >
> > -v13: change resource_list to resource_list_x, to save size and flags
> > aside, otherwise grandchild res will get confused with son's res as
> > could be used

One difficulty that I have in reviewing is the multiple patch
revisions buried deep within a thread.

Blame my small brain for not being able to keep track of all
the revision-as-mail-responses in one coherent body of work.

> Patches 4-6 make me the most nervous, since they have the potential of
> really changing our resource allocation (hopefully for the better) on
> many platforms.
>
> Linus, can you check them out and see if you're ok with the direction?
>
> Patches 7-9 seem like they've recieved some review from Alex and
> Kenji-san, but I don't see acks or reviewed-bys on them.
>
> Alex and Kenji-san, are you ok with them assuming the previous patches
> or something like them go upstream?

Can you please repost your next revision (after taking Jesse's
review comments) in a new thread?

Thank you.
/ac

2009-12-16 22:22:49

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Alex Chiang wrote:
> Hi Yinghai,
>
> * Jesse Barnes <[email protected]>:
>>> -v13: change resource_list to resource_list_x, to save size and flags
>>> aside, otherwise grandchild res will get confused with son's res as
>>> could be used
>
> One difficulty that I have in reviewing is the multiple patch
> revisions buried deep within a thread.
>
> Blame my small brain for not being able to keep track of all
> the revision-as-mail-responses in one coherent body of work.
>
>> Patches 4-6 make me the most nervous, since they have the potential of
>> really changing our resource allocation (hopefully for the better) on
>> many platforms.
>>
>> Linus, can you check them out and see if you're ok with the direction?
>>
>> Patches 7-9 seem like they've recieved some review from Alex and
>> Kenji-san, but I don't see acks or reviewed-bys on them.
>>
>> Alex and Kenji-san, are you ok with them assuming the previous patches
>> or something like them go upstream?
>
> Can you please repost your next revision (after taking Jesse's
> review comments) in a new thread?

ok.

YH

2009-12-16 22:29:08

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

Alex Chiang wrote:
> Hi Yinghai,
>
> * Jesse Barnes <[email protected]>:
>>> -v13: change resource_list to resource_list_x, to save size and flags
>>> aside, otherwise grandchild res will get confused with son's res as
>>> could be used
>
> One difficulty that I have in reviewing is the multiple patch
> revisions buried deep within a thread.
>
> Blame my small brain for not being able to keep track of all
> the revision-as-mail-responses in one coherent body of work.
>
>> Patches 4-6 make me the most nervous, since they have the potential of
>> really changing our resource allocation (hopefully for the better) on
>> many platforms.
>>
>> Linus, can you check them out and see if you're ok with the direction?
>>
>> Patches 7-9 seem like they've recieved some review from Alex and
>> Kenji-san, but I don't see acks or reviewed-bys on them.
>>
>> Alex and Kenji-san, are you ok with them assuming the previous patches
>> or something like them go upstream?
>
> Can you please repost your next revision (after taking Jesse's
> review comments) in a new thread?
>

can you check


git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-2.6-yinghai.git >> master

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-2.6-yinghai.git;a=shortlog;h=refs/heads/master

i rebased them to linus tree 12-12-2009.

Thanks

Yinghai

2009-12-16 22:44:52

by Alex Chiang

[permalink] [raw]
Subject: Re: [PATCH 0/9] pci: update pci bridge resource to get more big range for devices under it - v13

* Yinghai Lu <[email protected]>:
> Alex Chiang wrote:
> > * Jesse Barnes <[email protected]>:
> >>
> >> Patches 7-9 seem like they've recieved some review from Alex
> >> and Kenji-san, but I don't see acks or reviewed-bys on them.
> >>
> >> Alex and Kenji-san, are you ok with them assuming the
> >> previous patches or something like them go upstream?
> >
> > Can you please repost your next revision (after taking
> > Jesse's review comments) in a new thread?
> >
>
> can you check
>
> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-2.6-yinghai.git
> >> master
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-2.6-yinghai.git;a=shortlog;h=refs/heads/master
>
> i rebased them to linus tree 12-12-2009.

Well, yes, I can read those, but now I have to try and guess
which patches are 7-9 that Jesse asked me to review.

One reason that it's hard to review your patches is because there
are many revisions, buried deep within threads. It's not easy for
reviewers to a) keep track of all them or b) keep track of how
they inter-relate.

I might be following one subthread, see N revisions for patch x/y
as replies in there, keep track of the changes mentally, and then
keep all that state in my mind as I navigate around in the thread
to try and review patch x+1/y.

Then, trying to figure out how x, x+1, x+2 are all related into
one coherent change is difficult.

In the future, for PCI patch series, when you have to make
revisions, please repost as a new thread every time. This will
make life much easier for reviewers.

Thanks,
/ac