[Skiboot] [PATCH 05/13] hw/npu2: Don't repopulate NPU devices in NVLink init path

[Andrew Donnellan]

On 14/12/18 4:14 pm, Alexey Kardashevskiy wrote:
>> -static uint32_t npu_allocate_bdfn(struct npu2 *p, uint32_t group)
>> +static uint32_t npu_allocate_bdfn(struct npu2 *p, uint32_t group, int size)
> 
> 
> Why this change?

Because we're no longer incrementing total_devices as we go through the 
loop. This function originally used total_devices as a counter of how 
many devices have been setup so far.

> 
> 
>>   {
>>   	int i;
>>   	int bdfn = (group << 3);
>>   
>> -	for (i = 0; i < p->total_devices; i++) {
>> +	for (i = 0; i < size; i++) {
>>   		if ((p->devices[i].bdfn & 0xf8) == (bdfn & 0xf8))
>>   			bdfn++;
>>   	}
>> @@ -1657,45 +1657,25 @@ static uint32_t npu_allocate_bdfn(struct npu2 *p, uint32_t group)
>>   	return bdfn;
>>   }
>>   
>> -static void npu2_populate_devices(struct npu2 *p,
>> -				  struct dt_node *dn)
>> +static void npu2_configure_devices(struct npu2 *p)
>>   {
>>   	struct npu2_dev *dev;
>> -	struct dt_node *npu2_dn, *link;
>> -	uint32_t npu_phandle, index = 0;
>> -
>> -	/*
>> -	 * Get the npu node which has the links which we expand here
>> -	 * into pci like devices attached to our emulated phb.
>> -	 */
>> -	npu_phandle = dt_prop_get_u32(dn, "ibm,npcq");
>> -	npu2_dn = dt_find_by_phandle(dt_root, npu_phandle);
>> -	assert(npu2_dn);
>> +	uint32_t index = 0;
>>   
>> -	/* Walk the link at x nodes to initialize devices */
>> -	p->total_devices = 0;
>> -	p->phb_nvlink.scan_map = 0;
>> -	dt_for_each_compatible(npu2_dn, link, "ibm,npu-link") {
>> +	for (index = 0; index < p->total_devices; index++) {
>>   		uint32_t group_id;
>>   
>>   		dev = &p->devices[index];
>> -		dev->type = NPU2_DEV_TYPE_NVLINK;
>> -		dev->npu = p;
>> -		dev->dt_node = link;
>> -		dev->link_index = dt_prop_get_u32(link, "ibm,npu-link-index");
>> -		dev->brick_index = dev->link_index;
>> +		if (dev->type != NPU2_DEV_TYPE_NVLINK)
>> +			continue;
>>   
>> -		group_id = dt_prop_get_u32(link, "ibm,npu-group-id");
>> -		dev->bdfn = npu_allocate_bdfn(p, group_id);
>> +		group_id = dt_prop_get_u32(dev->dt_node, "ibm,npu-group-id");
>> +		dev->bdfn = npu_allocate_bdfn(p, group_id, index);
>>   
>>   		/* This must be done after calling
>>   		 * npu_allocate_bdfn() */
>> -		p->total_devices++;
>>   		p->phb_nvlink.scan_map |= 0x1 << ((dev->bdfn & 0xf8) >> 3);
>>   
>> -		dev->pl_xscom_base = dt_prop_get_u64(link, "ibm,npu-phy");
>> -		dev->lane_mask = dt_prop_get_u32(link, "ibm,npu-lane-mask");
>> -
>>   		/* Initialize PCI virtual device */
>>   		dev->nvlink.pvd = pci_virt_add_device(&p->phb_nvlink, dev->bdfn, 0x100, dev);
>>   		if (dev->nvlink.pvd) {
>> @@ -1703,8 +1683,6 @@ static void npu2_populate_devices(struct npu2 *p,
>>   				0x1 << ((dev->nvlink.pvd->bdfn & 0xf8) >> 3);
>>   			npu2_populate_cfg(dev);
>>   		}
>> -
>> -		index++;
>>   	}
>>   }
>>   
>> @@ -1899,13 +1877,14 @@ void npu2_nvlink_create_phb(struct npu2 *npu, struct dt_node *dn)
>>   	npu->phb_nvlink.dt_node = dn;
>>   	npu->phb_nvlink.ops = &npu_ops;
>>   	npu->phb_nvlink.phb_type = phb_type_npu_v2;
>> +	npu->phb_nvlink.scan_map = 0;
> 
> 
> Is this really needed?

Probably not, that should be zero at allocation anyway

> 
> 
>>   	init_lock(&npu->lock);
>>   	init_lock(&npu->phb_nvlink.lock);
>>   	list_head_init(&npu->phb_nvlink.devices);
>>   	list_head_init(&npu->phb_nvlink.virt_devices);
>>   
>>   	npu2_setup_irqs(npu);
>> -	npu2_populate_devices(npu, dn);
>> +	npu2_configure_devices(npu);
>>   	npu2_add_interrupt_map(npu, dn);
>>   	npu2_add_phb_properties(npu);
>>   
>>
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited