Skip to content

Commit

Permalink
npu2: Rework phb-index assignments for virtual PHBs
Browse files Browse the repository at this point in the history
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 on P9. There's one PHB per (opencapi) brick. Therefore
there's no 1-to-1 mapping between the NPU and PHB index and it no
longer makes 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.

Reviewed-by: Reza Arbab <arbab@linux.ibm.com>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
  • Loading branch information
fbarrat authored and oohal committed Jan 29, 2020
1 parent 120b1b7 commit da28a66
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 3 deletions.
1 change: 0 additions & 1 deletion hw/npu2-common.c
Expand Up @@ -537,7 +537,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
Expand Down
2 changes: 2 additions & 0 deletions hw/npu2-opencapi.c
Expand Up @@ -1734,6 +1734,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_get_phb_index(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);
Expand Down
2 changes: 1 addition & 1 deletion hw/npu2.c
Expand Up @@ -1484,7 +1484,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_get_phb_index(0));
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);
Expand Down
20 changes: 19 additions & 1 deletion include/npu2.h
Expand Up @@ -178,7 +178,6 @@ struct npu2 {

/* NVLink */
struct phb phb_nvlink;
uint32_t phb_index;

/* OCAPI */
uint64_t i2c_port_id_ocapi;
Expand Down Expand Up @@ -256,4 +255,23 @@ int64_t npu2_set_relaxed_order(struct phb *phb, uint32_t gcid, int pec,

void npu2_opencapi_set_broken(struct npu2 *npu, int brick);

#define NPU2_PHB_INDEX_BASE 7
/* to avoid conflicts with PCI and for historical reasons */

static inline int npu2_get_phb_index(unsigned int brick_index)
{
/*
* There's one virtual PHB per brick with opencapi, so we no
* longer have a 1-to-1 mapping between a NPU and a virtual
* PHB. And we want a static phb-index, as it is needed to use
* a slot table on some platforms. So we associate a per-chip
* phb-index based on the brick index.
*
* nvlink only creates one virtual PHB per chip, so it is
* treated as if using brick 0, which is never used by
* opencapi.
*/
return NPU2_PHB_INDEX_BASE + brick_index;
}

#endif /* __NPU2_H */

0 comments on commit da28a66

Please sign in to comment.