[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