Message ID | 30cda5baa5bafbb1070fcd2b1bab1150a7b42b89.1547168645.git-series.andrew.donnellan@au1.ibm.com |
---|---|
State | Changes Requested |
Headers | show |
Series | Support OpenCAPI and NVLink devices on same NPU on Witherspoon | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | warning | master/apply_patch Patch failed to apply |
snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
On 11/01/2019 12:09, Andrew Donnellan wrote: > We currently maintain the state of the NTL and GENID BARs for each device > in npu2_dev::bars[2], with the first BAR being the NTL BAR and the second > being the GENID BAR. > > Given we only have two BARs, it's a lot clearer to get rid of the array and > just have two struct members, ntl_bar and genid_bar. > > No functional change. > > Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > --- > v1->v2: > - split this patch out > --- > hw/npu2-opencapi.c | 36 ++++++++++++++++----------------- > hw/npu2.c | 52 ++++++++++++++++++++++------------------------- > include/npu2.h | 3 ++- > 3 files changed, 45 insertions(+), 46 deletions(-) > > diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c > index ffa4d706bbad..8dac552adea8 100644 > --- a/hw/npu2-opencapi.c > +++ b/hw/npu2-opencapi.c > @@ -768,18 +768,18 @@ static void setup_afu_mmio_bars(uint32_t gcid, uint32_t scom_base, > uint64_t reg; > > prlog(PR_DEBUG, "OCAPI: %s: Setup AFU MMIO BARs\n", __func__); > - dev->bars[0].type = NPU_OCAPI_MMIO; > - dev->bars[0].index = dev->brick_index; > - dev->bars[0].reg = NPU2_REG_OFFSET(stack, 0, offset); > - dev->bars[0].enabled = true; > - npu2_get_bar(gcid, &dev->bars[0]); > + dev->ntl_bar.type = NPU_OCAPI_MMIO; > + dev->ntl_bar.index = dev->brick_index; > + dev->ntl_bar.reg = NPU2_REG_OFFSET(stack, 0, offset); > + dev->ntl_bar.enabled = true; > + npu2_get_bar(gcid, &dev->ntl_bar); > > prlog(PR_DEBUG, "OCAPI: AFU MMIO set to %llx, size %llx\n", > - dev->bars[0].base, dev->bars[0].size); > - npu2_write_bar(NULL, &dev->bars[0], gcid, scom_base); > + dev->ntl_bar.base, dev->ntl_bar.size); > + npu2_write_bar(NULL, &dev->ntl_bar, gcid, scom_base); > > - reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_ADDR, 0ull, dev->bars[0].base >> 16); > - reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_SIZE, reg, ilog2(dev->bars[0].size >> 16)); > + reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_ADDR, 0ull, dev->ntl_bar.base >> 16); > + reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_SIZE, reg, ilog2(dev->ntl_bar.size >> 16)); > prlog(PR_DEBUG, "OCAPI: PA translation %llx\n", reg); > npu2_scom_write(gcid, scom_base, > NPU2_REG_OFFSET(stack, NPU2_BLOCK_CTL, > @@ -795,13 +795,13 @@ static void setup_afu_config_bars(uint32_t gcid, uint32_t scom_base, > int stack_num = stack - NPU2_STACK_STCK_0; > > prlog(PR_DEBUG, "OCAPI: %s: Setup AFU Config BARs\n", __func__); > - dev->bars[1].type = NPU_GENID; > - dev->bars[1].index = stack_num; > - dev->bars[1].reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); > - dev->bars[1].enabled = true; > - npu2_get_bar(gcid, &dev->bars[1]); > - prlog(PR_DEBUG, "OCAPI: Assigning GENID BAR: %016llx\n", dev->bars[1].base); > - npu2_write_bar(NULL, &dev->bars[1], gcid, scom_base); > + dev->genid_bar.type = NPU_GENID; > + dev->genid_bar.index = stack_num; > + dev->genid_bar.reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); > + dev->genid_bar.enabled = true; > + npu2_get_bar(gcid, &dev->genid_bar); > + prlog(PR_DEBUG, "OCAPI: Assigning GENID BAR: %016llx\n", dev->genid_bar.base); > + npu2_write_bar(NULL, &dev->genid_bar, gcid, scom_base); > } > > static void otl_enabletx(uint32_t gcid, uint32_t scom_base, > @@ -1261,7 +1261,7 @@ static int64_t npu2_opencapi_pcicfg_read(struct phb *phb, uint32_t bdfn, > if (rc) > return rc; > > - genid_base = dev->bars[1].base + > + genid_base = dev->genid_bar.base + > (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); > > cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; > @@ -1319,7 +1319,7 @@ static int64_t npu2_opencapi_pcicfg_write(struct phb *phb, uint32_t bdfn, > if (rc) > return rc; > > - genid_base = dev->bars[1].base + > + genid_base = dev->genid_bar.base + > (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); > > cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; > diff --git a/hw/npu2.c b/hw/npu2.c > index 8176a81262f0..60619f381301 100644 > --- a/hw/npu2.c > +++ b/hw/npu2.c > @@ -114,7 +114,6 @@ static int64_t npu2_cfg_write_cmd(void *dev, > { > struct pci_virt_device *pvd = dev; > struct npu2_dev *ndev = pvd->data; > - struct npu2_bar *ntl_npu_bar, *genid_npu_bar; > bool enabled; > > if (!write) > @@ -130,11 +129,9 @@ static int64_t npu2_cfg_write_cmd(void *dev, > * one GENID BAR, which is exposed via the first brick. > */ > enabled = !!(*data & PCI_CFG_CMD_MEM_EN); > - ntl_npu_bar = &ndev->bars[0]; > - genid_npu_bar = &ndev->bars[1]; > > - ntl_npu_bar->enabled = enabled; > - npu2_write_bar(ndev->npu, ntl_npu_bar, 0, 0); > + ndev->ntl_bar.enabled = enabled; > + npu2_write_bar(ndev->npu, &ndev->ntl_bar, 0, 0); > > /* > * Enable/disable the GENID BAR. Two bricks share one GENID > @@ -142,13 +139,14 @@ static int64_t npu2_cfg_write_cmd(void *dev, > * track the enables separately. > */ > if (NPU2DEV_BRICK(ndev)) > - genid_npu_bar->enabled1 = enabled; > + ndev->genid_bar.enabled1 = enabled; > else > - genid_npu_bar->enabled0 = enabled; > + ndev->genid_bar.enabled0 = enabled; > > /* Enable the BAR if either device requests it enabled, otherwise disable it */ > - genid_npu_bar->enabled = genid_npu_bar->enabled0 || genid_npu_bar->enabled1; > - npu2_write_bar(ndev->npu, genid_npu_bar, 0, 0); > + ndev->genid_bar.enabled = ndev->genid_bar.enabled0 || > + ndev->genid_bar.enabled1; > + npu2_write_bar(ndev->npu, &ndev->genid_bar, 0, 0); > > return OPAL_PARTIAL; > } > @@ -1606,7 +1604,7 @@ static void npu2_populate_cfg(struct npu2_dev *dev) > PCI_VIRT_CFG_INIT_RO(pvd, PCI_CFG_CACHE_LINE_SIZE, 4, 0x00800000); > > /* 0x10/14 - BAR#0, NTL BAR */ > - bar = &dev->bars[0]; > + bar = &dev->ntl_bar; > PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR0, 4, > (bar->base & 0xfffffff0) | bar->flags, > 0x0000000f, 0x00000000); > @@ -1617,7 +1615,7 @@ static void npu2_populate_cfg(struct npu2_dev *dev) > npu2_dev_cfg_bar, bar); > > /* 0x18/1c - BAR#1, GENID BAR */ > - bar = &dev->bars[1]; > + bar = &dev->genid_bar; > if (NPU2DEV_BRICK(dev) == 0) > PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, (bar->base & 0xfffffff0) | > bar->flags, > @@ -1696,7 +1694,6 @@ static void npu2_populate_devices(struct npu2 *p, > p->phb_nvlink.scan_map = 0; > dt_for_each_compatible(npu2_dn, link, "ibm,npu-link") { > uint32_t group_id; > - struct npu2_bar *npu2_bar; > > dev = &p->devices[index]; > dev->type = NPU2_DEV_TYPE_NVLINK; > @@ -1718,28 +1715,29 @@ static void npu2_populate_devices(struct npu2 *p, > > /* Populate BARs. BAR0/1 is the NTL bar. */ > stack = NPU2_STACK_STCK_0 + NPU2DEV_STACK(dev); > - npu2_bar = &dev->bars[0]; > - npu2_bar->type = NPU_NTL; > - npu2_bar->index = dev->brick_index; > - npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2DEV_BRICK(dev) == 0 ? > - NPU2_NTL0_BAR : NPU2_NTL1_BAR); > - npu2_get_bar(p->chip_id, npu2_bar); > + dev->ntl_bar.type = NPU_NTL; > + dev->ntl_bar.index = dev->brick_index; > + dev->ntl_bar.reg = NPU2_REG_OFFSET(stack, 0, > + NPU2DEV_BRICK(dev) == 0 ? > + NPU2_NTL0_BAR : > + NPU2_NTL1_BAR); > + npu2_get_bar(p->chip_id, &dev->ntl_bar); > > - dev->bars[0].flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; > + dev->ntl_bar.flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; > > /* BAR2/3 is the GENID bar. */ > - npu2_bar = &dev->bars[1]; > - npu2_bar->type = NPU_GENID; > - npu2_bar->index = NPU2DEV_STACK(dev); > - npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); > - npu2_get_bar(p->chip_id, npu2_bar); > + dev->genid_bar.type = NPU_GENID; > + dev->genid_bar.index = NPU2DEV_STACK(dev); > + dev->genid_bar.reg = NPU2_REG_OFFSET(stack, 0, > + NPU2_GENID_BAR); > + npu2_get_bar(p->chip_id, &dev->genid_bar); > > /* The GENID is a single physical BAR that we split > * for each emulated device */ > - npu2_bar->size = 0x10000; > + dev->genid_bar.size = 0x10000; > if (NPU2DEV_BRICK(dev)) > - npu2_bar->base += 0x10000; > - dev->bars[1].flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; > + dev->genid_bar.base += 0x10000; > + dev->genid_bar.flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; > > /* Initialize PCI virtual device */ > dev->nvlink.pvd = pci_virt_add_device(&p->phb_nvlink, dev->bdfn, 0x100, dev); > diff --git a/include/npu2.h b/include/npu2.h > index b2a3ead858d2..c2ba4370c063 100644 > --- a/include/npu2.h > +++ b/include/npu2.h > @@ -116,7 +116,8 @@ struct npu2_dev { > uint32_t brick_index; > uint64_t pl_xscom_base; > struct dt_node *dt_node; > - struct npu2_bar bars[2]; > + struct npu2_bar ntl_bar; > + struct npu2_bar genid_bar; > struct npu2 *npu; > > uint32_t bdfn; >
diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c index ffa4d706bbad..8dac552adea8 100644 --- a/hw/npu2-opencapi.c +++ b/hw/npu2-opencapi.c @@ -768,18 +768,18 @@ static void setup_afu_mmio_bars(uint32_t gcid, uint32_t scom_base, uint64_t reg; prlog(PR_DEBUG, "OCAPI: %s: Setup AFU MMIO BARs\n", __func__); - dev->bars[0].type = NPU_OCAPI_MMIO; - dev->bars[0].index = dev->brick_index; - dev->bars[0].reg = NPU2_REG_OFFSET(stack, 0, offset); - dev->bars[0].enabled = true; - npu2_get_bar(gcid, &dev->bars[0]); + dev->ntl_bar.type = NPU_OCAPI_MMIO; + dev->ntl_bar.index = dev->brick_index; + dev->ntl_bar.reg = NPU2_REG_OFFSET(stack, 0, offset); + dev->ntl_bar.enabled = true; + npu2_get_bar(gcid, &dev->ntl_bar); prlog(PR_DEBUG, "OCAPI: AFU MMIO set to %llx, size %llx\n", - dev->bars[0].base, dev->bars[0].size); - npu2_write_bar(NULL, &dev->bars[0], gcid, scom_base); + dev->ntl_bar.base, dev->ntl_bar.size); + npu2_write_bar(NULL, &dev->ntl_bar, gcid, scom_base); - reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_ADDR, 0ull, dev->bars[0].base >> 16); - reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_SIZE, reg, ilog2(dev->bars[0].size >> 16)); + reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_ADDR, 0ull, dev->ntl_bar.base >> 16); + reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_SIZE, reg, ilog2(dev->ntl_bar.size >> 16)); prlog(PR_DEBUG, "OCAPI: PA translation %llx\n", reg); npu2_scom_write(gcid, scom_base, NPU2_REG_OFFSET(stack, NPU2_BLOCK_CTL, @@ -795,13 +795,13 @@ static void setup_afu_config_bars(uint32_t gcid, uint32_t scom_base, int stack_num = stack - NPU2_STACK_STCK_0; prlog(PR_DEBUG, "OCAPI: %s: Setup AFU Config BARs\n", __func__); - dev->bars[1].type = NPU_GENID; - dev->bars[1].index = stack_num; - dev->bars[1].reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); - dev->bars[1].enabled = true; - npu2_get_bar(gcid, &dev->bars[1]); - prlog(PR_DEBUG, "OCAPI: Assigning GENID BAR: %016llx\n", dev->bars[1].base); - npu2_write_bar(NULL, &dev->bars[1], gcid, scom_base); + dev->genid_bar.type = NPU_GENID; + dev->genid_bar.index = stack_num; + dev->genid_bar.reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); + dev->genid_bar.enabled = true; + npu2_get_bar(gcid, &dev->genid_bar); + prlog(PR_DEBUG, "OCAPI: Assigning GENID BAR: %016llx\n", dev->genid_bar.base); + npu2_write_bar(NULL, &dev->genid_bar, gcid, scom_base); } static void otl_enabletx(uint32_t gcid, uint32_t scom_base, @@ -1261,7 +1261,7 @@ static int64_t npu2_opencapi_pcicfg_read(struct phb *phb, uint32_t bdfn, if (rc) return rc; - genid_base = dev->bars[1].base + + genid_base = dev->genid_bar.base + (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; @@ -1319,7 +1319,7 @@ static int64_t npu2_opencapi_pcicfg_write(struct phb *phb, uint32_t bdfn, if (rc) return rc; - genid_base = dev->bars[1].base + + genid_base = dev->genid_bar.base + (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; diff --git a/hw/npu2.c b/hw/npu2.c index 8176a81262f0..60619f381301 100644 --- a/hw/npu2.c +++ b/hw/npu2.c @@ -114,7 +114,6 @@ static int64_t npu2_cfg_write_cmd(void *dev, { struct pci_virt_device *pvd = dev; struct npu2_dev *ndev = pvd->data; - struct npu2_bar *ntl_npu_bar, *genid_npu_bar; bool enabled; if (!write) @@ -130,11 +129,9 @@ static int64_t npu2_cfg_write_cmd(void *dev, * one GENID BAR, which is exposed via the first brick. */ enabled = !!(*data & PCI_CFG_CMD_MEM_EN); - ntl_npu_bar = &ndev->bars[0]; - genid_npu_bar = &ndev->bars[1]; - ntl_npu_bar->enabled = enabled; - npu2_write_bar(ndev->npu, ntl_npu_bar, 0, 0); + ndev->ntl_bar.enabled = enabled; + npu2_write_bar(ndev->npu, &ndev->ntl_bar, 0, 0); /* * Enable/disable the GENID BAR. Two bricks share one GENID @@ -142,13 +139,14 @@ static int64_t npu2_cfg_write_cmd(void *dev, * track the enables separately. */ if (NPU2DEV_BRICK(ndev)) - genid_npu_bar->enabled1 = enabled; + ndev->genid_bar.enabled1 = enabled; else - genid_npu_bar->enabled0 = enabled; + ndev->genid_bar.enabled0 = enabled; /* Enable the BAR if either device requests it enabled, otherwise disable it */ - genid_npu_bar->enabled = genid_npu_bar->enabled0 || genid_npu_bar->enabled1; - npu2_write_bar(ndev->npu, genid_npu_bar, 0, 0); + ndev->genid_bar.enabled = ndev->genid_bar.enabled0 || + ndev->genid_bar.enabled1; + npu2_write_bar(ndev->npu, &ndev->genid_bar, 0, 0); return OPAL_PARTIAL; } @@ -1606,7 +1604,7 @@ static void npu2_populate_cfg(struct npu2_dev *dev) PCI_VIRT_CFG_INIT_RO(pvd, PCI_CFG_CACHE_LINE_SIZE, 4, 0x00800000); /* 0x10/14 - BAR#0, NTL BAR */ - bar = &dev->bars[0]; + bar = &dev->ntl_bar; PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR0, 4, (bar->base & 0xfffffff0) | bar->flags, 0x0000000f, 0x00000000); @@ -1617,7 +1615,7 @@ static void npu2_populate_cfg(struct npu2_dev *dev) npu2_dev_cfg_bar, bar); /* 0x18/1c - BAR#1, GENID BAR */ - bar = &dev->bars[1]; + bar = &dev->genid_bar; if (NPU2DEV_BRICK(dev) == 0) PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, (bar->base & 0xfffffff0) | bar->flags, @@ -1696,7 +1694,6 @@ static void npu2_populate_devices(struct npu2 *p, p->phb_nvlink.scan_map = 0; dt_for_each_compatible(npu2_dn, link, "ibm,npu-link") { uint32_t group_id; - struct npu2_bar *npu2_bar; dev = &p->devices[index]; dev->type = NPU2_DEV_TYPE_NVLINK; @@ -1718,28 +1715,29 @@ static void npu2_populate_devices(struct npu2 *p, /* Populate BARs. BAR0/1 is the NTL bar. */ stack = NPU2_STACK_STCK_0 + NPU2DEV_STACK(dev); - npu2_bar = &dev->bars[0]; - npu2_bar->type = NPU_NTL; - npu2_bar->index = dev->brick_index; - npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2DEV_BRICK(dev) == 0 ? - NPU2_NTL0_BAR : NPU2_NTL1_BAR); - npu2_get_bar(p->chip_id, npu2_bar); + dev->ntl_bar.type = NPU_NTL; + dev->ntl_bar.index = dev->brick_index; + dev->ntl_bar.reg = NPU2_REG_OFFSET(stack, 0, + NPU2DEV_BRICK(dev) == 0 ? + NPU2_NTL0_BAR : + NPU2_NTL1_BAR); + npu2_get_bar(p->chip_id, &dev->ntl_bar); - dev->bars[0].flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; + dev->ntl_bar.flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; /* BAR2/3 is the GENID bar. */ - npu2_bar = &dev->bars[1]; - npu2_bar->type = NPU_GENID; - npu2_bar->index = NPU2DEV_STACK(dev); - npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); - npu2_get_bar(p->chip_id, npu2_bar); + dev->genid_bar.type = NPU_GENID; + dev->genid_bar.index = NPU2DEV_STACK(dev); + dev->genid_bar.reg = NPU2_REG_OFFSET(stack, 0, + NPU2_GENID_BAR); + npu2_get_bar(p->chip_id, &dev->genid_bar); /* The GENID is a single physical BAR that we split * for each emulated device */ - npu2_bar->size = 0x10000; + dev->genid_bar.size = 0x10000; if (NPU2DEV_BRICK(dev)) - npu2_bar->base += 0x10000; - dev->bars[1].flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; + dev->genid_bar.base += 0x10000; + dev->genid_bar.flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; /* Initialize PCI virtual device */ dev->nvlink.pvd = pci_virt_add_device(&p->phb_nvlink, dev->bdfn, 0x100, dev); diff --git a/include/npu2.h b/include/npu2.h index b2a3ead858d2..c2ba4370c063 100644 --- a/include/npu2.h +++ b/include/npu2.h @@ -116,7 +116,8 @@ struct npu2_dev { uint32_t brick_index; uint64_t pl_xscom_base; struct dt_node *dt_node; - struct npu2_bar bars[2]; + struct npu2_bar ntl_bar; + struct npu2_bar genid_bar; struct npu2 *npu; uint32_t bdfn;
We currently maintain the state of the NTL and GENID BARs for each device in npu2_dev::bars[2], with the first BAR being the NTL BAR and the second being the GENID BAR. Given we only have two BARs, it's a lot clearer to get rid of the array and just have two struct members, ntl_bar and genid_bar. No functional change. Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> --- v1->v2: - split this patch out --- hw/npu2-opencapi.c | 36 ++++++++++++++++----------------- hw/npu2.c | 52 ++++++++++++++++++++++------------------------- include/npu2.h | 3 ++- 3 files changed, 45 insertions(+), 46 deletions(-)