[PATCH] powerpc/pci: Only do fixed PHB numbering on powernv

Michael Ellerman mpe at ellerman.id.au
Fri Aug 5 16:40:56 AEST 2016


The recent commit 63a72284b159 ("powerpc/pci: Assign fixed PHB number
based on device-tree properties"), added code to read a 64-bit property
from the device tree, and if not found read a 32-bit property (reg).

There was a bug in the 32-bit case, on big endian machines, due to the
use of the 64-bit value to read the 32-bit property. The cast of &prop
means we end up writing to the high 32-bit of prop, leaving the low
32-bits containing whatever junk was on the stack.

If that junk value was non-zero, and < MAX_PHBS, we would end up using
it as the PHB id.

This exposed a second problem, which is that Xorg can't cope with a
domain number > 256.

So for now drop the fallback to reg, and only use "ibm,opal-phbid" for
PHB numbering.

Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
---
 arch/powerpc/kernel/pci-common.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)


This is my bad. Guilherme limited the reg case to pseries only, but I made it
generic. I tested it on G5, Cell etc. which all booted - but that's not really
a good enough test.

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index f93942b4b6a6..f9c2ffaa1884 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -77,29 +77,23 @@ EXPORT_SYMBOL(get_pci_dma_ops);
  */
 static int get_phb_number(struct device_node *dn)
 {
-	int ret, phb_id = -1;
+	int ret, phb_id;
 	u64 prop;
 
 	/*
-	 * Try fixed PHB numbering first, by checking archs and reading
-	 * the respective device-tree properties. Firstly, try powernv by
-	 * reading "ibm,opal-phbid", only present in OPAL environment.
+	 * On powernv machines we should have the "ibm,opal-phbid" property, if
+	 * so use that so that PHB numbers are predictable.
 	 */
 	ret = of_property_read_u64(dn, "ibm,opal-phbid", &prop);
-	if (ret)
-		ret = of_property_read_u32_index(dn, "reg", 1, (u32 *)&prop);
-
-	if (!ret)
+	if (ret == 0) {
 		phb_id = (int)(prop & (MAX_PHBS - 1));
 
-	/* We need to be sure to not use the same PHB number twice. */
-	if ((phb_id >= 0) && !test_and_set_bit(phb_id, phb_bitmap))
-		return phb_id;
+		/* Make sure never to use the same PHB number twice. */
+		if (!test_and_set_bit(phb_id, phb_bitmap))
+			return phb_id;
+	}
 
-	/*
-	 * If not pseries nor powernv, or if fixed PHB numbering tried to add
-	 * the same PHB number twice, then fallback to dynamic PHB numbering.
-	 */
+	/* If fixed PHB numbering failed, fallback to dynamic PHB numbering. */
 	phb_id = find_first_zero_bit(phb_bitmap, MAX_PHBS);
 	BUG_ON(phb_id >= MAX_PHBS);
 	set_bit(phb_id, phb_bitmap);
-- 
2.7.4



More information about the Linuxppc-dev mailing list