From 82e3e767c21fef2b1b38868e20eb4e470a1e38e3 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Wed, 10 Nov 2010 10:26:07 -0700 Subject: PCI: fix pci_bus_alloc_resource() hang, prefer positive decode When a PCI bus has two resources with the same start/end, e.g., pci_bus 0000:04: resource 2 [mem 0xd0000000-0xd7ffffff pref] pci_bus 0000:04: resource 7 [mem 0xd0000000-0xd7ffffff] the previous pci_bus_find_resource_prev() implementation would alternate between them forever: pci_bus_find_resource_prev(... [mem 0xd0000000-0xd7ffffff pref]) returns [mem 0xd0000000-0xd7ffffff] pci_bus_find_resource_prev(... [mem 0xd0000000-0xd7ffffff]) returns [mem 0xd0000000-0xd7ffffff pref] pci_bus_find_resource_prev(... [mem 0xd0000000-0xd7ffffff pref]) returns [mem 0xd0000000-0xd7ffffff] ... This happened because there was no ordering between two resources with the same start and end. A resource that had the same start and end as the cursor, but was not itself the cursor, was considered to be before the cursor. This patch fixes the hang by making a fixed ordering between any two resources. In addition, it tries to allocate from positively decoded regions before using any subtractively decoded resources. This means we will use a positive decode region before a subtractive decode one, even if it means using a smaller address. Reference: https://bugzilla.kernel.org/show_bug.cgi?id=22062 Reported-by: Borislav Petkov Tested-by: Borislav Petkov Acked-by: Linus Torvalds Signed-off-by: Bjorn Helgaas Signed-off-by: Jesse Barnes --- drivers/pci/bus.c | 70 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 21 deletions(-) (limited to 'drivers/pci/bus.c') diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 5624db8c9ad0..003170ea2e39 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -64,17 +64,57 @@ void pci_bus_remove_resources(struct pci_bus *bus) } } +static bool pci_bus_resource_better(struct resource *res1, bool pos1, + struct resource *res2, bool pos2) +{ + /* If exactly one is positive decode, always prefer that one */ + if (pos1 != pos2) + return pos1 ? true : false; + + /* Prefer the one that contains the highest address */ + if (res1->end != res2->end) + return (res1->end > res2->end) ? true : false; + + /* Otherwise, prefer the one with highest "center of gravity" */ + if (res1->start != res2->start) + return (res1->start > res2->start) ? true : false; + + /* Otherwise, choose one arbitrarily (but consistently) */ + return (res1 > res2) ? true : false; +} + +static bool pci_bus_resource_positive(struct pci_bus *bus, struct resource *res) +{ + struct pci_bus_resource *bus_res; + + /* + * This relies on the fact that pci_bus.resource[] refers to P2P or + * CardBus bridge base/limit registers, which are always positively + * decoded. The pci_bus.resources list contains host bridge or + * subtractively decoded resources. + */ + list_for_each_entry(bus_res, &bus->resources, list) { + if (bus_res->res == res) + return (bus_res->flags & PCI_SUBTRACTIVE_DECODE) ? + false : true; + } + return true; +} + /* - * Find the highest-address bus resource below the cursor "res". If the - * cursor is NULL, return the highest resource. + * Find the next-best bus resource after the cursor "res". If the cursor is + * NULL, return the best resource. "Best" means that we prefer positive + * decode regions over subtractive decode, then those at higher addresses. */ static struct resource *pci_bus_find_resource_prev(struct pci_bus *bus, unsigned int type, struct resource *res) { + bool res_pos, r_pos, prev_pos = false; struct resource *r, *prev = NULL; int i; + res_pos = pci_bus_resource_positive(bus, res); pci_bus_for_each_resource(bus, r, i) { if (!r) continue; @@ -82,26 +122,14 @@ static struct resource *pci_bus_find_resource_prev(struct pci_bus *bus, if ((r->flags & IORESOURCE_TYPE_BITS) != type) continue; - /* If this resource is at or past the cursor, skip it */ - if (res) { - if (r == res) - continue; - if (r->end > res->end) - continue; - if (r->end == res->end && r->start > res->start) - continue; + r_pos = pci_bus_resource_positive(bus, r); + if (!res || pci_bus_resource_better(res, res_pos, r, r_pos)) { + if (!prev || pci_bus_resource_better(r, r_pos, + prev, prev_pos)) { + prev = r; + prev_pos = r_pos; + } } - - if (!prev) - prev = r; - - /* - * A small resource is higher than a large one that ends at - * the same address. - */ - if (r->end > prev->end || - (r->end == prev->end && r->start > prev->start)) - prev = r; } return prev; -- cgit v1.2.3 From ac57cd5ee1935d1e60de86d75f13f377775f1c96 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 16 Dec 2010 10:38:20 -0700 Subject: Revert "PCI: fix pci_bus_alloc_resource() hang, prefer positive decode" This reverts commit 82e3e767c21fef2b1b38868e20eb4e470a1e38e3. We're going back to considering bus resources in the order we found them (in _CRS order, when we're using _CRS), so we don't need to define any ordering. Acked-by: H. Peter Anvin Signed-off-by: Bjorn Helgaas Signed-off-by: Jesse Barnes --- drivers/pci/bus.c | 70 +++++++++++++++++-------------------------------------- 1 file changed, 21 insertions(+), 49 deletions(-) (limited to 'drivers/pci/bus.c') diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 003170ea2e39..5624db8c9ad0 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -64,57 +64,17 @@ void pci_bus_remove_resources(struct pci_bus *bus) } } -static bool pci_bus_resource_better(struct resource *res1, bool pos1, - struct resource *res2, bool pos2) -{ - /* If exactly one is positive decode, always prefer that one */ - if (pos1 != pos2) - return pos1 ? true : false; - - /* Prefer the one that contains the highest address */ - if (res1->end != res2->end) - return (res1->end > res2->end) ? true : false; - - /* Otherwise, prefer the one with highest "center of gravity" */ - if (res1->start != res2->start) - return (res1->start > res2->start) ? true : false; - - /* Otherwise, choose one arbitrarily (but consistently) */ - return (res1 > res2) ? true : false; -} - -static bool pci_bus_resource_positive(struct pci_bus *bus, struct resource *res) -{ - struct pci_bus_resource *bus_res; - - /* - * This relies on the fact that pci_bus.resource[] refers to P2P or - * CardBus bridge base/limit registers, which are always positively - * decoded. The pci_bus.resources list contains host bridge or - * subtractively decoded resources. - */ - list_for_each_entry(bus_res, &bus->resources, list) { - if (bus_res->res == res) - return (bus_res->flags & PCI_SUBTRACTIVE_DECODE) ? - false : true; - } - return true; -} - /* - * Find the next-best bus resource after the cursor "res". If the cursor is - * NULL, return the best resource. "Best" means that we prefer positive - * decode regions over subtractive decode, then those at higher addresses. + * Find the highest-address bus resource below the cursor "res". If the + * cursor is NULL, return the highest resource. */ static struct resource *pci_bus_find_resource_prev(struct pci_bus *bus, unsigned int type, struct resource *res) { - bool res_pos, r_pos, prev_pos = false; struct resource *r, *prev = NULL; int i; - res_pos = pci_bus_resource_positive(bus, res); pci_bus_for_each_resource(bus, r, i) { if (!r) continue; @@ -122,14 +82,26 @@ static struct resource *pci_bus_find_resource_prev(struct pci_bus *bus, if ((r->flags & IORESOURCE_TYPE_BITS) != type) continue; - r_pos = pci_bus_resource_positive(bus, r); - if (!res || pci_bus_resource_better(res, res_pos, r, r_pos)) { - if (!prev || pci_bus_resource_better(r, r_pos, - prev, prev_pos)) { - prev = r; - prev_pos = r_pos; - } + /* If this resource is at or past the cursor, skip it */ + if (res) { + if (r == res) + continue; + if (r->end > res->end) + continue; + if (r->end == res->end && r->start > res->start) + continue; } + + if (!prev) + prev = r; + + /* + * A small resource is higher than a large one that ends at + * the same address. + */ + if (r->end > prev->end || + (r->end == prev->end && r->start > prev->start)) + prev = r; } return prev; -- cgit v1.2.3 From 6db45b76eaa08133187f2cb44d496de7e9503aa8 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 16 Dec 2010 10:38:36 -0700 Subject: Revert "PCI: allocate bus resources from the top down" This reverts commit b126b4703afa4010b161784a43650337676dd03b. We're going back to the old behavior of allocating from bus resources in _CRS order. Acked-by: H. Peter Anvin Signed-off-by: Bjorn Helgaas Signed-off-by: Jesse Barnes --- drivers/pci/bus.c | 53 +++++------------------------------------------------ 1 file changed, 5 insertions(+), 48 deletions(-) (limited to 'drivers/pci/bus.c') diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 5624db8c9ad0..69546e9213dd 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -64,49 +64,6 @@ void pci_bus_remove_resources(struct pci_bus *bus) } } -/* - * Find the highest-address bus resource below the cursor "res". If the - * cursor is NULL, return the highest resource. - */ -static struct resource *pci_bus_find_resource_prev(struct pci_bus *bus, - unsigned int type, - struct resource *res) -{ - struct resource *r, *prev = NULL; - int i; - - pci_bus_for_each_resource(bus, r, i) { - if (!r) - continue; - - if ((r->flags & IORESOURCE_TYPE_BITS) != type) - continue; - - /* If this resource is at or past the cursor, skip it */ - if (res) { - if (r == res) - continue; - if (r->end > res->end) - continue; - if (r->end == res->end && r->start > res->start) - continue; - } - - if (!prev) - prev = r; - - /* - * A small resource is higher than a large one that ends at - * the same address. - */ - if (r->end > prev->end || - (r->end == prev->end && r->start > prev->start)) - prev = r; - } - - return prev; -} - /** * pci_bus_alloc_resource - allocate a resource from a parent bus * @bus: PCI bus @@ -132,10 +89,9 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res, resource_size_t), void *alignf_data) { - int ret = -ENOMEM; + int i, ret = -ENOMEM; struct resource *r; resource_size_t max = -1; - unsigned int type = res->flags & IORESOURCE_TYPE_BITS; type_mask |= IORESOURCE_IO | IORESOURCE_MEM; @@ -143,9 +99,10 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res, if (!(res->flags & IORESOURCE_MEM_64)) max = PCIBIOS_MAX_MEM_32; - /* Look for space at highest addresses first */ - r = pci_bus_find_resource_prev(bus, type, NULL); - for ( ; r; r = pci_bus_find_resource_prev(bus, type, r)) { + pci_bus_for_each_resource(bus, r, i) { + if (!r) + continue; + /* type_mask must match */ if ((res->flags ^ r->flags) & type_mask) continue; -- cgit v1.2.3