[PATCH 4/4] pci: don't hide hard to understand sanity check in size calculation

Uwe Kleine-König u.kleine-koenig at pengutronix.de
Fri Apr 13 16:30:53 PDT 2018


pci_size() had a check that made it return 0 (i.e. an error) in some
cases. It took me a while to work out in which situations exactly this
check triggered: If writing 0xfffffffe to the BAR register doesn't
change it's value and the masked value isn't in the form 0b1....10...0.

This patch replaces the check in pci_size() by a similar check in
setup_device() where it is more expected to happen. This allows to
simplify pci_size() considerably.

The new check is a bit more strict as it doesn't consider the value the
BAR register holds initially and bails out iff the maximal base value
isn't in the expected format.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
---
 drivers/pci/pci.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 837e17aa8397..72689560d598 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -144,18 +144,10 @@ static struct pci_dev *alloc_pci_dev(void)
 	return dev;
 }
 
-static u32 pci_size(u32 base, u32 maxbase, u32 mask)
+static u32 pci_size(u32 maxbase)
 {
-	u32 size = maxbase & mask;
-	if (!size)
-		return 0;
-
-	size = (size & ~(size-1)) - 1;
-
-	if (base == maxbase && ((base | size) & mask) != mask)
-		return 0;
-
-	return size + 1;
+	/* isolate right-most set bit */
+	return maxbase & ~(maxbase - 1);
 }
 
 
@@ -169,7 +161,7 @@ static void setup_device(struct pci_dev *dev, int max_bar)
 			      cmd & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY));
 
 	for (bar = 0; bar < max_bar; bar++) {
-		u32 orig, barval, size, mask;
+		u32 orig, barval, maxbase, size, mask;
 		const char *type;
 		int restype;
 		unsigned long resflags;
@@ -206,13 +198,19 @@ static void setup_device(struct pci_dev *dev, int max_bar)
 			last = &last_mem;
 		}
 
-		size = pci_size(orig, barval, mask);
-		if (!size) {
-			pr_debug(" pbar%d: bad %s size (orig=%08x, barval=%08x, type=%s)\n",
-				 bar, type, orig, barval, type);
+		maxbase = barval & mask;
+
+		/*
+		 * if maxbase isn't in the form 0b1...10...0 there is something
+		 * fishy
+		 */
+		if ((maxbase | (maxbase - 1)) != (u32)-1 || maxbase == 0) {
+			pr_debug(" pbar%d: !! barval=%08x maxbase=0x%08x\n", bar, barval, maxbase);
 			continue;
 		}
 
+		size = pci_size(maxbase);
+
 		if (ALIGN(*last, size) + size > dev->bus->resource[restype]->end) {
 			pr_debug(" pbar%d: !! type=%s allocbase=0x%08x + size=0x%08x > resource_end=%pa (barval=%08x)\n",
 				 bar, type, ALIGN(*last, size), size, &dev->bus->resource[restype]->end, barval);
-- 
2.16.3




More information about the barebox mailing list