Re: [PATCH] pci: Allow very large resource windows

[Kevin Hilman]

Hi Bjorn,

Bjorn Helgaas <bhelgaas@xxxxxxxxxx> writes:

> On Mon, May 19, 2014 at 02:03:14PM +0100, Alan wrote:
>> From: Alan <alan@xxxxxxxxxxxxxxx>
>> 
>> This is needed for some of the Xeon Phi type systems.
>> 
>> Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
>
> I applied this to my pci/resource branch for v3.16.  Nikhil
> posted essentially the same patch a couple years ago, so I added
> his signed-off-by and adopted his use of ARRAY_SIZE() to connect the
> "order > 13" test with the aligns[] declaration.

This patch (with the ARRAY_SIZE changes) has hit next-20140253 (in the
form of commit 272c5a886c57) and according to my bisect, is the cause of
a new boot failure on a 32-bit ARM platform (Marvell Armada 370,
Mirabox, boot log excerpt down below[2].)

While debugging, I found that Alan's original patch (without the
ARRAY_SIZE change) booted just fine so I started looking closely at the
ARRAY_SIZE() change.  After some high-tech printk debugging, I noticed
that order was negative when doing the compare, and found that this hack
got things booting again:

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 47482b27fa12..792db3477fd5 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -966,7 +966,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			/* For bridges size != alignment */
 			align = pci_resource_alignment(dev, r);
 			order = __ffs(align) - 20;
-			if (order >= ARRAY_SIZE(aligns)) {
+			if (order >= (int)ARRAY_SIZE(aligns)) {
 				dev_warn(&dev->dev, "disabling BAR %d: %pR "
 					 "(bad alignment %#llx)\n", i, r,
 					 (unsigned long long) align);

which led me to realize that the ARRAY_SIZE() change converted the >=
into an unsigned compare where before it was a signed one, and as an
unsigned compare, it was always true, resulting in those BARs being
disabled.

Below is a patch[1] which fixes the problem for me, and attempts a more
detailed description of the problem in the changelog.

Kevin

[1]