[dpdk-dev,v9,02/14] eal_pci: Add flag to hold kernel driver type

Message ID 1424314187-25177-3-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tetsuya Mukawa Feb. 19, 2015, 2:49 a.m. UTC
  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(-)
  

Comments

Thomas Monjalon Feb. 19, 2015, 11:17 a.m. UTC | #1
> @@ -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 */
[...]
> +static int
> +pci_get_kernel_driver_by_path(const char *filename, char *dri_name)

I think "kernel driver" is a good name. Why not using this name in the
pci_device struct to be more consistent?

Thanks
  
Tetsuya Mukawa Feb. 19, 2015, 1:29 p.m. UTC | #2
On 2015/02/19 20:17, Thomas Monjalon wrote:
>> @@ -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 */
> [...]
>> +static int
>> +pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
> I think "kernel driver" is a good name. Why not using this name in the
> pci_device struct to be more consistent?

Hi Michael,

Could you please let me know what do you think about it?

Thanks
Tetsuya

> Thanks
  
Tetsuya Mukawa Feb. 20, 2015, 5:18 a.m. UTC | #3
On 2015/02/19 22:29, Tetsuya Mukawa wrote:
> On 2015/02/19 20:17, Thomas Monjalon wrote:
>>> @@ -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 */
>> [...]
>>> +static int
>>> +pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
>> I think "kernel driver" is a good name. Why not using this name in the
>> pci_device struct to be more consistent?
> Hi Michael,
>
> Could you please let me know what do you think about it?

Hi Thomas,

Could you please check below reply from Michael?
He has already replied it.
http://dpdk.org/dev/patchwork/patch/3363/

According to Tim's email, he might be out of office until middle of next
week.
I cannot rewrite his patch without his agreement. So I will submit
patches without this.
If we decide to change it when he comes back, I will send v11 patches.

Thanks,
Tetsuya

> Thanks
> Tetsuya
>
>> Thanks
  

Patch

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,
+};
+
 /**
  * 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 */
 };
 
 /** 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;
+
 	/* device is valid, add in list (sorted) */
 	if (TAILQ_EMPTY(&pci_device_list)) {
 		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);