Re: [RFC PATCH 05/29] net/qdma: add device init and uninit functions

[Aman Kumar <aman.kumar@vvdntech.in>]

On 06/07/22 9:05 pm, Stephen Hemminger wrote:
> On Wed,  6 Jul 2022 13:21:55 +0530
> Aman Kumar <aman.kumar@vvdntech.in> wrote:
>
>> +/* parse a sysfs file containing one integer value */
>> +static int parse_sysfs_value(const char *filename, uint32_t *val)
>> +{
>> +	FILE *f;
>> +	char buf[BUFSIZ];
>> +	char *end = NULL;
>> +
>> +	f = fopen(filename, "r");
>> +	if (f == NULL) {
>> +		PMD_DRV_LOG(ERR, "%s(): Failed to open sysfs file %s\n",
>> +				__func__, filename);
>> +		return -1;
>> +	}
>> +
>> +	if (fgets(buf, sizeof(buf), f) == NULL) {
>> +		PMD_DRV_LOG(ERR, "%s(): Failed to read sysfs value %s\n",
>> +			__func__, filename);
>> +		fclose(f);
>> +		return -1;
>> +	}
>> +	*val = (uint32_t)strtoul(buf, &end, 0);
>> +	if ((buf[0] == '\0') || end == NULL || (*end != '\n')) {
>> +		PMD_DRV_LOG(ERR, "%s(): Failed to parse sysfs value %s\n",
>> +				__func__, filename);
>> +		fclose(f);
>> +		return -1;
>> +	}
>> +	fclose(f);
>> +	return 0;
>> +}
> Why reinvent eal_parse_sysfs_value?
I'll rework this and adapt to existing DPDK APIs.
>
>> +/* Split up a pci address into its constituent parts. */
>> +static int parse_pci_addr_format(const char *buf,
>> +		int bufsize, struct rte_pci_addr *addr)
>> +{
>> +	/* first split on ':' */
>> +	union splitaddr {
>> +		struct {
>> +			char *domain;
>> +			char *bus;
>> +			char *devid;
>> +			char *function;
>> +		};
>> +		/* last element-separator is "." not ":" */
>> +		char *str[PCI_FMT_NVAL];
>> +	} splitaddr;
>> +
>> +	char *buf_copy = strndup(buf, bufsize);
>> +	if (buf_copy == NULL) {
>> +		PMD_DRV_LOG(ERR, "Failed to get pci address duplicate copy\n");
>> +		return -1;
>> +	}
>> +
>> +	if (rte_strsplit(buf_copy, bufsize, splitaddr.str, PCI_FMT_NVAL, ':')
>> +			!= PCI_FMT_NVAL - 1) {
>> +		PMD_DRV_LOG(ERR, "Failed to split pci address string\n");
>> +		goto error;
>> +	}
>> +
>> +	/* final split is on '.' between devid and function */
>> +	splitaddr.function = strchr(splitaddr.devid, '.');
>> +	if (splitaddr.function == NULL) {
>> +		PMD_DRV_LOG(ERR, "Failed to split pci devid and function\n");
>> +		goto error;
>> +	}
>> +	*splitaddr.function++ = '\0';
>> +
>> +	/* now convert to int values */
>> +	addr->domain = strtoul(splitaddr.domain, NULL, 16);
>> +	addr->bus = strtoul(splitaddr.bus, NULL, 16);
>> +	addr->devid = strtoul(splitaddr.devid, NULL, 16);
>> +	addr->function = strtoul(splitaddr.function, NULL, 10);
>> +
>> +	free(buf_copy); /* free the copy made with strdup */
>> +	return 0;
>> +
>> +error:
>> +	free(buf_copy);
>> +	return -1;
>> +}
> Looks like you didn't see rte_pci_addr_parse..
I'll rework this and adapt to existing DPDK APIs.
>> +/* Get max pci bus number from the corresponding pci bridge device */
>> +static int get_max_pci_bus_num(uint8_t start_bus, uint8_t *end_bus)
>> +{
>> +	char dirname[PATH_MAX];
>> +	char filename[PATH_MAX];
>> +	char cfgname[PATH_MAX];
>> +	struct rte_pci_addr addr;
>> +	struct dirent *dp;
>> +	uint32_t pci_class_code;
>> +	uint8_t sec_bus_num, sub_bus_num;
>> +	DIR *dir;
>> +	int ret, fd;
>> +
>> +	/* Initialize end bus number to zero */
>> +	*end_bus = 0;
>> +
>> +	/* Open pci devices directory */
>> +	dir = opendir(rte_pci_get_sysfs_path());
>> +	if (dir == NULL) {
>> +		PMD_DRV_LOG(ERR, "%s(): opendir failed\n",
>> +			__func__);
>> +		return -1;
>> +	}
>> +
>> +	while ((dp = readdir(dir)) != NULL) {
>> +		if (dp->d_name[0] == '.')
>> +			continue;
>> +
>> +		/* Split pci address to get bus, devid and function numbers */
>> +		if (parse_pci_addr_format(dp->d_name,
>> +				sizeof(dp->d_name), &addr) != 0)
>> +			continue;
>> +
>> +		snprintf(dirname, sizeof(dirname), "%s/%s",
>> +				rte_pci_get_sysfs_path(), dp->d_name);
>> +
>> +		/* get class code */
>> +		snprintf(filename, sizeof(filename), "%s/class", dirname);
>> +		if (parse_sysfs_value(filename, &pci_class_code) < 0) {
>> +			PMD_DRV_LOG(ERR, "Failed to get pci class code\n");
>> +			goto error;
>> +		}
>> +
>> +		/* Get max pci number from pci bridge device */
>> +		if ((((pci_class_code >> PCI_CONFIG_CLASS_CODE_SHIFT) & 0xFF) ==
>> +				PCI_CONFIG_BRIDGE_DEVICE)) {
>> +			snprintf(cfgname, sizeof(cfgname),
>> +					"%s/config", dirname);
>> +			fd = open(cfgname, O_RDWR);
>> +			if (fd < 0) {
>> +				PMD_DRV_LOG(ERR, "Failed to open %s\n",
>> +					cfgname);
>> +				goto error;
>> +			}
>> +
>> +			/* get secondary bus number */
>> +			ret = pread(fd, &sec_bus_num, sizeof(uint8_t),
>> +						PCI_SECONDARY_BUS);
>> +			if (ret == -1) {
>> +				PMD_DRV_LOG(ERR, "Failed to read secondary bus number\n");
>> +				close(fd);
>> +				goto error;
>> +			}
>> +
>> +			/* get subordinate bus number */
>> +			ret = pread(fd, &sub_bus_num, sizeof(uint8_t),
>> +						PCI_SUBORDINATE_BUS);
>> +			if (ret == -1) {
>> +				PMD_DRV_LOG(ERR, "Failed to read subordinate bus number\n");
>> +				close(fd);
>> +				goto error;
>> +			}
>> +
>> +			/* Get max bus number by checking if given bus number
>> +			 * falls in between secondary and subordinate bus
>> +			 * numbers of this pci bridge device.
>> +			 */
>> +			if (start_bus >= sec_bus_num &&
>> +			    start_bus <= sub_bus_num) {
>> +				*end_bus = sub_bus_num;
>> +				close(fd);
>> +				closedir(dir);
>> +				return 0;
>> +			}
>> +
>> +			close(fd);
>> +		}
>> +	}
>> +
>> +error:
>> +	closedir(dir);
>> +	return -1;
>> +}
>> +
>>   /**
>>    * DPDK callback to register a PCI device.
>>    *
>> @@ -39,7 +232,12 @@ static struct rte_pci_id qdma_pci_id_tbl[] = {
>>    */
>>   static int qdma_eth_dev_init(struct rte_eth_dev *dev)
>>   {
>> +	struct qdma_pci_dev *dma_priv;
>> +	uint8_t *baseaddr;
>> +	int i, idx, ret;
>>   	struct rte_pci_device *pci_dev;
>> +	uint16_t num_vfs;
>> +	uint8_t max_pci_bus = 0;
>>   
>>   	/* sanity checks */
>>   	if (dev == NULL)
>> @@ -59,6 +257,88 @@ static int qdma_eth_dev_init(struct rte_eth_dev *dev)
>>   	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>   		return 0;
>>   
>> +	/* allocate space for a single Ethernet MAC address */
>> +	dev->data->mac_addrs = rte_zmalloc("qdma", RTE_ETHER_ADDR_LEN * 1, 0);
>> +	if (dev->data->mac_addrs == NULL)
>> +		return -ENOMEM;
>> +
>> +	/* Copy some dummy Ethernet MAC address for QDMA device
>> +	 * This will change in real NIC device...
>> +	 * TODO: Read MAC from EEPROM
>> +	 */
>> +	for (i = 0; i < RTE_ETHER_ADDR_LEN; ++i)
>> +		dev->data->mac_addrs[0].addr_bytes[i] = 0x15 + i;
> If you don't have a real EEPROM to read, use rte_eth_random_addr() instead.

We do have an on-board EEPROM, but accessing it is not straight forward 
hardware wise as it is behind FPGA control.
The hardware team is working to simplify this. Meanwhile we were just 
using dummy values, but it is smart to use existing APIs.
I'll adapt to rte_eth_random_addr() in v2. Will also look for other 
places where I can reuse existing DPDK APIs. Thanks.

>
>> +
>> +	/* Init system & device */
>> +	dma_priv = (struct qdma_pci_dev *)dev->data->dev_private;
>> +	dma_priv->is_vf = 0;
>> +	dma_priv->is_master = 0;
>> +	dma_priv->vf_online_count = 0;
>> +	dma_priv->timer_count = DEFAULT_TIMER_CNT_TRIG_MODE_TIMER;
>> +
>> +	dma_priv->en_desc_prefetch = 0; /* Keep prefetch default to 0 */
>> +	dma_priv->cmpt_desc_len = 0;
>> +	dma_priv->c2h_bypass_mode = 0;
>> +	dma_priv->h2c_bypass_mode = 0;
>> +
>> +	dma_priv->config_bar_idx = DEFAULT_PF_CONFIG_BAR;
>> +	dma_priv->bypass_bar_idx = BAR_ID_INVALID;
>> +	dma_priv->user_bar_idx = BAR_ID_INVALID;
>> +
>> +	/* Check and handle device devargs */
>> +	if (qdma_check_kvargs(dev->device->devargs, dma_priv)) {
>> +		PMD_DRV_LOG(INFO, "devargs failed\n");
>> +		rte_free(dev->data->mac_addrs);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Store BAR address and length of Config BAR */
>> +	baseaddr = (uint8_t *)
>> +			pci_dev->mem_resource[dma_priv->config_bar_idx].addr;
>> +	dma_priv->bar_addr[dma_priv->config_bar_idx] = baseaddr;
>> +
>> +	idx = qdma_identify_bars(dev);
>> +	if (idx < 0) {
>> +		rte_free(dev->data->mac_addrs);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Store BAR address and length of AXI Master Lite BAR(user bar) */
>> +	if (dma_priv->user_bar_idx >= 0) {
>> +		baseaddr = (uint8_t *)
>> +			    pci_dev->mem_resource[dma_priv->user_bar_idx].addr;
>> +		dma_priv->bar_addr[dma_priv->user_bar_idx] = baseaddr;
>> +	}
>> +
>> +	PMD_DRV_LOG(INFO, "QDMA device driver probe:");
>> +
>> +	ret = get_max_pci_bus_num(pci_dev->addr.bus, &max_pci_bus);
>> +	if (ret != 0 && !max_pci_bus) {
>> +		PMD_DRV_LOG(ERR, "Failed to get max pci bus number\n");
>> +		rte_free(dev->data->mac_addrs);
>> +		return -EINVAL;
>> +	}
>> +	PMD_DRV_LOG(INFO, "PCI max bus number : 0x%x", max_pci_bus);
>> +
>> +	if (!dma_priv->reset_in_progress) {
>> +		num_vfs = pci_dev->max_vfs;
>> +		if (num_vfs) {
>> +			dma_priv->vfinfo = rte_zmalloc("vfinfo",
>> +				sizeof(struct qdma_vf_info) * num_vfs, 0);
>> +			if (dma_priv->vfinfo == NULL) {
>> +				PMD_DRV_LOG(ERR, "Cannot allocate memory for private VF info\n");
>> +				return -ENOMEM;
>> +			}
>> +
>> +			/* Mark all VFs with invalid function id mapping*/
>> +			for (i = 0; i < num_vfs; i++)
>> +				dma_priv->vfinfo[i].func_id =
>> +					QDMA_FUNC_ID_INVALID;
>> +		}
>> +	}
>> +
>> +	dma_priv->reset_in_progress = 0;
>> +
>>   	return 0;
>>   }