Re: [dpdk-dev] [PATCH v8 01/14] eal_pci: Add flag to hold kernel driver type

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2015-02-16 13:14, Tetsuya Mukawa:
> From: Michael Qiu <michael.qiu@intel.com>
> 
> Currently, dpdk has no ability to know which type of driver(
> vfio-pci/igb_uio/uio_pci_generic) the device used. It only can
> check whether vfio is enabled or not staticly.
> 
> It really useful to have the flag, becasue different type need to
> handle differently in runtime. For example, pci memory map,
> pot hotplug, and so on.
> 
> This patch add a flag field for pci device to solve above issue.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_eal/common/include/rte_pci.h |  8 +++++
>  lib/librte_eal/linuxapp/eal/eal_pci.c   | 53 +++++++++++++++++++++++++++++++--
>  2 files changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index 66ed793..7b48b55 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -139,6 +139,13 @@ struct rte_pci_addr {
>  
>  struct rte_devargs;
>  
> +enum rte_pt_driver {
> +	RTE_PT_UNKNOWN		= 0,
> +	RTE_PT_IGB_UIO		= 1,
> +	RTE_PT_VFIO		= 2,
> +	RTE_PT_UIO_GENERIC	= 3,
> +};

What means PT?

> +
>  /**
>   * A structure describing a PCI device.
>   */
> @@ -152,6 +159,7 @@ struct rte_pci_device {
>  	uint16_t max_vfs;                       /**< sriov enable if not zero */
>  	int numa_node;                          /**< NUMA node connection */
>  	struct rte_devargs *devargs;            /**< Device user arguments */
> +	enum rte_pt_driver pt_driver;		/**< Driver of passthrough */

I'm not sure passtrough is the right word for this kind of driver.
What about "kernel driver"?

>  };
>  
>  /** Any PCI device identifier (vendor, device, ...) */
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 15db9c4..e760452 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -97,6 +97,35 @@ error:
>  	return -1;
>  }
>  
> +static int
> +pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
> +{
> +	int count;
> +	char path[PATH_MAX];
> +	char *name;
> +
> +	if (!filename || !dri_name)
> +		return -1;
> +
> +	count = readlink(filename, path, PATH_MAX);
> +	if (count >= PATH_MAX)
> +		return -1;
> +
> +	/* For device does not have a driver */
> +	if (count < 0)
> +		return 1;
> +
> +	path[count] = '\0';
> +
> +	name = strrchr(path, '/');
> +	if (name) {
> +		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
> +		return 0;
> +	}
> +
> +	return -1;
> +}
> +
>  void *
>  pci_find_max_end_va(void)
>  {
> @@ -222,11 +251,12 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
>  	char filename[PATH_MAX];
>  	unsigned long tmp;
>  	struct rte_pci_device *dev;
> +	char driver[PATH_MAX];
> +	int ret;
>  
>  	dev = malloc(sizeof(*dev));
> -	if (dev == NULL) {
> +	if (dev == NULL)
>  		return -1;
> -	}
>  
>  	memset(dev, 0, sizeof(*dev));
>  	dev->addr.domain = domain;
> @@ -305,6 +335,25 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
>  		return -1;
>  	}
>  
> +	/* parse driver */
> +	snprintf(filename, sizeof(filename), "%s/driver", dirname);
> +	ret = pci_get_kernel_driver_by_path(filename, driver);
> +	if (!ret) {
> +		if (!strcmp(driver, "vfio-pci"))
> +			dev->pt_driver = RTE_PT_VFIO;
> +		else if (!strcmp(driver, "igb_uio"))
> +			dev->pt_driver = RTE_PT_IGB_UIO;
> +		else if (!strcmp(driver, "uio_pci_generic"))
> +			dev->pt_driver = RTE_PT_UIO_GENERIC;
> +		else
> +			dev->pt_driver = RTE_PT_UNKNOWN;
> +	} else if (ret < 0) {
> +		RTE_LOG(ERR, EAL, "Fail to get kernel driver\n");
> +		free(dev);
> +		return -1;
> +	} else
> +		dev->pt_driver = RTE_PT_UNKNOWN;

Why not considering a NONE value? Example: for virtio with port I/O.

> +
>  	/* device is valid, add in list (sorted) */
>  	if (TAILQ_EMPTY(&pci_device_list)) {
>  		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
>