[Skiboot] [PATCH] pci: Use a fixed numbering of PHBs on OPAL and improve log consistency

Benjamin Herrenschmidt benh at kernel.crashing.org
Thu Sep 10 14:35:52 AEST 2015


On P8, we calculate the OPAL ID of the PHB as a function of the physical
chip number and PHB index on that chip. P7 continues using "allocated"
numbers for now.

We also consistently print the PHB ID as a 4-digit hex number which
facilitates decoding it, and print the chip:index location in the probe
code to make it easier to correlate log entries.

Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
---
 core/pci.c      | 81 ++++++++++++++++++++++++++++++++++-----------------------
 hw/p5ioc2-phb.c |  6 ++---
 hw/p7ioc-phb.c  |  6 ++---
 hw/phb3.c       | 21 ++++++++++-----
 include/pci.h   |  2 +-
 5 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/core/pci.c b/core/pci.c
index 26ed48c..9457e1f 100644
--- a/core/pci.c
+++ b/core/pci.c
@@ -22,27 +22,26 @@
 #include <device.h>
 #include <fsp.h>
 
-/* The eeh event code will need updating if this is ever increased to
- * support more than 64 phbs */
-static struct phb *phbs[64];
+#define MAX_PHB_ID	256
+static struct phb *phbs[MAX_PHB_ID];
 
 #define PCITRACE(_p, _bdfn, fmt, a...) \
-	prlog(PR_TRACE, "PHB%d:%02x:%02x.%x " fmt,	\
+	prlog(PR_TRACE, "PHB#%04x:%02x:%02x.%x " fmt,	\
 	      (_p)->opal_id,				\
 	      ((_bdfn) >> 8) & 0xff,			\
 	      ((_bdfn) >> 3) & 0x1f, (_bdfn) & 0x7, ## a)
 #define PCIDBG(_p, _bdfn, fmt, a...) \
-	prlog(PR_DEBUG, "PHB%d:%02x:%02x.%x " fmt,	\
+	prlog(PR_DEBUG, "PHB#%04x:%02x:%02x.%x " fmt,	\
 	      (_p)->opal_id,				\
 	      ((_bdfn) >> 8) & 0xff,			\
 	      ((_bdfn) >> 3) & 0x1f, (_bdfn) & 0x7, ## a)
 #define PCINOTICE(_p, _bdfn, fmt, a...) \
-	prlog(PR_NOTICE, "PHB%d:%02x:%02x.%x " fmt,	\
+	prlog(PR_NOTICE, "PHB#%04x:%02x:%02x.%x " fmt,	\
 	      (_p)->opal_id,				\
 	      ((_bdfn) >> 8) & 0xff,			\
 	      ((_bdfn) >> 3) & 0x1f, (_bdfn) & 0x7, ## a)
 #define PCIERR(_p, _bdfn, fmt, a...) \
-	prlog(PR_ERR, "PHB%d:%02x:%02x.%x " fmt,	\
+	prlog(PR_ERR, "PHB#%04x:%02x:%02x.%x " fmt,	\
 	      (_p)->opal_id,				\
 	      ((_bdfn) >> 8) & 0xff,			\
 	      ((_bdfn) >> 3) & 0x1f, (_bdfn) & 0x7, ## a)
@@ -536,7 +535,7 @@ static uint8_t pci_scan(struct phb *phb, uint8_t bus, uint8_t max_bus,
 		 *    bridge (when we need to give aligned powers of two's
 		 *    on P7IOC). If is is set to false, we just adjust the
 		 *    subordinate bus number based on what we probed.
-		 *    
+		 *
 		 */
 		max_bus = save_max;
 		next_bus = phb->ops->choose_bus(phb, pd, next_bus,
@@ -771,28 +770,38 @@ static void pci_scan_phb(void *data)
 	pci_walk_dev(phb, pci_configure_mps, NULL);
 }
 
-int64_t pci_register_phb(struct phb *phb)
+int64_t pci_register_phb(struct phb *phb, int opal_id)
 {
-	int64_t rc = OPAL_SUCCESS;
-	unsigned int i;
-
-	/* This is called at init time in non-concurrent way, so no lock needed */
-	for (i = 0; i < ARRAY_SIZE(phbs); i++)
-		if (!phbs[i])
-			break;
-	if (i >= ARRAY_SIZE(phbs)) {
-		prerror("PHB: Failed to find a free ID slot\n");
-		rc = OPAL_RESOURCE;
+	/* The user didn't specify an opal_id, allocate one */
+	if (opal_id < 0) {
+		/* This is called at init time in non-concurrent way, so no lock needed */
+		for (opal_id = 0; opal_id < ARRAY_SIZE(phbs); opal_id++)
+			if (!phbs[opal_id])
+				break;
+		if (opal_id >= ARRAY_SIZE(phbs)) {
+			prerror("PHB: Failed to find a free ID slot\n");
+			return OPAL_RESOURCE;
+		}
 	} else {
-		phbs[i] = phb;
-		phb->opal_id = i;
-		dt_add_property_cells(phb->dt_node, "ibm,opal-phbid",
-				      0, phb->opal_id);
-		PCIDBG(phb, 0, "PCI: Registered PHB\n");
+		if (opal_id >= ARRAY_SIZE(phbs)) {
+			prerror("PHB: ID %d out of range !\n", opal_id);
+			return OPAL_PARAMETER;
+		}
+		/* The user did specify an opal_id, check it's free */
+		if (phbs[opal_id]) {
+			prerror("PHB: Duplicate registration of ID %d\n", opal_id);
+			return OPAL_PARAMETER;
+		}
 	}
+
+	phbs[opal_id] = phb;
+	phb->opal_id = opal_id;
+	dt_add_property_cells(phb->dt_node, "ibm,opal-phbid", 0, phb->opal_id);
+	PCIDBG(phb, 0, "PCI: Registered PHB\n");
+
 	list_head_init(&phb->devices);
 
-	return rc;
+	return OPAL_SUCCESS;
 }
 
 int64_t pci_unregister_phb(struct phb *phb)
@@ -1172,10 +1181,15 @@ static void pci_add_loc_code(struct dt_node *np, struct pci_device *pd)
 	uint8_t class, sub;
 	uint8_t pos, len;
 
-	/* Look for a parent with a slot-location-code */
-	while (p && !blcode) {
-		blcode = dt_prop_get_def(p, "ibm,slot-location-code", NULL);
-		p = p->parent;
+	/* If there is a label assigned to the function, use it on openpower machines */
+	if (pd->slot_info && strlen(pd->slot_info->label) && !fsp_present()) {
+		blcode = pd->slot_info->label;
+	} else {
+		/* Look for a parent with a slot-location-code */
+		while (p && !blcode) {
+			blcode = dt_prop_get_def(p, "ibm,slot-location-code", NULL);
+			p = p->parent;
+		}
 	}
 	if (!blcode)
 		return;
@@ -1333,8 +1347,8 @@ static void pci_add_one_node(struct phb *phb, struct pci_device *pd,
 	 *  - ...
 	 */
 
-	/* Add slot properties if needed */
-	if (pd->slot_info)
+	/* Add slot properties if needed and iff this is a bridge */
+	if (pd->slot_info && pd->is_bridge)
 		pci_add_slot_properties(phb, pd->slot_info, np);
 
 	/* Make up location code */
@@ -1432,9 +1446,11 @@ void pci_reset(void)
 
 static void pci_do_jobs(void (*fn)(void *))
 {
-	void *jobs[ARRAY_SIZE(phbs)];
+	struct cpu_job **jobs;
 	int i;
 
+	jobs = zalloc(sizeof(struct cpu_job *) * ARRAY_SIZE(phbs));
+	assert(jobs);
 	for (i = 0; i < ARRAY_SIZE(phbs); i++) {
 		if (!phbs[i]) {
 			jobs[i] = NULL;
@@ -1457,6 +1473,7 @@ static void pci_do_jobs(void (*fn)(void *))
 
 		cpu_wait_job(jobs[i], true);
 	}
+	free(jobs);
 }
 
 void pci_init_slots(void)
diff --git a/hw/p5ioc2-phb.c b/hw/p5ioc2-phb.c
index 06c2cc2..1c9dbb6 100644
--- a/hw/p5ioc2-phb.c
+++ b/hw/p5ioc2-phb.c
@@ -25,9 +25,9 @@
 #include <interrupts.h>
 #include <ccan/str/str.h>
 
-#define PHBDBG(p, fmt, a...)	prlog(PR_DEBUG, "PHB%d: " fmt, \
+#define PHBDBG(p, fmt, a...)	prlog(PR_DEBUG, "PHB#%04x: " fmt, \
 				      (p)->phb.opal_id, ## a)
-#define PHBERR(p, fmt, a...)	prlog(PR_ERR, "PHB%d: " fmt, \
+#define PHBERR(p, fmt, a...)	prlog(PR_ERR, "PHB#%04x: " fmt, \
 				      (p)->phb.opal_id, ## a)
 
 /*
@@ -1197,7 +1197,7 @@ void p5ioc2_phb_setup(struct p5ioc2 *ioc, struct p5ioc2_phb *p,
 	/* We register the PHB before we initialize it so we
 	 * get a useful OPAL ID for it
 	 */
-	pci_register_phb(&p->phb);
+	pci_register_phb(&p->phb, -1);
 
 	/* Platform additional setup */
 	if (platform.pci_setup_phb)
diff --git a/hw/p7ioc-phb.c b/hw/p7ioc-phb.c
index 5167832..4ea0369 100644
--- a/hw/p7ioc-phb.c
+++ b/hw/p7ioc-phb.c
@@ -26,9 +26,9 @@
 #include <opal.h>
 #include <ccan/str/str.h>
 
-#define PHBDBG(p, fmt, a...)	prlog(PR_DEBUG, "PHB%d: " fmt, \
+#define PHBDBG(p, fmt, a...)	prlog(PR_DEBUG, "PHB#%04x: " fmt, \
 				      (p)->phb.opal_id, ## a)
-#define PHBERR(p, fmt, a...)	prlog(PR_ERR, "PHB%d: " fmt, \
+#define PHBERR(p, fmt, a...)	prlog(PR_ERR, "PHB#%04x: " fmt, \
 				      (p)->phb.opal_id, ## a)
 
 /* Helper to select an IODA table entry */
@@ -2929,7 +2929,7 @@ void p7ioc_phb_setup(struct p7ioc *ioc, uint8_t index)
 	/* We register the PHB before we initialize it so we
 	 * get a useful OPAL ID for it
 	 */
-	pci_register_phb(&p->phb);
+	pci_register_phb(&p->phb, -1);
 
 	/* Platform additional setup */
 	if (platform.pci_setup_phb)
diff --git a/hw/phb3.c b/hw/phb3.c
index 5fd0130..ac31b48 100644
--- a/hw/phb3.c
+++ b/hw/phb3.c
@@ -53,11 +53,11 @@
 
 static void phb3_init_hw(struct phb3 *p, bool first_init);
 
-#define PHBDBG(p, fmt, a...)	prlog(PR_DEBUG, "PHB%d: " fmt, \
+#define PHBDBG(p, fmt, a...)	prlog(PR_DEBUG, "PHB#%04x: " fmt, \
 				      (p)->phb.opal_id, ## a)
-#define PHBINF(p, fmt, a...)	prlog(PR_INFO, "PHB%d: " fmt, \
+#define PHBINF(p, fmt, a...)	prlog(PR_INFO, "PHB#%04x: " fmt, \
 				      (p)->phb.opal_id, ## a)
-#define PHBERR(p, fmt, a...)	prlog(PR_ERR, "PHB%d: " fmt, \
+#define PHBERR(p, fmt, a...)	prlog(PR_ERR, "PHB#%04x: " fmt, \
 				      (p)->phb.opal_id, ## a)
 
 /*
@@ -4209,6 +4209,8 @@ static void phb3_create(struct dt_node *np)
 	struct phb3 *p = zalloc(sizeof(struct phb3));
 	size_t lane_eq_len;
 	struct dt_node *iplp;
+	struct proc_chip *chip;
+	int opal_id;
 	char *path;
 
 	assert(p);
@@ -4260,13 +4262,20 @@ static void phb3_create(struct dt_node *np)
 	p->has_link = false;
 
 	/* We register the PHB before we initialize it so we
-	 * get a useful OPAL ID for it
+	 * get a useful OPAL ID for it. We use a different numbering here
+	 * between Naples and Venice/Murano in order to leave room for the
+	 * NPU on Naples.
 	 */
-	pci_register_phb(&p->phb);
+	chip = get_chip(NULL);
+	if (chip && chip->type == PROC_CHIP_P8_NAPLES)
+		opal_id = p->chip_id * 8 + p->index;
+	else
+		opal_id = p->chip_id * 4 + p->index;
+	pci_register_phb(&p->phb, opal_id);
 
 	/* Hello ! */
 	path = dt_get_path(np);
-	PHBINF(p, "Found %s @%p\n", path, p->regs);
+	PHBINF(p, "Found %s @[%d:%d]\n", path, p->chip_id, p->index);
 	PHBINF(p, "  M32 [0x%016llx..0x%016llx]\n",
 	       p->mm1_base, p->mm1_base + p->mm1_size - 1);
 	PHBINF(p, "  M64 [0x%016llx..0x%016llx]\n",
diff --git a/include/pci.h b/include/pci.h
index 2385163..764ae5b 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -493,7 +493,7 @@ extern struct pci_device *pci_find_dev(struct phb *phb, uint16_t bdfn);
 extern void pci_restore_bridge_buses(struct phb *phb);
 
 /* Manage PHBs */
-extern int64_t pci_register_phb(struct phb *phb);
+extern int64_t pci_register_phb(struct phb *phb, int opal_id);
 extern int64_t pci_unregister_phb(struct phb *phb);
 extern struct phb *pci_get_phb(uint64_t phb_id);
 static inline void pci_put_phb(struct phb *phb __unused) { }




More information about the Skiboot mailing list