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 <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/bus.c | 70 +++++++++++++++++++++++++++++++++++++----------------
1 files changed, 49 insertions(+), 21 deletions(-)
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 5624db8..003170e 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;
On Wed, 10 Nov 2010 10:26:07 -0700
Bjorn Helgaas <[email protected]> wrote:
>
> 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 <[email protected]>
> Signed-off-by: Bjorn Helgaas <[email protected]>
> ---
How does this one look, Linus? If you're ok with this version I'll
include it in my next pull request.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
On Thu, Nov 11, 2010 at 10:48 AM, Jesse Barnes <[email protected]> wrote:
>
> How does this one look, Linus? ?If you're ok with this version I'll
> include it in my next pull request.
Sure, I'm ok with it.
Linus
On Wed, Nov 10, 2010 at 12:26:07PM -0500, Bjorn Helgaas wrote:
>
> 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
> Signed-off-by: Bjorn Helgaas <[email protected]>
Looks good.
Reported-and-tested-by: Borislav Petkov <[email protected]>
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
On Wed, 10 Nov 2010 10:26:07 -0700
Bjorn Helgaas <[email protected]> wrote:
>
> 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 <[email protected]>
> Signed-off-by: Bjorn Helgaas <[email protected]>
> ---
Applied to my for-linus branch, thanks.
--
Jesse Barnes, Intel Open Source Technology Center