[Skiboot] [PATCH 1/2] npu2: Rework phb-index assignments for virtual PHBs

[Frederic Barrat]

Until now, opencapi PHBs were not using the 'ibm,phb-index' property,
as it was thought unnecessary. For nvlink, a phb-index was associated
to the npu when parsing hdat data, and the nvlink PHB was reusing the
same value.

It turns out it helps to have the 'ibm,phb-index' property for
opencapi PHBs after all. Otherwise it can lead to wrong results on
platforms like mihawk when trying to match entries in the slot
table. We end up with an opencapi device inheriting wrong properties
in the device tree, because match_slot_phb_entry() default to
phb-index 0 if it cannot find the property. Though it doesn't seem to
cause any harm, it's wrong and a future patch is expected to start
using the slot table for opencapi, so it needs fixing.

The twist is that with opencapi, we can have multiple virtual PHBs for
a single NPU, at least on P9. Therefore there's no 1-to-1 mapping
between the NPU and PHB index. It doesn't really make sense to
associate a phb-index to a npu.

With this patch, opencapi PHBs created under a NPU use a fixed mapping
for their phb-index, based on the brick index. The range of possible
values is 7 to 12. Because there can only be one nvlink PHB per NPU,
it is always using a phb-index of 7.

A side effect is that 2 virtual PHBs on 2 different chips can have the
same phb-index, which is similar to what happens for 'real' PCI PHBs,
but is different from what was happening on a nvlink-only witherspoon
so far.

This doesn't really affect Axone, where we have a 1-to-1 mapping
between NPU and PHB for nvlink (and it will remain true with
opencapi), but to be consistent, we define the phb-index when creating
the PHB like on P9.

Signed-off-by: Frederic Barrat <fbarrat at linux.ibm.com>
---

Reza, Alistair: see the blurb above about the phb-index of the nvlink
PHBs changing from 7 and 8 on witherspoon, to 7 and 7. My
understanding is that 1) it's not really used on witherspoon and 2)
when it is used, the phb-index is associated to the chip ID, as it's
needed for real PHBs anyway, but I thought I should highlight it.


 hw/npu2-common.c   | 1 -
 hw/npu2-opencapi.c | 2 ++
 hw/npu2.c          | 2 +-
 hw/npu3-nvlink.c   | 2 +-
 include/npu2.h     | 2 +-
 include/npu3.h     | 2 ++
 6 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/npu2-common.c b/hw/npu2-common.c
index 6d5c35af..7326e165 100644
--- a/hw/npu2-common.c
+++ b/hw/npu2-common.c
@@ -530,7 +530,6 @@ static struct npu2 *setup_npu(struct dt_node *dn)
 	npu->index = dt_prop_get_u32(dn, "ibm,npu-index");
 	npu->chip_id = gcid;
 	npu->xscom_base = dt_get_address(dn, 0, NULL);
-	npu->phb_index = dt_prop_get_u32(dn, "ibm,phb-index");
 
 	init_lock(&npu->i2c_lock);
 	npu->i2c_pin_mode = ~0; // input mode by default
diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c
index 9a391bb0..106ce0ac 100644
--- a/hw/npu2-opencapi.c
+++ b/hw/npu2-opencapi.c
@@ -1665,6 +1665,8 @@ static void setup_device(struct npu2_dev *dev)
 	dt_add_property_strings(dn_phb, "device_type", "pciex");
 	dt_add_property(dn_phb, "reg", mm_win, sizeof(mm_win));
 	dt_add_property_cells(dn_phb, "ibm,npu-index", dev->npu->index);
+	dt_add_property_cells(dn_phb, "ibm,phb-index",
+			      NPU2_PHB_INDEX_BASE + dev->brick_index);
 	dt_add_property_cells(dn_phb, "ibm,chip-id", dev->npu->chip_id);
 	dt_add_property_cells(dn_phb, "ibm,xscom-base", dev->npu->xscom_base);
 	dt_add_property_cells(dn_phb, "ibm,npcq", dev->npu->dt_node->phandle);
diff --git a/hw/npu2.c b/hw/npu2.c
index 29c998f6..19ca70a4 100644
--- a/hw/npu2.c
+++ b/hw/npu2.c
@@ -1478,7 +1478,7 @@ int npu2_nvlink_init_npu(struct npu2 *npu)
 				"ibm,ioda2-npu2-phb");
 	dt_add_property_strings(np, "device_type", "pciex");
 	dt_add_property(np, "reg", reg, sizeof(reg));
-	dt_add_property_cells(np, "ibm,phb-index", npu->phb_index);
+	dt_add_property_cells(np, "ibm,phb-index", NPU2_PHB_INDEX_BASE);
 	dt_add_property_cells(np, "ibm,npu-index", npu->index);
 	dt_add_property_cells(np, "ibm,chip-id", npu->chip_id);
 	dt_add_property_cells(np, "ibm,xscom-base", npu->xscom_base);
diff --git a/hw/npu3-nvlink.c b/hw/npu3-nvlink.c
index 7e7a10e8..a4e633e6 100644
--- a/hw/npu3-nvlink.c
+++ b/hw/npu3-nvlink.c
@@ -1461,7 +1461,7 @@ static void npu3_dt_add_props(struct npu3 *npu)
 				"ibm,ioda2-npu2-phb");
 
 	dt_add_property_cells(dn, "ibm,phb-index",
-			      dt_prop_get_u32(npu->dt_node, "ibm,phb-index"));
+			      NPU3_PHB_INDEX_BASE + npu->index);
 	dt_add_property_cells(dn, "ibm,phb-diag-data-size", 0);
 	dt_add_property_cells(dn, "ibm,opal-num-pes", NPU3_MAX_PE_NUM);
 	dt_add_property_cells(dn, "ibm,opal-reserved-pe", NPU3_RESERVED_PE_NUM);
diff --git a/include/npu2.h b/include/npu2.h
index 92b58988..47317c8c 100644
--- a/include/npu2.h
+++ b/include/npu2.h
@@ -53,6 +53,7 @@
 #define NPU2_OCAPI_PE(ndev) ((ndev)->brick_index + NPU2_MAX_PE_NUM)
 
 #define NPU2_LINKS_PER_CHIP 6
+#define NPU2_PHB_INDEX_BASE 7 // enough to avoid conflicts with PCI
 
 /* Link flags */
 #define NPU2_DEV_PCI_LINKED	0x1
@@ -175,7 +176,6 @@ struct npu2 {
 
 	/* NVLink */
 	struct phb	phb_nvlink;
-	uint32_t	phb_index;
 
 	/* OCAPI */
 	uint64_t	i2c_port_id_ocapi;
diff --git a/include/npu3.h b/include/npu3.h
index 1c657f94..417756c4 100644
--- a/include/npu3.h
+++ b/include/npu3.h
@@ -78,6 +78,8 @@ struct npu3_dev {
 	struct npu3_dev_nvlink	nvlink;
 };
 
+#define NPU3_PHB_INDEX_BASE 7 // enough to avoid conflicts with PCI
+
 struct npu3_nvlink {
 	struct phb		phb;
 	uint32_t		ctx_ref[NPU3_XTS_BDF_MAP_MAX];
-- 
2.21.0