[Skiboot] [PATCH v2 03/15] hw/npu2: Replace npu2_dev::bars array with individual BARs
Alexey Kardashevskiy
aik at ozlabs.ru
Thu Jan 17 16:01:04 AEDT 2019
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 at au1.ibm.com>
Reviewed-by: Alexey Kardashevskiy <aik at 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;
>
--
Alexey
More information about the Skiboot
mailing list